Failed Dirty detection on value in array changed back to previous value

[simonaberry]

If you have a object with a field of type array.

1. Perform initial save, with object.arrayField = [5,5] => ok (PUT request includes [5,5])
2. modify array and save, i.e. object.arrayField = [5,0] => ok (PUT request includes [5,0])
3. change array back to first value & save, i.e. object.arrayField = [5,5] => problem (PUT request does not include the arrayField ... seems like it doesn't detect the change)

however if you do a object.arrayField = object.arrayField.slice() before saving, the sdk does detect the change in step 3 and sends the correct payload

error is happening with lastest version 1.11.0

[montymxb]

@simonaberry can you please put up the exact code to replicate this?

[simonaberry]

https://plnkr.co/edit/OSnoIAitEqBqqtVRfE5q

[montymxb]

Can you inline the relevant code here. Doesn't necessarily need to be the whole file, just strip out anything sensitive. I don't need to see the entire project.

…

On Jan 15, 2018 23:43, "simonaberry" ***@***.***> wrote: https://plnkr.co/edit/OSnoIAitEqBqqtVRfE5q — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#542 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AE1d4Dtf7k3YgSt3JI9LcyZ5ccM_f539ks5tLFMNgaJpZM4RcR_H> .

[simonaberry]

The plunker isn't a full project, just a small angularjs project that illustrates the bug. However here are the important bits :

preparation : extend a Parse.Object to include a query and getter/setters :

var parseObject = Parse.Object.extend ('OBJECTTEST', {}, {
    getTestObject : function (id) {
      var query = new Parse.Query(this);
      return query.get(id);
    }
  })
  
  Object.defineProperty(parseObject.prototype, 'arrayField', {
				get: function () {
					return this.get('arrayField');
				},
				set: function (aValue) {
					return this.set('arrayField', aValue);
				}
			});
			

start network monitor in dev tools
then after invoking getTestObject('xxx') and saving result to parseObject,

step 1 set arrayField to [5,5] and save :

parseObject.arrayField = [5,5];  
parseObject.save(); // => request payload includes arrayField of [5,5]

step 2 change a value in the array and save :

parseObject.arrayField[1] = 0;  
parseObject.save(); // => request payload includes arrayField of [5,0]

step 3 change a value in the array back to original value and save :

parseObject.arrayField[1] = 5;  
parseObject.save(); // => request payload *does not* include *dirty* arrayField

I suspect this has to do with the mutability of arrays, but am not an expert in the sdk...

[montymxb]

Following off of what you've provided I put together a little test.

// initialized before and such...

var ObjectTest = Parse.Object.extend('ObjectTest');

Object.defineProperty(ObjectTest.prototype, 'arrayField', {
  get: function () {
    return this.get('arrayField');
  },
  set: function (aValue) {
    return this.set('arrayField', aValue);
  }
});

var parseObject = new ObjectTest();

parseObject.arrayField = [5,5];

parseObject.save().then(function(obj) {
  // arrayField = 5,5
  obj.arrayField[1] = 0;
  return obj.save();

}).then(function(obj) {
  // arrayField = 5,0
  obj.arrayField[1] = 5;
  return obj.save();

}).then(function(obj) {
  // arrayField = 5,5 again
  return obj.destroy();

})

Note that I ran this in the browser rather than in node. Also I replaced the logging code with comments so you can just see the process here.

Using this approach I didn't have any issues with dirty value detection when changing the arrayField property. Let me know if your setup is different enough to require a further look (on node for example), otherwise I would closely inspect your code for a logic error somewhere.

[simonaberry]

I beg to differ...

I have cut and paste your code above into a plunker (so running in the browser): https://plnkr.co/edit/xoTmWyQPEmHjDqE7qziL?p=preview

If you open your network monitor in dev tools (I am using chrome) : you will see that the payload in third request (ie the last save()) does not include the arrayField field

I am not arguing that the value of arrayField in the browser is not correct (it is correct)... my point is that the updated value is not being sent to the server to be saved & updated ... which results in the server data being incorrect compared to the client data

For reference, here is the payload data for the second save() (works ok):

{
  "arrayField": [
    5,
    0
  ],
  "_method": "PUT",
  "_ApplicationId": "xxx",
  "_JavaScriptKey": "xxx",
  "_ClientVersion": "js1.11.0",
  "_InstallationId": "xxx"
}

and here for the third save() (problem!):

{
  "_method": "PUT",
  "_ApplicationId": "xxx",
  "_JavaScriptKey": "xxx",
  "_ClientVersion": "js1.11.0",
  "_InstallationId": "xxx"
}

[montymxb]

@simonaberry I took a deeper look regarding the actual save JSON being passed to the server. I can confirm that the 3rd save is omitting the change. Looking into this a bit more I figured I would check the object's dirty state, which is false when the change is not persisted.

Playing around with this a bit I'm suspecting the residual values [5,5] are hanging around in the array. Turns out if, before the 3rd save, you set the second value in the array field to anything other than 5, it will carry the change. This seems to only happen on that object instance, including that which is returned after save. Querying for a fresh object (or fetching on the existing object) temporarily alleviates this problem.

I'll have to take a look at the state internally, something residual must be hanging around after the initial set. Thanks for bringing this up. I can't guarantee I'll be able to put up a PR anytime soon, but if you're up for it (and you think you can) see if you can't isolate the underlying cause. From there a PR and a test or two would be trivial.

[simonaberry]

So I've been down the rabbit hole and can report :

• in _getDirtyObjectAttributes the problem is that the ObjectCache is always reporting a value of [5,5] for the arrayField, even when the value was saved as [5,0]
• commitServerChanges does not update the ObjectCache because the changes passed to it from _handleSaveResponse does not included the changed array
• this is because the pending object from stateController.popPendingState function does not include the changed array
• this is because the newOps is not being generated by the set function
• and, finally, that is because the set function is not being called when the array is modified

which then lead me to read this stackoverflow post

which, I believe, concludes that is is a bad idea to use getters and setters on array members .....

so long & short of it.... don't do this :

Object.defineProperty(ObjectTest.prototype, 'arrayField', {
  get: function () {
    return this.get('arrayField');
  },
  set: function (aValue) {
    return this.set('arrayField', aValue);
  }
});

but rather do this :

parseObject.set('arrayField', arrayVal)

if the the plunker/test code is updated with the .set('arrayField, ...) call instead of the arrayField[1] = .., the 'bug' goes away !

happy to close..?

[montymxb]

@simonaberry thanks for digging into this a bit more. I did some initial investigation and came to a similar conclusion regarding the object cache. I've put time aside to look into it further, but honestly this does feel like something that should be avoided in practice.

I'll keep this open in case someone else has a similar issue, but I will be flagging this (at least for now) as a 'wont fix' due to the nature of the problem. If we later come to the conclusion that there is a practical way to resolve this we'll post about it here.