From 6b2fdd167284c4a7b06a15865b8d640d1c1f3a5d Mon Sep 17 00:00:00 2001 From: pubkey <8926560+pubkey@users.noreply.github.com> Date: Sun, 30 Oct 2022 21:08:30 +0100 Subject: [PATCH 1/5] ADD tests for query correctness --- src/custom-index.ts | 10 +- .../memory/rx-storage-instance-memory.ts | 5 + test/unit.test.ts | 3 +- test/unit/query-correctness.test.ts | 156 ---------------- .../unit/rx-storage-query-correctness.test.ts | 173 ++++++++++++++++++ 5 files changed, 185 insertions(+), 162 deletions(-) delete mode 100644 test/unit/query-correctness.test.ts create mode 100644 test/unit/rx-storage-query-correctness.test.ts diff --git a/src/custom-index.ts b/src/custom-index.ts index 498a6281697..9b181332d7e 100644 --- a/src/custom-index.ts +++ b/src/custom-index.ts @@ -83,7 +83,7 @@ export function getIndexableStringMonad( if (!fieldValue) { fieldValue = ''; } - str += fieldValue.padStart(schemaPart.maxLength as number, ' '); + str += fieldValue.padEnd(schemaPart.maxLength as number, ' '); } else if (type === 'boolean') { const boolToStr = fieldValue ? '1' : '0'; str += boolToStr; @@ -164,9 +164,9 @@ export function getStartIndexStringFromLowerBound( case 'string': const maxLength = ensureNotFalsy(schemaPart.maxLength); if (typeof bound === 'string') { - str += (bound as string).padStart(maxLength, ' '); + str += (bound as string).padEnd(maxLength, ' '); } else { - str += ''.padStart(maxLength, ' '); + str += ''.padEnd(maxLength, ' '); } break; case 'boolean': @@ -218,9 +218,9 @@ export function getStartIndexStringFromUpperBound( case 'string': const maxLength = ensureNotFalsy(schemaPart.maxLength); if (typeof bound === 'string') { - str += (bound as string).padStart(maxLength, INDEX_MAX); + str += (bound as string).padEnd(maxLength, INDEX_MAX); } else { - str += ''.padStart(maxLength, INDEX_MAX); + str += ''.padEnd(maxLength, INDEX_MAX); } break; case 'boolean': diff --git a/src/plugins/memory/rx-storage-instance-memory.ts b/src/plugins/memory/rx-storage-instance-memory.ts index b4fd958fd35..86f21837c07 100644 --- a/src/plugins/memory/rx-storage-instance-memory.ts +++ b/src/plugins/memory/rx-storage-instance-memory.ts @@ -229,11 +229,16 @@ export class RxStorageInstanceMemory implements RxStorageInstance< compareDocsWithIndex ); + + console.log('docsWithIndex:'); + console.dir(docsWithIndex); + let rows: RxDocumentData[] = []; let done = false; while (!done) { const currentDoc = docsWithIndex[indexOfLower]; + if ( !currentDoc || currentDoc.indexString > upperBoundString diff --git a/test/unit.test.ts b/test/unit.test.ts index b53532c552c..271a28dd899 100644 --- a/test/unit.test.ts +++ b/test/unit.test.ts @@ -20,6 +20,8 @@ import './unit/query-planner.test'; * Do not commit this file if you modified the order. */ import './unit/rx-storage-implementations.test'; +import './unit/rx-storage-query-correctness.test'; + import './unit/rx-storage-pouchdb.test'; import './unit/rx-storage-lokijs.test'; import './unit/rx-storage-dexie.test'; @@ -35,7 +37,6 @@ import './unit/attachments.test'; import './unit/encryption.test'; import './unit/rx-document.test'; import './unit/rx-query.test'; -import './unit/query-correctness.test'; import './unit/primary.test'; import './unit/local-documents.test'; import './unit/change-event-buffer.test'; diff --git a/test/unit/query-correctness.test.ts b/test/unit/query-correctness.test.ts deleted file mode 100644 index 4ed8701e531..00000000000 --- a/test/unit/query-correctness.test.ts +++ /dev/null @@ -1,156 +0,0 @@ -import assert from 'assert'; - -import config from './config'; -import { - RxJsonSchema, - randomCouchString, - createRxDatabase -} from '../../'; - -config.parallel('query-correctness.test.ts', () => { - - type TestDoc = { - id: string; - age: number; - alwaysX: 'X'; - } - const schema: RxJsonSchema = { - version: 0, - primaryKey: 'id', - type: 'object', - properties: { - id: { - type: 'string', - maxLength: 100 - }, - age: { - type: 'number', - minimum: 100, - maximum: 200, - multipleOf: 1 - }, - alwaysX: { - type: 'string', - enum: ['X'], - maxLength: 3 - } - }, - required: [ - 'id', - 'alwaysX', - 'age' - ], - indexes: [ - [ - 'alwaysX', - 'age' - ] - ] - }; - - async function getTestCollection() { - const db = await createRxDatabase({ - name: randomCouchString(10), - storage: config.storage.getStorage(), - }); - await db.addCollections({ - docs: { - schema - } - }); - return db.docs; - } - - describe('$gt', () => { - it('should find correct if the $gt value is lower then the minimum or higher then the maximum', async () => { - const collection = await getTestCollection(); - - await collection.insert({ - id: 'A', - alwaysX: 'X', - age: 100 - }); - await collection.insert({ - id: 'B', - alwaysX: 'X', - age: 150 - }); - await collection.insert({ - id: 'C', - alwaysX: 'X', - age: 200 - }); - - const lowerThenMin = await collection.find({ - selector: { - age: { - $gt: 1 - } - } - }).exec(); - assert.strictEqual( - lowerThenMin.length, - 3 - ); - - const higherThenMax = await collection.find({ - selector: { - age: { - $gt: 300 - } - } - }).exec(); - assert.strictEqual( - higherThenMax.length, - 0 - ); - collection.database.destroy(); - }); - }); - describe('$lt', () => { - it('should find correct if the $lt value is lower then the minimum or higher then the maximum', async () => { - const collection = await getTestCollection(); - - await collection.insert({ - id: 'A', - alwaysX: 'X', - age: 100 - }); - await collection.insert({ - id: 'B', - alwaysX: 'X', - age: 150 - }); - await collection.insert({ - id: 'C', - alwaysX: 'X', - age: 200 - }); - - const lowerThenMin = await collection.find({ - selector: { - age: { - $lt: 1 - } - } - }).exec(); - assert.strictEqual( - lowerThenMin.length, - 0 - ); - - const higherThenMax = await collection.find({ - selector: { - age: { - $lt: 300 - } - } - }).exec(); - assert.strictEqual( - higherThenMax.length, - 3 - ); - collection.database.destroy(); - }); - }); -}); diff --git a/test/unit/rx-storage-query-correctness.test.ts b/test/unit/rx-storage-query-correctness.test.ts new file mode 100644 index 00000000000..8219d53e874 --- /dev/null +++ b/test/unit/rx-storage-query-correctness.test.ts @@ -0,0 +1,173 @@ +import assert from 'assert'; + +import config from './config'; +import { + RxJsonSchema, + randomCouchString, + MangoQuery, + fillWithDefaultSettings, + normalizeMangoQuery, + now, + getPrimaryFieldOfPrimaryKey +} from '../../'; +import { EXAMPLE_REVISION_1 } from '../helper/revisions'; +import * as schemas from '../helper/schemas'; +import { human } from '../helper/schema-objects'; + +const TEST_CONTEXT = 'rx-storage-query-correctness.test.ts'; +config.parallel('rx-storage-query-correctness.test.ts', () => { + type TestCorrectQueriesInput = { + testTitle: string; + schema: RxJsonSchema; + data: RxDocType[]; + queries: { + query: MangoQuery; + expectedResultDocIds: string[]; + }[] + }; + function testCorrectQueries( + input: TestCorrectQueriesInput + ) { + it(input.testTitle, async () => { + const schema = fillWithDefaultSettings(input.schema); + const primaryPath = getPrimaryFieldOfPrimaryKey(schema.primaryKey); + const storageInstance = await config.storage.getStorage().createStorageInstance({ + databaseInstanceToken: randomCouchString(10), + databaseName: randomCouchString(12), + collectionName: randomCouchString(12), + schema, + options: {}, + multiInstance: false + }); + + + const rawDocsData = input.data.map(row => { + const writeData = Object.assign( + {}, + row, + { + _deleted: false, + _attachments: {}, + _meta: { + lwt: now() + }, + _rev: EXAMPLE_REVISION_1 + } + ); + return writeData; + }); + await storageInstance.bulkWrite( + rawDocsData.map(document => ({ document })), + TEST_CONTEXT + ); + + + for (const queryData of input.queries) { + const normalizedQuery = normalizeMangoQuery(schema, queryData.query); + const skip = normalizedQuery.skip ? normalizedQuery.skip : 0; + const limit = normalizedQuery.limit ? normalizedQuery.limit : Infinity; + const skipPlusLimit = skip + limit; + + const preparedQuery = config.storage.getStorage().statics.prepareQuery( + schema, + normalizedQuery + ); + + // Test output of RxStorageInstance.query(); + const resultFromStorage = await storageInstance.query(preparedQuery); + const resultIds = resultFromStorage.documents.map(d => (d as any)[primaryPath]); + try { + assert.deepStrictEqual(resultIds, queryData.expectedResultDocIds); + } catch (err) { + console.log('WRONG QUERY RESULTS FROM .query():'); + console.dir(queryData); + throw err; + } + + // Test output of RxStorageStatics + const queryMatcher = config.storage.getStorage().statics.getQueryMatcher(schema, preparedQuery); + const sortComparator = config.storage.getStorage().statics.getSortComparator(schema, preparedQuery); + const staticsResult = rawDocsData.slice(0) + .filter(d => queryMatcher(d)) + .sort(sortComparator) + .slice(skip, skipPlusLimit); + const resultStaticsIds = staticsResult.map(d => (d as any)[primaryPath]); + try { + assert.deepStrictEqual(resultStaticsIds, queryData.expectedResultDocIds); + } catch (err) { + console.log('WRONG QUERY RESULTS FROM STATICS:'); + console.dir(queryData); + throw err; + } + } + + storageInstance.close(); + }); + } + + testCorrectQueries({ + testTitle: '$gt/$gte with number', + data: [ + human('aa', 10), + human('bb', 20), + /** + * One must have a longer id + * because we had many bugs around how padLeft + * works on custom indexes. + */ + human('cc-looong-id', 30), + human('dd', 40), + human('ee', 50) + ], + queries: [ + { + query: { + selector: { + age: { + $gt: 20 + } + }, + sort: [{ age: 'asc' }] + }, + expectedResultDocIds: [ + 'cc-looong-id', + 'dd', + 'ee' + ] + }, + { + query: { + selector: { + age: { + $gte: 20 + } + }, + sort: [{ age: 'asc' }] + }, + expectedResultDocIds: [ + 'bb', + 'cc-looong-id', + 'dd', + 'ee' + ] + }, + // sort by sth that is not in the selector + { + query: { + selector: { + age: { + $gt: 20 + } + }, + sort: [{ passportId: 'asc' }] + }, + expectedResultDocIds: [ + 'cc-looong-id', + 'dd', + 'ee' + ] + }, + ], + schema: schemas.human + }); +}); From 195f1ade5d73be71838bc85ef604c9dfa4b21fbd Mon Sep 17 00:00:00 2001 From: pubkey <8926560+pubkey@users.noreply.github.com> Date: Sun, 30 Oct 2022 21:19:00 +0100 Subject: [PATCH 2/5] FIX more stuff --- test/helper/schemas.ts | 3 ++- test/unit/rx-storage-query-correctness.test.ts | 15 +++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/test/helper/schemas.ts b/test/helper/schemas.ts index cedd43ac60e..943ae0ee54b 100644 --- a/test/helper/schemas.ts +++ b/test/helper/schemas.ts @@ -54,7 +54,8 @@ export const humanSchemaLiteral = overwritable.deepFreezeWhenDevMode({ description: 'age in years', type: 'integer', minimum: 0, - maximum: 150 + maximum: 150, + multipleOf: 1 } }, required: ['firstName', 'lastName', 'passportId'], diff --git a/test/unit/rx-storage-query-correctness.test.ts b/test/unit/rx-storage-query-correctness.test.ts index 8219d53e874..e9a9684bd94 100644 --- a/test/unit/rx-storage-query-correctness.test.ts +++ b/test/unit/rx-storage-query-correctness.test.ts @@ -8,7 +8,8 @@ import { fillWithDefaultSettings, normalizeMangoQuery, now, - getPrimaryFieldOfPrimaryKey + getPrimaryFieldOfPrimaryKey, + clone } from '../../'; import { EXAMPLE_REVISION_1 } from '../helper/revisions'; import * as schemas from '../helper/schemas'; @@ -25,6 +26,14 @@ config.parallel('rx-storage-query-correctness.test.ts', () => { expectedResultDocIds: string[]; }[] }; + function withIndexes( + schema: RxJsonSchema, + indexes: string[][] + ): RxJsonSchema { + schema = clone(schema); + schema.indexes = indexes; + return schema; + } function testCorrectQueries( input: TestCorrectQueriesInput ) { @@ -168,6 +177,8 @@ config.parallel('rx-storage-query-correctness.test.ts', () => { ] }, ], - schema: schemas.human + schema: withIndexes(schemas.human, [ + ['age'] + ]) }); }); From 5b814e76006a7940ba3eb4f0de21f02f15884b6e Mon Sep 17 00:00:00 2001 From: pubkey <8926560+pubkey@users.noreply.github.com> Date: Sun, 30 Oct 2022 21:21:34 +0100 Subject: [PATCH 3/5] FIX remove logs --- src/plugins/memory/rx-storage-instance-memory.ts | 4 ---- test/unit/rx-storage-query-correctness.test.ts | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/plugins/memory/rx-storage-instance-memory.ts b/src/plugins/memory/rx-storage-instance-memory.ts index 86f21837c07..a5e79d77706 100644 --- a/src/plugins/memory/rx-storage-instance-memory.ts +++ b/src/plugins/memory/rx-storage-instance-memory.ts @@ -229,10 +229,6 @@ export class RxStorageInstanceMemory implements RxStorageInstance< compareDocsWithIndex ); - - console.log('docsWithIndex:'); - console.dir(docsWithIndex); - let rows: RxDocumentData[] = []; let done = false; while (!done) { diff --git a/test/unit/rx-storage-query-correctness.test.ts b/test/unit/rx-storage-query-correctness.test.ts index e9a9684bd94..4861c37c191 100644 --- a/test/unit/rx-storage-query-correctness.test.ts +++ b/test/unit/rx-storage-query-correctness.test.ts @@ -160,7 +160,7 @@ config.parallel('rx-storage-query-correctness.test.ts', () => { 'ee' ] }, - // sort by sth that is not in the selector + // sort by something that is not in the selector { query: { selector: { From caba598525a41aa1bda458eb340f6cb5b504f1bd Mon Sep 17 00:00:00 2001 From: pubkey <8926560+pubkey@users.noreply.github.com> Date: Sun, 30 Oct 2022 21:58:48 +0100 Subject: [PATCH 4/5] ADD tests for $lt/$lte --- test/unit/config.ts | 11 ++- .../unit/rx-storage-query-correctness.test.ts | 82 ++++++++++++++++++- 2 files changed, 88 insertions(+), 5 deletions(-) diff --git a/test/unit/config.ts b/test/unit/config.ts index 22a5820cbfc..03819a4ca80 100644 --- a/test/unit/config.ts +++ b/test/unit/config.ts @@ -76,12 +76,21 @@ const config: { rootPath: string; isFastMode: () => boolean; storage: RxTestStorage; + isNotOneOfTheseStorages: (names: string[]) => boolean; } = { platform: detect(), parallel: useParallel, rootPath: '', isFastMode, - storage: {} as any + storage: {} as any, + isNotOneOfTheseStorages(storageNames: string[]) { + const isName = this.storage.name; + if (storageNames.includes(isName)) { + return false; + } else { + return true; + } + } }; const DEFAULT_STORAGE = ENV_VARIABLES.DEFAULT_STORAGE as string; diff --git a/test/unit/rx-storage-query-correctness.test.ts b/test/unit/rx-storage-query-correctness.test.ts index 4861c37c191..f8ce2f25bfb 100644 --- a/test/unit/rx-storage-query-correctness.test.ts +++ b/test/unit/rx-storage-query-correctness.test.ts @@ -21,10 +21,10 @@ config.parallel('rx-storage-query-correctness.test.ts', () => { testTitle: string; schema: RxJsonSchema; data: RxDocType[]; - queries: { + queries: ({ query: MangoQuery; expectedResultDocIds: string[]; - }[] + } | undefined)[] }; function withIndexes( schema: RxJsonSchema, @@ -72,6 +72,9 @@ config.parallel('rx-storage-query-correctness.test.ts', () => { for (const queryData of input.queries) { + if (!queryData) { + continue; + } const normalizedQuery = normalizeMangoQuery(schema, queryData.query); const skip = normalizedQuery.skip ? normalizedQuery.skip : 0; const limit = normalizedQuery.limit ? normalizedQuery.limit : Infinity; @@ -129,7 +132,11 @@ config.parallel('rx-storage-query-correctness.test.ts', () => { human('ee', 50) ], queries: [ - { + /** + * TODO using $gte in pouchdb returns the wrong results, + * create an issue at the PouchDB repo. + */ + config.isNotOneOfTheseStorages(['pouchdb']) ? { query: { selector: { age: { @@ -143,7 +150,7 @@ config.parallel('rx-storage-query-correctness.test.ts', () => { 'dd', 'ee' ] - }, + } : undefined, { query: { selector: { @@ -181,4 +188,71 @@ config.parallel('rx-storage-query-correctness.test.ts', () => { ['age'] ]) }); + testCorrectQueries({ + testTitle: '$lt/$lte with number', + data: [ + human('aa', 10), + human('bb', 20), + /** + * One must have a longer id + * because we had many bugs around how padLeft + * works on custom indexes. + */ + human('cc-looong-id', 30), + human('dd', 40), + human('ee', 50) + ], + queries: [ + { + query: { + selector: { + age: { + $lt: 40 + } + }, + sort: [{ age: 'asc' }] + }, + expectedResultDocIds: [ + 'aa', + 'bb', + 'cc-looong-id' + ] + }, + { + query: { + selector: { + age: { + $lte: 40 + } + }, + sort: [{ age: 'asc' }] + }, + expectedResultDocIds: [ + 'aa', + 'bb', + 'cc-looong-id', + 'dd' + ] + }, + // sort by something that is not in the selector + { + query: { + selector: { + age: { + $lt: 40 + } + }, + sort: [{ passportId: 'asc' }] + }, + expectedResultDocIds: [ + 'aa', + 'bb', + 'cc-looong-id' + ] + }, + ], + schema: withIndexes(schemas.human, [ + ['age'] + ]) + }); }); From a369c91ffa9fb113752a35c9f7ebda40cdab91d8 Mon Sep 17 00:00:00 2001 From: pubkey <8926560+pubkey@users.noreply.github.com> Date: Sun, 30 Oct 2022 22:19:09 +0100 Subject: [PATCH 5/5] ADD changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e60e5199fb6..31b305b9a41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ # RxDB Changelog +- FIX critical bug in query correctness. **IMPORTANT:** If you use the RxStorage [IndexedDB](https://rxdb.info/rx-storage-indexeddb.html) or [FoundationDB](https://rxdb.info/rx-storage-foundationdb.html), you have to rebuild the indexes by increasing your schema version and running a migration. [#4098](https://github.com/pubkey/rxdb/pull/4098) - FIX Typo in CRDT Plugin: `RxDDcrdtPlugin` is now `RxDBcrdtPlugin` [#4094](https://github.com/pubkey/rxdb/pull/4094) Thanks [@jwallet](https://github.com/jwallet)