Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#13727 closed defect (fixed) leaves garbage

Reported by: bill Owned by: Kris Zyp
Priority: high Milestone: 1.7
Component: Data Version: 1.6.1
Keywords: Cc:
Blocked By: Blocking:


Looks like resets the data[] array, but doesn't reset the index hash, so index has stale pointers in it.

For example:

var store = new{
	data: [
		{id: 1, name: "one", prime: false},
	{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.

Change History (5)

comment:1 Changed 10 years ago by Kris Zyp

Resolution: fixed
Status: newclosed

In [26723]:

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

comment:2 Changed 10 years ago by Kenneth G. Franqueiro

Resolution: fixed
Status: closedreopened

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.

comment:3 Changed 10 years ago by Kenneth G. Franqueiro

Milestone: tbd1.7
Priority: normalhigh

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

comment:4 Changed 10 years ago by Kris Zyp

Resolution: fixed
Status: reopenedclosed

In [26770]:

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

comment:5 Changed 10 years ago by Kris Zyp

Nice catch, thank you Ken.

Note: See TracTickets for help on using tickets.