From 3bc1e6b3871ca7cfe34b8ee18a79aec035c92303 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 7 May 2024 16:46:54 +0100 Subject: [PATCH 1/2] Fix behaviour of 'when filter empty' for empty and notEmpty filter types. --- packages/backend-core/src/db/lucene.ts | 16 ++++++++++++ .../src/api/routes/tests/search.spec.ts | 25 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/packages/backend-core/src/db/lucene.ts b/packages/backend-core/src/db/lucene.ts index d9dddd0097..09d0224014 100644 --- a/packages/backend-core/src/db/lucene.ts +++ b/packages/backend-core/src/db/lucene.ts @@ -432,9 +432,25 @@ export class QueryBuilder { } if (this.#query.empty) { build(this.#query.empty, (key: string) => `(*:* -${key}:["" TO *])`) + + // Because the structure of an empty filter looks like this: + // { empty: { someKey: null } } + // + // The check inside of `build` does not set `allFiltersEmpty`, which results + // in weird behaviour when the empty filter is the only filter. We get around + // this by setting `allFiltersEmpty` to false here. + allFiltersEmpty = false } if (this.#query.notEmpty) { build(this.#query.notEmpty, (key: string) => `${key}:["" TO *]`) + + // Because the structure of a notEmpty filter looks like this: + // { notEmpty: { someKey: null } } + // + // The check inside of `build` does not set `allFiltersEmpty`, which results + // in weird behaviour when the empty filter is the only filter. We get around + // this by setting `allFiltersEmpty` to false here. + allFiltersEmpty = false } if (this.#query.oneOf) { build(this.#query.oneOf, oneOf) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 6ab7ac61d4..9e1511105d 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -252,6 +252,31 @@ describe.each([ }).toFindNothing()) }) + describe("empty", () => { + it("finds no empty rows", () => + expectQuery({ empty: { name: null } }).toFindNothing()) + + it("should not be affected by when filter empty behaviour", () => + expectQuery({ + empty: { name: null }, + onEmptyFilter: EmptyFilterOption.RETURN_ALL, + }).toFindNothing()) + }) + + describe("notEmpty", () => { + it("finds all non-empty rows", () => + expectQuery({ notEmpty: { name: null } }).toContainExactly([ + { name: "foo" }, + { name: "bar" }, + ])) + + it("should not be affected by when filter empty behaviour", () => + expectQuery({ + notEmpty: { name: null }, + onEmptyFilter: EmptyFilterOption.RETURN_NONE, + }).toContainExactly([{ name: "foo" }, { name: "bar" }])) + }) + describe("sort", () => { it("sorts ascending", () => expectSearch({ From 946bd0ef7dae8dc3b388b6deea2f7132e5aa41c3 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 7 May 2024 17:11:05 +0100 Subject: [PATCH 2/2] Fix lucene.spec.ts. --- packages/backend-core/src/db/lucene.ts | 38 ++++++++++++++------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/packages/backend-core/src/db/lucene.ts b/packages/backend-core/src/db/lucene.ts index 09d0224014..37768e934e 100644 --- a/packages/backend-core/src/db/lucene.ts +++ b/packages/backend-core/src/db/lucene.ts @@ -431,26 +431,28 @@ export class QueryBuilder { }) } if (this.#query.empty) { - build(this.#query.empty, (key: string) => `(*:* -${key}:["" TO *])`) - - // Because the structure of an empty filter looks like this: - // { empty: { someKey: null } } - // - // The check inside of `build` does not set `allFiltersEmpty`, which results - // in weird behaviour when the empty filter is the only filter. We get around - // this by setting `allFiltersEmpty` to false here. - allFiltersEmpty = false + build(this.#query.empty, (key: string) => { + // Because the structure of an empty filter looks like this: + // { empty: { someKey: null } } + // + // The check inside of `build` does not set `allFiltersEmpty`, which results + // in weird behaviour when the empty filter is the only filter. We get around + // this by setting `allFiltersEmpty` to false here. + allFiltersEmpty = false + return `(*:* -${key}:["" TO *])` + }) } if (this.#query.notEmpty) { - build(this.#query.notEmpty, (key: string) => `${key}:["" TO *]`) - - // Because the structure of a notEmpty filter looks like this: - // { notEmpty: { someKey: null } } - // - // The check inside of `build` does not set `allFiltersEmpty`, which results - // in weird behaviour when the empty filter is the only filter. We get around - // this by setting `allFiltersEmpty` to false here. - allFiltersEmpty = false + build(this.#query.notEmpty, (key: string) => { + // Because the structure of a notEmpty filter looks like this: + // { notEmpty: { someKey: null } } + // + // The check inside of `build` does not set `allFiltersEmpty`, which results + // in weird behaviour when the empty filter is the only filter. We get around + // this by setting `allFiltersEmpty` to false here. + allFiltersEmpty = false + return `${key}:["" TO *]` + }) } if (this.#query.oneOf) { build(this.#query.oneOf, oneOf)