#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 12 years ago by
Owner: | changed from Jared Jurkiewicz to Kris Zyp |
---|
comment:2 Changed 12 years ago by
Owner: | changed from Kris Zyp to kriszyp |
---|
comment:3 Changed 12 years ago by
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 12 years ago by
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 12 years ago by
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 12 years ago by
Cc: | maulin added |
---|
comment:7 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:9 Changed 12 years ago by
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 12 years ago by
Milestone: | tbd → 1.2 |
---|
comment:11 Changed 11 years ago by
Owner: | changed from kriszyp to Kris Zyp |
---|
Assigning to owner of JsonRestStore?.