PDA

View Full Version : [CLOSED-464][3.1] Bug in the REST store?



misha
5 Jan 2010, 2:57 AM
When I create an item in the store, my server script returns HTTP response code 201 (as it should for REST), but then Ext breaks!

If I set my server code to return 200 it works!

Does this means that Ext REST is not exactly implemented according to the REST specification?

Note: I used ExtJS 3.1.0

jtruffot
21 Jan 2010, 2:20 AM
I have the same problem since 3.1.0 (no problem with 3.0.0).

When I add an item in my restful store, my server return 201 response ("created") and I get DataReader exception in realize method: "#realize was called with invalid remote-data". In this method, the data argument is strangely undefined. I tried to debug and found "new" interceptor on proxy onWrite method, created by restify method of Ext.data.Api (new compared to 3.0.0 version).

This interceptor test the response status:


switch (response.status) {
case 200: // standard 200 response, send control back to HttpProxy#onWrite by returning true from this intercepted #onWrite
return true;
break;
case 201: // entity created but no response returned
res.success = true;
break;
case 204: // no-content. Create a fake response.
res.success = true;
res.data = null;
break;
default:
return true;
break;
}In 200 response case, "return true" send control back to HttpProxy#onWrite where reader read the response:


res = reader.readResponse(action, response);I don't understand the comment of the 201 response case. Why no response should be returned? I have entity created and response returned. So in the callback, "res.data" is undefined and DataReader send invalid remote-data exception...

I implemented the following solution: "return true" to send control back to HttpProxy#onWrite in 201 response case too.


case 201:
// entity created AND response returned
// send control back to HttpProxy#onWrite like standard 200 response
return true;
I'm not sure it suits everyone but it works for me! ;)

misha
21 Jan 2010, 6:48 AM
Great fix!

But, I'm kinda hopping its a bug that will be solved.

abrezinsky
26 Jan 2010, 6:24 PM
That fix doesn't really work well. You hand back control to HttpProxy#onWrite so none of the events or callbacks fire at the bottom of the proxy.onWrite function.

If res.data does not exist then #realize does not ever execute the code to convert a phantom record to a normal record. It also does not get the ID of the record to use for subsequent updates. If you try to edit that record immediately then it just ends up creating a new one.

I think the misunderstanding is around a 201 Created response. It should return the location of the new entity. It CAN also return a representation of the status of the request (see: http://restpatterns.org/HTTP_Verbs/POST).

Changing the code for the 201 case like the following makes it work:



case 201: // entity created but no response returned
//res[reader.meta.successProperty] = true;
var r = reader.readResponse(action, response);
res.success = true;
res.data = r.data;
break;
I think this boils down to what the ExtJS core team thinks of how a 201 Created should be dealt with. Are we expected to:

1) have a RESTful endpoint return a representation of the created record and ExtJS process it to apply() missing data to the record we already have in our Record object

2) reload() the store after adding a record to get the single new record

3) let ExtJS follow the location header and go fetch the new entity and add that representation to the store? This would require one more request to go fetch what we just created

I think the ideal solution is (1). Thoughts?

edspencer
1 Feb 2010, 6:07 PM
The HTTP spec allows the response entity to take a number of forms on a 201. The standard way of fetching the data seems to be following the Location header on the response, as there's no guarantee that the 201 response contains all or even some of the data.

That said, I see no reason not to treat 201 the same way as 200 IF there is data in the response. If there is no data, we still treat it the way we were previously. The events at the bottom are still fired as they have simply been copied and pasted from DataWriter's onWrite, so there should be no break in compatibility here.

The changes are now in trunk and look like the following. This no longer fires an exception for either an empty response or one containing data:



case 201: // entity created but no response returned
if (Ext.isEmpty(res.raw.responseText)) {
res.success = true;
} else {
//if the response contains data, treat it like a 200
return true;
}
break;