yes, test in pull request.
cheers,
Tibor
On Jul 24, 2015, at 7:01 AM, Niclas Hedhman <[email protected]> wrote:
> The "this testcase" is referring to the pull request? I'll take a look a
> bit later.... Checking the "Release" at the moment.
>
> Cheers
> Niclas
>
> On Fri, Jul 24, 2015 at 12:14 PM, Tibor Mlynarik <[email protected]>
> wrote:
>
>> 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 <[email protected]> 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 <
>> [email protected]>
>>> 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 <[email protected]> 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
>>
>>
>
>
> --
> Niclas Hedhman, Software Developer
> http://zest.apache.org - New Energy for Java
