aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack/lib/action_dispatch/routing/mapper.rb
Commit message (Collapse)AuthorAgeFilesLines
...
* Remove deprecated support for passing `:path` and route path as stings in ↵Rafael Mendonça França2016-10-101-12/+1
| | | | `ActionDispatch::Routing::Mapper#match`
* Remove deprecated support passing path as `nil` in ↵Rafael Mendonça França2016-10-101-5/+1
| | | | `ActionDispatch::Routing::Mapper#match`
* fixes remaining RuboCop issues [Vipul A M, Xavier Noria]Xavier Noria2016-09-011-1/+1
|
* Fix nested multiple rootsRyo Hashimoto2016-08-291-1/+1
| | | | | | | | | | | | | | | The PR #20940 enabled the use of multiple roots with different constraints at the top level but unfortunately didn't work when those roots were inside a namespace and also broke the use of root inside a namespace after a top level root was defined because the check for the existence of the named route used the global :root name and not the namespaced name. This is fixed by using the name_for_action method to expand the :root name to the full namespaced name. We can pass nil for the second argument as we're not dealing with resource definitions so don't need to handle the cases for edit and new routes. Fixes #26148.
* Change method visibility to be privateRafael Mendonça França2016-08-171-57/+57
| | | | | Those methods are only using inside this module and by a private method so they all should be private.
* Push :defaults extraction down one levelRafael Mendonça França2016-08-171-60/+60
| | | | | | | | | | | | | Since e852daa6976cc6b6b28ad0c80a188c06e226df3c only the verb methods where extracting the defaults options. It was merged a fix for the `root` method in 31fbbb7faccba25b2e3b5e10b8fca1468579d629 but `match` was still broken since `:defaults` where not extracted. This was causing routes defined using `match` and having the `:defaults` keys to not be recognized. To fix this it was extracted a new private method with the actual content of `match` and the `:defaults` extracting was moved to `match`.
* Add three new rubocop rulesRafael Mendonça França2016-08-161-2/+2
| | | | | | | | Style/SpaceBeforeBlockBraces Style/SpaceInsideBlockBraces Style/SpaceInsideHashLiteralBraces Fix all violations in the repository.
* revises more Lint/EndAlignment offensesXavier Noria2016-08-081-3/+3
|
* normalizes indentation and whitespace across the projectXavier Noria2016-08-061-16/+16
|
* remove redundant curlies from hash argumentsXavier Noria2016-08-061-1/+1
|
* modernizes hash syntax in actionpackXavier Noria2016-08-061-7/+7
|
* applies new string literal convention in actionpack/libXavier Noria2016-08-061-22/+22
| | | | | The current code base is not uniform. After some discussion, we have chosen to go with double quotes by default.
* Merge pull request #25913 from chrisarcand/fix-keyed-defaults-with-rootRafael Mendonça França2016-07-271-1/+8
|\ | | | | | | Fix keyed defaults with root
| * Fix 'defaults' option for root routeChris Arcand2016-07-211-1/+8
|/ | | | | | | | | The merging of the 'defaults' option was moved up the stack in e852daa This allows us to see where these options originate from the standard HttpHelpers (get, post, patch, put, delete) Unfortunately this move didn't incorporate the 'root' method, which has always allowed the same 'defaults' option before.
* Deprecate usage of nil as route pathVolmer2016-07-051-0/+6
| | | | | | | | | | | | | | | | | | | In Rails 4 these kind of routes used to work: ```ruby scope '/*id', controller: :builds, as: :build do get action: :show end ``` But since 1a830cbd830c7f80936dff7e3c8b26f60dcc371d, routes are only created for paths specified as strings or symbols. Implicit `nil` paths are just ignored, with no deprecation warnings or errors. Routes are simply not created. This come as a surprise for people migrating to Rails 5, since the lack of logs or errors makes hard to understand where the problem is. This commit introduces a deprecation warning in case of path as `nil`, while still allowing the route definition.
* Fix setting route's to in a scopePiotr Jakubowski2016-06-281-1/+9
| | | | | | Fixes #25488 97d7dc4 introduced a regression that resulted in ArgumentError when to was in options of the scope and not of particular route.
* Prevent `{ internal: true }` from being stored in the routerJon Moss2016-06-071-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Forgotten followup to #23669 :grimacing: If you went to an internal route (e.g. `/rails/info/routes`), you would previously see the following in your logger: ```bash Processing by Rails::InfoController#routes as HTML Parameters: {"internal"=>true} Rendering /Users/jon/code/rails/rails/railties/lib/rails/templates/rails/info/routes.html.erb within layouts/application Rendered collection of /Users/jon/code/rails/rails/actionpack/lib/action_dispatch/middleware/templates/routes/_route.html.erb [2 times] (10.5ms) Rendered /Users/jon/code/rails/rails/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb (2.5ms) Rendered /Users/jon/code/rails/rails/railties/lib/rails/templates/rails/info/routes.html.erb within layouts/application (23.5ms) Completed 200 OK in 50ms (Views: 35.1ms | ActiveRecord: 0.0ms) ``` Now, with this change, you would see: ```bash Processing by Rails::InfoController#routes as HTML Rendering /Users/jon/code/rails/rails/railties/lib/rails/templates/rails/info/routes.html.erb within layouts/application Rendered collection of /Users/jon/code/rails/rails/actionpack/lib/action_dispatch/middleware/templates/routes/_route.html.erb [2 times] (1.6ms) Rendered /Users/jon/code/rails/rails/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb (10.2ms) Rendered /Users/jon/code/rails/rails/railties/lib/rails/templates/rails/info/routes.html.erb within layouts/application (17.4ms) Completed 200 OK in 44ms (Views: 28.0ms | ActiveRecord: 0.0ms) ```
* Support for unified Integer class in Ruby 2.4+Jeremy Daer2016-05-181-2/+2
| | | | | | | | Ruby 2.4 unifies Fixnum and Bignum into Integer: https://bugs.ruby-lang.org/issues/12005 * Forward compat with new unified Integer class in Ruby 2.4+. * Backward compat with separate Fixnum/Bignum in Ruby 2.2 & 2.3. * Drops needless Fixnum distinction in docs, preferring Integer.
* Replace `loop` to `until`Molchanov Andrey2016-05-071-2/+1
|
* Merge pull request #23103 from rails/refactor-handling-of-action-defaultJeremy Daer2016-04-241-0/+4
|\ | | | | | | Refactor handling of :action default in routing
| * Refactor handling of :action default in routingAndrew White2016-02-161-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The longstanding convention in Rails is that if the :action parameter is missing or nil then it defaults to 'index'. Up until Rails 5.0.0.beta1 this was handled slightly differently than other routing defaults by deleting it from the route options and adding it to the recall parameters. With the recent focus of removing unnecessary duplications this has exposed a problem in this strategy - we are now mutating the request's path parameters and causing problems for later url generation. This will typically affect url_for rather a named url helper since the latter explicitly pass :controller, :action, etc. The fix is to add a default for :action in the route class if the path contains an :action segment and no default is passed. This change also revealed an issue with the parameterized part expiry in that it doesn't follow a right to left order - as soon as a dynamic segment is required then all other segments become required. Fixes #23019.
* | Merge pull request #24031 from ↵Jeremy Daer2016-04-191-2/+1
|\ \ | | | | | | | | | | | | | | | samphilipd/sam/do_not_clobber_options_in_route_definitions Do not destructively mutate passed options hash in route definitions
| * | Do not destructively mutate passed options hash in route definitionsSam Davies2016-03-031-2/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | - Fixes #24030 An example scope might be specified as such: ```ruby HTML = { constraints: { format: :html } }.freeze scope HTML do get 'x' end ``` This currently raises an error because the mapper attempts to destructively modify the passed options hash. This is dangerous because this options hash might even be shared with other scopes. We should instead always instantiate a new object instead of modifying the passed options.
* | | Pass over all Rails 5 warnings, to make sure:Vipul A M2016-04-121-1/+1
|/ / | | | | | | | | | | | | | | | | | | - we are ending sentences properly - fixing of space issues - fixed continuity issues in some sentences. Reverts https://github.com/rails/rails/commit/8fc97d198ef31c1d7a4b9b849b96fc08a667fb02 . This change reverts making sure we add '.' at end of deprecation sentences. This is to keep sentences within Rails itself consistent and with a '.' at the end.
* / Add `internal` attribute to routesJon Moss2016-02-221-1/+3
|/ | | | | | | | | | | | | | This is meant to provide a way for Action Cable, Sprockets, and possibly other Rack applications to mark themselves as internal, and to exclude themselves from the routing inspector, and thus `rails routes` / `rake routes`. I think this is the only way to have mounted Rack apps be marked as internal, within AD/Journey. Another option would be to create an array of regexes for internal apps, and then to iterate over that everytime a request comes through. Also, I only had the first `add_route` method set `internal`'s default to false, to avoid littering it all over the codebase.
* Fix marking of custom routes for JourneyAndrew White2016-01-201-14/+20
| | | | | | | | | | | | The Mapper build_path method marks routes where path parameters are part of a path segment as custom routes by altering the regular expression, e.g: get '/foo-:bar', to: 'foo#bar' There were some edge cases where certain constructs weren't being picked up and this commit fixes those. Fixes #23069.
* Revert "Remove literal? check to fix issue with prefixed optionals"eileencodes2016-01-201-1/+1
| | | | | | | | | This reverts commit 5d1b7c3b441654e8008dcd303f5367883ec660a6. The change here didn't actually fix the issue it was trying to fix, and this isn't the correct way to fix either issue. The problem is switching from the builder to grouping with find_all/regex is now very dependent on how you structure your path pattern.
* Remove literal? check to fix issue with prefixed optionalseileencodes2016-01-161-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In commit d993cb3 `build_path` was changed from using `grep` to `find_all` to save array allocations. This change was a little too aggressive in that when the dash comes before the symbol like `/omg-:song` the symbol is skipped. Removing the check for `n.right.left.literal?` fixes this issue, but does add back some allocations. The number of allocations are still well less than before. I've added a regression test to test this behavior for the future. Fixes #23069. Array allocations as of d993cb3: ``` {:T_SYMBOL=>11} {:T_REGEXP=>17} {:T_STRUCT=>6500} {:T_MATCH=>12004} {:T_OBJECT=>91009} {:T_DATA=>100088} {:T_HASH=>114013} {:T_STRING=>159637} {:T_ARRAY=>321056} {:T_IMEMO=>351133} ``` Array allocations after this change: ``` {:T_SYMBOL=>11} {:T_REGEXP=>1017} {:T_STRUCT=>6500} {:T_MATCH=>12004} {:T_DATA=>84092} {:T_OBJECT=>87009} {:T_HASH=>110015} {:T_STRING=>166152} {:T_ARRAY=>322056} {:T_NODE=>343558} ```
* Kept overridden root method and removed original methodPrathamesh Sonpatki2016-01-071-21/+21
| | | | | | | | | | | | | | | - The root method is defined and documented in Base module and decorated in Resources module. - The documentation in Base module actually talks about method signature of decorated method from Resources module. - Argument handling was moved to decorated method in https://github.com/rails/rails/commit/977455cc2efb94f40b4c0d46d1842be198ed7c4c to handle options such as :as with directly passed path parameter. - To avoid the confusion, removed original root method from Base module and only kept overridden version in Resources module. - References - https://github.com/rails/rails/pull/12208 & https://github.com/rails/rails/pull/12208#issuecomment-24350897.
* Merge pull request #22373 from yui-knk/ad_constraintsYves Senn2015-11-301-1/+1
|\ | | | | Add `Routing` namespace to point appropriate constant
| * Add `Routing` namespace to point appropriate constantyui-knk2015-11-221-1/+1
| | | | | | | | Make it clear we use `ActionDispatch::Routing::Endpoint`
* | Brush up errors of `ActionDispatch::Routing::Mapper#mount`yui-knk2015-11-281-6/+9
|/ | | | | | * Integrate to raise `ArgumentError` * Detailed error message when `path` is not defined * Add a test case, invalid rack app is passed
* Delete needless `require 'active_support/deprecation'`yui-knk2015-10-201-1/+0
| | | | | When `require 'active_support/rails'`, 'active_support/deprecation' is automatically loaded.
* Allow multiple `root` routes in same scope levelRafael Sales2015-10-101-2/+3
| | | | | | | | | | | | | | | | | | | | | | | When an application has multiple root entries with different constraints, the current solution is to use `get '/'`. Example: **Currently I have to do:** ```ruby get '/', to: 'portfolio#show', constraints: ->(req) { Hostname.portfolio_site?(req.host) } get '/', to: 'blog#show', constraints: ->(req) { Hostname.blog_site?(req.host) } root 'landing#show' ``` **But I would like to do:** ```ruby root 'portfolio#show', constraints: ->(req) { Hostname.portfolio_site?(req.host) } root 'blog#show', constraints: ->(req) { Hostname.blog_site?(req.host) } root 'landing#show' ``` Other URL matchers such as `get`, `post`, etc, already allows this, so I think it's fair that `root` also allow it since it's just a shortcut for a `get` internally.
* Fix mounted engine named routes regressionMatthew Erhard2015-10-071-0/+1
| | | | | | | | | | | | | | | | | When generating the url for a mounted engine through its proxy, the path should be the sum of three parts: 1. Any `SCRIPT_NAME` request header or the value of `ActionDispatch::Routing::RouteSet#relative_url_root`. 2. A prefix (the engine's mounted path). 3. The path of the named route inside the engine. Since commit https://github.com/rails/rails/commit/44ff0313c121f528a68b3bd21d6c7a96f313e3d3, this has been broken. Step 2 has been changed to: 2. A prefix (the value of `ActionDispatch::Routing::RouteSet#relative_url_root` + the engine's mounted path). The value of `ActionDispatch::Routing::RouteSet#relative_url_root` is taken into account in step 1 of the route generation and should be ignored when generating the mounted engine's prefix in step 2. This commit fixes the regression by having `ActionDispatch::Routing::RouteSet#url_for` check `options[:relative_url_root]` before falling back to `ActionDispatch::Routing::RouteSet#relative_url_root`. The prefix generating code then sets `options[:relative_url_root]` to an empty string. This empty string is used instead of `ActionDispatch::Routing::RouteSet#relative_url_root` and avoids the duplicate `relative_url_root` value in the final result. This resolves #20920 and resolves #21459
* [ci skip] Change 'an URL' to 'a URL' as URL doesn't have a vowel soundtanmay30112015-10-061-1/+1
|
* Remove not used requiresMarcin Olichwirowicz2015-09-011-3/+1
|
* always dispatch to controllers the same wayAaron Patterson2015-08-251-5/+9
| | | | | controllers should always go through the `action` class method so that their middleware is respected.
* pull up dispatcher allocationAaron Patterson2015-08-241-1/+1
| | | | | the dispatcher class isn't configurable anymore, so pull up allocation to the method that needs it.
* set route precedence at allocation timeAaron Patterson2015-08-201-2/+2
| | | | This way we can make the Route object a read-only data structure.
* don't touch internalsAaron Patterson2015-08-181-1/+1
| | | | | We shouldn't be messing with the NamedRouteCollection internals. Just ask the object if the named route is in there.
* drop array allocations when building pathsAaron Patterson2015-08-181-8/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ```ruby require 'action_pack' require 'action_dispatch' require 'benchmark/ips' route_set = ActionDispatch::Routing::RouteSet.new routes = ActionDispatch::Routing::Mapper.new route_set ObjectSpace::AllocationTracer.setup(%i{path line type}) result = ObjectSpace::AllocationTracer.trace do 500.times do routes.resources :foo end end sorted = ObjectSpace::AllocationTracer.allocated_count_table.sort_by(&:last) sorted.each do |k,v| next if v == 0 p k => v end __END__ Before: {:T_SYMBOL=>11} {:T_REGEXP=>17} {:T_STRUCT=>6500} {:T_MATCH=>12004} {:T_OBJECT=>99009} {:T_DATA=>100088} {:T_HASH=>122015} {:T_STRING=>159637} {:T_IMEMO=>363134} {:T_ARRAY=>433056} After: {:T_SYMBOL=>11} {:T_REGEXP=>17} {:T_STRUCT=>6500} {:T_MATCH=>12004} {:T_OBJECT=>91009} {:T_DATA=>100088} {:T_HASH=>114013} {:T_STRING=>159637} {:T_ARRAY=>321056} {:T_IMEMO=>351133} ```
* use the strategy pattern to match request verbsAaron Patterson2015-08-171-10/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rather than building a regexp for every route, lets use the strategy pattern to select among objects that can match HTTP verbs. This commit introduces strategy objects for each verb that has a predicate method on the request object like `get?`, `post?`, etc. When we build the route object, look up the strategy for the verbs the user specified. If we can't find it, fall back on string matching. Using a strategy / null object pattern (the `All` VerbMatcher is our "null" object in this case) we can: 1) Remove conditionals 2) Drop boot time allocations 2) Drop run time allocations 3) Improve runtime performance Here is our boot time allocation benchmark: ```ruby require 'action_pack' require 'action_dispatch' route_set = ActionDispatch::Routing::RouteSet.new routes = ActionDispatch::Routing::Mapper.new route_set result = ObjectSpace::AllocationTracer.trace do 500.times do routes.resources :foo end end sorted = ObjectSpace::AllocationTracer.allocated_count_table.sort_by(&:last) sorted.each do |k,v| next if v == 0 p k => v end __END__ Before: $ be ruby -rallocation_tracer route_test.rb {:T_SYMBOL=>11} {:T_REGEXP=>4017} {:T_STRUCT=>6500} {:T_MATCH=>12004} {:T_DATA=>84092} {:T_OBJECT=>99009} {:T_HASH=>122015} {:T_STRING=>216652} {:T_IMEMO=>355137} {:T_ARRAY=>441057} After: $ be ruby -rallocation_tracer route_test.rb {:T_SYMBOL=>11} {:T_REGEXP=>17} {:T_STRUCT=>6500} {:T_MATCH=>12004} {:T_DATA=>84092} {:T_OBJECT=>99009} {:T_HASH=>122015} {:T_STRING=>172647} {:T_IMEMO=>355136} {:T_ARRAY=>433056} ``` This benchmark adds 500 resources. Each resource has 8 routes, so it adds 4000 routes. You can see from the results that this patch eliminates 4000 Regexp allocations, ~44000 String allocations, and ~8000 Array allocations. With that, we can figure out that the previous code would allocate 1 regexp, 11 strings, and 2 arrays per route *more* than this patch in order to handle verb matching. Next lets look at runtime allocations: ```ruby require 'action_pack' require 'action_dispatch' require 'benchmark/ips' route_set = ActionDispatch::Routing::RouteSet.new routes = ActionDispatch::Routing::Mapper.new route_set routes.resources :foo route = route_set.routes.first request = ActionDispatch::Request.new("REQUEST_METHOD" => "GET") result = ObjectSpace::AllocationTracer.trace do 500.times do route.matches? request end end sorted = ObjectSpace::AllocationTracer.allocated_count_table.sort_by(&:last) sorted.each do |k,v| next if v == 0 p k => v end __END__ Before: $ be ruby -rallocation_tracer route_test.rb {:T_MATCH=>500} {:T_STRING=>501} {:T_IMEMO=>1501} After: $ be ruby -rallocation_tracer route_test.rb {:T_IMEMO=>1001} ``` This benchmark runs 500 calls against the `matches?` method on the route object. We check this method in the case that there are two methods that match the same path, but they are differentiated by the verb (or other conditionals). For example `POST /users` vs `GET /users`, same path, different action. Previously, we were using regexps to match against the verb. You can see that doing the regexp match would allocate 1 match object and 1 string object each time it was called. This patch eliminates those allocations. Next lets look at runtime performance. ```ruby require 'action_pack' require 'action_dispatch' require 'benchmark/ips' route_set = ActionDispatch::Routing::RouteSet.new routes = ActionDispatch::Routing::Mapper.new route_set routes.resources :foo route = route_set.routes.first match = ActionDispatch::Request.new("REQUEST_METHOD" => "GET") no_match = ActionDispatch::Request.new("REQUEST_METHOD" => "POST") Benchmark.ips do |x| x.report("match") do route.matches? match end x.report("no match") do route.matches? no_match end end __END__ Before: $ be ruby -rallocation_tracer runtime.rb Calculating ------------------------------------- match 17.145k i/100ms no match 24.244k i/100ms ------------------------------------------------- match 259.708k (± 4.3%) i/s - 1.303M no match 453.376k (± 5.9%) i/s - 2.279M After: $ be ruby -rallocation_tracer runtime.rb Calculating ------------------------------------- match 23.958k i/100ms no match 29.402k i/100ms ------------------------------------------------- match 465.063k (± 3.8%) i/s - 2.324M no match 691.956k (± 4.5%) i/s - 3.469M ``` This tests tries to see how many times it can match a request per second. Switching to method calls and string comparison makes the successful match case about 79% faster, and the unsuccessful case about 52% faster. That was fun!
* switch Route constructors and pass in the regexpAaron Patterson2015-08-171-9/+15
| | | | | We don't need to add and delete from the conditions hash anymore, just pass the regexp directly to the constructor.
* introduce an alternate constructor for Route objectsAaron Patterson2015-08-171-1/+1
| | | | | I want to change the real constructor to take a particular parameter for matching the request method
* default pattern to use a joined stringAaron Patterson2015-08-171-1/+3
| | | | | The string we create is almost always the same, so rather than joining all the time, lets join once, then reuse that string everywhere.
* move route allocation to a factory method on the mapping objectAaron Patterson2015-08-151-0/+12
| | | | | | I would like to change the signature of the Route constructor. Since the mapping object has all the data required to construct a Route object, move the allocation to a factory method.
* only process `via` onceAaron Patterson2015-08-151-5/+3
| | | | | we can directly turn it in to a regular expression here, so we don't need to test its value twice
* pass pass the mapping object down the add_route stackAaron Patterson2015-08-141-6/+59
| | | | | then we can let the mapping object derive stuff that the Route object needs.
* pass the mapping object to build_routeAaron Patterson2015-08-141-5/+5
| | | | | now that we aren't doing options manipulations, we can just pass the mapping object down and read values from it.
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-01 11:10:52 +0000