Bug - Mash with value coercion

[antulik]

class UserMash < Hashie::Mash
end
class TweetMash < Hashie::Mash
  include Hashie::Extensions::Coercion
  coerce_value Hash, UserMash
end

t = TweetMash.new(:user => {:email => 'foo@bar.com'})
t.user.class
=> TweetMash

it returns TweetMash but should return UserMash

However it works correctly when assigning value after

t.a = {a:1}
t.a.class
=> UserMash

[antulik]

did some investigation with this example

class TweetMash < Hashie::Mash
  include Hashie::Extensions::Coercion
  coerce_value Hash, Hashie::Mash
end
t = TweetMash.new(:user => {:email => 'foo@bar.com'})

first Mash#convert_value converts new hash to self class (TweetMash) before it's passed to coercion module, but then in lib/hashie/extensions/coercion.rb#coerce_or_init

          ....
          elsif type.respond_to?(:new)
            lambda do |v|
              return v if v.is_a? type
              type.new(v)
            end
          else
          ...

line return v if v.is_a? type is triggered because v(TweetMash) is child of type(Hashie::Mash)

The workaround I've found is to use proc

class TweetMash < Hashie::Mash
  include Hashie::Extensions::Coercion
  coerce_value self, -> (v) do
    Hashie::Mash.new(v)
  end, strict: true

  coerce_value Hash, Hashie::Mash
end

the first coerce_value is with proc and strict mode to force self class into Mash, the second is to use normal coercion behaviour.

[dblock]

This smells like a bug to me. The is_a? should be rewritten to not consider parent/child classes?

[michaelherold]

Analysis

I looked into this some. The main issue with the example is that Hashie::Mash deep_updates the source hash when passing into the initializer. So this:

TweetMash.new(:user => {:email => 'foo@bar.com'})

effectively becomes this:

TweetMash.new(TweetMash[:user => TweetMash[:email => 'foo@bar.com']])

Thus, the coerce_value never triggers, since a Hash is never sent to it.

As noted above, if the example is changed to this:

t = TweetMash.new
t.user = {:email => 'foo@bar.com'}

then the coercion works as expected. This is because the coercion takes precedence over the Hash-to-Mashifying writer, since the coercion was mixed into the Mash.

To me, this is a non-obvious interaction with an equally non-obvious solution. Mash's specification is such that it's expected to convert all lower keys recursively to its class (see these lines, which are called from the deep_update in the initializer), so I don't know whether this is really a bug or if it's just an unfortunate inability for Hashie::Extensions::Coercion to work within the context of Mash's initializer.

The only recourse for changing this that I can see is to make the Coercion mixin override some Mash behavior when it is mixed into a Mash. This sounds very fragile, since it will strictly couple that behavior to the Mash implementation, which would necessitate a change to the override whenever a pertinent piece of the Mash implementation changes.

Suggested alternative implementation

@antulik, here is a possible solution you could use.

require 'hashie'

class UserMash < Hashie::Mash
end

class TweetHash < Hash
  include Hashie::Extensions::Coercion
  include Hashie::Extensions::MergeInitializer
  include Hashie::Extensions::MethodAccess

  coerce_value Hash, UserMash
end

t = TweetHash.new(:user => {:email => 'foo@bar.com'})
t.user.class #=> UserMash

By using the lighter weight mixins instead of the heavyweight Mash, it's easier to reason about the behavior. I would much rather create the individual pieces of the overall "Tweet[H|M]ash" than rely on the behavior of Mash if I wanted to customize the behavior. There are a lot of moving pieces to Mash, so in cases like this I feel it's not well-suited to the task.

Maintenance concerns

What do we want to do about this? Should we spend the time and energy to make a Mash-specific override for Coercion? Should we write documentation about this? What would the documentation say? What is a useful byproduct of this bug report and analysis?

[dblock]

First, hats off for debugging this.

I think that this should be closed or left open as a bug without a fix. At best it should be a blog post in the WTF category. Mash is such a crapshoot with lots of unexpected side effects - I wouldn't pollute clean extensions with more hacks around Mash. The alternative implementation by using well behaved extensions is much better.