(#5427) Using Propery::OrderedList for host_alias

This uses the propertyclass Puppet::Property::OrderedList to represent
the list of host_aliases. This lets us remove the in_sync, should_to_s
etc overrides.

In the provider class the list is represented by a string (=no array)
so there were a few changes necessary as well.

Because Puppet::Property::List uses the specified delimiter when
converting should values to strings, I changed the delimiter to a simple
space instead a tab. This keeps messages produced by puppet in a nice
format.

The tests had to be changed to work with the new behaviour of
host_aliases. There are a few additional tests as well.

2 files changed, 68 insertions, 13 deletions

diff --git a/spec/unit/provider/host/parsed_spec.rb b/spec/unit/provider/host/parsed_spec.rb
index 3e00a2190..5704304ba 100644
--- a/spec/unit/provider/host/parsed_spec.rb
+++ b/spec/unit/provider/host/parsed_spec.rb
@@ -28,9 +28,13 @@ describe provider_class do
     hostresource = Puppet::Type::Host.new(:name => args[:name])
     hostresource.stubs(:should).with(:target).returns @hostfile
 
-    # Using setters of provider
+    # Using setters of provider to build our testobject
+    # Note: We already proved, that in case of host_aliases
+    # the provider setter "host_aliases=(value)" will be
+    # called with the joined array, so we just simulate that
     host = @provider.new(hostresource)
     args.each do |property,value|
+      value = value.join(" ") if property == :host_aliases and value.is_a?(Array)
       host.send("#{property}=", value)
     end
     host
@@ -63,6 +67,10 @@ describe provider_class do
       @provider.parse_line("::1     localhost")[:comment].should == ""
     end
 
+    it "should set host_aliases to :absent" do
+      @provider.parse_line("::1     localhost")[:host_aliases].should == :absent
+    end
+
   end
 
   describe "when parsing a line with ip, hostname and comment" do
@@ -87,13 +95,13 @@ describe provider_class do
   describe "when parsing a line with ip, hostname and aliases" do
 
     it "should parse alias from the third field" do
-      @provider.parse_line("127.0.0.1   localhost   localhost.localdomain")[:host_aliases].should == ["localhost.localdomain"]
+      @provider.parse_line("127.0.0.1   localhost   localhost.localdomain")[:host_aliases].should == "localhost.localdomain"
     end
 
     it "should parse multiple aliases" do
-      @provider.parse_line("127.0.0.1 host alias1 alias2")[:host_aliases].should == ['alias1', 'alias2']
-      @provider.parse_line("127.0.0.1 host alias1\talias2")[:host_aliases].should == ['alias1', 'alias2']
-      @provider.parse_line("127.0.0.1 host alias1\talias2   alias3")[:host_aliases].should == ['alias1', 'alias2', 'alias3']
+      @provider.parse_line("127.0.0.1 host alias1 alias2")[:host_aliases].should == 'alias1 alias2'
+      @provider.parse_line("127.0.0.1 host alias1\talias2")[:host_aliases].should == 'alias1 alias2'
+      @provider.parse_line("127.0.0.1 host alias1\talias2   alias3")[:host_aliases].should == 'alias1 alias2 alias3'
     end
 
   end
@@ -114,7 +122,7 @@ describe provider_class do
     end
 
     it "should parse all host_aliases from the third field" do
-      @provider.parse_line(@testline)[:host_aliases].should == ['alias1' ,'alias2', 'alias3']
+      @provider.parse_line(@testline)[:host_aliases].should == 'alias1 alias2 alias3'
     end
 
     it "should parse the comment after the first '#' character" do
@@ -143,7 +151,7 @@ describe provider_class do
       host = mkhost(
         :name   => 'localhost.localdomain',
         :ip     => '127.0.0.1',
-        :host_aliases => ['localhost'],
+        :host_aliases => 'localhost',
         :ensure => :present
       )
       genhost(host).should == "127.0.0.1\tlocalhost.localdomain\tlocalhost\n"
@@ -156,7 +164,7 @@ describe provider_class do
         :host_aliases => [ 'a1','a2','a3','a4' ],
         :ensure     => :present
       )
-      genhost(host).should == "192.0.0.1\thost\ta1\ta2\ta3\ta4\n"
+      genhost(host).should == "192.0.0.1\thost\ta1 a2 a3 a4\n"
     end
 
     it "should be able to generate a simple hostfile entry with comments" do
@@ -173,7 +181,7 @@ describe provider_class do
       host = mkhost(
         :name   => 'localhost.localdomain',
         :ip     => '127.0.0.1',
-        :host_aliases => ['localhost'],
+        :host_aliases => 'localhost',
         :comment => 'Bazinga!',
         :ensure => :present
       )
@@ -188,7 +196,7 @@ describe provider_class do
         :comment      => 'Bazinga!',
         :ensure       => :present
       )
-      genhost(host).should == "192.0.0.1\thost\ta1\ta2\ta3\ta4\t# Bazinga!\n"
+      genhost(host).should == "192.0.0.1\thost\ta1 a2 a3 a4\t# Bazinga!\n"
     end
 
   end
diff --git a/spec/unit/type/host_spec.rb b/spec/unit/type/host_spec.rb
index 36ef460c1..60ce73c6d 100755
--- a/spec/unit/type/host_spec.rb
+++ b/spec/unit/type/host_spec.rb
@@ -2,12 +2,14 @@
 
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper')
 
-ssh_authorized_key = Puppet::Type.type(:ssh_authorized_key)
+host = Puppet::Type.type(:host)
 
-describe Puppet::Type.type(:host) do
+describe host do
   before do
-    @class = Puppet::Type.type(:host)
+    @class = host
     @catalog = Puppet::Resource::Catalog.new
+    @provider = stub 'provider'
+    @resource = stub 'resource', :resource => nil, :provider => @provider
   end
 
   it "should have :name be its namevar" do
@@ -26,6 +28,11 @@ describe Puppet::Type.type(:host) do
         @class.attrtype(property).should == :property
       end
     end
+
+    it "should have a list host_aliases" do
+      @class.attrclass(:host_aliases).ancestors.should be_include(Puppet::Property::OrderedList)
+    end
+
   end
 
   describe "when validating values" do
@@ -80,4 +87,44 @@ describe Puppet::Type.type(:host) do
       proc { @class.new(:name => "foo", :host_aliases => ['alias1','']) }.should raise_error
     end
   end
+
+  describe "when syncing" do
+
+    it "should send the first value to the provider for ip property" do
+      @ip = @class.attrclass(:ip).new(:resource => @resource, :should => %w{192.168.0.1 192.168.0.2})
+      @provider.expects(:ip=).with '192.168.0.1'
+      @ip.sync
+    end
+
+    it "should send the first value to the provider for comment property" do
+      @comment = @class.attrclass(:comment).new(:resource => @resource, :should => %w{Bazinga Notme})
+      @provider.expects(:comment=).with 'Bazinga'
+      @comment.sync
+    end
+
+    it "should send the joined array to the provider for host_alias" do
+      @host_aliases = @class.attrclass(:host_aliases).new(:resource => @resource, :should => %w{foo bar})
+      @provider.expects(:host_aliases=).with 'foo bar'
+      @host_aliases.sync
+    end
+
+    it "should also use the specified delimiter for joining" do
+      @host_aliases = @class.attrclass(:host_aliases).new(:resource => @resource, :should => %w{foo bar})
+      @host_aliases.stubs(:delimiter).returns "\t"
+      @provider.expects(:host_aliases=).with "foo\tbar"
+      @host_aliases.sync
+    end
+
+    it "should care about the order of host_aliases" do
+      @host_aliases = @class.attrclass(:host_aliases).new(:resource => @resource, :should => %w{foo bar})
+      @host_aliases.insync?(%w{foo bar}).should == true
+      @host_aliases.insync?(%w{bar foo}).should == false
+    end
+
+    it "should not consider aliases to be in sync if should is a subset of current" do
+      @host_aliases = @class.attrclass(:host_aliases).new(:resource => @resource, :should => %w{foo bar})
+      @host_aliases.insync?(%w{foo bar anotherone}).should == false
+    end
+
+  end
 end