You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@polygene.apache.org by Niclas Hedhman <ni...@hedhman.org> on 2015/06/24 16:51:18 UTC
Bug found in UnitOfWork
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
Re: Bug found in UnitOfWork
Posted by Paul Merlin <pa...@nosphere.org>.
Niclas Hedhman a écrit :
> 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?
Here is what we state in documentation about SPI breakage:
"For SPIs and especially Runtimes, we reserve the right of incompatible
changes between releases in the second digit." -
http://zest.apache.org/download.html
So, from my point of view, we can release your FIX in 2.1 and your
changes can go into develop now, rather than later.
/Paul
Re: Bug found in UnitOfWork
Posted by Niclas Hedhman <ni...@hedhman.org>.
Regarding the "multiple roles" of Module;
Yes, this is a well-known design compromise. Way back in time, there were
separated UnitOfWorkFactory, ValueBuilderFactory and so on, roles that then
later the Module came to fulfill.
So, one *should* (but I guess few does) actually write;
@Structure
UnitOfWorkFactory uowf;
instead of
@Structure
Module module;
which would better allow future design changes without breaking
compatibility. We *could* go back and say that the "Module" interface
doesn't have all these roles, even if the ModuleInstance implementation
under the hood actually does implement them.
The downside is a couple of more injections, but perhaps worthwhile, for
the sake of tuning internal design.
Cheers
Niclas
On Mon, Jun 29, 2015 at 11:37 PM, Kent Sølvsten <ke...@gmail.com>
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 <ni...@hedhman.org>
> 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 <
> kent.soelvsten@gmail.com>
> >> 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 <ni...@hedhman.org>
> >>> 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
Re: Bug found in UnitOfWork
Posted by Kent Sølvsten <ke...@gmail.com>.
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 <ke...@gmail.com>
> 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 <ni...@hedhman.org>
>> 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 <
>> kent.soelvsten@gmail.com>
>>>> 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 <ni...@hedhman.org>
>>>>> 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
>>>>
>>>
>>
>
Re: Bug found in UnitOfWork
Posted by Niclas Hedhman <ni...@hedhman.org>.
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 <ke...@gmail.com>
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 <ni...@hedhman.org>
> 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 <
> kent.soelvsten@gmail.com>
> >> 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 <ni...@hedhman.org>
> >>> 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
Re: Bug found in UnitOfWork
Posted by Kent Sølvsten <ke...@gmail.com>.
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 <ni...@hedhman.org> 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 <ke...@gmail.com>
>> 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 <ni...@hedhman.org>
>>> 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
>>
>
>
Re: Bug found in UnitOfWork
Posted by Niclas Hedhman <ni...@hedhman.org>.
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 <ni...@hedhman.org> 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 <ke...@gmail.com>
> 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 <ni...@hedhman.org>
>> 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
Re: Bug found in UnitOfWork
Posted by Niclas Hedhman <ni...@hedhman.org>.
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 <ke...@gmail.com>
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 <ni...@hedhman.org>
> 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
Re: Bug found in UnitOfWork
Posted by Niclas Hedhman <ni...@hedhman.org>.
Yes, you are right, the cache key can't be done as I suggested.
Further, I have looked into why there is a Module assigned into the
EntityStoreUnitOfWork, and it boils down to, as usual, to the
deserialization of entities. It basically needs to know where to look for
the entity types.
And that is quite interesting; Is it simply a matter of bad design, i.e.
violation of the Aggregate concept of DDD?
So, analyzing my particular case; Yes, I would say so.
What I am doing is using the Scheduling Library, and upon creating certain
entities, it also creates a schedule to callback to those entities every 5
minutes. So, my fault is that this is inlined in the normal entity
creation, but should actually be a SideEffect on a new UnitOfWork.
So, is that always the case? Is it simply that one needs to think harder
about the design choices and not do these things? There is obviously a lot
of other cases where it works more like expected, since I have problem to
re-implement the issue in a testcase.
Niclas
On Thu, Jun 25, 2015 at 5:44 AM, Kent Sølvsten <ke...@gmail.com>
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 <ni...@hedhman.org>
> 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
Re: Bug found in UnitOfWork
Posted by Kent Sølvsten <ke...@gmail.com>.
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 <ni...@hedhman.org> 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
>>
>
>
Re: Bug found in UnitOfWork
Posted by Niclas Hedhman <ni...@hedhman.org>.
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 <ni...@hedhman.org> 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