Skip to content
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

More robust partial data handling #104

Merged
merged 14 commits into from
Nov 3, 2020
Merged

Conversation

micimize
Copy link
Contributor

  • Adds a path to PartialDataException to surface invalid paths
  • Adds acceptPartialData=true to normalizeFragment and normalizeOperation
    to allow for the rejection of structurally incomplete results (i.e. {} != '{ foo: null }')
  • denormalizeFragment and denormalizeOperation now have a handleException=true flag which can be disabled to throw rather than return null
  • Adds a validateOperationDataStructure closing [normalize feature req] validation and safety utils #102 (handleException=false by default)
  • moved returnPartialData to config.allowPartialData in denormalizeNode as partial data is now a cross cutting concern
  • add tests and a few docstrings elsewhere

@micimize
Copy link
Contributor Author

Sorry this does more than #102 specified – I realized there's an issue with write/read round-trips always resulting in valid data, because normalizeOperation will fill in nulls and then denormalizeOperation will accept them.

@micimize
Copy link
Contributor Author

hmm... validateOperationDataStructure won't work for purely virtualized fields because it ignores typePolicies, but we don't have the ability to do that yet anyways. And once we do we'll be stripping out purely client-side virtualized fields from the query so the validation will stand.

@smkhalsa
Copy link
Member

hmm... validateOperationDataStructure won't work for purely virtualized fields because it ignores typePolicies, but we don't have the ability to do that yet anyways. And once we do we'll be stripping out purely client-side virtualized fields from the query so the validation will stand.

Is there a reason why validateOperationDataStructure shouldn't just call denormalizeOperation internally rather than re-implementing it? It seems like it'd resolve this issue and keep things more DRY.

@micimize
Copy link
Contributor Author

@smkhalsa right you are – did that before implementing the handleException flag.

Went ahead and refactored, added validateFragmentDataStructure and null rejection, along with tests and some docs on my rationale there.

@micimize
Copy link
Contributor Author

micimize commented Nov 3, 2020

@smkhalsa could you take a look at this when you get the chance? I kind of ended up making zino-hofmann/graphql-flutter#754 fairly large, and it depends on this

Copy link
Member

@smkhalsa smkhalsa left a comment

Choose a reason for hiding this comment

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

LGTM

@smkhalsa smkhalsa merged commit ca7d6ec into gql-dart:master Nov 3, 2020
micimize added a commit to micimize/ferry that referenced this pull request Nov 6, 2020
smkhalsa pushed a commit that referenced this pull request Nov 6, 2020
smkhalsa added a commit that referenced this pull request Nov 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants