Link handling is now in the file serving classes.

This was done by putting all of the functionality in the
Content and Metadata class (actually, in a new base class
for them).

There are still some issues, and there need to be integration
tests between the :local (soon to be renamed :file) termini for
these classes.

10 files changed, 268 insertions, 55 deletions

diff --git a/spec/integration/indirector/module_files.rb b/spec/integration/indirector/module_files.rb
index 67209fb39..3725a1286 100755
--- a/spec/integration/indirector/module_files.rb
+++ b/spec/integration/indirector/module_files.rb
@@ -19,7 +19,7 @@ describe Puppet::Indirector::ModuleFiles, " when interacting with Puppet::Module
 
         FileTest.expects(:exists?).with(filepath).returns(true)
 
-        @terminus.model.expects(:new).with(filepath)
+        @terminus.model.expects(:new).with(filepath, :links => nil)
 
         @terminus.find("puppetmounts://host/modules/mymod/myfile")
     end
diff --git a/spec/lib/shared_behaviours/file_server_terminus.rb b/spec/lib/shared_behaviours/file_server_terminus.rb
index c18d74f81..e1ec35251 100644
--- a/spec/lib/shared_behaviours/file_server_terminus.rb
+++ b/spec/lib/shared_behaviours/file_server_terminus.rb
@@ -32,7 +32,7 @@ describe "Puppet::Indirector::FileServerTerminus", :shared => true do
 
         path = "/my/mount/path/my/file"
         FileTest.stubs(:exists?).with(path).returns(true)
-        @test_class.expects(:new).with(path).returns(:myinstance)
+        @test_class.expects(:new).with(path, :links => nil).returns(:myinstance)
         FileTest.stubs(:exists?).with("/my/mount/path").returns(true)
         @mount1.expects(:file).with("my/file", :node => nil).returns(path)
 
diff --git a/spec/unit/file_serving/content.rb b/spec/unit/file_serving/content.rb
index 593278bf4..e15aa8be6 100755
--- a/spec/unit/file_serving/content.rb
+++ b/spec/unit/file_serving/content.rb
@@ -5,6 +5,10 @@ require File.dirname(__FILE__) + '/../../spec_helper'
 require 'puppet/file_serving/content'
 
 describe Puppet::FileServing::Content do
+    it "should should be a subclass of FileBase" do
+        Puppet::FileServing::Content.superclass.should equal(Puppet::FileServing::FileBase)
+    end
+
     it "should indirect file_content" do
         Puppet::FileServing::Content.indirection.name.should == :file_content
     end
@@ -15,41 +19,80 @@ describe Puppet::FileServing::Content do
 end
 
 describe Puppet::FileServing::Content, " when initializing" do
+    it "should accept a file path" do
+        Puppet::FileServing::Content.new("not/qualified").path.should == "not/qualified"
+    end
+
+    it "should not allow a fully qualified file path" do
+        proc { Puppet::FileServing::Content.new("/fully/qualified") }.should raise_error(ArgumentError)
+    end
+
+    it "should allow specification of whether links should be managed" do
+        Puppet::FileServing::Content.new("not/qualified", :links => :manage)
+    end
+
+    it "should fail if :links is set to anything other than :manage or :follow" do
+        Puppet::FileServing::Content.new("not/qualified", :links => :manage)
+    end
+
+    it "should default to :manage for :links" do
+        Puppet::FileServing::Content.new("not/qualified", :links => :manage)
+    end
+end
+
+describe Puppet::FileServing::Content, " when returning the contents" do
     before do
-        @path = "/my/file"
+        @content = Puppet::FileServing::Content.new("sub/path", :links => :follow)
+        @base = "/my/base"
+        @full = "/my/base/sub/path"
     end
 
-    it "should accept a file path" do
-        FileTest.expects(:exists?).with(@path).returns(true)
-        Puppet::FileServing::Content.new(@path).path.should == @path
+    it "should fail if the file is a symlink and links are set to :manage" do
+        @content.links = :manage
+        File.expects(:lstat).with(@full).returns stub("stat", :ftype => "symlink")
+        proc { @content.content(@base) }.should raise_error(ArgumentError)
     end
 
-    it "should require a fully qualified file path" do
-        proc { Puppet::FileServing::Content.new("unqualified") }.should raise_error(ArgumentError)
+    it "should accept a base path path to which the file should be relative" do
+        File.expects(:stat).with(@full).returns stub("stat", :ftype => "file")
+        File.expects(:read).with(@full).returns(:mycontent)
+        @content.content(@base).should == :mycontent
     end
 
-    it "should require the path to exist" do
-        FileTest.expects(:exists?).with(@path).returns(false)
-        proc { Puppet::FileServing::Content.new(@path) }.should raise_error(ArgumentError)
+    it "should use the set base path if one is not provided" do
+        @content.base_path = @base
+        File.expects(:stat).with(@full).returns stub("stat", :ftype => "file")
+        File.expects(:read).with(@full).returns(:mycontent)
+        @content.content()
     end
 
-    it "should not stat the file" do
-        FileTest.expects(:exists?).with(@path).returns(true)
-        File.expects(:read).with(@path).never
-        Puppet::FileServing::Content.new(@path)
+    it "should fail if a base path is neither set nor provided" do
+        proc { @content.content() }.should raise_error(ArgumentError)
+    end
+
+    it "should raise Errno::ENOENT if the file is absent" do
+        @content.base_path = "/there/is/absolutely/no/chance/that/this/path/exists"
+        proc { @content.content() }.should raise_error(Errno::ENOENT)
+    end
+
+    it "should return the contents of the path if the file exists" do
+        File.expects(:stat).with(@full).returns stub("stat", :ftype => "file")
+        File.expects(:read).with(@full).returns(:mycontent)
+        @content.content(@base).should == :mycontent
     end
 end
 
 describe Puppet::FileServing::Content, " when converting to yaml" do
-    before do
-        @path = "/my/file"
-        FileTest.expects(:exists?).with(@path).returns(true)
-        @content = Puppet::FileServing::Content.new(@path)
+    it "should fail if no base path has been set" do
+        @content = Puppet::FileServing::Content.new("some/path")
+        proc { @content.to_yaml }.should raise_error(ArgumentError)
     end
 
     it "should return the file contents" do
-        File.expects(:read).with(@path).returns("mycontent")
-        @content.to_yaml.should == "mycontent"
+        @content = Puppet::FileServing::Content.new("some/path")
+        @content.base_path = "/base/path"
+        @content.expects(:content).returns(:content)
+        @content.to_yaml.should == :content
     end
 end
 
diff --git a/spec/unit/file_serving/file_base.rb b/spec/unit/file_serving/file_base.rb
new file mode 100755
index 000000000..14be6d003
--- /dev/null
+++ b/spec/unit/file_serving/file_base.rb
@@ -0,0 +1,85 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+require 'puppet/file_serving/file_base'
+
+describe Puppet::FileServing::FileBase, " when initializing" do
+    it "should accept a file path" do
+        Puppet::FileServing::FileBase.new("not/qualified").path.should == "not/qualified"
+    end
+
+    it "should not allow a fully qualified file path" do
+        proc { Puppet::FileServing::FileBase.new("/fully/qualified") }.should raise_error(ArgumentError)
+    end
+
+    it "should allow specification of whether links should be managed" do
+        Puppet::FileServing::FileBase.new("not/qualified", :links => :manage).links.should == :manage
+    end
+
+    it "should fail if :links is set to anything other than :manage or :follow" do
+        proc { Puppet::FileServing::FileBase.new("not/qualified", :links => :else) }.should raise_error(ArgumentError)
+    end
+
+    it "should default to :manage for :links" do
+        Puppet::FileServing::FileBase.new("not/qualified").links.should == :manage
+    end
+end
+
+describe Puppet::FileServing::FileBase do
+    it "should provide a method for setting the base path" do
+        @file = Puppet::FileServing::FileBase.new("not/qualified")
+        @file.base_path = "/something"
+        @file.base_path.should == "/something"
+    end
+end
+
+describe Puppet::FileServing::FileBase, " when determining the full file path" do
+    it "should return the provided path joined with the qualified path if a path is provided" do
+        @file = Puppet::FileServing::FileBase.new("not/qualified")
+        @file.full_path("/this/file").should == "/this/file/not/qualified"
+    end
+
+    it "should return the set base path joined with the qualified path if a base path is set" do
+        @file = Puppet::FileServing::FileBase.new("not/qualified")
+        @file.base_path = "/this/file"
+        @file.full_path.should == "/this/file/not/qualified"
+    end
+
+    it "should should fail if a base path is neither provided nor set" do
+        @file = Puppet::FileServing::FileBase.new("not/qualified")
+        proc { @file.full_path }.should raise_error(ArgumentError)
+    end
+end
+
+describe Puppet::FileServing::FileBase, " when stat'ing files" do
+    before do
+        @file = Puppet::FileServing::FileBase.new("not/qualified")
+    end
+
+    it "should join the provided path with the qualified path is a path is provided" do
+        File.expects(:lstat).with("/this/file/not/qualified").returns stub("stat", :ftype => "file")
+        @file.stat("/this/file")
+    end
+
+    it "should use the set base path if no base is provided" do
+        @file.base_path = "/this/file"
+        File.expects(:lstat).with("/this/file/not/qualified").returns stub("stat", :ftype => "file")
+        @file.stat
+    end
+
+    it "should fail if a base path is neither set nor provided" do
+        proc { @file.stat }.should raise_error(ArgumentError)
+    end
+
+    it "should use :lstat if :links is set to :manage" do
+        File.expects(:lstat).with("/this/file/not/qualified").returns stub("stat", :ftype => "file")
+        @file.stat("/this/file")
+    end
+
+    it "should use :stat if :links is set to :follow" do
+        File.expects(:stat).with("/this/file/not/qualified").returns stub("stat", :ftype => "file")
+        @file.links = :follow
+        @file.stat("/this/file")
+    end
+end
diff --git a/spec/unit/file_serving/metadata.rb b/spec/unit/file_serving/metadata.rb
index 1237c3184..27ebe2471 100755
--- a/spec/unit/file_serving/metadata.rb
+++ b/spec/unit/file_serving/metadata.rb
@@ -15,40 +15,80 @@ describe Puppet::FileServing::Metadata do
 end
 
 describe Puppet::FileServing::Metadata, " when initializing" do
-    it "should allow initialization without a path" do
-        proc { Puppet::FileServing::Metadata.new() }.should_not raise_error
+    it "should not allow initialization without a path" do
+        proc { Puppet::FileServing::Metadata.new() }.should raise_error(ArgumentError)
     end
 
-    it "should allow initialization with a path" do
-        proc { Puppet::FileServing::Metadata.new("unqualified") }.should raise_error(ArgumentError)
+    it "should not allow the path to be fully qualified if it is provided" do
+        proc { Puppet::FileServing::Metadata.new("/fully/qualified") }.should raise_error(ArgumentError)
     end
 
-    it "should the path to be fully qualified if it is provied" do
-        proc { Puppet::FileServing::Metadata.new("unqualified") }.should raise_error(ArgumentError)
+    it "should allow initialization with a relative path" do
+        Puppet::FileServing::Metadata.new("not/fully/qualified")
     end
 
-    it "should require the path to exist if it is provided" do
-        FileTest.expects(:exists?).with("/no/such/path").returns(false)
-        proc { Puppet::FileServing::Metadata.new("/no/such/path") }.should raise_error(ArgumentError)
+    it "should allow specification of whether links should be managed" do
+        Puppet::FileServing::Metadata.new("not/qualified", :links => :manage)
+    end
+
+    it "should fail if :links is set to anything other than :manage or :follow" do
+        Puppet::FileServing::Metadata.new("not/qualified", :links => :manage)
+    end
+
+    it "should default to :manage for :links" do
+        Puppet::FileServing::Metadata.new("not/qualified", :links => :manage)
     end
 end
 
-describe Puppet::FileServing::Metadata do
+describe Puppet::FileServing::Metadata, " when finding the file to use for setting attributes" do
+    before do
+        @metadata = Puppet::FileServing::Metadata.new("my/path")
+
+        @base = "/base/path"
+        @full = "/base/path/my/path"
+
+        # Use a symlink because it's easier to test -- no checksumming
+        @stat = stub "stat", :uid => 10, :gid => 20, :mode => 0755, :ftype => "symlink"
+    end
+
+    it "should accept a base path path to which the file should be relative" do
+        File.expects(:lstat).with(@full).returns @stat
+        File.expects(:readlink).with(@full).returns "/what/ever"
+        @metadata.collect_attributes(@base)
+    end
+
+    it "should use the set base path if one is not provided" do
+        @metadata.base_path = @base
+        File.expects(:lstat).with(@full).returns @stat
+        File.expects(:readlink).with(@full).returns "/what/ever"
+        @metadata.collect_attributes()
+    end
+
+    it "should fail if a base path is neither set nor provided" do
+        proc { @metadata.collect_attributes() }.should raise_error(ArgumentError)
+    end
+
+    it "should raise an exception if the file does not exist" do
+        File.expects(:lstat).with("/base/dir/my/path").raises(Errno::ENOENT)
+        proc { @metadata.collect_attributes("/base/dir")}.should raise_error(Errno::ENOENT)
+    end
+end
+
+describe Puppet::FileServing::Metadata, " when collecting attributes" do
     before do
         @path = "/my/file"
-        @stat = mock 'stat', :uid => 10, :gid => 20, :mode => 0755
-        File.stubs(:stat).returns(@stat)
+        @stat = stub 'stat', :uid => 10, :gid => 20, :mode => 0755, :ftype => "file"
+        File.stubs(:lstat).returns(@stat)
         @filehandle = mock 'filehandle'
         @filehandle.expects(:each_line).yields("some content\n")
         File.stubs(:open).with(@path, 'r').yields(@filehandle)
         @checksum = Digest::MD5.hexdigest("some content\n")
-        FileTest.expects(:exists?).with(@path).returns(true)
-        @metadata = Puppet::FileServing::Metadata.new(@path)
-        @metadata.get_attributes
+        @metadata = Puppet::FileServing::Metadata.new("file")
+        @metadata.collect_attributes("/my")
     end
 
     it "should accept a file path" do
-        @metadata.path.should == @path
+        @metadata.path.should == "file"
     end
 
     # LAK:FIXME This should actually change at some point
@@ -86,6 +126,28 @@ describe Puppet::FileServing::Metadata do
     end
 end
 
+describe Puppet::FileServing::Metadata, " when pointing to a symlink" do
+    it "should store the destination of the symlink in :destination if links are :manage" do
+        file = Puppet::FileServing::Metadata.new("my/file", :links => :manage)
+
+        File.expects(:lstat).with("/base/path/my/file").returns stub("stat", :uid => 1, :gid => 2, :ftype => "symlink", :mode => 0755)
+        File.expects(:readlink).with("/base/path/my/file").returns "/some/other/path"
+
+        file.collect_attributes("/base/path")
+        file.destination.should == "/some/other/path"
+    end
+
+    it "should not collect the checksum" do
+        file = Puppet::FileServing::Metadata.new("my/file", :links => :manage)
+
+        File.expects(:lstat).with("/base/path/my/file").returns stub("stat", :uid => 1, :gid => 2, :ftype => "symlink", :mode => 0755)
+        File.expects(:readlink).with("/base/path/my/file").returns "/some/other/path"
+
+        file.collect_attributes("/base/path")
+        file.checksum.should be_nil
+    end
+end
+
 describe Puppet::FileServing::Metadata, " when converting from yaml" do
     # LAK:FIXME This isn't in the right place, but we need some kind of
     # control somewhere that requires that all REST connections only pull
