Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] collection.findByIds$ behaves weirdly #3659

Closed
Hideman85 opened this issue Jan 31, 2022 · 7 comments
Closed

[Bug] collection.findByIds$ behaves weirdly #3659

Hideman85 opened this issue Jan 31, 2022 · 7 comments

Comments

@Hideman85
Copy link
Contributor

Tested today on master and it does the same weird behavior than on my app.

  • The subscription is triggered so much time
  • Sometime the map does not have the objects even if the store have it for sure + funny thing executing findByIds(...) will get the objects
  • The subscription is trigerred even if the update do not concerns the list of ids looking for
I made a repro out of bug-report test unit
/**
 * this is a template for a test.
 * If you found a bug, edit this test to reproduce it
 * and than make a pull-request with that failing test.
 * The maintainer will later move your test to the correct possition in the test-suite.
 *
 * To run this test do:
 * - 'npm run test:node' so it runs in nodejs
 * - 'npm run test:browser' so it runs in the browser
 */
import assert from 'assert';
import AsyncTestUtil from 'async-test-util';
import config from './config';

import {
    createRxDatabase,
    randomCouchString
} from '../../plugins/core';

import {
    getRxStoragePouch,
} from '../../plugins/pouchdb';
import {
    RxReplicationStateBase,
} from '../../plugins/replication';


describe('bug-report.test.js', () => {
    it('should fail because it reproduces the bug', async () => {

        /**
         * If your test should only run in nodejs or only run in the browser,
         * you should comment in the return operator and addapt the if statement.
         */
        if (
            config.platform.isNode() // runs only in node
            // config.platform.isNode() // runs only in the browser
        ) {
            return;
        }

        // create a schema
        const mySchema = {
            version: 0,
            primaryKey: 'passportId',
            type: 'object',
            properties: {
                passportId: {
                    type: 'string'
                },
                firstName: {
                    type: 'string'
                },
                lastName: {
                    type: 'string'
                },
                age: {
                    type: 'integer',
                    minimum: 0,
                    maximum: 150
                }
            }
        };

        // generate a random database-name
        const name = randomCouchString(10);

        // create a database
        const db = await createRxDatabase({
            name,
            storage: getRxStoragePouch('memory'),
            eventReduce: true,
            ignoreDuplicate: true
        });
        // create a collection
        const collections = await db.addCollections({
            mycollection: {
                schema: mySchema
            }
        });


        //  Simple helper to create data
        const createObject = id => ({
            passportId: id,
            firstName: id,
            lastName: id,
            age: 56
        })



        //  Record subscription
        const updates = []
        const errors = []

        const sub = collections.mycollection.findByIds$([
            'a', 'b', 'c', 'd'
        ]).subscribe({
            next: data => {
                console.info('Got update', data)
                updates.push(data)
            },
            error: err => {
                console.error('Got error', err)
                errors.push(err)
            }
        })

        //  The subscription should return a map
        await AsyncTestUtil.waitUntil(() => updates.length > 0 || errors.length > 0);


        //  test we have a map and no error
        assert.strictEqual(updates.length, 1);
        assert.strictEqual(errors.length, 0);

        //  I found out in my app it returns a map of keys => undefined but in this test the map is empty
        // assert.strictEqual(updates[0].size, 4);

        //  Our object should not be known
        assert.strictEqual(updates[0].get('a'), undefined);
        assert.strictEqual(updates[0].get('b'), undefined);
        assert.strictEqual(updates[0].get('c'), undefined);
        assert.strictEqual(updates[0].get('d'), undefined);



        //  Simulate a primitive replication
        const replication = new RxReplicationStateBase('__id__', collections.mycollection)
        await replication.handleDocumentsFromRemote([
            createObject('a'),
            createObject('b'),
            createObject('c'),
            createObject('d'),
        ])


        //  Now we should have 2 updates or 1 error
        await AsyncTestUtil.waitUntil(() => updates.length > 1 || errors.length > 0);


        //  Verify we do have strictly two updates and no error
        assert.strictEqual(updates.length, 2);
        assert.strictEqual(errors.length, 0);

        //  The map should be of size 4
        assert.strictEqual(updates[1].size, 4);

        //  And contains the right data
        assert.strictEqual(updates[1].get('a')?.passportId, 'a');
        assert.strictEqual(updates[1].get('b')?.passportId, 'b');
        assert.strictEqual(updates[1].get('c')?.passportId, 'c');
        assert.strictEqual(updates[1].get('d')?.passportId, 'd');


        //  Let's try to update something different that should be ignored
        await replication.handleDocumentsFromRemote([
            createObject('e'),
            createObject('f'),
            createObject('g'),
            createObject('h'),
        ])

        //  Wait a bit to see if we catch anything
        await new Promise(resolve => setTimeout(resolve, 500))


        //  Verify that the subscription has not been triggered and no error has been added
        assert.strictEqual(updates.length, 2);
        assert.strictEqual(errors.length, 0);


        // clean up afterwards
        sub.unsubscribe();
        db.destroy();
    });
});

Note: I cannot run test with nodejs (performance not found) but it works nice in browser (chrome & firefox)

Please have a look and let me know if I done something wrong or this is the expected behavior.

@pubkey
Copy link
Owner

pubkey commented Jan 31, 2022

Please make a PR with the test, so that the CI runs.

@Hideman85
Copy link
Contributor Author

PR added the test-browser-couch is failing like locally

@pubkey
Copy link
Owner

pubkey commented Feb 1, 2022

Thank you for reporting this.
I adapted your test a bit and applied a fix that fixes multiple problems.

@Hideman85
Copy link
Contributor Author

Oh that is a great news, will check this immediately 🙂

@Hideman85
Copy link
Contributor Author

Hideman85 commented Feb 1, 2022

Looks awesome 👍

@Hideman85
Copy link
Contributor Author

Hi, do you have a release date of the fix? Would be so much appreciated 🙂

@pubkey
Copy link
Owner

pubkey commented Feb 10, 2022

Released already. Closing this. Reopen if still not working.

@pubkey pubkey closed this as completed Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants