From 69723138ad67a3c0b708b907f2831531cf87e55b Mon Sep 17 00:00:00 2001
From: Aleksey Magusev <lexmag@gmail.com>
Date: Mon, 3 Sep 2012 00:40:49 +0400
Subject: Refactor `Mime::Type`
`parse` method performance improvements - ~27-33%:
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, */*"
Benchmark.measure{ 1_000_0.times { Mime::Type.parse(accept) }}
old: 1.430000 0.000000 1.430000 ( 1.440977)
new: 0.920000 0.000000 0.920000 ( 0.921813)
---
actionpack/lib/action_dispatch/http/mime_type.rb | 177 ++++++++++++-----------
actionpack/test/dispatch/mime_type_test.rb | 2 +-
2 files changed, 94 insertions(+), 85 deletions(-)
diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb
index fd86966c50..3c1f1458ea 100644
--- a/actionpack/lib/action_dispatch/http/mime_type.rb
+++ b/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)
- 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)
+ params = parse_trailing_star(params) || [params]
- 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
+ params.each do |m|
+ list << AcceptItem.new(index, m.to_s, q)
+ index += 1
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
diff --git a/actionpack/test/dispatch/mime_type_test.rb b/actionpack/test/dispatch/mime_type_test.rb
index ed012093a7..3e83f3d4fa 100644
--- a/actionpack/test/dispatch/mime_type_test.rb
+++ b/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
--
cgit v1.2.3