diff options
author | Jay Mundrawala <jdmundrawala@gmail.com> | 2014-10-24 07:34:35 -0700 |
---|---|---|
committer | Jay Mundrawala <jdmundrawala@gmail.com> | 2014-10-26 16:22:00 -0700 |
commit | 0fa60bbec69600750f421e0c0bb2c5033ab05224 (patch) | |
tree | b29bd0927c1c5b8a74d00bd40e16ae6f022d159d | |
parent | 8d21ba34f272f98fcb387d0306b6dbd8b0a13e64 (diff) | |
download | chef-0fa60bbec69600750f421e0c0bb2c5033ab05224.tar.gz |
Merge pull request #2252 from opscode/jdmundrawala/env-splitjdmundrawala/11-env-split
Modified env resource to break values up by delimiter before comparing
-rw-r--r-- | lib/chef/provider/env.rb | 35 | ||||
-rw-r--r-- | spec/unit/provider/env/windows_spec.rb | 4 | ||||
-rw-r--r-- | spec/unit/provider/env_spec.rb | 87 |
3 files changed, 103 insertions, 23 deletions
diff --git a/lib/chef/provider/env.rb b/lib/chef/provider/env.rb index e857d74d68..600cac1e6b 100644 --- a/lib/chef/provider/env.rb +++ b/lib/chef/provider/env.rb @@ -58,20 +58,22 @@ class Chef # ==== Returns # <true>:: If a change is required # <false>:: If a change is not required - def compare_value + def requires_modify_or_create? if @new_resource.delim #e.g. check for existing value within PATH - not @current_resource.value.split(@new_resource.delim).any? do |val| - val == @new_resource.value + not new_values.all? do |val| + current_values.include? val end else @new_resource.value != @current_resource.value end end + alias_method :compare_value, :requires_modify_or_create? + def action_create if @key_exists - if compare_value + if requires_modify_or_create? modify_env Chef::Log.info("#{@new_resource} altered") @new_resource.updated_by_last_action(true) @@ -91,13 +93,14 @@ class Chef # after we removed the element. def delete_element return false unless @new_resource.delim #no delim: delete the key - if compare_value + needs_delete = new_values.any? { |v| current_values.include?(v) } + if !needs_delete Chef::Log.debug("#{@new_resource} element '#{@new_resource.value}' does not exist") return true #do not delete the key else new_value = - @current_resource.value.split(@new_resource.delim).select { |item| - item != @new_resource.value + current_values.select { |item| + not new_values.include?(item) }.join(@new_resource.delim) if new_value.empty? @@ -122,7 +125,7 @@ class Chef def action_modify if @key_exists - if compare_value + if requires_modify_or_create? modify_env Chef::Log.info("#{@new_resource} modified") @new_resource.updated_by_last_action(true) @@ -142,11 +145,23 @@ class Chef def modify_env if @new_resource.delim - #e.g. add to PATH - @new_resource.value << @new_resource.delim << @current_resource.value + values = new_values.reject do |v| + current_values.include?(v) + end + @new_resource.value((values + [@current_resource.value]).join(@new_resource.delim)) end create_env end + + # Returns the current values to split by delimiter + def current_values + @current_values ||= @current_resource.value.split(@new_resource.delim) + end + + # Returns the new values to split by delimiter + def new_values + @new_values ||= @new_resource.value.split(@new_resource.delim) + end end end end diff --git a/spec/unit/provider/env/windows_spec.rb b/spec/unit/provider/env/windows_spec.rb index 2ea137c1d9..84582d8b4d 100644 --- a/spec/unit/provider/env/windows_spec.rb +++ b/spec/unit/provider/env/windows_spec.rb @@ -53,7 +53,7 @@ describe Chef::Provider::Env::Windows, :windows_only do end it "should update the ruby ENV object when it updates the value" do - provider.should_receive(:compare_value).and_return(true) + provider.should_receive(:requires_modify_or_create?).and_return(true) new_resource.value("foobar") provider.action_modify expect(ENV['CHEF_WINDOWS_ENV_TEST']).to eql('foobar') @@ -92,7 +92,7 @@ describe Chef::Provider::Env::Windows, :windows_only do end it "replaces Windows system variables" do - provider.should_receive(:compare_value).and_return(true) + provider.should_receive(:requires_modify_or_create?).and_return(true) provider.should_receive(:expand_path).with(system_root).and_return(system_root_value) provider.action_modify expect(ENV['PATH']).to eql(system_root_value) diff --git a/spec/unit/provider/env_spec.rb b/spec/unit/provider/env_spec.rb index 0bc5117e48..f8803f9bb6 100644 --- a/spec/unit/provider/env_spec.rb +++ b/spec/unit/provider/env_spec.rb @@ -88,20 +88,20 @@ describe Chef::Provider::Env do it "should check to see if the values are the same if the key exists" do @provider.key_exists = true - @provider.should_receive(:compare_value).and_return(false) + @provider.should_receive(:requires_modify_or_create?).and_return(false) @provider.action_create end it "should call modify_env if the key exists and values are not equal" do @provider.key_exists = true - @provider.stub(:compare_value).and_return(true) + @provider.stub(:requires_modify_or_create?).and_return(true) @provider.should_receive(:modify_env).and_return(true) @provider.action_create end it "should set the new_resources updated flag when it updates an existing value" do @provider.key_exists = true - @provider.stub(:compare_value).and_return(true) + @provider.stub(:requires_modify_or_create?).and_return(true) @provider.stub(:modify_env).and_return(true) @provider.action_create @new_resource.should be_updated @@ -147,20 +147,20 @@ describe Chef::Provider::Env do end it "should call modify_group if the key exists and values are not equal" do - @provider.should_receive(:compare_value).and_return(true) + @provider.should_receive(:requires_modify_or_create?).and_return(true) @provider.should_receive(:modify_env).and_return(true) @provider.action_modify end it "should set the new resources updated flag to true if modify_env is called" do - @provider.stub(:compare_value).and_return(true) + @provider.stub(:requires_modify_or_create?).and_return(true) @provider.stub(:modify_env).and_return(true) @provider.action_modify @new_resource.should be_updated end it "should not call modify_env if the key exists but the values are equal" do - @provider.should_receive(:compare_value).and_return(false) + @provider.should_receive(:requires_modify_or_create?).and_return(false) @provider.should_not_receive(:modify_env) @provider.action_modify end @@ -198,9 +198,31 @@ describe Chef::Provider::Env do @provider.delete_element.should eql(true) @new_resource.should be_updated end + + context "when new_resource's value contains the delimiter" do + it "should return false if all the elements are deleted" do + # This indicates that the entire key needs to be deleted + @new_resource.value("C:/foo/bin;C:/bar/bin") + @provider.delete_element.should eql(false) + @new_resource.should_not be_updated # This will be updated in action_delete + end + + it "should return true if any, but not all, of the elements are deleted" do + @new_resource.value("C:/foo/bin;C:/notbaz/bin") + @provider.should_receive(:create_env) + @provider.delete_element.should eql(true) + @new_resource.should be_updated + end + + it "should return true if none of the elements are deleted" do + @new_resource.value("C:/notfoo/bin;C:/notbaz/bin") + @provider.delete_element.should eql(true) + @new_resource.should_not be_updated + end + end end - describe "compare_value" do + describe "requires_modify_or_create?" do before(:each) do @new_resource.value("C:/bar") @current_resource = @new_resource.clone @@ -208,25 +230,68 @@ describe Chef::Provider::Env do end it "should return false if the values are equal" do - @provider.compare_value.should be_false + @provider.requires_modify_or_create?.should be_false end it "should return true if the values not are equal" do @new_resource.value("C:/elsewhere") - @provider.compare_value.should be_true + @provider.requires_modify_or_create?.should be_true end it "should return false if the current value contains the element" do @new_resource.delim(";") @current_resource.value("C:/bar;C:/foo;C:/baz") - @provider.compare_value.should be_false + @provider.requires_modify_or_create?.should be_false end it "should return true if the current value does not contain the element" do @new_resource.delim(";") @current_resource.value("C:/biz;C:/foo/bin;C:/baz") - @provider.compare_value.should be_true + @provider.requires_modify_or_create?.should be_true + end + + context "when new_resource's value contains the delimiter" do + it "should return false if all the current values are contained" do + @new_resource.value("C:/biz;C:/baz") + @new_resource.delim(";") + @current_resource.value("C:/biz;C:/foo/bin;C:/baz") + @provider.requires_modify_or_create?.should be_false + end + + it "should return true if any of the new values are not contained" do + @new_resource.value("C:/biz;C:/baz;C:/bin") + @new_resource.delim(";") + @current_resource.value("C:/biz;C:/foo/bin;C:/baz") + @provider.requires_modify_or_create?.should be_true + end + end + end + + describe "modify_env" do + before(:each) do + @provider.stub(:create_env).and_return(true) + @new_resource.delim ";" + + @current_resource = Chef::Resource::Env.new("FOO") + @current_resource.value "C:/foo/bin" + @provider.current_resource = @current_resource + end + + it "should not modify the variable passed to the resource" do + new_value = "C:/bar/bin" + passed_value = new_value.dup + @new_resource.value(passed_value) + @provider.modify_env + passed_value.should == new_value + end + + it "should only add values not already contained when a delimiter is provided" do + @new_resource.value("C:/foo;C:/bar;C:/baz") + @new_resource.delim(";") + @current_resource.value("C:/foo/bar;C:/bar;C:/baz") + @provider.modify_env + @new_resource.value.should eq("C:/foo;C:/foo/bar;C:/bar;C:/baz") end end end |