feat: timeout long running pact content diff requests (#555)

docs/CONFIGURATION.md

@@ -655,6 +655,15 @@ with a unique version number.
 **Allowed values:** `true`, `false`<br/>
 **More information:** https://docs.pact.io/versioning<br/>
 
+### pact_content_diff_timeout
+
+The maximum amount of time in seconds to attempt to generate the diff between two pacts before aborting the request. This is required due to performance issues in the underlying diff generation code.
+
+**Supported versions:** From 2.99.0<br/>
+**Environment variable name:** `PACT_BROKER_PACT_CONTENT_DIFF_TIMEOUT`<br/>
+**YAML configuration key name:** `pact_content_diff_timeout`<br/>
+**Default:** `15`<br/>
+
 <br/>
 
 ## Miscellaneous

docs/configuration.yml

@@ -453,6 +453,11 @@ groups:
           - true
           - false
         more_info: https://docs.pact.io/versioning
+      pact_content_diff_timeout:
+        description: |-
+          The maximum amount of time in seconds to attempt to generate the diff between two pacts before aborting the request. This is required due to performance issues in the underlying diff generation code.
+        default_value: 15
+        supported_versions: From 2.99.0
   - title: Miscellaneous
     vars:
       features:

lib/pact_broker/api/resources/pact_content_diff.rb

@@ -1,6 +1,7 @@
 require "pact_broker/api/resources/base_resource"
 require "pact_broker/pacts/pact_params"
 require "pact_broker/pacts/diff"
+require "timeout"
 
 module PactBroker
   module Api
@@ -19,8 +20,12 @@ def resource_exists?
         end
 
         def to_text
-          output = PactBroker::Pacts::Diff.new.process pact_params.merge(base_url: base_url), comparison_pact_params, raw: false
-          response.body = output
+          Timeout::timeout(PactBroker.configuration.pact_content_diff_timeout) do
+            output = PactBroker::Pacts::Diff.new.process pact_params.merge(base_url: base_url), comparison_pact_params, raw: false
+            response.body = output
+          end
+        rescue Timeout::Error
+          408
         end
 
         def pact

lib/pact_broker/config/runtime_configuration.rb

@@ -71,7 +71,8 @@ class RuntimeConfiguration < Anyway::Config
         badge_provider_mode: :redirect,
         enable_public_badge_access: false,
         shields_io_base_url: "https://img.shields.io",
-        use_case_sensitive_resource_names: true
+        use_case_sensitive_resource_names: true,
+        pact_content_diff_timeout: 15
       )
 
       # domain attributes

lib/pact_broker/pacts/create_formatted_diff.rb

@@ -19,6 +19,7 @@ def self.call pact_json_content, previous_pact_json_content, raw: false
         end
 
         difference = diff(previous_pact_hash, pact_hash)
+
         Pact::Matchers::UnixDiffFormatter.call(difference, colour: false, include_explanation: false)
       end
     end

spec/features/get_diff_spec.rb

@@ -1,12 +1,10 @@
 require "spec/support/test_data_builder"
 
 describe "Get diff between versions" do
-
   let(:path) { "/pacts/provider/Provider/consumer/Consumer/version/3/diff/previous-distinct" }
-
   let(:last_response_body) { subject.body }
 
-  subject { get path; last_response }
+  subject { get(path) }
 
   let(:pact_content_version_1) do
     hash = load_json_fixture("consumer-provider.json")
@@ -42,12 +40,10 @@
   end
 
   context "when either version does not exist" do
-
     let(:path) { "/pacts/provider/Provider/consumer/Consumer/versions/1/diff/previous-distinct" }
 
     it "returns a 404 response" do
       expect(subject).to be_a_404_response
     end
-
   end
 end

spec/lib/pact_broker/api/resources/pact_content_diff_spec.rb

@@ -0,0 +1,36 @@
+require "pact_broker/api/resources/pact_content_diff"
+
+module PactBroker
+  module Api
+    module Resources
+      describe PactContentDiff do
+        include_context "stubbed services"
+
+        before do
+          allow(pact_service).to receive(:find_pact).and_return(pact)
+          allow(PactBroker::Pacts::Diff).to receive(:new).and_return(diff)
+        end
+
+        let(:pact) { double("pact") }
+        let(:diff) { instance_double("PactBroker::Pacts::Diff", process: diff_content) }
+        let(:diff_content) { "diff_content" }
+        let(:path) { "/pacts/provider/Provider/consumer/Consumer/version/3/diff/previous-distinct" }
+        let(:last_response_body) { subject.body }
+
+        subject { get(path) }
+
+        its(:status) { is_expected.to eq 200 }
+        its(:body) { is_expected.to eq "diff_content" }
+
+        context "when the diff takes too long to generate" do
+          before do
+            allow(diff).to receive(:process).and_raise(Timeout::Error)
+          end
+
+          its(:status) { is_expected.to eq 408 }
+          its(:body) { is_expected.to eq "" }
+        end
+      end
+    end
+  end
+end