Use ActiveSupport's deep_dup under Rails 4.x+ to clone OpenStructWith…

…HashAccess instances

lib/blacklight/catalog/document_actions.rb

@@ -10,29 +10,9 @@ module Catalog::DocumentActions
       helper_method :document_actions
     end
 
-    class InheritableHash < SimpleDelegator
-      def initialize *args, &block
-        super(Hash.new *args, &block)
-      end
-
-      def inheritable_copy
-        h = self.class.new
-        each do |k,v|
-          case v
-            when Hash, OpenStruct, Array
-              h[k] = v.dup
-            else
-              h[k] = v
-          end
-        end
-        h
-      end
-
-    end
-
     module ClassMethods
       def document_actions
-        @document_actions ||= (inheritable_document_actions || InheritableHash.new).inheritable_copy
+        @document_actions ||= (inheritable_document_actions || Hash.new).deep_dup
       end
 
       ##
@@ -86,7 +66,7 @@ def add_document_action name, opts = {}
     end
 
     def document_actions
-      @document_actions ||= self.class.document_actions.inheritable_copy
+      @document_actions ||= self.class.document_actions.deep_dup
     end
   end
 end

lib/blacklight/configuration.rb

@@ -227,10 +227,15 @@ def facet_fields_to_add_to_solr
     # Provide a 'deep copy' of Blacklight::Configuration that can be modifyed without affecting
     # the original Blacklight::Configuration instance.
     #
-    def deep_copy
-      Marshal.load(Marshal.dump(self))
+    # The Rails 3.x version only copies hashes, and ignores arrays and similar structures
+    if ::Rails.version < "4.0"
+      def deep_copy
+        Marshal.load(Marshal.dump(self))
+      end
+    else
+      alias_method :deep_copy, :deep_dup
     end
-    alias_method :inheritable_copy, :deep_copy
+    alias_method :inheritable_copy, :deep_dup
 
     ##
     # Get a view configuration for the given view type

lib/blacklight/utils.rb

@@ -44,7 +44,11 @@ def merge other_hash
     # @return [OpenStructWithHashAccess] a new instance of an OpenStructWithHashAccess
     def merge! other_hash
       @table.merge!((other_hash if other_hash.is_a? Hash) || other_hash.to_h)
-    end 
+    end
+
+    def deep_dup
+      self.class.new @table.deep_dup
+    end
   end
 
 
@@ -123,6 +127,10 @@ def marshal_load x
       set_default_proc!
     end
 
+    def deep_dup
+      self.class.new self.nested_class, @table.deep_dup
+    end
+
     def select *args, &block
       self.class.new nested_class, to_h.select(*args, &block)
     end

spec/lib/blacklight/configuration_spec.rb

@@ -83,6 +83,21 @@
       expect(@config.a).to be_nil
       expect(@config.facet_fields).to_not include(@mock_facet)
     end
+
+    it "should provide cloned copies of mutable data structures" do
+      @config.a = { value: 1 }
+      @config.b = [1,2,3]
+
+      config_copy = @config.inheritable_copy
+
+      config_copy.a[:value] = 2
+      config_copy.b << 5
+
+      expect(@config.a[:value]).to eq 1
+      expect(config_copy.a[:value]).to eq 2
+      expect(@config.b).to match_array [1,2,3]
+      expect(config_copy.b).to match_array [1,2,3,5]
+    end
   end
 
   describe "add alternative solr fields" do

spec/lib/utils_spec.rb

@@ -92,5 +92,32 @@
         expect(subject.to_json).to eq ({a: 1, b:2}).to_json
       end
     end
+
+    describe "#deep_dup" do
+      subject { Blacklight::OpenStructWithHashAccess.new a: 1, b: { c: 1} }
+
+      it "should duplicate nested hashes" do
+        copy = subject.deep_dup
+        copy.a = 2
+        copy.b[:c] = 2
+
+        expect(subject.a).to eq 1
+        expect(subject.b[:c]).to eq 1
+        expect(copy.a).to eq 2
+        expect(copy.b[:c]).to eq 2
+      end
+
+      it "should preserve the current class" do
+        expect(Blacklight::NestedOpenStructWithHashAccess.new(Blacklight::NestedOpenStructWithHashAccess).deep_dup).to be_a_kind_of Blacklight::NestedOpenStructWithHashAccess
+      end
+
+      it "should preserve the default proc" do
+        nested = Blacklight::NestedOpenStructWithHashAccess.new Hash
+
+        copy = nested.deep_dup
+        copy.a[:b] = 1
+        expect(copy.a[:b]).to eq 1
+      end
+    end
   end
 end