fix: ensure matrix is updated when pacticipant is deleted

lib/pact_broker/api.rb

@@ -41,7 +41,7 @@ module PactBroker
         # Pacticipants
         add ['pacticipants'], Api::Resources::Pacticipants, {resource_name: "pacticipants"}
         add ['pacticipants', 'label', :label_name], PactBroker::Api::Resources::PacticipantsForLabel, {resource_name: "pacticipants_for_label"}
-        add ['pacticipants', :name], Api::Resources::Pacticipant, {resource_name: "pacticipant"}
+        add ['pacticipants', :pacticipant_name], Api::Resources::Pacticipant, {resource_name: "pacticipant"}
         add ['pacticipants', :pacticipant_name, 'versions'], Api::Resources::Versions, {resource_name: "pacticipant_versions"}
         add ['pacticipants', :pacticipant_name, 'versions', :pacticipant_version_number], Api::Resources::Version, {resource_name: "pacticipant_version"}
         add ['pacticipants', :pacticipant_name, 'latest-version', :tag], Api::Resources::Version, {resource_name: "latest_tagged_pacticipant_version"}

lib/pact_broker/api/resources/base_resource.rb

@@ -40,8 +40,12 @@ def initialize
           PactBroker.configuration.before_resource.call(self)
         end
 
+        def update_matrix_after_request?
+          false
+        end
+
         def finish_request
-          if !request.get? && !request.head?
+          if update_matrix_after_request?
             matrix_service.refresh(identifier_from_path)
           end
           PactBroker.configuration.after_resource.call(self)
@@ -155,6 +159,9 @@ def contract_validation_errors? contract, params
           invalid
         end
 
+        def with_matrix_refresh &block
+          matrix_service.refresh(identifier_from_path, &block)
+        end
       end
     end
   end

lib/pact_broker/api/resources/pact.rb

@@ -87,7 +87,9 @@ def to_html
         end
 
         def delete_resource
-          pact_service.delete(pact_params)
+          with_matrix_refresh do
+            pact_service.delete(pact_params)
+          end
           true
         end
 
@@ -101,6 +103,9 @@ def pact_params
           @pact_params ||= PactBroker::Pacts::PactParams.from_request request, path_info
         end
 
+        def update_matrix_after_request?
+          request.put? || request.patch?
+        end
       end
     end
   end

lib/pact_broker/api/resources/pacticipant.rb

@@ -45,7 +45,9 @@ def resource_exists?
         end
 
         def delete_resource
-          pacticipant_service.delete pacticipant_name
+          with_matrix_refresh do
+            pacticipant_service.delete pacticipant_name
+          end
           true
         end
 
@@ -60,7 +62,11 @@ def pacticipant
         end
 
         def pacticipant_name
-          identifier_from_path[:name]
+          identifier_from_path[:pacticipant_name]
+        end
+
+        def update_matrix_after_request?
+          request.patch?
         end
       end
     end

lib/pact_broker/api/resources/tag.rb

@@ -22,6 +22,7 @@ def from_json
             @tag = tag_service.create identifier_from_path
             # Make it return a 201 by setting the Location header
             response.headers["Location"] = tag_url(base_url, tag)
+            matrix_service.refresh_tags(identifier_from_path)
           end
           response.body = to_json
         end
@@ -39,10 +40,11 @@ def tag
         end
 
         def delete_resource
-          tag_service.delete identifier_from_path
+          matrix_service.refresh_tags(identifier_from_path) do
+            tag_service.delete identifier_from_path
+          end
           true
         end
-
       end
     end
 

lib/pact_broker/api/resources/verifications.rb

@@ -61,6 +61,10 @@ def next_verification_number
         def decorator_for model
           PactBroker::Api::Decorators::VerificationDecorator.new(model)
         end
+
+        def update_matrix_after_request?
+          request.post?
+        end
       end
     end
   end

lib/pact_broker/api/resources/version.rb

@@ -25,7 +25,9 @@ def to_json
         end
 
         def delete_resource
-          version_service.delete version
+          with_matrix_refresh do
+            version_service.delete version
+          end
           true
         end
 

lib/pact_broker/matrix/repository.rb

@@ -22,9 +22,45 @@ class Repository
       GROUP_BY_PROVIDER = [:consumer_name, :consumer_version_number, :provider_name]
       GROUP_BY_PACT = [:consumer_name, :provider_name]
 
+      # Use a block when the refresh is caused by a resource deletion
+      # This allows us to store the correct object ids for use afterwards
       def refresh params
