From 92957ebb437d705fe3b0774adfae8aafbf5c0886 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Tue, 6 Dec 2022 08:49:28 +1100 Subject: [PATCH] feat: support problem+json for error messages (#583) --- .../custom_error_problem_json_decorator.rb | 36 +++++ .../runtime_error_problem_json_decorator.rb | 34 +++++ ...alidation_errors_problem_json_decorator.rb | 57 ++++++++ .../api/resources/badge_methods.rb | 2 +- .../api/resources/base_resource.rb | 29 +--- .../api/resources/error_handling_methods.rb | 57 ++++++++ .../error_response_body_generator.rb | 41 ------ .../api/resources/error_response_generator.rb | 70 ++++++++++ lib/pact_broker/api/resources/pact_version.rb | 2 +- lib/pact_broker/application_context.rb | 8 +- lib/pact_broker/pacts/selected_pact.rb | 4 + lib/rack/pact_broker/request_target.rb | 2 +- spec/features/error_spec.rb | 50 +++++++ ...ustom_error_problem_json_decorator_spec.rb | 22 +++ ...tion_errors_problem_json_decorator_spec.rb | 130 ++++++++++++++++++ .../api/resources/base_resource_spec.rb | 21 ++- ...ec.rb => error_response_generator_spec.rb} | 26 +++- 17 files changed, 514 insertions(+), 77 deletions(-) create mode 100644 lib/pact_broker/api/decorators/custom_error_problem_json_decorator.rb create mode 100644 lib/pact_broker/api/decorators/runtime_error_problem_json_decorator.rb create mode 100644 lib/pact_broker/api/decorators/validation_errors_problem_json_decorator.rb create mode 100644 lib/pact_broker/api/resources/error_handling_methods.rb delete mode 100644 lib/pact_broker/api/resources/error_response_body_generator.rb create mode 100644 lib/pact_broker/api/resources/error_response_generator.rb create mode 100644 spec/features/error_spec.rb create mode 100644 spec/lib/pact_broker/api/decorators/custom_error_problem_json_decorator_spec.rb create mode 100644 spec/lib/pact_broker/api/decorators/validation_errors_problem_json_decorator_spec.rb rename spec/lib/pact_broker/api/resources/{error_response_body_generator_spec.rb => error_response_generator_spec.rb} (67%) diff --git a/lib/pact_broker/api/decorators/custom_error_problem_json_decorator.rb b/lib/pact_broker/api/decorators/custom_error_problem_json_decorator.rb new file mode 100644 index 000000000..2509086f3 --- /dev/null +++ b/lib/pact_broker/api/decorators/custom_error_problem_json_decorator.rb @@ -0,0 +1,36 @@ +# Formats a message string into application/problem+json format. + +module PactBroker + module Api + module Decorators + class CustomErrorProblemJSONDecorator + + # @option title [String] + # @option type [String] + # @option detail [String] + # @option status [Integer] HTTP status code + def initialize(title:, type:, detail:, status: ) + @title = title + @type = type + @detail = detail + @status = status + end + + # @return [Hash] + def to_hash(decorator_options = {}) + { + "title" => @title, + "type" => "#{decorator_options.dig(:user_options, :base_url)}/problem/#{@type}", + "detail" => @detail, + "status" => @status + } + end + + # @return [String] JSON + def to_json(decorator_options = {}) + to_hash(decorator_options).to_json + end + end + end + end +end diff --git a/lib/pact_broker/api/decorators/runtime_error_problem_json_decorator.rb b/lib/pact_broker/api/decorators/runtime_error_problem_json_decorator.rb new file mode 100644 index 000000000..10f4504a9 --- /dev/null +++ b/lib/pact_broker/api/decorators/runtime_error_problem_json_decorator.rb @@ -0,0 +1,34 @@ +# Formats a message string into application/problem+json format. + +module PactBroker + module Api + module Decorators + class RuntimeErrorProblemJSONDecorator + + # @param message [String] + def initialize(message) + @message = message + end + + # @return [Hash] + def to_hash(decorator_options = {}) + { + "title" => "Server error", + "type" => "#{decorator_options.dig(:user_options, :base_url)}/problems/server_error", + "detail" => message, + "status" => 500 + } + end + + # @return [String] JSON + def to_json(decorator_options = {}) + to_hash(decorator_options).to_json + end + + private + + attr_reader :message + end + end + end +end diff --git a/lib/pact_broker/api/decorators/validation_errors_problem_json_decorator.rb b/lib/pact_broker/api/decorators/validation_errors_problem_json_decorator.rb new file mode 100644 index 000000000..96d806190 --- /dev/null +++ b/lib/pact_broker/api/decorators/validation_errors_problem_json_decorator.rb @@ -0,0 +1,57 @@ +# Formats a nested Hash of errors as it comes out of the Dry Validation library +# into application/problem+json format. + +module PactBroker + module Api + module Decorators + class ValidationErrorsProblemJSONDecorator + + # @param errors [Hash] + def initialize(errors) + @errors = errors + end + + # @return [Hash] + def to_hash(decorator_options = {}) + error_list = [] + walk_errors(errors, error_list, "", decorator_options.dig(:user_options, :base_url)) + { + "title" => "Validation errors", + "type" => "#{decorator_options.dig(:user_options, :base_url)}/problems/validation-error", + "status" => 400, + "errors" => error_list + } + end + + # @return [String] JSON + def to_json(decorator_options = {}) + to_hash(decorator_options).to_json + end + + private + + attr_reader :errors + + def walk_errors(object, list, path, base_url) + if object.is_a?(Hash) + object.each { | key, value | walk_errors(value, list, "#{path}/#{key}", base_url) } + elsif object.is_a?(Array) + object.each { | value | walk_errors(value, list, path, base_url) } + elsif object.is_a?(String) + append_error(list, object, path, base_url) + end + end + + def append_error(list, message, path, base_url) + list << { + "type" => "#{base_url}/problems/invalid-body-property-value", + "title" => "Validation error", + "detail" => message, + "instance" => path, + "status" => 400 + } + end + end + end + end +end diff --git a/lib/pact_broker/api/resources/badge_methods.rb b/lib/pact_broker/api/resources/badge_methods.rb index 7fd386283..d57e89436 100644 --- a/lib/pact_broker/api/resources/badge_methods.rb +++ b/lib/pact_broker/api/resources/badge_methods.rb @@ -37,7 +37,7 @@ def moved_temporarily? badge_url rescue StandardError => e # Want to render a badge, even if there's an error - badge_service.error_badge_url("error", ErrorResponseBodyGenerator.display_message(e, "reference: #{PactBroker::Errors.generate_error_reference}")) + badge_service.error_badge_url("error", ErrorResponseGenerator.display_message(e, "reference: #{PactBroker::Errors.generate_error_reference}")) end end diff --git a/lib/pact_broker/api/resources/base_resource.rb b/lib/pact_broker/api/resources/base_resource.rb index c52a9ada2..c20fc69e2 100644 --- a/lib/pact_broker/api/resources/base_resource.rb +++ b/lib/pact_broker/api/resources/base_resource.rb @@ -10,6 +10,7 @@ require "pact_broker/api/resources/authentication" require "pact_broker/api/resources/authorization" require "pact_broker/errors" +require "pact_broker/api/resources/error_handling_methods" module PactBroker module Api @@ -22,6 +23,7 @@ class BaseResource < Webmachine::Resource include PactBroker::Api::PactBrokerUrls include PactBroker::Api::Resources::Authentication include PactBroker::Api::Resources::Authorization + include PactBroker::Api::Resources::ErrorHandlingMethods include PactBroker::Logging @@ -111,15 +113,6 @@ def decorator_options options = {} { user_options: decorator_context(options) } end - def handle_exception(error) - error_reference = PactBroker::Errors.generate_error_reference - application_context.error_logger.call(error, error_reference, request.env) - if PactBroker::Errors.reportable_error?(error) - PactBroker::Errors.report(error, error_reference, request.env) - end - response.body = application_context.error_response_body_generator.call(error, error_reference, request.env) - end - # rubocop: disable Metrics/CyclomaticComplexity def params(options = {}) return options[:default] if options.key?(:default) && request_body.empty? @@ -158,16 +151,6 @@ def pact_params @pact_params ||= PactBroker::Pacts::PactParams.from_request(request, identifier_from_path) end - def set_json_error_message message - response.headers["Content-Type"] = "application/hal+json;charset=utf-8" - response.body = { error: message }.to_json - end - - def set_json_validation_error_messages errors - response.headers["Content-Type"] = "application/hal+json;charset=utf-8" - response.body = { errors: errors }.to_json - end - def request_body @request_body ||= request.body.to_s end @@ -215,13 +198,13 @@ def invalid_json? rescue NonUTF8CharacterFound => e logger.info(e.message) # Don't use the default SemanticLogger error logging method because it will try and print out the cause which will contain non UTF-8 chars in the message set_json_error_message(e.message) - response.headers["Content-Type"] = "application/hal+json;charset=utf-8" + response.headers["Content-Type"] = error_response_content_type true rescue StandardError => e message = "#{e.cause ? e.cause.class.name : e.class.name} - #{e.message}" logger.info(message) set_json_error_message(message) - response.headers["Content-Type"] = "application/hal+json;charset=utf-8" + response.headers["Content-Type"] = error_response_content_type true end end @@ -244,7 +227,9 @@ def contract_validation_errors? contract, params def find_pacticipant name, role pacticipant_service.find_pacticipant_by_name(name).tap do | pacticipant | - set_json_error_message("No #{role} with name '#{name}' found") if pacticipant.nil? + if pacticipant.nil? + set_json_error_message("No #{role} with name '#{name}' found", title: "Not found", type: "not_found", status: 404) + end end end diff --git a/lib/pact_broker/api/resources/error_handling_methods.rb b/lib/pact_broker/api/resources/error_handling_methods.rb new file mode 100644 index 000000000..ac8c4dcee --- /dev/null +++ b/lib/pact_broker/api/resources/error_handling_methods.rb @@ -0,0 +1,57 @@ +require "pact_broker/api/decorators/validation_errors_problem_json_decorator" +require "pact_broker/api/decorators/custom_error_problem_json_decorator" + +module PactBroker + module Api + module Resources + module ErrorHandlingMethods + + # @override + def handle_exception(error) + error_reference = PactBroker::Errors.generate_error_reference + application_context.error_logger.call(error, error_reference, request.env) + if PactBroker::Errors.reportable_error?(error) + PactBroker::Errors.report(error, error_reference, request.env) + end + headers, body = application_context.error_response_generator.call(error, error_reference, request.env) + headers.each { | key, value | response.headers[key] = value } + response.body = body + end + + def set_json_error_message detail, title: "Server error", type: "server_error", status: 500 + response.headers["Content-Type"] = error_response_content_type + response.body = error_response_body(detail, title, type, status) + end + + def set_json_validation_error_messages errors + response.headers["Content-Type"] = error_response_content_type + if problem_json_error_content_type? + response.body = PactBroker::Api::Decorators::ValidationErrorsProblemJSONDecorator.new(errors).to_json(decorator_options) + else + response.body = { errors: errors }.to_json + end + end + + def error_response_content_type + if problem_json_error_content_type? + "application/problem+json;charset=utf-8" + else + "application/hal+json;charset=utf-8" + end + end + + def error_response_body(detail, title, type, status) + if problem_json_error_content_type? + PactBroker::Api::Decorators::CustomErrorProblemJSONDecorator.new(detail: detail, title: title, type: type, status: status).to_json(decorator_options) + else + { error: detail }.to_json + end + end + + def problem_json_error_content_type? + request.headers["Accept"]&.include?("application/problem+json") + end + end + end + end +end diff --git a/lib/pact_broker/api/resources/error_response_body_generator.rb b/lib/pact_broker/api/resources/error_response_body_generator.rb deleted file mode 100644 index 7907d71f6..000000000 --- a/lib/pact_broker/api/resources/error_response_body_generator.rb +++ /dev/null @@ -1,41 +0,0 @@ -require "pact_broker/configuration" - -module PactBroker - module Api - module Resources - class ErrorResponseBodyGenerator - include PactBroker::Logging - - # env not needed, just passing in in case PF ever needs it - def self.call error, error_reference, _env = {} - response_body_hash(error, error_reference).to_json - end - - def self.display_message(error, obfuscated_message) - if PactBroker.configuration.show_backtrace_in_error_response? - error.message || obfuscated_message - else - PactBroker::Errors.reportable_error?(error) ? obfuscated_message : error.message - end - end - - def self.obfuscated_error_message(error_reference) - "An error has occurred. The details have been logged with the reference #{error_reference}" - end - - def self.response_body_hash(error, error_reference) - response_body = { - error: { - message: display_message(error, obfuscated_error_message(error_reference)), - reference: error_reference - } - } - if PactBroker.configuration.show_backtrace_in_error_response? - response_body[:error][:backtrace] = error.backtrace - end - response_body - end - end - end - end -end diff --git a/lib/pact_broker/api/resources/error_response_generator.rb b/lib/pact_broker/api/resources/error_response_generator.rb new file mode 100644 index 000000000..538b858df --- /dev/null +++ b/lib/pact_broker/api/resources/error_response_generator.rb @@ -0,0 +1,70 @@ +require "pact_broker/configuration" +require "pact_broker/api/decorators/runtime_error_problem_json_decorator" + +module PactBroker + module Api + module Resources + class ErrorResponseGenerator + include PactBroker::Logging + + # @param error [StandardError] + # @param error_reference [String] an error reference to display to the user + # @param env [Hash] the rack env + # @return [Hash, String] the response headers to set, the response body to set + def self.call error, error_reference, env = {} + body = response_body_hash(error, error_reference, env, display_message(error, obfuscated_error_message(error_reference))) + return headers(env), body.to_json + end + + def self.display_message(error, obfuscated_message) + if PactBroker.configuration.show_backtrace_in_error_response? + error.message || obfuscated_message + else + PactBroker::Errors.reportable_error?(error) ? obfuscated_message : error.message + end + end + + private_class_method def self.response_body_hash(error, error_reference, env, message) + if problem_json?(env) + problem_json_response_body(message, env) + else + hal_json_response_body(error, error_reference, message) + end + end + + private_class_method def self.hal_json_response_body(error, error_reference, message) + response_body = { + error: { + message: message, + reference: error_reference + } + } + if PactBroker.configuration.show_backtrace_in_error_response? + response_body[:error][:backtrace] = error.backtrace + end + response_body + end + + private_class_method def self.problem_json_response_body(message, env) + PactBroker::Api::Decorators::RuntimeErrorProblemJSONDecorator.new(message).to_hash(user_options: { base_url: env["pactbroker.base_url" ] }) + end + + private_class_method def self.obfuscated_error_message(error_reference) + "An error has occurred. The details have been logged with the reference #{error_reference}" + end + + private_class_method def self.headers(env) + if problem_json?(env) + { "Content-Type" => "application/problem+json;charset=utf-8" } + else + { "Content-Type" => "application/hal+json;charset=utf-8" } + end + end + + private_class_method def self.problem_json?(env) + env["HTTP_ACCEPT"]&.include?("application/problem+json") + end + end + end + end +end diff --git a/lib/pact_broker/api/resources/pact_version.rb b/lib/pact_broker/api/resources/pact_version.rb index cf55b1fe9..6c044b895 100644 --- a/lib/pact_broker/api/resources/pact_version.rb +++ b/lib/pact_broker/api/resources/pact_version.rb @@ -11,7 +11,7 @@ def allowed_methods ["GET", "OPTIONS"] end - def decorator_options(options) + def decorator_options(options = {}) super(options.merge(consumer_versions: consumer_versions_from_metadata&.reverse)) end end diff --git a/lib/pact_broker/application_context.rb b/lib/pact_broker/application_context.rb index a7d427ee8..09714f82f 100644 --- a/lib/pact_broker/application_context.rb +++ b/lib/pact_broker/application_context.rb @@ -3,7 +3,7 @@ require "pact_broker/api/decorators/decorator_context_creator" require "pact_broker/webhooks/execution_configuration_creator" require "pact_broker/errors/error_logger" -require "pact_broker/api/resources/error_response_body_generator" +require "pact_broker/api/resources/error_response_generator" module PactBroker class ApplicationContext @@ -15,7 +15,7 @@ class ApplicationContext :before_resource, :after_resource, :error_logger, - :error_response_body_generator + :error_response_generator def initialize(params = {}) params_with_defaults = { @@ -24,7 +24,7 @@ def initialize(params = {}) decorator_context_creator: PactBroker::Api::Decorators::DecoratorContextCreator, webhook_execution_configuration_creator: PactBroker::Webhooks::ExecutionConfigurationCreator, error_logger: PactBroker::Errors::ErrorLogger, - error_response_body_generator: PactBroker::Api::Resources::ErrorResponseBodyGenerator + error_response_generator: PactBroker::Api::Resources::ErrorResponseGenerator }.merge(params) @decorator_configuration = params_with_defaults[:decorator_configuration] @@ -35,7 +35,7 @@ def initialize(params = {}) @before_resource = params_with_defaults[:before_resource] @after_resource = params_with_defaults[:after_resource] @error_logger = params_with_defaults[:error_logger] - @error_response_body_generator = params_with_defaults[:error_response_body_generator] + @error_response_generator = params_with_defaults[:error_response_generator] end diff --git a/lib/pact_broker/pacts/selected_pact.rb b/lib/pact_broker/pacts/selected_pact.rb index 70e3cea8a..677eca78d 100644 --- a/lib/pact_broker/pacts/selected_pact.rb +++ b/lib/pact_broker/pacts/selected_pact.rb @@ -2,9 +2,13 @@ module PactBroker module Pacts + class SelectedPact < SimpleDelegator attr_reader :pact, :selectors + # @param pact [PactBroker::Domain::Pact] + # @param selectors [PactBroker::Pacts::Selectors] the Selectors object + # containing the ResolvedSelector objects that caused the pact to be selected. def initialize(pact, selectors) super(pact) @pact = pact diff --git a/lib/rack/pact_broker/request_target.rb b/lib/rack/pact_broker/request_target.rb index 1ead58510..dc4a888b4 100644 --- a/lib/rack/pact_broker/request_target.rb +++ b/lib/rack/pact_broker/request_target.rb @@ -7,7 +7,7 @@ module RequestTarget extend self WEB_ASSET_EXTENSIONS = %w[.js .woff .woff2 .css .png .html .map .ttf .ico].freeze - API_CONTENT_TYPES = %w[application/hal+json application/json text/csv application/yaml text/plain].freeze + API_CONTENT_TYPES = %w[application/hal+json application/problem+json application/json text/csv application/yaml text/plain].freeze def request_for_ui?(env) !(request_for_api?(env)) diff --git a/spec/features/error_spec.rb b/spec/features/error_spec.rb new file mode 100644 index 000000000..d0fd7dc67 --- /dev/null +++ b/spec/features/error_spec.rb @@ -0,0 +1,50 @@ + +RSpec.describe "error handling" do + let(:rack_headers) { {} } + + subject { post("/test/error", nil, rack_headers) } + + its(:status) { is_expected.to eq 500 } + its(:body) { is_expected.to include "Don't panic" } + + it "returns application/hal+json" do + expect(subject.headers["Content-Type"]).to eq "application/hal+json;charset=utf-8" + end + + context "when the Accept header includes application/problem+json" do + let(:rack_headers) do + { + "HTTP_ACCEPT" => "application/hal+json, application/problem+json" + } + end + + it "returns application/problem+json" do + expect(subject.headers["Content-Type"]).to eq "application/problem+json;charset=utf-8" + expect(JSON.parse(subject.body)["title"]).to eq "Server error" + end + end + + context "when a pacticipant does not exist" do + subject { get("/pacticipants/foo", nil, rack_headers) } + + it "returns application/problem+json" do + expect(subject.status).to eq 404 + expect(subject.headers["Content-Type"]).to eq "application/hal+json;charset=utf-8" + expect(JSON.parse(subject.body)).to_not have_key("title") + end + + context "when the Accept header includes application/problem+json" do + let(:rack_headers) do + { + "HTTP_ACCEPT" => "application/hal+json, application/problem+json" + } + end + + it "returns application/problem+json" do + expect(subject.status).to eq 404 + expect(subject.headers["Content-Type"]).to eq "application/problem+json;charset=utf-8" + expect(JSON.parse(subject.body)["title"]).to eq "Not found" + end + end + end +end diff --git a/spec/lib/pact_broker/api/decorators/custom_error_problem_json_decorator_spec.rb b/spec/lib/pact_broker/api/decorators/custom_error_problem_json_decorator_spec.rb new file mode 100644 index 000000000..92e7eec4f --- /dev/null +++ b/spec/lib/pact_broker/api/decorators/custom_error_problem_json_decorator_spec.rb @@ -0,0 +1,22 @@ +require "pact_broker/api/decorators/custom_error_problem_json_decorator" + +module PactBroker + module Api + module Decorators + describe CustomErrorProblemJSONDecorator do + let(:decorator_options) { { user_options: { base_url: "http://example.org" } } } + let(:params) { { title: "Title", type: "type", detail: "Detail", status: 400 } } + + subject { CustomErrorProblemJSONDecorator.new(**params).to_hash(decorator_options) } + + let(:expected_hash) do + { + "detail" => "Detail", "status" => 400, "title" => "Title", "type" => "http://example.org/problem/type" + } + end + + it { is_expected.to eq expected_hash } + end + end + end +end diff --git a/spec/lib/pact_broker/api/decorators/validation_errors_problem_json_decorator_spec.rb b/spec/lib/pact_broker/api/decorators/validation_errors_problem_json_decorator_spec.rb new file mode 100644 index 000000000..75872d346 --- /dev/null +++ b/spec/lib/pact_broker/api/decorators/validation_errors_problem_json_decorator_spec.rb @@ -0,0 +1,130 @@ +require "pact_broker/api/decorators/validation_errors_problem_json_decorator" + +module PactBroker + module Api + module Decorators + describe ValidationErrorsProblemJSONDecorator do + describe "#to_json" do + let(:decorator_options) { { user_options: { base_url: "http://example.org" } } } + + subject { ValidationErrorsProblemJSONDecorator.new(validation_errors).to_hash(decorator_options) } + + context "with a hash of errors" do + let(:validation_errors) do + { + contract: { content: ["this is some error text" ] } + } + end + + let(:expected_hash) do + { + "title" => "Validation errors", + "type" => "http://example.org/problems/validation-error", + "status" => 400, + + "errors" => [ + { + "type" => "http://example.org/problems/invalid-body-property-value", + "instance" => "/contract/content", + "title" => "Validation error", + "detail" => "this is some error text", + "status" => 400 + } + ] + } + end + + it { is_expected.to match_pact(expected_hash, allow_unexpected_keys: false)} + end + + context "with an indexed hash of errors" do + let(:validation_errors) do + { + contract: { content: { 1 => "this is some error text" } } + } + end + + let(:expected_hash) do + { + "title" => "Validation errors", + "type" => "http://example.org/problems/validation-error", + "status" => 400, + + "errors" => [ + { + "type" => "http://example.org/problems/invalid-body-property-value", + "instance" => "/contract/content/1", + "title" => "Validation error", + "detail" => "this is some error text", + "status" => 400 + } + ] + } + end + + it { is_expected.to match_pact(expected_hash, allow_unexpected_keys: false)} + end + + context "with an array of strings (shouldn't happen, but can't guarantee it doesn't" do + let(:validation_errors) do + ["error 1", "error 2"] + end + + let(:expected_hash) do + { + "title" => "Validation errors", + "type" => "http://example.org/problems/validation-error", + "status" => 400, + + "errors" => [ + { + "type" => "http://example.org/problems/invalid-body-property-value", + "instance" => "", + "title" => "Validation error", + "detail" => "error 1", + "status" => 400 + }, + { + "type" => "http://example.org/problems/invalid-body-property-value", + "instance" => "", + "title" => "Validation error", + "detail" => "error 2", + "status" => 400 + } + ] + } + end + + it { is_expected.to match_pact(expected_hash, allow_unexpected_keys: false)} + end + + context "with a string (shouldn't happen, but can't guarantee it doesn't" do + let(:validation_errors) do + "error 1" + end + + let(:expected_hash) do + { + "title" => "Validation errors", + "type" => "http://example.org/problems/validation-error", + "status" => 400, + + "errors" => [ + { + "type" => "http://example.org/problems/invalid-body-property-value", + "instance" => "", + "title" => "Validation error", + "detail" => "error 1", + "status" => 400 + } + ] + } + end + + it { is_expected.to match_pact(expected_hash, allow_unexpected_keys: false)} + end + end + end + end + end +end diff --git a/spec/lib/pact_broker/api/resources/base_resource_spec.rb b/spec/lib/pact_broker/api/resources/base_resource_spec.rb index c06c9972e..6312dd355 100644 --- a/spec/lib/pact_broker/api/resources/base_resource_spec.rb +++ b/spec/lib/pact_broker/api/resources/base_resource_spec.rb @@ -1,10 +1,23 @@ require "pact_broker/api/resources/base_resource" require "pact_broker/application_context" +require "webmachine/application_monkey_patch" module PactBroker module Api module Resources describe BaseResource do + + class TestResource < BaseResource + def allowed_methods + ["POST"] + end + + def process_post + raise "This is a test error" + end + end + + before do allow(env).to receive(:[]).with("pactbroker.base_url").and_return(nil) end @@ -169,11 +182,11 @@ module Resources end end end - end - context "with no resource_authorizer configured" do - it "returns false" do - expect(resource.forbidden?).to eq false + context "with no resource_authorizer configured" do + it "returns false" do + expect(resource.forbidden?).to eq false + end end end end diff --git a/spec/lib/pact_broker/api/resources/error_response_body_generator_spec.rb b/spec/lib/pact_broker/api/resources/error_response_generator_spec.rb similarity index 67% rename from spec/lib/pact_broker/api/resources/error_response_body_generator_spec.rb rename to spec/lib/pact_broker/api/resources/error_response_generator_spec.rb index 22143a694..75052c0fd 100644 --- a/spec/lib/pact_broker/api/resources/error_response_body_generator_spec.rb +++ b/spec/lib/pact_broker/api/resources/error_response_generator_spec.rb @@ -1,9 +1,9 @@ -require "pact_broker/api/resources/error_response_body_generator" +require "pact_broker/api/resources/error_response_generator" module PactBroker module Api module Resources - describe ErrorResponseBodyGenerator do + describe ErrorResponseGenerator do describe ".call" do before do allow(error).to receive(:backtrace).and_return(["backtrace"]) @@ -11,12 +11,32 @@ module Resources let(:error) { StandardError.new("test error") } let(:error_reference) { "bYWfnyWPlf" } - subject { JSON.parse(ErrorResponseBodyGenerator.call(error, error_reference)) } + let(:headers_and_body) { ErrorResponseGenerator.call(error, error_reference, rack_env) } + let(:rack_env) { { "pactbroker.base_url" => "http://example.org" } } + let(:headers) { headers_and_body.first } + + subject { JSON.parse(headers_and_body.last) } + + it "returns headers" do + expect(headers).to eq("Content-Type" => "application/hal+json;charset=utf-8") + end it "includes an error reference" do expect(subject["error"]).to include "reference" => "bYWfnyWPlf" end + context "when the Accept header includes application/problem+json" do + let(:rack_env) { { "HTTP_ACCEPT" => "application/hal+json, application/problem+json", "pactbroker.base_url" => "http://example.org" } } + + it "returns headers" do + expect(headers).to eq("Content-Type" => "application/problem+json;charset=utf-8") + end + + it "returns a problem JSON body" do + expect(subject).to include("title" => "Server error", "type" => "http://example.org/problems/server_error") + end + end + context "when show_backtrace_in_error_response? is true" do before do allow(PactBroker.configuration).to receive(:show_backtrace_in_error_response?).and_return(true)