Opened 11 years ago

Closed 11 years ago

Last modified 9 years ago

#7311 closed defect (fixed)

JsonRestStore onPostCommit property is not defined

Reported by: syq Owned by: Kris Zyp
Priority: high Milestone: 1.2
Component: DojoX Data Version: 1.1.1
Keywords: Cc: maulin
Blocked By: Blocking:

Description

In dojox.data.JsonRestStore?, the save function,

if(actions[i].method == 'post' && this.onPostCommit){
					var self = this;
					// some REST stores need to do some processing after a post has been committed
					(function(object){
						dfd.addCallback(function(value){
							self.onPostCommit(object);
							return value;
						});
					})(actions[i].content);

onPostCommit is not defined anywhere. According to code(self.onPostCommit), it should be a property of store. I find this problem when I use the newItem function of the store. Please refer to the code:

var store = new dojox.data.JsonRestStore({target: '/rest_resource/'});
var parentInfo.parent = parent;
store.newItem(data, parentInfo);
store.save();

Everything works perfect except that the value of (underscore)id property of the new store item ends up with the random number in the place of 'id' like:

"__id": "/rest_resource/20cdd9bfd64847a37fc09ea7" 

The (underscore)id property would be used as the url for any further update on this new item, which needs to be sent back to server via HTTP.

This is a reasonable approach on the assumption that the id of the new item in store should be determined by server. When the item with the correct id is sent back from server, the store should have certain way to reconstruct the value of (underscore)id property of the new item with the information of the returned item from server. Otherwise, the bad url would be used and hence causes problem.

Looking into the source code, it looks onPostCommit is for this purpose, but it is not implemented.

Change History (11)

comment:1 Changed 11 years ago by Jared Jurkiewicz

Owner: changed from Jared Jurkiewicz to Kris Zyp

Assigning to owner of JsonRestStore?.

comment:2 Changed 11 years ago by Jared Jurkiewicz

Owner: changed from Kris Zyp to kriszyp

comment:3 Changed 11 years ago by Kris Zyp

I can update JsonRestStore? to update objects based on the ids assigned by the server (per the HTTP spec, this should come from the Content-Location header). However, I have been hesitant to code this because it is such a fragile solution. If you create a new item and save it (triggering a POST), and then make a modification and save it before the first save's response is received, it is impossible for the JsonRestStore? to know the assigned id. It would be possible to delay the second save until the first save is received, but this would significantly increase the size and complexity of the code, so I will surely avoid this solution. There is also additional complexity because we can't change the client reported id (from getIdentity(item)) or it will break widgets, so must retain the client assigned id for getIdentity calls. Consequently, I believe it is better for the server to maintain a session based temporary map of client ids that map to the assigned server ids. However, I can still code this change for those that want to use it.

comment:4 Changed 11 years ago by syq

I agree it is not easy to synchronize the id on both sides. Here are my test.

  • when onPostCommit is implemented in subclass of JsonRestStore?, there is an error reported. The variable dfd(around line 199) in the save function definition in JsonRestStore?.js is not defined. dfd is defined in JsonRest?.js as a local variable in function commit and is not passed to JsonRestStore?.js.
  • In the same save function, actions[i].content is passed to onPostCommit. actions[i].content is about the new item before it gets POSTed to server. Should not it be actions[i].results(or anything that gets returned from server)? This way onPostCommit is able to get and process the returned value from server up the commitment of the new item.
  • Even if the server would save the same id of the new item that the store delivers, there is still a very little chance that the randomly selected id might be identical.

Yes, it is not a good idea to have the store to save the new item twice. It would be better if the new item won't be put in store until the data with the assigned it from the server arrives in client, such that the new item can be saved with the assigned it.

comment:5 Changed 11 years ago by Kris Zyp

I am thinking that it might be best to just eliminate onPostCommit, and let subclasses override the save function in order to implement object id updating (with some documentation help of course). I would add the deferred object as a property to the list of actions so that the you could do some meaningful updates on the object based on the server returned data. Here is an example of some code that would set the server-assigned id (without modifying the client-assigned id for getIdentity reporting) based on the HTTP specification: save: function(kwArgs){

var actions = this.inherited(arguments); do this for(var i = 0; i < actions.length; i++){

if(actions[i].method == 'post'){

some REST stores need to do some processing after a post has been committed (function(object,dfd){

dfd.addCallback(function(value){

try{

object.assignedId = dfd.ioArgs.xhr.getResponseHeader("Content-Location");

}catch(e){} return value;

});

})(actions[i].content,actions[i].deferred);

}

}

}

I am considering adding this to JsonRestStore? as the default mechanism for server-assigned ids (of course you could still override this with a subclass). Do you think it would be helpful to have this in JsonRestStore? for default id assignment?

comment:6 Changed 11 years ago by Kris Zyp

Cc: maulin added

comment:7 Changed 11 years ago by Kris Zyp

Resolution: fixed
Status: newclosed

(In [14702]) Updated to honor id assignment from server, altered lazy loading mechanism, and updated CouchDBRestStore. fixes #7359 and fixes #7311

comment:8 Changed 11 years ago by Kris Zyp

(In [14704]) fixes #7311

comment:9 Changed 11 years ago by Kris Zyp

The JsonRestStore? will now update ids based on server assignment from the Location header. If a POST command gets a response with a Location header, the object will get the id from the header. If you want to implement a custom strategy for id assignment, you can see CouchDBRestStore for an example.

comment:10 Changed 11 years ago by dylan

Milestone: tbd1.2

comment:11 Changed 9 years ago by bill

Owner: changed from kriszyp to Kris Zyp
Note: See TracTickets for help on using tickets.