The patch looks good but one comment below. Otherwise, +1 On Nov 2, 2009, at 5:45 PM, Markus Roberts wrote:
> > The fundemental problem was that, despite what the comment said, > the early bailout for file content management only applied to > directories, not to links. Making links bail out at as well fixed > the problem for most users. > > However, it would still occur for users with mixed ruby version > system since there were no to_/from_pson methods for file metadata. > So the second (and far larger) part of this patch adds metadata > pson support. > > The testing is unit level only, as there's no pratical way to do > the cross-ruby-version acceptance testing and no benifit to doing > "integration" testing short of that. > > Signed-off-by: Markus Roberts <[email protected]> > Signed-off-by: Markus Roberts <[email protected]> > --- > lib/puppet/external/pson/common.rb | 2 +- > lib/puppet/file_serving/base.rb | 15 +++++++ > lib/puppet/file_serving/metadata.rb | 46 +++++++++++++++++++- > lib/puppet/type/file/content.rb | 2 +- > spec/unit/file_serving/metadata.rb | 78 ++++++++++++++++++++++++++ > +++++++++ > spec/unit/type/file/content.rb | 9 ++++ > 6 files changed, 147 insertions(+), 5 deletions(-) > > diff --git a/lib/puppet/external/pson/common.rb b/lib/puppet/ > external/pson/common.rb > index 71b85ce..87bce98 100644 > --- a/lib/puppet/external/pson/common.rb > +++ b/lib/puppet/external/pson/common.rb > @@ -48,7 +48,7 @@ module PSON > case > when c.empty? then p > when p.const_defined?(c) then p.const_get(c) > - else raise ArgumentError, "can't find > const #{path}" > + else raise ArgumentError, "can't find > const for unregistered document type #{path}" > end > end > end > diff --git a/lib/puppet/file_serving/base.rb b/lib/puppet/ > file_serving/base.rb > index c82bb19..02132e8 100644 > --- a/lib/puppet/file_serving/base.rb > +++ b/lib/puppet/file_serving/base.rb > @@ -74,4 +74,19 @@ class Puppet::FileServing::Base > end > File.send(@stat_method, full_path()) > end > + > + def to_pson_data_hash > + { > + # No 'document_type' since we don't send these bare > + 'data' => { > + 'path' => @path, > + 'relative_path' => @relative_path, > + 'links' => @links > + }, > + 'metadata' => { > + 'api_version' => 1 > + } > + } > + end > + > end > diff --git a/lib/puppet/file_serving/metadata.rb b/lib/puppet/ > file_serving/metadata.rb > index 335dad4..8e49784 100644 > --- a/lib/puppet/file_serving/metadata.rb > +++ b/lib/puppet/file_serving/metadata.rb > @@ -71,8 +71,48 @@ class Puppet::FileServing::Metadata < > Puppet::FileServing::Base > end > end > > - def initialize(*args) > - @checksum_type = "md5" > - super > + def initialize(path,data={}) > + @owner = data.delete('owner') > + @group = data.delete('group') > + @mode = data.delete('mode') > + if checksum = data.delete('checksum') > + @checksum_type = checksum['type'] > + @checksum = checksum['value'] > + else > + @checksum_type = "md5" > + end > + @ftype = data.delete('type') > + @destination = data.delete('destination') > + super(path,data) > end > + > + PSON.register_document_type('FileMetadata',self) > + def to_pson_data_hash > + { > + 'document_type' => 'FileMetadata', > + 'data' => super['data'].update({ > + 'owner' => owner, > + 'group' => group, > + 'mode' => mode, > + 'checksum' => { > + 'type' => checksum_type, > + 'value' => checksum > + }, > + 'type' => ftype, > + 'destination' => destination, > + }), > + 'metadata' => { > + 'api_version' => 1 > + } > + } > + end > + > + def to_pson(*args) > + to_pson_data_hash.to_pson(*args) > + end > + > + def self.from_pson(data) > + new(data.delete('path'), data) > + end > + > end > diff --git a/lib/puppet/type/file/content.rb b/lib/puppet/type/file/ > content.rb > index ff71a55..e0e1188 100755 > --- a/lib/puppet/type/file/content.rb > +++ b/lib/puppet/type/file/content.rb > @@ -116,7 +116,7 @@ module Puppet > return :absent unless stat = @resource.stat > > # Don't even try to manage the content on directories or > links > - return nil if stat.ftype == "directory" > + return nil if ["directory","link"].include? stat.ftype > > begin > return "{#{checksum_type}}" + > send(checksum_type.to_s + "_file", resource[:path]).to_s > diff --git a/spec/unit/file_serving/metadata.rb b/spec/unit/ > file_serving/metadata.rb > index de0c457..790bbf6 100755 > --- a/spec/unit/file_serving/metadata.rb > +++ b/spec/unit/file_serving/metadata.rb > @@ -20,6 +20,84 @@ describe Puppet::FileServing::Metadata do > it "should have a method that triggers attribute collection" do > Puppet::FileServing::Metadata.new("/foo/bar").should > respond_to(:collect) > end > + > + it "shoud support pson serialization" do s/shoud/should/g :) > + Puppet::FileServing::Metadata.new("/foo/bar").should > respond_to(:to_pson) > + end > + > + it "shoud support to_pson_data_hash" do > + Puppet::FileServing::Metadata.new("/foo/bar").should > respond_to(:to_pson_data_hash) > + end > + > + it "shoud support pson deserialization" do > + Puppet::FileServing::Metadata.should respond_to(:from_pson) > + end > + > + describe "when serializing" do > + before do > + @metadata = Puppet::FileServing::Metadata.new("/foo/bar") > + end > + it "shoud perform pson serialization by calling to_pson on > it's pson_data_hash" do > + pdh = mock "data hash" > + pdh_as_pson = mock "data as pson" > + @metadata.expects(:to_pson_data_hash).returns pdh > + pdh.expects(:to_pson).returns pdh_as_pson > + @metadata.to_pson.should == pdh_as_pson > + end > + > + it "should serialize as FileMetadata" do > + @metadata.to_pson_data_hash['document_type'].should == > "FileMetadata" > + end > + > + it "the data should include the path, relative_path, links, > owner, group, mode, checksum, type, and destination" do > + @metadata.to_pson_data_hash['data'].keys.sort.should == > %w{ path relative_path links owner group mode checksum type > destination }.sort > + end > + > + it "should pass the path in the hash verbatum" do > + @metadata.to_pson_data_hash['data']['path'] == > @metadata.path > + end > + > + it "should pass the relative_path in the hash verbatum" do > + @metadata.to_pson_data_hash['data']['relative_path'] == > @metadata.relative_path > + end > + > + it "should pass the links in the hash verbatum" do > + @metadata.to_pson_data_hash['data']['links'] == > @metadata.links > + end > + > + it "should pass the path owner in the hash verbatum" do > + @metadata.to_pson_data_hash['data']['owner'] == > @metadata.owner > + end > + > + it "should pass the group in the hash verbatum" do > + @metadata.to_pson_data_hash['data']['group'] == > @metadata.group > + end > + > + it "should pass the mode in the hash verbatum" do > + @metadata.to_pson_data_hash['data']['mode'] == > @metadata.mode > + end > + > + it "should pass the ftype in the hash verbatum as the > 'type'" do > + @metadata.to_pson_data_hash['data']['type'] == > @metadata.ftype > + end > + > + it "should pass the destination verbatum" do > + @metadata.to_pson_data_hash['data']['destination'] == > @metadata.destination > + end > + > + it "should pass the checksum in the hash as a nested hash" do > + @metadata.to_pson_data_hash['data']['checksum'].should > be_is_a Hash > + end > + > + it "should pass the checksum_type in the hash verbatum as > the checksum's type" do > + @metadata.to_pson_data_hash['data']['checksum']['type'] > == @metadata.checksum_type > + end > + > + it "should pass the checksum in the hash verbatum as the > checksum's value" do > + @metadata.to_pson_data_hash['data']['checksum'] > ['value'] == @metadata.checksum > + end > + > + end > end > > describe Puppet::FileServing::Metadata, " when finding the file to > use for setting attributes" do > diff --git a/spec/unit/type/file/content.rb b/spec/unit/type/file/ > content.rb > index 8bdb1f2..901d52d 100755 > --- a/spec/unit/type/file/content.rb > +++ b/spec/unit/type/file/content.rb > @@ -119,6 +119,15 @@ describe content do > @content.retrieve.should be_nil > end > > + it "should not manage content on links" do > + @content = content.new(:resource => @resource) > + > + stat = mock 'stat', :ftype => "link" > + @resource.expects(:stat).returns stat > + > + @content.retrieve.should be_nil > + end > + > it "should always return the checksum as a string" do > @content = content.new(:resource => @resource) > @content.stubs(:checksum_type).returns "mtime" > -- > 1.6.4 > > > > -- There are three social classes in America: upper middle class, middle class, and lower middle class. --Judith Martin --------------------------------------------------------------------- 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 -~----------~----~----~----~------~----~------~--~---
