PDA

View Full Version : [OPEN-280] WebStorageProxy#getIds() returns an array of strings, not integers



mystix
14 Sep 2010, 11:11 AM
In Sencha Touch 0.94, WebStorageProxy#getIds() returns an array of strings, while Model#getId() returns a Number.

this causes WebStorageProxy#destroy() to fail because the following line


newIds.remove(records[i].getId());

will never remove the numeric id from the array of string ids.

possible fix:


destroy: function(operation, callback, scope) {
var records = operation.records,
length = records.length,
ids = this.getIds(),


newIds = [].concat(ids),
i;

for (i = 0; i < length; i++) {
newIds.remove(records[i].getId() + '');
this.removeRecord(records[i], false);
}

this.setIds(newIds);

if (typeof callback == 'function') {
callback.call(scope || this, operation);
}
},

aconran
14 Sep 2010, 11:23 AM
Thx for the report. Dup of #280.

mystix
14 Sep 2010, 11:30 AM
i tried searching for OPEN-280 (and getIds() for the matter), but this is the only thread marked [OPEN-280].
do you have a link of the original bugreport on hand?

aconran
15 Sep 2010, 4:00 PM
Marc - There is no forum thread on it. We opened the ticket internally.

edspencer
15 Sep 2010, 8:01 PM
I've updated WebStorageProxy to cast each id returned by getIds() into a number. This will be present in the next build.

mystix
15 Sep 2010, 9:10 PM
super. thanks!

holdenmatt
5 Jun 2011, 4:53 PM
This fix seems to have created a new bug:

WebStorageProxy#getIds() is now always returning an array of ints, but Model#getId() will return a string when a model has an "id" field of type string (e.g. the default type).

This makes WebStorageProxy#destroy fail since
newIds.remove(records[i].getId());
tries to remove a string from an array of ints, and so never removes anything.

holdenmatt
5 Jun 2011, 5:08 PM
If you're using string ids for your Models (as I am), here's a workaround to fix this:



Ext.override(Ext.data.WebStorageProxy, {
/**
* Make getIds() return an array of strings instead of ints.
*/
getIds: function() {
var ids = (this.getStorageObject().getItem(this.id) || "").split(",");
if (ids.length == 1 && ids[0] == "") {
ids = [];
}
return ids;
}
});


This fixes my problem, but WebStorageProxy should probably be fixed to work with either string or int ids, e.g. by:
- removing the parseInt loop from getIds, and
- casting model ids to strings before making comparisons

Mis63
9 Jun 2011, 11:44 PM
WebStorageProxy has many bugs and/or limitations (string key, associations reading/writing...) and I think that it should be a priority for next release.

About this thread : to do working it in all cases (numeric and string keys), the id should be converted to int only if the key of model is a numeric.



getIds: function() {
var ids = (this.getStorageObject().getItem(this.id) || "").split(","),
length = ids.length,
i;

if (length == 1 && ids[0] == "") {
ids = [];
} else {
if (<key of model is numeric>) {
for (i = 0; i < length; i++) {
ids[i] = parseInt(ids[i], 10);
}
}
}

return ids;
}
}



How to write the test ? Any idea ?

Mis63
10 Jun 2011, 1:41 AM
A test could be :


getIds: function() {
var ids = (this.getStorageObject().getItem(this.id) || "").split(","),
length = ids.length,
i;
if (length == 1 && ids[0] == "") {
ids = [];
} else {
for (i = 0; i < length; i++) {
if (!isNaN(ids[i])) {
ids[i] = parseInt(ids[i], 10);
}
}
}
return ids;
}


It works but it is not optimized as :
1. test is done for each id (inside for loop)
2. it does not check datatype of model's id

welshcathy
5 Feb 2012, 9:14 AM
How about this ... ST1 patch that should accept strings and numbers as model ids ... whilst we wait for bug to be patched in ST2 ....

WARNING I've only done basic testing on this so far .. use at your own risk!
I suspect there are other bugs that remain around the setting of the phantom flag ....

Includes additional check when web storage quota is exceeded ....


