Hashie::Extensions::IgnoreRequired mixin for ignoring required constraints

[maxlinc]

I just noticed Hashie::Extensions::IgnoreUndeclared and thought that a similar mixin could be useful to implement a more maintainable Hashie::Bash like I proposed in #229.

This PR implements Hashie::Extensions::IgnoreRequired which is similar to IgnoreUndeclared and could be an important building-block of a future Hashie::Bash implementation. This mixin disables required constraints, which was a main feature of the Hashie::Bash, but it does not handle nested structures or have a fluent API for assignment like the original proposal in #229.

[dblock]

"be match" is probably a typo :)

[dblock]

This is interesting. It undoes what a parent class mixes in, which is to enable these required validations. My first question was "why does someone need this?", then I read the description in #229.

I am 50/50 on this. Why wouldn't you work with a Mash or a hash until the object is complete?

[maxlinc]

I didn't notice your comment until now, but to answer your question it's because I want to ensure that the data is appropriate as it is entered, just not necessarily complete.

Let's say for example you have this:

class Coordinate < Hashie::Dash
  include Hashie::Extensions::Coercion
  property :latitude, required: true
  coerce_key :latitude, ->(v) { Float(v) }
  property :longitude, required: true
  coerce_key :longitude, ->(v) { Float(v) }
end

class PartialCoordinate < Coordinate
  include Hashie::Extensions::IgnoreRequired
end

# Cannot create a partial Coordinate
Coordinate.new(latitude: 40.712784)
# ArgumentError: The property 'longitude' is required for Coordinate.

# So create a PartialCoordinate instead...
latlong = PartialCoordinate.new(latitude: 40.712784)
# =>  {:latitude=>40.712784}

# Unlike a Mash or Hash, you'll still get errors if you use a bad key
latlong.long = -74.005941
# NoMethodError: undefined method `long=' for #<PartialCoordinate latitude=40.712784>

# Or if you try to set invalid data
latlong.longitude = "74° 0' 21.3876\" W"
# ArgumentError: invalid value for Float(): "74\xC2\xB0 0' 21.3876\" W"

# If you try to coerce it to a full coordinate you'll get an error if it's incomplete
Coordinate.new(latlong)
# ArgumentError: The property 'longitude' is required for Coordinate.

# But once it's complete...
latlong.longitude = -74.005941
Coordinate.new(latlong)
# => {:latitude=>40.712784, :longitude=>-74.005941}

Another thing about this I really like is that data errors are raised at the line where the data is set rather than where it's converted. If you have code like this:

data = Hashie::Mash.new
data.latitude = 40.712784

# This is the line where I *want* the error to be raised
data.longitude = "74° 0' 21.3876\" W"

# This is the line where the error is actually raised...
coord = Coordinate.new(data)

That can be particularly useful in long methods or methods with a lot of branches.

So I think there's clear advantages of something like this over a Hash/Mash, but I'm not sure this is the implementation I want. I agree w/ you that I'd rather have mixins that add behavior that remove it.

[dblock]

I see, and I am now 80/20 :) I'd like to hear some other people's opinions about this, but in the meantime you should fix the specs, squash, update CHANGELOG, README & al. I think I would merge this.

[maxlinc]

Ha - I'm 20/80 now ;)

I still want to be able to ignore required constraints, but I think this is a hacky way to do it. I'm thinking about other problems as well, like being able to skipping coercion for rspec instance_doubles, and supporting enums or other validations (#182).

So I think I'd rather see a proper validate/validator API first. This might still be the solution, but the code could turn into something more readable like:

def included(base)
  base.hashie_validators.each do | validator |
    validator.disable if validator === Hashie::Validator::Required
  end
end

Or an API might even make something like this possible:

Hashie.without_validating(Hashie::Validator::Required) do
  latlong = Coordinate.new
  latlong.latitude = 40.712784
  latlong.longitude = -74.005941
  latlong
end.validate

The second approach would actually work better for the Bash / "Builder Hash" proposal, assuming it (temporarily) disables the validator on nested objects as well.

[maxlinc]

I think to move forward:

• I'd like to discuss Added support for enumerated types in Dash #182 a bit more, and come up with a proposal for a simple but flexible validation system
• Then see if the built-in validators can be refactored to match whatever design comes out of Added support for enumerated types in Dash #182.

[dblock]

I really like the without_validating (maybe without_validation) suggestion, this is much cleaner. This is similar to save(validate: false) of some ORMs.

[maxlinc]

I thought about without_validation, but then I leaned towards without_validating because it looks better if you pass arguments. Remember that in my scenario I don't want to turn off all validations, just require validations. I still want undeclared method or invalid type validations.

Do you know an appropriate way to store the state? I'm planning to use Thread.current[...] unless you know a better pattern.

[dblock]

I think Thread.current is fairly standard, check out https://github.com/mongoid/mongoid/blob/1f3875a0a98a089f5d54331191ea9bf8a2c3e755/lib/mongoid/relations/accessors.rb#L138 which uses some solid infrastructure for this kind of stuff.