Bug #1778 - Solaris RBAC profiles should maintain order

Created OrderedList property
Added to profile property
small refactor in List to make inheriting easier

6 files changed, 120 insertions, 3 deletions

diff --git a/lib/puppet/property/list.rb b/lib/puppet/property/list.rb
index 4e7f6ec90..0c933f164 100644
--- a/lib/puppet/property/list.rb
+++ b/lib/puppet/property/list.rb
@@ -28,6 +28,11 @@ module Puppet
                 @resource[membership] == :inclusive
             end
 
+            #dearrayify was motivated because to simplify the implementation of the OrderedList property
+            def dearrayify(array)
+                array.sort.join(delimiter)
+            end
+
             def should
                 unless defined? @should and @should
                     return nil
@@ -39,7 +44,7 @@ module Puppet
                     members = add_should_with_current(members, retrieve)
                 end
 
-                members.sort.join(delimiter)
+                dearrayify(members)
             end
 
             def delimiter
@@ -57,7 +62,7 @@ module Puppet
 
             def prepare_is_for_comparison(is)
                 if is.is_a? Array
-                    is = is.sort.join(delimiter)
+                    is = dearrayify(is)
                 end
                 is
             end
diff --git a/lib/puppet/property/ordered_list.rb b/lib/puppet/property/ordered_list.rb
new file mode 100644
index 000000000..816b16c48
--- /dev/null
+++ b/lib/puppet/property/ordered_list.rb
@@ -0,0 +1,22 @@
+require 'puppet/property/list'
+
+module Puppet
+    class Property
+        class OrderedList < List
+
+            def add_should_with_current(should, current)
+                if current.is_a?(Array)
+                    #tricky trick
+                    #Preserve all the current items in the list
+                    #but move them to the back of the line
+                    should = should + (current - should)
+                end
+                should
+            end
+
+            def dearrayify(array)
+                array.join(delimiter)
+            end
+        end
+    end
+end
diff --git a/lib/puppet/type/user.rb b/lib/puppet/type/user.rb
index 0fe7928e6..c6f1eccf6 100755
--- a/lib/puppet/type/user.rb
+++ b/lib/puppet/type/user.rb
@@ -1,6 +1,7 @@
 require 'etc'
 require 'facter'
 require 'puppet/property/list'
+require 'puppet/property/ordered_list'
 require 'puppet/property/keyvalue'
 
 module Puppet
@@ -316,7 +317,7 @@ module Puppet
             defaultto :minimum
         end
 
-        newproperty(:profiles, :parent => Puppet::Property::List, :required_features => :manages_solaris_rbac) do
+        newproperty(:profiles, :parent => Puppet::Property::OrderedList, :required_features => :manages_solaris_rbac) do
             desc "The profiles the user has.  Multiple profiles should be
                 specified as an array."
 
diff --git a/spec/unit/property/list.rb b/spec/unit/property/list.rb
index 9c832c0cd..2fab868db 100644
--- a/spec/unit/property/list.rb
+++ b/spec/unit/property/list.rb
@@ -143,5 +143,14 @@ describe list_class do
                 @property.insync?(["bar","foo"]).must == false
             end
         end
+
+        describe "when calling dearrayify" do
+            it "should sort and join the array with 'delimiter'" do
+                array = mock "array"
+                array.expects(:sort).returns(array)
+                array.expects(:join).with(@property.delimiter)
+                @property.dearrayify(array)
+            end
+        end
     end
 end
diff --git a/spec/unit/property/ordered_list.rb b/spec/unit/property/ordered_list.rb
new file mode 100644
index 000000000..51c59a7dd
--- /dev/null
+++ b/spec/unit/property/ordered_list.rb
@@ -0,0 +1,64 @@
+#!/usr/bin/env ruby
+
+Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
+
+require 'puppet/property/ordered_list'
+
+ordered_list_class = Puppet::Property::OrderedList
+
+describe ordered_list_class do
+
+    it "should be a subclass of List" do
+        ordered_list_class.superclass.must == Puppet::Property::List
+    end
+
+    describe "as an instance" do
+        before do
+            # Wow that's a messy interface to the resource.
+            ordered_list_class.initvars
+            @resource = stub 'resource', :[]= => nil, :property => nil
+            @property = ordered_list_class.new(:resource => @resource)
+        end
+
+        describe "when adding should to current" do
+            it "should add the arrays when current is an array" do
+                @property.add_should_with_current(["should"], ["current"]).should == ["should", "current"]
+            end
+
+            it "should return 'should' if current is not a array" do
+                @property.add_should_with_current(["should"], :absent).should == ["should"]
+            end
+
+            it "should return only the uniq elements leading with the order of 'should'" do
+                @property.add_should_with_current(["this", "is", "should"], ["is", "this", "current"]).should == ["this", "is", "should", "current"]
+            end
+        end
+
+        describe "when calling should" do
+            it "should return nil if @should is nil" do
+                @property.should.must == nil
+            end
+
+            it "should return the values of @should (without sorting) as a string if inclusive" do
+                @property.should = ["foo", "bar"]
+                @property.expects(:inclusive?).returns(true)
+                @property.should.must == "foo,bar"
+            end
+
+            it "should return the uniq values of @should + retrieve as a string if !inclusive with the @ values leading" do
+                @property.should = ["foo", "bar"]
+                @property.expects(:inclusive?).returns(false)
+                @property.expects(:retrieve).returns(["foo","baz"])
+                @property.should.must == "foo,bar,baz"
+            end
+        end
+
+        describe "when calling dearrayify" do
+            it "should join the array with the delimiter" do
+                array = mock "array"
+                array.expects(:join).with(@property.delimiter)
+                @property.dearrayify(array)
+            end
+        end
+    end
+end
diff --git a/spec/unit/type/user.rb b/spec/unit/type/user.rb
index 19690eee1..6f01ab35a 100755
--- a/spec/unit/type/user.rb
+++ b/spec/unit/type/user.rb
@@ -54,6 +54,22 @@ describe user do
         end
     end
 
+    list_properties = [:groups, :roles, :auths]
+
+    list_properties.each do |property|
+        it "should have a list '%s'" % property do
+            user.attrclass(property).ancestors.should be_include(Puppet::Property::List)
+        end
+    end
+
+    it "should have an ordered list 'profiles'" do
+        user.attrclass(:profiles).ancestors.should be_include(Puppet::Property::OrderedList)
+    end
+
+    it "should have key values 'keys'" do
+        user.attrclass(:keys).ancestors.should be_include(Puppet::Property::KeyValue)
+    end
+
     describe "when retrieving all current values" do
         before do
             @user = user.create(:name => "foo", :uid => 10, :gid => 10)