1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

deprecate accessing mime types via constants

We don't want to manage a list of constants on `Mime::`.  Managing
constants is strange because it will break method caches, not to mention
looking up by a constant could cause troubles.  For example suppose
there is a top level constant `HTML`, but nobody registers the HTML mime
type and someone accesses `Mime::HTML`.  Instead of getting an error
about how the mime type doesn't exist, instead you'll get the top level
constant.

So, instead of directly accessing the constants, change this:

  Mime::HTML

To this:

  Mime::Type[:HTML]
This commit is contained in:
Aaron Patterson 2015-09-21 11:36:10 -07:00
parent ad1d0b8408
commit efc6dd550e
4 changed files with 92 additions and 42 deletions

View file

@ -2,6 +2,7 @@ require 'set'
require 'singleton' require 'singleton'
require 'active_support/core_ext/module/attribute_accessors' require 'active_support/core_ext/module/attribute_accessors'
require 'active_support/core_ext/string/starts_ends_with' require 'active_support/core_ext/string/starts_ends_with'
require 'active_support/deprecation'
module Mime module Mime
class Mimes < Array class Mimes < Array
@ -35,6 +36,40 @@ module Mime
return type if type.is_a?(Type) return type if type.is_a?(Type)
EXTENSION_LOOKUP.fetch(type.to_s) { |k| yield k } EXTENSION_LOOKUP.fetch(type.to_s) { |k| yield k }
end end
def const_missing(sym)
if Mime::Type.registered?(sym)
ActiveSupport::Deprecation.warn <<-eow
Accessing mime types via constants is deprecated. Please change:
`Mime::#{sym}`
to:
`Mime::Type[:#{sym}]`
eow
Mime::Type[sym]
else
super
end
end
def const_defined?(sym, inherit = true)
if Mime::Type.registered?(sym)
ActiveSupport::Deprecation.warn <<-eow
Accessing mime types via constants is deprecated. Please change:
`Mime::#{sym}`
to:
`Mime::Type[:#{sym}]`
eow
true
else
super
end
end
end end
# Encapsulates the notion of a mime type. Can be used at render time, for example, with: # Encapsulates the notion of a mime type. Can be used at render time, for example, with:
@ -66,7 +101,7 @@ module Mime
def initialize(index, name, q = nil) def initialize(index, name, q = nil)
@index = index @index = index
@name = name @name = name
q ||= 0.0 if @name == Mime::ALL.to_s # default wildcard match to end of list q ||= 0.0 if @name == Mime::Type[:ALL].to_s # default wildcard match to end of list
@q = ((q || 1.0).to_f * 100).to_i @q = ((q || 1.0).to_f * 100).to_i
end end
@ -120,7 +155,7 @@ module Mime
end end
def app_xml_idx def app_xml_idx
@app_xml_idx ||= index(Mime::XML.to_s) @app_xml_idx ||= index(Mime::Type[:XML].to_s)
end end
def text_xml def text_xml
@ -137,6 +172,8 @@ module Mime
end end
end end
TYPES = {}
class << self class << self
TRAILING_STAR_REGEXP = /(text|application)\/\*/ TRAILING_STAR_REGEXP = /(text|application)\/\*/
PARAMETER_SEPARATOR_REGEXP = /;\s*\w+="?\w+"?/ PARAMETER_SEPARATOR_REGEXP = /;\s*\w+="?\w+"?/
@ -145,6 +182,18 @@ module Mime
@register_callbacks << block @register_callbacks << block
end end
def registered?(symbol)
TYPES.key? symbol
end
def [](symbol)
TYPES[symbol]
end
def add_type(symbol, type)
TYPES[symbol] = type
end
def lookup(string) def lookup(string)
LOOKUP[string] LOOKUP[string]
end end
@ -161,7 +210,7 @@ module Mime
def register(string, symbol, mime_type_synonyms = [], extension_synonyms = [], skip_lookup = false) def register(string, symbol, mime_type_synonyms = [], extension_synonyms = [], skip_lookup = false)
new_mime = Type.new(string, symbol, mime_type_synonyms) new_mime = Type.new(string, symbol, mime_type_synonyms)
Mime.const_set(symbol.upcase, new_mime) add_type symbol.upcase, new_mime
SET << new_mime SET << new_mime
@ -216,8 +265,7 @@ module Mime
# Mime::Type.unregister(:mobile) # Mime::Type.unregister(:mobile)
def unregister(symbol) def unregister(symbol)
symbol = symbol.upcase symbol = symbol.upcase
mime = Mime.const_get(symbol) mime = TYPES.delete symbol
Mime.instance_eval { remove_const(symbol) }
SET.delete_if { |v| v.eql?(mime) } SET.delete_if { |v| v.eql?(mime) }
LOOKUP.delete_if { |_,v| v.eql?(mime) } LOOKUP.delete_if { |_,v| v.eql?(mime) }

