From 2e12af46d17cfd9927438e83758c19dcb8f7b749 Mon Sep 17 00:00:00 2001 From: "Patrik J. Braun" Date: Mon, 31 May 2021 16:44:37 +0200 Subject: [PATCH] Fixing SOME_OF query flatteing issue --- package.json | 2 +- .../model/database/sql/SearchManager.ts | 150 ++++++++++-------- src/common/SearchQueryParser.ts | 29 +++- .../unit/model/sql/SearchManager.spec.ts | 73 ++++++++- test/common/unit/SearchQueryParser.ts | 37 +---- 5 files changed, 190 insertions(+), 101 deletions(-) diff --git a/package.json b/package.json index 9d0effc7..bc6836d9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "pigallery2", - "version": "1.9.0", + "version": "1.9.1", "description": "This is a photo gallery optimised for running low resource servers (especially on raspberry pi)", "author": "Patrik J. Braun", "homepage": "https://github.com/bpatrik/PiGallery2", diff --git a/src/backend/model/database/sql/SearchManager.ts b/src/backend/model/database/sql/SearchManager.ts index ebc9d06a..d2dea7f2 100644 --- a/src/backend/model/database/sql/SearchManager.ts +++ b/src/backend/model/database/sql/SearchManager.ts @@ -237,6 +237,89 @@ export class SearchManager implements ISQLSearchManager { .getOne(); } + protected flattenSameOfQueries(query: SearchQueryDTO): SearchQueryDTO { + switch (query.type) { + case SearchQueryTypes.AND: + case SearchQueryTypes.OR: + return { + type: query.type, + list: (query as SearchListQuery).list.map((q): SearchQueryDTO => this.flattenSameOfQueries(q)) + } as SearchListQuery; + case SearchQueryTypes.SOME_OF: + const someOfQ = query as SomeOfSearchQuery; + someOfQ.min = someOfQ.min || 1; + + if (someOfQ.min === 1) { + return this.flattenSameOfQueries({ + type: SearchQueryTypes.OR, + list: (someOfQ as SearchListQuery).list + } as ORSearchQuery); + } + + if (someOfQ.min === (query as SearchListQuery).list.length) { + return this.flattenSameOfQueries({ + type: SearchQueryTypes.AND, + list: (someOfQ as SearchListQuery).list + } as ANDSearchQuery); + } + + const getAllCombinations = (num: number, arr: SearchQueryDTO[], start = 0): SearchQueryDTO[] => { + if (num <= 0 || num > arr.length || start >= arr.length) { + return null; + } + if (num <= 1) { + return arr.slice(start); + } + if (num === arr.length - start) { + return [{ + type: SearchQueryTypes.AND, + list: arr.slice(start) + } as ANDSearchQuery]; + } + const ret: ANDSearchQuery[] = []; + for (let i = start; i < arr.length; ++i) { + const subRes = getAllCombinations(num - 1, arr, i + 1); + if (subRes === null) { + break; + } + const and: ANDSearchQuery = { + type: SearchQueryTypes.AND, + list: [ + arr[i] + ] + }; + if (subRes.length === 1) { + if (subRes[0].type === SearchQueryTypes.AND) { + and.list.push(...(subRes[0] as ANDSearchQuery).list); + } else { + and.list.push(subRes[0]); + } + } else { + and.list.push({ + type: SearchQueryTypes.OR, + list: subRes + } as ORSearchQuery); + } + ret.push(and); + + } + + if (ret.length === 0) { + return null; + } + return ret; + }; + + + return this.flattenSameOfQueries({ + type: SearchQueryTypes.OR, + list: getAllCombinations(someOfQ.min, (query as SearchListQuery).list) + } as ORSearchQuery); + + } + return query; + } + private async prepareQuery(queryIN: SearchQueryDTO): Promise { let query: SearchQueryDTO = this.assignQueryIDs(Utils.clone(queryIN)); // assign local ids before flattening SOME_OF queries query = this.flattenSameOfQueries(query); @@ -611,73 +694,6 @@ export class SearchManager implements ISQLSearchManager { }); } - private flattenSameOfQueries(query: SearchQueryDTO): SearchQueryDTO { - switch (query.type) { - case SearchQueryTypes.AND: - case SearchQueryTypes.OR: - return { - type: query.type, - list: (query as SearchListQuery).list.map((q): SearchQueryDTO => this.flattenSameOfQueries(q)) - } as SearchListQuery; - case SearchQueryTypes.SOME_OF: - const someOfQ = query as SomeOfSearchQuery; - someOfQ.min = someOfQ.min || 1; - - if (someOfQ.min === 1) { - return this.flattenSameOfQueries({ - type: SearchQueryTypes.OR, - list: (someOfQ as SearchListQuery).list - } as ORSearchQuery); - } - - if (someOfQ.min === (query as SearchListQuery).list.length) { - return this.flattenSameOfQueries({ - type: SearchQueryTypes.AND, - list: (someOfQ as SearchListQuery).list - } as ANDSearchQuery); - } - - const getAllCombinations = (num: number, arr: SearchQueryDTO[], start = 0): SearchQueryDTO[] => { - if (num <= 0 || num > arr.length || start >= arr.length) { - return []; - } - if (num <= 1) { - return arr.slice(start); - } - if (num === arr.length - start) { - return arr.slice(start); - } - const ret: ANDSearchQuery[] = []; - for (let i = start; i < arr.length - num + 1; ++i) { - const subRes = getAllCombinations(num - 1, arr, i + 1); - const and: ANDSearchQuery = { - type: SearchQueryTypes.AND, - list: [ - arr[i], - subRes.length === 1 ? subRes[0] : ( - { - type: SearchQueryTypes.OR, - list: subRes - } as ORSearchQuery) - ] - }; - - ret.push(and); - - } - return ret; - }; - - - return this.flattenSameOfQueries({ - type: SearchQueryTypes.OR, - list: getAllCombinations(someOfQ.min, (query as SearchListQuery).list) - } as ORSearchQuery); - - } - return query; - } - private encapsulateAutoComplete(values: string[], type: SearchQueryTypes): Array { const res: AutoCompleteItem[] = []; values.forEach((value): void => { diff --git a/src/common/SearchQueryParser.ts b/src/common/SearchQueryParser.ts index 6df897f4..190377e9 100644 --- a/src/common/SearchQueryParser.ts +++ b/src/common/SearchQueryParser.ts @@ -46,8 +46,35 @@ export interface QueryKeywords { position: string; } + +export const defaultQueryKeywords: QueryKeywords = { + NSomeOf: 'of', + and: 'and', + or: 'or', + + from: 'after', + to: 'before', + landscape: 'landscape', + maxRating: 'max-rating', + maxResolution: 'max-resolution', + minRating: 'min-rating', + minResolution: 'min-resolution', + orientation: 'orientation', + + any_text: 'any-text', + keyword: 'keyword', + caption: 'caption', + directory: 'directory', + file_name: 'file-name', + person: 'person', + portrait: 'portrait', + position: 'position', + someOf: 'some-of', + kmFrom: 'km-from' +}; + export class SearchQueryParser { - constructor(private keywords: QueryKeywords) { + constructor(private keywords: QueryKeywords = defaultQueryKeywords) { } public static stringifyText(text: string, matchType = TextSearchQueryMatchTypes.like): string { diff --git a/test/backend/unit/model/sql/SearchManager.spec.ts b/test/backend/unit/model/sql/SearchManager.spec.ts index b66e6ec3..3e07dfb7 100644 --- a/test/backend/unit/model/sql/SearchManager.spec.ts +++ b/test/backend/unit/model/sql/SearchManager.spec.ts @@ -13,6 +13,7 @@ import { MinResolutionSearch, OrientationSearch, ORSearchQuery, + SearchListQuery, SearchQueryDTO, SearchQueryTypes, SomeOfSearchQuery, @@ -32,6 +33,7 @@ import {VideoDTO} from '../../../../../src/common/entities/VideoDTO'; import {MediaDTO} from '../../../../../src/common/entities/MediaDTO'; import {AutoCompleteItem} from '../../../../../src/common/entities/AutoCompleteItem'; import {Config} from '../../../../../src/common/config/private/Config'; +import {SearchQueryParser} from '../../../../../src/common/SearchQueryParser'; const deepEqualInAnyOrder = require('deep-equal-in-any-order'); const chai = require('chai'); @@ -54,6 +56,14 @@ class IndexingManagerTest extends IndexingManager { } } +class SearchManagerTest extends SearchManager { + + public flattenSameOfQueries(query: SearchQueryDTO): SearchQueryDTO { + return super.flattenSameOfQueries(query); + } + +} + class GalleryManagerTest extends GalleryManager { public async selectParentDir(connection: Connection, directoryName: string, directoryParent: string): Promise { @@ -1114,7 +1124,68 @@ describe('SearchManager', (sqlHelper: DBTestHelper) => { }); - (it('should execute complex SOME_OF querry', async () => { + /** + * flattenSameOfQueries converts converts some-of querries to AND and OR queries + * E.g.: + * 2-of:(A B C) to (A and (B or C)) or (B and C) + * this tests makes sure that all queries has at least 2 constraints + */ + (it('should flatter SOME_OF query', () => { + const sm = new SearchManagerTest(); + const parser = new SearchQueryParser(); + const alphabet = 'abcdefghijklmnopqrstu'; + + + const shortestDepth = (q: SearchQueryDTO): number => { + let depth = 0; + if ((q as SearchListQuery).list) { + if (q.type === SearchQueryTypes.AND) { + for (const l of (q as SearchListQuery).list) { + depth += shortestDepth(l); + } + return depth; + } + // its an or + const lengths = (q as SearchListQuery).list.map(l => shortestDepth(l)).sort(); + + if (lengths[0] !== lengths[lengths.length - 1]) { + for (const l of (q as SearchListQuery).list) { + console.log(shortestDepth(l)); + console.log(parser.stringify(l)); + } + } + return lengths[0]; + } + return 1; + }; + + const checkBoolLogic = (q: SearchQueryDTO) => { + if ((q as SearchListQuery).list) { + expect((q as SearchListQuery).list).to.not.equal(1); + for (const l of (q as SearchListQuery).list) { + checkBoolLogic(l); + } + } + }; + + // tslint:disable-next-line:prefer-for-of + for (let i = 1; i < alphabet.length / 2; ++i) { + const query: SomeOfSearchQuery = { + type: SearchQueryTypes.SOME_OF, + min: i, + // + list: alphabet.split('').map(t => ({ + type: SearchQueryTypes.file_name, + text: t + } as TextSearch)) + }; + const q = sm.flattenSameOfQueries(query); + expect(shortestDepth(q)).to.equal(i, parser.stringify(query) + '\n' + parser.stringify(q)); + checkBoolLogic(q); + } + }) as any).timeout(20000); + + (it('should execute complex SOME_OF query', async () => { const sm = new SearchManager(); const query: SomeOfSearchQuery = { diff --git a/test/common/unit/SearchQueryParser.ts b/test/common/unit/SearchQueryParser.ts index d254665a..cf7f4e0b 100644 --- a/test/common/unit/SearchQueryParser.ts +++ b/test/common/unit/SearchQueryParser.ts @@ -17,38 +17,13 @@ import { TextSearchQueryMatchTypes, ToDateSearch } from '../../../src/common/entities/SearchQueryDTO'; -import {QueryKeywords, SearchQueryParser} from '../../../src/common/SearchQueryParser'; +import {defaultQueryKeywords, SearchQueryParser} from '../../../src/common/SearchQueryParser'; -const queryKeywords: QueryKeywords = { - NSomeOf: 'of', - and: 'and', - or: 'or', - - from: 'after', - to: 'before', - landscape: 'landscape', - maxRating: 'max-rating', - maxResolution: 'max-resolution', - minRating: 'min-rating', - minResolution: 'min-resolution', - orientation: 'orientation', - - any_text: 'any-text', - keyword: 'keyword', - caption: 'caption', - directory: 'directory', - file_name: 'file-name', - person: 'person', - portrait: 'portrait', - position: 'position', - someOf: 'some-of', - kmFrom: 'km-from' -}; describe('SearchQueryParser', () => { const check = (query: SearchQueryDTO) => { - const parser = new SearchQueryParser(queryKeywords); + const parser = new SearchQueryParser(defaultQueryKeywords); expect(parser.parse(parser.stringify(query))).to.deep.equals(query, parser.stringify(query)); }; @@ -92,14 +67,14 @@ describe('SearchQueryParser', () => { check({type: SearchQueryTypes.from_date, value: (Date.UTC(2020, 1, 1)), negate: true} as FromDateSearch); check({type: SearchQueryTypes.to_date, value: (Date.UTC(2020, 1, 1)), negate: true} as ToDateSearch); - const parser = new SearchQueryParser(queryKeywords); + const parser = new SearchQueryParser(defaultQueryKeywords); // test if date gets simplified on 1st of Jan. let query: RangeSearch = {type: SearchQueryTypes.to_date, value: (Date.UTC(2020, 0, 1))} as ToDateSearch; - expect(parser.parse(queryKeywords.to + ':' + (new Date(query.value)).getFullYear())) + expect(parser.parse(defaultQueryKeywords.to + ':' + (new Date(query.value)).getFullYear())) .to.deep.equals(query, parser.stringify(query)); query = ({type: SearchQueryTypes.from_date, value: (Date.UTC(2020, 0, 1))} as FromDateSearch); - expect(parser.parse(queryKeywords.from + ':' + (new Date(query.value)).getFullYear())) + expect(parser.parse(defaultQueryKeywords.from + ':' + (new Date(query.value)).getFullYear())) .to.deep.equals(query, parser.stringify(query)); }); @@ -125,7 +100,7 @@ describe('SearchQueryParser', () => { }); it('Default logical operator should be AND', () => { - const parser = new SearchQueryParser(queryKeywords); + const parser = new SearchQueryParser(defaultQueryKeywords); expect(parser.parse('a b')).to.deep.equals({ type: SearchQueryTypes.AND, list: [