Converting the file 'source' property to a parameter.

This makes a lot of sense because source was always
more of a metaparameter than a property -- it affected
the 'should' values of other properties, but it shouldn't
have done any other work.

It will hopefully make everything else much cleaner.

This is such a large commit mostly because of the need
to fix a lot of tests.

Signed-off-by: Luke Kanies <luke@madstop.com>

3 files changed, 119 insertions, 153 deletions

diff --git a/spec/integration/type/file.rb b/spec/integration/type/file.rb
index c9e0d19c6..5e32332d3 100755
--- a/spec/integration/type/file.rb
+++ b/spec/integration/type/file.rb
@@ -175,10 +175,39 @@ describe Puppet::Type.type(:file) do
 
             # And make sure it's changed
             File.read(dest).should == "bar"
+        end
+
+        it "should be able to copy individual files even if recurse has been specified" do
+            source = tmpfile("source")
+            dest = tmpfile("dest")
+
+            File.open(source, "w") { |f| f.print "foo" }
+
+            file = Puppet::Type::File.create(:name => dest, :source => source, :recurse => true)
 
+            catalog = Puppet::Node::Catalog.new
+            catalog.add_resource file
+            catalog.apply
+
+            File.read(dest).should == "foo"
         end
     end
 
+    it "should be able to create files when 'content' is specified but 'ensure' is not" do
+        dest = tmpfile("files_with_content")
+
+        file = Puppet.type(:file).create(
+            :name => dest,
+            :content => "this is some content, yo"
+        )
+
+        catalog = Puppet::Node::Catalog.new
+        catalog.add_resource file
+        catalog.apply
+
+        File.read(dest).should == "this is some content, yo"
+    end
+
     it "should create files with content if both 'content' and 'ensure' are set" do
         dest = tmpfile("files_with_content")
 
diff --git a/spec/unit/type/file.rb b/spec/unit/type/file.rb
index 533f19556..8c1eef177 100755
--- a/spec/unit/type/file.rb
+++ b/spec/unit/type/file.rb
@@ -14,25 +14,59 @@ describe Puppet::Type.type(:file) do
         @file.catalog = @catalog
     end
 
-    describe "when used with content and replace=>false" do
-        before do
-            @file[:content] = "foo"
-            @file[:replace] = false
-        end
+    it "should have a method for determining if the file is present" do
+        @file.must respond_to(:exist?)
+    end
 
-        it "should be insync if the file exists and the content is different" do
-            File.open(@path, "w") do |f| f.puts "bar" end
-            @file.property(:content).insync?("bar").should be_true
-        end
+    it "should be considered existent if it can be stat'ed" do
+        @file.expects(:stat).returns mock('stat')
+        @file.must be_exist
+    end
 
-        it "should be insync if the file exists and the content is right" do
-            File.open(@path, "w") do |f| f.puts "foo" end
-            @file.property(:content).insync?("foo").should be_true
-        end
+    it "should be considered nonexistent if it can not be stat'ed" do
+        @file.expects(:stat).returns nil
+        @file.must_not be_exist
+    end
 
-        it "should not be insync if the file does not exist" do
-            @file.property(:content).insync?(:nil).should be_false
-        end
+    it "should have a method for determining if the file should be a normal file" do
+        @file.must respond_to(:should_be_file?)
+    end
+
+    it "should be a file if :ensure is set to :file" do
+        @file[:ensure] = :file
+        @file.must be_should_be_file
+    end
+
+    it "should be a file if :ensure is set to :present and the file exists as a normal file" do
+        @file.stubs(:stat).returns(mock('stat', :ftype => "file"))
+        @file[:ensure] = :present
+        @file.must be_should_be_file
+    end
+
+    it "should not be a file if :ensure is set to something other than :file" do
+        @file[:ensure] = :directory
+        @file.must_not be_should_be_file
+    end
+
+    it "should not be a file if :ensure is set to :present and the file exists but is not a normal file" do
+        @file.stubs(:stat).returns(mock('stat', :ftype => "directory"))
+        @file[:ensure] = :present
+        @file.must_not be_should_be_file
+    end
+
+    it "should be a file if :ensure is not set and :content is" do
+        @file[:content] = "foo"
+        @file.must be_should_be_file
+    end
+
+    it "should be a file if neither :ensure nor :content is set but the file exists as a normal file" do
+        @file.stubs(:stat).returns(mock("stat", :ftype => "file"))
+        @file.must be_should_be_file
+    end
+
+    it "should not be a file if neither :ensure nor :content is set but the file exists but not as a normal file" do
+        @file.stubs(:stat).returns(mock("stat", :ftype => "directory"))
+        @file.must_not be_should_be_file
     end
 
     describe "when managing links" do
