Merge branch 'fledman-dry_options_handling_and_subelement_creation' into main

* fledman-dry_options_handling_and_subelement_creation:
  Fix up David Feldman's PR to work better than my online editor attempt at resolving merge conflicts
  one more test case
  remove ruby 1.9 backport
  unify all mutation code paths to fix bug
  setup failing test case for subscript mutation notation bug
  explicitly merge to avoid kwargs bug in rubies before 2.2
  dry options handling & subelement creation

Author: aetherknight

lib/recursive_open_struct.rb

@@ -3,7 +3,6 @@
 
 require 'recursive_open_struct/debug_inspect'
 require 'recursive_open_struct/deep_dup'
-require 'recursive_open_struct/ruby_19_backport'
 require 'recursive_open_struct/dig'
 
 # TODO: When we care less about Rubies before 2.4.0, match OpenStruct's method
@@ -14,23 +13,28 @@
 # `#to_h`.
 
 class RecursiveOpenStruct < OpenStruct
-  include Ruby19Backport if RUBY_VERSION =~ /\A1.9/
   include Dig if OpenStruct.public_instance_methods.include? :dig
 
   # TODO: deprecated, possibly remove or make optional an runtime so that it
   # doesn't normally pollute the public method namespace
   include DebugInspect
 
-  def initialize(hash=nil, args={})
+  def self.default_options
+    {
+      mutate_input_hash: false,
+      recurse_over_arrays: false,
+      preserve_original_keys: false
+    }
+  end
+
+  def initialize(hash=nil, passed_options={})
     hash ||= {}
-    @recurse_over_arrays = args.fetch(:recurse_over_arrays, false)
-    @preserve_original_keys = args.fetch(:preserve_original_keys, false)
-    @deep_dup = DeepDup.new(
-      recurse_over_arrays: @recurse_over_arrays,
-      preserve_original_keys: @preserve_original_keys
-    )
 
-    @table = args.fetch(:mutate_input_hash, false) ? hash : @deep_dup.call(hash)
+    @options = self.class.default_options.merge!(passed_options).freeze
+
+    @deep_dup = DeepDup.new(@options)
+
+    @table = @options[:mutate_input_hash] ? hash : @deep_dup.call(hash)
 
     @sub_elements = {}
   end
@@ -56,20 +60,22 @@ def [](name)
     key_name = _get_key_from_table_(name)
     v = @table[key_name]
     if v.is_a?(Hash)
-      @sub_elements[key_name] ||= self.class.new(
-        v,
-        recurse_over_arrays: @recurse_over_arrays,
-        preserve_original_keys: @preserve_original_keys,
-        mutate_input_hash: true
-      )
-    elsif v.is_a?(Array) and @recurse_over_arrays
+      @sub_elements[key_name] ||= _create_sub_element_(v, mutate_input_hash: true)
+    elsif v.is_a?(Array) and @options[:recurse_over_arrays]
       @sub_elements[key_name] ||= recurse_over_array(v)
       @sub_elements[key_name] = recurse_over_array(@sub_elements[key_name])
     else
       v
     end
   end
 
+  def []=(name, value)
+    key_name = _get_key_from_table_(name)
+    tbl = modifiable?  # Ensure we are modifiable
+    @sub_elements.delete(key_name)
+    tbl[key_name] = value
+  end
+
   # Makes sure ROS responds as expected on #respond_to? and #method requests
   def respond_to_missing?(mid, include_private = false)
     mname = _get_key_from_table_(mid.to_s.chomp('=').chomp('_as_a_hash'))
@@ -95,13 +101,16 @@ def method_missing(mid, *args)
       if len != 1
         raise ArgumentError, "wrong number of arguments (#{len} for 1)", caller(1)
       end
-      modifiable?[new_ostruct_member!($1.to_sym)] = args[0]
+      # self[$1.to_sym] = args[0]
+      # modifiable?[new_ostruct_member!($1.to_sym)] = args[0]
+      new_ostruct_member!($1.to_sym)
+      public_send(mid, args[0])
     elsif len == 0
       key = mid
       key = $1 if key =~ /^(.*)_as_a_hash$/
       if @table.key?(_get_key_from_table_(key))
         new_ostruct_member!(key)
-        send(mid)
+        public_send(mid)
       end
     else
       err = NoMethodError.new "undefined method `#{mid}' for #{self}", mid, args
@@ -120,8 +129,7 @@ class << self; self; end.class_eval do
           self[key_name]
         end
         define_method("#{name}=") do |x|
-          @sub_elements.delete(key_name)
-          modifiable?[key_name] = x
+          self[key_name] = x
         end
         define_method("#{name}_as_a_hash") { @table[key_name] }
       end
@@ -141,8 +149,8 @@ class << self; self; end.class_eval do
   def delete_field(name)
     sym = _get_key_from_table_(name)
     singleton_class.__send__(:remove_method, sym, "#{sym}=") rescue NoMethodError # ignore if methods not yet generated.
-    @sub_elements.delete sym
-    @table.delete sym
+    @sub_elements.delete(sym)
+    @table.delete(sym)
   end
 
   private
@@ -153,11 +161,14 @@ def _get_key_from_table_(name)
     name
   end
 
+  def _create_sub_element_(hash, **overrides)
+    self.class.new(hash, @options.merge(overrides))
+  end
+
   def recurse_over_array(array)
     array.each_with_index do |a, i|
       if a.is_a? Hash
-        array[i] = self.class.new(a, :recurse_over_arrays => true,
-          :mutate_input_hash => true, :preserve_original_keys => @preserve_original_keys)
+        array[i] = _create_sub_element_(a, mutate_input_hash: true, recurse_over_arrays: true)
       elsif a.is_a? Array
         array[i] = recurse_over_array a
       end

lib/recursive_open_struct/ruby_19_backport.rb

@@ -28,6 +28,15 @@
       expect(subject.blah_as_a_hash).to eq({ :another => 'value' })
     end
 
+    it "handles sub-element replacement with dotted notation before member setup" do
+      expect(ros[:blah][:another]).to eql 'value'
+      expect(ros.methods).not_to include(:blah)
+
+      ros.blah = { changed: 'backing' }
+
+      expect(ros.blah.changed).to eql 'backing'
+    end
+
     describe "handling loops in the original Hashes" do
       let(:h1) { { :a => 'a'} }
       let(:h2) { { :a => 'b', :h1 => h1 } }
@@ -55,6 +64,34 @@
       expect(ros.blah.blargh).to eq "Janet"
     end
 
+    describe 'subscript mutation notation' do
+      it 'handles the basic case' do
+        subject[:blah] = 12345
+        expect(subject.blah).to eql 12345
+      end
+
+      it 'recurses properly' do
+        subject[:blah][:another] = 'abc'
+        expect(subject.blah.another).to eql 'abc'
+        expect(subject.blah_as_a_hash).to eql({ :another => 'abc' })
+      end
+
+      let(:diff){ { :different => 'thing' } }
+
+      it 'can replace the entire hash' do
+        expect(subject.to_h).to eql(h)
+        subject[:blah] = diff
+        expect(subject.to_h).to eql({ :blah => diff })
+      end
+
+      it 'updates sub-element cache' do
+        expect(subject.blah.different).to be_nil
+        subject[:blah] = diff
+        expect(subject.blah.different).to eql 'thing'
+        expect(subject.blah_as_a_hash).to eql(diff)
+      end
+    end
+
     context "after a sub-element has been modified" do
       let(:hash) do
         { :blah => { :blargh => "Brad" }, :some_array => [ 1, 2, 3] }