Skip to content

Commit

Permalink
Optionally raise on empty hash filter definitions
Browse files Browse the repository at this point in the history
I've recently spent several hours debugging a problem that was caused by an empty hash definition
```ruby
required do
   hash :hash_name
end
```

Such definition discards any hash content being passed in, which is very
confusing and I reckon rarely useful, but very unintuitive interface
(found other 3 bugs in the codebase caused by the exact same problem).

This change makes such definitions raise, but only if
Mutations.raise_on_empty_hash_filter is set to true (defualt false).
Such optional raising avoids breaking change.

Overall, I recommend changing the interface to raise on bogus definition
as a default, instead of the opt-in.
  • Loading branch information
mariokostelac committed Aug 5, 2021
1 parent fd1859b commit d245226
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 2 deletions.
9 changes: 9 additions & 0 deletions lib/mutations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,16 @@ def cache_constants=(val)
def cache_constants?
@cache_constants
end

def raise_on_empty_hash_filter=(val)
@raise_on_empty_hash_filter = val
end

def raise_on_empty_hash_filter
@raise_on_empty_hash_filter
end
end
end

Mutations.cache_constants = true
Mutations.raise_on_empty_hash_filter = false
4 changes: 4 additions & 0 deletions lib/mutations/hash_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ def initialize(opts = {}, &block)
@required_inputs = {}
@current_inputs = @required_inputs

if Mutations.raise_on_empty_hash_filter && !block_given?
raise ArgumentError.new("Hash parameter can't be created without passing the block")
end

if block_given?
instance_eval(&block)
end
Expand Down
20 changes: 18 additions & 2 deletions spec/hash_filter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,22 @@ def input.to_hash
assert_equal ({"baz" => :integer}), errors.symbolic
end

it "allows empty hash definitions" do
Mutations::HashFilter.new
# the fact that we're here means it didn't raise
end

describe "with raise_on_empty_hash_filter=true" do
it "raises for empty hash definition" do
old_value = Mutations.raise_on_empty_hash_filter
Mutations.raise_on_empty_hash_filter = true

assert_raises(ArgumentError) { Mutations::HashFilter.new }
ensure
Mutations.raise_on_empty_hash_filter = old_value
end
end

describe "optional params and nils" do
it "bar is optional -- it works if not passed" do
hf = Mutations::HashFilter.new do
Expand Down Expand Up @@ -167,7 +183,7 @@ def input.to_hash
assert_equal ({"foo" => "bar"}), filtered
assert_equal nil, errors
end

it "bar is optional -- discards empty if it needs to be stripped" do
hf = Mutations::HashFilter.new do
required do
Expand All @@ -182,7 +198,7 @@ def input.to_hash
assert_equal ({"foo" => "bar"}), filtered
assert_equal nil, errors
end

it "bar is optional -- don't discard empty if it's spaces but stripping is off" do
hf = Mutations::HashFilter.new do
required do
Expand Down

0 comments on commit d245226

Please sign in to comment.