From edad876ca6cf8e3c3e7a16767d37de8f7205ac05 Mon Sep 17 00:00:00 2001 From: Davide Paolo Tua Date: Tue, 25 Oct 2022 10:06:37 +0200 Subject: [PATCH 1/2] Introduce error message when parsing invalid values. Implements #893. There is a breaking change, though. Previously, calling FailedCast#value would always return a nil value, without providing any clue on which value was actually breaking. Now, it raises a FailCastError with a talking error message, in the form of - for instance - "Value ->Invalid<- is not a valid constant for enum WhateverEnum". --- spec/avram/queryable_spec.cr | 4 ++-- spec/avram/type_extensions/bool_spec.cr | 17 +++++++++++++++++ spec/avram/type_extensions/enum_spec.cr | 15 +++++++++++++++ spec/avram/type_extensions/int32_spec.cr | 2 +- src/avram/charms/bool_extensions.cr | 8 +++++--- src/avram/charms/enum_extensions.cr | 4 ++-- src/avram/charms/float64_extensions.cr | 2 +- src/avram/charms/int16_extensions.cr | 4 ++-- src/avram/charms/int32_extensions.cr | 4 ++-- src/avram/charms/int64_extensions.cr | 2 +- src/avram/charms/time_extensions.cr | 2 +- src/avram/charms/uuid_extensions.cr | 2 +- src/avram/errors.cr | 4 ++++ src/avram/type.cr | 20 +++++++++++++++----- src/avram/uploadable.cr | 4 ++-- 15 files changed, 71 insertions(+), 23 deletions(-) diff --git a/spec/avram/queryable_spec.cr b/spec/avram/queryable_spec.cr index b6137b7ad..803808ef8 100644 --- a/spec/avram/queryable_spec.cr +++ b/spec/avram/queryable_spec.cr @@ -374,7 +374,7 @@ describe Avram::Queryable do end it "raises PQ::PQError if no record is found with letter-only id (String)" do - expect_raises(Exception, "FailedCast") do + expect_raises(Avram::FailedCastError) do UserQuery.find("id") end end @@ -401,7 +401,7 @@ describe Avram::Queryable do end it "raises PQ::PQError if no record is found with letter-only id (String)" do - expect_raises(Exception, "FailedCast") do + expect_raises(Avram::FailedCastError) do UserQuery.new.find("id") end end diff --git a/spec/avram/type_extensions/bool_spec.cr b/spec/avram/type_extensions/bool_spec.cr index 20f99945d..d5e498ec0 100644 --- a/spec/avram/type_extensions/bool_spec.cr +++ b/spec/avram/type_extensions/bool_spec.cr @@ -6,4 +6,21 @@ describe Bool do true.blank?.should be_false end end + + describe "parsing value" do + it "works with correct strings" do + correct_values = %w(true 1 false 0) + expected = [true, true, false, false] + correct_values.each_with_index do |val, ix| + Bool.adapter.parse(val).value.should eq expected[ix] + end + end + + it "fails with malformed strings" do + wrong_value = "fAl;se" + expect_raises(Avram::FailedCastError) { + Bool.adapter.parse(wrong_value).value + } + end + end end diff --git a/spec/avram/type_extensions/enum_spec.cr b/spec/avram/type_extensions/enum_spec.cr index 5ff520008..02da100c2 100644 --- a/spec/avram/type_extensions/enum_spec.cr +++ b/spec/avram/type_extensions/enum_spec.cr @@ -32,6 +32,21 @@ describe "models using enums" do query.status(0).first.should eq(issue) end + it "fails when querying for non defined enum constants" do + issue = IssueFactory.create + query = IssueQuery.new + error_message = ->(x : String | Int32) { "Value ->#{x}<- is not a valid constant for enum Issue::Status" } + + wrong_enum_constant = "oh no!" + expect_raises(Avram::FailedCastError, error_message.call(wrong_enum_constant)) { + query.status(wrong_enum_constant) + } + wrong_enum_constant = 999 + expect_raises(Avram::FailedCastError, error_message.call(wrong_enum_constant)) { + query.status(wrong_enum_constant) + } + end + it "handles other queries" do IssueFactory.create &.role(Issue::Role::Issue) IssueFactory.create &.role(Issue::Role::Critical) diff --git a/spec/avram/type_extensions/int32_spec.cr b/spec/avram/type_extensions/int32_spec.cr index e2e0c87a4..73cdb8280 100644 --- a/spec/avram/type_extensions/int32_spec.cr +++ b/spec/avram/type_extensions/int32_spec.cr @@ -13,7 +13,7 @@ describe "Int32" do it "returns FailedCast when overflow from Int64 to Int32" do result = Int32.adapter.parse(2147483648) - result.value.should eq(nil) result.should be_a(Avram::Type::FailedCast) + expect_raises(Avram::FailedCastError) { result.value } end end diff --git a/src/avram/charms/bool_extensions.cr b/src/avram/charms/bool_extensions.cr index 3e238bde7..282fbd4cf 100644 --- a/src/avram/charms/bool_extensions.cr +++ b/src/avram/charms/bool_extensions.cr @@ -12,12 +12,14 @@ struct Bool end def parse(value : String) - if %w(true 1).includes? value + true_vals = %w(true 1) + false_vals = %w(false 0) + if true_vals.includes? value SuccessfulCast(Bool).new true - elsif %w(false 0).includes? value + elsif false_vals.includes? value SuccessfulCast(Bool).new false else - FailedCast.new + FailedCast.new("Value ->#{value}<- could not be converted to boolean - allowed values: #{true_vals + false_vals}") end end diff --git a/src/avram/charms/enum_extensions.cr b/src/avram/charms/enum_extensions.cr index cfad7b3ef..dcc7b3ff4 100644 --- a/src/avram/charms/enum_extensions.cr +++ b/src/avram/charms/enum_extensions.cr @@ -16,7 +16,7 @@ abstract struct Enum if result = T.parse?(value) SuccessfulCast.new(result) else - FailedCast.new + FailedCast.new("Value ->#{value}<- is not a valid constant for enum #{T}") end end @@ -24,7 +24,7 @@ abstract struct Enum if result = T.from_value?(value) SuccessfulCast.new(result) else - FailedCast.new + FailedCast.new("Value ->#{value}<- is not a valid constant for enum #{T}") end end diff --git a/src/avram/charms/float64_extensions.cr b/src/avram/charms/float64_extensions.cr index 012d5be58..e17e2e174 100644 --- a/src/avram/charms/float64_extensions.cr +++ b/src/avram/charms/float64_extensions.cr @@ -38,7 +38,7 @@ struct Float64 def parse(value : String) SuccessfulCast(Float64).new value.to_f64 rescue ArgumentError - FailedCast.new + FailedCast.new("Value ->#{value}<- could not be converted to Float64") end def parse(value : Int32) diff --git a/src/avram/charms/int16_extensions.cr b/src/avram/charms/int16_extensions.cr index 31aff404a..e42742263 100644 --- a/src/avram/charms/int16_extensions.cr +++ b/src/avram/charms/int16_extensions.cr @@ -26,13 +26,13 @@ struct Int16 def parse(value : String) SuccessfulCast(Int16).new value.to_i16 rescue ArgumentError - FailedCast.new + FailedCast.new("Value ->#{value}<- could not be converted to Int16") end def parse(value : Int32) SuccessfulCast(Int16).new value.to_i16 rescue OverflowError - FailedCast.new + FailedCast.new("Value ->#{value}<- overflowed while being converted to Int16") end def to_db(value : Int16) diff --git a/src/avram/charms/int32_extensions.cr b/src/avram/charms/int32_extensions.cr index 0a8b801d2..247864584 100644 --- a/src/avram/charms/int32_extensions.cr +++ b/src/avram/charms/int32_extensions.cr @@ -18,7 +18,7 @@ struct Int32 def parse(value : String) SuccessfulCast(Int32).new value.to_i rescue ArgumentError - FailedCast.new + FailedCast.new("Value ->#{value}<- could not be converted to Int32") end def parse(value : Int32) @@ -28,7 +28,7 @@ struct Int32 def parse(value : Int64) SuccessfulCast(Int32).new value.to_i32 rescue OverflowError - FailedCast.new + FailedCast.new("Value ->#{value}<- overflowed while being converted to Int32") end def parse(values : Array(Int32)) diff --git a/src/avram/charms/int64_extensions.cr b/src/avram/charms/int64_extensions.cr index 9eee61c98..270e4cef1 100644 --- a/src/avram/charms/int64_extensions.cr +++ b/src/avram/charms/int64_extensions.cr @@ -26,7 +26,7 @@ struct Int64 def parse(value : String) SuccessfulCast(Int64).new value.to_i64 rescue ArgumentError - FailedCast.new + FailedCast.new("Value ->#{value}<- could not be cast to Int64") end def parse(value : Int32) diff --git a/src/avram/charms/time_extensions.cr b/src/avram/charms/time_extensions.cr index 1bfc508cd..cd7d90afa 100644 --- a/src/avram/charms/time_extensions.cr +++ b/src/avram/charms/time_extensions.cr @@ -36,7 +36,7 @@ struct Time # Then try default formats try_parsing_with_default_formatters(value) || # Fail if none of them work - FailedCast.new + FailedCast.new("->#{value}<- cannot be parsed with current formatters to Time") end def self.try_parsing_with_default_formatters(value : String) diff --git a/src/avram/charms/uuid_extensions.cr b/src/avram/charms/uuid_extensions.cr index cdd9c8795..25946acd9 100644 --- a/src/avram/charms/uuid_extensions.cr +++ b/src/avram/charms/uuid_extensions.cr @@ -22,7 +22,7 @@ struct UUID def parse(value : String) SuccessfulCast(UUID).new(UUID.new(value)) rescue - FailedCast.new + FailedCast.new("->#{value}<- is not a valid UUID") end def to_db(value : UUID) diff --git a/src/avram/errors.cr b/src/avram/errors.cr index c9462b943..f80b40ec6 100644 --- a/src/avram/errors.cr +++ b/src/avram/errors.cr @@ -153,6 +153,10 @@ module Avram class InvalidQueryError < AvramError end + # Used when there is a failed cast. E.g: overflowing, failed time parsing, etc... + class FailedCastError < AvramError + end + # Used when `Avram::Operation` fails. class FailedOperation < AvramError end diff --git a/src/avram/type.cr b/src/avram/type.cr index 7249b72cb..a7f46e0c6 100644 --- a/src/avram/type.cr +++ b/src/avram/type.cr @@ -17,12 +17,15 @@ module Avram::Type values = casts.map { |c| c.as(SuccessfulCast).value } parse(values) else - FailedCast.new + failure_reasons = casts.select(FailedCast) + .map(&.reason) + .join("\n") + FailedCast.new(failure_reasons) end end def parse!(value) - parse(value).as(SuccessfulCast).value + parse(value).value end def to_db(value : Nil) @@ -35,15 +38,22 @@ module Avram::Type end class SuccessfulCast(T) - getter :value - def initialize(@value : T) end + + def value : T + @value + end end class FailedCast + getter reason + + def initialize(@reason : String?) + end + def value - nil + raise FailedCastError.new(reason || "Failed to cast value") end end end diff --git a/src/avram/uploadable.cr b/src/avram/uploadable.cr index 197526a93..5cd730362 100644 --- a/src/avram/uploadable.cr +++ b/src/avram/uploadable.cr @@ -22,11 +22,11 @@ module Avram::Uploadable end def parse(value : String?) - FailedCast.new + FailedCast.new("Cannot parse an uploadeable") end def parse(values : Array(String)) - FailedCast.new + FailedCast.new("Cannot parse an array of uploadeable") end end end From ff44b5c40f7ac1a2db9b289e73c363d0a4f155fc Mon Sep 17 00:00:00 2001 From: Davide Paolo Tua Date: Tue, 25 Oct 2022 10:11:02 +0200 Subject: [PATCH 2/2] Stop ameba from complaining --- spec/avram/type_extensions/enum_spec.cr | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/avram/type_extensions/enum_spec.cr b/spec/avram/type_extensions/enum_spec.cr index 02da100c2..926d05b8f 100644 --- a/spec/avram/type_extensions/enum_spec.cr +++ b/spec/avram/type_extensions/enum_spec.cr @@ -33,7 +33,6 @@ describe "models using enums" do end it "fails when querying for non defined enum constants" do - issue = IssueFactory.create query = IssueQuery.new error_message = ->(x : String | Int32) { "Value ->#{x}<- is not a valid constant for enum Issue::Status" }