From 9796ffe378f56310fe2284a31da5bd20814ce487 Mon Sep 17 00:00:00 2001 From: Adam Petro Date: Tue, 18 Jan 2022 11:54:05 -0500 Subject: [PATCH] [Scripts] Handle Configuration Definition Error (#1919) * [Scripts] Handle configuration definition error from script service GraphQL * [Scripts] Refactor error messages so that config file name is not hard-coded * Modify configuration definition error message --- .../script/layers/domain/errors.rb | 5 +- .../script/layers/domain/script_config.rb | 10 +- .../script/layers/infrastructure/errors.rb | 61 ++++++---- .../script_project_repository.rb | 56 ++++----- .../layers/infrastructure/script_service.rb | 27 ++++- lib/project_types/script/messages/messages.rb | 32 +++-- lib/project_types/script/ui/error_handler.rb | 70 ++++++----- test/minitest_ext.rb | 8 ++ .../layers/domain/script_config_test.rb | 4 +- .../layers/domain/script_project_test.rb | 8 +- .../script_project_repository_test.rb | 87 ++++++++++---- .../infrastructure/script_service_test.rb | 112 +++++++++++++++++- .../fake_script_project_repository.rb | 16 ++- .../script/ui/error_handler_test.rb | 74 ++++++++---- 14 files changed, 411 insertions(+), 159 deletions(-) diff --git a/lib/project_types/script/layers/domain/errors.rb b/lib/project_types/script/layers/domain/errors.rb index c918d1ba34..3523b03a98 100644 --- a/lib/project_types/script/layers/domain/errors.rb +++ b/lib/project_types/script/layers/domain/errors.rb @@ -15,10 +15,11 @@ def initialize(type) end class MissingScriptConfigFieldError < ScriptProjectError - attr_reader :field - def initialize(field) + attr_reader :field, :filename + def initialize(field:, filename:) super() @field = field + @filename = filename end end diff --git a/lib/project_types/script/layers/domain/script_config.rb b/lib/project_types/script/layers/domain/script_config.rb index 022aa3daa2..aea89167f3 100644 --- a/lib/project_types/script/layers/domain/script_config.rb +++ b/lib/project_types/script/layers/domain/script_config.rb @@ -4,13 +4,13 @@ module Script module Layers module Domain class ScriptConfig - attr_reader :content, :version, :title, :description, :configuration_ui, :configuration + attr_reader :content, :version, :title, :description, :configuration_ui, :configuration, :filename REQUIRED_FIELDS = %w(version title) - def initialize(content:) + def initialize(content:, filename:) + @filename = filename validate_content!(content) - @content = content @version = @content["version"].to_s @title = @content["title"] @@ -23,7 +23,9 @@ def initialize(content:) def validate_content!(content) REQUIRED_FIELDS.each do |field| - raise Errors::MissingScriptConfigFieldError, field if content[field].nil? + if content[field].nil? + raise Errors::MissingScriptConfigFieldError.new(field: field, filename: filename) + end end end end diff --git a/lib/project_types/script/layers/infrastructure/errors.rb b/lib/project_types/script/layers/infrastructure/errors.rb index 9f61e5b14c..9497a6da95 100644 --- a/lib/project_types/script/layers/infrastructure/errors.rb +++ b/lib/project_types/script/layers/infrastructure/errors.rb @@ -5,65 +5,78 @@ module Layers module Infrastructure module Errors class BuildError < ScriptProjectError; end - class ScriptConfigSyntaxError < ScriptProjectError; end + + class ScriptConfigurationDefinitionError < ScriptProjectError + attr_reader :filename + def initialize(message:, filename:) + @filename = filename + super(message) + end + end + + class ScriptConfigSyntaxError < ScriptProjectError + attr_reader :filename + def initialize(filename) + @filename = filename + super() + end + end class ScriptConfigMissingKeysError < ScriptProjectError - attr_reader :missing_keys - def initialize(missing_keys) + attr_reader :missing_keys, :filename + def initialize(missing_keys:, filename:) super() @missing_keys = missing_keys + @filename = filename end end class ScriptConfigInvalidValueError < ScriptProjectError - attr_reader :valid_input_modes - def initialize(valid_input_modes) + attr_reader :valid_input_modes, :filename + def initialize(valid_input_modes:, filename:) super() @valid_input_modes = valid_input_modes + @filename = filename end end class ScriptConfigFieldsMissingKeysError < ScriptProjectError - attr_reader :missing_keys - def initialize(missing_keys) + attr_reader :missing_keys, :filename + def initialize(missing_keys:, filename:) super() @missing_keys = missing_keys + @filename = filename end end class ScriptConfigFieldsInvalidValueError < ScriptProjectError - attr_reader :valid_types - def initialize(valid_types) + attr_reader :valid_types, :filename + def initialize(valid_types:, filename:) super() @valid_types = valid_types + @filename = filename end end class ScriptEnvAppNotConnectedError < ScriptProjectError; end - class InvalidScriptConfigYmlDefinitionError < ScriptProjectError; end - - class InvalidScriptJsonDefinitionError < ScriptProjectError; end - - class MissingScriptConfigYmlFieldError < ScriptProjectError - attr_reader :field - def initialize(field) + class ScriptConfigParseError < ScriptProjectError + attr_reader :filename, :serialization_format + def initialize(filename:, serialization_format:) super() - @field = field + @filename = filename + @serialization_format = serialization_format end end - class MissingScriptJsonFieldError < ScriptProjectError - attr_reader :field - def initialize(field) + class NoScriptConfigFileError < ScriptProjectError + attr_reader :filename + def initialize(filename) super() - @field = field + @filename = filename end end - class NoScriptConfigYmlFileError < ScriptProjectError; end - class NoScriptConfigFileError < ScriptProjectError; end - class APILibraryNotFoundError < ScriptProjectError attr_reader :library_name def initialize(library_name) diff --git a/lib/project_types/script/layers/infrastructure/script_project_repository.rb b/lib/project_types/script/layers/infrastructure/script_project_repository.rb index c965ba3460..d1eb33fdde 100644 --- a/lib/project_types/script/layers/infrastructure/script_project_repository.rb +++ b/lib/project_types/script/layers/infrastructure/script_project_repository.rb @@ -160,12 +160,15 @@ def validate_metadata!(extension_point_type, language) def script_config_repository @script_config_repository ||= begin + script_config_yml_repo = ScriptConfigYmlRepository.new(ctx: ctx) supported_repos = [ - ScriptConfigYmlRepository.new(ctx: ctx), + script_config_yml_repo, ScriptJsonRepository.new(ctx: ctx), ] repo = supported_repos.find(&:active?) - raise Infrastructure::Errors::NoScriptConfigYmlFileError if repo.nil? + if repo.nil? + raise Infrastructure::Errors::NoScriptConfigFileError, script_config_yml_repo.filename + end repo end end @@ -179,7 +182,7 @@ def active? end def get! - raise Infrastructure::Errors::NoScriptConfigFileError unless active? + raise Infrastructure::Errors::NoScriptConfigFileError, filename unless active? content = ctx.read(filename) hash = file_content_to_hash(content) @@ -196,6 +199,10 @@ def update!(title:) from_h(hash) end + def filename + raise NotImplementedError + end + private def update_hash(hash:, title:) @@ -204,14 +211,7 @@ def update_hash(hash:, title:) end def from_h(hash) - Domain::ScriptConfig.new(content: hash) - rescue Domain::Errors::MissingScriptConfigFieldError => e - raise missing_field_error, e.field - end - - # to be implemented by subclasses - def filename - raise NotImplementedError + Domain::ScriptConfig.new(content: hash, filename: filename) end def file_content_to_hash(file_content) @@ -221,26 +221,22 @@ def file_content_to_hash(file_content) def hash_to_file_content(hash) raise NotImplementedError end - - def missing_field_error - raise NotImplementedError - end end class ScriptConfigYmlRepository < ScriptConfigRepository - private - def filename "script.config.yml" end + private + def file_content_to_hash(file_content) begin hash = YAML.load(file_content) rescue Psych::SyntaxError - raise Errors::InvalidScriptConfigYmlDefinitionError + raise parse_error end - raise Errors::InvalidScriptConfigYmlDefinitionError unless hash.is_a?(Hash) + raise parse_error unless hash.is_a?(Hash) hash end @@ -248,30 +244,34 @@ def hash_to_file_content(hash) YAML.dump(hash) end - def missing_field_error - Errors::MissingScriptConfigYmlFieldError + def parse_error + Errors::ScriptConfigParseError.new(filename: filename, serialization_format: "YAML") end end class ScriptJsonRepository < ScriptConfigRepository - private - def filename "script.json" end + private + def file_content_to_hash(file_content) - JSON.parse(file_content) - rescue JSON::ParserError - raise Errors::InvalidScriptJsonDefinitionError + begin + hash = JSON.parse(file_content) + rescue JSON::ParserError + raise parse_error + end + raise parse_error unless hash.is_a?(Hash) + hash end def hash_to_file_content(hash) JSON.pretty_generate(hash) end - def missing_field_error - Errors::MissingScriptJsonFieldError + def parse_error + Errors::ScriptConfigParseError.new(filename: filename, serialization_format: "JSON") end end end diff --git a/lib/project_types/script/layers/infrastructure/script_service.rb b/lib/project_types/script/layers/infrastructure/script_service.rb index 9f22690187..b2bad903b4 100644 --- a/lib/project_types/script/layers/infrastructure/script_service.rb +++ b/lib/project_types/script/layers/infrastructure/script_service.rb @@ -46,20 +46,37 @@ def set_app_script( if user_errors.any? { |e| e["tag"] == "already_exists_error" } raise Errors::ScriptRepushError, uuid + elsif (e = user_errors.find { |err| err["tag"] == "configuration_definition_error" }) + raise Errors::ScriptConfigurationDefinitionError.new( + message: e["message"], + filename: script_config.filename, + ) elsif (e = user_errors.any? { |err| err["tag"] == "configuration_definition_syntax_error" }) - raise Errors::ScriptConfigSyntaxError + raise Errors::ScriptConfigSyntaxError, script_config.filename elsif (e = user_errors.find { |err| err["tag"] == "configuration_definition_missing_keys_error" }) - raise Errors::ScriptConfigMissingKeysError, e["message"] + raise Errors::ScriptConfigMissingKeysError.new( + missing_keys: e["message"], + filename: script_config.filename, + ) elsif (e = user_errors.find { |err| err["tag"] == "configuration_definition_invalid_value_error" }) - raise Errors::ScriptConfigInvalidValueError, e["message"] + raise Errors::ScriptConfigInvalidValueError.new( + valid_input_modes: e["message"], + filename: script_config.filename, + ) elsif (e = user_errors.find do |err| err["tag"] == "configuration_definition_schema_field_missing_keys_error" end) - raise Errors::ScriptConfigFieldsMissingKeysError, e["message"] + raise Errors::ScriptConfigFieldsMissingKeysError.new( + missing_keys: e["message"], + filename: script_config.filename, + ) elsif (e = user_errors.find do |err| err["tag"] == "configuration_definition_schema_field_invalid_value_error" end) - raise Errors::ScriptConfigFieldsInvalidValueError, e["message"] + raise Errors::ScriptConfigFieldsInvalidValueError.new( + valid_types: e["message"], + filename: script_config.filename, + ) elsif user_errors.find { |err| %w(not_use_msgpack_error schema_version_argument_error).include?(err["tag"]) } raise Domain::Errors::MetadataValidationError else diff --git a/lib/project_types/script/messages/messages.rb b/lib/project_types/script/messages/messages.rb index c67912f435..984b9c5fed 100644 --- a/lib/project_types/script/messages/messages.rb +++ b/lib/project_types/script/messages/messages.rb @@ -47,41 +47,39 @@ module Messages invalid_language_cause: "Invalid language %s.", invalid_language_help: "Allowed values: %s.", - missing_script_config_yml_field_cause: "The script.config.yml file is missing the required %s field.", - missing_script_config_yml_field_help: "Add the field and try again.", + missing_script_config_field_cause: "The %{filename} file is missing the required %{field} field.", + missing_script_config_field_help: "Add the field and try again.", - missing_script_json_field_cause: "The script.json file is missing the required %s field.", - missing_script_json_field_help: "Add the field and try again.", + script_config_parse_error_cause: "The %{filename} file contains invalid %{serialization_format}.", + script_config_parse_error_help: "Fix the errors and try again.", - invalid_script_json_definition_cause: "The script.json file contains invalid JSON.", - invalid_script_json_definition_help: "Fix the errors and try again.", - - invalid_script_config_yml_definition_cause: "The script.config.yml file contains invalid YAML.", - invalid_script_config_yml_definition_help: "Fix the errors and try again.", - - no_script_config_yml_file_cause: "The script.config.yml file is missing.", - no_script_config_yml_file_help: "Create this file and try again.", + no_script_config_file_cause: "The %{filename} file is missing.", + no_script_config_file_help: "Create this file and try again.", app_not_connected_cause: "Script is not connected to an app.", app_not_connected_help: "Run shopify connect or enter fields for api-key and api-secret.", - configuration_syntax_error_cause: "The script.json is not formatted properly.", + configuration_definition_error_cause: "In the %{filename} file, there was a problem with the "\ + "configuration. %{message}", + configuration_definition_error_help: "Fix the error and try again.", + + configuration_syntax_error_cause: "The %{filename} is not formatted properly.", configuration_syntax_error_help: "Fix the errors and try again.", - configuration_missing_keys_error_cause: "The script.json file is missing required keys: "\ + configuration_missing_keys_error_cause: "The %{filename} file is missing required keys: "\ "%{missing_keys}.", configuration_missing_keys_error_help: "Add the keys and try again.", - configuration_invalid_value_error_cause: "The script.json configuration only accepts "\ + configuration_invalid_value_error_cause: "The %{filename} configuration only accepts "\ "one of the following types(s): %{valid_input_modes}.", configuration_invalid_value_error_help: "Change the type and try again.", - configuration_schema_field_missing_keys_error_cause: "A configuration entry in the script.json file "\ + configuration_schema_field_missing_keys_error_cause: "A configuration entry in the %{filename} file "\ "is missing required keys: %{missing_keys}.", configuration_definition_schema_field_missing_keys_error_help: "Add the keys and try again.", configuration_schema_field_invalid_value_error_cause: "The configuration entries in the "\ - "script.json file only accept one of the following "\ + "%{filename} file only accept one of the following "\ "type(s): %{valid_types}.", configuration_schema_field_invalid_value_error_help: "Change the types and try again.", diff --git a/lib/project_types/script/ui/error_handler.rb b/lib/project_types/script/ui/error_handler.rb index 0bddb91513..a88f34431e 100644 --- a/lib/project_types/script/ui/error_handler.rb +++ b/lib/project_types/script/ui/error_handler.rb @@ -103,44 +103,52 @@ def self.error_messages(e) cause_of_error: ShopifyCLI::Context.message("script.error.metadata_not_found_cause"), help_suggestion: ShopifyCLI::Context.message("script.error.metadata_not_found_help"), } + when Layers::Domain::Errors::MissingScriptConfigFieldError + { + cause_of_error: ShopifyCLI::Context.message( + "script.error.missing_script_config_field_cause", + field: e.field, + filename: e.filename, + ), + help_suggestion: ShopifyCLI::Context.message("script.error.missing_script_config_field_help"), + } when Layers::Infrastructure::Errors::BuildError { cause_of_error: ShopifyCLI::Context.message("script.error.build_error_cause"), help_suggestion: ShopifyCLI::Context.message("script.error.build_error_help"), } - when Layers::Infrastructure::Errors::InvalidScriptConfigYmlDefinitionError - { - cause_of_error: ShopifyCLI::Context.message("script.error.invalid_script_config_yml_definition_cause"), - help_suggestion: ShopifyCLI::Context.message("script.error.invalid_script_config_yml_definition_help"), - } - when Layers::Infrastructure::Errors::InvalidScriptJsonDefinitionError - { - cause_of_error: ShopifyCLI::Context.message("script.error.invalid_script_json_definition_cause"), - help_suggestion: ShopifyCLI::Context.message("script.error.invalid_script_json_definition_help"), - } - when Layers::Infrastructure::Errors::MissingScriptConfigYmlFieldError - { - cause_of_error: ShopifyCLI::Context.message("script.error.missing_script_config_yml_field_cause", e.field), - help_suggestion: ShopifyCLI::Context.message("script.error.missing_script_config_yml_field_help"), - } - when Layers::Infrastructure::Errors::MissingScriptConfigYmlFieldError + when Layers::Infrastructure::Errors::ScriptConfigParseError { - cause_of_error: ShopifyCLI::Context.message("script.error.missing_script_config_yml_field_cause", e.field), - help_suggestion: ShopifyCLI::Context.message("script.error.missing_script_config_yml_field_help"), + cause_of_error: ShopifyCLI::Context.message( + "script.error.script_config_parse_error_cause", + filename: e.filename, + serialization_format: e.serialization_format, + ), + help_suggestion: ShopifyCLI::Context.message("script.error.script_config_parse_error_help"), } - when Layers::Infrastructure::Errors::MissingScriptJsonFieldError + when Layers::Infrastructure::Errors::NoScriptConfigFileError { - cause_of_error: ShopifyCLI::Context.message("script.error.missing_script_json_field_cause", e.field), - help_suggestion: ShopifyCLI::Context.message("script.error.missing_script_json_field_help"), + cause_of_error: ShopifyCLI::Context.message( + "script.error.no_script_config_file_cause", + filename: e.filename, + ), + help_suggestion: ShopifyCLI::Context.message("script.error.no_script_config_file_help"), } - when Layers::Infrastructure::Errors::NoScriptConfigYmlFileError + when Layers::Infrastructure::Errors::ScriptConfigurationDefinitionError { - cause_of_error: ShopifyCLI::Context.message("script.error.no_script_config_yml_file_cause"), - help_suggestion: ShopifyCLI::Context.message("script.error.no_script_config_yml_file_help"), + cause_of_error: ShopifyCLI::Context.message( + "script.error.configuration_definition_error_cause", + message: e.message, + filename: e.filename, + ), + help_suggestion: ShopifyCLI::Context.message("script.error.configuration_definition_error_help"), } when Layers::Infrastructure::Errors::ScriptConfigSyntaxError { - cause_of_error: ShopifyCLI::Context.message("script.error.configuration_syntax_error_cause"), + cause_of_error: ShopifyCLI::Context.message( + "script.error.configuration_syntax_error_cause", + filename: e.filename, + ), help_suggestion: ShopifyCLI::Context.message("script.error.configuration_syntax_error_help"), } when Layers::Infrastructure::Errors::ScriptEnvAppNotConnectedError @@ -152,7 +160,8 @@ def self.error_messages(e) { cause_of_error: ShopifyCLI::Context.message( "script.error.configuration_missing_keys_error_cause", - missing_keys: e.missing_keys + missing_keys: e.missing_keys, + filename: e.filename, ), help_suggestion: ShopifyCLI::Context.message("script.error.configuration_missing_keys_error_help"), } @@ -160,7 +169,8 @@ def self.error_messages(e) { cause_of_error: ShopifyCLI::Context.message( "script.error.configuration_invalid_value_error_cause", - valid_input_modes: e.valid_input_modes + valid_input_modes: e.valid_input_modes, + filename: e.filename, ), help_suggestion: ShopifyCLI::Context.message("script.error.configuration_invalid_value_error_help"), } @@ -168,7 +178,8 @@ def self.error_messages(e) { cause_of_error: ShopifyCLI::Context.message( "script.error.configuration_schema_field_missing_keys_error_cause", - missing_keys: e.missing_keys + missing_keys: e.missing_keys, + filename: e.filename, ), help_suggestion: ShopifyCLI::Context.message( "script.error.configuration_definition_schema_field_missing_keys_error_help" @@ -178,7 +189,8 @@ def self.error_messages(e) { cause_of_error: ShopifyCLI::Context.message( "script.error.configuration_schema_field_invalid_value_error_cause", - valid_types: e.valid_types + valid_types: e.valid_types, + filename: e.filename, ), help_suggestion: ShopifyCLI::Context.message( "script.error.configuration_schema_field_invalid_value_error_help" diff --git a/test/minitest_ext.rb b/test/minitest_ext.rb index 40edd4d710..345c4100f0 100644 --- a/test/minitest_ext.rb +++ b/test/minitest_ext.rb @@ -93,6 +93,14 @@ def to_s # :nodoc: end.join("\n") end + def assert_raises_and_validate(exception_class, field_expectation_proc) + yield + rescue exception_class => e + field_expectation_proc.call(e) + else + flunk("Expected a #{exception_class} to be raised but it was not") + end + private def stub_prompt_for_cli_updates diff --git a/test/project_types/script/layers/domain/script_config_test.rb b/test/project_types/script/layers/domain/script_config_test.rb index 4dfca79b80..72b8a93f40 100644 --- a/test/project_types/script/layers/domain/script_config_test.rb +++ b/test/project_types/script/layers/domain/script_config_test.rb @@ -12,8 +12,9 @@ "configuration" => {}, } end + let(:filename) { "script.config.yml" } - subject { Script::Layers::Domain::ScriptConfig.new(content: content) } + subject { Script::Layers::Domain::ScriptConfig.new(content: content, filename: filename) } describe "#initialize" do it "constructs a ScriptConfig" do @@ -22,6 +23,7 @@ assert_equal("Some Description", subject.description) assert(subject.configuration_ui) assert_equal({}, subject.configuration) + assert_equal(filename, subject.filename) end end diff --git a/test/project_types/script/layers/domain/script_project_test.rb b/test/project_types/script/layers/domain/script_project_test.rb index 2d05ef77ad..48e637ac7d 100644 --- a/test/project_types/script/layers/domain/script_project_test.rb +++ b/test/project_types/script/layers/domain/script_project_test.rb @@ -8,7 +8,13 @@ let(:extension_point_type) { "discount" } let(:script_name) { "foo_script" } let(:language) { "assemblyscript" } - let(:script_config) { Script::Layers::Domain::ScriptConfig.new(content: script_config_content) } + let(:script_config_filename) { "script.config.yml" } + let(:script_config) do + Script::Layers::Domain::ScriptConfig.new( + content: script_config_content, + filename: script_config_filename, + ) + end let(:script_config_content) do { "version" => "1", diff --git a/test/project_types/script/layers/infrastructure/script_project_repository_test.rb b/test/project_types/script/layers/infrastructure/script_project_repository_test.rb index bbc720445d..8c55df8415 100644 --- a/test/project_types/script/layers/infrastructure/script_project_repository_test.rb +++ b/test/project_types/script/layers/infrastructure/script_project_repository_test.rb @@ -317,8 +317,11 @@ def hash_except(config, *keys) subject { instance.update_script_config(title: new_title) } describe "script.config.yml does not exist" do - it "raises NoScriptConfigYmlFileError" do - assert_raises(Script::Layers::Infrastructure::Errors::NoScriptConfigYmlFileError) { subject } + it "raises NoScriptConfigFileError" do + assert_raises_and_validate( + Script::Layers::Infrastructure::Errors::NoScriptConfigFileError, + proc { |e| assert_equal(script_config_filename, e.filename) } + ) { subject } end end @@ -443,7 +446,10 @@ def hash_except(config, *keys) describe "when file does not exist" do it "raises NoScriptConfigFileError" do - assert_raises(Script::Layers::Infrastructure::Errors::NoScriptConfigFileError) { subject } + assert_raises_and_validate( + Script::Layers::Infrastructure::Errors::NoScriptConfigFileError, + proc { |e| assert_equal(script_config_filename, e.filename) } + ) { subject } end end @@ -455,24 +461,39 @@ def hash_except(config, *keys) describe "when content is invalid yaml" do let(:content) { "*" } - it "raises InvalidScriptConfigYmlDefinitionError" do - assert_raises(Script::Layers::Infrastructure::Errors::InvalidScriptConfigYmlDefinitionError) { subject } + it "raises ScriptConfigParseError" do + assert_raises_and_validate( + Script::Layers::Infrastructure::Errors::ScriptConfigParseError, + proc do |e| + assert_equal(script_config_filename, e.filename) + assert_equal("YAML", e.serialization_format) + end, + ) { subject } end end describe "when content is not a hash" do - let(:content) { "" } - - it "raises InvalidScriptConfigYmlDefinitionError" do - assert_raises(Script::Layers::Infrastructure::Errors::InvalidScriptConfigYmlDefinitionError) { subject } + let(:content) { "[]" } + + it "raises ScriptConfigParseError" do + assert_raises_and_validate( + Script::Layers::Infrastructure::Errors::ScriptConfigParseError, + proc do |e| + assert_equal(script_config_filename, e.filename) + assert_equal("YAML", e.serialization_format) + end, + ) { subject } end end describe "when content is missing fields" do let(:content) { {}.to_yaml } - it "raises MissingScriptConfigYmlFieldError" do - assert_raises(Script::Layers::Infrastructure::Errors::MissingScriptConfigYmlFieldError) { subject } + it "raises MissingScriptJsonFieldError" do + assert_raises_and_validate( + Script::Layers::Domain::Errors::MissingScriptConfigFieldError, + proc { |e| assert_equal(script_config_filename, e.filename) }, + ) { subject } end end @@ -494,7 +515,10 @@ def hash_except(config, *keys) describe "when file does not exist" do it "raises NoScriptConfigFileError" do - assert_raises(Script::Layers::Infrastructure::Errors::NoScriptConfigFileError) { subject } + assert_raises_and_validate( + Script::Layers::Infrastructure::Errors::NoScriptConfigFileError, + proc { |e| assert_equal(script_config_filename, e.filename) } + ) { subject } end end @@ -615,7 +639,10 @@ def hash_except(config, *keys) describe "when file does not exist" do it "raises NoScriptConfigFileError" do - assert_raises(Script::Layers::Infrastructure::Errors::NoScriptConfigFileError) { subject } + assert_raises_and_validate( + Script::Layers::Infrastructure::Errors::NoScriptConfigFileError, + proc { |e| assert_equal(script_config_filename, e.filename) } + ) { subject } end end @@ -627,16 +654,28 @@ def hash_except(config, *keys) describe "when content is invalid json" do let(:content) { "{[}]" } - it "raises InvalidScriptJsonDefinitionError" do - assert_raises(Script::Layers::Infrastructure::Errors::InvalidScriptJsonDefinitionError) { subject } + it "raises ScriptConfigParseError" do + assert_raises_and_validate( + Script::Layers::Infrastructure::Errors::ScriptConfigParseError, + proc do |e| + assert_equal(script_config_filename, e.filename) + assert_equal("JSON", e.serialization_format) + end, + ) { subject } end end describe "when content is not a hash" do - let(:content) { "" } - - it "raises InvalidScriptJsonDefinitionError" do - assert_raises(Script::Layers::Infrastructure::Errors::InvalidScriptJsonDefinitionError) { subject } + let(:content) { "[]" } + + it "raises ScriptConfigParseError" do + assert_raises_and_validate( + Script::Layers::Infrastructure::Errors::ScriptConfigParseError, + proc do |e| + assert_equal(script_config_filename, e.filename) + assert_equal("JSON", e.serialization_format) + end, + ) { subject } end end @@ -644,7 +683,10 @@ def hash_except(config, *keys) let(:content) { {}.to_json } it "raises MissingScriptJsonFieldError" do - assert_raises(Script::Layers::Infrastructure::Errors::MissingScriptJsonFieldError) { subject } + assert_raises_and_validate( + Script::Layers::Domain::Errors::MissingScriptConfigFieldError, + proc { |e| assert_equal(script_config_filename, e.filename) }, + ) { subject } end end @@ -666,7 +708,10 @@ def hash_except(config, *keys) describe "when file does not exist" do it "raises NoScriptConfigFileError" do - assert_raises(Script::Layers::Infrastructure::Errors::NoScriptConfigFileError) { subject } + assert_raises_and_validate( + Script::Layers::Infrastructure::Errors::NoScriptConfigFileError, + proc { |e| assert_equal(script_config_filename, e.filename) } + ) { subject } end end diff --git a/test/project_types/script/layers/infrastructure/script_service_test.rb b/test/project_types/script/layers/infrastructure/script_service_test.rb index 58a614e272..ea47c2b2e6 100644 --- a/test/project_types/script/layers/infrastructure/script_service_test.rb +++ b/test/project_types/script/layers/infrastructure/script_service_test.rb @@ -11,8 +11,12 @@ let(:schema_major_version) { "1" } let(:schema_minor_version) { "0" } let(:use_msgpack) { true } + let(:script_config_filename) { "script.config.yml" } let(:script_config) do - Script::Layers::Domain::ScriptConfig.new(content: expected_script_config_content) + Script::Layers::Domain::ScriptConfig.new( + content: expected_script_config_content, + filename: script_config_filename, + ) end let(:script_name) { "script name" } let(:script_config_version) { "1" } @@ -175,6 +179,112 @@ end end + describe "when v2 configuration is invalid" do + let(:response) do + { + "data" => { + "appScriptSet" => { + "userErrors" => [{ "message" => "error", "tag" => "configuration_definition_error" }], + }, + }, + } + end + + it "should raise ScriptConfigurationDefinitionError" do + assert_raises_and_validate( + Script::Layers::Infrastructure::Errors::ScriptConfigurationDefinitionError, + proc do |e| + assert_equal("error", e.message) + assert_equal(script_config_filename, e.filename) + end, + ) { subject } + end + end + + describe "when v1 configuration is invalid" do + let(:response) do + { + "data" => { + "appScriptSet" => { + "userErrors" => [{ "message" => error_message, "tag" => error_tag }], + }, + }, + } + end + + describe "when missing keys" do + let(:error_message) { "keys" } + let(:error_tag) { "configuration_definition_missing_keys_error" } + + it "should raise ScriptConfigMissingKeysError" do + assert_raises_and_validate( + Script::Layers::Infrastructure::Errors::ScriptConfigMissingKeysError, + proc do |e| + assert_equal(error_message, e.missing_keys) + assert_equal(script_config_filename, e.filename) + end, + ) { subject } + end + end + + describe "when fields missing keys" do + let(:error_message) { "keys" } + let(:error_tag) { "configuration_definition_schema_field_missing_keys_error" } + + it "should raise ScriptConfigFieldsMissingKeysError" do + assert_raises_and_validate( + Script::Layers::Infrastructure::Errors::ScriptConfigFieldsMissingKeysError, + proc do |e| + assert_equal(error_message, e.missing_keys) + assert_equal(script_config_filename, e.filename) + end, + ) { subject } + end + end + + describe "when invalid value" do + let(:error_message) { "input modes" } + let(:error_tag) { "configuration_definition_invalid_value_error" } + + it "should raise ScriptConfigInvalidValueError" do + assert_raises_and_validate( + Script::Layers::Infrastructure::Errors::ScriptConfigInvalidValueError, + proc do |e| + assert_equal(error_message, e.valid_input_modes) + assert_equal(script_config_filename, e.filename) + end, + ) { subject } + end + end + + describe "when fields invalid value" do + let(:error_message) { "valid types" } + let(:error_tag) { "configuration_definition_schema_field_invalid_value_error" } + + it "should raise ScriptConfigFieldsInvalidValueError" do + assert_raises_and_validate( + Script::Layers::Infrastructure::Errors::ScriptConfigFieldsInvalidValueError, + proc do |e| + assert_equal(error_message, e.valid_types) + assert_equal(script_config_filename, e.filename) + end, + ) { subject } + end + end + + describe "when invalid syntax" do + let(:error_message) { "invalid syntax" } + let(:error_tag) { "configuration_definition_syntax_error" } + + it "should raise ScriptConfigSyntaxError" do + assert_raises_and_validate( + Script::Layers::Infrastructure::Errors::ScriptConfigSyntaxError, + proc { |e| assert_equal(script_config_filename, e.filename) }, + ) { subject } + end + end + end + describe "when response is empty" do let(:response) { nil } diff --git a/test/project_types/script/test_helpers/fake_script_project_repository.rb b/test/project_types/script/test_helpers/fake_script_project_repository.rb index ec365bc520..33a9e28c5a 100644 --- a/test/project_types/script/test_helpers/fake_script_project_repository.rb +++ b/test/project_types/script/test_helpers/fake_script_project_repository.rb @@ -69,20 +69,30 @@ def initialize end def create(content) - @cache = Script::Layers::Domain::ScriptConfig.new(content: content) + @cache = from_h(content) end def update!(title:) hash = get!.content hash["title"] = title - @cache = Script::Layers::Domain::ScriptConfig.new(content: hash) + @cache = from_h(hash) end def get! - raise Script::Layers::Infrastructure::Errors::NoScriptConfigFileError if @cache.nil? + raise Script::Layers::Infrastructure::Errors::NoScriptConfigFileError, filename if @cache.nil? @cache end + + def filename + "script.config.yml" + end + + private + + def from_h(hash) + Script::Layers::Domain::ScriptConfig.new(content: hash, filename: filename) + end end end end diff --git a/test/project_types/script/ui/error_handler_test.rb b/test/project_types/script/ui/error_handler_test.rb index f43789c6b2..a8e84162f2 100644 --- a/test/project_types/script/ui/error_handler_test.rb +++ b/test/project_types/script/ui/error_handler_test.rb @@ -170,36 +170,32 @@ def should_call_display_and_raise end end - describe "when InvalidScriptConfigYmlDefinitionError" do - let(:err) { Script::Layers::Infrastructure::Errors::InvalidScriptConfigYmlDefinitionError.new("filename") } - it "should call display_and_raise" do - should_call_display_and_raise + describe "when ScriptConfigParseError" do + let(:err) do + Script::Layers::Infrastructure::Errors::ScriptConfigParseError.new( + filename: "filename", + serialization_format: "serialization_format", + ) end - end - - describe "when InvalidScriptJsonDefinitionError" do - let(:err) { Script::Layers::Infrastructure::Errors::InvalidScriptJsonDefinitionError.new("filename") } it "should call display_and_raise" do should_call_display_and_raise end end - describe "when MissingScriptConfigYmlFieldError" do - let(:err) { Script::Layers::Infrastructure::Errors::MissingScriptConfigYmlFieldError.new("field") } - it "should call display_and_raise" do - should_call_display_and_raise + describe "when MissingScriptConfigFieldError" do + let(:err) do + Script::Layers::Domain::Errors::MissingScriptConfigFieldError.new( + field: "field", + filename: "filename", + ) end - end - - describe "when MissingScriptJsonFieldError" do - let(:err) { Script::Layers::Infrastructure::Errors::MissingScriptJsonFieldError.new("field") } it "should call display_and_raise" do should_call_display_and_raise end end - describe "when NoScriptConfigYmlFileError" do - let(:err) { Script::Layers::Infrastructure::Errors::NoScriptConfigYmlFileError.new("filename") } + describe "when NoScriptConfigFileError" do + let(:err) { Script::Layers::Infrastructure::Errors::NoScriptConfigFileError.new("filename") } it "should call display_and_raise" do should_call_display_and_raise end @@ -233,36 +229,68 @@ def should_call_display_and_raise end end + describe "when ScriptConfigurationDefinitionError" do + let(:err) do + Script::Layers::Infrastructure::Errors::ScriptConfigurationDefinitionError.new( + message: "message", + filename: "filename", + ) + end + it "should call display_and_raise" do + should_call_display_and_raise + end + end + describe "when ScriptConfigSyntaxError" do - let(:err) { Script::Layers::Infrastructure::Errors::ScriptConfigSyntaxError.new } + let(:err) { Script::Layers::Infrastructure::Errors::ScriptConfigSyntaxError.new("filename") } it "should call display_and_raise" do should_call_display_and_raise end end describe "when ScriptConfigMissingKeysError" do - let(:err) { Script::Layers::Infrastructure::Errors::ScriptConfigMissingKeysError.new("keys") } + let(:err) do + Script::Layers::Infrastructure::Errors::ScriptConfigMissingKeysError.new( + missing_keys: "keys", + filename: "filename", + ) + end it "should call display_and_raise" do should_call_display_and_raise end end describe "when ScriptConfigInvalidValueError" do - let(:err) { Script::Layers::Infrastructure::Errors::ScriptConfigInvalidValueError.new("input modes") } + let(:err) do + Script::Layers::Infrastructure::Errors::ScriptConfigInvalidValueError.new( + valid_input_modes: "input modes", + filename: "filename", + ) + end it "should call display_and_raise" do should_call_display_and_raise end end describe "when ScriptConfigFieldsMissingKeysError" do - let(:err) { Script::Layers::Infrastructure::Errors::ScriptConfigFieldsMissingKeysError.new("keys") } + let(:err) do + Script::Layers::Infrastructure::Errors::ScriptConfigFieldsMissingKeysError.new( + missing_keys: "keys", + filename: "filename", + ) + end it "should call display_and_raise" do should_call_display_and_raise end end describe "when ScriptConfigFieldsInvalidValueError" do - let(:err) { Script::Layers::Infrastructure::Errors::ScriptConfigFieldsInvalidValueError.new("types") } + let(:err) do + Script::Layers::Infrastructure::Errors::ScriptConfigFieldsInvalidValueError.new( + valid_types: "types", + filename: "filename", + ) + end it "should call display_and_raise" do should_call_display_and_raise end