From 93c677a6a955c75318c184d167737836c8c36cd5 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Thu, 3 Oct 2024 15:59:50 +0100 Subject: [PATCH] Searching: Added negation support to UI and term handling Updated/added tests to cover. Support for actual search queries still remains. --- app/Search/Options/ExactSearchOption.php | 13 +++ app/Search/Options/FilterSearchOption.php | 37 +++++++ app/Search/Options/SearchOption.php | 26 +++++ app/Search/Options/TagSearchOption.php | 11 ++ app/Search/Options/TermSearchOption.php | 11 ++ app/Search/SearchOption.php | 12 --- app/Search/SearchOptionSet.php | 35 +++++-- app/Search/SearchOptions.php | 119 ++++++++++++---------- resources/views/search/all.blade.php | 14 ++- tests/Entity/EntitySearchTest.php | 7 +- tests/Entity/SearchOptionsTest.php | 63 ++++++++++-- 11 files changed, 252 insertions(+), 96 deletions(-) create mode 100644 app/Search/Options/ExactSearchOption.php create mode 100644 app/Search/Options/FilterSearchOption.php create mode 100644 app/Search/Options/SearchOption.php create mode 100644 app/Search/Options/TagSearchOption.php create mode 100644 app/Search/Options/TermSearchOption.php delete mode 100644 app/Search/SearchOption.php diff --git a/app/Search/Options/ExactSearchOption.php b/app/Search/Options/ExactSearchOption.php new file mode 100644 index 000000000..5651fb99b --- /dev/null +++ b/app/Search/Options/ExactSearchOption.php @@ -0,0 +1,13 @@ +value); + $escaped = str_replace('"', '\"', $escaped); + return ($this->negated ? '-' : '') . '"' . $escaped . '"'; + } +} diff --git a/app/Search/Options/FilterSearchOption.php b/app/Search/Options/FilterSearchOption.php new file mode 100644 index 000000000..1f64f4f9e --- /dev/null +++ b/app/Search/Options/FilterSearchOption.php @@ -0,0 +1,37 @@ +name = $name; + } + + public function toString(): string + { + $valueText = ($this->value ? ':' . $this->value : ''); + $filterBrace = '{' . $this->name . $valueText . '}'; + return ($this->negated ? '-' : '') . $filterBrace; + } + + public function getKey(): string + { + return $this->name; + } + + public static function fromContentString(string $value, bool $negated = false): self + { + $explodedFilter = explode(':', $value, 2); + $filterValue = (count($explodedFilter) > 1) ? $explodedFilter[1] : ''; + $filterName = $explodedFilter[0]; + return new self($filterValue, $filterName, $negated); + } +} diff --git a/app/Search/Options/SearchOption.php b/app/Search/Options/SearchOption.php new file mode 100644 index 000000000..483f2123f --- /dev/null +++ b/app/Search/Options/SearchOption.php @@ -0,0 +1,26 @@ +negated ? '-' : '') . "[{$this->value}]"; + } +} diff --git a/app/Search/Options/TermSearchOption.php b/app/Search/Options/TermSearchOption.php new file mode 100644 index 000000000..c78829fc8 --- /dev/null +++ b/app/Search/Options/TermSearchOption.php @@ -0,0 +1,11 @@ +value; + } +} diff --git a/app/Search/SearchOption.php b/app/Search/SearchOption.php deleted file mode 100644 index 74fc7be38..000000000 --- a/app/Search/SearchOption.php +++ /dev/null @@ -1,12 +0,0 @@ -options as $key => $option) { + foreach ($this->options as $index => $option) { + $key = $option->getKey() ?? $index; $map[$key] = $option->value; } return $map; @@ -35,22 +38,32 @@ class SearchOptionSet public function filterEmpty(): self { - $filteredOptions = array_filter($this->options, fn (SearchOption $option) => !empty($option->value)); + $filteredOptions = array_values(array_filter($this->options, fn (SearchOption $option) => !empty($option->value))); return new self($filteredOptions); } - public static function fromValueArray(array $values): self + /** + * @param class-string $class + */ + public static function fromValueArray(array $values, string $class): self { - $options = array_map(fn($val) => new SearchOption($val), $values); + $options = array_map(fn($val) => new $class($val), $values); return new self($options); } - public static function fromMapArray(array $values): self + /** + * @return SearchOption[] + */ + public function all(): array { - $options = []; - foreach ($values as $key => $value) { - $options[$key] = new SearchOption($value); - } - return new self($options); + return $this->options; + } + + /** + * @return SearchOption[] + */ + public function negated(): array + { + return array_values(array_filter($this->options, fn (SearchOption $option) => $option->negated)); } } diff --git a/app/Search/SearchOptions.php b/app/Search/SearchOptions.php index 09981c75d..98f731ee7 100644 --- a/app/Search/SearchOptions.php +++ b/app/Search/SearchOptions.php @@ -2,6 +2,11 @@ namespace BookStack\Search; +use BookStack\Search\Options\ExactSearchOption; +use BookStack\Search\Options\FilterSearchOption; +use BookStack\Search\Options\SearchOption; +use BookStack\Search\Options\TagSearchOption; +use BookStack\Search\Options\TermSearchOption; use Illuminate\Http\Request; class SearchOptions @@ -45,29 +50,38 @@ class SearchOptions } $instance = new SearchOptions(); - $inputs = $request->only(['search', 'types', 'filters', 'exact', 'tags']); + $inputs = $request->only(['search', 'types', 'filters', 'exact', 'tags', 'extras']); $parsedStandardTerms = static::parseStandardTermString($inputs['search'] ?? ''); $inputExacts = array_filter($inputs['exact'] ?? []); - $instance->searches = SearchOptionSet::fromValueArray(array_filter($parsedStandardTerms['terms'])); - $instance->exacts = SearchOptionSet::fromValueArray(array_filter($parsedStandardTerms['exacts'])); - $instance->exacts = $instance->exacts->merge(SearchOptionSet::fromValueArray($inputExacts)); - $instance->tags = SearchOptionSet::fromValueArray(array_filter($inputs['tags'] ?? [])); + $instance->searches = SearchOptionSet::fromValueArray(array_filter($parsedStandardTerms['terms']), TermSearchOption::class); + $instance->exacts = SearchOptionSet::fromValueArray(array_filter($parsedStandardTerms['exacts']), ExactSearchOption::class); + $instance->exacts = $instance->exacts->merge(SearchOptionSet::fromValueArray($inputExacts, ExactSearchOption::class)); + $instance->tags = SearchOptionSet::fromValueArray(array_filter($inputs['tags'] ?? []), TagSearchOption::class); - $keyedFilters = []; + $cleanedFilters = []; foreach (($inputs['filters'] ?? []) as $filterKey => $filterVal) { if (empty($filterVal)) { continue; } $cleanedFilterVal = $filterVal === 'true' ? '' : $filterVal; - $keyedFilters[$filterKey] = new SearchOption($cleanedFilterVal); + $cleanedFilters[] = new FilterSearchOption($cleanedFilterVal, $filterKey); } if (isset($inputs['types']) && count($inputs['types']) < 4) { - $keyedFilters['type'] = new SearchOption(implode('|', $inputs['types'])); + $cleanedFilters[] = new FilterSearchOption(implode('|', $inputs['types']), 'types'); } - $instance->filters = new SearchOptionSet($keyedFilters); + $instance->filters = new SearchOptionSet($cleanedFilters); + + // Parse and merge in extras if provided + if (!empty($inputs['extras'])) { + $extras = static::fromString($inputs['extras']); + $instance->searches = $instance->searches->merge($extras->searches); + $instance->exacts = $instance->exacts->merge($extras->exacts); + $instance->tags = $instance->tags->merge($extras->tags); + $instance->filters = $instance->filters->merge($extras->filters); + } return $instance; } @@ -77,7 +91,7 @@ class SearchOptions */ protected function addOptionsFromString(string $searchString): void { - /** @var array $terms */ + /** @var array $terms */ $terms = [ 'exacts' => [], 'tags' => [], @@ -85,9 +99,15 @@ class SearchOptions ]; $patterns = [ - 'exacts' => '/"((?:\\\\.|[^"\\\\])*)"/', - 'tags' => '/\[(.*?)\]/', - 'filters' => '/\{(.*?)\}/', + 'exacts' => '/-?"((?:\\\\.|[^"\\\\])*)"/', + 'tags' => '/-?\[(.*?)\]/', + 'filters' => '/-?\{(.*?)\}/', + ]; + + $constructors = [ + 'exacts' => fn(string $value, bool $negated) => new ExactSearchOption($value, $negated), + 'tags' => fn(string $value, bool $negated) => new TagSearchOption($value, $negated), + 'filters' => fn(string $value, bool $negated) => FilterSearchOption::fromContentString($value, $negated), ]; // Parse special terms @@ -95,36 +115,32 @@ class SearchOptions $matches = []; preg_match_all($pattern, $searchString, $matches); if (count($matches) > 0) { - $terms[$termType] = $matches[1]; + foreach ($matches[1] as $index => $value) { + $negated = str_starts_with($matches[0][$index], '-'); + $terms[$termType][] = $constructors[$termType]($value, $negated); + } $searchString = preg_replace($pattern, '', $searchString); } } // Unescape exacts and backslash escapes - $escapedExacts = array_map(fn(string $term) => static::decodeEscapes($term), $terms['exacts']); + foreach ($terms['exacts'] as $exact) { + $exact->value = static::decodeEscapes($exact->value); + } // Parse standard terms $parsedStandardTerms = static::parseStandardTermString($searchString); $this->searches = $this->searches - ->merge(SearchOptionSet::fromValueArray($parsedStandardTerms['terms'])) + ->merge(SearchOptionSet::fromValueArray($parsedStandardTerms['terms'], TermSearchOption::class)) ->filterEmpty(); $this->exacts = $this->exacts - ->merge(SearchOptionSet::fromValueArray($escapedExacts)) - ->merge(SearchOptionSet::fromValueArray($parsedStandardTerms['exacts'])) + ->merge(new SearchOptionSet($terms['exacts'])) + ->merge(SearchOptionSet::fromValueArray($parsedStandardTerms['exacts'], ExactSearchOption::class)) ->filterEmpty(); - // Add tags - $this->tags = $this->tags->merge(SearchOptionSet::fromValueArray($terms['tags'])); - - // Split filter values out - /** @var array $splitFilters */ - $splitFilters = []; - foreach ($terms['filters'] as $filter) { - $explodedFilter = explode(':', $filter, 2); - $filterValue = (count($explodedFilter) > 1) ? $explodedFilter[1] : ''; - $splitFilters[$explodedFilter[0]] = new SearchOption($filterValue); - } - $this->filters = $this->filters->merge(new SearchOptionSet($splitFilters)); + // Add tags & filters + $this->tags = $this->tags->merge(new SearchOptionSet($terms['tags'])); + $this->filters = $this->filters->merge(new SearchOptionSet($terms['filters'])); } /** @@ -185,7 +201,7 @@ class SearchOptions public function setFilter(string $filterName, string $filterValue = ''): void { $this->filters = $this->filters->merge( - new SearchOptionSet([$filterName => new SearchOption($filterValue)]) + new SearchOptionSet([new FilterSearchOption($filterValue, $filterName)]) ); } @@ -194,21 +210,14 @@ class SearchOptions */ public function toString(): string { - $parts = $this->searches->toValueArray(); + $options = [ + ...$this->searches->all(), + ...$this->exacts->all(), + ...$this->tags->all(), + ...$this->filters->all(), + ]; - foreach ($this->exacts->toValueArray() as $term) { - $escaped = str_replace('\\', '\\\\', $term); - $escaped = str_replace('"', '\"', $escaped); - $parts[] = '"' . $escaped . '"'; - } - - foreach ($this->tags->toValueArray() as $term) { - $parts[] = "[{$term}]"; - } - - foreach ($this->filters->toValueMap() as $filterName => $filterVal) { - $parts[] = '{' . $filterName . ($filterVal ? ':' . $filterVal : '') . '}'; - } + $parts = array_map(fn(SearchOption $o) => $o->toString(), $options); return implode(' ', $parts); } @@ -217,24 +226,24 @@ class SearchOptions * Get the search options that don't have UI controls provided for. * Provided back as a key => value array with the keys being expected * input names for a search form, and values being the option value. - * - * @return array */ - public function getHiddenInputValuesByFieldName(): array + public function getAdditionalOptionsString(): string { $options = []; // Non-[created/updated]-by-me options - $filterMap = $this->filters->toValueMap(); - foreach (['updated_by', 'created_by', 'owned_by'] as $filter) { - $value = $filterMap[$filter] ?? null; - if ($value !== null && $value !== 'me') { - $options["filters[$filter]"] = $value; + $userFilters = ['updated_by', 'created_by', 'owned_by']; + foreach ($this->filters->all() as $filter) { + if (in_array($filter->getKey(), $userFilters, true) && $filter->value !== null && $filter->value !== 'me') { + $options[] = $filter; } } - // TODO - Negated + // Negated items + array_push($options, ...$this->exacts->negated()); + array_push($options, ...$this->tags->negated()); + array_push($options, ...$this->filters->negated()); - return $options; + return implode(' ', array_map(fn(SearchOption $o) => $o->toString(), $options)); } } diff --git a/resources/views/search/all.blade.php b/resources/views/search/all.blade.php index aa7ae0aff..2a0d63a6e 100644 --- a/resources/views/search/all.blade.php +++ b/resources/views/search/all.blade.php @@ -25,8 +25,8 @@ @include('search.parts.type-filter', ['checked' => !$hasTypes || in_array('page', $types), 'entity' => 'page', 'transKey' => 'page']) @include('search.parts.type-filter', ['checked' => !$hasTypes || in_array('chapter', $types), 'entity' => 'chapter', 'transKey' => 'chapter'])
- @include('search.parts.type-filter', ['checked' => !$hasTypes || in_array('book', $types), 'entity' => 'book', 'transKey' => 'book']) - @include('search.parts.type-filter', ['checked' => !$hasTypes || in_array('bookshelf', $types), 'entity' => 'bookshelf', 'transKey' => 'shelf']) + @include('search.parts.type-filter', ['checked' => !$hasTypes || in_array('book', $types), 'entity' => 'book', 'transKey' => 'book']) + @include('search.parts.type-filter', ['checked' => !$hasTypes || in_array('bookshelf', $types), 'entity' => 'bookshelf', 'transKey' => 'shelf'])
{{ trans('entities.search_exact_matches') }}
@@ -64,10 +64,7 @@ @include('search.parts.date-filter', ['name' => 'created_after', 'filters' => $filterMap]) @include('search.parts.date-filter', ['name' => 'created_before', 'filters' => $filterMap]) - @foreach($options->getHiddenInputValuesByFieldName() as $fieldName => $value) - - @endforeach - + @@ -77,8 +74,9 @@

