From 177cfd72bf22c78823cc46dc5c44df542f5f1fd2 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 2 Oct 2024 17:31:45 +0100 Subject: [PATCH 1/4] Search: Added structure for search term inputs Sets things up to allow more complex terms ready to handle negation. --- app/Search/SearchOption.php | 12 +++ app/Search/SearchOptionSet.php | 56 +++++++++++++ app/Search/SearchOptions.php | 114 +++++++++++++++++--------- app/Search/SearchResultsFormatter.php | 5 +- app/Search/SearchRunner.php | 22 ++--- resources/views/search/all.blade.php | 40 ++++----- tests/Entity/SearchOptionsTest.php | 31 +++---- 7 files changed, 194 insertions(+), 86 deletions(-) create mode 100644 app/Search/SearchOption.php create mode 100644 app/Search/SearchOptionSet.php diff --git a/app/Search/SearchOption.php b/app/Search/SearchOption.php new file mode 100644 index 000000000..74fc7be38 --- /dev/null +++ b/app/Search/SearchOption.php @@ -0,0 +1,12 @@ +options = $options; + } + + public function toValueArray(): array + { + return array_map(fn(SearchOption $option) => $option->value, $this->options); + } + + public function toValueMap(): array + { + $map = []; + foreach ($this->options as $key => $option) { + $map[$key] = $option->value; + } + return $map; + } + + public function merge(SearchOptionSet $set): self + { + return new self(array_merge($this->options, $set->options)); + } + + public function filterEmpty(): self + { + $filteredOptions = array_filter($this->options, fn (SearchOption $option) => !empty($option->value)); + return new self($filteredOptions); + } + + public static function fromValueArray(array $values): self + { + $options = array_map(fn($val) => new SearchOption($val), $values); + return new self($options); + } + + public static function fromMapArray(array $values): self + { + $options = []; + foreach ($values as $key => $value) { + $options[$key] = new SearchOption($value); + } + return new self($options); + } +} diff --git a/app/Search/SearchOptions.php b/app/Search/SearchOptions.php index fffa03db0..09981c75d 100644 --- a/app/Search/SearchOptions.php +++ b/app/Search/SearchOptions.php @@ -6,22 +6,26 @@ use Illuminate\Http\Request; class SearchOptions { - public array $searches = []; - public array $exacts = []; - public array $tags = []; - public array $filters = []; + public SearchOptionSet $searches; + public SearchOptionSet $exacts; + public SearchOptionSet $tags; + public SearchOptionSet $filters; + + public function __construct() + { + $this->searches = new SearchOptionSet(); + $this->exacts = new SearchOptionSet(); + $this->tags = new SearchOptionSet(); + $this->filters = new SearchOptionSet(); + } /** * Create a new instance from a search string. */ public static function fromString(string $search): self { - $decoded = static::decode($search); - $instance = new SearchOptions(); - foreach ($decoded as $type => $value) { - $instance->$type = $value; - } - + $instance = new self(); + $instance->addOptionsFromString($search); return $instance; } @@ -44,34 +48,37 @@ class SearchOptions $inputs = $request->only(['search', 'types', 'filters', 'exact', 'tags']); $parsedStandardTerms = static::parseStandardTermString($inputs['search'] ?? ''); - $instance->searches = array_filter($parsedStandardTerms['terms']); - $instance->exacts = array_filter($parsedStandardTerms['exacts']); - - array_push($instance->exacts, ...array_filter($inputs['exact'] ?? [])); - - $instance->tags = array_filter($inputs['tags'] ?? []); + $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'] ?? [])); + $keyedFilters = []; foreach (($inputs['filters'] ?? []) as $filterKey => $filterVal) { if (empty($filterVal)) { continue; } - $instance->filters[$filterKey] = $filterVal === 'true' ? '' : $filterVal; + $cleanedFilterVal = $filterVal === 'true' ? '' : $filterVal; + $keyedFilters[$filterKey] = new SearchOption($cleanedFilterVal); } if (isset($inputs['types']) && count($inputs['types']) < 4) { - $instance->filters['type'] = implode('|', $inputs['types']); + $keyedFilters['type'] = new SearchOption(implode('|', $inputs['types'])); } + $instance->filters = new SearchOptionSet($keyedFilters); + return $instance; } /** - * Decode a search string into an array of terms. + * Decode a search string and add its contents to this instance. */ - protected static function decode(string $searchString): array + protected function addOptionsFromString(string $searchString): void { + /** @var array $terms */ $terms = [ - 'searches' => [], 'exacts' => [], 'tags' => [], 'filters' => [], @@ -94,28 +101,30 @@ class SearchOptions } // Unescape exacts and backslash escapes - foreach ($terms['exacts'] as $index => $exact) { - $terms['exacts'][$index] = static::decodeEscapes($exact); - } + $escapedExacts = array_map(fn(string $term) => static::decodeEscapes($term), $terms['exacts']); // Parse standard terms $parsedStandardTerms = static::parseStandardTermString($searchString); - array_push($terms['searches'], ...$parsedStandardTerms['terms']); - array_push($terms['exacts'], ...$parsedStandardTerms['exacts']); + $this->searches = $this->searches + ->merge(SearchOptionSet::fromValueArray($parsedStandardTerms['terms'])) + ->filterEmpty(); + $this->exacts = $this->exacts + ->merge(SearchOptionSet::fromValueArray($escapedExacts)) + ->merge(SearchOptionSet::fromValueArray($parsedStandardTerms['exacts'])) + ->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); - $splitFilters[$explodedFilter[0]] = (count($explodedFilter) > 1) ? $explodedFilter[1] : ''; + $filterValue = (count($explodedFilter) > 1) ? $explodedFilter[1] : ''; + $splitFilters[$explodedFilter[0]] = new SearchOption($filterValue); } - $terms['filters'] = $splitFilters; - - // Filter down terms where required - $terms['exacts'] = array_filter($terms['exacts']); - $terms['searches'] = array_filter($terms['searches']); - - return $terms; + $this->filters = $this->filters->merge(new SearchOptionSet($splitFilters)); } /** @@ -175,7 +184,9 @@ class SearchOptions */ public function setFilter(string $filterName, string $filterValue = ''): void { - $this->filters[$filterName] = $filterValue; + $this->filters = $this->filters->merge( + new SearchOptionSet([$filterName => new SearchOption($filterValue)]) + ); } /** @@ -183,22 +194,47 @@ class SearchOptions */ public function toString(): string { - $parts = $this->searches; + $parts = $this->searches->toValueArray(); - foreach ($this->exacts as $term) { + foreach ($this->exacts->toValueArray() as $term) { $escaped = str_replace('\\', '\\\\', $term); $escaped = str_replace('"', '\"', $escaped); $parts[] = '"' . $escaped . '"'; } - foreach ($this->tags as $term) { + foreach ($this->tags->toValueArray() as $term) { $parts[] = "[{$term}]"; } - foreach ($this->filters as $filterName => $filterVal) { + foreach ($this->filters->toValueMap() as $filterName => $filterVal) { $parts[] = '{' . $filterName . ($filterVal ? ':' . $filterVal : '') . '}'; } return implode(' ', $parts); } + + /** + * 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 + { + $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; + } + } + + // TODO - Negated + + return $options; + } } diff --git a/app/Search/SearchResultsFormatter.php b/app/Search/SearchResultsFormatter.php index 02a40632e..b06f81e0e 100644 --- a/app/Search/SearchResultsFormatter.php +++ b/app/Search/SearchResultsFormatter.php @@ -25,11 +25,12 @@ class SearchResultsFormatter * Update the given entity model to set attributes used for previews of the item * primarily within search result lists. */ - protected function setSearchPreview(Entity $entity, SearchOptions $options) + protected function setSearchPreview(Entity $entity, SearchOptions $options): void { $textProperty = $entity->textField; $textContent = $entity->$textProperty; - $terms = array_merge($options->exacts, $options->searches); + $relevantSearchOptions = $options->exacts->merge($options->searches); + $terms = $relevantSearchOptions->toValueArray(); $originalContentByNewAttribute = [ 'preview_name' => $entity->name, diff --git a/app/Search/SearchRunner.php b/app/Search/SearchRunner.php index 94518dbf7..855140508 100644 --- a/app/Search/SearchRunner.php +++ b/app/Search/SearchRunner.php @@ -55,10 +55,11 @@ class SearchRunner $entityTypes = array_keys($this->entityProvider->all()); $entityTypesToSearch = $entityTypes; + $filterMap = $searchOpts->filters->toValueMap(); if ($entityType !== 'all') { $entityTypesToSearch = [$entityType]; - } elseif (isset($searchOpts->filters['type'])) { - $entityTypesToSearch = explode('|', $searchOpts->filters['type']); + } elseif (isset($filterMap['type'])) { + $entityTypesToSearch = explode('|', $filterMap['type']); } $results = collect(); @@ -97,7 +98,8 @@ class SearchRunner { $opts = SearchOptions::fromString($searchString); $entityTypes = ['page', 'chapter']; - $entityTypesToSearch = isset($opts->filters['type']) ? explode('|', $opts->filters['type']) : $entityTypes; + $filterMap = $opts->filters->toValueMap(); + $entityTypesToSearch = isset($filterMap['type']) ? explode('|', $filterMap['type']) : $entityTypes; $results = collect(); foreach ($entityTypesToSearch as $entityType) { @@ -161,7 +163,7 @@ class SearchRunner $this->applyTermSearch($entityQuery, $searchOpts, $entityType); // Handle exact term matching - foreach ($searchOpts->exacts as $inputTerm) { + foreach ($searchOpts->exacts->toValueArray() as $inputTerm) { $entityQuery->where(function (EloquentBuilder $query) use ($inputTerm, $entityModelInstance) { $inputTerm = str_replace('\\', '\\\\', $inputTerm); $query->where('name', 'like', '%' . $inputTerm . '%') @@ -170,12 +172,12 @@ class SearchRunner } // Handle tag searches - foreach ($searchOpts->tags as $inputTerm) { + foreach ($searchOpts->tags->toValueArray() as $inputTerm) { $this->applyTagSearch($entityQuery, $inputTerm); } // Handle filters - foreach ($searchOpts->filters as $filterTerm => $filterValue) { + foreach ($searchOpts->filters->toValueMap() as $filterTerm => $filterValue) { $functionName = Str::camel('filter_' . $filterTerm); if (method_exists($this, $functionName)) { $this->$functionName($entityQuery, $entityModelInstance, $filterValue); @@ -190,7 +192,7 @@ class SearchRunner */ protected function applyTermSearch(EloquentBuilder $entityQuery, SearchOptions $options, string $entityType): void { - $terms = $options->searches; + $terms = $options->searches->toValueArray(); if (count($terms) === 0) { return; } @@ -209,8 +211,8 @@ class SearchRunner $subQuery->where('entity_type', '=', $entityType); $subQuery->where(function (Builder $query) use ($terms) { foreach ($terms as $inputTerm) { - $inputTerm = str_replace('\\', '\\\\', $inputTerm); - $query->orWhere('term', 'like', $inputTerm . '%'); + $escapedTerm = str_replace('\\', '\\\\', $inputTerm); + $query->orWhere('term', 'like', $escapedTerm . '%'); } }); $subQuery->groupBy('entity_type', 'entity_id'); @@ -264,7 +266,7 @@ class SearchRunner $whenStatements = []; $whenBindings = []; - foreach ($options->searches as $term) { + foreach ($options->searches->toValueArray() as $term) { $whenStatements[] = 'WHEN term LIKE ? THEN ?'; $whenBindings[] = $term . '%'; $whenBindings[] = $term; diff --git a/resources/views/search/all.blade.php b/resources/views/search/all.blade.php index 2d410dbd1..aa7ae0aff 100644 --- a/resources/views/search/all.blade.php +++ b/resources/views/search/all.blade.php @@ -8,15 +8,18 @@
{{ trans('entities.search_advanced') }}
+ @php + $filterMap = $options->filters->toValueMap(); + @endphp
{{ trans('entities.search_terms') }}
- +
{{ trans('entities.search_content_type') }}
filters['type'] ?? ''); + $types = explode('|', $filterMap['type'] ?? ''); $hasTypes = $types[0] !== ''; ?> @include('search.parts.type-filter', ['checked' => !$hasTypes || in_array('page', $types), 'entity' => 'page', 'transKey' => 'page']) @@ -27,46 +30,43 @@
{{ trans('entities.search_exact_matches') }}
- @include('search.parts.term-list', ['type' => 'exact', 'currentList' => $options->exacts]) + @include('search.parts.term-list', ['type' => 'exact', 'currentList' => $options->exacts->toValueArray()])
{{ trans('entities.search_tags') }}
- @include('search.parts.term-list', ['type' => 'tags', 'currentList' => $options->tags]) + @include('search.parts.term-list', ['type' => 'tags', 'currentList' => $options->tags->toValueArray()]) @if(!user()->isGuest())
{{ trans('entities.search_options') }}
- @component('search.parts.boolean-filter', ['filters' => $options->filters, 'name' => 'viewed_by_me', 'value' => null]) + @component('search.parts.boolean-filter', ['filters' => $filterMap, 'name' => 'viewed_by_me', 'value' => null]) {{ trans('entities.search_viewed_by_me') }} @endcomponent - @component('search.parts.boolean-filter', ['filters' => $options->filters, 'name' => 'not_viewed_by_me', 'value' => null]) + @component('search.parts.boolean-filter', ['filters' => $filterMap, 'name' => 'not_viewed_by_me', 'value' => null]) {{ trans('entities.search_not_viewed_by_me') }} @endcomponent - @component('search.parts.boolean-filter', ['filters' => $options->filters, 'name' => 'is_restricted', 'value' => null]) + @component('search.parts.boolean-filter', ['filters' => $filterMap, 'name' => 'is_restricted', 'value' => null]) {{ trans('entities.search_permissions_set') }} @endcomponent - @component('search.parts.boolean-filter', ['filters' => $options->filters, 'name' => 'created_by', 'value' => 'me']) + @component('search.parts.boolean-filter', ['filters' => $filterMap, 'name' => 'created_by', 'value' => 'me']) {{ trans('entities.search_created_by_me') }} @endcomponent - @component('search.parts.boolean-filter', ['filters' => $options->filters, 'name' => 'updated_by', 'value' => 'me']) + @component('search.parts.boolean-filter', ['filters' => $filterMap, 'name' => 'updated_by', 'value' => 'me']) {{ trans('entities.search_updated_by_me') }} @endcomponent - @component('search.parts.boolean-filter', ['filters' => $options->filters, 'name' => 'owned_by', 'value' => 'me']) + @component('search.parts.boolean-filter', ['filters' => $filterMap, 'name' => 'owned_by', 'value' => 'me']) {{ trans('entities.search_owned_by_me') }} @endcomponent @endif
{{ trans('entities.search_date_options') }}
- @include('search.parts.date-filter', ['name' => 'updated_after', 'filters' => $options->filters]) - @include('search.parts.date-filter', ['name' => 'updated_before', 'filters' => $options->filters]) - @include('search.parts.date-filter', ['name' => 'created_after', 'filters' => $options->filters]) - @include('search.parts.date-filter', ['name' => 'created_before', 'filters' => $options->filters]) + @include('search.parts.date-filter', ['name' => 'updated_after', 'filters' => $filterMap]) + @include('search.parts.date-filter', ['name' => 'updated_before', 'filters' => $filterMap]) + @include('search.parts.date-filter', ['name' => 'created_after', 'filters' => $filterMap]) + @include('search.parts.date-filter', ['name' => 'created_before', 'filters' => $filterMap]) - @if(isset($options->filters['created_by']) && $options->filters['created_by'] !== "me") - - @endif - @if(isset($options->filters['updated_by']) && $options->filters['updated_by'] !== "me") - - @endif + @foreach($options->getHiddenInputValuesByFieldName() as $fieldName => $value) + + @endforeach
diff --git a/tests/Entity/SearchOptionsTest.php b/tests/Entity/SearchOptionsTest.php index ea4d727a4..7ab150e91 100644 --- a/tests/Entity/SearchOptionsTest.php +++ b/tests/Entity/SearchOptionsTest.php @@ -3,6 +3,7 @@ namespace Tests\Entity; use BookStack\Search\SearchOptions; +use BookStack\Search\SearchOptionSet; use Illuminate\Http\Request; use Tests\TestCase; @@ -12,27 +13,27 @@ class SearchOptionsTest extends TestCase { $options = SearchOptions::fromString('cat "dog" [tag=good] {is_tree}'); - $this->assertEquals(['cat'], $options->searches); - $this->assertEquals(['dog'], $options->exacts); - $this->assertEquals(['tag=good'], $options->tags); - $this->assertEquals(['is_tree' => ''], $options->filters); + $this->assertEquals(['cat'], $options->searches->toValueArray()); + $this->assertEquals(['dog'], $options->exacts->toValueArray()); + $this->assertEquals(['tag=good'], $options->tags->toValueArray()); + $this->assertEquals(['is_tree' => ''], $options->filters->toValueMap()); } public function test_from_string_properly_parses_escaped_quotes() { $options = SearchOptions::fromString('"\"cat\"" surprise "\"\"" "\"donkey" "\"" "\\\\"'); - $this->assertEquals(['"cat"', '""', '"donkey', '"', '\\'], $options->exacts); + $this->assertEquals(['"cat"', '""', '"donkey', '"', '\\'], $options->exacts->toValueArray()); } public function test_to_string_includes_all_items_in_the_correct_format() { $expected = 'cat "dog" [tag=good] {is_tree}'; $options = new SearchOptions(); - $options->searches = ['cat']; - $options->exacts = ['dog']; - $options->tags = ['tag=good']; - $options->filters = ['is_tree' => '']; + $options->searches = SearchOptionSet::fromValueArray(['cat']); + $options->exacts = SearchOptionSet::fromValueArray(['dog']); + $options->tags = SearchOptionSet::fromValueArray(['tag=good']); + $options->filters = SearchOptionSet::fromMapArray(['is_tree' => '']); $output = $options->toString(); foreach (explode(' ', $expected) as $term) { @@ -43,7 +44,7 @@ class SearchOptionsTest extends TestCase public function test_to_string_escapes_as_expected() { $options = new SearchOptions(); - $options->exacts = ['"cat"', '""', '"donkey', '"', '\\', '\\"']; + $options->exacts = SearchOptionSet::fromValueArray(['"cat"', '""', '"donkey', '"', '\\', '\\"']); $output = $options->toString(); $this->assertEquals('"\"cat\"" "\"\"" "\"donkey" "\"" "\\\\" "\\\\\""', $output); @@ -57,14 +58,14 @@ class SearchOptionsTest extends TestCase 'is_tree' => '', 'name' => 'dan', 'cat' => 'happy', - ], $opts->filters); + ], $opts->filters->toValueMap()); } public function test_it_cannot_parse_out_empty_exacts() { $options = SearchOptions::fromString('"" test ""'); - $this->assertEmpty($options->exacts); - $this->assertCount(1, $options->searches); + $this->assertEmpty($options->exacts->toValueArray()); + $this->assertCount(1, $options->searches->toValueArray()); } public function test_from_request_properly_parses_exacts_from_search_terms() @@ -74,7 +75,7 @@ class SearchOptionsTest extends TestCase ]); $options = SearchOptions::fromRequest($request); - $this->assertEquals(["biscuits"], $options->searches); - $this->assertEquals(['"cheese"', '""', '"baked', 'beans"'], $options->exacts); + $this->assertEquals(["biscuits"], $options->searches->toValueArray()); + $this->assertEquals(['"cheese"', '""', '"baked', 'beans"'], $options->exacts->toValueArray()); } } From 93c677a6a955c75318c184d167737836c8c36cd5 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Thu, 3 Oct 2024 15:59:50 +0100 Subject: [PATCH 2/4] 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); + } } From cd84d08157255dd2464a6dcc257355ffeca21c96 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Thu, 3 Oct 2024 19:27:03 +0100 Subject: [PATCH 3/4] Search: Added exact/filter/tag term negation support --- app/Search/Options/TagSearchOption.php | 26 ++++ app/Search/SearchOptionSet.php | 9 +- app/Search/SearchOptions.php | 4 + app/Search/SearchRunner.php | 205 +++++++++++-------------- tests/Entity/EntitySearchTest.php | 26 ++++ 5 files changed, 154 insertions(+), 116 deletions(-) diff --git a/app/Search/Options/TagSearchOption.php b/app/Search/Options/TagSearchOption.php index e61c55ae4..dd2803db4 100644 --- a/app/Search/Options/TagSearchOption.php +++ b/app/Search/Options/TagSearchOption.php @@ -4,8 +4,34 @@ namespace BookStack\Search\Options; class TagSearchOption extends SearchOption { + /** + * Acceptable operators to be used within a tag search option. + * + * @var string[] + */ + protected array $queryOperators = ['<=', '>=', '=', '<', '>', 'like', '!=']; + public function toString(): string { return ($this->negated ? '-' : '') . "[{$this->value}]"; } + + /** + * @return array{name: string, operator: string, value: string} + */ + public function getParts(): array + { + $operatorRegex = implode('|', array_map(fn($op) => preg_quote($op), $this->queryOperators)); + preg_match('/^(.*?)((' . $operatorRegex . ')(.*?))?$/', $this->value, $tagSplit); + + $extractedOperator = count($tagSplit) > 2 ? $tagSplit[3] : ''; + $tagOperator = in_array($extractedOperator, $this->queryOperators) ? $extractedOperator : '='; + $tagValue = count($tagSplit) > 3 ? $tagSplit[4] : ''; + + return [ + 'name' => $tagSplit[1], + 'operator' => $tagOperator, + 'value' => $tagValue, + ]; + } } diff --git a/app/Search/SearchOptionSet.php b/app/Search/SearchOptionSet.php index 467dc9f5a..f421abe2c 100644 --- a/app/Search/SearchOptionSet.php +++ b/app/Search/SearchOptionSet.php @@ -4,10 +4,13 @@ namespace BookStack\Search; use BookStack\Search\Options\SearchOption; +/** + * @template T of SearchOption + */ class SearchOptionSet { /** - * @var SearchOption[] + * @var T[] */ protected array $options = []; @@ -52,7 +55,7 @@ class SearchOptionSet } /** - * @return SearchOption[] + * @return T[] */ public function all(): array { @@ -60,7 +63,7 @@ class SearchOptionSet } /** - * @return SearchOption[] + * @return T[] */ public function negated(): array { diff --git a/app/Search/SearchOptions.php b/app/Search/SearchOptions.php index 98f731ee7..e362a8a01 100644 --- a/app/Search/SearchOptions.php +++ b/app/Search/SearchOptions.php @@ -11,9 +11,13 @@ use Illuminate\Http\Request; class SearchOptions { + /** @var SearchOptionSet */ public SearchOptionSet $searches; + /** @var SearchOptionSet */ public SearchOptionSet $exacts; + /** @var SearchOptionSet */ public SearchOptionSet $tags; + /** @var SearchOptionSet */ public SearchOptionSet $filters; public function __construct() diff --git a/app/Search/SearchRunner.php b/app/Search/SearchRunner.php index 855140508..265b9eecc 100644 --- a/app/Search/SearchRunner.php +++ b/app/Search/SearchRunner.php @@ -7,6 +7,7 @@ use BookStack\Entities\Models\Entity; use BookStack\Entities\Models\Page; use BookStack\Entities\Queries\EntityQueries; use BookStack\Permissions\PermissionApplicator; +use BookStack\Search\Options\TagSearchOption; use BookStack\Users\Models\User; use Illuminate\Database\Connection; use Illuminate\Database\Eloquent\Builder as EloquentBuilder; @@ -16,31 +17,21 @@ use Illuminate\Database\Query\Builder; use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; use Illuminate\Support\Str; -use SplObjectStorage; +use WeakMap; class SearchRunner { - /** - * Acceptable operators to be used in a query. - * - * @var string[] - */ - protected array $queryOperators = ['<=', '>=', '=', '<', '>', 'like', '!=']; - /** * Retain a cache of score adjusted terms for specific search options. - * From PHP>=8 this can be made into a WeakMap instead. - * - * @var SplObjectStorage */ - protected $termAdjustmentCache; + protected WeakMap $termAdjustmentCache; public function __construct( protected EntityProvider $entityProvider, protected PermissionApplicator $permissions, protected EntityQueries $entityQueries, ) { - $this->termAdjustmentCache = new SplObjectStorage(); + $this->termAdjustmentCache = new WeakMap(); } /** @@ -163,24 +154,26 @@ class SearchRunner $this->applyTermSearch($entityQuery, $searchOpts, $entityType); // Handle exact term matching - foreach ($searchOpts->exacts->toValueArray() as $inputTerm) { - $entityQuery->where(function (EloquentBuilder $query) use ($inputTerm, $entityModelInstance) { - $inputTerm = str_replace('\\', '\\\\', $inputTerm); + foreach ($searchOpts->exacts->all() as $exact) { + $filter = function (EloquentBuilder $query) use ($exact, $entityModelInstance) { + $inputTerm = str_replace('\\', '\\\\', $exact->value); $query->where('name', 'like', '%' . $inputTerm . '%') ->orWhere($entityModelInstance->textField, 'like', '%' . $inputTerm . '%'); - }); + }; + + $exact->negated ? $entityQuery->whereNot($filter) : $entityQuery->where($filter); } // Handle tag searches - foreach ($searchOpts->tags->toValueArray() as $inputTerm) { - $this->applyTagSearch($entityQuery, $inputTerm); + foreach ($searchOpts->tags->all() as $tagOption) { + $this->applyTagSearch($entityQuery, $tagOption); } // Handle filters - foreach ($searchOpts->filters->toValueMap() as $filterTerm => $filterValue) { - $functionName = Str::camel('filter_' . $filterTerm); + foreach ($searchOpts->filters->all() as $filterOption) { + $functionName = Str::camel('filter_' . $filterOption->getKey()); if (method_exists($this, $functionName)) { - $this->$functionName($entityQuery, $entityModelInstance, $filterValue); + $this->$functionName($entityQuery, $entityModelInstance, $filterOption->value, $filterOption->negated); } } @@ -312,179 +305,165 @@ class SearchRunner } /** - * Get the available query operators as a regex escaped list. + * Apply a tag search term onto an entity query. */ - protected function getRegexEscapedOperators(): string + protected function applyTagSearch(EloquentBuilder $query, TagSearchOption $option): void { - $escapedOperators = []; - foreach ($this->queryOperators as $operator) { - $escapedOperators[] = preg_quote($operator); - } + $filter = function (EloquentBuilder $query) use ($option): void { + $tagParts = $option->getParts(); + if (empty($tagParts['operator']) || empty($tagParts['value'])) { + $query->where('name', '=', $tagParts['name']); + return; + } - return implode('|', $escapedOperators); + if (!empty($tagParts['name'])) { + $query->where('name', '=', $tagParts['name']); + } + + if (is_numeric($tagParts['value']) && $tagParts['operator'] !== 'like') { + // We have to do a raw sql query for this since otherwise PDO will quote the value and MySQL will + // search the value as a string which prevents being able to do number-based operations + // on the tag values. We ensure it has a numeric value and then cast it just to be sure. + /** @var Connection $connection */ + $connection = $query->getConnection(); + $quotedValue = (float) trim($connection->getPdo()->quote($tagParts['value']), "'"); + $query->whereRaw("value {$tagParts['operator']} {$quotedValue}"); + } else if ($tagParts['operator'] === 'like') { + $query->where('value', $tagParts['operator'], str_replace('\\', '\\\\', $tagParts['value'])); + } else { + $query->where('value', $tagParts['operator'], $tagParts['value']); + } + }; + + $option->negated ? $query->whereDoesntHave('tags', $filter) : $query->whereHas('tags', $filter); } - /** - * Apply a tag search term onto a entity query. - */ - protected function applyTagSearch(EloquentBuilder $query, string $tagTerm): EloquentBuilder + protected function applyNegatableWhere(EloquentBuilder $query, bool $negated, string $column, string $operator, mixed $value): void { - preg_match('/^(.*?)((' . $this->getRegexEscapedOperators() . ')(.*?))?$/', $tagTerm, $tagSplit); - $query->whereHas('tags', function (EloquentBuilder $query) use ($tagSplit) { - $tagName = $tagSplit[1]; - $tagOperator = count($tagSplit) > 2 ? $tagSplit[3] : ''; - $tagValue = count($tagSplit) > 3 ? $tagSplit[4] : ''; - $validOperator = in_array($tagOperator, $this->queryOperators); - if (!empty($tagOperator) && !empty($tagValue) && $validOperator) { - if (!empty($tagName)) { - $query->where('name', '=', $tagName); - } - if (is_numeric($tagValue) && $tagOperator !== 'like') { - // We have to do a raw sql query for this since otherwise PDO will quote the value and MySQL will - // search the value as a string which prevents being able to do number-based operations - // on the tag values. We ensure it has a numeric value and then cast it just to be sure. - /** @var Connection $connection */ - $connection = $query->getConnection(); - $tagValue = (float) trim($connection->getPdo()->quote($tagValue), "'"); - $query->whereRaw("value {$tagOperator} {$tagValue}"); - } else { - if ($tagOperator === 'like') { - $tagValue = str_replace('\\', '\\\\', $tagValue); - } - $query->where('value', $tagOperator, $tagValue); - } - } else { - $query->where('name', '=', $tagName); - } - }); - - return $query; + if ($negated) { + $query->whereNot($column, $operator, $value); + } else { + $query->where($column, $operator, $value); + } } /** * Custom entity search filters. */ - protected function filterUpdatedAfter(EloquentBuilder $query, Entity $model, $input): void + protected function filterUpdatedAfter(EloquentBuilder $query, Entity $model, string $input, bool $negated): void { - try { - $date = date_create($input); - $query->where('updated_at', '>=', $date); - } catch (\Exception $e) { - } + $date = date_create($input); + $this->applyNegatableWhere($query, $negated, 'updated_at', '>=', $date); } - protected function filterUpdatedBefore(EloquentBuilder $query, Entity $model, $input): void + protected function filterUpdatedBefore(EloquentBuilder $query, Entity $model, string $input, bool $negated): void { - try { - $date = date_create($input); - $query->where('updated_at', '<', $date); - } catch (\Exception $e) { - } + $date = date_create($input); + $this->applyNegatableWhere($query, $negated, 'updated_at', '<', $date); } - protected function filterCreatedAfter(EloquentBuilder $query, Entity $model, $input): void + protected function filterCreatedAfter(EloquentBuilder $query, Entity $model, string $input, bool $negated): void { - try { - $date = date_create($input); - $query->where('created_at', '>=', $date); - } catch (\Exception $e) { - } + $date = date_create($input); + $this->applyNegatableWhere($query, $negated, 'created_at', '>=', $date); } - protected function filterCreatedBefore(EloquentBuilder $query, Entity $model, $input) + protected function filterCreatedBefore(EloquentBuilder $query, Entity $model, string $input, bool $negated) { - try { - $date = date_create($input); - $query->where('created_at', '<', $date); - } catch (\Exception $e) { - } + $date = date_create($input); + $this->applyNegatableWhere($query, $negated, 'created_at', '<', $date); } - protected function filterCreatedBy(EloquentBuilder $query, Entity $model, $input) + protected function filterCreatedBy(EloquentBuilder $query, Entity $model, string $input, bool $negated) { $userSlug = $input === 'me' ? user()->slug : trim($input); $user = User::query()->where('slug', '=', $userSlug)->first(['id']); if ($user) { - $query->where('created_by', '=', $user->id); + $this->applyNegatableWhere($query, $negated, 'created_by', '=', $user->id); } } - protected function filterUpdatedBy(EloquentBuilder $query, Entity $model, $input) + protected function filterUpdatedBy(EloquentBuilder $query, Entity $model, string $input, bool $negated) { $userSlug = $input === 'me' ? user()->slug : trim($input); $user = User::query()->where('slug', '=', $userSlug)->first(['id']); if ($user) { - $query->where('updated_by', '=', $user->id); + $this->applyNegatableWhere($query, $negated, 'updated_by', '=', $user->id); } } - protected function filterOwnedBy(EloquentBuilder $query, Entity $model, $input) + protected function filterOwnedBy(EloquentBuilder $query, Entity $model, string $input, bool $negated) { $userSlug = $input === 'me' ? user()->slug : trim($input); $user = User::query()->where('slug', '=', $userSlug)->first(['id']); if ($user) { - $query->where('owned_by', '=', $user->id); + $this->applyNegatableWhere($query, $negated, 'owned_by', '=', $user->id); } } - protected function filterInName(EloquentBuilder $query, Entity $model, $input) + protected function filterInName(EloquentBuilder $query, Entity $model, string $input, bool $negated) { - $query->where('name', 'like', '%' . $input . '%'); + $this->applyNegatableWhere($query, $negated, 'name', 'like', '%' . $input . '%'); } - protected function filterInTitle(EloquentBuilder $query, Entity $model, $input) + protected function filterInTitle(EloquentBuilder $query, Entity $model, string $input, bool $negated) { - $this->filterInName($query, $model, $input); + $this->filterInName($query, $model, $input, $negated); } - protected function filterInBody(EloquentBuilder $query, Entity $model, $input) + protected function filterInBody(EloquentBuilder $query, Entity $model, string $input, bool $negated) { - $query->where($model->textField, 'like', '%' . $input . '%'); + $this->applyNegatableWhere($query, $negated, $model->textField, 'like', '%' . $input . '%'); } - protected function filterIsRestricted(EloquentBuilder $query, Entity $model, $input) + protected function filterIsRestricted(EloquentBuilder $query, Entity $model, string $input, bool $negated) { - $query->whereHas('permissions'); + $negated ? $query->whereDoesntHave('permissions') : $query->whereHas('permissions'); } - protected function filterViewedByMe(EloquentBuilder $query, Entity $model, $input) + protected function filterViewedByMe(EloquentBuilder $query, Entity $model, string $input, bool $negated) { - $query->whereHas('views', function ($query) { + $filter = function ($query) { $query->where('user_id', '=', user()->id); - }); + }; + + $negated ? $query->whereDoesntHave('views', $filter) : $query->whereHas('views', $filter); } - protected function filterNotViewedByMe(EloquentBuilder $query, Entity $model, $input) + protected function filterNotViewedByMe(EloquentBuilder $query, Entity $model, string $input, bool $negated) { - $query->whereDoesntHave('views', function ($query) { + $filter = function ($query) { $query->where('user_id', '=', user()->id); - }); + }; + + $negated ? $query->whereHas('views', $filter) : $query->whereDoesntHave('views', $filter); } - protected function filterIsTemplate(EloquentBuilder $query, Entity $model, $input) + protected function filterIsTemplate(EloquentBuilder $query, Entity $model, string $input, bool $negated) { if ($model instanceof Page) { - $query->where('template', '=', true); + $this->applyNegatableWhere($query, $negated, 'template', '=', true); } } - protected function filterSortBy(EloquentBuilder $query, Entity $model, $input) + protected function filterSortBy(EloquentBuilder $query, Entity $model, string $input, bool $negated) { $functionName = Str::camel('sort_by_' . $input); if (method_exists($this, $functionName)) { - $this->$functionName($query, $model); + $this->$functionName($query, $model, $negated); } } /** * Sorting filter options. */ - protected function sortByLastCommented(EloquentBuilder $query, Entity $model) + protected function sortByLastCommented(EloquentBuilder $query, Entity $model, bool $negated) { $commentsTable = DB::getTablePrefix() . 'comments'; $morphClass = str_replace('\\', '\\\\', $model->getMorphClass()); $commentQuery = DB::raw('(SELECT c1.entity_id, c1.entity_type, c1.created_at as last_commented FROM ' . $commentsTable . ' c1 LEFT JOIN ' . $commentsTable . ' c2 ON (c1.entity_id = c2.entity_id AND c1.entity_type = c2.entity_type AND c1.created_at < c2.created_at) WHERE c1.entity_type = \'' . $morphClass . '\' AND c2.created_at IS NULL) as comments'); - $query->join($commentQuery, $model->getTable() . '.id', '=', 'comments.entity_id')->orderBy('last_commented', 'desc'); + $query->join($commentQuery, $model->getTable() . '.id', '=', 'comments.entity_id') + ->orderBy('last_commented', $negated ? 'asc' : 'desc'); } } diff --git a/tests/Entity/EntitySearchTest.php b/tests/Entity/EntitySearchTest.php index c3e959345..bb1021a67 100644 --- a/tests/Entity/EntitySearchTest.php +++ b/tests/Entity/EntitySearchTest.php @@ -118,6 +118,32 @@ class EntitySearchTest extends TestCase $exactSearchB->assertStatus(200)->assertDontSee($page->name); } + public function test_negated_searches() + { + $page = $this->entities->newPage(['name' => 'My new test negation page', 'html' => '

An angry tortoise wore trumpeted plimsoles

']); + $page->tags()->saveMany([new Tag(['name' => 'DonkCount', 'value' => '500'])]); + $page->created_by = $this->users->admin()->id; + $page->save(); + + $editor = $this->users->editor(); + $this->actingAs($editor); + + $exactSearch = $this->get('/search?term=' . urlencode('negation -"tortoise"')); + $exactSearch->assertStatus(200)->assertDontSeeText($page->name); + + $tagSearchA = $this->get('/search?term=' . urlencode('negation [DonkCount=500]')); + $tagSearchA->assertStatus(200)->assertSeeText($page->name); + $tagSearchB = $this->get('/search?term=' . urlencode('negation -[DonkCount=500]')); + $tagSearchB->assertStatus(200)->assertDontSeeText($page->name); + + $filterSearchA = $this->get('/search?term=' . urlencode('negation -{created_by:me}')); + $filterSearchA->assertStatus(200)->assertSeeText($page->name); + $page->created_by = $editor->id; + $page->save(); + $filterSearchB = $this->get('/search?term=' . urlencode('negation -{created_by:me}')); + $filterSearchB->assertStatus(200)->assertDontSeeText($page->name); + } + public function test_search_terms_with_delimiters_are_converted_to_exact_matches() { $this->asEditor(); From 966ff9138632a47966c217089f0534ecf262d829 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Thu, 3 Oct 2024 19:38:07 +0100 Subject: [PATCH 4/4] Search: Prevented negated terms filling in UI inputs Added test to cover. --- app/Search/SearchOptionSet.php | 16 +++++++++++++--- app/Search/SearchOptions.php | 6 +++--- resources/views/search/all.blade.php | 6 +++--- tests/Entity/EntitySearchTest.php | 8 ++++++++ tests/Entity/SearchOptionsTest.php | 2 +- 5 files changed, 28 insertions(+), 10 deletions(-) diff --git a/app/Search/SearchOptionSet.php b/app/Search/SearchOptionSet.php index f421abe2c..bd5e5a5b2 100644 --- a/app/Search/SearchOptionSet.php +++ b/app/Search/SearchOptionSet.php @@ -63,10 +63,20 @@ class SearchOptionSet } /** - * @return T[] + * @return self */ - public function negated(): array + public function negated(): self { - return array_values(array_filter($this->options, fn (SearchOption $option) => $option->negated)); + $values = array_values(array_filter($this->options, fn (SearchOption $option) => $option->negated)); + return new self($values); + } + + /** + * @return self + */ + public function nonNegated(): self + { + $values = array_values(array_filter($this->options, fn (SearchOption $option) => !$option->negated)); + return new self($values); } } diff --git a/app/Search/SearchOptions.php b/app/Search/SearchOptions.php index e362a8a01..7f9db2a64 100644 --- a/app/Search/SearchOptions.php +++ b/app/Search/SearchOptions.php @@ -244,9 +244,9 @@ class SearchOptions } // Negated items - array_push($options, ...$this->exacts->negated()); - array_push($options, ...$this->tags->negated()); - array_push($options, ...$this->filters->negated()); + array_push($options, ...$this->exacts->negated()->all()); + array_push($options, ...$this->tags->negated()->all()); + array_push($options, ...$this->filters->negated()->all()); 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 2a0d63a6e..ad437604b 100644 --- a/resources/views/search/all.blade.php +++ b/resources/views/search/all.blade.php @@ -9,7 +9,7 @@
{{ trans('entities.search_advanced') }}
@php - $filterMap = $options->filters->toValueMap(); + $filterMap = $options->filters->nonNegated()->toValueMap(); @endphp
{{ trans('entities.search_terms') }}
@@ -30,10 +30,10 @@
{{ trans('entities.search_exact_matches') }}
- @include('search.parts.term-list', ['type' => 'exact', 'currentList' => $options->exacts->toValueArray()]) + @include('search.parts.term-list', ['type' => 'exact', 'currentList' => $options->exacts->nonNegated()->toValueArray()])
{{ trans('entities.search_tags') }}
- @include('search.parts.term-list', ['type' => 'tags', 'currentList' => $options->tags->toValueArray()]) + @include('search.parts.term-list', ['type' => 'tags', 'currentList' => $options->tags->nonNegated()->toValueArray()]) @if(!user()->isGuest())
{{ trans('entities.search_options') }}
diff --git a/tests/Entity/EntitySearchTest.php b/tests/Entity/EntitySearchTest.php index bb1021a67..3a1a0a662 100644 --- a/tests/Entity/EntitySearchTest.php +++ b/tests/Entity/EntitySearchTest.php @@ -577,6 +577,14 @@ class EntitySearchTest extends TestCase $this->withHtml($resp)->assertFieldHasValue('extras', '{updated_by:dan} {created_by:dan} -"dog" -[a=b] -{viewed_by_me}'); } + public function test_negated_searches_dont_show_in_inputs() + { + $resp = $this->asEditor()->get('/search?term=' . urlencode('-{created_by:me} -[a=b] -"dog"')); + $this->withHtml($resp)->assertElementNotExists('input[name="tags[]"][value="a=b"]'); + $this->withHtml($resp)->assertElementNotExists('input[name="exact[]"][value="dog"]'); + $this->withHtml($resp)->assertElementNotExists('input[name="filters[created_by]"][value="me"][checked="checked"]'); + } + public function test_searches_with_user_filters_using_me_adds_them_into_advanced_search_form() { $resp = $this->asEditor()->get('/search?term=' . urlencode('test {updated_by:me} {created_by:me}')); diff --git a/tests/Entity/SearchOptionsTest.php b/tests/Entity/SearchOptionsTest.php index 543badcef..ae0f1e56a 100644 --- a/tests/Entity/SearchOptionsTest.php +++ b/tests/Entity/SearchOptionsTest.php @@ -123,7 +123,7 @@ class SearchOptionsTest extends TestCase $options = SearchOptions::fromRequest($request); $this->assertCount(2, $options->tags->all()); - $this->assertEquals('b=c', $options->tags->negated()[0]->value); + $this->assertEquals('b=c', $options->tags->negated()->all()[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);