Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Bang Validations #10

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .ruby-version
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
3.0.2
50 changes: 23 additions & 27 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6,43 +6,39 @@ PATH
GEM
remote: https://rubygems.org/
specs:
codecov (0.1.4)
codecov (0.2.12)
json
simplecov
url
coderay (1.1.0)
diff-lcs (1.2.5)
coderay (1.1.3)
diff-lcs (1.4.4)
docile (1.1.5)
json (1.8.3)
method_source (0.8.2)
pry (0.10.3)
coderay (~> 1.1.0)
method_source (~> 0.8.1)
slop (~> 3.4)
rake (10.4.2)
rspec (3.3.0)
rspec-core (~> 3.3.0)
rspec-expectations (~> 3.3.0)
rspec-mocks (~> 3.3.0)
rspec-core (3.3.2)
rspec-support (~> 3.3.0)
rspec-expectations (3.3.1)
json (1.8.6)
method_source (1.0.0)
pry (0.14.1)
coderay (~> 1.1)
method_source (~> 1.0)
rake (10.5.0)
rspec (3.10.0)
rspec-core (~> 3.10.0)
rspec-expectations (~> 3.10.0)
rspec-mocks (~> 3.10.0)
rspec-core (3.10.1)
rspec-support (~> 3.10.0)
rspec-expectations (3.10.1)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.3.0)
rspec-mocks (3.3.2)
rspec-support (~> 3.10.0)
rspec-mocks (3.10.2)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.3.0)
rspec-support (3.3.0)
rspec-support (~> 3.10.0)
rspec-support (3.10.3)
simplecov (0.10.0)
docile (~> 1.1.0)
json (~> 1.8)
simplecov-html (~> 0.10.0)
simplecov-html (0.10.0)
slop (3.6.0)
url (0.3.2)
simplecov-html (0.10.2)

PLATFORMS
ruby
x86_64-darwin-19

DEPENDENCIES
action_logic!
Expand All @@ -53,4 +49,4 @@ DEPENDENCIES
simplecov (~> 0.10.0)

BUNDLED WITH
1.10.6
2.2.33
20 changes: 14 additions & 6 deletions lib/action_logic/action_validation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,36 @@
module ActionLogic
module ActionValidation
module ClassMethods
def validates_before!(args)
@validates_before = args.merge(raise_action_logic_exception: true)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will eventually eliminate this dupe, but for now I just wanted to see what it looked like to use the validations hash to signal to the validation classes whether to raise an exception or not.

end

def validates_before(args)
@validates_before = args
@validates_before = args.merge(raise_action_logic_exception: false)
end

def validates_after(args)
@validates_after = args
@validates_after = args.merge(raise_action_logic_exception: true)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how I feel about using an extra attribute to signal the validation classes. This is just a rough draft. If this is the approach to take, it needs to be a very specific name to avoid potential collisions, hence "raise_action_logic_exception".

end

def validates_around!(args)
@validates_around = args.merge(raise_action_logic_exception: true)
end

def validates_around(args)
@validates_around = args
@validates_around = args.merge(raise_action_logic_exception: false)
end

def get_validates_before
@validates_before ||= {}
@validates_before ||= { raise_action_logic_exception: true }
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: fix dupe.

end

def get_validates_after
@validates_after ||= {}
@validates_after ||= { raise_action_logic_exception: true }
end

def get_validates_around
@validates_around ||= {}
@validates_around ||= { raise_action_logic_exception: true }
end
end

Expand Down
8 changes: 6 additions & 2 deletions lib/action_logic/action_validation/attribute_validation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,15 @@ module ActionValidation
class AttributeValidation < BaseValidation

def self.validate!(validation_rules, context)
tmp_rules = validation_rules.clone
raise_exception = tmp_rules.delete(:raise_action_logic_exception)
existing_attributes = context.to_h.keys
expected_attributes = validation_rules.keys || []
expected_attributes = tmp_rules.keys || []
missing_attributes = expected_attributes - existing_attributes

raise ActionLogic::MissingAttributeError.new(error_message_format(missing_attributes.join(", ") + " attributes are missing")) if missing_attributes.any?
if raise_exception
raise ActionLogic::MissingAttributeError.new(error_message_format(missing_attributes.join(", ") + " attributes are missing")) if missing_attributes.any?
end
end
end
end
Expand Down
14 changes: 9 additions & 5 deletions lib/action_logic/action_validation/presence_validation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,17 @@ module ActionValidation
class PresenceValidation < BaseValidation

def self.validate!(validation_rules, context)
return unless validation_rules.values.find { |expected_validation| expected_validation[:presence] }
errors = presence_errors(validation_rules, context)
raise ActionLogic::PresenceError.new(errors) if errors.any?
tmp_rules = validation_rules.clone
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have to clone a copy of validation_rules here instead of deleting the :raise_action_logic_exception directly, since for 'around' validations, validation_rules will be accessed twice, once before and once after.

raise_exception = tmp_rules.delete(:raise_action_logic_exception)
return unless tmp_rules.values.find { |expected_validation| expected_validation[:presence] }
errors = presence_errors(tmp_rules, context)
if raise_exception
raise ActionLogic::PresenceError.new(errors) if errors.any?
end
end

def self.presence_errors(validation_rules, context)
validation_rules.reduce([]) do |error_collection, (expected_attribute, expected_validation)|
def self.presence_errors(tmp_rules, context)
tmp_rules.reduce([]) do |error_collection, (expected_attribute, expected_validation)|
next unless expected_validation[:presence]
error_collection << error_message(expected_attribute, expected_validation, context)
error_collection
Expand Down
10 changes: 7 additions & 3 deletions lib/action_logic/action_validation/type_validation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ module ActionValidation
class TypeValidation < BaseValidation

def self.validate!(validation_rules, context)
return unless validation_rules.values.find { |expected_validation| expected_validation[:type] }
tmp_rules = validation_rules.clone
raise_exception = tmp_rules.delete(:raise_action_logic_exception)
return unless tmp_rules.values.find { |expected_validation| expected_validation[:type] }

type_errors = validation_rules.reduce([]) do |collection, (expected_attribute, expected_validation)|
type_errors = tmp_rules.reduce([]) do |collection, (expected_attribute, expected_validation)|
next unless expected_validation[:type]

if context.to_h[expected_attribute].class != expected_validation[:type]
Expand All @@ -17,7 +19,9 @@ def self.validate!(validation_rules, context)
collection
end

raise ActionLogic::AttributeTypeError.new(error_message_format(type_errors.join(", "))) if type_errors.any?
if raise_exception
raise ActionLogic::AttributeTypeError.new(error_message_format(type_errors.join(", "))) if type_errors.any?
end
end
end
end
Expand Down
70 changes: 63 additions & 7 deletions spec/action_logic/action_task_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,35 @@ module ActionLogic

describe "around validations" do
describe "required attributes and type validation" do
it "does not have errors if context has required keys and values are of the correct type" do
result = ValidateAroundTestTask.execute(Constants::VALID_ATTRIBUTES)
expect(result.message).to be_nil
expect(result.success?).to be_truthy
end

it "does not raise error if context is missing required keys" do
expect { ValidateAroundTestTask.execute() }.to_not\
raise_error(ActionLogic::MissingAttributeError)
end

it "raises error if context has required keys but values are not of correct type" do
expect { ValidateAroundTestTask.execute(Constants::INVALID_ATTRIBUTES) }.to_not\
raise_error(ActionLogic::AttributeTypeError)
end
end

describe "required attributes and type validation with bang" do
it "does not raise error if context has required keys and values are of the correct type" do
expect { ValidateAroundTestTask.execute(Constants::VALID_ATTRIBUTES) }.to_not raise_error
expect { ValidateAroundTestTaskWithBang.execute(Constants::VALID_ATTRIBUTES) }.to_not raise_error
end

it "raises error if context is missing required keys" do
expect { ValidateAroundTestTask.execute() }.to\
expect { ValidateAroundTestTaskWithBang.execute() }.to\
raise_error(ActionLogic::MissingAttributeError)
end

it "raises error if context has required keys but values are not of correct type" do
expect { ValidateAroundTestTask.execute(Constants::INVALID_ATTRIBUTES) }.to\
expect { ValidateAroundTestTaskWithBang.execute(Constants::INVALID_ATTRIBUTES) }.to\
raise_error(ActionLogic::AttributeTypeError)
end
end
Expand All @@ -42,8 +60,19 @@ module ActionLogic
expect { ValidateAroundCustomTypeTestTask.execute(:custom_type => CustomType1.new) }.to_not raise_error
end

it "doesn't raise error if context has custom type attribute but value is not correct custom type" do
expect { ValidateAroundCustomTypeTestTask.execute(:custom_type => CustomType2.new) }.to_not\
raise_error(ActionLogic::AttributeTypeError)
end
end

describe "custom types with bang" do
it "allows validation against custom defined types" do
expect { ValidateAroundCustomTypeTestTaskWithBang.execute(:custom_type => CustomType1.new) }.to_not raise_error
end

it "raises error if context has custom type attribute but value is not correct custom type" do
expect { ValidateAroundCustomTypeTestTask.execute(:custom_type => CustomType2.new) }.to\
expect { ValidateAroundCustomTypeTestTaskWithBang.execute(:custom_type => CustomType2.new) }.to\
raise_error(ActionLogic::AttributeTypeError)
end
end
Expand All @@ -53,8 +82,19 @@ module ActionLogic
expect { ValidateAroundPresenceTestTask.execute(:integer_test => 1) }.to_not raise_error
end

it "doesn't raise error if context has required key but value is not defined when validation requires presence" do
expect { ValidateAroundPresenceTestTask.execute(:integer_test => nil) }.to_not\
raise_error(ActionLogic::PresenceError)
end
end

describe "presence with bang" do
it "validates presence if presence is specified" do
expect { ValidateAroundPresenceTestTaskWithBang.execute(:integer_test => 1) }.to_not raise_error
end

it "raises error if context has required key but value is not defined when validation requires presence" do
expect { ValidateAroundPresenceTestTask.execute(:integer_test => nil) }.to\
expect { ValidateAroundPresenceTestTaskWithBang.execute(:integer_test => nil) }.to\
raise_error(ActionLogic::PresenceError)
end
end
Expand All @@ -64,8 +104,8 @@ module ActionLogic
expect { ValidateAroundCustomPresenceTestTask.execute(:array_test => [1]) }.to_not raise_error
end

it "raises error if custom presence validation is not satisfied" do
expect { ValidateAroundCustomPresenceTestTask.execute(:array_test => []) }.to\
it "doesn't raise error if custom presence validation is not satisfied" do
expect { ValidateAroundCustomPresenceTestTask.execute(:array_test => []) }.to_not\
raise_error(ActionLogic::PresenceError)
end

Expand All @@ -74,6 +114,22 @@ module ActionLogic
raise_error(ActionLogic::UnrecognizablePresenceValidatorError)
end
end

describe "custom presence with bang" do
it "allows custom presence validation if custom presence is defined" do
expect { ValidateAroundCustomPresenceTestTaskWithBang.execute(:array_test => [1]) }.to_not raise_error
end

it "raises error if custom presence validation is not satisfied" do
expect { ValidateAroundCustomPresenceTestTaskWithBang.execute(:array_test => []) }.to\
raise_error(ActionLogic::PresenceError)
end

it "raises error if custom presence validation is not supported" do
expect { ValidateAroundUnrecognizablePresenceTestTaskWithBang.execute(:integer_test => 1) }.to\
raise_error(ActionLogic::UnrecognizablePresenceValidatorError)
end
end
end

describe "before validations" do
Expand Down
Loading