Object column not updating properly

[jaeggerr]

Issue Description

Columns of type Object (JSON object) from Parse classes are not updating properly.
New and previous values are merged.

Steps to reproduce

I'm using the Parse dashboard but this also happens in code.

1. Create a Parse class with a field of type Object.
2. Add a new row
3. Set any JSON value in the field.
   { "a": "b" }
4. Replace the value by an empty JSON object
   {}
5. Replace the value by a JSON object with other key / values.
   { "c": "d" }

Expected Results

Object field should be replaced by the provided values.

4. New value should be {}
5. New value should be { "c": "d" }

Actual Outcome

4. Value will still be { "a": "b" }.
5. Old and new values will be merged. {"a": "b", "c": "d" }

Environment Setup

• Server

  • parse-server version (Be specific! Don't say 'latest'.) : 2.8.2
  • Operating System: Mac OS 10.13.5
  • Hardware: Mac Mini
  • Localhost or remote server? (AWS, Heroku, Azure, Digital Ocean, etc): Localhost

• Database

  • Postgres version: 10.3
  • Hardware: Mac Mini
  • Localhost or remote server? (AWS, mLab, ObjectRocket, Digital Ocean, etc): Localhost

Logs/Trace

verbose: REQUEST for [PUT] /parse/classes/MyClass/zHsUsMqd8t: {
"te": {
"a": "b"
}
} method=PUT, url=/parse/classes/MyClass/zHsUsMqd8t, host=localhost:1337, user-agent=Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:60.0) Gecko/20100101 Firefox/60.0, accept=/, accept-language=fr,fr-FR;q=0.8,en-US;q=0.5,en;q=0.3, accept-encoding=gzip, deflate, referer=http://0.0.0.0:4040/apps/TestApp%20Server/browser/MyClass, content-type=text/plain, content-length=164, origin=http://0.0.0.0:4040, connection=keep-alive, client-ip=::1, a=b
debug: PG: findOneAndUpdate MyClass { objectId: 'zHsUsMqd8t' } a=b, updatedAt=2018-06-05T15:57:32.856Z
debug: PG: updateObjectsByQuery MyClass { objectId: 'zHsUsMqd8t' } a=b, updatedAt=2018-06-05T15:57:32.856Z
debug: PG: update: UPDATE $1:name SET $2:name = ( COALESCE($2:name, '{}'::jsonb) || $3::jsonb ),$4:name = $5 WHERE $6:name = $7 RETURNING * 0=MyClass, 1=te, 2={"a":"b"}, 3=updatedAt, 4=2018-06-05T15:57:32.856Z, 5=objectId, 6=zHsUsMqd8t
verbose: RESPONSE from [PUT] /parse/classes/MyClass/zHsUsMqd8t: {
"response": {
"updatedAt": "2018-06-05T15:57:32.856Z"
}
} updatedAt=2018-06-05T15:57:32.856Z
verbose: REQUEST for [PUT] /parse/classes/MyClass/zHsUsMqd8t: {
"te": {}
} method=PUT, url=/parse/classes/MyClass/zHsUsMqd8t, host=localhost:1337, user-agent=Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:60.0) Gecko/20100101 Firefox/60.0, accept=/, accept-language=fr,fr-FR;q=0.8,en-US;q=0.5,en;q=0.3, accept-encoding=gzip, deflate, referer=http://0.0.0.0:4040/apps/TestApp%20Server/browser/MyClass, content-type=text/plain, content-length=157, origin=http://0.0.0.0:4040, connection=keep-alive, client-ip=::1,
debug: PG: findOneAndUpdate MyClass { objectId: 'zHsUsMqd8t' } , updatedAt=2018-06-05T15:57:37.445Z
debug: PG: updateObjectsByQuery MyClass { objectId: 'zHsUsMqd8t' } , updatedAt=2018-06-05T15:57:37.445Z
debug: PG: update: UPDATE $1:name SET $2:name = ( COALESCE($2:name, '{}'::jsonb) || $3::jsonb ),$4:name = $5 WHERE $6:name = $7 RETURNING * 0=MyClass, 1=te, 2={}, 3=updatedAt, 4=2018-06-05T15:57:37.445Z, 5=objectId, 6=zHsUsMqd8t
verbose: RESPONSE from [PUT] /parse/classes/MyClass/zHsUsMqd8t: {
"response": {
"updatedAt": "2018-06-05T15:57:37.445Z"
}
} updatedAt=2018-06-05T15:57:37.445Z
verbose: REQUEST for [PUT] /parse/classes/MyClass/zHsUsMqd8t: {
"te": {
"c": "d"
}
} method=PUT, url=/parse/classes/MyClass/zHsUsMqd8t, host=localhost:1337, user-agent=Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:60.0) Gecko/20100101 Firefox/60.0, accept=/, accept-language=fr,fr-FR;q=0.8,en-US;q=0.5,en;q=0.3, accept-encoding=gzip, deflate, referer=http://0.0.0.0:4040/apps/TestApp%20Server/browser/MyClass, content-type=text/plain, content-length=164, origin=http://0.0.0.0:4040, connection=keep-alive, client-ip=::1, c=d
debug: PG: findOneAndUpdate MyClass { objectId: 'zHsUsMqd8t' } c=d, updatedAt=2018-06-05T15:57:41.848Z
debug: PG: updateObjectsByQuery MyClass { objectId: 'zHsUsMqd8t' } c=d, updatedAt=2018-06-05T15:57:41.848Z
debug: PG: update: UPDATE $1:name SET $2:name = ( COALESCE($2:name, '{}'::jsonb) || $3::jsonb ),$4:name = $5 WHERE $6:name = $7 RETURNING * 0=MyClass, 1=te, 2={"c":"d"}, 3=updatedAt, 4=2018-06-05T15:57:41.848Z, 5=objectId, 6=zHsUsMqd8t
verbose: RESPONSE from [PUT] /parse/classes/MyClass/zHsUsMqd8t: {
"response": {
"updatedAt": "2018-06-05T15:57:41.848Z"
}
} updatedAt=2018-06-05T15:57:41.848Z

[flovilmart]

@dplewis this is something that’s quite trivial to test I believe against both db’s. @jaeggerr would you be willing to attempt to provide a fix?

[jaeggerr]

I am not familiar with the source code but yes, I can try.
I just wanted to be sure that this was not on my side before starting to investigate more.

[flovilmart]

@jaeggerr the easiest way to tests that, would be to add a test ot the suite. It's relatively easy to run an edge to edge test as the full test suite is setup this way. You can run 'focused tests' by making the test with fit instead of it.

Also, to run the tests against POSTGRES, have a look at the contriguting guide: https://github.com/parse-community/parse-server/blob/master/CONTRIBUTING.md

[jaeggerr]

OK, I already implemented some tests.
It's quite vicious because the returned object after save() has the correct values.
It is only when you fetch the object from the database that we get the merged values.

fit ('Update object field', async (done) => {
    let object = new TestObject();

    // Set initial data
    object.set("jsonData", { a: "b" });
    object = await object.save();
    equal(object.get('jsonData'), { a: "b" });

    // Set empty JSON
    object.set("jsonData", {});
    object = await object.save();
    equal(object.get('jsonData'), {});

    // Set new JSON data
    object.set("jsonData", { c: "d" });
    object = await object.save();
    equal(object.get('jsonData'), { c: "d" });

    // Fetch object from server
    object = await object.fetch()
    console.log(object.id, object.get('jsonData'))
    equal(object.get('jsonData'), { c: "d" });

    done();
  });

[flovilmart]

Ahah awesome then!

[jaeggerr]

After investigation, this is due to this PR.
#2984

While I think it is a good idea to be able to add new key / values to the JSON without altering the rest of object:

• There is no documentation about how to update the whole object.
• The behaviour should be consistent with MongoDB.
• The returned updated object after save() does not match what's on the database.

[flovilmart]

@jaeggerr I believe this is an unintended side effect as the original motivation was to provide dotNotation for the PG Objects. do you have an idea how to improve it?

[jaeggerr]

I don't even know if the dot notation still works. Is this documented somewhere?
I tried something like this, but it did not seem work.

object.set("jsonData.a", "bb");  // Did not consider my update
object.set("jsonData", { "a.b": "c" }); // Obviously, "a.b" worked as a key

Maybe we could

• Remove the dot notation. I'm not sure that people are aware of its existence anyway.
• Adapt the add and remove methods of Parse.Object to support associated arrays.
• Keep object.set() to replace the whole JSON object

// Initial value of jsonData is {}

// Add a new value for 'a'
object.add('jsonData', 'a', 'b'); // {"a": "b"}

// Update value of 'a'
object.add('jsonData', 'a', 'bb'); // {"a": "bb"}

// Deeply add or update the value of b.c
// An array would be better than the not notation
// to prevent wierd behaviours issues when keys contain dots.
//
// If a was already defined with another type,
// it is replaced by an object or an error is thrown ?
object.add('jsonData', ['a', 'b'], 'c'); // {"a": { "b": "c" }} or throw error

// Remove b
object.remove(jsonData, ['a', 'b']); // { "a": {} }

// Remove unexisting key
object.remove(jsonData, ['a', 'z']); // { "a": {} } or throw error.

// Remove a
object.remove(jsonData, 'a'); // {}

// Replace whole object. Just use object.set()
object.set({ "a": { "b": "c" } }); // {"a": { "b": "c" }} 

[flovilmart]

I don't even know if the dot notation still works. Is this documented somewhere?

It should be working as the test you have seen is explicitely testing for that:

https://github.com/parse-community/parse-server/pull/2984/files#diff-c0a955924798d6590413b75d68149b2fL66

Also, it's not because from the REST API it looks like it's not working that it's not used internally or else. Making assumptions on the usage of a particular feature based on the public API is usually wrong now. Some documentation cycles have been skipped, and I know for sure many places which leverage the dot notation to update particular keys on a certain object.

That being said, dot notation should work on mongodb, as expected. On postgres, this is another story, perhaps some corner cases haven't been properly addressed nor have arisen over time as not everybody uses postgres, nor this particular feature :)

Changing the front facing API would be ideal, but forces to introduce a new lingua which need to be tought of. I'm not strictly against, but I'd rather have the feature working properly on both DB's before we start piling more changes.

[jaeggerr]

Yes, I understand your point and you are right, it's better to first fix the issue.

I compared with MongoDB on my computer and I can confirm that Mongo does not merge JSON objects.

So maybe the best solution for the moment would be to remove the code that merges the previous and the new JSON value to replicate the behaviour of MongoDB, as I guess it prevails over Postgres.

The line that merges objects is this one:
https://github.com/parse-community/parse-server/pull/2984/files#diff-b0af2ee2c26859a8aa6d8d0a0cf3a64aR927

Here is the fix to remove the merge.

updatePatterns.push(`$${index}:name = ('{}'::jsonb ${deletePatterns} ${incrementPatterns} || $${index + 1 + keysToDelete.length}::jsonb )`);

[flovilmart]

Can you open a PR with the fix? That would be amazing!

[jaeggerr]

Here you are, Sir #4815!