rubyconfig / config

Easiest way to add multi-environment yaml settings to Rails, Sinatra, Padrino and other Ruby projects.
Other
2.1k stars 230 forks source link

Address edge case with table config param #339

Closed krasnoukhov closed 1 year ago

krasnoukhov commented 1 year ago

There's a weird edge case when config parameter is named table but at the same time config is not provided. I expect it could be a common thing, like a database table that is not provided by default. Added the specs for this scenario that fail before the patch.

On Ruby 3.0+ it fails with this:

Failures:

  1) Config::Options when Settings file is using keywords reserved for OpenStruct when empty should allow to access them via object member notation
     Failure/Error: expect(config.table).to be_nil

       expected: nil
            got: {}
     # ./spec/options_spec.rb:51:in `block (4 levels) in <main>'

  2) Config::Options when Settings file is using keywords reserved for OpenStruct when empty should allow to access them using [] operator
     Failure/Error: expect(config['table']).to be_nil

       expected: nil
            got: {}
     # ./spec/options_spec.rb:56:in `block (4 levels) in <main>'

Finished in 0.15976 seconds (files took 0.40783 seconds to load)
137 examples, 2 failures

Failed examples:

rspec ./spec/options_spec.rb:49 # Config::Options when Settings file is using keywords reserved for OpenStruct when empty should allow to access them via object member notation
rspec ./spec/options_spec.rb:54 # Config::Options when Settings file is using keywords reserved for OpenStruct when empty should allow to access them using [] operator

On Ruby 2.7:

Failures:

  1) Config::Options when Settings file is using keywords reserved for OpenStruct should allow to access them using [] operator
     Failure/Error: expect(config['table']).to eq('strawberry')

       expected: "strawberry"
            got: {:collect=>"banana", :count=>"lemon", :exit!=>"taro", :max=>"kumquat", :min=>"fig", :select=>"apple", :table=>"strawberry", :zip=>"cherry"}

       (compared using ==)

       Diff:
       @@ -1,8 +1,15 @@
       -"strawberry"
       +:collect => "banana",
       +:count => "lemon",
       +:exit! => "taro",
       +:max => "kumquat",
       +:min => "fig",
       +:select => "apple",
       +:table => "strawberry",
       +:zip => "cherry",

     # ./spec/options_spec.rb:32:in `block (3 levels) in <main>'

  2) Config::Options when Settings file is using keywords reserved for OpenStruct when empty should allow to access them using [] operator
     Failure/Error: expect(config['table']).to be_nil

       expected: nil
            got: {}
     # ./spec/options_spec.rb:56:in `block (4 levels) in <main>'

Finished in 0.13898 seconds (files took 0.362 seconds to load)
137 examples, 2 failures

Failed examples:

rspec ./spec/options_spec.rb:24 # Config::Options when Settings file is using keywords reserved for OpenStruct should allow to access them using [] operator
rspec ./spec/options_spec.rb:54 # Config::Options when Settings file is using keywords reserved for OpenStruct when empty should allow to access them using 
pkuczynski commented 1 year ago

@cjlarose what do you think? For me it looks legit...

cjlarose commented 1 year ago

Yeah, found a relevant SO post about this issue with OpenStruct

cjlarose commented 1 year ago

Published as version 4.2.1

krasnoukhov commented 1 year ago

Thanks folks! Appreciate it :shipit: