diff --git a/db/migrations/20210216_add_pacticipant_columns.rb b/db/migrations/20210216_add_pacticipant_columns.rb new file mode 100644 index 000000000..b91d21ef7 --- /dev/null +++ b/db/migrations/20210216_add_pacticipant_columns.rb @@ -0,0 +1,10 @@ +Sequel.migration do + change do + alter_table(:pacticipants) do + add_column(:display_name, String) + add_column(:repository_name, String) + add_column(:repository_namespace, String) + add_column(:main_development_branches, String) + end + end +end diff --git a/lib/pact_broker/api.rb b/lib/pact_broker/api.rb index 9fb2505ed..b4240544e 100644 --- a/lib/pact_broker/api.rb +++ b/lib/pact_broker/api.rb @@ -11,6 +11,15 @@ class Request def patch? method == "PATCH" end + + # This makes PATCH go through the PUT state machine path + def put? + method == "PUT" || method == "PATCH" + end + + def really_put? + method == "PUT" + end end end diff --git a/lib/pact_broker/api/contracts/dry_validation_workarounds.rb b/lib/pact_broker/api/contracts/dry_validation_workarounds.rb index c2487828a..3c1a8390b 100644 --- a/lib/pact_broker/api/contracts/dry_validation_workarounds.rb +++ b/lib/pact_broker/api/contracts/dry_validation_workarounds.rb @@ -14,8 +14,13 @@ def select_first_message(messages) end def flatten_array_of_hashes(array_of_hashes) - new_messages = array_of_hashes.collect do | index, hash | - hash.values.flatten.collect { | text | "#{text} at index #{index}"} + new_messages = array_of_hashes.collect do | index, hash_or_array | + array = if hash_or_array.is_a?(Hash) + hash_or_array.values.flatten + else + hash_or_array + end + array.collect { | text | "#{text} at index #{index}"} end.flatten end diff --git a/lib/pact_broker/api/contracts/pacticipant_schema.rb b/lib/pact_broker/api/contracts/pacticipant_schema.rb new file mode 100644 index 000000000..677e6c509 --- /dev/null +++ b/lib/pact_broker/api/contracts/pacticipant_schema.rb @@ -0,0 +1,34 @@ +require 'dry-validation' +require 'pact_broker/api/contracts/dry_validation_workarounds' +require 'pact_broker/api/contracts/dry_validation_predicates' +require 'pact_broker/messages' + +module PactBroker + module Api + module Contracts + class PacticipantSchema + extend DryValidationWorkarounds + extend PactBroker::Messages + using PactBroker::HashRefinements + + SCHEMA = Dry::Validation.Schema do + configure do + predicates(DryValidationPredicates) + config.messages_file = File.expand_path("../../../locale/en.yml", __FILE__) + end + optional(:name).filled(:str?, :single_line?) + optional(:displayName).maybe(:str?, :single_line?, :not_blank?) + optional(:mainDevelopmentBranches).each(:str?, :single_line?, :no_spaces?) + optional(:repositoryUrl).maybe(:str?, :single_line?) + optional(:repositoryName).maybe(:str?, :single_line?) + optional(:repositoryNamespace).maybe(:str?, :single_line?) + end + + def self.call(params_with_string_keys) + params = params_with_string_keys&.symbolize_keys + select_first_message(flatten_indexed_messages(SCHEMA.call(params).messages(full: true))) + end + end + end + end +end diff --git a/lib/pact_broker/api/decorators/pacticipant_decorator.rb b/lib/pact_broker/api/decorators/pacticipant_decorator.rb index 4345941f6..0ef0aff69 100644 --- a/lib/pact_broker/api/decorators/pacticipant_decorator.rb +++ b/lib/pact_broker/api/decorators/pacticipant_decorator.rb @@ -8,9 +8,12 @@ module PactBroker module Api module Decorators class PacticipantDecorator < BaseDecorator - property :name - property :repository_url, as: :repositoryUrl + property :display_name, camelize: true + property :repository_url, camelize: true + property :repository_name, camelize: true + property :repository_namespace, camelize: true + property :main_development_branches, camelize: true property :latest_version, as: :latestVersion, :class => PactBroker::Domain::Version, extend: PactBroker::Api::Decorators::EmbeddedVersionDecorator, embedded: true, writeable: false collection :labels, :class => PactBroker::Domain::Label, extend: PactBroker::Api::Decorators::EmbeddedLabelDecorator, embedded: true diff --git a/lib/pact_broker/api/resources/default_base_resource.rb b/lib/pact_broker/api/resources/default_base_resource.rb index e901a637b..2bfe11bcb 100644 --- a/lib/pact_broker/api/resources/default_base_resource.rb +++ b/lib/pact_broker/api/resources/default_base_resource.rb @@ -249,8 +249,12 @@ def decorator_class(name) application_context.decorator_configuration.class_for(name) end - def validation_errors_for_schema?(schema, params_to_validate = params) - if (errors = schema.call(params_to_validate)).any? + def schema + nil + end + + def validation_errors_for_schema?(schema_to_use = schema, params_to_validate = params) + if (errors = schema_to_use.call(params_to_validate)).any? set_json_validation_error_messages(errors) true else diff --git a/lib/pact_broker/api/resources/pacticipant.rb b/lib/pact_broker/api/resources/pacticipant.rb index a6b2eae3c..35db96806 100644 --- a/lib/pact_broker/api/resources/pacticipant.rb +++ b/lib/pact_broker/api/resources/pacticipant.rb @@ -1,12 +1,5 @@ require 'pact_broker/api/resources/base_resource' - -module Webmachine - class Request - def put? - method == "PUT" || method == "PATCH" - end - end -end +require 'pact_broker/api/contracts/pacticipant_schema' module PactBroker module Api @@ -22,19 +15,31 @@ def content_types_accepted end def allowed_methods - ["GET", "PATCH", "DELETE", "OPTIONS"] + ["GET", "PUT", "PATCH", "DELETE", "OPTIONS"] end def known_methods super + ['PATCH'] end + def malformed_request? + if request.patch? || request.put? + invalid_json? || validation_errors_for_schema? + else + false + end + end + def from_json if pacticipant - @pacticipant = pacticipant_service.update params(symbolize_names: false).merge('name' => pacticipant_name) + @pacticipant = update_existing_pacticipant else - @pacticipant = pacticipant_service.create params.merge(:name => pacticipant_name) - response.headers["Location"] = pacticipant_url(base_url, pacticipant) + if request.patch? # for backwards compatibility, wish I hadn't done this + @pacticipant = create_new_pacticipant + response.headers["Location"] = pacticipant_url(base_url, pacticipant) + else + return 404 + end end response.body = to_json end @@ -52,9 +57,29 @@ def to_json decorator_class(:pacticipant_decorator).new(pacticipant).to_json(decorator_options) end + def parsed_pacticipant(pacticipant) + decorator_class(:pacticipant_decorator).new(pacticipant).from_json(request_body) + end + def policy_name :'pacticipants::pacticipant' end + + def schema + PactBroker::Api::Contracts::PacticipantSchema + end + + def update_existing_pacticipant + if request.really_put? + @pacticipant = pacticipant_service.replace(pacticipant_name, parsed_pacticipant(OpenStruct.new)) + else + @pacticipant = pacticipant_service.update(pacticipant_name, parsed_pacticipant(pacticipant)) + end + end + + def create_new_pacticipant + pacticipant_service.create parsed_pacticipant(OpenStruct.new).to_h.merge(:name => pacticipant_name) + end end end end diff --git a/lib/pact_broker/api/resources/pacticipants.rb b/lib/pact_broker/api/resources/pacticipants.rb index f34e188b8..3517ea136 100644 --- a/lib/pact_broker/api/resources/pacticipants.rb +++ b/lib/pact_broker/api/resources/pacticipants.rb @@ -2,6 +2,7 @@ require 'pact_broker/api/decorators/pacticipant_decorator' require 'pact_broker/domain/pacticipant' require 'pact_broker/hash_refinements' +require 'pact_broker/api/contracts/pacticipant_schema' module PactBroker module Api @@ -23,7 +24,7 @@ def allowed_methods def malformed_request? if request.post? - return invalid_json? || validation_errors?(new_model) + return invalid_json? || validation_errors_for_schema? end false end @@ -33,7 +34,7 @@ def post_is_create? end def from_json - created_model = pacticipant_service.create(params.symbolize_keys.snakecase_keys.slice(:name, :repository_url)) + created_model = pacticipant_service.create(parsed_pacticipant.to_h) response.body = decorator_for(created_model).to_json(decorator_options) end @@ -53,8 +54,8 @@ def decorator_for model decorator_class(:pacticipant_decorator).new(model) end - def new_model - @new_model ||= decorator_for(PactBroker::Domain::Pacticipant.new).from_json(request.body.to_s) + def parsed_pacticipant + @new_model ||= decorator_for(OpenStruct.new).from_json(request_body) end def policy_name @@ -63,6 +64,10 @@ def policy_name private + def schema + PactBroker::Api::Contracts::PacticipantSchema + end + def pacticipants @pacticipants ||= pacticipant_service.find_all_pacticipants end diff --git a/lib/pact_broker/db/data_migrations/set_pacticipant_display_name.rb b/lib/pact_broker/db/data_migrations/set_pacticipant_display_name.rb new file mode 100644 index 000000000..c7a32903d --- /dev/null +++ b/lib/pact_broker/db/data_migrations/set_pacticipant_display_name.rb @@ -0,0 +1,23 @@ +require 'pact_broker/db/data_migrations/helpers' +require 'pact_broker/pacticipants/generate_display_name' + +module PactBroker + module DB + module DataMigrations + class SetPacticipantDisplayName + extend Helpers + extend PactBroker::Pacticipants::GenerateDisplayName + + def self.call(connection) + if columns_exist?(connection, :pacticipants, [:name, :display_name]) + connection[:pacticipants].where(display_name: nil).each do | row | + connection[:pacticipants] + .where(id: row[:id]) + .update(display_name: generate_display_name(row[:name])) + end + 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 906eeb3d4..224835dcc 100644 --- a/lib/pact_broker/db/migrate_data.rb +++ b/lib/pact_broker/db/migrate_data.rb @@ -22,6 +22,7 @@ def self.call database_connection, options = {} DataMigrations::SetCreatedAtForLatestPactPublications.call(database_connection) DataMigrations::SetCreatedAtForLatestVerifications.call(database_connection) DataMigrations::SetExtraColumnsForTags.call(database_connection) + DataMigrations::SetPacticipantDisplayName.call(database_connection) end end end diff --git a/lib/pact_broker/deployments/environment.rb b/lib/pact_broker/deployments/environment.rb index 32dfe5c37..b6101ccb2 100644 --- a/lib/pact_broker/deployments/environment.rb +++ b/lib/pact_broker/deployments/environment.rb @@ -5,7 +5,7 @@ module PactBroker module Deployments class Environment < Sequel::Model - OPEN_STRUCT_TO_JSON = lambda { |thing| Sequel.object_to_json(thing.collect(&:to_h)) } + OPEN_STRUCT_TO_JSON = lambda { | open_struct | Sequel.object_to_json(open_struct.collect(&:to_h)) } JSON_TO_OPEN_STRUCT = lambda { | json | Sequel.parse_json(json).collect{ | hash| OpenStruct.new(hash) } } plugin :upsert, identifying_columns: [:uuid] plugin :serialization diff --git a/lib/pact_broker/domain/pacticipant.rb b/lib/pact_broker/domain/pacticipant.rb index 410133c7f..e087aca74 100644 --- a/lib/pact_broker/domain/pacticipant.rb +++ b/lib/pact_broker/domain/pacticipant.rb @@ -3,14 +3,22 @@ require 'pact_broker/repositories/helpers' require 'pact_broker/versions/latest_version' require 'pact_broker/domain/label' +require 'pact_broker/string_refinements' +require 'pact_broker/pacticipants/generate_display_name' module PactBroker module Domain class Pacticipant < Sequel::Model - include Messages - plugin :insert_ignore, identifying_columns: [:name] + include PactBroker::Pacticipants::GenerateDisplayName + using PactBroker::StringRefinements + + plugin :serialization + SPACE_DELIMITED_STRING_TO_ARRAY = lambda { |string| string.split(" ") } + ARRAY_TO_SPACE_DELIMITED_STRING = lambda { |array| array.join(" ") } + serialize_attributes [ARRAY_TO_SPACE_DELIMITED_STRING, SPACE_DELIMITED_STRING_TO_ARRAY], :main_development_branches + plugin :insert_ignore, identifying_columns: [:name] plugin :timestamps, update_on_create: true set_primary_key :id @@ -42,6 +50,13 @@ def before_destroy super end + def before_save + super + if display_name.nil? || display_name.to_s.blank? + self.display_name = generate_display_name(name) + end + end + def latest_version versions.last end @@ -50,12 +65,6 @@ def to_s "Pacticipant: id=#{id}, name=#{name}" end - def validate - messages = [] - messages << message('errors.validation.attribute_missing', attribute: 'name') unless name - messages - end - def any_versions? PactBroker::Domain::Version.where(pacticipant: self).any? end diff --git a/lib/pact_broker/pacticipants/generate_display_name.rb b/lib/pact_broker/pacticipants/generate_display_name.rb new file mode 100644 index 000000000..ce87cafa2 --- /dev/null +++ b/lib/pact_broker/pacticipants/generate_display_name.rb @@ -0,0 +1,27 @@ +require 'pact_broker/string_refinements' + +module PactBroker + module Pacticipants + module GenerateDisplayName + using PactBroker::StringRefinements + + def self.call(name) + return nil if name.nil? + name + .to_s + .gsub(/([A-Z])([A-Z])([a-z])/,'\1 \2\3') + .gsub(/([a-z\d])([A-Z])(\S)/,'\1 \2\3') + .gsub(/(\S)([\-_\s\.])(\S)/, '\1 \3') + .gsub(/\s+/, " ") + .strip + .split(" ") + .collect{ |word| word.camelcase(true) } + .join(" ") + end + + def generate_display_name(name) + GenerateDisplayName.call(name) + end + end + end +end diff --git a/lib/pact_broker/pacticipants/repository.rb b/lib/pact_broker/pacticipants/repository.rb index f38d3076f..4a112ed5e 100644 --- a/lib/pact_broker/pacticipants/repository.rb +++ b/lib/pact_broker/pacticipants/repository.rb @@ -47,21 +47,37 @@ def find_by_name_or_create name if pacticipant = find_by_name(name) pacticipant else - create name: name + create(name: name) end end # Need to be able to handle two calls that make the pacticipant at the same time. # TODO raise error if attributes apart from name are different, because this indicates that # the second request is not at the same time. - def create args + def create params PactBroker::Domain::Pacticipant.new( - name: args[:name], - repository_url: args[:repository_url], - created_at: Sequel.datetime_class.now, - updated_at: Sequel.datetime_class.now + name: params.fetch(:name), + display_name: params[:display_name], + repository_url: params[:repository_url], + repository_name: params[:repository_name], + repository_namespace: params[:repository_namespace], + main_development_branches: params[:main_development_branches] || [] ).insert_ignore - PactBroker::Domain::Pacticipant.find(name: args[:name]) + end + + def update(pacticipant_name, pacticipant) + pacticipant.name = pacticipant_name + pacticipant.save + end + + def replace(pacticipant_name, open_struct_pacticipant) + PactBroker::Domain::Pacticipant.new( + display_name: open_struct_pacticipant.display_name, + repository_url: open_struct_pacticipant.repository_url, + repository_name: open_struct_pacticipant.repository_name, + repository_namespace: open_struct_pacticipant.repository_namespace, + main_development_branches: open_struct_pacticipant.main_development_branches || [] + ).save end def pacticipant_names diff --git a/lib/pact_broker/pacticipants/service.rb b/lib/pact_broker/pacticipants/service.rb index f070a5b9b..fb6d2c889 100644 --- a/lib/pact_broker/pacticipants/service.rb +++ b/lib/pact_broker/pacticipants/service.rb @@ -12,7 +12,7 @@ class Service extend PactBroker::Services include PactBroker::Logging - def self.messages_for_potential_duplicate_pacticipants pacticipant_names, base_url + def self.messages_for_potential_duplicate_pacticipants(pacticipant_names, base_url) messages = [] pacticipant_names.each do | name | potential_duplicate_pacticipants = find_potential_duplicate_pacticipants(name) @@ -36,27 +36,27 @@ def self.find_all_pacticipants pacticipant_repository.find_all end - def self.find_pacticipant_by_name name + def self.find_pacticipant_by_name(name) pacticipant_repository.find_by_name(name) end - def self.find_pacticipant_by_name! name + def self.find_pacticipant_by_name!(name) pacticipant_repository.find_by_name!(name) end - def self.find_by_id id + def self.find_by_id(id) pacticipant_repository.find_by_id(id) end - def self.find options + def self.find(options) pacticipant_repository.find options end - def self.find_all_pacticipant_versions_in_reverse_order name, pagination_options = nil + def self.find_all_pacticipant_versions_in_reverse_order(name, pagination_options = nil) pacticipant_repository.find_all_pacticipant_versions_in_reverse_order(name, pagination_options) end - def self.find_pacticipant_repository_url_by_pacticipant_name name + def self.find_pacticipant_repository_url_by_pacticipant_name(name) pacticipant = pacticipant_repository.find_by_name(name) if pacticipant && pacticipant.repository_url pacticipant.repository_url @@ -65,19 +65,19 @@ def self.find_pacticipant_repository_url_by_pacticipant_name name end end - def self.update params - # TODO move this to the repository! - pacticipant = pacticipant_repository.find_by_name(params.fetch('name')) - PactBroker::Api::Decorators::PacticipantDecorator.new(pacticipant).from_hash(params) - pacticipant.save - pacticipant_repository.find_by_name(params.fetch('name')) + def self.update(pacticipant_name, pacticipant) + pacticipant_repository.update(pacticipant_name, pacticipant) end - def self.create params + def self.create(params) pacticipant_repository.create(params) end - def self.delete name + def self.replace(pacticipant_name, open_struct_pacticipant) + pacticipant_repository.replace(pacticipant_name, open_struct_pacticipant) + end + + def self.delete(name) pacticipant = find_pacticipant_by_name name webhook_service.delete_all_webhhook_related_objects_by_pacticipant(pacticipant) pacticipant.destroy diff --git a/lib/pact_broker/test/http_test_data_builder.rb b/lib/pact_broker/test/http_test_data_builder.rb index a116d6c0d..53367c3d3 100644 --- a/lib/pact_broker/test/http_test_data_builder.rb +++ b/lib/pact_broker/test/http_test_data_builder.rb @@ -62,7 +62,7 @@ def deploy_to_prod(pacticipant:, version:) def create_pacticipant(name) puts "Creating pacticipant with name #{name}" - client.post("pacticipants", { name: name}).tap { |response| check_for_error(response) } + client.post("pacticipants", { name: name }).tap { |response| check_for_error(response) } separate self end diff --git a/lib/pact_broker/test/test_data_builder.rb b/lib/pact_broker/test/test_data_builder.rb index c86be3b1b..a53c5dfa0 100644 --- a/lib/pact_broker/test/test_data_builder.rb +++ b/lib/pact_broker/test/test_data_builder.rb @@ -127,7 +127,9 @@ def create_tag_with_hierarchy pacticipant_name, pacticipant_version, tag_name def create_pacticipant pacticipant_name, params = {} params.delete(:comment) - @pacticipant = PactBroker::Domain::Pacticipant.create({ :name => pacticipant_name, repository_url: "https://github.com/example-organization/#{pacticipant_name}" }.merge(params)) + repository_url = "https://github.com/#{params[:repository_namespace] || "example-organization"}/#{params[:repository_name] || pacticipant_name}" + merged_params = { name: pacticipant_name, repository_url: repository_url }.merge(params) + @pacticipant = PactBroker::Domain::Pacticipant.create(merged_params) self end diff --git a/spec/features/create_pacticipant_spec.rb b/spec/features/create_pacticipant_spec.rb index f3d191047..9a3ccaa03 100644 --- a/spec/features/create_pacticipant_spec.rb +++ b/spec/features/create_pacticipant_spec.rb @@ -1,12 +1,18 @@ -require 'support/test_data_builder' - describe "Creating a pacticipant" do let(:path) { "/pacticipants" } let(:headers) { {'CONTENT_TYPE' => 'application/json'} } let(:response_body) { JSON.parse(last_response.body, symbolize_names: true)} - let(:pacticipant_hash) { { name: 'Foo Thing' } } + let(:pacticipant_hash) do + { + name: 'Foo Thing', + mainDevelopmentBranches: ["main"], + repositoryUrl: "http://url", + repositoryName: "foo-thing", + repositoryNamespace: "some-group" + } + end - subject { post path, pacticipant_hash.to_json, headers; last_response } + subject { post(path, pacticipant_hash.to_json, headers) } it "returns a 201 response" do subject diff --git a/spec/features/update_pacticipant_spec.rb b/spec/features/update_pacticipant_spec.rb index 361623011..67d6b73c5 100644 --- a/spec/features/update_pacticipant_spec.rb +++ b/spec/features/update_pacticipant_spec.rb @@ -1,23 +1,70 @@ -describe "Publishing a pact" do +describe "Update a pacticipant" do let(:request_body) { {'repositoryUrl' => 'http://foo'} } let(:path) { "/pacticipants/Some%20Consumer" } - let(:response_body_json) { JSON.parse(subject.body) } + let(:response_body_hash) { JSON.parse(subject.body, symbolize_names: true) } - subject { patch path, request_body.to_json, {'CONTENT_TYPE' => 'application/json' }; last_response } + context "with a PUT" do + let(:request_body_hash) do + { + repositoryUrl: "http://foo", + repositoryName: "name", + repositoryNamespace: "org", + mainDevelopmentBranches: ["main"] + } + end + + subject { put(path, request_body_hash.to_json, {'CONTENT_TYPE' => 'application/json' }) } + + context "when the pacticipant exists" do + before do + td.create_pacticipant("Some Consumer") + end - context "when the pacticipant exists" do + it { is_expected.to be_a_hal_json_success_response } - before do - TestDataBuilder.new.create_pacticipant("Some Consumer") + it "updates the properties" do + expect(response_body_hash).to include(request_body_hash) + end + + context "with only some of the properties set" do + let(:request_body_hash) do + { + repositoryUrl: "http://foo", + } + end + + it "blanks out the missing ones" do + expect(response_body_hash[:repositoryName]).to be nil + end + end end - it "returns a 200 OK" do - puts subject.body unless subject.status == 200 - expect(subject.status).to be 200 + + context "when the pacticipant does not exist" do + it { is_expected.to be_a_404_response } end + end + + context "with a PATCH" do + subject { patch(path, request_body.to_json, {'CONTENT_TYPE' => 'application/json' }) } + + context "when the pacticipant exists" do + before do + td.create_pacticipant("Some Consumer", repository_name: "existing") + end + + it "returns a 200 OK" do + puts subject.body unless subject.status == 200 + expect(subject.status).to be 200 + end + + it "leaves any existing properties that were not defined" do + expect(response_body_hash[:repositoryName]).to eq "existing" + end - it "returns a json body with the updated pacticipant" do - expect(subject.headers['Content-Type']).to eq "application/hal+json;charset=utf-8" + it "returns a json body with the updated pacticipant" do + expect(subject.headers['Content-Type']).to eq "application/hal+json;charset=utf-8" + end end end end diff --git a/spec/lib/pact_broker/api/contracts/pacticipant_schema_spec.rb b/spec/lib/pact_broker/api/contracts/pacticipant_schema_spec.rb new file mode 100644 index 000000000..87a6b3e3c --- /dev/null +++ b/spec/lib/pact_broker/api/contracts/pacticipant_schema_spec.rb @@ -0,0 +1,46 @@ +require 'pact_broker/api/contracts/pacticipant_schema' + +module PactBroker + module Api + module Contracts + describe PacticipantSchema do + let(:params) do + { + name: "pact-broker", + displayName: "Pact Broker", + mainDevelopmentBranches: branches, + repositoryUrl: "https://github.com/pact-foundation/pact_broker", + repositoryName: "pact_broker", + repositoryNamespace: "pact-foundation" + } + end + + let(:branches) { ["main"] } + + subject { PacticipantSchema.call(params) } + + context "with valid params" do + it { is_expected.to be_empty } + end + + context "with empty params" do + let(:params) do + { + repositoryUrl: "", + repositoryName: "", + repositoryNamespace: "" + } + end + + it { is_expected.to be_empty } + end + + context "with branch names that contain spaces" do + let(:branches) { ["main foo"] } + + its([:mainDevelopmentBranches, 0]) { is_expected.to include "cannot contain spaces" } + end + end + end + end +end diff --git a/spec/lib/pact_broker/api/decorators/pacticipant_decorator_spec.rb b/spec/lib/pact_broker/api/decorators/pacticipant_decorator_spec.rb index 8f95d69c5..9ed455368 100644 --- a/spec/lib/pact_broker/api/decorators/pacticipant_decorator_spec.rb +++ b/spec/lib/pact_broker/api/decorators/pacticipant_decorator_spec.rb @@ -3,71 +3,82 @@ require 'pact_broker/domain/pacticipant' module PactBroker - module Api - module Decorators - describe PacticipantDecorator do + describe "from_json" do + let(:pacticipant) { OpenStruct.new } + let(:decorator) { PacticipantDecorator.new(pacticipant) } + let(:hash) do + { + name: "Foo", + mainDevelopmentBranches: ["main"] + } + end - let(:test_data_builder) { TestDataBuilder.new } - let(:pacticipant) do - test_data_builder - .create_pacticipant('Name') - .create_label('foo') - .and_return(:pacticipant) - end - - let(:created_at) { Time.new(2014, 3, 4) } - let(:updated_at) { Time.new(2014, 3, 5) } - let(:base_url) { 'http://example.org' } + subject { decorator.from_json(hash.to_json) } - before do - pacticipant.created_at = created_at - pacticipant.updated_at = updated_at - allow_any_instance_of(PacticipantDecorator).to receive(:templated_tag_url_for_pacticipant).and_return('version_tag_url') + its(:name) { is_expected.to eq "Foo" } + its(:main_development_branches) { is_expected.to eq ["main"] } end + describe "to_json" do + let(:pacticipant) do + td.create_pacticipant('Name') + .create_label('foo') + .and_return(:pacticipant) + end - subject { JSON.parse PacticipantDecorator.new(pacticipant).to_json(user_options: {base_url: base_url}), symbolize_names: true } + let(:created_at) { Time.new(2014, 3, 4) } + let(:updated_at) { Time.new(2014, 3, 5) } + let(:base_url) { 'http://example.org' } - it "includes timestamps" do - expect(subject[:createdAt]).to eq FormatDateTime.call(created_at) - expect(subject[:updatedAt]).to eq FormatDateTime.call(updated_at) - end + before do + pacticipant.created_at = created_at + pacticipant.updated_at = updated_at + allow_any_instance_of(PacticipantDecorator).to receive(:templated_tag_url_for_pacticipant).and_return('version_tag_url') + end - it "includes embedded labels" do - expect(subject[:_embedded][:labels].first).to include name: 'foo' - expect(subject[:_embedded][:labels].first[:_links][:self][:href]).to match %r{http://example.org/.*foo} - end + subject { JSON.parse PacticipantDecorator.new(pacticipant).to_json(user_options: { base_url: base_url }), symbolize_names: true } - it "creates the URL for a version tag" do - expect_any_instance_of(PacticipantDecorator).to receive(:templated_tag_url_for_pacticipant).with("Name", base_url) - subject - end + it "includes timestamps" do + expect(subject[:createdAt]).to eq FormatDateTime.call(created_at) + expect(subject[:updatedAt]).to eq FormatDateTime.call(updated_at) + end - it "includes a relation for a version tag" do - expect(subject[:_links][:'pb:version-tag'][:href]).to eq "version_tag_url" - end + it "includes embedded labels" do + expect(subject[:_embedded][:labels].first).to include name: 'foo' + expect(subject[:_embedded][:labels].first[:_links][:self][:href]).to match %r{http://example.org/.*foo} + end - context "when there is a latest_version" do - before { test_data_builder.create_version("1.2.107") } - it "includes an embedded latestVersion" do - expect(subject[:_embedded][:latestVersion]).to include number: "1.2.107" + it "creates the URL for a version tag" do + expect_any_instance_of(PacticipantDecorator).to receive(:templated_tag_url_for_pacticipant).with("Name", base_url) + subject end - it "includes an embedded latest-version for backwards compatibility" do - expect(subject[:_embedded][:'latest-version']).to include number: "1.2.107" + it "includes a relation for a version tag" do + expect(subject[:_links][:'pb:version-tag'][:href]).to eq "version_tag_url" end - it "includes a deprecation warning" do - expect(subject[:_embedded][:'latest-version']).to include title: "DEPRECATED - please use latestVersion" + context "when there is a latest_version" do + before { td.create_version("1.2.107") } + it "includes an embedded latestVersion" do + expect(subject[:_embedded][:latestVersion]).to include number: "1.2.107" + end + + it "includes an embedded latest-version for backwards compatibility" do + expect(subject[:_embedded][:'latest-version']).to include number: "1.2.107" + end + + it "includes a deprecation warning" do + expect(subject[:_embedded][:'latest-version']).to include title: "DEPRECATED - please use latestVersion" + end end - end - context "when there is no latest_version" do - it "doesn't blow up" do - expect(subject[:_embedded]).to_not have_key(:latestVersion) - expect(subject[:_embedded]).to_not have_key(:'latest-version') + context "when there is no latest_version" do + it "doesn't blow up" do + expect(subject[:_embedded]).to_not have_key(:latestVersion) + expect(subject[:_embedded]).to_not have_key(:'latest-version') + end end end end diff --git a/spec/lib/pact_broker/api/resources/pacticipant_spec.rb b/spec/lib/pact_broker/api/resources/pacticipant_spec.rb index e388726fa..372ce681a 100644 --- a/spec/lib/pact_broker/api/resources/pacticipant_spec.rb +++ b/spec/lib/pact_broker/api/resources/pacticipant_spec.rb @@ -2,11 +2,8 @@ require 'pact_broker/api/resources/pacticipant' module PactBroker::Api - module Resources - describe Pacticipant do - describe "DELETE" do let(:pacticpant) { double("pacticpant") } diff --git a/spec/lib/pact_broker/api/resources/pacticipants_spec.rb b/spec/lib/pact_broker/api/resources/pacticipants_spec.rb index cf289bc97..8cacf2562 100644 --- a/spec/lib/pact_broker/api/resources/pacticipants_spec.rb +++ b/spec/lib/pact_broker/api/resources/pacticipants_spec.rb @@ -4,31 +4,29 @@ module PactBroker module Api module Resources - describe Pacticipants do - describe "POST" do - let(:params) { {name: 'New Consumer'} } - let(:json) { params.to_json } - let(:model) { instance_double(model_class, validate: errors) } + let(:params) { { name: 'New Consumer' } } + let(:request_body) { params.to_json } let(:created_model) { instance_double(model_class) } - let(:errors) { [] } + let(:errors) { {} } let(:model_class) { PactBroker::Domain::Pacticipant } let(:decorator_class) { PactBroker::Api::Decorators::PacticipantDecorator } - let(:decorator) { instance_double(decorator_class, to_json: response_json, from_json: model) } + let(:parsed_model) { OpenStruct.new(name: "New Consumer") } + let(:decorator) { instance_double(decorator_class, to_json: response_json, from_json: parsed_model) } let(:response_json) { {some: 'json'}.to_json } + let(:schema) { PactBroker::Api::Contracts::PacticipantSchema } before do - allow(model_class).to receive(:new).and_return(model) allow(PactBroker::Pacticipants::Service).to receive(:create).and_return(created_model) - allow(decorator_class).to receive(:new).with(model).and_return(decorator) - allow(decorator_class).to receive(:new).with(created_model).and_return(decorator) + allow(decorator_class).to receive(:new).and_return(decorator) + allow(schema).to receive(:call).and_return(errors) end - subject { post "/pacticipants", json, 'CONTENT_TYPE' => 'application/json' } + subject { post "/pacticipants", request_body, 'CONTENT_TYPE' => 'application/json' } context "structurally incorrect JSON" do - let(:json) { "{" } + let(:request_body) { "{" } it "returns a 400" do subject @@ -37,7 +35,7 @@ module Resources end context "when the model is invalid" do - let(:errors) { ['error'] } + let(:errors) { { "some" => ["errors"] } } it "returns a 400" do subject @@ -52,7 +50,7 @@ module Resources end it "parses the request JSON" do - expect(decorator).to receive(:from_json).with(json) + expect(decorator).to receive(:from_json).with(request_body) subject end @@ -83,9 +81,7 @@ module Resources end end end - end end - end end diff --git a/spec/lib/pact_broker/domain/pacticipant_spec.rb b/spec/lib/pact_broker/domain/pacticipant_spec.rb deleted file mode 100644 index 928ac37d5..000000000 --- a/spec/lib/pact_broker/domain/pacticipant_spec.rb +++ /dev/null @@ -1,26 +0,0 @@ -require 'spec_helper' -require 'pact_broker/domain/pacticipant' - -module PactBroker - module Domain - describe Pacticipant do - describe "validate" do - context "with all valid attributes" do - subject { Pacticipant.new name: 'Name' } - - it "returns an empty array" do - expect(subject.validate).to eq [] - end - end - - context "with no name" do - subject { Pacticipant.new } - - it "returns an error" do - expect(subject.validate).to eq ["Missing required attribute 'name'"] - end - end - end - end - end -end diff --git a/spec/lib/pact_broker/pacticipants/generate_display_name_spec.rb b/spec/lib/pact_broker/pacticipants/generate_display_name_spec.rb new file mode 100644 index 000000000..59edbfeea --- /dev/null +++ b/spec/lib/pact_broker/pacticipants/generate_display_name_spec.rb @@ -0,0 +1,39 @@ +require 'pact_broker/pacticipants/generate_display_name' + +module PactBroker + module Pacticipants + describe GenerateDisplayName do + describe ".call" do + TEST_CASES = { + "foo" => "Foo", + "MyService" => "My Service", + "my-service" => "My Service", + "my_service" => "My Service", + "my service" => "My Service", + "ABCService" => "ABC Service", + "A4Service" => "A4 Service", + "SNSPactEventConsumer" => "SNS Pact Event Consumer", + "AWSSummiteerWeb" => "AWS Summiteer Web", + "Beer-Consumer" => "Beer Consumer", + "foo.pretend-consumer" => "Foo Pretend Consumer", + "Client-XX" => "Client XX", + "providerJSWorkshop" => "Provider JS Workshop", + "e2e Provider Example" => "E2e Provider Example", + "MP - Our Provider" => "MP - Our Provider", + "PoC - Pact-broker-consumer" => "PoC - Pact Broker Consumer", + "QB-DATABASE Service" => "QB DATABASE Service", + "Support Species App (Provider)" => "Support Species App (Provider)", + 9 => "9", + "" => "", + nil => nil + } + + TEST_CASES.each do | name, expected_display_name | + it "converts #{name.inspect} to #{expected_display_name.inspect}" do + expect(GenerateDisplayName.call(name)).to eq expected_display_name + end + end + end + end + end +end diff --git a/spec/lib/pact_broker/pacticipants/repository_spec.rb b/spec/lib/pact_broker/pacticipants/repository_spec.rb index 0e1845a6b..d981db75c 100644 --- a/spec/lib/pact_broker/pacticipants/repository_spec.rb +++ b/spec/lib/pact_broker/pacticipants/repository_spec.rb @@ -12,20 +12,34 @@ module Pacticipants context "when the pacticipant does not already exist" do before do - TestDataBuilder.new.create_pacticipant("Bar") + td.create_pacticipant("Bar") + allow_any_instance_of(PactBroker::Domain::Pacticipant).to receive(:generate_display_name).and_return("display_name") end - subject { repository.create(name: "Foo") } + let(:display_name) { "Foo" } + + subject { repository.create(name: "foo", display_name: display_name, repository_url: "url", main_development_branches: ["main"]) } it "returns the new pacticipant" do expect(subject).to be_a(PactBroker::Domain::Pacticipant) - expect(subject.name).to eq "Foo" + expect(subject.name).to eq "foo" + expect(subject.main_development_branches).to eq ["main"] + expect(subject.repository_url).to eq "url" + expect(subject.display_name).to eq "Foo" + end + + context "when no display name is provided" do + let(:display_name) { nil } + + it "generates one" do + expect(subject.display_name).to eq "display_name" + end end end context "when a race condition occurs and the pacticipant was already created by another request" do before do - TestDataBuilder.new.create_pacticipant("Foo") + td.create_pacticipant("Foo", repository_url: "original") end it "does not raise an error" do @@ -35,13 +49,29 @@ module Pacticipants it "returns the existing pacticipant" do expect(subject).to be_a(PactBroker::Domain::Pacticipant) expect(subject.name).to eq "Foo" + expect(subject.repository_url).to eq "original" end end end + describe "replace" do + before do + td.create_pacticipant("Bar", main_development_branches: ["foo"], repository_namespace: "foo") + allow_any_instance_of(PactBroker::Domain::Pacticipant).to receive(:generate_display_name).and_return("display_name") + end + + subject { Repository.new.replace("Bar", OpenStruct.new(main_development_branches: ["bar"], repository_url: "new_url")) } + + it "replaces the pacticipant" do + expect(subject.main_development_branches).to eq ["bar"] + expect(subject.repository_namespace).to eq nil + expect(subject.display_name).to eq "display_name" + end + end + describe "#find" do before do - TestDataBuilder.new + td .create_pacticipant("Foo") .create_label("in") .create_pacticipant("Bar") @@ -124,9 +154,8 @@ module Pacticipants end describe "#pacticipant_names" do - before do - TestDataBuilder.new + td .create_pacticipant("Plants") .create_pacticipant("Animals") end @@ -141,7 +170,7 @@ module Pacticipants describe "#find_all_pacticipant_versions_in_reverse_order" do before do - TestDataBuilder.new + td .create_consumer("Foo") .create_consumer_version("1.2.3") .create_consumer_version("4.5.6") diff --git a/spec/lib/pact_broker/pacticipants/service_spec.rb b/spec/lib/pact_broker/pacticipants/service_spec.rb index a033eeaff..34124a4e8 100644 --- a/spec/lib/pact_broker/pacticipants/service_spec.rb +++ b/spec/lib/pact_broker/pacticipants/service_spec.rb @@ -15,20 +15,6 @@ module Pacticipants subject{ Service } - describe ".update" do - before do - td.create_pacticipant("Foo") - end - - let(:params) { { 'name' => 'Foo', 'repositoryUrl' => 'http://foo' } } - - subject { Service.update(params) } - - it "updates the repositoryUrl" do - expect(subject.repository_url).to eq 'http://foo' - end - end - describe ".messages_for_potential_duplicate_pacticipants" do let(:base_url) { 'http://example.org' }