polygene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tibor Mlynarik <tibor.mlyna...@gmail.com>
Subject Re: JSONEntityState internal clone logic
Date Fri, 24 Jul 2015 04:14:59 GMT
Hi Niclas,

> 
> The "isStateNotCloned()" fix seems legit and a testcase for that would be
> awesome (can't figure one out in my at this early hour)

this testcase should be showing that "isStateNotCloned()" is broken. 
Change to entity in uow3, which is discarded, is visible to others, as cloning is not working.

account3.balance().set( BigDecimal.TEN); 
uow3.discard() ; 

on other side, clone is executed with each set to entity, what is probably only unnecessary
performance penalty.

> As for the testcase, the bug that it exposes is that it doesn't prevent you
> from using the "Account account1" references outside the UoW it was created
> in.


I see your point that accessing entity fetched from other uow ( closed one) should be prohibited.
But maybe accessing just identity value should be allowed.
But this is not what I wanted to report.

thanks ,

	Tibor

On Jul 24, 2015, at 3:50 AM, Niclas Hedhman <niclas@hedhman.org> wrote:

> I have not checked the code yet, but the clone is supposed to be there to
> provide UoW isolation, i.e. a value pulled out of the cache, and modified
> (any of the set methods) must not be visible into the cached state until
> the UoW completes, at which time I presume the entity state is replaced in
> the the cache.
> 
> As for the testcase, the bug that it exposes is that it doesn't prevent you
> from using the "Account account1" references outside the UoW it was created
> in. That is the bug, and I would be happy to look into that (perhaps for
> 3.0 just in case you depend on this fault behavior). If identity is used,
> it is more likely working as expected.
> 
> 
> The "isStateNotCloned()" fix seems legit and a testcase for that would be
> awesome (can't figure one out in my at this early hour)
> 
> 
> Thanks, Tibor
> 
> 
> On Fri, Jul 24, 2015 at 12:31 AM, Tibor Mlynarik <tibor.mlynarik@gmail.com>
> wrote:
> 
>> Hi Paul,
>> 
>> I have prepared test case to demonstrate issue.
>> Problem is if there is enabled cache in json based entity store.
>> Entities could be in wrong state.
>> 
>> see here :
>> https://github.com/apache/zest-qi4j/pull/2
>> 
>> to make test success, fix :
>> 
>> JSONEntityState
>>    void cloneStateIfGlobalStateLoaded()
>>    {
>>        if( ! isStateNotCloned() )
>> 
>> 
>> there is one more fix needed in JSONMapEntityStoreMixin#fetchCachedState
>> method .
>> 
>> Note: I was not able to configure entity store with cache service in one
>> layer ,
>> probably because of circular dependency: store uses cache, cache need
>> store for config .
>> 
>> cheers,
>> 
>>        Tibor
>> 
>> 
>> On Jul 23, 2015, at 1:49 PM, Paul Merlin <paul@nosphere.org> wrote:
>> 
>>> Hi Tibor,
>>> 
>>> Tibor Mlynarik a écrit :
>>>> Hi,
>>>> 
>>>> I have noticed code in JSONEntityState that seems suspicious for me.
>>>> 
>>>> In method JSONEntityState#cloneStateIfGlobalStateLoaded
>>>> if parts are swapped.
>>>> 
>>>> I suppose that by global state is meant copy taken from cache that is
>> shared across UoWs.
>>>> And before first entity change own copy for UoW should be cloned.
>>>> 
>>>> But as it is now, cloning is not done at first change and redundantly
>> executed with each entity change.
>>>> Also maybe whole cloning could be avoided if cache is not used.
>>>> 
>>>> Or am I wrong in understanding of purpose of this state clone ?
>>> IIRC this is the sole purpose of this state clone.
>>> As you pointed out there may be room for improvements here.
>>> 
>>> Niclas, WDYT?
>>> 
>>> /Paul
>>> 
>> 
>> 
> 
> 
> -- 
> Niclas Hedhman, Software Developer
> http://zest.apache.org - New Energy for Java


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message