Merge pull request #754 from micimize/norm_data_handling

Structural data validation on cache writes and more

examples/starwars/lib/episode/hero_query.dart

@@ -39,10 +39,10 @@ class HeroForEpisode extends StatelessWidget {
         // NOTE: a loading message is always sent, but if you're developing locally,
         // the network result might be returned so fast that
         // flutter rebuilds again too quickly for you don't see the loading result on the stream
-        print([
-          result.source,
-          if (result.data != null) result.data['hero']['name']
-        ]);
+        // print([
+        //   result.source,
+        //   if (result.data != null) result.data['hero']['name']
+        // ]);
         if (result.hasException) {
           return Text(result.exception.toString());
         }

examples/starwars/lib/generated_plugin_registrant.dart

@@ -0,0 +1,16 @@
+//
+// Generated file. Do not edit.
+//
+
+// ignore: unused_import
+import 'dart:ui';
+
+import 'package:connectivity_for_web/connectivity_for_web.dart';
+
+import 'package:flutter_web_plugins/flutter_web_plugins.dart';
+
+// ignore: public_member_api_docs
+void registerPlugins(PluginRegistry registry) {
+  ConnectivityPlugin.registerWith(registry.registrarFor(ConnectivityPlugin));
+  registry.registerMessageHandler();
+}

examples/starwars/lib/reviews/review_page_list.dart

@@ -12,6 +12,8 @@ class PagingReviews extends StatelessWidget {
   Widget build(BuildContext context) {
     return Query(
       options: QueryOptions(
+        // if we have cached results, don't clobber them
+        fetchPolicy: FetchPolicy.cacheFirst,
         document: gql(r'''
           query Reviews($page: Int!) {
             reviews(page: $page) {
@@ -59,7 +61,7 @@ class PagingReviews extends StatelessWidget {
                 : RaisedButton(
                     onPressed: () {
                       fetchMore(
-                        FetchMoreOptions(
+                        FetchMoreOptions.partial(
                           variables: {'page': nextPage},
                           updateQuery: (existing, newReviews) => ({
                             'reviews': {

packages/graphql/README.md

@@ -30,6 +30,9 @@ As of `v4`, it is built on foundational libraries from the [gql-dart project], i
   - [Direct Cache Access API](#direct-cache-access-api)
     - [`Request`, `readQuery`, and `writeQuery`](#request-readquery-and-writequery)
     - [`FragmentRequest`, `readFragment`, and `writeFragment`](#fragmentrequest-readfragment-and-writefragment)
+  - [Other Cache Considerations](#other-cache-considerations)
+    - [Write strictness and `partialDataPolicy`](#write-strictness-and-partialdatapolicy)
+    - [Possible cache write exceptions](#possible-cache-write-exceptions)
   - [Policies](#policies)
   - [Exceptions](#exceptions)
   - [Links](#links)
@@ -440,6 +443,26 @@ client.writeFragment(fragmentRequest, data);
 
 > **NB** You likely want to call the cache access API from your `client` for automatic broadcasting support.
 
+## Other Cache Considerations
+
+### Write strictness and `partialDataPolicy`
+
+As of [#754](https://github.com/zino-app/graphql-flutter/pull/754) we can now enforce strict structural constraints on data written to the cache. This means that if the client receives structurally invalid data from the network or on `client.writeQuery`, it will throw an exception.
+
+By default, optimistic data is excluded from these constraints for ease of use via `PartialDataCachePolicy.acceptForOptimisticData`, as it is easy to miss `__typename`, etc.
+This behavior is configurable via `GraphQLCache.partialDataPolicy`, which can be set to `accept` for no constraints or `reject` for full constraints.
+
+### Possible cache write exceptions
+
+At link execution time, one of the following exceptions can be thrown:
+
+* `CacheMisconfigurationException` if the structure seems like it should write properly, and is perhaps failing due to a `typePolicy`
+* `UnexpectedResponseStructureException` if the server response looks malformed.
+* `MismatchedDataStructureException` in the event of a malformed optimistic result (and `PartialDataCachePolicy.reject`).
+* `CacheMissException` if write succeeds but `readQuery` then returns `null` (though **data will not be overwritten**)
+
+</details>
+
 ## Policies
 
 Policies are used to configure execution and error behavior for a given request.

packages/graphql/lib/src/cache/_normalizing_data_proxy.dart

@@ -1,4 +1,5 @@
 import 'package:graphql/src/cache/fragment.dart';
+import 'package:graphql/src/exceptions/exceptions_next.dart';
 import "package:meta/meta.dart";
 
 import 'package:gql_exec/gql_exec.dart' show Request;
@@ -29,10 +30,24 @@ abstract class NormalizingDataProxy extends GraphQLDataProxy {
   /// [returnPartialData] is `true`
   bool addTypename = false;
 
-  /// Used for testing
+  /// Used for testing.
+  ///
+  /// Passed through to normalize. When [denormalizeOperation] isn't passed [returnPartialData],
+  /// It will simply return `null` if any part of the query can't be constructed.
+  ///
+  /// **NOTE**: This is not exposed as a configuration for a reason.
+  /// If enabled, it would be easy to eagerly return an unexpected partial result from the cache,
+  /// resulting in mangled and hard-to-reason-about app state.
   @protected
   bool get returnPartialData => false;
 
+  /// Whether it is permissible to write partial data to the this proxy.
+  /// Determined by [PartialDataCachePolicy]
+  ///
+  /// Passed through to normalize. When [normalizeOperation] isn't passed [acceptPartialData],
+  /// It will set missing fields to `null` if any part of a structurally valid query result is missing.
+  bool get acceptPartialData;
+
   /// Flag used to request a (re)broadcast from the [QueryManager].
   ///
   /// This is set on every [writeQuery] and [writeFragment] by default.
@@ -68,8 +83,11 @@ abstract class NormalizingDataProxy extends GraphQLDataProxy {
         // provided from cache
         read: (dataId) => readNormalized(dataId, optimistic: optimistic),
         typePolicies: typePolicies,
+        //dataIdFromObject: dataIdFromObject,
         returnPartialData: returnPartialData,
         addTypename: addTypename ?? false,
+        // if there is partial data, we cannot read and return null
+        handleException: true,
         // provided from request
         document: request.operation.document,
         operationName: request.operation.operationName,
@@ -87,6 +105,8 @@ abstract class NormalizingDataProxy extends GraphQLDataProxy {
         dataIdFromObject: dataIdFromObject,
         returnPartialData: returnPartialData,
         addTypename: addTypename ?? false,
+        // if there is partial data, we cannot read and return null
+        handleException: true,
         // provided from request
         document: fragmentRequest.fragment.document,
         idFields: fragmentRequest.idFields,
@@ -99,21 +119,34 @@ abstract class NormalizingDataProxy extends GraphQLDataProxy {
     Map<String, dynamic> data,
     bool broadcast = true,
   }) {
-    normalizeOperation(
-      // provided from cache
-      write: (dataId, value) => writeNormalized(dataId, value),
-      read: (dataId) => readNormalized(dataId),
-      typePolicies: typePolicies,
-      dataIdFromObject: dataIdFromObject,
-      // provided from request
-      document: request.operation.document,
-      operationName: request.operation.operationName,
-      variables: sanitizeVariables(request.variables),
-      // data
-      data: data,
-    );
-    if (broadcast ?? true) {
-      broadcastRequested = true;
+    try {
+      normalizeOperation(
+        // provided from cache
+        write: (dataId, value) => writeNormalized(dataId, value),
+        read: (dataId) => readNormalized(dataId),
+        typePolicies: typePolicies,
+        dataIdFromObject: dataIdFromObject,
+        acceptPartialData: acceptPartialData,
+        addTypename: addTypename ?? false,
+        // provided from request
+        document: request.operation.document,
+        operationName: request.operation.operationName,
+        variables: sanitizeVariables(request.variables),
+        // data
+        data: data,
+      );
+      if (broadcast ?? true) {
+        broadcastRequested = true;
+      }
+    } on PartialDataException catch (e) {
+      if (request.validatesStructureOf(data)) {
+        throw CacheMisconfigurationException(
+          e,
+          request: request,
+          data: data,
+        );
+      }
+      rethrow;
     }
   }
 
@@ -122,22 +155,35 @@ abstract class NormalizingDataProxy extends GraphQLDataProxy {
     @required Map<String, dynamic> data,
     bool broadcast = true,
   }) {
-    normalizeFragment(
-      // provided from cache
-      write: (dataId, value) => writeNormalized(dataId, value),
-      read: (dataId) => readNormalized(dataId),
-      typePolicies: typePolicies,
-      dataIdFromObject: dataIdFromObject,
-      // provided from request
-      document: request.fragment.document,
-      idFields: request.idFields,
-      fragmentName: request.fragment.fragmentName,
-      variables: sanitizeVariables(request.variables),
-      // data
-      data: data,
-    );
-    if (broadcast ?? true) {
-      broadcastRequested = true;
+    try {
+      normalizeFragment(
+        // provided from cache
+        write: (dataId, value) => writeNormalized(dataId, value),
+        read: (dataId) => readNormalized(dataId),
+        typePolicies: typePolicies,
+        dataIdFromObject: dataIdFromObject,
+        acceptPartialData: acceptPartialData,
+        addTypename: addTypename ?? false,
+        // provided from request
+        document: request.fragment.document,
+        idFields: request.idFields,
+        fragmentName: request.fragment.fragmentName,
+        variables: sanitizeVariables(request.variables),
+        // data
+        data: data,
+      );
+      if (broadcast ?? true) {
+        broadcastRequested = true;
+      }
+    } on PartialDataException catch (e) {
+      if (request.validatesStructureOf(data)) {
+        throw CacheMisconfigurationException(
+          e,
+          fragmentRequest: request,
+          data: data,
+        );
+      }
+      rethrow;
     }
   }
 }

packages/graphql/lib/src/cache/_optimistic_transactions.dart

@@ -7,7 +7,8 @@ import 'package:meta/meta.dart';
 import 'package:graphql/src/cache/_normalizing_data_proxy.dart';
 import 'package:graphql/src/cache/data_proxy.dart';
 
-import 'package:graphql/src/cache/cache.dart' show GraphQLCache;
+import 'package:graphql/src/cache/cache.dart'
+    show GraphQLCache, PartialDataCachePolicy;
 
 /// API for users to provide cache updates through
 typedef CacheTransaction = GraphQLDataProxy Function(GraphQLDataProxy proxy);
@@ -32,6 +33,14 @@ class OptimisticProxy extends NormalizingDataProxy {
 
   GraphQLCache cache;
 
+  /// Whether it is permissible to write partial data, as determined by [partialDataPolicy].
+  ///
+  /// Passed through to normalize. When [normalizeOperation] isn't passed [acceptPartialData],
+  /// It will set missing fields to `null` if any part of a structurally valid query result is missing.
+  @override
+  bool get acceptPartialData =>
+      cache.partialDataPolicy != PartialDataCachePolicy.reject;
+
   /// `typePolicies` to pass down to `normalize` (proxied from [cache])
   get typePolicies => cache.typePolicies;
 

packages/graphql/lib/src/cache/cache.dart

@@ -31,11 +31,7 @@ class GraphQLCache extends NormalizingDataProxy {
     Store store,
     this.dataIdFromObject,
     this.typePolicies = const {},
-
-    /// Input variable sanitizer for referencing custom scalar types in cache keys.
-    ///
-    /// Defaults to [sanitizeFilesForCache]. Can be set to `null` to disable sanitization.
-    /// If present, a sanitizer will be built with [variableSanitizer]
+    this.partialDataPolicy = PartialDataCachePolicy.acceptForOptimisticData,
     Object Function(Object) sanitizeVariables = sanitizeFilesForCache,
   })  : sanitizeVariables = variableSanitizer(sanitizeVariables),
         store = store ?? InMemoryStore();
@@ -46,12 +42,30 @@ class GraphQLCache extends NormalizingDataProxy {
   /// rebroadcast operations.
   final Store store;
 
+  /// Determines how partial data should be handled when written to the cache
+  ///
+  /// [acceptForOptimisticData] is the default and recommended, as it provides
+  /// the best balance between safety and usability.
+  final PartialDataCachePolicy partialDataPolicy;
+
+  /// Whether it is permissible to write partial data, as determined by [partialDataPolicy].
+  ///
+  /// Passed through to normalize. When [normalizeOperation] isn't passed [acceptPartialData],
+  /// It will simply return `null` if any part of the query can't be constructed.
+  @override
+  bool get acceptPartialData =>
+      partialDataPolicy == PartialDataCachePolicy.accept;
+
   /// `typePolicies` to pass down to [normalize]
   final Map<String, TypePolicy> typePolicies;
 
   /// Optional `dataIdFromObject` function to pass through to [normalize]
   final DataIdResolver dataIdFromObject;
 
+  /// Input variable sanitizer for referencing custom scalar types in cache keys.
+  ///
+  /// Defaults to [sanitizeFilesForCache]. Can be set to `null` to disable sanitization.
+  /// If present, a sanitizer will be built with [variableSanitizer]
   @override
   final SanitizeVariables sanitizeVariables;
 
@@ -116,7 +130,7 @@ class GraphQLCache extends NormalizingDataProxy {
   /// Write normalized data into the cache,
   /// deeply merging maps with existing values
   ///
-  /// Called from [writeQuery] and [writeFragment].
+  /// Called from [witeQuery] and [writeFragment].
   void writeNormalized(String dataId, dynamic value) {
     if (value is Map<String, Object>) {
       final existing = store.get(dataId);
@@ -190,3 +204,19 @@ class GraphQLCache extends NormalizingDataProxy {
     broadcastRequested = true;
   }
 }
+
+/// Determines how partial data should be handled when written to the cache
+///
+/// [acceptForOptimisticData] is the default and recommended, as it provides
+/// the best balance between safety and usability.
+enum PartialDataCachePolicy {
+  /// Accept partial data in all cases (default).
+  accept,
+
+  /// Accept partial data, but only for transient optimistic writes made with
+  /// [GraphQLCache.recordOptimisticTransaction]
+  acceptForOptimisticData,
+
+  /// Reject partial data in all cases.
+  reject,
+}

packages/graphql/lib/src/cache/data_proxy.dart

@@ -1,4 +1,6 @@
 import 'package:gql_exec/gql_exec.dart' show Request;
+import 'package:graphql/src/cache/_normalizing_data_proxy.dart';
+import 'package:graphql/src/exceptions/exceptions_next.dart';
 
 import './fragment.dart';
 
@@ -118,6 +120,10 @@ abstract class GraphQLDataProxy {
   ///
   /// For complex `normalize` type policies that involve custom reads,
   /// `optimistic` will be the default.
+  ///
+  /// Will throw a [PartialDataException] if the [data] structure
+  /// doesn't match that of the [request] `operation.document`,
+  /// or a [CacheMisconfigurationException] if the write fails for some other reason.
   void writeQuery(
     Request request, {
     Map<String, dynamic> data,
@@ -133,6 +139,10 @@ abstract class GraphQLDataProxy {
   ///
   /// For complex `normalize` type policies that involve custom reads,
   /// `optimistic` will be the default.
+  ///
+  /// Will throw a [PartialDataException] if the [data] structure
+  /// doesn't match that of the [fragmentRequest] `fragment.document`,
+  /// or a [CacheMisconfigurationException] if the write fails for some other reason.
   void writeFragment(
     FragmentRequest fragmentRequest, {
     Map<String, dynamic> data,