pubkey / rxdb

A fast, local first, reactive Database for JavaScript Applications https://rxdb.info/
https://rxdb.info/
Apache License 2.0
21k stars 1.02k forks source link

Incorrect data returned in find functions #66

Closed S4RD7R closed 7 years ago

S4RD7R commented 7 years ago

There appears to be an issue with the data being returned in the Find. What is returned does not match what comes out of the PouchDB Find functions.

It would appear that the issue stems from RxCollection _createDocument

My issue is this. I have the following document in PouchDB

{ _id: 'abc', _rev:'1' }

Call RxDocument.remove() and the data would become this. { _id: 'abc', _rev:'1', _deleted: true}

Now add a new document with the same Primary { _id: 'abc', _rev:'2'}

Run a Find query and you will see that the data will probably as in my case have this entry { _id: 'abc', _rev:'1', _deleted: true}

Which is not what the PouchDB Find returned.

As I said I think the issue stems from RxCollection _createDocument

In here _rev 1 is stored in _docCache for Primary abc When the code checks the cache for the primary from the PouchDB find it matches and returns the entry in _docCache being _rev 1 and not _rev 2 which was returned from PouchDB.

I think there needs to be a check on the _rev key and not just the Primary.

Perhaps something like this is enough. if (cacheDoc && cacheDoc._rev == json["_rev"]) return cacheDoc;

Does that make any sense?

pubkey commented 7 years ago

Hi @MMJM Can you post some code the reproduce the expected behavior?

S4RD7R commented 7 years ago

Here is a sample bit of code

The first FindOne should return testValue = 0 The second FindOne should return testValue = 1

You will notice the _rev value output is the same which is not correct. I deleted the original rev and added a new entry.

As I mentioned above I think the issue stems from RxCollection _createDocument

A modification in that function to this seems to return what I want.

if (cacheDoc && cacheDoc._rev == json["_rev"]) return cacheDoc;

As I don't know the side effects of doing this on your _docCache you would know if this is enough.

`

const bugSchema = { "$schema": "http://json-schema.org/draft-04/schema#", "version": 0, "disableKeyCompression": true, "type": "object", "properties": { "testKey": { "type": "string", "primary": true }, "testValue": { "type": "number"
} } }

var bugDB;
_RxDB_.create({
  name: _os_.homedir() + '/Temp/bugdb', 
  adapter: 'leveldb', 
  password: 'b4503b21-b49e-435d-aba1-a17a16b36f93', 
  multiInstance: true})
.then( (db) => {
  bugDB = db;
  bugDB.collection({ 
    name: 'bug', 
    schema: bugSchema
  })
  .then( () => {
    return bugDB.collections.bug.upsert({
      testKey: 'abc',
      testValue: 0
    })
  })
  .then( () => {
    return bugDB.collections.bug.findOne({testKey: {$eq: 'abc'}})
      .exec()
      .then( (document) => {
        console.debug(document._rev)
        console.debug(document.testValue)
        document.remove();
      });
  })
  .then( () => {
    return bugDB.collections.bug.upsert({
      testKey: 'abc',
      testValue: 1
    })
  })
  .then( () => {      
    return bugDB.collections.bug.findOne({testKey: {$eq: 'abc'}})
      .exec()
      .then( (document) => {
        console.debug(document._rev)
        console.debug(document.testValue)
      });
  })
});

`

pubkey commented 7 years ago

Hi @MMJM Adding the _rev to the key of DocCache has wrong side-effects. I pushed 1f0d0c9140b3065e0befa3adcd396f4d53b3b8f4 can you check if this test describes the problem?

S4RD7R commented 7 years ago

Hi @pubkey

That looks like it describes the problem to me.

A lot tidier than my code :( I really need to get on to the Await stuff

However the same thing can happen if you use INSERT instead of UPSERT for the second document revision.

It would seem that just tidying up in the upsert will not cover all cases hence my original suggestion of the cleanup in the _createDocument function.

S4RD7R commented 7 years ago

I've not looked into why but using the original rxdb code and running upsert then upsert without the delete does return the correct revision

pubkey commented 7 years ago

I also added a test with insert -> remove -> insert

I published version 3.0.1 to npm. Please try it out and close this if fixed

S4RD7R commented 7 years ago

@pubkey Looks good.

Many thanks.