From 2356764843fdab59fe0c568abb076ad6f1a236ee Mon Sep 17 00:00:00 2001
From: Tim Riley <tim@riley.id.au>
Date: Mon, 1 Jul 2024 23:01:37 +1000
Subject: [PATCH] Allow multiple includes of Dry::Configurable (#164)

Instead of raising AlreadyIncludedError, update the method undef code in `.included` to handle already-undefined methods gracefully.

Allowing Dry::Configurable to be included multiple times across subclasses now allows for uses cases like a subclass adjusting various extension settings, e.g. `config_class`.
---
 lib/dry/configurable/errors.rb                |  9 +-
 lib/dry/configurable/extension.rb             |  6 +-
 .../dry/configurable/included_spec.rb         | 96 +++++++++++++++++--
 3 files changed, 100 insertions(+), 11 deletions(-)

diff --git a/lib/dry/configurable/errors.rb b/lib/dry/configurable/errors.rb
index 37962f8..d04b4ca 100644
--- a/lib/dry/configurable/errors.rb
+++ b/lib/dry/configurable/errors.rb
@@ -5,9 +5,16 @@ module Dry
   #
   # @api public
   module Configurable
+    extend Dry::Core::Deprecations["dry-configurable"]
+
     Error = Class.new(::StandardError)
 
-    AlreadyIncludedError = Class.new(Error)
     FrozenConfigError = Class.new(Error)
+
+    AlreadyIncludedError = Class.new(Error)
+    deprecate_constant(
+      :AlreadyIncludedError,
+      message: "`include Dry::Configurable` more than once (if needed)"
+    )
   end
 end
diff --git a/lib/dry/configurable/extension.rb b/lib/dry/configurable/extension.rb
index 8e6bddc..ecfb3f0 100644
--- a/lib/dry/configurable/extension.rb
+++ b/lib/dry/configurable/extension.rb
@@ -26,8 +26,6 @@ def extended(klass)
 
       # @api private
       def included(klass)
-        raise AlreadyIncludedError if klass.include?(InstanceMethods)
-
         super
 
         klass.class_eval do
@@ -36,8 +34,8 @@ def included(klass)
           prepend(Initializer)
 
           class << self
-            undef :config
-            undef :configure
+            undef :config if method_defined?(:config)
+            undef :configure if method_defined?(:configure)
           end
         end
 
diff --git a/spec/integration/dry/configurable/included_spec.rb b/spec/integration/dry/configurable/included_spec.rb
index cb427f9..280565e 100644
--- a/spec/integration/dry/configurable/included_spec.rb
+++ b/spec/integration/dry/configurable/included_spec.rb
@@ -12,12 +12,6 @@
         .to include(Dry::Configurable::InstanceMethods)
     end
 
-    it "raises when Dry::Configurable has already been included" do
-      expect {
-        configurable_klass.include(Dry::Configurable)
-      }.to raise_error(Dry::Configurable::AlreadyIncludedError)
-    end
-
     it "ensures `.config` is not defined" do
       expect(configurable_klass).not_to respond_to(:config)
     end
@@ -77,4 +71,94 @@ def finalize!
       expect(instance.finalized).to be(true)
     end
   end
+
+  context "with deep class hierarchy" do
+    let(:configurable_class) do
+      Class.new do
+        include Dry::Configurable
+      end
+    end
+
+    it "allows subclasses also to include Dry::Configurable" do
+      subclass = Class.new(configurable_class) do
+        include Dry::Configurable
+      end
+
+      expect(subclass.new.config).to be_a(Dry::Configurable::Config)
+    end
+
+    it "allows subclasses to reconfigure the behavior" do
+      custom_config_class_1 = Class.new(Dry::Configurable::Config) do
+        def db
+          "#{super}!!"
+        end
+      end
+
+      custom_config_class_2 = Class.new(Dry::Configurable::Config) do
+        def db
+          "#{super}??"
+        end
+      end
+
+      subclass_l1 = Class.new(configurable_class) do
+        setting :db
+      end
+
+      subclass_l2 = Class.new(subclass_l1) do
+        include Dry::Configurable(config_class: custom_config_class_1)
+      end
+
+      subclass_l3 = Class.new(subclass_l2)
+
+      subclass_l4 = Class.new(subclass_l3) do
+        include Dry::Configurable(config_class: custom_config_class_2)
+      end
+
+      obj = subclass_l2.new
+      obj.config.db = "sqlite"
+      expect(obj.config.db).to eq "sqlite!!"
+
+      obj = subclass_l3.new
+      obj.config.db = "postgres"
+      expect(obj.config.db).to eq "postgres!!"
+
+      obj = subclass_l4.new
+      obj.config.db = "sqlite"
+      expect(obj.config.db).to eq "sqlite??"
+    end
+
+    it "sets up config across multiple prepended initialize methods" do
+      custom_config_class = Class.new(Dry::Configurable::Config) do
+        def db
+          "#{super}!!"
+        end
+      end
+
+      subclass_l1 = Class.new(configurable_class) do
+        setting :db
+
+        def initialize
+          super
+        end
+      end
+
+      subclass_l2 = Class.new(subclass_l1) do
+        def initialize
+          super
+          config.db = "l2_db"
+        end
+      end
+
+      subclass_l3 = Class.new(subclass_l2) do
+        include Dry::Configurable(config_class: custom_config_class)
+
+        def initialize
+          super
+        end
+      end
+
+      obj = subclass_l3.new
+      expect(obj.config.db).to eq "l2_db!!"
+    end
+  end
 end