Changeset 9837

Timestamp:

07/27/07 14:29:47 (16 months ago)

Author:

skinner

Message:

fixes #3873 -- Bug in ItemFileWriteStore? that would show up when using a file that did not specify an identifier and then calling newItem() after calling deleteItem(). Changed deleteItem() to place null values in the _arrayOfAllItems, rather than removing elements from the array. Should also improve performance of deleteItem() and revert().

Location:

dojo/trunk

Files:

• data/ItemFileReadStore.js (modified) (4 diffs)
• data/ItemFileWriteStore.js (modified) (5 diffs)
• tests/data/ItemFileWriteStore.js (modified) (1 diff)

dojo/trunk/data/ItemFileReadStore.js

@@ -233 +233 @@
                 var self = this;
                 var filter = function(requestArgs, arrayOfItems){
-                        var items = null;
-
+                        var items = [];
                         if(requestArgs.query){
                                 var ignoreCase = requestArgs.queryOptions ? requestArgs.queryOptions.ignoreCase : false; 
-                                items = [];
 
                                 //See if there are any string values that can be regexp parsed first to avoid multiple regexp gens on the
@@ -252 +250 @@
                                         var match = true;
                                         var candidateItem = arrayOfItems[i];
-                                        for(var key in requestArgs.query) {
-                                                var value = requestArgs.query[key];
-                                                if (!self._containsValue(candidateItem, key, value, regexpList[key])){
-                                                        match = false;
+                                        if(candidateItem === null){
+                                                match = false;
+                                        }else{
+                                                for(var key in requestArgs.query) {
+                                                        var value = requestArgs.query[key];
+                                                        if (!self._containsValue(candidateItem, key, value, regexpList[key])){
+                                                                match = false;
+                                                        }
                                                 }
                                         }
@@ -264 +266 @@
                                 findCallback(items, requestArgs);
                         }else{
-                                // We want a copy to pass back in case the parent wishes to sort the array.  We shouldn't allow resort 
-                                // of the internal list so that multiple callers can get lists and sort without affecting each other.
-                                if(arrayOfItems.length> 0){
-                                        items = arrayOfItems.slice(0,arrayOfItems.length); 
+                                // We want a copy to pass back in case the parent wishes to sort the array. 
+                                // We shouldn't allow resort of the internal list, so that multiple callers 
+                                // can get lists and sort without affecting each other.  We also need to
+                                // filter out any null values that have been left as a result of deleteItem()
+                                // calls in ItemFileWriteStore.
+                                for(var i = 0; i < arrayOfItems.length; ++i){
+                                        var item = arrayOfItems[i];
+                                        if(item !== null){
+                                                items.push(item);
+                                        }
                                 }
                                 findCallback(items, requestArgs);
@@ -454 +462 @@
 
                 var identifier = dataObject.identifier;
+                this._itemsByIdentity = {};
                 if(identifier){
                         this._features['dojo.data.api.Identity'] = identifier;
-                        this._itemsByIdentity = {};
                         for(i = 0; i < this._arrayOfAllItems.length; ++i){
                                 item = this._arrayOfAllItems[i];

dojo/trunk/data/ItemFileWriteStore.js

@@ -176 +176 @@
                 this._assertIsItem(item);
 
-                var found = this._removeArrayElement(this._arrayOfAllItems, item);
-                if(found){
-                        var identity = this.getIdentity(item);
-                        item[this._storeRefPropName] = null;
-                        delete this._itemsByIdentity[identity];
-                        this._pending._deletedItems[identity] = item;
-                        this._updateItemIdIndexValues();
-                        this.onDelete(item); // dojo.data.api.Notification call
-                        
-                        //Remove from the toplevel items, if necessary...
-                        if(item[this._rootItemPropName]){
-                                this._removeArrayElement(this._arrayOfTopLevelItems, item);
-                        }
-                        return true;
-                }
-                return false; // boolean
+                // remove this item from the _arrayOfAllItems, but leave a null value in place
+                // of the item, so as not to change the length of the array, so that in newItem() 
+                // we can still safely do: newIdentity = this._arrayOfAllItems.length;
+                var indexInArrayOfAllItems = item[this._itemNumPropName];
+                this._arrayOfAllItems[indexInArrayOfAllItems] = null;
+                
+                var identity = this.getIdentity(item);
+                item[this._storeRefPropName] = null;
+                delete this._itemsByIdentity[identity];
+                this._pending._deletedItems[identity] = item;
+                
+                //Remove from the toplevel items, if necessary...
+                if(item[this._rootItemPropName]){
+                        this._removeArrayElement(this._arrayOfTopLevelItems, item);
+                }
+                this.onDelete(item); // dojo.data.api.Notification call
+                return true;
         },
 
@@ -351 +352 @@
                 for(var i = 0; i < this._arrayOfAllItems.length; ++i){
                         var item = this._arrayOfAllItems[i];
-                        serializableItem = {};
-                        for(var key in item){
-                                if(key !== this._storeRefPropName && key !== this._itemNumPropName){
-                                        var attribute = key;
-                                        var valueArray = this.getValues(item, attribute);
-                                        if(valueArray.length == 1){
-                                                serializableItem[attribute] = this._flatten(valueArray[0]);
-                                        }else{
-                                                var serializableArray = [];
-                                                for(var j = 0; j < valueArray.length; ++j){
-                                                        serializableArray.push(this._flatten(valueArray[j]));
-                                                        serializableItem[attribute] = serializableArray;
+                        if(item !== null){
+                                serializableItem = {};
+                                for(var key in item){
+                                        if(key !== this._storeRefPropName && key !== this._itemNumPropName){
+                                                var attribute = key;
+                                                var valueArray = this.getValues(item, attribute);
+                                                if(valueArray.length == 1){
+                                                        serializableItem[attribute] = this._flatten(valueArray[0]);
+                                                }else{
+                                                        var serializableArray = [];
+                                                        for(var j = 0; j < valueArray.length; ++j){
+                                                                serializableArray.push(this._flatten(valueArray[j]));
+                                                                serializableItem[attribute] = serializableArray;
+                                                        }
                                                 }
                                         }
                                 }
-                        }
-                        serializableStructure.items.push(serializableItem);
+                                serializableStructure.items.push(serializableItem);
+                        }
                 }
                 var prettyPrint = true;
@@ -436 +439 @@
                         // make the original item into a full-fledged item again
                         originalItem[this._storeRefPropName] = this;
-                        //originalItem[this._itemNumPropName] = identity; // WRONG! this should be a number N, the index into this._arrayOfAllItems
                         modifiedItem[this._storeRefPropName] = null;
 
                         // replace the modified item with the original one
-                        this._removeArrayElement(this._arrayOfAllItems, modifiedItem);
-                        this._arrayOfAllItems.push(originalItem);
+                        var arrayIndex = modifiedItem[this._itemNumPropName];
+                        this._arrayOfAllItems[arrayIndex] = originalItem;
+                        
                         if(modifiedItem[this._rootItemPropName]){
-                                this._removeArrayElement(this._arrayOfTopLevelItems, modifiedItem);
-                                this._arrayOfTopLevelItems.push(originalItem);
+                                arrayIndex = modifiedItem[this._itemNumPropName];
+                                this._arrayOfTopLevelItems[arrayIndex] = originalItem;
                         }
                         this._itemsByIdentity[identity] = originalItem;
@@ -452 +455 @@
                         deletedItem[this._storeRefPropName] = this;
                         this._itemsByIdentity[identity] = deletedItem;
-                        this._arrayOfAllItems.push(deletedItem);
+                        var index = deletedItem[this._itemNumPropName];
+                        this._arrayOfAllItems[index] = deletedItem;
                         if(deletedItem[this._rootItemPropName]){
                                 this._arrayOfTopLevelItems.push(deletedItem);
@@ -462 +466 @@
                         _deletedItems:{}
                 };
-                this._updateItemIdIndexValues();
                 return true; // boolean
-        },
-        
-        _updateItemIdIndexValues: function(){
-                for(var i = 0; i < this._arrayOfAllItems.length; ++i){
-                        var item = this._arrayOfAllItems[i];
-                        item[this._itemNumPropName] = i;
-                }
         },
         

dojo/trunk/tests/data/ItemFileWriteStore.js

@@ -734 +734 @@
                         }
                         doh.assertTrue(passed);
+                },
+                function testIdentityAPI_noIdentifierSpecified(){
+                        //      summary: 
+                        //              Test for bug #3873. Given a datafile that does not specify an
+                        //              identifier, make sure ItemFileWriteStore auto-creates identities 
+                        //              that are unique even after calls to deleteItem() and newItem()
+
+                        var args = {data: {
+                                label:"name",
+                                items:[
+                                        {name:'Ecuador', capital:'Quito'},
+                                        {name:'Egypt', capital:'Cairo'},
+                                        {name:'El Salvador', capital:'San Salvador'},
+                                        {name:'Equatorial Guinea', capital:'Malabo'},
+                                        {name:'Eritrea', capital:'Asmara'},
+                                        {name:'Estonia', capital:'Tallinn'},
+                                        {name:'Ethiopia', capital:'Addis Ababa'}
+                                ]
+                        } }; 
+                        var store = new dojo.data.ItemFileWriteStore(args);
+                        var deferred = new doh.Deferred();
+                        
+                        function onError(error, request){
+                                deferred.errback(error);
+                        }
+                        function onComplete(items, request){
+                                doh.assertEqual(7, items.length);
+                                
+                                var lastItem = items[(items.length - 1)];
+                                var idOfLastItem = store.getIdentity(lastItem);
+                                store.deleteItem(lastItem);
+                                store.newItem({name:'Canada', capital:'Ottawa'});
+                                
+                                function onCompleteAgain(itemsAgain, requestAgain){
+                                        doh.assertEqual(7, itemsAgain.length);
+                                        var identitiesInUse = {};
+                                        for(var i = 0; i < itemsAgain.length; ++i){
+                                                var item = itemsAgain[i];
+                                                var id = store.getIdentity(item);
+                                                if(identitiesInUse.hasOwnProperty(id)){
+                                                        // there should not already be an entry for this id
+                                                        doh.assertTrue(false);
+                                                }else{
+                                                        // we want to add the entry now
+                                                        identitiesInUse[id] = item;
+                                                }
+                                        }
+                                        deferred.callback(true);
+                                }
+                                store.fetch({onComplete:onCompleteAgain, onError:onError});
+                        }
+                        
+                        store.fetch({onComplete:onComplete, onError:onError});
+                        return deferred;
                 }
         ]