Fix Connection Cache for ActiveRecord

Gemfile.lock

@@ -2,25 +2,39 @@ PATH
   remote: .
   specs:
     graphql-cache (0.6.0)
-      graphql (~> 1, > 1.8)
+      graphql (~> 1, > 1.9.3)
 
 GEM
   remote: https://rubygems.org/
   specs:
+    activemodel (6.0.2.1)
+      activesupport (= 6.0.2.1)
+    activerecord (6.0.2.1)
+      activemodel (= 6.0.2.1)
+      activesupport (= 6.0.2.1)
+    activesupport (6.0.2.1)
+      concurrent-ruby (~> 1.0, >= 1.0.2)
+      i18n (>= 0.7, < 2)
+      minitest (~> 5.1)
+      tzinfo (~> 1.1)
+      zeitwerk (~> 2.2)
     appraisal (2.2.0)
       bundler
       rake
       thor (>= 0.14.0)
     codeclimate-test-reporter (1.0.9)
       simplecov (<= 0.13)
     coderay (1.1.2)
+    concurrent-ruby (1.1.5)
     diff-lcs (1.3)
     docile (1.1.5)
-    graphql (1.9.3)
+    graphql (1.9.17)
+    i18n (1.7.0)
+      concurrent-ruby (~> 1.0)
     json (2.2.0)
     method_source (0.9.2)
     mini_cache (1.1.0)
-    promise.rb (0.7.4)
+    minitest (5.13.0)
     pry (0.12.2)
       coderay (~> 1.1.0)
       method_source (~> 0.9.0)
@@ -46,16 +60,20 @@ GEM
     simplecov-html (0.10.2)
     sqlite3 (1.4.0)
     thor (0.20.3)
+    thread_safe (0.3.6)
+    tzinfo (1.2.5)
+      thread_safe (~> 0.1)
+    zeitwerk (2.2.2)
 
 PLATFORMS
   ruby
 
 DEPENDENCIES
+  activerecord
   appraisal
   codeclimate-test-reporter
   graphql-cache!
   mini_cache
-  promise.rb
   pry
   rake (~> 10.0)
   rspec (~> 3.0)

bin/console

@@ -20,7 +20,7 @@ end
 
 # required after GraphQL::Cache initialization because dev
 # schema uses cache and logger objects from it.
-require_relative '../test_schema'
+require_relative '../test_schema/sequel/init'
 
 require "pry"
 Pry.start

graphql-cache.gemspec

@@ -28,8 +28,9 @@ Gem::Specification.new do |s|
   s.add_development_dependency 'rake',    '~> 10.0'
   s.add_development_dependency 'rspec',   '~> 3.0'
   s.add_development_dependency 'sequel'
+  s.add_development_dependency 'activerecord'
   s.add_development_dependency 'simplecov'
   s.add_development_dependency 'sqlite3'
 
-  s.add_dependency 'graphql', '~> 1', '> 1.8'
+  s.add_dependency 'graphql', '~> 1', '> 1.9.3'
 end

lib/graphql/cache.rb

@@ -5,6 +5,8 @@
 require 'graphql/cache/key'
 require 'graphql/cache/marshal'
 require 'graphql/cache/fetcher'
+require 'graphql/cache/field_extension'
+require 'graphql/cache/patch/connection_extension'
 
 module GraphQL
   module Cache
@@ -45,8 +47,11 @@ def configure
     # bootstrap necessary instrumentation and tracing
     # tie-ins
     def self.use(schema_def, options: {})
-      fetcher = ::GraphQL::Cache::Fetcher.new
-      schema_def.instrument(:field, fetcher)
+      # please, use GraphQL::Cache::FieldExtension if use Interpreter mode
+      if Gem::Version.new(GraphQL::VERSION) >= Gem::Version.new('1.9.0.pre3')
+        fetcher = ::GraphQL::Cache::Fetcher.new
+        schema_def.instrument(:field, fetcher)
+      end
     end
   end
 end

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/field_extension.rb

@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+module GraphQL
+  module Cache
+    class FieldExtension < GraphQL::Schema::FieldExtension
+      def apply
+        field.instance_variable_set(:@__cache_config, options.present? ? options : true)
+      end
+
+      def resolve(object:, arguments:, **rest)
+        if field.connection?
+          yield(object, arguments, object: object, arguments: arguments)
+        else
+          GraphQL::Cache::Resolver.new(field.owner, field)
+                                  .call(object, arguments, rest[:context], proc { yield(object, arguments) })
+        end
+      end
+
+      def after_resolve(value:, memo:, **rest)
+        return value unless field.connection?
+
+        arguments = memo[:arguments]
+        object = memo[:object]
+
+        GraphQL::Cache::Resolver.new(field.owner, field)
+                                .call(object, arguments, rest[:context], proc { value })
+      end
+    end
+  end
+end

lib/graphql/cache/key.rb

@@ -33,7 +33,7 @@ def initialize(obj, args, type, field, context = {})
         @type      = type
         @field     = field
         @context   = context
-        @metadata  = field.metadata[:cache]
+        @metadata  = field.instance_variable_get(:@cache_config)
 
         @metadata = { cache: @metadata } unless @metadata.is_a?(Hash)
       end

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
 
@@ -55,6 +44,7 @@ def write(config)
 
         with_resolved_document(document) do |resolved_document|
           cache.write(key, resolved_document, expires_in: expiry(config))
+          logger.debug "Cache was added: (#{key} with config #{config})"
 
           resolved
         end

lib/graphql/cache/patch/connection_extension.rb

@@ -0,0 +1,19 @@
+module GraphQL
+  module Cache
+    module Patch
+      module ConnectionExtension
+        def after_resolve(value:, object:, arguments:, context:, memo:)
+          # in Cached Extension we wrap the original value to the Connection
+          # so we do not have to do it againt
+          return value if value.is_a?(GraphQL::Relay::BaseConnection)
+
+          super
+        end
+      end
+    end
+  end
+end
+
+GraphQL::Schema::Field::ConnectionExtension.prepend(
+  GraphQL::Cache::Patch::ConnectionExtension
+)

lib/graphql/cache/resolver.rb

@@ -4,29 +4,26 @@ 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
-
+      def call(obj, args, ctx, block)
+        resolve_proc = block #proc { block.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)
-        end
+        cache_config = field.instance_variable_get(:@__cache_config)
 
-        wrap_connections(value, args, parent: obj, context: ctx)
+        if field.connection?
+          Resolvers::ConnectionResolver.new(resolve_proc, key, cache_config).call(
+            args: args, field: field, parent: obj, context: ctx, force_cache: ctx[:force_cache]
+          )
+        else
+          Resolvers::ScalarResolver.new(resolve_proc, key, cache_config).call(force_cache: ctx[:force_cache])
+        end
       end
 
       protected
@@ -35,32 +32,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, cache_config)
+          @resolve_proc = resolve_proc
+          @key = key
+          @cache_config = cache_config
+        end
+
+        def call(*args)
+          raise NotImplementedError
+        end
+
+        private
+
+        attr_reader :resolve_proc, :key, :cache_config
+
+        def read
+          Marshal[key].read
+        end
+
+        def write(&block)
+          Marshal[key].write(cache_config, &block)
+        end
+      end
+    end
+  end
+end

lib/graphql/cache/resolvers/connection_resolver.rb

@@ -0,0 +1,80 @@
+# frozen_string_literal: true
+
+module GraphQL
+  module Cache
+    module Resolvers
+      class ConnectionResolver < BaseResolver
+        NodesCache = Struct.new(:nodes, :paged_nodes)
+
+        # Pass cache write method into GraphQL::Relay::RelationConnection
+        class RelationConnectionOverload < Module
+          module WrappedMethods
+            def paged_nodes
+              cache_write = instance_variable_get(:@__cache_write)
+
+              super.tap do |result|
+                # save original relation (aka @nodes) and loaded records
+                cache_write.call { NodesCache.new(@nodes, result) }
+              end
+            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 || (cache = read).nil?
+            define_relation_cache(resolve_proc.call, args, field, parent: parent, context: context)
+          else
+            use(cache, args, field, parent: parent, context: context)
+          end
+        end
+
+        private
+
+        def define_relation_cache(nodes, args, field, **kwargs)
+          if nodes.is_a?(GraphQL::Relay::RelationConnection)
+            # inject cached logic into the relation connection
+            # works with non Interpreter mode
+            nodes
+          else
+            # nodes are Array or ActiveRecord relation
+            wrap_to_connection(nodes, args, field, kwargs)
+          end.extend(RelationConnectionOverload.new(method(:write)))
+        end
+
+        def use(cache, args, field, **kwargs)
+          nodes, paged_nodes = parse(cache)
+
+          wrap_to_connection(nodes, args, field, kwargs).tap do |conn|
+            # restore cached paged_nodes (works for AR relations)
+            conn.instance_variable_set(:@paged_nodes, paged_nodes) if paged_nodes
+          end
+        end
+
+        def wrap_to_connection(nodes, args, field, **kwargs)
+          GraphQL::Relay::BaseConnection.connection_for_nodes(nodes).new(
+            nodes,
+            args,
+            field: field,
+            parent: kwargs[:parent],
+            context: kwargs[:context]
+          )
+        end
+
+        def parse(cache)
+          return [cache, nil] unless cache.is_a?(NodesCache)
+
+          [cache.nodes, cache.paged_nodes]
+        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

@@ -1,14 +1,21 @@
 require 'spec_helper'
 
-def execute(query, context = {})
-  CacheSchema.execute(query, context: context)
-end
-
 RSpec.describe 'caching connection fields' do
+  class StubLogger < Logger
+    def initialize
+      @strio = StringIO.new
+      super(@strio)
+    end
+
+    def messages
+      @strio.string
+    end
+  end
+
   let(:query) do
     %Q{
       {
-        customer(id: #{Customer.last.id}) {
+        customer(id: #{customer.id}) {
           orders {
             edges {
               node {
@@ -21,10 +28,91 @@ def execute(query, context = {})
     }
   end
 
-  it 'produces the same result on miss or hit' do
-    cold_results = execute(query)
-    warm_results = execute(query)
+  let(:sql_logger) do
+    StubLogger.new.tap do |logger|
+      logger.formatter = proc do |_severity, _datetime, _progname, msg|
+        raw_sql = msg.match(/.*(?<sql>SELECT .*)/)["sql"]
+
+        "#{raw_sql}\n"
+      end
+    end
+  end
+
+  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}}
+              ]
+            }
+          }
+        }
+     }
+    end
+
+    it 'produces the same result on miss or hit' do
+      cold_results = execute(query)
+      warm_results = execute(query)
+
+      expect(cold_results).to eq(reference)
+      expect(cold_results).to eq warm_results
+    end
+  end
+
+  describe 'Seqeul' do
+    def execute(query, context = {})
+      CacheSchema.execute(query, context: context)
+    end
+    let(:customer) { Customer.last }
+
+    before { DB.logger = sql_logger }
+
+    it_behaves_like "be a correct cold and warm"
+
+    it 'calls sql engine only one time per cached field' do
+      5.times { execute(query) }
+
+      expect(sql_logger.messages).to eq(
+        <<~SQL
+          SELECT * FROM `customers` ORDER BY `id` DESC LIMIT 1
+          SELECT * FROM `customers` WHERE `id` = '1'
+          SELECT * FROM `orders` WHERE (`orders`.`customer_id` = 1)
+        SQL
+      )
+    end
+  end
+
+  describe 'ActiveRecord' do
+    def execute(query, context = {})
+      AR::CacheSchema.execute(query, context: context)
+    end
+
+    let(:customer) { AR::Customer.last }
+
+    around(:each) do |example|
+      default_logger = ActiveRecord::Base.logger
+      ActiveRecord::Base.logger = sql_logger
+      example.run
+      ActiveRecord::Base.logger = default_logger
+    end
+
+    it_behaves_like "be a correct cold and warm"
+
+    it 'calls sql engine only one time per cached field' do
+      5.times { execute(query) }
 
-    expect(cold_results).to eq warm_results
+      expect(sql_logger.messages.squish).to eq(
+        <<~SQL.squish
+          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\" = ? LIMIT ?\e[0m  [[\"customer_id\", 1], [\"LIMIT\", 50]]
+        SQL
+      )
+    end
   end
 end

spec/features/scalar_spec.rb

@@ -0,0 +1,38 @@
+require 'spec_helper'
+
+RSpec.describe 'caching scalar fields' do
+  let(:query) do
+    %Q{
+      {
+        customer(id: #{customer.id}) {
+          orders {
+            edges {
+              node {
+                totalPriceCents
+              }
+            }
+          }
+        }
+      }
+    }
+  end
+
+  describe 'ActiveRecord' do
+    def execute(query, context = {})
+      AR::CacheSchema.execute(query, context: context)
+    end
+
+    let(:customer) { AR::Customer.last }
+
+    before do
+      customer.orders.delete_all
+      customer.orders.create(total_price_cents: 100)# only one order
+    end
+
+    it 'calls order total_price_cents only one times' do
+      expect_any_instance_of(AR::Order).to receive(:total_price_cents).once.and_call_original
+
+      5.times { execute(query) }
+    end
+  end
+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

spec/spec_helper.rb

@@ -1,3 +1,7 @@
+# ORMs should be required before graphql-ruby
+require 'active_record'
+require 'sequel'
+
 require 'bundler/setup'
 require 'pry'
 
@@ -29,9 +33,15 @@
     DB.logger = GraphQL::Cache.logger
   end
 
+  config.before(:each) do
+    GraphQL::Cache.cache.clear
+  end
+
   # required after GraphQL::Cache initialization because dev
   # schema uses cache and logger objects from it.
-  require_relative '../test_schema'
+  %i[sequel active_record].each do |orm|
+    require_relative "../test_schema/#{orm}/init"
+  end
 
   config.include TestMacros
   config.extend  TestMacros::ClassMethods

spec/support/test_cache.rb

@@ -2,11 +2,13 @@
 
 class TestCache
   def write(key, doc, opts={})
-    cache[key] = doc
+    # duplicate the value to get rid of ruby object level caching
+    # and reproduce Rails.cache logic
+    cache[key] = ::Marshal.load(::Marshal.dump(doc))
   end
 
   def read(key)
-    cache[key]
+    ::Marshal.load(::Marshal.dump(cache[key]))
   end
 
   def cache

test_schema/active_record/graphql_schema.rb

@@ -0,0 +1,25 @@
+require 'benchmark'
+
+module AR
+  class BaseType < ::BaseType; end
+  class OrderType < ::OrderType; end
+
+  class CustomerType < ::CustomerType; end
+
+  class QueryType < ::QueryType
+    def customer(id:)
+      AR::Customer.find(id)
+    end
+  end
+
+  class CacheSchema < ::CacheSchema
+    query AR::QueryType
+    use GraphQL::Cache
+
+    default_max_page_size 50
+
+    def self.resolve_type(_type, obj, _ctx)
+      "AR::#{obj.class.name}Type"
+    end
+  end
+end

test_schema/active_record/init.rb

@@ -0,0 +1,10 @@
+require 'logger'
+
+require_relative './schema'
+require_relative './models'
+require_relative './graphql_schema'
+require_relative '../factories'
+
+ActiveRecord::Base.logger = GraphQL::Cache.logger
+Factories.new(order: AR::Order, customer: AR::Customer).bootstrap
+

test_schema/active_record/models.rb

@@ -0,0 +1,9 @@
+module AR
+  class Order < ActiveRecord::Base
+    belongs_to :customer
+  end
+
+  class Customer < ActiveRecord::Base
+    has_many :orders
+  end
+end

test_schema/active_record/schema.rb

@@ -0,0 +1,20 @@
+require 'active_record'
+
+ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
+
+ActiveRecord::Schema.define do
+  self.verbose = false
+
+  create_table :schema_migrations, force: true
+
+  create_table :customers, force: true do |t|
+    t.string :display_name
+    t.string :email
+  end
+
+  create_table :orders, force: true do |t|
+    t.integer :customer_id
+    t.integer :number
+    t.integer :total_price_cents
+  end
+end

test_schema/factories.rb

@@ -1,16 +1,25 @@
-module Factories
-  def self.bootstrap
-    customer = Customer.create(
+class Factories
+  def initialize(order:, customer:)
+    @order_class = order
+    @customer_class = customer
+  end
+
+  def bootstrap
+    customer = customer_class.create(
       display_name: 'Michael',
       email: 'michael@example.com'
     )
 
-    Order.create(customer_id: customer.id, number: new_num, total_price_cents: 1399)
-    Order.create(customer_id: customer.id, number: new_num, total_price_cents: 1399)
-    Order.create(customer_id: customer.id, number: new_num, total_price_cents: 1399)
+    order_class.create(customer_id: customer.id, number: new_num, total_price_cents: 1399)
+    order_class.create(customer_id: customer.id, number: new_num, total_price_cents: 1399)
+    order_class.create(customer_id: customer.id, number: new_num, total_price_cents: 1399)
   end
 
-  def self.new_num
-    Order.count + 1000
+  def new_num
+    order_class.count + 1000
   end
+
+  private
+
+  attr_reader :order_class, :customer_class
 end

test_schema/graphql_schema.rb → test_schema/sequel/graphql_schema.rb

@@ -7,17 +7,19 @@ class BaseType < GraphQL::Schema::Object
 class OrderType < BaseType
   field :id, Int, null: false
   field :number, Int, null: true
-  field :total_price_cents, Int, null: true
+  field :total_price_cents, Int, null: true, extensions: [::GraphQL::Cache::FieldExtension]
 end
 
 class CustomerType < BaseType
   field :display_name, String, null: false
   field :email, String, null: false
-  field :orders, OrderType.connection_type, null: false, cache: true
+  field :orders, OrderType.connection_type, null: false, extensions: [
+    ::GraphQL::Cache::FieldExtension
+  ]
 end
 
 class QueryType < BaseType
-  field :customer, CustomerType, null: true, cache: true do
+  field :customer, CustomerType, null: true, extensions: [::GraphQL::Cache::FieldExtension] do
     argument :id, ID, 'Unique Identifier for querying a specific user', required: true
   end
 
@@ -29,8 +31,11 @@ def customer(id:)
 class CacheSchema < GraphQL::Schema
   query QueryType
 
+  use GraphQL::Execution::Interpreter
   use GraphQL::Cache
 
+  default_max_page_size 50
+
   def self.resolve_type(_type, obj, _ctx)
     "#{obj.class.name}Type"
   end

test_schema/sequel/init.rb

@@ -0,0 +1,9 @@
+require 'logger'
+
+require_relative './schema'
+require_relative './models'
+require_relative './graphql_schema'
+require_relative '../factories'
+
+Factories.new(order: Order, customer: Customer).bootstrap
+DB.loggers = [GraphQL::Cache.logger]