Skip to content

Conversation

codelovercc
Copy link

@codelovercc codelovercc commented Jun 30, 2023

Change the property types of ISpecification<T>.AsNoTracking and ISpecification<T>.AsNoTrackingWithIdentityResolution to Nullable<bool>. This will allow consumers to control the default tracking behavior and ignore the evaluation order. This means that if AsTracking, AsNoTracking, and AsNoTrackingWithIdentityResolution are called at the same time, only the last call will be active, If none of AsTracking, AsNoTracking, and AsNoTrackingWithIdentityResolution is called, the consumer's default tracking behavior will be used(default tracking behavior could be configured by using DbContextOptionsBuilder.UseQueryTrackingBehavior method).

Related issue: #336

Change the property types of ISpecification<T>.AsNoTracking and ISpecification<T>.AsNoTrackingWithIdentityResolution to Nullable<bool>. This will allow consumers to control the default tracking behavior and ignore the evaluation order. This means that if AsTracking, AsNoTracking, and AsNoTrackingWithIdentityResolution are called at the same time, only the last call will be active, If none of AsTracking, AsNoTracking, and AsNoTrackingWithIdentityResolution is called, the consumer's default tracking behavior will be used(default tracking behavior could be configured by using DbContextOptionsBuilder.UseQueryTrackingBehavior method).
@codelovercc
Copy link
Author

Sorry, I forget to create a new branch for the commit :(

coolbyte added 2 commits July 1, 2023 00:32
…ification<T>.AsNoTrackingWithIdentityResolution back to bool, add a property ISpecification<T>.TrackingFlag, so this is not a breaking change. Change AsNoTrackingEvaluator, AsNoTrackingWithIdentityResolutionEvaluator and AsTrackingEvaluator to check the ISpecification<T>.TrackingFlag value. If AsTracking, AsNoTracking, and AsNoTrackingWithIdentityResolution are called at the same time, only the last call will be active, If none of AsTracking, AsNoTracking, and AsNoTrackingWithIdentityResolution is called, the consumer's default tracking behavior will be used(default tracking behavior could be configured by using DbContextOptionsBuilder.UseQueryTrackingBehavior method).
@codelovercc
Copy link
Author

codelovercc commented Jul 1, 2023

About commit d30a4b9

Change the property types of ISpecification<T>.AsNoTracking and ISpecification<T>.AsNoTrackingWithIdentityResolution back to bool, add a property ISpecification<T>.TrackingFlag, so this is not a breaking change.

@fiseni
Copy link
Collaborator

fiseni commented Jul 1, 2023

Hi @codelovercc,

Thank you for your work and support here.
I'm afraid we're introducing hidden logic in this PR. The consumers should know the implementation beforehand to use it properly, and that's something we try to avoid at all costs. We should keep it as simple as possible.

I created a PR here #338 as a sample. Can you let me know why that won't work for you?

@codelovercc
Copy link
Author

Hi @fiseni ,

PR #338 can't convert AsTracking to AsNoTracking because evaluators are ordered.
Consider this case:

public class CompanySpecification : Specification<Company>
{
    public sealed override ISpecificationBuilder<T> Query => null!;

    public PaginationSpecification(CompanyQueryModel model)
    {
        // some query logic here
       // As tracked
        Query.AsTracking();
    }
}

public class CompanyPagedSpecification : CompanySpecification
{
    public PaginationSpecification(CompanyQueryModel model, IPagination paging)
    {
        Query.Skip(paging.skip).Take(paging.take);
        // As untracked
        Query.AsNoTracking();
    }
}

or

Query.AsTracking().AsNoTracking();

AsTrackingEvaluator will apply tracking behavior always be tracked.

@codelovercc
Copy link
Author

If We support AsNoTracking, we need to support AsTracking correctly, or we should drop tracking behavior in specification, let the repository or EF core configuration control this.

Query.AsNoTracking().AsTracking().AsNoTracking();

What tracking behavior would take in this code? According PR #338 ,the tracked behavior will be active. but this code just told you that it want untracked behavior.
This code could happen when consumers use multiple inheritance Specification<T>.

If we use IQueryable<T>,

var queryable = context.Set<Company>();
queryable = queryable.AsNoTracking().AsTracking().AsNoTracking();

the queryable tracking behavior is no tracking just like the code says.

@fiseni
Copy link
Collaborator

fiseni commented Jul 2, 2023

Ok, I understand your scenarios now. In simple terms, the last applied tracking behavior should be the active one. If that's the case, we can control that in the builder methods. That's why we have them, to control the mutation of the state. The rest can remain as it is. The state and conditions in the evaluators should be plain simple. Also, we don't have the luxury of modifying the evaluators, since there are users that consume these partial evaluators directly.

I updated the PR #338 too

public static ISpecificationBuilder<T> AsTracking<T>(
	this ISpecificationBuilder<T> specificationBuilder,
	bool condition) where T : class
{
  if (condition)
  {
	specificationBuilder.Specification.AsNoTracking = false;
	specificationBuilder.Specification.AsNoTrackingWithIdentityResolution = false;
	specificationBuilder.Specification.AsTracking = true;
  }

  return specificationBuilder;
}

public static ISpecificationBuilder<T> AsNoTracking<T>(
	this ISpecificationBuilder<T> specificationBuilder,
	bool condition) where T : class
{
  if (condition)
  {
	specificationBuilder.Specification.AsTracking = false;
	specificationBuilder.Specification.AsNoTrackingWithIdentityResolution = false;
	specificationBuilder.Specification.AsNoTracking = true;
  }

  return specificationBuilder;
}

public static ISpecificationBuilder<T> AsNoTrackingWithIdentityResolution<T>(
	this ISpecificationBuilder<T> specificationBuilder,
	bool condition) where T : class
{
  if (condition)
  {
	specificationBuilder.Specification.AsTracking = false;
	specificationBuilder.Specification.AsNoTracking = false;
	specificationBuilder.Specification.AsNoTrackingWithIdentityResolution = true;
  }

  return specificationBuilder;
}

Note: The specifications will always apply the features in a predefined order since we don't have a state that can incrementally change. And that's true for all other combinations, not only the tracking behavior. We do acknowledge that. On the other hand, on many occasions, we've stated that specs should be kept simple, you get what you see, no complex inheritance chain, and so on. That's why we decided not to support composite specification either. It sounds good on paper, but it hurts in the long run. Instead, we offered all possible mechanisms (spec extensions, recently offered the possibility for state change upon creation, arbitrary Items state, etc), so the users can build their specific scenarios.

@codelovercc
Copy link
Author

Glad to know, can you merge the PR #338 ? Thank you.

@fiseni
Copy link
Collaborator

fiseni commented Jul 2, 2023

Thank you for your engagement @codelovercc. Sorry for the opposition to this PR. Once we publish features, it's hard to take them back, so we're more careful. I hope you understand.

Yea, we'll merge #338. In coming weeks we'll publish a new version.

@fiseni fiseni closed this Jul 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants