[jira] Created: (OFBIZ-2882) EntityList cache clearing issues when removing generic entity via DelegatorImpl

[Nicolas Malin (Jira)]

EntityList cache clearing issues when removing generic entity via DelegatorImpl
-------------------------------------------------------------------------------

                 Key: OFBIZ-2882
                 URL: https://issues.apache.org/jira/browse/OFBIZ-2882
             Project: OFBiz
          Issue Type: Bug
          Components: framework
    Affects Versions: SVN trunk
            Reporter: Bob Morley
            Priority: Critical


For more information refer to http://www.nabble.com/EntityList-cache-issues-...-to25179637.html

Ran into some trouble when we turned out caching and started to dependent on the EntityList cache.  The two problems were:

1) When attempting to storeHook to the entityListCache or entityObjectCache via the Cache.remove method, the NEW entity was being passed into the OLD entity.  This caused condition matching (in the list cache) to sometimes fail if a matched entity no longer matches do to it being modified.

2) During the matching logic the EntityJoinOperator was incorrectly short circuiting.  It was always checking if the lhs/rhs condition was true and if so returning the short-circuit value.  A concrete example is as such -- (A is funny) && (B is funny).  The short-circuit value for this expression is "FALSE" which means that if the first expression is FALSE we short-circuit and return FALSE.  What was happening was "if (A is funny) then return FALSE" rather then "if (A is funny == FALSE) then return FALSE".

I have a patch in the works for both of these issues and will include a unit tester that illustrates the problems (pre-patch) and passes successfully (post-patch).

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-2882?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bob Morley updated OFBIZ-2882:
------------------------------

    Attachment: OFBIZ-2882_EntityCacheListTest.patch

This file contains a new unit test class for cache and exposes the problem with flushing when an entity gets updated.

> EntityList cache clearing issues when removing generic entity via DelegatorImpl
> -------------------------------------------------------------------------------
>
>                 Key: OFBIZ-2882
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-2882
>             Project: OFBiz
>          Issue Type: Bug
>          Components: framework
>    Affects Versions: SVN trunk
>            Reporter: Bob Morley
>            Priority: Critical
>         Attachments: OFBIZ-2882_EntityCacheListTest.patch
>
>
> For more information refer to http://www.nabble.com/EntityList-cache-issues-...-to25179637.html
> Ran into some trouble when we turned out caching and started to dependent on the EntityList cache.  The two problems were:
> 1) When attempting to storeHook to the entityListCache or entityObjectCache via the Cache.remove method, the NEW entity was being passed into the OLD entity.  This caused condition matching (in the list cache) to sometimes fail if a matched entity no longer matches do to it being modified.
> 2) During the matching logic the EntityJoinOperator was incorrectly short circuiting.  It was always checking if the lhs/rhs condition was true and if so returning the short-circuit value.  A concrete example is as such -- (A is funny) && (B is funny).  The short-circuit value for this expression is "FALSE" which means that if the first expression is FALSE we short-circuit and return FALSE.  What was happening was "if (A is funny) then return FALSE" rather then "if (A is funny == FALSE) then return FALSE".
> I have a patch in the works for both of these issues and will include a unit tester that illustrates the problems (pre-patch) and passes successfully (post-patch).

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-2882?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bob Morley updated OFBIZ-2882:
------------------------------

    Attachment: OFBIZ-2882_EntityCacheListFix.patch

Includes the fix as well as the entitytests.xml update that I had forgotten to include with the test in the previous file.

> EntityList cache clearing issues when removing generic entity via DelegatorImpl
> -------------------------------------------------------------------------------
>
>                 Key: OFBIZ-2882
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-2882
>             Project: OFBiz
>          Issue Type: Bug
>          Components: framework
>    Affects Versions: SVN trunk
>            Reporter: Bob Morley
>            Priority: Critical
>         Attachments: OFBIZ-2882_EntityCacheListFix.patch, OFBIZ-2882_EntityCacheListTest.patch
>
>
> For more information refer to http://www.nabble.com/EntityList-cache-issues-...-to25179637.html
> Ran into some trouble when we turned out caching and started to dependent on the EntityList cache.  The two problems were:
> 1) When attempting to storeHook to the entityListCache or entityObjectCache via the Cache.remove method, the NEW entity was being passed into the OLD entity.  This caused condition matching (in the list cache) to sometimes fail if a matched entity no longer matches do to it being modified.
> 2) During the matching logic the EntityJoinOperator was incorrectly short circuiting.  It was always checking if the lhs/rhs condition was true and if so returning the short-circuit value.  A concrete example is as such -- (A is funny) && (B is funny).  The short-circuit value for this expression is "FALSE" which means that if the first expression is FALSE we short-circuit and return FALSE.  What was happening was "if (A is funny) then return FALSE" rather then "if (A is funny == FALSE) then return FALSE".
> I have a patch in the works for both of these issues and will include a unit tester that illustrates the problems (pre-patch) and passes successfully (post-patch).

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-2882?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Adam Heath reassigned OFBIZ-2882:
---------------------------------

    Assignee: Adam Heath

