#13727 closed defect (fixed)
dojo.store.Memory.setData() 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: |
Description
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.
Change History (5)
comment:1 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:2 Changed 10 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
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
Milestone: | tbd → 1.7 |
---|---|
Priority: | normal → high |
Marking this as high / 1.7 because under current circumstances, the edge case I noted above is a regression introduced in [26723].
In [26723]: