Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize CheckRules to minimize re-running rules #3736

Open
rockfordlhotka opened this issue Mar 14, 2024 · 1 comment
Open

Optimize CheckRules to minimize re-running rules #3736

rockfordlhotka opened this issue Mar 14, 2024 · 1 comment

Comments

@rockfordlhotka
Copy link
Member

This is not code, but it has the logic flow. These changes are within CheckRules. CheckRulesForProperty would revert to its previous code, and this change would not otherwise modify CheckRulesForProperty.

The motivation for this design is that CheckRules is a batch operation. It's going to attempt to run all rules for every property. Dependencies are only affected if the rules cause values to change.

Logic flow

  1. Remember CascadeOnDirtyProperties setting (local variable) and set CascadeOnDirtyProperties = false.
    • This is important because CheckRulesForProperty will always attempt to cascade if this property is enabled.
    • We want to honor the advanced CascadeOnDirtyProperties, but from within the batch nature of CheckRules.
  2. Run all rules via CheckRulesForProperty, gathering distinct result of affected properties, with no cascading of any sort
    • Current behavior sets cascade property = true
    • New behavior would set cascade property = false
  3. For each dirty property, run CheckRulesForProperty again, gathering distinct result of affected properties, with no cascading of any sort
    • Note: Dirty properties are added to the results from affected properties that had a change in value. If an affected property does not change value during the rule execution, it won't be in the dirty properties collection.
    • Cascade property = false
    • Include rules where the dirty property is the primary property
    • Include rules where the dirty property is the input property
    • With the previous behavior, the cascade = true would cause a single recursive level of cascade for both dirty properties and input properties within CheckRulesForProperty. This step does the functional equivalent since CheckRules is not allowing CheckRulesForProperty to do it.
    • This affected / dirty properties that are the output of this step is different from the original set of affected / dirty properties.
  4. If CascadeOnDirtyProperties was true, repeat the previous step until no new dirty properties are returned
    • This preserves the behavior when CascadeOnDirtyProperties was enabled at the beginning of CheckRules
    • Include rules where the dirty property is the primary property
    • Include rules where the dirty property is the input property
    • This is the functional equivalent of the block in CheckRulesForProperty that handles when the CascadeOnDirtyProperties is true.
  5. Restore CascadeOnDirtyProperties value from the local variable in the first step.

The code details within CheckRules will be very similar to the code within CheckRulesForProperty. The difference is that CheckRuelsForProperty is recursive per property, while CheckRules is batch oriented.

Rules example scenario for desk check

This example is for an order scenario and has a sample of the complexity I deal with on a regular basis.

This scenario has a common that is used by multiple properties

  • SignAgreement rule
    • Has a primary property and an array of input properties
    • Rule execution confirms that all properties have the same sign and sets validation status of the property

Here are the Involved properties and the rules assigned to each one as the primary property. All rules have equal priority.

  • Customer credit limit property
    • No rules
  • Customer current credit balance property
    • No rules
  • On credit hold property
    • SetCheckoutStatus rule
      • Has no input properties other than the primary property
      • Has affected property for can check out property
      • Execution adds out value for can check out to be the opposite of the on credit hold property value
  • Order total property
    • Sum rule with input properties for product amount property, shipping charge property and sales tax property
    • SetCreditHold rule
      • Has input properties of customer credit limit, current balance, and on credit hold property
      • Has affected property for credit hold property
      • Execution adds out value for credit hold property based on order total + credit balance > credit limit
  • Product amount property
    • SignAgreement rule with input properties of sales tax and sign of freight charge
  • Freight charge property
    • SignAgreement rule with input properties of sales tax and sign of product amount
  • Sales tax property
    • SignAgreement rule with input properties of freight charge and sign of product amount
  • Can check out property
    • No rules

Current behavior

Because a change to the product amount affects the order total, and this then affects the credit hold status, and that then affects the can check out property, this business object uses the advanced feature to cascade on dirty properties. Presume that on credit hold and can check out properties are virtual and both initialized to false. Also presume that the order amounts are such that the order would be considered to be on credit hold. All other properties were initialized with LoadProperty.

This list attempts to show the recursive nature of how the rules are being executed.

  • On credit hold SetCheckoutStatus rule runs
    • Can check out property is changed from false to true
    • No cascade calls because can check out has no rules and is not used as an input property.
  • Order total Sum rule runs and Order total SetCreditHold rule runs
    • On credit hold property changes from false to true, triggering 1st-level cascade for on credit hold primary rules and no rules based on input properties
    • On credit hold SetCheckoutStatus rule runs - 2nd time
      • Can check out property is changed from true to false
      • No more cascade calls because can check out has no rules and is not used as an input property
  • Product amount SignAgreement rule runs
    • Order total sum rule runs - 2nd time
      • No more cascade
  • Freight charge SignAgreement rule runs
    • Order total sum rule runs - 3rd time
      • No more cascade
  • Sales tax SignAgreement rule runs
    • Order total sum rule runs - 4th time
      • No more cascade

New behavior

  • Advanced cascade is turned off
  • All rules run with no cascade
    • On credit hold SetCheckoutStatus rule runs and adds dirty property for Can check out property (it went from false to true)
    • Order total Sum rule runs with no dirty properties generated
    • Order total SetCreditHold rule runs and adds dirty property for can On credit hold property (it went from false to true)
    • Product amount SignAgreement rule runs with no dirty properties generated
    • Freight charge SignAgreement rule runs with no dirty properties generated
    • Sales tax SignAgreement rule runs with no dirty properties generated
      Note: If we stopped here, the object does not have a valid state because on credit hold and can check out are both true.
  • Run rules for dirty properties with no cascade
    • Can check out property has no rules
    • On credit hold SetCheckoutStatus rule runs (2nd time) and it adds a dirty property for Can check out property (it went from true to false)
  • Advanced cascade was enabled, so repeat until no more dirty properties
    • Can check out property has no rules
    • No new dirty properties since no rules were run

A couple rules are still run multiple times, but this is unavoidable when you have actual complex rule dependencies that cause property changes. In many scenarios where there are many complex dependencies but not affected properties, CheckRules will simply run every rule a single time and have no more work to do.

Originally posted by @hurcane in #3720 (reply in thread)

@rockfordlhotka
Copy link
Member Author

This is based on a long conversation in CSLA 8 in #3720

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

1 participant