dojo.store.Memory.setData() leaves garbage

[bill]

Looks like dojo.store.Memory.setData() resets the data[] array, but doesn't reset the index hash, so index has stale pointers in it.

For example:

var store = new dojo.store.Memory({
	data: [
		{id: 1, name: "one", prime: false},
	]
});
store.setData([
	{id: 2, name: "one", prime: true}
]);
store.get(1) ==> returns 1 (even though it shouldn't return anything)

I'm not sure you want a (public) setData() method at all, since it likely will break people observing existing queries, etc. But if you do, then should move the this.index = {} from the constructor to setData(). Should also add tests for setData(), like above.

[Kris Zyp]

In [26723]:

Optimize Memory store for better add()/put() performance, reset index on setData, fixes #13989, fixes #13727 !strict

[Kenneth G. Franqueiro]

I'm reopening this due to an edge case I just accidentally ran into last night.

With the new code changes, the remove function has side effects if you accidentally pass it a nonexistent id, or no id at all. It ends up removing an item anyway, because splice(undefined, 1) behaves like splice(0, 1).

This could be guarded against by checking typeof this.index[id] != "undefined" first.

[Kenneth G. Franqueiro]

Marking this as high / 1.7 because under current circumstances, the edge case I noted above is a regression introduced in [26723].

[Kris Zyp]

In [26770]:

Fix remove() for non-existent ids, fixes #13727 !strict

[Kris Zyp]

Nice catch, thank you Ken.