summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJay Mundrawala <jdmundrawala@gmail.com>2014-10-24 07:34:35 -0700
committerJay Mundrawala <jdmundrawala@gmail.com>2014-10-24 07:34:35 -0700
commit9d02186475f02d3cd29e6a0c2b4585d3a88f8c4f (patch)
treeb7b529a708958b09c9c5fb44bd87c782f72e7399
parent34ad58fb936b293d31476fddcece658f1f23b179 (diff)
parent421cabc9d82bfddeb1b18660f03e7de2be676272 (diff)
downloadchef-9d02186475f02d3cd29e6a0c2b4585d3a88f8c4f.tar.gz
Merge pull request #2252 from opscode/jdmundrawala/env-split
Modified env resource to break values up by delimiter before comparing
-rw-r--r--lib/chef/provider/env.rb35
-rw-r--r--spec/unit/provider/env/windows_spec.rb4
-rw-r--r--spec/unit/provider/env_spec.rb68
3 files changed, 84 insertions, 23 deletions
diff --git a/lib/chef/provider/env.rb b/lib/chef/provider/env.rb
index e91b8bbb97..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.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 dc6176d45c..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,41 @@ 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
@@ -247,5 +285,13 @@ describe Chef::Provider::Env do
@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