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

Introduce RepositoryInterceptor and Enable/DisableTracking() extension methods. #17491

Merged
merged 14 commits into from
Sep 3, 2023

Conversation

maliming
Copy link
Member

@maliming maliming commented Aug 27, 2023

Added two extension methods so the developer can disable/enable entity tracking by these methods manually.

IDisposable DisableTracking(this IRepository repository)
IDisposable EnableTracking(this IRepository repository)

The EnableEntityChangeTrackingAttribute and DisableEntityChangeTrackingAttribute can be used by class/interface/method. The RepositoryInterceptor will get its Enabled property and change the current Tracking value by IEntityChangeTrackingProvider.

The Repository will check these conditions to decide whether to enable tracking.

  • IsChangeTrackingEnabled (null by default. You can change it by DisableTracking or EnableTracking)
  • The current Tracking value of IEntityChangeTrackingProvider . (It will be null if there is no EntityChangeTrackingAttribute)
  • true by default.

Resolves #17487

Checklist

  • I fully tested it as developer / designer and created unit / integration tests
  • I documented it (or no need to document or I will create a separate documentation issue)

@maliming maliming marked this pull request as ready for review August 27, 2023 09:21
@maliming maliming marked this pull request as draft August 27, 2023 09:21
@maliming maliming added this to the 8.0-preview milestone Aug 27, 2023
@codecov
Copy link

codecov bot commented Aug 27, 2023

Codecov Report

Merging #17491 (9356884) into dev (51c742e) will decrease coverage by 0.05%.
Report is 1 commits behind head on dev.
The diff coverage is 41.04%.

@@            Coverage Diff             @@
##              dev   #17491      +/-   ##
==========================================
- Coverage   53.60%   53.55%   -0.05%     
==========================================
  Files        3025     3032       +7     
  Lines       94418    94604     +186     
==========================================
+ Hits        50610    50664      +54     
- Misses      43808    43940     +132     
Files Changed Coverage Δ
...olo/Abp/Application/Services/ApplicationService.cs 82.69% <ø> (ø)
.../ChangeTracking/ChangeTrackingInterceptor_Tests.cs 0.00% <0.00%> (ø)
...eworkCore/Repositories/ReadOnlyRepository_Tests.cs 0.00% <0.00%> (ø)
...positories/EntityFrameworkCore/EfCoreRepository.cs 76.45% <66.66%> (-1.65%) ⬇️
...Domain/ChangeTracking/ChangeTrackingInterceptor.cs 78.57% <78.57%> (ø)
...olo/Abp/Domain/Repositories/BasicRepositoryBase.cs 52.32% <84.61%> (+5.02%) ⬆️
.../Abp/Domain/ChangeTracking/ChangeTrackingHelper.cs 89.65% <89.65%> (ø)
...lo/Abp/Domain/Repositories/RepositoryExtensions.cs 86.72% <93.75%> (+1.87%) ⬆️
...Injection/ServiceCollectionRepositoryExtensions.cs 100.00% <100.00%> (ø)
...p.Ddd.Domain/Volo/Abp/Domain/AbpDddDomainModule.cs 100.00% <100.00%> (ø)
... and 5 more

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@maliming maliming changed the title Introduce RepositoryInterceptor and IRepository.DisableTracking() extension methods. Introduce RepositoryInterceptor and Enable/DisableTracking() extension methods. Aug 27, 2023
@maliming maliming marked this pull request as ready for review August 28, 2023 02:54
@maliming maliming marked this pull request as draft August 28, 2023 03:12
@hikalkan hikalkan self-requested a review August 29, 2023 07:19
@@ -107,7 +108,7 @@ public EfCoreRepository(IDbContextProvider<TDbContext> dbContextProvider)

public async override Task<TEntity> InsertAsync(TEntity entity, bool autoSave = false, CancellationToken cancellationToken = default)
{
CheckReadOnly();
CheckChangeTracking();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inserting/updating/deleting an entity work even if you don't track changes for queried entities.

Copy link
Member Author

@maliming maliming Aug 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the entity returned by the method or entity from parameters is no tracking after the method is called.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where it is done? I didn't see any code part that detaches inserted/updated entities.

Copy link
Member Author

@maliming maliming Sep 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be code like this, So I think output warning logs are necessary.

using (var uow = uowManager.Begin())
{
    var author = await authorRepository.InsertAsync(new Author(Guid.NewGuid(), "Douglas"));

    // Entity still under tracking
    author.Name = "Douglas2";
    await uow.CompleteAsync();
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, in this example, author object is being tracked, and the name change is auto-saved, right? But the warning message says "Your changes may not be saved". Isn't that misleading the developer?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code I share will not work if we disable entity tracking.
But developers may use such code, so I think we should output warnings when entity tracking is disabled.

@maliming maliming marked this pull request as ready for review September 3, 2023 09:48
@hikalkan hikalkan merged commit 9971dc9 into dev Sep 3, 2023
2 of 4 checks passed
@hikalkan hikalkan deleted the IsChangeTrackingEnabled branch September 3, 2023 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce IRepository.DisableTracking() extension method
2 participants