Community
Participate
Working Groups
Hi Martin! Object are added to resource.getContents() only in StoreResource.load(Map): if (!elist.contains(obj)) // can maybe happen with extents? { StoreUtil.setEResource((InternalEObject) obj, this, true); elist.basicAdd(obj, null); attached((EObject) obj); } but notification is not set. When objects are lazy loaded (into lists e.g.) StoreUtil.setEResource() is called but this method doesn't add objects to resource.getContents cause when currentEObject.eSetResource(resource, null); is called inverse end of link (resource.getContents()) remains the same - you can see it debug mode. BTW I have attached patch. I will be happy if my patch is OK and will be included in the next teneo build:
Created attachment 54324 [details] patch I have tested it - it works for me
Hi Ilya, Thanks for the patch, a few questions though. I could not see changes for notifications (and its related option) in case of lazy loading of lists. This was not required anymore in your case? Also imo the change should take into account if the resource is loading and the notification during load option (we discussed on the newsgroup). If okay, I can look at this. Ofcourse if you have time and want to look at this then this good for me also. gr. Martin
Hi Martin! As you can see I have removed elist.basicAdd from StoreResource and inserted it in StoreUtil.setEResource. When object is lazy loaded then StoreUtil.setEResource() is called cause resource.attached(object) was called before - so in my patch notifications are ALWAYS SENT - cause notification is created in NotifyingListImpl.basicAdd() and I just dispatch it - you can try it. I suppose that behavior when notification is always sent (if there are listeners - but it is determined in NotifyingListImpl.basicAdd()) is the most correct. I was thinking about options and came to conclusion (you could disagree of course) that options would complicate all code. Here why thinking why they will complicate all: If you try to investigate how XML loading with XMLResource.OPTION_DISABLE_NOTIFY=true works you will see that during loading org.eclipse.emf.ecore.xmi.impl.XMLHandler calls eObject.eSetDeliver(false) - it affects dispatching notifications FOR ALL OBJECTS. But after loading XMLHandler calls eObject.eSetDeliver(true) FOR ALL OBJECTS. So EMF common way to control dispatching notification is to do it via eObject.eSetDeliver(boolean) - but we can't do it in this way cause we depends on context - for example whether object is added via lazy loading or is added by client call. And we can't say to notifier: if there is a client call do dispatch, if there is a lazy loading and appropriate option also do dispatch. Also in our case we need to pass to notifier additional parameters which characterizes kind of call (client, lazy, additional loading by query) - it will be terrific. So I suppose that no options are required for controlling notifications on loading objects to resource - it will not affect performance cause if there are no listeners associated with resource notification just will not be created - see NotifyingListImpl.basicAdd() method for details and EMF guys says that call to isNotificationRequired() is very fast. I will try to create patch for additional loading by query till Friday - I have wrote about it in newsgroup.
Hi Martin If it's not clear for you: When object is lazy loading in HibernatePersistableEList: if (res != null && res instanceof ResourceImpl) { try { // attach the new objects so that they are adapted when required for (int i = 0; i < objs.length; i++) { if (objs[i] instanceof EObject) { ((ResourceImpl) res).attached((EObject) objs[i]); ... StoreResource.attached() will result to call to StoreUtil.setEResource - so NOTIFICATION WILL BE SENT. THANK }
Hi Ilya, Thanks for the detailed description. I am not sure that I agree 100% with you, what I am thinking of is to disable notification (or just not do the dispatch, so simpler than the current emf approach) if the resource is loaded with XMLResource.OPTION_DISABLE_NOTIFY=true. It keeps track of this original setting and then for any subsequence lazy loads then again no notifications are send. This would work in your case also because you set: XMLResource.OPTION_DISABLE_NOTIFY=false. This is all fairly easy to do because also in case of lazy loading isLoading on the resource will be true. I will add this logic to your changes (if you do not have other fundamental objections ofcourse). I will probably wait with a new build for a few days, so your query changes can be incorporated. gr. Martin
I have have other fundamental objections :) Thanks
I nave NO other fundamental objections I am sorry for misprint Thanks (In reply to comment #6) > I have have other fundamental objections :) > > Thanks >
I am happy to hear that (although I would have been curious :-). gr. Martin
Hi Ilya, I looked at this in more detail. I am afraid that doing the add call in StoreUtil.setEResource won't work as this means that contained objects are also added to the root of the contentlist. I am thinking of a different solution. I think the issue you have is that a lazy load will also load non-contained objects (you don't have containment I think). These non-contained objects are not added to the resource from which they are loaded. I think that this is the real issue (in addition of not sending notifications when loading). I have changed this in the svn version. Can you try it out? You need to get a latest of StoreResource and HibernatePersistableEList. gr. Martin
Hi Martin! I have no containment relations in my model so your changes are fine for me. But I have some thoughts. It seems to me that you have conceptual contradiction in your code: when some object is "lazy" loaded and is inside container then container will not be added to resource but resource of container will be set via StoreUtil. So there will be: someContainer.eResource() == myResource is true BUT myResource.getContents().contains(someContainer) is false - for me it's bad situation. Anyway your changes are fine for me. Thanks.
Hi Ilya, Yes your remark is completely correct. I will look at this some further. I am moving the setEResource method to the StoreResource itself and rearranging a bit to make it better maintainable. gr. Martin
Hi Ilya, Some further thoughts about this subject. Currently Teneo works as you mentioned. It sets the eresource of the container but does not add it to the contents list. The same for any references to other objects. I can change this (following your remark) to also add the container to the contents, but then this has as sideeffect that when you load a resource using a query that also other (possibly unexpected) objects are present in the contents. For example (using the library example), when you load Books also their Library will be present in the contents although you did not explicitly load (query) them. You don't have containment relations but this also applies to references. For example when I load an object then also its single references will be loaded and added to the resource (and be present in the contents). Also (even more unexpected), when you load a resource using a query for Books then the contents will not show the Book but the Library (as the Book is contained in the Library). I think that this is not the intended behavior of loading a resource using queries. When using queries you would want to see the objects you query and not others. So another approach would be not to set the eresource of the container or referenced objects until they are explicitly added to the resource. This would be conceptually more correct and also prevent the issue I mention above. This means that also lazily loaded non-contained objects are not added to the contents of the resource automatically. So to summarize there are three approaches: 1) Current, set eResource of container but do not add to contents 2) Set eResource of container and add to contents of the resource 3) Do not set eResource of container until it is explicitly loaded. Looking at the above I am not in favor of 2, I am hesitant about 1 or 3. Can you let me know if/which of the above works/applies for your situation? gr. Martin
Hi Martin! I would like the second option - since EMF approach that container and its children are always in the same resource. Anyway I think that fact that we are arguing shows that people could prefer 1st, 2nd or 3rd one. So I suppose the best way to solve this problem is to introduce some options - client would have choice to get behavior they prefer. Thanks. Ilya.
Hi Ilya, Yes I agree that this can be option controlled. I will look at it, I am targeting for a new release tomorrow (with this option). gr. Martin
This will be available in a release later today. gr. Martin
Should be fixed in this build: 0.7.5 I200611290220 (second build on 29-11-2006) gr. Martin
Move to verified as per bug 206558.