@@ -149,7 +183,7 @@ describe Puppet::Type.type(:file) do
     describe "when flushing" do
         it "should flush all properties that respond to :flush" do
             @resource = Puppet.type(:file).create(:path => "/foo/bar", :source => "/bar/foo")
-            @resource.property(:source).expects(:flush)
+            @resource.parameter(:source).expects(:flush)
             @resource.flush
         end
 
@@ -318,9 +352,11 @@ describe Puppet::Type.type(:file) do
 
             @first = Puppet::FileServing::Metadata.new("/my", :relative_path => "first")
             @second = Puppet::FileServing::Metadata.new("/my", :relative_path => "second")
+            @first.stubs(:ftype).returns "directory"
+            @second.stubs(:ftype).returns "directory"
 
-            @property = stub 'property', :metadata= => nil
-            @resource = stub 'file', :[]= => nil, :property => @property
+            @parameter = stub 'property', :metadata= => nil
+            @resource = stub 'file', :[]= => nil, :parameter => @parameter
         end
 
         it "should pass its source to the :perform_recursion method" do
@@ -330,6 +366,14 @@ describe Puppet::Type.type(:file) do
             @file.recurse_remote({})
         end
 
+        it "should not recurse when the remote file is not a directory" do
+            data = Puppet::FileServing::Metadata.new("/whatever", :relative_path => ".")
+            data.stubs(:ftype).returns "file"
+            @file.expects(:perform_recursion).with("puppet://foo/bar").returns [data]
+            @file.expects(:newchild).never
+            @file.recurse_remote({})
+        end
+
         it "should set the source of each returned file to the searched-for URI plus the found relative path" do
             @first.expects(:source=).with File.join("puppet://foo/bar", @first.relative_path)
             @file.expects(:perform_recursion).returns [@first]
@@ -368,9 +412,9 @@ describe Puppet::Type.type(:file) do
 
         it "should store the metadata in the source property for each resource so the source does not have to requery the metadata" do
             @file.stubs(:perform_recursion).returns [@first]
-            @resource.expects(:property).with(:source).returns @property
+            @resource.expects(:parameter).with(:source).returns @parameter
             
-            @property.expects(:metadata=).with(@first)
+            @parameter.expects(:metadata=).with(@first)
 
             @file.recurse_remote("first" => @resource)
         end
@@ -388,7 +432,7 @@ describe Puppet::Type.type(:file) do
             @first.stubs(:relative_path).returns "."
             @file.stubs(:perform_recursion).returns [@first]
 
-            @file.property(:source).expects(:metadata=).with @first
+            @file.parameter(:source).expects(:metadata=).with @first
             
             @file.recurse_remote("first" => @resource)
         end
@@ -403,7 +447,7 @@ describe Puppet::Type.type(:file) do
 
                 @resource.expects(:[]=).with(:ensure, :absent)
 
-                @file.expects(:newchild).returns stub('secondfile', :[]= => nil, :property => @property)
+                @file.expects(:newchild).returns stub('secondfile', :[]= => nil, :parameter => @parameter)
 
                 @file.recurse_remote("first" => @resource)
             end
@@ -582,8 +626,14 @@ describe Puppet::Type.type(:file) do
                 end
             end
 
