Hydra::Derivatives::IoDecorator objects cannot be compared

[atz]

> file = File.open('/etc/passwd')
 => #<File:/etc/passwd> 
> io1 = Hydra::Derivatives::IoDecorator.new(file, 'image/png', "foobar.png")
 => #<File:/etc/passwd> 
> io2 = Hydra::Derivatives::IoDecorator.new(file, 'image/png', "foobar.png")
 => #<File:/etc/passwd> 
> file == file
 => true 
> io1.original_name == io2.original_name
 => true 
> io1.mime_type == io2.mime_type
 => true
> io1.__getobj__ == io2.__getobj__
 => true 
> io1 == io2
 => false 

Final line should be true. Literally all aspects of the two objects are identical, including the internal SimpleDelegator delegate object! Failing to provide adequate comparison makes the class untestable.

Indeed, in rspec testing, I get unhelpful failures like:

         expected: (#<Hydra::Derivatives::IoDecorator(#<File:/Users/atz/repos/hyrax/spec/fixtures/world.png>)>)
              got: (#<Hydra::Derivatives::IoDecorator(#<File:/Users/atz/repos/hyrax/spec/fixtures/world.png>)>)
       Diff:

Literally, "I got exactly what I expected, there is no discernable difference, but I failed anyway."

Note that this seems to be a limitation of SimpleDelegator with IO objects that is not part of the objects themselves:

> file == file
 => true 
> SimpleDelegator.new(1) == SimpleDelegator.new(1)
 => true 
> SimpleDelegator.new(file) == SimpleDelegator.new(file)
 => false 

That makes it a particularly questionable choice for Hydra::Derivatives::IoDecorator, since all it does is wrap IO.

[atz]

Looking at: https://github.com/ruby/ruby/blob/trunk/lib/delegate.rb#L137

You can compare a SimpleDelegate object to an identical object of the class it wraps, and everything is OK. But if you compare it to another SimpleDelegate-wrapped object, now you depend on the interior object’s willingness to declare itself == to the wrapped object. Worse, == ignores the attributes we have set at the IoDecorator level, since the internal object doesn't know about them.

IoDecorator might override ==, like:

def ==(other)
  original_name == other.original_name && mime_type == other.mime_type && super
end

BUT, this breaks comparison to the raw delegate (e.g. File objects like file don't support .original_name). So anything useful we do in our subclass is a problem. Equality can be tested against objects either:

• of (or descending from) the wrapped class, or
• fitting the profile of the subclass (IoDecorator),
• BUT NOT BOTH.

SimpleDelegate chooses the former. This pattern breaks OO completely and should be universally discouraged.