fix: fix clean performance fix (#530)

* fix(cleanup): Improve delete orphans SQL query (#527)

* fix(cleanup): Improve delete orphans SQL query

Original query to delete orphans was terribly slow and was unable to
finish within default 15s timeout. Instead of excluding all ids from
union, we left join all 3 tables and select only rows that have no
joined values in pact_publications or verifications.

Additionaly, the query was fixed for mysql. Whenever the Mysql error
occures instead of embedded query, ids are fetched and then directly
used in delete query.

* fix: Add sql_enable_caller_logging documentation

* fix: Fix unreliable spec

* fix: fully qualify pact_version table joins in database clean

Co-authored-by: Bartek Bułat <[email protected]>

lib/pact_broker/db/clean_incremental.rb

@@ -94,8 +94,23 @@ def dry_run?
       end
 
       def delete_orphan_pact_versions
-        referenced_pact_version_ids = db[:pact_publications].select(:pact_version_id).union(db[:verifications].select(:pact_version_id))
-        db[:pact_versions].where(id: referenced_pact_version_ids).invert.delete
+        db[:pact_versions].where(id: orphan_pact_versions).delete
+      rescue Sequel::DatabaseError => e
+        raise unless e.cause.class.name == "Mysql2::Error"
+
+        ids = orphan_pact_versions.map { |row| row[:id] }
+        db[:pact_versions].where(id: ids).delete
+      end
+
+      def orphan_pact_versions
+        db[:pact_versions]
+          .left_join(:pact_publications, Sequel[:pact_publications][:pact_version_id]=> Sequel[:pact_versions][:id])
+          .left_join(:verifications, Sequel[:verifications][:pact_version_id]=> Sequel[:pact_versions][:id])
+          .select(Sequel[:pact_versions][:id])
+          .where(
+            Sequel[:pact_publications][:id] => nil,
+            Sequel[:verifications][:id] => nil
+          )
       end
 
       def version_info(version)

spec/lib/pact_broker/db/clean_incremental_spec.rb

@@ -3,8 +3,7 @@
 
 module PactBroker
   module DB
-    # Inner queries don't work on MySQL. Seriously, MySQL???
-    xdescribe CleanIncremental do
+    describe CleanIncremental do
       def pact_publication_count_for(consumer_name, version_number)
         PactBroker::Pacts::PactPublication.where(consumer_version: PactBroker::Domain::Version.where_pacticipant_name(consumer_name).where(number: version_number)).count
       end
@@ -85,19 +84,24 @@ def pact_publication_count_for(consumer_name, version_number)
               expect { subject }.to_not change { PactBroker::Domain::Version.count }
             end
 
-            # Always fails on github actions, never locally :shrug:
-            it "returns info on what will be deleted", pending: ENV["CI"] == "true" do
+            # Randomly fails on github actions, never locally :shrug:
+            it "returns info on what will be deleted", skip: ENV["CI"] == "true" do
               Approvals.verify(subject, :name => "clean_incremental_dry_run", format: :json)
             end
           end
         end
 
-
         context "with orphan pact versions" do
           before do
-            # Create a pact that will not be deleted
-            td.create_pact_with_hierarchy("Foo", "0", "Bar", json_content_1)
+            # json_content_3 referenced by pact_publication for Foo v1
+            td.create_pact_with_hierarchy("Foo", "1", "Bar", json_content_3).comment("Foo v1 kept because latest dev")
               .create_consumer_version_tag("dev")
+
+            # json_content_4 referenced by a verification (but not a pact_publication)
+            td.create_pact_with_hierarchy("Waffle", "0", "Meep", json_content_4)
+              .create_verification(provider_version: "5", tag_names: ["dev"], comment: "Meep v5 kept because latest dev")
+            PactBroker::Pacts::PactPublication.order(:id).last.delete
+
             # Create an orphan pact version
             pact_version_params = PactBroker::Pacts::PactVersion.first.to_hash
             pact_version_params.delete(:id)
@@ -107,6 +111,8 @@ def pact_publication_count_for(consumer_name, version_number)
 
           let(:json_content_1) { { interactions: ["a", "b"]}.to_json }
           let(:json_content_2) { { interactions: ["a", "c"]}.to_json }
+          let(:json_content_3) { { interactions: ["a", "f"]}.to_json }
+          let(:json_content_4) { { interactions: ["a", "h"]}.to_json }
 
           let(:options) { { keep: [latest_dev_selector] } }
 

spec/lib/pact_broker/verifications/repository_spec.rb

@@ -46,9 +46,9 @@ module Verifications
 
           it "creates a PactVersionProviderTagSuccessfulVerification for each tag" do
             expect { subject }.to change { PactVersionProviderTagSuccessfulVerification.count }.by(2)
-            expect(PactVersionProviderTagSuccessfulVerification.first).to have_attributes(
-              wip: false,
-              provider_version_tag_name: "foo"
+            expect(PactVersionProviderTagSuccessfulVerification.all).to contain_exactly(
+              have_attributes(wip: false, provider_version_tag_name: "foo"),
+              have_attributes(wip: false, provider_version_tag_name: "bar"),
             )
           end
         end