PDA

View Full Version : [OPEN] Handling of success property differs between Forms and Data Stores (Ext 1.0.1)



CableDawg
21 Jun 2007, 6:43 AM
If a server response contains the "success=false" XML attribute (or, I would assume, the JSON value as well), Form's "actionfailed" event is fired. When the same response is sent to a Data Store however, the Store's "load" event is fired instead of "loadexception".

I couldn't find a way to easily determine the status of the success attribute through the "load" event and it seems sort of silly to have to handle an error via the "load" event. If this is done intentionally, then I would appreciate some tips on how to easily handle a failed query.

If this is an oversight, it would be nice if a Data Store could automatically parse the error nodes just like Form Actions do.

Thanks!

EDIT:
Replacing line 88 of HttpProxy.js from


this.fireEvent("load", this, o, o.request.arg);

to



if(result.success) {
this.fireEvent("load", this, o, o.request.arg);
} else {
this.fireEvent("loadexception", this, o, response);
}


seems to do the trick.

tryanDLS
21 Jun 2007, 7:00 AM
That change would break code b/c a result won't necessarily have 'success' property. 'Success' has to be considered in the context it applies to - an HTTP GET/POST or a result string (JSON/XML) returned by some server method.

In the proxy code, success is an indication of whether the HTTP request worked or not (http response code 200, 404, etc), not whether your server method returns a value of success=true. Look at the source of asyncRequest ext-base.js to understand how the success value is derived.

A Form submit request can return an http success, but success:false in the result b/c the validation process failed.

CableDawg
21 Jun 2007, 7:45 AM
I see your point, but don't you think it would make HttpProxy easier to use by including support for the success value? As it stands now, you can tell the reader to use it and the value is parsed, but then you can't do anything with it or even get it returned without re-parsing it manually.

As far as Forms are concerned, an HTTP error or a data validation error both trip up "loadexception". Why not extend that to Data Store? Just because the HTTP request went through OK does not mean that the request was valid. If I am trying to access a database record, that service obviously exists (assuming the URL was correct) so there would be no 404. And assuming there is no problem in the server-side code, no 500 would get returned. But if the record does not exist or has some other problem (i.e., registration has not be validated, the record is locked or something similar), that sounds like a load exception to me and that event should be triggered. Obviously you can't do that by just looking at the status of the HTTP request.

And to address my edit:


if(result.success !== false) {
this.fireEvent("load", this, o, o.request.arg);
} else {
this.fireEvent("loadexception", this, o, response);
}

That should handle if success is false or undefined. That is just quick and dirty. I am sure there is a more elegant solution out there.

tryanDLS
21 Jun 2007, 8:31 AM
Making the proxy look for a result.success property means the proxy has to be smarter. The point of separating proxy from store is to provide a level of abstraction. A proxy doesn't know anything other than it's sending an HTTP request and getting back an HTTP response object with it's standard properties (e.g. status, responseText, responseXml, etc). Processing the content of the response is delegated to another object(s) b/c there could be any number of different things to be done and you don't want to force all that knowledge into one place (the proxy). It might not even be result.success - it could be some other property.



Just because the HTTP request went through OK does not mean that the request was valid.From the point of HTTP and the proxy, it does mean it's valid - it's solely based on the returned status code.

The proxy only fires loadexception when the HTTP status indicates failure or the response is malformed (bad JSON). Part of the question may stem from the fact that store has a loadexception event, but that's only relaying the proxy event, it doesn't happen in the store' load. Maybe the event names should have been request/requestexception, but that's legacy from pre-1.0 when there was much closer binding of the classes.

As far as Forms go, it doesn't go thru proxy - it directly makes the request via Ext.Ajax; neither load or loadexception are involved (based on 1.1beta code). But similarly, success/failure is determined by HTTP status, before an Action object looks for result.success.

CableDawg
21 Jun 2007, 8:59 AM
Again, I see your point and completely agree. The proxy should not be handling the success value.

I do, however, believe that the Data Store should handle the success value by interpreting the loadexception event fired by the proxy instead of just relaying it (or like you said, rename the events... probably not worth it.)

I understand that Form uses the new Ajax lib, but I still think that the handling of the server response within Form is a lot more flexible and should be copied over to Data Store. The proxy is doing the work to interpret the success value, it might as well be usable data. And I think it would help to keep the behavior of the "loadexception" event more consistent throught the library.

CableDawg
21 Jun 2007, 11:40 AM
Ok, here is my attempt at getting the Data Store to recognize an un-successful record request.

First, I added an errorReader to the Data Store:



var ds = new Ext.data.Store({
proxy : new Ext.data.HttpProxy({ url:'/foo/bar' }),
reader : new Ext.data.XmlReader({ record:'foo', success:'@success' }, [
{ name:'bar' }
]),
errorReader : new Ext.data.XmlReader({ record:'error' }, [
{ name:'header' },
{ name:'text' }
])
});



Then, I modified HttpProxy to look for the success property and call the request callback with different parameters depending on its value. Since we are dealing with error data, obviously there is no parsed result set, so we send it the XHR response as the first argument and "false" for the last argument (which the callback expects to be the success value)



Ext.override(Ext.data.HttpProxy, {
loadResponse : function(o, success, response) {
if(!success){
this.fireEvent("loadexception", this, o, response);
o.request.callback.call(o.request.scope, null, o.request.arg, false);
return;
}
var result;
try {
result = o.reader.read(response);
}catch(e){
this.fireEvent("loadexception", this, o, response, e);
o.request.callback.call(o.request.scope, null, o.request.arg, false);
return;
}
// BEGIN EDIT
this.fireEvent("load", this, o, o.request.arg);
if(result.success !== false) {
o.request.callback.call(o.request.scope, result, o.request.arg, true);
} else {
o.request.callback.call(o.request.scope, response, o.request.arg, false);
}
// END EDIT
}
});


Then, I modified Data Store to check for a "false" success value and then the presence of an errorReader. If the errorReader exists, then the errors are parsed and assigned to the result variable. This is copied straight from Action.Submit's handleResponse method in Action.js.

Once the resposne is parsed, the "loadException" event is fired with the same signature as the Proxy's "loadException" event with the addition of the last argument, the parsed error object.



Ext.override(Ext.data.Store, {
loadRecords : function(o, options, success){
if(!o || success === false){
if(success !== false){
this.fireEvent("load", this, [], options);
// BEGIN EDIT
} else {
var result;
if(this.errorReader) {
var rs = this.errorReader.read(o);
var errors = [];
if(rs.records) {
for(var i = 0, len = rs.records.length; i < len; i++) {
var r = rs.records[i];
errors[i] = r.data;
}
}
result = {
success : rs.success,
errors : errors
};
 }
this.fireEvent("loadexception", this, options, o, result);
// END EDIT
}
if(options.callback){
options.callback.call(options.scope || this, [], options, false);
}
return;
}
var r = o.records, t = o.totalRecords || r.length;
if(!options || options.add !== true){
for(var i = 0, len = r.length; i < len; i++){
r[i].join(this);
}
this.data.clear();
this.data.addAll(r);
this.totalLength = t;
this.applySort();
this.fireEvent("datachanged", this);
}else{
this.totalLength = Math.max(t, this.data.length+r.length);
this.add(r);
}
this.fireEvent("load", this, r, options);
if(options.callback){
options.callback.call(options.scope || this, r, options, true);
}
}
});


So now, when "loadExeception" fires, you will get the response object to handle 404s and 500s as well as error data to handle any server validation errors and the like.

It seems to work alright on my basic test here, but I haven't tested every possible situation, so please don't flame me if I left something out.