Skip to content

Improve support nested sparse field selection #583

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
maurei opened this issue Oct 17, 2019 · 0 comments
Closed

Improve support nested sparse field selection #583

maurei opened this issue Oct 17, 2019 · 0 comments

Comments

@maurei
Copy link
Member

maurei commented Oct 17, 2019

This is the recommendation from the official json:api spec with respect to sparse field selection:

GET /articles?include=author&fields[articles]=title,body&fields[people]=name

There is a serious limitation with this. This is described in json-api/json-api#1091. If article contains two relationships to people, eg a reviewer and an author, then we would not be able to do a unique field selection for either one of them. For that we would need a sparse field query syntax of the form fields[ relationship_navigation ] rather than fields[ related_type ].

We could then do things like

GET /articles?include=author,reviewer.employer
                      &fields[author]=name
                      &fields[reviewer]=age
                      &fields[reviewer.employer]=foundedAt

To support fully support such nested sparse field selection, we would have to switch to the fields[ relationship_navigation ] syntax. It seems that @milosloub already implemented this syntax for 1 level deep relationship inclusions.

Right now the fields[ type ] as well as fields[ relationship_navigation ] syntax is allowed. This is inconsistent and incompatible. What is left to do is disallow the fields[ type ] syntax.

  • This means that articles?fields[articles]=title is no longer valid. For field selection of the main request resource I propose to simply use articles?fields=title, no navigation is needed.
  • To give users a constructive error we should generate a constructive error message when the case of the previous bulletin occurs
    • fields[articles]=title would give a 400 with a message to just use fields=title
  • in the rare case that the main request resource actually has a relationship that is identical to the resource name, we will allow it. This could happen with a nested, circular data structure like directories
    • eg: folders?fields[folders]=isEndNode would still be possible
