Skip to content

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

Merged
merged 22 commits into from
May 19, 2021
Merged

Write callbacks #977

merged 22 commits into from
May 19, 2021

Conversation

bart-degreed
Copy link
Contributor

@bart-degreed bart-degreed commented Apr 12, 2021

The PR extends IResourceDefinition with callbacks for:

  • Resource write operations (post/patch/delete resource)
  • Relationship write operations (post/patch/delete relationship and post/patch-resource-with-relationships)
  • Deserialization of incoming resource from post/patch request body
  • Serialization of outgoing resources (primary and side-loaded includes)
Additions to IResourceDefinition (click to expand)
namespace JsonApiDotNetCore.Resources
{
    public interface IResourceDefinition<TResource, TId>
        where TResource : class, IIdentifiable<TId>
    {
        /// <summary>
        /// Executes after the original version of the resource has been retrieved from the underlying data store, as part of a write request.
        /// <para>
        /// Implementing this method enables to perform validations and make changes to <paramref name="resource" />, before the fields from the request are
        /// copied into it.
        /// </para>
        /// <para>
        /// For POST resource requests, this method is typically used to assign property default values or to set required relationships by side-loading the
        /// related resources and linking them.
        /// </para>
        /// </summary>
        /// <param name="resource">
        /// The original resource retrieved from the underlying data store, or a freshly instantiated resource in case of a POST resource request.
        /// </param>
        /// <param name="operationKind">
        /// Identifies from which endpoint this method was called. Possible values: <see cref="OperationKind.CreateResource" />,
        /// <see cref="OperationKind.UpdateResource" />, <see cref="OperationKind.SetRelationship" /> and <see cref="OperationKind.RemoveFromRelationship" />.
        /// Note this intentionally excludes <see cref="OperationKind.DeleteResource" /> and <see cref="OperationKind.AddToRelationship" />, because for those
        /// endpoints no resource is retrieved upfront.
        /// </param>
        /// <param name="cancellationToken">
        /// Propagates notification that request handling should be canceled.
        /// </param>
        Task OnPrepareWriteAsync(TResource resource, OperationKind operationKind, CancellationToken cancellationToken);

        /// <summary>
        /// Executes before setting (or clearing) the resource at the right side of a to-one relationship.
        /// <para>
        /// Implementing this method enables to perform validations and change <paramref name="rightResourceId" />, before the relationship is updated.
        /// </para>
        /// </summary>
        /// <param name="leftResource">
        /// The original resource as retrieved from the underlying data store. The indication "left" specifies that <paramref name="hasOneRelationship" /> is
        /// declared on <typeparamref name="TResource" />.
        /// </param>
        /// <param name="hasOneRelationship">
        /// The to-one relationship being set.
        /// </param>
        /// <param name="rightResourceId">
        /// The new resource identifier (or <c>null</c> to clear the relationship), coming from the request.
        /// </param>
        /// <param name="operationKind">
        /// Identifies from which endpoint this method was called. Possible values: <see cref="OperationKind.CreateResource" />,
        /// <see cref="OperationKind.UpdateResource" /> and <see cref="OperationKind.SetRelationship" />.
        /// </param>
        /// <param name="cancellationToken">
        /// Propagates notification that request handling should be canceled.
        /// </param>
        /// <returns>
        /// The replacement resource identifier, or <c>null</c> to clear the relationship. Returns <paramref name="rightResourceId" /> by default.
        /// </returns>
        Task<IIdentifiable> OnSetToOneRelationshipAsync(TResource leftResource, HasOneAttribute hasOneRelationship, IIdentifiable rightResourceId,
            OperationKind operationKind, CancellationToken cancellationToken);

        /// <summary>
        /// Executes before setting the resources at the right side of a to-many relationship. This replaces on existing set.
        /// <para>
        /// Implementing this method enables to perform validations and make changes to <paramref name="rightResourceIds" />, before the relationship is updated.
        /// </para>
        /// </summary>
        /// <param name="leftResource">
        /// The original resource as retrieved from the underlying data store. The indication "left" specifies that <paramref name="hasManyRelationship" /> is
        /// declared on <typeparamref name="TResource" />.
        /// </param>
        /// <param name="hasManyRelationship">
        /// The to-many relationship being set.
        /// </param>
        /// <param name="rightResourceIds">
        /// The set of resource identifiers to replace any existing set with, coming from the request.
        /// </param>
        /// <param name="operationKind">
        /// Identifies from which endpoint this method was called. Possible values: <see cref="OperationKind.CreateResource" />,
        /// <see cref="OperationKind.UpdateResource" /> and <see cref="OperationKind.SetRelationship" />.
        /// </param>
        /// <param name="cancellationToken">
        /// Propagates notification that request handling should be canceled.
        /// </param>
        Task OnSetToManyRelationshipAsync(TResource leftResource, HasManyAttribute hasManyRelationship, ISet<IIdentifiable> rightResourceIds,
            OperationKind operationKind, CancellationToken cancellationToken);

        /// <summary>
        /// Executes before adding resources to the right side of a to-many relationship, as part of a POST relationship request.
        /// <para>
        /// Implementing this method enables to perform validations and make changes to <paramref name="rightResourceIds" />, before the relationship is updated.
        /// </para>
        /// </summary>
        /// <param name="leftResourceId">
        /// Identifier of the left resource. The indication "left" specifies that <paramref name="hasManyRelationship" /> is declared on
        /// <typeparamref name="TResource" />.
        /// </param>
        /// <param name="hasManyRelationship">
        /// The to-many relationship being added to.
        /// </param>
        /// <param name="rightResourceIds">
        /// The set of resource identifiers to add to the to-many relationship, coming from the request.
        /// </param>
        /// <param name="cancellationToken">
        /// Propagates notification that request handling should be canceled.
        /// </param>
        Task OnAddToRelationshipAsync(TId leftResourceId, HasManyAttribute hasManyRelationship, ISet<IIdentifiable> rightResourceIds,
            CancellationToken cancellationToken);

        /// <summary>
        /// Executes before removing resources from the right side of a to-many relationship, as part of a DELETE relationship request.
        /// <para>
        /// Implementing this method enables to perform validations and make changes to <paramref name="rightResourceIds" />, before the relationship is updated.
        /// </para>
        /// </summary>
        /// <param name="leftResource">
        /// The original resource as retrieved from the underlying data store. The indication "left" specifies that <paramref name="hasManyRelationship" /> is
        /// declared on <typeparamref name="TResource" />.
        /// </param>
        /// <param name="hasManyRelationship">
        /// The to-many relationship being removed from.
        /// </param>
        /// <param name="rightResourceIds">
        /// The set of resource identifiers to remove from the to-many relationship, coming from the request.
        /// </param>
        /// <param name="cancellationToken">
        /// Propagates notification that request handling should be canceled.
        /// </param>
        Task OnRemoveFromRelationshipAsync(TResource leftResource, HasManyAttribute hasManyRelationship, ISet<IIdentifiable> rightResourceIds,
            CancellationToken cancellationToken);

        /// <summary>
        /// Executes before writing the changed resource to the underlying data store, as part of a write request.
        /// <para>
        /// Implementing this method enables to perform validations and make changes to <paramref name="resource" />, after the fields from the request have been
        /// copied into it.
        /// </para>
        /// <para>
        /// An example usage is to set the last-modification timestamp, overwriting the value from the incoming request.
        /// </para>
        /// <para>
        /// Another use case is to add a notification message to an outbox table, which gets committed along with the resource write in a single transaction (see
        /// https://microservices.io/patterns/data/transactional-outbox.html).
        /// </para>
        /// </summary>
        /// <param name="resource">
        /// The original resource retrieved from the underlying data store (or a freshly instantiated resource in case of a POST resource request), updated with
        /// the changes from the incoming request. Exception: In case <paramref name="operationKind" /> is <see cref="OperationKind.DeleteResource" /> or
        /// <see cref="OperationKind.AddToRelationship" />, this is an empty object with only the <see cref="Identifiable{T}.Id" /> property set, because for
        /// those endpoints no resource is retrieved upfront.
        /// </param>
        /// <param name="operationKind">
        /// Identifies from which endpoint this method was called. Possible values: <see cref="OperationKind.CreateResource" />,
        /// <see cref="OperationKind.UpdateResource" />, <see cref="OperationKind.DeleteResource" />, <see cref="OperationKind.SetRelationship" />,
        /// <see cref="OperationKind.AddToRelationship" /> and <see cref="OperationKind.RemoveFromRelationship" />.
        /// </param>
        /// <param name="cancellationToken">
        /// Propagates notification that request handling should be canceled.
        /// </param>
        Task OnWritingAsync(TResource resource, OperationKind operationKind, CancellationToken cancellationToken);

        /// <summary>
        /// Executes after successfully writing the changed resource to the underlying data store, as part of a write request.
        /// <para>
        /// Implementing this method enables to run additional logic, for example enqueue a notification message on a service bus.
        /// </para>
        /// </summary>
        /// <param name="resource">
        /// The resource as written to the underlying data store.
        /// </param>
        /// <param name="operationKind">
        /// Identifies from which endpoint this method was called. Possible values: <see cref="OperationKind.CreateResource" />,
        /// <see cref="OperationKind.UpdateResource" />, <see cref="OperationKind.DeleteResource" />, <see cref="OperationKind.SetRelationship" />,
        /// <see cref="OperationKind.AddToRelationship" /> and <see cref="OperationKind.RemoveFromRelationship" />.
        /// </param>
        /// <param name="cancellationToken">
        /// Propagates notification that request handling should be canceled.
        /// </param>
        Task OnWriteSucceededAsync(TResource resource, OperationKind operationKind, CancellationToken cancellationToken);

        /// <summary>
        /// Executes after a resource has been deserialized from an incoming request body.
        /// </summary>
        /// <para>
        /// Implementing this method enables to change the incoming resource before it enters an ASP.NET Controller Action method.
        /// </para>
        /// <para>
        /// Changing attributes on <paramref name="resource" /> from this method may break detection of side effects on resource POST/PATCH requests, because
        /// side effect detection considers any changes done from this method to be part of the incoming request body. So setting additional attributes from this
        /// method (that were not sent by the client) are not considered side effects, resulting in incorrectly reporting that there were no side effects.
        /// </para>
        /// <param name="resource">
        /// The deserialized resource.
        /// </param>
        void OnDeserialize(TResource resource);

        /// <summary>
        /// Executes before a (primary or included) resource is serialized into an outgoing response body.
        /// </summary>
        /// <para>
        /// Implementing this method enables to change the returned resource, for example scrub sensitive data or transform returned attribute values.
        /// </para>
        /// <para>
        /// Changing attributes on <paramref name="resource" /> from this method may break detection of side effects on resource POST/PATCH requests. What this
        /// means is that if side effects were detected before, this is not re-evaluated after running this method, so it may incorrectly report side effects if
        /// they were undone by this method.
        /// </para>
        /// <param name="resource">
        /// The serialized resource.
        /// </param>
        void OnSerialize(TResource resource);
    }
}

