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 > -- Niclas Hedhman, Software Developer http://zest.apache.org - New Energy for Java
