Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/configuration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,15 @@ groups:
default_value: 5
allowed_values_description: A positive integer or float, as a string.
more_info: https://sequel.jeremyevans.net/rdoc/files/doc/opening_databases_rdoc.html#label-General+connection+options
sql_enable_caller_logging:
description: |-
Whether or not to enable caller_logging extension for database connection.
When enabled it logs source path that caused SQL query.
default_value: false
allowed_values:
- true
- false
more_info: https://sequel.jeremyevans.net/rdoc-plugins/files/lib/sequel/extensions/caller_logging_rb.html
database_max_connections:
description: "The maximum size of the connection pool (4 connections by default on most databases)"
default_value: 4
Expand Down
19 changes: 17 additions & 2 deletions lib/pact_broker/db/clean_incremental.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,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, pact_version_id: :id)
.left_join(:verifications, pact_version_id: :id)
.select(Sequel[:pact_versions][:id])
.where(
Sequel[:pact_publications][:id] => nil,
Sequel[:verifications][:id] => nil
)
end

def version_info(version)
Expand Down
8 changes: 3 additions & 5 deletions spec/lib/pact_broker/db/clean_incremental_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -85,14 +84,13 @@ 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
Expand Down
6 changes: 3 additions & 3 deletions spec/lib/pact_broker/verifications/repository_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down