Skip to content

Commit

Permalink
Revert "Deprecate hash key path mapping"
Browse files Browse the repository at this point in the history
This reverts commit 1b40bfc.

Reason:

So far the deprecation didn't show that will provide performance
improvements, and it's not clear that the new syntax is better than the
old one.

To avoid churn to the users, we should keep the old syntax for now.
  • Loading branch information
rafaelfranca committed Aug 30, 2024
1 parent 7ed09d3 commit 5c0b749
Show file tree
Hide file tree
Showing 42 changed files with 431 additions and 466 deletions.
2 changes: 1 addition & 1 deletion actioncable/lib/action_cable/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class Engine < Rails::Engine # :nodoc:
config = app.config
unless config.action_cable.mount_path.nil?
app.routes.prepend do
mount ActionCable.server, at: config.action_cable.mount_path, internal: true, anchor: true
mount ActionCable.server => config.action_cable.mount_path, internal: true, anchor: true
end
end
end
Expand Down
16 changes: 8 additions & 8 deletions actionmailbox/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@

Rails.application.routes.draw do
scope "/rails/action_mailbox", module: "action_mailbox/ingresses" do
post "/postmark/inbound_emails", to: "postmark/inbound_emails#create", as: :rails_postmark_inbound_emails
post "/relay/inbound_emails", to: "relay/inbound_emails#create", as: :rails_relay_inbound_emails
post "/sendgrid/inbound_emails", to: "sendgrid/inbound_emails#create", as: :rails_sendgrid_inbound_emails
post "/postmark/inbound_emails" => "postmark/inbound_emails#create", as: :rails_postmark_inbound_emails
post "/relay/inbound_emails" => "relay/inbound_emails#create", as: :rails_relay_inbound_emails
post "/sendgrid/inbound_emails" => "sendgrid/inbound_emails#create", as: :rails_sendgrid_inbound_emails

# Mandrill checks for the existence of a URL with a HEAD request before it will create the webhook.
get "/mandrill/inbound_emails", to: "mandrill/inbound_emails#health_check", as: :rails_mandrill_inbound_health_check
post "/mandrill/inbound_emails", to: "mandrill/inbound_emails#create", as: :rails_mandrill_inbound_emails
get "/mandrill/inbound_emails" => "mandrill/inbound_emails#health_check", as: :rails_mandrill_inbound_health_check
post "/mandrill/inbound_emails" => "mandrill/inbound_emails#create", as: :rails_mandrill_inbound_emails

# Mailgun requires that a webhook's URL end in 'mime' for it to receive the raw contents of emails.
post "/mailgun/inbound_emails/mime", to: "mailgun/inbound_emails#create", as: :rails_mailgun_inbound_emails
post "/mailgun/inbound_emails/mime" => "mailgun/inbound_emails#create", as: :rails_mailgun_inbound_emails
end

# TODO: Should these be mounted within the engine only?
Expand All @@ -20,7 +20,7 @@
get "inbound_emails/sources/new", to: "inbound_emails/sources#new", as: :new_rails_conductor_inbound_email_source
post "inbound_emails/sources", to: "inbound_emails/sources#create", as: :rails_conductor_inbound_email_sources

post ":inbound_email_id/reroute", to: "reroutes#create", as: :rails_conductor_inbound_email_reroute
post ":inbound_email_id/incinerate", to: "incinerates#create", as: :rails_conductor_inbound_email_incinerate
post ":inbound_email_id/reroute" => "reroutes#create", as: :rails_conductor_inbound_email_reroute
post ":inbound_email_id/incinerate" => "incinerates#create", as: :rails_conductor_inbound_email_incinerate
end
end
6 changes: 3 additions & 3 deletions actionmailer/lib/action_mailer/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ class Railtie < Rails::Railtie # :nodoc:

if options.show_previews
app.routes.prepend do
get "/rails/mailers", to: "rails/mailers#index", internal: true
get "/rails/mailers/download/*path", to: "rails/mailers#download", internal: true
get "/rails/mailers/*path", to: "rails/mailers#preview", internal: true
get "/rails/mailers" => "rails/mailers#index", internal: true
get "/rails/mailers/download/*path" => "rails/mailers#download", internal: true
get "/rails/mailers/*path" => "rails/mailers#preview", internal: true
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions actionmailer/test/url_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ class WelcomeController < ActionController::Base
AppRoutes = ActionDispatch::Routing::RouteSet.new

AppRoutes.draw do
get "/welcome", to: "foo#bar", as: "welcome"
get "/dummy_model", to: "foo#baz", as: "dummy_model"
get "/welcome" => "foo#bar", as: "welcome"
get "/dummy_model" => "foo#baz", as: "dummy_model"
get "/welcome/greeting", to: "welcome#greeting"
get "/a/b(/:id)", to: "a#b"
end
Expand Down
16 changes: 0 additions & 16 deletions actionpack/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,6 @@

*Matthew Nguyen*

* Deprecate drawing routes with hash key paths to make routing faster.

```ruby
# Before
get "/users" => "users#index"
post "/logout" => :sessions
mount MyApp => "/my_app"

# After
get "/users", to: "users#index"
post "/logout", to: "sessions#logout"
mount MyApp, at: "/my_app"
```

*Gannon McGibbon*

* Deprecate drawing routes with multiple paths to make routing faster.
You may use `with_options` or a loop to make drawing multiple paths easier.

Expand Down
6 changes: 3 additions & 3 deletions actionpack/lib/action_dispatch/routing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,9 @@ module ActionDispatch
#
# # In config/routes.rb
# controller :blog do
# get 'blog/show', to: :list
# get 'blog/delete', to: :delete
# get 'blog/edit', to: :edit
# get 'blog/show' => :list
# get 'blog/delete' => :delete
# get 'blog/edit' => :edit
# end
#
# # provides named routes for show, delete, and edit
Expand Down
27 changes: 0 additions & 27 deletions actionpack/lib/action_dispatch/routing/mapper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,6 @@ def mount(app, options = nil)
options = app
app, path = options.find { |k, _| k.respond_to?(:call) }
options.delete(app) if app
hash_key_app = true
end

raise ArgumentError, "A rack application must be specified" unless app.respond_to?(:call)
Expand All @@ -642,18 +641,6 @@ def mount(app, options = nil)
or
mount(SomeRackApp => "some_route")
MSG
ActionDispatch.deprecator.warn(<<-MSG.squish) if hash_key_app
Mounting an engine with a hash key name is deprecated and
will be removed in Rails 8.1. Please use the `at:` option instead.
Instead of:
mount(SomeRackApp => "some_route")
Please use:
mount SomeRackApp, at: "some_route"
MSG

rails_app = rails_app? app
options[:as] ||= app_name(app, rails_app)
Expand Down Expand Up @@ -1699,20 +1686,6 @@ def match(path, *rest, &block)

raise ArgumentError, "Route path not specified" if path.nil?

ActionDispatch.deprecator.warn(<<-MSG.squish)
Drawing a route with a hash key name is deprecated and
will be removed in Rails 8.1. Please use the `to:` option with
"controller#action" syntax instead.
Instead of:
match "path" => "controller#action`
Please use:
match "path", to: "controller#action"
MSG

case to
when Symbol
options[:action] = to
Expand Down
6 changes: 3 additions & 3 deletions actionpack/lib/action_dispatch/routing/redirection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ def inspect
module Redirection
# Redirect any path to another path:
#
# get "/stories", to: redirect("/posts")
# get "/stories" => redirect("/posts")
#
# This will redirect the user, while ignoring certain parts of the request,
# including query string, etc. `/stories`, `/stories?foo=bar`, etc all redirect
Expand All @@ -157,7 +157,7 @@ module Redirection
# The redirect will use a `301 Moved Permanently` status code by default. This
# can be overridden with the `:status` option:
#
# get "/stories", to: redirect("/posts", status: 307)
# get "/stories" => redirect("/posts", status: 307)
#
# You can also use interpolation in the supplied redirect argument:
#
Expand Down Expand Up @@ -199,7 +199,7 @@ module Redirection
# allowing you to reuse common redirect routes. The call method must accept two
# arguments, params and request, and return a string.
#
# get 'accounts/:name', to: redirect(SubdomainRedirector.new('api'))
# get 'accounts/:name' => redirect(SubdomainRedirector.new('api'))
#
def redirect(*args, &block)
options = args.extract_options!
Expand Down
4 changes: 2 additions & 2 deletions actionpack/test/controller/base_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ def setup
def test_url_for_query_params_included
rs = ActionDispatch::Routing::RouteSet.new
rs.draw do
get "home", to: "pages#home"
get "home" => "pages#home"
end

