#9022 closed defect (fixed)
ItemFileWriteStore does not correctly revert with modified new items
| Reported by: | Phil DeJarnett | Owned by: | Jared Jurkiewicz |
|---|---|---|---|
| Priority: | high | Milestone: | 1.4 |
| Component: | Data | Version: | 1.2.3 |
| Keywords: | ItemFileWriteStore revert newItem | Cc: | |
| Blocked By: | Blocking: |
Description
If a new item is added to an ItemFileWriteStore?, then that item is modified through setValue(), when ItemFileWriteStore?.revert() is called, a null value will be left behind in _arrayOfAllItems, causing unusual errors.
It is important that both adding the new item and modifying the item take place before the revert() for the bug to show up.
Attachments (3)
Change History (9)
comment:1 Changed 12 years ago by
comment:2 Changed 12 years ago by
I ran the test from the forum and I get a different error: Error: dojo.data.ItemFileReadStore?: Invalid item argument.
Which means something is accessing a now dead/removed item.
I put in some trace in revert and revert flows through fine: Done with modified processing. Done with deleted processing. Done with newItem processing. Done with revert.
No errors from revert.
So ... my assumption is the failure is in datagrid itself, where it's trying to look up data on an old handle, perhaps. Going to investigate further.
comment:3 Changed 12 years ago by
Okay, I found the bug and it is in the revert logic with how it updates some indexes. Basically, there's a call to remove a new item from the array of top level items if it happens to be present, the problem is it uses the dojo.indexOf(), and since the object to remove is not the same instance (a copy), it fails to remove it and leaves a bad item in the array.
Looking at possible fixes for this. One which gets rid of the copy and just restores state to the same object instance, and another which does a loop-through. The first will be more efficient, but I worry on other side effects (probably worrying needlessly). So, I want to think on this for a bit.
Changed 12 years ago by
| Attachment: | dojox.AndOrWriteStore_revert.patch added |
|---|
Similar patch for the AndOr?*Store
Changed 12 years ago by
| Attachment: | dojox.AndOrWriteStore_revert.2.patch added |
|---|
Similar patch for the AndOr?*Store
comment:5 Changed 12 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
comment:6 Changed 12 years ago by
| Milestone: | tbd → 1.4 |
|---|
This problem shouldn't exist in 1.2.3 (It did in 1.1.1). The revert flow was explicitly changed so that 'new' items were handled last in the changed so something newed, then modified, didn't cause a revert on blowup.
Do you have an explicit testcase that shows this still failing?