View file

@ -33,4 +33,4 @@ Mime::Type.register "application/pdf", :pdf, [], %w(pdf)
Mime::Type.register "application/zip", :zip, [], %w(zip) Mime::Type.register "application/zip", :zip, [], %w(zip)
# Create Mime::ALL but do not add it to the SET. # Create Mime::ALL but do not add it to the SET.
Mime::ALL = Mime::Type.new("*/*", :all, []) Mime::Type.add_type :ALL, Mime::Type.new("*/*", :all, [])

View file

@ -4,7 +4,7 @@ module ActionDispatch
PARAMETERS_KEY = 'action_dispatch.request.path_parameters' PARAMETERS_KEY = 'action_dispatch.request.path_parameters'
DEFAULT_PARSERS = { DEFAULT_PARSERS = {
Mime::JSON => lambda { |raw_post| Mime::Type[:JSON] => lambda { |raw_post|
data = ActiveSupport::JSON.decode(raw_post) data = ActiveSupport::JSON.decode(raw_post)
data.is_a?(Hash) ? data : {:_json => data} data.is_a?(Hash) ? data : {:_json => data}
} }

View file

@ -13,76 +13,75 @@ class MimeTypeTest < ActiveSupport::TestCase
test "unregister" do test "unregister" do
begin begin
Mime::Type.register("text/x-mobile", :mobile) Mime::Type.register("text/x-mobile", :mobile)
assert defined?(Mime::MOBILE) assert Mime.const_defined?(:MOBILE)
assert_equal Mime::MOBILE, Mime::LOOKUP['text/x-mobile'] assert_equal Mime::Type[:MOBILE], Mime::LOOKUP['text/x-mobile']
assert_equal Mime::MOBILE, Mime::EXTENSION_LOOKUP['mobile'] assert_equal Mime::Type[:MOBILE], Mime::EXTENSION_LOOKUP['mobile']
Mime::Type.unregister(:mobile) Mime::Type.unregister(:mobile)
assert !defined?(Mime::MOBILE), "Mime::MOBILE should not be defined" assert !Mime.const_defined?(:MOBILE), "Mime::MOBILE should not be defined"
assert !Mime::LOOKUP.has_key?('text/x-mobile'), "Mime::LOOKUP should not have key ['text/x-mobile]" assert !Mime::LOOKUP.has_key?('text/x-mobile'), "Mime::LOOKUP should not have key ['text/x-mobile]"
assert !Mime::EXTENSION_LOOKUP.has_key?('mobile'), "Mime::EXTENSION_LOOKUP should not have key ['mobile]" assert !Mime::EXTENSION_LOOKUP.has_key?('mobile'), "Mime::EXTENSION_LOOKUP should not have key ['mobile]"
ensure ensure
Mime.module_eval { remove_const :MOBILE if const_defined?(:MOBILE) }
Mime::LOOKUP.reject!{|key,_| key == 'text/x-mobile'} Mime::LOOKUP.reject!{|key,_| key == 'text/x-mobile'}
end end
end end
test "parse text with trailing star at the beginning" do test "parse text with trailing star at the beginning" do
accept = "text/*, text/html, application/json, multipart/form-data" accept = "text/*, text/html, application/json, multipart/form-data"
expect = [Mime::HTML, Mime::TEXT, Mime::JS, Mime::CSS, Mime::ICS, Mime::CSV, Mime::VCF, Mime::XML, Mime::YAML, Mime::JSON, Mime::MULTIPART_FORM] expect = [Mime::Type[:HTML], Mime::Type[:TEXT], Mime::Type[:JS], Mime::Type[:CSS], Mime::Type[:ICS], Mime::Type[:CSV], Mime::Type[:VCF], Mime::Type[:XML], Mime::Type[:YAML], Mime::Type[:JSON], Mime::Type[:MULTIPART_FORM]]
parsed = Mime::Type.parse(accept) parsed = Mime::Type.parse(accept)
assert_equal expect, parsed assert_equal expect, parsed
end end
test "parse text with trailing star in the end" do test "parse text with trailing star in the end" do
accept = "text/html, application/json, multipart/form-data, text/*" accept = "text/html, application/json, multipart/form-data, text/*"
expect = [Mime::HTML, Mime::JSON, Mime::MULTIPART_FORM, Mime::TEXT, Mime::JS, Mime::CSS, Mime::ICS, Mime::CSV, Mime::VCF, Mime::XML, Mime::YAML] expect = [Mime::Type[:HTML], Mime::Type[:JSON], Mime::Type[:MULTIPART_FORM], Mime::Type[:TEXT], Mime::Type[:JS], Mime::Type[:CSS], Mime::Type[:ICS], Mime::Type[:CSV], Mime::Type[:VCF], Mime::Type[:XML], Mime::Type[:YAML]]
parsed = Mime::Type.parse(accept) parsed = Mime::Type.parse(accept)
assert_equal expect, parsed assert_equal expect, parsed
end end
test "parse text with trailing star" do test "parse text with trailing star" do
accept = "text/*" accept = "text/*"
expect = [Mime::HTML, Mime::TEXT, Mime::JS, Mime::CSS, Mime::ICS, Mime::CSV, Mime::VCF, Mime::XML, Mime::YAML, Mime::JSON] expect = [Mime::Type[:HTML], Mime::Type[:TEXT], Mime::Type[:JS], Mime::Type[:CSS], Mime::Type[:ICS], Mime::Type[:CSV], Mime::Type[:VCF], Mime::Type[:XML], Mime::Type[:YAML], Mime::Type[:JSON]]
parsed = Mime::Type.parse(accept) parsed = Mime::Type.parse(accept)
assert_equal expect, parsed assert_equal expect, parsed
end end
test "parse application with trailing star" do test "parse application with trailing star" do
accept = "application/*" accept = "application/*"
expect = [Mime::HTML, Mime::JS, Mime::XML, Mime::RSS, Mime::ATOM, Mime::YAML, Mime::URL_ENCODED_FORM, Mime::JSON, Mime::PDF, Mime::ZIP] expect = [Mime::Type[:HTML], Mime::Type[:JS], Mime::Type[:XML], Mime::Type[:RSS], Mime::Type[:ATOM], Mime::Type[:YAML], Mime::Type[:URL_ENCODED_FORM], Mime::Type[:JSON], Mime::Type[:PDF], Mime::Type[:ZIP]]
parsed = Mime::Type.parse(accept) parsed = Mime::Type.parse(accept)
assert_equal expect, parsed assert_equal expect, parsed
end end
test "parse without q" do test "parse without q" do
accept = "text/xml,application/xhtml+xml,text/yaml,application/xml,text/html,image/png,text/plain,application/pdf,*/*" accept = "text/xml,application/xhtml+xml,text/yaml,application/xml,text/html,image/png,text/plain,application/pdf,*/*"
expect = [Mime::HTML, Mime::XML, Mime::YAML, Mime::PNG, Mime::TEXT, Mime::PDF, Mime::ALL] expect = [Mime::Type[:HTML], Mime::Type[:XML], Mime::Type[:YAML], Mime::Type[:PNG], Mime::Type[:TEXT], Mime::Type[:PDF], Mime::Type[:ALL]]
assert_equal expect, Mime::Type.parse(accept) assert_equal expect, Mime::Type.parse(accept)
end end
test "parse with q" do test "parse with q" do
accept = "text/xml,application/xhtml+xml,text/yaml; q=0.3,application/xml,text/html; q=0.8,image/png,text/plain; q=0.5,application/pdf,*/*; q=0.2" accept = "text/xml,application/xhtml+xml,text/yaml; q=0.3,application/xml,text/html; q=0.8,image/png,text/plain; q=0.5,application/pdf,*/*; q=0.2"
expect = [Mime::HTML, Mime::XML, Mime::PNG, Mime::PDF, Mime::TEXT, Mime::YAML, Mime::ALL] expect = [Mime::Type[:HTML], Mime::Type[:XML], Mime::Type[:PNG], Mime::Type[:PDF], Mime::Type[:TEXT], Mime::Type[:YAML], Mime::Type[:ALL]]
assert_equal expect, Mime::Type.parse(accept) assert_equal expect, Mime::Type.parse(accept)
end end
test "parse single media range with q" do test "parse single media range with q" do
accept = "text/html;q=0.9" accept = "text/html;q=0.9"
expect = [Mime::HTML] expect = [Mime::Type[:HTML]]
assert_equal expect, Mime::Type.parse(accept) assert_equal expect, Mime::Type.parse(accept)
end end
test "parse arbitrary media type parameters" do test "parse arbitrary media type parameters" do
accept = 'multipart/form-data; boundary="simple boundary"' accept = 'multipart/form-data; boundary="simple boundary"'
expect = [Mime::MULTIPART_FORM] expect = [Mime::Type[:MULTIPART_FORM]]
assert_equal expect, Mime::Type.parse(accept) assert_equal expect, Mime::Type.parse(accept)
end end
# Accept header send with user HTTP_USER_AGENT: Sunrise/0.42j (Windows XP) # Accept header send with user HTTP_USER_AGENT: Sunrise/0.42j (Windows XP)
test "parse broken acceptlines" do test "parse broken acceptlines" do
accept = "text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/*,,*/*;q=0.5" accept = "text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/*,,*/*;q=0.5"
expect = [Mime::HTML, Mime::XML, "image/*", Mime::TEXT, Mime::ALL] expect = [Mime::Type[:HTML], Mime::Type[:XML], "image/*", Mime::Type[:TEXT], Mime::Type[:ALL]]
assert_equal expect, Mime::Type.parse(accept).collect(&:to_s) assert_equal expect, Mime::Type.parse(accept).collect(&:to_s)
end end
@ -90,16 +89,14 @@ class MimeTypeTest < ActiveSupport::TestCase
# (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322; InfoPath.1) # (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322; InfoPath.1)
test "parse other broken acceptlines" do 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, */*" 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::Type[:ALL]]
assert_equal expect, Mime::Type.parse(accept).collect(&:to_s) assert_equal expect, Mime::Type.parse(accept).collect(&:to_s)
end end
test "custom type" do test "custom type" do
begin begin
Mime::Type.register("image/foo", :foo) Mime::Type.register("image/foo", :foo)
assert_nothing_raised do assert_equal Mime::Type[:FOO], Mime::SET.last
assert_equal Mime::FOO, Mime::SET.last
end
ensure ensure
Mime::Type.unregister(:FOO) Mime::Type.unregister(:FOO)
end end
@ -109,7 +106,7 @@ class MimeTypeTest < ActiveSupport::TestCase
begin begin
Mime::Type.register "text/foobar", :foobar, ["text/foo", "text/bar"] Mime::Type.register "text/foobar", :foobar, ["text/foo", "text/bar"]
%w[text/foobar text/foo text/bar].each do |type| %w[text/foobar text/foo text/bar].each do |type|
assert_equal Mime::FOOBAR, type assert_equal Mime::Type[:FOOBAR], type
end end
ensure ensure
Mime::Type.unregister(:FOOBAR) Mime::Type.unregister(:FOOBAR)
@ -124,7 +121,7 @@ class MimeTypeTest < ActiveSupport::TestCase
end end
Mime::Type.register("text/foo", :foo) Mime::Type.register("text/foo", :foo)
assert_equal [Mime::FOO], registered_mimes assert_equal [Mime::Type[:FOO]], registered_mimes
ensure ensure
Mime::Type.unregister(:FOO) Mime::Type.unregister(:FOO)
end end
@ -134,7 +131,7 @@ class MimeTypeTest < ActiveSupport::TestCase
begin begin
Mime::Type.register "text/foobar", :foobar, [], [:foo, "bar"] Mime::Type.register "text/foobar", :foobar, [], [:foo, "bar"]
%w[foobar foo bar].each do |extension| %w[foobar foo bar].each do |extension|
assert_equal Mime::FOOBAR, Mime::EXTENSION_LOOKUP[extension] assert_equal Mime::Type[:FOOBAR], Mime::EXTENSION_LOOKUP[extension]
end end
ensure ensure
Mime::Type.unregister(:FOOBAR) Mime::Type.unregister(:FOOBAR)
@ -144,15 +141,15 @@ class MimeTypeTest < ActiveSupport::TestCase
test "register alias" do test "register alias" do
begin begin
Mime::Type.register_alias "application/xhtml+xml", :foobar Mime::Type.register_alias "application/xhtml+xml", :foobar
assert_equal Mime::HTML, Mime::EXTENSION_LOOKUP['foobar'] assert_equal Mime::Type[:HTML], Mime::EXTENSION_LOOKUP['foobar']
ensure ensure
Mime::Type.unregister(:FOOBAR) Mime::Type.unregister(:FOOBAR)
end end
end end
test "type should be equal to symbol" do test "type should be equal to symbol" do
assert_equal Mime::HTML, 'application/xhtml+xml' assert_equal Mime::Type[:HTML], 'application/xhtml+xml'
assert_equal Mime::HTML, :html assert_equal Mime::Type[:HTML], :html
end end
test "type convenience methods" do test "type convenience methods" do
@ -162,9 +159,8 @@ class MimeTypeTest < ActiveSupport::TestCase
# Remove custom Mime::Type instances set in other tests, like Mime::GIF and Mime::IPHONE # Remove custom Mime::Type instances set in other tests, like Mime::GIF and Mime::IPHONE
types.delete_if { |type| !Mime.const_defined?(type.upcase) } types.delete_if { |type| !Mime.const_defined?(type.upcase) }
types.each do |type| types.each do |type|
mime = Mime.const_get(type.upcase) mime = Mime::Type[type.upcase]
assert mime.respond_to?("#{type}?"), "#{mime.inspect} does not respond to #{type}?" assert mime.respond_to?("#{type}?"), "#{mime.inspect} does not respond to #{type}?"
assert mime.send("#{type}?"), "#{mime.inspect} is not #{type}?" assert mime.send("#{type}?"), "#{mime.inspect} is not #{type}?"
invalid_types = types - [type] invalid_types = types - [type]
@ -174,8 +170,14 @@ class MimeTypeTest < ActiveSupport::TestCase
end end
test "mime all is html" do test "mime all is html" do
assert Mime::Type[:ALL].all?, "Mime::ALL is not all?"
assert Mime::Type[:ALL].html?, "Mime::ALL is not html?"
end
test "deprecated lookup" do
assert_deprecated do
assert Mime::ALL.all?, "Mime::ALL is not all?" assert Mime::ALL.all?, "Mime::ALL is not all?"
assert Mime::ALL.html?, "Mime::ALL is not html?" end
end end
test "verifiable mime types" do test "verifiable mime types" do
@ -186,18 +188,18 @@ class MimeTypeTest < ActiveSupport::TestCase
end end
test "references gives preference to symbols before strings" do test "references gives preference to symbols before strings" do
assert_equal :html, Mime::HTML.ref assert_equal :html, Mime::Type[:HTML].ref
another = Mime::Type.lookup("foo/bar") another = Mime::Type.lookup("foo/bar")
assert_nil another.to_sym assert_nil another.to_sym
assert_equal "foo/bar", another.ref assert_equal "foo/bar", another.ref
end end
test "regexp matcher" do test "regexp matcher" do
assert Mime::JS =~ "text/javascript" assert Mime::Type[:JS] =~ "text/javascript"
assert Mime::JS =~ "application/javascript" assert Mime::Type[:JS] =~ "application/javascript"
assert Mime::JS !~ "text/html" assert Mime::Type[:JS] !~ "text/html"
assert !(Mime::JS !~ "text/javascript") assert !(Mime::Type[:JS] !~ "text/javascript")
assert !(Mime::JS !~ "application/javascript") assert !(Mime::Type[:JS] !~ "application/javascript")
assert Mime::HTML =~ 'application/xhtml+xml' assert Mime::Type[:HTML] =~ 'application/xhtml+xml'
end end
end end