options = {
Expand Down Expand Up @@ -316,7 +316,7 @@ class OptionalDefaultUrlOptionsControllerTest < ActionController::TestCase
def test_default_url_options_override_missing_positional_arguments
with_routing do |set|
set.draw do
get "/things/:id(.:format)", to: "things#show", as: :thing
get "/things/:id(.:format)" => "things#show", :as => :thing
end
assert_equal "/things/1.atom", thing_path("1")
assert_equal "/things/default-id.atom", thing_path
Expand Down
40 changes: 20 additions & 20 deletions actionpack/test/controller/integration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ def with_test_route_set
end

set.draw do
get "moved", to: redirect("/method")
get "moved" => redirect("/method")

ActionDispatch.deprecator.silence do
match ":action", to: controller, via: [:get, :post], as: :action
Expand Down Expand Up @@ -784,9 +784,9 @@ def self.call(*)
get "foo", to: "application_integration_test/test#index", as: :foo
get "bar", to: "application_integration_test/test#index", as: :bar

mount MountedApp, at: "/mounted", as: "mounted"
get "fooz", to: proc { |env| [ 200, { ActionDispatch::Constants::X_CASCADE => "pass" }, [ "omg" ] ] },
anchor: false
mount MountedApp => "/mounted", :as => "mounted"
get "fooz" => proc { |env| [ 200, { ActionDispatch::Constants::X_CASCADE => "pass" }, [ "omg" ] ] },
:anchor => false
get "fooz", to: "application_integration_test/test#index"
end

Expand Down Expand Up @@ -890,7 +890,7 @@ def headers
test "doesn't call controller's headers method" do
with_routing do |routes|
routes.draw do
get "/ok", to: "controller_with_headers_method_integration_test/test#index"
get "/ok" => "controller_with_headers_method_integration_test/test#index"
end

get "/ok"
Expand Down Expand Up @@ -941,10 +941,10 @@ def app
default_url_options host: "foo.com"

scope module: "url_options_integration_test" do
get "/foo", to: "foo#index", as: :foos
get "/foo/:id", to: "foo#show", as: :foo
get "/foo/:id/edit", to: "foo#edit", as: :edit_foo
get "/bar", to: "bar#index", as: :bars
get "/foo" => "foo#index", :as => :foos
get "/foo/:id" => "foo#show", :as => :foo
get "/foo/:id/edit" => "foo#edit", :as => :edit_foo
get "/bar" => "bar#index", :as => :bars
end
end

Expand Down Expand Up @@ -1004,7 +1004,7 @@ def app
end

routes.draw do
get "/foo/status", to: "head_with_status_action_integration_test/foo#status"
get "/foo/status" => "head_with_status_action_integration_test/foo#status"
end

test "get /foo/status with head result does not cause stack overflow error" do
Expand Down Expand Up @@ -1067,7 +1067,7 @@ def test_request
with_routing do |routes|
routes.draw do
ActionDispatch.deprecator.silence do
get ":action", to: FooController
get ":action" => FooController
end
end

Expand Down Expand Up @@ -1115,7 +1115,7 @@ def test_standard_json_encoding_works
with_routing do |routes|
routes.draw do
ActionDispatch.deprecator.silence do
post ":action", to: FooController
post ":action" => FooController
end
end

Expand All @@ -1142,7 +1142,7 @@ def test_doesnt_mangle_request_path
with_routing do |routes|
routes.draw do
ActionDispatch.deprecator.silence do
post ":action", to: FooController
post ":action" => FooController
end
end

Expand Down Expand Up @@ -1203,7 +1203,7 @@ def test_parsed_body_without_as_option
with_routing do |routes|
routes.draw do
ActionDispatch.deprecator.silence do
get ":action", to: FooController
get ":action" => FooController
end
end

Expand All @@ -1217,7 +1217,7 @@ def test_get_parameters_with_as_option
with_routing do |routes|
routes.draw do
ActionDispatch.deprecator.silence do
get ":action", to: FooController
get ":action" => FooController
end
end

Expand All @@ -1231,7 +1231,7 @@ def test_get_request_with_json_uses_method_override_and_sends_a_post_request
with_routing do |routes|
routes.draw do
ActionDispatch.deprecator.silence do
get ":action", to: FooController
get ":action" => FooController
end
end

Expand All @@ -1247,7 +1247,7 @@ def test_get_request_with_json_excludes_null_query_string
with_routing do |routes|
routes.draw do
ActionDispatch.deprecator.silence do
get ":action", to: FooController
get ":action" => FooController
end
end

Expand All @@ -1262,7 +1262,7 @@ def post_to_foos(as:)
with_routing do |routes|
routes.draw do
ActionDispatch.deprecator.silence do
post ":action", to: FooController
post ":action" => FooController
end
end

Expand Down Expand Up @@ -1358,8 +1358,8 @@ def remove_dumps
end

routes.draw do
get "/", to: "page_dump_integration_test/foo#index"
get "/redirect", to: "page_dump_integration_test/foo#redirect"
get "/" => "page_dump_integration_test/foo#index"
get "/redirect" => "page_dump_integration_test/foo#redirect"
end

test "save_and_open_page saves a copy of the page and call to Launchy" do
Expand Down
2 changes: 1 addition & 1 deletion actionpack/test/controller/live_stream_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,7 @@ def self.routes
end

routes.draw do
get "/test", to: "live_stream_router_test/test#index"
get "/test" => "live_stream_router_test/test#index"
end

def app
Expand Down
Loading

0 comments on commit 5c0b749

Please sign in to comment.