search

1602 / jugglingdb

Multi-database ORM for nodejs: redis, mongodb, mysql, sqlite3, postgresql, arango, in-memory...
http://1602.github.io/jugglingdb/
2.04k stars 241 forks source link

Fixed bad update of record in the memory on save() #364

Closed ierceg closed 3 years ago

ierceg commented 10 years ago

The invocation of inst._adapter().save passes a data object that is immediately discarded so even if the adapter changes the data object during the saving (e.g. sets a new revision) it has no effect in inst._initProperties(data, false); as that data object was never changed (nor could it be as adapter never had access to it).

I fixed this problem by keeping a reference to the adapted data object and then using that object to update inst.

1602 commented 10 years ago

Thanks for implementing this. If you have some more time, could you please add test which will modify object in adapter.save() and ensure that actual changes present in resulting object. We need it to prevent regression in future.

ierceg commented 10 years ago

No problem. I'll add the tests when I get some time.

On Fri, Jan 24, 2014 at 7:01 AM, Anatoliy Chakkaev <notifications@github.com

wrote:

Thanks for implementing this. If you have some more time, could you please add test which will modify object in adapter.save() and ensure that actual changes present in resulting object. We need it to prevent regression in future.

— Reply to this email directly or view it on GitHubhttps://github.com/1602/jugglingdb/pull/364#issuecomment-33210541 .

anatoliychakkaev commented 10 years ago

Thank you, just ping me when you push, because github doesn't send notification on new commits to existing PR.

On Fri, Jan 24, 2014 at 10:04 AM, Ivan Erceg notifications@github.comwrote:

No problem. I'll add the tests when I get some time.

On Fri, Jan 24, 2014 at 7:01 AM, Anatoliy Chakkaev < notifications@github.com

wrote:

Thanks for implementing this. If you have some more time, could you please add test which will modify object in adapter.save() and ensure that actual changes present in resulting object. We need it to prevent regression in future.

— Reply to this email directly or view it on GitHub< https://github.com/1602/jugglingdb/pull/364#issuecomment-33210541> .

— Reply to this email directly or view it on GitHubhttps://github.com/1602/jugglingdb/pull/364#issuecomment-33210673 .