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

Iterating an index fails if entries are added in the wrong order #20

Closed
glasserc opened this issue May 4, 2017 · 4 comments
Closed

Comments

@glasserc
Copy link

glasserc commented May 4, 2017

Hi! Thanks for fakeIndexedDB, we're using it in kinto.js for testing and verifying that everything works the same way as it would in a browser.

I'm investigating Kinto/kinto.js#690, which demonstrates our code failing with fakeIndexedDB 2.0.2. Previously we were using fakeIndexedDB 1.0.12 and everything seemed to work OK.

I boiled down this minimal example that demonstrates the problem:

"use strict";

import fakeIndexedDB from "fake-indexeddb";
import { expect } from "chai";

describe("FakeIndexedDB", () => {
    let db;
    beforeEach((done) => {
        const request = fakeIndexedDB.open("test" + Math.random());
        request.onupgradeneeded = (e) => {
            const db2 = e.target.result;
            const collStore = db2.createObjectStore("store", {keyPath: "id"});

            // Primary key (generated by IdSchema, UUID by default)
            collStore.createIndex("id", "id", { unique: true });
            // Local record status ("synced", "created", "updated", "deleted")
            collStore.createIndex("_status", "_status", {unique: false});

            collStore.add({id: "5", _status: "created"});
            collStore.add({id: "0", _status: "created"});
        };
        request.onsuccess = (e) => {
            db = e.target.result;
            done();
        };
        request.onerror = (e) => {
            done(e.target.error);
        };
    });

    it("should be awesome", (done) => {
        const results = [];
        const finish = () => {
            expect(results).lengthOf(2);
            done();
        };

        const txn = db.transaction(["store"]);
        const store = txn.objectStore("store");
        const request = store.index("_status").openCursor();
        request.onsuccess = (event) => {
            const cursor = event.target.result;
            if (!cursor) {
                finish();
                return;
            }
            const { key, value } = cursor;

            if (key > "created") {
                finish();
                return;
            }
            if (key === "created") {
                results.push(value);
                cursor.continue();
            }
            if (key < "created") {
                cursor.continue("created");
            }
        };
        request.onerror = (e) => {
            done(e.target.error);
        };
    });
});

I can construct a complete git repo with this code (and the necessary package.json stuff) if that would make it easier to try out. The behavior is that only the first record ({id: "5", _status: "created"}) is produced by the cursor. The second ({id: "0", _status: "created"}) never gets produced. Changing the second record's ID to "6" or something greater than the first one causes the example to succeed.

I'm not really an expert with IndexedDB so I wasn't sure whether the problem is in my example or in the library. I tried tracing through the fakeIndexedDB code and it seems like the second record gets added later in the index than the first one, but it seems like _iterate method skips the second record because it compares as less than the first one. Does that seem right to you?

Thanks for any help!

@dumbmatter
Copy link
Owner

That sounds very likely to be a bug, thanks for reporting. The cursor iteration code is a bit hairy, and I made it even harrier in 2.0 to support key cursors.

I'm on vacation now, hopefully I will fix it next week :)

@dumbmatter
Copy link
Owner

I am a man of my word! v2.0.3 has the fix (and hopefully not any new bugs created by the fix). Thanks again for the bug report, and sorry for the bug in the first place!

@glasserc
Copy link
Author

Thanks! I'll try it tomorrow!

@glasserc
Copy link
Author

(Never mind.. I see from Kinto/kinto.js#695 that everything works OK already! Thanks for the quick turnaround!)

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