Skip to content

Remove Mustermann leaf matcher to improve performance #284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
fd355d2
store Mustermann matchers (instead of routes) in leaves
dcr8898 Mar 27, 2025
b82b44d
split routes and paths using string instead of regular expression
dcr8898 Mar 27, 2025
6c87d3f
basic proof of concept for Mustermann removal
dcr8898 Mar 29, 2025
891d752
implement basic regexp constraints
dcr8898 Mar 30, 2025
dcb35a3
refactor unit tests for leaf to match updated leaf interface
dcr8898 Apr 15, 2025
3962c53
remove unnecessary guard clause in leaf#match
dcr8898 Apr 15, 2025
ecf0f93
refactor unit tests for node to match updated node interface
dcr8898 Apr 15, 2025
f650ebd
remove deprecated attr_reader for :to in node
dcr8898 Apr 15, 2025
3ca7d5c
refactor unit tests for trie to match use standard regex (not posix)
dcr8898 Apr 16, 2025
900b6f5
skip remaining failing tests as not presently supported
dcr8898 Apr 16, 2025
1aea397
appease rubocop
dcr8898 Apr 16, 2025
a134c19
unskip generation_spec test for route with optional clause
dcr8898 Apr 18, 2025
af8d51d
add tests for routes with optional clauses in recognition_spec
dcr8898 Apr 18, 2025
3125d49
create InvalidRouteDefinitionError to use with broken optional clauses
dcr8898 Apr 18, 2025
3840939
update Leaf#match to ignore constraints placed on non-existent parama…
dcr8898 Apr 18, 2025
e89a025
implement optional route definition clauses
dcr8898 Apr 18, 2025
65dd7a1
remove namesaces when raising InvalidRouteDefinition in Router#add_op…
dcr8898 Apr 18, 2025
8110cb0
replace regex test for variable segments with string test
dcr8898 Apr 22, 2025
3b44f9c
remove some indirection in Trie when parsing segments
dcr8898 Apr 24, 2025
43ef3d1
convert globbed route detection from regexp to string method
dcr8898 Apr 24, 2025
4eedc9c
inline path splitting in Trie#find
dcr8898 Apr 25, 2025
f1c9bda
remove unnecessary local variable derived_paths in Router#add_optiona…
dcr8898 Apr 25, 2025
8f3e8e9
update Node#put unit test to expect key without ':' prefix
dcr8898 Apr 25, 2025
8de377b
update Node to transform param keys on the fly, instead of in Leaf
dcr8898 Apr 25, 2025
183639a
do not process param_keys in Leaf
dcr8898 Apr 25, 2025
e96b123
update Leaf#match unit test to remove leading ':' in test param_keys …
dcr8898 Apr 25, 2025
bc81af1
simplify Trie#find method
dcr8898 Apr 27, 2025
4286baf
improved the simplification of Trie#find method :)
dcr8898 Apr 27, 2025
341bb81
use #slice instead of #[] in Trie#find
dcr8898 Apr 29, 2025
a61b405
use interpolation instead of << in Router#add_optional_routes
dcr8898 Apr 29, 2025
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
2 changes: 1 addition & 1 deletion lib/hanami/middleware/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def leaf?
# @api private
# @since 2.0.3
def variable?(segment)
Router::ROUTE_VARIABLE_MATCHER.match?(segment)
segment.include?(Router::ROUTE_VARIABLE_INDICATOR)
end

# @api private
Expand Down
60 changes: 54 additions & 6 deletions lib/hanami/router.rb
Original file line number Diff line number Diff line change
Expand Up @@ -745,13 +745,21 @@ def env_for(env, params = {}, options = {})
# @api private
PARAMS = "router.params"

# @since 2.0.0
# @since 2.2.1
# @api private
ROUTE_VARIABLE_MATCHER = /:/
ROUTE_OPTIONAL_INDICATOR = "("

# @since 2.0.0
# @since 2.2.1
# @api private
ROUTE_VARIABLE_INDICATOR = ":"

# @since 2.2.1
# @api private
ROUTE_GLOBBED_INDICATOR = "*"

# @since 2.2.1
# @api private
ROUTE_GLOBBED_MATCHER = /\*/
ROUTE_INNER_PARENTHESES_MATCHER = /\(([^()]*)\)/

# Default response when the route method was not allowed
#
Expand Down Expand Up @@ -793,6 +801,8 @@ def add_route(http_method, path, to, as, constraints, &blk)

if globbed?(path)
add_globbed_route(http_method, path, endpoint, constraints)
elsif optional?(path)
add_optional_routes(http_method, path, to, constraints, &blk)
elsif variable?(path)
add_variable_route(http_method, path, endpoint, constraints)
else
Expand Down Expand Up @@ -848,16 +858,54 @@ def add_named_route(path, as, constraints)
@url_helpers.add(as, Segment.fabricate(path, **constraints))
end

# @since 2.2.1
# @api private
def add_optional_routes(http_method, path, to, constraints, &blk)
as = nil # avoid creating named routes for all optional permutations
optional_paths = [path]

optional_paths.each do |optional_path|
match_data = ROUTE_INNER_PARENTHESES_MATCHER.match(optional_path)

if match_data.nil?
raise InvalidRouteDefinitionError.new(http_method, path, "unmatched parenthesis in route")
end

optional_routes_from(match_data).each do |new_path|
if optional?(new_path)
optional_paths << new_path
else
add_route(http_method, new_path, to, as, constraints, &blk)
end
end
end
end

# @since 2.2.1
# @api private
def optional_routes_from(match_data)
[
-"#{match_data.pre_match}#{match_data[1]}#{match_data.post_match}",
-"#{match_data.pre_match}#{match_data.post_match}"
]
end

# @since 2.0.0
# @api private
def variable?(path)
ROUTE_VARIABLE_MATCHER.match?(path)
path.include?(ROUTE_VARIABLE_INDICATOR)
end

# @since 2.2.1
# @api private
def optional?(path)
path.include?(ROUTE_OPTIONAL_INDICATOR)
end

# @since 2.0.0
# @api private
def globbed?(path)
ROUTE_GLOBBED_MATCHER.match?(path)
path.include?(ROUTE_GLOBBED_INDICATOR)
end

# @since 2.0.0
Expand Down
15 changes: 15 additions & 0 deletions lib/hanami/router/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,21 @@ def initialize(name)
end
end

# Error raised when a route definition is invalid
#
# @see Hanami::Router#path
# @see Hanami::Router#url
#
# @since 2.2.1
# @api public
class InvalidRouteDefinitionError < Error
# @since 2.2.1
# @api private
def initialize(http_method, path, message)
super("Invalid route definition '#{http_method.downcase} #{path}': #{message}")
end
end

# Error raised when variables given for route cannot be expanded into a full path.
#
# @see Hanami::Router#path
Expand Down
24 changes: 12 additions & 12 deletions lib/hanami/router/leaf.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,29 +13,29 @@ class Leaf

# @api private
# @since 2.2.0
def initialize(route, to, constraints)
@route = route
def initialize(param_keys, to, constraints)
@param_keys = param_keys.freeze
@to = to
@constraints = constraints
@params = nil
end

# @api private
# @since 2.2.0
def match(path)
match = matcher.match(path)
def match(param_values)
@params = @param_keys.zip(param_values).to_h

@params = match.named_captures if match
return true if @constraints.empty?

match
end
@constraints.all? do |key, constraint|
param_value = @params[key.to_s]

private
next true if param_value.nil? # ignore constraints on nonexistent keys

# @api private
# @since 2.2.0
def matcher
@matcher ||= Mustermann.new(@route, type: :rails, version: "5.0", capture: @constraints)
match = constraint.match(param_value)

match && (match.string == match[0])
end
end
end
end
Expand Down
26 changes: 14 additions & 12 deletions lib/hanami/router/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ class Router
# @api private
# @since 2.0.0
class Node
# @api private
# @since 2.0.0
attr_reader :to

# @api private
# @since 2.0.0
def initialize
Expand All @@ -23,8 +19,9 @@ def initialize

# @api private
# @since 2.0.0
def put(segment)
def put(segment, param_keys)
if variable?(segment)
param_keys << segment.delete_prefix(Router::ROUTE_VARIABLE_INDICATOR).freeze
@variable ||= self.class.new
else
@fixed ||= {}
Expand All @@ -34,29 +31,34 @@ def put(segment)

# @api private
# @since 2.0.0
def get(segment)
@fixed&.fetch(segment, nil) || @variable
def get(segment, param_values)
fixed = @fixed&.fetch(segment, nil)
return fixed if fixed

param_values << segment

@variable
end

# @api private
# @since 2.0.0
def leaf!(route, to, constraints)
def leaf!(param_keys, to, constraints)
@leaves ||= []
@leaves << Leaf.new(route, to, constraints)
@leaves << Leaf.new(param_keys, to, constraints)
end

# @api private
# @since 2.2.0
def match(path)
@leaves&.find { |leaf| leaf.match(path) }
def match(param_values)
@leaves&.find { |leaf| leaf.match(param_values) }
end

private

# @api private
# @since 2.0.0
def variable?(segment)
Router::ROUTE_VARIABLE_MATCHER.match?(segment)
segment.include?(Router::ROUTE_VARIABLE_INDICATOR)
end
end
end
Expand Down
20 changes: 12 additions & 8 deletions lib/hanami/router/trie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,32 +22,36 @@ def initialize
# @api private
# @since 2.0.0
def add(route, to, constraints)
segments = segments_from(route)
node = @root
param_keys = []

segments.each do |segment|
node = node.put(segment)
segments_from(route).each do |segment|
node = node.put(segment, param_keys)
end

node.leaf!(route, to, constraints)
node.leaf!(param_keys, to, constraints)
end

# @api private
# @since 2.0.0
def find(path)
segments = segments_from(path)
node = @root
param_values = []

return unless segments.all? { |segment| node = node.get(segment) }
path.slice(1..).split(SEGMENT_SEPARATOR) do |segment|
node = node.get(segment, param_values)

node.match(path)&.then { |found| [found.to, found.params] }
break if node.nil?
end

node&.match(param_values)&.then { |found| [found.to, found.params] }
end

private

# @api private
# @since 2.0.0
SEGMENT_SEPARATOR = /\//
SEGMENT_SEPARATOR = "/"
private_constant :SEGMENT_SEPARATOR

# @api private
Expand Down
4 changes: 2 additions & 2 deletions spec/integration/hanami/router/generation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@
end
end

xit "generates relative and absolute URLs" do
it "generates relative and absolute URLs" do
runner.run!([
[:a, "/foo/bar", {var1: "foo", var2: "bar"}],
[:a, "/foo", {var1: "foo"}]
Expand Down Expand Up @@ -177,7 +177,7 @@
end
end

xit "generates relative and absolute URLs" do
it "generates relative and absolute URLs" do
runner.run!([
[:a, "/var/fooz/baz", {var1: "var", var2: "fooz", var3: "baz"}],
[:a, "/var/fooz", {var1: "var", var2: "fooz"}],
Expand Down
Loading
Loading