Moved perform_initial_import from Puppet::Resource::TypeCollection to Puppet::Node::Environment.

This change is part of an ongoing effort to remove functionality from
TypeCollection that is not related to keeping track of a collection of
types.  This reduces TypeCollection's linkage to the environment,
which is a step toward decoupling it from the type loading mechanism.

Also, added a spec test to verify that TypeCollection.version is
correctly recomputed when types are re-imported.

7 files changed, 96 insertions, 86 deletions

diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
index 3f67474f9..ad11a0476 100644
--- a/lib/puppet/node/environment.rb
+++ b/lib/puppet/node/environment.rb
@@ -81,7 +81,7 @@ class Puppet::Node::Environment
     Thread.current[:known_resource_types] ||= synchronize {
       if @known_resource_types.nil? or @known_resource_types.stale?
         @known_resource_types = Puppet::Resource::TypeCollection.new(self)
-        @known_resource_types.perform_initial_import
+        perform_initial_import
       end
       @known_resource_types
     }
@@ -143,5 +143,25 @@ class Puppet::Node::Environment
     end
   end
 
+  private
+
+  def perform_initial_import
+    return if Puppet.settings[:ignoreimport]
+    parser = Puppet::Parser::Parser.new(self)
+    if code = Puppet.settings.uninterpolated_value(:code, name.to_s) and code != ""
+      parser.string = code
+    else
+      file = Puppet.settings.value(:manifest, name.to_s)
+      return unless File.exist?(file)
+      parser.file = file
+    end
+    parser.parse
+  rescue => detail
+    msg = "Could not parse for environment #{self}: #{detail}"
+    error = Puppet::Error.new(msg)
+    error.set_backtrace(detail.backtrace)
+    raise error
+  end
+
   @root = new(:'*root*')
 end
diff --git a/lib/puppet/resource/type_collection.rb b/lib/puppet/resource/type_collection.rb
index 3a327e264..4254b037e 100644
--- a/lib/puppet/resource/type_collection.rb
+++ b/lib/puppet/resource/type_collection.rb
@@ -110,24 +110,6 @@ class Puppet::Resource::TypeCollection
     end
   end
 
-  def perform_initial_import
-    return if Puppet.settings[:ignoreimport]
-    parser = Puppet::Parser::Parser.new(environment)
-    if code = Puppet.settings.uninterpolated_value(:code, environment.to_s) and code != ""
-      parser.string = code
-    else
-      file = Puppet.settings.value(:manifest, environment.to_s)
-      return unless File.exist?(file)
-      parser.file = file
-    end
-    parser.parse
-  rescue => detail
-    msg = "Could not parse for environment #{environment}: #{detail}"
-    error = Puppet::Error.new(msg)
-    error.set_backtrace(detail.backtrace)
-    raise error
-  end
-
   def stale?
     @watched_files.values.detect { |file| file.changed? }
   end
diff --git a/spec/integration/parser/compiler_spec.rb b/spec/integration/parser/compiler_spec.rb
index 9158ad1c2..67e8e5e62 100755
--- a/spec/integration/parser/compiler_spec.rb
+++ b/spec/integration/parser/compiler_spec.rb
@@ -69,4 +69,15 @@ describe Puppet::Parser::Compiler do
       notify_resource[:require].title.should == "Experiment::Baz"
     end
   end
+
+  it "should recompute the version after input files are re-parsed" do
+    Puppet[:code] = 'class foo { }'
+    Time.stubs(:now).returns(1)
+    node = Puppet::Node.new('mynode')
+    Puppet::Parser::Compiler.compile(node).version.should == 1
+    Time.stubs(:now).returns(2)
+    Puppet::Parser::Compiler.compile(node).version.should == 1 # no change because files didn't change
+    Puppet::Resource::TypeCollection.any_instance.stubs(:stale?).returns(true).then.returns(false) # pretend change
+    Puppet::Parser::Compiler.compile(node).version.should == 2
+  end
 end
diff --git a/spec/unit/application/apply_spec.rb b/spec/unit/application/apply_spec.rb
index 0b00d1a2e..a79ddbad3 100755
--- a/spec/unit/application/apply_spec.rb
+++ b/spec/unit/application/apply_spec.rb
@@ -142,17 +142,16 @@ describe Puppet::Application::Apply do
 
     describe "the parseonly command" do
       before :each do
