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

Structural data validation on cache writes and more #754

Merged
merged 18 commits into from
Nov 6, 2020

Conversation

micimize
Copy link
Collaborator

@micimize micimize commented Oct 24, 2020

Closes #747, #744, #718, #683, #687, #616, #405

depends on gql-dart/ferry#104

result.data carry forward on exception (#718)

for watchQuery, when an exception results in null data, the previous result will be used by default. Can be disabled by WatchQueryOptions(carryForwardDataOnException: false)

only rebroadcast when !deep equals by default (#616)

alwaysRebroadcast can be used to get the old behavior back.

strict cache writes (semi-breaking)

cache writes are now strict and throw PartialDataException (from normalize) or a CacheMisconfigurationException if the structures seem like they should be valid (configurable by partialDataPolicy)

At link execution time, one of the following exceptions can be thrown:

  • CacheMisconfigurationException if the structure seems like it should
    write properly.
  • UnexpectedResponseStructureException if the server response looks malformed.
  • MismatchedDataStructureException in the event of a malformed
    optimistic result.
  • If write succeeds but readQuery then returns null, a CacheMissException will be added and the data will not be overwritten

before, one could attempt to write any data structure to the cache, and it would write whatever document-matching data there was.
Now, if one attempts to write structurally partial data, appropriate
exceptions will be thrown.

fechMore updates

  • Corresponding to the above, FetchMoreOptions.partial is added for safer update operations
  • unrelatedly, UpdateQuery is now strictly typed with Maps

Skip cache writes when errors && ErrorPolicy.none (#405)

  /// If we have no data, we skip caching, thus taking [ErrorPolicy.none]
  /// into account

I also verified examples/starwars works on web and bumped the connectivity plugin to silence an error.

@micimize
Copy link
Collaborator Author

This depends on gql-dart/ferry#104 getting merged, but can be tried out like so:

dependency_overrides:
  graphql:
    git: 
      url: git@github.com:micimize/graphql-flutter.git
      ref: norm_data_handling
      path: packages/graphql

@codecov
Copy link

codecov bot commented Oct 24, 2020

Codecov Report

Merging #754 (1a73495) into beta (be1951e) will decrease coverage by 1.18%.
The diff coverage is 43.39%.

Impacted file tree graph

@@            Coverage Diff             @@
##             beta     #754      +/-   ##
==========================================
- Coverage   50.48%   49.29%   -1.19%     
==========================================
  Files          34       35       +1     
  Lines        1240     1349     +109     
==========================================
+ Hits          626      665      +39     
- Misses        614      684      +70     
Flag Coverage Δ
graphql_client 52.54% <43.67%> (-1.78%) ⬇️
graphql_flutter 31.57% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/graphql/lib/src/core/fetch_more.dart 0.00% <0.00%> (ø)
packages/graphql/lib/src/core/policies.dart 49.18% <ø> (ø)
packages/graphql/lib/src/core/query_options.dart 26.41% <0.00%> (-4.02%) ⬇️
packages/graphql/lib/src/core/query_result.dart 47.22% <0.00%> (-7.96%) ⬇️
packages/graphql/lib/src/graphql_client.dart 82.85% <ø> (ø)
...ages/graphql_flutter/lib/src/widgets/mutation.dart 0.00% <0.00%> (ø)
...ackages/graphql_flutter/lib/src/widgets/query.dart 97.14% <ø> (ø)
...es/graphql/lib/src/exceptions/exceptions_next.dart 16.66% <4.87%> (-20.18%) ⬇️
...ackages/graphql/lib/src/core/observable_query.dart 44.80% <25.00%> (-0.81%) ⬇️
packages/graphql/lib/src/cache/fragment.dart 19.14% <50.00%> (+8.33%) ⬆️
... and 6 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 be1951e...1a73495. Read the comment docs.

@micimize
Copy link
Collaborator Author

Note to self: Update Exceptions section with strictness errors https://github.com/zino-app/graphql-flutter/tree/beta/packages/graphql#exceptions

@micimize micimize changed the title Structural data validation on cache writes Structural data validation on cache writes and more Oct 26, 2020
@micimize micimize added the v4 Issue with a v4 library label Nov 3, 2020
…tion (from normalize),

or a CacheMisconfigurationException if the structures seem like they
should be valid.

At link execution time, one of the following exceptions can be thrown:
* CacheMisconfigurationException if the structure seems like it should
  write properly.
* UnexpectedResponseStructureException if the server response looks malformed.
* MismatchedDataStructureException in the event of a malformed
  optimistic result.

before, one could attempt to write any data structure to the cache, and it would write whatever document-matching data there was.
Now, if one attempts to write structurally partial data, appropriate
exceptions will be thrown.
strict data structure exception handling for fetchMore, FetchMoreOptions.partial for dealing with __typename/structural inconsistencies, better docs.
Also tightened the fetchMore types, as operation data can only be Maps.
@HofmannZ
Copy link
Member

HofmannZ commented Nov 7, 2020

🎉 This PR is included in version 4.0.0-beta.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@HofmannZ
Copy link
Member

🎉 This PR is included in version 4.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@EightRice
Copy link

I got here cause Uncaught (in promise) Error: Expected a value of type 'String', but got one of type 'NativeJavaScriptObject'
and this was supposed to be the solution... I have no idea what all this text is but if anyone can kindly tell me what to do about my error, I'd be really grateful.

@micimize
Copy link
Collaborator Author

micimize commented Apr 5, 2021

@EightRice open a new issue – that sounds like a web-related issue and at first blush doesn't look necessarily related to strict cache writes (maybe that's why the error was surfaced, but not the root cause)

@micimize micimize deleted the norm_data_handling branch April 5, 2021 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released on @beta released v4 Issue with a v4 library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[v4] Both result data and exception are sometimes null on network result
3 participants