Mostly finishing refactoring file recursion to use REST.

We have the majority of the work done (and it's a *lot* less
code).  We just have six more tests we need to implement the code
for.

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

4 files changed, 250 insertions, 19 deletions

diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index bc2e53c9f..2e3a585c1 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -329,7 +329,7 @@ module Puppet
         
         # Create any children via recursion or whatever.
         def eval_generate
-            recurse()
+            recurse() if self.recurse?
         end
 
         def flush
@@ -679,27 +679,54 @@ module Puppet
             @parameters.include?(:purge) and (self[:purge] == :true or self[:purge] == "true")
         end
 
+        # Recurse the target of the link.
+        def recurse_link
+            perform_recursion(self[:target])
+        end
+
+        # Recurse the file itself, returning a Metadata instance for every found file.
+        def recurse_local
+            perform_recursion(self[:path])
+        end
+
+        # Recurse against our remote file.
+        def recurse_remote
+            total = self[:source].collect do |source|
+                next unless result = perform_recursion(source)
+                result.each { |data| data.source = "%s/%s" % [source, data.relative_path] }
+                return result if result and ! result.empty? and self[:sourceselect] == :first
+                result
+            end.flatten
+
+            # This only happens if we have sourceselect == :all
+            found = []
+            total.find_all do |data|
+                result = ! found.include?(data.relative_path)
+                found << data.relative_path unless found.include?(data.relative_path)
+                result
+            end
+        end
+
+        def perform_recursion(path)
+            Puppet::FileServing::Metadata.search(self[:path], :links => self[:links], :recurse => self[:recurse], :ignore => self[:ignore])
+        end
+
         # Recurse into the directory.  This basically just calls 'localrecurse'
         # and maybe 'sourcerecurse', returning the collection of generated
         # files.
         def recurse
-            # are we at the end of the recursion?
-            return unless self.recurse?
-
-            recurse = self[:recurse]
-            # we might have a string, rather than a number
-            if recurse.is_a?(String)
-                if recurse =~ /^[0-9]+$/
-                    recurse = Integer(recurse)
-                else # anything else is infinite recursion
-                    recurse = true
-                end
+            children = recurse_local
+
+            if self[:target]
+                children += recurse_link 
             end
 
-            if recurse.is_a?(Integer)
-                recurse -= 1
+            if self[:source]
+                recurse_remote 
             end
-            
+
+            return children.collect { |child| newchild(child.relative_path) }.reject { |child| child.nil? }
+
             children = []
             
             # We want to do link-recursing before normal recursion so that all
diff --git a/lib/puppet/type/file/source.rb b/lib/puppet/type/file/source.rb
index 5eefb1dcb..03cc311b4 100755
--- a/lib/puppet/type/file/source.rb
+++ b/lib/puppet/type/file/source.rb
@@ -66,8 +66,14 @@ module Puppet
         uncheckable
         
         validate do |source|
-            unless @resource.uri2obj(source)
-                raise Puppet::Error, "Invalid source %s" % source
+            begin
+                uri = URI.parse(URI.escape(source))
+            rescue => detail
+                self.fail "Could not understand source %s: %s" % [source, detail.to_s]
+            end
+
+            unless uri.scheme.nil? or %w{file puppet}.include?(uri.scheme)
+                self.fail "Cannot use URLs of type '%s' as source for fileserving" % [uri.scheme]
             end
         end
             
diff --git a/spec/unit/type/file.rb b/spec/unit/type/file.rb
index 8b7bedee6..7bbd2a4b2 100755
--- a/spec/unit/type/file.rb
+++ b/spec/unit/type/file.rb
@@ -78,4 +78,195 @@ describe Puppet::Type.type(:file) do
             @resource.flush
         end
     end
+
+    it "should have a method for performing recursion" do
+        @file.must respond_to(:perform_recursion)
+    end
+
+    describe "when executing a recursive search" do
+        it "should use Metadata to do its recursion" do
+            Puppet::FileServing::Metadata.expects(:search)
+            @file.perform_recursion(@file[:path])
+        end
+
+        it "should use its path as the key to the search" do
+            Puppet::FileServing::Metadata.expects(:search).with { |key, options| key = @file[:path] }
+            @file.perform_recursion(@file[:path])
+        end
+
+        it "should return the results of the metadata search" do
+            Puppet::FileServing::Metadata.expects(:search).returns "foobar"
+            @file.perform_recursion(@file[:path]).should == "foobar"
+        end
+
+        it "should pass its recursion value to the search" do
+            @file[:recurse] = 10
+            Puppet::FileServing::Metadata.expects(:search).with { |key, options| options[:recurse] == 10 }
+            @file.perform_recursion(@file[:path])
+        end
+
+        it "should configure the search to ignore or manage links" do
+            @file[:links] = :manage
+            Puppet::FileServing::Metadata.expects(:search).with { |key, options| options[:links] == :manage }
+            @file.perform_recursion(@file[:path])
+        end
+
+        it "should pass its 'ignore' setting to the search if it has one" do
+            @file[:ignore] = %w{.svn CVS}
+            Puppet::FileServing::Metadata.expects(:search).with { |key, options| options[:ignore] == %w{.svn CVS} }
+            @file.perform_recursion(@file[:path])
+        end
+    end
+
+    it "should have a method for performing local recursion" do
+        @file.must respond_to(:recurse_local)
+    end
+
+    it "should pass its path to the :perform_recursion method to do local recursion" do
+        @file.expects(:perform_recursion).with(@file[:path]).returns "foobar"
+        @file.recurse_local.should == "foobar"
+    end
+
+    it "should have a method for performing link recursion" do
+        @file.must respond_to(:recurse_link)
+    end
+
+    it "should pass its target to the :perform_recursion method to do link recursion" do
+        @file[:target] = "mylinks"
+        @file.expects(:perform_recursion).with("mylinks").returns "foobar"
+        @file.recurse_link.should == "foobar"
+    end
+
+    it "should have a method for performing remote recursion" do
+        @file.must respond_to(:recurse_remote)
+    end
+
+    it "should pass its source to the :perform_recursion method to do source recursion" do
+        data = Puppet::FileServing::Metadata.new("/whatever", :relative_path => "foobar")
+        @file[:source] = "puppet://foo/bar"
+        @file.expects(:perform_recursion).with("puppet://foo/bar").returns [data]
+        @file.recurse_remote.should == [data]
+    end
+
+    it "should set the source of each returned file to the searched-for URI plus the found relative path" do
+        metadata = stub 'metadata', :relative_path => "foobar"
+        metadata.expects(:source=).with "puppet://foo/bar/foobar"
+        @file[:source] = "puppet://foo/bar"
+        @file.expects(:perform_recursion).with("puppet://foo/bar").returns [metadata]
+        @file.recurse_remote.should == [metadata]
+    end
+
+    describe "when multiple sources are provided" do
+        describe "and :sourceselect is set to :first" do
+            it "should return the results for the first source to return any values" do
+                data = Puppet::FileServing::Metadata.new("/whatever", :relative_path => "foobar")
+                @file[:source] = %w{/one /two /three /four}
+                @file.expects(:perform_recursion).with("/one").returns nil
+                @file.expects(:perform_recursion).with("/two").returns []
+                @file.expects(:perform_recursion).with("/three").returns [data]
+                @file.expects(:perform_recursion).with("/four").never
+                @file.recurse_remote.should == [data]
+            end
+        end
+
+        describe "and :sourceselect is set to :all" do
+            before do
+                @file[:sourceselect] = :all
+            end
+
+            it "should return every found file that is not in a previous source" do
+                klass = Puppet::FileServing::Metadata
+                @file[:source] = %w{/one /two /three /four}
+
+                one = [klass.new("/one", :relative_path => "a")]
+                @file.expects(:perform_recursion).with("/one").returns one
+
+                two = [klass.new("/two", :relative_path => "a"), klass.new("/two", :relative_path => "b")]
+                @file.expects(:perform_recursion).with("/two").returns two
+
+                three = [klass.new("/three", :relative_path => "a"), klass.new("/three", :relative_path => "c")]
+                @file.expects(:perform_recursion).with("/three").returns three
+
+                @file.expects(:perform_recursion).with("/four").returns []
+
+                @file.recurse_remote.should == [one[0], two[1], three[1]]
+            end
+        end
+    end
+
+    it "should recurse during eval_generate if recursion is enabled" do
+        @file.expects(:recurse?).returns true
+        @file.expects(:recurse).returns "foobar"
+        @file.eval_generate.should == "foobar"
+    end
+
+    it "should not recurse during eval_generate if recursion is disabled" do
+        @file.expects(:recurse?).returns false
+        @file.expects(:recurse).never
+        @file.eval_generate.should be_nil
+    end
+
+    describe "when recursing" do
+        before do
+            @file[:recurse] = true
+            @metadata = Puppet::FileServing::Metadata
+        end
+
+        describe "and a source is set" do
+            before { @file[:source] = "/my/source" }
+
+            it "should use recurse_remote" do
+                @file.stubs(:recurse_local).returns []
+                @file.expects(:recurse_remote)
+                @file.recurse
+            end
+
+            it "should create a new file resource for each remote file"
+
+            it "should set the source for each new file resource"
+
+            it "should copy the metadata to the new file's source property so the file does not have to requery the remote system for metadata"
+
+            describe "and purging is enabled" do
+                it "should configure each file not on the remote system to be removed"
+            end
+        end
+
+        describe "and a target is set" do
+            before { @file[:target] = "/link/target" }
+
+            it "should use recurse_link" do
+                @file.stubs(:recurse_local).returns []
+                @file.expects(:recurse_link).returns []
+                @file.recurse
+            end
+
+            it "should return a new file resource for each link destination found"
+
+            it "should set the target for each new file resource"
+        end
+
+        it "should use recurse_local" do
+            @file.expects(:recurse_local).returns []
+            @file.recurse
+        end
+
+        it "should attempt to turn each found file into a child resource" do
+            a = @metadata.new("/foo", :relative_path => "a")
+            @file.expects(:recurse_local).returns [a]
+            @file.expects(:newchild).with("a")
+
+            @file.recurse
+        end
+
+        it "should not return nil for those files that could not be turned into children" do
+            a = @metadata.new("/foo", :relative_path => "a")
+            b = @metadata.new("/foo", :relative_path => "b")
+            @file.expects(:recurse_local).returns [a, b]
+            @file.expects(:newchild).with("a").returns "A"
+            @file.expects(:newchild).with("b").returns nil
+
+            @file.recurse.should == ["A"]
+        end
+    end
 end
diff --git a/spec/unit/type/file/source.rb b/spec/unit/type/file/source.rb
index d6aa25fe7..7c9850822 100755
--- a/spec/unit/type/file/source.rb
+++ b/spec/unit/type/file/source.rb
@@ -15,9 +15,16 @@ describe Puppet::Type.type(:file).attrclass(:source) do
 
     describe "when initializing" do
         it "should fail if the 'should' values are not URLs" do
-            @resource.expects(:uri2obj).with("foo").returns false
+            s = source.new(:resource => @resource)
+            URI.expects(:parse).with('foo').raises RuntimeError
 
-            lambda { source.new(:resource => @resource, :should => %w{foo}) }.must raise_error(Puppet::Error)
+            lambda { s.should = %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)
         end
     end