From 9bfc6112f6dd4ea46a5798f71bce8cf230df4c38 Mon Sep 17 00:00:00 2001 From: Vladimir Kochnev Date: Mon, 1 Feb 2016 04:24:27 +0300 Subject: [PATCH 1/6] Pass &callback to Loader constructor, not to #load method. --- lib/nenv/environment.rb | 2 +- lib/nenv/environment/loader.rb | 5 +++-- spec/lib/nenv/environment/loader_spec.rb | 2 +- spec/lib/nenv/environment_spec.rb | 6 +++--- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/nenv/environment.rb b/lib/nenv/environment.rb index 782e393..5a1c3e5 100644 --- a/lib/nenv/environment.rb +++ b/lib/nenv/environment.rb @@ -65,7 +65,7 @@ def _create_env_reader(klass, meth, &block) env_name = nil klass.send(:define_method, meth) do env_name ||= _namespaced_sanitize(meth) - Loader.new(meth).load(ENV[env_name], &block) + Loader.new(meth, &block).load(ENV[env_name]) end end diff --git a/lib/nenv/environment/loader.rb b/lib/nenv/environment/loader.rb index fb8dc29..26d9aa5 100644 --- a/lib/nenv/environment/loader.rb +++ b/lib/nenv/environment/loader.rb @@ -1,12 +1,13 @@ module Nenv class Environment class Loader - def initialize(meth) + def initialize(meth, &callback) @bool = meth.to_s.end_with?('?') + @callback = callback end def load(raw_value, &callback) - return callback.call(raw_value) if callback + return @callback.call(raw_value) if @callback @bool ? _to_bool(raw_value) : raw_value end diff --git a/spec/lib/nenv/environment/loader_spec.rb b/spec/lib/nenv/environment/loader_spec.rb index 0bdfaa7..d457601 100644 --- a/spec/lib/nenv/environment/loader_spec.rb +++ b/spec/lib/nenv/environment/loader_spec.rb @@ -49,7 +49,7 @@ context 'with a block' do subject do - described_class.new(:foo).load(value) { |data| YAML.load(data) } + described_class.new(:foo) { |data| YAML.load(data) }.load(value) end context 'with a yaml string' do let(:value) { "--- foo\n...\n" } diff --git a/spec/lib/nenv/environment_spec.rb b/spec/lib/nenv/environment_spec.rb index 2835055..9949043 100644 --- a/spec/lib/nenv/environment_spec.rb +++ b/spec/lib/nenv/environment_spec.rb @@ -92,8 +92,9 @@ end context 'with a block' do + let(:block) { proc { |data| YAML.load(data) } } before do - instance.create_method(:foo) { |data| YAML.load(data) } + instance.create_method(:foo, &block) end let(:value) { "---\n:foo: 5\n" } @@ -102,8 +103,7 @@ allow(ENV).to receive(:[]).with('BAR_FOO') .and_return(value) - allow(loader).to receive(:load).with(value) do |arg, &block| - expect(block).to be + allow(loader).to receive(:load).with(value) do |arg| block.call(arg) end From 387f88699d150717d36993db793bfe52ab3ca246 Mon Sep 17 00:00:00 2001 From: Vladimir Kochnev Date: Mon, 1 Feb 2016 04:35:11 +0300 Subject: [PATCH 2/6] Pass &callback to Dumper constructor, not to #dump method. --- lib/nenv/environment.rb | 2 +- lib/nenv/environment/dumper.rb | 8 ++++++-- spec/lib/nenv/environment/dumper_spec.rb | 2 +- spec/lib/nenv/environment_spec.rb | 6 +++--- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/nenv/environment.rb b/lib/nenv/environment.rb index 5a1c3e5..0dea0c6 100644 --- a/lib/nenv/environment.rb +++ b/lib/nenv/environment.rb @@ -57,7 +57,7 @@ def _create_env_writer(klass, meth, &block) env_name = nil klass.send(:define_method, meth) do |raw_value| env_name ||= _namespaced_sanitize(meth) - ENV[env_name] = Dumper.new.dump(raw_value, &block) + ENV[env_name] = Dumper.new(&block).dump(raw_value) end end diff --git a/lib/nenv/environment/dumper.rb b/lib/nenv/environment/dumper.rb index a702259..f5b0561 100644 --- a/lib/nenv/environment/dumper.rb +++ b/lib/nenv/environment/dumper.rb @@ -1,8 +1,12 @@ module Nenv class Environment class Dumper - def dump(raw_value, &callback) - return callback.call(raw_value) if callback + def initialize(&callback) + @callback = callback + end + + def dump(raw_value) + return @callback.call(raw_value) if @callback raw_value.nil? ? nil : raw_value.to_s end end diff --git a/spec/lib/nenv/environment/dumper_spec.rb b/spec/lib/nenv/environment/dumper_spec.rb index 6500ce4..75b647a 100644 --- a/spec/lib/nenv/environment/dumper_spec.rb +++ b/spec/lib/nenv/environment/dumper_spec.rb @@ -22,7 +22,7 @@ context 'with a block' do subject do - described_class.new.dump(value) { |data| YAML.dump(data) } + described_class.new { |data| YAML.dump(data) }.dump(value) end context 'with a yaml string' do diff --git a/spec/lib/nenv/environment_spec.rb b/spec/lib/nenv/environment_spec.rb index 9949043..a2df38b 100644 --- a/spec/lib/nenv/environment_spec.rb +++ b/spec/lib/nenv/environment_spec.rb @@ -138,8 +138,9 @@ end context 'with a block' do + let(:block) { proc { |data| YAML.dump(data) } } before do - instance.create_method(:foo=) { |data| YAML.dump(data) } + instance.create_method(:foo=, &block) end let(:result) { "---\n:foo: 5\n" } @@ -147,8 +148,7 @@ it 'marshals using the block' do allow(ENV).to receive(:[]=).with('BAR_FOO', result) - allow(dumper).to receive(:dump).with(foo: 5) do |arg, &block| - expect(block).to be + allow(dumper).to receive(:dump).with(foo: 5) do |arg| block.call(arg) end From a26b127c4110e7899a9c971202ba13050327907b Mon Sep 17 00:00:00 2001 From: Vladimir Kochnev Date: Mon, 1 Feb 2016 04:37:14 +0300 Subject: [PATCH 3/6] Lazily initialize loader and dumper. --- lib/nenv/environment.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/nenv/environment.rb b/lib/nenv/environment.rb index 0dea0c6..3b1961d 100644 --- a/lib/nenv/environment.rb +++ b/lib/nenv/environment.rb @@ -55,17 +55,21 @@ def _create_env_accessor(klass, meth, &block) def _create_env_writer(klass, meth, &block) env_name = nil + dumper = nil klass.send(:define_method, meth) do |raw_value| env_name ||= _namespaced_sanitize(meth) - ENV[env_name] = Dumper.new(&block).dump(raw_value) + dumper ||= Dumper.new(&block) + ENV[env_name] = dumper.dump(raw_value) end end def _create_env_reader(klass, meth, &block) env_name = nil + loader = nil klass.send(:define_method, meth) do env_name ||= _namespaced_sanitize(meth) - Loader.new(meth, &block).load(ENV[env_name]) + loader ||= Loader.new(meth, &block) + loader.load(ENV[env_name]) end end From c568e8eaf83b109cfe8b203f41a9d7d27cf57899 Mon Sep 17 00:00:00 2001 From: Vladimir Kochnev Date: Tue, 2 Feb 2016 02:00:45 +0300 Subject: [PATCH 4/6] Refactor main specs. Creepy mocks are removed. Common behavior is extracted to shared examples. --- spec/lib/nenv/environment_spec.rb | 254 +++++++++++------------------- 1 file changed, 96 insertions(+), 158 deletions(-) diff --git a/spec/lib/nenv/environment_spec.rb b/spec/lib/nenv/environment_spec.rb index a2df38b..e1c3e46 100644 --- a/spec/lib/nenv/environment_spec.rb +++ b/spec/lib/nenv/environment_spec.rb @@ -3,47 +3,60 @@ require 'nenv/environment' RSpec.describe Nenv::Environment do - let(:env) { instance_double(Hash) } # a hash is close enough - before(:each) { stub_const('ENV', env) } + class MockEnv < Hash # a hash is close enough + def []=(k, v) + super(k.to_s, v.to_s) + end + end - context 'without integration' do - let(:dumper) { instance_double(described_class::Dumper) } - let(:loader) { instance_double(described_class::Loader) } + before { stub_const('ENV', MockEnv.new) } + subject { instance } - before do - allow(described_class::Dumper).to receive(:new).and_return(dumper) - allow(described_class::Loader).to receive(:new).and_return(loader) - end + shared_examples 'accessor methods' do + describe 'predicate method' do + before { subject.create_method(:foo?) } - context 'with no namespace' do - let(:instance) { described_class.new } + it 'responds to it' do + expect(subject).to respond_to(:foo?) + end - context 'with an existing method' do - before do - subject.create_method(:foo?) + context 'when the method already exists' do + let(:error) { described_class::AlreadyExistsError } + let(:message) { 'Method :foo? already exists' } + specify do + expect do + subject.create_method(:foo?) + end.to raise_error(error, message) end + end - it 'uses the name as full key' do - expect(ENV).to receive(:[]).with('FOO').and_return('true') - expect(loader).to receive(:load).with('true').and_return(true) - expect(subject.foo?).to eq(true) + context 'with value stored in ENV' do + before { ENV[sample_key] = value } + + describe 'when value is truthy' do + let(:value) { 'true' } + it 'should return true' do + expect(subject.foo?).to eq true + end + end + + describe 'when value is falsey' do + let(:value) { '0' } + it 'should return false' do + expect(subject.foo?).to eq false + end end end end - context 'with any namespace' do - let(:namespace) { 'bar' } - let(:instance) { described_class.new(namespace) } - - describe 'creating a method' do - subject { instance } + describe 'reader method' do + context 'when added' do + before { subject.create_method(:foo) } - before do - subject.create_method(:foo) + it 'responds to it' do + expect(subject).to respond_to(:foo) end - it { is_expected.to respond_to(:foo) } - context 'when the method already exists' do let(:error) { described_class::AlreadyExistsError } let(:message) { 'Method :foo already exists' } @@ -55,165 +68,90 @@ end end - describe 'calling' do - subject { instance } - - context 'when method does not exist' do - let(:error) { NoMethodError } - let(:message) { /undefined method `foo' for/ } - it { expect { subject.foo }.to raise_error(error, message) } - end - - context 'with a reader method' do - context 'with no block' do - before { instance.create_method(meth) } - - context 'with a normal method' do - let(:meth) { :foo } - before do - allow(loader).to receive(:load).with('123').and_return(123) - end - - it 'returns unmarshalled stored value' do - expect(ENV).to receive(:[]).with('BAR_FOO').and_return('123') - expect(subject.foo).to eq 123 - end - end - - context 'with a bool method' do - let(:meth) { :foo? } - - it 'references the proper ENV variable' do - allow(loader).to receive(:load).with('false').and_return(false) - expect(ENV).to receive(:[]).with('BAR_FOO').and_return('false') - expect(subject.foo?).to eq false - end - end - end - - context 'with a block' do - let(:block) { proc { |data| YAML.load(data) } } - before do - instance.create_method(:foo, &block) - end + context 'with value stored in ENV' do + before { ENV[sample_key] = value } - let(:value) { "---\n:foo: 5\n" } - - it 'unmarshals using the block' do - allow(ENV).to receive(:[]).with('BAR_FOO') - .and_return(value) - - allow(loader).to receive(:load).with(value) do |arg| - block.call(arg) - end + context 'with no block' do + before { instance.create_method(:foo) } + let(:value) { 123 } - expect(subject.foo).to eq(foo: 5) - end + it 'returns marshalled stored value' do + expect(subject.foo).to eq '123' end end - context 'with a writer method' do - before { instance.create_method(:foo=) } + context 'with block' do + before { instance.create_method(:foo) { |data| YAML.load(data) } } + let(:value) { "---\n:foo: 5\n" } - it 'set the environment variable' do - expect(ENV).to receive(:[]=).with('BAR_FOO', '123') - allow(dumper).to receive(:dump).with(123).and_return('123') - subject.foo = 123 - end - - it 'marshals and stores the value' do - expect(ENV).to receive(:[]=).with('BAR_FOO', '123') - allow(dumper).to receive(:dump).with(123).and_return('123') - subject.foo = 123 - end - end - - context 'with a method containing underscores' do - before { instance.create_method(:foo_baz) } - it 'reads the correct variable' do - expect(ENV).to receive(:[]).with('BAR_FOO_BAZ').and_return('123') - allow(loader).to receive(:load).with('123').and_return(123) - subject.foo_baz + it 'returns unmarshalled stored value' do + expect(subject.foo).to eq(foo: 5) end end + end + end - context 'with a block' do - let(:block) { proc { |data| YAML.dump(data) } } - before do - instance.create_method(:foo=, &block) - end - - let(:result) { "---\n:foo: 5\n" } - - it 'marshals using the block' do - allow(ENV).to receive(:[]=).with('BAR_FOO', result) - - allow(dumper).to receive(:dump).with(foo: 5) do |arg| - block.call(arg) - end + describe 'writer method' do + context 'when added' do + before { subject.create_method(:foo=) } - subject.foo = { foo: 5 } - end + it 'responds to it' do + expect(subject).to respond_to(:foo=) end - context 'with an unsanitized name' do - pending + context 'when the method already exists' do + let(:error) { described_class::AlreadyExistsError } + let(:message) { 'Method :foo= already exists' } + specify do + expect do + subject.create_method(:foo=) + end.to raise_error(error, message) + end end end - end - end - describe 'with integration' do - context 'with any namespace' do - let(:namespace) { 'baz' } - let(:instance) { described_class.new(namespace) } - subject { instance } + describe 'env variable' do + after { expect(ENV[sample_key]).to eq result } - context 'with a reader method' do context 'with no block' do - before { instance.create_method(:foo) } + before { subject.create_method(:foo=) } + let(:result) { '123' } - it 'returns the stored value' do - allow(ENV).to receive(:[]).with('BAZ_FOO').and_return('123') - expect(subject.foo).to eq '123' + it 'stores a converted to string value' do + subject.foo = 123 end end - context 'with a block' do - before do - instance.create_method(:foo) { |data| YAML.load(data) } - end - - it 'unmarshals the value' do - expect(ENV).to receive(:[]).with('BAZ_FOO') - .and_return("---\n:foo: 5\n") + context 'with block' do + before { subject.create_method(:foo=) { |data| YAML.dump(data) } } + let(:result) { "---\n:foo: 5\n" } - expect(subject.foo).to eq(foo: 5) + it 'stores a marshaled value' do + subject.foo = { foo: 5 } end end end + end + end - context 'with a writer method' do - context 'with no block' do - before { instance.create_method(:foo=) } - - it 'marshals and stores the value' do - expect(ENV).to receive(:[]=).with('BAZ_FOO', '123') - subject.foo = 123 - end - end + context 'with no namespace' do + let(:instance) { described_class.new } + let(:sample_key) { 'FOO' } + include_examples 'accessor methods' + end - context 'with a block' do - before do - instance.create_method(:foo=) { |data| YAML.dump(data) } - end + context 'with any namespace' do + let(:namespace) { 'bar' } + let(:sample_key) { 'BAR_FOO' } + let(:instance) { described_class.new(namespace) } + include_examples 'accessor methods' - it 'nmarshals the value' do - expect(ENV).to receive(:[]=).with('BAZ_FOO', "---\n:foo: 5\n") + context 'with a method containing underscores' do + before { instance.create_method(:foo_baz) } - subject.foo = { foo: 5 } - end - end + it 'reads the correct variable' do + ENV['BAR_FOO_BAZ'] = 123 + expect(subject.foo_baz).to eq '123' end end end From 60afe589ea652c224698b6ffdbbf766577c82cda Mon Sep 17 00:00:00 2001 From: Vladimir Kochnev Date: Sun, 7 Feb 2016 04:22:16 +0300 Subject: [PATCH 5/6] Loader/Dumper strategy factory. --- lib/nenv/environment.rb | 8 +++--- lib/nenv/environment/dumper.rb | 15 +++++------ lib/nenv/environment/dumper/default.rb | 9 +++++++ lib/nenv/environment/loader.rb | 32 ++++++++---------------- lib/nenv/environment/loader/default.rb | 9 +++++++ lib/nenv/environment/loader/predicate.rb | 18 +++++++++++++ spec/lib/nenv/environment/dumper_spec.rb | 4 +-- spec/lib/nenv/environment/loader_spec.rb | 4 +-- 8 files changed, 63 insertions(+), 36 deletions(-) create mode 100644 lib/nenv/environment/dumper/default.rb create mode 100644 lib/nenv/environment/loader/default.rb create mode 100644 lib/nenv/environment/loader/predicate.rb diff --git a/lib/nenv/environment.rb b/lib/nenv/environment.rb index 3b1961d..63c3399 100644 --- a/lib/nenv/environment.rb +++ b/lib/nenv/environment.rb @@ -58,8 +58,8 @@ def _create_env_writer(klass, meth, &block) dumper = nil klass.send(:define_method, meth) do |raw_value| env_name ||= _namespaced_sanitize(meth) - dumper ||= Dumper.new(&block) - ENV[env_name] = dumper.dump(raw_value) + dumper ||= Dumper.setup(&block) + ENV[env_name] = dumper.(raw_value) end end @@ -68,8 +68,8 @@ def _create_env_reader(klass, meth, &block) loader = nil klass.send(:define_method, meth) do env_name ||= _namespaced_sanitize(meth) - loader ||= Loader.new(meth, &block) - loader.load(ENV[env_name]) + loader ||= Loader.setup(meth, &block) + loader.(ENV[env_name]) end end diff --git a/lib/nenv/environment/dumper.rb b/lib/nenv/environment/dumper.rb index f5b0561..c83b11f 100644 --- a/lib/nenv/environment/dumper.rb +++ b/lib/nenv/environment/dumper.rb @@ -1,13 +1,14 @@ module Nenv class Environment - class Dumper - def initialize(&callback) - @callback = callback - end + module Dumper + require 'nenv/environment/dumper/default' - def dump(raw_value) - return @callback.call(raw_value) if @callback - raw_value.nil? ? nil : raw_value.to_s + def self.setup(&callback) + if callback + callback + else + Default + end end end end diff --git a/lib/nenv/environment/dumper/default.rb b/lib/nenv/environment/dumper/default.rb new file mode 100644 index 0000000..368b7bc --- /dev/null +++ b/lib/nenv/environment/dumper/default.rb @@ -0,0 +1,9 @@ +module Nenv + class Environment + module Dumper::Default + def self.call(raw_value) + raw_value.nil? ? nil : raw_value.to_s + end + end + end +end diff --git a/lib/nenv/environment/loader.rb b/lib/nenv/environment/loader.rb index 26d9aa5..9d688e7 100644 --- a/lib/nenv/environment/loader.rb +++ b/lib/nenv/environment/loader.rb @@ -1,28 +1,18 @@ module Nenv class Environment - class Loader - def initialize(meth, &callback) - @bool = meth.to_s.end_with?('?') - @callback = callback - end - - def load(raw_value, &callback) - return @callback.call(raw_value) if @callback - @bool ? _to_bool(raw_value) : raw_value - end - - private + module Loader + require 'nenv/environment/loader/predicate' + require 'nenv/environment/loader/default' - def _to_bool(raw_value) - case raw_value - when nil - nil - when '' - fail ArgumentError, "Can't convert empty string into Bool" - when '0', 'false', 'n', 'no', 'NO', 'FALSE' - false + def self.setup(meth, &callback) + if callback + callback else - true + if meth.to_s.end_with? '?' + Predicate + else + Default + end end end end diff --git a/lib/nenv/environment/loader/default.rb b/lib/nenv/environment/loader/default.rb new file mode 100644 index 0000000..9325f56 --- /dev/null +++ b/lib/nenv/environment/loader/default.rb @@ -0,0 +1,9 @@ +module Nenv + class Environment + module Loader::Default + def self.call(raw_value) + raw_value + end + end + end +end diff --git a/lib/nenv/environment/loader/predicate.rb b/lib/nenv/environment/loader/predicate.rb new file mode 100644 index 0000000..28b5531 --- /dev/null +++ b/lib/nenv/environment/loader/predicate.rb @@ -0,0 +1,18 @@ +module Nenv + class Environment + module Loader::Predicate + def self.call(raw_value) + case raw_value + when nil + nil + when '' + fail ArgumentError, "Can't convert empty string into Bool" + when '0', 'false', 'n', 'no', 'NO', 'FALSE' + false + else + true + end + end + end + end +end diff --git a/spec/lib/nenv/environment/dumper_spec.rb b/spec/lib/nenv/environment/dumper_spec.rb index 75b647a..4de927f 100644 --- a/spec/lib/nenv/environment/dumper_spec.rb +++ b/spec/lib/nenv/environment/dumper_spec.rb @@ -3,7 +3,7 @@ require 'nenv/environment/dumper' RSpec.describe Nenv::Environment::Dumper do - subject { described_class.new.dump(value) } + subject { described_class.setup.(value) } context "with \"abc\"" do let(:value) { 'abc' } @@ -22,7 +22,7 @@ context 'with a block' do subject do - described_class.new { |data| YAML.dump(data) }.dump(value) + described_class.setup { |data| YAML.dump(data) }.(value) end context 'with a yaml string' do diff --git a/spec/lib/nenv/environment/loader_spec.rb b/spec/lib/nenv/environment/loader_spec.rb index d457601..7583da8 100644 --- a/spec/lib/nenv/environment/loader_spec.rb +++ b/spec/lib/nenv/environment/loader_spec.rb @@ -3,7 +3,7 @@ RSpec.describe Nenv::Environment::Loader do context 'with no block' do - subject { described_class.new(meth).load(value) } + subject { described_class.setup(meth).(value) } context 'with a normal method' do let(:meth) { :foo } @@ -49,7 +49,7 @@ context 'with a block' do subject do - described_class.new(:foo) { |data| YAML.load(data) }.load(value) + described_class.setup(:foo) { |data| YAML.load(data) }.(value) end context 'with a yaml string' do let(:value) { "--- foo\n...\n" } From ecdf57ab39729aa371dcdfbfd1bf47a83cd0d2dd Mon Sep 17 00:00:00 2001 From: Vladimir Kochnev Date: Sun, 7 Feb 2016 05:09:32 +0300 Subject: [PATCH 6/6] Make MockEnv#[]= more compatible with ENV. --- spec/lib/nenv/environment_spec.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/spec/lib/nenv/environment_spec.rb b/spec/lib/nenv/environment_spec.rb index e1c3e46..75f17b1 100644 --- a/spec/lib/nenv/environment_spec.rb +++ b/spec/lib/nenv/environment_spec.rb @@ -5,7 +5,9 @@ RSpec.describe Nenv::Environment do class MockEnv < Hash # a hash is close enough def []=(k, v) - super(k.to_s, v.to_s) + fail TypeError, "no implicit conversion of #{k.class} into String" unless k.respond_to? :to_str + fail TypeError, "no implicit conversion of #{v.class} into String" unless v.respond_to? :to_str + super(k.to_str, v.to_str) end end @@ -73,7 +75,7 @@ def []=(k, v) context 'with no block' do before { instance.create_method(:foo) } - let(:value) { 123 } + let(:value) { '123' } it 'returns marshalled stored value' do expect(subject.foo).to eq '123' @@ -150,7 +152,7 @@ def []=(k, v) before { instance.create_method(:foo_baz) } it 'reads the correct variable' do - ENV['BAR_FOO_BAZ'] = 123 + ENV['BAR_FOO_BAZ'] = '123' expect(subject.foo_baz).to eq '123' end end