PDA

View Full Version : [FIXED] [2.0.1 FINAL] Error when modifying stores



CarlosMS
25 Apr 2012, 7:43 AM
REQUIRED INFORMATION

Ext version tested:

Sencha Touch 2.0.1 FINAL
Browser versions tested against:

Safari 5.1.5
DOCTYPE tested against:

html
Description:

For an existing Ext.data.Store that has been loaded once, (1) deleting an item from the store or (2) re-loading the store and then either modifying an existing store model or adding/deleting an item from it fails with a "TypeError: 'null' is not an object (evaluating 'stores.length') Model.js:1132"
Steps to reproduce the problem:

Create a simple, non-hierarchical Ext.data.Model that includes an 'id' integer field, a few extra string fields, and a date field. Create an Ext.data.Store that associates the model with a proxy, auto-load is set to "true", and defines a simple sorter and grouper (in my case the proxy is a JSON RESTful service). Create an Ext.Container that defines an Ext.dataview.List component and associates the list component with the store. Create CRUD triggers for list events; these could be buttons or similar that permit refreshing the list as well as adding/deleting/modifying items. Run the app and add some items to the list.
For the first case, select an item from the list and delete the item so that the item is removed with a itemStore.remove(currentItem); call and immediately call itemStore.sync();. The remote service will record a DELETE call, and then the browser console will log a "TypeError: 'null' is not an object (evaluating 'stores.length') Model.js:1132" Any subsequent item delete calls will trigger the exact same behavior and error.
For the second case, run the application and make sure there are items loaded (ie. the service already returns existing records). After the application is up and displaying the items' list, trigger a store re-load by calling Ext.getStore("Items").load(); (in my example my store's name is 'Items'). The item list will refresh without problems. Now, either modify an existing item in the store or add a new item to the store. In this "save" operation of the new/edited item call itemsStore.add(currentItem); follows by a itemsStore.sync();. The remote service will record a POST call for an added item, followed by an unexpected DELETE for the first item in the store, and then the browser console will log a "TypeError: 'null' is not an object (evaluating 'stores.length') Model.js:1132" Any subsequent item add/edit calls will trigger the exact same behavior and error.
The result that was expected:

CRUD operations on the store should behave properly. This functionality works correctly in Sencha Touch 2.0.0 and 2.0.1RC. There have been no application code changes prior to testing with 2.0.1 FINAL.
The result that occurs instead:

"TypeError: 'null' is not an object (evaluating 'stores.length') Model.js:1132" error on item add/edit when a store is modified via a store re-load, or when an item is deleted from the store.
Test Case:
Relevant code parts from my Item.js store class


onReloadItemsCommand: function() {
console.log("onReloadCommand");

Ext.getStore("Items").load();
}

onSaveItemCommand: function() {
console.log("onSaveItemCommand");

var itemEditor = this.getItemEditor();
var currentItem = itemEditor.getRecord();
var newValues = itemEditor.getValues();
currentItem.set("name", newValues.name);
currentItem.set("priority", newValues.priority);

var errors = currentItem.validate();
if (!errors.isValid()) {
Ext.Msg.alert(null, errors.getByField('name')[0].getMessage(), Ext.emptyFn);
currentItem.reject();

return;
}

var itemsStore = Ext.getStore("Items");
if (itemsStore.findRecord('id', currentItem.data.id) == null) {
itemsStore.add(currentItem);
}
itemsStore.sync();
itemsStore.sort([{ property: 'addedTimestamp', direction: 'DESC'}]);

this.activateWishList();
}

onDeleteItemCommand: function() {
console.log("onDeleteItemCommand");

var itemEditor = this.getItemEditor();
var currentItem = itemEditor.getRecord();

var itemList = this.getItemList();
var itemStore = itemList.getStore();
if (itemStore.findRecord('id', currentItem.data.id)) {
itemStore.remove(currentItem);
}
itemStore.sync();
itemList.refresh();

this.activateWishList();
}

HELPFUL INFORMATION

Screenshot or Video:

None
See this URL for live test case:

None
Debugging already done:

Yes, as described in the sections Steps to reproduce problem, The result that was expected, The result that occurs instead, and Possible fix.
Possible fix:

None provided. This seems to be related to work done in "[TOUCH-2641, TOUCH-2682, TOUCH-2689] Fixed memory leaks related to removing items from Collections, Model Cache and Stores," which in turn seems to be documented in at least the following threads: http://www.sencha.com/forum/showthread.php?191718-store-garbage-collection&highlight=garbage and http://www.sencha.com/forum/showthread.php?192844-Garbage-collection-of-Models-(ST2)&highlight=stores.length. The override code provided in the former thread looks exactly like the code that now exists in 2.0.1 FINAL's Model.js and is failing.
Additional CSS used:

All defaults
Operating System:

Mac OS X 10.7.3

mitchellsimoens
25 Apr 2012, 8:24 AM
I'm a little lost in your explanation. I created this test case and tapping on any button in any order I'm not getting any errors using 2.0.1 final. What steps do I need to take or modify this test case to reproduce?


Ext.define('MyModel', {
extend : 'Ext.data.Model',

config : {
fields : [
'id',
'text'
],
proxy : {
type : 'ajax',
url : 'data/json.json'
}
}
});

new Ext.dataview.List({
fullscreen : true,
itemTpl : '{id} {text}',
store : new Ext.data.Store({
model : 'MyModel'
}),
items : [
{
xtype : 'toolbar',
docked : 'top',
items : [
{
text : 'Load',
handler : function (button) {
var list = button.up('list'),
store = list.getStore();

store.load();
}
},
{
text : 'Create',
handler : function (button) {
var list = button.up('list'),
store = list.getStore();

store.add({
id : 10,
text : 'Ten'
});
store.sync();
}
},
{
text : 'Edit',
handler : function (button) {
var list = button.up('list'),
store = list.getStore(),
rec = store.getAt(1);

rec.set('text', 'Twenty');
store.sync();
}
},
{
text : 'Delete',
handler : function (button) {
var list = button.up('list'),
store = list.getStore(),
rec = store.getAt(1);

store.remove(rec);
store.sync();
}
}
]
}
]
});

With this JSON:


[
{
"id" : 1,
"text" : "One"
},
{
"id" : 2,
"text" : "Two"
},
{
"id" : 3,
"text" : "Three"
},
{
"id" : 4,
"text" : "Four"
},
{
"id" : 5,
"text" : "Five"
}
]

CarlosMS
25 Apr 2012, 8:49 AM
Hello Mitchell,

The error happens only after calling the store's "sync()" right after the "add()" and "remove()" calls in my code snippets. I notice that in the test case you created "sync()" is not being called. The "sync()" will trigger the remote call CRUD operations during the internal model management logic where I believe the problem lies.

mitchellsimoens
25 Apr 2012, 8:52 AM
Hello Mitchell,

The error happens only after calling the store's "sync()" right after the "add()" and "remove()" calls in my code snippets. I notice that in the test case you created "sync()" is not being called. The "sync()" will trigger the remote call CRUD operations during the internal model management logic where I believe the problem lies.

Now I see it and I have updated my code. Looks like the stores property of the records are null so will open the bug.

CarlosMS
25 Apr 2012, 9:17 AM
Thank you Mitchell.

Of no particular value (so feel free to ignore the following note), I did a knee-jerk override of Model.js' "notifyStores()" with a quick and dirty modification of the line at fault so that it would do an "ln = (stores) ? stores.length : 0"; that eliminated the error but ended up messing up badly the store's model management, triggering the addition of duplicate models, and deletion of the whole contents of the store on multiple re-loads. I didn't try anything else because I am not an expert on Sencha Touch's internals.

For the time being I need to revert to 2.0.1RC as my application no longer works because of this issue, and I have an app. demo to do in a few days.

Would love to hear back if/when there is at least a temporary work around to get things back on track.

TommyMaintz
25 Apr 2012, 9:48 AM
Hi Carlos,

Thanks for the detailed test report. Unfortunately these issues were already resolved in the codebase but I commited it to the 2.1 branch by mistake and thus they didn't make it into the 2.0.1 release. We are investigating the best way of notifying others about this but anyway, here is an override that should solve the issues you were running into.



Ext.define('Ext.data.StoreFix', {
override: 'Ext.data.Store',

remove: function (records) {
if (records.isModel) {
records = [records];
}

var me = this,
sync = false,
i = 0,
autoSync = this.getAutoSync(),
syncRemovedRecords = me.getSyncRemovedRecords(),
destroyRemovedRecords = this.getDestroyRemovedRecords(),
ln = records.length,
indices = [],
removed = [],
isPhantom,
items = me.data.items,
record, index, j;

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

if (me.data.contains(record)) {
isPhantom = (record.phantom === true);

index = items.indexOf(record);
if (index !== -1) {
removed.push(record);
indices.push(index);
}

record.unjoin(me);
me.data.remove(record);

if (destroyRemovedRecords && !syncRemovedRecords && !record.stores.length) {
record.destroy();
}
else if (!isPhantom && syncRemovedRecords) {
// don't push phantom records onto removed
me.removed.push(record);
}

sync = sync || !isPhantom;
}
}

me.fireEvent('removerecords', me, removed, indices);

if (autoSync && sync) {
me.sync();
}
},

doRemoveAll: function (silent) {
var me = this,
destroyRemovedRecords = this.getDestroyRemovedRecords(),
syncRemovedRecords = this.getSyncRemovedRecords(),
records = me.data.all.slice(),
ln = records.length,
i, record;

for (i = 0; i < ln; i++) {
record = records[i];
record.unjoin(me);

if (destroyRemovedRecords && !syncRemovedRecords && !record.stores.length) {
record.destroy();
}
else if (record.phantom !== true && syncRemovedRecords) {
me.removed.push(record);
}
}

me.data.clear();

if (silent !== true) {
me.fireEvent('refresh', me, me.data);
}

if (me.getAutoSync()) {
this.sync();
}
}
});

Ext.define('Ext.data.ModelFix', {
override: 'Ext.data.Model',

constructor: function (data, id, raw, convertedData) {
var me = this,
cached = null,
idProperty = this.getIdProperty();

/**
* @property {Object} modified key/value pairs of all fields whose values have changed.
* The value is the original value for the field.
*/
me.modified = {};

/**
* @property {Object} raw The raw data used to create this model if created via a reader.
*/
me.raw = raw || data || {};

/**
* @property {Array} stores
* An array of {@link Ext.data.Store} objects that this record is bound to.
*/
me.stores = [];

data = data || convertedData || {};

// We begin by checking if an id is passed to the constructor. If this is the case we override
// any possible id value that was passed in the data.
if (id || id === 0) {
// Lets skip using set here since it's so much faster
data[idProperty] = me.internalId = id;
}

id = data[idProperty];
if (id || id === 0) {
cached = Ext.data.Model.cache[Ext.data.Model.generateCacheId(this, id)];
if (cached) {
return cached.mergeData(convertedData || data || {});
}
}

if (convertedData) {
me.setConvertedData(data);
} else {
me.setData(data);
}

// If it does not have an id at this point, we generate it using the id strategy. This means
// that we will treat this record as a phantom record from now on
id = me.data[idProperty];
if (!id && id !== 0) {
me.data[idProperty] = me.internalId = me.id = me.getIdentifier().generate(me);
me.phantom = true;

if (this.associations.length) {
this.handleInlineAssociationData(data);
}
} else {
me.id = me.getIdentifier().generate(me);
}

Ext.data.Model.cache[Ext.data.Model.generateCacheId(me)] = me;

if (this.init && typeof this.init == 'function') {
this.init();
}
}
});


Best,
Tommy

CarlosMS
25 Apr 2012, 10:11 AM
Hello Tommy,

Thank you for the quick turn-around and the overrides. I am not too familiar with the best way to implement/run overrides. In my application, what is the best way to handle platform overrides? Specifically, should the override code (above) live in a separate JS file? And, at which point in my application should I call/execute the override code (e.x. somewhere in my application's "app.js" file, or in a specific "init" function, etc.)?

Thanks again!

Jamie Avins
25 Apr 2012, 10:34 AM
You would want this to be called before any of your stores are created. You can include them as a requirement of you application in a separate file, or as the first part of the launch method.

CarlosMS
25 Apr 2012, 11:35 AM
Thanks Jamie. I still had to do some detective work but figured out what I may have been doing wrong integrating the overrides. This is what I did to integrate the overrides (feel free to let me know if I misunderstood something):
Created an "overrides.js" file and pasted Tommy's code in it. I placed the new file at the same level of the "app.js"file for now.
I had to edit Tommy's code so that
Ext.override('Ext.data.StoreFix', { override: 'Ext.data.Store'... becomes
Ext.define('Ext.data.StoreFix', { override: 'Ext.data.Store'... and
Ext.override('Ext.data.ModelFix', { override: 'Ext.data.Model'... becomes
Ext.define('Ext.data.ModelFix', { override: 'Ext.data.Model'...
Last I edited "app.json" in order to load "overrides.js" after "sencha-touch.js" but before "app.js", which makes things in that file look like this
... "js": [
{ "path": "sdk/sencha-touch.js" },
{
"path": "overrides.js"
},
{
"path": "app.js",
"bundle": true, /* Indicates that all class dependencies are concatenated into this file when build */
"update": "delta"
}] ...
Re-ran the application, tested all CRUD operations as I described, and things are now working A-OK! Many thanks to you and your team for the quick turn-around on this issue.

Jamie Avins
25 Apr 2012, 11:59 AM
In his haste, Tommy made the mistake of using an old syntax. I'll update his post. Looks good.

CarlosMS
25 Apr 2012, 1:19 PM
Great, thanks for the feedback.

olouvignes
25 Apr 2012, 3:45 PM
Is this fix still required if I re-download 2.0.1 now?

Jamie Avins
25 Apr 2012, 4:34 PM
Is this fix still required if I re-download 2.0.1 now?

Yes.

tareed
29 Apr 2012, 9:30 AM
After upgrading to 2.0.1 final I was getting an error IndexOf - null at sencha-touch.js at line 1551 when reloading a store of a nested list. This would occur if the structure (depth) of the list changed.

The fix in this post solved the problem.

FrankK
2 May 2012, 1:43 PM
I'm actually still having issues after this fix. If the store is empty and I am adding the first record, I am still seeing the random DELETE action come out to my REST API.

FrankK
2 May 2012, 2:36 PM
My issue actually seems be a problem with how I implemented my stores. I'm using two stores in a customer/order type configuration. Depending on which customer is selected, the order store will only be populated with the orders for that customer (customer 1). If you navigate back one level and choose a different customer (customer 2), all of the orders from the customer 1 were automatically removed on the load of the new records. The next time I sync the store, all of the orders from customer 1 get "DELETE" actions sent to my REST API.

I'll try and find a workaround for my issue, but you can disregard my previous comment as it appears the previous override provided is working correctly.

CarlosMS
2 May 2012, 3:08 PM
Thanks for the follow-up to this thread Frank. Even if unrelated to this bug, I would like to hear how you go about solving your issue. I'm working on a design pretty similar to yours and may run into a similar situation.

FrankK
3 May 2012, 5:00 AM
I was able to fix this by using the model.erase() method to delete records from my REST API and local store. In the store, I added "syncRemovedRecords: false" to the config. It works for my case as I only ever delete one record at a time, but if you wanted to remove more than one record I'm not sure how you would accomplish that.

CarlosMS
3 May 2012, 5:12 PM
Thank you for the follow-up Frank!

acajulis
8 May 2012, 6:42 PM
I tried to incorporate this workaround for Sencha 2.0.0.

In Safari, I get an error since it can't find this.getDestroyRemovedRecords(). Do I need Sencha 2.0.1?