Updates aren't retained after app restart

[mingard]

When using JSONBlockStore on the server, with clear=false, activate=true

Steps to reproduce:

• Launch app
• Insert document
• Update document
• Observe that document is updated (1)
• Relaunch app
• Get document (looks fine)
• Update document
• Observe that document is updated (2)
• Relaunch app
• Get document
• Observe that document has reverted to the last version before the first App relaunch

[mingard]

@anywhichway I can look at providing an example app build if that helps

[anywhichway]

Researching...

[anywhichway]

@mingard an example would certainly speed things-up. Would you like us to add a directory in master called issues/13? You could then create a repo and throw a pull request at us.

[mingard]

@anywhichway perfect - I'll start work on it now

[anywhichway]

@mingard we created a sample app in reasondb/test/issues/13/index.js and are unable to duplicate your issue. Please review the app to confirm it is able to accurately re-produce your steps. We will leave this issue open for a few days to give you an opportunity to respond.

[mingard]

@anywhichway thanks - I'll try and replicate it in-branch

[mingard]

@anywhichway I can't see the branch

[anywhichway]

@mingard Here is a direct link: https://github.com/anywhichway/reasondb/tree/master/test/issues/13

[mingard]

@anywhichway still working on this. Had some time off for Christmas

[anywhichway]

No problem. Happy New Year!

…

On Fri, Dec 30, 2016 at 5:11 AM, Arthur Mingard ***@***.***> wrote: @anywhichway <https://github.com/anywhichway> still working on this. Had some time off for Christmas — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#13 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEuqSz6od6CUhTzrgAmuUl9qzS5VZmxhks5rNNjUgaJpZM4LRAxY> .

[mingard]

@anywhichway thanks, you too!

What version of Node/NPM are you running? I'm assuming higher than 4.7 as currently I strict mode errors where "use strict" hasn't been defined, like src/index.js

[mingard]

Once I upgrade to 6.x I get the following error:

const asynchronize = async (value) => {
	                           ^
SyntaxError: Unexpected token (

[anywhichway]

Try turning harmony options on on Node or run our code through Babel. If that does not work, you will need to go to 7. Sorry, we will try to clarify the docs. Note, we have had previous breakage issues with Babel.

…

On Jan 3, 2017 8:34 AM, "Arthur Mingard" ***@***.***> wrote: Once I upgrade to 6.x I get the following error: const asynchronize = async (value) => { ^SyntaxError: Unexpected token ( — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#13 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEuqSwg4dNb8Yx3reB4cw0ZG5_ZpONgIks5rOk5egaJpZM4LRAxY> .

[mingard]

@anywhichway Okay, I've done the test a slightly different way. I need to be able to commit to a branch. You okay to create one, perhaps /issues/13?

[anywhichway]

@mingard I have created the branch issues/13

[mingard]

@anywhichway I am unable to commit to that branch

[anywhichway]

Can you clone it and submit a pull request?

[mingard]

@anywhichway see #14. I forked and opened a PR

[mingard]

@anywhichway any luck reproducing?

[anywhichway]

We are digging into why the select for Jack in script three returns "undefined". However, it does look like there is an error in the test, you explicitly change firstName to James below but then say you expect Jack to be returned.

 this.get(thirdInsert).then((docs) => {
    console.log(`We expect a result, as the Second script changed the name to "Jack", but we get ${docs[0]}`)
    console.log(`We revert back to "James" and try again`)
    thirdInsert.firstname = "James"
    this.get(thirdInsert).then((docs) => {
      console.log(`On the sixth GET, the returned document is ${docs[0].firstname} ${docs[0].lastname}. It should be Jack Smith`)
    })
  })

[mingard]

@anywhichway That's right, it should actually read James.

It seems that changes made during the same session as the document was created are stored, but those made in a subsequent process are stored in memory, but not persisted. Even a fetch during a second process returns the correct result suggesting that it has to be memory resident.

[anywhichway]

@mingard you are correct, We reached the same conclusion as you about the time of your post. We think the process may be ending prior to the data being written to disk. The manual termination we were using with the example of sequential runs that worked may be causing the write as a side effect. BTW, we duplicated the issue using ReasonDB.LocalStore which is a heck of a lot easier to debug than block storage.

[mingard]

@anywhichway That makes sense. I was using LocalStore originally but switched because it wasn't resolving referenced sub-documents after relaunch. I should probably raise a ticket for that too

[anywhichway]

@mingard we have isolated the issue. ReasonDB caches constructors as it becomes aware of them. If it can't find a constructor it defaults to restoring data to plain Objects. In round two of and three of your test no inserts are occurring prior to selecting and no constructor has been cached. Your updates were actually getting written to disk but under the Object index not the Person index. We think this will be an easy fix, we just need to cache any constructors referenced by select, insert, update and delete prior to executing them. Currently the caching has been occurring lower in the call stack in case people are not using insert, select, etc.

[anywhichway]

@mingard We think we have a fix. It is going to require some regression and we know we will have to make changes in a couple of spots; however, this seems to fix the situation for your example.

Modify the constructor for Index to be this:

constructor(cls,keyProperty="@key",db,StorageType=(db ? db.storageType : MemStore),clear=(db ? db.clear : false)) {
			const me = this;
			cls.index = me;
			me.saveAsync = db.saveIndexAsync;
			me.keys = {};
			me.store = new StorageType(cls.name,keyProperty,db,clear);
			me.store.scope[cls.name] = cls; // <--- this is the change
			me.name = cls.name;
			me.pending = {};
		}

Let us know if you confirm the fix. We may also be able to fix the issue with you having to manually create folders with the final patch.

[mingard]

@anywhichway that did it! Fix confirmed.

Am I right in thinking that if I'd inserted a document into Person on each process launch, it would've worked?

[anywhichway]

Yes, it should have ... Although we have uncovered an issue with our regression tests, too many use just Object rather than custom classes. We will push a new version for now and then add more tests.

Thanks for finding the bug! ... and confirming the fix.