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

Add support for extending default evaluator list #308

Closed
HenricFranssonXM opened this issue Jan 31, 2023 · 3 comments · Fixed by #328
Closed

Add support for extending default evaluator list #308

HenricFranssonXM opened this issue Jan 31, 2023 · 3 comments · Fixed by #328
Assignees
Labels
enhancement New feature or request

Comments

@HenricFranssonXM
Copy link

There exists no possibility to extend the default evaluators listed in SpecificationEvaluator.Default. There exists an example of how to write your own evaluator in in #247 but it doesn't touch the subject of how to inject it.

If I want to extend with a custom evaluator I've to create my own list of all the default evaluators and then add my custom one. This gives a hard coded dependency and if Ardalis makes a change to the default list of evaluators I will have to update my own code as well to keep up.

Suggestion: at least change the private readonly List<IEvaluator> evaluators to protected so projects can create their own custom SpecificationEvaluator extending from Ardalis SpecificationEvaluator and add their on evaluators to the default ones.

This would result in:

  public class CustomSpecificationEvaluator : SpecificationEvaluator
  {
    public CustomSpecificationEvaluator(bool cacheEnabled = false) : base(cacheEnabled)
    {
      evaluators.Add(AsSingleQueryEvaluator.Instance);
    }
  }

instead as of today being forced to code:

  public class CustomSpecificationEvaluator : SpecificationEvaluator
  {
    public CustomSpecificationEvaluator(bool cacheEnabled = false) : base(GetAllEvaluators(cacheEnabled))
    {
    }

    private static IEnumerable<IEvaluator> GetAllEvaluators(bool cacheEnabled = false)
    {
      var defaultAndCustomEvaluators = new List<IEvaluator>();
      // add Ardalis default evaluators
      defaultAndCustomEvaluators.AddRange(new IEvaluator[]
      {
                // this list will need to be maintained
                WhereEvaluator.Instance,
                SearchEvaluator.Instance,
                cacheEnabled ? IncludeEvaluator.Cached : IncludeEvaluator.Default,
                OrderEvaluator.Instance,
                PaginationEvaluator.Instance,
                AsNoTrackingEvaluator.Instance,
                IgnoreQueryFiltersEvaluator.Instance,
#if !NETSTANDARD2_0
                AsSplitQueryEvaluator.Instance,
                AsNoTrackingWithIdentityResolutionEvaluator.Instance
#endif
      });

      // add custom evaluators
      defaultAndCustomEvaluators.AddRange(new IEvaluator[]
      {
        AsSingleQueryEvaluator.Instance
      });

      return defaultAndCustomEvaluators;
    }
  }

There exists many solutions to this problem. Another one would be to expose the evaluators property. Whatever solution that don't force you to copy-paste the default evaluator list is fine. 🙂

@fiseni
Copy link
Collaborator

fiseni commented Jan 31, 2023

Hi @HenricFranssonXM

Please refer to this comment on why we didn't add support for appending custom evaluators (the order might be important).
Also, here is an example of how to create and inject a custom evaluator. I know we have to consolidate the documentation. Any help is welcome.

Back to the question, ahh I give up, let's update the state as protected 😄 We'll update it in the next release.

@fiseni
Copy link
Collaborator

fiseni commented Jan 31, 2023

To clarify for future readers. In some specific cases, the order of statements while building the IQueryable is important to EF Core. For instance, if you have some filtering logic (e.g. Where statement), it should be placed before any paging statements (Take/Skip). So, if you're writing your custom evaluators, be careful how you inject them.

@HenricFranssonXM
Copy link
Author

@fiseni Thanks for the quick response and support! 🙂
If you want I can make a proposal for what to write in the currently empty chapter https://specification.ardalis.com/extensions/extend-define-evaluators.html when the change is out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants