Skip to content

Commit

Permalink
Optimize default path settings
Browse files Browse the repository at this point in the history
Use const static function for route match?
  • Loading branch information
ericproulx committed Nov 25, 2024
1 parent a3aea21 commit b3c5fd7
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 47 deletions.
9 changes: 4 additions & 5 deletions benchmark/compile_many_routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
$LOAD_PATH.unshift(File.join(File.dirname(__FILE__), '..', 'lib'))
require 'grape'
require 'benchmark/ips'
require 'memory_profiler'

class API < Grape::API
prefix :api
Expand All @@ -15,8 +16,6 @@ class API < Grape::API
end
end

Benchmark.ips do |ips|
ips.report('Compiling 2000 routes') do
API.compile!
end
end
MemoryProfiler.report(allow_files: 'grape') do
API.compile!
end.pretty_print(to_file: 'optimize_path.txt')
18 changes: 9 additions & 9 deletions lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,13 @@ def mount_in(router)
end

def to_routes
route_options = prepare_default_route_attributes
map_routes do |method, path|
path = prepare_path(path)
route_options[:suffix] = path.suffix
params = options[:route_options].merge(route_options)
route = Grape::Router::Route.new(method, path.path, params)
default_route_options = prepare_default_route_attributes
default_path_settings = prepare_default_path_settings

map_routes do |method, raw_path|
prepared_path = Path.new(raw_path, namespace, default_path_settings)
params = options[:route_options].present? ? options[:route_options].merge(default_route_options) : default_route_options
route = Grape::Router::Route.new(method, prepared_path.origin, prepared_path.suffix, params)
route.apply(self)
end.flatten
end
Expand Down Expand Up @@ -200,11 +201,10 @@ def map_routes
options[:method].map { |method| options[:path].map { |path| yield method, path } }
end

def prepare_path(path)
def prepare_default_path_settings
namespace_stackable_hash = inheritable_setting.namespace_stackable.to_hash
namespace_inheritable_hash = inheritable_setting.namespace_inheritable.to_hash
path_settings = namespace_stackable_hash.merge!(namespace_inheritable_hash)
Path.new(path, namespace, path_settings)
namespace_stackable_hash.merge!(namespace_inheritable_hash)
end

def namespace
Expand Down
6 changes: 3 additions & 3 deletions lib/grape/path.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ class Path
NO_VERSIONING_WITH_VALID_PATH_FORMAT_SEGMENT = '(.:format)'
VERSION_SEGMENT = ':version'

attr_reader :path, :suffix
attr_reader :origin, :suffix

def initialize(raw_path, raw_namespace, settings)
@path = PartsCache[build_parts(raw_path, raw_namespace, settings)]
@origin = PartsCache[build_parts(raw_path, raw_namespace, settings)]
@suffix = build_suffix(raw_path, raw_namespace, settings)
end

def to_s
"#{path}#{suffix}"
"#{origin}#{suffix}"
end

private
Expand Down
43 changes: 24 additions & 19 deletions lib/grape/router/pattern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ class Pattern

attr_reader :origin, :path, :pattern, :to_regexp

def_delegators :pattern, :named_captures, :params
def_delegators :pattern, :params
def_delegators :to_regexp, :===
alias match? ===

def initialize(pattern, options)
@origin = pattern
@path = build_path(pattern, options)
@pattern = build_pattern(@path, options)
def initialize(origin, suffix, options)
@origin = origin
@path = build_path(origin, options[:anchor], suffix)
@pattern = build_pattern(@path, options[:params], options[:format], options[:version], options[:requirements])
@to_regexp = @pattern.to_regexp
end

Expand All @@ -28,33 +28,34 @@ def captures_default

private

def build_pattern(path, options)
def build_pattern(path, params, format, version, requirements)
Mustermann::Grape.new(
path,
uri_decode: true,
params: options[:params],
capture: extract_capture(options)
params: params,
capture: extract_capture(format, version, requirements)
)
end

def build_path(pattern, options)
PatternCache[[build_path_from_pattern(pattern, options), options[:suffix]]]
def build_path(pattern, anchor, suffix)
PatternCache[[build_path_from_pattern(pattern, anchor), suffix]]
end

