feat(cache): handle full blown cyclical pointers in normalization

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

@@ -65,7 +65,7 @@ class NormalizedInMemoryCache extends InMemoryCache {
   /// *WARNING* if your system allows cyclical references, this will break
   dynamic denormalizedRead(String key) {
     try {
-      return traverse(super.read(key), _denormalizingDereference);
+      return Traversal(_denormalizingDereference).traverse(super.read(key));
     } catch (error) {
       if (error is StackOverflowError) {
         throw NormalizationException(
@@ -90,11 +90,11 @@ class NormalizedInMemoryCache extends InMemoryCache {
     return value is Map<String, Object> ? lazilyDenormalized(value) : value;
   }
 
+  // get a normalizer for a given target map
   Normalizer _normalizerFor(Map<String, Object> into) {
     List<String> normalizer(Object node) {
       final String dataId = dataIdFromObject(node);
       if (dataId != null) {
-        writeInto(dataId, node, into, normalizer);
         return <String>[prefix, dataId];
       }
       return null;
@@ -103,10 +103,10 @@ class NormalizedInMemoryCache extends InMemoryCache {
     return normalizer;
   }
 
+  // [_normalizerFor] for this cache's data
   List<String> _normalize(Object node) {
     final String dataId = dataIdFromObject(node);
     if (dataId != null) {
-      writeInto(dataId, node, data, _normalize);
       return <String>[prefix, dataId];
     }
     return null;
@@ -120,14 +120,15 @@ class NormalizedInMemoryCache extends InMemoryCache {
     Map<String, Object> into, [
     Normalizer normalizer,
   ]) {
+    normalizer ??= _normalizerFor(into);
     if (value is Map<String, Object>) {
-      final Object existing = into[key];
-      final Map<String, Object> merged = (existing is Map<String, Object>)
-          ? deeplyMergeLeft(<Map<String, Object>>[existing, value])
-          : value;
-
+      final Map<String, Object> merged = _mergedWithExisting(into, key, value);
+      final Traversal traversal = Traversal(
+        normalizer,
+        transformSideEffect: _traversingWriteInto(into),
+      );
       // normalized the merged value
-      into[key] = traverseValues(merged, normalizer ?? _normalizerFor(into));
+      into[key] = traversal.traverseValues(merged);
     } else {
       // writing non-map data to the store is allowed,
       // but there is no merging strategy
@@ -151,3 +152,34 @@ String typenameDataIdFromObject(Object object) {
   }
   return null;
 }
+
+/// Writing side effect for traverse
+///
+/// Essentially, we avoid problems with cyclical objects by
+/// tracking seen nodes in the [Traversal],
+/// and we pass this as a side effect to take advantage of that tracking
+SideEffect _traversingWriteInto(Map<String, Object> into) {
+  void sideEffect(Object ref, Object value, Traversal traversal) {
+    final String key = (ref as List<String>)[1];
+    if (value is Map<String, Object>) {
+      final Map<String, Object> merged = _mergedWithExisting(into, key, value);
+      into[key] = traversal.traverseValues(merged);
+    } else {
+      // writing non-map data to the store is allowed,
+      // but there is no merging strategy
+      into[key] = value;
+      return;
+    }
+  }
+
+  return sideEffect;
+}
+
+/// get the given value merged with any pre-existing map with the same key
+Map<String, Object> _mergedWithExisting(
+    Map<String, Object> into, String key, Map<String, Object> value) {
+  final Object existing = into[key];
+  return (existing is Map<String, Object>)
+      ? deeplyMergeLeft(<Map<String, Object>>[existing, value])
+      : value;
+}

packages/graphql/lib/src/utilities/traverse.dart

@@ -1,32 +1,62 @@
+import 'dart:collection';
+
 typedef Transform = Object Function(Object node);
+typedef SideEffect = void Function(
+  Object transformResult,
+  Object node,
+  Traversal traversal,
+);
 
-Map<String, Object> traverseValues(
-  Map<String, Object> node,
-  Transform transform,
-) {
-  return node.map<String, Object>(
-    (String key, Object value) => MapEntry<String, Object>(
-          key,
-          traverse(value, transform),
-        ),
-  );
-}
+class Traversal {
+  Traversal(
+    this.transform, {
+    this.transformSideEffect,
+    this.seenObjects,
+  }) {
+    seenObjects ??= HashSet<int>();
+  }
+
+  Transform transform;
 
-// Attempts to apply the transform to every leaf of the data structure recursively.
-// Stops recursing when a node is transformed (returns non-null)
-Object traverse(Object node, Transform transform) {
-  final Object transformed = transform(node);
-  if (transformed != null) {
-    return transformed;
+  /// An optional side effect to call when a node is transformed.
+  SideEffect transformSideEffect;
+  HashSet<int> seenObjects;
+
+  bool hasAlreadySeen(Object node) {
+    final bool wasAdded = seenObjects.add(node.hashCode);
+    return !wasAdded;
   }
 
-  if (node is List<Object>) {
-    return node
-        .map<Object>((Object node) => traverse(node, transform))
-        .toList();
+  /// Traverse only the values of the given map
+  Map<String, Object> traverseValues(Map<String, Object> node) {
+    return node.map<String, Object>(
+      (String key, Object value) => MapEntry<String, Object>(
+            key,
+            traverse(value),
+          ),
+    );
   }
-  if (node is Map<String, Object>) {
-    return traverseValues(node, transform);
+
+  // Attempts to apply the transform to every leaf of the data structure recursively.
+  // Stops recursing when a node is transformed (returns non-null)
+  Object traverse(Object node) {
+    final Object transformed = transform(node);
+    if (hasAlreadySeen(node)) {
+      return transformed ?? node;
+    }
+    if (transformed != null) {
+      if (transformSideEffect != null) {
+        transformSideEffect(transformed, node, this);
+      }
+      return transformed;
+    }
+
+    if (node is List<Object>) {
+      return node.map<Object>((Object node) => traverse(node)).toList();
+    }
+    if (node is Map<String, Object>) {
+      return traverseValues(node);
+    }
+    return node;
   }
-  return node;
 }

packages/graphql/test/normalized_in_memory_test.dart

@@ -133,10 +133,12 @@ final Map<String, Object> cyclicalOperationData = <String, Object>{
     'b': <String, Object>{
       '__typename': 'B',
       'id': 5,
-      'a': <String, Object>{
-        '__typename': 'A',
-        'id': 1,
-      },
+      'as': [
+        <String, Object>{
+          '__typename': 'A',
+          'id': 1,
+        },
+      ]
     },
   },
 };
@@ -150,7 +152,39 @@ final Map<String, Object> cyclicalNormalizedA = <String, Object>{
 final Map<String, Object> cyclicalNormalizedB = <String, Object>{
   '__typename': 'B',
   'id': 5,
-  'a': <String>['@cache/reference', 'A/1'],
+  'as': [
+    <String>['@cache/reference', 'A/1']
+  ],
+};
+
+Map<String, Object> get cyclicalObjOperationData {
+  Map<String, Object> a;
+  Map<String, Object> b;
+  a = {
+    '__typename': 'A',
+    'id': 1,
+  };
+  b = <String, Object>{
+    '__typename': 'B',
+    'id': 5,
+    'as': [a]
+  };
+  a['b'] = b;
+  return {'a': a};
+}
+
+final Map<String, Object> cyclicalObjNormalizedA = <String, Object>{
+  '__typename': 'A',
+  'id': 1,
+  'b': <String>['@cache/reference', 'B/5'],
+};
+
+final Map<String, Object> cyclicalObjNormalizedB = <String, Object>{
+  '__typename': 'B',
+  'id': 5,
+  'as': [
+    <String>['@cache/reference', 'A/1']
+  ],
 };
 
 NormalizedInMemoryCache getTestCache() => NormalizedInMemoryCache(
@@ -187,4 +221,15 @@ void main() {
       expect(b.data, equals(cyclicalNormalizedB));
     });
   });
+
+  group('Handles Object/pointer self-references/cycles', () {
+    final NormalizedInMemoryCache cache = getTestCache();
+    test('lazily reads cyclical references', () {
+      cache.write(rawOperationKey, cyclicalObjOperationData);
+      final LazyCacheMap a = cache.read('A/1') as LazyCacheMap;
+      expect(a.data, equals(cyclicalObjNormalizedA));
+      final LazyCacheMap b = a['b'] as LazyCacheMap;
+      expect(b.data, equals(cyclicalObjNormalizedB));
+    });
+  });
 }