> EntityList cache clearing issues when removing generic entity via DelegatorImpl
> -------------------------------------------------------------------------------
>
>                 Key: OFBIZ-2882
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-2882
>             Project: OFBiz
>          Issue Type: Bug
>          Components: framework
>    Affects Versions: SVN trunk
>            Reporter: Bob Morley
>            Assignee: Adam Heath
>            Priority: Critical
>         Attachments: OFBIZ-2882_EntityCacheListFix.patch, OFBIZ-2882_EntityCacheListTest.patch
>
>
> For more information refer to http://www.nabble.com/EntityList-cache-issues-...-to25179637.html
> Ran into some trouble when we turned out caching and started to dependent on the EntityList cache.  The two problems were:
> 1) When attempting to storeHook to the entityListCache or entityObjectCache via the Cache.remove method, the NEW entity was being passed into the OLD entity.  This caused condition matching (in the list cache) to sometimes fail if a matched entity no longer matches do to it being modified.
> 2) During the matching logic the EntityJoinOperator was incorrectly short circuiting.  It was always checking if the lhs/rhs condition was true and if so returning the short-circuit value.  A concrete example is as such -- (A is funny) && (B is funny).  The short-circuit value for this expression is "FALSE" which means that if the first expression is FALSE we short-circuit and return FALSE.  What was happening was "if (A is funny) then return FALSE" rather then "if (A is funny == FALSE) then return FALSE".
> I have a patch in the works for both of these issues and will include a unit tester that illustrates the problems (pre-patch) and passes successfully (post-patch).

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-2882?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12756130#action_12756130 ]

Bob Morley commented on OFBIZ-2882:
-----------------------------------

FYI -- I have not updated the patch; but we did find a bug with this patch in our code.  In Cache.java remove(GenericEntity) it was checking to see if it could get the oldEntity, but what it did not realize was that the oldEntity could actually exist but be set to the internal NULL.  This is true on creates of brand-new entities.

We changed the method to the following:

    public GenericValue remove(GenericEntity entity) {
        if (Debug.verboseOn()) Debug.logVerbose("Cache remove GenericEntity: " + entity, module);
        GenericValue oldEntity = entityCache.remove(entity.getPrimaryKey());

        if (oldEntity == null) {
        // Unable to find the old entity in the case, so we will create a version of the original entity to properly remove the items from the related caches
                GenericEntity origEntity = GenericEntity.createGenericEntity(entity);
                if (entity instanceof GenericValue) {
                origEntity.setFields(((GenericValue)entity).getOriginalDbValues());
                }
                entityListCache.storeHook(origEntity, null);
                entityObjectCache.storeHook(origEntity, null);

        } else {
        if( !(oldEntity instanceof NULL )) {
                // Remove the old entity from the related caches
                    entityListCache.storeHook(oldEntity, null);
                    entityObjectCache.storeHook(oldEntity, null);
        }
        }
       
        return oldEntity;
    }

** We basically added the instanceof NULL check before going to storeHook with an oldEntity that represents the NULL generic entity.

> EntityList cache clearing issues when removing generic entity via DelegatorImpl
> -------------------------------------------------------------------------------
>
>                 Key: OFBIZ-2882
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-2882
>             Project: OFBiz
>          Issue Type: Bug
>          Components: framework
>    Affects Versions: SVN trunk
>            Reporter: Bob Morley
>            Assignee: Adam Heath
>            Priority: Critical
>         Attachments: OFBIZ-2882_EntityCacheListFix.patch, OFBIZ-2882_EntityCacheListTest.patch
>
>
> For more information refer to http://www.nabble.com/EntityList-cache-issues-...-to25179637.html
> Ran into some trouble when we turned out caching and started to dependent on the EntityList cache.  The two problems were:
> 1) When attempting to storeHook to the entityListCache or entityObjectCache via the Cache.remove method, the NEW entity was being passed into the OLD entity.  This caused condition matching (in the list cache) to sometimes fail if a matched entity no longer matches do to it being modified.
> 2) During the matching logic the EntityJoinOperator was incorrectly short circuiting.  It was always checking if the lhs/rhs condition was true and if so returning the short-circuit value.  A concrete example is as such -- (A is funny) && (B is funny).  The short-circuit value for this expression is "FALSE" which means that if the first expression is FALSE we short-circuit and return FALSE.  What was happening was "if (A is funny) then return FALSE" rather then "if (A is funny == FALSE) then return FALSE".
> I have a patch in the works for both of these issues and will include a unit tester that illustrates the problems (pre-patch) and passes successfully (post-patch).

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-2882?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bob Morley updated OFBIZ-2882:
------------------------------

    Attachment: OFBIZ-2882_EntityCacheListTest_V2.patch
                OFBIZ-2882_EntityCacheListFix_V2.patch

NOTE: I think this patch is pretty important.  I have re-packaged it and tested it again on Trunk, here are the details ...  Only apply the two V2 patch files!!!

This patch fixes a pretty serious caching issue.  I just applied to two patches to trunk and made a few modifications based on the current state of trunk.  Here are the changes:

a) The unit tester was making use of reflection to get at the cache (which was not exposed at the time of coding).  It turns out that there is a getCache on delegator that is now available so I removed the reflection code and used this.

b) The last assert on the unit test was assuming that the result of the cache (after clear would be null); I felt this was too close to the implementation so I eased this assert up to check for empty (as long as we do not have a cache hit on the removed entity we are ok)

The changes to the fix were:

a) Half of the fix was related to fixing EntityJoinOperator which was not properly checking the short circuit.  It appears Adam Heath has applied this fix so I removed it from this patch.

b) There was an additional "GenericValue.NULL" check that was written in the comments but not put into the patch file.  The new patch file has this in it.

As before, I ran with the unit test that exposes the failure and then ran again with the patch applied which causes the unit tester to pass.

> EntityList cache clearing issues when removing generic entity via DelegatorImpl
> -------------------------------------------------------------------------------
>
>                 Key: OFBIZ-2882
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-2882
>             Project: OFBiz
>          Issue Type: Bug
>          Components: framework
>    Affects Versions: SVN trunk
>            Reporter: Bob Morley
>            Assignee: Adam Heath
>            Priority: Critical
>         Attachments: OFBIZ-2882_EntityCacheListFix.patch, OFBIZ-2882_EntityCacheListFix_V2.patch, OFBIZ-2882_EntityCacheListTest.patch, OFBIZ-2882_EntityCacheListTest_V2.patch
>
>
> For more information refer to http://www.nabble.com/EntityList-cache-issues-...-to25179637.html
> Ran into some trouble when we turned out caching and started to dependent on the EntityList cache.  The two problems were:
> 1) When attempting to storeHook to the entityListCache or entityObjectCache via the Cache.remove method, the NEW entity was being passed into the OLD entity.  This caused condition matching (in the list cache) to sometimes fail if a matched entity no longer matches do to it being modified.
> 2) During the matching logic the EntityJoinOperator was incorrectly short circuiting.  It was always checking if the lhs/rhs condition was true and if so returning the short-circuit value.  A concrete example is as such -- (A is funny) && (B is funny).  The short-circuit value for this expression is "FALSE" which means that if the first expression is FALSE we short-circuit and return FALSE.  What was happening was "if (A is funny) then return FALSE" rather then "if (A is funny == FALSE) then return FALSE".
> I have a patch in the works for both of these issues and will include a unit tester that illustrates the problems (pre-patch) and passes successfully (post-patch).

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-2882?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12875536#action_12875536 ]

Jacques Le Roux commented on OFBIZ-2882:
----------------------------------------

What is the state of this issue?

> EntityList cache clearing issues when removing generic entity via DelegatorImpl
> -------------------------------------------------------------------------------
>
>                 Key: OFBIZ-2882
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-2882
>             Project: OFBiz
>          Issue Type: Bug
>          Components: framework
>    Affects Versions: SVN trunk
>            Reporter: Bob Morley
>            Assignee: Adam Heath
>            Priority: Critical
>         Attachments: OFBIZ-2882_EntityCacheListFix.patch, OFBIZ-2882_EntityCacheListFix_V2.patch, OFBIZ-2882_EntityCacheListTest.patch, OFBIZ-2882_EntityCacheListTest_V2.patch
>
>
> For more information refer to http://www.nabble.com/EntityList-cache-issues-...-to25179637.html
> Ran into some trouble when we turned out caching and started to dependent on the EntityList cache.  The two problems were:
> 1) When attempting to storeHook to the entityListCache or entityObjectCache via the Cache.remove method, the NEW entity was being passed into the OLD entity.  This caused condition matching (in the list cache) to sometimes fail if a matched entity no longer matches do to it being modified.
> 2) During the matching logic the EntityJoinOperator was incorrectly short circuiting.  It was always checking if the lhs/rhs condition was true and if so returning the short-circuit value.  A concrete example is as such -- (A is funny) && (B is funny).  The short-circuit value for this expression is "FALSE" which means that if the first expression is FALSE we short-circuit and return FALSE.  What was happening was "if (A is funny) then return FALSE" rather then "if (A is funny == FALSE) then return FALSE".
> I have a patch in the works for both of these issues and will include a unit tester that illustrates the problems (pre-patch) and passes successfully (post-patch).

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-2882?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12876240#action_12876240 ]

Bob Morley commented on OFBIZ-2882:
-----------------------------------

I believe the V2 set of patches just need a review and can be committed.  You can apply just the unit tester first to expose the problems (perhaps do a little digging if you wish) and then apply the fix and the unit test will work properly.

> EntityList cache clearing issues when removing generic entity via DelegatorImpl
> -------------------------------------------------------------------------------
>
>                 Key: OFBIZ-2882
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-2882
>             Project: OFBiz
>          Issue Type: Bug
>          Components: framework
>    Affects Versions: SVN trunk
>            Reporter: Bob Morley
>            Assignee: Adam Heath
>            Priority: Critical
>         Attachments: OFBIZ-2882_EntityCacheListFix.patch, OFBIZ-2882_EntityCacheListFix_V2.patch, OFBIZ-2882_EntityCacheListTest.patch, OFBIZ-2882_EntityCacheListTest_V2.patch
>
>
> For more information refer to http://www.nabble.com/EntityList-cache-issues-...-to25179637.html
> Ran into some trouble when we turned out caching and started to dependent on the EntityList cache.  The two problems were:
> 1) When attempting to storeHook to the entityListCache or entityObjectCache via the Cache.remove method, the NEW entity was being passed into the OLD entity.  This caused condition matching (in the list cache) to sometimes fail if a matched entity no longer matches do to it being modified.
> 2) During the matching logic the EntityJoinOperator was incorrectly short circuiting.  It was always checking if the lhs/rhs condition was true and if so returning the short-circuit value.  A concrete example is as such -- (A is funny) && (B is funny).  The short-circuit value for this expression is "FALSE" which means that if the first expression is FALSE we short-circuit and return FALSE.  What was happening was "if (A is funny) then return FALSE" rather then "if (A is funny == FALSE) then return FALSE".
> I have a patch in the works for both of these issues and will include a unit tester that illustrates the problems (pre-patch) and passes successfully (post-patch).

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-2882?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12876588#action_12876588 ]

Jacques Le Roux commented on OFBIZ-2882:
----------------------------------------

Thanks Bob,

Hopefully Adam will get a chance to had a look, else I will try to review at least...

> EntityList cache clearing issues when removing generic entity via DelegatorImpl
> -------------------------------------------------------------------------------
>
>                 Key: OFBIZ-2882
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-2882
>             Project: OFBiz
>          Issue Type: Bug
>          Components: framework
>    Affects Versions: SVN trunk
>            Reporter: Bob Morley
>            Assignee: Adam Heath
>            Priority: Critical
>         Attachments: OFBIZ-2882_EntityCacheListFix.patch, OFBIZ-2882_EntityCacheListFix_V2.patch, OFBIZ-2882_EntityCacheListTest.patch, OFBIZ-2882_EntityCacheListTest_V2.patch
>
>
> For more information refer to http://www.nabble.com/EntityList-cache-issues-...-to25179637.html
> Ran into some trouble when we turned out caching and started to dependent on the EntityList cache.  The two problems were:
> 1) When attempting to storeHook to the entityListCache or entityObjectCache via the Cache.remove method, the NEW entity was being passed into the OLD entity.  This caused condition matching (in the list cache) to sometimes fail if a matched entity no longer matches do to it being modified.
> 2) During the matching logic the EntityJoinOperator was incorrectly short circuiting.  It was always checking if the lhs/rhs condition was true and if so returning the short-circuit value.  A concrete example is as such -- (A is funny) && (B is funny).  The short-circuit value for this expression is "FALSE" which means that if the first expression is FALSE we short-circuit and return FALSE.  What was happening was "if (A is funny) then return FALSE" rather then "if (A is funny == FALSE) then return FALSE".
> I have a patch in the works for both of these issues and will include a unit tester that illustrates the problems (pre-patch) and passes successfully (post-patch).

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.