-        PactBroker::Matrix::Row.refresh(params)
-        PactBroker::Matrix::HeadRow.refresh(params)
+        criteria = find_ids_for_pacticipant_names(params)
+        yield if block_given?
+        PactBroker::Matrix::Row.refresh(criteria)
+        PactBroker::Matrix::HeadRow.refresh(criteria)
+      end
+
+      # Only need to update the HeadRow table when tags change
+      # because it only changes which rows are the latest tagged ones -
+      # it doesn't change the actual values in the underlying matrix.
+      def refresh_tags params
+        criteria = find_ids_for_pacticipant_names(params)
+        yield if block_given?
+        PactBroker::Matrix::HeadRow.refresh(criteria)
+      end
+
+      def find_ids_for_pacticipant_names params
+        criteria  = {}
+
+        if params[:consumer_name] || params[:provider_name]
+          if params[:consumer_name]
+            pacticipant = PactBroker::Domain::Pacticipant.where(name_like(:name, params[:consumer_name])).single_record
+            criteria[:consumer_id] = pacticipant.id if pacticipant
+          end
+
+          if params[:provider_name]
+            pacticipant = PactBroker::Domain::Pacticipant.where(name_like(:name, params[:provider_name])).single_record
+            criteria[:provider_id] = pacticipant.id if pacticipant
+          end
+        end
+
+        if params[:pacticipant_name]
+          pacticipant = PactBroker::Domain::Pacticipant.where(name_like(:name, params[:pacticipant_name])).single_record
+          criteria[:pacticipant_id] = pacticipant.id if pacticipant
+        end
+
+        criteria
       end
 
       # Return the latest matrix row (pact/verification) for each consumer_version_number/provider_version_number

lib/pact_broker/matrix/row.rb

@@ -19,42 +19,28 @@ class Row < Sequel::Model(:materialized_matrix)
         include PactBroker::Repositories::Helpers
         include PactBroker::Logging
 
-        def refresh params
-          logger.debug("Refreshing #{model.table_name} for #{params}")
+        def refresh ids
+          logger.debug("Refreshing #{model.table_name} for #{ids}")
 
           db = model.db
           table_name = model.table_name
 
-          source_view_name = model.table_name.to_s.gsub('materialized_', '').to_sym
-          if params[:consumer_name] || params[:provider_name]
-            criteria = {}
-            if params[:consumer_name]
-              pacticipant = PactBroker::Domain::Pacticipant.where(name_like(:name, params[:consumer_name])).single_record
-              criteria[:consumer_id] = pacticipant.id if pacticipant
+          if ids[:pacticipant_id]
+            db.transaction do
+              db[table_name].where(consumer_id: ids[:pacticipant_id]).or(provider_id: ids[:pacticipant_id]).delete
+              new_rows = db[source_view_name].where(consumer_id: ids[:pacticipant_id]).or(provider_id: ids[:pacticipant_id]).distinct
+              db[table_name].insert(new_rows)
             end
-
-            if params[:provider_name]
-              pacticipant = PactBroker::Domain::Pacticipant.where(name_like(:name, params[:provider_name])).single_record
-              criteria[:provider_id] = pacticipant.id if pacticipant
-            end
-            if criteria.any?
-              db.transaction do
-                db[table_name].where(criteria).delete
-                db[table_name].insert(db[source_view_name].where(criteria).distinct)
-              end
+          elsif ids.any?
+            db.transaction do
+              db[table_name].where(ids).delete
+              db[table_name].insert(db[source_view_name].where(ids).distinct)
             end
           end
+        end
 
-          if params[:pacticipant_name]
-            pacticipant = PactBroker::Domain::Pacticipant.where(name_like(:name, params[:pacticipant_name])).single_record
-            if pacticipant
-              db.transaction do
-                db[table_name].where(consumer_id: pacticipant.id).or(provider_id: pacticipant.id).delete
-                new_rows = db[source_view_name].where(consumer_id: pacticipant.id).or(provider_id: pacticipant.id).distinct
-                db[table_name].insert(new_rows)
-              end
-            end
-          end
+        def source_view_name
+          model.table_name.to_s.gsub('materialized_', '').to_sym
         end
 
         def matching_selectors selectors

lib/pact_broker/matrix/service.rb

@@ -9,8 +9,12 @@ module Service
       extend PactBroker::Repositories
       extend PactBroker::Services
 
-      def refresh params
-        matrix_repository.refresh params
+      def refresh params, &block
+        matrix_repository.refresh(params, &block)
+      end
+
+      def refresh_tags params, &block
+        matrix_repository.refresh_tags(params, &block)
       end
 
       def find criteria, options = {}

spec/features/publish_verification_spec.rb

@@ -10,7 +10,6 @@
 
   subject { post path, verification_content, {'CONTENT_TYPE' => 'application/json' }; last_response  }
 
-
   before do
     td.create_provider("Provider")
       .create_consumer("Consumer")

spec/features/update_matrix_spec.rb

@@ -0,0 +1,127 @@
+=begin
+
+Things that should update the matrix (ignoring tags):
+* pact created
+* pact content updated
+* pact deleted
+* verification created
+* verification deleted (not yet implemented)
+
+It's easier to update the matrix at the resource level, so we actually update the matrix when:
+* pacticipant deleted
+* version deleted
+* pact deleted
+
+Things that should update the head matrix
+* All of the above
+* tag created
+* tag deleted
+
+=end
+
+describe "Deleting a resource that affects the matrix" do
+
+  let(:td) { TestDataBuilder.new }
+  let(:response_body_json) { JSON.parse(subject.body) }
+
+  subject { delete path; last_response  }
+
+  before do
+    td.create_pact_with_hierarchy("Foo", "1", "Bar")
+      .create_consumer_version_tag("prod")
+      .create_verification(provider_version: "2")
+  end
+
+  context "deleting a pact" do
+    let(:path) { "/pacts/provider/Bar/consumer/Foo/version/1" }
+
+    it "deletes the relevant lines from the matrix" do
+      expect{ subject }.to change{ PactBroker::Matrix::Row.count }.by(-1)
+    end
+  end
+
+  context "deleting a pacticipant" do
+    let(:path) { "/pacticipants/Bar" }
+
+    it "deletes the relevant lines from the matrix" do
+      expect{ subject }.to change{ PactBroker::Matrix::Row.count }.by(-1)
+    end
+  end
+
+  context "deleting a version" do
+    let(:path) { "/pacticipants/Foo/versions/1" }
+
+    it "deletes the relevant lines from the matrix" do
+      expect{ subject }.to change{ PactBroker::Matrix::Row.count }.by(-1)
+    end
+  end
+
+  context "deleting a tag" do
+    let(:path) { "/pacticipants/Foo/versions/1/tags/prod" }
+
+    it "deletes the relevant lines from the matrix" do
+      expect{ subject }.to change{ PactBroker::Matrix::HeadRow.count }.by(-1)
+    end
+  end
+end
+
+describe "Creating a resource that affects the matrix" do
+
+  let(:td) { TestDataBuilder.new }
+  let(:response_body_json) { JSON.parse(subject.body) }
+
+  subject { put(path, nil, {'CONTENT_TYPE' => 'application/json'}); last_response }
+
+  context "creating a tag" do
+    before do
+      td.create_pact_with_hierarchy("Foo", "1", "Bar")
+    end
+
+    let(:path) { "/pacticipants/Foo/versions/1/tags/prod" }
+
+    it "adds the relevant lines to the head matrix" do
+      expect{ subject }.to change{ PactBroker::Matrix::HeadRow.count }.by(1)
+    end
+
+    it "does not add any lines to the matrix" do
+      expect{ subject }.to change{ PactBroker::Matrix::Row.count }.by(0)
+    end
+  end
+
+  context "creating a pact" do
+    let(:pact_content) { load_fixture('foo-bar.json') }
+    let(:path) { "/pacts/provider/Bar/consumer/Foo/versions/1.2.3" }
+
+    subject { put path, pact_content, {'CONTENT_TYPE' => 'application/json'}; last_response }
+
+    it "adds the relevant lines to the matrix" do
+      expect{ subject }.to change{ PactBroker::Matrix::Row.count }.by(1)
+    end
+
+    it "adds the relevant lines to the head matrix" do
+      expect{ subject }.to change{ PactBroker::Matrix::HeadRow.count }.by(1)
+    end
+  end
+
+  context "creating a verification" do
+    let(:td) { TestDataBuilder.new }
+    let(:path) { "/pacts/provider/Bar/consumer/Foo/pact-version/#{pact.pact_version_sha}/verification-results" }
+    let(:verification_content) { load_fixture('verification.json') }
+    let(:parsed_response_body) { JSON.parse(subject.body) }
+    let(:pact) { td.pact }
+
+    subject { post path, verification_content, {'CONTENT_TYPE' => 'application/json' }; last_response  }
+
+    before do
+      td.create_pact_with_hierarchy("Foo", "1", "Bar")
+    end
+
+    it "updates the relevant lines in the matrix" do
+      expect{ subject }.to change{ PactBroker::Matrix::Row.first.provider_version_number }.from(nil).to("4.5.6")
+    end
+
+    it "updates the relevant lines in the head matrix" do
+      expect{ subject }.to change{ PactBroker::Matrix::HeadRow.first.provider_version_number }.from(nil).to("4.5.6")
+    end
+  end
+end