On Wed, Oct 27, 2010 at 1:27 AM, Sandor Szuecs
<[email protected]> wrote:
> Signed-off-by: Sandor Szuecs <[email protected]>
> ---
> lib/puppet/provider/mcx/mcxcontent.rb | 194
> +++++++++++++----------------
> spec/unit/provider/mcx/mcxcontent_spec.rb | 26 ++--
> 2 files changed, 102 insertions(+), 118 deletions(-)
>
> diff --git a/lib/puppet/provider/mcx/mcxcontent.rb
> b/lib/puppet/provider/mcx/mcxcontent.rb
> index cb5adc6..4f14faa 100644
> --- a/lib/puppet/provider/mcx/mcxcontent.rb
> +++ b/lib/puppet/provider/mcx/mcxcontent.rb
> @@ -45,58 +45,92 @@ Puppet::Type.type(:mcx).provide :mcxcontent, :parent =>
> Puppet::Provider do
> :computerlist => "ComputerLists",
> }
>
> - class MCXContentProviderException < Exception
> -
> - end
> -
> commands :dscl => "/usr/bin/dscl"
> confine :operatingsystem => :darwin
> defaultfor :operatingsystem => :darwin
> -
> - # self.instances is all important.
> - # This is the only class method, it returns
> - # 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]}"
> - 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."
> - else
> - # This node has MCX data.
> -
> - rsrc = self.new(
> +
> + # FIXME: dslocal name is fixed. The default should be "Default" if we do
> not
> + # want to break existing installations.
> + DSLOCAL_DIR = "Default"
If this is a pure refactor with no additional functionality, I'd like
to see some text explaining why you've refactored the way you did, as
I'm not entirely convinced that this is all worth the effort.
> +
> + class << self
> + # It returns an array of instances of this class.
> + def instances
> + mcx_list = []
> + TypeMap.keys.each do |ds_type|
> + ds_path = "/Local/#{DSLOCAL_DIR}/#{TypeMap[ds_type]}"
> + output = dscl 'localhost', '-list', ds_path
> + member_list = output.split
> + member_list.each do |ds_name|
> + content = mcxexport(ds_type, ds_name)
> + if content.empty?
> + Puppet.debug "/#{TypeMap[ds_type]}/#{ds_name} has no MCX data."
> + else
> + # This node has MCX data.
> + mcx_list << self.new(
> :name => "/#{TypeMap[ds_type]}/#{ds_name}",
> - :ds_type => ds_type,
> - :ds_name => ds_name,
> -
> - :content => content)
> - mcx_list << rsrc
> + :ds_type => ds_type,
> + :ds_name => ds_name,
> + :content => content)
> + end
> end
> end
> + mcx_list
> + end
> +
> + def mcxexport(ds_type, ds_name)
> + ds_t = TypeMap[ds_type]
> + ds_n = ds_name.to_s
> + ds_path = "/Local/#{DSLOCAL_DIR}/#{ds_t}/#{ds_n}"
> + dscl 'localhost', '-mcxexport', ds_path
> end
> - mcx_list
> end
> +
> + public
>
> - private
> + def create
> + self.content=(resource[:content])
> + end
>
> - # mcxexport is used by instances, and therefore
> - # a class method.
> - def self.mcxexport(ds_type, ds_name)
> - ds_t = TypeMap[ds_type]
> - ds_n = ds_name.to_s
> - ds_path = "/Local/Default/#{ds_t}/#{ds_n}"
> - dscl 'localhost', '-mcxexport', ds_path
> + def destroy
> + ds_parms = get_dsparams
> + ds_t = TypeMap[ds_parms[:ds_type]]
> + ds_n = ds_parms[:ds_name].to_s
> + ds_path = "/Local/#{DSLOCAL_DIR}/#{ds_t}/#{ds_n}"
> +
> + dscl 'localhost', '-mcxdelete', ds_path
> end
>
> + def exists?
> + begin
> + has_mcx?
> + rescue Puppet::ExecutionFailure => e
> + return false
> + end
> + end
> +
> + def content
> + ds_parms = get_dsparams
> +
> + self.class.mcxexport(ds_parms[:ds_type], ds_parms[:ds_name])
> + end
> +
> + def content=(value)
> + # dscl localhost -mcximport
> + ds_parms = get_dsparams
> +
> + mcximport(ds_parms[:ds_type], ds_parms[:ds_name], resource[:content])
> + end
> +
> + private
> +
> + def has_mcx?
> + !content.empty?
> + end
> +
> def mcximport(ds_type, ds_name, val)
> ds_t = TypeMap[ds_type]
> - ds_n = ds_name.to_s
> - ds_path = "/Local/Default/#{ds_t}/#{ds_name}"
> + ds_path = "/Local/#{DSLOCAL_DIR}/#{ds_t}/#{ds_name}"
>
> tmp = Tempfile.new('puppet_mcx')
> begin
> @@ -111,95 +145,45 @@ Puppet::Type.type(:mcx).provide :mcxcontent, :parent =>
> Puppet::Provider do
>
> # 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."
> + ds_type = name.split('/')[1]
> + unless ds_type
> + error("Could 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."
> + ds_type = ds_type.chop.downcase.to_sym
> + unless TypeMap.key? ds_type
> + error("Could not parse ds_type from resource name '#{name}'. Specify
> with ds_type parameter.")
> end
> - tmp
> + ds_type
> 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."
> + unless ds_name
> + error("Could not parse ds_name from resource name '#{name}'. Specify
> with ds_name parameter.")
> end
> 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.
> + # Gather ds_type and ds_name from resource or parse it out of the name.
> def get_dsparams
> ds_type = resource[:ds_type]
> ds_type ||= parse_type(resource[:name])
> - raise MCXContentProviderException unless TypeMap.keys.include?
> ds_type.to_sym
> + unless TypeMap.key? ds_type.to_sym
> + raise Puppet::Error.new("#{ds_type} is not a key in TypeMap")
> + end
>
> ds_name = resource[:ds_name]
> ds_name ||= parse_name(resource[:name])
>
> - rval = {
> + {
> :ds_type => ds_type.to_sym,
> :ds_name => ds_name,
> }
> -
> - return rval
> -
> end
> -
> - public
> -
> - def create
> - self.content=(resource[:content])
> +
> + def error(msg)
> + raise Puppet::Error.new(msg)
> end
> -
> - def destroy
> - ds_parms = get_dsparams
> - ds_t = TypeMap[ds_parms[:ds_type]]
> - ds_n = ds_parms[:ds_name].to_s
> - ds_path = "/Local/Default/#{ds_t}/#{ds_n}"
> -
> - dscl 'localhost', '-mcxdelete', ds_path
> - end
> -
> - def exists?
> - # JJM Just re-use the content method and see if it's empty.
> - begin
> - mcx = content
> - rescue Puppet::ExecutionFailure => e
> - return false
> - end
> - has_mcx = ! mcx.empty?
> - end
> -
> - def content
> - ds_parms = get_dsparams
> -
> - mcx = self.class.mcxexport(
> - ds_parms[:ds_type],
> -
> - ds_parms[:ds_name])
> - mcx
> - end
> -
> - def content=(value)
> - # dscl localhost -mcximport
> - ds_parms = get_dsparams
> -
> - mcx = mcximport(
> - ds_parms[:ds_type],
> - ds_parms[:ds_name],
> -
> - resource[:content])
> - mcx
> - end
> -
> end
> diff --git a/spec/unit/provider/mcx/mcxcontent_spec.rb
> b/spec/unit/provider/mcx/mcxcontent_spec.rb
> index d3fb043..d6db168 100755
> --- a/spec/unit/provider/mcx/mcxcontent_spec.rb
> +++ b/spec/unit/provider/mcx/mcxcontent_spec.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::Error)
> 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(Puppet::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(Puppet::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(Puppet::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(Puppet::Error)
> end
> end
>
> @@ -123,40 +123,40 @@ 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::Error)
> 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::Error)
> 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::Error)
> 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(Puppet::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(Puppet::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(Puppet::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(Puppet::Error)
> end
> it "should not accept :ds_type => bogustype, ds_name => foo" do
> @resource.stubs(:[]).with(:ds_type).returns "bogustype"
> @resource.stubs(:[]).with(:ds_name).returns "foo"
> - lambda { @provider.create }.should
> raise_error(MCXContentProviderException)
> + lambda { @provider.create }.should raise_error(Puppet::Error)
> end
> end
>
> --
> 1.7.3.1
>
> --
> 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.
>
>
--
Nigel Kersten - Puppet Labs - http://www.puppetlabs.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.