Part of this PR adds/improves examples for:

  • Soft-deletion
  • Multi-tenancy
  • Archiving (excludes archived resources in collection requests)
  • The Transactional Outbox microservices pattern
  • Fire-and-forget notification
  • Custom business logic: state-transition validation
  • Edit incoming/outgoing resources (serialization)

Fixes #934
Fixes #952

@codecov
Copy link

codecov bot commented Apr 13, 2021

Codecov Report

Merging #977 (3bd29b5) into master (d033166) will increase coverage by 0.74%.
The diff coverage is 98.86%.

Impacted file tree graph

@@            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     
Impacted Files Coverage Δ
...onApiDotNetCore/Services/JsonApiResourceService.cs 98.44% <97.29%> (+0.04%) ⬆️
...Core/Repositories/EntityFrameworkCoreRepository.cs 99.20% <98.11%> (-0.33%) ⬇️
...tCore/Configuration/ServiceCollectionExtensions.cs 100.00% <100.00%> (ø)
...iDotNetCore/Queries/Internal/QueryLayerComposer.cs 97.47% <100.00%> (+0.01%) ⬆️
...iDotNetCore/Resources/JsonApiResourceDefinition.cs 100.00% <100.00%> (ø)
...DotNetCore/Resources/ResourceDefinitionAccessor.cs 100.00% <100.00%> (ø)
src/JsonApiDotNetCore/Resources/ResourceFactory.cs 69.81% <100.00%> (+0.58%) ⬆️
...erialization/AtomicOperationsResponseSerializer.cs 98.14% <100.00%> (+0.52%) ⬆️
...lization/Building/IncludedResourceObjectBuilder.cs 97.59% <100.00%> (+0.15%) ⬆️
...onApiDotNetCore/Serialization/FieldsToSerialize.cs 100.00% <100.00%> (ø)
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d033166...3bd29b5. Read the comment docs.

