Irrelon / ForerunnerDB

A JavaScript database with mongo-like query language, data-binding support, runs in browsers and hybrid mobile apps as a client-side DB or on the server via Node.js!
http://www.irrelon.com
721 stars 72 forks source link

upsert that resolves to update does not update fields that start with '$' #235

Closed urossmerdelj closed 6 years ago

urossmerdelj commented 6 years ago

We have an issue of updating entities that use field names that start with '$', for instance:

{
    _id: 1, 
    name: 'test2', 
    $type: 'type1', 
    parent: {
        name: 'parent2', 
        $type: 'type2'
    }
}

Updating this kind of object with upsert operation that results in update operation then results in transforming whatever is in the $type field to an array like associative properties, while the original property not being updated at all.

Issue seems to be in Collection.prototype.updateObject method's switch statement that defaults to transforming this property.

Using the update operation directly does not result in a malformed object.

image

Irrelon commented 6 years ago

Hey ya, while I'm not in a position to write a unit test to reproduce this issue immediately, the first thing that comes to mind is that ForerunnerDB treats fields starting with a dollar sign differently as it assumes they are operators like $cast, $replace etc.

Is it possible to avoid using field names starting with a dollar? The main reason to do this is that if for some reason in the future we decided to create a new operator called for instance "$type", it would break any queries you are running in your code that rely on $type NOT being a defined operator at present (I guess this scenario assumes you would update ForerunnerDB to the latest version of course).

If you absolutely cannot change the name of your field, I will attempt to reproduce this error in a unit test later this evening and see what the cause is, unless you are in a position to fork the repo, create a new branch called "bug_235" and create a unit test for this scenario and then push the code to a PR for me? Creating unit tests is pretty easy, you can copy an existing one in the testsCollection.js file if you a feeling up for helping with the debug :)

urossmerdelj commented 6 years ago

Hi, I'll see if I can get our BE team to change the field name, but for now I needed to fork the project so it supports the '$type' as field name.

Issue is easily reproducible, I'll try to get around to writing the unit test today/tomorrow when time permits.

Irrelon commented 6 years ago

@Ulkuurz I have confirmed, built a test and written a fix. Rolling out to dev and master branches now...

Irrelon commented 6 years ago

OK, that is now fixed in the latest version available on master. :)

urossmerdelj commented 6 years ago

Thanks, can confirm it works, much appreciated! :)