aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack/lib/action_dispatch/routing/mapper.rb
Commit message (Collapse)AuthorAgeFilesLines
...
* 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.
* remove `process_path`Aaron Patterson2015-08-141-6/+2
| | | | | since we've extracted the `to` initialization, there's no need for `process_path`
* explicitly return nil from `get_to_from_path`Aaron Patterson2015-08-141-3/+3
| | | | | | if `to` was initialized, this method would return, so we can eliminate the to ||= in the conditional. Finally, let's return a nil in the else block so that it's explicit that this method can return nil
* extract method on determining :to from the pathAaron Patterson2015-08-141-5/+9
| | | | Eventually we'll pull this up and delete `process_path`.
* deprecate passing a string for both the beginning path and :path optionAaron Patterson2015-08-141-1/+16
|
* rm path_params methodAaron Patterson2015-08-141-5/+1
| | | | | | We don't need a method for something like this. I want to pull this up the stack as well and move the module + :controller ArgumentError up the stack as well
* extract method on wildcard path parameter handlingAaron Patterson2015-08-141-6/+11
|
* pass the path ast downAaron Patterson2015-08-141-2/+1
| | | | | now we don't need to add it to a hash and delete it from the hash later just to pass it around
* pull up path parsingAaron Patterson2015-08-141-10/+6
| | | | | `add_route` needs the AST, so rather than shove it in a hash and delete later, lets move parsing up the stack so we can pass down later
* stop adding path_info to the conditions hashAaron Patterson2015-08-141-1/+0
| | | | we don't need to keep adding it and deleting if from hashes.
* pull up path normalization.Aaron Patterson2015-08-141-18/+18
| | | | | Eventually I want to pull up AST generation so that we don't have to add it to the `conditions` hash.
* pull up options_constrants extractionAaron Patterson2015-08-131-16/+15
|
* remove `as`Aaron Patterson2015-08-131-8/+7
| | | | the caller already has access to `as`, so we can stop passing it around.
* remove anchor from mappingAaron Patterson2015-08-131-8/+7
| | | | | | the same value that is extracted from the options hash earlier is returned, so we don't need to pass it in in the first place. The caller already has the data, so stop passing it around.
* pull `anchor` extraction upAaron Patterson2015-08-131-17/+16
| | | | | this way we don't have to mutate the options hash so far away from where the user passed it in
* raise if `anchor` is passed to `scope`Aaron Patterson2015-08-131-0/+4
| | | | | | The `anchor` parameter [is overridden](https://github.com/rails/rails/blob/b4b4a611d0eb9aa1c640c5f521c6a43bf2a65bab/actionpack/lib/action_dispatch/routing/mapper.rb#L1528) unless it is directly passed to `match`, so setting it in a scope must be a mistake.
* remove the `add_request_method` methodAaron Patterson2015-08-121-7/+3
| | | | | I didn't like this method because it mutates the parameters. Now that the method is so small, just push it up to `initialize`
* remove side effects in `normalize_defaults`Aaron Patterson2015-08-121-8/+3
| | | | | now the `@defaults` variable doesn't need to be set before calling `normalize_defaults`
* remove unnecessary deletesAaron Patterson2015-08-121-4/+0
| | | | | | | These three options are stored in the `scope` chain outside of the options hash. If they are in the options hash, then someone passed them in to `match` and they don't really do anything. So lets remove the code.
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-20 16:38:17 +0000