-        Puppet.stubs(:[]).with(:environment)
+        @environment = Puppet::Node::Environment.new("env")
+        Puppet.stubs(:[]).with(:environment).returns(@environment)
         Puppet.stubs(:[]).with(:manifest).returns("site.pp")
         Puppet.stubs(:err)
         @apply.stubs(:exit)
         @apply.options.stubs(:[]).with(:code).returns "some code"
-        @collection = stub_everything
-        Puppet::Resource::TypeCollection.stubs(:new).returns(@collection)
       end
 
-      it "should use a Puppet Resource Type Collection to parse the file" do
-        @collection.expects(:perform_initial_import)
+      it "should use the environment to parse the file" do
+        @environment.stubs(:perform_initial_import)
         @apply.parseonly
       end
 
@@ -162,7 +161,7 @@ describe Puppet::Application::Apply do
       end
 
       it "should exit with exit code 1 if error" do
-        @collection.stubs(:perform_initial_import).raises(Puppet::ParseError)
+        @environment.stubs(:perform_initial_import).raises(Puppet::ParseError)
         @apply.expects(:exit).with(1)
         @apply.parseonly
       end
diff --git a/spec/unit/application/master_spec.rb b/spec/unit/application/master_spec.rb
index 0baa8229b..d23021317 100644
--- a/spec/unit/application/master_spec.rb
+++ b/spec/unit/application/master_spec.rb
@@ -257,16 +257,15 @@ describe Puppet::Application::Master do
 
     describe "the parseonly command" do
       before :each do
-        Puppet.stubs(:[]).with(:environment)
+        @environment = Puppet::Node::Environment.new("env")
+        Puppet.stubs(:[]).with(:environment).returns(@environment)
         Puppet.stubs(:[]).with(:manifest).returns("site.pp")
         Puppet.stubs(:err)
         @master.stubs(:exit)
-        @collection = stub_everything
-        Puppet::Resource::TypeCollection.stubs(:new).returns(@collection)
       end
 
       it "should use a Puppet Resource Type Collection to parse the file" do
-        @collection.expects(:perform_initial_import)
+        @environment.expects(:perform_initial_import)
         @master.parseonly
       end
 
@@ -276,7 +275,7 @@ describe Puppet::Application::Master do
       end
 
       it "should exit with exit code 1 if error" do
-        @collection.stubs(:perform_initial_import).raises(Puppet::ParseError)
+        @environment.stubs(:perform_initial_import).raises(Puppet::ParseError)
         @master.expects(:exit).with(1)
         @master.parseonly
       end
diff --git a/spec/unit/node/environment_spec.rb b/spec/unit/node/environment_spec.rb
index 6edcce56c..e495d30a6 100755
--- a/spec/unit/node/environment_spec.rb
+++ b/spec/unit/node/environment_spec.rb
@@ -52,7 +52,7 @@ describe Puppet::Node::Environment do
     before do
       @env = Puppet::Node::Environment.new("dev")
       @collection = Puppet::Resource::TypeCollection.new(@env)
-      @collection.stubs(:perform_initial_import)
+      @env.stubs(:perform_initial_import)
       Thread.current[:known_resource_types] = nil
     end
 
@@ -66,9 +66,8 @@ describe Puppet::Node::Environment do
     end
 
     it "should perform the initial import when creating a new collection" do
-      @collection.expects(:perform_initial_import)
-      Puppet::Resource::TypeCollection.expects(:new).returns @collection
-
+      @env = Puppet::Node::Environment.new("dev")
+      @env.expects(:perform_initial_import)
       @env.known_resource_types
     end
 
@@ -274,4 +273,56 @@ describe Puppet::Node::Environment do
       @helper.environment.name.should == :foo
     end
   end
