-
- Notifications
You must be signed in to change notification settings - Fork 159
Write callbacks #977
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
Write callbacks #977
Conversation
215d664 to 6e72392 Compare Codecov Report
@@ Coverage Diff @@ ## master #977 +/- ## ========================================== + Coverage 91.26% 92.00% +0.74% ========================================== Files 281 283 +2 Lines 7429 7658 +229 ========================================== + Hits 6780 7046 +266 + Misses 649 612 -37
Continue to review full report at Codecov.
|
63a1455 to 7bfc5df Compare 506dbb0 to efebf5e Compare efebf5e to 723d325 Compare There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First review pass
src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs Outdated Show resolved Hide resolved
src/JsonApiDotNetCore/Serialization/Building/IncludedResourceObjectBuilder.cs Show resolved Hide resolved
60d2da0 to 2ef8424 Compare …cks; various minor clarifications and corrections elsewhere
...nApiDotNetCoreExampleTests/IntegrationTests/SoftDeletion/SoftDeletionAwareResourceService.cs Show resolved Hide resolved
test/JsonApiDotNetCoreExampleTests/IntegrationTests/SoftDeletion/SoftDeletionTests.cs Show resolved Hide resolved
...NetCoreExampleTests/IntegrationTests/ResourceDefinitions/Serialization/SerializationTests.cs Show resolved Hide resolved
test/JsonApiDotNetCoreExampleTests/IntegrationTests/Archiving/ArchiveTests.cs Show resolved Hide resolved
test/JsonApiDotNetCoreExampleTests/IntegrationTests/MultiTenancy/MultiTenancyTests.cs Outdated Show resolved Hide resolved
test/JsonApiDotNetCoreExampleTests/IntegrationTests/MultiTenancy/MultiTenancyTests.cs Outdated Show resolved Hide resolved
test/JsonApiDotNetCoreExampleTests/IntegrationTests/MultiTenancy/MultiTenancyTests.cs Show resolved Hide resolved
...DotNetCoreExampleTests/IntegrationTests/Microservices/FireAndForgetDelivery/MessageBroker.cs Show resolved Hide resolved
...eExampleTests/IntegrationTests/Microservices/TransactionalOutboxPattern/OutboxTests.Group.cs Outdated Show resolved Hide resolved
| @bart-degreed, I've looked at this again and it seems it would work for my role-based access control use case. However, I would have one question. Before I can ask that, I need to provide some background. Background: I've now implemented role-based access control, using two extension points. Firstly, read access is controlled with resource definitions (but I decided to use those a while after you proposed to look at this PR, because I first tried resource services, which did not work out). I created a base class Secondly, I then also needed to control write access but, to be honest, did not consider those write callbacks carefully enough. The reason is that, at first, I did not consider resource definitions for read/query access control (see above). Therefore, I first played with resource services (derived from
Question: At last, the question. Using resource definitions with write callbacks, is it possible to avoid having to implement two mechanisms, i.e., the First Mechanism that is based on Based on the access permissions, you might want to return the following results for a GET request:
For the first point, the API would have to include something like: Task BeforeSerializeAsync(ISet<TResource> resources);to let the implementer remove resources from the set, using Linq expressions rather than |
| @ThomasBarnekow see #1001 |
The PR extends
IResourceDefinitionwith callbacks for:Additions to IResourceDefinition (click to expand)
Part of this PR adds/improves examples for:
Fixes #934
Fixes #952