Better Redis configuration

Author: DmitryTsepelev

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## master
 
+- [PR#7](https://github.com/DmitryTsepelev/graphql-ruby-persisted_queries/pull/7) Improved Redis configuration – added `Proc` and `ConnectionPool` support ([@DmitryTsepelev][])
+
 ## 0.1.0 (2019-10-21)
 
 - Initial version ([@DmitryTsepelev][])

README.md

@@ -63,11 +63,27 @@ All the queries are stored in memory by default, but you can easily switch to _r
 
 ```ruby
 class GraphqlSchema < GraphQL::Schema
-  use GraphQL::PersistedQueries, store: :redis, redis_url: ENV["MY_REDIS_URL"]
+  use GraphQL::PersistedQueries, store: :redis, redis_client: { redis_url: ENV["MY_REDIS_URL"] }
 end
 ```
 
-If you have `ENV["REDIS_URL"]` configured – you don't need to pass it explicitly. Also, you can pass `:redis_host`, `:redis_port` and `:redis_db_name` to build the URL from scratch or configure Redis client as you want and pass it as the `:client` option.
+If you have `ENV["REDIS_URL"]` configured – you don't need to pass it explicitly. Also, you can pass `:redis_host`, `:redis_port` and `:redis_db_name` inside the `:redis_client` hash to build the URL from scratch or pass the configured `Redis` or `ConnectionPool` object:
+
+```ruby
+class GraphqlSchema < GraphQL::Schema
+  use GraphQL::PersistedQueries,
+      store: :redis,
+      redis_client: { redis_host: "127.0.0.2", redis_port: "2214", redis_db_name: "7" }
+  # or
+  use GraphQL::PersistedQueries,
+      store: :redis,
+      redis_client: Redis.new(url: "redis://127.0.0.2:2214/7")
+  # or
+  use GraphQL::PersistedQueries,
+      store: :redis,
+      redis_client: ConnectionPool.new { Redis.new(url: "redis://127.0.0.2:2214/7") }
+end
+```
 
 ## Alternative hash functions
 

graphql-persisted_queries.gemspec

@@ -28,4 +28,5 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency "rake", ">= 10.0"
   spec.add_development_dependency "rubocop", "0.75"
   spec.add_development_dependency "redis"
+  spec.add_development_dependency "connection_pool"
 end

lib/graphql/persisted_queries/store_adapters/redis_client_builder.rb

@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+module GraphQL
+  module PersistedQueries
+    module StoreAdapters
+      # Builds Redis object instance based on passed hash
+      class RedisClientBuilder
+        def initialize(redis_url: nil, redis_host: nil, redis_port: nil, redis_db_name: nil)
+          require "redis"
+
+          @redis_url = redis_url
+          @redis_host = redis_host
+          @redis_port = redis_port
+          @redis_db_name = redis_db_name
+        rescue LoadError => e
+          msg = "Could not load the 'redis' gem, please add it to your gemfile or " \
+                "configure a different adapter, e.g. use GraphQL::PersistedQueries, store: :memory"
+          raise e.class, msg, e.backtrace
+        end
+
+        def build
+          if @redis_url && (@redis_host || @redis_port || @redis_db_name)
+            raise ArgumentError, "redis_url cannot be passed along with redis_host, redis_port " \
+                                 "or redis_db_name options"
+          end
+
+          ::Redis.new(url: @redis_url || build_redis_url)
+        end
+
+        private
+
+        DEFAULT_REDIS_DB = "0"
+
+        def build_redis_url
+          db_name = @redis_db_name || DEFAULT_REDIS_DB
+          base_url = ENV["REDIS_URL"] || "redis://#{@redis_host}:#{@redis_port}"
+          URI.join(base_url, db_name).to_s
+        end
+      end
+    end
+  end
+end

lib/graphql/persisted_queries/store_adapters/redis_store_adapter.rb

@@ -1,65 +1,49 @@
 # frozen_string_literal: true
 
+require "graphql/persisted_queries/store_adapters/redis_client_builder"
+
 module GraphQL
   module PersistedQueries
     module StoreAdapters
       # Redis adapter for storing persisted queries
       class RedisStoreAdapter < BaseStoreAdapter
-        attr_reader :storage
-
-        def initialize(client: nil, **options)
-          require "redis"
-
-          if client && options.any?
-            raise ArgumentError, "client cannot be passed along with redis_url, redis_host" \
-                                 ", redis_port or redis_db_name options"
-          end
-
-          @storage = client || configure_redis_client(options)
-        rescue LoadError => e
-          msg = "Could not load the 'redis' gem, please add it to your gemfile or " \
-                "configure a different adapter, e.g. use GraphQL::PersistedQueries, store: :memory"
-          raise e.class, msg, e.backtrace
+        def initialize(redis_client:)
+          @redis_proc = build_redis_proc(redis_client)
         end
 
         def fetch_query(hash)
-          storage.get(key_for(hash))
+          @redis_proc.call { |redis| redis.get(key_for(hash)) }
         end
 
         def save_query(hash, query)
-          storage.set(key_for(hash), query)
+          @redis_proc.call { |redis| redis.set(key_for(hash), query) }
         end
 
         private
 
         def key_for(hash)
-          "persisted-query-#{hash}"
+          "graphql-persisted-query-#{hash}"
         end
 
-        # rubocop:disable Metrics/LineLength
-        def configure_redis_client(redis_url: nil, redis_host: nil, redis_port: nil, redis_db_name: nil)
-          if redis_url && (redis_host || redis_port || redis_db_name)
-            raise ArgumentError, "redis_url cannot be passed along with redis_host, redis_port " \
-                                 "or redis_db_name options"
+        # rubocop: disable Metrics/MethodLength
+        # rubocop: disable Metrics/CyclomaticComplexity
+        # rubocop: disable Metrics/PerceivedComplexity
+        def build_redis_proc(redis_client)
+          if redis_client.is_a?(Hash)
+            build_redis_proc(RedisClientBuilder.new(redis_client).build)
+          elsif redis_client.is_a?(Proc)
+            redis_client
+          elsif defined?(::Redis) && redis_client.is_a?(::Redis)
+            proc { |&b| b.call(redis_client) }
+          elsif defined?(ConnectionPool) && redis_client.is_a?(ConnectionPool)
+            proc { |&b| redis_client.with { |r| b.call(r) } }
+          else
+            raise ArgumentError, ":redis_client accepts Redis, ConnectionPool, Hash or Proc only"
           end
-
-          redis_url ||= build_redis_url(
-            redis_host: redis_host,
-            redis_port: redis_port,
-            redis_db_name: redis_db_name
-          )
-
-          Redis.new(url: redis_url)
-        end
-        # rubocop:enable Metrics/LineLength
-
-        DEFAULT_REDIS_DB = "0"
-
-        def build_redis_url(redis_host: nil, redis_port: nil, redis_db_name: nil)
-          redis_db_name ||= DEFAULT_REDIS_DB
-          redis_base_url = ENV["REDIS_URL"] || "redis://#{redis_host}:#{redis_port}"
-          URI.join(redis_base_url, redis_db_name).to_s
         end
+        # rubocop: enable Metrics/MethodLength
+        # rubocop: enable Metrics/CyclomaticComplexity
+        # rubocop: enable Metrics/PerceivedComplexity
       end
     end
   end

spec/graphql/persisted_queries/store_adapters/redis_client_builder_spec.rb

@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+RSpec.describe GraphQL::PersistedQueries::StoreAdapters::RedisClientBuilder do
+  describe "#initialize" do
+    let(:options) { {} }
+
+    subject { described_class.new(options).build }
+
+    context "when redis_host, redis_port and redis_db_name are passed" do
+      let(:options) do
+        { redis_host: "127.0.0.2", redis_port: "2214", redis_db_name: "7" }
+      end
+
+      it "builds redis URL" do
+        expect(subject.connection[:id]).to eq("redis://127.0.0.2:2214/7")
+      end
+    end
+
+    context "when REDIS_URL is configured" do
+      let(:redis_url) { "redis://127.0.0.1:6379" }
+
+      before do
+        allow(ENV).to receive(:[]).with("REDIS_URL").and_return(redis_url)
+      end
+
+      it "uses default db" do
+        expect(subject.connection[:id]).to eq("redis://127.0.0.1:6379/0")
+      end
+
+      context "when redis_db_name is configured" do
+        let(:options) { { redis_db_name: "42" } }
+
+        it "uses configured db" do
+          expect(subject.connection[:id]).to eq("redis://127.0.0.1:6379/42")
+        end
+      end
+    end
+
+    context "when redis_url is passed" do
+      let(:options) { { redis_url: "redis://127.0.0.4:2177/22" } }
+
+      it "uses passed redis_url" do
+        expect(subject.connection[:id]).to eq("redis://127.0.0.4:2177/22")
+      end
+
+      context "when passed along with other parameters" do
+        let(:options) do
+          {
+            redis_url: "redis://127.0.0.1:6379",
+            redis_host: "127.0.0.1",
+            redis_port: "6379",
+            redis_db_name: "0"
+          }
+        end
+
+        it "raises error" do
+          expect { subject }.to raise_error(
+            ArgumentError,
+            "redis_url cannot be passed along with redis_host, redis_port or redis_db_name options"
+          )
+        end
+      end
+    end
+  end
+end

spec/graphql/persisted_queries/store_adapters/redis_store_adapter_spec.rb

@@ -3,95 +3,51 @@
 require "spec_helper"
 
 require "redis"
+require "connection_pool"
 
 RSpec.describe GraphQL::PersistedQueries::StoreAdapters::RedisStoreAdapter do
-  describe "#initialize" do
-    let(:options) { {} }
+  subject { described_class.new(redis_client: redis_client) }
 
-    subject { described_class.new(options) }
+  context "when Hash instance is passed" do
+    let(:redis_client) { { redis_url: "redis://127.0.0.3:8791/3" } }
 
-    context "when client is passed" do
-      let(:options) { { client: Redis.new(url: "redis://127.0.0.3:8791/3") } }
-
-      it "uses client" do
-        expect(subject.storage.connection[:id]).to eq("redis://127.0.0.3:8791/3")
-      end
-
-      context "when passed along with other parameters" do
-        let(:options) do
-          {
-            client: Redis.new(url: "redis://127.0.0.3:8791/3"),
-            redis_url: "redis://127.0.0.1:6379",
-            redis_host: "127.0.0.1",
-            redis_port: "6379",
-            redis_db_name: "0"
-          }
-        end
-
-        it "raises error" do
-          expect { subject }.to raise_error(
-            ArgumentError,
-            "client cannot be passed along with redis_url, redis_host, " \
-            "redis_port or redis_db_name options"
-          )
-        end
-      end
+    it "wraps with proc" do
+      expect(subject.instance_variable_get("@redis_proc")).to be_kind_of(Proc)
     end
+  end
 
-    context "when redis_host, redis_port and redis_db_name are passed" do
-      let(:options) do
-        { redis_host: "127.0.0.2", redis_port: "2214", redis_db_name: "7" }
-      end
+  context "when Proc instance is passed" do
+    let(:redis_client) { proc {} }
 
-      it "builds redis URL" do
-        expect(subject.storage.connection[:id]).to eq("redis://127.0.0.2:2214/7")
-      end
+    it "wraps with proc" do
+      expect(subject.instance_variable_get("@redis_proc")).to be_kind_of(Proc)
     end
+  end
 
-    context "when REDIS_URL is configured" do
-      let(:redis_url) { "redis://127.0.0.1:6379" }
-
-      before do
-        allow(ENV).to receive(:[]).with("REDIS_URL").and_return(redis_url)
-      end
-
-      it "uses default db" do
-        expect(subject.storage.connection[:id]).to eq("redis://127.0.0.1:6379/0")
-      end
-
-      context "when redis_db_name is configured" do
-        let(:options) { { redis_db_name: "42" } }
+  context "when Redis instance is passed" do
+    let(:redis_client) { Redis.new(url: "redis://127.0.0.3:8791/3") }
 
-        it "uses configured db" do
-          expect(subject.storage.connection[:id]).to eq("redis://127.0.0.1:6379/42")
-        end
-      end
+    it "wraps with proc" do
+      expect(subject.instance_variable_get("@redis_proc")).to be_kind_of(Proc)
     end
+  end
 
-    context "when redis_url is passed" do
-      let(:options) { { redis_url: "redis://127.0.0.4:2177/22" } }
+  context "when ConnectionPool instance is passed" do
+    let(:redis_client) { ConnectionPool.new { Redis.new(url: "redis://127.0.0.3:8791/3") } }
 
-      it "uses passed redis_url" do
-        expect(subject.storage.connection[:id]).to eq("redis://127.0.0.4:2177/22")
-      end
+    it "wraps with proc" do
+      expect(subject.instance_variable_get("@redis_proc")).to be_kind_of(Proc)
+    end
+  end
 
-      context "when passed along with other parameters" do
-        let(:options) do
-          {
-            redis_url: "redis://127.0.0.1:6379",
-            redis_host: "127.0.0.1",
-            redis_port: "6379",
-            redis_db_name: "0"
-          }
-        end
+  context "when not supported object is passed" do
+    let(:redis_client) { 42 }
 
-        it "raises error" do
-          expect { subject }.to raise_error(
-            ArgumentError,
-            "redis_url cannot be passed along with redis_host, redis_port or redis_db_name options"
-          )
-        end
-      end
+    it "raises error" do
+      expect { subject }.to raise_error(
+        ArgumentError,
+        ":redis_client accepts Redis, ConnectionPool, Hash or Proc only"
+      )
     end
   end
 end