Opened 9 years ago

Closed 3 years ago

Last modified 3 years ago

#11084 closed defect (fixed)

dojox/json/ref.js __parent updated incorrectly/duplicated

Reported by: tadams Owned by: dylan
Priority: high Milestone: 1.7.9
Component: DojoX Data Version: 1.5.0b1
Keywords: Cc:
Blocked By: Blocking:

Description

Usecase: After querying with JSONRestStore, using the double underscore parent property on a given item to traverse up the parent/child structure, results in cyclical parent references (ie a parent that refers to itself). Additionally traversing up the structure using parent, can result in encountering items that do not have a parent property. [the actual property is two underscores followed by parent, but the bug trakker is given me funny behaviour when I use underscore, so I refer to the property here out as 2uparent)

Cause: There are two issues.

Issue 1: The first is that in the walk function of resolveJson in ref.js, when an array property which contains a lazy reference for example:

children:[{$ref:123, name:'xyz}, ...]

is encountered, and the reference is not found in the index table, the method marks that a rewalk will be needed, by adding the array into the reWalk array. Then when the walk method is called for the second time, passing in the rewalk array, the walk method still updates the parent property, namely here:

if(!ref || !val.__parent){
   val.__parent=it;
}

Note that it is setting the parent to 'it', but in this case 'it' is the rewalk collection. This stomps over the real parent, from looking over the code I dont' think it would ever be appropriate to set the parent to the rewalk collection? [Suggested fix's follow at the end of this description].

Issue 2: The 2uparent property can become out of sync with the actual parent item. This occurs because the 2uparent property is being set to 'it', but 'it' may have its properties merged with a definition that already exists in the index table. Specifically the walk function handles this through the local 'target' which is set by default to 'it' but updated if the item is found in the index table. The 2uparent property I believe should be being set to target, not 'it'. The end result of this was a 2uparent property which was not a true reference to the parent.

Suggested Fix:

These are the two fixes I made to my locally to ref.js to address both issues. Change is in the walk method of resolveJson in ref.js

resolveJson(){
...
...
if(!ref || !val.__parent){
 //CHANGE: Add if statement before setting parent
  if (it != reWalk){
    //CHANGE: set parent to target, not 'it'
     val.__parent = target;
  }
   //original
   //val.__parent = it;
}

I don't have a readily available test case, as this occurred in the scope of a larger application. I think there is enough detail here for the issue to be understood, if not I can provide more detail or come up with a standalone test case.

Attachments (1)

test.patch (1.8 KB) - added by Wouter Hager 6 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 9 years ago by Adam Peller

Owner: changed from Adam Peller to Kris Zyp

comment:2 Changed 9 years ago by Kris Zyp

Resolution: fixed
Status: newclosed

(In [22332]) Maintains correct references to parent, fixes #11084

comment:3 Changed 9 years ago by bill

Milestone: tbd1.5

comment:4 Changed 6 years ago by Wouter Hager

It seems to me this was never fixed. In above case 1, __parent is set to an array, not the object containing the array. I think the parent needs to be available to the walk function.

I think the easiest way to fix this is to add an optional parent attribute to walk, and set it to 'it' when reWalking.

comment:5 Changed 6 years ago by Wouter Hager

Also, I don't think arrays should have __parent properties. I don't know what to do with this, it really seems like overly complex code to do something that should be quite simple conceptually.

Last edited 6 years ago by Wouter Hager (previous) (diff)

comment:6 Changed 6 years ago by bill

Milestone: 1.5tbd
Resolution: fixed
Status: closedreopened

Changed 6 years ago by Wouter Hager

Attachment: test.patch added

comment:7 Changed 3 years ago by dylan

Component: DojoxDojoX Data
Milestone: tbd1.11
Owner: changed from Kris Zyp to dylan
Status: reopenedassigned

comment:8 Changed 3 years ago by dylans <dylan@…>

Resolution: fixed
Status: assignedclosed

In 0846a86f1171cc7819d1ea95b58fb903561777b4/dojox:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:9 Changed 3 years ago by dylans <dylan@…>

In 3ee9bd304431012b0095b735ccdafbf4ed90aefc/dojox:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:10 Changed 3 years ago by dylans <dylan@…>

In a295c115eb5553d76232f394831d1e8e8f6f4c98/dojox:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:11 Changed 3 years ago by dylans <dylan@…>

In c26f73a5a8bd956b3da672f1cc865f2cf19bfbce/dojox:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:12 Changed 3 years ago by dylans <dylan@…>

In 15fe4aa38672989ecf3c69cbebed684a36efa4dd/dojox:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:13 Changed 3 years ago by dylan

Milestone: 1.111.7.9

Thanks @wshager for the patch, I've landed it. I'm not going to address the issue about arrays having parents at this time.

Note: See TracTickets for help on using tickets.