Should I also submit this patch against the master branch, or should it just apply cleanly since master is derived from 0.24.x ?
-Jeff On Tue, Jan 6, 2009 at 2:46 PM, Luke Kanies <[email protected]> wrote: > > +1 > > On Jan 6, 2009, at 3:16 PM, Jeffrey McCune wrote: > > > > > Moved parse_type and parse_name from the provider into a module. > > Mixed the module into both the MCX type and mcxcontent provider. > > > > Updated tests and added tests to ensure users, groups and computers > > are automatically required. > > > > Qualified TypeMap constant and moved into the module. > > > > The TypeMap constant is used by both the mcx type and provider > > and therefore should be moved into the mcx utility module > > included in both. Furthermore, references to TypeMap should > > probably be fully qualified to prevent namespace collisions. > > > > Signed-off-by: Jeffrey McCune <[email protected]> > > --- > > lib/puppet/provider/mcx/mcxcontent.rb | 54 ++++--------------- > > lib/puppet/type/mcx.rb | 10 ++++ > > lib/puppet/util/mcx.rb | 62 +++++++++++++++++++++ > > spec/unit/provider/mcx/mcxcontent.rb | 24 ++++---- > > spec/unit/type/mcx.rb | 97 ++++++++++++++++++++++++ > > +++++++++ > > 5 files changed, 192 insertions(+), 55 deletions(-) > > create mode 100644 lib/puppet/util/mcx.rb > > > > diff --git a/lib/puppet/provider/mcx/mcxcontent.rb b/lib/puppet/ > > provider/mcx/mcxcontent.rb > > index 1fea60c..e6c202a 100644 > > --- a/lib/puppet/provider/mcx/mcxcontent.rb > > +++ b/lib/puppet/provider/mcx/mcxcontent.rb > > @@ -18,9 +18,12 @@ > > # Author: Jeff McCune <[email protected]> > > > > require 'tempfile' > > +require 'puppet/util/mcx' > > > > Puppet::Type.type(:mcx).provide :mcxcontent, :parent => > > Puppet::Provider do > > > > + include Puppet::Util::Mcx > > + > > desc "MCX Settings management using DirectoryService on OS X. > > > > This provider manages the entire MCXSettings attribute available > > @@ -36,15 +39,6 @@ > > Puppet::Type.type(:mcx).provide :mcxcontent, :parent => > > Puppet::Provider do > > > > " > > > > - # This provides a mapping of puppet types to DirectoryService > > - # type strings. > > - TypeMap = { > > - :user => "Users", > > - :group => "Groups", > > - :computer => "Computers", > > - :computerlist => "ComputerLists", > > - } > > - > > class MCXContentProviderException < Exception > > > > end > > @@ -58,17 +52,17 @@ > > Puppet::Type.type(:mcx).provide :mcxcontent, :parent => > > Puppet::Provider do > > # an array of instances of this class. > > def self.instances > > mcx_list = [] > > - for ds_type in TypeMap.keys > > - ds_path = "/Local/Default/#{TypeMap[ds_type]}" > > + for ds_type in Puppet::Util::Mcx::TypeMap.keys > > + ds_path = "/Local/Default/ > > #{Puppet::Util::Mcx::TypeMap[ds_type]}" > > output = dscl 'localhost', '-list', ds_path > > member_list = output.split > > for ds_name in member_list > > content = mcxexport(ds_type, ds_name) > > if content.empty? > > - Puppet.debug "/#{TypeMap[ds_type]}/#{ds_name} > > has no MCX data." > > + Puppet.debug "/ > > #{Puppet::Util::Mcx::TypeMap[ds_type]}/#{ds_name} has no MCX data." > > else > > # This node has MCX data. > > - rsrc = self.new(:name => "/#{TypeMap[ds_type]}/ > > #{ds_name}", > > + rsrc = self.new(:name => "/ > > #{Puppet::Util::Mcx::TypeMap[ds_type]}/#{ds_name}", > > :ds_type => ds_type, > > :ds_name => ds_name, > > :content => content) > > @@ -84,14 +78,14 @@ > > Puppet::Type.type(:mcx).provide :mcxcontent, :parent => > > Puppet::Provider do > > # mcxexport is used by instances, and therefore > > # a class method. > > def self.mcxexport(ds_type, ds_name) > > - ds_t = TypeMap[ds_type] > > + ds_t = Puppet::Util::Mcx::TypeMap[ds_type] > > ds_n = ds_name.to_s > > ds_path = "/Local/Default/#{ds_t}/#{ds_n}" > > dscl 'localhost', '-mcxexport', ds_path > > end > > > > def mcximport(ds_type, ds_name, val) > > - ds_t = TypeMap[ds_type] > > + ds_t = Puppet::Util::Mcx::TypeMap[ds_type] > > ds_n = ds_name.to_s > > ds_path = "/Local/Default/#{ds_t}/#{ds_name}" > > > > @@ -106,32 +100,6 @@ > > Puppet::Type.type(:mcx).provide :mcxcontent, :parent => > > Puppet::Provider do > > end > > end > > > > - # Given the resource name string, parse ds_type out. > > - def parse_type(name) > > - tmp = name.split('/')[1] > > - if ! tmp.is_a? String > > - raise MCXContentProviderException, > > - "Coult not parse ds_type from resource name '#{name}'. > > Specify with ds_type parameter." > > - end > > - # De-pluralize and downcase. > > - tmp = tmp.chop.downcase.to_sym > > - if not TypeMap.keys.member? tmp > > - raise MCXContentProviderException, > > - "Coult not parse ds_type from resource name '#{name}'. > > Specify with ds_type parameter." > > - end > > - return tmp > > - end > > - > > - # Given the resource name string, parse ds_name out. > > - def parse_name(name) > > - ds_name = name.split('/')[2] > > - if ! ds_name.is_a? String > > - raise MCXContentProviderException, > > - "Could not parse ds_name from resource name '#{name}'. > > Specify with ds_name parameter." > > - end > > - return ds_name > > - end > > - > > # Gather ds_type and ds_name from resource or > > # parse it out of the name. > > # This is a private instance method, not a class method. > > @@ -140,7 +108,7 @@ > > Puppet::Type.type(:mcx).provide :mcxcontent, :parent => > > Puppet::Provider do > > if ds_type.nil? > > ds_type = parse_type(resource[:name]) > > end > > - raise MCXContentProviderException unless > > TypeMap.keys.include? ds_type.to_sym > > + raise MCXContentProviderException unless > > Puppet::Util::Mcx::TypeMap.keys.include? ds_type.to_sym > > > > ds_name = resource[:ds_name] > > if ds_name.nil? > > @@ -164,7 +132,7 @@ > > Puppet::Type.type(:mcx).provide :mcxcontent, :parent => > > Puppet::Provider do > > > > def destroy > > ds_parms = get_dsparams > > - ds_t = TypeMap[ds_parms[:ds_type]] > > + ds_t = Puppet::Util::Mcx::TypeMap[ds_parms[:ds_type]] > > ds_n = ds_parms[:ds_name].to_s > > ds_path = "/Local/Default/#{ds_t}/#{ds_n}" > > > > diff --git a/lib/puppet/type/mcx.rb b/lib/puppet/type/mcx.rb > > index f92cc46..3612f0d 100644 > > --- a/lib/puppet/type/mcx.rb > > +++ b/lib/puppet/type/mcx.rb > > @@ -17,8 +17,12 @@ > > > > # Author: Jeff McCune <[email protected]> > > > > +require 'puppet/util/mcx' > > + > > Puppet::Type.newtype(:mcx) do > > > > + include Puppet::Util::Mcx > > + > > @doc = "MCX object management using DirectoryService on OS X. > > > > Original Author: Jeff McCune <[email protected]> > > @@ -91,7 +95,13 @@ to other machines. > > # value returns a Symbol > > name = value(:name) > > ds_type = value(:ds_type) > > + if ds_type.nil? > > + ds_type = parse_type(name) > > + end > > ds_name = value(:ds_name) > > + if ds_name.nil? > > + ds_name = parse_name(name) > > + end > > if ds_type == type > > rval = [ ds_name.to_s ] > > else > > diff --git a/lib/puppet/util/mcx.rb b/lib/puppet/util/mcx.rb > > new file mode 100644 > > index 0000000..a057c7f > > --- /dev/null > > +++ b/lib/puppet/util/mcx.rb > > @@ -0,0 +1,62 @@ > > +#-- > > +# Copyright (C) 2008 Jeffrey J McCune. > > + > > +# This program and entire repository is free software; you can > > +# redistribute it and/or modify it under the terms of the GNU > > +# General Public License as published by the Free Software > > +# Foundation; either version 2 of the License, or any later version. > > + > > +# This program is distributed in the hope that it will be useful, > > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > +# GNU General Public License for more details. > > + > > +# You should have received a copy of the GNU General Public License > > +# along with this program; if not, write to the Free Software > > +# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA > > 02110-1301 USA > > + > > +# Author: Jeff McCune <[email protected]> > > + > > +# Helper methods common to the MCX Type and Providers > > +module Puppet::Util::Mcx > > + > > + # This provides a mapping of puppet types to DirectoryService > > + # type strings. > > + TypeMap = { > > + :user => "Users", > > + :group => "Groups", > > + :computer => "Computers", > > + :computerlist => "ComputerLists", > > + } > > + > > + class MCXUtilException < Exception > > + > > + end > > + > > + # Given the resource name string, parse ds_type out. > > + def parse_type(name) > > + tmp = name.split('/')[1] > > + if ! tmp.is_a? String > > + raise MCXUtilException, > > + "Coult not parse ds_type from resource name '#{name}'. > > Specify with ds_type parameter." > > + end > > + # De-pluralize and downcase. > > + tmp = tmp.chop.downcase.to_sym > > + if not Puppet::Util::Mcx::TypeMap.keys.member? tmp > > + raise MCXUtilException, > > + "Coult not parse ds_type from resource name '#{name}'. > > Specify with ds_type parameter." > > + end > > + return tmp > > + end > > + > > + # Given the resource name string, parse ds_name out. > > + def parse_name(name) > > + ds_name = name.split('/')[2] > > + if ! ds_name.is_a? String > > + raise MCXUtilException, > > + "Could not parse ds_name from resource name '#{name}'. > > Specify with ds_name parameter." > > + end > > + return ds_name > > + end > > + > > +end > > diff --git a/spec/unit/provider/mcx/mcxcontent.rb b/spec/unit/ > > provider/mcx/mcxcontent.rb > > index eedff7d..740baaf 100755 > > --- a/spec/unit/provider/mcx/mcxcontent.rb > > +++ b/spec/unit/provider/mcx/mcxcontent.rb > > @@ -98,23 +98,23 @@ describe provider_class do > > @resource.stubs(:[]).with(:name).returns "/Foo/bar" > > end > > it "should not accept /Foo/bar" do > > - lambda { @provider.create }.should > > raise_error(MCXContentProviderException) > > + lambda { @provider.create }.should > > raise_error(Puppet::Util::Mcx::MCXUtilException) > > end > > it "should accept /Foo/bar with ds_type => user" do > > @resource.stubs(:[]).with(:ds_type).returns "user" > > - lambda { @provider.create }.should_not > > raise_error(MCXContentProviderException) > > + lambda { @provider.create }.should_not raise_error > > end > > it "should accept /Foo/bar with ds_type => group" do > > @resource.stubs(:[]).with(:ds_type).returns "group" > > - lambda { @provider.create }.should_not > > raise_error(MCXContentProviderException) > > + lambda { @provider.create }.should_not raise_error > > end > > it "should accept /Foo/bar with ds_type => computer" do > > @resource.stubs(:[]).with(:ds_type).returns "computer" > > - lambda { @provider.create }.should_not > > raise_error(MCXContentProviderException) > > + lambda { @provider.create }.should_not raise_error > > end > > it "should accept :name => /Foo/bar with ds_type => > > computerlist" do > > @resource.stubs(:[]).with(:ds_type).returns "computerlist" > > - lambda { @provider.create }.should_not > > raise_error(MCXContentProviderException) > > + lambda { @provider.create }.should_not raise_error > > end > > end > > > > @@ -123,35 +123,35 @@ describe provider_class do > > @resource.stubs(:[]).with(:name).returns "foobar" > > end > > it "should not accept unspecified :ds_type and :ds_name" do > > - lambda { @provider.create }.should > > raise_error(MCXContentProviderException) > > + lambda { @provider.create }.should > > raise_error(Puppet::Util::Mcx::MCXUtilException) > > end > > it "should not accept unspecified :ds_type" do > > @resource.stubs(:[]).with(:ds_type).returns "user" > > - lambda { @provider.create }.should > > raise_error(MCXContentProviderException) > > + lambda { @provider.create }.should > > raise_error(Puppet::Util::Mcx::MCXUtilException) > > end > > it "should not accept unspecified :ds_name" do > > @resource.stubs(:[]).with(:ds_name).returns "foo" > > - lambda { @provider.create }.should > > raise_error(MCXContentProviderException) > > + lambda { @provider.create }.should > > raise_error(Puppet::Util::Mcx::MCXUtilException) > > end > > it "should accept :ds_type => user, ds_name => foo" do > > @resource.stubs(:[]).with(:ds_type).returns "user" > > @resource.stubs(:[]).with(:ds_name).returns "foo" > > - lambda { @provider.create }.should_not > > raise_error(MCXContentProviderException) > > + lambda { @provider.create }.should_not raise_error > > end > > it "should accept :ds_type => group, ds_name => foo" do > > @resource.stubs(:[]).with(:ds_type).returns "group" > > @resource.stubs(:[]).with(:ds_name).returns "foo" > > - lambda { @provider.create }.should_not > > raise_error(MCXContentProviderException) > > + lambda { @provider.create }.should_not raise_error > > end > > it "should accept :ds_type => computer, ds_name => foo" do > > @resource.stubs(:[]).with(:ds_type).returns "computer" > > @resource.stubs(:[]).with(:ds_name).returns "foo" > > - lambda { @provider.create }.should_not > > raise_error(MCXContentProviderException) > > + lambda { @provider.create }.should_not raise_error > > end > > it "should accept :ds_type => computerlist, ds_name => foo" do > > @resource.stubs(:[]).with(:ds_type).returns "computerlist" > > @resource.stubs(:[]).with(:ds_name).returns "foo" > > - lambda { @provider.create }.should_not > > raise_error(MCXContentProviderException) > > + lambda { @provider.create }.should_not raise_error > > end > > it "should not accept :ds_type => bogustype, ds_name => foo" > > do > > @resource.stubs(:[]).with(:ds_type).returns "bogustype" > > diff --git a/spec/unit/type/mcx.rb b/spec/unit/type/mcx.rb > > index 8016377..ce2baaf 100755 > > --- a/spec/unit/type/mcx.rb > > +++ b/spec/unit/type/mcx.rb > > @@ -76,6 +76,103 @@ describe mcx_type, "default values" do > > > > end > > > > +describe mcx_type, "when managing resources when the ds_type and > > ds_name are specified" do > > + before :each do > > + provider_class = mcx_type.provider(mcx_type.providers[0]) > > + mcx_type.stubs(:defaultprovider).returns provider_class > > + end > > + > > + after :each do > > + mcx_type.clear > > + end > > + > > + it "should autorequire groups when the type is specified" do > > + foo_mcx = mcx_type.create(:name => 'foo1', :ds_type => > > 'group', :ds_name => 'foo1') > > + foo = Puppet::Type.type(:group).create(:name => 'foo1') > > + > > + config = Puppet::Node::Catalog.new :testing do |conf| > > + [foo_mcx, foo].each { |resource| conf.add_resource > > resource } > > + end > > + > > + rel = foo_mcx.autorequire[0] > > + rel.source.ref.should == foo.ref > > + rel.target.ref.should == foo_mcx.ref > > + end > > + it "should autorequire users when the type is specified" do > > + foo_mcx = mcx_type.create(:name => 'foo1', :ds_type => > > 'user', :ds_name => 'foo1') > > + foo = Puppet::Type.type(:user).create(:name => 'foo1') > > + > > + config = Puppet::Node::Catalog.new :testing do |conf| > > + [foo_mcx, foo].each { |resource| conf.add_resource > > resource } > > + end > > + > > + rel = foo_mcx.autorequire[0] > > + rel.source.ref.should == foo.ref > > + rel.target.ref.should == foo_mcx.ref > > + end > > + it "should autorequire computers when the type is specified" do > > + foo_mcx = mcx_type.create(:name => 'foo1', :ds_type => > > 'computer', :ds_name => 'foo1') > > + foo = Puppet::Type.type(:computer).create(:name => 'foo1') > > + > > + config = Puppet::Node::Catalog.new :testing do |conf| > > + [foo_mcx, foo].each { |resource| conf.add_resource > > resource } > > + end > > + > > + rel = foo_mcx.autorequire[0] > > + rel.source.ref.should == foo.ref > > + rel.target.ref.should == foo_mcx.ref > > + end > > + > > +end > > + > > +describe mcx_type, "when managing resources when the ds_type and > > ds_name are not specified" do > > + before :each do > > + provider_class = mcx_type.provider(mcx_type.providers[0]) > > + mcx_type.stubs(:defaultprovider).returns provider_class > > + end > > + > > + after :each do > > + mcx_type.clear > > + end > > + > > + it "should autorequire groups when parsing the type" do > > + foo_mcx = mcx_type.create(:name => '/Groups/foo') > > + foo = Puppet::Type.type(:group).create(:name => 'foo') > > + > > + config = Puppet::Node::Catalog.new :testing do |conf| > > + [foo_mcx, foo].each { |resource| conf.add_resource > > resource } > > + end > > + > > + rel = foo_mcx.autorequire[0] > > + rel.source.ref.should == foo.ref > > + rel.target.ref.should == foo_mcx.ref > > + end > > + it "should autorequire users when parsing the type" do > > + foo_mcx = mcx_type.create(:name => '/Users/foo') > > + foo = Puppet::Type.type(:user).create(:name => 'foo') > > + > > + config = Puppet::Node::Catalog.new :testing do |conf| > > + [foo_mcx, foo].each { |resource| conf.add_resource > > resource } > > + end > > + > > + rel = foo_mcx.autorequire[0] > > + rel.source.ref.should == foo.ref > > + rel.target.ref.should == foo_mcx.ref > > + end > > + it "should autorequire computers when parsing the type" do > > + foo_mcx = mcx_type.create(:name => '/Computers/foo') > > + foo = Puppet::Type.type(:computer).create(:name => 'foo') > > + > > + config = Puppet::Node::Catalog.new :testing do |conf| > > + [foo_mcx, foo].each { |resource| conf.add_resource > > resource } > > + end > > + > > + rel = foo_mcx.autorequire[0] > > + rel.source.ref.should == foo.ref > > + rel.target.ref.should == foo_mcx.ref > > + end > > +end > > + > > describe mcx_type, "when validating properties" do > > > > before :each do > > -- > > 1.6.0.4 > > > > > > > > > > -- > The only thing that saves us from the bureaucracy is inefficiency. An > efficient bureaucracy is the greatest threat to liberty. > --Eugene McCarthy > --------------------------------------------------------------------- > Luke Kanies | http://reductivelabs.com | http://madstop.com > > > > > --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