Ext.override(Ext.data.WebStorageProxy, { /**
* @private
* Saves the array of ids representing the set of all records in the Proxy
* @param {Array} ids The ids to set
*
* PATCHED TO CHECK FOR STORAGE QUOTA OVERFLOW
*/
setIds : function(ids) {

var obj = this.getStorageObject(),
str = ids.join(",");


obj.removeItem(this.id);


if(!Ext.isEmpty(str)) {
try { // + PATCH
obj.setItem(this.id, str);
} catch(e) { // + PATCH
if((e.name).toUpperCase() == 'QUOTA_EXCEEDED_ERR') { // + PATCH
// do something here (fire error event, etc.)
// only when in the desktop ... iPad will prompt you
Ext.Msg.alert('WebStorageProxy', 'You have exceeded your local storage quota');
} // + PATCH
// else throw event to caller // + PATCH
} // + PATCH
}
},

// PATCHED TO CHECK FOR DUPLICATE IDS
create: function(operation, callback, scope) {
var records = operation.records,
length = records.length,
ids = this.getIds(),
id, record, i;

operation.setStarted();


for (i = 0; i < length; i++) {
record = records[i];


if (record.phantom) {
record.phantom = false;
id = this.getNextId();
} else {
id = record.getId();
}


this.setRecord(record, id);

// only add if not already in the key // + PATCH
if (ids.indexOf(id) < 0){ // + PATCH
ids.push(id);
} // + PATCH

}


this.setIds(ids);


operation.setCompleted();
operation.setSuccessful();


if (typeof callback == 'function') {
callback.call(scope || this, operation);
}
},



/**
* @private
* Fetches a model instance from the Proxy by ID. Runs each field's decode function (if present) to decode the data
* @param {String} id The record's unique ID
* @return {Ext.data.Model} The model instance
*
* PATCHED TO ALLOW FOR STRING OR INTEGER MODEL ID
*/
getRecord: function(id) {
if (this.cache[id] == undefined) {
var rawData = Ext.decode(this.getStorageObject().getItem(this.getRecordKey(id))),
data = {},
Model = this.model,
fields = Model.prototype.fields.items,
length = fields.length,
i, field, name, record;


for (i = 0; i < length; i++) {
field = fields[i];
name = field.name;


if (typeof field.decode == 'function') {
data[name] = field.decode(rawData[name]);
} else {
if (field.type.type == 'date') { // +patch
data[name] = new Date(rawData[name]); // +patch
} else { // +patch
data[name] = rawData[name];
} // +patch
}
}


record = new Model(data, id);
// @TODO: make sure we don't have to set phantom to false. we give it an id, so it shouldn't
// be phantom after we created it right?
// record.phantom = false;


this.cache[id] = record;
}

return this.cache[id];
},


/**
* Saves the given record in the Proxy. Runs each field's encode function (if present) to encode the data
* @param {Ext.data.Model} record The model instance
* @param {String} id The id to save the record under (defaults to the value of the record's getId() function)
*
* PATCHED TO ALLOW FOR STRING OR INTEGER MODEL ID
*/
setRecord: function(record, id) {
if (id) {
record.setId(id);
} else {
id = record.getId();
}


var rawData = record.data,
data = {},
model = this.model,
fields = model.prototype.fields.items,
length = fields.length,
i = 0,
field, name, obj, key;


for (i = 0; i < length; i++) {
field = fields[i];
name = field.name;


if (typeof field.encode == 'function') {
data[name] = field.encode(rawData[name], record);
} else {
if (field.type.type == 'date' && Ext.isDate(rawData[name])) { // + PATCH
data[name] = rawData[name].getTime(); // + PATCH
} else { // + PATCH
data[name] = rawData[name];
} // + PATCH
}
}


obj = this.getStorageObject();
key = this.getRecordKey(id);

//keep the cache up to date
this.cache[id] = record;

//iPad bug requires that we remove the item before setting it
obj.removeItem(key);
obj.setItem(key, Ext.encode(data));
},




/**
* @private
* Returns the array of record IDs stored in this Proxy
* @return {Array} The record IDs. Each is cast as a Number
*
* PATCHED TO ALLOW FOR STRING OR INTEGER MODEL ID
*/
getIds: function() {
var ids = (this.getStorageObject().getItem(this.id) || "").split(","),
length = ids.length,
i,
idType = this.getModelIdType(); // + PATCH



if (length == 1 && ids[0] === "") {
ids = [];
} else {
// cast to numbers if model id is a number too
if (idType === 'number'){ // + PATCH
for (i = 0; i < length; i++) { // + PATCH
ids[i] = parseInt(ids[i], 10);
} // + PATCH
}
}


return ids;
},

/*
* NEW METHOD FOR PATCH TO ALLOW FOR STRING OR INTEGER MODEL ID
* get data type of model
* @return {String} data type of the model id
*/
getModelIdType: function(){

// has this already been checked?
// assumes that models won't change their key type dynamically!
if (this.modelIdType){
return this.modelIdType;
}

var model = this.model.prototype,
idProperty = model.idProperty
idField = model.fields.get(idProperty);


this.modelIdType = 'number';

// if idProperty not found in field list -> will be a generated integer
if (idField){
this.modelIdType = idField.type.type;
}
// handle other configuration values
if (this.modelIdType === 'int'){
this.modelIdType = 'number';
}

return this.modelIdType;

},



//inherit
// PATCHED TO ALLOW FOR STRING OR INTEGER MODEL ID
destroy: function(operation, callback, scope) {
var records = operation.records,
length = records.length,
ids = this.getIds(),


//newIds is a copy of ids, from which we remove the destroyed records
newIds = [].concat(ids),
i,
// to ensure correct casting
idType = this.getModelIdType();


for( i = 0; i < length; i++) {


// fix to ensure both numeric and string entries are matched -> removed
if( idType === 'number' ) { // + PATCH
newIds.remove(records[i].getId()); //
} else { // + PATCH
newIds.remove(records[i].getId() + ''); // + PATCH
} // + PATCH


this.removeRecord(records[i], false);
}


this.setIds(newIds);


if( typeof callback == 'function') {
callback.call(scope || this, operation);
}


}




});