PDA

View Full Version : [FIXED][3.x] Ext.data.Record.prototype.realize



dj
21 Apr 2009, 1:45 PM
In my opinion the realize method of Ext.data.Record should set the record's data with the convert-method of the corresponding field. Otherwise e.g. date-conversions doesn't work.



Ext.override(Ext.data.Record, {
realize : function(data, id) {
if (!id) {
// TODO: Make better exception message
throw new Error("Second paramater to Record#realize must be provided");
}
this.editing = true; // <-- prevent unwanted afterEdit calls by record.
this.phantom = false; // <-- The purpose of this method is to "un-phantom" a record
this.id = id;
this.fields.each(function(f) { // <-- update record fields with data from server if was sent
if (data[f.name] || data[f.mapping]) {
this.set(f.name, f.convert((f.mapping) ? data[f.mapping] : data[f.name]));
}
},this);
this.commit();
this.editing = false;
}
});

christocracy
21 Apr 2009, 9:30 PM
Sure. Thanks.

Added to svn.

Condor
22 Apr 2009, 12:15 AM
The current code is completely ignoring the defaultValue config option of the field (used when the convert function returns undefined).

Animal
22 Apr 2009, 12:18 AM
Really, it should use a method from the associated Reader class to correctly pull values based on mappings (which may be in dot notation), and perform conversion and defaulting.

Condor
22 Apr 2009, 12:26 AM
A record doesn't know it's reader, so that isn't feasible. Instead, JsonReader and XmlReader should call record.realize instead of using their own logic in loadRecords.

The problem with the original design is that 'mapping' and 'convert' are properties that should belong in the reader configuration and not in the field.
Now there is no way to use the same record in 2 differently configured readers.

Animal
22 Apr 2009, 12:28 AM
The code snippet from JsonReader.readRecords which pulls values from a data row object should be broken out and made callable...


old code:



readRecords : function(o){
/**
* After any data loads, the raw JSON data is available for further custom processing. If no data is
* loaded or there is a load exception this property will be undefined.
* @type Object
*/
this.jsonData = o;
if(o.metaData){
delete this.ef;
this.meta = o.metaData;
this.recordType = Ext.data.Record.create(o.metaData.fields);
this.onMetaChange(this.meta, this.recordType, o);
}
var s = this.meta, Record = this.recordType,
f = Record.prototype.fields, fi = f.items, fl = f.length;

// Generate extraction functions for the totalProperty, the root, the id, and for each field
if (!this.ef) {
if(s.totalProperty) {
this.getTotal = this.getJsonAccessor(s.totalProperty);
}
if(s.successProperty) {
this.getSuccess = this.getJsonAccessor(s.successProperty);
}
this.getRoot = s.root ? this.getJsonAccessor(s.root) : function(p){return p;};
if (s.id || s.idProperty) {
var g = this.getJsonAccessor(s.id || s.idProperty);
this.getId = function(rec) {
var r = g(rec);
return (r === undefined || r === "") ? null : r;
};
} else {
this.getId = function(){return null;};
}
this.ef = [];
for(var i = 0; i < fl; i++){
f = fi[i];
var map = (f.mapping !== undefined && f.mapping !== null) ? f.mapping : f.name;
this.ef[i] = this.getJsonAccessor(map);
}
}

var root = this.getRoot(o), c = root.length, totalRecords = c, success = true;
if(s.totalProperty){
var v = parseInt(this.getTotal(o), 10);
if(!isNaN(v)){
totalRecords = v;
}
}
if(s.successProperty){
var v = this.getSuccess(o);
if(v === false || v === 'false'){
success = false;
}
}
var records = [];
for(var i = 0; i < c; i++){
var n = root[i];
var values = {};
var id = this.getId(n);
for(var j = 0; j < fl; j++){
f = fi[j];
var v = this.ef[j](n);
values[f.name] = f.convert((v !== undefined) ? v : f.defaultValue, n);
}
var record = new Record(values, id);
record.json = n;
records[i] = record;
}
return {
success : success,
records : records,
totalRecords : totalRecords
};
},


new code:



readRecords : function(o){
/**
* After any data loads, the raw JSON data is available for further custom processing. If no data is
* loaded or there is a load exception this property will be undefined.
* @type Object
*/
this.jsonData = o;
if(o.metaData){
delete this.ef;
this.meta = o.metaData;
this.recordType = Ext.data.Record.create(o.metaData.fields);
this.onMetaChange(this.meta, this.recordType, o);
}
var s = this.meta, Record = this.recordType,
f = Record.prototype.fields, fi = f.items, fl = f.length;

// Generate extraction functions for the totalProperty, the root, the id, and for each field
if (!this.ef) {
if(s.totalProperty) {
this.getTotal = this.getJsonAccessor(s.totalProperty);
}
if(s.successProperty) {
this.getSuccess = this.getJsonAccessor(s.successProperty);
}
this.getRoot = s.root ? this.getJsonAccessor(s.root) : function(p){return p;};
if (s.id || s.idProperty) {
var g = this.getJsonAccessor(s.id || s.idProperty);
this.getId = function(rec) {
var r = g(rec);
return (r === undefined || r === "") ? null : r;
};
} else {
this.getId = function(){return null;};
}
this.ef = [];
for(var i = 0; i < fl; i++){
f = fi[i];
var map = (f.mapping !== undefined && f.mapping !== null) ? f.mapping : f.name;
this.ef[i] = this.getJsonAccessor(map);
}
}

var root = this.getRoot(o), c = root.length, totalRecords = c, success = true;
if(s.totalProperty){
var v = parseInt(this.getTotal(o), 10);
if(!isNaN(v)){
totalRecords = v;
}
}
if(s.successProperty){
var v = this.getSuccess(o);
if(v === false || v === 'false'){
success = false;
}
}
var records = [];
for(var i = 0; i < c; i++){
var n = root[i];
var record = new Record(this.extractValues(n), this.getId(n));
record.json = n;
records[i] = record;
}
return {
success : success,
records : records,
totalRecords : totalRecords
};
},

extractValues: function(data) {
var values = {};
for(var j = 0; j < fl; j++){
f = fi[j];
var v = this.ef[j](data);
values[f.name] = f.convert((v !== undefined) ? v : f.defaultValue, data);
}
return values;
},

Animal
22 Apr 2009, 12:29 AM
Then call extractValues from Record.realize

Animal
22 Apr 2009, 12:30 AM
A record doesn't know it's reader, so that isn't feasible.

Ah yes, I forgot this. But the Store does and it can pass the Reader which it knows must be used to extract the values.

Condor
22 Apr 2009, 12:32 AM
So, realize should be a method of DataReader and not of Record!

Animal
22 Apr 2009, 12:36 AM
I think so. It reads Record data from a data object.

christocracy
22 Apr 2009, 8:08 AM
Thanks guys. I'll work today on moving Record#realize into DataReader.

christocracy
22 Apr 2009, 12:37 PM
Ok, moved Record#realize into DataReader#realize & JsonReader#realize I did as you suggested, Animal, and implemented JsonReader#extractValues.

DataReader#realize


realize: function(record, data){
var values = this.extractValues(data, record.fields.items, record.fields.items.length);
record.editing = true; // <-- prevent unwanted afterEdit calls by record.
record.phantom = false; // <-- The purpose of this method is to "un-phantom" a record
record.id = values[this.meta.idProperty];
record.fields.each(function(f) { // <-- update record fields with data from server if was sent
if (values[f.name] || values[f.mapping]) {
record.set(f.name, (f.mapping) ? values[f.mapping] : values[f.name]);
}
});
record.commit();
record.editing = false;
}


JsonReader#realize


realize : function(record, data) {
if (!data[this.meta.idProperty]) {
throw new Error("JsonReader attempted to realize a record but could not find the idProperty '" + this.meta.idProperty + "' in the returned data. Please ensure you send the '" + this.meta.idProperty + "' back in your response from the server using the meta-data defined in your DataReader when creating new records.");
}
Ext.data.JsonReader.superclass.realize.apply(this, arguments);
}



Store#onCreateRecords will catch exceptions thrown by JsonReader#realize.
Notice I forced a store.reload if an exception was caught.
Too intrusive? I'm thinking we need to prevent possibility of duplicate creates.



// private onCreateRecord proxy callback for create action
onCreateRecords : function(rs, data) {
if (Ext.isArray(rs)) {
for (var i=0,len=rs.length;i<len;i++) {
this.onCreateRecords(rs[i], data[i]);
}
}
else if (rs.phantom) {
try {
this.reader.realize(rs, data);
}
catch (e) {
// force reload if we couldn't realize the record? Otherwise Store might re-execute a create request.
this.reload();
e += ' A Store reload have been executed in order to prevent a duplicate record being created.';

// Framework needs an exceptionHandler to send execptions to that would detect the existence of Firebug, etc.
if (typeof(console) == 'object' && typeof(console.error) == 'function') {
console.error(e);
}
else if (typeof(Ext.log) == 'function') {
Ext.log(e);
}
}
}
},

mschwartz
22 Apr 2009, 12:41 PM
record.phantom = false; // <-- The purpose of this method is to "un-phantom" a record


Awesome comment.
:D

Animal
22 Apr 2009, 12:51 PM
DataReader now has



realize: function(record, data){
var values = this.extractValues(data, record.fields.items, record.fields.items.length);
record.editing = true; // <-- prevent unwanted afterEdit calls by record.
record.phantom = false; // <-- The purpose of this method is to "un-phantom" a record
record.id = values[this.meta.idProperty];
record.fields.each(function(f) { // <-- update record fields with data from server if was sent
if (values[f.name] || values[f.mapping]) {
record.set(f.name, (f.mapping) ? values[f.mapping] : values[f.name]);
}
});
record.commit();
record.editing = false;
}


The red code will likely not work. The extractValues method recently added to JsonReader extracts the fields using the field's mapping from raw data into a data object who's property names are field names. The result may be used as the data for a Record.

The method should be



realize: function(record, data){
record.data = this.extractValues(data, record.fields.items, record.fields.items.length);
record.phantom = false; // <-- The purpose of this method is to "un-phantom" a record
record.id = values[this.meta.idProperty];
record.markDirty();
record.commit();
}

christocracy
22 Apr 2009, 12:58 PM
Done. committed.

Animal
22 Apr 2009, 12:58 PM
I don't understand this in Store



/**
* Send all {@link #getModifiedRecords modifiedRecords} to the server using the
* api's configured save url.
* @param {Object} options
*/
save : function(rs) {
rs = rs || this.getModifiedRecords();


is "rs" intended to be Records or options?

christocracy
22 Apr 2009, 1:06 PM
updateRecord : function(store, record, action) {
if (action != Ext.data.Record.EDIT || this.batchSave) {
return;
}
if (!record.phantom || (record.phantom && record.isValid)) {
this.save(record);
}
},

christocracy
22 Apr 2009, 1:10 PM
It might be unecessary though to call this.save(record).

Can probably just do this.save() now.

I'll investigate and test.

christocracy
22 Apr 2009, 1:19 PM
Yes, it was unnecessary.

I think I'll give Store#save the ability to accept options argument now (as the function header was mistakenly documented) just as Store#load.

Animal
22 Apr 2009, 11:43 PM
OK, looking good.

realize needs removing from the Record class now.

Also, in DataReader.realize:



var values = this.extractValues(data, rs.fields.items, rs.fields.items.length)
rs.editing = true; // <-- prevent unwanted afterEdit calls by record.
rs.phantom = false; // <-- The purpose of this method is to "un-phantom" a record
rs.id = data[this.meta.idProperty];
rs.data = values;
rs.commit();
rs.editing = false;


I think the red lines are unnecessary. They were only needed if calling Record.set to set individual field values.

The id will not be in the extracted values. It is not a field. It will be in the raw data.

christocracy
23 Apr 2009, 6:32 PM
Oops, sorry I missed this til now.

I missed this one because my test Record.create includes the id-column.


rs.id = data[this.meta.idProperty];


All suggestions committed.

Animal
23 Apr 2009, 10:21 PM
One suggestion.

The constants in Ext.data

Really then, Ext.data should be a singleton...



/**
* @class Ext.data
* @singleton
* A repository for static data values used by the Ext.data classes
*/
Ext.data = {
CREATE: 'create',

...
};


Obviously, this would have to be defined before the other Ext.data classes.

Currently Ext.data is created in core/core/Ext.js:



Ext.ns("Ext", "Ext.util", "Ext.lib", "Ext.data");

christocracy
23 Apr 2009, 10:33 PM
Yup, I agree.

Animal
23 Apr 2009, 10:35 PM
Then this could be expanded to allow the docs for it to become a kind of summary area for descriptions of and examples of using the Ext.data "package".