feat: make compilation stale on asset host change (shakacode#364)

ahangarha · web-flow · commit bcc2fbc556c1 · 2023-11-30T20:03:57.000-10:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,7 @@ _Please add entries here for your pull requests that are not yet released._
 
 ### Fixed
 - Recommend `server` option instead of deprecated `https` option when `--https` is provided [PR 380](https://github.com/shakacode/shakapacker/pull/380) by [G-Rath](https://github.com/g-rath)
+- Recompile assets on asset host change [PR 364](https://github.com/shakacode/shakapacker/pull/364) by [ahangarha](https://github.com/ahangarha).
 
 ## [v7.1.0] - September 30, 2023
 
diff --git a/lib/shakapacker/digest_strategy.rb b/lib/shakapacker/digest_strategy.rb
@@ -44,7 +44,9 @@ def watched_files_digest
         end
         files = Dir[*expanded_paths].reject { |f| File.directory?(f) }
         file_ids = files.sort.map { |f| "#{File.basename(f)}/#{Digest::SHA1.file(f).hexdigest}" }
-        Digest::SHA1.hexdigest(file_ids.join("/"))
+
+        asset_host = Shakapacker.config.asset_host.to_s
+        Digest::SHA1.hexdigest(file_ids.join("/").concat(asset_host))
       end
 
       def record_compilation_digest
diff --git a/spec/backward_compatibility_specs/digest_strategy_spec.rb b/spec/backward_compatibility_specs/digest_strategy_spec.rb
@@ -9,6 +9,7 @@ def remove_compilation_digest_path
 
   before :all do
     @digest_strategy = Webpacker::DigestStrategy.new
+    ENV["SHAKAPACKER_ASSET_HOST"] = nil
     remove_compilation_digest_path
   end
 
diff --git a/spec/shakapacker/compiler_spec.rb b/spec/shakapacker/compiler_spec.rb
@@ -50,14 +50,10 @@
     expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to be nil
     expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to be nil
 
-    custom_env_variables = {
-      "SHAKAPACKER_ASSET_HOST" => "foo.bar",
-      "SHAKAPACKER_RELATIVE_URL_ROOT" => "/baz"
-    }
-
-    with_env_variable(custom_env_variables) do
-      expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to eq "foo.bar"
-      expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to eq "/baz"
-    end
+    allow(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return("foo.bar")
+    allow(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", nil).and_return("/baz")
+
+    expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_ASSET_HOST"]).to eq "foo.bar"
+    expect(Shakapacker.compiler.send(:webpack_env)["SHAKAPACKER_RELATIVE_URL_ROOT"]).to eq "/baz"
   end
 end
diff --git a/spec/shakapacker/configuration_spec.rb b/spec/shakapacker/configuration_spec.rb
@@ -341,17 +341,16 @@
     end
 
     it "returns the value of SHAKAPACKER_ASSET_HOST if set" do
-      with_env_variable("SHAKAPACKER_ASSET_HOST" => "custom_host.abc") do
-        expect(config.asset_host).to eq "custom_host.abc"
-      end
+      expect(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return("custom_host.abc")
+
+      expect(config.asset_host).to eq "custom_host.abc"
     end
 
     it "returns ActionController::Base.helpers.compute_asset_host if SHAKAPACKER_ASSET_HOST is not set" do
       allow(ActionController::Base.helpers).to receive(:compute_asset_host).and_return("domain.abc")
+      allow(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", "domain.abc").and_return("domain.abc")
 
-      with_env_variable("SHAKAPACKER_ASSET_HOST" => nil) do
-        expect(config.asset_host).to eq "domain.abc"
-      end
+      expect(config.asset_host).to eq "domain.abc"
     end
   end
 
@@ -365,17 +364,16 @@
     end
 
     it "returns the value of SHAKAPACKER_RELATIVE_URL_ROOT if set" do
-      with_env_variable("SHAKAPACKER_RELATIVE_URL_ROOT" => "custom_value") do
-        expect(config.relative_url_root).to eq "custom_value"
-      end
+      allow(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", nil).and_return("custom_value")
+
+      expect(config.relative_url_root).to eq "custom_value"
     end
 
     it "returns ActionController::Base.helpers.compute_asset_host if SHAKAPACKER_RELATIVE_URL_ROOT is not set" do
       allow(ActionController::Base).to receive(:relative_url_root).and_return("abcd")
+      allow(ENV).to receive(:fetch).with("SHAKAPACKER_RELATIVE_URL_ROOT", "abcd").and_return("abcd")
 
-      with_env_variable("SHAKAPACKER_RELATIVE_URL_ROOT" => nil) do
-        expect(config.relative_url_root).to eq "abcd"
-      end
+      expect(config.relative_url_root).to eq "abcd"
     end
   end
 end
diff --git a/spec/shakapacker/digest_strategy_spec.rb b/spec/shakapacker/digest_strategy_spec.rb
@@ -28,7 +28,29 @@ def remove_compilation_digest_path
     expect(@digest_strategy.fresh?).to be true
   end
 
+  it "is stale when host changes" do
+    allow(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return("old-host")
+    # Record the digests for old-host
+    @digest_strategy.after_compile_hook
+
+    allow(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return("new-host")
+    expect(@digest_strategy.stale?).to be true
+    expect(@digest_strategy.fresh?).to be_falsey
+  end
+
   it "generates correct compilation_digest_path" do
+    allow(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return("custom-path")
+
+    actual_path = @digest_strategy.send(:compilation_digest_path).basename.to_s
+    host_hash = Digest::SHA1.hexdigest("custom-path")
+    expected_path = "last-compilation-digest-#{Shakapacker.env}"
+
+    expect(actual_path).to eq expected_path
+  end
+
+  it "generates correct compilation_digest_path without the digest of the asset host if asset host is not set" do
+    allow(ENV).to receive(:fetch).with("SHAKAPACKER_ASSET_HOST", nil).and_return(nil)
+
     actual_path = @digest_strategy.send(:compilation_digest_path).basename.to_s
     expected_path = "last-compilation-digest-#{Shakapacker.env}"
 
diff --git a/spec/shakapacker/spec_helper_initializer.rb b/spec/shakapacker/spec_helper_initializer.rb
@@ -22,21 +22,3 @@ def with_rails_env(env)
   Rails.env = ActiveSupport::StringInquirer.new(original)
   reloaded_config
 end
-
-# Temportarily set env variables to a custom value
-# arg: a hash with key:value for each custom env
-#      Keys could be string or symbol
-def with_env_variable(custom_env_hash)
-  original_env = {}
-  custom_env_hash.each do |key, new_value|
-    upcased_key = key.to_s.upcase
-    original_env[upcased_key] = new_value
-    ENV[upcased_key] = new_value
-  end
-
-  yield
-ensure
-  original_env.each do |key, original_value|
-    ENV[key] = original_value
-  end
-end
