OFBiz
  1. OFBiz
  2. OFBIZ-3847

Entity ECAs not triggered correctly when using Delegator.storeAll() method

    Details

    • Type: Bug Bug
    • Status: Patch Available
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: Release Branch 10.04
    • Fix Version/s: None
    • Component/s: framework
    • Labels:
      None
    • Sprint:
      Bug Crush Event - 21/2/2015

      Description

      The conditions don't work when updating (not creating) entities using the Delegator.storeAll() method. E.g. the following condition does not work:

      <eca entity="Product" operation="create-store" event="return">
              <condition field-name="autoCreateKeywords" operator="not-equals" value="N"/>
              <action service="indexProductKeywords" mode="sync" value-attr="productInstance"/>
      </eca>
      

      The indexProductKeywords service is called anyway when the product is updated and the autoCreateKeywords was "N" and stays "N". It works correctly for newly created products.

      The problem is in the method GenericDelegator.storeAll(), where unchanged field values are not passed down to the store() method. The store method calls the ECA engine, which does not receive the unchanged values at all and thus cannot evaluate the EECA conditions correctly.

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Patch Available Patch Available
        476d 11h 20m 1 Anne Jessel 25/Oct/11 04:41
        Sharan Foga made changes -
        Rank Ranked higher
        Sharan Foga made changes -
        Rank Ranked higher
        Sharan Foga made changes -
        Rank Ranked higher
        Sharan Foga made changes -
        Sprint Bug Crush Event - 21/2/2015 [ 91 ]
        Anne Jessel made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Anne Jessel added a comment -

        I've created a patch modeled after Scott's suggestion. I'd appreciate someone reviewing it. It does work for me, however I am concerned that it might fail when the value of a field that is part of the EECA condition is being changed from non-null to null using GenericDelegator.storeAll, because of the way storeAll works. There may be a simple solution I've missed.

        Show
        Anne Jessel added a comment - I've created a patch modeled after Scott's suggestion. I'd appreciate someone reviewing it. It does work for me, however I am concerned that it might fail when the value of a field that is part of the EECA condition is being changed from non-null to null using GenericDelegator.storeAll, because of the way storeAll works. There may be a simple solution I've missed.
        Anne Jessel made changes -
        Hide
        Anne Jessel added a comment -

        This bug is one I'd like to fix. Scott's suggestion looks good to me. Anyone have any comments or better ideas before I go ahead and implement it?

        Show
        Anne Jessel added a comment - This bug is one I'd like to fix. Scott's suggestion looks good to me. Anyone have any comments or better ideas before I go ahead and implement it?
        Hide
        Jacques Le Roux added a comment -

        I did not look into code, but I can't find a better idea Scott

        Show
        Jacques Le Roux added a comment - I did not look into code, but I can't find a better idea Scott
        Hide
        Scott Gray added a comment -

        I think this points to a larger problem whereby there is no guarantee that any entity ECA will receive a complete GenericValue representing the entire record.

        For example GenericDelegator.removeByPrimaryKey() only passes the primary key fields to the ECA rule runner and any ECAs that depend on other non-pk fields won't be able to evaluate their condition properly.

        I'm not sure what the full solution would look like, it would be good to get some thoughts of others on this.

        We could consider having EntityEcaCondition.eval() perform a db lookup if any of the fields it needs to evaluate are missing. If they are missing it could fully populate the entity with the missing fields so that a lookup is only required at most once per delegator operation. If no ECA conditions require any missing fields then the lookup wouldn't be performed, so the performance penalty would only be incurred when necessary.

        Show
        Scott Gray added a comment - I think this points to a larger problem whereby there is no guarantee that any entity ECA will receive a complete GenericValue representing the entire record. For example GenericDelegator.removeByPrimaryKey() only passes the primary key fields to the ECA rule runner and any ECAs that depend on other non-pk fields won't be able to evaluate their condition properly. I'm not sure what the full solution would look like, it would be good to get some thoughts of others on this. We could consider having EntityEcaCondition.eval() perform a db lookup if any of the fields it needs to evaluate are missing. If they are missing it could fully populate the entity with the missing fields so that a lookup is only required at most once per delegator operation. If no ECA conditions require any missing fields then the lookup wouldn't be performed, so the performance penalty would only be incurred when necessary.
        Martin Kreidenweis made changes -
        Field Original Value New Value
        Attachment GenericDelegator.java.diff [ 12448711 ]
        Hide
        Martin Kreidenweis added a comment -

        This should fix the issue in GenericDelegator. We can't see why not all the fields should be passed down to store().

        Show
        Martin Kreidenweis added a comment - This should fix the issue in GenericDelegator. We can't see why not all the fields should be passed down to store().
        Martin Kreidenweis created issue -

          People

          • Assignee:
            Unassigned
            Reporter:
            Martin Kreidenweis
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:

              Development

                Agile