@bart-degreed bart-degreed force-pushed the write-callbacks branch 7 times, most recently from 63a1455 to 7bfc5df Compare April 16, 2021 15:56
@bart-degreed bart-degreed requested a review from maurei April 30, 2021 16:07
@bart-degreed bart-degreed marked this pull request as ready for review April 30, 2021 16:07
Copy link
Member

@maurei maurei left a comment

Choose a reason for hiding this comment

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

First review pass

@ThomasBarnekow
Copy link

@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 AccessControlledResourceDefinition<TResource, TId> that derives from JsonApiResourceDefinition<TResource, TId>. It overrides the OnApplyFilter(FilterExpfression existingFilter):FilterExpression method to provide a general read access control framework and, if resource-specific permissions are required, calls an abstract method AddPermissions():void to be implemented by concrete resource definitions. Thus, to control read access, subclasses will have to construct collections of FilterExpressions. Let's call that the "First Mechanism".

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 JsonApiResourceService<TResource, TId>) but found that those would not provide what I needed because two important base class methods could not be overridden. Therefore, I thought that controllers would be a better extension point because I would have to implement one for each resource anyhow. Therefore, I created a base class called AccessControlledJsonApiController<TResource, TId> that derives from JsonApiController<TResource, TId>. It requires an ICommandAccessController that provides methods like MayCreate(IAccessControlledResource):bool. The interface IAccessControlledResource defines properties like CreatedBy:Guid (e.g., to identify the authors of a resource). My base class for all resources implements both IAccessControlledResource and IIdentifiable<TId>.

