Skip to content

Commit 754ed1f

Browse files
Restore the behavior of journey root node methods
rails#39935 changed the behavior of `Path::Pattern#spec` and `Route#ast` to return an `Ast` rather than the root `Node`. After eager loading, however, we clear out the `Ast` to limit retained memory and these methods return `nil`. While these methods are not public and they aren't used internally after eager loading, having them return `nil` makes it difficult for applications that had been using these methods to get access to the root `Node`. This commit restores the behavior of these two methods to return the root `Node`. The `Ast` is still available via `Path::Pattern#ast`, and we still clear it out after eager loading. Now that spec is a `Node` and not an `Ast` masquerading as one, we can get rid of the delegate methods on `Ast. Since `Route#ast` now returns the root `Node`, the newly added `Route#ast_root` is no longer necessary so I've removed it. I also removed the unused `@decorated_ast` method, which should have been removed in rails#39935.
1 parent b669e87 commit 754ed1f

File tree

6 files changed

+8
-14
lines changed

6 files changed

+8
-14
lines changed

actionpack/lib/action_dispatch/journey/nodes/node.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
module ActionDispatch
66
module Journey # :nodoc:
77
class Ast # :nodoc:
8-
delegate :find_all, :left, :right, :to_s, :to_sym, :type, to: :tree
98
attr_reader :names, :path_params, :tree, :wildcard_options, :terminals
109
alias :root :tree
1110

actionpack/lib/action_dispatch/journey/path/pattern.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ module ActionDispatch
44
module Journey # :nodoc:
55
module Path # :nodoc:
66
class Pattern # :nodoc:
7-
attr_reader :ast, :names, :requirements, :anchored
8-
alias :spec :ast
7+
attr_reader :ast, :names, :requirements, :anchored, :spec
98

109
def initialize(ast, requirements, separators, anchored)
1110
@ast = ast
11+
@spec = ast.root
1212
@requirements = requirements
1313
@separators = separators
1414
@anchored = anchored

actionpack/lib/action_dispatch/journey/route.rb

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ module ActionDispatch
55
module Journey
66
class Route
77
attr_reader :app, :path, :defaults, :name, :precedence, :constraints,
8-
:internal, :scope_options, :ast_root
8+
:internal, :scope_options, :ast
99

1010
alias :conditions :constraints
1111

@@ -65,13 +65,12 @@ def initialize(name:, app: nil, path:, constraints: {}, required_defaults: [], d
6565
@_required_defaults = required_defaults
6666
@required_parts = nil
6767
@parts = nil
68-
@decorated_ast = nil
6968
@precedence = precedence
7069
@path_formatter = @path.build_formatter
7170
@scope_options = scope_options
7271
@internal = internal
7372

74-
@ast_root = @path.ast.root
73+
@ast = @path.ast.root
7574
@path.ast.route = self
7675
end
7776

@@ -82,10 +81,6 @@ def eager_load!
8281
nil
8382
end
8483

85-
def ast
86-
path.ast
87-
end
88-
8984
# Needed for `bin/rails routes`. Picks up succinctly defined requirements
9085
# for a route, for example route
9186
#
@@ -140,7 +135,7 @@ def required_defaults
140135
end
141136

142137
def glob?
143-
ast.glob?
138+
path.ast.glob?
144139
end
145140

146141
def dispatcher?

actionpack/lib/action_dispatch/journey/router.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ def recognize(rails_req)
7777

7878
def visualizer
7979
tt = GTG::Builder.new(ast).transition_table
80-
groups = partitioned_routes.first.map(&:ast_root).group_by(&:to_s)
80+
groups = partitioned_routes.first.map(&:ast).group_by(&:to_s)
8181
asts = groups.values.map(&:first)
8282
tt.visualizer(asts)
8383
end

actionpack/lib/action_dispatch/journey/routes.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ def partition_route(route)
5050

5151
def ast
5252
@ast ||= begin
53-
nodes = anchored_routes.map(&:ast_root)
53+
nodes = anchored_routes.map(&:ast)
5454
Nodes::Or.new(nodes)
5555
end
5656
end

actionpack/test/journey/route_test.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ def test_route_adds_itself_as_memo
2424
path = path_from_string "/:controller(/:action(/:id(.:format)))"
2525
route = Route.new(name: "name", app: app, path: path)
2626

27-
route.ast.root.grep(Nodes::Terminal).each do |node|
27+
route.ast.grep(Nodes::Terminal).each do |node|
2828
assert_equal route, node.memo
2929
end
3030
end

0 commit comments

Comments
 (0)