Doesn't unnecessarily regenerate files when compressing a directory. …

…Also, fixes tests.

lib/jekyll/gzip/compressor.rb

@@ -25,7 +25,7 @@ module Compressor
       # @return void
       def self.compress_site(site)
         site.each_site_file do |file|
-          next unless regenerate? file, site
+          next unless regenerate? file.destination(site.dest), site
 
           compress_file(
             file.destination(site.dest),
@@ -51,8 +51,16 @@ def self.compress_site(site)
       def self.compress_directory(dir, site)
         extensions = zippable_extensions(site).join(',')
         replace_file = replace_files(site)
-        files = Dir.glob(dir + "**/*{#{extensions}}")
-        files.each { |file| compress_file(file, extensions: extensions, replace_file: replace_file) }
+        files = Dir.glob(dir + "/**/*{#{extensions}}")
+        files.each do |file|
+          next unless regenerate?(file, site)
+
+          compress_file(
+            file,
+            extensions: extensions,
+            replace_file: replace_file
+          )
+        end
       end
 
       ##
@@ -103,12 +111,23 @@ def self.replace_files(site)
       # Compresses the file if the site is built incrementally and the
       # source was modified or the compressed file doesn't exist
       def self.regenerate?(file, site)
-        orig   = file.destination(site.dest)
-        zipped = zipped(orig, replace_files(site))
+        zipped = zipped(file, replace_files(site))
 
+        # Definitely generate the file if it doesn't exist yet.
         return true unless File.exist? zipped
+        # If we are replacing files and this file is not a gzip file, then it
+        # has been edited so we need to re-gzip it in place.
+        return !is_already_gzipped?(file) if replace_files(site)
+
+        # If the modified time of the new file is greater than the modified time
+        # of the old file, then we need to regenerate.
+        File.mtime(file) > File.mtime(zipped)
+      end
 
-        File.mtime(orig) > File.mtime(zipped)
+      # First two bytes of a gzipped file are 1f and 8b. This tests for those
+      # bytes.
+      def self.is_already_gzipped?(file)
+        ["1f", "8b"] == File.read(file, 2).unpack("H2H2")
       end
     end
   end

spec/jekyll/gzip/compressor_spec.rb

@@ -51,20 +51,44 @@
   end
 
   describe "given a Jekyll site" do
-    it "compresses all files in the site" do
-      Jekyll::Gzip::Compressor.compress_site(site)
-      files = [
+    let(:files) {
+      [
         dest_dir("index.html"),
         dest_dir("css/main.css"),
         dest_dir("about/index.html"),
-        dest_dir("jekyll/update/2018/01/01/welcome-to-jekyll.htlm"),
+        dest_dir("jekyll/update/2018/01/01/welcome-to-jekyll.html"),
         dest_dir("feed.xml")
       ]
+    }
+    it "compresses all files in the site" do
+      Jekyll::Gzip::Compressor.compress_site(site)
+
       files.each do |file_name|
-        expect(File.exist?("#{file_name}.gz"))
+        expect(File.exist?("#{file_name}.gz")).to be true
       end
     end
 
+    it "replaces the files if the settings say so" do
+      original_stats = files.inject({}) { |hash, file|
+        hash[file] = {size: File.size(file), content: File.read(file)}
+        hash
+      }
+
+      site.config['gzip'] ||= {}
+      site.config['gzip']['replace_files'] = true
+      Jekyll::Gzip::Compressor.compress_site(site)
+
+      files.each { |file|
+        expect(File.exist?("#{file}")).to be true
+        expect(File.exist?("#{file}.gz")).to be false
+        expect(File.size(file)).to be < original_stats[file][:size]
+        Zlib::GzipReader.open("#{file}") {|gz|
+          expect(gz.read).to eq(original_stats[file][:content])
+        }
+      }
+    end
+
+
     it "doesn't compress the files again if they are already compressed and haven't changed" do
       Jekyll::Gzip::Compressor.compress_site(site)
 
@@ -83,21 +107,84 @@
       Jekyll::Gzip::Compressor.compress_site(site)
       expect(Zlib::GzipWriter).to have_received(:open).once
     end
+
+    it "doesn't recompress files when in replacement mode" do
+      site.config['gzip'] ||= {}
+      site.config['gzip']['replace_files'] = true
+      Jekyll::Gzip::Compressor.compress_site(site)
+
+      allow(Zlib::GzipWriter).to receive(:open)
+
+      Jekyll::Gzip::Compressor.compress_site(site)
+      expect(Zlib::GzipWriter).not_to have_received(:open)
+    end
   end
 
   describe "given a destination directory" do
+    let(:files) {[
+      dest_dir("index.html"),
+      dest_dir("css/main.css"),
+      dest_dir("about/index.html"),
+      dest_dir("jekyll/update/2018/01/01/welcome-to-jekyll.html"),
+      dest_dir("feed.xml")
+    ]}
+
     it "compresses all the text files in the directory" do
       Jekyll::Gzip::Compressor.compress_directory(dest_dir, site)
-      files = [
-        dest_dir("index.html"),
-        dest_dir("css/main.css"),
-        dest_dir("about/index.html"),
-        dest_dir("jekyll/update/2018/01/01/welcome-to-jekyll.htlm"),
-        dest_dir("feed.xml")
-      ]
+
       files.each do |file_name|
-        expect(File.exist?("#{file_name}.gz"))
+        expect(File.exist?("#{file_name}.gz")).to be true
       end
     end
+
+    it "replaces the files if the settings say so" do
+      original_stats = files.inject({}) { |hash, file|
+        hash[file] = {size: File.size(file), content: File.read(file)}
+        hash
+      }
+
+      site.config['gzip'] ||= {}
+      site.config['gzip']['replace_files'] = true
+      Jekyll::Gzip::Compressor.compress_directory(dest_dir, site)
+
+      files.each { |file|
+        expect(File.exist?("#{file}")).to be true
+        expect(File.exist?("#{file}.gz")).to be false
+        expect(File.size(file)).to be < original_stats[file][:size]
+        Zlib::GzipReader.open("#{file}") {|gz|
+          expect(gz.read).to eq(original_stats[file][:content])
+        }
+      }
+    end
+
+
+    it "doesn't compress the files again if they are already compressed and haven't changed" do
+      Jekyll::Gzip::Compressor.compress_directory(dest_dir, site)
+
+      allow(Zlib::GzipWriter).to receive(:open)
+
+      Jekyll::Gzip::Compressor.compress_directory(dest_dir, site)
+      expect(Zlib::GzipWriter).not_to have_received(:open)
+    end
+
+    it "does compress files that change between compressions" do
+      Jekyll::Gzip::Compressor.compress_directory(dest_dir, site)
+      allow(Zlib::GzipWriter).to receive(:open).and_call_original
+
+      FileUtils.touch(dest_dir("about/index.html"), mtime: Time.now + 1)
+      Jekyll::Gzip::Compressor.compress_directory(dest_dir, site)
+      expect(Zlib::GzipWriter).to have_received(:open).once
+    end
+
+    it "doesn't recompress files when in replacement mode" do
+      site.config['gzip'] ||= {}
+      site.config['gzip']['replace_files'] = true
+      Jekyll::Gzip::Compressor.compress_directory(dest_dir, site)
+
+      allow(Zlib::GzipWriter).to receive(:open)
+
+      Jekyll::Gzip::Compressor.compress_directory(dest_dir, site)
+      expect(Zlib::GzipWriter).not_to have_received(:open)
+    end
   end
 end