From 127db6c6921e3fa4ba28b08edc92088958d4a519 Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Sat, 30 Apr 2022 14:01:23 -0400 Subject: [PATCH 01/13] Simplify `ArgumentMetadata` type Add value resolver for enum types --- .../spec/arguments/argument_metadata_spec.cr | 7 +++- .../request_attribute_value_resolver_spec.cr | 8 ++-- .../resolvers/request_value_resolver_spec.cr | 4 +- src/components/framework/spec/spec_helper.cr | 4 +- .../src/arguments/argument_metadata.cr | 12 +++--- .../resolvers/enum_value_resolver.cr | 40 +++++++++++++++++++ .../request_attribute_value_resolver.cr | 2 +- .../resolvers/request_value_resolver.cr | 6 ++- .../ext/routing/annotation_route_loader.cr | 2 +- 9 files changed, 67 insertions(+), 18 deletions(-) create mode 100644 src/components/framework/src/arguments/resolvers/enum_value_resolver.cr diff --git a/src/components/framework/spec/arguments/argument_metadata_spec.cr b/src/components/framework/spec/arguments/argument_metadata_spec.cr index b1e36c53a..78573220f 100644 --- a/src/components/framework/spec/arguments/argument_metadata_spec.cr +++ b/src/components/framework/spec/arguments/argument_metadata_spec.cr @@ -4,11 +4,14 @@ describe ATH::Arguments::ArgumentMetadata do describe "#initialize" do describe "#nilable?" do it "when is_nilable is true" do - ATH::Arguments::ArgumentMetadata(Int32).new("id", true).nilable?.should be_true + ATH::Arguments::ArgumentMetadata(Int32).new("id").nilable?.should be_false + ATH::Arguments::ArgumentMetadata(String | Bool).new("id").nilable?.should be_false end it "type is Nil" do - ATH::Arguments::ArgumentMetadata(Nil).new("id", false).nilable?.should be_true + ATH::Arguments::ArgumentMetadata(Nil).new("id").nilable?.should be_true + ATH::Arguments::ArgumentMetadata(Int32?).new("id").nilable?.should be_true + ATH::Arguments::ArgumentMetadata(String | Bool | Nil).new("id").nilable?.should be_true end end end diff --git a/src/components/framework/spec/arguments/resolvers/request_attribute_value_resolver_spec.cr b/src/components/framework/spec/arguments/resolvers/request_attribute_value_resolver_spec.cr index b9ab4d8e3..34dbf4617 100644 --- a/src/components/framework/spec/arguments/resolvers/request_attribute_value_resolver_spec.cr +++ b/src/components/framework/spec/arguments/resolvers/request_attribute_value_resolver_spec.cr @@ -24,7 +24,7 @@ describe ATH::Arguments::Resolvers::RequestAttribute do describe "that needs to be converted" do it String do - argument = ATH::Arguments::ArgumentMetadata(Int32).new("id", false) + argument = ATH::Arguments::ArgumentMetadata(Int32).new "id" request = new_request request.attributes.set "id", "1" @@ -33,7 +33,7 @@ describe ATH::Arguments::Resolvers::RequestAttribute do end it Bool do - argument = ATH::Arguments::ArgumentMetadata(Bool).new("id", false) + argument = ATH::Arguments::ArgumentMetadata(Bool).new "id" request = new_request request.attributes.set "id", "false" @@ -42,12 +42,12 @@ describe ATH::Arguments::Resolvers::RequestAttribute do end it "that fails conversion" do - argument = ATH::Arguments::ArgumentMetadata(Int32).new("id", false) + argument = ATH::Arguments::ArgumentMetadata(Int32).new "id" request = new_request request.attributes.set "id", "foo" - expect_raises ATH::Exceptions::BadRequest, "Required parameter 'id' with value 'foo' could not be converted into a valid 'Int32'" do + expect_raises ATH::Exceptions::BadRequest, "Parameter 'id' with value 'foo' could not be converted into a valid 'Int32'." do ATH::Arguments::Resolvers::RequestAttribute.new.resolve request, argument end end diff --git a/src/components/framework/spec/arguments/resolvers/request_value_resolver_spec.cr b/src/components/framework/spec/arguments/resolvers/request_value_resolver_spec.cr index 3c4b30411..2a13d0efd 100644 --- a/src/components/framework/spec/arguments/resolvers/request_value_resolver_spec.cr +++ b/src/components/framework/spec/arguments/resolvers/request_value_resolver_spec.cr @@ -3,13 +3,13 @@ require "../../spec_helper" describe ATH::Arguments::Resolvers::Request do describe "#supports" do it ATH::Request do - argument = ATH::Arguments::ArgumentMetadata(ATH::Request).new("id", false) + argument = ATH::Arguments::ArgumentMetadata(ATH::Request).new "id" ATH::Arguments::Resolvers::Request.new.supports?(new_request, argument).should be_true end it TestController do - argument = ATH::Arguments::ArgumentMetadata(TestController).new("id", false) + argument = ATH::Arguments::ArgumentMetadata(TestController).new "id" ATH::Arguments::Resolvers::Request.new.supports?(new_request, argument).should be_false end diff --git a/src/components/framework/spec/spec_helper.cr b/src/components/framework/spec/spec_helper.cr index 343822414..74e006c2f 100644 --- a/src/components/framework/spec/spec_helper.cr +++ b/src/components/framework/spec/spec_helper.cr @@ -68,8 +68,8 @@ def new_context(*, request : ATH::Request = new_request, response : HTTP::Server HTTP::Server::Context.new request, response end -def new_argument(is_nilable : Bool = false) : ATH::Arguments::ArgumentMetadata - ATH::Arguments::ArgumentMetadata(Int32).new("id", is_nilable) +def new_argument : ATH::Arguments::ArgumentMetadata + ATH::Arguments::ArgumentMetadata(Int32).new "id" end def new_action( diff --git a/src/components/framework/src/arguments/argument_metadata.cr b/src/components/framework/src/arguments/argument_metadata.cr index b6a9bfae3..6d558231c 100644 --- a/src/components/framework/src/arguments/argument_metadata.cr +++ b/src/components/framework/src/arguments/argument_metadata.cr @@ -3,13 +3,15 @@ struct Athena::Framework::Arguments::ArgumentMetadata(T) # The name of the argument. getter name : String - # The type of the parameter, i.e. what its type restriction is. - getter type : T.class + def initialize(@name : String); end # If `nil` is a valid argument for the argument. - getter? nilable : Bool + def nilable? : Bool + {{T.nilable?}} + end - def initialize(@name : String, is_nilable : Bool = false, @type : T.class = T) - @nilable = is_nilable || @type == Nil + # The type of the parameter, i.e. what its type restriction is. + def type : T.class + T end end diff --git a/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr b/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr new file mode 100644 index 000000000..cee407408 --- /dev/null +++ b/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr @@ -0,0 +1,40 @@ +@[ADI::Register(tags: [{name: ATH::Arguments::Resolvers::TAG, priority: 105}])] +struct Athena::Framework::Arguments::Resolvers::Enum + include Athena::Framework::Arguments::Resolvers::ArgumentValueResolverInterface + + # :inherit: + def supports?(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata(::Enum?)) : Bool + request.attributes.has? argument.name + end + + # :inherit: + def supports?(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata) : Bool + false + end + + # :inherit: + def resolve(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata(EnumType)) forall EnumType + {% if EnumType <= ::Enum %} + return unless (value = request.attributes.get(argument.name, String?)) + + enum_type = {{ EnumType.nilable? ? EnumType.union_types.reject(&.nilable?).first : EnumType }} + + member = if (num = value.to_i64?(whitespace: false)) && (m = enum_type.from_value? num) + m + elsif (m = enum_type.parse? value) + m + end + + unless member + raise ATH::Exceptions::BadRequest.new "Parameter '#{argument.name}' of enum type '#{EnumType}' has no valid member for '#{value}'." + end + + member + {% end %} + end + + # :inherit: + def resolve(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata) + # Noop overload for non Enum types. + end +end diff --git a/src/components/framework/src/arguments/resolvers/request_attribute_value_resolver.cr b/src/components/framework/src/arguments/resolvers/request_attribute_value_resolver.cr index 8b5307f8d..bab080a41 100644 --- a/src/components/framework/src/arguments/resolvers/request_attribute_value_resolver.cr +++ b/src/components/framework/src/arguments/resolvers/request_attribute_value_resolver.cr @@ -23,6 +23,6 @@ struct Athena::Framework::Arguments::Resolvers::RequestAttribute argument.type.from_parameter value rescue ex : ArgumentError # Catch type cast errors and bubble it up as an BadRequest - raise ATH::Exceptions::BadRequest.new "Required parameter '#{argument.name}' with value '#{value}' could not be converted into a valid '#{argument.type}'", cause: ex + raise ATH::Exceptions::BadRequest.new "Parameter '#{argument.name}' with value '#{value}' could not be converted into a valid '#{argument.type}'.", cause: ex end end diff --git a/src/components/framework/src/arguments/resolvers/request_value_resolver.cr b/src/components/framework/src/arguments/resolvers/request_value_resolver.cr index 36d3c43b4..ba224a6fb 100644 --- a/src/components/framework/src/arguments/resolvers/request_value_resolver.cr +++ b/src/components/framework/src/arguments/resolvers/request_value_resolver.cr @@ -11,8 +11,12 @@ struct Athena::Framework::Arguments::Resolvers::Request include Athena::Framework::Arguments::Resolvers::ArgumentValueResolverInterface # :inherit: + def supports?(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata(ATH::Request)) : Bool + true + end + def supports?(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata) : Bool - argument.type == ATH::Request + false end # :inherit: diff --git a/src/components/framework/src/ext/routing/annotation_route_loader.cr b/src/components/framework/src/ext/routing/annotation_route_loader.cr index f63bf1b43..84b476f79 100644 --- a/src/components/framework/src/ext/routing/annotation_route_loader.cr +++ b/src/components/framework/src/ext/routing/annotation_route_loader.cr @@ -179,7 +179,7 @@ module Athena::Framework::Routing::AnnotationRouteLoader # Process controller action arguments. m.args.each do |arg| arg.raise "Route action argument '#{klass.name}##{m.name}:#{arg.name}' must have a type restriction." if arg.restriction.is_a? Nop - arguments << %(ATH::Arguments::ArgumentMetadata(#{arg.restriction}).new(#{arg.name.stringify}, #{arg.restriction.resolve.nilable?})).id + arguments << %(ATH::Arguments::ArgumentMetadata(#{arg.restriction}).new(#{arg.name.stringify})).id end # Process param converters From aa038eeaf3787c1cb82da1b8b520036fc9272d5b Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Sat, 30 Apr 2022 20:27:31 -0400 Subject: [PATCH 02/13] Add test coverage --- .../resolvers/enum_value_resolver_spec.cr | 105 ++++++++++++++++++ .../request_attribute_value_resolver_spec.cr | 2 +- .../resolvers/request_value_resolver_spec.cr | 2 +- .../resolvers/enum_value_resolver.cr | 40 ++++--- 4 files changed, 130 insertions(+), 19 deletions(-) create mode 100644 src/components/framework/spec/arguments/resolvers/enum_value_resolver_spec.cr diff --git a/src/components/framework/spec/arguments/resolvers/enum_value_resolver_spec.cr b/src/components/framework/spec/arguments/resolvers/enum_value_resolver_spec.cr new file mode 100644 index 000000000..3e50317cc --- /dev/null +++ b/src/components/framework/spec/arguments/resolvers/enum_value_resolver_spec.cr @@ -0,0 +1,105 @@ +require "../../spec_helper" + +enum TestEnum + A + B + C +end + +describe ATH::Arguments::Resolvers::Enum, focus: true do + describe "#supports?" do + describe ::Enum do + it "that does exist in request attributes" do + argument = ATH::Arguments::ArgumentMetadata(TestEnum).new "enum" + request = new_request + request.attributes.set "enum", 1 + + ATH::Arguments::Resolvers::Enum.new.supports?(request, argument).should be_true + end + + it "that does exist in request attributes" do + argument = ATH::Arguments::ArgumentMetadata(TestEnum?).new "enum" + request = new_request + request.attributes.set "enum", "1" + + ATH::Arguments::Resolvers::Enum.new.supports?(request, argument).should be_true + end + + it "that does not exist in request attributes" do + argument = ATH::Arguments::ArgumentMetadata(TestEnum).new "enum" + + ATH::Arguments::Resolvers::Enum.new.supports?(new_request, argument).should be_false + end + + it "that is a union of another type" do + argument = ATH::Arguments::ArgumentMetadata(TestEnum | String).new "enum" + + ATH::Arguments::Resolvers::Enum.new.supports?(new_request, argument).should be_false + end + end + + it "some other type" do + ATH::Arguments::Resolvers::Enum.new.supports?(new_request, ATH::Arguments::ArgumentMetadata(Int32).new "enum").should be_false + ATH::Arguments::Resolvers::Enum.new.supports?(new_request, ATH::Arguments::ArgumentMetadata(Int32?).new "enum").should be_false + ATH::Arguments::Resolvers::Enum.new.supports?(new_request, ATH::Arguments::ArgumentMetadata(Bool | String).new "enum").should be_false + end + end + + describe "#resolve" do + it "with a nil value in attributes" do + argument = ATH::Arguments::ArgumentMetadata(TestEnum).new "enum" + + request = new_request + request.attributes.set "enum", nil + + ATH::Arguments::Resolvers::Enum.new.resolve(request, argument).should be_nil + end + + it "with a numeric based value" do + argument = ATH::Arguments::ArgumentMetadata(TestEnum).new "enum" + + request = new_request + request.attributes.set "enum", "2" + + ATH::Arguments::Resolvers::Enum.new.resolve(request, argument).should eq TestEnum::C + end + + it "with a numeric based value with whitespace" do + argument = ATH::Arguments::ArgumentMetadata(TestEnum).new "enum" + + request = new_request + request.attributes.set "enum", "2" + + ATH::Arguments::Resolvers::Enum.new.resolve(request, argument).should eq TestEnum::C + end + + it "with a string based value" do + argument = ATH::Arguments::ArgumentMetadata(TestEnum).new "enum" + + request = new_request + request.attributes.set "enum", "B" + + ATH::Arguments::Resolvers::Enum.new.resolve(request, argument).should eq TestEnum::B + end + + it "with a nilable enum type" do + argument = ATH::Arguments::ArgumentMetadata(TestEnum?).new "enum" + + request = new_request + request.attributes.set "enum", "C" + + ATH::Arguments::Resolvers::Enum.new.resolve(request, argument).should eq TestEnum::C + end + + it "with an unknown member value" do + argument = ATH::Arguments::ArgumentMetadata(TestEnum).new "enum" + + request = new_request + request.attributes.set "enum", " 4 " + + expect_raises ATH::Exceptions::BadRequest, "Parameter 'enum' of enum type 'TestEnum' has no valid member for ' 4 '." do + ATH::Arguments::Resolvers::Enum.new.resolve request, argument + end + end + end +end diff --git a/src/components/framework/spec/arguments/resolvers/request_attribute_value_resolver_spec.cr b/src/components/framework/spec/arguments/resolvers/request_attribute_value_resolver_spec.cr index 34dbf4617..ef36cd847 100644 --- a/src/components/framework/spec/arguments/resolvers/request_attribute_value_resolver_spec.cr +++ b/src/components/framework/spec/arguments/resolvers/request_attribute_value_resolver_spec.cr @@ -1,7 +1,7 @@ require "../../spec_helper" describe ATH::Arguments::Resolvers::RequestAttribute do - describe "#supports" do + describe "#supports?" do it "that exists in the request attributes" do request = new_request request.attributes.set "id", 1 diff --git a/src/components/framework/spec/arguments/resolvers/request_value_resolver_spec.cr b/src/components/framework/spec/arguments/resolvers/request_value_resolver_spec.cr index 2a13d0efd..80c48c7bc 100644 --- a/src/components/framework/spec/arguments/resolvers/request_value_resolver_spec.cr +++ b/src/components/framework/spec/arguments/resolvers/request_value_resolver_spec.cr @@ -1,7 +1,7 @@ require "../../spec_helper" describe ATH::Arguments::Resolvers::Request do - describe "#supports" do + describe "#supports?" do it ATH::Request do argument = ATH::Arguments::ArgumentMetadata(ATH::Request).new "id" diff --git a/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr b/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr index cee407408..e7f1b1c79 100644 --- a/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr +++ b/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr @@ -13,28 +13,34 @@ struct Athena::Framework::Arguments::Resolvers::Enum end # :inherit: - def resolve(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata(EnumType)) forall EnumType - {% if EnumType <= ::Enum %} - return unless (value = request.attributes.get(argument.name, String?)) + def resolve(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata(::Enum?)) + return unless (value = request.attributes.get(argument.name, String?)) - enum_type = {{ EnumType.nilable? ? EnumType.union_types.reject(&.nilable?).first : EnumType }} - - member = if (num = value.to_i64?(whitespace: false)) && (m = enum_type.from_value? num) - m - elsif (m = enum_type.parse? value) - m - end - - unless member - raise ATH::Exceptions::BadRequest.new "Parameter '#{argument.name}' of enum type '#{EnumType}' has no valid member for '#{value}'." - end - - member - {% end %} + self.resolve value, argument.name, argument.type end # :inherit: def resolve(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata) # Noop overload for non Enum types. end + + private def resolve(value, name : String, _enum_type : EnumType.class) forall EnumType + {% begin %} + {% enum_type = EnumType.nilable? ? EnumType.union_types.reject(&.nilable?).first : EnumType %} + + {% if enum_type && enum_type <= ::Enum %} + member = if (num = value.to_i128?(whitespace: false)) && (m = {{enum_type}}.from_value? num) + m + elsif (m = {{enum_type}}.parse? value) + m + end + + unless member + raise ATH::Exceptions::BadRequest.new "Parameter '#{name}' of enum type '#{{{enum_type}}}' has no valid member for '#{value}'." + end + + member + {% end %} + {% end %} + end end From d8bb4c1c413384ef4e3399d817549e68bc01f89d Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Sun, 1 May 2022 18:41:48 -0400 Subject: [PATCH 03/13] Leverage new version of `assert_error` Also add `assert_success` --- .../framework/spec/compiler_spec.cr | 26 ++++------ src/components/spec/spec/abstract_class.cr | 3 -- src/components/spec/spec/methods_spec.cr | 5 +- src/components/spec/src/methods.cr | 51 ++++++++++++------- 4 files changed, 48 insertions(+), 37 deletions(-) delete mode 100644 src/components/spec/spec/abstract_class.cr diff --git a/src/components/framework/spec/compiler_spec.cr b/src/components/framework/spec/compiler_spec.cr index 2c91690c5..b93606b34 100644 --- a/src/components/framework/spec/compiler_spec.cr +++ b/src/components/framework/spec/compiler_spec.cr @@ -1,25 +1,19 @@ require "./spec_helper" -private def assert_error(message : String | Bool, code : String, *, line : Int32 = __LINE__) : Nil - input = IO::Memory.new <<-CR +private def assert_error(message : String, code : String, *, line : Int32 = __LINE__) : Nil + ASPEC::Methods.assert_error message, <<-CR, line: line require "./spec_helper.cr" #{code} ATH.run CR +end - buffer = IO::Memory.new - result = Process.run("crystal", ["run", "--no-color", "--no-codegen", "--stdin-filename", "#{__DIR__}/test.cr"], input: input.rewind, output: buffer, error: buffer) - - case message - in false - fail buffer.to_s unless result.success? - in true - raise ArgumentError.new "'message' cannot be 'true'." - in String - fail buffer.to_s if result.success? - buffer.to_s.should contain(message), line: line - buffer.close - end +private def assert_success(code : String, *, line : Int32 = __LINE__) : Nil + ASPEC::Methods.assert_success <<-CR, line: line + require "./spec_helper.cr" + #{code} + ATH.run + CR end describe Athena::Framework do @@ -98,7 +92,7 @@ describe Athena::Framework do end it "within a different controller" do - assert_error false, <<-CODE + assert_success <<-CODE class ExampleController < ATH::Controller @[ARTA::Get(path: "/foo")] def action : String diff --git a/src/components/spec/spec/abstract_class.cr b/src/components/spec/spec/abstract_class.cr deleted file mode 100644 index c4cb625ba..000000000 --- a/src/components/spec/spec/abstract_class.cr +++ /dev/null @@ -1,3 +0,0 @@ -abstract class Foo; end - -Foo.new diff --git a/src/components/spec/spec/methods_spec.cr b/src/components/spec/spec/methods_spec.cr index b32828071..6cae673d3 100644 --- a/src/components/spec/spec/methods_spec.cr +++ b/src/components/spec/spec/methods_spec.cr @@ -3,7 +3,10 @@ require "./spec_helper" describe ASPEC::Methods do describe "#assert_error" do it do - assert_error "abstract_class.cr", "can't instantiate abstract class Foo", prefix: "#{__DIR__}/" + assert_error "can't instantiate abstract class Foo", <<-CR + abstract class Foo; end + Foo.new + CR end end diff --git a/src/components/spec/src/methods.cr b/src/components/spec/src/methods.cr index c89002740..67f496882 100644 --- a/src/components/spec/src/methods.cr +++ b/src/components/spec/src/methods.cr @@ -9,34 +9,51 @@ module Athena::Spec::Methods extend self - # Runs the Crystal program at the provided *file_path* and asserts it errors with the provided *message*. + # Executes the provided Crystal *code* asserts it errors with the provided *message*. # The main purpose of this method is to test compile time errors. # - # By default, *file_path* is assumed to be within `spec/`, but can be customized via the *prefix* named argument. - # - # NOTE: - # # ``` - # # ./spec/abstract_class.cr - # abstract class Foo; end - # - # Foo.new + # ASPEC::Methods.assert_error "can't instantiate abstract class Foo", <<-CR + # abstract class Foo; end + # Foo.new + # CR # ``` # - # ``` - # # ./spec/abstract_class_spec.cr - # require "athena-spec" + # NOTE: When files are required within the *code*, they are relative to the file calling this method. + def assert_error(message : String, code : String, *, line : Int32 = __LINE__, file : String = __FILE__) : Nil + buffer = IO::Memory.new + result = execute code, buffer, file + + fail buffer.to_s, line: line if result.success? + buffer.to_s.should contain(message), line: line + buffer.close + end + + # Similar to `.assert_error`, but asserts the provided Crystal *code* successfully compiles. # - # ASPEC::Methods.assert_error "abstract_class.cr", "can't instantiate abstract class Foo" # ``` - def assert_error(file_path : String, message : String, *, prefix : String = "spec/") : Nil + # ASPEC::Methods.assert_success <<-CR + # puts 2 + 2 + # CR + # ``` + # + # NOTE: When files are required within the *code*, they are relative to the file calling this method. + def assert_success(code : String, *, line : Int32 = __LINE__, file : String = __FILE__) : Nil buffer = IO::Memory.new - result = Process.run("crystal", ["run", "--no-color", "--no-codegen", "#{prefix}#{file_path}"], error: buffer) - fail buffer.to_s if result.success? - buffer.to_s.should contain message + result = execute code, buffer, file + + fail buffer.to_s, line: line unless result.success? buffer.close end + private def execute(code : String, buffer : IO, file : String) : Process::Status + input = IO::Memory.new <<-CR + #{code} + CR + + Process.run("crystal", ["run", "--no-color", "--no-codegen", "--stdin-filename", "#{file}"], input: input.rewind, output: buffer, error: buffer) + end + # Runs the executable at the given *path*, optionally with the provided *args*. # # The standard output, error output, and status of the execution are yielded. From d8940f36eabf49f55720f8d579478c0f338d22b3 Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Sun, 1 May 2022 18:50:08 -0400 Subject: [PATCH 04/13] Add helper type to create requirements based on enum members Refactor enum value resolver to not use macros --- .../resolvers/enum_value_resolver_spec.cr | 2 +- .../resolvers/enum_value_resolver.cr | 38 ++++++++++--------- .../routing/spec/requirement/enum_spec.cr | 33 ++++++++++++++++ src/components/routing/src/athena-routing.cr | 1 + .../routing/src/requirement/enum.cr | 17 +++++++++ .../routing/src/requirement/requirement.cr | 20 ++++++++++ 6 files changed, 93 insertions(+), 18 deletions(-) create mode 100644 src/components/routing/spec/requirement/enum_spec.cr create mode 100644 src/components/routing/src/requirement/enum.cr create mode 100644 src/components/routing/src/requirement/requirement.cr diff --git a/src/components/framework/spec/arguments/resolvers/enum_value_resolver_spec.cr b/src/components/framework/spec/arguments/resolvers/enum_value_resolver_spec.cr index 3e50317cc..342e9ffec 100644 --- a/src/components/framework/spec/arguments/resolvers/enum_value_resolver_spec.cr +++ b/src/components/framework/spec/arguments/resolvers/enum_value_resolver_spec.cr @@ -6,7 +6,7 @@ enum TestEnum C end -describe ATH::Arguments::Resolvers::Enum, focus: true do +describe ATH::Arguments::Resolvers::Enum do describe "#supports?" do describe ::Enum do it "that does exist in request attributes" do diff --git a/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr b/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr index e7f1b1c79..874fdc70b 100644 --- a/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr +++ b/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr @@ -24,23 +24,27 @@ struct Athena::Framework::Arguments::Resolvers::Enum # Noop overload for non Enum types. end + private def resolve(value, name : String, _enum_type : Nil.class) + # Noop if the argument has a type of explicitly `Nil`. + end + private def resolve(value, name : String, _enum_type : EnumType.class) forall EnumType - {% begin %} - {% enum_type = EnumType.nilable? ? EnumType.union_types.reject(&.nilable?).first : EnumType %} - - {% if enum_type && enum_type <= ::Enum %} - member = if (num = value.to_i128?(whitespace: false)) && (m = {{enum_type}}.from_value? num) - m - elsif (m = {{enum_type}}.parse? value) - m - end - - unless member - raise ATH::Exceptions::BadRequest.new "Parameter '#{name}' of enum type '#{{{enum_type}}}' has no valid member for '#{value}'." - end - - member - {% end %} - {% end %} + # Remove `Nil` from the union. + enum_type = typeof(begin + val = uninitialized EnumType + val.nil? ? raise("") : val + end) + + member = if (num = value.to_i128?(whitespace: false)) && (m = enum_type.from_value? num) + m + elsif (m = enum_type.parse? value) + m + end + + unless member + raise ATH::Exceptions::BadRequest.new "Parameter '#{name}' of enum type '#{enum_type}' has no valid member for '#{value}'." + end + + member end end diff --git a/src/components/routing/spec/requirement/enum_spec.cr b/src/components/routing/spec/requirement/enum_spec.cr new file mode 100644 index 000000000..131196ea0 --- /dev/null +++ b/src/components/routing/spec/requirement/enum_spec.cr @@ -0,0 +1,33 @@ +require "../spec_helper" + +enum EnumRequirementEnum + A + B + C +end + +@[Flags] +enum EnumRequirementEnumFlags + A + B + C +end + +struct EnumRequirementTest < ASPEC::TestCase + def test_to_s_no_members : Nil + ART::Requirement::Enum(EnumRequirementEnum).new.to_s.should eq "a|b|c" + ART::Requirement::Enum(EnumRequirementEnumFlags).new.to_s.should eq "a|b|c" + end + + def test_to_s_with_members : Nil + ART::Requirement::Enum(EnumRequirementEnum).new(:a, :c).to_s.should eq "a|c" + ART::Requirement::Enum(EnumRequirementEnumFlags).new(:b, :c).to_s.should eq "b|c" + end + + def test_constructor_non_enum_type : Nil + self.assert_error "'Int32' is not an Enum type.", <<-CR + require "../spec_helper" + ART::Requirement::Enum(Int32).new + CR + end +end diff --git a/src/components/routing/src/athena-routing.cr b/src/components/routing/src/athena-routing.cr index 28848a9e0..2a5d8c28d 100644 --- a/src/components/routing/src/athena-routing.cr +++ b/src/components/routing/src/athena-routing.cr @@ -15,6 +15,7 @@ require "./router" require "./exception/*" require "./generator/*" require "./matcher/*" +require "./requirement/*" # Convenience alias to make referencing `Athena::Routing` types easier. alias ART = Athena::Routing diff --git a/src/components/routing/src/requirement/enum.cr b/src/components/routing/src/requirement/enum.cr new file mode 100644 index 000000000..697b1fe7b --- /dev/null +++ b/src/components/routing/src/requirement/enum.cr @@ -0,0 +1,17 @@ +struct Athena::Routing::Requirement::Enum(EnumType) + getter members : Set(EnumType)? = nil + + def self.new(*cases : EnumType) + new cases.to_set + end + + def initialize(@members : Set(EnumType)? = nil) + {% raise "'#{EnumType}' is not an Enum type." unless EnumType <= ::Enum %} + end + + def to_s(io : IO) : Nil + (@members || EnumType.values).join io, '|' do |member, io| + io << Regex.escape member.to_s.underscore + end + end +end diff --git a/src/components/routing/src/requirement/requirement.cr b/src/components/routing/src/requirement/requirement.cr new file mode 100644 index 000000000..751719d8f --- /dev/null +++ b/src/components/routing/src/requirement/requirement.cr @@ -0,0 +1,20 @@ +module Athena::Routing::Requirement + # Sourced from https://github.com/symfony/symfony/blob/c70be0957a11fd8b7aa687d6173e76724068daa4/src/Symfony/Component/Routing/Requirement/Requirement.php + + ASCII_SLUG = /[A-Za-z0-9]+(?:-[A-Za-z0-9]+)*/ + CATCH_ALL = /.+/ + + # Matches a date string in the format of `YYYY-MM-DD`. + DATE_YMD = /[0-9]{4}-(?:0[1-9]|1[012])-(?:0[1-9]|[12][0-9]|(? Date: Sun, 1 May 2022 19:01:08 -0400 Subject: [PATCH 05/13] Stop including `SPEC::Methods` in top level of framework specs --- src/components/framework/spec/spec_helper.cr | 2 -- src/components/framework/spec/streamed_response_spec.cr | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/framework/spec/spec_helper.cr b/src/components/framework/spec/spec_helper.cr index 74e006c2f..1388786c2 100644 --- a/src/components/framework/spec/spec_helper.cr +++ b/src/components/framework/spec/spec_helper.cr @@ -9,8 +9,6 @@ require "athena-event_dispatcher/spec" require "athena-validator/spec" require "../src/spec" -include ASPEC::Methods - ASPEC.run_all class TestController < ATH::Controller diff --git a/src/components/framework/spec/streamed_response_spec.cr b/src/components/framework/spec/streamed_response_spec.cr index 78e09809d..63e5b8395 100644 --- a/src/components/framework/spec/streamed_response_spec.cr +++ b/src/components/framework/spec/streamed_response_spec.cr @@ -7,6 +7,9 @@ private struct TestWriter < ATH::Response::Writer end end +# FIXME: Refactor these specs to not depend on calling a protected method. +include ATHA + describe ATH::StreamedResponse do describe ".new" do it "accepts a block" do From ac487e617cf35614a04d495f3a2e1271ee816520 Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Sun, 1 May 2022 20:28:16 -0400 Subject: [PATCH 06/13] Migrate DI component specs to new `assert_error` method Remove global include of `ASPEC::Methods` --- .../compiler/array_named_argument_missing.cr | 10 - .../binding_array_argument_missing.cr | 12 - .../cannot_auto_register_missing_service.cr | 11 - ...annot_auto_register_multiple_candidates.cr | 21 -- .../spec/compiler/factory_instance_method.cr | 10 - .../spec/compiler/factory_missing_method.cr | 10 - ...generic_service_generics_count_mismatch.cr | 8 - .../generic_service_generics_not_provided.cr | 8 - .../generic_service_name_not_provided.cr | 8 - .../multiple_services_on_type_missing_name.cr | 12 - .../compiler/nested_array_named_argument.cr | 8 - .../spec/compiler/private_unused_service.cr | 8 - .../tagged_service_invalid_tag_list_type.cr | 7 - .../tagged_service_invalid_tag_type.cr | 7 - .../tagged_service_name_not_provided.cr | 7 - .../spec/compiler_spec.cr | 252 ++++++++++++------ .../dependency_injection/spec/spec_helper.cr | 2 - 17 files changed, 173 insertions(+), 228 deletions(-) delete mode 100644 src/components/dependency_injection/spec/compiler/array_named_argument_missing.cr delete mode 100644 src/components/dependency_injection/spec/compiler/binding_array_argument_missing.cr delete mode 100644 src/components/dependency_injection/spec/compiler/cannot_auto_register_missing_service.cr delete mode 100644 src/components/dependency_injection/spec/compiler/cannot_auto_register_multiple_candidates.cr delete mode 100644 src/components/dependency_injection/spec/compiler/factory_instance_method.cr delete mode 100644 src/components/dependency_injection/spec/compiler/factory_missing_method.cr delete mode 100644 src/components/dependency_injection/spec/compiler/generic_service_generics_count_mismatch.cr delete mode 100644 src/components/dependency_injection/spec/compiler/generic_service_generics_not_provided.cr delete mode 100644 src/components/dependency_injection/spec/compiler/generic_service_name_not_provided.cr delete mode 100644 src/components/dependency_injection/spec/compiler/multiple_services_on_type_missing_name.cr delete mode 100644 src/components/dependency_injection/spec/compiler/nested_array_named_argument.cr delete mode 100644 src/components/dependency_injection/spec/compiler/private_unused_service.cr delete mode 100644 src/components/dependency_injection/spec/compiler/tagged_service_invalid_tag_list_type.cr delete mode 100644 src/components/dependency_injection/spec/compiler/tagged_service_invalid_tag_type.cr delete mode 100644 src/components/dependency_injection/spec/compiler/tagged_service_name_not_provided.cr diff --git a/src/components/dependency_injection/spec/compiler/array_named_argument_missing.cr b/src/components/dependency_injection/spec/compiler/array_named_argument_missing.cr deleted file mode 100644 index fd9536cb0..000000000 --- a/src/components/dependency_injection/spec/compiler/array_named_argument_missing.cr +++ /dev/null @@ -1,10 +0,0 @@ -require "../spec_helper" - -record Foo - -@[ADI::Register(_values: ["@foo"])] -class Klass - def initialize(@values : Array(Foo)); end -end - -ADI::ServiceContainer.new diff --git a/src/components/dependency_injection/spec/compiler/binding_array_argument_missing.cr b/src/components/dependency_injection/spec/compiler/binding_array_argument_missing.cr deleted file mode 100644 index 6a18ce5ac..000000000 --- a/src/components/dependency_injection/spec/compiler/binding_array_argument_missing.cr +++ /dev/null @@ -1,12 +0,0 @@ -require "../spec_helper" - -record Foo - -ADI.bind values, ["@foo"] - -@[ADI::Register] -class Klass - def initialize(@values : Array(Foo)); end -end - -ADI::ServiceContainer.new diff --git a/src/components/dependency_injection/spec/compiler/cannot_auto_register_missing_service.cr b/src/components/dependency_injection/spec/compiler/cannot_auto_register_missing_service.cr deleted file mode 100644 index 8f41c9ee2..000000000 --- a/src/components/dependency_injection/spec/compiler/cannot_auto_register_missing_service.cr +++ /dev/null @@ -1,11 +0,0 @@ -require "../spec_helper" - -class MissingService -end - -@[ADI::Register] -class Klass - def initialize(@service : MissingService); end -end - -ADI::ServiceContainer.new diff --git a/src/components/dependency_injection/spec/compiler/cannot_auto_register_multiple_candidates.cr b/src/components/dependency_injection/spec/compiler/cannot_auto_register_multiple_candidates.cr deleted file mode 100644 index 1fd2206f6..000000000 --- a/src/components/dependency_injection/spec/compiler/cannot_auto_register_multiple_candidates.cr +++ /dev/null @@ -1,21 +0,0 @@ -require "../spec_helper" - -module Interface -end - -@[ADI::Register] -class One - include Interface -end - -@[ADI::Register] -class Two - include Interface -end - -@[ADI::Register] -class Klass - def initialize(@service : Interface); end -end - -ADI::ServiceContainer.new diff --git a/src/components/dependency_injection/spec/compiler/factory_instance_method.cr b/src/components/dependency_injection/spec/compiler/factory_instance_method.cr deleted file mode 100644 index 43db57995..000000000 --- a/src/components/dependency_injection/spec/compiler/factory_instance_method.cr +++ /dev/null @@ -1,10 +0,0 @@ -require "../spec_helper" - -@[ADI::Register(_value: 10, factory: "double")] -class Klass - def double(value : Int32) : self - new value - end - - def initialize(@value : Int32); end -end diff --git a/src/components/dependency_injection/spec/compiler/factory_missing_method.cr b/src/components/dependency_injection/spec/compiler/factory_missing_method.cr deleted file mode 100644 index c1862e4dc..000000000 --- a/src/components/dependency_injection/spec/compiler/factory_missing_method.cr +++ /dev/null @@ -1,10 +0,0 @@ -require "../spec_helper" - -@[ADI::Register(_value: 10, factory: "double")] -class Klass - def self.triple(value : Int32) : self - new value - end - - def initialize(@value : Int32); end -end diff --git a/src/components/dependency_injection/spec/compiler/generic_service_generics_count_mismatch.cr b/src/components/dependency_injection/spec/compiler/generic_service_generics_count_mismatch.cr deleted file mode 100644 index c86d1740e..000000000 --- a/src/components/dependency_injection/spec/compiler/generic_service_generics_count_mismatch.cr +++ /dev/null @@ -1,8 +0,0 @@ -require "../spec_helper" - -@[ADI::Register(Int32, name: "generic_service")] -class GenericService(A, B) - def initialize(@value : B); end -end - -ADI::ServiceContainer.new diff --git a/src/components/dependency_injection/spec/compiler/generic_service_generics_not_provided.cr b/src/components/dependency_injection/spec/compiler/generic_service_generics_not_provided.cr deleted file mode 100644 index 175aec332..000000000 --- a/src/components/dependency_injection/spec/compiler/generic_service_generics_not_provided.cr +++ /dev/null @@ -1,8 +0,0 @@ -require "../spec_helper" - -@[ADI::Register(name: "generic_service")] -class GenericService(T) - def initialize(@value : T); end -end - -ADI::ServiceContainer.new diff --git a/src/components/dependency_injection/spec/compiler/generic_service_name_not_provided.cr b/src/components/dependency_injection/spec/compiler/generic_service_name_not_provided.cr deleted file mode 100644 index 0c8a40b33..000000000 --- a/src/components/dependency_injection/spec/compiler/generic_service_name_not_provided.cr +++ /dev/null @@ -1,8 +0,0 @@ -require "../spec_helper" - -@[ADI::Register(generics: {Int32})] -class GenericService(T) - def initialize(@value : T); end -end - -ADI::ServiceContainer.new diff --git a/src/components/dependency_injection/spec/compiler/multiple_services_on_type_missing_name.cr b/src/components/dependency_injection/spec/compiler/multiple_services_on_type_missing_name.cr deleted file mode 100644 index a9cc38a72..000000000 --- a/src/components/dependency_injection/spec/compiler/multiple_services_on_type_missing_name.cr +++ /dev/null @@ -1,12 +0,0 @@ -require "../spec_helper" - -class MissingService -end - -@[ADI::Register(_id: 1, name: "one")] -@[ADI::Register(_id: 2)] -class Klass - def initialize(@id : Int32); end -end - -ADI::ServiceContainer.new diff --git a/src/components/dependency_injection/spec/compiler/nested_array_named_argument.cr b/src/components/dependency_injection/spec/compiler/nested_array_named_argument.cr deleted file mode 100644 index e07d0bcbb..000000000 --- a/src/components/dependency_injection/spec/compiler/nested_array_named_argument.cr +++ /dev/null @@ -1,8 +0,0 @@ -require "../spec_helper" - -@[ADI::Register(_values: [[[1]]])] -class Klass - def initialize(@values : Array(Array(Array(Int32)))); end -end - -ADI::ServiceContainer.new diff --git a/src/components/dependency_injection/spec/compiler/private_unused_service.cr b/src/components/dependency_injection/spec/compiler/private_unused_service.cr deleted file mode 100644 index 109e32d12..000000000 --- a/src/components/dependency_injection/spec/compiler/private_unused_service.cr +++ /dev/null @@ -1,8 +0,0 @@ -require "../spec_helper" - -@[ADI::Register] -class Service - property name : String = "Jim" -end - -ADI::ServiceContainer.new.service diff --git a/src/components/dependency_injection/spec/compiler/tagged_service_invalid_tag_list_type.cr b/src/components/dependency_injection/spec/compiler/tagged_service_invalid_tag_list_type.cr deleted file mode 100644 index eb2db9945..000000000 --- a/src/components/dependency_injection/spec/compiler/tagged_service_invalid_tag_list_type.cr +++ /dev/null @@ -1,7 +0,0 @@ -require "../spec_helper" - -@[ADI::Register(tags: 42)] -class TaggedService -end - -ADI::ServiceContainer.new diff --git a/src/components/dependency_injection/spec/compiler/tagged_service_invalid_tag_type.cr b/src/components/dependency_injection/spec/compiler/tagged_service_invalid_tag_type.cr deleted file mode 100644 index 98dc85360..000000000 --- a/src/components/dependency_injection/spec/compiler/tagged_service_invalid_tag_type.cr +++ /dev/null @@ -1,7 +0,0 @@ -require "../spec_helper" - -@[ADI::Register(tags: [true])] -class TaggedService -end - -ADI::ServiceContainer.new diff --git a/src/components/dependency_injection/spec/compiler/tagged_service_name_not_provided.cr b/src/components/dependency_injection/spec/compiler/tagged_service_name_not_provided.cr deleted file mode 100644 index 2f2fff85b..000000000 --- a/src/components/dependency_injection/spec/compiler/tagged_service_name_not_provided.cr +++ /dev/null @@ -1,7 +0,0 @@ -require "../spec_helper" - -@[ADI::Register(tags: [{priority: 42}])] -class TaggedService -end - -ADI::ServiceContainer.new diff --git a/src/components/dependency_injection/spec/compiler_spec.cr b/src/components/dependency_injection/spec/compiler_spec.cr index 281e2f67d..27a76ebc4 100644 --- a/src/components/dependency_injection/spec/compiler_spec.cr +++ b/src/components/dependency_injection/spec/compiler_spec.cr @@ -1,88 +1,182 @@ require "./spec_helper" -private TEST_CASES = { - { - # A named argument is an array with a service reference that doesn't exist - "array_named_argument_missing", - "Failed to register service 'klass'. Could not resolve argument 'values : Array(Foo)' from named argument value '[\"@foo\"]'.", - }, - { - # A binding value is an array with a service reference that doesn't exist - "binding_array_argument_missing", - "Failed to register service 'klass'. Could not resolve argument 'values : Array(Foo)' from binding value '[\"@foo\"]'.", - }, - { - # More than one service of a given type exist, but ivar name doesn't match any, nor is an alias defined - "cannot_auto_register_multiple_candidates", - "Failed to auto register service 'klass'. Could not resolve argument 'service : Interface'.", - }, - { - # Service could not be auto registered due to an argument not resolving to any services - "cannot_auto_register_missing_service", - "Failed to auto register service 'klass'. Could not resolve argument 'service : MissingService'.", - }, - { - "factory_instance_method", - "Failed to register service `klass`. Factory method `double` within `Klass` is an instance method.", - }, - { - "factory_missing_method", - "Failed to register service `klass`. Factory method `double` within `Klass` does not exist.", - }, - { - # Service based on type that has multiple generic arguments does not provide the correct amount of generic arguments - "generic_service_generics_count_mismatch", - "Failed to register service 'generic_service'. Expected 2 generics types got 1.", - }, - { - # Service based on generic type does not provide any generic arguments - "generic_service_generics_not_provided", - "Failed to register service 'generic_service'. Generic services must provide the types to use via the 'generics' field.", - }, - { - # Service based on generic type does not explicitly provide a name - "generic_service_name_not_provided", - "Failed to register services for 'GenericService(T)'. Generic services must explicitly provide a name.", - }, - { - # Multiple services are based on a type, but not all provide a name - "multiple_services_on_type_missing_name", - "Failed to register services for 'Klass'. Services based on this type must each explicitly provide a name.", - }, - { - # A named argument is an array more than 2 levels deep - "nested_array_named_argument", - "Failed to register service 'klass'. Arrays more than two levels deep are not currently supported.", - }, - { - # Assert the service has been removed from the container - "private_unused_service", - "undefined method 'service' for Athena::DependencyInjection::ServiceContainer", - }, - { - # A name must be supplied if using a NamedTupleLiteral tag - "tagged_service_invalid_tag_list_type", - "Failed to register service `tagged_service`. Tags must be an ArrayLiteral or TupleLiteral, not NumberLiteral.", - }, - { - # Tags can only be NamedTupleLiterals or StringLiterals - "tagged_service_invalid_tag_type", - "Failed to register service `tagged_service`. A tag must be a StringLiteral or NamedTupleLiteral not BoolLiteral.", - }, - { - # A name must be supplied if using a NamedTupleLiteral tag - "tagged_service_name_not_provided", - "Failed to register service `tagged_service`. All tags must have a name.", - }, -} +private def assert_error(message : String, code : String, *, line : Int32 = __LINE__) : Nil + ASPEC::Methods.assert_error message, <<-CR, line: line + require "./spec_helper.cr" + #{code} + ADI::ServiceContainer.new + CR +end describe Athena::DependencyInjection do describe Athena::DependencyInjection::ServiceContainer do describe "compiler errors" do - TEST_CASES.each do |(file_path, message)| - it file_path do - assert_error "compiler/#{file_path}.cr", message, prefix: "#{__DIR__}/" - end + it "named argument is an array with a service reference that doesn't exist" do + assert_error "Failed to register service 'klass'. Could not resolve argument 'values : Array(Foo)' from named argument value '[\"@foo\"]'.", <<-CR + record Foo + + @[ADI::Register(_values: ["@foo"])] + class Klass + def initialize(@values : Array(Foo)); end + end + CR + end + + it "binding value is an array with a service reference that doesn't exist" do + assert_error "Failed to register service 'klass'. Could not resolve argument 'values : Array(Foo)' from binding value '[\"@foo\"]'.", <<-CR + record Foo + + ADI.bind values, ["@foo"] + + @[ADI::Register] + class Klass + def initialize(@values : Array(Foo)); end + end + CR + end + + it "service could not be auto registered due to an argument not resolving to any services" do + assert_error "Failed to auto register service 'klass'. Could not resolve argument 'service : MissingService'.", <<-CR + class MissingService + end + + @[ADI::Register] + class Klass + def initialize(@service : MissingService); end + end + CR + end + + it "more than one service of a given type exist, but ivar name doesn't match any, nor is an alias defined" do + assert_error "Failed to auto register service 'klass'. Could not resolve argument 'service : Interface'.", <<-CR + module Interface + end + + @[ADI::Register] + class One + include Interface + end + + @[ADI::Register] + class Two + include Interface + end + + @[ADI::Register] + class Klass + def initialize(@service : Interface); end + end + CR + end + + it "factory method is instance method" do + assert_error "Failed to register service `klass`. Factory method `double` within `Klass` is an instance method.", <<-CR + @[ADI::Register(_value: 10, factory: "double")] + class Klass + def double(value : Int32) : self + new value + end + + def initialize(@value : Int32); end + end + CR + end + + it "missing factory method" do + assert_error "Failed to register service `klass`. Factory method `double` within `Klass` does not exist.", <<-CR + @[ADI::Register(_value: 10, factory: "double")] + class Klass + def self.triple(value : Int32) : self + new value + end + + def initialize(@value : Int32); end + end + CR + end + + it "service based on type that has multiple generic arguments does not provide the correct amount of generic arguments" do + assert_error "Failed to register service 'generic_service'. Expected 2 generics types got 1.", <<-CR + @[ADI::Register(Int32, name: "generic_service")] + class GenericService(A, B) + def initialize(@value : B); end + end + CR + end + + it "service based on generic type does not provide any generic arguments" do + assert_error "Failed to register service 'generic_service'. Generic services must provide the types to use via the 'generics' field.", <<-CR + @[ADI::Register(name: "generic_service")] + class GenericService(T) + def initialize(@value : T); end + end + CR + end + + it "service based on generic type does not explicitly provide a name" do + assert_error "Failed to register services for 'GenericService(T)'. Generic services must explicitly provide a name.", <<-CR + @[ADI::Register(generics: {Int32})] + class GenericService(T) + def initialize(@value : T); end + end + CR + end + + it "multiple services are based on a type, but not all provide a name" do + assert_error "Failed to register services for 'Klass'. Services based on this type must each explicitly provide a name.", <<-CR + class MissingService + end + + @[ADI::Register(_id: 1, name: "one")] + @[ADI::Register(_id: 2)] + class Klass + def initialize(@id : Int32); end + end + CR + end + + it "a named argument is an array more than 2 levels deep" do + assert_error "Failed to register service 'klass'. Arrays more than two levels deep are not currently supported.", <<-CR + @[ADI::Register(_values: [[[1]]])] + class Klass + def initialize(@values : Array(Array(Array(Int32)))); end + end + CR + end + + it "assert the service has been removed from the container" do + assert_error "undefined method 'service' for Athena::DependencyInjection::ServiceContainer", <<-CR + @[ADI::Register] + class Service + property name : String = "Jim" + end + + ADI::ServiceContainer.new.service + CR + end + + it "a name must be supplied if using a NamedTupleLiteral tag" do + assert_error "Failed to register service `tagged_service`. Tags must be an ArrayLiteral or TupleLiteral, not NumberLiteral.", <<-CR + @[ADI::Register(tags: 42)] + class TaggedService + end + CR + end + + it "tags can only be NamedTupleLiterals or StringLiterals" do + assert_error "Failed to register service `tagged_service`. A tag must be a StringLiteral or NamedTupleLiteral not BoolLiteral.", <<-CR + @[ADI::Register(tags: [true])] + class TaggedService + end + CR + end + + it "a name must be supplied if using a NamedTupleLiteral tag" do + assert_error "Failed to register service `tagged_service`. All tags must have a name.", <<-CR + @[ADI::Register(tags: [{priority: 42}])] + class TaggedService + end + CR end end end diff --git a/src/components/dependency_injection/spec/spec_helper.cr b/src/components/dependency_injection/spec/spec_helper.cr index 4294b69e5..6ed393a1a 100644 --- a/src/components/dependency_injection/spec/spec_helper.cr +++ b/src/components/dependency_injection/spec/spec_helper.cr @@ -5,8 +5,6 @@ require "./service_mocks" require "athena-spec" require "../src/spec" -include ASPEC::Methods - record DBConfig, username : String, password : String, host : String class ACF::Parameters From 3989e29c9234d4c1b37d2e12d05f7fcb301ee3b8 Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Sun, 1 May 2022 20:37:46 -0400 Subject: [PATCH 07/13] Fix lint issue --- src/components/routing/src/requirement/enum.cr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/routing/src/requirement/enum.cr b/src/components/routing/src/requirement/enum.cr index 697b1fe7b..ea1032ec8 100644 --- a/src/components/routing/src/requirement/enum.cr +++ b/src/components/routing/src/requirement/enum.cr @@ -10,8 +10,8 @@ struct Athena::Routing::Requirement::Enum(EnumType) end def to_s(io : IO) : Nil - (@members || EnumType.values).join io, '|' do |member, io| - io << Regex.escape member.to_s.underscore + (@members || EnumType.values).join io, '|' do |member, join_io| + join_io << Regex.escape member.to_s.underscore end end end From fa82f824a5627308519204ee1e05493587a234c3 Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Sun, 1 May 2022 23:41:17 -0400 Subject: [PATCH 08/13] Update serializer to use underscored string for enum serialization --- .../serializer/spec/visitors/json_serialization_visitor_spec.cr | 2 +- .../serializer/spec/visitors/yaml_serialization_visitor_spec.cr | 2 +- .../serializer/src/visitors/json_serialization_visitor.cr | 2 +- .../serializer/src/visitors/yaml_serialization_visitor.cr | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/serializer/spec/visitors/json_serialization_visitor_spec.cr b/src/components/serializer/spec/visitors/json_serialization_visitor_spec.cr index 346da98e8..49ff88d32 100644 --- a/src/components/serializer/spec/visitors/json_serialization_visitor_spec.cr +++ b/src/components/serializer/spec/visitors/json_serialization_visitor_spec.cr @@ -96,7 +96,7 @@ describe ASR::Visitors::JSONSerializationVisitor do end it Enum do - assert_serialized_output(ASR::Visitors::JSONSerializationVisitor, "2") do |visitor| + assert_serialized_output(ASR::Visitors::JSONSerializationVisitor, "two") do |visitor| visitor.visit TestEnum::Two end end diff --git a/src/components/serializer/spec/visitors/yaml_serialization_visitor_spec.cr b/src/components/serializer/spec/visitors/yaml_serialization_visitor_spec.cr index 07620cd4d..f183e5037 100644 --- a/src/components/serializer/spec/visitors/yaml_serialization_visitor_spec.cr +++ b/src/components/serializer/spec/visitors/yaml_serialization_visitor_spec.cr @@ -100,7 +100,7 @@ describe ASR::Visitors::YAMLSerializationVisitor do end it Enum do - assert_serialized_output(ASR::Visitors::YAMLSerializationVisitor, build_expected_yaml_string "--- 2\n") do |visitor| + assert_serialized_output(ASR::Visitors::YAMLSerializationVisitor, build_expected_yaml_string "--- two\n") do |visitor| visitor.visit TestEnum::Two end end diff --git a/src/components/serializer/src/visitors/json_serialization_visitor.cr b/src/components/serializer/src/visitors/json_serialization_visitor.cr index 163b40846..a1864028b 100644 --- a/src/components/serializer/src/visitors/json_serialization_visitor.cr +++ b/src/components/serializer/src/visitors/json_serialization_visitor.cr @@ -76,7 +76,7 @@ class Athena::Serializer::Visitors::JSONSerializationVisitor end def visit(data : Enum) : Nil - visit data.value + visit data.to_s.underscore end def visit(data : UUID) : Nil diff --git a/src/components/serializer/src/visitors/yaml_serialization_visitor.cr b/src/components/serializer/src/visitors/yaml_serialization_visitor.cr index 4c1d37c4f..36409bd31 100644 --- a/src/components/serializer/src/visitors/yaml_serialization_visitor.cr +++ b/src/components/serializer/src/visitors/yaml_serialization_visitor.cr @@ -59,7 +59,7 @@ class Athena::Serializer::Visitors::YAMLSerializationVisitor end def visit(data : Enum) : Nil - visit data.value + visit data.to_s.underscore end def visit(data : UUID) : Nil From 7cb8862c009701c121368f6c6fb96893f67f5f55 Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Sun, 1 May 2022 23:46:40 -0400 Subject: [PATCH 09/13] Fix serializer spec --- .../serializer/spec/visitors/json_serialization_visitor_spec.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/serializer/spec/visitors/json_serialization_visitor_spec.cr b/src/components/serializer/spec/visitors/json_serialization_visitor_spec.cr index 49ff88d32..dca587491 100644 --- a/src/components/serializer/spec/visitors/json_serialization_visitor_spec.cr +++ b/src/components/serializer/spec/visitors/json_serialization_visitor_spec.cr @@ -96,7 +96,7 @@ describe ASR::Visitors::JSONSerializationVisitor do end it Enum do - assert_serialized_output(ASR::Visitors::JSONSerializationVisitor, "two") do |visitor| + assert_serialized_output(ASR::Visitors::JSONSerializationVisitor, %("two")) do |visitor| visitor.visit TestEnum::Two end end From 89f4221fe1ed9ce33ee2c0f320f70cca4d859e94 Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Mon, 2 May 2022 16:36:07 -0400 Subject: [PATCH 10/13] Add docs and more test coverage Drop support for nilable Enum types --- .../resolvers/enum_value_resolver_spec.cr | 13 +--- .../routing/annotation_route_loader_spec.cr | 21 ++++++ .../resolvers/enum_value_resolver.cr | 66 ++++++++++++------- .../ext/routing/annotation_route_loader.cr | 8 ++- .../routing/src/requirement/enum.cr | 39 ++++++++++- 5 files changed, 110 insertions(+), 37 deletions(-) diff --git a/src/components/framework/spec/arguments/resolvers/enum_value_resolver_spec.cr b/src/components/framework/spec/arguments/resolvers/enum_value_resolver_spec.cr index 342e9ffec..fd289edf6 100644 --- a/src/components/framework/spec/arguments/resolvers/enum_value_resolver_spec.cr +++ b/src/components/framework/spec/arguments/resolvers/enum_value_resolver_spec.cr @@ -17,12 +17,12 @@ describe ATH::Arguments::Resolvers::Enum do ATH::Arguments::Resolvers::Enum.new.supports?(request, argument).should be_true end - it "that does exist in request attributes" do + it "that does exist in request attributes, but the enum member is nilable" do argument = ATH::Arguments::ArgumentMetadata(TestEnum?).new "enum" request = new_request request.attributes.set "enum", "1" - ATH::Arguments::Resolvers::Enum.new.supports?(request, argument).should be_true + ATH::Arguments::Resolvers::Enum.new.supports?(request, argument).should be_false end it "that does not exist in request attributes" do @@ -82,15 +82,6 @@ describe ATH::Arguments::Resolvers::Enum do ATH::Arguments::Resolvers::Enum.new.resolve(request, argument).should eq TestEnum::B end - it "with a nilable enum type" do - argument = ATH::Arguments::ArgumentMetadata(TestEnum?).new "enum" - - request = new_request - request.attributes.set "enum", "C" - - ATH::Arguments::Resolvers::Enum.new.resolve(request, argument).should eq TestEnum::C - end - it "with an unknown member value" do argument = ATH::Arguments::ArgumentMetadata(TestEnum).new "enum" diff --git a/src/components/framework/spec/ext/routing/annotation_route_loader_spec.cr b/src/components/framework/spec/ext/routing/annotation_route_loader_spec.cr index 340852975..8a94bcc41 100644 --- a/src/components/framework/spec/ext/routing/annotation_route_loader_spec.cr +++ b/src/components/framework/spec/ext/routing/annotation_route_loader_spec.cr @@ -126,6 +126,19 @@ class RouteDefaultHelpers < ATH::Controller def action : Nil; end end +enum StringificationColor + Red + Green + Blue +end + +class StringificationController < ATH::Controller + @[ARTA::Get("/color/{color}", requirements: {"color" => ART::Requirement::Enum(StringificationColor).new, "foo" => /foo/})] + def get_color(color : StringificationColor) : StringificationColor + color + end +end + describe ATH::Routing::AnnotationRouteLoader do describe ".route_collection" do it "simple route" do @@ -159,6 +172,14 @@ describe ATH::Routing::AnnotationRouteLoader do ) end + it "with a stringable route requirement" do + assert_route( + ATH::Routing::AnnotationRouteLoader.populate_collection(StringificationController), + path: "/color/{color}", + requirements: {"color" => "red|green|blue", "foo" => /foo/} + ) + end + describe "custom route methods" do it String do assert_route( diff --git a/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr b/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr index 874fdc70b..f1a4330de 100644 --- a/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr +++ b/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr @@ -1,9 +1,41 @@ @[ADI::Register(tags: [{name: ATH::Arguments::Resolvers::TAG, priority: 105}])] +# Handles resolving an [Enum](https://crystal-lang.org/api/Enum.html) member from a value that is stored in the request's `ATH::Request#attributes`. +# This resolver supports both numeric and string based parsing, returning a proper error response if the provided value does not map to any valid member. +# +# ``` +# require "athena" +# +# enum Color +# Red +# Blue +# Green +# end +# +# class ExampleController < ATH::Controller +# @[ARTA::Get("/numeric/{color}")] +# def get_color_numeric(color : Color) : Color +# color +# end +# +# @[ARTA::Get("/string/{color}")] +# def get_color_string(color : Color) : Color +# color +# end +# end +# +# ATH.run +# +# # GET /numeric/1 # => "blue" +# # GET /string/red # => "red" +# ``` +# +# TIP: Checkout `ART::Requirement::Enum` for an easy way to restrict routing to an enum's members, or a subset of them. +# WARNING: Only non-nilable Enum types are supported. struct Athena::Framework::Arguments::Resolvers::Enum include Athena::Framework::Arguments::Resolvers::ArgumentValueResolverInterface # :inherit: - def supports?(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata(::Enum?)) : Bool + def supports?(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata(::Enum)) : Bool request.attributes.has? argument.name end @@ -13,38 +45,24 @@ struct Athena::Framework::Arguments::Resolvers::Enum end # :inherit: - def resolve(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata(::Enum?)) + def resolve(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata(::Enum)) return unless (value = request.attributes.get(argument.name, String?)) - self.resolve value, argument.name, argument.type - end - - # :inherit: - def resolve(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata) - # Noop overload for non Enum types. - end - - private def resolve(value, name : String, _enum_type : Nil.class) - # Noop if the argument has a type of explicitly `Nil`. - end - - private def resolve(value, name : String, _enum_type : EnumType.class) forall EnumType - # Remove `Nil` from the union. - enum_type = typeof(begin - val = uninitialized EnumType - val.nil? ? raise("") : val - end) - - member = if (num = value.to_i128?(whitespace: false)) && (m = enum_type.from_value? num) + member = if (num = value.to_i128?(whitespace: false)) && (m = argument.type.from_value? num) m - elsif (m = enum_type.parse? value) + elsif (m = argument.type.parse? value) m end unless member - raise ATH::Exceptions::BadRequest.new "Parameter '#{name}' of enum type '#{enum_type}' has no valid member for '#{value}'." + raise ATH::Exceptions::BadRequest.new "Parameter '#{argument.name}' of enum type '#{argument.type}' has no valid member for '#{value}'." end member end + + # :inherit: + def resolve(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata) + # Noop overload for non Enum types. + end end diff --git a/src/components/framework/src/ext/routing/annotation_route_loader.cr b/src/components/framework/src/ext/routing/annotation_route_loader.cr index 84b476f79..4d6279d25 100644 --- a/src/components/framework/src/ext/routing/annotation_route_loader.cr +++ b/src/components/framework/src/ext/routing/annotation_route_loader.cr @@ -392,7 +392,13 @@ module Athena::Framework::Routing::AnnotationRouteLoader ann_requirements.raise "Route action '#{klass.name}##{m.name}' expects a 'HashLiteral(StringLiteral, StringLiteral | RegexLiteral)' for its '#{route_def.name}#requirements' field, but got a '#{ann_requirements.class_name.id}'." end - ann_requirements.each { |k, v| requirements[k] = v } + ann_requirements.each do |k, v| + requirements[k] = if v.is_a?(StringLiteral) || v.is_a?(RegexLiteral) + v + else + "#{v}.to_s".id + end + end end if (value = route_def[:host]) != nil diff --git a/src/components/routing/src/requirement/enum.cr b/src/components/routing/src/requirement/enum.cr index ea1032ec8..83e4ac585 100644 --- a/src/components/routing/src/requirement/enum.cr +++ b/src/components/routing/src/requirement/enum.cr @@ -1,3 +1,40 @@ +# Provides an easier way to define a [route requirement][Athena::Routing::Route--parameter-validation] for all, or a subset of, Enum members. +# +# For example: +# ``` +# require "athena" +# +# enum Color +# Red +# Blue +# Green +# Black +# end +# +# class ExampleController < ATH::Controller +# @[ARTA::Get("/color/{color}", requirements: {"color" => ART::Requirement::Enum(Color).new})] +# def get_color(color : Color) : Color +# color +# end +# +# @[ARTA::Get("/rgb-color/{color}", requirements: {"color" => ART::Requirement::Enum(Color).new(:red, :green, :blue)})] +# def get_rgb_color(color : Color) : Color +# color +# end +# end +# +# ATH.run +# +# # GET /color/red # => "red" +# # GET /color/pink # => 404 +# # +# # GET /rgb-color/red # => "red" +# # GET /rgb-color/green # => "green" +# # GET /rgb-color/blue # => "blue" +# # GET /rgb-color/black # => 404 +# ``` +# +# NOTE: This type _ONLY_ supports the string representation of enum members. struct Athena::Routing::Requirement::Enum(EnumType) getter members : Set(EnumType)? = nil @@ -10,7 +47,7 @@ struct Athena::Routing::Requirement::Enum(EnumType) end def to_s(io : IO) : Nil - (@members || EnumType.values).join io, '|' do |member, join_io| + (@members || EnumType.names).join io, '|' do |member, join_io| join_io << Regex.escape member.to_s.underscore end end From 49c0453686a9537c3e754fbbc9059d1d90be0344 Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Mon, 2 May 2022 19:22:56 -0400 Subject: [PATCH 11/13] Re-support nilable Enum types Leverages a bit more flexible implementation Fix route loader spec --- .../resolvers/enum_value_resolver_spec.cr | 17 +++++++++++++- .../routing/annotation_route_loader_spec.cr | 2 +- .../src/arguments/argument_metadata.cr | 16 ++++++++++++++ .../resolvers/enum_value_resolver.cr | 22 +++++-------------- .../routing/src/requirement/enum.cr | 16 ++++++++++++-- 5 files changed, 53 insertions(+), 20 deletions(-) diff --git a/src/components/framework/spec/arguments/resolvers/enum_value_resolver_spec.cr b/src/components/framework/spec/arguments/resolvers/enum_value_resolver_spec.cr index fd289edf6..a57b5e04a 100644 --- a/src/components/framework/spec/arguments/resolvers/enum_value_resolver_spec.cr +++ b/src/components/framework/spec/arguments/resolvers/enum_value_resolver_spec.cr @@ -22,7 +22,7 @@ describe ATH::Arguments::Resolvers::Enum do request = new_request request.attributes.set "enum", "1" - ATH::Arguments::Resolvers::Enum.new.supports?(request, argument).should be_false + ATH::Arguments::Resolvers::Enum.new.supports?(request, argument).should be_true end it "that does not exist in request attributes" do @@ -31,6 +31,12 @@ describe ATH::Arguments::Resolvers::Enum do ATH::Arguments::Resolvers::Enum.new.supports?(new_request, argument).should be_false end + it "that is nilable and not exist in request attributes" do + argument = ATH::Arguments::ArgumentMetadata(TestEnum?).new "enum" + + ATH::Arguments::Resolvers::Enum.new.supports?(new_request, argument).should be_false + end + it "that is a union of another type" do argument = ATH::Arguments::ArgumentMetadata(TestEnum | String).new "enum" @@ -82,6 +88,15 @@ describe ATH::Arguments::Resolvers::Enum do ATH::Arguments::Resolvers::Enum.new.resolve(request, argument).should eq TestEnum::B end + it "with a string based nilable value" do + argument = ATH::Arguments::ArgumentMetadata(TestEnum?).new "enum" + + request = new_request + request.attributes.set "enum", "B" + + ATH::Arguments::Resolvers::Enum.new.resolve(request, argument).should eq TestEnum::B + end + it "with an unknown member value" do argument = ATH::Arguments::ArgumentMetadata(TestEnum).new "enum" diff --git a/src/components/framework/spec/ext/routing/annotation_route_loader_spec.cr b/src/components/framework/spec/ext/routing/annotation_route_loader_spec.cr index 8a94bcc41..c4b58a4f9 100644 --- a/src/components/framework/spec/ext/routing/annotation_route_loader_spec.cr +++ b/src/components/framework/spec/ext/routing/annotation_route_loader_spec.cr @@ -176,7 +176,7 @@ describe ATH::Routing::AnnotationRouteLoader do assert_route( ATH::Routing::AnnotationRouteLoader.populate_collection(StringificationController), path: "/color/{color}", - requirements: {"color" => "red|green|blue", "foo" => /foo/} + requirements: {"color" => /red|green|blue/, "foo" => /foo/} ) end diff --git a/src/components/framework/src/arguments/argument_metadata.cr b/src/components/framework/src/arguments/argument_metadata.cr index 6d558231c..4e6cecc79 100644 --- a/src/components/framework/src/arguments/argument_metadata.cr +++ b/src/components/framework/src/arguments/argument_metadata.cr @@ -14,4 +14,20 @@ struct Athena::Framework::Arguments::ArgumentMetadata(T) def type : T.class T end + + def type_of?(klass : Type.class) : Bool forall Type + {{ T.union? ? T.union_types.any? { |t| t <= Type } : T <= Type }} + end + + def first_type_of(klass : Type.class) forall Type + {% if T.union? %} + {% for t in T.union_types %} + {% if t <= Type %} + return {{t}} + {% end %} + {% end %} + {% elsif T <= Type %} + {{T}} + {% end %} + end end diff --git a/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr b/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr index f1a4330de..b88d6f809 100644 --- a/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr +++ b/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr @@ -30,39 +30,29 @@ # ``` # # TIP: Checkout `ART::Requirement::Enum` for an easy way to restrict routing to an enum's members, or a subset of them. -# WARNING: Only non-nilable Enum types are supported. struct Athena::Framework::Arguments::Resolvers::Enum include Athena::Framework::Arguments::Resolvers::ArgumentValueResolverInterface - # :inherit: - def supports?(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata(::Enum)) : Bool - request.attributes.has? argument.name - end - # :inherit: def supports?(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata) : Bool - false + argument.type_of?(::Enum) && request.attributes.has? argument.name end # :inherit: - def resolve(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata(::Enum)) + def resolve(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata) return unless (value = request.attributes.get(argument.name, String?)) + return unless (enum_type = argument.first_type_of ::Enum) - member = if (num = value.to_i128?(whitespace: false)) && (m = argument.type.from_value? num) + member = if (num = value.to_i128?(whitespace: false)) && (m = enum_type.from_value? num) m - elsif (m = argument.type.parse? value) + elsif (m = enum_type.parse? value) m end unless member - raise ATH::Exceptions::BadRequest.new "Parameter '#{argument.name}' of enum type '#{argument.type}' has no valid member for '#{value}'." + raise ATH::Exceptions::BadRequest.new "Parameter '#{argument.name}' of enum type '#{enum_type}' has no valid member for '#{value}'." end member end - - # :inherit: - def resolve(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata) - # Noop overload for non Enum types. - end end diff --git a/src/components/routing/src/requirement/enum.cr b/src/components/routing/src/requirement/enum.cr index 83e4ac585..9e598b3a7 100644 --- a/src/components/routing/src/requirement/enum.cr +++ b/src/components/routing/src/requirement/enum.cr @@ -12,12 +12,22 @@ # end # # class ExampleController < ATH::Controller -# @[ARTA::Get("/color/{color}", requirements: {"color" => ART::Requirement::Enum(Color).new})] +# @[ARTA::Get( +# "/color/{color}", +# requirements: { +# "color" => ART::Requirement::Enum(Color).new, +# } +# )] # def get_color(color : Color) : Color # color # end # -# @[ARTA::Get("/rgb-color/{color}", requirements: {"color" => ART::Requirement::Enum(Color).new(:red, :green, :blue)})] +# @[ARTA::Get( +# "/rgb-color/{color}", +# requirements: { +# "color" => ART::Requirement::Enum(Color).new(:red, :green, :blue), +# } +# )] # def get_rgb_color(color : Color) : Color # color # end @@ -36,6 +46,7 @@ # # NOTE: This type _ONLY_ supports the string representation of enum members. struct Athena::Routing::Requirement::Enum(EnumType) + # Returns the set of allowed enum members, or `nil` if all members are allowed. getter members : Set(EnumType)? = nil def self.new(*cases : EnumType) @@ -46,6 +57,7 @@ struct Athena::Routing::Requirement::Enum(EnumType) {% raise "'#{EnumType}' is not an Enum type." unless EnumType <= ::Enum %} end + # :nodoc: def to_s(io : IO) : Nil (@members || EnumType.names).join io, '|' do |member, join_io| join_io << Regex.escape member.to_s.underscore From 56a0fdc19e1265c2eb430fc27f31e471ef21bade Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Mon, 2 May 2022 20:07:15 -0400 Subject: [PATCH 12/13] Doc and test coverage on new metadata types --- .../spec/arguments/argument_metadata_spec.cr | 64 +++++++++++++++---- .../routing/annotation_route_loader_spec.cr | 4 +- .../src/arguments/argument_metadata.cr | 18 +++++- .../resolvers/enum_value_resolver.cr | 2 +- 4 files changed, 72 insertions(+), 16 deletions(-) diff --git a/src/components/framework/spec/arguments/argument_metadata_spec.cr b/src/components/framework/spec/arguments/argument_metadata_spec.cr index 78573220f..5dbaa1848 100644 --- a/src/components/framework/spec/arguments/argument_metadata_spec.cr +++ b/src/components/framework/spec/arguments/argument_metadata_spec.cr @@ -1,18 +1,58 @@ require "../spec_helper" describe ATH::Arguments::ArgumentMetadata do - describe "#initialize" do - describe "#nilable?" do - it "when is_nilable is true" do - ATH::Arguments::ArgumentMetadata(Int32).new("id").nilable?.should be_false - ATH::Arguments::ArgumentMetadata(String | Bool).new("id").nilable?.should be_false - end - - it "type is Nil" do - ATH::Arguments::ArgumentMetadata(Nil).new("id").nilable?.should be_true - ATH::Arguments::ArgumentMetadata(Int32?).new("id").nilable?.should be_true - ATH::Arguments::ArgumentMetadata(String | Bool | Nil).new("id").nilable?.should be_true - end + describe "#nilable?" do + it "type is not nilable" do + ATH::Arguments::ArgumentMetadata(Int32).new("foo").nilable?.should be_false + ATH::Arguments::ArgumentMetadata(String | Bool).new("foo").nilable?.should be_false + end + + it "type is nilable" do + ATH::Arguments::ArgumentMetadata(Nil).new("foo").nilable?.should be_true + ATH::Arguments::ArgumentMetadata(Int32?).new("foo").nilable?.should be_true + ATH::Arguments::ArgumentMetadata(String | Bool | Nil).new("foo").nilable?.should be_true + end + end + + describe "#instance_of?" do + it "with a scalar type" do + ATH::Arguments::ArgumentMetadata(Int32).new("foo").instance_of?(Int32).should be_true + ATH::Arguments::ArgumentMetadata(Int32).new("foo").instance_of?(Number).should be_true + ATH::Arguments::ArgumentMetadata(Int32).new("foo").instance_of?(String).should be_false + end + + it "with a union" do + ATH::Arguments::ArgumentMetadata(String | Bool).new("foo").instance_of?(String).should be_true + ATH::Arguments::ArgumentMetadata(Array(Bool) | Array(String)).new("foo").instance_of?(Array(String)).should be_true + end + + it "nilable" do + ATH::Arguments::ArgumentMetadata(String | Bool | Nil).new("foo").instance_of?(Bool).should be_true + ATH::Arguments::ArgumentMetadata(String | Bool | Nil).new("foo").instance_of?(Int32).should be_false + ATH::Arguments::ArgumentMetadata(Array(Bool) | Array(String) | Nil).new("foo").instance_of?(Array(String)).should be_true + ATH::Arguments::ArgumentMetadata(Array(Bool) | Array(String) | Nil).new("foo").instance_of?(Array(Float64)).should be_false + end + end + + describe "#first_type_of" do + it "with a single type var" do + ATH::Arguments::ArgumentMetadata(Int32).new("foo").first_type_of(Int32).should eq Int32 + ATH::Arguments::ArgumentMetadata(Array(Int32)).new("foo").first_type_of(Array).should eq Array(Int32) + end + + it "with a union" do + ATH::Arguments::ArgumentMetadata(String | Int32 | Bool).new("foo").first_type_of(Int32).should eq Int32 + ATH::Arguments::ArgumentMetadata(Array(Int32) | Array(String)).new("foo").first_type_of(Array).should eq Array(Int32) + end + + it "with a union of multiple valid type vars" do + # Is Float64 because the union gets alphabetized + ATH::Arguments::ArgumentMetadata(String | Int8 | Float64 | Int64).new("foo").first_type_of(Number).should eq Float64 + end + + it "with no matching type var" do + ATH::Arguments::ArgumentMetadata(String | Int32 | Bool).new("foo").first_type_of(Array).should be_nil + ATH::Arguments::ArgumentMetadata(String | Int32 | Bool).new("foo").first_type_of(Float64).should be_nil end end end diff --git a/src/components/framework/spec/ext/routing/annotation_route_loader_spec.cr b/src/components/framework/spec/ext/routing/annotation_route_loader_spec.cr index c4b58a4f9..a1d74c7c1 100644 --- a/src/components/framework/spec/ext/routing/annotation_route_loader_spec.cr +++ b/src/components/framework/spec/ext/routing/annotation_route_loader_spec.cr @@ -133,7 +133,7 @@ enum StringificationColor end class StringificationController < ATH::Controller - @[ARTA::Get("/color/{color}", requirements: {"color" => ART::Requirement::Enum(StringificationColor).new, "foo" => /foo/})] + @[ARTA::Get("/color/{color}", requirements: {"color" => ART::Requirement::Enum(StringificationColor).new, "foo" => /foo/, "bar" => "bar"})] def get_color(color : StringificationColor) : StringificationColor color end @@ -176,7 +176,7 @@ describe ATH::Routing::AnnotationRouteLoader do assert_route( ATH::Routing::AnnotationRouteLoader.populate_collection(StringificationController), path: "/color/{color}", - requirements: {"color" => /red|green|blue/, "foo" => /foo/} + requirements: {"color" => /red|green|blue/, "foo" => /foo/, "bar" => /bar/} ) end diff --git a/src/components/framework/src/arguments/argument_metadata.cr b/src/components/framework/src/arguments/argument_metadata.cr index 4e6cecc79..1a10f49a8 100644 --- a/src/components/framework/src/arguments/argument_metadata.cr +++ b/src/components/framework/src/arguments/argument_metadata.cr @@ -15,10 +15,26 @@ struct Athena::Framework::Arguments::ArgumentMetadata(T) T end - def type_of?(klass : Type.class) : Bool forall Type + # Returns `true` if this argument's `#type` includes the provided *klass*. + # + # ``` + # ATH::Arguments::ArgumentMetadata(Int32).new("foo").instance_of?(Int32) # => true + # ATH::Arguments::ArgumentMetadata(Int32 | Bool).new("foo").instance_of?(Bool) # => true + # ATH::Arguments::ArgumentMetadata(Int32).new("foo").instance_of?(String) # => false + # ``` + def instance_of?(klass : Type.class) : Bool forall Type {{ T.union? ? T.union_types.any? { |t| t <= Type } : T <= Type }} end + # Returns the metaclass of the first matching type variable that is an `#instance_of?` the provided *klass*, or `nil` if none match. + # If this the `#type` is union, this would be the first viable type. + # + # ``` + # ATH::Arguments::ArgumentMetadata(Int32).new("foo").first_type_of(Int32) # => Int32.class + # ATH::Arguments::ArgumentMetadata(String | Int32 | Bool).new("foo").first_type_of(Int32) # => Int32.class + # ATH::Arguments::ArgumentMetadata(String | Int8 | Float64 | Int64).new("foo").first_type_of(Number) # => Float64.class + # ATH::Arguments::ArgumentMetadata(String | Int32 | Bool).new("foo").first_type_of(Float64) # => nil + # ``` def first_type_of(klass : Type.class) forall Type {% if T.union? %} {% for t in T.union_types %} diff --git a/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr b/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr index b88d6f809..8c41229a5 100644 --- a/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr +++ b/src/components/framework/src/arguments/resolvers/enum_value_resolver.cr @@ -35,7 +35,7 @@ struct Athena::Framework::Arguments::Resolvers::Enum # :inherit: def supports?(request : ATH::Request, argument : ATH::Arguments::ArgumentMetadata) : Bool - argument.type_of?(::Enum) && request.attributes.has? argument.name + argument.instance_of?(::Enum) && request.attributes.has? argument.name end # :inherit: From 90847f5f96ba05053d39762a02d39c0b965e87cb Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Mon, 2 May 2022 20:19:50 -0400 Subject: [PATCH 13/13] Document `ART::Requirement` namespace --- .../routing/src/requirement/requirement.cr | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/components/routing/src/requirement/requirement.cr b/src/components/routing/src/requirement/requirement.cr index 751719d8f..43f448b49 100644 --- a/src/components/routing/src/requirement/requirement.cr +++ b/src/components/routing/src/requirement/requirement.cr @@ -1,3 +1,30 @@ +# Includes types related to [route requirements][Athena::Routing::Route--parameter-validation]. +# +# The namespace also exposes various regex constants representing common universal requirements to make using them in routes easier. +# +# ``` +# class ExampleController < ATH::Controller +# @[ARTA::Get( +# "/user/{id}", +# requirements: { +# "id" => ART::Requirement::DIGITS, +# } +# )] +# def get_user(id : Int64) : Int64 +# id +# end +# +# @[ARTA::Get( +# "/article/{slug}", +# requirements: { +# "slug" => ART::Requirement::ASCII_SLUG, +# } +# )] +# def get_article(slug : String) : String +# slug +# end +# end +# ``` module Athena::Routing::Requirement # Sourced from https://github.com/symfony/symfony/blob/c70be0957a11fd8b7aa687d6173e76724068daa4/src/Symfony/Component/Routing/Requirement/Requirement.php