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) 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..a5e79d77706 100644 --- a/src/plugins/memory/rx-storage-instance-memory.ts +++ b/src/plugins/memory/rx-storage-instance-memory.ts @@ -234,6 +234,7 @@ export class RxStorageInstanceMemory implements RxStorageInstance< while (!done) { const currentDoc = docsWithIndex[indexOfLower]; + if ( !currentDoc || currentDoc.indexString > upperBoundString 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.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/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/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..f8ce2f25bfb --- /dev/null +++ b/test/unit/rx-storage-query-correctness.test.ts @@ -0,0 +1,258 @@ +import assert from 'assert'; + +import config from './config'; +import { + RxJsonSchema, + randomCouchString, + MangoQuery, + fillWithDefaultSettings, + normalizeMangoQuery, + now, + getPrimaryFieldOfPrimaryKey, + clone +} 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[]; + } | undefined)[] + }; + function withIndexes( + schema: RxJsonSchema, + indexes: string[][] + ): RxJsonSchema { + schema = clone(schema); + schema.indexes = indexes; + return schema; + } + 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) { + if (!queryData) { + continue; + } + 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: [ + /** + * TODO using $gte in pouchdb returns the wrong results, + * create an issue at the PouchDB repo. + */ + config.isNotOneOfTheseStorages(['pouchdb']) ? { + query: { + selector: { + age: { + $gt: 20 + } + }, + sort: [{ age: 'asc' }] + }, + expectedResultDocIds: [ + 'cc-looong-id', + 'dd', + 'ee' + ] + } : undefined, + { + query: { + selector: { + age: { + $gte: 20 + } + }, + sort: [{ age: 'asc' }] + }, + expectedResultDocIds: [ + 'bb', + 'cc-looong-id', + 'dd', + 'ee' + ] + }, + // sort by something that is not in the selector + { + query: { + selector: { + age: { + $gt: 20 + } + }, + sort: [{ passportId: 'asc' }] + }, + expectedResultDocIds: [ + 'cc-looong-id', + 'dd', + 'ee' + ] + }, + ], + schema: withIndexes(schemas.human, [ + ['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'] + ]) + }); +});