Merge pull request #7522 from lexmag/mime_type

Refactor `Mime::Type`

actionpack/lib/action_dispatch/http/mime_type.rb

@@ -4,7 +4,7 @@ require 'active_support/core_ext/class/attribute_accessors'
 module Mime
   class Mimes < Array
     def symbols
-      @symbols ||= map {|m| m.to_sym }
+      @symbols ||= map { |m| m.to_sym }
     end
 
     %w(<< concat shift unshift push pop []= clear compact! collect!
@@ -23,14 +23,16 @@ module Mime
   EXTENSION_LOOKUP = {}
   LOOKUP           = Hash.new { |h, k| h[k] = Type.new(k) unless k.blank? }
 
-  def self.[](type)
-    return type if type.is_a?(Type)
-    Type.lookup_by_extension(type.to_s)
-  end
+  class << self
+    def [](type)
+      return type if type.is_a?(Type)
+      Type.lookup_by_extension(type)
+    end
 
-  def self.fetch(type)
-    return type if type.is_a?(Type)
-    EXTENSION_LOOKUP.fetch(type.to_s) { |k| yield k }
+    def fetch(type)
+      return type if type.is_a?(Type)
+      EXTENSION_LOOKUP.fetch(type.to_s) { |k| yield k }
+    end
   end
 
   # Encapsulates the notion of a mime type. Can be used at render time, for example, with:
@@ -61,32 +63,84 @@ module Mime
 
     # A simple helper class used in parsing the accept header
     class AcceptItem #:nodoc:
-      attr_accessor :order, :name, :q
+      attr_accessor :index, :name, :q
+      alias :to_s :name
 
-      def initialize(order, name, q=nil)
-        @order = order
-        @name = name.strip
-        q ||= 0.0 if @name == Mime::ALL # default wildcard match to end of list
+      def initialize(index, name, q = nil)
+        @index = index
+        @name = name
+        q ||= 0.0 if @name == Mime::ALL.to_s # default wildcard match to end of list
         @q = ((q || 1.0).to_f * 100).to_i
       end
 
-      def to_s
-        @name
-      end
-
       def <=>(item)
-        result = item.q <=> q
-        result = order <=> item.order if result == 0
+        result = item.q <=> @q
+        result = @index <=> item.index if result == 0
         result
       end
 
       def ==(item)
-        name == (item.respond_to?(:name) ? item.name : item)
+        @name == item.to_s
       end
     end
 
-    class << self
+    class AcceptList < Array #:nodoc:
+      def assort!
+        sort!
 
+        # Take care of the broken text/xml entry by renaming or deleting it
+        if text_xml_idx && app_xml_idx
+          app_xml.q = [text_xml.q, app_xml.q].max # set the q value to the max of the two
+          exchange_xml_items if app_xml_idx > text_xml_idx  # make sure app_xml is ahead of text_xml in the list
+          delete_at(text_xml_idx)                 # delete text_xml from the list
+        elsif text_xml_idx
+          text_xml.name = Mime::XML.to_s
+        end
+
+        # Look for more specific XML-based types and sort them ahead of app/xml
+        if app_xml_idx
+          idx = app_xml_idx
+
+          while idx < length
+            type = self[idx]
+            break if type.q < app_xml.q
+
+            if type.name.ends_with? '+xml'
+              self[app_xml_idx], self[idx] = self[idx], app_xml
+              @app_xml_idx = idx
+            end
+            idx += 1
+          end
+        end
+
+        map! { |i| Mime::Type.lookup(i.name) }.uniq!
+        to_a
+      end
+
+      private
+        def text_xml_idx
+          @text_xml_idx ||= index('text/xml')
+        end
+
+        def app_xml_idx
+          @app_xml_idx ||= index(Mime::XML.to_s)
+        end
+
+        def text_xml
+          self[text_xml_idx]
+        end
+
+        def app_xml
+          self[app_xml_idx]
+        end
+
+        def exchange_xml_items
+          self[app_xml_idx], self[text_xml_idx] = text_xml, app_xml
+          @app_xml_idx, @text_xml_idx = text_xml_idx, app_xml_idx
+        end
+    end
+
+    class << self
       TRAILING_STAR_REGEXP = /(text|application)\/\*/
       PARAMETER_SEPARATOR_REGEXP = /;\s*\w+="?\w+"?/
 
@@ -125,75 +179,30 @@ module Mime
       def parse(accept_header)
         if accept_header !~ /,/
           accept_header = accept_header.split(PARAMETER_SEPARATOR_REGEXP).first
-          if accept_header =~ TRAILING_STAR_REGEXP
-            parse_data_with_trailing_star($1)
-          else
-            [Mime::Type.lookup(accept_header)]
-          end
+          parse_trailing_star(accept_header) || [Mime::Type.lookup(accept_header)]
         else
-          # keep track of creation order to keep the subsequent sort stable
-          list, index = [], 0
-          accept_header.split(/,/).each do |header|
+          list, index = AcceptList.new, 0
+          accept_header.split(',').each do |header|
             params, q = header.split(PARAMETER_SEPARATOR_REGEXP)
             if params.present?
               params.strip!
 
-              if params =~ TRAILING_STAR_REGEXP
-                parse_data_with_trailing_star($1).each do |m|
-                  list << AcceptItem.new(index, m.to_s, q)
-                  index += 1
-                end
-              else
-                list << AcceptItem.new(index, params, q)
+              params = parse_trailing_star(params) || [params]
+
+              params.each do |m|
+                list << AcceptItem.new(index, m.to_s, q)
                 index += 1
               end
             end
           end
-          list.sort!
-
-          # Take care of the broken text/xml entry by renaming or deleting it
-          text_xml = list.index("text/xml")
-          app_xml = list.index(Mime::XML.to_s)
-
-          if text_xml && app_xml
-            # set the q value to the max of the two
-            list[app_xml].q = [list[text_xml].q, list[app_xml].q].max
-
-            # make sure app_xml is ahead of text_xml in the list
-            if app_xml > text_xml
-              list[app_xml], list[text_xml] = list[text_xml], list[app_xml]
-              app_xml, text_xml = text_xml, app_xml
-            end
-
-            # delete text_xml from the list
-            list.delete_at(text_xml)
-
-          elsif text_xml
-            list[text_xml].name = Mime::XML.to_s
-          end
-
-          # Look for more specific XML-based types and sort them ahead of app/xml
-
-          if app_xml
-            idx = app_xml
-            app_xml_type = list[app_xml]
-
-            while(idx < list.length)
-              type = list[idx]
-              break if type.q < app_xml_type.q
-              if type.name =~ /\+xml$/
-                list[app_xml], list[idx] = list[idx], list[app_xml]
-                app_xml = idx
-              end
-              idx += 1
-            end
-          end
-
-          list.map! { |i| Mime::Type.lookup(i.name) }.uniq!
-          list
+          list.assort!
         end
       end
 
+      def parse_trailing_star(accept_header)
+        parse_data_with_trailing_star($1) if accept_header =~ TRAILING_STAR_REGEXP
+      end
+
       # For an input of <tt>'text'</tt>, returns <tt>[Mime::JSON, Mime::XML, Mime::ICS,
       # Mime::HTML, Mime::CSS, Mime::CSV, Mime::JS, Mime::YAML, Mime::TEXT]</tt>.
       #
@@ -273,18 +282,18 @@ module Mime
       @@html_types.include?(to_sym) || @string =~ /html/
     end
 
-    def respond_to?(method, include_private = false) #:nodoc:
-      super || method.to_s =~ /(\w+)\?$/
-    end
-
     private
       def method_missing(method, *args)
-        if method.to_s =~ /(\w+)\?$/
-          $1.downcase.to_sym == to_sym
+        if method.to_s.ends_with? '?'
+          method[0..-2].downcase.to_sym == to_sym
         else
           super
         end
       end
+
+      def respond_to_missing?(method, include_private = false) #:nodoc:
+        method.to_s.ends_with? '?'
+      end
   end
 end
 

actionpack/test/dispatch/mime_type_test.rb

@@ -92,7 +92,7 @@ class MimeTypeTest < ActiveSupport::TestCase
   #  (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322; InfoPath.1)
   test "parse other broken acceptlines" do
     accept = "image/gif, image/x-xbitmap, image/jpeg, image/pjpeg, application/x-shockwave-flash, application/vnd.ms-excel, application/vnd.ms-powerpoint, application/msword,  , pronto/1.00.00, sslvpn/1.00.00.00, */*"
-    expect = ['image/gif', 'image/x-xbitmap', 'image/jpeg','image/pjpeg', 'application/x-shockwave-flash', 'application/vnd.ms-excel', 'application/vnd.ms-powerpoint', 'application/msword', 'pronto/1.00.00', 'sslvpn/1.00.00.00', Mime::ALL  ]
+    expect = ['image/gif', 'image/x-xbitmap', 'image/jpeg','image/pjpeg', 'application/x-shockwave-flash', 'application/vnd.ms-excel', 'application/vnd.ms-powerpoint', 'application/msword', 'pronto/1.00.00', 'sslvpn/1.00.00.00', Mime::ALL]
     assert_equal expect, Mime::Type.parse(accept).collect { |c| c.to_s }
   end