Opened 10 years ago

Closed 10 years ago

Last modified 9 years ago

#10569 closed defect (invalid)

Tree: does not work when working with dojox.data.JsonRestStore

Reported by: riceyeh Owned by:
Priority: high Milestone: tbd
Component: Dijit Version: 1.4.0
Keywords: Cc: Jared Jurkiewicz
Blocked By: Blocking:

Description (last modified by bill)

Hi,

I find a bug in dijit.tree.TreeStoreModel which causes dijit.Tree not working when associated with dojox.data.JsonRestStore. This bug causes that child items lazily loaded by store are not correctly updated to model. Each item in childItems at line 162 in dijit.tree.TreeStoreMode source code stands for the set of children to be loaded when children attribute is set to {$ref: ...} for lazily loading. That is, an item in childItems could fetch back a set of items. It is this set of items supposed to be updated to model. The following is the change I have done to the source code.

For the original mail I post to dojo-interest, see http://mail.dojotoolkit.org/pipermail/dojo-interest/2009-December/041883.html.

               var children = [];
line 158                      // still waiting for some or all of the items to load
                var onItem = function onItem(item){
                    dojo.forEach(item, function(n) { children.push(n) });
                    if(--_waitCount == 0){
                        // all nodes have been loaded, send them to the tree
line 162                        onComplete(children);
                    }
                }

Attachments (1)

loadItem.patch (999 bytes) - added by bill 10 years ago.
patch to allow for onItem() callback returning a pointer to a different Object that the original unloaded item

Download all attachments as: .zip

Change History (13)

comment:1 Changed 10 years ago by bill

Description: modified (diff)
Summary: dijit.Tree does not work when working with dojox.data.JsonRestStoreTree: does not work when working with dojox.data.JsonRestStore

Thanks for the ticket.

It looks like your code will fail in the case where some of the children are already loaded and some of them aren't. Probably a rare case but we should handle it.

My question is why the current code isn't working with JsonRestStore. Can you supply a test case?

In the code below (for !jsonRestStore, you are saying that item != item2

if(!store.isItemLoaded(item)){
    store.loadItem({
	item: item,
	onItem: function(item2){  // item2 != item1 ??? }
    }
}

comment:2 Changed 10 years ago by riceyeh

Yes, item2 != item1. In my case,

item1 = {"$ref": "http://localhost:8080/personnel/bureaucracy/object/2/children" }

item2 = {

"class": "xs.personnel.Organization", "name": "Finance", "oid": 5, "type": "department", "effectivePeriod": {

"start": /Date(1259396001526)/, "end": null

}, "children": {

"$ref": "http://localhost:8080/personnel/bureaucracy/object/5/children"

}

}, {

"class": "xs.personnel.Organization", "name": "Logistics", "oid": 14, "type": "department", "effectivePeriod": {

"start": /Date(1259396001526)/, "end": null

}, "children": {

"$ref": "http://localhost:8080/personnel/bureaucracy/object/14/children"

}

}, {

"class": "xs.personnel.Position", "name": "President", "oid": 7, "type": "President", "effectivePeriod": {

"start": /Date(1259396001526)/, "end": null

}, "children": {

"$ref": "http://localhost:8080/personnel/bureaucracy/object/7/children"

}

}

comment:3 in reply to:  2 Changed 10 years ago by riceyeh

Replying to riceyeh: I missed [ and ] to enclose item2 in my previous post. Further, I think the following code modification can solve the problem when some of the children are loaded.

dojo.forEach(childItems, function(item){

if(!store.isItemLoaded(item)){

store.loadItem({

item: item, onItem: onItem, onError: onError

});

} else {

children.push(item);

}

});

Yes, item2 != item1. In my case,

item1 = {"$ref": "http://localhost:8080/personnel/bureaucracy/object/2/children" }

item2 = {

"class": "xs.personnel.Organization", "name": "Finance", "oid": 5, "type": "department", "effectivePeriod": {

"start": /Date(1259396001526)/, "end": null

}, "children": {

"$ref": "http://localhost:8080/personnel/bureaucracy/object/5/children"

}

}, {

"class": "xs.personnel.Organization", "name": "Logistics", "oid": 14, "type": "department", "effectivePeriod": {

"start": /Date(1259396001526)/, "end": null

}, "children": {

"$ref": "http://localhost:8080/personnel/bureaucracy/object/14/children"

}

}, {

"class": "xs.personnel.Position", "name": "President", "oid": 7, "type": "President", "effectivePeriod": {

"start": /Date(1259396001526)/, "end": null

}, "children": {

"$ref": "http://localhost:8080/personnel/bureaucracy/object/7/children"

}

}

comment:4 Changed 10 years ago by bill

Cc: Jared Jurkiewicz added

OK, thanks, that explains the problem.

Jared, do you have thoughts on whether it's kosher for loadItem() to switch out the item, so that the loaded item is a different Object from the unloaded item, rather than just filling in attributes in the old item Object?

Changed 10 years ago by bill

Attachment: loadItem.patch added

patch to allow for onItem() callback returning a pointer to a different Object that the original unloaded item

comment:5 Changed 10 years ago by bill

PS: Riceyeh, your patch looks good except that it might put the items out of order. Does the patch I attached work for you?

comment:6 in reply to:  5 Changed 10 years ago by riceyeh

Replying to bill:

No, your patch does not work for my case. It shows the following error:

[Widget dijit.Tree, bureaucracy] _connects=[4] _subscribes=[0] : error loading root children: ReferenceError?: onItem is not defined message=onItem is not defined

I think, there is one point I am going to clarify. In your code, for a parent tree node, it seems that you assume that some of its child nodes are lazily loaded. In my case, it is the set of child nodes of the parent tree node are lazily loaded as a whole. Hence, in terms of code, there is only one item in childItems array, and the only one item will load many child items as an array. So, in my case, there is no chance to put the child items out of order.

Regards, Rice

comment:7 Changed 10 years ago by bill

Ah, OK, because your model has deferItemLoadingUntilExpand = true?

comment:8 in reply to:  7 Changed 10 years ago by riceyeh

Replying to bill:

Ah, OK, because your model has deferItemLoadingUntilExpand = true?

No.

comment:9 Changed 10 years ago by bill

Resolution: invalid
Status: newclosed

I talked to Jared about this.

The thing is that you aren't really following the rules of dojo.data. Your children attribute looks like an unloaded (single) item, yet when we call loadItem() it suddenly turns into an array.

|I understand why you did it the way you did, because it's more efficient to not query an item's children (or even query how many children an item has) until you need them.

What you need to do is to either write your own tree model (see the I18N demo for an example), or subclass an existing model, and override the getChildren() method. It should do the

http://localhost:8080/personnel/bureaucracy/object/2/children

query to get the list of children and then return the results. I also recommend to stop using the

"children": {
      "$ref": "http://localhost:8080/personnel/bureaucracy/object/2/children"
}

syntax in your JSON because of the item/array confusion, although I guess you could still get everything to work without changing that.

comment:10 in reply to:  9 Changed 10 years ago by riceyeh

Replying to bill:

I talked to Jared about this.

The thing is that you aren't really following the rules of dojo.data. Your children attribute looks like an unloaded (single) item, yet when we call loadItem() it suddenly turns into an array.

|I understand why you did it the way you did, because it's more efficient to not query an item's children (or even query how many children an item has) until you need them.

What you need to do is to either write your own tree model (see the I18N demo for an example), or subclass an existing model, and override the getChildren() method. It should do the

http://localhost:8080/personnel/bureaucracy/object/2/children

query to get the list of children and then return the results. I also recommend to stop using the

"children": {
      "$ref": "http://localhost:8080/personnel/bureaucracy/object/2/children"
}

syntax in your JSON because of the item/array confusion, although I guess you could still get everything to work without changing that.

Maybe a more meaningful syntax, like $ref:http://localhost:8080/personnel/bureaucracy/object/2/children?, should be supported in dojox.json.ref.

Rice

comment:11 Changed 10 years ago by bill

(In [21176]) Usually loadItem(item) just fills in attribute inside the specified Object, but theoretically onItem(item) could return a completely new object. Refs #10569 !strict.

comment:12 Changed 9 years ago by Adam Peller

(In [21525]) Fix item/onItem references, regression from [21176] Refs #10569

Note: See TracTickets for help on using tickets.