mime-types/ruby-mime-types

Nondeterministic order of MIME::Types.type_for

slonopotamus opened this issue · 9 comments

I ran a simple test case on multiple Ruby implementations:

    it 'is returns stable order for types with equal priority' do
      assert_equal %w(audio/webm video/webm),
                   MIME::Types.type_for('foo.webm')
    end

On Travis CI, it passes with whole range of Rubies that you test against.

But it fails on my Windows 10 machine because actual return value is reversed (first video/webm, second audio/webm):

  1) Failure:
MIME::Types::registry::.type_for#test_0006_is returns stable order for types with equal priority [C:/Users/Marat/Documents/ruby-mime-types/test/test
_mime_types_class.rb:105]:
Expected: ["audio/webm", "video/webm"]
  Actual: [#<MIME::Type: video/webm>, #<MIME::Type: audio/webm>]

My Ruby is:

$ ruby --version
ruby 2.6.5p114 (2019-10-01 revision 67812) [x64-mingw32]

I also ran this test on GitHub Actions and it fails on Windows too. But note that Ruby--2.2 and 2.3 did pass on Windows.

UPD: I messed up everything a bit so had to update PR since it was initially created.

Thanks for the report. I don’t have access to a Windows machine to do this testing on. The implementation of MIME::Types#type_for is currently this:

  def type_for(filename)
    Array(filename).flat_map { |fn|
      @extension_index[fn.chomp.downcase[/\.?([^.]*?)$/, 1]]
    }.compact.inject(Set.new, :+).sort { |a, b|
      a.priority_compare(b)
    }
  end

Can you try this implementation instead?

  def type_for(filename)
    Array(filename).flat_map { |fn|
      @extension_index[fn.chomp.downcase[/\.?([^.]*?)$/, 1]]
    }.compact.uniq.sort { |a, b|
      a.priority_compare(b)
    }
  end

I’m not necessarily expecting any difference

I had to tweak your suggested variant a bit:

  def type_for(filename)
    Array(filename).flat_map { |fn|
      @extension_index[fn.chomp.downcase[/\.?([^.]*?)$/, 1]].to_a
    }.compact.uniq.sort { |a, b|
      a.priority_compare(b)
    }
  end

As you can see here, the picture is the same - Rubies 2.4+ on Windows get different order.

I believe the problem is in sort. Its docs say:

The result is not guaranteed to be stable. When the comparison of two elements returns +0+, the order of the elements is unpredictable.

There are ways to make it stable: https://stackoverflow.com/a/15442966

I found an even more interesting information about sort stability: https://stackoverflow.com/questions/15442298/is-sort-in-ruby-stable/44486562#44486562

Good find. I would happily accept a PR that adds stability to the sort, but I have no time to do this myself.

Okay, I'll work on a PR. Just wanted to be sure that it is accepted as a valid bug.

It’s supposed to be stable.

Just found this ticket due to the differing order (for "file.mp4" in my case) of returned types. I see there's a PR but a comment on it saying something along the lines of "this might not be performant so I won't merge it".

Any chance of adding a flag / option to types_for / of to get this stable sort behavior in a way that's also backwards compatible?

I’ll need to look through the PR again.