Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
* [#2882](https://github.com/ruby-grape/grape/pull/2882): Strip mount path and prefix in `Grape::Middleware::Versioner::Path` with `each` instead of `Enumerable#reduce` - [@ericproulx](https://github.com/ericproulx).
* [#2881](https://github.com/ruby-grape/grape/pull/2881): Decide whether a request carries a body from the env, so `Grape::Middleware::Formatter` no longer builds a `Rack::Request` for GET, HEAD and OPTIONS - [@ericproulx](https://github.com/ericproulx).
* [#2883](https://github.com/ruby-grape/grape/pull/2883): Resolve a format's content type through a plain Hash carrying both spellings instead of a `HashWithIndifferentAccess` that converts the key on every read - [@ericproulx](https://github.com/ericproulx).
* [#2885](https://github.com/ruby-grape/grape/pull/2885): Share the content-type lookup and mime-type tables between middleware instances that register the same content types, instead of building a copy per API - [@ericproulx](https://github.com/ericproulx).
* Your contribution here.

#### Fixes
Expand Down
48 changes: 47 additions & 1 deletion lib/grape/content_types.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,20 @@ def content_types_for(from_settings)
def mime_types_for(from_settings)
return MIME_TYPES if from_settings == Grape::ContentTypes::DEFAULTS

from_settings.invert.transform_keys! { |k| media_type(k) }
MimeTypesCache[from_settings]
end

# Every format under both spellings in one plain Hash, so a lookup is a
# single +Hash#[]+. +HashWithIndifferentAccess+ converted the key on every
# read instead, and this is read two or three times per request — to
# negotiate the format, and again to set the response content type.
#
# Keys arrive as Symbols: the +content_type+ DSL symbolizes what it is
# given and the defaults are Symbols. The key is stored as it came too,
# so a middleware constructed directly with String keys still answers to
# either spelling, as the indifferent hash did.
def lookup_for(from_settings)
LookupCache[from_settings]
end

# The media type of a content-type header: the part before any `;`
Expand All @@ -36,5 +49,38 @@ def media_type(content_type)
base = content_type.include?(';') ? content_type.split(';', 2).first : content_type
base.strip
end

# Both tables below are derived from nothing but the content-type registry,
# and one content-type-aware middleware is built per API instance — so an
# app mounting N APIs held N copies of tables it only ever reads. Keying
# the cache on the registry itself collapses them: Hash keys compare by
# value, so every API that registers the same content types shares one
# table.
#
# Both the key and the table are frozen: the caller's registry stays
# reachable (through +middleware.options[:content_types]+, among others)
# and mutating a live key would corrupt a cache that is now shared
# process-wide.

class MimeTypesCache < Grape::Util::Cache
def initialize
super
@cache = Hash.new do |h, from_settings|
h[from_settings.dup.freeze] = from_settings.invert.transform_keys! { |mime_type| Grape::ContentTypes.media_type(mime_type) }.freeze
end
end
end

class LookupCache < Grape::Util::Cache
def initialize
super
@cache = Hash.new do |h, from_settings|
h[from_settings.dup.freeze] = from_settings.each_with_object({}) do |(format, media_type), lookup|
lookup[format] = media_type
lookup[format.is_a?(String) ? format.to_sym : format.to_s] = media_type
end.freeze
end
end
end
end
end
8 changes: 8 additions & 0 deletions lib/grape/middleware/formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ def initialize(content_types: nil, default_format: :txt, format: nil, formatters

def_delegators :config, :default_format, :format, :formatters, :parsers

# The formatter is the only middleware that maps an incoming media type
# back to a format, so it warms +mime_types+ itself rather than making
# every content-type-aware middleware build a table none of them read.
def initialize(app, **options)
super
mime_types
end

def before
negotiate_content_type
read_body_input
Expand Down
21 changes: 7 additions & 14 deletions lib/grape/middleware/precomputed_content_types.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,18 @@ module Middleware
# at initialization so per-request +dup+s inherit them rather than
# rebuilding them.
#
# +mime_types+ is not warmed here: Formatter is the only middleware that
# reads it, and it warms it itself. The tables behind +mime_types+ and
# +content_type_for+ are shared process-wide per content-type registry
# (see Grape::ContentTypes), so the ivars below memoize a lookup, not a
# copy.
#
# Opt-in: plain +Grape::Middleware::Base+ subclasses that don't need
# content-type-aware helpers don't pay for them.
module PrecomputedContentTypes
def initialize(app, **options)
super
content_types
mime_types
content_types_lookup
end

Expand All @@ -38,20 +43,8 @@ def content_type

private

# Every format under both spellings in one plain Hash, so a lookup is a
# single +Hash#[]+. +HashWithIndifferentAccess+ converted the key on every
# read instead, and this is read two or three times per request — to
# negotiate the format, and again to set the response content type.
#
# Keys arrive as Symbols: the +content_type+ DSL symbolizes what it is
# given and the defaults are Symbols. The key is stored as it came too,
# so a middleware constructed directly with String keys still answers to
# either spelling, as the indifferent hash did.
def content_types_lookup
@content_types_lookup ||= content_types.each_with_object({}) do |(format, media_type), lookup|
lookup[format] = media_type
lookup[format.is_a?(String) ? format.to_sym : format.to_s] = media_type
end.freeze
@content_types_lookup ||= Grape::ContentTypes.lookup_for(content_types)
end
end
end
Expand Down
40 changes: 40 additions & 0 deletions spec/grape/content_types_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,46 @@
end

it { is_expected.to eq('application/xml' => :xml) }

it { is_expected.to be_frozen }

it 'returns the same table for an equal registry' do
expect(subject).to be(described_class.mime_types_for({ xml: 'application/xml;charset=utf-8' }))
end
end
end

describe '.lookup_for' do
subject { described_class.lookup_for(from_settings) }

let(:from_settings) { { json: 'application/json' } }

it 'holds every format under both spellings' do
expect(subject).to eq(json: 'application/json', 'json' => 'application/json')
end

it { is_expected.to be_frozen }

context 'with String keys' do
let(:from_settings) { { 'json' => 'application/json' } }

it 'answers to either spelling' do
expect(subject).to eq('json' => 'application/json', json: 'application/json')
end
end

context 'when another registry has the same content types' do
it 'returns the same table' do
expect(subject).to be(described_class.lookup_for({ json: 'application/json' }))
end
end

context 'when the registry is mutated after being looked up' do
it 'keeps answering the original registry' do
subject
from_settings[:xml] = 'application/xml'
expect(described_class.lookup_for({ json: 'application/json' })).to eq(json: 'application/json', 'json' => 'application/json')
end
end
end

Expand Down
Loading