Skip to content

JsonApiContext should be split into multiple classes for easy testability #504

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

Closed
wisepotato opened this issue May 3, 2019 · 1 comment
Labels
breaking-change enhancement refactor RFC Request for comments. These issues have major impact on the direction of the library.

Comments

@wisepotato
Copy link
Contributor

wisepotato commented May 3, 2019

And we are mostly concerned with making sure that the deserializer has no need for the current HttpContext so that we can stop with E2E testing trivial stuff.

  • JsonApiContext is scoped

With these notes we want to see the lifecycle of the JsonApiContext

Where does Middleware touch JsonApiContext

These fields are being set by means of JsonApiContext :

  • JsonApiContext.BeginOperation()
    • IncludedRelationships = new List<string>(); - Is populated in ApplyContext(controller)
    • AttributesToUpdate = new Dictionary<AttrAttribute, object>(); - Populated in deserializer
    • RelationshipsToUpdate = new Dictionary<RelationshipAttribute, object>(); - populated in deserializer
    • HasManyRelationshipPointers = new HasManyRelationshipPointers(); - Populated in deserializer
    • HasOneRelationshipPointers = new HasOneRelationshipPointers(); - populated in deserializer

Controller

jsonApiContext.applyContext(this ) this = controller
QuerySet = _queryParser.Parse(context.Request.Query);
BasePath = new LinkBuilder(this).GetBasePath(context, _controllerContext.RequestEntity.EntityName);
PageManager = GetPageManager();
IsRelationshipPath = PathIsRelationship(context.Request.Path.Value);
so

  • Initilization of JsonApiContext field happens in the deserializer (Middleware) and controller
  • afterwards the fields of JsonApiContext are used readonly (I think

Things that need to be taken out of the deserializer

  • Its convenient that when you are going through it you register the JsonApiContext because we need it later on in the repository/service layer. But we don't need it for `JsonApiClient in the repo/service layer
    • How to solve this .. and stay flexible?
  • Things that need to be parsed in the controller
    • In stead of using the JsonApiContext itself we should use a relevant service for this, maybe even multiple, and inject them when needed

      JsonApiContext wordt dan eigenlijk een JsonApiContextInitializer, in de zin van dat het niet de context zelf meer is maar alleen de context populate in de set van context services.

Scoped

  • URL query parameters -> QueryParamFetcher
  • Body -> BodyInterpreter
  • Hooks -> HookExecutor
  • ResourceDefinition

Singleton

  • Options -> OptionsFetcher
  • Generic processor factory GenericProcessorFactory
  • LoggerFactory
  • IResourceMapper - MapIn + MapOut
  • MetaBuilder - Inject when needed
@wisepotato wisepotato changed the title HttpContext should not be needed in the deserializer layer JsonApiContext should not be needed in the deserializer layer May 3, 2019
@wisepotato wisepotato changed the title JsonApiContext should not be needed in the deserializer layer JsonApiContext should be split into multiple classes for easy testability May 13, 2019
@wisepotato wisepotato added this to the v4.0 milestone May 13, 2019
@wisepotato wisepotato added breaking-change enhancement refactor RFC Request for comments. These issues have major impact on the direction of the library. labels May 13, 2019
@wisepotato wisepotato removed this from the v4.0 milestone Oct 9, 2019
@maurei
Copy link
Member

maurei commented Oct 10, 2019

resolved in #558

@maurei maurei closed this as completed Oct 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change enhancement refactor RFC Request for comments. These issues have major impact on the direction of the library.
Development

No branches or pull requests

2 participants