Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#9061 closed defect (fixed)

ServiceStore: Should fetchItemByIdentity guarantee a loaded item?

Reported by: Jarrod Carlson Owned by: Kris Zyp
Priority: high Milestone: 1.4
Component: DojoX Data Version: 1.3.0
Keywords: data api identity servicestore filteringselect Cc:
Blocked By: Blocking:

Description

In reference to: source:dojo/data/api/Identity.js#L59

Does this imply that if fetchItemByIdentity is able to find an item, the returned item will be loaded and guaranteed to satisfy calls to store.isItemLoaded(item) == true? If so, then there might be a bug in dojox.data.ServiceStore#fetchItemByIdentity?.

Looking at: source:dojox/data/ServiceStore.js#L362 if the item requested is already in cache, loaded or not, it is returned as is. The issue is that if you make a call to fetchItemByIdentity and do not supply an onItem handler, the item is forcibly loaded from the server, regardless of whether the item is already loaded or not.

store.fetch(); // pre-loads the data for a FilteringSelect

store.fetchItemByIdentity({
  identity: "123",
  onItem: function(item){
    // item is whatever was in cache, loaded or not
  }
});

// For this call, item is forcibly loaded, even though it is in cache
store.fetchItemByIdentity({
  identity: "123"
});

// For this call, item is forcibly loaded, even though it is in cache
store.fetchItemByIdentity({
  identity: "123",
  onItem: function(item){
    // item is fully loaded
  }
});

source:dijit/form/FilteringSelect.js#L142, for example, provides an onItem handler, which means that the FilteringSelect? cannot assume the returned item will be fully loaded (see #9023). Thus, it cannot be assumed that the returned item will have the attribute necessary to populate the display value. Remember that the displayed value may be an attribute other than the item's label by setting FilteringSelect?'s labelAttr property.

The primary reason for this ticket is that I feel the implementation of dojo.data.api.Identity may be somewhat ambiguous in ServiceStore? (and descendent classes). If we are to assume that fetchItemByIdentity will always be given an onItem handler, then it should produce an error when one is not provided. It think it might be presumptuous, though, to assume there is never a good reason to call it without a handler.

Change History (7)

comment:1 Changed 11 years ago by Jarrod Carlson

Correction on the sample code: The comment on the last call to fetchItemByIdentity does not forcibly load the item. It returns an item that is already fully loaded.

That's why the returned item is ambiguous.

comment:2 Changed 11 years ago by Jaanus Heeringson

As the issuer of #9023 i can only give my opinion as an end user, but I can at least describe the behavior as it would benefit me, even though it might conflict with the current data API intentions.

The ability to access partially loaded Items is a good thing - specially in cases of filteringSelect, tree etc. This enables me to optimize the data fetching from the server in cases where the same store is used in multiple views. In some case you just might need label/id, in others the full item. From my point of view it would be beneficial if the decision to load full items would be left to the store, perhaps when you try to access an attribute that does not exist and really try to restrict accessing widgets from making those decisions. In my opinion accessors should not bother with loading of data outside informing users when data really is not available.

comment:3 Changed 11 years ago by bill

Hi squashee,

I'm sure a lot of people would agree w/you. Unfortunately the dojo.data API currently doesn't support this well, since getValue() is a synchronous call, not an asynchronous one (so there's no chance to make an XHR request to a server except if you want to make a sync request, which is bad because it locks up the browser).

This was an intentional design choice.. if all the dojo.data API's were asynchronous (with onItem callbacks) it would make the API much harder to use in the simple case where items are either fully loaded or not.

comment:4 Changed 11 years ago by Jared Jurkiewicz

Owner: changed from Jared Jurkiewicz to Kris Zyp

Kris,

ServiceStore? should probably do an isItemLoaded() check on the item and call loadItem on it if it isn't, then only pass back the fully inflated item to the fetch call. fetchByIdentity() should assume what it hands back is inflated.

comment:5 Changed 11 years ago by bill

Milestone: tbd1.4
Resolution: fixed
Status: newclosed

Kris just fixed this in [17212].

comment:6 Changed 11 years ago by Jaanus Heeringson

*sniff* and I thought it was so convenient with partially loaded items... should never have brought his up.

comment:7 in reply to:  6 Changed 11 years ago by Kris Zyp

Replying to squashee:

*sniff* and I thought it was so convenient with partially loaded items... should never have brought his up.

It was convenient to return partially loaded items from fetch or fetchItemByIdentity? We are not changing the behavior of fetch, nor the behavior of sub-objects (partially loaded items are still allowed embedded in other items).

Note: See TracTickets for help on using tickets.