def extract_capture(options)
sliced_options = options
.slice(:format, :version)
.delete_if { |_k, v| v.blank? }
.transform_values { |v| Array.wrap(v).map(&:to_s) }
return sliced_options if options[:requirements].blank?
def extract_capture(format, version, requirements)
capture = {}.tap do |h|
h[:format] = map_str(format) if format.present?
h[:version] = map_str(version) if version.present?
end

return capture if requirements.blank?

options[:requirements].merge(sliced_options)
requirements.merge(capture)
end

def build_path_from_pattern(pattern, options)
def build_path_from_pattern(pattern, anchor)
if pattern.end_with?('*path')
pattern.dup.insert(pattern.rindex('/') + 1, '?')
elsif options[:anchor]
elsif anchor
pattern
elsif pattern.end_with?('/')
"#{pattern}?*path"
Expand All @@ -63,6 +64,10 @@ def build_path_from_pattern(pattern, options)
end
end

def map_str(value)
Array.wrap(value).map(&:to_s)
end

class PatternCache < Grape::Util::Cache
def initialize
super
Expand Down
12 changes: 8 additions & 4 deletions lib/grape/router/route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,17 @@ class Router
class Route < BaseRoute
extend Forwardable

FORWARD_MATCH_METHOD = ->(input, pattern) { input.start_with?(pattern.origin) }
NON_FORWARD_MATCH_METHOD = ->(input, pattern) { pattern.match?(input) }

attr_reader :app, :request_method

def_delegators :pattern, :path, :origin

def initialize(method, pattern, options)
def initialize(method, origin, path, options)
@request_method = upcase_method(method)
@pattern = Grape::Router::Pattern.new(pattern, options)
@pattern = Grape::Router::Pattern.new(origin, path, options)
@match_function = options[:forward_match] ? FORWARD_MATCH_METHOD : NON_FORWARD_MATCH_METHOD
super(options)
end

Expand All @@ -31,7 +35,7 @@ def apply(app)
def match?(input)
return false if input.blank?

options[:forward_match] ? input.start_with?(pattern.origin) : pattern.match?(input)
@match_function.call(input, pattern)
end

def params(input = nil)
Expand All @@ -46,7 +50,7 @@ def params(input = nil)
private

def params_without_input
pattern.captures_default.merge(attributes.params)
@params_without_input ||= pattern.captures_default.merge(attributes.params)
end

def upcase_method(method)
Expand Down
14 changes: 7 additions & 7 deletions spec/grape/path_spec.rb
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
# frozen_string_literal: true

describe Grape::Path do
describe '#path' do
describe '#origin' do
context 'mount_path' do
it 'is not included when it is nil' do
path = described_class.new(nil, nil, mount_path: '/foo/bar')
expect(path.path).to eql '/foo/bar'
expect(path.origin).to eql '/foo/bar'
end

it 'is included when it is not nil' do
path = described_class.new(nil, nil, {})
expect(path.path).to eql('/')
expect(path.origin).to eql('/')
end
end

context 'root_prefix' do
it 'is not included when it is nil' do
path = described_class.new(nil, nil, {})
expect(path.path).to eql('/')
expect(path.origin).to eql('/')
end

it 'is included after the mount path' do
Expand All @@ -28,7 +28,7 @@
root_prefix: '/hello'
)

expect(path.path).to eql('/foo/hello')
expect(path.origin).to eql('/foo/hello')
end
end

Expand All @@ -40,7 +40,7 @@
root_prefix: '/hello'
)

expect(path.path).to eql('/foo/hello/namespace')
expect(path.origin).to eql('/foo/hello/namespace')
end

it 'uses the raw path after the namespace' do
Expand All @@ -51,7 +51,7 @@
root_prefix: '/hello'
)

expect(path.path).to eql('/foo/hello/namespace/raw_path')
expect(path.origin).to eql('/foo/hello/namespace/raw_path')
end
end

Expand Down

0 comments on commit b3c5fd7

Please sign in to comment.