From 1bb6088da7790c20405f0774ae497b94d6915774 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Wed, 13 Sep 2023 07:18:17 +1000 Subject: [PATCH] feat: add branch endpoint supporting GET and DELETE (#635) --- lib/pact_broker/api.rb | 1 + .../api/decorators/branch_decorator.rb | 29 ++++++++++++ .../decorators/branch_version_decorator.rb | 16 +++++++ .../embedded_branch_version_decorator.rb | 2 +- lib/pact_broker/api/pact_broker_urls.rb | 6 ++- lib/pact_broker/api/resources/branch.rb | 40 ++++++++++++++++ lib/pact_broker/repositories.rb | 9 ++++ lib/pact_broker/versions/branch_repository.rb | 28 +++++++++++ lib/pact_broker/versions/branch_service.rb | 27 +---------- .../versions/branch_version_repository.rb | 17 +++++++ spec/features/delete_branch_spec.rb | 46 +++++++++++++++++++ spec/features/get_branch_spec.rb | 12 +++++ .../versions/branch_repository_spec.rb | 38 +++++++++++++++ spec/support/all_routes_spec_support.yml | 2 + 14 files changed, 246 insertions(+), 27 deletions(-) create mode 100644 lib/pact_broker/api/decorators/branch_decorator.rb create mode 100644 lib/pact_broker/api/resources/branch.rb create mode 100644 lib/pact_broker/versions/branch_repository.rb create mode 100644 spec/features/delete_branch_spec.rb create mode 100644 spec/features/get_branch_spec.rb create mode 100644 spec/lib/pact_broker/versions/branch_repository_spec.rb diff --git a/lib/pact_broker/api.rb b/lib/pact_broker/api.rb index ba98e6c99..1847108ee 100644 --- a/lib/pact_broker/api.rb +++ b/lib/pact_broker/api.rb @@ -95,6 +95,7 @@ def self.build_api(application_context = PactBroker::ApplicationContext.default_ add ["pacticipants", :pacticipant_name, "latest-version", :tag, "can-i-deploy", "to", :to, "badge"], Api::Resources::CanIDeployPacticipantVersionByTagToTagBadge, { resource_name: "can_i_deploy_latest_tagged_version_to_tag_badge" } 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, "branches", :branch_name], Api::Resources::Branch, { resource_name: "branch" } add ["pacticipants", :pacticipant_name, "branches", :branch_name, "versions", :version_number], Api::Resources::BranchVersion, { resource_name: "branch_version" } add ["pacticipants", :pacticipant_name, "branches", :branch_name, "latest-version", "can-i-deploy", "to-environment", :environment_name], Api::Resources::CanIDeployPacticipantVersionByBranchToEnvironment, { resource_name: "can_i_deploy_latest_branch_version_to_environment" } add ["pacticipants", :pacticipant_name, "branches", :branch_name, "latest-version", "can-i-deploy", "to-environment", :environment_name, "badge"], Api::Resources::CanIDeployPacticipantVersionByBranchToEnvironmentBadge, { resource_name: "can_i_deploy_latest_branch_version_to_environment_badge" } diff --git a/lib/pact_broker/api/decorators/branch_decorator.rb b/lib/pact_broker/api/decorators/branch_decorator.rb new file mode 100644 index 000000000..759687e57 --- /dev/null +++ b/lib/pact_broker/api/decorators/branch_decorator.rb @@ -0,0 +1,29 @@ +require "pact_broker/api/decorators/base_decorator" +require "pact_broker/api/decorators/timestamps" + +module PactBroker + module Api + module Decorators + class BranchDecorator < BaseDecorator + + property :name + + link :self do | user_options | + { + title: "Branch", + href: branch_url(represented, user_options.fetch(:base_url)) + } + end + + link "pb:latest-version" do | user_options | + { + title: "Latest version for branch", + href: branch_versions_url(represented, user_options.fetch(:base_url)) + "?pageSize=1" + } + end + + include Timestamps + end + end + end +end diff --git a/lib/pact_broker/api/decorators/branch_version_decorator.rb b/lib/pact_broker/api/decorators/branch_version_decorator.rb index e59a4d6ad..8c5dbd9ad 100644 --- a/lib/pact_broker/api/decorators/branch_version_decorator.rb +++ b/lib/pact_broker/api/decorators/branch_version_decorator.rb @@ -13,6 +13,22 @@ class BranchVersionDecorator < BaseDecorator } end + link :"pb:branch" do | user_options | + { + title: "Branch", + name: represented.branch.name, + href: branch_url(represented.branch, user_options.fetch(:base_url)) + } + end + + link :"pb:version" do | user_options | + { + title: "Version", + name: represented.version.number, + href: version_url(user_options.fetch(:base_url), represented.version) + } + end + include Timestamps end end 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 68a84595f..133691f17 100644 --- a/lib/pact_broker/api/decorators/embedded_branch_version_decorator.rb +++ b/lib/pact_broker/api/decorators/embedded_branch_version_decorator.rb @@ -10,7 +10,7 @@ class EmbeddedBranchVersionDecorator < BaseDecorator link :self do | options | { - title: "Version branch", + title: "Branch version", name: represented.branch_name, href: branch_version_url(represented, options[:base_url]) } diff --git a/lib/pact_broker/api/pact_broker_urls.rb b/lib/pact_broker/api/pact_broker_urls.rb index dbff67645..01f4f1412 100644 --- a/lib/pact_broker/api/pact_broker_urls.rb +++ b/lib/pact_broker/api/pact_broker_urls.rb @@ -227,8 +227,12 @@ def tag_url base_url, tag "#{tags_url(base_url, tag.version)}/#{url_encode(tag.name)}" end + def branch_url(branch, base_url = "") + "#{pacticipant_url(base_url, branch.pacticipant)}/branches/#{url_encode(branch.name)}" + end + def branch_versions_url(branch, base_url = "") - "#{pacticipant_url(base_url, branch.pacticipant)}/branches/#{url_encode(branch.name)}/versions" + "#{branch_url(branch, base_url)}/versions" end def branch_version_url(branch_version, base_url = "") diff --git a/lib/pact_broker/api/resources/branch.rb b/lib/pact_broker/api/resources/branch.rb new file mode 100644 index 000000000..38125d16e --- /dev/null +++ b/lib/pact_broker/api/resources/branch.rb @@ -0,0 +1,40 @@ +require "pact_broker/api/resources/base_resource" + +module PactBroker + module Api + module Resources + class Branch < BaseResource + def content_types_provided + [["application/hal+json", :to_json]] + end + + def allowed_methods + ["GET", "DELETE", "OPTIONS"] + end + + def resource_exists? + !!branch + end + + def to_json + decorator_class(:branch_decorator).new(branch).to_json(**decorator_options) + end + + def delete_resource + branch_service.delete_branch(branch) + true + end + + def policy_name + :'versions::branch' + end + + private + + def branch + @branch_version ||= branch_service.find_branch(**identifier_from_path.slice(:pacticipant_name, :branch_name)) + end + end + end + end +end diff --git a/lib/pact_broker/repositories.rb b/lib/pact_broker/repositories.rb index 184c19be1..6b8f86365 100644 --- a/lib/pact_broker/repositories.rb +++ b/lib/pact_broker/repositories.rb @@ -47,6 +47,10 @@ def matrix_repository get_repository(:matrix_repository) end + def branch_repository + get_repository(:branch_repository) + end + def branch_version_repository get_repository(:branch_version_repository) end @@ -96,6 +100,11 @@ def register_default_repositories Matrix::Repository.new end + register_repository(:branch_repository) do + require "pact_broker/versions/branch_repository" + PactBroker::Versions::BranchRepository.new + end + register_repository(:branch_version_repository) do require "pact_broker/versions/branch_version_repository" PactBroker::Versions::BranchVersionRepository.new diff --git a/lib/pact_broker/versions/branch_repository.rb b/lib/pact_broker/versions/branch_repository.rb new file mode 100644 index 000000000..3635413eb --- /dev/null +++ b/lib/pact_broker/versions/branch_repository.rb @@ -0,0 +1,28 @@ +module PactBroker + module Versions + class BranchRepository + include PactBroker::Services + + # @param [String] pacticipant_name + # @param [String] branch_name + # @return [PactBroker::Versions::Branch, nil] + def find_branch(pacticipant_name:, branch_name:) + Branch + .select_all_qualified + .join(:pacticipants, { Sequel[:branches][:pacticipant_id] => Sequel[:pacticipants][:id] }) do + Sequel.name_like(Sequel[:pacticipants][:name], pacticipant_name) + end + .where(Sequel[:branches][:name] => branch_name) + .single_record + end + + # Deletes a branch, its branch head and branch_version objects, without deleting the + # pacticipant version objects + # + # @param [PactBroker::Versions::Branch] the branch to delete + def delete_branch(branch) + branch.delete + end + end + end +end diff --git a/lib/pact_broker/versions/branch_service.rb b/lib/pact_broker/versions/branch_service.rb index 439ef9299..de5c2903e 100644 --- a/lib/pact_broker/versions/branch_service.rb +++ b/lib/pact_broker/versions/branch_service.rb @@ -10,31 +10,8 @@ class BranchService class << self extend Forwardable - delegate [:delete_branch_version] => :branch_version_repository - end - - - 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_version_repository.add_branch(version, branch_name) - end - - def self.find_branch(pacticipant_name:, branch_name:) - Branch - .select_all_qualified - .join(:pacticipants, { Sequel[:branches][:pacticipant_id] => Sequel[:pacticipants][:id] }) do - Sequel.name_like(Sequel[:pacticipants][:name], pacticipant_name) - end - .where(Sequel[:branches][:name] => branch_name) - .single_record + delegate [:find_branch_version, :find_or_create_branch_version, :delete_branch_version] => :branch_version_repository + delegate [:find_branch, :delete_branch] => :branch_repository end end end diff --git a/lib/pact_broker/versions/branch_version_repository.rb b/lib/pact_broker/versions/branch_version_repository.rb index 7d14f1fc5..1b69e2910 100644 --- a/lib/pact_broker/versions/branch_version_repository.rb +++ b/lib/pact_broker/versions/branch_version_repository.rb @@ -1,7 +1,24 @@ +require "pact_broker/versions/branch_version" +require "pact_broker/services" + module PactBroker module Versions class BranchVersionRepository include PactBroker::Services + include PactBroker::Repositories + + def 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 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_version_repository.add_branch(version, branch_name) + end def add_branch(version, branch_name, auto_created: false) branch = find_or_create_branch(version.pacticipant, branch_name) diff --git a/spec/features/delete_branch_spec.rb b/spec/features/delete_branch_spec.rb new file mode 100644 index 000000000..b67ef29b6 --- /dev/null +++ b/spec/features/delete_branch_spec.rb @@ -0,0 +1,46 @@ +describe "Deleting a branch (removing all versions from a branch)" do + before do + td.create_consumer("foo") + .create_consumer_version("1234", branch: "main") + .create_consumer_version("1234", branch: "not-main") + .create_consumer_version("555", branch: "main") + .create_consumer("bar") + .create_consumer_version("1234", branch: "main") + end + + let(:path) { "/pacticipants/foo/branches/main" } + let(:headers) { {} } + let(:response_body) { JSON.parse(subject.body, symbolize_names: true) } + + subject { delete(path, nil, headers) } + + it "returns a 204 response" do + expect(subject.status).to be 204 + end + + it "deletes the branch" do + expect { subject }.to change { PactBroker::Versions::Branch.count }.by(-1) + end + + it "does not delete the pacticipant versions" do + expect { subject }.to_not change { PactBroker::Domain::Version.count } + end + + context "when the branch version does not exist" do + let(:path) { "/pacticipants/waffle/branches/main" } + + its(:status) { is_expected.to eq 404 } + end + + context "when there is some flag to indicate that the versions should be deleted too" do + subject { delete(path, { deletedAssociatedVersions: true }, headers) } + + it "deletes the branch" do + expect { subject }.to change { PactBroker::Versions::Branch.count }.by(-1) + end + + it "DOES delete the pacticipant versions", pending: true do + expect { subject }.to change { PactBroker::Domain::Version.count }.by(-2) + end + end +end diff --git a/spec/features/get_branch_spec.rb b/spec/features/get_branch_spec.rb new file mode 100644 index 000000000..d3e234689 --- /dev/null +++ b/spec/features/get_branch_spec.rb @@ -0,0 +1,12 @@ +describe "Get a branch" do + before do + td.create_consumer("Foo") + .create_consumer_version("1234", branch: "main") + end + let(:path) { PactBroker::Api::PactBrokerUrls.branch_url(PactBroker::Versions::Branch.first) } + let(:headers) { { "CONTENT_TYPE" => "application/json" } } + + subject { get(path, nil, headers) } + + it { is_expected.to be_a_hal_json_success_response } +end diff --git a/spec/lib/pact_broker/versions/branch_repository_spec.rb b/spec/lib/pact_broker/versions/branch_repository_spec.rb new file mode 100644 index 000000000..b966bbe50 --- /dev/null +++ b/spec/lib/pact_broker/versions/branch_repository_spec.rb @@ -0,0 +1,38 @@ +require "pact_broker/versions/branch_repository" + +module PactBroker + module Versions + describe BranchRepository do + describe "delete_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: "not-main") + .create_consumer("bar") + .create_consumer_version("1", branch: "main") + end + + let(:branch) { BranchRepository.new.find_branch(pacticipant_name: "foo", branch_name: "main") } + + subject { BranchRepository.new.delete_branch(branch) } + + it "deletes the branch" do + expect{ subject }.to change { Branch.count }.by(-1) + end + + it "deletes the branch versions" do + expect{ subject }.to change { BranchVersion.count }.by(-2) + end + + it "deletes the branch head" do + expect{ subject }.to change { BranchHead.count }.by(-1) + end + + it "does not delete the versions" do + expect{ subject }.to_not change { PactBroker::Domain::Version.count } + end + end + end + end +end diff --git a/spec/support/all_routes_spec_support.yml b/spec/support/all_routes_spec_support.yml index a40240834..92cf0845c 100644 --- a/spec/support/all_routes_spec_support.yml +++ b/spec/support/all_routes_spec_support.yml @@ -31,6 +31,8 @@ requests_which_are_exected_to_have_no_policy_record: - pacticipant DELETE - can_i_deploy_latest_branch_version_to_environment GET - can_i_deploy_latest_branch_version_to_environment_badge GET + - branch GET + - branch DELETE - branch_version GET - branch_version PUT - branch_version DELETE