{{ trans('entities.search_results') }}

-
- + + diff --git a/tests/Entity/EntitySearchTest.php b/tests/Entity/EntitySearchTest.php index 4b032bfc3..c3e959345 100644 --- a/tests/Entity/EntitySearchTest.php +++ b/tests/Entity/EntitySearchTest.php @@ -545,11 +545,10 @@ class EntitySearchTest extends TestCase $search->assertSee($page->getUrl(), false); } - public function test_searches_with_user_filters_adds_them_into_advanced_search_form() + public function test_searches_with_terms_without_controls_includes_them_in_extras() { - $resp = $this->asEditor()->get('/search?term=' . urlencode('test {updated_by:dan} {created_by:dan}')); - $this->withHtml($resp)->assertElementExists('form input[name="filters[updated_by]"][value="dan"]'); - $this->withHtml($resp)->assertElementExists('form input[name="filters[created_by]"][value="dan"]'); + $resp = $this->asEditor()->get('/search?term=' . urlencode('test {updated_by:dan} {created_by:dan} -{viewed_by_me} -[a=b] -"dog"')); + $this->withHtml($resp)->assertFieldHasValue('extras', '{updated_by:dan} {created_by:dan} -"dog" -[a=b] -{viewed_by_me}'); } public function test_searches_with_user_filters_using_me_adds_them_into_advanced_search_form() diff --git a/tests/Entity/SearchOptionsTest.php b/tests/Entity/SearchOptionsTest.php index 7ab150e91..543badcef 100644 --- a/tests/Entity/SearchOptionsTest.php +++ b/tests/Entity/SearchOptionsTest.php @@ -2,6 +2,10 @@ namespace Tests\Entity; +use BookStack\Search\Options\ExactSearchOption; +use BookStack\Search\Options\FilterSearchOption; +use BookStack\Search\Options\TagSearchOption; +use BookStack\Search\Options\TermSearchOption; use BookStack\Search\SearchOptions; use BookStack\Search\SearchOptionSet; use Illuminate\Http\Request; @@ -19,6 +23,16 @@ class SearchOptionsTest extends TestCase $this->assertEquals(['is_tree' => ''], $options->filters->toValueMap()); } + public function test_from_string_parses_negations() + { + $options = SearchOptions::fromString('cat -"dog" -[tag=good] -{is_tree}'); + + $this->assertEquals(['cat'], $options->searches->toValueArray()); + $this->assertTrue($options->exacts->all()[0]->negated); + $this->assertTrue($options->tags->all()[0]->negated); + $this->assertTrue($options->filters->all()[0]->negated); + } + public function test_from_string_properly_parses_escaped_quotes() { $options = SearchOptions::fromString('"\"cat\"" surprise "\"\"" "\"donkey" "\"" "\\\\"'); @@ -28,12 +42,32 @@ class SearchOptionsTest extends TestCase public function test_to_string_includes_all_items_in_the_correct_format() { - $expected = 'cat "dog" [tag=good] {is_tree}'; + $expected = 'cat "dog" [tag=good] {is_tree} {beans:valid}'; $options = new SearchOptions(); - $options->searches = SearchOptionSet::fromValueArray(['cat']); - $options->exacts = SearchOptionSet::fromValueArray(['dog']); - $options->tags = SearchOptionSet::fromValueArray(['tag=good']); - $options->filters = SearchOptionSet::fromMapArray(['is_tree' => '']); + $options->searches = SearchOptionSet::fromValueArray(['cat'], TermSearchOption::class); + $options->exacts = SearchOptionSet::fromValueArray(['dog'], ExactSearchOption::class); + $options->tags = SearchOptionSet::fromValueArray(['tag=good'], TagSearchOption::class); + $options->filters = new SearchOptionSet([ + new FilterSearchOption('', 'is_tree'), + new FilterSearchOption('valid', 'beans'), + ]); + + $output = $options->toString(); + foreach (explode(' ', $expected) as $term) { + $this->assertStringContainsString($term, $output); + } + } + + public function test_to_string_handles_negations_as_expected() + { + $expected = 'cat -"dog" -[tag=good] -{is_tree}'; + $options = new SearchOptions(); + $options->searches = new SearchOptionSet([new TermSearchOption('cat')]); + $options->exacts = new SearchOptionSet([new ExactSearchOption('dog', true)]); + $options->tags = new SearchOptionSet([new TagSearchOption('tag=good', true)]); + $options->filters = new SearchOptionSet([ + new FilterSearchOption('', 'is_tree', true), + ]); $output = $options->toString(); foreach (explode(' ', $expected) as $term) { @@ -44,7 +78,7 @@ class SearchOptionsTest extends TestCase public function test_to_string_escapes_as_expected() { $options = new SearchOptions(); - $options->exacts = SearchOptionSet::fromValueArray(['"cat"', '""', '"donkey', '"', '\\', '\\"']); + $options->exacts = SearchOptionSet::fromValueArray(['"cat"', '""', '"donkey', '"', '\\', '\\"'], ExactSearchOption::class); $output = $options->toString(); $this->assertEquals('"\"cat\"" "\"\"" "\"donkey" "\"" "\\\\" "\\\\\""', $output); @@ -78,4 +112,21 @@ class SearchOptionsTest extends TestCase $this->assertEquals(["biscuits"], $options->searches->toValueArray()); $this->assertEquals(['"cheese"', '""', '"baked', 'beans"'], $options->exacts->toValueArray()); } + + public function test_from_request_properly_parses_out_extras_as_string() + { + $request = new Request([ + 'search' => '', + 'tags' => ['a=b'], + 'extras' => '-[b=c] -{viewed_by_me} -"dino"' + ]); + + $options = SearchOptions::fromRequest($request); + $this->assertCount(2, $options->tags->all()); + $this->assertEquals('b=c', $options->tags->negated()[0]->value); + $this->assertEquals('viewed_by_me', $options->filters->all()[0]->getKey()); + $this->assertTrue($options->filters->all()[0]->negated); + $this->assertEquals('dino', $options->exacts->all()[0]->value); + $this->assertTrue($options->exacts->all()[0]->negated); + } }