+
+  describe "when performing initial import" do
+    before do
+      @parser = stub 'parser', :file= => nil, :string => nil, :parse => nil
+      Puppet::Parser::Parser.stubs(:new).returns @parser
+      @env = Puppet::Node::Environment.new("env")
+    end
+
+    it "should create a new parser instance" do
+      Puppet::Parser::Parser.expects(:new).returns @parser
+      @env.instance_eval { perform_initial_import }
+    end
+
+    it "should set the parser's string to the 'code' setting and parse if code is available" do
+      Puppet.settings[:code] = "my code"
+      @parser.expects(:string=).with "my code"
+      @parser.expects(:parse)
+      @env.instance_eval { perform_initial_import }
+    end
+
+    it "should set the parser's file to the 'manifest' setting and parse if no code is available and the manifest is available" do
+      File.stubs(:expand_path).with("/my/file").returns "/my/file"
+      File.expects(:exist?).with("/my/file").returns true
+      Puppet.settings[:manifest] = "/my/file"
+      @parser.expects(:file=).with "/my/file"
+      @parser.expects(:parse)
+      @env.instance_eval { perform_initial_import }
+    end
+
+    it "should not attempt to load a manifest if none is present" do
+      File.stubs(:expand_path).with("/my/file").returns "/my/file"
+      File.expects(:exist?).with("/my/file").returns false
+      Puppet.settings[:manifest] = "/my/file"
+      @parser.expects(:file=).never
+      @parser.expects(:parse).never
+      @env.instance_eval { perform_initial_import }
+    end
+
+    it "should fail helpfully if there is an error importing" do
+      File.stubs(:exist?).returns true
+      @parser.expects(:parse).raises ArgumentError
+      lambda { @env.instance_eval { perform_initial_import } }.should raise_error(Puppet::Error)
+    end
+
+    it "should not do anything if the ignore_import settings is set" do
+      Puppet.settings[:ignoreimport] = true
+      @parser.expects(:string=).never
+      @parser.expects(:file=).never
+      @parser.expects(:parse).never
+      @env.instance_eval { perform_initial_import }
+    end
+  end
 end
diff --git a/spec/unit/resource/type_collection_spec.rb b/spec/unit/resource/type_collection_spec.rb
index 4bad29116..fc53e35b1 100644
--- a/spec/unit/resource/type_collection_spec.rb
+++ b/spec/unit/resource/type_collection_spec.rb
@@ -416,58 +416,6 @@ describe Puppet::Resource::TypeCollection do
     end
   end
 
-  describe "when performing initial import" do
-    before do
-      @parser = stub 'parser', :file= => nil, :string => nil, :parse => nil
-      Puppet::Parser::Parser.stubs(:new).returns @parser
-      @code = Puppet::Resource::TypeCollection.new("env")
-    end
-
-    it "should create a new parser instance" do
-      Puppet::Parser::Parser.expects(:new).returns @parser
-      @code.perform_initial_import
-    end
-
-    it "should set the parser's string to the 'code' setting and parse if code is available" do
-      Puppet.settings[:code] = "my code"
-      @parser.expects(:string=).with "my code"
-      @parser.expects(:parse)
-      @code.perform_initial_import
-    end
-
-    it "should set the parser's file to the 'manifest' setting and parse if no code is available and the manifest is available" do
-      File.stubs(:expand_path).with("/my/file").returns "/my/file"
-      File.expects(:exist?).with("/my/file").returns true
-      Puppet.settings[:manifest] = "/my/file"
-      @parser.expects(:file=).with "/my/file"
-      @parser.expects(:parse)
-      @code.perform_initial_import
-    end
-
-    it "should not attempt to load a manifest if none is present" do
-      File.stubs(:expand_path).with("/my/file").returns "/my/file"
-      File.expects(:exist?).with("/my/file").returns false
-      Puppet.settings[:manifest] = "/my/file"
-      @parser.expects(:file=).never
-      @parser.expects(:parse).never
-      @code.perform_initial_import
-    end
-
-    it "should fail helpfully if there is an error importing" do
-      File.stubs(:exist?).returns true
-      @parser.expects(:parse).raises ArgumentError
-      lambda { @code.perform_initial_import }.should raise_error(Puppet::Error)
-    end
-
-    it "should not do anything if the ignore_import settings is set" do
-      Puppet.settings[:ignoreimport] = true
-      @parser.expects(:string=).never
-      @parser.expects(:file=).never
-      @parser.expects(:parse).never
-      @code.perform_initial_import
-    end
-  end
-
   describe "when determining the configuration version" do
     before do
       @code = Puppet::Resource::TypeCollection.new("env")