Skip to content

Commit

Permalink
Handle params in forward, closes #3428
Browse files Browse the repository at this point in the history
  • Loading branch information
José Valim committed Jun 1, 2019
1 parent 8cff311 commit b10fb7d
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 12 deletions.
13 changes: 5 additions & 8 deletions lib/phoenix/router.ex
Expand Up @@ -337,9 +337,9 @@ defmodule Phoenix.Router do
{matches, _} = Enum.map_reduce(routes_with_exprs, %{}, &build_match/2)

checks =
for {%{line: line}, %{dispatch: {plug, params}}} <- routes_with_exprs, into: %{} do
for {%{line: line, plug: plug, plug_opts: plug_opts}, _} <- routes_with_exprs, into: %{} do
quote line: line do
{unquote(plug).init(unquote(params)), true}
{unquote(plug).init(unquote(Macro.escape(plug_opts))), []}
end
end

Expand Down Expand Up @@ -823,12 +823,9 @@ defmodule Phoenix.Router do
The router pipelines will be invoked prior to forwarding the
connection.
The forwarded plug will be initialized at compile time.
Note, however, that we don't advise forwarding to another
endpoint. The reason is that plugs defined by your app
and the forwarded endpoint would be invoked twice, which
may lead to errors.
However, we don't advise forwarding to another endpoint.
The reason is that plugs defined by your app and the forwarded
endpoint would be invoked twice, which may lead to errors.
## Examples
Expand Down
14 changes: 10 additions & 4 deletions test/phoenix/router/forward_test.exs
Expand Up @@ -14,6 +14,7 @@ defmodule Phoenix.Router.ForwardTest do

def index(conn, _params), do: text(conn, "admin index")
def stats(conn, _params), do: text(conn, "stats")
def params(conn, params), do: text(conn, inspect(params))
def api_users(conn, _params), do: text(conn, "api users")
def api_root(conn, _params), do: text(conn, "api root")
defp assign_fwd_script(conn, _), do: assign(conn, :fwd_script, conn.script_name)
Expand Down Expand Up @@ -56,8 +57,9 @@ defmodule Phoenix.Router.ForwardTest do
forward "/admin", AdminDashboard
forward "/init", InitPlug
forward "/assign/opts", AssignOptsPlug, %{foo: "bar"}
scope "/internal" do
forward "/api/v1", ApiRouter

scope "/params/:param" do
forward "/", Controller, :params
end
end
end
Expand Down Expand Up @@ -114,9 +116,9 @@ defmodule Phoenix.Router.ForwardTest do
conn = call(Router, :get, "admin")
assert conn.private[Router] == {[], %{
Phoenix.Router.ForwardTest.AdminDashboard => ["admin"],
Phoenix.Router.ForwardTest.ApiRouter => ["api", "v1"],
Phoenix.Router.ForwardTest.InitPlug => ["init"],
Phoenix.Router.ForwardTest.AssignOptsPlug => ["assign", "opts"]
Phoenix.Router.ForwardTest.AssignOptsPlug => ["assign", "opts"],
Phoenix.Router.ForwardTest.Controller => []
}}
assert conn.private[AdminDashboard] ==
{["admin"], %{Phoenix.Router.ForwardTest.ApiRouter => ["api-admin"]}}
Expand Down Expand Up @@ -148,6 +150,10 @@ defmodule Phoenix.Router.ForwardTest do
assert call(Router, :get, "assign/opts").assigns.opts == %{foo: "bar"}
end

test "forward can handle params" do
assert call(Router, :get, "params/hello_world").resp_body =~ ~s["param" => "hello_world"]
end

test "forward with scoped alias" do
conn = call(ApiRouter, :get, "health")
assert conn.resp_body == "health"
Expand Down

1 comment on commit b10fb7d

@barthez
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @josevalim,

I did git bisect and this commit introduced a dialyzer warning on forward macro:

router.ex:76:unmatched_return
The expression produces a value of type:

%{
  :adapter => _,
  :before_send => _,
  :content_type => _,
  :context => _,
  :document_providers => _,
  :json_codec => _,
  :log_level => _,
  :no_query_message => _,
  :pipeline => _,
  :pubsub => _,
  :raw_options => Keyword.t(),
  :schema_mod => atom(),
  :serializer => _
}

but this value is unmatched.

Related router.ex code looks like:

  scope "/graphql" do
    pipe_through :graphql

    forward "/", Absinthe.Plug,
      schema: Graphql.Schema,
      json_codec: Jason,
      before_send: {NewRelic.Absinthe, :set_transaction_name}
  end

I'm not quite sure if it is phoenix forward macro or Absinthe.Plug.init/1 call fault. Any hint how to fix it?

Please sign in to comment.