[CHEF-3715] decouple SyntaxCheck from ChecksumCache

SyntaxCheck was the only place to use ChecksumCache directly (to keep
track of which files have already been checked and can be skipped).
A simple file-based set now tracks which files have been checked.

2 files changed, 101 insertions, 57 deletions

diff --git a/lib/chef/cookbook/syntax_check.rb b/lib/chef/cookbook/syntax_check.rb
index bf7c45e252..2ba62e568e 100644
--- a/lib/chef/cookbook/syntax_check.rb
+++ b/lib/chef/cookbook/syntax_check.rb
@@ -16,8 +16,8 @@
 # limitations under the License.
 #
 
-require 'chef/checksum_cache'
 require 'chef/mixin/shell_out'
+require 'chef/mixin/checksum'
 
 class Chef
   class Cookbook
@@ -25,10 +25,56 @@ class Chef
     # Encapsulates the process of validating the ruby syntax of files in Chef
     # cookbooks.
     class SyntaxCheck
+
+      # == Chef::Cookbook::SyntaxCheck::PersistentSet
+      # Implements set behavior with disk-based persistence. Objects in the set
+      # are expected to be strings containing only characters that are valid in
+      # filenames.
+      #
+      # This class is used to track which files have been syntax checked so
+      # that known good files are not rechecked.
+      class PersistentSet
+
+        attr_reader :cache_path
+
+        # Create a new PersistentSet. Values in the set are persisted by
+        # creating a file in the +cache_path+ directory. If not given, the
+        # value of Chef::Config[:cache_options][:path] is used.
+        def initialize(cache_path=Chef::Config[:cache_options][:path])
+          @cache_path = cache_path
+          @cache_path_created = false
+        end
+
+        # Adds +value+ to the set's collection.
+        def add(value)
+          ensure_cache_path_created
+          FileUtils.touch(File.join(cache_path, value))
+        end
+
+        # Returns true if the set includes +value+
+        def include?(value)
+          File.exist?(File.join(cache_path, value))
+        end
+
+        private
+
+        def ensure_cache_path_created
+          return true if @cache_path_created
+          FileUtils.mkdir_p(cache_path)
+          @cache_path_created = true
+        end
+
+      end
+
       include Chef::Mixin::ShellOut
+      include Chef::Mixin::Checksum
 
       attr_reader :cookbook_path
 
+      # A PersistentSet object that tracks which files have already been
+      # validated.
+      attr_reader :validated_files
+
       # Creates a new SyntaxCheck given the +cookbook_name+ and a +cookbook_path+.
       # If no +cookbook_path+ is given, +Chef::Config.cookbook_path+ is used.
       def self.for_cookbook(cookbook_name, cookbook_path=nil)
@@ -44,10 +90,7 @@ class Chef
       # cookbook_path::: the (on disk) path to the cookbook
       def initialize(cookbook_path)
         @cookbook_path = cookbook_path
-      end
-
-      def cache
-        Chef::ChecksumCache.instance
+        @validated_files = PersistentSet.new
       end
 
       def ruby_files
@@ -81,16 +124,11 @@ class Chef
       end
 
       def validated?(file)
-        !!cache.lookup_checksum(cache_key(file), File.stat(file))
+        validated_files.include?(checksum(file))
       end
 
       def validated(file)
-        cache.generate_checksum(cache_key(file), file, File.stat(file))
-      end
-
-      def cache_key(file)
-        @cache_keys ||= {}
-        @cache_keys[file] ||= cache.generate_key(file, "chef-test")
+        validated_files.add(checksum(file))
       end
 
       def validate_ruby_files
@@ -118,7 +156,7 @@ class Chef
         result.stderr.each_line { |l| Chef::Log.fatal(l.chomp) }
         false
       end
-      
+
       def validate_ruby_file(ruby_file)
         Chef::Log.debug("Testing #{ruby_file} for syntax errors...")
         result = shell_out("ruby -c #{ruby_file}")
@@ -130,7 +168,7 @@ class Chef
         result.stderr.each_line { |l| Chef::Log.fatal(l.chomp) }
         false
       end
-      
+
     end
   end
 end
diff --git a/spec/unit/cookbook/syntax_check_spec.rb b/spec/unit/cookbook/syntax_check_spec.rb
index b41c2ddf0a..e7eaba07bc 100644
--- a/spec/unit/cookbook/syntax_check_spec.rb
+++ b/spec/unit/cookbook/syntax_check_spec.rb
@@ -82,109 +82,115 @@ require 'spec_helper'
 require "chef/cookbook/syntax_check"
 
 describe Chef::Cookbook::SyntaxCheck do
+
+  let(:cookbook_path) { File.join(CHEF_SPEC_DATA, 'cookbooks', 'openldap') }
+  let(:syntax_check) { Chef::Cookbook::SyntaxCheck.new(cookbook_path) }
+
   before do
     Chef::Log.logger = Logger.new(StringIO.new)
+    Chef::Log.level = :warn # suppress "Syntax OK" messages
 
-    @cookbook_path = File.join(CHEF_SPEC_DATA, 'cookbooks', 'openldap')
 
-    @attr_files = %w{default.rb smokey.rb}.map { |f| File.join(@cookbook_path, 'attributes', f) }
-    @defn_files = %w{client.rb server.rb}.map { |f| File.join(@cookbook_path, 'definitions', f)}
-    @recipes = %w{default.rb gigantor.rb one.rb}.map { |f| File.join(@cookbook_path, 'recipes', f) }
+    @attr_files = %w{default.rb smokey.rb}.map { |f| File.join(cookbook_path, 'attributes', f) }
+    @defn_files = %w{client.rb server.rb}.map { |f| File.join(cookbook_path, 'definitions', f)}
+    @recipes = %w{default.rb gigantor.rb one.rb}.map { |f| File.join(cookbook_path, 'recipes', f) }
     @ruby_files = @attr_files + @defn_files + @recipes
 
-    @template_files = %w{openldap_stuff.conf.erb openldap_variable_stuff.conf.erb test.erb}.map { |f| File.join(@cookbook_path, 'templates', 'default', f)}
+    @template_files = %w{openldap_stuff.conf.erb openldap_variable_stuff.conf.erb test.erb}.map { |f| File.join(cookbook_path, 'templates', 'default', f)}
 
-    @syntax_check = Chef::Cookbook::SyntaxCheck.new(@cookbook_path)
   end
 
   it "creates a syntax checker given the cookbook name when Chef::Config.cookbook_path is set" do
-    Chef::Config[:cookbook_path] = File.dirname(@cookbook_path)
+    Chef::Config[:cookbook_path] = File.dirname(cookbook_path)
     syntax_check = Chef::Cookbook::SyntaxCheck.for_cookbook(:openldap)
-    syntax_check.cookbook_path.should == @cookbook_path
+    syntax_check.cookbook_path.should == cookbook_path
   end
 
   describe "when first created" do
     it "has the path to the cookbook to syntax check" do
-      @syntax_check.cookbook_path.should == @cookbook_path
-    end
-
-    it "has access to the checksum cache" do
-      @syntax_check.cache.should equal(Chef::ChecksumCache.instance)
+      syntax_check.cookbook_path.should == cookbook_path
     end
 
     it "lists the ruby files in the cookbook" do
-      @syntax_check.ruby_files.sort.should == @ruby_files.sort
+      syntax_check.ruby_files.sort.should == @ruby_files.sort
     end
 
     it "lists the erb templates in the cookbook" do
-      @syntax_check.template_files.sort.should == @template_files.sort
+      syntax_check.template_files.sort.should == @template_files.sort
     end
 
   end
 
   describe "when validating cookbooks" do
+    let(:cache_path) { Dir.mktmpdir }
+
     before do
+      Chef::Config[:cache_options] = {:path => cache_path }
       Chef::Config[:cache_type] = 'Memory'
       @checksum_cache_klass = Class.new(Chef::ChecksumCache)
       @checksum_cache = @checksum_cache_klass.instance
       @checksum_cache.reset!('Memory')
-      @syntax_check.stub!(:cache).and_return(@checksum_cache)
-      $stdout.stub!(:write)
+      syntax_check.stub!(:cache).and_return(@checksum_cache)
+    end
+
+    after do
+      FileUtils.rm_rf(cache_path) if File.exist?(cache_path)
+      Chef::Config[:cache_options] = {:path => nil }
     end
 
     describe "and the files have not been syntax checked previously" do
       it "shows that all ruby files require a syntax check" do
-        @syntax_check.untested_ruby_files.sort.should == @ruby_files.sort
+        syntax_check.untested_ruby_files.sort.should == @ruby_files.sort
       end
 
       it "shows that all template files require a syntax check" do
-        @syntax_check.untested_template_files.sort.should == @template_files.sort
+        syntax_check.untested_template_files.sort.should == @template_files.sort
       end
 
       it "removes a ruby file from the list of untested files after it is marked as validated" do
-        recipe = File.join(@cookbook_path, 'recipes', 'default.rb')
-        @syntax_check.validated(recipe)
-        @syntax_check.untested_ruby_files.should_not include(recipe)
+        recipe = File.join(cookbook_path, 'recipes', 'default.rb')
+        syntax_check.validated(recipe)
+        syntax_check.untested_ruby_files.should_not include(recipe)
       end
 
       it "removes a template file from the list of untested files after it is marked as validated" do
-        template = File.join(@cookbook_path, 'templates', 'default', 'test.erb')
-        @syntax_check.validated(template)
-        @syntax_check.untested_template_files.should_not include(template)
+        template = File.join(cookbook_path, 'templates', 'default', 'test.erb')
+        syntax_check.validated(template)
+        syntax_check.untested_template_files.should_not include(template)
       end
 
       it "validates all ruby files" do
-        @syntax_check.validate_ruby_files.should be_true
-        @syntax_check.untested_ruby_files.should be_empty
+        syntax_check.validate_ruby_files.should be_true
+        syntax_check.untested_ruby_files.should be_empty
       end
 
       it "validates all templates" do
-        @syntax_check.validate_templates.should be_true
-        @syntax_check.untested_template_files.should be_empty
+        syntax_check.validate_templates.should be_true
+        syntax_check.untested_template_files.should be_empty
       end
 
       describe "and a file has a syntax error" do
         before do
-          @cookbook_path = File.join(CHEF_SPEC_DATA, 'cookbooks', 'borken')
-          @syntax_check.cookbook_path.replace(@cookbook_path)
+          cookbook_path = File.join(CHEF_SPEC_DATA, 'cookbooks', 'borken')
+          syntax_check.cookbook_path.replace(cookbook_path)
         end
 
         it "it indicates that a ruby file has a syntax error" do
-          @syntax_check.validate_ruby_files.should be_false
+          syntax_check.validate_ruby_files.should be_false
         end
 
         it "does not remove the invalid file from the list of untested files" do
-          @syntax_check.untested_ruby_files.should include(File.join(@cookbook_path, 'recipes', 'default.rb'))
-          lambda { @syntax_check.validate_ruby_files }.should_not change(@syntax_check, :untested_ruby_files)
+          syntax_check.untested_ruby_files.should include(File.join(cookbook_path, 'recipes', 'default.rb'))
+          lambda { syntax_check.validate_ruby_files }.should_not change(syntax_check, :untested_ruby_files)
         end
 
         it "indicates that a template file has a syntax error" do
-          @syntax_check.validate_templates.should be_false
+          syntax_check.validate_templates.should be_false
         end
 
         it "does not remove the invalid template from the list of untested templates" do
-          @syntax_check.untested_template_files.should include(File.join(@cookbook_path, 'templates', 'default', 'borken.erb'))
-          lambda {@syntax_check.validate_templates}.should_not change(@syntax_check, :untested_template_files)
+          syntax_check.untested_template_files.should include(File.join(cookbook_path, 'templates', 'default', 'borken.erb'))
+          lambda {syntax_check.validate_templates}.should_not change(syntax_check, :untested_template_files)
         end
 
       end
@@ -193,18 +199,18 @@ describe Chef::Cookbook::SyntaxCheck do
 
     describe "and the files have been syntax checked previously" do
       before do
-        @syntax_check.untested_ruby_files.each { |f| @syntax_check.validated(f) }
-        @syntax_check.untested_template_files.each { |f| @syntax_check.validated(f) }
+        syntax_check.untested_ruby_files.each { |f| syntax_check.validated(f) }
+        syntax_check.untested_template_files.each { |f| syntax_check.validated(f) }
       end
 
       it "does not syntax check ruby files" do
-        @syntax_check.should_not_receive(:shell_out)
-        @syntax_check.validate_ruby_files.should be_true
+        syntax_check.should_not_receive(:shell_out)
+        syntax_check.validate_ruby_files.should be_true
       end
 
       it "does not syntax check templates" do
-        @syntax_check.should_not_receive(:shell_out)
-        @syntax_check.validate_templates.should be_true
+        syntax_check.should_not_receive(:shell_out)
+        syntax_check.validate_templates.should be_true
       end
     end
   end