MayCreate(IAccessControlledResource):bool, for example, has access to the ApplicationUser resource that represents the authenticated user of the current request. The ApplicationUser is injected into the HttpContext by middleware that queries the database (using the DbContext directly) based on information contained in the access token. The query makes sure all other resources required to make role-based access control decisions are included and accessible via navigation properties. Thus, MayCreate(IAccessControlledResource):bool and other methods use Linq expressions to determine whether an ApplicationUser has the required ApplicationUserRole to perform the respective operation. Let's call that the "Second Mechanism".

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 FilterExpressions (also a new trick to learn for almost everybody) and the Second Mechanism that is based on Linq expressions?

Based on the access permissions, you might want to return the following results for a GET request:

  1. Where the expected result is a resource collection (e.g., GET /resources), you would return:
    • 200 OK with zero or more resources to which the user has resource-specific read access, if the user has read access to those resources in general (but there might still not be any resource that is specifically accessible); or
    • 403 Forbidden, if the user does not have the general permission to read resources of that type, for example.
  2. Where the expected result is a single resource (e.g., GET /resources/{id}), you would return:
    • 200 OK with that single resource, if the user has resource-specific read access; or
    • 403 Forbidden, if the user does not have the resource-specific read access permission.

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 FilterExpressions and, therefore, use a single mechanism instead of two. But I don't know whether or not that is possible. And whether or not that needs to be async could be discussed.

@bart-degreed bart-degreed merged commit 1157dde into master May 19, 2021
@bart-degreed bart-degreed deleted the write-callbacks branch May 19, 2021 09:22
@maurei
Copy link
Member

maurei commented May 19, 2021

@ThomasBarnekow see #1001

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add integration tests for tenant filtering Provide write callbacks in IResourceDefinition
3 participants