ADD test for #4775 (#4825)

* ADD test for #4775 * FIX count() is incorrect * FIX cleanup code * FIX foundationdb query
pubkey · Jul 17, 2023 · d03008d · d03008d
1 parent c2c855f
commit d03008d
Show file tree

Hide file tree

Showing 9 changed files with 115 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,7 +2,7 @@
 # RxDB Changelog
 
 <!-- CHANGELOG NEWEST -->
-
+- FIX count() is incorrect [#4755](https://github.com/pubkey/rxdb/issues/4755)
 <!-- ADD new changes here! -->
 
 <!-- /CHANGELOG NEWEST -->

diff --git a/src/custom-index.ts b/src/custom-index.ts
@@ -314,8 +314,8 @@ export function getStartIndexStringFromUpperBound(
         switch (type) {
             case 'string':
                 const maxLength = ensureNotFalsy(schemaPart.maxLength);
-                if (typeof bound === 'string') {
-                    str += (bound as string).padEnd(maxLength, inclusiveEnd ? INDEX_MAX : ' ');
+                if (typeof bound === 'string' && bound !== INDEX_MAX) {
+                    str += (bound as string).padEnd(maxLength, ' ');
                 } else {
                     str += ''.padEnd(maxLength, inclusiveEnd ? INDEX_MAX : ' ');
                 }

diff --git a/src/plugins/storage-foundationdb/foundationdb-query.ts b/src/plugins/storage-foundationdb/foundationdb-query.ts
@@ -66,9 +66,31 @@ export async function queryFoundationDB<RxDocType>(
         const indexTx = tx.at(indexDB.subspace);
         const mainTx = tx.at(dbs.main.subspace);
 
+
+        /**
+         * TODO for whatever reason the keySelectors like firstGreaterThan etc.
+         * do not work properly. So we have to hack here to find the correct
+         * document in case lowerBoundString===upperBoundString.
+         * This likely must be fixed in the foundationdb library.
+         * When it is fixed, we do not need this if-case and instead
+         * can rely on .getRangeBatch() in all cases.
+         */
+        if (lowerBoundString === upperBoundString) {
+            const docId: string = await indexTx.get(lowerBoundString);
+            if (docId) {
+                const docData = await mainTx.get(docId);
+                if (!queryMatcher || queryMatcher(docData)) {
+                    innerResult.push(docData);
+                }
+            }
+            return innerResult;
+        }
+
         const range = indexTx.getRangeBatch(
             lowerBoundString,
             upperBoundString,
+            // queryPlan.inclusiveStart ? keySelector.firstGreaterThan(lowerBoundString) : keySelector.firstGreaterOrEqual(lowerBoundString),
+            // queryPlan.inclusiveEnd ? keySelector.lastLessOrEqual(upperBoundString) : keySelector.lastLessThan(upperBoundString),
             {
                 // TODO these options seem to be broken in the foundationdb node bindings
                 // limit: instance.settings.batchSize,
@@ -84,6 +106,7 @@ export async function queryFoundationDB<RxDocType>(
             }
             const docIds = next.value.map((row: string[]) => row[1]);
             const docsData: RxDocumentData<RxDocType>[] = await Promise.all(docIds.map((docId: string) => mainTx.get(docId)));
+
             docsData.forEach((docData) => {
                 if (!done) {
                     if (!queryMatcher || queryMatcher(docData)) {

diff --git a/src/plugins/storage-memory/rx-storage-instance-memory.ts b/src/plugins/storage-memory/rx-storage-instance-memory.ts
@@ -239,6 +239,7 @@ export class RxStorageInstanceMemory<RxDocType> implements RxStorageInstance<
         );
         const indexName = getMemoryIndexName(index);
         const docsWithIndex = this.internals.byIndex[indexName].docsWithIndex;
+
         let indexOfLower = boundGE(
             docsWithIndex,
             {
@@ -258,8 +259,6 @@ export class RxStorageInstanceMemory<RxDocType> implements RxStorageInstance<
         let done = false;
         while (!done) {
             const currentDoc = docsWithIndex[indexOfLower];
-
-
             if (
                 !currentDoc ||
                 indexOfLower > indexOfUpper

diff --git a/test/unit/config.ts b/test/unit/config.ts
@@ -211,7 +211,7 @@ export function setDefaultStorage(storageKey: string) {
             };
             break;
         case 'foundationdb':
-            const foundationDBAPIVersion = 620;
+            const foundationDBAPIVersion = 630;
 
             // use a dynamic import so it does not break browser bundling
             const { getRxStorageFoundationDB } = require('../../plugins/storage-foundationdb' + '');

diff --git a/test/unit/rx-collection.test.ts b/test/unit/rx-collection.test.ts
@@ -1261,6 +1261,76 @@ describe('rx-collection.test.ts', () => {
                     c.database.destroy();
                 });
             });
+            /**
+             * @link https://github.com/pubkey/rxdb/pull/4775
+             */
+            it('#4775 count() broken on primary key', async () => {
+                // create a schema
+                const mySchema = {
+                    version: 0,
+                    primaryKey: 'passportId',
+                    type: 'object',
+                    properties: {
+                        passportId: {
+                            type: 'string',
+                            maxLength: 30
+                        },
+                        firstName: {
+                            type: 'string'
+                        },
+                        lastName: {
+                            type: 'string'
+                        },
+                        age: {
+                            type: 'integer',
+                            minimum: 0,
+                            maximum: 150
+                        }
+                    }
+                };
+
+                const name = randomCouchString(10);
+                const db = await createRxDatabase({
+                    name,
+                    storage: config.storage.getStorage(),
+                    eventReduce: true,
+                    ignoreDuplicate: true
+                });
+                const collections = await db.addCollections({
+                    mycollection: {
+                        schema: mySchema
+                    }
+                });
+
+                // insert a document
+                await collections.mycollection.insert({
+                    passportId: 'foobar2',
+                    firstName: 'Bob',
+                    lastName: 'Kelso',
+                    age: 56
+                });
+                await collections.mycollection.insert({
+                    passportId: 'foobar3',
+                    firstName: 'Bob',
+                    lastName: 'Kelso',
+                    age: 56
+                });
+
+                const countResult = await collections.mycollection
+                    .count({
+                        selector: {
+                            passportId: {
+                                $eq: 'foobar'
+                            }
+                        }
+                    })
+                    .exec();
+
+                assert.strictEqual(countResult, 0);
+
+                // clean up afterwards
+                db.destroy();
+            });
         });
         config.parallel('.bulkUpsert()', () => {
             it('insert and update', async () => {

diff --git a/test/unit/rx-query.test.ts b/test/unit/rx-query.test.ts
@@ -904,7 +904,6 @@ describe('rx-query.test.ts', () => {
             });
 
 
-            /* eslint-disable */
             const selector = {
                 $and: [{
                     event_id: {
@@ -931,7 +930,6 @@ describe('rx-query.test.ts', () => {
                 }
                 ]
             };
-            /* eslint-enable */
 
             const resultDocs1 = await collection
                 .find({

diff --git a/test/unit/rx-storage-implementations.test.ts b/test/unit/rx-storage-implementations.test.ts
@@ -1493,6 +1493,7 @@ config.parallel('rx-storage-implementations.test.ts (implementation: ' + config.
                 );
                 const allDocs = await storageInstance.query(preparedQuery);
 
+                assert.strictEqual(allDocs.documents.length, 3);
                 assert.strictEqual(allDocs.documents[0].value, 'c');
                 assert.strictEqual(allDocs.documents[1].value, 'b');
                 assert.strictEqual(allDocs.documents[2].value, 'a');

diff --git a/test/unit/rx-storage-query-correctness.test.ts b/test/unit/rx-storage-query-correctness.test.ts
@@ -705,13 +705,29 @@ config.parallel('rx-storage-query-correctness.test.ts', () => {
     testCorrectQueries({
         testTitle: '$eq operator',
         data: [
+            {
+                id: 'zero',
+                nonPrimaryString: 'zero',
+                integer: 0,
+                number: 0,
+                boolean: false,
+                null: 'not-null'
+            },
             {
                 id: 'one',
                 nonPrimaryString: 'one',
                 integer: 1,
                 number: 1,
                 boolean: true,
                 null: null
+            },
+            {
+                id: 'two',
+                nonPrimaryString: 'two',
+                integer: 2,
+                number: 2,
+                boolean: false,
+                null: 'not-null'
             }
         ],
         schema: {