From 42d2feb8021dfe4a9bfa21b0a620e57fdf7199e4 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Mon, 16 Aug 2021 12:03:51 +1000 Subject: [PATCH 01/45] chore: add branches tables --- .../20210816_create_branches_tables.rb | 25 ++++++ lib/pact_broker/domain/tag.rb | 3 +- lib/pact_broker/domain/version.rb | 5 ++ lib/pact_broker/test/test_data_builder.rb | 9 +- lib/pact_broker/versions/repository.rb | 34 ++++++-- .../pact_broker/versions/repository_spec.rb | 87 +++++++++++++++++++ 6 files changed, 153 insertions(+), 10 deletions(-) create mode 100644 db/migrations/20210816_create_branches_tables.rb diff --git a/db/migrations/20210816_create_branches_tables.rb b/db/migrations/20210816_create_branches_tables.rb new file mode 100644 index 000000000..b7352c0c7 --- /dev/null +++ b/db/migrations/20210816_create_branches_tables.rb @@ -0,0 +1,25 @@ +Sequel.migration do + change do + create_table(:branches, charset: "utf8") do + primary_key :id + String :name + foreign_key :pacticipant_id, :pacticipants, null: false + DateTime :created_at, null: false + DateTime :updated_at, null: false + index [:name, :pacticipant_id], unique: true, name: :branches_name_pacticipant_id_index + end + + create_table(:branch_versions, charset: "utf8") do + primary_key :id + foreign_key :branch_id, :branches, null: false, foreign_key_constraint_name: :branch_versions_branches_fk + String :branch_name, null: false # duplicate + foreign_key :version_id, :versions, null: false, foreign_key_constraint_name: :branch_versions_versions_fk + Integer :version_order, null: false + Integer :pacticipant_id, null: false + DateTime :created_at, null: false + DateTime :updated_at, null: false + index [:branch_id, :version_id], unique: true, name: :branch_versions_branch_id_version_id_index + index [:pacticipant_id, :branch_id, :version_order], name: :branch_versions_pacticipant_id_branch_id_version_order_index + end + end +end diff --git a/lib/pact_broker/domain/tag.rb b/lib/pact_broker/domain/tag.rb index c510b32dc..28879e6be 100644 --- a/lib/pact_broker/domain/tag.rb +++ b/lib/pact_broker/domain/tag.rb @@ -130,6 +130,7 @@ def head_tags_for_pact_publication(pact_publication) # rubocop: disable Metrics/CyclomaticComplexity def before_save + super if version if version.order && self.version_order.nil? self.version_order = version.order @@ -146,8 +147,6 @@ def before_save if version_order.nil? || pacticipant_id.nil? raise PactBroker::Error.new("Need to set version_order and pacticipant_id for tags now") - else - super end end # rubocop: enable Metrics/CyclomaticComplexity diff --git a/lib/pact_broker/domain/version.rb b/lib/pact_broker/domain/version.rb index 0e12b2f44..c4972b50b 100644 --- a/lib/pact_broker/domain/version.rb +++ b/lib/pact_broker/domain/version.rb @@ -30,6 +30,7 @@ class Version < Sequel::Model one_to_many :pact_publications, order: :revision_number, class: "PactBroker::Pacts::PactPublication", key: :consumer_version_id associate(:many_to_one, :pacticipant, :class => "PactBroker::Domain::Pacticipant", :key => :pacticipant_id, :primary_key => :id) one_to_many :tags, :reciprocal => :version, order: :created_at + one_to_many :branch_versions, :reciprocal => :branch_version, class: "PactBroker::Versions::BranchVersion", order: [:created_at, :id] one_to_many :current_deployed_versions, class: "PactBroker::Deployments::DeployedVersion", key: :version_id, primary_key: :id, order: [:created_at, :id] do | ds | ds.currently_deployed end @@ -232,6 +233,10 @@ def latest_for_branch? def latest_for_pacticipant? latest_version_for_pacticipant == self end + + def belongs_to_branch?(branch) + branch_versions.any? { | branch_version | branch_version.branch_id == branch.id } + end end end end diff --git a/lib/pact_broker/test/test_data_builder.rb b/lib/pact_broker/test/test_data_builder.rb index 5153ca33a..1710ca50a 100644 --- a/lib/pact_broker/test/test_data_builder.rb +++ b/lib/pact_broker/test/test_data_builder.rb @@ -170,12 +170,15 @@ def create_version version_number = "1.0.#{model_counter}", params = {} def create_consumer_version version_number = "1.0.#{model_counter}", params = {} params.delete(:comment) tag_names = [params.delete(:tag_names), params.delete(:tag_name)].flatten.compact - @consumer_version = PactBroker::Domain::Version.create( + args = { number: version_number, - pacticipant: @consumer, + pacticipant_id: @consumer.id, branch: params[:branch], build_url: params[:build_url] - ) + } + + @consumer_version = PactBroker::Versions::Repository.new.create(args) + set_created_at_if_set params[:created_at], :versions, { id: @consumer_version.id } tag_names.each do | tag_name | tag = PactBroker::Domain::Tag.create(name: tag_name, version: consumer_version) diff --git a/lib/pact_broker/versions/repository.rb b/lib/pact_broker/versions/repository.rb index 66d2d462f..9b227e653 100644 --- a/lib/pact_broker/versions/repository.rb +++ b/lib/pact_broker/versions/repository.rb @@ -2,6 +2,8 @@ require "pact_broker/logging" require "pact_broker/domain/version" require "pact_broker/tags/repository" +require "pact_broker/versions/branch" +require "pact_broker/versions/branch_version" module PactBroker module Versions @@ -55,16 +57,25 @@ def create args number: args[:number], pacticipant_id: args[:pacticipant_id], created_at: Sequel.datetime_class.now, - updated_at: Sequel.datetime_class.now - } + updated_at: Sequel.datetime_class.now, + build_url: args[:build_url], + branch: args[:branch] + }.compact - PactBroker::Domain::Version.new(version_params).upsert + version = PactBroker::Domain::Version.new(version_params).upsert + + if args[:branch] + add_branch(version, args[:branch]) + end + + version end def create_or_update(pacticipant, version_number, open_struct_version) saved_version = PactBroker::Domain::Version.where(pacticipant_id: pacticipant.id, number: version_number).single_record params = open_struct_version.to_h tags = params.delete(:tags) + branch_name = params[:branch] # TODO branches if saved_version saved_version.update(params) else @@ -73,11 +84,13 @@ def create_or_update(pacticipant, version_number, open_struct_version) saved_version = PactBroker::Domain::Version.new( params.merge( pacticipant_id: pacticipant.id, - number: version_number - ) + number: version_number, + branch: branch_name, + ).compact ).upsert end + add_branch(saved_version, branch_name) if branch_name replace_tags(saved_version, tags) if tags saved_version end @@ -94,6 +107,10 @@ def create_or_overwrite(pacticipant, version_number, open_struct_version) replace_tags(saved_version, open_struct_version.tags) end + if open_struct_version.branches + update_branches(saved_version, open_struct_version.branches) + end + saved_version end @@ -143,6 +160,13 @@ def find_latest_version_from_main_branch(pacticipant) latest_from_main_branch || find_by_pacticipant_name_and_latest_tag(pacticipant.name, pacticipant.main_branch) end end + + def add_branch(version, branch_name) + branch = PactBroker::Versions::Branch.new(pacticipant: version.pacticipant, name: branch_name).insert_ignore + if !version.belongs_to_branch?(branch) + PactBroker::Versions::BranchVersion.new(version: version, branch: branch, branch_name: branch_name).insert_ignore + end + end end end end diff --git a/spec/lib/pact_broker/versions/repository_spec.rb b/spec/lib/pact_broker/versions/repository_spec.rb index 32e4faa92..35f340ef7 100644 --- a/spec/lib/pact_broker/versions/repository_spec.rb +++ b/spec/lib/pact_broker/versions/repository_spec.rb @@ -121,6 +121,93 @@ module Versions end end + describe "#create_or_update" do + before do + td.subtract_day + .create_consumer("Foo") + .create_consumer_version(version_number, branch: "original-branch", build_url: "original-build-url") + .create_consumer_version_tag("dev") + end + + let(:pacticipant) { td.and_return(:consumer) } + let(:version_number) { "1234" } + let(:tags) { nil } + let(:open_struct_version) { OpenStruct.new(branch: new_branch, tags: tags) } + let(:new_branch) { "new-branch" } + + subject { Repository.new.create_or_update(pacticipant, version_number, open_struct_version) } + + it "does not overwrite missing values the values" do + expect(subject.branch).to eq "new-branch" + expect(subject.build_url).to eq "original-build-url" + end + + it "does not change the tags" do + expect { subject }.to_not change { PactBroker::Domain::Version.for("Foo", "1234").tags } + end + + context "when the branch does not already exist" do + it "creates a branch" do + expect { subject }.to change { PactBroker::Versions::Branch.count }.by(1) + end + + it "creates a branch_version" do + expect { subject }.to change { PactBroker::Versions::BranchVersion.count }.by(1) + end + + it "adds the branch_version to the version" do + expect(subject.branch_versions.count).to eq 2 + expect(subject.branch_versions.last.branch_name).to eq "new-branch" + end + end + + context "when the branch and branch version do already exist" do + let(:new_branch) { "original-branch" } + + it "does not creates a branch" do + expect { subject }.to_not change { PactBroker::Versions::Branch.order(:name).collect(&:name) } + end + + it "does not create a branch_version" do + expect { subject }.to change { PactBroker::Versions::BranchVersion.count }.by(0) + end + + it "keeps the branch_version on the version" do + expect(subject.branch_versions.count).to eq 1 + expect(subject.branch_versions.first.branch_name).to eq "original-branch" + end + end + + context "when there are tags specified" do + let(:tags) { [ OpenStruct.new(name: "main")] } + + it "overwrites the tags" do + expect(subject.tags.count).to eq 1 + expect(subject.tags.first.name).to eq "main" + end + end + + it "does not change the created date" do + expect { subject }.to_not change { PactBroker::Domain::Version.for("Foo", "1234").created_at } + end + + it "does change the updated date" do + expect { subject }.to change { PactBroker::Domain::Version.for("Foo", "1234").updated_at } + end + + it "maintains the order" do + expect { subject }.to_not change { PactBroker::Domain::Version.for("Foo", "1234").order } + end + + context "when the version does not already exist" do + let(:version) { OpenStruct.new(number: "555", branch: "new-branch") } + + it "sets the order" do + expect(subject.order).to_not be nil + end + end + end + describe "#create_or_overwrite" do before do td.subtract_day From 9b043d59e152edafed48dd2180a36384bfbe1721 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Mon, 16 Aug 2021 13:44:20 +1000 Subject: [PATCH 02/45] chore: keep track of the latest version for a branch --- .../20210816_create_branches_tables.rb | 18 +++++++-- lib/pact_broker/db/models.rb | 6 +++ lib/pact_broker/domain/pacticipant.rb | 1 + lib/pact_broker/domain/version.rb | 4 +- lib/pact_broker/versions/branch.rb | 13 +++++++ lib/pact_broker/versions/branch_head.rb | 24 ++++++++++++ lib/pact_broker/versions/branch_version.rb | 38 +++++++++++++++++++ lib/pact_broker/versions/repository.rb | 7 +++- .../pact_broker/versions/repository_spec.rb | 10 +++++ 9 files changed, 113 insertions(+), 8 deletions(-) create mode 100644 lib/pact_broker/versions/branch.rb create mode 100644 lib/pact_broker/versions/branch_head.rb create mode 100644 lib/pact_broker/versions/branch_version.rb diff --git a/db/migrations/20210816_create_branches_tables.rb b/db/migrations/20210816_create_branches_tables.rb index b7352c0c7..9aac84b02 100644 --- a/db/migrations/20210816_create_branches_tables.rb +++ b/db/migrations/20210816_create_branches_tables.rb @@ -3,7 +3,9 @@ create_table(:branches, charset: "utf8") do primary_key :id String :name - foreign_key :pacticipant_id, :pacticipants, null: false + foreign_key :pacticipant_id, :pacticipants, null: false, on_delete: :cascade + Integer :latest_branch_version_id + Integer :latest_version_id DateTime :created_at, null: false DateTime :updated_at, null: false index [:name, :pacticipant_id], unique: true, name: :branches_name_pacticipant_id_index @@ -11,9 +13,8 @@ create_table(:branch_versions, charset: "utf8") do primary_key :id - foreign_key :branch_id, :branches, null: false, foreign_key_constraint_name: :branch_versions_branches_fk - String :branch_name, null: false # duplicate - foreign_key :version_id, :versions, null: false, foreign_key_constraint_name: :branch_versions_versions_fk + foreign_key :branch_id, :branches, null: false, foreign_key_constraint_name: :branch_versions_branches_fk, on_delete: :cascade + foreign_key :version_id, :versions, null: false, foreign_key_constraint_name: :branch_versions_versions_fk, on_delete: :cascade Integer :version_order, null: false Integer :pacticipant_id, null: false DateTime :created_at, null: false @@ -21,5 +22,14 @@ index [:branch_id, :version_id], unique: true, name: :branch_versions_branch_id_version_id_index index [:pacticipant_id, :branch_id, :version_order], name: :branch_versions_pacticipant_id_branch_id_version_order_index end + + create_table(:branch_heads) do + primary_key :id + foreign_key :branch_id, :branches, null: false, on_delete: :cascade + foreign_key :branch_version_id, :branch_versions, null: false, on_delete: :cascade + Integer :version_id, null: false + Integer :pacticipant_id, null: false + index([:branch_id], unique: true, name: :branch_heads_branch_id_index) + end end end diff --git a/lib/pact_broker/db/models.rb b/lib/pact_broker/db/models.rb index 5bc949644..489692b06 100644 --- a/lib/pact_broker/db/models.rb +++ b/lib/pact_broker/db/models.rb @@ -16,6 +16,9 @@ require "pact_broker/deployments/released_version" require "pact_broker/matrix/row" require "pact_broker/matrix/head_row" +require "pact_broker/versions/branch" +require "pact_broker/versions/branch_version" +require "pact_broker/versions/branch_head" module PactBroker INTEGRATIONS_TABLES = [ @@ -30,6 +33,9 @@ module PactBroker PactBroker::Domain::Tag, PactBroker::Deployments::DeployedVersion, PactBroker::Deployments::ReleasedVersion, + PactBroker::Versions::BranchHead, + PactBroker::Versions::BranchVersion, + PactBroker::Versions::Branch, PactBroker::Domain::Version, PactBroker::Domain::Label, PactBroker::Domain::Pacticipant diff --git a/lib/pact_broker/domain/pacticipant.rb b/lib/pact_broker/domain/pacticipant.rb index 69e49ce77..01da309ef 100644 --- a/lib/pact_broker/domain/pacticipant.rb +++ b/lib/pact_broker/domain/pacticipant.rb @@ -22,6 +22,7 @@ class Pacticipant < Sequel::Model one_to_many :labels, :order => :name, :reciprocal => :pacticipant one_to_many :pacts one_to_one :latest_version, :class => "PactBroker::Versions::LatestVersion", primary_key: :id, key: :pacticipant_id + one_to_many :branch_heads, class: "PactBroker::Versions::BranchHead", primary_key: :id, key: :pacticipant_id dataset_module do include PactBroker::Repositories::Helpers diff --git a/lib/pact_broker/domain/version.rb b/lib/pact_broker/domain/version.rb index c4972b50b..c622d4a72 100644 --- a/lib/pact_broker/domain/version.rb +++ b/lib/pact_broker/domain/version.rb @@ -234,8 +234,8 @@ def latest_for_pacticipant? latest_version_for_pacticipant == self end - def belongs_to_branch?(branch) - branch_versions.any? { | branch_version | branch_version.branch_id == branch.id } + def branch_version_for_branch(branch) + branch_versions.find { | branch_version | branch_version.branch_id == branch.id } end end end diff --git a/lib/pact_broker/versions/branch.rb b/lib/pact_broker/versions/branch.rb new file mode 100644 index 000000000..80de7e0ee --- /dev/null +++ b/lib/pact_broker/versions/branch.rb @@ -0,0 +1,13 @@ +require "pact_broker/db" +require "pact_broker/repositories/helpers" + +module PactBroker + module Versions + class Branch < Sequel::Model(:branches) + plugin :timestamps, update_on_create: true + plugin :insert_ignore, identifying_columns: [:name, :pacticipant_id] + + associate(:many_to_one, :pacticipant, :class => "PactBroker::Domain::Pacticipant", :key => :pacticipant_id, :primary_key => :id) + end + end +end diff --git a/lib/pact_broker/versions/branch_head.rb b/lib/pact_broker/versions/branch_head.rb new file mode 100644 index 000000000..2faac8219 --- /dev/null +++ b/lib/pact_broker/versions/branch_head.rb @@ -0,0 +1,24 @@ +require "pact_broker/db" +require "pact_broker/repositories/helpers" + +module PactBroker + module Versions + class BranchHead < Sequel::Model + plugin :upsert, identifying_columns: [:branch_id] + + associate(:many_to_one, :branch, :class => "PactBroker::Versions::Branch", :key => :branch_id, :primary_key => :id) + associate(:many_to_one, :branch_version, :class => "PactBroker::Versions::BranchVersion", :key => :branch_version_id, :primary_key => :id) + associate(:many_to_one, :version, :class => "PactBroker::Domain::Version", :key => :version_id, :primary_key => :id) + + def before_save + super + self.pacticipant_id = branch.pacticipant_id + self.version_id = branch_version.version_id + end + + def branch_name + branch.name + end + end + end +end diff --git a/lib/pact_broker/versions/branch_version.rb b/lib/pact_broker/versions/branch_version.rb new file mode 100644 index 000000000..12e1f5a83 --- /dev/null +++ b/lib/pact_broker/versions/branch_version.rb @@ -0,0 +1,38 @@ +require "pact_broker/db" +require "pact_broker/repositories/helpers" + +module PactBroker + module Versions + class BranchVersion < Sequel::Model(:branch_versions) + plugin :timestamps, update_on_create: true + plugin :insert_ignore, identifying_columns: [:branch_id, :version_id] + + associate(:many_to_one, :branch, :class => "PactBroker::Versions::Branch", :key => :branch_id, :primary_key => :id) + associate(:many_to_one, :version, :class => "PactBroker::Domain::Version", :key => :version_id, :primary_key => :id) + + def before_save + super + + if version.order && self.version_order.nil? + self.version_order = version.order + end + + if self.pacticipant_id.nil? + if version.pacticipant_id + self.pacticipant_id = version.pacticipant_id + elsif version&.pacticipant&.id + self.pacticipant_id = version.pacticipant.id + end + end + + if version_order.nil? || pacticipant_id.nil? + raise PactBroker::Error.new("Need to set version_order and pacticipant_id for tags now") + end + end + + def branch_name + branch.name + end + end + end +end diff --git a/lib/pact_broker/versions/repository.rb b/lib/pact_broker/versions/repository.rb index 9b227e653..3ae357212 100644 --- a/lib/pact_broker/versions/repository.rb +++ b/lib/pact_broker/versions/repository.rb @@ -4,6 +4,7 @@ require "pact_broker/tags/repository" require "pact_broker/versions/branch" require "pact_broker/versions/branch_version" +require "pact_broker/versions/branch_head" module PactBroker module Versions @@ -163,9 +164,11 @@ def find_latest_version_from_main_branch(pacticipant) def add_branch(version, branch_name) branch = PactBroker::Versions::Branch.new(pacticipant: version.pacticipant, name: branch_name).insert_ignore - if !version.belongs_to_branch?(branch) - PactBroker::Versions::BranchVersion.new(version: version, branch: branch, branch_name: branch_name).insert_ignore + branch_version = version.branch_version_for_branch(branch) + if !branch_version + branch_version = PactBroker::Versions::BranchVersion.new(version: version, branch: branch).insert_ignore end + PactBroker::Versions::BranchHead.new(branch: branch, branch_version: branch_version).upsert end end end diff --git a/spec/lib/pact_broker/versions/repository_spec.rb b/spec/lib/pact_broker/versions/repository_spec.rb index 35f340ef7..51e01f935 100644 --- a/spec/lib/pact_broker/versions/repository_spec.rb +++ b/spec/lib/pact_broker/versions/repository_spec.rb @@ -159,6 +159,11 @@ module Versions expect(subject.branch_versions.count).to eq 2 expect(subject.branch_versions.last.branch_name).to eq "new-branch" end + + it "updates the branch head" do + branch_head = subject.pacticipant.branch_heads.find{ | branch_head | branch_head.branch_name == "new-branch" } + expect(branch_head.version).to eq subject + end end context "when the branch and branch version do already exist" do @@ -176,6 +181,11 @@ module Versions expect(subject.branch_versions.count).to eq 1 expect(subject.branch_versions.first.branch_name).to eq "original-branch" end + + it "does not change the branch head" do + branch_head = subject.pacticipant.branch_heads.find{ | branch_head | branch_head.branch_name == "original-branch" } + expect(branch_head.version).to eq subject + end end context "when there are tags specified" do From c4b5a5c142b3f619eeb6444684df12b54bea753b Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Mon, 16 Aug 2021 14:02:44 +1000 Subject: [PATCH 03/45] chore: expose branches in version resource --- .../embedded_branch_version_decorator.rb | 20 +++++++++++++++++++ .../api/decorators/embedded_tag_decorator.rb | 5 ----- .../api/decorators/version_decorator.rb | 2 +- lib/pact_broker/api/pact_broker_urls.rb | 5 +++++ .../api/decorators/version_decorator_spec.rb | 9 ++++++--- 5 files changed, 32 insertions(+), 9 deletions(-) create mode 100644 lib/pact_broker/api/decorators/embedded_branch_version_decorator.rb diff --git a/lib/pact_broker/api/decorators/embedded_branch_version_decorator.rb b/lib/pact_broker/api/decorators/embedded_branch_version_decorator.rb new file mode 100644 index 000000000..0be01294e --- /dev/null +++ b/lib/pact_broker/api/decorators/embedded_branch_version_decorator.rb @@ -0,0 +1,20 @@ +require_relative "base_decorator" +require_relative "timestamps" + +module PactBroker + module Api + module Decorators + class EmbeddedBranchVersionDecorator < BaseDecorator + property :branch_name, as: :name + + link :self do | options | + { + title: "Version branch", + name: represented.branch_name, + href: branch_version_url(represented, options[:base_url]) + } + end + end + end + end +end diff --git a/lib/pact_broker/api/decorators/embedded_tag_decorator.rb b/lib/pact_broker/api/decorators/embedded_tag_decorator.rb index 376b66041..53a990cf7 100644 --- a/lib/pact_broker/api/decorators/embedded_tag_decorator.rb +++ b/lib/pact_broker/api/decorators/embedded_tag_decorator.rb @@ -1,15 +1,10 @@ require_relative "base_decorator" -require_relative "pact_pacticipant_decorator" require_relative "timestamps" module PactBroker - module Api - module Decorators - class EmbeddedTagDecorator < BaseDecorator - property :name link :self do | options | diff --git a/lib/pact_broker/api/decorators/version_decorator.rb b/lib/pact_broker/api/decorators/version_decorator.rb index 5c482818b..b2400a87d 100644 --- a/lib/pact_broker/api/decorators/version_decorator.rb +++ b/lib/pact_broker/api/decorators/version_decorator.rb @@ -7,7 +7,7 @@ module Decorators class VersionDecorator < BaseDecorator property :number, writeable: false - property :branch + collection :branch_versions, as: :branches, embedded: true, writeable: false, extend: PactBroker::Api::Decorators::EmbeddedBranchVersionDecorator property :build_url, as: :buildUrl collection :tags, embedded: true, :extend => PactBroker::Api::Decorators::EmbeddedTagDecorator, class: OpenStruct diff --git a/lib/pact_broker/api/pact_broker_urls.rb b/lib/pact_broker/api/pact_broker_urls.rb index 7d1c98bbe..f9f95fed4 100644 --- a/lib/pact_broker/api/pact_broker_urls.rb +++ b/lib/pact_broker/api/pact_broker_urls.rb @@ -199,6 +199,11 @@ def tag_url base_url, tag "#{tags_url(base_url, tag.version)}/#{url_encode(tag.name)}" end + # This resource does not actually exist + def branch_version_url(branch_version, base_url = "") + "#{version_url(base_url, branch_version.version)}/branches/#{branch_version.branch_name}" + end + def templated_tag_url_for_pacticipant pacticipant_name, base_url = "" pacticipant_url_from_params({ pacticipant_name: pacticipant_name }, base_url) + "/versions/{version}/tags/{tag}" end diff --git a/spec/lib/pact_broker/api/decorators/version_decorator_spec.rb b/spec/lib/pact_broker/api/decorators/version_decorator_spec.rb index ec438da00..ead888436 100644 --- a/spec/lib/pact_broker/api/decorators/version_decorator_spec.rb +++ b/spec/lib/pact_broker/api/decorators/version_decorator_spec.rb @@ -8,7 +8,6 @@ module Decorators describe "from_json" do let(:hash) do { - branch: "branch", buildUrl: "buildUrl", tags: [{ name: "main" }] } @@ -17,7 +16,6 @@ module Decorators subject { VersionDecorator.new(OpenStruct.new).from_json(hash.to_json) } it "sets the properties" do - expect(subject.branch).to eq "branch" expect(subject.build_url).to eq "buildUrl" expect(subject.tags.first.name).to eq "main" end @@ -32,7 +30,7 @@ module Decorators TestDataBuilder.new .create_consumer("Consumer") .create_provider("providerA") - .create_consumer_version("1.2.3") + .create_consumer_version("1.2.3", branch: "main") .create_consumer_version_tag("prod") .create_pact .create_provider("ProviderB") @@ -85,6 +83,11 @@ module Decorators expect(subject[:_embedded][:tags].first[:name]).to eq "prod" end + it "includes the branches" do + expect(subject[:_embedded][:branches]).to be_instance_of(Array) + expect(subject[:_embedded][:branches].first[:name]).to eq "main" + end + it "includes the timestamps" do expect(subject[:createdAt]).to_not be nil end From 7bdd6a4437d6fe73559ae5332b9bebea7efff84e Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Mon, 16 Aug 2021 14:32:36 +1000 Subject: [PATCH 04/45] test: remove branch expectations for creating versions --- spec/features/create_version_spec.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/spec/features/create_version_spec.rb b/spec/features/create_version_spec.rb index 1fc04db42..1e4fc8ea6 100644 --- a/spec/features/create_version_spec.rb +++ b/spec/features/create_version_spec.rb @@ -5,7 +5,6 @@ let(:response_body) { JSON.parse(subject.body, symbolize_names: true)} let(:version_hash) do { - branch: "main", buildUrl: "http://build", tags: [{ name: "foo" }, { name: "bar" }] } @@ -23,7 +22,7 @@ end it "returns the newly created version" do - expect(response_body).to include branch: "main", buildUrl: "http://build" + expect(response_body).to include buildUrl: "http://build" expect(response_body[:_embedded][:tags].size).to eq 2 end @@ -46,7 +45,7 @@ end it "returns the newly created version" do - expect(response_body).to include branch: "main", buildUrl: "http://build" + expect(response_body).to include buildUrl: "http://build" expect(response_body[:_embedded][:tags].size).to eq 2 end @@ -54,5 +53,4 @@ expect { subject }.to change { PactBroker::Domain::Tag.count }.by(2) end end - end From 13aa7dca3c19cc21b921164dccf4d9956cedc265 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Mon, 16 Aug 2021 14:35:19 +1000 Subject: [PATCH 05/45] test: remove tests for updating versions with a branch --- spec/features/update_version_spec.rb | 55 ---------------------------- 1 file changed, 55 deletions(-) diff --git a/spec/features/update_version_spec.rb b/spec/features/update_version_spec.rb index e7bdfa8b9..9550abc9e 100644 --- a/spec/features/update_version_spec.rb +++ b/spec/features/update_version_spec.rb @@ -28,49 +28,9 @@ end it "does not overwrites any properties that weren't specified" do - expect(response_body[:branch]).to eq "original-branch" expect(response_body[:buildUrl]).to eq "new-build-url" end - context "when the same not null branch is specified" do - let(:version_hash) { { branch: "original-branch" } } - - its(:status) { is_expected.to eq 200 } - end - - context "when the existing version has a branch, and the new branch is different", skip: true do - let(:version_hash) { { branch: "new-branch" } } - - its(:status) { is_expected.to eq 409 } - - it "returns an error" do - expect(response_body[:errors][:branch].first).to include "cannot be changed" - end - end - - context "when the existing version has a branch, and the new branch is nil", skip: true do - let(:version_hash) { { branch: nil } } - - its(:status) { is_expected.to eq 409 } - - it "returns an error" do - expect(response_body[:errors][:branch].first).to include "cannot be changed" - end - end - - context "when the existing version does not have a branch, and the new branch is specified" do - let(:original_branch) { nil } - - its(:status) { is_expected.to eq 200 } - end - - context "when the existing version does not have a branch, and the new branch is also nil" do - let(:original_branch) { nil } - let(:version_hash) { { branch: nil } } - - its(:status) { is_expected.to eq 200 } - end - context "when no tags are specified" do it "does not change the tags" do expect { subject }.to_not change { PactBroker::Domain::Version.for("Foo", "1234").tags } @@ -117,21 +77,6 @@ .create_consumer_version_tag("dev") end - context "when the branch is attempted to be changed", skip: true do - let(:version_hash) { { branch: "new-branch" } } - - its(:status) { is_expected.to eq 409 } - end - - context "when the branch is not attempted to be changed" do - let(:version_hash) { { branch: "original-branch" } } - - it "overwrites the direct properties and blanks out any unprovided ones" do - expect(response_body[:branch]).to eq "original-branch" - expect(response_body).to_not have_key(:buildUrl) - end - end - context "when no tags are specified" do it "does not change the tags" do expect { subject }.to_not change { PactBroker::Domain::Version.for("Foo", "1234").tags } From 631788ac348c824909d2342c357deedf9fee0385 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Mon, 16 Aug 2021 16:24:38 +1000 Subject: [PATCH 06/45] chore: update branch head when a version is deleted --- lib/pact_broker/domain/pacticipant.rb | 5 +++ lib/pact_broker/versions/branch_version.rb | 7 ++++ lib/pact_broker/versions/repository.rb | 8 +++++ .../pact_broker/versions/repository_spec.rb | 35 ++++++++++++++++--- 4 files changed, 51 insertions(+), 4 deletions(-) diff --git a/lib/pact_broker/domain/pacticipant.rb b/lib/pact_broker/domain/pacticipant.rb index 01da309ef..36a9029de 100644 --- a/lib/pact_broker/domain/pacticipant.rb +++ b/lib/pact_broker/domain/pacticipant.rb @@ -23,6 +23,7 @@ class Pacticipant < Sequel::Model one_to_many :pacts one_to_one :latest_version, :class => "PactBroker::Versions::LatestVersion", primary_key: :id, key: :pacticipant_id one_to_many :branch_heads, class: "PactBroker::Versions::BranchHead", primary_key: :id, key: :pacticipant_id + one_to_many :branches, class: "PactBroker::Versions::Branch", primary_key: :id, key: :pacticipant_id dataset_module do include PactBroker::Repositories::Helpers @@ -67,6 +68,10 @@ def to_s def any_versions? PactBroker::Domain::Version.where(pacticipant: self).any? end + + def branch_head_for(branch_name) + branch_heads.find{ | branch_head | branch_head.branch_name == branch_name } + end end end end diff --git a/lib/pact_broker/versions/branch_version.rb b/lib/pact_broker/versions/branch_version.rb index 12e1f5a83..0d14c7104 100644 --- a/lib/pact_broker/versions/branch_version.rb +++ b/lib/pact_broker/versions/branch_version.rb @@ -10,6 +10,13 @@ class BranchVersion < Sequel::Model(:branch_versions) associate(:many_to_one, :branch, :class => "PactBroker::Versions::Branch", :key => :branch_id, :primary_key => :id) associate(:many_to_one, :version, :class => "PactBroker::Domain::Version", :key => :version_id, :primary_key => :id) + dataset_module do + def find_latest_for_branch(branch) + max_version_order = BranchVersion.select(Sequel.function(:max, :version_order)).where(branch_id: branch.id) + BranchVersion.where(branch_id: branch.id, version_order: max_version_order).single_record + end + end + def before_save super diff --git a/lib/pact_broker/versions/repository.rb b/lib/pact_broker/versions/repository.rb index 3ae357212..5bca39486 100644 --- a/lib/pact_broker/versions/repository.rb +++ b/lib/pact_broker/versions/repository.rb @@ -130,7 +130,15 @@ def find_by_pacticipant_id_and_number_or_create pacticipant_id, number end def delete_by_id version_ids + branches = Versions::Branch.where(id: Versions::BranchHead.select(:branch_id).where(version_id: version_ids)).all # these will be deleted Domain::Version.where(id: version_ids).delete + branches.each do | branch | + new_head_branch_version = Versions::BranchVersion.find_latest_for_branch(branch) + if new_head_branch_version + PactBroker::Versions::BranchHead.new(branch: branch, branch_version: new_head_branch_version).upsert + end + end + nil end def delete_orphan_versions consumer, provider diff --git a/spec/lib/pact_broker/versions/repository_spec.rb b/spec/lib/pact_broker/versions/repository_spec.rb index 51e01f935..0632adc47 100644 --- a/spec/lib/pact_broker/versions/repository_spec.rb +++ b/spec/lib/pact_broker/versions/repository_spec.rb @@ -63,8 +63,7 @@ module Versions describe "#delete_by_id" do let!(:version) do - TestDataBuilder.new - .create_consumer + td.create_consumer("Foo") .create_consumer_version("1.2.3") .create_consumer_version("4.5.6") .and_return(:consumer_version) @@ -75,6 +74,35 @@ module Versions it "deletes the version" do expect { subject }.to change{ PactBroker::Domain::Version.count }.by(-1) end + + context "when the deleted version is the latest for a branch" do + let!(:version) do + td.create_consumer("Foo") + .create_consumer_version("1.2.3", branch: "main") + .create_consumer_version("4.5.6", branch: "main") + .and_return(:consumer_version) + end + + it "updates the branch head" do + subject + expect(td.find_pacticipant("Foo").branch_head_for("main").version.number).to eq "1.2.3" + end + end + + context "when the deleted version is the latest and last for a branch" do + let!(:version) do + td.create_consumer("Foo") + .create_consumer_version("4.5.6", branch: "main") + .and_return(:consumer_version) + end + + it "leaves the branch, but deletes the branch head (not sure about this, but thinking the branch creation date is handy for the WIP/pending calculation)" do + subject + foo = td.find_pacticipant("Foo") + expect(foo.branches.collect(&:name)).to include "main" + expect(td.find_pacticipant("Foo").branch_head_for("main")).to be nil + end + end end describe "#find_by_pacticipant_name_and_number" do @@ -83,8 +111,7 @@ module Versions context "when the version exists" do before do - TestDataBuilder.new - .create_consumer("Another Consumer") + td.create_consumer("Another Consumer") .create_consumer(pacticipant_name) .create_consumer_version(version_number) .create_consumer_version_tag("prod") From 1980e6aef3441b2bed3295cda9dec3b3d25d86f3 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Mon, 16 Aug 2021 18:57:10 +1000 Subject: [PATCH 07/45] chore: update code to find versions by branch using the matrix selector --- .../20210816_create_branches_tables.rb | 6 ++++- lib/pact_broker/domain/version.rb | 22 +++++++++++++++---- lib/pact_broker/test/test_data_builder.rb | 9 +++++++- lib/pact_broker/versions/branch_head.rb | 1 + lib/pact_broker/versions/branch_version.rb | 8 +------ 5 files changed, 33 insertions(+), 13 deletions(-) diff --git a/db/migrations/20210816_create_branches_tables.rb b/db/migrations/20210816_create_branches_tables.rb index 9aac84b02..936c4ad4d 100644 --- a/db/migrations/20210816_create_branches_tables.rb +++ b/db/migrations/20210816_create_branches_tables.rb @@ -17,10 +17,12 @@ foreign_key :version_id, :versions, null: false, foreign_key_constraint_name: :branch_versions_versions_fk, on_delete: :cascade Integer :version_order, null: false Integer :pacticipant_id, null: false + String :branch_name, null: false DateTime :created_at, null: false DateTime :updated_at, null: false index [:branch_id, :version_id], unique: true, name: :branch_versions_branch_id_version_id_index index [:pacticipant_id, :branch_id, :version_order], name: :branch_versions_pacticipant_id_branch_id_version_order_index + index [:branch_name], name: :branch_versions_branch_name_index end create_table(:branch_heads) do @@ -29,7 +31,9 @@ foreign_key :branch_version_id, :branch_versions, null: false, on_delete: :cascade Integer :version_id, null: false Integer :pacticipant_id, null: false - index([:branch_id], unique: true, name: :branch_heads_branch_id_index) + String :branch_name, null: false + index [:branch_id], unique: true, name: :branch_heads_branch_id_index + index [:branch_name], name: :branch_heads_branch_name_index end end end diff --git a/lib/pact_broker/domain/version.rb b/lib/pact_broker/domain/version.rb index c622d4a72..6f2a4bbc1 100644 --- a/lib/pact_broker/domain/version.rb +++ b/lib/pact_broker/domain/version.rb @@ -129,8 +129,16 @@ def where_tag(tag) end end - def where_branch(branch) - where(branch: branch) + def where_branch_name(branch_name) + matching_branch_ids = PactBroker::Versions::Branch.select(:id).where(name: branch_name) + matching_branch_version_ids = PactBroker::Versions::BranchVersion + .select(:version_id) + .where(branch_id: matching_branch_ids) + where(id: matching_branch_version_ids) + end + + def where_branch_head_name(branch_name) + where(id: PactBroker::Versions::BranchHead.select(:version_id).where(branch_name: branch_name)) end def where_number(number) @@ -163,11 +171,17 @@ def for_selector(selector) query = query.where_pacticipant_name(selector.pacticipant_name) if selector.pacticipant_name query = query.currently_in_environment(selector.environment_name, selector.pacticipant_name) if selector.environment_name query = query.where_tag(selector.tag) if selector.tag - query = query.where_branch(selector.branch) if selector.branch query = query.where_number(selector.pacticipant_version_number) if selector.pacticipant_version_number query = query.where_age_less_than(selector.max_age) if selector.max_age + if selector.branch + if selector.latest + query = query.where_branch_head_name(selector.branch) + else + query = query.where_branch_name(selector.branch) + end + end - if selector.latest + if selector.latest && !selector.branch calculate_max_version_order_and_join_back_to_versions(query, selector) else query diff --git a/lib/pact_broker/test/test_data_builder.rb b/lib/pact_broker/test/test_data_builder.rb index 1710ca50a..b92e636fd 100644 --- a/lib/pact_broker/test/test_data_builder.rb +++ b/lib/pact_broker/test/test_data_builder.rb @@ -190,7 +190,14 @@ def create_consumer_version version_number = "1.0.#{model_counter}", params = {} def create_provider_version version_number = "1.0.#{model_counter}", params = {} params.delete(:comment) tag_names = [params.delete(:tag_names), params.delete(:tag_name)].flatten.compact - @version = PactBroker::Domain::Version.create(:number => version_number, :pacticipant => @provider, branch: params[:branch]) + args = { + number: version_number, + pacticipant_id: @provider.id, + branch: params[:branch], + build_url: params[:build_url] + } + @version = PactBroker::Versions::Repository.new.create(args) + @provider_version = @version tag_names.each do | tag_name | tag = PactBroker::Domain::Tag.create(name: tag_name, version: provider_version) diff --git a/lib/pact_broker/versions/branch_head.rb b/lib/pact_broker/versions/branch_head.rb index 2faac8219..25729514a 100644 --- a/lib/pact_broker/versions/branch_head.rb +++ b/lib/pact_broker/versions/branch_head.rb @@ -14,6 +14,7 @@ def before_save super self.pacticipant_id = branch.pacticipant_id self.version_id = branch_version.version_id + self.branch_name = branch.name end def branch_name diff --git a/lib/pact_broker/versions/branch_version.rb b/lib/pact_broker/versions/branch_version.rb index 0d14c7104..3c33d47e6 100644 --- a/lib/pact_broker/versions/branch_version.rb +++ b/lib/pact_broker/versions/branch_version.rb @@ -32,13 +32,7 @@ def before_save end end - if version_order.nil? || pacticipant_id.nil? - raise PactBroker::Error.new("Need to set version_order and pacticipant_id for tags now") - end - end - - def branch_name - branch.name + self.branch_name = branch.name end end end From 575ceb25d27cbb319419d3bd0aa0a34de61cdeb0 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Mon, 16 Aug 2021 19:05:56 +1000 Subject: [PATCH 08/45] chore: update latest_for_branch? --- lib/pact_broker/domain/version.rb | 3 ++- spec/lib/pact_broker/domain/version_spec.rb | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/pact_broker/domain/version.rb b/lib/pact_broker/domain/version.rb index 6f2a4bbc1..81a08e66c 100644 --- a/lib/pact_broker/domain/version.rb +++ b/lib/pact_broker/domain/version.rb @@ -31,6 +31,7 @@ class Version < Sequel::Model associate(:many_to_one, :pacticipant, :class => "PactBroker::Domain::Pacticipant", :key => :pacticipant_id, :primary_key => :id) one_to_many :tags, :reciprocal => :version, order: :created_at one_to_many :branch_versions, :reciprocal => :branch_version, class: "PactBroker::Versions::BranchVersion", order: [:created_at, :id] + one_to_many :branch_heads, reciprocal: :branch_head, class: "PactBroker::Versions::BranchHead", order: :id one_to_many :current_deployed_versions, class: "PactBroker::Deployments::DeployedVersion", key: :version_id, primary_key: :id, order: [:created_at, :id] do | ds | ds.currently_deployed end @@ -241,7 +242,7 @@ def latest_pact_publication end def latest_for_branch? - branch ? latest_version_for_branch.order == order : nil + branch_heads.any? end def latest_for_pacticipant? diff --git a/spec/lib/pact_broker/domain/version_spec.rb b/spec/lib/pact_broker/domain/version_spec.rb index 1468df41c..9c85a24a3 100644 --- a/spec/lib/pact_broker/domain/version_spec.rb +++ b/spec/lib/pact_broker/domain/version_spec.rb @@ -367,7 +367,7 @@ def version_numbers context "when there is no branch" do let(:version) { Version.for("Foo", "3") } - it { is_expected.to be nil } + it { is_expected.to be false } end end From 59bdb331d8a00cc50ec8998c11a487d4a85132d8 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Mon, 16 Aug 2021 19:22:19 +1000 Subject: [PATCH 09/45] refactor: remove latest_version_for_branch code --- lib/pact_broker/domain/version.rb | 6 --- lib/pact_broker/index/service.rb | 8 ++-- lib/pact_broker/versions/eager_loaders.rb | 42 --------------------- lib/pact_broker/versions/lazy_loaders.rb | 13 ------- spec/lib/pact_broker/domain/version_spec.rb | 34 ----------------- 5 files changed, 4 insertions(+), 99 deletions(-) delete mode 100644 lib/pact_broker/versions/lazy_loaders.rb diff --git a/lib/pact_broker/domain/version.rb b/lib/pact_broker/domain/version.rb index 81a08e66c..94fefecbc 100644 --- a/lib/pact_broker/domain/version.rb +++ b/lib/pact_broker/domain/version.rb @@ -3,7 +3,6 @@ require "pact_broker/repositories/helpers" require "pact_broker/tags/tag_with_latest_flag" require "pact_broker/versions/eager_loaders" -require "pact_broker/versions/lazy_loaders" module PactBroker module Domain @@ -46,11 +45,6 @@ class Version < Sequel::Model dataset: lambda { Version.latest_version_for_pacticipant(pacticipant) }, eager_loader: PactBroker::Versions::EagerLoaders::LatestVersionForPacticipant - many_to_one :latest_version_for_branch, read_only: true, key: :id, - class: Version, - dataset: PactBroker::Versions::LazyLoaders::LATEST_VERSION_FOR_BRANCH, - eager_loader: PactBroker::Versions::EagerLoaders::LatestVersionForBranch - dataset_module do include PactBroker::Repositories::Helpers diff --git a/lib/pact_broker/index/service.rb b/lib/pact_broker/index/service.rb index b4d0d006f..9c1e4574b 100644 --- a/lib/pact_broker/index/service.rb +++ b/lib/pact_broker/index/service.rb @@ -44,9 +44,9 @@ def self.find_index_items options = {} pact_publications = pact_publication_query .eager(:consumer) .eager(:provider) - .eager(pact_version: { latest_verification: { provider_version: [{ current_deployed_versions: :environment }, { current_supported_released_versions: :environment }, :latest_version_for_branch, { tags: :head_tag } ] } }) + .eager(pact_version: { latest_verification: { provider_version: [{ current_deployed_versions: :environment }, { current_supported_released_versions: :environment }, :branch_heads, { tags: :head_tag } ] } }) .eager(integration: [{latest_verification: :provider_version}, :latest_triggered_webhooks]) - .eager(consumer_version: [{ current_deployed_versions: :environment }, { current_supported_released_versions: :environment }, :latest_version_for_branch, { tags: :head_tag }]) + .eager(consumer_version: [{ current_deployed_versions: :environment }, { current_supported_released_versions: :environment }, :branch_heads, { tags: :head_tag }]) .eager(:head_pact_publications_for_tags) index_items = pact_publications.all.collect do | pact_publication | @@ -106,8 +106,8 @@ def self.find_index_items_for_api(consumer_name: nil, provider_name: nil, page_n pact_publications = head_pact_publications(consumer_name: consumer_name, provider_name: provider_name, tags: true, page_number: page_number, page_size: page_size) .eager(:consumer) .eager(:provider) - .eager(pact_version: { latest_verification: { provider_version: [{ current_deployed_versions: :environment }, { current_supported_released_versions: :environment }, :latest_version_for_branch, { tags: :head_tag }]} }) - .eager(consumer_version: [{ current_deployed_versions: :environment }, { current_supported_released_versions: :environment }, :latest_version_for_branch, { tags: :head_tag }]) + .eager(pact_version: { latest_verification: { provider_version: [{ current_deployed_versions: :environment }, { current_supported_released_versions: :environment }, :branch_heads, { tags: :head_tag }]} }) + .eager(consumer_version: [{ current_deployed_versions: :environment }, { current_supported_released_versions: :environment }, :branch_heads, { tags: :head_tag }]) .eager(:head_pact_publications_for_tags) pact_publications.all.collect do | pact_publication | diff --git a/lib/pact_broker/versions/eager_loaders.rb b/lib/pact_broker/versions/eager_loaders.rb index 718f0a526..114e64362 100644 --- a/lib/pact_broker/versions/eager_loaders.rb +++ b/lib/pact_broker/versions/eager_loaders.rb @@ -1,48 +1,6 @@ module PactBroker module Versions module EagerLoaders - class LatestVersionForBranch - def self.call(eo, **_other) - initialize_association(eo[:rows]) - populate_associations(eo[:rows]) - end - - def self.initialize_association(versions) - versions.each{|version| version.associations[:latest_version_for_branch] = nil } - end - - def self.populate_associations(versions) - group_by_pacticipant_id(versions).each do | pacticipant, participant_versions | - populate_associations_by_pacticipant(pacticipant, participant_versions) - end - end - - def self.group_by_pacticipant_id(versions) - versions.to_a.group_by(&:pacticipant_id) - end - - def self.populate_associations_by_pacticipant(pacticipant_id, versions) - latest_versions_for_branches = latest_versions_for_pacticipant_branches( - pacticipant_id, - versions.collect(&:branch).uniq.compact, - versions.first.class - ) - self.populate_versions_with_branches(versions, latest_versions_for_branches) - end - - def self.populate_versions_with_branches(versions, latest_versions_for_branches) - versions.select(&:branch).each do | version | - version.associations[:latest_version_for_branch] = latest_versions_for_branches[[version.pacticipant_id, version.branch]] - end - end - - def self.latest_versions_for_pacticipant_branches(pacticipant_id, branches, version_class) - version_class.latest_versions_for_pacticipant_branches(pacticipant_id, branches).each_with_object({}) do | row, hash | - hash[[row.pacticipant_id, row.branch]] = row - end - end - end - class LatestVersionForPacticipant def self.call(eo, **_other) populate_associations(eo[:rows]) diff --git a/lib/pact_broker/versions/lazy_loaders.rb b/lib/pact_broker/versions/lazy_loaders.rb deleted file mode 100644 index 5b9e3def1..000000000 --- a/lib/pact_broker/versions/lazy_loaders.rb +++ /dev/null @@ -1,13 +0,0 @@ -module PactBroker - module Versions - module LazyLoaders - LATEST_VERSION_FOR_BRANCH = lambda { - self.class - .where(branch: branch, pacticipant_id: pacticipant_id) - .exclude(branch: nil) - .order(Sequel.desc(:order)) - .limit(1) - } - end - end -end diff --git a/spec/lib/pact_broker/domain/version_spec.rb b/spec/lib/pact_broker/domain/version_spec.rb index 9c85a24a3..59af5da3d 100644 --- a/spec/lib/pact_broker/domain/version_spec.rb +++ b/spec/lib/pact_broker/domain/version_spec.rb @@ -277,40 +277,6 @@ def version_numbers end end - describe "latest_version_for_branch" do - before do - td.create_consumer("Foo") - .create_consumer_version("1", branch: "main") - .create_consumer_version("2", branch: "main") - .create_consumer_version("3", branch: "feat/x") - .create_consumer_version("4", branch: "feat/x") - .create_consumer_version("5") - end - - subject { Version.order(:order) } - - it "lazy loads" do - expect(subject.all[0].latest_version_for_branch.number).to eq "2" - expect(subject.all[2].latest_version_for_branch.number).to eq "4" - expect(subject.all[4].latest_version_for_branch).to eq nil - end - - it "eager loads" do - all = subject.eager(:latest_version_for_branch).all - - expect(all[0].associations[:latest_version_for_branch]).to_not be nil - expect(all[1].associations[:latest_version_for_branch]).to_not be nil - expect(all[2].associations[:latest_version_for_branch]).to_not be nil - expect(all[3].associations[:latest_version_for_branch]).to_not be nil - expect(all[4].associations.fetch(:latest_version_for_branch)).to be nil - - expect(all[0].latest_version_for_branch.number).to eq "2" - expect(all[1].latest_version_for_branch.number).to eq "2" - expect(all[2].latest_version_for_branch.number).to eq "4" - expect(all[4].latest_version_for_branch).to eq nil - end - end - describe "#latest_pact_publication" do let!(:pact) do TestDataBuilder.new From 295eb06a827efefef2477bd7af10d8c3e3fcf331 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Mon, 16 Aug 2021 19:27:32 +1000 Subject: [PATCH 10/45] style: rubocop --- lib/pact_broker/versions/branch_version.rb | 15 ++------------- spec/lib/pact_broker/versions/repository_spec.rb | 4 ++-- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/lib/pact_broker/versions/branch_version.rb b/lib/pact_broker/versions/branch_version.rb index 3c33d47e6..10d922e15 100644 --- a/lib/pact_broker/versions/branch_version.rb +++ b/lib/pact_broker/versions/branch_version.rb @@ -19,19 +19,8 @@ def find_latest_for_branch(branch) def before_save super - - if version.order && self.version_order.nil? - self.version_order = version.order - end - - if self.pacticipant_id.nil? - if version.pacticipant_id - self.pacticipant_id = version.pacticipant_id - elsif version&.pacticipant&.id - self.pacticipant_id = version.pacticipant.id - end - end - + self.version_order = version.order + self.pacticipant_id = version.pacticipant_id self.branch_name = branch.name end end diff --git a/spec/lib/pact_broker/versions/repository_spec.rb b/spec/lib/pact_broker/versions/repository_spec.rb index 0632adc47..2705c0caf 100644 --- a/spec/lib/pact_broker/versions/repository_spec.rb +++ b/spec/lib/pact_broker/versions/repository_spec.rb @@ -188,7 +188,7 @@ module Versions end it "updates the branch head" do - branch_head = subject.pacticipant.branch_heads.find{ | branch_head | branch_head.branch_name == "new-branch" } + branch_head = subject.pacticipant.branch_heads.find{ | bh | bh.branch_name == "new-branch" } expect(branch_head.version).to eq subject end end @@ -210,7 +210,7 @@ module Versions end it "does not change the branch head" do - branch_head = subject.pacticipant.branch_heads.find{ | branch_head | branch_head.branch_name == "original-branch" } + branch_head = subject.pacticipant.branch_heads.find{ | bh | bh.branch_name == "original-branch" } expect(branch_head.version).to eq subject end end From 83902a916dcd44bfb2e6ac757964277042836814 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Mon, 16 Aug 2021 20:03:16 +1000 Subject: [PATCH 11/45] chore: update PactPublication.latest_for_consumer_branch --- .../pacts/pact_publication_dataset_module.rb | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/lib/pact_broker/pacts/pact_publication_dataset_module.rb b/lib/pact_broker/pacts/pact_publication_dataset_module.rb index 43421278d..81850a7e5 100644 --- a/lib/pact_broker/pacts/pact_publication_dataset_module.rb +++ b/lib/pact_broker/pacts/pact_publication_dataset_module.rb @@ -89,30 +89,32 @@ def overall_latest_for_consumer_id_and_provider_id(consumer_id, provider_id) .limit(1) end - def latest_for_consumer_branch(branch) - versions_join = { - Sequel[:pact_publications][:consumer_version_id] => Sequel[:cv][:id], - Sequel[:cv][:branch] => branch + def latest_for_consumer_branch(branch_name) + branch_versions_join = { + Sequel[:pact_publications][:consumer_version_id] => Sequel[:branch_versions][:version_id] } - base_query = join(:versions, versions_join, { table_alias: :cv }) do - Sequel.lit("cv.branch is not null") - end + # TODO use name like + branches_join = { + Sequel[:branch_versions][:branch_id] => Sequel[:branches][:id], + Sequel[:branches][:name] => branch_name + } - self_join = { - Sequel[:pact_publications][:consumer_id] => Sequel[:pp2][:consumer_id], - Sequel[:cv][:branch] => Sequel[:pp2][:branch] + max_orders = join(:branch_versions, branch_versions_join) + .join(:branches, branches_join) + .select_group(:consumer_id, Sequel[:branches][:name].as(:branch_name)) + .select_append{ max(consumer_version_order).as(latest_consumer_version_order) } + + max_join = { + Sequel[:max_orders][:consumer_id] => Sequel[:pact_publications][:consumer_id], + Sequel[:max_orders][:latest_consumer_version_order] => Sequel[:pact_publications][:consumer_version_order] } + base_query = self if no_columns_selected? - base_query = base_query.select_all_qualified.select_append(Sequel[:cv][:branch]) - end - - base_query.left_join(base_query.select(:consumer_id, :branch, :order), self_join, { table_alias: :pp2 } ) do - Sequel[:pp2][:order] > Sequel[:cv][:order] + base_query = base_query.select_all_qualified.select_append(Sequel[:max_orders][:branch_name].as(:branch)) end - .where(Sequel[:pp2][:order] => nil) - .remove_overridden_revisions_from_complete_query + base_query.join(max_orders, max_join, { table_alias: :max_orders }) end def latest_by_consumer_tag From 573373ca01b2094c7ef380c3b3cfeb50ee518155 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Tue, 17 Aug 2021 07:30:00 +1000 Subject: [PATCH 12/45] test: update expectations --- spec/lib/pact_broker/versions/repository_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/lib/pact_broker/versions/repository_spec.rb b/spec/lib/pact_broker/versions/repository_spec.rb index 2705c0caf..670c8d994 100644 --- a/spec/lib/pact_broker/versions/repository_spec.rb +++ b/spec/lib/pact_broker/versions/repository_spec.rb @@ -188,8 +188,8 @@ module Versions end it "updates the branch head" do - branch_head = subject.pacticipant.branch_heads.find{ | bh | bh.branch_name == "new-branch" } - expect(branch_head.version).to eq subject + branch_head = subject.pacticipant.branch_head_for("new-branch") + expect(branch_head.version.id).to eq subject.refresh.id end end @@ -210,7 +210,7 @@ module Versions end it "does not change the branch head" do - branch_head = subject.pacticipant.branch_heads.find{ | bh | bh.branch_name == "original-branch" } + branch_head = subject.pacticipant.branch_head_for("original-branch") expect(branch_head.version).to eq subject end end From 5b5321f35f8cbf63685d0968108c7042c6a48347 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Tue, 17 Aug 2021 09:24:22 +1000 Subject: [PATCH 13/45] chore: update PactPublication.latest_by_consumer_branch --- .../20210817_add_pact_publication_indexes.rb | 7 ++++ .../pacts/pact_publication_dataset_module.rb | 36 +++++++++++-------- .../pact_publication_dataset_module_spec.rb | 4 +++ 3 files changed, 32 insertions(+), 15 deletions(-) create mode 100644 db/migrations/20210817_add_pact_publication_indexes.rb diff --git a/db/migrations/20210817_add_pact_publication_indexes.rb b/db/migrations/20210817_add_pact_publication_indexes.rb new file mode 100644 index 000000000..2b59734c7 --- /dev/null +++ b/db/migrations/20210817_add_pact_publication_indexes.rb @@ -0,0 +1,7 @@ +Sequel.migration do + change do + alter_table(:pact_publications) do + add_index [:consumer_id, :consumer_version_order], name: :pact_publications_consumer_id_consumer_version_order + end + end +end diff --git a/lib/pact_broker/pacts/pact_publication_dataset_module.rb b/lib/pact_broker/pacts/pact_publication_dataset_module.rb index 81850a7e5..46fee8be3 100644 --- a/lib/pact_broker/pacts/pact_publication_dataset_module.rb +++ b/lib/pact_broker/pacts/pact_publication_dataset_module.rb @@ -43,28 +43,32 @@ def for_consumer_name_and_maybe_version_number(consumer_name, consumer_version_n end def latest_by_consumer_branch - versions_join = { - Sequel[:pact_publications][:consumer_version_id] => Sequel[:cv][:id] + branch_versions_join = { + Sequel[:pact_publications][:consumer_version_id] => Sequel[:branch_versions][:version_id] } - base_query = join(:versions, versions_join, { table_alias: :cv }) do - Sequel.lit("cv.branch is not null") - end + branches_join = { + Sequel[:branch_versions][:branch_id] => Sequel[:branches][:id] + } - self_join = { - Sequel[:pact_publications][:consumer_id] => Sequel[:pp2][:consumer_id], - Sequel[:cv][:branch] => Sequel[:pp2][:branch] + max_orders = join(:branch_versions, branch_versions_join) + .join(:branches, branches_join) + .select_group(Sequel[:pact_publications][:consumer_id], Sequel[:branches][:name].as(:branch_name)) + .select_append{ max(consumer_version_order).as(latest_consumer_version_order) } + + max_join = { + Sequel[:max_orders][:consumer_id] => Sequel[:pact_publications][:consumer_id], + Sequel[:max_orders][:latest_consumer_version_order] => Sequel[:pact_publications][:consumer_version_order] } + base_query = self if no_columns_selected? - base_query = base_query.select_all_qualified.select_append(Sequel[:cv][:branch]) + base_query = base_query.select_all_qualified.select_append(Sequel[:max_orders][:branch_name].as(:branch)) end - base_query.left_join(base_query.select(:consumer_id, :branch, :order), self_join, { table_alias: :pp2 } ) do - Sequel[:pp2][:order] > Sequel[:cv][:order] - end - .where(Sequel[:pp2][:order] => nil) - .remove_overridden_revisions_from_complete_query + base_query + .remove_overridden_revisions + .join(max_orders, max_join, { table_alias: :max_orders }) end def overall_latest @@ -114,7 +118,9 @@ def latest_for_consumer_branch(branch_name) if no_columns_selected? base_query = base_query.select_all_qualified.select_append(Sequel[:max_orders][:branch_name].as(:branch)) end - base_query.join(max_orders, max_join, { table_alias: :max_orders }) + base_query + .join(max_orders, max_join, { table_alias: :max_orders }) + .remove_overridden_revisions_from_complete_query end def latest_by_consumer_tag diff --git a/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb b/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb index 075ea27d3..3457d38a9 100644 --- a/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb +++ b/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb @@ -23,6 +23,7 @@ module Pacts .create_consumer("FooZ") .create_consumer_version("6", branch: "main", comment: "latest, different consumer") .create_pact + .revise_pact .set_now(Date.new(2020, 1, 6)) .create_consumer_version("7", comment: "No branch") .create_pact @@ -42,6 +43,7 @@ module Pacts expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:branch] == "main" }.consumer_version.number).to eq "3" expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:branch] == "feat/x" }.consumer_version.number).to eq "4" expect(subject.find { |pp| pp.consumer_id == foo_z.id && pp[:branch] == "main" }.consumer_version.number).to eq "6" + expect(subject.find { |pp| pp.consumer_id == foo_z.id && pp[:branch] == "main" }.revision_number).to eq 2 end it "does not return extra columns" do @@ -68,6 +70,7 @@ module Pacts .create_pact .create_consumer_version("2", branch: "main") .create_pact + .revise_pact .create_consumer_version("3", branch: "feat-x") .create_pact .create_consumer("Foo2") @@ -86,6 +89,7 @@ module Pacts expect(all.first.consumer.name).to eq "Foo" expect(all.first.provider.name).to eq "Bar" expect(all.first.consumer_version.number).to eq "2" + expect(all.first.revision_number).to eq 2 expect(all.last.consumer.name).to eq "Foo2" expect(all.last.provider.name).to eq "Bar2" From f7271563e709600bfa56e3af22201948612309a1 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Tue, 17 Aug 2021 09:24:55 +1000 Subject: [PATCH 14/45] docs: update sequel annotations --- lib/pact_broker/domain/pacticipant.rb | 1 + lib/pact_broker/domain/verification.rb | 1 + lib/pact_broker/domain/version.rb | 1 + lib/pact_broker/pacts/pact_publication.rb | 9 +++++---- lib/pact_broker/versions/branch.rb | 18 ++++++++++++++++++ lib/pact_broker/versions/branch_head.rb | 16 ++++++++++++++++ lib/pact_broker/versions/branch_version.rb | 21 +++++++++++++++++++++ 7 files changed, 63 insertions(+), 4 deletions(-) diff --git a/lib/pact_broker/domain/pacticipant.rb b/lib/pact_broker/domain/pacticipant.rb index 36a9029de..fda56915f 100644 --- a/lib/pact_broker/domain/pacticipant.rb +++ b/lib/pact_broker/domain/pacticipant.rb @@ -93,6 +93,7 @@ def branch_head_for(branch_name) # pacticipants_name_key | UNIQUE btree (name) # ndx_ppt_name | btree (name) # Referenced By: +# branches | branches_pacticipant_id_fkey | (pacticipant_id) REFERENCES pacticipants(id) ON DELETE CASCADE # currently_deployed_version_ids | currently_deployed_version_ids_pacticipant_id_fkey | (pacticipant_id) REFERENCES pacticipants(id) ON DELETE CASCADE # labels | labels_pacticipant_id_fkey | (pacticipant_id) REFERENCES pacticipants(id) # latest_pact_publication_ids_for_consumer_versions | latest_pact_publication_ids_for_consumer_versi_consumer_id_fkey | (consumer_id) REFERENCES pacticipants(id) ON DELETE CASCADE diff --git a/lib/pact_broker/domain/verification.rb b/lib/pact_broker/domain/verification.rb index c01e7aa5b..7d3e1ca98 100644 --- a/lib/pact_broker/domain/verification.rb +++ b/lib/pact_broker/domain/verification.rb @@ -236,6 +236,7 @@ def method_missing(m, *args, &block) # verifications_pkey | PRIMARY KEY btree (id) # verifications_pact_version_id_number_index | UNIQUE btree (pact_version_id, number) # verifications_consumer_id_index | btree (consumer_id) +# verifications_pact_version_id_id_index | btree (pact_version_id, id) # verifications_provider_id_consumer_id_index | btree (provider_id, consumer_id) # verifications_provider_id_index | btree (provider_id) # Foreign key constraints: diff --git a/lib/pact_broker/domain/version.rb b/lib/pact_broker/domain/version.rb index 94fefecbc..1333881ef 100644 --- a/lib/pact_broker/domain/version.rb +++ b/lib/pact_broker/domain/version.rb @@ -272,6 +272,7 @@ def branch_version_for_branch(branch) # Foreign key constraints: # versions_pacticipant_id_fkey | (pacticipant_id) REFERENCES pacticipants(id) # Referenced By: +# branch_versions | branch_versions_versions_fk | (version_id) REFERENCES versions(id) ON DELETE CASCADE # currently_deployed_version_ids | currently_deployed_version_ids_version_id_fkey | (version_id) REFERENCES versions(id) ON DELETE CASCADE # deployed_versions | deployed_versions_version_id_fkey | (version_id) REFERENCES versions(id) # latest_pact_publication_ids_for_consumer_versions | latest_pact_publication_ids_for_consum_consumer_version_id_fkey | (consumer_version_id) REFERENCES versions(id) ON DELETE CASCADE diff --git a/lib/pact_broker/pacts/pact_publication.rb b/lib/pact_broker/pacts/pact_publication.rb index 4c07dac1f..ab1aefbcf 100644 --- a/lib/pact_broker/pacts/pact_publication.rb +++ b/lib/pact_broker/pacts/pact_publication.rb @@ -178,10 +178,11 @@ def cached_domain_for_delegation # consumer_id | integer | # consumer_version_order | integer | # Indexes: -# pact_publications_pkey | PRIMARY KEY btree (id) -# cv_prov_revision_unq | UNIQUE btree (consumer_version_id, provider_id, revision_number) -# cv_prov_id_ndx | btree (consumer_version_id, provider_id, id) -# pact_publications_consumer_id_index | btree (consumer_id) +# pact_publications_pkey | PRIMARY KEY btree (id) +# cv_prov_revision_unq | UNIQUE btree (consumer_version_id, provider_id, revision_number) +# cv_prov_id_ndx | btree (consumer_version_id, provider_id, id) +# pact_publications_consumer_id_consumer_version_order | btree (consumer_id, consumer_version_order) +# pact_publications_consumer_id_index | btree (consumer_id) # Foreign key constraints: # pact_publications_consumer_id_fkey | (consumer_id) REFERENCES pacticipants(id) # pact_publications_consumer_version_id_fkey | (consumer_version_id) REFERENCES versions(id) diff --git a/lib/pact_broker/versions/branch.rb b/lib/pact_broker/versions/branch.rb index 80de7e0ee..379340d88 100644 --- a/lib/pact_broker/versions/branch.rb +++ b/lib/pact_broker/versions/branch.rb @@ -11,3 +11,21 @@ class Branch < Sequel::Model(:branches) end end end + +# Table: branches +# Columns: +# id | integer | PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY +# name | text | +# pacticipant_id | integer | NOT NULL +# latest_branch_version_id | integer | +# latest_version_id | integer | +# created_at | timestamp without time zone | NOT NULL +# updated_at | timestamp without time zone | NOT NULL +# Indexes: +# branches_pkey | PRIMARY KEY btree (id) +# branches_name_pacticipant_id_index | UNIQUE btree (name, pacticipant_id) +# Foreign key constraints: +# branches_pacticipant_id_fkey | (pacticipant_id) REFERENCES pacticipants(id) ON DELETE CASCADE +# Referenced By: +# branch_heads | branch_heads_branch_id_fkey | (branch_id) REFERENCES branches(id) ON DELETE CASCADE +# branch_versions | branch_versions_branches_fk | (branch_id) REFERENCES branches(id) ON DELETE CASCADE diff --git a/lib/pact_broker/versions/branch_head.rb b/lib/pact_broker/versions/branch_head.rb index 25729514a..d10f0b2d7 100644 --- a/lib/pact_broker/versions/branch_head.rb +++ b/lib/pact_broker/versions/branch_head.rb @@ -23,3 +23,19 @@ def branch_name end end end + +# Table: branch_heads +# Columns: +# id | integer | PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY +# branch_id | integer | NOT NULL +# branch_version_id | integer | NOT NULL +# version_id | integer | NOT NULL +# pacticipant_id | integer | NOT NULL +# branch_name | text | NOT NULL +# Indexes: +# branch_heads_pkey | PRIMARY KEY btree (id) +# branch_heads_branch_id_index | UNIQUE btree (branch_id) +# branch_heads_branch_name_index | btree (branch_name) +# Foreign key constraints: +# branch_heads_branch_id_fkey | (branch_id) REFERENCES branches(id) ON DELETE CASCADE +# branch_heads_branch_version_id_fkey | (branch_version_id) REFERENCES branch_versions(id) ON DELETE CASCADE diff --git a/lib/pact_broker/versions/branch_version.rb b/lib/pact_broker/versions/branch_version.rb index 10d922e15..ec7493d4f 100644 --- a/lib/pact_broker/versions/branch_version.rb +++ b/lib/pact_broker/versions/branch_version.rb @@ -26,3 +26,24 @@ def before_save end end end + +# Table: branch_versions +# Columns: +# id | integer | PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY +# branch_id | integer | NOT NULL +# version_id | integer | NOT NULL +# version_order | integer | NOT NULL +# pacticipant_id | integer | NOT NULL +# branch_name | text | NOT NULL +# created_at | timestamp without time zone | NOT NULL +# updated_at | timestamp without time zone | NOT NULL +# Indexes: +# branch_versions_pkey | PRIMARY KEY btree (id) +# branch_versions_branch_id_version_id_index | UNIQUE btree (branch_id, version_id) +# branch_versions_branch_name_index | btree (branch_name) +# branch_versions_pacticipant_id_branch_id_version_order_index | btree (pacticipant_id, branch_id, version_order) +# Foreign key constraints: +# branch_versions_branches_fk | (branch_id) REFERENCES branches(id) ON DELETE CASCADE +# branch_versions_versions_fk | (version_id) REFERENCES versions(id) ON DELETE CASCADE +# Referenced By: +# branch_heads | branch_heads_branch_version_id_fkey | (branch_version_id) REFERENCES branch_versions(id) ON DELETE CASCADE From 8b7d90526fa1122afdfd7435490d3008d194a36e Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Tue, 17 Aug 2021 09:27:25 +1000 Subject: [PATCH 15/45] chore: update overall_latest test --- lib/pact_broker/pacts/pact_publication_dataset_module.rb | 2 +- .../pact_broker/pacts/pact_publication_dataset_module_spec.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/pact_broker/pacts/pact_publication_dataset_module.rb b/lib/pact_broker/pacts/pact_publication_dataset_module.rb index 46fee8be3..5471f9ed4 100644 --- a/lib/pact_broker/pacts/pact_publication_dataset_module.rb +++ b/lib/pact_broker/pacts/pact_publication_dataset_module.rb @@ -84,7 +84,7 @@ def overall_latest Sequel[:pp2][:consumer_version_order] > Sequel[:pact_publications][:consumer_version_order] end .where(Sequel[:pp2][:consumer_version_order] => nil) - .remove_overridden_revisions_from_complete_query # do we need this? + .remove_overridden_revisions_from_complete_query end def overall_latest_for_consumer_id_and_provider_id(consumer_id, provider_id) diff --git a/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb b/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb index 3457d38a9..7a406e4cd 100644 --- a/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb +++ b/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb @@ -174,6 +174,7 @@ module Pacts .create_pact .create_consumer_version("3", tag_names: ["feat/x"]) .create_pact + .revise_pact .create_consumer("Foo2") .create_provider("Bar2") .create_consumer_version("10", tag_names: ["main"]) @@ -187,6 +188,7 @@ module Pacts it "returns the latest by consumer/provider" do all = subject.all.sort_by{ | pact_publication | pact_publication.consumer_version.order } expect(all.size).to eq 2 + expect(all.first.revision_number).to eq 2 end it "does not return extra columns" do From 233afc837f7561629f4ef088a4954662b3cdd38e Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Wed, 1 Sep 2021 11:07:54 +1000 Subject: [PATCH 16/45] chore: use more efficient query for selecting latest for branches --- lib/pact_broker/domain/version.rb | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/lib/pact_broker/domain/version.rb b/lib/pact_broker/domain/version.rb index 44ee4157d..70312f751 100644 --- a/lib/pact_broker/domain/version.rb +++ b/lib/pact_broker/domain/version.rb @@ -164,6 +164,18 @@ def for_main_branches where(id: matching_branch_version_ids) end + def latest_for_main_branches + pacticipants_join = { + Sequel[:branch_heads][:pacticipant_id] => Sequel[:pacticipants][:id], + Sequel[:branch_heads][:branch_name] => Sequel[:pacticipants][:main_branch] + } + matching_branch_version_ids = PactBroker::Versions::BranchHead + .select(:version_id) + .join(:pacticipants, pacticipants_join) + + where(id: matching_branch_version_ids) + end + def where_number(number) where(name_like(:number, number)) end @@ -198,16 +210,28 @@ def for_selector(selector) query = query.where_tag(selector.tag) if selector.tag query = query.where_number(selector.pacticipant_version_number) if selector.respond_to?(:pacticipant_version_number) && selector.pacticipant_version_number query = query.where_age_less_than(selector.max_age) if selector.respond_to?(:max_age) && selector.max_age - query = query.for_main_branches if selector.respond_to?(:main_branch) && selector.main_branch + + latest_applied = false + + if selector.respond_to?(:main_branch) && selector.main_branch + if selector.latest + latest_applied = true + query = query.latest_for_main_branches + else + query = query.for_main_branches + end + end + if selector.branch if selector.latest + latest_applied = true query = query.where_branch_head_name(selector.branch) else query = query.where_branch_name(selector.branch) end end - if selector.latest && !selector.branch # latest branch logic already done + if selector.latest && !latest_applied calculate_max_version_order_and_join_back_to_versions(query, selector) else query From 9dfac4034f753da582d72e9d57bc83abe7c8c7a4 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Wed, 1 Sep 2021 13:47:12 +1000 Subject: [PATCH 17/45] chore: display multiple branches on matrix and index pages --- lib/pact_broker/domain/index_item.rb | 20 ++++++---- lib/pact_broker/index/service.rb | 2 + lib/pact_broker/matrix/quick_row.rb | 12 +++++- ...act_publication_selector_dataset_module.rb | 18 ++------- .../test/http_test_data_builder.rb | 26 ++++++++++++- lib/pact_broker/ui/view_models/index_item.rb | 2 + .../ui/view_models/matrix_branch.rb | 39 +++++++++++++++++++ lib/pact_broker/ui/view_models/matrix_line.rb | 27 ++++++------- .../ui/views/index/show-with-tags.haml | 7 ++-- lib/pact_broker/ui/views/matrix/show.haml | 16 ++++---- lib/pact_broker/versions/branch_version.rb | 5 +++ spec/features/publish_pact_all_in_one_spec.rb | 1 - spec/integration/ui/index_spec.rb | 2 - spec/integration/ui/matrix_spec.rb | 1 - .../lib/pact_broker/contracts/service_spec.rb | 4 +- .../lib/pact_broker/domain/index_item_spec.rb | 2 +- .../relationships/groupify_spec.rb | 5 --- .../ui/view_models/index_item_spec.rb | 8 ++-- .../versions/branch_version_spec.rb | 27 +++++++++++++ 19 files changed, 156 insertions(+), 68 deletions(-) create mode 100644 lib/pact_broker/ui/view_models/matrix_branch.rb create mode 100644 spec/lib/pact_broker/versions/branch_version_spec.rb diff --git a/lib/pact_broker/domain/index_item.rb b/lib/pact_broker/domain/index_item.rb index 32948a6a7..6815ab783 100644 --- a/lib/pact_broker/domain/index_item.rb +++ b/lib/pact_broker/domain/index_item.rb @@ -6,6 +6,7 @@ module Domain class IndexItem attr_reader :consumer, :provider, + :consumer_version, :latest_pact, :latest_verification, :webhooks, @@ -13,15 +14,16 @@ class IndexItem :latest_verification_latest_tags # rubocop:disable Metrics/ParameterLists - def self.create(consumer, provider, latest_pact, latest, latest_verification, webhooks = [], triggered_webhooks = [], tags = [], latest_verification_latest_tags = [], latest_for_branch = nil) - new(consumer, provider, latest_pact, latest, latest_verification, webhooks, triggered_webhooks, tags, latest_verification_latest_tags, latest_for_branch) + def self.create(consumer, provider, consumer_version, latest_pact, latest, latest_verification, webhooks = [], triggered_webhooks = [], tags = [], latest_verification_latest_tags = [], latest_for_branch = nil) + new(consumer, provider, consumer_version, latest_pact, latest, latest_verification, webhooks, triggered_webhooks, tags, latest_verification_latest_tags, latest_for_branch) end # rubocop:enable Metrics/ParameterLists # rubocop:disable Metrics/ParameterLists - def initialize(consumer, provider, latest_pact = nil, latest = true, latest_verification = nil, webhooks = [], triggered_webhooks = [], tags = [], latest_verification_latest_tags = [], latest_for_branch = nil) + def initialize(consumer, provider, consumer_version = nil, latest_pact = nil, latest = true, latest_verification = nil, webhooks = [], triggered_webhooks = [], tags = [], latest_verification_latest_tags = [], latest_for_branch = nil) @consumer = consumer @provider = provider + @consumer_version = consumer_version @latest_pact = latest_pact @latest = latest @latest_verification = latest_verification @@ -68,14 +70,14 @@ def consumer_version_order consumer_version.order end - def consumer_version - @latest_pact.consumer_version - end - def consumer_version_branch consumer_version.branch end + def consumer_version_branches + consumer_version.branch_heads.collect(&:branch_name) + end + def consumer_version_environment_names (consumer_version.current_deployed_versions.collect(&:environment).collect(&:name) + consumer_version.current_supported_released_versions.collect(&:environment).collect(&:name)).uniq end @@ -96,6 +98,10 @@ def provider_version_branch provider_version&.branch end + def provider_version_branches + provider_version&.branch_heads&.collect(&:branch_name) || [] + end + def provider_version_environment_names if provider_version (provider_deployed_environment_names + provider_released_environment_names).uniq diff --git a/lib/pact_broker/index/service.rb b/lib/pact_broker/index/service.rb index 9c1e4574b..b4455b62c 100644 --- a/lib/pact_broker/index/service.rb +++ b/lib/pact_broker/index/service.rb @@ -58,6 +58,7 @@ def self.find_index_items options = {} PactBroker::Domain::IndexItem.create( pact_publication.consumer, pact_publication.provider, + pact_publication.consumer_version, pact_publication.to_domain_lightweight, is_overall_latest_for_integration, latest_verification, @@ -116,6 +117,7 @@ def self.find_index_items_for_api(consumer_name: nil, provider_name: nil, page_n PactBroker::Domain::IndexItem.create( pact_publication.consumer, pact_publication.provider, + pact_publication.consumer_version, pact_publication.to_domain_lightweight, is_overall_latest_for_integration, pact_publication.latest_verification, diff --git a/lib/pact_broker/matrix/quick_row.rb b/lib/pact_broker/matrix/quick_row.rb index 2c69b31fc..fe4c89a67 100644 --- a/lib/pact_broker/matrix/quick_row.rb +++ b/lib/pact_broker/matrix/quick_row.rb @@ -127,8 +127,8 @@ def order_by_pact_publication_created_at def eager_all_the_things eager(:consumer) .eager(:provider) - .eager(consumer_version: { current_deployed_versions: :environment }) - .eager(provider_version: { current_deployed_versions: :environment }) + .eager(consumer_version: [{ current_deployed_versions: :environment }, { current_supported_released_versions: :environment }, { branch_versions: :branch_head }]) + .eager(provider_version: [{ current_deployed_versions: :environment }, { current_supported_released_versions: :environment }, { branch_versions: :branch_head }]) .eager(:verification) .eager(:pact_publication) .eager(:pact_version) @@ -362,6 +362,10 @@ def consumer_version_branch consumer_version.branch end + def consumer_version_branch_versions + consumer_version.branch_versions + end + def consumer_version_order consumer_version.order end @@ -378,6 +382,10 @@ def provider_version_branch provider_version&.branch end + def provider_version_branch_versions + provider_version&.branch_versions || [] + end + def provider_version_order provider_version&.order end diff --git a/lib/pact_broker/pacts/pact_publication_selector_dataset_module.rb b/lib/pact_broker/pacts/pact_publication_selector_dataset_module.rb index 701fcb19f..30a288c6e 100644 --- a/lib/pact_broker/pacts/pact_publication_selector_dataset_module.rb +++ b/lib/pact_broker/pacts/pact_publication_selector_dataset_module.rb @@ -22,22 +22,10 @@ def for_provider_and_consumer_version_selector provider, selector end # rubocop: enable Metrics/CyclomaticComplexity + # Updated logic - the pacts for the latest version of each main branch, + # not the latest pact that belongs to the main branch. def latest_for_main_branches - self_join = { - Sequel[:pact_publications][:provider_id] => Sequel[:pp2][:provider_id], - Sequel[:pact_publications][:consumer_id] => Sequel[:pp2][:consumer_id], - Sequel[:cv][:branch] => Sequel[:pp2][:branch] - } - - base_query = join_consumers(:consumers) - .join_consumer_versions(:cv, { Sequel[:cv][:branch] => Sequel[:consumers][:main_branch] }) - - base_query = base_query.select_all_qualified if no_columns_selected? - - base_query.left_join(base_query.select(:provider_id, :consumer_id, Sequel[:cv][:branch], :consumer_version_order), self_join, { table_alias: :pp2 }) do - Sequel[:pp2][:consumer_version_order] > Sequel[:pact_publications][:consumer_version_order] - end - .where(Sequel[:pp2][:consumer_version_order] => nil) + where(consumer_version: PactBroker::Domain::Version.latest_for_main_branches) end def for_currently_deployed_versions(environment_name) diff --git a/lib/pact_broker/test/http_test_data_builder.rb b/lib/pact_broker/test/http_test_data_builder.rb index e0bbcea10..98ddad7dd 100644 --- a/lib/pact_broker/test/http_test_data_builder.rb +++ b/lib/pact_broker/test/http_test_data_builder.rb @@ -3,6 +3,7 @@ require "logger" require "erb" require "yaml" +require "base64" module PactBroker module Test @@ -100,6 +101,29 @@ def create_pacticipant(name, main_branch: nil) self end + def publish_contract(consumer: last_consumer_name, consumer_version:, provider: last_provider_name, content_id:, tag: nil, branch: nil) + content = generate_content(consumer, provider, content_id) + request_body_hash = { + :pacticipantName => consumer, + :pacticipantVersionNumber => consumer_version, + :branch => branch, + :tags => tag ? [tag] : nil, + :contracts => [ + { + :consumerName => consumer, + :providerName => provider, + :specification => "pact", + :contentType => "application/json", + :content => Base64.strict_encode64(content.to_json) + } + ] + }.compact + response = client.post("contracts/publish", request_body_hash).tap { |response| check_for_error(response) } + puts response.body["logs"].collect{ |log| log["message"]} + separate + self + end + def publish_pact(consumer: last_consumer_name, consumer_version:, provider: last_provider_name, content_id:, tag: nil, branch: nil) @last_consumer_name = consumer @last_provider_name = provider @@ -120,7 +144,7 @@ def publish_pact(consumer: last_consumer_name, consumer_version:, provider: last self end - def get_pacts_for_verification(provider: last_provider_name, provider_version_tag: nil, provider_version_branch: nil, consumer_version_selectors: [], enable_pending: false, include_wip_pacts_since: nil) + def get_pacts_for_verification(provider: last_provider_name, provider_version_tag: nil, provider_version_branch: nil, consumer_version_selectors: nil, enable_pending: false, include_wip_pacts_since: nil) @last_provider_name = provider @last_provider_version_tag = provider_version_tag @last_provder_version_branch = provider_version_branch diff --git a/lib/pact_broker/ui/view_models/index_item.rb b/lib/pact_broker/ui/view_models/index_item.rb index 98aea5ed2..eb63f6490 100644 --- a/lib/pact_broker/ui/view_models/index_item.rb +++ b/lib/pact_broker/ui/view_models/index_item.rb @@ -13,7 +13,9 @@ class IndexItem delegate [ :consumer_version_branch, + :consumer_version_branches, :provider_version_branch, + :provider_version_branches, :latest_for_branch?, :consumer_version_environment_names, :provider_version_environment_names diff --git a/lib/pact_broker/ui/view_models/matrix_branch.rb b/lib/pact_broker/ui/view_models/matrix_branch.rb new file mode 100644 index 000000000..581622551 --- /dev/null +++ b/lib/pact_broker/ui/view_models/matrix_branch.rb @@ -0,0 +1,39 @@ +require "pact_broker/api/pact_broker_urls" +require "pact_broker/ui/helpers/url_helper" +require "pact_broker/date_helper" + +module PactBroker + module UI + module ViewDomain + class MatrixBranch + + include PactBroker::Api::PactBrokerUrls + + def initialize branch_version, pacticipant_name + @branch_version = branch_version + @pacticipant_name = pacticipant_name + end + + def name + branch_version.branch_name + end + + def tooltip + if branch_version.latest? + "This is the latest version of #{pacticipant_name} from branch \"#{branch_version.branch_name}\"." + else + "A more recent version of #{pacticipant_name} from branch \"#{branch_version.branch_name}\" exists." + end + end + + def latest? + branch_version.latest? + end + + private + + attr_reader :branch_version, :pacticipant_name + end + end + end +end diff --git a/lib/pact_broker/ui/view_models/matrix_line.rb b/lib/pact_broker/ui/view_models/matrix_line.rb index dd5ddabab..c4e10d2f8 100644 --- a/lib/pact_broker/ui/view_models/matrix_line.rb +++ b/lib/pact_broker/ui/view_models/matrix_line.rb @@ -2,6 +2,7 @@ require "pact_broker/ui/helpers/url_helper" require "pact_broker/date_helper" require "pact_broker/ui/view_models/matrix_tag" +require "pact_broker/ui/view_models/matrix_branch" require "pact_broker/ui/view_models/matrix_deployed_version" require "pact_broker/ui/view_models/matrix_released_version" require "pact_broker/versions/abbreviate_number" @@ -16,8 +17,6 @@ class MatrixLine include PactBroker::Messages extend Forwardable - delegate [:consumer_version_branch, :provider_version_branch] => :line - def initialize line, options = {} @line = line @options = options @@ -103,20 +102,10 @@ def provider_version_order end end - def consumer_version_branch_tooltip - branch_tooltip(consumer_name, consumer_version_branch, consumer_version_latest_for_branch?) - end - - def consumer_version_latest_for_branch? - @line.consumer_version.latest_for_branch? - end - - def provider_version_branch_tooltip - branch_tooltip(provider_name, provider_version_branch, provider_version_latest_for_branch?) - end - - def provider_version_latest_for_branch? - @line.provider_version.latest_for_branch? + def consumer_version_branches + @line.consumer_version_branch_versions.collect do | branch_version | + MatrixBranch.new(branch_version, consumer_name) + end end def latest_consumer_version_tags @@ -141,6 +130,12 @@ def provider_versions_in_environments provider_deployed_versions + provider_released_versions end + def provider_version_branches + @line.provider_version_branch_versions.collect do | branch_version | + MatrixBranch.new(branch_version, provider_name) + end + end + def latest_provider_version_tags @line.provider_version_tags .select(&:latest) diff --git a/lib/pact_broker/ui/views/index/show-with-tags.haml b/lib/pact_broker/ui/views/index/show-with-tags.haml index bbcb58d72..73331fe29 100644 --- a/lib/pact_broker/ui/views/index/show-with-tags.haml +++ b/lib/pact_broker/ui/views/index/show-with-tags.haml @@ -58,10 +58,9 @@ - if index_item.display_consumer_version_number %button.clippy.invisible{ title: "Copy to clipboard" } %span.copy-icon - - if index_item.consumer_version_branch - - branch_class = index_item.latest_for_branch? ? "tag badge badge-dark" : "tag badge badge-secondary" - %div{"class": branch_class} - = "branch: " + index_item.consumer_version_branch + - index_item.consumer_version_branches.each do | branch_name | + %div{"class": "tag badge badge-dark"} + = "branch: " + branch_name - index_item.consumer_version_latest_tag_names.each do | tag_name | .tag.badge.badge-primary = "tag: " + tag_name diff --git a/lib/pact_broker/ui/views/matrix/show.haml b/lib/pact_broker/ui/views/matrix/show.haml index 222c0793f..859eec6c0 100644 --- a/lib/pact_broker/ui/views/matrix/show.haml +++ b/lib/pact_broker/ui/views/matrix/show.haml @@ -136,11 +136,11 @@ - if line.display_consumer_version_number %button.clippy.invisible{ title: "Copy to clipboard" } %span.copy-icon - - if line.consumer_version_branch - .tag-parent{"title": line.consumer_version_branch_tooltip, "data-toggle": "tooltip", "data-placement": "right"} - - branch_class = line.consumer_version_latest_for_branch? ? "tag badge badge-dark" : "tag badge badge-secondary" + - line.consumer_version_branches.each do | branch | + .tag-parent{"title": branch.tooltip, "data-toggle": "tooltip", "data-placement": "right"} + - branch_class = branch.latest? ? "tag badge badge-dark" : "tag badge badge-secondary" %div{"class": branch_class} - = "branch: " + line.consumer_version_branch + = "branch: " + branch.name - line.consumer_versions_in_environments.each do | version_in_environment | .tag-parent{"title": version_in_environment.tooltip, "data-toggle": "tooltip", "data-placement": "right"} %a{href: version_in_environment.url} @@ -173,11 +173,11 @@ - if line.display_provider_version_number %button.clippy.invisible{ title: "Copy to clipboard" } %span.copy-icon - - if line.provider_version_branch - .tag-parent{"title": line.provider_version_branch_tooltip, "data-toggle": "tooltip", "data-placement": "right"} - - branch_class = line.provider_version_latest_for_branch? ? "tag badge badge-dark" : "tag badge badge-secondary" + - line.provider_version_branches.each do | branch | + .tag-parent{"title": branch.tooltip, "data-toggle": "tooltip", "data-placement": "right"} + - branch_class = branch.latest? ? "tag badge badge-dark" : "tag badge badge-secondary" %div{"class": branch_class} - = "branch: " + line.provider_version_branch + = "branch: " + branch.name - line.provider_versions_in_environments.each do | version_in_environment | .tag-parent{"title": version_in_environment.tooltip, "data-toggle": "tooltip", "data-placement": "right"} %a{href: version_in_environment.url} diff --git a/lib/pact_broker/versions/branch_version.rb b/lib/pact_broker/versions/branch_version.rb index ec7493d4f..dbe29ecef 100644 --- a/lib/pact_broker/versions/branch_version.rb +++ b/lib/pact_broker/versions/branch_version.rb @@ -9,6 +9,7 @@ class BranchVersion < Sequel::Model(:branch_versions) associate(:many_to_one, :branch, :class => "PactBroker::Versions::Branch", :key => :branch_id, :primary_key => :id) associate(:many_to_one, :version, :class => "PactBroker::Domain::Version", :key => :version_id, :primary_key => :id) + associate(:many_to_one, :branch_head, :class => "PactBroker::Versions::BranchHead", :key => :branch_id, :primary_key => :branch_id) dataset_module do def find_latest_for_branch(branch) @@ -23,6 +24,10 @@ def before_save self.pacticipant_id = version.pacticipant_id self.branch_name = branch.name end + + def latest? + branch_head.branch_version_id == id + end end end end diff --git a/spec/features/publish_pact_all_in_one_spec.rb b/spec/features/publish_pact_all_in_one_spec.rb index a91a03ec5..b11a9f204 100644 --- a/spec/features/publish_pact_all_in_one_spec.rb +++ b/spec/features/publish_pact_all_in_one_spec.rb @@ -1,5 +1,4 @@ RSpec.describe "publishing a pact using the all in one endpoint" do - # TODO merge branches let(:request_body_hash) do { :pacticipantName => "Foo", diff --git a/spec/integration/ui/index_spec.rb b/spec/integration/ui/index_spec.rb index a4f75d49b..9ee69396c 100644 --- a/spec/integration/ui/index_spec.rb +++ b/spec/integration/ui/index_spec.rb @@ -1,9 +1,7 @@ require "pact_broker/ui/app" describe "UI index" do - let(:app) { PactBroker::UI::App.new } - let(:td) { TestDataBuilder.new } let(:params) { {} } before do diff --git a/spec/integration/ui/matrix_spec.rb b/spec/integration/ui/matrix_spec.rb index b62b3a4f2..3a9463abb 100644 --- a/spec/integration/ui/matrix_spec.rb +++ b/spec/integration/ui/matrix_spec.rb @@ -2,7 +2,6 @@ describe "UI matrix" do let(:app) { PactBroker::UI::App.new } - let(:td) { TestDataBuilder.new } let(:params) { {} } before do diff --git a/spec/lib/pact_broker/contracts/service_spec.rb b/spec/lib/pact_broker/contracts/service_spec.rb index 5933fd87b..25f8b9079 100644 --- a/spec/lib/pact_broker/contracts/service_spec.rb +++ b/spec/lib/pact_broker/contracts/service_spec.rb @@ -74,8 +74,8 @@ module Contracts expect { subject }.to change { PactBroker::Domain::Version.order(:id).last.tags.count}.from(1).to(3) end - it "updates the branch (TODO this should add to the branches when branches is updated to be a collection)" do - expect { subject }.to change { PactBroker::Domain::Version.order(:id).last.branch}.from("feat/x").to("main") + it "adds the branch to the existing version" do + expect { subject }.to change { PactBroker::Domain::Version.order(:id).last.branch_versions.collect(&:branch_name)}.from(["feat/x"]).to(["feat/x", "main"]) end context "when the write mode is overwrite" do diff --git a/spec/lib/pact_broker/domain/index_item_spec.rb b/spec/lib/pact_broker/domain/index_item_spec.rb index 7f806d97f..fcf3e16a9 100644 --- a/spec/lib/pact_broker/domain/index_item_spec.rb +++ b/spec/lib/pact_broker/domain/index_item_spec.rb @@ -13,7 +13,7 @@ module Domain allow(webhook_executions).to receive(:sort).and_return(webhook_executions) end - subject { IndexItem.create(nil, nil, nil, true, nil, [], webhook_executions) } + subject { IndexItem.create(nil, nil, nil, nil, true, nil, [], webhook_executions) } it "returns the created_at date of the last execution" do expect(subject.last_webhook_execution_date).to eq DateTime.new(2015) diff --git a/spec/lib/pact_broker/relationships/groupify_spec.rb b/spec/lib/pact_broker/relationships/groupify_spec.rb index 4c01c5dc5..83450e87c 100644 --- a/spec/lib/pact_broker/relationships/groupify_spec.rb +++ b/spec/lib/pact_broker/relationships/groupify_spec.rb @@ -3,11 +3,8 @@ require "pact_broker/domain/index_item" module PactBroker - module Relationships - describe Groupify do - describe ".call" do let(:consumer_a) { double("consumer a", id: 1, name: "consumer a") } @@ -40,9 +37,7 @@ module Relationships expect(groups[1]).to eq(Domain::Group.new(relationship_3)) expect(groups[2]).to eq(Domain::Group.new(relationship_5, relationship_6)) end - end end - end end diff --git a/spec/lib/pact_broker/ui/view_models/index_item_spec.rb b/spec/lib/pact_broker/ui/view_models/index_item_spec.rb index aeed8f097..27f5b9164 100644 --- a/spec/lib/pact_broker/ui/view_models/index_item_spec.rb +++ b/spec/lib/pact_broker/ui/view_models/index_item_spec.rb @@ -7,11 +7,13 @@ module UI module ViewDomain describe IndexItem do - let(:consumer) { instance_double("PactBroker::Domain::Pacticipant", name: "Consumer Name")} - let(:provider) { instance_double("PactBroker::Domain::Pacticipant", name: "Provider Name")} + let(:consumer) { instance_double("PactBroker::Domain::Pacticipant", name: "Consumer Name") } + let(:provider) { instance_double("PactBroker::Domain::Pacticipant", name: "Provider Name") } + let(:consumer_version) { instance_double("PactBroker::Domain::Version") } + let(:latest_pact) { instance_double("PactBroker::Domain::Pact", consumer_version_number: "1.2.3") } let(:latest_verification) { instance_double("PactBroker::Domain::Verification") } - let(:domain_relationship) { PactBroker::Domain::IndexItem.new(consumer, provider, latest_pact, latest, latest_verification, [], [], tags, latest_verification_latest_tags)} + let(:domain_relationship) { PactBroker::Domain::IndexItem.new(consumer, provider, consumer_version, latest_pact, latest, latest_verification, [], [], tags, latest_verification_latest_tags)} let(:tags) { [] } let(:verification_tag_1) { instance_double("PactBroker::Tags::TagWithLatestFlag", name: "dev") } let(:verification_tag_2) { instance_double("PactBroker::Tags::TagWithLatestFlag", name: "prod") } diff --git a/spec/lib/pact_broker/versions/branch_version_spec.rb b/spec/lib/pact_broker/versions/branch_version_spec.rb new file mode 100644 index 000000000..a005f5d1f --- /dev/null +++ b/spec/lib/pact_broker/versions/branch_version_spec.rb @@ -0,0 +1,27 @@ +require 'pact_broker/versions/branch_version' + +module PactBroker + module Versions + describe BranchVersion do + describe "#latest?" do + before do + td.create_consumer("Foo") + .create_consumer_version("2", branch: "foo") + .create_consumer_version("3", branch: "foo") + end + + context "when it is the latest version for the branch" do + subject { td.find_version("Foo", "3").branch_versions.first } + + its(:latest?) { is_expected.to be true } + end + + context "when it is not the latest version for the branch" do + subject { td.find_version("Foo", "2").branch_versions.first } + + its(:latest?) { is_expected.to be false } + end + end + end + end +end From 984cefe0538f92ed12afb4a5dce5c696980c5b78 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Wed, 1 Sep 2021 15:35:53 +1000 Subject: [PATCH 18/45] feat: support adding a version to a branch --- lib/pact_broker/api.rb | 1 + .../decorators/branch_version_decorator.rb | 20 ++++++ lib/pact_broker/api/pact_broker_urls.rb | 3 +- .../api/resources/branch_version.rb | 50 +++++++++++++ lib/pact_broker/api/resources/index.rb | 6 ++ lib/pact_broker/domain/pacticipant.rb | 4 ++ lib/pact_broker/services.rb | 9 +++ .../test/http_test_data_builder.rb | 9 +-- lib/pact_broker/versions/branch_service.rb | 30 ++++++++ lib/pact_broker/versions/branch_version.rb | 9 +++ script/data/branches.rb | 35 +++++++++ spec/features/create_branch_version_spec.rb | 29 ++++++++ spec/features/get_branch_version_spec.rb | 12 ++++ .../api/resources/branch_version_spec.rb | 61 ++++++++++++++++ .../versions/branch_service_spec.rb | 71 +++++++++++++++++++ 15 files changed, 343 insertions(+), 6 deletions(-) create mode 100644 lib/pact_broker/api/decorators/branch_version_decorator.rb create mode 100644 lib/pact_broker/api/resources/branch_version.rb create mode 100644 lib/pact_broker/versions/branch_service.rb create mode 100755 script/data/branches.rb create mode 100644 spec/features/create_branch_version_spec.rb create mode 100644 spec/features/get_branch_version_spec.rb create mode 100644 spec/lib/pact_broker/api/resources/branch_version_spec.rb create mode 100644 spec/lib/pact_broker/versions/branch_service_spec.rb diff --git a/lib/pact_broker/api.rb b/lib/pact_broker/api.rb index 9c4c3c2e9..8166e0122 100644 --- a/lib/pact_broker/api.rb +++ b/lib/pact_broker/api.rb @@ -87,6 +87,7 @@ def self.build_api(application_context = PactBroker::ApplicationContext.default_ add ["pacticipants", :pacticipant_name, "latest-version"], Api::Resources::LatestVersion, {resource_name: "latest_pacticipant_version"} add ["pacticipants", :pacticipant_name, "versions", :pacticipant_version_number, "tags", :tag_name], Api::Resources::Tag, {resource_name: "pacticipant_version_tag"} add ["pacticipants", :pacticipant_name, "labels", :label_name], Api::Resources::Label, {resource_name: "pacticipant_label"} + add ["pacticipants", :pacticipant_name, "branches", :branch_name, "versions", :version_number], Api::Resources::BranchVersion, { resource_name: "branch_version" } # Webhooks add ["webhooks", "provider", :provider_name, "consumer", :consumer_name ], Api::Resources::PacticipantWebhooks, {resource_name: "pacticipant_webhooks"} diff --git a/lib/pact_broker/api/decorators/branch_version_decorator.rb b/lib/pact_broker/api/decorators/branch_version_decorator.rb new file mode 100644 index 000000000..e59a4d6ad --- /dev/null +++ b/lib/pact_broker/api/decorators/branch_version_decorator.rb @@ -0,0 +1,20 @@ +require "pact_broker/api/decorators/base_decorator" +require "pact_broker/api/decorators/timestamps" + +module PactBroker + module Api + module Decorators + class BranchVersionDecorator < BaseDecorator + + link :self do | user_options | + { + title: "Branch version", + href: branch_version_url(represented, user_options.fetch(:base_url)) + } + end + + include Timestamps + end + end + end +end diff --git a/lib/pact_broker/api/pact_broker_urls.rb b/lib/pact_broker/api/pact_broker_urls.rb index f9f95fed4..823184615 100644 --- a/lib/pact_broker/api/pact_broker_urls.rb +++ b/lib/pact_broker/api/pact_broker_urls.rb @@ -199,9 +199,8 @@ def tag_url base_url, tag "#{tags_url(base_url, tag.version)}/#{url_encode(tag.name)}" end - # This resource does not actually exist def branch_version_url(branch_version, base_url = "") - "#{version_url(base_url, branch_version.version)}/branches/#{branch_version.branch_name}" + "#{pacticipant_url(base_url, branch_version.pacticipant)}/branches/#{url_encode(branch_version.branch_name)}/versions/#{url_encode(branch_version.version_number)}" end def templated_tag_url_for_pacticipant pacticipant_name, base_url = "" diff --git a/lib/pact_broker/api/resources/branch_version.rb b/lib/pact_broker/api/resources/branch_version.rb new file mode 100644 index 000000000..e51d609d4 --- /dev/null +++ b/lib/pact_broker/api/resources/branch_version.rb @@ -0,0 +1,50 @@ +require 'pact_broker/api/resources/base_resource' +require 'pact_broker/api/decorators/branch_version_decorator' + +module PactBroker + module Api + module Resources + class BranchVersion < BaseResource + def content_types_provided + [["application/hal+json", :to_json]] + end + + def content_types_accepted + [["application/json", :from_json]] + end + + def allowed_methods + ["GET", "PUT", "OPTIONS"] + end + + def resource_exists? + !!branch_version + end + + def to_json + decorator_class(:branch_version_decorator).new(branch_version).to_json(decorator_options) + end + + def from_json + already_existed = !!branch_version + @branch_version = branch_service.find_or_create_branch_version(identifier_from_path) + # Make it return a 201 by setting the Location header + response.headers["Location"] = branch_version_url(branch_version, base_url) unless already_existed + response.body = to_json + end + + def policy_name + :'versions::branch_version' + end + + private + + attr_reader :branch_version + + def branch_version + @branch_version ||= branch_service.find_branch_version(identifier_from_path) + end + end + end + end +end diff --git a/lib/pact_broker/api/resources/index.rb b/lib/pact_broker/api/resources/index.rb index 4957e4989..672556e4a 100644 --- a/lib/pact_broker/api/resources/index.rb +++ b/lib/pact_broker/api/resources/index.rb @@ -116,6 +116,12 @@ def links title: "Get, create or delete a tag for a pacticipant version", templated: true }, + "pb:pacticipant-branch-version" => + { + href: base_url + "/pacticipants/{pacticipant}/branches/{branch}/versions/{version}", + title: "Get or add/create a pacticipant version for a branch", + templated: true + }, "pb:pacticipant-version" => { href: base_url + "/pacticipants/{pacticipant}/versions/{version}", diff --git a/lib/pact_broker/domain/pacticipant.rb b/lib/pact_broker/domain/pacticipant.rb index fda56915f..ca9c5bd8e 100644 --- a/lib/pact_broker/domain/pacticipant.rb +++ b/lib/pact_broker/domain/pacticipant.rb @@ -40,6 +40,10 @@ def label label_name def find_by_name(name) where(name_like(:name, name)) end + + def where_name(name) + where(name_like(:name, name)) + end end def before_destroy diff --git a/lib/pact_broker/services.rb b/lib/pact_broker/services.rb index fccad7fcf..dcc9768a4 100644 --- a/lib/pact_broker/services.rb +++ b/lib/pact_broker/services.rb @@ -89,6 +89,10 @@ def contract_service get(:contract_service) end + def branch_service + get(:branch_service) + end + # rubocop: disable Metrics/MethodLength def register_default_services register_service(:index_service) do @@ -185,6 +189,11 @@ def register_default_services require "pact_broker/contracts/service" PactBroker::Contracts::Service end + + register_service(:branch_service) do + require "pact_broker/versions/branch_service" + PactBroker::Versions::BranchService + end end # rubocop: enable Metrics/MethodLength end diff --git a/lib/pact_broker/test/http_test_data_builder.rb b/lib/pact_broker/test/http_test_data_builder.rb index 98ddad7dd..27db57ddb 100644 --- a/lib/pact_broker/test/http_test_data_builder.rb +++ b/lib/pact_broker/test/http_test_data_builder.rb @@ -55,10 +55,11 @@ def create_tag(pacticipant:, version:, tag:) end def create_version(pacticipant:, version:, branch:) - request_body = { - branch: branch - } - client.put("pacticipants/#{encode(pacticipant)}/versions/#{encode(version)}", request_body).tap { |response| check_for_error(response) } + if branch + client.put("pacticipants/#{encode(pacticipant)}/branches/#{encode(branch)}/versions/#{encode(version)}", {}).tap { |response| check_for_error(response) } + else + client.put("pacticipants/#{encode(pacticipant)}/versions/#{encode(version)}").tap { |response| check_for_error(response) } + end self end diff --git a/lib/pact_broker/versions/branch_service.rb b/lib/pact_broker/versions/branch_service.rb new file mode 100644 index 000000000..eb007164a --- /dev/null +++ b/lib/pact_broker/versions/branch_service.rb @@ -0,0 +1,30 @@ +require "pact_broker/logging" +require "pact_broker/repositories" +require "pact_broker/messages" + +module PactBroker + module Versions + class BranchService + extend PactBroker::Repositories + + def self.find_branch_version(pacticipant_name:, branch_name:, version_number:, **) + BranchVersion.where( + version: PactBroker::Domain::Version.where_pacticipant_name_and_version_number(pacticipant_name, version_number), + branch: Branch.where(name: branch_name) + ).single_record + end + + def self.find_or_create_branch_version(pacticipant_name:, branch_name:, version_number:, **) + pacticipant = pacticipant_repository.find_by_name_or_create(pacticipant_name) + version = version_repository.find_by_pacticipant_id_and_number_or_create(pacticipant.id, version_number) + branch = Branch.find_or_create(pacticipant: pacticipant, name: branch_name) + branch_version = BranchVersion.new( + version: version, + branch: branch + ).upsert + PactBroker::Versions::BranchHead.new(branch: branch, branch_version: branch_version).upsert + branch_version + end + end + end +end diff --git a/lib/pact_broker/versions/branch_version.rb b/lib/pact_broker/versions/branch_version.rb index dbe29ecef..2761c7168 100644 --- a/lib/pact_broker/versions/branch_version.rb +++ b/lib/pact_broker/versions/branch_version.rb @@ -6,6 +6,7 @@ module Versions class BranchVersion < Sequel::Model(:branch_versions) plugin :timestamps, update_on_create: true plugin :insert_ignore, identifying_columns: [:branch_id, :version_id] + plugin :upsert, identifying_columns: [:branch_id, :version_id] associate(:many_to_one, :branch, :class => "PactBroker::Versions::Branch", :key => :branch_id, :primary_key => :id) associate(:many_to_one, :version, :class => "PactBroker::Domain::Version", :key => :version_id, :primary_key => :id) @@ -28,6 +29,14 @@ def before_save def latest? branch_head.branch_version_id == id end + + def version_number + version.number + end + + def pacticipant + branch.pacticipant + end end end end diff --git a/script/data/branches.rb b/script/data/branches.rb new file mode 100755 index 000000000..ff90d0d49 --- /dev/null +++ b/script/data/branches.rb @@ -0,0 +1,35 @@ +#!/usr/bin/env ruby +begin + + $LOAD_PATH << "#{Dir.pwd}/lib" + require "pact_broker/test/http_test_data_builder" + base_url = ENV["PACT_BROKER_BASE_URL"] || "http://localhost:9292" + + td = PactBroker::Test::HttpTestDataBuilder.new(base_url) + td.delete_pacticipant("branch-provider") + .delete_pacticipant("branch-consumer") + .publish_contract(consumer: "branch-consumer", provider: "branch-provider", consumer_version: "1", content_id: "1111", branch: "main") + .publish_contract(consumer: "branch-consumer", provider: "branch-provider", consumer_version: "1", content_id: "1111", branch: "feat/x") + .publish_contract(consumer: "branch-consumer", provider: "branch-provider", consumer_version: "2", content_id: "1111", branch: "feat/x") + .get_pacts_for_verification(provider: "branch-provider") + .verify_pact( + provider_version_branch: "main", + provider_version: "1", + success: true + ) + .verify_pact( + provider_version_branch: "feat/y", + provider_version: "1", + success: true + ) + .verify_pact( + provider_version_branch: "feat/y", + provider_version: "2", + success: true + ) + +rescue StandardError => e + puts "#{e.class} #{e.message}" + puts e.backtrace + exit 1 +end diff --git a/spec/features/create_branch_version_spec.rb b/spec/features/create_branch_version_spec.rb new file mode 100644 index 000000000..ab99b9734 --- /dev/null +++ b/spec/features/create_branch_version_spec.rb @@ -0,0 +1,29 @@ +describe "Creating a branch version" do + let(:path) { "/pacticipants/Foo/branches/main/versions/1234" } + let(:headers) { { "CONTENT_TYPE" => "application/json" } } + let(:response_body) { JSON.parse(subject.body, symbolize_names: true) } + + subject { put(path, {}, headers) } + + it "returns a 201 response" do + expect(subject.status).to be 201 + end + + it "returns a HAL JSON Content Type" do + expect(subject.headers["Content-Type"]).to eq "application/hal+json;charset=utf-8" + end + + context "when the branch version already exists" do + before do + td.subtract_day + .create_consumer("Foo") + .create_consumer_version("1234", branch: "main") + end + + its(:status) { is_expected.to eq 200 } + end + + context "when the branch version does not exist" do + its(:status) { is_expected.to eq 201 } + end +end diff --git a/spec/features/get_branch_version_spec.rb b/spec/features/get_branch_version_spec.rb new file mode 100644 index 000000000..db5f89628 --- /dev/null +++ b/spec/features/get_branch_version_spec.rb @@ -0,0 +1,12 @@ +describe "Get a branch version" do + before do + td.create_consumer("Foo") + .create_consumer_version("1234", branch: "main") + end + let(:path) { PactBroker::Api::PactBrokerUrls.branch_version_url(PactBroker::Versions::BranchVersion.first) } + let(:headers) { { "CONTENT_TYPE" => "application/json" } } + + subject { get(path, {}, headers) } + + it { is_expected.to be_a_hal_json_success_response } +end diff --git a/spec/lib/pact_broker/api/resources/branch_version_spec.rb b/spec/lib/pact_broker/api/resources/branch_version_spec.rb new file mode 100644 index 000000000..fa6d76678 --- /dev/null +++ b/spec/lib/pact_broker/api/resources/branch_version_spec.rb @@ -0,0 +1,61 @@ +require 'pact_broker/api/resources/branch_version' + +module PactBroker + module Api + module Resources + describe BranchVersion do + before do + allow_any_instance_of(described_class).to receive(:branch_version_service).and_return(branch_version_service) + allow(branch_version_service).to receive(:find_by_uuid).and_return(branch_version) + allow(PactBroker::Api::Decorators::BranchVersionDecorator).to receive(:new).and_return(decorator) + end + + let(:branch_version) { instance_double("PactBroker::Versions::BranchVersion") } + let(:parsed_branch_version) { double("parsed branch_version") } + let(:branch_version_service) { class_double("PactBroker::Versions::Service").as_stubbed_const } + let(:path) { "/branch-versions/#{uuid}" } + let(:uuid) { "12345678" } + let(:rack_headers) do + { + "HTTP_ACCEPT" => "application/hal+json" + } + end + let(:decorator) do + instance_double("PactBroker::Api::Decorators::BranchVersionDecorator", + to_json: "response", + from_json: parsed_branch_version + ) + end + + describe "GET" do + subject { get(path, nil, rack_headers) } + + it "attempts to find the BranchVersion" do + expect(PactBroker::Versions::Service).to receive(:find_by_uuid).with(uuid) + subject + end + + context "when the branch_version does not exist" do + let(:branch_version) { nil } + + it { is_expected.to be_a_404_response } + end + + context "when the BranchVersion exists" do + it "generates a JSON representation of the BranchVersion" do + expect(PactBroker::Api::Decorators::BranchVersionDecorator).to receive(:new).with(branch_version) + expect(decorator).to receive(:to_json).with(user_options: hash_including(base_url: "http://example.org")) + subject + end + + it { is_expected.to be_a_hal_json_success_response } + + it "includes the JSON representation in the response body" do + expect(subject.body).to eq "response" + end + end + end + end + end + end +end diff --git a/spec/lib/pact_broker/versions/branch_service_spec.rb b/spec/lib/pact_broker/versions/branch_service_spec.rb new file mode 100644 index 000000000..20cae7ceb --- /dev/null +++ b/spec/lib/pact_broker/versions/branch_service_spec.rb @@ -0,0 +1,71 @@ +require 'pact_broker/versions/branch_service' + +module PactBroker + module Versions + describe BranchService do + describe ".find_branch_version" do + before do + td.create_consumer("Foo") + .create_consumer_version("1", branch: "main") + .create_consumer_version("1", branch: "feat/x") + .create_consumer_version("2", branch: "main") + end + + subject { BranchService.find_branch_version(pacticipant_name: "Foo", version_number: "1", branch_name: "main") } + + its(:version_number) { is_expected.to eq "1" } + end + + describe "#create_branch_version" do + subject { BranchService.find_or_create_branch_version(pacticipant_name: "Foo", version_number: "1", branch_name: "main") } + + context "when nothing exists" do + it "creates and returns the branch version" do + expect(subject.pacticipant.name).to eq "Foo" + expect(subject.version_number).to eq "1" + expect(subject.branch_name).to eq "main" + end + end + + context "when everything exists except the branch version" do + before do + td.create_consumer("Foo") + .create_consumer_version("0", branch: "main") + end + + it "does not create the branch" do + expect{ subject }.to_not change { PactBroker::Versions::Branch.where(name: "main").count } + end + + it "creates and returns the branch version" do + expect(subject.pacticipant.name).to eq "Foo" + expect(subject.version_number).to eq "1" + expect(subject.branch_name).to eq "main" + expect(subject.branch_head).to_not be nil + end + end + + context "when everything exists" do + before do + td.create_consumer("Foo") + .create_consumer_version("1", branch: "main") + + BranchVersion.dataset.update(updated_at: Sequel.datetime_class.now - 10, created_at: Sequel.datetime_class.now - 10) + end + + it "does not create a new branch version" do + expect{ subject }.to_not change { PactBroker::Versions::BranchVersion.count } + end + + it "updates the updated_at" do + expect{ subject }.to change { PactBroker::Versions::BranchVersion.first.updated_at } + end + + it "does not change the created_at" do + expect{ subject }.to_not change { PactBroker::Versions::BranchVersion.first.created_at } + end + end + end + end + end +end From 0ec47044e3d77d08ae5d1b785af58964c6f37c9a Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Wed, 1 Sep 2021 16:17:07 +1000 Subject: [PATCH 19/45] chore: add branch-version relation to pacticipant --- lib/pact_broker/api/decorators/pacticipant_decorator.rb | 8 ++++++++ lib/pact_broker/api/pact_broker_urls.rb | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/lib/pact_broker/api/decorators/pacticipant_decorator.rb b/lib/pact_broker/api/decorators/pacticipant_decorator.rb index 54678a170..5b24dba8f 100644 --- a/lib/pact_broker/api/decorators/pacticipant_decorator.rb +++ b/lib/pact_broker/api/decorators/pacticipant_decorator.rb @@ -49,6 +49,14 @@ class PacticipantDecorator < BaseDecorator } end + link :'pb:branch-version' do | options | + { + title: "Get or add/create a version for a branch of #{represented.name}", + href: templated_branch_version_url_for_pacticipant(represented.name, options[:base_url]), + templated: true + } + end + link :'pb:label' do | options | { title: "Get, create or delete a label for #{represented.name}", diff --git a/lib/pact_broker/api/pact_broker_urls.rb b/lib/pact_broker/api/pact_broker_urls.rb index 823184615..3a06f4567 100644 --- a/lib/pact_broker/api/pact_broker_urls.rb +++ b/lib/pact_broker/api/pact_broker_urls.rb @@ -207,6 +207,10 @@ def templated_tag_url_for_pacticipant pacticipant_name, base_url = "" pacticipant_url_from_params({ pacticipant_name: pacticipant_name }, base_url) + "/versions/{version}/tags/{tag}" end + def templated_branch_version_url_for_pacticipant pacticipant_name, base_url = "" + pacticipant_url_from_params({ pacticipant_name: pacticipant_name }, base_url) + "/branches/{branch}/versions/{version}" + end + def templated_version_url_for_pacticipant pacticipant_name, base_url = "" pacticipant_url_from_params({ pacticipant_name: pacticipant_name }, base_url) + "/versions/{version}" end From 747c73d57efc2a11a584004a1fc1ff0c557e8038 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 2 Sep 2021 09:39:51 +1000 Subject: [PATCH 20/45] chore: remove references to version.branch --- lib/pact_broker/contracts/service.rb | 12 ++-- .../index/pacticipant-branch-version.markdown | 14 ++++ lib/pact_broker/domain/version.rb | 34 +++++---- lib/pact_broker/pacts/pact_publication.rb | 25 +++++-- .../pact_publication_wip_dataset_module.rb | 20 ++++-- .../pacts_for_verification_repository.rb | 2 +- lib/pact_broker/repositories.rb | 5 ++ lib/pact_broker/test/test_data_builder.rb | 59 +++++++-------- lib/pact_broker/versions/branch_service.rb | 8 +-- .../versions/branch_version_repository.rb | 34 +++++++++ lib/pact_broker/versions/repository.rb | 39 +++------- lib/pact_broker/versions/service.rb | 6 +- .../modifiable_resources.approved.json | 3 + ...lish_contract_nothing_exists.approved.json | 3 +- ..._nothing_exists_with_webhook.approved.json | 3 +- ..._verification_already_exists.approved.json | 3 +- .../api/resources/branch_version_spec.rb | 61 ---------------- .../lib/pact_broker/contracts/service_spec.rb | 2 +- ...ublication_selector_dataset_module_spec.rb | 4 +- .../branch_version_repository_spec.rb | 71 +++++++++++++++++++ .../pact_broker/versions/repository_spec.rb | 59 +++------------ spec/lib/pact_broker/versions/service_spec.rb | 7 +- 22 files changed, 247 insertions(+), 227 deletions(-) create mode 100644 lib/pact_broker/doc/views/index/pacticipant-branch-version.markdown create mode 100644 lib/pact_broker/versions/branch_version_repository.rb delete mode 100644 spec/lib/pact_broker/api/resources/branch_version_spec.rb create mode 100644 spec/lib/pact_broker/versions/branch_version_repository_spec.rb diff --git a/lib/pact_broker/contracts/service.rb b/lib/pact_broker/contracts/service.rb index 603e6a561..4e8afa0f4 100644 --- a/lib/pact_broker/contracts/service.rb +++ b/lib/pact_broker/contracts/service.rb @@ -6,6 +6,7 @@ require "pact_broker/contracts/notice" require "pact_broker/events/subscriber" require "pact_broker/api/pact_broker_urls" +require "pact_broker/versions/branch_version_repository" module PactBroker module Contracts @@ -44,12 +45,11 @@ def publish(parsed_contracts, base_url: ) def create_version(parsed_contracts) version_params = { - build_url: parsed_contracts.build_url, - branch: parsed_contracts.branch + build_url: parsed_contracts.build_url }.compact existing_version = find_existing_version(parsed_contracts) - version = create_or_update_version(parsed_contracts, version_params) + version = create_or_update_version(parsed_contracts, version_params, parsed_contracts.branch) return version, notices_for_version_creation(existing_version, parsed_contracts) end @@ -64,12 +64,14 @@ def find_existing_version(parsed_contracts) private :find_existing_version - def create_or_update_version(parsed_contracts, version_params) - version_service.create_or_update( + def create_or_update_version(parsed_contracts, version_params, branch_name) + version = version_service.create_or_update( parsed_contracts.pacticipant_name, parsed_contracts.pacticipant_version_number, OpenStruct.new(version_params) ) + branch_version_repository.add_branch(version, branch_name) if branch_name + version end private :create_or_update_version diff --git a/lib/pact_broker/doc/views/index/pacticipant-branch-version.markdown b/lib/pact_broker/doc/views/index/pacticipant-branch-version.markdown new file mode 100644 index 000000000..a7fb5b15d --- /dev/null +++ b/lib/pact_broker/doc/views/index/pacticipant-branch-version.markdown @@ -0,0 +1,14 @@ +# Pacticipant branch version + +Allowed methods: `GET`, `PUT` + +Path: `/pacticipants/{pacticipant}/branches/{branch}/versions/{version}` + +Get or add/create a pacticipant version for a branch. + +## Example + +Add a version to a branch. The pacticipant and branch are automatically created if they do not exist. + + curl -XPUT http://broker/pacticipants/Bar/branches/main/versions/1e70030c6579915e5ff56b107a0fd25cf5df7464 \ + -H "Content-Type: application/json" -d "" diff --git a/lib/pact_broker/domain/version.rb b/lib/pact_broker/domain/version.rb index 70312f751..95c8f40eb 100644 --- a/lib/pact_broker/domain/version.rb +++ b/lib/pact_broker/domain/version.rb @@ -21,7 +21,9 @@ def latest? end end - class Version < Sequel::Model + VERSION_COLUMNS = [:id, :number, :repository_ref, :pacticipant_id, :order, :created_at, :updated_at, :build_url] + + class Version < Sequel::Model(Sequel::Model.db[:versions].select(*VERSION_COLUMNS.collect{ | column | Sequel.qualify(:versions, column) })) plugin :timestamps, update_on_create: true plugin :upsert, { identifying_columns: [:pacticipant_id, :number], ignore_columns_on_update: [:id, :created_at, :order] } @@ -49,7 +51,7 @@ class Version < Sequel::Model include PactBroker::Repositories::Helpers def with_branch_set - exclude(branch: nil) + where(id: PactBroker::Versions::BranchVersion.select(:version_id)) end def latest_version_for_pacticipant(pacticipant) @@ -67,20 +69,16 @@ def where_pacticipant_name_and_version_number(pacticipant_name, version_number) end def first_for_pacticipant_id_and_branch(pacticipant_id, branch) - where(pacticipant_id: pacticipant_id, branch: branch).order(:created_at).first + first_version_id = PactBroker::Versions::BranchVersion + .select(:version_id) + .where(pacticipant_id: pacticipant_id, branch_name: branch) + .order(:created_at) + .limit(1) + where(id: first_version_id).single_record end - def latest_versions_for_pacticipant_branches(pacticipant_id, branches) - query = Version.where(Sequel[:versions][:pacticipant_id] => pacticipant_id, Sequel[:versions][:branch] => branches) - - self_join = { - Sequel[:versions][:pacticipant_id] => Sequel[:versions_2][:pacticipant_id], - Sequel[:versions][:branch] => Sequel[:versions_2][:branch] - } - query.select_all_qualified.left_join(query, self_join, table_alias: :versions_2) do - Sequel[:versions_2][:order] > Sequel[:versions][:order] - end - .where(Sequel[:versions_2][:order] => nil) + def latest_versions_for_pacticipant_branches(pacticipant_id, branch_names) + where(id: PactBroker::Versions::BranchHead.where(pacticipant_id: pacticipant_id, branch_name: branch_names).select(:version_id)) end def where_pacticipant_name(pacticipant_name) @@ -302,6 +300,14 @@ def branch_version_for_branch(branch) branch_versions.find { | branch_version | branch_version.branch_id == branch.id } end + def branch_version_for_branch_name(branch_name) + branch_versions.find { | branch_version | branch_version.branch_name == branch_name } + end + + def branch_names + branch_versions.collect(&:branch_name) + end + def tag_names tags.collect(&:name) end diff --git a/lib/pact_broker/pacts/pact_publication.rb b/lib/pact_broker/pacts/pact_publication.rb index ab1aefbcf..13e9fe453 100644 --- a/lib/pact_broker/pacts/pact_publication.rb +++ b/lib/pact_broker/pacts/pact_publication.rb @@ -74,13 +74,26 @@ def latest_verification end def latest_for_branch? - return nil unless consumer_version.branch - self_order = self.consumer_version.order - PactPublication.where(consumer_id: consumer_id, provider_id: provider_id) - .join_consumer_versions(:cv, { Sequel[:cv][:branch] => consumer_version.branch} ) do - Sequel[:cv][:order] > self_order + if !defined?(@latest_for_branch) + if consumer_version.branch_versions.empty? + @latest_for_branch = nil + else + self_order = self.consumer_version.order + @latest_for_branch = consumer_version.branch_versions.any? do | branch_version | + branch_versions_join = { + Sequel[:cv][:id] => Sequel[:branch_versions][:version_id], + Sequel[:branch_versions][:branch_name] => branch_version.branch_name + } + PactPublication.where(consumer_id: consumer_id, provider_id: provider_id) + .join_consumer_versions(:cv) do + Sequel[:cv][:order] > self_order + end + .join(:branch_versions, branch_versions_join) + .empty? + end end - .empty? + end + @latest_for_branch end def to_domain diff --git a/lib/pact_broker/pacts/pact_publication_wip_dataset_module.rb b/lib/pact_broker/pacts/pact_publication_wip_dataset_module.rb index f56379361..5605a691c 100644 --- a/lib/pact_broker/pacts/pact_publication_wip_dataset_module.rb +++ b/lib/pact_broker/pacts/pact_publication_wip_dataset_module.rb @@ -16,14 +16,22 @@ def successfully_verified_by_provider_another_branch_before_this_branch_first_cr from_self(alias: :pp) .select(Sequel[:pp].*) .join_successful_non_wip_verifications_for_provider_id(provider_id) - .join_provider_versions_for_provider_id(provider_id) do - Sequel.lit("provider_versions.branch != ?", provider_version_branch) - end + .join_provider_versions_for_provider_id(provider_id) + .join_branch_versions_excluding_branch(provider_version_branch) .where(Sequel[:pp][:provider_id] => provider_id) .verified_before_creation_date_of(first_version_for_branch) .distinct end + def join_branch_versions_excluding_branch(branch_name) + branch_versions_join = { + Sequel[:provider_versions][:id] => Sequel[:branch_versions][:version_id] + } + join(:branch_versions, branch_versions_join) do + Sequel.lit("branch_versions.branch_name != ?", branch_name) + end + end + def successfully_verified_by_provider_tag_when_not_wip(provider_id, provider_tag) from_self(alias: :pp) .select(Sequel[:pp].*) @@ -80,10 +88,14 @@ def join_provider_version_tags_for_tag(tag) def join_provider_versions_for_provider_id_and_branch(provider_id, provider_version_branch) versions_join = { Sequel[:verifications][:provider_version_id] => Sequel[:provider_versions][:id], - Sequel[:provider_versions][:branch] => provider_version_branch, Sequel[:provider_versions][:pacticipant_id] => provider_id } + branch_versions_join = { + Sequel[:provider_versions][:id] => Sequel[:branch_versions][:version_id], + Sequel[:branch_versions][:branch_name] => provider_version_branch + } join(:versions, versions_join, { table_alias: :provider_versions } ) + .join(:branch_versions, branch_versions_join) end def join_provider_versions_for_provider_id(provider_id, &block) diff --git a/lib/pact_broker/pacts/pacts_for_verification_repository.rb b/lib/pact_broker/pacts/pacts_for_verification_repository.rb index 83d0c7e77..c41127a41 100644 --- a/lib/pact_broker/pacts/pacts_for_verification_repository.rb +++ b/lib/pact_broker/pacts/pacts_for_verification_repository.rb @@ -215,7 +215,7 @@ def create_selectors_for_wip_pact(pact_publication) if pact_publication.values[:tag_name] Selectors.create_for_latest_for_tag(pact_publication.values[:tag_name]) else - Selectors.create_for_latest_for_branch(pact_publication.consumer_version.branch) + Selectors.create_for_latest_for_branch(pact_publication.values.fetch(:branch)) end end diff --git a/lib/pact_broker/repositories.rb b/lib/pact_broker/repositories.rb index d8d4788f9..992184785 100644 --- a/lib/pact_broker/repositories.rb +++ b/lib/pact_broker/repositories.rb @@ -42,6 +42,11 @@ def matrix_repository Matrix::Repository.new end + def branch_version_repository + require "pact_broker/versions/branch_version_repository" + PactBroker::Versions::BranchVersionRepository.new + end + extend self end end diff --git a/lib/pact_broker/test/test_data_builder.rb b/lib/pact_broker/test/test_data_builder.rb index fabe5c5f7..695491acc 100644 --- a/lib/pact_broker/test/test_data_builder.rb +++ b/lib/pact_broker/test/test_data_builder.rb @@ -29,6 +29,7 @@ require "pact_broker/deployments/environment_service" require "pact_broker/deployments/deployed_version_service" require "pact_broker/deployments/released_version_service" +require "pact_broker/versions/branch_version_repository" require "ostruct" module PactBroker @@ -162,47 +163,17 @@ def use_provider provider_name end def create_version version_number = "1.0.#{model_counter}", params = {} - params.delete(:comment) - @version = PactBroker::Domain::Version.create(:number => version_number, :pacticipant => @pacticipant) + @version = create_pacticipant_version(version_number, pacticipant, params) self end def create_consumer_version version_number = "1.0.#{model_counter}", params = {} - params.delete(:comment) - tag_names = [params.delete(:tag_names), params.delete(:tag_name)].flatten.compact - args = { - number: version_number, - pacticipant_id: @consumer.id, - branch: params[:branch], - build_url: params[:build_url] - } - - @consumer_version = PactBroker::Versions::Repository.new.create(args) - - set_created_at_if_set params[:created_at], :versions, { id: @consumer_version.id } - tag_names.each do | tag_name | - tag = PactBroker::Domain::Tag.create(name: tag_name, version: consumer_version) - set_created_at_if_set(params[:created_at], :tags, { name: tag.name, version_id: consumer_version.id }) - end + @consumer_version = create_pacticipant_version(version_number, consumer, params) self end def create_provider_version version_number = "1.0.#{model_counter}", params = {} - params.delete(:comment) - tag_names = [params.delete(:tag_names), params.delete(:tag_name)].flatten.compact - args = { - number: version_number, - pacticipant_id: @provider.id, - branch: params[:branch], - build_url: params[:build_url] - } - @version = PactBroker::Versions::Repository.new.create(args) - - @provider_version = @version - tag_names.each do | tag_name | - tag = PactBroker::Domain::Tag.create(name: tag_name, version: provider_version) - set_created_at_if_set(params[:created_at], :tags, { name: tag.name, version_id: provider_version.id }) - end + @provider_version = create_pacticipant_version(version_number, provider, params) self end @@ -386,7 +357,7 @@ def create_verification parameters = {} pact_version = PactBroker::Pacts::Repository.new.find_pact_version(@consumer, @provider, pact.pact_version_sha) @verification = PactBroker::Verifications::Repository.new.create(verification, provider_version_number, pact_version) @provider_version = PactBroker::Domain::Version.where(pacticipant_id: @provider.id, number: provider_version_number).single_record - @provider_version.update(branch: branch) if branch + PactBroker::Versions::BranchVersionRepository.new.add_branch(@provider_version, branch) if branch set_created_at_if_set(parameters[:created_at], :verifications, id: @verification.id) set_created_at_if_set(parameters[:created_at], :versions, id: @provider_version.id) @@ -552,6 +523,26 @@ def random_json_content(consumer_name, provider_name) private + def create_pacticipant_version(version_number, pacticipant, params = {}) + params.delete(:comment) + tag_names = [params.delete(:tag_names), params.delete(:tag_name)].flatten.compact + args = { + number: version_number, + pacticipant_id: pacticipant.id, + branch: params[:branch], + build_url: params[:build_url] + } + + version = PactBroker::Versions::Repository.new.create(args) + + set_created_at_if_set params[:created_at], :versions, { id: version.id } + tag_names.each do | tag_name | + tag = PactBroker::Domain::Tag.create(name: tag_name, version: version) + set_created_at_if_set(params[:created_at], :tags, { name: tag.name, version_id: version.id }) + end + version + end + def create_deployed_version(uuid: , currently_deployed: , version:, environment_name: , target: nil, created_at: nil) env = find_environment(environment_name) @deployed_version = PactBroker::Deployments::DeployedVersionService.find_or_create(uuid, version, env, target) diff --git a/lib/pact_broker/versions/branch_service.rb b/lib/pact_broker/versions/branch_service.rb index eb007164a..826825bb0 100644 --- a/lib/pact_broker/versions/branch_service.rb +++ b/lib/pact_broker/versions/branch_service.rb @@ -17,13 +17,7 @@ def self.find_branch_version(pacticipant_name:, branch_name:, version_number:, * def self.find_or_create_branch_version(pacticipant_name:, branch_name:, version_number:, **) pacticipant = pacticipant_repository.find_by_name_or_create(pacticipant_name) version = version_repository.find_by_pacticipant_id_and_number_or_create(pacticipant.id, version_number) - branch = Branch.find_or_create(pacticipant: pacticipant, name: branch_name) - branch_version = BranchVersion.new( - version: version, - branch: branch - ).upsert - PactBroker::Versions::BranchHead.new(branch: branch, branch_version: branch_version).upsert - branch_version + branch_version_repository.add_branch(version, branch_name) end end end diff --git a/lib/pact_broker/versions/branch_version_repository.rb b/lib/pact_broker/versions/branch_version_repository.rb new file mode 100644 index 000000000..a0c7a3a47 --- /dev/null +++ b/lib/pact_broker/versions/branch_version_repository.rb @@ -0,0 +1,34 @@ +module PactBroker + module Versions + class BranchVersionRepository + include PactBroker::Services + + def add_branch(version, branch_name) + branch = find_or_create_branch(version.pacticipant, branch_name) + branch_version = version.branch_version_for_branch(branch) + if branch_version + branch_version.update(updated_at: Sequel.datetime_class.now) + else + branch_version = PactBroker::Versions::BranchVersion.new(version: version, branch: branch).insert_ignore + PactBroker::Versions::BranchHead.new(branch: branch, branch_version: branch_version).upsert + end + pacticipant_service.maybe_set_main_branch(version.pacticipant, branch_name) + branch_version + end + + private + + def find_or_create_branch(pacticipant, branch_name) + find_branch(pacticipant, branch_name) || create_branch(pacticipant, branch_name) + end + + def find_branch(pacticipant, branch_name) + PactBroker::Versions::Branch.where(pacticipant: pacticipant, name: branch_name).single_record + end + + def create_branch(pacticipant, branch_name) + PactBroker::Versions::Branch.new(pacticipant: pacticipant, name: branch_name).insert_ignore + end + end + end +end diff --git a/lib/pact_broker/versions/repository.rb b/lib/pact_broker/versions/repository.rb index 5bca39486..ff66507dd 100644 --- a/lib/pact_broker/versions/repository.rb +++ b/lib/pact_broker/versions/repository.rb @@ -52,23 +52,21 @@ def find_by_pacticipant_name_and_number pacticipant_name, number end # There may be a race condition if two simultaneous requests come in to create the same version - def create args + def create(args) logger.info "Upserting version #{args[:number]} for pacticipant_id=#{args[:pacticipant_id]}" version_params = { number: args[:number], pacticipant_id: args[:pacticipant_id], created_at: Sequel.datetime_class.now, updated_at: Sequel.datetime_class.now, - build_url: args[:build_url], - branch: args[:branch] + build_url: args[:build_url] }.compact - version = PactBroker::Domain::Version.new(version_params).upsert - - if args[:branch] - add_branch(version, args[:branch]) - end + version = PactBroker::Domain::Version.new(version_params).upsert + # branch can't be set from CRUD on the version resource, but it's convenient to be able + # to make a version with a branch for internal code. + branch_version_repository.add_branch(version, args[:branch]) if args[:branch] version end @@ -76,7 +74,7 @@ def create_or_update(pacticipant, version_number, open_struct_version) saved_version = PactBroker::Domain::Version.where(pacticipant_id: pacticipant.id, number: version_number).single_record params = open_struct_version.to_h tags = params.delete(:tags) - branch_name = params[:branch] # TODO branches + branch_name = params.delete(:branch) if saved_version saved_version.update(params) else @@ -85,13 +83,12 @@ def create_or_update(pacticipant, version_number, open_struct_version) saved_version = PactBroker::Domain::Version.new( params.merge( pacticipant_id: pacticipant.id, - number: version_number, - branch: branch_name, + number: version_number ).compact ).upsert end - add_branch(saved_version, branch_name) if branch_name + branch_version_repository.add_branch(saved_version, branch_name) if branch_name replace_tags(saved_version, tags) if tags saved_version end @@ -108,10 +105,6 @@ def create_or_overwrite(pacticipant, version_number, open_struct_version) replace_tags(saved_version, open_struct_version.tags) end - if open_struct_version.branches - update_branches(saved_version, open_struct_version.branches) - end - saved_version end @@ -155,11 +148,6 @@ def find_versions_for_selector(selector) PactBroker::Domain::Version.select_all_qualified.for_selector(selector).all end - def set_branch_if_unset(version, branch) - version.update(branch: branch) if version.branch.nil? - version - end - def find_latest_version_from_main_branch(pacticipant) if pacticipant.main_branch latest_from_main_branch = PactBroker::Domain::Version @@ -169,15 +157,6 @@ def find_latest_version_from_main_branch(pacticipant) latest_from_main_branch || find_by_pacticipant_name_and_latest_tag(pacticipant.name, pacticipant.main_branch) end end - - def add_branch(version, branch_name) - branch = PactBroker::Versions::Branch.new(pacticipant: version.pacticipant, name: branch_name).insert_ignore - branch_version = version.branch_version_for_branch(branch) - if !branch_version - branch_version = PactBroker::Versions::BranchVersion.new(version: version, branch: branch).insert_ignore - end - PactBroker::Versions::BranchHead.new(branch: branch, branch_version: branch_version).upsert - end end end end diff --git a/lib/pact_broker/versions/service.rb b/lib/pact_broker/versions/service.rb index fd5cc4396..67a473d1e 100644 --- a/lib/pact_broker/versions/service.rb +++ b/lib/pact_broker/versions/service.rb @@ -42,14 +42,12 @@ def self.find_by_pacticipant_name_and_latest_tag(pacticipant_name, tag) def self.create_or_overwrite(pacticipant_name, version_number, version) pacticipant = pacticipant_repository.find_by_name_or_create(pacticipant_name) version = version_repository.create_or_overwrite(pacticipant, version_number, version) - pacticipant_service.maybe_set_main_branch(pacticipant, version.branch) version end def self.create_or_update(pacticipant_name, version_number, version) pacticipant = pacticipant_repository.find_by_name_or_create(pacticipant_name) version = version_repository.create_or_update(pacticipant, version_number, version) - pacticipant_service.maybe_set_main_branch(pacticipant, version.branch) version end @@ -66,9 +64,9 @@ def self.delete version end def self.maybe_set_version_branch_from_tag(version, tag_name) - if use_tag_as_branch?(version) && !version.branch + if use_tag_as_branch?(version) && version.branch_versions.empty? logger.info "Setting #{version.pacticipant.name} version #{version.number} branch to '#{tag_name}' from first tag (because use_first_tag_as_branch=true)" - version_repository.set_branch_if_unset(version, tag_name) + branch_version_repository.add_branch(version, tag_name) end end diff --git a/spec/fixtures/approvals/modifiable_resources.approved.json b/spec/fixtures/approvals/modifiable_resources.approved.json index 48366278e..c5447560e 100644 --- a/spec/fixtures/approvals/modifiable_resources.approved.json +++ b/spec/fixtures/approvals/modifiable_resources.approved.json @@ -3,6 +3,9 @@ { "resource_class_name": "PactBroker::Api::Resources::AllWebhooks" }, + { + "resource_class_name": "PactBroker::Api::Resources::BranchVersion" + }, { "resource_class_name": "PactBroker::Api::Resources::Clean" }, diff --git a/spec/fixtures/approvals/publish_contract_nothing_exists.approved.json b/spec/fixtures/approvals/publish_contract_nothing_exists.approved.json index 17ac9eefa..3941f1b75 100644 --- a/spec/fixtures/approvals/publish_contract_nothing_exists.approved.json +++ b/spec/fixtures/approvals/publish_contract_nothing_exists.approved.json @@ -30,7 +30,7 @@ "status": 200, "headers": { "Content-Type": "application/hal+json;charset=utf-8", - "Content-Length": "2992" + "Content-Length": "2976" }, "body": { "logs": [ @@ -111,7 +111,6 @@ }, "version": { "number": "1", - "branch": "main", "buildUrl": "http://ci/builds/1234", "_links": { "self": { diff --git a/spec/fixtures/approvals/publish_contract_nothing_exists_with_webhook.approved.json b/spec/fixtures/approvals/publish_contract_nothing_exists_with_webhook.approved.json index b9e086d6c..1116b5501 100644 --- a/spec/fixtures/approvals/publish_contract_nothing_exists_with_webhook.approved.json +++ b/spec/fixtures/approvals/publish_contract_nothing_exists_with_webhook.approved.json @@ -30,7 +30,7 @@ "status": 200, "headers": { "Content-Type": "application/hal+json;charset=utf-8", - "Content-Length": "2972" + "Content-Length": "2956" }, "body": { "logs": [ @@ -111,7 +111,6 @@ }, "version": { "number": "1", - "branch": "main", "buildUrl": "http://ci/builds/1234", "_links": { "self": { diff --git a/spec/fixtures/approvals/publish_contract_verification_already_exists.approved.json b/spec/fixtures/approvals/publish_contract_verification_already_exists.approved.json index daf2e8841..d6f3070da 100644 --- a/spec/fixtures/approvals/publish_contract_verification_already_exists.approved.json +++ b/spec/fixtures/approvals/publish_contract_verification_already_exists.approved.json @@ -30,7 +30,7 @@ "status": 200, "headers": { "Content-Type": "application/hal+json;charset=utf-8", - "Content-Length": "2929" + "Content-Length": "2913" }, "body": { "logs": [ @@ -102,7 +102,6 @@ }, "version": { "number": "1", - "branch": "main", "buildUrl": "http://ci/builds/1234", "_links": { "self": { diff --git a/spec/lib/pact_broker/api/resources/branch_version_spec.rb b/spec/lib/pact_broker/api/resources/branch_version_spec.rb deleted file mode 100644 index fa6d76678..000000000 --- a/spec/lib/pact_broker/api/resources/branch_version_spec.rb +++ /dev/null @@ -1,61 +0,0 @@ -require 'pact_broker/api/resources/branch_version' - -module PactBroker - module Api - module Resources - describe BranchVersion do - before do - allow_any_instance_of(described_class).to receive(:branch_version_service).and_return(branch_version_service) - allow(branch_version_service).to receive(:find_by_uuid).and_return(branch_version) - allow(PactBroker::Api::Decorators::BranchVersionDecorator).to receive(:new).and_return(decorator) - end - - let(:branch_version) { instance_double("PactBroker::Versions::BranchVersion") } - let(:parsed_branch_version) { double("parsed branch_version") } - let(:branch_version_service) { class_double("PactBroker::Versions::Service").as_stubbed_const } - let(:path) { "/branch-versions/#{uuid}" } - let(:uuid) { "12345678" } - let(:rack_headers) do - { - "HTTP_ACCEPT" => "application/hal+json" - } - end - let(:decorator) do - instance_double("PactBroker::Api::Decorators::BranchVersionDecorator", - to_json: "response", - from_json: parsed_branch_version - ) - end - - describe "GET" do - subject { get(path, nil, rack_headers) } - - it "attempts to find the BranchVersion" do - expect(PactBroker::Versions::Service).to receive(:find_by_uuid).with(uuid) - subject - end - - context "when the branch_version does not exist" do - let(:branch_version) { nil } - - it { is_expected.to be_a_404_response } - end - - context "when the BranchVersion exists" do - it "generates a JSON representation of the BranchVersion" do - expect(PactBroker::Api::Decorators::BranchVersionDecorator).to receive(:new).with(branch_version) - expect(decorator).to receive(:to_json).with(user_options: hash_including(base_url: "http://example.org")) - subject - end - - it { is_expected.to be_a_hal_json_success_response } - - it "includes the JSON representation in the response body" do - expect(subject.body).to eq "response" - end - end - end - end - end - end -end diff --git a/spec/lib/pact_broker/contracts/service_spec.rb b/spec/lib/pact_broker/contracts/service_spec.rb index 25f8b9079..cb2ba6533 100644 --- a/spec/lib/pact_broker/contracts/service_spec.rb +++ b/spec/lib/pact_broker/contracts/service_spec.rb @@ -39,7 +39,7 @@ module Contracts it "sets the version branch" do subject - expect(PactBroker::Domain::Version.order(:id).last.branch).to eq "main" + expect(PactBroker::Domain::Version.order(:id).last.branch_names).to include "main" end it "returns a results object" do diff --git a/spec/lib/pact_broker/pacts/pact_publication_selector_dataset_module_spec.rb b/spec/lib/pact_broker/pacts/pact_publication_selector_dataset_module_spec.rb index a8f8c3f8a..bc84bc9bf 100644 --- a/spec/lib/pact_broker/pacts/pact_publication_selector_dataset_module_spec.rb +++ b/spec/lib/pact_broker/pacts/pact_publication_selector_dataset_module_spec.rb @@ -62,11 +62,11 @@ module PactPublicationSelectorDatasetModule it "returns the latest pact for the main branch of every consumer" do expect(subject.size).to eq 2 expect(subject.sort_by(&:id).first.consumer.name).to eq "Foo" - expect(subject.sort_by(&:id).first.consumer_version.branch).to eq "main" + expect(subject.sort_by(&:id).first.consumer_version.branch_names).to eq ["main"] expect(subject.sort_by(&:id).first.consumer_version.number).to eq "2" expect(subject.sort_by(&:id).last.consumer.name).to eq "Bob" - expect(subject.sort_by(&:id).last.consumer_version.branch).to eq "develop" + expect(subject.sort_by(&:id).last.consumer_version.branch_names).to eq ["develop"] expect(subject.sort_by(&:id).last.consumer_version.number).to eq "5" end end diff --git a/spec/lib/pact_broker/versions/branch_version_repository_spec.rb b/spec/lib/pact_broker/versions/branch_version_repository_spec.rb new file mode 100644 index 000000000..00939fa51 --- /dev/null +++ b/spec/lib/pact_broker/versions/branch_version_repository_spec.rb @@ -0,0 +1,71 @@ +require "pact_broker/versions/branch_version_repository" + +module PactBroker + module Versions + describe BranchVersionRepository do + describe "add_branch" do + before do + allow(repository).to receive(:pacticipant_service).and_return(pacticipant_service) + allow(pacticipant_service).to receive(:maybe_set_main_branch) + end + let!(:version) { td.create_consumer("Foo").create_consumer_version("1", branch: "original-branch").and_return(:consumer_version) } + let(:new_branch_name) { "new-branch" } + let(:pacticipant_service) { class_double("PactBroker::Pacticipants::Service").as_stubbed_const } + let(:repository) { BranchVersionRepository.new } + + subject { repository.add_branch(version, new_branch_name).version.refresh } + + it "calls the pacticipant_service.maybe_set_main_branch" do + expect(pacticipant_service).to receive(:maybe_set_main_branch).with(instance_of(PactBroker::Domain::Pacticipant), new_branch_name) + subject + end + + context "when the branch does not already exist" do + it "creates a branch" do + expect { subject }.to change { PactBroker::Versions::Branch.count }.by(1) + end + + it "creates a branch_version" do + expect { subject }.to change { PactBroker::Versions::BranchVersion.count }.by(1) + end + + it "adds the branch_version to the version" do + expect(subject.branch_versions.count).to eq 2 + expect(subject.branch_versions.last.branch_name).to eq "new-branch" + end + + it "updates the branch head" do + branch_head = subject.pacticipant.branch_head_for("new-branch") + expect(branch_head.version.id).to eq subject.refresh.id + end + end + + context "when the branch and branch version do already exist" do + let(:new_branch_name) { "original-branch" } + + it "does not creates a branch" do + expect { subject }.to_not change { PactBroker::Versions::Branch.order(:name).collect(&:name) } + end + + it "does not create a branch_version" do + expect { subject }.to change { PactBroker::Versions::BranchVersion.count }.by(0) + end + + it "keeps the branch_version on the version" do + expect(subject.branch_versions.count).to eq 1 + expect(subject.branch_versions.first.branch_name).to eq "original-branch" + end + + it "does not change the branch head" do + branch_head = subject.pacticipant.branch_head_for("original-branch") + expect(branch_head.version).to eq subject + end + end + end + + + end + end +end + + diff --git a/spec/lib/pact_broker/versions/repository_spec.rb b/spec/lib/pact_broker/versions/repository_spec.rb index 670c8d994..e57bc401d 100644 --- a/spec/lib/pact_broker/versions/repository_spec.rb +++ b/spec/lib/pact_broker/versions/repository_spec.rb @@ -159,59 +159,20 @@ module Versions let(:pacticipant) { td.and_return(:consumer) } let(:version_number) { "1234" } let(:tags) { nil } - let(:open_struct_version) { OpenStruct.new(branch: new_branch, tags: tags) } - let(:new_branch) { "new-branch" } + let(:open_struct_version) { OpenStruct.new(build_url: new_build_url, tags: tags) } + let(:new_build_url) { "new-build-url" } subject { Repository.new.create_or_update(pacticipant, version_number, open_struct_version) } - it "does not overwrite missing values the values" do - expect(subject.branch).to eq "new-branch" - expect(subject.build_url).to eq "original-build-url" - end - - it "does not change the tags" do - expect { subject }.to_not change { PactBroker::Domain::Version.for("Foo", "1234").tags } - end - - context "when the branch does not already exist" do - it "creates a branch" do - expect { subject }.to change { PactBroker::Versions::Branch.count }.by(1) - end - - it "creates a branch_version" do - expect { subject }.to change { PactBroker::Versions::BranchVersion.count }.by(1) - end - - it "adds the branch_version to the version" do - expect(subject.branch_versions.count).to eq 2 - expect(subject.branch_versions.last.branch_name).to eq "new-branch" - end - - it "updates the branch head" do - branch_head = subject.pacticipant.branch_head_for("new-branch") - expect(branch_head.version.id).to eq subject.refresh.id - end - end - - context "when the branch and branch version do already exist" do - let(:new_branch) { "original-branch" } - - it "does not creates a branch" do - expect { subject }.to_not change { PactBroker::Versions::Branch.order(:name).collect(&:name) } - end - - it "does not create a branch_version" do - expect { subject }.to change { PactBroker::Versions::BranchVersion.count }.by(0) - end + context "with empty properties" do + let(:open_struct_version) { OpenStruct.new } - it "keeps the branch_version on the version" do - expect(subject.branch_versions.count).to eq 1 - expect(subject.branch_versions.first.branch_name).to eq "original-branch" + it "does not overwrite missing values the values" do + expect(subject.build_url).to eq "original-build-url" end - it "does not change the branch head" do - branch_head = subject.pacticipant.branch_head_for("original-branch") - expect(branch_head.version).to eq subject + it "does not change the tags" do + expect { subject }.to_not change { PactBroker::Domain::Version.for("Foo", "1234").tags } end end @@ -256,12 +217,12 @@ module Versions let(:pacticipant) { td.and_return(:consumer) } let(:version_number) { "1234" } let(:tags) { nil } - let(:open_struct_version) { OpenStruct.new(branch: "new-branch", tags: tags) } + let(:open_struct_version) { OpenStruct.new(tags: tags) } subject { Repository.new.create_or_overwrite(pacticipant, version_number, open_struct_version) } it "overwrites the values" do - expect(subject.branch).to eq "new-branch" + expect(subject.branch_names).to eq ["original-branch"] expect(subject.build_url).to eq nil end diff --git a/spec/lib/pact_broker/versions/service_spec.rb b/spec/lib/pact_broker/versions/service_spec.rb index f0b325e4b..4136182bf 100644 --- a/spec/lib/pact_broker/versions/service_spec.rb +++ b/spec/lib/pact_broker/versions/service_spec.rb @@ -37,7 +37,7 @@ module Versions it "does not update the branch" do subject - expect(td.find_version(pacticipant_name, version_number).branch).to eq "foo" + expect(td.find_version(pacticipant_name, version_number).branch_names).to eq ["foo"] end end @@ -77,7 +77,8 @@ module Versions context "when the version was created within the limit" do before do - version = td.create_consumer(pacticipant_name) + version = td + .create_consumer(pacticipant_name) .create_consumer_version(version_number) .and_return(:consumer_version) @@ -91,7 +92,7 @@ module Versions it "sets the branch" do subject - expect(td.find_version(pacticipant_name, version_number).branch).to eq "prod" + expect(td.find_version(pacticipant_name, version_number).branch_names).to include "prod" end end end From 9b2316215da6168c8a75ed0258bd288c0edb47f3 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 2 Sep 2021 16:10:30 +1000 Subject: [PATCH 21/45] chore: update index and dashboard --- .../api/decorators/dashboard_decorator.rb | 6 +++-- .../api/decorators/matrix_decorator.rb | 13 +++++++++-- lib/pact_broker/api/resources/version.rb | 8 ------- lib/pact_broker/domain/index_item.rb | 8 ------- lib/pact_broker/matrix/quick_row.rb | 8 ------- lib/pact_broker/pacts/pact_publication.rb | 21 ++--------------- .../ui/views/index/show-with-tags.haml | 7 +++--- lib/pact_broker/versions/repository.rb | 3 +-- lib/pact_broker/versions/service.rb | 17 -------------- spec/fixtures/dashboard.json | 6 +++-- .../decorators/dashboard_decorator_spec.rb | 14 +++++------ .../api/decorators/matrix_decorator_spec.rb | 23 +++++++++++++++---- spec/lib/sequel/plugins/upsert_spec.rb | 16 +++++++++---- 13 files changed, 62 insertions(+), 88 deletions(-) diff --git a/lib/pact_broker/api/decorators/dashboard_decorator.rb b/lib/pact_broker/api/decorators/dashboard_decorator.rb index e5afe96cf..195158e67 100644 --- a/lib/pact_broker/api/decorators/dashboard_decorator.rb +++ b/lib/pact_broker/api/decorators/dashboard_decorator.rb @@ -58,7 +58,8 @@ def consumer_hash(index_item, _consumer, _consumer_version, base_url) name: index_item.consumer_name, version: { number: index_item.consumer_version_number, - branch: index_item.consumer_version_branch, + branch: index_item.consumer_version_branches.last, + headBranchNames: index_item.consumer_version_branches, _links: { self: { href: version_url(base_url, index_item.consumer_version) @@ -87,7 +88,8 @@ def provider_hash(index_item, _provider, base_url) if index_item.latest_verification hash[:version] = { number: index_item.provider_version_number, - branch: index_item.provider_version_branch + branch: index_item.provider_version_branches.last, + headBranchNames: index_item.provider_version_branches } end diff --git a/lib/pact_broker/api/decorators/matrix_decorator.rb b/lib/pact_broker/api/decorators/matrix_decorator.rb index 60e2bf6a5..3fcb450ee 100644 --- a/lib/pact_broker/api/decorators/matrix_decorator.rb +++ b/lib/pact_broker/api/decorators/matrix_decorator.rb @@ -2,6 +2,7 @@ require "pact_broker/api/pact_broker_urls" require "pact_broker/api/decorators/reason_decorator" require "pact_broker/api/decorators/format_date_time" +require "pact_broker/api/decorators/embedded_branch_version_decorator" module PactBroker module Api @@ -83,7 +84,8 @@ def consumer_hash(line, consumer, consumer_version, base_url) name: line.consumer_name, version: { number: line.consumer_version_number, - branch: line.consumer_version_branch, + branch: line.consumer_version_branch_versions.last&.branch_name, + branches: branches(line.consumer_version_branch_versions, base_url), _links: { self: { href: version_url(base_url, consumer_version) @@ -99,6 +101,12 @@ def consumer_hash(line, consumer, consumer_version, base_url) } end + def branches(branch_versions, base_url) + branch_versions.collect do | branch_version | + PactBroker::Api::Decorators::EmbeddedBranchVersionDecorator.new(branch_version).to_hash(user_options: { base_url: base_url }) + end + end + def tags(tags, base_url) tags.sort_by(&:created_at).collect do | tag | { @@ -127,7 +135,8 @@ def provider_hash(line, provider, provider_version, base_url) if !line.provider_version_number.nil? hash[:version] = { number: line.provider_version_number, - branch: line.provider_version_branch, + branch: line.provider_version_branch_versions.last&.branch_name, + branches: branches(line.provider_version_branch_versions, base_url), _links: { self: { href: version_url(base_url, provider_version) diff --git a/lib/pact_broker/api/resources/version.rb b/lib/pact_broker/api/resources/version.rb index 5732e1961..cf23c8069 100644 --- a/lib/pact_broker/api/resources/version.rb +++ b/lib/pact_broker/api/resources/version.rb @@ -21,14 +21,6 @@ def allowed_methods ["GET", "PUT", "PATCH", "DELETE", "OPTIONS"] end - def is_conflict? - if (errors = version_service.conflict_errors(version, parsed_version, resource_url)).any? - set_json_validation_error_messages(errors) - else - false - end - end - def resource_exists? !!version end diff --git a/lib/pact_broker/domain/index_item.rb b/lib/pact_broker/domain/index_item.rb index 6815ab783..6bb6d0a8f 100644 --- a/lib/pact_broker/domain/index_item.rb +++ b/lib/pact_broker/domain/index_item.rb @@ -70,10 +70,6 @@ def consumer_version_order consumer_version.order end - def consumer_version_branch - consumer_version.branch - end - def consumer_version_branches consumer_version.branch_heads.collect(&:branch_name) end @@ -94,10 +90,6 @@ def provider_version_number @latest_verification ? @latest_verification.provider_version_number : nil end - def provider_version_branch - provider_version&.branch - end - def provider_version_branches provider_version&.branch_heads&.collect(&:branch_name) || [] end diff --git a/lib/pact_broker/matrix/quick_row.rb b/lib/pact_broker/matrix/quick_row.rb index fe4c89a67..20fc96ac5 100644 --- a/lib/pact_broker/matrix/quick_row.rb +++ b/lib/pact_broker/matrix/quick_row.rb @@ -358,10 +358,6 @@ def consumer_version_number consumer_version.number end - def consumer_version_branch - consumer_version.branch - end - def consumer_version_branch_versions consumer_version.branch_versions end @@ -378,10 +374,6 @@ def provider_version_number provider_version&.number end - def provider_version_branch - provider_version&.branch - end - def provider_version_branch_versions provider_version&.branch_versions || [] end diff --git a/lib/pact_broker/pacts/pact_publication.rb b/lib/pact_broker/pacts/pact_publication.rb index 13e9fe453..68341f2e3 100644 --- a/lib/pact_broker/pacts/pact_publication.rb +++ b/lib/pact_broker/pacts/pact_publication.rb @@ -129,28 +129,11 @@ def to_domain_lightweight # Think we really could just use the version here. def to_version_domain - OpenStruct.new( - id: consumer_version.id, - number: consumer_version.number, - pacticipant: consumer, - tags: consumer_version.tags, - order: consumer_version.order, - branch: consumer_version.branch, - current_deployed_versions: consumer_version.current_deployed_versions, - current_supported_released_versions: consumer_version.current_supported_released_versions - ) + consumer_version end def to_version_domain_lightweight - OpenStruct.new( - id: consumer_version.id, - number: consumer_version.number, - pacticipant: consumer, - order: consumer_version.order, - branch: consumer_version.branch, - current_deployed_versions: consumer_version.associations[:current_deployed_versions], - current_supported_released_versions: consumer_version.associations[:current_supported_released_versions], - ) + consumer_version end def to_domain_with_content diff --git a/lib/pact_broker/ui/views/index/show-with-tags.haml b/lib/pact_broker/ui/views/index/show-with-tags.haml index 73331fe29..49693f41b 100644 --- a/lib/pact_broker/ui/views/index/show-with-tags.haml +++ b/lib/pact_broker/ui/views/index/show-with-tags.haml @@ -84,10 +84,9 @@ - if index_item.display_provider_version_number %button.clippy.invisible{ title: "Copy to clipboard" } %span.copy-icon - - if index_item.provider_version_branch - - branch_class = "tag badge badge-dark" - %div{"class": branch_class} - = "branch: " + index_item.provider_version_branch + - index_item.provider_version_branches.each do | branch_name | + %div{"class": "tag badge badge-dark"} + = "branch: " + branch_name - index_item.provider_version_latest_tag_names.each do | tag_name | .tag.badge.badge-primary = "tag: " + tag_name diff --git a/lib/pact_broker/versions/repository.rb b/lib/pact_broker/versions/repository.rb index ff66507dd..18c82c3a1 100644 --- a/lib/pact_broker/versions/repository.rb +++ b/lib/pact_broker/versions/repository.rb @@ -97,8 +97,7 @@ def create_or_overwrite(pacticipant, version_number, open_struct_version) saved_version = PactBroker::Domain::Version.new( number: version_number, pacticipant: pacticipant, - build_url: open_struct_version.build_url, - branch: open_struct_version.branch + build_url: open_struct_version.build_url ).upsert if open_struct_version.tags diff --git a/lib/pact_broker/versions/service.rb b/lib/pact_broker/versions/service.rb index 67a473d1e..34a68bbd7 100644 --- a/lib/pact_broker/versions/service.rb +++ b/lib/pact_broker/versions/service.rb @@ -10,23 +10,6 @@ class Service extend PactBroker::Services include PactBroker::Logging - def self.conflict_errors(_existing_version, _open_struct_version, _version_url) - # This validation is causing problems in the PF build when branches are merged - # TODO remove this properly when re-doing the version -> branch relationship - {} - # if existing_version&.branch && open_struct_version.to_h.key?(:branch) && existing_version.branch != open_struct_version.branch - # message_params = { - # old_branch: existing_version&.branch, - # new_branch: open_struct_version.branch, - # version_url: version_url - # } - # error_message = message("errors.validation.cannot_modify_version_branch", message_params) - # { branch: [error_message] } - # else - # {} - # end - end - def self.find_latest_by_pacticpant_name params version_repository.find_latest_by_pacticpant_name params.fetch(:pacticipant_name) end diff --git a/spec/fixtures/dashboard.json b/spec/fixtures/dashboard.json index 01b812ead..f3f319ec2 100644 --- a/spec/fixtures/dashboard.json +++ b/spec/fixtures/dashboard.json @@ -21,7 +21,8 @@ } }, "number": "1", - "branch": "main" + "branch": "main", + "headBranchNames": ["main"] } }, "latestVerificationResult": { @@ -75,7 +76,8 @@ "name": "Bar", "version": { "number": "2", - "branch": "main" + "branch": "main", + "headBranchNames": ["main"] } }, "verificationStatus": "wiffle", diff --git a/spec/lib/pact_broker/api/decorators/dashboard_decorator_spec.rb b/spec/lib/pact_broker/api/decorators/dashboard_decorator_spec.rb index 4734bdfb7..e4780dc41 100644 --- a/spec/lib/pact_broker/api/decorators/dashboard_decorator_spec.rb +++ b/spec/lib/pact_broker/api/decorators/dashboard_decorator_spec.rb @@ -21,9 +21,9 @@ module Decorators webhook_status: "blah", pseudo_branch_verification_status: "wiffle", provider_version_number: provider_version.number, - provider_version_branch: provider_version.branch, + provider_version_branches: ["main"], consumer_version_number: consumer_version.number, - consumer_version_branch: consumer_version.branch, + consumer_version_branches: ["main"], tag_names: ["prod"], latest_verification_latest_tags: [double("tag", name: "dev", latest?: true)] ) @@ -32,8 +32,8 @@ module Decorators let(:provider) { instance_double("PactBroker::Domain::Pacticipant", name: "Bar") } let(:pact) { instance_double("PactBroker::Domain::Pact", created_at: created_at) } let(:verification) { instance_double("PactBroker::Domain::Verification", success: true, created_at: created_at) } - let(:consumer_version) { instance_double("PactBroker::Domain::Version", number: "1", pacticipant: consumer, branch: "main") } - let(:provider_version) { instance_double("PactBroker::Domain::Version", number: "2", pacticipant: provider, branch: "main") } + let(:consumer_version) { instance_double("PactBroker::Domain::Version", number: "1", pacticipant: consumer) } + let(:provider_version) { instance_double("PactBroker::Domain::Version", number: "2", pacticipant: provider) } let(:last_webhook_execution_date) { created_at } let(:base_url) { "http://example.org" } let(:options) { { user_options: { base_url: base_url } } } @@ -67,7 +67,7 @@ module Decorators subject { JSON.parse(dashboard_json) } it "creates some json" do - expect(subject).to match_pact(expected_hash, {allow_unexpected_keys: false}) + expect(subject).to match_pact(expected_hash, { allow_unexpected_keys: false }) end context "when the pact has never been verified" do @@ -76,7 +76,7 @@ module Decorators it "has a null last verification and provider version" do expected_hash["items"][0]["latestVerificationResult"] = nil expected_hash["items"][0]["provider"]["version"] = nil - expect(subject).to match_pact(expected_hash, {allow_unexpected_keys: false}) + expect(subject).to match_pact(expected_hash, { allow_unexpected_keys: false }) end end @@ -85,7 +85,7 @@ module Decorators it "has a null latestWebhookExecution" do expected_hash["items"][0]["latestWebhookExecution"] = nil - expect(subject).to match_pact(expected_hash, {allow_unexpected_keys: false}) + expect(subject).to match_pact(expected_hash, { allow_unexpected_keys: false }) end end end diff --git a/spec/lib/pact_broker/api/decorators/matrix_decorator_spec.rb b/spec/lib/pact_broker/api/decorators/matrix_decorator_spec.rb index da8408bee..c38128edf 100644 --- a/spec/lib/pact_broker/api/decorators/matrix_decorator_spec.rb +++ b/spec/lib/pact_broker/api/decorators/matrix_decorator_spec.rb @@ -10,6 +10,7 @@ module Decorators describe "to_json" do before do allow_any_instance_of(ReasonDecorator).to receive(:to_s).and_return("foo") + allow_any_instance_of(PactBroker::Api::PactBrokerUrls).to receive(:branch_version_url).and_return("branch_version_url") end let(:verification_date) { DateTime.new(2017, 12, 31) } let(:pact_created_at) { DateTime.new(2017, 1, 1) } @@ -20,13 +21,13 @@ module Decorators { consumer_name: "Consumer", consumer_version_number: "1.0.0", - consumer_version_branch: "main", + consumer_version_branch_versions: consumer_version_branch_versions, consumer_version_tags: consumer_version_tags, provider_version_tags: provider_version_tags, pact_version_sha: "1234", pact_created_at: pact_created_at, provider_version_number: "4.5.6", - provider_version_branch: "feat/x", + provider_version_branch_versions: provider_version_branch_versions, provider_name: "Provider", success: row_1_success, verification_number: 1, @@ -40,12 +41,12 @@ module Decorators { consumer_name: "Consumer", consumer_version_number: "1.0.0", - consumer_version_branch: "main", + consumer_version_branch_versions: [], consumer_version_tags: [], pact_version_sha: "1234", pact_created_at: pact_created_at, provider_version_number: nil, - provider_version_branch: nil, + provider_version_branch_versions: [], provider_name: "Provider", success: row_2_success, verification_number: nil, @@ -65,6 +66,12 @@ module Decorators version: { number: "1.0.0", branch: "main", + branches: [ + name: "main", + _links: { + + } + ], _links: { self: { href: "http://example.org/pacticipants/Consumer/versions/1.0.0" @@ -141,6 +148,10 @@ module Decorators let(:consumer_version) { double("consumer version", number: "1.0.0", pacticipant: double("consumer", name: "Consumer")) } + let(:consumer_version_branch_versions) do + [ instance_double("PactBroker::Versions::BranchVersion", branch_name: "main") ] + end + let(:consumer_version_tags) do [ double("tag", name: "prod", latest?: true, version: consumer_version, created_at: DateTime.now ) @@ -149,6 +160,10 @@ module Decorators let(:provider_version) { double("provider version", number: "4.5.6", pacticipant: double("provider", name: "Provider")) } + let(:provider_version_branch_versions) do + [ instance_double("PactBroker::Versions::BranchVersion", branch_name: "feat/x") ] + end + let(:provider_version_tags) do [ double("tag", name: "master", latest?: false, version: provider_version, created_at: DateTime.now) diff --git a/spec/lib/sequel/plugins/upsert_spec.rb b/spec/lib/sequel/plugins/upsert_spec.rb index a28cff16f..61869afe2 100644 --- a/spec/lib/sequel/plugins/upsert_spec.rb +++ b/spec/lib/sequel/plugins/upsert_spec.rb @@ -112,7 +112,6 @@ class LatestPactPublicationIdForConsumerVersion < Sequel::Model(:latest_pact_pub version = Version.new( number: "1", pacticipant_id: pacticipant.id, - branch: "original-branch", build_url: "original-url" ).upsert Version.where(id: version.id).update(created_at: yesterday, updated_at: yesterday) @@ -121,7 +120,7 @@ class LatestPactPublicationIdForConsumerVersion < Sequel::Model(:latest_pact_pub let(:yesterday) { DateTime.now - 2 } subject do - Version.new(number: "1", pacticipant_id: pacticipant.id, branch: "new-branch").upsert + Version.new(number: "1", pacticipant_id: pacticipant.id, build_url: "new-url").upsert end it "does not raise an error" do @@ -129,13 +128,20 @@ class LatestPactPublicationIdForConsumerVersion < Sequel::Model(:latest_pact_pub end it "sets the values on the object" do - expect(subject.branch).to eq "new-branch" + expect(subject.build_url).to eq "new-url" end - it "nils out values that weren't set on the second model" do - expect(subject.build_url).to eq nil + context "when an attribute is not set" do + subject do + Version.new(number: "1", pacticipant_id: pacticipant.id).upsert + end + + it "nils out values that weren't set on the second model" do + expect(subject.build_url).to eq nil + end end + it "does not insert another row" do expect { subject }.to_not change { Version.count } end From 2e621e36e5f28fa9b903f61a518e3469c13fb3c4 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 2 Sep 2021 16:28:45 +1000 Subject: [PATCH 22/45] chore: remove domain object usage from migration specs --- ...4_add_provider_version_to_verification_spec.rb | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/spec/migrations/44_add_provider_version_to_verification_spec.rb b/spec/migrations/44_add_provider_version_to_verification_spec.rb index 72877e346..9ddd475d5 100644 --- a/spec/migrations/44_add_provider_version_to_verification_spec.rb +++ b/spec/migrations/44_add_provider_version_to_verification_spec.rb @@ -31,29 +31,26 @@ subject { PactBroker::Database.migrate(46) } it "creates a version object" do - expect { subject }.to change { PactBroker::Domain::Version.count }.by(1) + expect { subject }.to change { database[:versions].count }.by(1) end it "sets the foreign key to the new version" do subject - expect(PactBroker::Domain::Verification.first.provider_version.number).to eq "1.2.3" - end - - it "sets the created_at date of the new version to the created_at of the verification" do - subject - expect(PactBroker::Domain::Verification.first.provider_version.created_at.to_datetime).to eq now + provider_version_id = database[:verifications].order(:id).first[:provider_version_id] + expect(database[:versions].where(id: provider_version_id).single_record[:number]).to eq "1.2.3" end context "when the version already exists" do let!(:provider_version) { create(:versions, {number: "1.2.3", order: 1, pacticipant_id: provider[:id], created_at: now, updated_at: now}) } it "does not create a version object" do - expect { subject }.to_not change { PactBroker::Domain::Version.count } + expect { subject }.to_not change { database[:versions].count } end it "sets the foreign key to the existing version" do subject - expect(PactBroker::Domain::Verification.first.provider_version.number).to eq "1.2.3" + provider_version_id = database[:verifications].order(:id).first[:provider_version_id] + expect(database[:versions].where(id: provider_version_id).single_record[:number]).to eq "1.2.3" end end end From 58c82cb216f9fcef421a5cc4fdc78510581c9b12 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 2 Sep 2021 16:31:23 +1000 Subject: [PATCH 23/45] style: rubocop --- lib/pact_broker/api/resources/branch_version.rb | 4 ++-- spec/lib/pact_broker/versions/branch_service_spec.rb | 2 +- spec/lib/pact_broker/versions/branch_version_spec.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/pact_broker/api/resources/branch_version.rb b/lib/pact_broker/api/resources/branch_version.rb index e51d609d4..f595d9da8 100644 --- a/lib/pact_broker/api/resources/branch_version.rb +++ b/lib/pact_broker/api/resources/branch_version.rb @@ -1,5 +1,5 @@ -require 'pact_broker/api/resources/base_resource' -require 'pact_broker/api/decorators/branch_version_decorator' +require "pact_broker/api/resources/base_resource" +require "pact_broker/api/decorators/branch_version_decorator" module PactBroker module Api diff --git a/spec/lib/pact_broker/versions/branch_service_spec.rb b/spec/lib/pact_broker/versions/branch_service_spec.rb index 20cae7ceb..33a6cb6bf 100644 --- a/spec/lib/pact_broker/versions/branch_service_spec.rb +++ b/spec/lib/pact_broker/versions/branch_service_spec.rb @@ -1,4 +1,4 @@ -require 'pact_broker/versions/branch_service' +require "pact_broker/versions/branch_service" module PactBroker module Versions diff --git a/spec/lib/pact_broker/versions/branch_version_spec.rb b/spec/lib/pact_broker/versions/branch_version_spec.rb index a005f5d1f..ecd77cd6a 100644 --- a/spec/lib/pact_broker/versions/branch_version_spec.rb +++ b/spec/lib/pact_broker/versions/branch_version_spec.rb @@ -1,4 +1,4 @@ -require 'pact_broker/versions/branch_version' +require "pact_broker/versions/branch_version" module PactBroker module Versions From 27630f9cd9636fffa45d9ff41f600f6f72a96e37 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 2 Sep 2021 16:33:36 +1000 Subject: [PATCH 24/45] style: rubocop --- lib/pact_broker/api/resources/branch_version.rb | 2 -- lib/pact_broker/index/service.rb | 2 ++ lib/pact_broker/test/http_test_data_builder.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/pact_broker/api/resources/branch_version.rb b/lib/pact_broker/api/resources/branch_version.rb index f595d9da8..c025c2917 100644 --- a/lib/pact_broker/api/resources/branch_version.rb +++ b/lib/pact_broker/api/resources/branch_version.rb @@ -39,8 +39,6 @@ def policy_name private - attr_reader :branch_version - def branch_version @branch_version ||= branch_service.find_branch_version(identifier_from_path) end diff --git a/lib/pact_broker/index/service.rb b/lib/pact_broker/index/service.rb index b4455b62c..9736121ba 100644 --- a/lib/pact_broker/index/service.rb +++ b/lib/pact_broker/index/service.rb @@ -31,6 +31,7 @@ def self.find_all_index_items end # rubocop: disable Metrics/CyclomaticComplexity + # rubocop: disable Metrics/MethodLength def self.find_index_items options = {} latest_verifications_for_cv_tags = latest_verifications_for_consumer_version_tags(options) latest_pp_ids = latest_pact_publication_ids @@ -72,6 +73,7 @@ def self.find_index_items options = {} Page.new(index_items, pagination_record_count) end + # rubocop: enable Metrics/MethodLength # rubocop: enable Metrics/CyclomaticComplexity # Worst. Code. Ever. diff --git a/lib/pact_broker/test/http_test_data_builder.rb b/lib/pact_broker/test/http_test_data_builder.rb index 27db57ddb..ef359f163 100644 --- a/lib/pact_broker/test/http_test_data_builder.rb +++ b/lib/pact_broker/test/http_test_data_builder.rb @@ -119,7 +119,7 @@ def publish_contract(consumer: last_consumer_name, consumer_version:, provider: } ] }.compact - response = client.post("contracts/publish", request_body_hash).tap { |response| check_for_error(response) } + response = client.post("contracts/publish", request_body_hash).tap { |resp| check_for_error(resp) } puts response.body["logs"].collect{ |log| log["message"]} separate self From 853729696dba7d0829749358ed5defa8d9d2593f Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 2 Sep 2021 18:23:31 +1000 Subject: [PATCH 25/45] chore: pass in branch for version creation --- lib/pact_broker/contracts/service.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pact_broker/contracts/service.rb b/lib/pact_broker/contracts/service.rb index 4e8afa0f4..5662d3183 100644 --- a/lib/pact_broker/contracts/service.rb +++ b/lib/pact_broker/contracts/service.rb @@ -45,7 +45,8 @@ def publish(parsed_contracts, base_url: ) def create_version(parsed_contracts) version_params = { - build_url: parsed_contracts.build_url + build_url: parsed_contracts.build_url, + branch: parsed_contracts.branch }.compact existing_version = find_existing_version(parsed_contracts) @@ -70,7 +71,6 @@ def create_or_update_version(parsed_contracts, version_params, branch_name) parsed_contracts.pacticipant_version_number, OpenStruct.new(version_params) ) - branch_version_repository.add_branch(version, branch_name) if branch_name version end From b90ee3a2209634debeee8935c155ac40b5f58613 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 2 Sep 2021 18:25:22 +1000 Subject: [PATCH 26/45] refactor: remove unused param --- lib/pact_broker/contracts/service.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pact_broker/contracts/service.rb b/lib/pact_broker/contracts/service.rb index 5662d3183..faeb79c48 100644 --- a/lib/pact_broker/contracts/service.rb +++ b/lib/pact_broker/contracts/service.rb @@ -50,7 +50,7 @@ def create_version(parsed_contracts) }.compact existing_version = find_existing_version(parsed_contracts) - version = create_or_update_version(parsed_contracts, version_params, parsed_contracts.branch) + version = create_or_update_version(parsed_contracts, version_params) return version, notices_for_version_creation(existing_version, parsed_contracts) end @@ -65,7 +65,7 @@ def find_existing_version(parsed_contracts) private :find_existing_version - def create_or_update_version(parsed_contracts, version_params, branch_name) + def create_or_update_version(parsed_contracts, version_params) version = version_service.create_or_update( parsed_contracts.pacticipant_name, parsed_contracts.pacticipant_version_number, From e3848af917056e9d1d844dccbe9827eb92285192 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 2 Sep 2021 18:26:05 +1000 Subject: [PATCH 27/45] style: rubocop --- lib/pact_broker/contracts/service.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/pact_broker/contracts/service.rb b/lib/pact_broker/contracts/service.rb index faeb79c48..4e85209a6 100644 --- a/lib/pact_broker/contracts/service.rb +++ b/lib/pact_broker/contracts/service.rb @@ -66,12 +66,11 @@ def find_existing_version(parsed_contracts) private :find_existing_version def create_or_update_version(parsed_contracts, version_params) - version = version_service.create_or_update( + version_service.create_or_update( parsed_contracts.pacticipant_name, parsed_contracts.pacticipant_version_number, OpenStruct.new(version_params) ) - version end private :create_or_update_version From d62cbeddd3ebe9e2988fc6289972947904afa0ae Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 2 Sep 2021 18:26:46 +1000 Subject: [PATCH 28/45] chore: remove unused require --- lib/pact_broker/contracts/service.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/pact_broker/contracts/service.rb b/lib/pact_broker/contracts/service.rb index 4e85209a6..603e6a561 100644 --- a/lib/pact_broker/contracts/service.rb +++ b/lib/pact_broker/contracts/service.rb @@ -6,7 +6,6 @@ require "pact_broker/contracts/notice" require "pact_broker/events/subscriber" require "pact_broker/api/pact_broker_urls" -require "pact_broker/versions/branch_version_repository" module PactBroker module Contracts From 69696cdf8bae362eb4db03299f24dfe9f8304d4e Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 2 Sep 2021 18:56:24 +1000 Subject: [PATCH 29/45] chore: add latest property to branch version decorator --- .../api/decorators/embedded_branch_version_decorator.rb | 1 + spec/lib/pact_broker/api/decorators/version_decorator_spec.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/pact_broker/api/decorators/embedded_branch_version_decorator.rb b/lib/pact_broker/api/decorators/embedded_branch_version_decorator.rb index 0be01294e..68a84595f 100644 --- a/lib/pact_broker/api/decorators/embedded_branch_version_decorator.rb +++ b/lib/pact_broker/api/decorators/embedded_branch_version_decorator.rb @@ -6,6 +6,7 @@ module Api module Decorators class EmbeddedBranchVersionDecorator < BaseDecorator property :branch_name, as: :name + property :latest?, as: :latest link :self do | options | { diff --git a/spec/lib/pact_broker/api/decorators/version_decorator_spec.rb b/spec/lib/pact_broker/api/decorators/version_decorator_spec.rb index ead888436..5e586f1ed 100644 --- a/spec/lib/pact_broker/api/decorators/version_decorator_spec.rb +++ b/spec/lib/pact_broker/api/decorators/version_decorator_spec.rb @@ -86,6 +86,7 @@ module Decorators it "includes the branches" do expect(subject[:_embedded][:branches]).to be_instance_of(Array) expect(subject[:_embedded][:branches].first[:name]).to eq "main" + expect(subject[:_embedded][:branches].first[:latest]).to eq true end it "includes the timestamps" do From 3c4cf5c98dfc2371ac8624106b8ab3b92444fa21 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 2 Sep 2021 18:56:38 +1000 Subject: [PATCH 30/45] chore: update indexes --- db/migrations/20210816_create_branches_tables.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/db/migrations/20210816_create_branches_tables.rb b/db/migrations/20210816_create_branches_tables.rb index 936c4ad4d..53f4d1920 100644 --- a/db/migrations/20210816_create_branches_tables.rb +++ b/db/migrations/20210816_create_branches_tables.rb @@ -4,11 +4,9 @@ primary_key :id String :name foreign_key :pacticipant_id, :pacticipants, null: false, on_delete: :cascade - Integer :latest_branch_version_id - Integer :latest_version_id DateTime :created_at, null: false DateTime :updated_at, null: false - index [:name, :pacticipant_id], unique: true, name: :branches_name_pacticipant_id_index + index [:pacticipant_id, :name], unique: true, name: :branches_pacticipant_id_name_index end create_table(:branch_versions, charset: "utf8") do @@ -21,8 +19,9 @@ DateTime :created_at, null: false DateTime :updated_at, null: false index [:branch_id, :version_id], unique: true, name: :branch_versions_branch_id_version_id_index - index [:pacticipant_id, :branch_id, :version_order], name: :branch_versions_pacticipant_id_branch_id_version_order_index index [:branch_name], name: :branch_versions_branch_name_index + # Can probably drop this index when the "latest pact" logic changes + index [:pacticipant_id, :branch_id, :version_order], name: :branch_versions_pacticipant_id_branch_id_version_order_index end create_table(:branch_heads) do @@ -34,6 +33,7 @@ String :branch_name, null: false index [:branch_id], unique: true, name: :branch_heads_branch_id_index index [:branch_name], name: :branch_heads_branch_name_index + index [:pacticipant_id], name: :branch_heads_pacticipant_id_index end end end From 7c3a8a326b8a861db2b4b53531beb1ed5cc368d0 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 2 Sep 2021 19:06:08 +1000 Subject: [PATCH 31/45] chore: update latest by branch query --- lib/pact_broker/pacts/pact_publication_dataset_module.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pact_broker/pacts/pact_publication_dataset_module.rb b/lib/pact_broker/pacts/pact_publication_dataset_module.rb index 5471f9ed4..62727dd36 100644 --- a/lib/pact_broker/pacts/pact_publication_dataset_module.rb +++ b/lib/pact_broker/pacts/pact_publication_dataset_module.rb @@ -53,7 +53,7 @@ def latest_by_consumer_branch max_orders = join(:branch_versions, branch_versions_join) .join(:branches, branches_join) - .select_group(Sequel[:pact_publications][:consumer_id], Sequel[:branches][:name].as(:branch_name)) + .select_group(Sequel[:branches][:pacticipant_id].as(:consumer_id), Sequel[:branches][:name].as(:branch_name)) .select_append{ max(consumer_version_order).as(latest_consumer_version_order) } max_join = { From fc3aa06923a5447c7740b428fa574c49ed21a9ca Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 2 Sep 2021 19:38:21 +1000 Subject: [PATCH 32/45] chore: refactor --- .../20210816_create_branches_tables.rb | 1 + .../pacts/pact_publication_dataset_module.rb | 8 +++--- ...act_publication_selector_dataset_module.rb | 27 ++++++++++++++++++- .../pacts_for_verification_repository.rb | 6 ++--- .../api/decorators/matrix_decorator_spec.rb | 4 +-- .../pact_publication_dataset_module_spec.rb | 16 +++++------ ...ublication_selector_dataset_module_spec.rb | 5 ++-- .../pacts/pact_publication_spec.rb | 10 +++---- 8 files changed, 53 insertions(+), 24 deletions(-) diff --git a/db/migrations/20210816_create_branches_tables.rb b/db/migrations/20210816_create_branches_tables.rb index 53f4d1920..e41b9832a 100644 --- a/db/migrations/20210816_create_branches_tables.rb +++ b/db/migrations/20210816_create_branches_tables.rb @@ -34,6 +34,7 @@ index [:branch_id], unique: true, name: :branch_heads_branch_id_index index [:branch_name], name: :branch_heads_branch_name_index index [:pacticipant_id], name: :branch_heads_pacticipant_id_index + index [:version_id], name: :branch_heads_version_id_index end end end diff --git a/lib/pact_broker/pacts/pact_publication_dataset_module.rb b/lib/pact_broker/pacts/pact_publication_dataset_module.rb index 62727dd36..926dbec65 100644 --- a/lib/pact_broker/pacts/pact_publication_dataset_module.rb +++ b/lib/pact_broker/pacts/pact_publication_dataset_module.rb @@ -31,7 +31,9 @@ def for_consumer_version_tag tag_name base_query = base_query.select_all_qualified.select_append(Sequel[:tags][:name].as(:tag_name)) end - base_query.join(:tags, { version_id: :consumer_version_id, Sequel[:tags][:name] => tag_name }) + base_query + .join(:tags, { version_id: :consumer_version_id, Sequel[:tags][:name] => tag_name }) + .remove_overridden_revisions_from_complete_query end def for_consumer_name_and_maybe_version_number(consumer_name, consumer_version_number) @@ -63,7 +65,7 @@ def latest_by_consumer_branch base_query = self if no_columns_selected? - base_query = base_query.select_all_qualified.select_append(Sequel[:max_orders][:branch_name].as(:branch)) + base_query = base_query.select_all_qualified.select_append(Sequel[:max_orders][:branch_name].as(:branch_name)) end base_query @@ -116,7 +118,7 @@ def latest_for_consumer_branch(branch_name) base_query = self if no_columns_selected? - base_query = base_query.select_all_qualified.select_append(Sequel[:max_orders][:branch_name].as(:branch)) + base_query = base_query.select_all_qualified.select_append(Sequel[:max_orders][:branch_name].as(:branch_name)) end base_query .join(max_orders, max_join, { table_alias: :max_orders }) diff --git a/lib/pact_broker/pacts/pact_publication_selector_dataset_module.rb b/lib/pact_broker/pacts/pact_publication_selector_dataset_module.rb index 30a288c6e..0a937282a 100644 --- a/lib/pact_broker/pacts/pact_publication_selector_dataset_module.rb +++ b/lib/pact_broker/pacts/pact_publication_selector_dataset_module.rb @@ -25,7 +25,30 @@ def for_provider_and_consumer_version_selector provider, selector # Updated logic - the pacts for the latest version of each main branch, # not the latest pact that belongs to the main branch. def latest_for_main_branches - where(consumer_version: PactBroker::Domain::Version.latest_for_main_branches) + pacticipants_join = { + Sequel[:branch_heads][:pacticipant_id] => Sequel[:pacticipants][:id], + Sequel[:branch_heads][:branch_name] => Sequel[:pacticipants][:main_branch] + } + matching_branch_version_ids = PactBroker::Versions::BranchHead + .select(:version_id) + .join(:pacticipants, pacticipants_join) + + consumers_join = { Sequel[:pact_publications][:consumer_id] => Sequel[:consumers][:id] } + + branch_heads_join = { + Sequel[:pact_publications][:consumer_version_id] => Sequel[:branch_heads][:version_id], + Sequel[:consumers][:main_branch] => Sequel[:branch_heads][:branch_name] + } + + query = self + if no_columns_selected? + query = query.select_all_qualified.select_append(Sequel[:branch_heads][:branch_name].as(:branch_name)) + end + + query + .join(:pacticipants, consumers_join, { table_alias: :consumers }) + .join(:branch_heads, branch_heads_join) + .remove_overridden_revisions_from_complete_query end def for_currently_deployed_versions(environment_name) @@ -48,6 +71,7 @@ def for_currently_deployed_versions(environment_name) .join(:deployed_versions, deployed_versions_join) .join(:currently_deployed_version_ids, currently_deployed_versions_join) .join(:environments, environments_join) + .remove_overridden_revisions_from_complete_query end def for_currently_supported_versions(environment_name) @@ -67,6 +91,7 @@ def for_currently_supported_versions(environment_name) query .join(:released_versions, released_versions_join) .join(:environments, environments_join) + .remove_overridden_revisions_from_complete_query end def for_environment(environment_name) diff --git a/lib/pact_broker/pacts/pacts_for_verification_repository.rb b/lib/pact_broker/pacts/pacts_for_verification_repository.rb index c41127a41..e27bb8bf5 100644 --- a/lib/pact_broker/pacts/pacts_for_verification_repository.rb +++ b/lib/pact_broker/pacts/pacts_for_verification_repository.rb @@ -215,7 +215,7 @@ def create_selectors_for_wip_pact(pact_publication) if pact_publication.values[:tag_name] Selectors.create_for_latest_for_tag(pact_publication.values[:tag_name]) else - Selectors.create_for_latest_for_branch(pact_publication.values.fetch(:branch)) + Selectors.create_for_latest_for_branch(pact_publication.values.fetch(:branch_name)) end end @@ -285,8 +285,8 @@ def collect_consumer_name_and_version_number(pact_publications_query) pact_publications_query.eager(:provider).eager(:consumer).eager(:consumer_version).order(:consumer_version_order).all.sort.collect do |p| suffix = if p.values[:tag_name] " (tag #{p.values[:tag_name]})" - elsif p.values[:branch] - " (branch #{p.values[:branch]})" + elsif p.values[:branch_name] + " (branch #{p.values[:branch_name]})" else "" end diff --git a/spec/lib/pact_broker/api/decorators/matrix_decorator_spec.rb b/spec/lib/pact_broker/api/decorators/matrix_decorator_spec.rb index c38128edf..8f00e4d9a 100644 --- a/spec/lib/pact_broker/api/decorators/matrix_decorator_spec.rb +++ b/spec/lib/pact_broker/api/decorators/matrix_decorator_spec.rb @@ -149,7 +149,7 @@ module Decorators let(:consumer_version) { double("consumer version", number: "1.0.0", pacticipant: double("consumer", name: "Consumer")) } let(:consumer_version_branch_versions) do - [ instance_double("PactBroker::Versions::BranchVersion", branch_name: "main") ] + [ instance_double("PactBroker::Versions::BranchVersion", branch_name: "main", latest?: true) ] end let(:consumer_version_tags) do @@ -161,7 +161,7 @@ module Decorators let(:provider_version) { double("provider version", number: "4.5.6", pacticipant: double("provider", name: "Provider")) } let(:provider_version_branch_versions) do - [ instance_double("PactBroker::Versions::BranchVersion", branch_name: "feat/x") ] + [ instance_double("PactBroker::Versions::BranchVersion", branch_name: "feat/x", latest?: true) ] end let(:provider_version_tags) do diff --git a/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb b/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb index 7a406e4cd..e828c37b7 100644 --- a/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb +++ b/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb @@ -40,14 +40,14 @@ module Pacts it "returns the latest pact publications for each consumer/branch" do expect(subject.size).to eq 3 - expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:branch] == "main" }.consumer_version.number).to eq "3" - expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:branch] == "feat/x" }.consumer_version.number).to eq "4" - expect(subject.find { |pp| pp.consumer_id == foo_z.id && pp[:branch] == "main" }.consumer_version.number).to eq "6" - expect(subject.find { |pp| pp.consumer_id == foo_z.id && pp[:branch] == "main" }.revision_number).to eq 2 + expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:branch_name] == "main" }.consumer_version.number).to eq "3" + expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:branch_name] == "feat/x" }.consumer_version.number).to eq "4" + expect(subject.find { |pp| pp.consumer_id == foo_z.id && pp[:branch_name] == "main" }.consumer_version.number).to eq "6" + expect(subject.find { |pp| pp.consumer_id == foo_z.id && pp[:branch_name] == "main" }.revision_number).to eq 2 end it "does not return extra columns" do - expect(subject.first.values.keys.sort).to eq (PactPublication.columns + [:branch]).sort + expect(subject.first.values.keys.sort).to eq (PactPublication.columns + [:branch_name]).sort end context "chained with created after" do @@ -56,8 +56,8 @@ module Pacts its(:size) { is_expected.to eq 2 } it "returns the right versions" do - expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:branch] == "feat/x" }.consumer_version.number).to eq "4" - expect(subject.find { |pp| pp.consumer_id == foo_z.id && pp[:branch] == "main" }.consumer_version.number).to eq "6" + expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:branch_name] == "feat/x" }.consumer_version.number).to eq "4" + expect(subject.find { |pp| pp.consumer_id == foo_z.id && pp[:branch_name] == "main" }.consumer_version.number).to eq "6" end end end @@ -97,7 +97,7 @@ module Pacts end it "does not return extra columns" do - expect(subject.first.values.keys.sort).to eq (PactPublication.columns + [:branch]).sort + expect(subject.first.values.keys.sort).to eq (PactPublication.columns + [:branch_name]).sort end context "when columns are already selected" do diff --git a/spec/lib/pact_broker/pacts/pact_publication_selector_dataset_module_spec.rb b/spec/lib/pact_broker/pacts/pact_publication_selector_dataset_module_spec.rb index bc84bc9bf..2a0238b40 100644 --- a/spec/lib/pact_broker/pacts/pact_publication_selector_dataset_module_spec.rb +++ b/spec/lib/pact_broker/pacts/pact_publication_selector_dataset_module_spec.rb @@ -43,6 +43,7 @@ module PactPublicationSelectorDatasetModule .create_pact .create_consumer_version("2", branch: "main") .create_pact + .revise_pact .create_consumer_version("3", branch: "not-main") .create_pact .create_consumer("Bob", main_branch: "develop") @@ -62,12 +63,12 @@ module PactPublicationSelectorDatasetModule it "returns the latest pact for the main branch of every consumer" do expect(subject.size).to eq 2 expect(subject.sort_by(&:id).first.consumer.name).to eq "Foo" - expect(subject.sort_by(&:id).first.consumer_version.branch_names).to eq ["main"] expect(subject.sort_by(&:id).first.consumer_version.number).to eq "2" + expect(subject.sort_by(&:id).first.values[:branch_name]).to eq "main" expect(subject.sort_by(&:id).last.consumer.name).to eq "Bob" - expect(subject.sort_by(&:id).last.consumer_version.branch_names).to eq ["develop"] expect(subject.sort_by(&:id).last.consumer_version.number).to eq "5" + expect(subject.sort_by(&:id).last.values[:branch_name]).to eq "develop" end end diff --git a/spec/lib/pact_broker/pacts/pact_publication_spec.rb b/spec/lib/pact_broker/pacts/pact_publication_spec.rb index 267d27244..a23d4642a 100644 --- a/spec/lib/pact_broker/pacts/pact_publication_spec.rb +++ b/spec/lib/pact_broker/pacts/pact_publication_spec.rb @@ -256,9 +256,9 @@ module Pacts expect(subject.size).to eq 3 subject.collect(&:values) - expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:branch] == "main" }.consumer_version.number).to eq "3" - expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:branch] == "feat/x" }.consumer_version.number).to eq "4" - expect(subject.find { |pp| pp.consumer_id == foo_z.id && pp[:branch] == "main" }.consumer_version.number).to eq "6" + expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:branch_name] == "main" }.consumer_version.number).to eq "3" + expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:branch_name] == "feat/x" }.consumer_version.number).to eq "4" + expect(subject.find { |pp| pp.consumer_id == foo_z.id && pp[:branch_name] == "main" }.consumer_version.number).to eq "6" end context "chained with created after" do @@ -267,8 +267,8 @@ module Pacts its(:size) { is_expected.to eq 2 } it "returns the right versions" do - expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:branch] == "feat/x" }.consumer_version.number).to eq "4" - expect(subject.find { |pp| pp.consumer_id == foo_z.id && pp[:branch] == "main" }.consumer_version.number).to eq "6" + expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:branch_name] == "feat/x" }.consumer_version.number).to eq "4" + expect(subject.find { |pp| pp.consumer_id == foo_z.id && pp[:branch_name] == "main" }.consumer_version.number).to eq "6" end end end From 012192969bfd4fdc9060660f75df98616d485251 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sat, 4 Sep 2021 18:36:31 +1000 Subject: [PATCH 33/45] chore: migrate version branch column to branch_version object --- .../db/data_migrations/create_branches.rb | 97 +++++++++++++++++++ lib/pact_broker/db/migrate_data.rb | 1 + .../data_migrations/create_branches_spec.rb | 58 +++++++++++ 3 files changed, 156 insertions(+) create mode 100644 lib/pact_broker/db/data_migrations/create_branches.rb create mode 100644 spec/lib/pact_broker/db/data_migrations/create_branches_spec.rb diff --git a/lib/pact_broker/db/data_migrations/create_branches.rb b/lib/pact_broker/db/data_migrations/create_branches.rb new file mode 100644 index 000000000..f93c7765d --- /dev/null +++ b/lib/pact_broker/db/data_migrations/create_branches.rb @@ -0,0 +1,97 @@ +require "pact_broker/db/data_migrations/helpers" + +module PactBroker + module DB + module DataMigrations + class CreateBranches + extend Helpers + + def self.call connection + if required_columns_exist?(connection) + branch_ids = create_branch_versions(connection) + upsert_branch_heads(connection, branch_ids) + end + end + + def self.required_columns_exist?(connection) + column_exists?(connection, :versions, :branch) && + connection.table_exists?(:branches) && + connection.table_exists?(:branch_versions) && + connection.table_exists?(:branch_heads) + end + + def self.create_branch_versions(connection) + versions_without_a_branch_version(connection).collect do | version | + create_branch_version(connection, version) + end.uniq + end + + def self.upsert_branch_heads(connection, branch_ids) + branch_ids.each do | branch_id | + upsert_branch_head(connection, branch_id) + end + end + + def self.versions_without_a_branch_version(connection) + branch_versions_join = { + Sequel[:versions][:id] => Sequel[:branch_versions][:version_id], + Sequel[:branch_versions][:branch_name] => Sequel[:versions][:branch] + } + + connection[:versions] + .select(Sequel[:versions].*) + .exclude(branch: nil) + .left_outer_join(:branch_versions, branch_versions_join) + .where(Sequel[:branch_versions][:branch_name] => nil) + .order(:pacticipant_id, :order) + end + + def self.create_branch_version(connection, version) + branch_values = { + name: version[:branch], + pacticipant_id: version[:pacticipant_id], + created_at: version[:created_at], + updated_at: version[:created_at] + } + connection[:branches].insert_ignore.insert(branch_values) + branch_id = connection[:branches].select(:id).where(pacticipant_id: version[:pacticipant_id], name: version[:branch]).single_record[:id] + + branch_version_values = { + pacticipant_id: version[:pacticipant_id], + version_id: version[:id], + version_order: version[:order], + branch_id: branch_id, + branch_name: version[:branch], + created_at: version[:created_at], + updated_at: version[:created_at] + } + + connection[:branch_versions].insert_ignore.insert(branch_version_values) + branch_id + end + + def self.upsert_branch_head(connection, branch_id) + latest_branch_version = connection[:branch_versions].where(branch_id: branch_id).order(:version_order).last + + if connection[:branch_heads].where(branch_id: branch_id).empty? + branch_head_values = { + pacticipant_id: latest_branch_version[:pacticipant_id], + branch_id: branch_id, + branch_version_id: latest_branch_version[:id], + version_id: latest_branch_version[:version_id], + branch_name: latest_branch_version[:branch_name] + } + connection[:branch_heads].insert(branch_head_values) + else + connection[:branch_heads] + .where(branch_id: branch_id) + .update( + branch_version_id: latest_branch_version[:id], + version_id: latest_branch_version[:version_id] + ) + end + end + end + end + end +end diff --git a/lib/pact_broker/db/migrate_data.rb b/lib/pact_broker/db/migrate_data.rb index c225c4c5e..3fd3bf619 100644 --- a/lib/pact_broker/db/migrate_data.rb +++ b/lib/pact_broker/db/migrate_data.rb @@ -27,6 +27,7 @@ def self.call database_connection, _options = {} DataMigrations::SetWebhookUuid.call(database_connection) DataMigrations::SetConsumerVersionOrderForPactPublications.call(database_connection) DataMigrations::SetExtraColumnsForTags.call(database_connection) + DataMigrations::CreateBranches.call(database_connection) end end end diff --git a/spec/lib/pact_broker/db/data_migrations/create_branches_spec.rb b/spec/lib/pact_broker/db/data_migrations/create_branches_spec.rb new file mode 100644 index 000000000..5bcbb2abe --- /dev/null +++ b/spec/lib/pact_broker/db/data_migrations/create_branches_spec.rb @@ -0,0 +1,58 @@ +require 'pact_broker/db/data_migrations/create_branches' + +module PactBroker + module DB + module DataMigrations + describe CreateBranches do + + let(:db) { PactBroker::Domain::Version.db } + + subject { CreateBranches.call(db) } + + context "when there are no branch objects" do + before do + td.create_pacticipant("Foo") + .create_version("1") + .create_version("2") + .create_version("3") + .create_pacticipant("Bar") + .create_version("10") + .create_version("11") + .create_version("12") + + db[:versions].where(number: ["1", "2"]).update(branch: "main") + db[:versions].where(number: ["10", "11"]).update(branch: "main") + end + + it "creates the missing branch versions" do + subject + expect(db[:branches].count).to eq 2 + expect(db[:branch_heads].count).to eq 2 + expect(db[:branch_versions].count).to eq 4 + expect(db[:branch_heads].order(:id).first[:version_id]).to eq db[:versions].where(number: "2").single_record[:id] + expect(db[:branch_heads].order(:id).last[:version_id]).to eq db[:versions].where(number: "11").single_record[:id] + end + end + + context "when there is a branch already" do + before do + td.create_pacticipant("Foo") + .create_version("1", branch: "main") + .create_version("2") + .create_version("3", branch: "main") + .create_version("4") + db[:versions].where(number: ["1", "2"]).update(branch: "main") + end + + it "creates the missing branch versionsq" do + subject + expect(db[:branches].count).to eq 1 + expect(db[:branch_heads].count).to eq 1 + expect(db[:branch_versions].count).to eq 3 + expect(db[:branch_heads].order(:id).last[:version_id]).to eq db[:versions].where(number: "3").single_record[:id] + end + end + end + end + end +end From b39a3909ef72b3396a330adff7124af1234ebfb4 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sat, 4 Sep 2021 18:36:48 +1000 Subject: [PATCH 34/45] style: rubocop --- .../pacts/pact_publication_selector_dataset_module.rb | 8 -------- .../db/data_migrations/create_branches_spec.rb | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/lib/pact_broker/pacts/pact_publication_selector_dataset_module.rb b/lib/pact_broker/pacts/pact_publication_selector_dataset_module.rb index 0a937282a..de05f4f3a 100644 --- a/lib/pact_broker/pacts/pact_publication_selector_dataset_module.rb +++ b/lib/pact_broker/pacts/pact_publication_selector_dataset_module.rb @@ -25,14 +25,6 @@ def for_provider_and_consumer_version_selector provider, selector # Updated logic - the pacts for the latest version of each main branch, # not the latest pact that belongs to the main branch. def latest_for_main_branches - pacticipants_join = { - Sequel[:branch_heads][:pacticipant_id] => Sequel[:pacticipants][:id], - Sequel[:branch_heads][:branch_name] => Sequel[:pacticipants][:main_branch] - } - matching_branch_version_ids = PactBroker::Versions::BranchHead - .select(:version_id) - .join(:pacticipants, pacticipants_join) - consumers_join = { Sequel[:pact_publications][:consumer_id] => Sequel[:consumers][:id] } branch_heads_join = { diff --git a/spec/lib/pact_broker/db/data_migrations/create_branches_spec.rb b/spec/lib/pact_broker/db/data_migrations/create_branches_spec.rb index 5bcbb2abe..b7a1c5bcd 100644 --- a/spec/lib/pact_broker/db/data_migrations/create_branches_spec.rb +++ b/spec/lib/pact_broker/db/data_migrations/create_branches_spec.rb @@ -1,4 +1,4 @@ -require 'pact_broker/db/data_migrations/create_branches' +require "pact_broker/db/data_migrations/create_branches" module PactBroker module DB From 5560e71f2a4ed4b941be5de09c9e532640ef17f1 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sat, 4 Sep 2021 19:45:35 +1000 Subject: [PATCH 35/45] chore: fix ordering of deployed/released selectors --- lib/pact_broker/pacts/selected_pact.rb | 4 +- lib/pact_broker/pacts/selector.rb | 125 +++++++++++++----- ...sitory_find_for_currently_deployed_spec.rb | 4 +- ...d_for_currently_supported_releases_spec.rb | 4 +- spec/lib/pact_broker/pacts/selector_spec.rb | 30 ++++- 5 files changed, 125 insertions(+), 42 deletions(-) diff --git a/lib/pact_broker/pacts/selected_pact.rb b/lib/pact_broker/pacts/selected_pact.rb index 4e2a8c08f..70e3cea8a 100644 --- a/lib/pact_broker/pacts/selected_pact.rb +++ b/lib/pact_broker/pacts/selected_pact.rb @@ -15,8 +15,8 @@ def self.merge_by_pact_version_sha(selected_pacts) selected_pacts .group_by{ |p| [p.consumer_name, p.pact_version_sha] } .values - .collect do | selected_pacts_for_pact_version_id | - SelectedPact.merge(selected_pacts_for_pact_version_id) + .collect do | selected_pacts_for_pact_version_sha | + SelectedPact.merge(selected_pacts_for_pact_version_sha) end .sort end diff --git a/lib/pact_broker/pacts/selector.rb b/lib/pact_broker/pacts/selector.rb index 8354808c8..282d50132 100644 --- a/lib/pact_broker/pacts/selector.rb +++ b/lib/pact_broker/pacts/selector.rb @@ -2,6 +2,7 @@ module PactBroker module Pacts + # rubocop: disable Metrics/ClassLength class Selector < Hash using PactBroker::HashRefinements @@ -266,51 +267,89 @@ def == other other.class == self.class && super end - # rubocop: disable Metrics/CyclomaticComplexity, Metrics/MethodLength + # rubocop: disable Metrics/CyclomaticComplexity def <=> other + # elsif consumer || other.consumer + # consumer_comparison(other) if overall_latest? || other.overall_latest? - if overall_latest? == other.overall_latest? - 0 - else - overall_latest? ? -1 : 1 - end + overall_latest_comparison(other) elsif latest_for_branch? || other.latest_for_branch? - if latest_for_branch? == other.latest_for_branch? - branch <=> other.branch - else - latest_for_branch? ? -1 : 1 - end + branch_comparison(other) + elsif latest_for_tag? || other.latest_for_tag? + latest_for_tag_comparison(other) + elsif tag || other.tag + tag_comparison(other) elsif currently_deployed? || other.currently_deployed? - if currently_deployed? == other.currently_deployed? - environment_name <=> other.environment_name - else - currently_deployed? ? -1 : 1 - end + currently_deployed_comparison(other) elsif currently_supported? || other.currently_supported? - if currently_supported? == other.currently_supported? - environment_name <=> other.environment_name - else - currently_supported? ? -1 : 1 - end - elsif latest_for_tag? || other.latest_for_tag? - if latest_for_tag? == other.latest_for_tag? - tag <=> other.tag + currently_supported_comparison(other) + else + 0 + end + end + # rubocop: enable Metrics/CyclomaticComplexity + + private + + def overall_latest_comparison(other) + if overall_latest? == other.overall_latest? + 0 + else + overall_latest? ? -1 : 1 + end + end + + def branch_comparison(other) + if latest_for_branch? == other.latest_for_branch? + branch <=> other.branch + else + latest_for_branch? ? -1 : 1 + end + end + + def currently_deployed_comparison(other) + if currently_deployed? == other.currently_deployed? + environment_name <=> other.environment_name + else + currently_deployed? ? -1 : 1 + end + end + + def currently_supported_comparison(other) + if currently_supported? == other.currently_supported? + environment_name <=> other.environment_name + else + currently_supported? ? -1 : 1 + end + end + + def latest_for_tag_comparison(other) + if latest_for_tag? == other.latest_for_tag? + tag <=> other.tag + else + latest_for_tag? ? -1 : 1 + end + end + + def tag_comparison(other) + if tag && other.tag + if tag == other.tag + consumer_comparison(other) else - latest_for_tag? ? -1 : 1 - end - elsif consumer || other.consumer - if consumer == other.consumer tag <=> other.tag - else - consumer ? -1 : 1 end else - tag <=> other.tag + tag ? -1 : 1 end end - # rubocop: enable Metrics/CyclomaticComplexity, Metrics/MethodLength - private + def consumer_comparison(other) + if consumer == other.consumer + 0 + else + consumer ? -1 : 1 + end + end def latest? !!self[:latest] @@ -347,6 +386,26 @@ def <=> other comparison end end + + # TODO sort by non prod first + def currently_deployed_comparison(other) + if currently_deployed? == other.currently_deployed? + environment.name <=> other.environment.name + else + currently_deployed? ? -1 : 1 + end + + end + + # TODO sort by non prod first + def currently_supported_comparison(other) + if currently_supported? == other.currently_supported? + environment.name <=> other.environment.name + else + currently_supported? ? -1 : 1 + end + end end + # rubocop: enable Metrics/ClassLength end end diff --git a/spec/lib/pact_broker/pacts/repository_find_for_currently_deployed_spec.rb b/spec/lib/pact_broker/pacts/repository_find_for_currently_deployed_spec.rb index aadab913e..2e5374a23 100644 --- a/spec/lib/pact_broker/pacts/repository_find_for_currently_deployed_spec.rb +++ b/spec/lib/pact_broker/pacts/repository_find_for_currently_deployed_spec.rb @@ -114,8 +114,8 @@ def find_by_consumer_name_and_consumer_version_number(consumer_name, consumer_ve it "returns one pact_publication with multiple selectors" do expect(subject.size).to eq 1 expect(subject.first.selectors.size).to eq 2 - expect(subject.first.selectors.first.environment.name).to eq "test" - expect(subject.first.selectors.last.environment.name).to eq "prod" + expect(subject.first.selectors.first.environment.name).to eq "prod" + expect(subject.first.selectors.last.environment.name).to eq "test" end end end diff --git a/spec/lib/pact_broker/pacts/repository_find_for_currently_supported_releases_spec.rb b/spec/lib/pact_broker/pacts/repository_find_for_currently_supported_releases_spec.rb index 671ff04d8..b54c26bd0 100644 --- a/spec/lib/pact_broker/pacts/repository_find_for_currently_supported_releases_spec.rb +++ b/spec/lib/pact_broker/pacts/repository_find_for_currently_supported_releases_spec.rb @@ -149,8 +149,8 @@ def find_by_consumer_name_and_consumer_version_number(consumer_name, consumer_ve it "returns one pact_publication with multiple selectors" do expect(subject.size).to eq 1 expect(subject.first.selectors.size).to eq 2 - expect(subject.first.selectors.first.environment.name).to eq "test" - expect(subject.first.selectors.last.environment.name).to eq "prod" + expect(subject.first.selectors.first.environment.name).to eq "prod" + expect(subject.first.selectors.last.environment.name).to eq "test" end end end diff --git a/spec/lib/pact_broker/pacts/selector_spec.rb b/spec/lib/pact_broker/pacts/selector_spec.rb index 60f972721..0b6a7df97 100644 --- a/spec/lib/pact_broker/pacts/selector_spec.rb +++ b/spec/lib/pact_broker/pacts/selector_spec.rb @@ -12,23 +12,47 @@ module Pacts let(:all_prod_for_consumer_1) { Selector.all_for_tag_and_consumer("prod", "Foo") } let(:all_prod_for_consumer_2) { Selector.all_for_tag_and_consumer("prod", "Bar") } let(:all_dev_for_consumer_1) { Selector.all_for_tag_and_consumer("dev", "Bar") } - let(:all_prod) { Selector.all_for_tag("prod") } + let(:all_tagged_prod) { Selector.all_for_tag("prod") } let(:all_dev) { Selector.all_for_tag("dev") } let(:currently_deployed_to_prod) { Selector.for_currently_deployed("prod") } let(:currently_deployed_to_test) { Selector.for_currently_deployed("test") } let(:currently_supported_in_prod) { Selector.for_currently_supported("prod") } let(:unsorted_selectors) do - [currently_supported_in_prod, all_prod, all_dev, currently_deployed_to_prod, all_dev_for_consumer_1, latest_for_branch_main, latest_for_tag_prod, currently_deployed_to_test, overall_latest_1, overall_latest_1, latest_for_tag_dev, all_prod_for_consumer_2, all_prod_for_consumer_1] + [currently_supported_in_prod, all_tagged_prod, all_dev, currently_deployed_to_prod, all_dev_for_consumer_1, latest_for_branch_main, latest_for_tag_prod, currently_deployed_to_test, overall_latest_1, overall_latest_1, latest_for_tag_dev, all_prod_for_consumer_2, all_prod_for_consumer_1] end let(:expected_sorted_selectors) do - [overall_latest_1, overall_latest_1, latest_for_branch_main, currently_deployed_to_prod, currently_deployed_to_test, currently_supported_in_prod, latest_for_tag_dev, latest_for_tag_prod, all_dev_for_consumer_1, all_prod_for_consumer_2, all_prod_for_consumer_1, all_dev, all_prod] + [ + overall_latest_1, + overall_latest_1, + latest_for_branch_main, + latest_for_tag_dev, + latest_for_tag_prod, + all_dev_for_consumer_1, + all_dev, + all_prod_for_consumer_2, + all_prod_for_consumer_1, + all_tagged_prod, + currently_deployed_to_prod, + currently_deployed_to_test, + currently_supported_in_prod, + ] end it "sorts the selectors" do expect(unsorted_selectors.sort).to eq(expected_sorted_selectors) end + + context "with resolved selectors" do + let(:currently_deployed_to_prod) { Selector.for_currently_deployed("prod").resolve_for_environment(double("version", order: 1), double("environment", name: "prod")) } + let(:currently_deployed_to_test) { Selector.for_currently_deployed("test").resolve_for_environment(double("version", order: 1), double("environment", name: "test")) } + let(:currently_supported_in_prod) { Selector.for_currently_supported("prod").resolve_for_environment(double("version", order: 1), double("environment", name: "prod")) } + + it "sorts the selectors" do + expect(unsorted_selectors.sort).to eq(expected_sorted_selectors) + end + end end end end From 5388b3357e9ddf78b247409762499dd4889544bd Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sat, 4 Sep 2021 19:57:57 +1000 Subject: [PATCH 36/45] chore: sort environment selectors by non prod first --- lib/pact_broker/deployments/environment.rb | 4 ++++ lib/pact_broker/pacts/selector.rb | 14 +++++++---- spec/lib/pact_broker/pacts/selector_spec.rb | 24 ++++++++++++++++--- .../pacts/verifiable_pact_messages_spec.rb | 10 ++++---- 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/lib/pact_broker/deployments/environment.rb b/lib/pact_broker/deployments/environment.rb index 279a68d43..5545b53c1 100644 --- a/lib/pact_broker/deployments/environment.rb +++ b/lib/pact_broker/deployments/environment.rb @@ -25,6 +25,10 @@ def delete PactBroker::Deployments::ReleasedVersion.where(environment: self).delete super end + + def production? + production + end end end end diff --git a/lib/pact_broker/pacts/selector.rb b/lib/pact_broker/pacts/selector.rb index 282d50132..722eef0bc 100644 --- a/lib/pact_broker/pacts/selector.rb +++ b/lib/pact_broker/pacts/selector.rb @@ -387,24 +387,30 @@ def <=> other end end - # TODO sort by non prod first def currently_deployed_comparison(other) if currently_deployed? == other.currently_deployed? - environment.name <=> other.environment.name + production_comparison(other) else currently_deployed? ? -1 : 1 end end - # TODO sort by non prod first def currently_supported_comparison(other) if currently_supported? == other.currently_supported? - environment.name <=> other.environment.name + production_comparison(other) else currently_supported? ? -1 : 1 end end + + def production_comparison(other) + if environment.production? == other.environment.production? + environment.name <=> other.environment.name + else + environment.production? ? 1 : -1 + end + end end # rubocop: enable Metrics/ClassLength end diff --git a/spec/lib/pact_broker/pacts/selector_spec.rb b/spec/lib/pact_broker/pacts/selector_spec.rb index 0b6a7df97..299ae71e6 100644 --- a/spec/lib/pact_broker/pacts/selector_spec.rb +++ b/spec/lib/pact_broker/pacts/selector_spec.rb @@ -45,9 +45,27 @@ module Pacts end context "with resolved selectors" do - let(:currently_deployed_to_prod) { Selector.for_currently_deployed("prod").resolve_for_environment(double("version", order: 1), double("environment", name: "prod")) } - let(:currently_deployed_to_test) { Selector.for_currently_deployed("test").resolve_for_environment(double("version", order: 1), double("environment", name: "test")) } - let(:currently_supported_in_prod) { Selector.for_currently_supported("prod").resolve_for_environment(double("version", order: 1), double("environment", name: "prod")) } + let(:currently_deployed_to_prod) { Selector.for_currently_deployed("prod").resolve_for_environment(double("version", order: 1), double("environment", name: "prod", production?: true)) } + let(:currently_deployed_to_test) { Selector.for_currently_deployed("test").resolve_for_environment(double("version", order: 1), double("environment", name: "test", production?: false)) } + let(:currently_supported_in_prod) { Selector.for_currently_supported("prod").resolve_for_environment(double("version", order: 1), double("environment", name: "prod", production?: true)) } + + let(:expected_sorted_selectors) do + [ + overall_latest_1, + overall_latest_1, + latest_for_branch_main, + latest_for_tag_dev, + latest_for_tag_prod, + all_dev_for_consumer_1, + all_dev, + all_prod_for_consumer_2, + all_prod_for_consumer_1, + all_tagged_prod, + currently_deployed_to_test, + currently_deployed_to_prod, + currently_supported_in_prod, + ] + end it "sorts the selectors" do expect(unsorted_selectors.sort).to eq(expected_sorted_selectors) diff --git a/spec/lib/pact_broker/pacts/verifiable_pact_messages_spec.rb b/spec/lib/pact_broker/pacts/verifiable_pact_messages_spec.rb index 01136f895..d6931c2a6 100644 --- a/spec/lib/pact_broker/pacts/verifiable_pact_messages_spec.rb +++ b/spec/lib/pact_broker/pacts/verifiable_pact_messages_spec.rb @@ -27,9 +27,9 @@ module Pacts ) end let(:consumer_version) { double("version", number: "1234", order: 1) } - let(:environment) { instance_double("PactBroker::Deployments::Environment", name: "test") } + let(:environment) { instance_double("PactBroker::Deployments::Environment", name: "test", production?: false) } let(:test_environment) { environment } - let(:prod_environment) { instance_double("PactBroker::Deployments::Environment", name: "prod") } + let(:prod_environment) { instance_double("PactBroker::Deployments::Environment", name: "prod", production?: true) } subject { VerifiablePactMessages.new(verifiable_pact, pact_version_url) } @@ -171,13 +171,13 @@ module Pacts context "when the consumer version is currently deployed to a multiple environments" do let(:selectors) do Selectors.new( - Selector.for_currently_deployed("dev").resolve_for_environment(consumer_version, double("environment", name: "dev")), + Selector.for_currently_deployed("dev").resolve_for_environment(consumer_version, double("environment", name: "dev", production?: false)), Selector.for_currently_deployed("test").resolve_for_environment(consumer_version, test_environment), Selector.for_currently_deployed("prod").resolve_for_environment(consumer_version, prod_environment) ) end - its(:inclusion_reason) { is_expected.to include "consumer version(s) currently deployed to dev (1234), prod (1234) and test (1234)"} + its(:inclusion_reason) { is_expected.to include "consumer version(s) currently deployed to dev (1234), test (1234) and prod (1234)"} end context "when the currently deployed consumer version is for a consumer" do @@ -190,7 +190,7 @@ module Pacts ) end - its(:inclusion_reason) { is_expected.to include "version(s) of Foo currently deployed to prod (1234) and test (1234)"} + its(:inclusion_reason) { is_expected.to include "version(s) of Foo currently deployed to test (1234) and prod (1234)"} its(:inclusion_reason) { is_expected.to include "version(s) of Bar currently deployed to test (1234)"} its(:inclusion_reason) { is_expected.to include "consumer version(s) currently deployed to test (1234)"} end From 6e8bc96b7b2c66bbc264ab14dff7f863c315bbee Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sun, 5 Sep 2021 20:42:49 +1000 Subject: [PATCH 37/45] test: include provider in latest_by_consumer_branch --- .../pacts/pact_publication_dataset_module.rb | 3 ++- .../pact_publication_dataset_module_spec.rb | 16 ++++++++++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/pact_broker/pacts/pact_publication_dataset_module.rb b/lib/pact_broker/pacts/pact_publication_dataset_module.rb index 926dbec65..0048a510f 100644 --- a/lib/pact_broker/pacts/pact_publication_dataset_module.rb +++ b/lib/pact_broker/pacts/pact_publication_dataset_module.rb @@ -55,11 +55,12 @@ def latest_by_consumer_branch max_orders = join(:branch_versions, branch_versions_join) .join(:branches, branches_join) - .select_group(Sequel[:branches][:pacticipant_id].as(:consumer_id), Sequel[:branches][:name].as(:branch_name)) + .select_group(Sequel[:pact_publications][:consumer_id], Sequel[:pact_publications][:provider_id], Sequel[:branches][:name].as(:branch_name)) .select_append{ max(consumer_version_order).as(latest_consumer_version_order) } max_join = { Sequel[:max_orders][:consumer_id] => Sequel[:pact_publications][:consumer_id], + Sequel[:max_orders][:provider_id] => Sequel[:pact_publications][:provider_id], Sequel[:max_orders][:latest_consumer_version_order] => Sequel[:pact_publications][:consumer_version_order] } diff --git a/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb b/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb index e828c37b7..5b20560bc 100644 --- a/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb +++ b/spec/lib/pact_broker/pacts/pact_publication_dataset_module_spec.rb @@ -29,6 +29,8 @@ module Pacts .create_pact .set_now(Date.new(2020, 1, 7)) .create_consumer_version("8", branch: "main", comment: "No pact") + .create_provider("NotBar") + .create_pact end subject { PactPublication.latest_by_consumer_branch.all } @@ -36,14 +38,16 @@ module Pacts let(:foo) { PactBroker::Domain::Pacticipant.where(name: "Foo").single_record } let(:bar) { PactBroker::Domain::Pacticipant.where(name: "Bar").single_record } let(:foo_z) { PactBroker::Domain::Pacticipant.where(name: "FooZ").single_record } + let(:not_bar) { td.find_pacticipant("NotBar") } it "returns the latest pact publications for each consumer/branch" do - expect(subject.size).to eq 3 + expect(subject.size).to eq 4 - expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:branch_name] == "main" }.consumer_version.number).to eq "3" - expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:branch_name] == "feat/x" }.consumer_version.number).to eq "4" - expect(subject.find { |pp| pp.consumer_id == foo_z.id && pp[:branch_name] == "main" }.consumer_version.number).to eq "6" - expect(subject.find { |pp| pp.consumer_id == foo_z.id && pp[:branch_name] == "main" }.revision_number).to eq 2 + expect(subject.find { |pp| pp.consumer_id == foo.id && pp.provider_id == bar.id && pp[:branch_name] == "main" }.consumer_version.number).to eq "3" + expect(subject.find { |pp| pp.consumer_id == foo.id && pp.provider_id == bar.id && pp[:branch_name] == "feat/x" }.consumer_version.number).to eq "4" + expect(subject.find { |pp| pp.consumer_id == foo_z.id && pp.provider_id == bar.id && pp[:branch_name] == "main" }.consumer_version.number).to eq "6" + expect(subject.find { |pp| pp.consumer_id == foo_z.id && pp.provider_id == bar.id && pp[:branch_name] == "main" }.revision_number).to eq 2 + expect(subject.find { |pp| pp.consumer_id == foo_z.id && pp.provider_id == not_bar.id && pp[:branch_name] == "main" }.consumer_version.number).to eq "8" end it "does not return extra columns" do @@ -53,7 +57,7 @@ module Pacts context "chained with created after" do subject { PactPublication.created_after(DateTime.new(2020, 1, 3)).latest_by_consumer_branch.all } - its(:size) { is_expected.to eq 2 } + its(:size) { is_expected.to eq 3 } it "returns the right versions" do expect(subject.find { |pp| pp.consumer_id == foo.id && pp[:branch_name] == "feat/x" }.consumer_version.number).to eq "4" From e1b21b1c381fa4e56c1b82f4862131ab043a0566 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sun, 5 Sep 2021 20:44:03 +1000 Subject: [PATCH 38/45] chore: remove unnecessary index --- db/migrations/20210817_add_pact_publication_indexes.rb | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 db/migrations/20210817_add_pact_publication_indexes.rb diff --git a/db/migrations/20210817_add_pact_publication_indexes.rb b/db/migrations/20210817_add_pact_publication_indexes.rb deleted file mode 100644 index 2b59734c7..000000000 --- a/db/migrations/20210817_add_pact_publication_indexes.rb +++ /dev/null @@ -1,7 +0,0 @@ -Sequel.migration do - change do - alter_table(:pact_publications) do - add_index [:consumer_id, :consumer_version_order], name: :pact_publications_consumer_id_consumer_version_order - end - end -end From 437671c42b43ea0219c17a0a51c65aeb5bc1b77b Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sun, 5 Sep 2021 20:48:11 +1000 Subject: [PATCH 39/45] chore: add index --- db/migrations/20210816_create_branches_tables.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/db/migrations/20210816_create_branches_tables.rb b/db/migrations/20210816_create_branches_tables.rb index e41b9832a..6ec0174be 100644 --- a/db/migrations/20210816_create_branches_tables.rb +++ b/db/migrations/20210816_create_branches_tables.rb @@ -19,6 +19,7 @@ DateTime :created_at, null: false DateTime :updated_at, null: false index [:branch_id, :version_id], unique: true, name: :branch_versions_branch_id_version_id_index + index [:version_id], name: :branch_versions_version_id_index index [:branch_name], name: :branch_versions_branch_name_index # Can probably drop this index when the "latest pact" logic changes index [:pacticipant_id, :branch_id, :version_order], name: :branch_versions_pacticipant_id_branch_id_version_order_index From 4264eafc74824f33311f235d22887fdee0b9687d Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sun, 5 Sep 2021 20:51:30 +1000 Subject: [PATCH 40/45] chore: update annotations --- lib/pact_broker/domain/verification.rb | 28 ++++++++++++---------- lib/pact_broker/domain/version.rb | 1 - lib/pact_broker/pacts/pact_publication.rb | 10 ++++---- lib/pact_broker/versions/branch.rb | 14 +++++------ lib/pact_broker/versions/branch_head.rb | 8 ++++--- lib/pact_broker/versions/branch_version.rb | 1 + 6 files changed, 32 insertions(+), 30 deletions(-) diff --git a/lib/pact_broker/domain/verification.rb b/lib/pact_broker/domain/verification.rb index a383d92e2..777e460c2 100644 --- a/lib/pact_broker/domain/verification.rb +++ b/lib/pact_broker/domain/verification.rb @@ -222,19 +222,21 @@ def method_missing(m, *args, &block) # Table: verifications # Columns: -# id | integer | PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY -# number | integer | -# success | boolean | NOT NULL -# provider_version | text | -# build_url | text | -# pact_version_id | integer | NOT NULL -# execution_date | timestamp without time zone | NOT NULL -# created_at | timestamp without time zone | NOT NULL -# provider_version_id | integer | -# test_results | text | -# consumer_id | integer | -# provider_id | integer | -# wip | boolean | NOT NULL DEFAULT false +# id | integer | PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY +# number | integer | +# success | boolean | NOT NULL +# provider_version | text | +# build_url | text | +# pact_version_id | integer | NOT NULL +# execution_date | timestamp without time zone | NOT NULL +# created_at | timestamp without time zone | NOT NULL +# provider_version_id | integer | +# test_results | text | +# consumer_id | integer | +# provider_id | integer | +# wip | boolean | NOT NULL DEFAULT false +# consumer_version_selector_hashes | text | +# tag_names | text | # Indexes: # verifications_pkey | PRIMARY KEY btree (id) # verifications_pact_version_id_number_index | UNIQUE btree (pact_version_id, number) diff --git a/lib/pact_broker/domain/version.rb b/lib/pact_broker/domain/version.rb index 95c8f40eb..d7e963d2c 100644 --- a/lib/pact_broker/domain/version.rb +++ b/lib/pact_broker/domain/version.rb @@ -324,7 +324,6 @@ def tag_names # order | integer | # created_at | timestamp without time zone | NOT NULL # updated_at | timestamp without time zone | NOT NULL -# branch | text | # build_url | text | # Indexes: # versions_pkey | PRIMARY KEY btree (id) diff --git a/lib/pact_broker/pacts/pact_publication.rb b/lib/pact_broker/pacts/pact_publication.rb index 68341f2e3..002384dc4 100644 --- a/lib/pact_broker/pacts/pact_publication.rb +++ b/lib/pact_broker/pacts/pact_publication.rb @@ -174,11 +174,11 @@ def cached_domain_for_delegation # consumer_id | integer | # consumer_version_order | integer | # Indexes: -# pact_publications_pkey | PRIMARY KEY btree (id) -# cv_prov_revision_unq | UNIQUE btree (consumer_version_id, provider_id, revision_number) -# cv_prov_id_ndx | btree (consumer_version_id, provider_id, id) -# pact_publications_consumer_id_consumer_version_order | btree (consumer_id, consumer_version_order) -# pact_publications_consumer_id_index | btree (consumer_id) +# pact_publications_pkey | PRIMARY KEY btree (id) +# cv_prov_revision_unq | UNIQUE btree (consumer_version_id, provider_id, revision_number) +# cv_prov_id_ndx | btree (consumer_version_id, provider_id, id) +# pact_publications_cid_pid_cvo_index | btree (consumer_id, provider_id, consumer_version_order) +# pact_publications_consumer_id_index | btree (consumer_id) # Foreign key constraints: # pact_publications_consumer_id_fkey | (consumer_id) REFERENCES pacticipants(id) # pact_publications_consumer_version_id_fkey | (consumer_version_id) REFERENCES versions(id) diff --git a/lib/pact_broker/versions/branch.rb b/lib/pact_broker/versions/branch.rb index 379340d88..d4f0ee3b3 100644 --- a/lib/pact_broker/versions/branch.rb +++ b/lib/pact_broker/versions/branch.rb @@ -14,16 +14,14 @@ class Branch < Sequel::Model(:branches) # Table: branches # Columns: -# id | integer | PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY -# name | text | -# pacticipant_id | integer | NOT NULL -# latest_branch_version_id | integer | -# latest_version_id | integer | -# created_at | timestamp without time zone | NOT NULL -# updated_at | timestamp without time zone | NOT NULL +# id | integer | PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY +# name | text | +# pacticipant_id | integer | NOT NULL +# created_at | timestamp without time zone | NOT NULL +# updated_at | timestamp without time zone | NOT NULL # Indexes: # branches_pkey | PRIMARY KEY btree (id) -# branches_name_pacticipant_id_index | UNIQUE btree (name, pacticipant_id) +# branches_pacticipant_id_name_index | UNIQUE btree (pacticipant_id, name) # Foreign key constraints: # branches_pacticipant_id_fkey | (pacticipant_id) REFERENCES pacticipants(id) ON DELETE CASCADE # Referenced By: diff --git a/lib/pact_broker/versions/branch_head.rb b/lib/pact_broker/versions/branch_head.rb index d10f0b2d7..77b72fcd5 100644 --- a/lib/pact_broker/versions/branch_head.rb +++ b/lib/pact_broker/versions/branch_head.rb @@ -33,9 +33,11 @@ def branch_name # pacticipant_id | integer | NOT NULL # branch_name | text | NOT NULL # Indexes: -# branch_heads_pkey | PRIMARY KEY btree (id) -# branch_heads_branch_id_index | UNIQUE btree (branch_id) -# branch_heads_branch_name_index | btree (branch_name) +# branch_heads_pkey | PRIMARY KEY btree (id) +# branch_heads_branch_id_index | UNIQUE btree (branch_id) +# branch_heads_branch_name_index | btree (branch_name) +# branch_heads_pacticipant_id_index | btree (pacticipant_id) +# branch_heads_version_id_index | btree (version_id) # Foreign key constraints: # branch_heads_branch_id_fkey | (branch_id) REFERENCES branches(id) ON DELETE CASCADE # branch_heads_branch_version_id_fkey | (branch_version_id) REFERENCES branch_versions(id) ON DELETE CASCADE diff --git a/lib/pact_broker/versions/branch_version.rb b/lib/pact_broker/versions/branch_version.rb index 2761c7168..d2198c3e0 100644 --- a/lib/pact_broker/versions/branch_version.rb +++ b/lib/pact_broker/versions/branch_version.rb @@ -56,6 +56,7 @@ def pacticipant # branch_versions_branch_id_version_id_index | UNIQUE btree (branch_id, version_id) # branch_versions_branch_name_index | btree (branch_name) # branch_versions_pacticipant_id_branch_id_version_order_index | btree (pacticipant_id, branch_id, version_order) +# branch_versions_version_id_index | btree (version_id) # Foreign key constraints: # branch_versions_branches_fk | (branch_id) REFERENCES branches(id) ON DELETE CASCADE # branch_versions_versions_fk | (version_id) REFERENCES versions(id) ON DELETE CASCADE From b90fc9245697e9de49fd86ad4b3554d747e507ed Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sun, 5 Sep 2021 20:59:26 +1000 Subject: [PATCH 41/45] chore: update logging --- lib/pact_broker/versions/service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pact_broker/versions/service.rb b/lib/pact_broker/versions/service.rb index 34a68bbd7..3b9ed3862 100644 --- a/lib/pact_broker/versions/service.rb +++ b/lib/pact_broker/versions/service.rb @@ -48,7 +48,7 @@ def self.delete version def self.maybe_set_version_branch_from_tag(version, tag_name) if use_tag_as_branch?(version) && version.branch_versions.empty? - logger.info "Setting #{version.pacticipant.name} version #{version.number} branch to '#{tag_name}' from first tag (because use_first_tag_as_branch=true)" + logger.info "Adding #{version.pacticipant.name} version #{version.number} to branch '#{tag_name}' (from first tag, because use_first_tag_as_branch=true)" branch_version_repository.add_branch(version, tag_name) end end From 47747703c84a1d89679f53aa69904b8d0e0864aa Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sun, 5 Sep 2021 21:07:18 +1000 Subject: [PATCH 42/45] chore: fix latest_for_consumer_branch --- lib/pact_broker/pacts/pact_publication_dataset_module.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pact_broker/pacts/pact_publication_dataset_module.rb b/lib/pact_broker/pacts/pact_publication_dataset_module.rb index 0048a510f..d560a058f 100644 --- a/lib/pact_broker/pacts/pact_publication_dataset_module.rb +++ b/lib/pact_broker/pacts/pact_publication_dataset_module.rb @@ -101,7 +101,6 @@ def latest_for_consumer_branch(branch_name) Sequel[:pact_publications][:consumer_version_id] => Sequel[:branch_versions][:version_id] } - # TODO use name like branches_join = { Sequel[:branch_versions][:branch_id] => Sequel[:branches][:id], Sequel[:branches][:name] => branch_name @@ -109,11 +108,12 @@ def latest_for_consumer_branch(branch_name) max_orders = join(:branch_versions, branch_versions_join) .join(:branches, branches_join) - .select_group(:consumer_id, Sequel[:branches][:name].as(:branch_name)) + .select_group(:consumer_id, :provider_id, Sequel[:branches][:name].as(:branch_name)) .select_append{ max(consumer_version_order).as(latest_consumer_version_order) } max_join = { Sequel[:max_orders][:consumer_id] => Sequel[:pact_publications][:consumer_id], + Sequel[:max_orders][:provider_id] => Sequel[:pact_publications][:provider_id], Sequel[:max_orders][:latest_consumer_version_order] => Sequel[:pact_publications][:consumer_version_order] } From 358fb25c0772256174d1408ff0325a55e6b0a64b Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Mon, 6 Sep 2021 09:44:16 +1000 Subject: [PATCH 43/45] style: whitespace --- .../pact_broker/versions/branch_version_repository_spec.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/spec/lib/pact_broker/versions/branch_version_repository_spec.rb b/spec/lib/pact_broker/versions/branch_version_repository_spec.rb index 00939fa51..aa476ce6a 100644 --- a/spec/lib/pact_broker/versions/branch_version_repository_spec.rb +++ b/spec/lib/pact_broker/versions/branch_version_repository_spec.rb @@ -62,10 +62,6 @@ module Versions end end end - - end end end - - From 7d6a0485ef54269faa8c7c46f9284ae74125f427 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Mon, 6 Sep 2021 09:48:13 +1000 Subject: [PATCH 44/45] style: whitespace --- spec/lib/pact_broker/db/data_migrations/create_branches_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/lib/pact_broker/db/data_migrations/create_branches_spec.rb b/spec/lib/pact_broker/db/data_migrations/create_branches_spec.rb index b7a1c5bcd..397dbc12f 100644 --- a/spec/lib/pact_broker/db/data_migrations/create_branches_spec.rb +++ b/spec/lib/pact_broker/db/data_migrations/create_branches_spec.rb @@ -4,7 +4,6 @@ module PactBroker module DB module DataMigrations describe CreateBranches do - let(:db) { PactBroker::Domain::Version.db } subject { CreateBranches.call(db) } From 7bc15792e7f9ed8a50a7b22d55c1563ccef9666e Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Mon, 6 Sep 2021 10:56:50 +1000 Subject: [PATCH 45/45] chore: update hal docs --- .../doc/views/index/pacticipant-branch-version.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pact_broker/doc/views/index/pacticipant-branch-version.markdown b/lib/pact_broker/doc/views/index/pacticipant-branch-version.markdown index a7fb5b15d..5e98a7fe4 100644 --- a/lib/pact_broker/doc/views/index/pacticipant-branch-version.markdown +++ b/lib/pact_broker/doc/views/index/pacticipant-branch-version.markdown @@ -11,4 +11,4 @@ Get or add/create a pacticipant version for a branch. Add a version to a branch. The pacticipant and branch are automatically created if they do not exist. curl -XPUT http://broker/pacticipants/Bar/branches/main/versions/1e70030c6579915e5ff56b107a0fd25cf5df7464 \ - -H "Content-Type: application/json" -d "" + -H "Content-Type: application/json" -d "{}"