diff --git a/spec/unit/file_serving/mount.rb b/spec/unit/file_serving/mount.rb
index e9a7f6ddc..ebe058301 100755
--- a/spec/unit/file_serving/mount.rb
+++ b/spec/unit/file_serving/mount.rb
@@ -105,10 +105,6 @@ describe Puppet::FileServing::Mount, " when finding files" do
         @mount.path().should == "/myhost/mydomain.com/myhost.mydomain.com"
     end
 
-    it "should ignore links by default"
-
-    it "should follow links when asked"
-
     after do
         Puppet::FileServing::Mount.clear_cache
     end
diff --git a/spec/unit/indirector/file_content/local.rb b/spec/unit/indirector/file_content/local.rb
index ca7202ae1..442667518 100755
--- a/spec/unit/indirector/file_content/local.rb
+++ b/spec/unit/indirector/file_content/local.rb
@@ -23,10 +23,19 @@ describe Puppet::Indirector::FileContent::Local, "when finding a single file" do
         @uri = "file:///my/local"
 
         FileTest.expects(:exists?).with("/my/local").returns true
-        Puppet::FileServing::Content.expects(:new).with("/my/local").returns(:mycontent)
+        Puppet::FileServing::Content.expects(:new).with("/my/local", :links => nil).returns(:mycontent)
         @content.find(@uri).should == :mycontent
     end
 
+    it "should pass the :links setting on to the created Content instance if the file exists" do
+        @content = Puppet::Indirector::FileContent::Local.new
+        @uri = "file:///my/local"
+
+        FileTest.expects(:exists?).with("/my/local").returns true
+        Puppet::FileServing::Content.expects(:new).with("/my/local", :links => :manage).returns(:mycontent)
+        @content.find(@uri, :links => :manage)
+    end
+
     it "should return nil if the file does not exist" do
         @content = Puppet::Indirector::FileContent::Local.new
         @uri = "file:///my/local"
diff --git a/spec/unit/indirector/file_metadata/local.rb b/spec/unit/indirector/file_metadata/local.rb
index f613d35c7..289aee449 100755
--- a/spec/unit/indirector/file_metadata/local.rb
+++ b/spec/unit/indirector/file_metadata/local.rb
@@ -20,19 +20,28 @@ describe Puppet::Indirector::FileMetadata::Local, "when finding a single file" d
 
         @data = mock 'metadata'
     end
+
     it "should return a Metadata instance created with the full path to the file if the file exists" do
-        @data.stubs(:get_attributes)
+        @data.stubs(:collect_attributes)
 
         FileTest.expects(:exists?).with("/my/local").returns true
-        Puppet::FileServing::Metadata.expects(:new).with("/my/local").returns(@data)
+        Puppet::FileServing::Metadata.expects(:new).with("/my/local", :links => nil).returns(@data)
         @metadata.find(@uri).should == @data
     end
 
+    it "should pass the :links setting on to the created Content instance if the file exists" do
+        @data.stubs(:collect_attributes)
+
+        FileTest.expects(:exists?).with("/my/local").returns true
+        Puppet::FileServing::Metadata.expects(:new).with("/my/local", :links => :manage).returns(@data)
+        @metadata.find(@uri, :links => :manage)
+    end
+
     it "should collect its attributes when a file is found" do
-        @data.expects(:get_attributes)
+        @data.expects(:collect_attributes)
 
         FileTest.expects(:exists?).with("/my/local").returns true
-        Puppet::FileServing::Metadata.expects(:new).with("/my/local").returns(@data)
+        Puppet::FileServing::Metadata.expects(:new).with("/my/local", :links => nil).returns(@data)
         @metadata.find(@uri).should == @data
     end
 
@@ -67,7 +76,7 @@ describe Puppet::Indirector::FileMetadata::Local, "when searching for multiple f
 
     it "should collect the attributes of the instances returned" do
         FileTest.expects(:exists?).with("/my/local").returns true
-        @metadata.expects(:path2instances).with("/my/local", {}).returns( [mock("one", :get_attributes => nil), mock("two", :get_attributes => nil)] )
+        @metadata.expects(:path2instances).with("/my/local", {}).returns( [mock("one", :collect_attributes => nil), mock("two", :collect_attributes => nil)] )
         @metadata.search(@uri)
     end
 end
diff --git a/spec/unit/indirector/file_server.rb b/spec/unit/indirector/file_server.rb
index 4bc56ce7d..ed36e180e 100755
--- a/spec/unit/indirector/file_server.rb
+++ b/spec/unit/indirector/file_server.rb
@@ -54,16 +54,20 @@ describe Puppet::Indirector::FileServer, " when finding files" do
 
     it "should return an instance of the model created with the full path if a file is found" do
         @configuration.expects(:file_path).with("/my/local/file", :node => nil).returns("/some/file")
-        @model.expects(:new).with("/some/file").returns(:myinstance)
+        @model.expects(:new).with("/some/file", :links => nil).returns(:myinstance)
         @file_server.find(@uri).should == :myinstance
     end
 end
 
 
-describe Puppet::Indirector::FileServer, " when returning file paths" do
-    it "should follow links if the links option is set to :follow"
+describe Puppet::Indirector::FileServer, " when returning instances" do
+    include FileServerTerminusTesting
 
-    it "should ignore links if the links option is not set to follow"
+    it "should pass the provided :links setting on to the instance if one is provided" do
+        @configuration.expects(:file_path).with("/my/local/file", :node => nil).returns("/some/file")
+        @model.expects(:new).with("/some/file", :links => :mytest)
+        @file_server.find(@uri, :links => :mytest)
+    end
 end
 
 describe Puppet::Indirector::FileServer, " when checking authorization" do
diff --git a/spec/unit/indirector/module_files.rb b/spec/unit/indirector/module_files.rb
index e65f15697..f2e43f771 100755
--- a/spec/unit/indirector/module_files.rb
+++ b/spec/unit/indirector/module_files.rb
@@ -65,7 +65,7 @@ describe Puppet::Indirector::ModuleFiles, " when finding files" do
     it "should return an instance of the model created with the full path if a module is found and the file exists" do
         Puppet::Module.expects(:find).with('my', nil).returns @module
         FileTest.expects(:exists?).with("/module/path/files/local/file").returns(true)
-        @model.expects(:new).with("/module/path/files/local/file").returns(:myinstance)
+        @model.expects(:new).with("/module/path/files/local/file", :links => nil).returns(:myinstance)
         @module_files.find(@uri).should == :myinstance
     end
 
@@ -89,10 +89,15 @@ describe Puppet::Indirector::ModuleFiles, " when finding files" do
     end
 end
 
-describe Puppet::Indirector::ModuleFiles, " when returning file paths" do
-    it "should follow links if the links option is set to :follow"
+describe Puppet::Indirector::ModuleFiles, " when returning instances" do
+    include ModuleFilesTerminusTesting
 
-    it "should ignore links if the links option is not set to follow"
+    it "should pass the provided :links setting on to the instance if one is provided" do
+        Puppet::Module.expects(:find).with('my', nil).returns @module
+        FileTest.expects(:exists?).with("/module/path/files/local/file").returns(true)
+        @model.expects(:new).with("/module/path/files/local/file", :links => :mytest)
+        @module_files.find(@uri, :links => :mytest)
+    end
 end
 
 describe Puppet::Indirector::ModuleFiles, " when authorizing" do