+            it "should fail if it has no catalog" do
+                file = @file.class.create(:path => "/foo/bar", :owner => "root", :group => "wheel")
+                lambda { file.newchild("foo/bar").should raise_error(ArgumentError)
+            end
+
             it "should copy all of the parent resource's 'should' values that were set at initialization" do
                 file = @file.class.create(:path => "/foo/bar", :owner => "root", :group => "wheel")
+                @catalog.add_resource(file)
                 file.class.expects(:create).with { |options| options[:owner] == "root" and options[:group] == "wheel" }
                 file.newchild("my/path")
             end
@@ -596,13 +646,18 @@ describe Puppet::Type.type(:file) do
             it "should not copy values to the child which were set by the source" do
                 @file[:source] = "/foo/bar"
                 metadata = stub 'metadata', :owner => "root", :group => "root", :mode => 0755, :ftype => "file", :checksum => "{md5}whatever"
-                @file.property(:source).stubs(:metadata).returns metadata
+                @file.parameter(:source).stubs(:metadata).returns metadata
 
-                @file.property(:source).copy_source_values
+                @file.parameter(:source).copy_source_values
 
                 @file.class.expects(:create).with { |params| params[:group].nil? }
                 @file.newchild("my/path")
             end
+
+            it "should return nil if the specified child resource already exists in the catalog" do
+                @catalog.expects(:resource).with(:file, File.join(@file[:path], "foo/bar")).returns mock("resource")
+                @file.newchild("foo/bar").should be_nil
+            end
         end
     end
 end
diff --git a/spec/unit/type/file/source.rb b/spec/unit/type/file/source.rb
index 2bb19eecc..2dcc1a557 100755
--- a/spec/unit/type/file/source.rb
+++ b/spec/unit/type/file/source.rb
@@ -9,22 +9,22 @@ describe Puppet::Type.type(:file).attrclass(:source) do
         @resource = stub 'resource', :[]= => nil, :property => nil
     end
 
-    it "should be a subclass of Property" do
-        source.superclass.must == Puppet::Property
+    it "should be a subclass of Parameter" do
+        source.superclass.must == Puppet::Parameter
     end
 
     describe "when initializing" do
-        it "should fail if the 'should' values are not URLs" do
+        it "should fail if the set values are not URLs" do
             s = source.new(:resource => @resource)
             URI.expects(:parse).with('foo').raises RuntimeError
 
-            lambda { s.should = %w{foo} }.must raise_error(Puppet::Error)
+            lambda { s.value = %w{foo} }.must raise_error(Puppet::Error)
         end
 
         it "should fail if the URI is not a local file, file URI, or puppet URI" do
             s = source.new(:resource => @resource)
 
-            lambda { s.should = %w{http://foo/bar} }.must raise_error(Puppet::Error)
+            lambda { s.value = %w{http://foo/bar} }.must raise_error(Puppet::Error)
         end
     end
 
@@ -53,14 +53,14 @@ describe Puppet::Type.type(:file).attrclass(:source) do
         end
 
         it "should collect its metadata using the Metadata class if it is not already set" do
-            @source = source.new(:resource => @resource, :should => "/foo/bar")
+            @source = source.new(:resource => @resource, :value => "/foo/bar")
             Puppet::FileServing::Metadata.expects(:find).with("/foo/bar").returns @metadata
             @source.metadata
         end
 
         it "should use the metadata from the first found source" do
             metadata = stub 'metadata', :source= => nil
-            @source = source.new(:resource => @resource, :should => ["/foo/bar", "/fee/booz"])
+            @source = source.new(:resource => @resource, :value => ["/foo/bar", "/fee/booz"])
             Puppet::FileServing::Metadata.expects(:find).with("/foo/bar").returns nil
             Puppet::FileServing::Metadata.expects(:find).with("/fee/booz").returns metadata
             @source.metadata.should equal(metadata)
@@ -68,7 +68,7 @@ describe Puppet::Type.type(:file).attrclass(:source) do
 
         it "should store the found source as the metadata's source" do
             metadata = mock 'metadata'
-            @source = source.new(:resource => @resource, :should => "/foo/bar")
+            @source = source.new(:resource => @resource, :value => "/foo/bar")
             Puppet::FileServing::Metadata.expects(:find).with("/foo/bar").returns metadata
 
             metadata.expects(:source=).with("/foo/bar")
@@ -76,7 +76,7 @@ describe Puppet::Type.type(:file).attrclass(:source) do
         end
 
         it "should fail intelligently if an exception is encountered while querying for metadata" do
-            @source = source.new(:resource => @resource, :should => "/foo/bar")
+            @source = source.new(:resource => @resource, :value => "/foo/bar")
             Puppet::FileServing::Metadata.expects(:find).with("/foo/bar").raises RuntimeError
 
             @source.expects(:fail).raises ArgumentError
@@ -84,7 +84,7 @@ describe Puppet::Type.type(:file).attrclass(:source) do
         end
 
         it "should fail if no specified sources can be found" do
-            @source = source.new(:resource => @resource, :should => "/foo/bar")
+            @source = source.new(:resource => @resource, :value => "/foo/bar")
             Puppet::FileServing::Metadata.expects(:find).with("/foo/bar").returns nil
 
             @source.expects(:fail).raises RuntimeError
@@ -200,15 +200,6 @@ describe Puppet::Type.type(:file).attrclass(:source) do
         end
     end
 
-    describe "when retrieving the property state" do
-        it "should copy all metadata to the resource" do
-            @source = source.new(:resource => @resource)
-            @source.expects(:copy_source_values)
-
-            @source.retrieve
-        end
-    end
-
     describe "when flushing" do
         it "should set its metadata to nil" do
             @source = source.new(:resource => @resource)
@@ -224,86 +215,6 @@ describe Puppet::Type.type(:file).attrclass(:source) do
             @source.instance_variable_get("@content").should be_nil
         end
     end
-    
-    describe "when testing whether the local file is in sync" do
-        before do
-            @source = source.new(:resource => @resource)
-        end
-
-        it "should be considered in sync if the remote file is a directory" do
-            metadata = mock 'data', :ftype => "directory"
-            @source.expects(:metadata).returns metadata
-
-            @source.must be_insync("some content")
-        end
-
-        it "should be considered in sync if the remote file is a symlink" do
-            metadata = mock 'data', :ftype => "link"
-            @source.expects(:metadata).returns metadata
-
-            @source.must be_insync("some content")
-        end
-
-        describe "and the remote file is a normal file" do
-            before do
-                @metadata = mock 'data', :ftype => "file"
-                @source.expects(:metadata).returns @metadata
-            end
-
-            it "should be not considered in sync if the file does not exist" do
-                @resource.expects(:stat).returns nil
-                @source.should_not be_insync("some content")
-            end
-
-            it "should be considered in sync if :replace is false and the file exists" do
-                @resource.expects(:stat).returns mock('stat')
-                @resource.expects(:replace?).returns false
-                @source.must be_insync("some content")
-            end
-
-            it "should be not considered in sync if :replace is false and the file does not exist" do
-                @resource.expects(:stat).returns nil
-                @resource.stubs(:replace?).returns false
-                @source.should_not be_insync("some content")
-            end
-
-            it "should not be considered in sync if the local file's contents are not the same as the remote file's contents"
-
-            it "should be considered in sync if the local file's content matches the remote file's contents"
-        end
-    end
-
-    def test_insync
-        source = tempfile()
-        dest = tempfile()
-        
-        file = Puppet::Type.type(:file).create :path => dest, :source => source, :title => "copier"
-        
-        property = file.property(:source)
-        assert(property, "did not get source property")
-        
-        # with a directory
-        Dir.mkdir(source)
-        currentvalues = file.retrieve
-        assert(property.insync?(currentvalues[property]), "source property not in sync with directory as source")
-        Dir.rmdir(source)
-        
-        # with a file
-        File.open(source, "w") { |f| f.puts "yay" }
-        currentvalues = file.retrieve
-        p currentvalues[property]
-        assert(!property.insync?(currentvalues[property]), "source property was in sync when file was missing")
-        
-        # With a different file
-        File.open(dest, "w") { |f| f.puts "foo" }
-        currentvalues = file.retrieve
-        assert(!property.insync?(currentvalues[property]), "source property was in sync with different file")
-        
-        # with matching files
-        File.open(dest, "w") { |f| f.puts "yay" }
-        currentvalues = file.retrieve
-        assert(property.insync?(currentvalues[property]), "source property was not in sync with matching file")
-    end
 
     it "should have a method for returning the content" do
         source.new(:resource => @resource).must respond_to(:content)
@@ -340,33 +251,4 @@ describe Puppet::Type.type(:file).attrclass(:source) do
             lambda { @source.content }.should raise_error(RuntimeError)
         end
     end
-
-    describe "when changing the content" do
-        before do
-            @source = source.new(:resource => @resource)
-            @source.stubs(:content).returns "foobar"
-
-            @metadata = stub 'metadata', :checksum => 123
-            @source.metadata = @metadata
-            @resource.stubs(:[]).with(:path).returns "/boo"
-        end
-
-        it "should use the file's :write method to write the content" do
-            @resource.expects(:write).with("foobar", :source, 123)
-
-            @source.sync
-        end
-
-        it "should return :file_changed if the file already existed" do
-            @resource.stubs(:write)
-            FileTest.expects(:exist?).with("/boo").returns true
-            @source.sync.should == :file_changed
-        end
-
-        it "should return :file_created if the file already existed" do
-            @resource.stubs(:write)
-            FileTest.expects(:exist?).with("/boo").returns false
-            @source.sync.should == :file_created
-        end
-    end
 end