Fix ActiveRecord connection caching

Author: ssnickolay

lib/graphql/cache/fetcher.rb

@@ -1,5 +1,8 @@
 # frozen_string_literal: true
 
+require 'graphql/cache/resolvers/base_resolver'
+require 'graphql/cache/resolvers/scalar_resolver'
+require 'graphql/cache/resolvers/connection_resolver'
 require 'graphql/cache/resolver'
 
 module GraphQL

lib/graphql/cache/marshal.rb

@@ -24,23 +24,12 @@ def initialize(key)
         self.key = key.to_s
       end
 
-      # Read a value from cache if it exists and re-hydrate it or
-      # execute the block and write it's result to cache
-      #
-      # @param config [Hash] The object passed to `cache:` on the field definition
+      # Read a value from cache
       # @return [Object]
-      def read(config, force: false, &block)
-        # write new data from resolver if forced
-        return write(config, &block) if force
-
-        cached = cache.read(key)
-
-        if cached.nil?
-          logger.debug "Cache miss: (#{key})"
-          write config, &block
-        else
-          logger.debug "Cache hit: (#{key})"
-          cached
+      def read
+        cache.read(key).tap do |cached|
+          logger.debug "Cache miss: (#{key})" if cached.nil?
+          logger.debug "Cache hit: (#{key})" if cached
         end
       end
 

lib/graphql/cache/resolver.rb

@@ -4,29 +4,25 @@ module GraphQL
   module Cache
     # Represents the caching resolver that wraps the existing resolver proc
     class Resolver
-      attr_accessor :type
-
-      attr_accessor :field
-
-      attr_accessor :orig_resolve_proc
+      attr_accessor :type, :field, :orig_resolve_proc
 
       def initialize(type, field)
         @type  = type
         @field = field
       end
 
       def call(obj, args, ctx)
-        @orig_resolve_proc = field.resolve_proc
-
+        resolve_proc = proc { field.resolve_proc.call(obj, args, ctx) }
         key = cache_key(obj, args, ctx)
-
-        value = Marshal[key].read(
-          field.metadata[:cache], force: ctx[:force_cache]
-        ) do
-          @orig_resolve_proc.call(obj, args, ctx)
+        metadata = field.metadata[:cache]
+
+        if field.connection?
+          Resolvers::ConnectionResolver.new(resolve_proc, key, metadata).call(
+            args: args, field: field, parent: obj, context: ctx, force_cache: ctx[:force_cache]
+          )
+        else
+          Resolvers::ScalarResolver.new(resolve_proc, key, metadata).call(force_cache: ctx[:force_cache])
         end
-
-        wrap_connections(value, args, parent: obj, context: ctx)
       end
 
       protected
@@ -35,32 +31,6 @@ def call(obj, args, ctx)
       def cache_key(obj, args, ctx)
         Key.new(obj, args, type, field, ctx).to_s
       end
-
-      # @private
-      def wrap_connections(value, args, **kwargs)
-        # return raw value if field isn't a connection (no need to wrap)
-        return value unless field.connection?
-
-        # return cached value if it is already a connection object
-        # this occurs when the value is being resolved by GraphQL
-        # and not being read from cache
-        return value if value.class.ancestors.include?(
-          GraphQL::Relay::BaseConnection
-        )
-
-        create_connection(value, args, **kwargs)
-      end
-
-      # @private
-      def create_connection(value, args, **kwargs)
-        GraphQL::Relay::BaseConnection.connection_for_nodes(value).new(
-          value,
-          args,
-          field: field,
-          parent: kwargs[:parent],
-          context: kwargs[:context]
-        )
-      end
     end
   end
 end

lib/graphql/cache/resolvers/base_resolver.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+module GraphQL
+  module Cache
+    module Resolvers
+      class BaseResolver
+        def initialize(resolve_proc, key, metadata)
+          @resolve_proc = resolve_proc
+          @key = key
+          @metadata = metadata
+        end
+
+        def call(*args)
+          raise NotImplementedError
+        end
+
+        private
+
+        attr_reader :resolve_proc, :key, :metadata
+
+        def read
+          Marshal[key].read
+        end
+
+        def write(&block)
+          Marshal[key].write(metadata, &block)
+        end
+      end
+    end
+  end
+end

lib/graphql/cache/resolvers/connection_resolver.rb

@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+module GraphQL
+  module Cache
+    module Resolvers
+      # Pass cache write method into GraphQL::Relay::BaseConnection
+      # and wrap them original Connection methods
+      class ConnectionResolver < BaseResolver
+        class ConnectionCache < Module
+          module WrappedMethods
+            def paged_nodes
+              cache_write = instance_variable_get(:@__cache_write)
+
+              cache_write.call { super }
+            end
+          end
+
+          def initialize(write)
+            @write = write
+          end
+
+          def extended(base)
+            base.extend(WrappedMethods)
+            base.instance_variable_set(:@__cache_write, @write)
+          end
+        end
+
+        def call(args:, field:, parent:, context:, force_cache:)
+          if force_cache || (cached = read).nil?
+            define_connection_cache(resolve_proc.call)
+          else
+            wrap_connection(cached, args, field, parent: parent, context: context)
+          end
+        end
+
+        private
+
+        def wrap_connection(value, args, field, **kwargs)
+          GraphQL::Relay::BaseConnection.connection_for_nodes(value).new(
+            value,
+            args,
+            field: field,
+            parent: kwargs[:parent],
+            context: kwargs[:context]
+          )
+        end
+
+        def define_connection_cache(connection)
+          connection.extend(ConnectionCache.new(method(:write)))
+        end
+      end
+    end
+  end
+end

lib/graphql/cache/resolvers/scalar_resolver.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module GraphQL
+  module Cache
+    module Resolvers
+      class ScalarResolver < BaseResolver
+        def call(force_cache:)
+          return write if force_cache
+
+          cached = read
+
+          cached.nil? ? write { resolve_proc.call } : cached
+        end
+      end
+    end
+  end
+end

spec/features/connections_spec.rb

@@ -40,7 +40,19 @@ def messages
 
   shared_examples "be a correct cold and warm" do
     let(:reference) do
-      {"data" => {"customer" => {"orders" => {"edges" => [{"node" => {"id" =>1 }}, {"node" => {"id" => 2}}, {"node" => {"id" => 3}}]}}}}
+      {
+        "data" => {
+          "customer" => {
+            "orders" => {
+              "edges" => [
+                {"node" => {"id" => 1}},
+                {"node" => {"id" => 2}},
+                {"node" => {"id" => 3}}
+              ]
+            }
+          }
+        }
+     }
     end
 
     it 'produces the same result on miss or hit' do
@@ -91,9 +103,9 @@ def execute(query, context = {})
 
       expect(sql_logger.messages).to eq(
         <<~SQL
-          SELECT "customers".* FROM "customers" ORDER BY "customers"."id" DESC LIMIT ?  [["LIMIT", 1]]
-          SELECT "customers".* FROM "customers" WHERE "customers"."id" = ? LIMIT ?  [["id", 1], ["LIMIT", 1]]
-          SELECT "orders".* FROM "orders" WHERE "orders"."customer_id" = ?  [["customer_id", 1]]
+          SELECT \"customers\".* FROM \"customers\" ORDER BY \"customers\".\"id\" DESC LIMIT ?\e[0m  [[\"LIMIT\", 1]]
+          SELECT \"customers\".* FROM \"customers\" WHERE \"customers\".\"id\" = ? LIMIT ?\e[0m  [[\"id\", 1], [\"LIMIT\", 1]]
+          SELECT \"orders\".* FROM \"orders\" WHERE \"orders\".\"customer_id\" = ?\e[0m  [[\"customer_id\", 1]]
         SQL
       )
     end

spec/graphql/cache/marshal_spec.rb

@@ -32,47 +32,22 @@ module Cache
 
       describe '#read' do
         let(:config) { true }
-        let(:block) { double('block', call: 'foo') }
-
-        context 'when force is set' do
-          it 'should execute the block' do
-            expect(block).to receive(:call)
-            subject.read(config, force: true) { block.call }
-          end
-
-          it 'should write to cache' do
-            expect(cache).to receive(:write).with(key, doc, expires_in: GraphQL::Cache.expiry)
-            subject.write(config) { doc }
-          end
-        end
 
         context 'when cache object exists' do
           before do
             cache.write(key, doc)
           end
 
           it 'should return cached value' do
-            expect(subject.read(config) { block.call }).to eq doc
-          end
-
-          it 'should not execute the block' do
-            expect(block).to_not receive(:call)
-            subject.read(config) { block.call }
+            expect(subject.read).to eq doc
           end
         end
 
         context 'when cache object does not exist' do
-          before do
-            cache.clear
-          end
-
-          it 'should return the evaluated value' do
-            expect(subject.read(config) { block.call }).to eq block.call
-          end
+          before { cache.clear }
 
-          it 'should execute the block' do
-            expect(block).to receive(:call)
-            subject.read(config) { block.call }
+          it 'should return nil' do
+            expect(subject.read).to be_nil
           end
         end
       end