apex-enterprise-patterns / fflib-apex-common

Common Apex Library supporting Apex Enterprise Patterns and much more!
BSD 3-Clause "New" or "Revised" License
903 stars 514 forks source link

criteria factory suggestion #393

Closed baksale closed 6 months ago

baksale commented 2 years ago

This change is Reviewable

Hi guys, please take a look at the suggestion for Criteria Factory. If you like the idea, I will add security checks and bind variables support.

Any other feedback is also welcome. This is related to #292

    String criteriaAsString =
      'IsWon = true AND Amount > 100000 AND' +
      ' ((Account.NumberOfEmployees > 1000 AND Account.BillingCountry = \'USA\') OR (Owner.Title = \'CEO\' AND Owner.Country = \'USA\'))';

    fflib_CriteriaFactory bigUsCompanies = new fflib_CriteriaFactory()
      .configureForReferenceField(Opportunity.AccountId)
      .greaterThan(Account.NumberOfEmployees, 1000)
      .equalsTo(Account.BillingCountry, 'USA');

    fflib_CriteriaFactory ownerInUs = new fflib_CriteriaFactory()
      .configureForReferenceField(Opportunity.OwnerId)
      .equalsTo(User.Title, 'CEO')
      .equalsTo(User.Country, 'USA');

    fflib_CriteriaFactory cb = new fflib_CriteriaFactory()
      .equalsTo(Opportunity.IsWon, true)
      .greaterThan(Opportunity.Amount, 100000)
      .composite(
            new fflib_CriteriaFactory()
                  .composite(bigUsCompanies)
                  .withOr()
                  .composite(ownerInUs));

    System.assertEquals(criteriaAsString, cb.toCriteria());

More details can be found here

wimvelzeboer commented 2 years ago

@baksale I like that this criteria/condition factory is a small class and resolves this long open issue. But have you ever looked at this fflib_Critera class? It has a wider scope and can also be used for realtime evaluations in domain classes.

baksale commented 2 years ago

Thanks for the feedback @wimvelzeboer

fflib_Criteria looks interesting and much more solid. Based on the code, I could see the class has 2 responsibilities:

  1. Construct criteria to be used in SOQL
  2. Construct criteria to evaluate against records in runtime

I would suggest to split it into 2 or even 3 classes:

  1. the criteria itself
  2. soql criteria converter
  3. evaluator

Evaluator class could contain a reference to Criteria class to do the evaluation of the records in runtime.

I did some load testing:

  1. on average fflib_CriteriaFactory consumes 4 times less heap vs fflib_Criteria
  2. cpu consumption is about the same or difference is not significant Can share the tests.

Two things I could not understand:

  1. Like and Not Like builder methods are absent, but saw the operator for Like - it is not supported yet, isn't it?
  2. Reference Attributes-based criteria construction - could not understand if it is supported or not; could not find builder methods.

fflib_CriteriaFactory alternatives for this:

cf().isNotLike(Contact.LastName, 'A%') // could not find analogue in fflib_Criteria
cf()
  .configureForReferenceField(Opportunity.AccountId) //similar to fflib_QueryFactory approach
  .greaterThan(Account.NumberOfEmployees, 1000)
  .equalsTo(Account.BillingCountry, 'USA')

We like fflib_QueryFactory but criteria build capability is missing and badly required there.

Do you think we can incorporate fflib_CriteriaFactory or fflib_Criteria into the main codebase?

Happy to incorporate any feedback if it is going to be useful. I like the fields accessibility check inside fflib_QueryFactory, so would like to apply the same for Fields used in Criteria.

baksale commented 2 years ago

Hu @stohn777 @daveespo

Could you provide any hints to move this forward?

This is the example from the PR description:

   String criteriaAsString =
      'IsWon = true AND Amount > 100000 AND' +
      ' ((Account.NumberOfEmployees > 1000 AND Account.BillingCountry = \'USA\') OR (Owner.Title = \'CEO\' AND Owner.Country = \'USA\'))';

    fflib_CriteriaFactory bigUsCompanies = new fflib_CriteriaFactory()
      .configureForReferenceField(Opportunity.AccountId)
      .greaterThan(Account.NumberOfEmployees, 1000)
      .equalsTo(Account.BillingCountry, 'USA');

    fflib_CriteriaFactory ownerInUs = new fflib_CriteriaFactory()
      .configureForReferenceField(Opportunity.OwnerId)
      .equalsTo(User.Title, 'CEO')
      .equalsTo(User.Country, 'USA');

    fflib_CriteriaFactory cb = new fflib_CriteriaFactory()
      .equalsTo(Opportunity.IsWon, true)
      .greaterThan(Opportunity.Amount, 100000)
      .composite(
            new fflib_CriteriaFactory()
                  .composite(bigUsCompanies)
                  .withOr()
                  .composite(ownerInUs));

    System.assertEquals(criteriaAsString, cb.toCriteria());

I need some guidance how to move this forward. If you like the idea - what are the next steps/feedback.

I saw in many other similar PRs/Issues you mentioned that you put decisions on hold as the strategy is not clear. Can I help with it anyhow?

daveespo commented 6 months ago

See #483