Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#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.

Forum Discussion

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)

dojox.AndOrWriteStore_revert.patch (6.4 KB) - added by Jared Jurkiewicz 10 years ago.
Similar patch for the AndOr?*Store
dojox.AndOrWriteStore_revert.2.patch (6.4 KB) - added by Jared Jurkiewicz 10 years ago.
Similar patch for the AndOr?*Store
IFWS_revert_9022.patch (6.3 KB) - added by Jared Jurkiewicz 10 years ago.
Patch + UT for this problem.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 10 years ago by Jared Jurkiewicz

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?

comment:2 Changed 10 years ago by Jared Jurkiewicz

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 10 years ago by Jared Jurkiewicz

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 10 years ago by Jared Jurkiewicz

Similar patch for the AndOr?*Store

Changed 10 years ago by Jared Jurkiewicz

Similar patch for the AndOr?*Store

Changed 10 years ago by Jared Jurkiewicz

Attachment: IFWS_revert_9022.patch added

Patch + UT for this problem.

comment:4 Changed 10 years ago by Jared Jurkiewicz

Fixed in [17217] and [17218]

comment:5 Changed 10 years ago by Jared Jurkiewicz

Resolution: fixed
Status: newclosed

comment:6 Changed 10 years ago by Jared Jurkiewicz

Milestone: tbd1.4
Note: See TracTickets for help on using tickets.