I think you are right, since we have just repaired the unfortunate coupling with module.
Den 30-06-2015 kl. 08:48 skrev Niclas Hedhman: > EntityStoreUnitOfWork intends to capture all actions required for a given > ES, within an UoW. I can't see how this could possibly be scrapped, but > willing to entertain the idea... > > // Niclas > > On Mon, Jun 29, 2015 at 11:37 PM, Kent Sølvsten <[email protected]> > wrote: > >> Great job! >> >> Actually I think this change is soooo obviously correct from a design >> perspective. Since the "module" used is not the "structural" module >> attached to the entitystore, but the "calling module", it simply seems >> wrong to couple those together in the EntityStoreUnitOfWork concept. >> >> I even start to wonder if the concept of EntityStoreUnitOfWork should >> simply be scrapped ? >> >> Another note: In the codebase it is sometimes not very obvious when >> seeing a module reference, whether we are considering a >> "structurally-attached module" (the one holding the entity store used >> for saving the entities) or just looking at a "calling module" (often >> used to lookup xxxDescriptors). DCI used the concept of roles - could it >> be useful in Qi4J ? Sometimes we are using a module as a >> "DescriptorSource" - other times we are using it as an >> "EntityStoreHolder" .... or something like that. >> >> Just toying with ideas here :-) >> >> /Kent >> >> >> >> Den 25-06-2015 kl. 14:29 skrev Niclas Hedhman: >>> And YES, the proposed approach and a handful of SPI interface changes >> made >>> my usecase work as expected. >>> >>> So, even if I had the wrong approach to my particular issue, the new >>> behavior is probably as expected, so I think it should go into the >>> codebase, and I think it might as well go in now, rather than later. >>> >>> EntityStore implementations will break at compile time, but the fix is >>> relatively straight forward. >>> >>> Any other opinions? >>> >>> // Niclas >>> >>> On Thu, Jun 25, 2015 at 7:23 PM, Niclas Hedhman <[email protected]> >> wrote: >>>> Kent, >>>> I tried to implement the ModuleEntityStoreUnitOfWork idea. It is not as >>>> smooth as it first seems, since the inner EntityStoreUnitOfWork needs to >>>> call entityStateOf() which needs the module. BUT, by modifying a couple >> of >>>> SPI interfaces, it is possible to pass the Module along in method calls. >>>> Along with removing EntityStoreUnitOfWork in some cases where only >>>> something inside the EsUoW is actually wanted, which in some cases >> resulted >>>> in creating a EsUoW for no practical reason, I have it working.... i.e. >>>> doesn't break any existing testcases. I haven't checked whether my case >> was >>>> solved as well. Will do that shortly. >>>> >>>> >>>> >>>> On Thu, Jun 25, 2015 at 5:44 AM, Kent Sølvsten < >> [email protected]> >>>> wrote: >>>> >>>>> Hmmm interesting problem . Could it be reproduced with something like >>>>> this: >>>>> >>>>> a. Creation of a UnitOfWork >>>>> b. Accessing entity M1 in module M >>>>> c. Entity E calls service MS in module M. >>>>> d. Service uses entity N1 in module N (N1 has some sort of shared >>>>> visibility) >>>>> .... (later in the same UOW) >>>>> e. Someone uses service NS in module N (some short of shared >> visibility). >>>>> f. Service NS uses entity N2 in module N (which has only module >>>>> visibility) >>>>> >>>>> I am not sure that using a composite module/entitystore cache key is >>>>> such a good idea. Wouldn't that lead to 2 instances of >>>>> EntityStoreUnitOfWork, basically splitting uow.complete() into 2 native >>>>> transactions in that case? >>>>> Could we instead reuse the "ModuleUnitOfWork-idea" ? >>>>> >>>>> Something like: >>>>> >>>>> public EntityStoreUnitOfWork getEntityStoreUnitOfWork( EntityStore >>>>> store, Module module ) >>>>> >>>>> { >>>>> >>>>> EntityStoreUnitOfWork uow = storeUnitOfWork.get( store ); >>>>> >>>>> if( uow == null ) >>>>> >>>>> { >>>>> >>>>> uow = store.newUnitOfWork( usecase, currentTime ); // no >>>>> module here! >>>>> >>>>> storeUnitOfWork.put( store, uow ); >>>>> >>>>> } >>>>> >>>>> return new ModuleEntityStoreUnitOfWork(uow, module); >>>>> >>>>> } >>>>> >>>>> As a side-note we could one day consider some sort of >>>>> entitystore-sharing between modules. If we had 2 modules, separated for >>>>> design reasons but sharing the same persistence, it would be nice to be >>>>> able to complete a UOW spanning those 2 modules in a single native >>>>> transaction. >>>>> >>>>> Regarding "bug-compatible" the safe option would be to postpone to 3.0 >> - >>>>> but i guess it is better to just fix the issue? >>>>> >>>>> /Kent >>>>> >>>>> >>>>> >>>>> Den 24-06-2015 kl. 17:40 skrev Niclas Hedhman: >>>>>> Follow-up; The case when it gets wrong is more complex than my list, >>>>> and I >>>>>> am not able to create a testcase for this... >>>>>> >>>>>> On Wed, Jun 24, 2015 at 10:51 PM, Niclas Hedhman <[email protected]> >>>>> wrote: >>>>>>> Gang, >>>>>>> >>>>>>> I have found a fairly sophisticated bug, which is rather severe and >>>>>>> possibly hard to fix without breaking applications, that might depend >>>>> on >>>>>>> wrong behavior... >>>>>>> >>>>>>> It manifests itself as not having the right Visibility of entities, >> and >>>>>>> the reason is because the EntityStoreUnitOfWork may be set up >>>>> incorrectly >>>>>>> sometimes. >>>>>>> >>>>>>> I think it happens if the following sequence occurs; >>>>>>> >>>>>>> a. Creation of a UnitOfWork >>>>>>> b. Accessing entity E in module M >>>>>>> c. Entity E calls service S in module N. >>>>>>> d. Service S tries to access entity F in module N. >>>>>>> >>>>>>> internally, the EntityStoreUnitOfWork will have been initialized with >>>>> the >>>>>>> Module M, and uses that to find entity F, which is not seen >>>>>>> (Visibility.module). >>>>>>> >>>>>>> This is very similar to a design flaw a long time ago, where the >>>>>>> visibility was effectively in the wrong Layer, and that is when the >>>>> "split" >>>>>>> UnitOfWork concept was introduced (2008?). >>>>>>> >>>>>>> The issue manifests itself in UnitOfWorkInstance in the following >>>>> method; >>>>>>> public EntityStoreUnitOfWork getEntityStoreUnitOfWork( EntityStore >>>>> store, Module module ) >>>>>>> { >>>>>>> EntityStoreUnitOfWork uow = storeUnitOfWork.get( store ); >>>>>>> if( uow == null ) >>>>>>> { >>>>>>> uow = store.newUnitOfWork( usecase, module, currentTime ); >>>>>>> storeUnitOfWork.put( store, uow ); >>>>>>> } >>>>>>> return uow; >>>>>>> } >>>>>>> >>>>>>> as the EntityStoreUnitOfWork will have been created with the wrong >>>>> module. >>>>>>> I think the solution is that the Module must also be part of the >> cache >>>>>>> key, and that this would solve it. However, I am very anxious that >>>>> this may >>>>>>> break something else very badly, as it is fairly central to the >> entire >>>>>>> UnitOfWork flow. >>>>>>> >>>>>>> The reason I say that it might break applications is that, the called >>>>>>> Service has access to the entities in the modules calling it, and >>>>> someone >>>>>>> might depend on that. But I think, we should be allowed to break bug >>>>>>> compatibility... Or?? >>>>>>> >>>>>>> BTW, this touches on the "multiple entitystore" feature that came up >>>>>>> during the discussion of the Messaging abstraction. >>>>>>> >>>>>>> Cheers >>>>>>> -- >>>>>>> Niclas Hedhman, Software Developer >>>>>>> http://zest.apache.org - New Energy for Java >>>>>>> >>>> -- >>>> Niclas Hedhman, Software Developer >>>> http://zest.apache.org - New Energy for Java >>>> >>> >> >