maurei added a commit that referenced this issue Oct 17, 2019
@maurei maurei closed this as completed Oct 22, 2019
maurei added a commit that referenced this issue Oct 30, 2019
* Feat/context decoupling (#557)

* feat: started work on decoupling

* feat: add total record count

* feat: green tests, new managers

* feat: changed startup

* feat: most annoying commit of my life, rewriting dozens of controllers

* feat: rename: QueryManager -> RequestManager, deeper seperation of concerns, 12 tests running...

* feat: removed JsonApiContext dependency from controller, fixed namespaced tests

* feat: decoupled controllers

* chore: renamed resourcegraphbuilder,  took out some extensions, made benchmarks work again

* feat: json api context decoupling mroe and more

* chore: readded solutions

* feat: upgrading to 2.2, setting contextentity in middleware

* fix: removed outdated authorization test

* fix: requestmeta tests

* fix: some acceptance tests

* fix: pagination

* fix: total records in meta

* feat: introduced JsonApiActionFilter

* fix: more tests

* Feat/serializer context decoupling (#558)

* feat: started work on decoupling

* feat: add total record count

* feat: green tests, new managers

* feat: changed startup

* feat: most annoying commit of my life, rewriting dozens of controllers

* feat: rename: QueryManager -> RequestManager, deeper seperation of concerns, 12 tests running...

* feat: removed JsonApiContext dependency from controller, fixed namespaced tests

* feat: decoupled controllers

* chore: renamed resourcegraphbuilder,  took out some extensions, made benchmarks work again

* feat: json api context decoupling mroe and more

* chore: readded solutions

* feat: upgrading to 2.2, setting contextentity in middleware

* fix: removed outdated authorization test

* fix: requestmeta tests

* fix: some acceptance tests

* fix: pagination

* fix: total records in meta

* feat: introduced JsonApiActionFilter

* fix: more tests

* feat: decoupled deserializer and serializer

* chore: remove / cleanup old serializers

* chore: remove operation services

* tests: unit tests client/server (de)serializers

* chore: various edits to run tests

* fix: rm dasherized resolver

* chore: remove JsonApiContext and corresponding interface

* chore: reorganize namespaces

* chore: rm old tests

* chore: rm unused document builder

* chore: add comments to deserialization classes

* chore: add comments

* chore: rm IJsonApiContext from sort attr instantiation

* feat: (re)introduced omit null value behaviour (and now omit default value behaviour too) using a serializer options provider

* chore: remove more jsonapicontext references

* chore: minor renames, add wiki for serialization

* chore: improve wiki

* chore: wiki

* chore: wiki

* chore: add a bunch of comments, renamed a few services

* chore: wired up nested sparse field selection

* chore: wired up omit behaviour

* chore: prettied some comments

* chore: remove (almost) all references to jsonapicontext in unit tests project

* chore: removed last bits of jsonapicontext

* fix: tests serialization passing again

* chore: rename included query service to include query service

* chore: improve comment

* chore: improve comment

* chore: delete jsonapicontext tests

* chore: remove space

* test: email config

* chore: rename client / server (de)serializer to request/response (de)serializer

* chore: rename client / server (de)serializer to request/response (de)serializer

* feat: introduce IQueryParameter

* chore: move request/response (de)serializer to client/server namespace for easier future isolation

* chore: update namespaces

* chore: rm textfile

* chore: fixing failing unit tests

* chore: rename query services, adjustment namespace

* chore: reorganised test files for serialization, added relationship path serialization tests

* feat: request relationship in response serializer

* chore: wired up response serializer in formatter layer

* feat: IGetRelationshipService now returns TResource instead of object, which decouples it from serializers requirements

* fix: support for serving document of resource type different from request resource, as required by resourceservice.getrelationships()

* chore: simplified linkbuilders

* chore: several fixes for e2e tests

* fix: various e2e tests, decoupled service layer from serialization format

* fix: inclusion edgecase

* chore: fix error formatting tests

* chore: naming consistency sparsefield and include query services

* chore: various adjustments to make e2e test project build and pass again

* chore: fix build various e2e tests

* chore: fix build various e2e tests

* fix: e2e test Can_Include_Nested_Relationships

* fix: e2e test Can_Patch_Entity

* fix: e2e test Patch_Entity_With_HasMany_Does_Not_Include_Relationships

* fix: e2e test Can_Create_Entity_With_Client_Defined_Id_If_Configured

* chore: rename base document parser and builder

* fix: e2e test Can_Create_Guid_Identifiable_Entity_With_Client_Defined_Id_If_Configured

* fix: unit tests various

* feat: reorganisation inheritance serialization layer

* fix: unit tests after inheritance update

* fix: wire up correct resource object builder implementation in serializers

* fix: e2e remaining CreatingDataTests

* chore: refactor creatingdata tests

* fix: e2e test paging

* fix: e2e controller tests

* chore: wiring up new resource object builders to dependency graph

* chore: response resource object builder unit test, restored repo and resourcedef unit tests

* chore: finishing touches comments serialization

* Feat/serialization wiki (#561)

* docs: add decoupling architecture, wiki folder

* docs: add v4 wiki

* chore: restructure of intro

* chore: v4

* chore: remove example projects

* chore: remove bulk related code

* chore: remove bulk related tests

* chore: remove logic related to ER splitting

* chore: remove split test project

* chore: update other test projects to pass again

* chore: fix typo

* Added value types support to the filters isnull & isnotnull. (#549) (#573)

* fix: missing reference'

* chore: fix spacing

* feat: draft of reflectively retrieving all query parameters

* chore: scaffolded required query parameter service

* chore: migrate parsing logic sparsefields

* chore: reorganising test models in unit test project

* tests: add includeservicetests

* chore: migrate parsing logic of filter to filter service

* chore: prefix private variables underscore in editorconfig

* feat: common query parameter base service

* feat: interfaces of query parameter services

* feat: sort service and filter service

* feat: wired up filter and sort service with refactored data and repo layer

* feat: new omit default and null value services, wired them up in serialization

* feat: refactored remaining query services, introduced new queryparser for better extensibility

* feat: wired up new query parser in action filter, slimmed down current request

* chores: various test fixes, minor fixes

* chores: various test fixes, minor fixes

* chore: add more unit tests

* docs: query parameters wiki

* fix: typo

* chore: rename, update wiki

* chore: various minor fixes, PR self review

* feat: added new overload in query controllers (#580)

* Cleaning repository of remaining business logic (#579)

* feat: first draft of attribute diffing in repo layer

* chore: remove EF Core-specific DetachRelationshipPointers from repository interface

* chore: replace SingleOrDefaultAsync with FirstOrDefaultAsync for increased performance

* chore: decoupled ResourceDefinition from repository layer in Filter implementation

* chore: remove updatevaluehelper

* fix: Can_Patch_Entity test

* fix: EntityResourceTest, variable name updatedFields -> targetedFields

* chore: add comment referencing to github issue

* chore: rename pageManager -> pageService

* chore: remove deprecations

* fix: undid wrong replace all of BeforeUpdate -> pageService

* chore: remove unreferenced method in ResponseSerializer

* Allow for multiple naming conventions (camelCase vs kebab-case) (#581)

* feat: draft of DI-registered ApplicationModelConvention

* feat: injectable IResourceNameFormatter and IJsonApiRoutingCOnvention now working

* test: kebab vs camelcase configuration

* feat: kebab vs camel completed

* chore: spacing

* style: spacing

* refactor: share logic between resource name formatter implementations

* docs: resource formatter comments

* docs: comments improved

* chore: remove static reference to ResourceNameFormatter

* chore: reviewer fixes

* style: removed spacing

* style: removed spacing

* fix: #583

* feat: throw error on deeply nested selections

* docs: add extra comment

* chore: processed Wisepotatos review

* Separation of concerns ResourceGraph (#586)

* refactor: move HasManyThrough GetValue and SetValue logic to HasManyThroughAttribute

* feat: remove GetPublicAttributeName from ResourceGraph and remove dependency on ResourceGraph of controller layer

* style: spacing in BaseJsonApiController<,>

* feat: remove GetEntityFromControllerName from ResourceGraph

* feat: remove GetInverseRelationship from ResourceGraph

* feat: decouples UsesDbContext from ResourceGraph and introduces decoupled application instantation

* chore: remove redundant injections of IResourceGraph

* refactor: merge IContextEntityProvider and IFieldsExplorer into IResourceGraphExplorer

* style: cleanup of variable names

* chore: various fixes for PR review, mostly style and variable naming

* style: rename IResourceGraphExplorer back to IResourceGraph, consitent usage of IContextEntityProvider

* docs: comments for DefaultActionFilter and DefaultTypeFilter

* docs: comments JsonApiApplicationBuilder

* style: consistent variable naming|

* chore: wire up IControllerResourceMapping

* refactor: routing convention implements controller mapping interface

* Update variable / class naming conventions (#589)



style: DefaultResourceRepository and DefaultEntityService as consie 833da6a
style: rename old repository interfaces 							519513d
style: ContextEntity --> ResourceContext 							d038582
style: rename Entity to Resource 									f2e411c
style: TEntity --> TResource 										3af6f9b
style: DependentType, PrincipalType --> RightType, LeftType 		ce08b4f
style: replaced principal with left and dependent with right whenev b162519
chore: remove redundant bindings throughout entire codebase 		e509956
refactor: regroupe hook container and executors 					48495d3
style: IGenericProcessor --> IHasManyThroughUpdateHelper 			f19e606
chore: delete rogue file 											7d235ab
style: rename GenericProcessorFactory --> GenericServiceFactory. 	6a186a5
chore: fix NoEntityFrameworkExample project							354a1be

* chore: process feedback

* Fix DiscoveryTests and NoEntityFrameworkExample project/tests (#590)

* fix: DiscoveryTests project

* fix: NoEntityFrameworkExample project + tests

* Simplify DefaultResourceService constructor (#592)

* refactor: simplify resource service constructor with IEnumerable<IQueryParameterService>
* feat: FirstOrDefault<TQueryParameterService>() extension method
* style: remove whitespace

* fix: fix build.sh

* fix: appveyor build

* fix: hotfix constructor ServiceDiscoveryFacadeTests

* upgrade to .net core 2.1

* .NET core 2.1 -> 2.2 (#594)

* merge: merge into correct email:

* feat: upgrade to 2.2

* chore: spacing fix

* chore: revert silencing error

* chore: small spacing fix

* chore: rm whitespace

* fix: public setters to allow testing

* .NET Core 2.2 -> 3.0  (#597)

* merge: merge into correct email:

* feat: upgrade to 2.2

* chore: spacing fix

* chore: revert silencing error

* chore: small spacing fix

* chore: remove unneeeded dependencies

* chore: fix framework dependencies

* chore: updates

* chore: test fixes

* chore: start integration tests

* chore: cleanup name of dertest

* chore: start of getRepostory

* chore: upgrade testsdk version + xunit version

* feat: use template in version control of packages in integrationTests

* feat: unit test assemblyinfo add, port old test to integration (attribute updater)

* docs: add version compatibility

* chore: add asterisk for version number

* feat: port defaultentityrepositorytests to integration tests

* chore: should add logging, done.

* tests: bad response from tests

* chore: fixes for lang version and making ifs more noticable

* fix: fix routing issue with upgrade to enableendpointrouting

* chore: HostEnvironment -> IWebHostEnvironment

* chore: act -> Act, assert -> Assert, arrange -> Arrange

* chore: fix tests

* chore: re-add unit test for defaultentityrepository for test IAsyncQueryProvider that's actually a List

* refactor: make if statements more conscise

* chore: final stretch

* tests: revert for good tests

* Revert "tests: revert for good tests"

This reverts commit 98b1119.

* Revert "chore: final stretch"

This reverts commit d6f763f.

* fix: client generated id tests

* fix: create data tests with workaround for efcore 3.0 bug

* refactor: DetachRelationships usage of is operator

* fix: move MetaStartup to other assembly as .net core 3 bug workaround

* fix: UpdatingRelationshipTests locally evaluated queries error

* feat: reintroduced generic processor (RepositoryRelationshipUpdateHelper) to fix locally evaluated expressions issue

* feat: full sql support for UpdateRelationshipsAsync using expression trees

* fix: DeletingDataTest

* fix: paging test

* fix: paging unit tests

* chore: remove RE-split models, duplicate models to NoEntityFrameworkExample

* fix: NoEntityFrameworkExample tests fixed by workaround assembly problem

* fix: connection string e2e test projects

* chore: minor cleanup

* chore: minor cleanup

* chore: cleanup

* chore: update travis.yml

* chore: retry travis.yml

* chore: retry .travis.yml

* chore: appveyor.yml

* fix: CI builds

* chore: attempt to fix travis.yml

* chore: spacing

* chore: postgres version bump

* chore: fix readme + downgrade postgres

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

No branches or pull requests

1 participant