From 798afce9217796f1b74292d27ab78a918d03afbf Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sun, 14 Feb 2021 13:15:47 +1100 Subject: [PATCH] feat: allow version to be created with tags --- .../api/decorators/version_decorator.rb | 13 ++++++- lib/pact_broker/api/resources/version.rb | 4 +-- lib/pact_broker/versions/repository.rb | 19 ++++++++--- lib/pact_broker/versions/service.rb | 4 +-- spec/features/create_tag_spec.rb | 32 +++++++++++++++++ spec/features/create_version_spec.rb | 34 ++++++++++++++++--- .../api/decorators/version_decorator_spec.rb | 18 ++++++++++ .../pact_broker/versions/repository_spec.rb | 18 +++++++--- 8 files changed, 125 insertions(+), 17 deletions(-) create mode 100644 spec/features/create_tag_spec.rb diff --git a/lib/pact_broker/api/decorators/version_decorator.rb b/lib/pact_broker/api/decorators/version_decorator.rb index 583df424e..8d4c455c6 100644 --- a/lib/pact_broker/api/decorators/version_decorator.rb +++ b/lib/pact_broker/api/decorators/version_decorator.rb @@ -10,7 +10,7 @@ class VersionDecorator < BaseDecorator property :branch property :build_url, as: :buildUrl - collection :tags, embedded: true, writeable: false, :extend => PactBroker::Api::Decorators::EmbeddedTagDecorator + collection :tags, embedded: true, :extend => PactBroker::Api::Decorators::EmbeddedTagDecorator, class: OpenStruct include Timestamps @@ -63,6 +63,17 @@ class VersionDecorator < BaseDecorator }] end + def from_hash(hash, options = {}) + if hash["tags"] + updated_hash = hash.dup + updated_hash["_embedded"] ||= {} + updated_hash["_embedded"]["tags"] = updated_hash.delete("tags") + super(updated_hash, options) + else + super + end + end + private def sorted_pacts diff --git a/lib/pact_broker/api/resources/version.rb b/lib/pact_broker/api/resources/version.rb index d04d762c0..d6a3e392f 100644 --- a/lib/pact_broker/api/resources/version.rb +++ b/lib/pact_broker/api/resources/version.rb @@ -24,8 +24,8 @@ def resource_exists? def from_json response_code = version ? 200 : 201 - parsed_version = Decorators::VersionDecorator.new(PactBroker::Domain::Version.new).from_json(request_body) - @version = version_service.create_or_update(pacticipant_name, pacticipant_version_number, parsed_version) + parsed_version = Decorators::VersionDecorator.new(OpenStruct.new).from_json(request_body) + @version = version_service.create_or_overwrite(pacticipant_name, pacticipant_version_number, parsed_version) response.body = to_json response_code end diff --git a/lib/pact_broker/versions/repository.rb b/lib/pact_broker/versions/repository.rb index 174b2afb8..9fe40c8d3 100644 --- a/lib/pact_broker/versions/repository.rb +++ b/lib/pact_broker/versions/repository.rb @@ -9,6 +9,7 @@ class Repository include PactBroker::Logging include PactBroker::Repositories::Helpers + include PactBroker::Repositories def find_by_pacticipant_id_and_number pacticipant_id, number PactBroker::Domain::Version.where(number: number, pacticipant_id: pacticipant_id).single_record @@ -60,13 +61,23 @@ def create args PactBroker::Domain::Version.new(version_params).upsert end - def create_or_update(pacticipant, version_number, version) - PactBroker::Domain::Version.new( + def create_or_overwrite(pacticipant, version_number, open_struct_version) + saved_version = PactBroker::Domain::Version.new( number: version_number, pacticipant: pacticipant, - build_url: version.build_url, - branch: version.branch + build_url: open_struct_version.build_url, + branch: open_struct_version.branch ).upsert + + if open_struct_version.tags + tag_repository.delete_by_version_id(saved_version.id) + open_struct_version.tags.collect do | open_struct_tag | + tag_repository.create(version: saved_version, name: open_struct_tag.name) + end + saved_version.refresh + end + + saved_version end def find_by_pacticipant_id_and_number_or_create pacticipant_id, number diff --git a/lib/pact_broker/versions/service.rb b/lib/pact_broker/versions/service.rb index b251264c5..2dc8c7327 100644 --- a/lib/pact_broker/versions/service.rb +++ b/lib/pact_broker/versions/service.rb @@ -18,9 +18,9 @@ def self.find_by_pacticipant_name_and_latest_tag(pacticipant_name, tag) version_repository.find_by_pacticipant_name_and_latest_tag(pacticipant_name, tag) end - def self.create_or_update(pacticipant_name, version_number, version) + def self.create_or_overwrite(pacticipant_name, version_number, version) pacticipant = pacticipant_repository.find_by_name_or_create(pacticipant_name) - version_repository.create_or_update(pacticipant, version_number, version) + version_repository.create_or_overwrite(pacticipant, version_number, version) end def self.delete version diff --git a/spec/features/create_tag_spec.rb b/spec/features/create_tag_spec.rb new file mode 100644 index 000000000..3c6436aa2 --- /dev/null +++ b/spec/features/create_tag_spec.rb @@ -0,0 +1,32 @@ +describe "Creating a tag" do + let(:path) { "/pacticipants/Foo/versions/1234/tags/foo" } + 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 + + it "returns the newly created tag" do + expect(response_body).to include name: "foo" + end + + context "when the tag already exists" do + before do + td.subtract_day + .create_consumer("Foo") + .create_consumer_version("1234") + .create_consumer_version_tag("foo") + end + + it "returns a 200 response" do + expect(subject.status).to be 200 + end + end +end diff --git a/spec/features/create_version_spec.rb b/spec/features/create_version_spec.rb index 79f9fdde4..3a7ed10e7 100644 --- a/spec/features/create_version_spec.rb +++ b/spec/features/create_version_spec.rb @@ -2,7 +2,13 @@ let(:path) { "/pacticipants/Foo/versions/1234" } let(:headers) { { 'CONTENT_TYPE' => 'application/json' } } let(:response_body) { JSON.parse(subject.body, symbolize_names: true)} - let(:version_hash) { { branch: "main", buildUrl: "http://build" } } + let(:version_hash) do + { + branch: "main", + buildUrl: "http://build", + tags: [{ name: "foo" }, { name: "bar" }] + } + end subject { put(path, version_hash.to_json, headers) } @@ -15,7 +21,12 @@ end it "returns the newly created version" do - expect(response_body).to include version_hash + expect(response_body).to include branch: "main", buildUrl: "http://build" + expect(response_body[:_embedded][:tags].size).to eq 2 + end + + it "creates the specified tags" do + expect { subject }.to change { PactBroker::Domain::Tag.count }.by(2) end context "when the version already exists" do @@ -28,13 +39,28 @@ let(:version_hash) { { branch: "new-branch" } } + it "returns a 200" do + expect(subject.status).to be 200 + end + it "overwrites the direct properties" do expect(response_body[:branch]).to eq "new-branch" expect(response_body).to_not have_key(:buildUrl) end - it "does not change the tags" do - expect { subject }.to_not change { PactBroker::Domain::Version.for("Foo", "1234").tags } + context "when not tags are specified" do + it "does not change the tags" do + expect { subject }.to_not change { PactBroker::Domain::Version.for("Foo", "1234").tags } + end + end + + context "when tags are specified" do + let(:version_hash) { { branch: "new-branch", tags: [ { name: "main" }] } } + + it "overwrites the tags" do + expect(response_body[:_embedded][:tags].size).to eq 1 + expect(response_body[:_embedded][:tags].first[:name]).to eq "main" + end end it "does not change the created date" do 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 858824be6..d1f49fac7 100644 --- a/spec/lib/pact_broker/api/decorators/version_decorator_spec.rb +++ b/spec/lib/pact_broker/api/decorators/version_decorator_spec.rb @@ -6,6 +6,24 @@ module Api module Decorators describe VersionDecorator do + describe "from_json" do + let(:hash) do + { + branch: "branch", + buildUrl: "buildUrl", + tags: [{ name: "main" }] + } + end + + 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 + end + let(:version) do TestDataBuilder.new .create_consumer("Consumer") diff --git a/spec/lib/pact_broker/versions/repository_spec.rb b/spec/lib/pact_broker/versions/repository_spec.rb index c088a3b30..396de5dcc 100644 --- a/spec/lib/pact_broker/versions/repository_spec.rb +++ b/spec/lib/pact_broker/versions/repository_spec.rb @@ -123,7 +123,7 @@ module Versions end end - describe "#create_or_update" do + describe "#create_or_overwrite" do before do td.subtract_day .create_consumer("Foo") @@ -133,9 +133,10 @@ module Versions let(:pacticipant) { td.and_return(:consumer) } let(:version_number) { "1234" } - let(:version) { PactBroker::Domain::Version.new(branch: "new-branch") } + let(:tags) { nil } + let(:open_struct_version) { OpenStruct.new(branch: "new-branch", tags: tags) } - subject { Repository.new.create_or_update(pacticipant, version_number, version) } + subject { Repository.new.create_or_overwrite(pacticipant, version_number, open_struct_version) } it "overwrites the values" do expect(subject.branch).to eq "new-branch" @@ -146,6 +147,15 @@ module Versions expect { subject }.to_not change { PactBroker::Domain::Version.for("Foo", "1234").tags } 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 @@ -159,7 +169,7 @@ module Versions end context "when the version does not already exist" do - let(:version) { PactBroker::Domain::Version.new(number: "555", branch: "new-branch") } + let(:version) { OpenStruct.new(number: "555", branch: "new-branch") } it "sets the order" do expect(subject.order).to_not be nil