Skip to content

RFC: Separate Concerns of JsonApiContext #253

Closed
@jaredcnance

Description

@jaredcnance

Start Date: N/A
Status: WIP
RFC PR: N/A

Summary

Refactor the JsonApiContext into classes with singular responsibilities.

Motivation

The JsonApiContext is a god container whose members are set in various places during the request cycle. Because of the large set of responsibilities this class takes on, it can make testing difficult. Its role, in general, is to ensure ambient data is available whenever necessary.

This was more of an issue during the early stages of development when the architecture was still in flux. Now that the internal APIs are solidifying, we can begin separating this class into single responsibility implementations.

Note: The primary benefit that still exists of the JsonApiContext is that it reduces the number of constructor dependencies required for each class.

Detailed Design

TODO: map all the consumption points and identify separation strategies

Controllers (#255)

ApplyContext<TController>. This method is responsible for:

  • Setting the controller type on the context
    • Move into ActionFilter
  • Setting RequestEntity
    • Move into ActionFilter
    • May create a scoped IResourceRequest that provides Get(/* ... */) and Set(/* ... */) methods to replace this functionality.
  • Parsing Query Params
    • Move into ActionFilter
  • Setting the list of IncludedRelationships
  • Determining if it is a RelationshipPath
  • Creating a PageManager

_jsonApiContext = jsonApiContext.ApplyContext<T>(this);

We should be able to remove the direct dependency by moving the functionality into an ActionFilter without any other modifications. However, the functionality defined above still needs to be factored out of the context where possible.

Deserializer

  • Sets RequestEntity. This is redundant with the controller and we should be able to remove this step.

    var contextEntity = _jsonApiContext.ContextGraph.GetContextEntity(data.Type?.ToString());
    _jsonApiContext.RequestEntity = contextEntity;

  • Sets IsBulkOperationRequest

  • Sets DocumentMeta: we should consider storing the deserialized document in the request somewhere

  • Sets AttributesToUpdate

  • Depends on _jsonApiContext.Options.SerializerSettings: we could directly depend on JsonApiOptions instead

  • Sets RelationshipsToUpdate

Proposed Approach:

  • Introduce JsonApiDeSerializerFactory that produces either:
    • JsonApiOperationsDeserializer
    • JsonApiDeSerializer (consider aliasing to a more descriptive name)
  • Optionally depend on JsonApiOptions directly
  • Depend on the ContextGraph directly
  • Extract out a DocumentRequest type that includes:
    • The original Documents or Document
    • The attributes to update
    • The relationships to update
public JsonApiDeSerializer(ContextGraph contextGraph, JsonApiOptions options = null)
{
    _contextGraph = contextGraph;
    _options = options ?? JsonApiOptions.Default;
}

Phased Implementation

The intent is to make changes in such a way that users of the library can gradually depend on the new structure without forced breaking changes. The rollout of this change will span 4 releases as outlined below:

  1. Allow internal consuming classes to depend on the new interfaces in a non-breaking way. For example, if we can find a way to remove the requirement that the controller call ApplyContext<T>, then there is no need for the controller to even depend on the JsonApiContext. In this case, we would keep the original constructor but provide an implementation that does not require the context:
public BaseJsonApiController(
            IJsonApiContext jsonApiContext,
            IGetAllService<T, TId> getAll = null,
            /* ... */
) { /* ... */ }

public BaseJsonApiController(
            IGetAllService<T, TId> getAll = null,
            /* ... */
) { /* ... */ }
  1. Obsolete the APIs dependent upon IJsonApiContext.
[Obsolete(/* ... */, error: false)] 
  1. Publish a breaking change release (v3 ?) that sets the ObsoleteAttribute error to true
[Obsolete(/* ... */, error: true)] 
  1. Drop the APIs in the following major release

App vs. Request vs. Operations

This needs its own issue

The JsonApiContext stores three kinds of information

  • Application: state that does not change for the life of the application
    • JsonApiOptions
    • ContextGraph
  • Request: state that applies to the entire request
    • BasePath
    • IsRelationshipPath
    • IsBulkOperationRequest
    • ControllerType
    • DocumentMeta
  • Operational: per-operation data
    • IncludedRelationships
    • AttributesToUpdate
    • RelationshipsToUpdate
    • HasManyRelationshipPointers
    • HasOneRelationshipPointers
    • QuerySet
    • ...

Metadata

Metadata

Assignees

No one assigned

    Labels

    RFCRequest for comments. These issues have major impact on the direction of the library.enhancementneeds-investigation

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions