Repository: phoenixframework/phoenix
Pull request #3130 (2018-10-28 18:31:24)

Document Phoenix.Channel callbacks
Add documentation and typespec parameter names to `Phoenix.Channel` callbacks, as well as missing
```elixir
{:noreply, Socket.t, timeout | :hibernate}
```
return type to `intercept/1` example.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d9a8900d1..0d41a0bcf 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -29,6 +29,11 @@ Note the websocket/longpoll configuration given to socket/3 will only apply afte
 The old APIs for building transports are also deprecated. The good news is: adapting an existing transport to the new API is a less error prone process where you should mostly remove code.
 
 
+## 1.4.0-rc.4
+
+### Enhancements
+  * [Endpoint] Allow custom keyword pairs to be passed to the socket `:connect_info` options.
+
 ## 1.4.0-rc.3 (2018-10-30)
 
 ### Enhancements
diff --git a/guides/ecto.md b/guides/ecto.md
index 17aac11c1..77a045282 100644
--- a/guides/ecto.md
+++ b/guides/ecto.md
@@ -472,7 +472,7 @@ defmodule HelloPhoenix.MixProject do
       {:phoenix, "~> 1.3.0"},
       {:phoenix_pubsub, "~> 1.0"},
       {:phoenix_ecto, "~> 4.0"},
-      {:ecto_sql, "~> 3.0-rc"},
+      {:ecto_sql, "~> 3.0"},
       {:mariaex, ">= 0.0.0"},
       {:phoenix_html, "~> 2.10"},
       {:phoenix_live_reload, "~> 1.2-rc", only: :dev},
diff --git a/installer/lib/mix/tasks/phx.new.ex b/installer/lib/mix/tasks/phx.new.ex
index a69b202e2..1eca2415e 100644
--- a/installer/lib/mix/tasks/phx.new.ex
+++ b/installer/lib/mix/tasks/phx.new.ex
@@ -136,25 +136,25 @@ defmodule Mix.Tasks.Phx.New do
   defp prompt_to_install_deps(%Project{} = project, generator) do
     install? = Mix.shell.yes?("\nFetch and install dependencies?")
 
+    cd_step = ["$ cd #{relative_app_path(project.project_path)}"]
+
     maybe_cd(project.project_path, fn ->
-      mix_pending =
-        install_mix(install?)
+      mix_step = install_mix(install?)
 
       compile =
-        case mix_pending do
+        case mix_step do
           [] -> Task.async(fn -> rebar_available?() && cmd("mix deps.compile") end)
           _  -> Task.async(fn -> :ok end)
         end
 
-      webpack_pending = install_webpack(install?, project)
+      webpack_step = install_webpack(install?, project)
       Task.await(compile, :infinity)
 
       if Project.webpack?(project) and !System.find_executable("npm") do
         print_webpack_info(project, generator)
       end
 
-      pending = mix_pending ++ (webpack_pending || [])
-      print_missing_commands(pending, project.project_path)
+      print_missing_steps(cd_step ++ mix_step ++ webpack_step)
 
       if Project.ecto?(project) do
         print_ecto_info(project, generator)
@@ -208,16 +208,7 @@ defmodule Mix.Tasks.Phx.New do
     """
   end
 
-  defp print_missing_commands([], path) do
-    Mix.shell.info """
-
-    We are all set! Go into your application by running:
-
-        $ cd #{relative_app_path(path)}
-    """
-  end
-  defp print_missing_commands(commands, path) do
-    steps = ["$ cd #{relative_app_path(path)}" | commands]
+  defp print_missing_steps(steps) do
     Mix.shell.info """
 
     We are almost there! The following steps are missing:
diff --git a/installer/templates/phx_single/README.md b/installer/templates/phx_single/README.md
index 8480b0a3d..95553d6c0 100644
--- a/installer/templates/phx_single/README.md
+++ b/installer/templates/phx_single/README.md
@@ -3,7 +3,7 @@
 To start your Phoenix server:
 
   * Install dependencies with `mix deps.get`<%= if ecto do %>
-  * Create and migrate your database with `mix ecto.create && mix ecto.migrate`<% end %><%= if webpack do %>
+  * Create and migrate your database with `mix ecto.setup`<% end %><%= if webpack do %>
   * Install Node.js dependencies with `cd assets && npm install`<% end %>
   * Start Phoenix endpoint with `mix phx.server`
 
diff --git a/installer/templates/phx_single/formatter.exs b/installer/templates/phx_single/formatter.exs
index be3b79494..1aee095fc 100644
--- a/installer/templates/phx_single/formatter.exs
+++ b/installer/templates/phx_single/formatter.exs
@@ -1,5 +1,6 @@
 [
-  import_deps: [<%= if ecto do %>:ecto, <% end %>:phoenix],
-  inputs: ["*.{ex,exs}", "{config,lib,priv,test}/**/*.{ex,exs}"]<%= if ecto do %>,
-  subdirectories: ["priv/*/migrations"]<% end %>
+  import_deps: [<%= if ecto do %>:ecto, <% end %>:phoenix],<%= if ecto do %>
+  inputs: ["*.{ex,exs}", "priv/*/seeds.exs", "{config,lib,test}/**/*.{ex,exs}"],
+  subdirectories: ["priv/*/migrations"]<% else %>
+  inputs: ["*.{ex,exs}", "{config,lib,test}/**/*.{ex,exs}"]<% end %>
 ]
diff --git a/installer/templates/phx_single/mix.exs b/installer/templates/phx_single/mix.exs
index 351beaec8..e05fce27e 100644
--- a/installer/templates/phx_single/mix.exs
+++ b/installer/templates/phx_single/mix.exs
@@ -40,8 +40,8 @@ defmodule <%= app_module %>.MixProject do
       <%= phoenix_dep %>,
       {:phoenix_pubsub, "~> 1.1"},<%= if ecto do %>
       {:phoenix_ecto, "~> 4.0"},
-      {:ecto_sql, "~> 3.0-rc"},
-      {<%= inspect adapter_app %>, ">= 0.0.0-rc"},<% end %><%= if html do %>
+      {:ecto_sql, "~> 3.0"},
+      {<%= inspect adapter_app %>, ">= 0.0.0"},<% end %><%= if html do %>
       {:phoenix_html, "~> 2.11"},
       {:phoenix_live_reload, "~> 1.2-rc", only: :dev},<% end %>
       {:gettext, "~> 0.11"},
diff --git a/installer/templates/phx_umbrella/apps/app_name/formatter.exs b/installer/templates/phx_umbrella/apps/app_name/formatter.exs
index 12c5d8dfc..b599203bb 100644
--- a/installer/templates/phx_umbrella/apps/app_name/formatter.exs
+++ b/installer/templates/phx_umbrella/apps/app_name/formatter.exs
@@ -1,5 +1,6 @@
 [<%= if ecto do %>
   import_deps: [:ecto],
-  subdirectories: ["priv/*/migrations"],<% end %>
-  inputs: ["*.{ex,exs}", "{config,lib,test}/**/*.{ex,exs}"]
+  inputs: ["*.{ex,exs}", "priv/*/seeds.exs", "{config,lib,test}/**/*.{ex,exs}"],
+  subdirectories: ["priv/*/migrations"]<% else %>
+  inputs: ["*.{ex,exs}", "{config,lib,test}/**/*.{ex,exs}"]<% end %>
 ]
diff --git a/installer/templates/phx_umbrella/apps/app_name/mix.exs b/installer/templates/phx_umbrella/apps/app_name/mix.exs
index 6adf5d043..1ae74e864 100644
--- a/installer/templates/phx_umbrella/apps/app_name/mix.exs
+++ b/installer/templates/phx_umbrella/apps/app_name/mix.exs
@@ -36,8 +36,8 @@ defmodule <%= app_module %>.MixProject do
   # Type `mix help deps` for examples and options.
   defp deps do
     [<%= if ecto do %>
-      {:<%= adapter_app %>, ">= 0.0.0-rc"},
-      {:ecto_sql, "~> 3.0-rc"},
+      {:ecto_sql, "~> 3.0"},
+      {:<%= adapter_app %>, ">= 0.0.0"}
     <% end %>]
   end
 
diff --git a/installer/templates/phx_umbrella/apps/app_name_web/README.md b/installer/templates/phx_umbrella/apps/app_name_web/README.md
index ec69404ee..ba77b2ce3 100644
--- a/installer/templates/phx_umbrella/apps/app_name_web/README.md
+++ b/installer/templates/phx_umbrella/apps/app_name_web/README.md
@@ -3,7 +3,7 @@
 To start your Phoenix server:
 
   * Install dependencies with `mix deps.get`
-  * Create and migrate your database with `mix ecto.create && mix ecto.migrate`
+  * Create and migrate your database with `mix ecto.setup`
   * Install Node.js dependencies with `cd assets && npm install`
   * Start Phoenix endpoint with `mix phx.server`
 
diff --git a/lib/phoenix/channel.ex b/lib/phoenix/channel.ex
index 6e528f1f5..312296134 100644
--- a/lib/phoenix/channel.ex
+++ b/lib/phoenix/channel.ex
@@ -20,12 +20,12 @@ defmodule Phoenix.Channel do
   matched in your channels' `join/3` callback to pluck out the scoped pattern:
 
       # handles the special `"lobby"` subtopic
-      def join("room:lobby", _auth_message, socket) do
+      def join("room:lobby", _payload, socket) do
         {:ok, socket}
       end
 
       # handles any other subtopic as the room ID, for example `"room:12"`, `"room:34"`
-      def join("room:" <> room_id, auth_message, socket) do
+      def join("room:" <> room_id, _payload, socket) do
         {:ok, socket}
       end
 
@@ -243,8 +243,8 @@ defmodule Phoenix.Channel do
   simply relay the `%Phoenix.Socket.Broadcast{}` event and payload:
 
       alias Phoenix.Socket.Broadcast
-      def handle_info(%Broadcast{topic: _, event: ev, payload: payload}, socket) do
-        push(socket, ev, payload)
+      def handle_info(%Broadcast{topic: _, event: event, payload: payload}, socket) do
+        push(socket, event, payload)
         {:noreply, socket}
       end
 
@@ -267,31 +267,74 @@ defmodule Phoenix.Channel do
   @type socket_ref :: {transport_pid :: Pid, serializer :: module,
                        topic :: binary, ref :: binary, join_ref :: binary}
 
-  @callback join(topic :: binary, auth_msg :: map, Socket.t) ::
+  @doc """
+  Handle channel joins by `topic`.
+
+  To authorize a socket, return `{:ok, socket}` or `{:ok, reply, socket}`. To
+  refuse authorization, return `{:error, reason}`.
+
+  ## Example
+
+      def join("room:lobby", payload, socket) do
+        if authorized?(payload) do
+          {:ok, socket}
+        else
+          {:error, %{reason: "unauthorized"}}
+        end
+      end
+
+  """
+  @callback join(topic :: binary, payload :: map, socket :: Socket.t) ::
               {:ok, Socket.t} |
-              {:ok, map, Socket.t} |
-              {:error, map}
+              {:ok, reply :: map, Socket.t} |
+              {:error, reason :: map}
+
+  @doc """
+  Handle incoming `event`s.
+
+  ## Example
 
-  @callback handle_in(event :: String.t, msg :: map, Socket.t) ::
+      def handle_in("ping", payload, socket) do
+        {:reply, {:ok, payload}, socket}
+      end
+
+  """
+  @callback handle_in(event :: String.t, payload :: map, socket :: Socket.t) ::
               {:noreply, Socket.t} |
               {:noreply, Socket.t, timeout | :hibernate} |
               {:reply, reply, Socket.t} |
               {:stop, reason :: term, Socket.t} |
               {:stop, reason :: term, reply, Socket.t}
 
-  @callback handle_out(event :: String.t, msg :: map, Socket.t) ::
+  @doc """
+  Intercepts outgoing `event`s.
+
+  See `intercept/1`.
+  """
+  @callback handle_out(event :: String.t, payload :: map, socket :: Socket.t) ::
               {:noreply, Socket.t} |
               {:noreply, Socket.t, timeout | :hibernate} |
               {:stop, reason :: term, Socket.t}
 
-  @callback handle_info(term, Socket.t) ::
+  @doc """
+  Handle regular Elixir process messages.
+
+  See `GenServer.handle_info/2`.
+  """
+  @callback handle_info(msg :: term, socket :: Socket.t) ::
               {:noreply, Socket.t} |
               {:stop, reason :: term, Socket.t}
 
+  @doc false
   @callback code_change(old_vsn, Socket.t, extra :: term) ::
               {:ok, Socket.t} |
               {:error, reason :: term} when old_vsn: term | {:down, term}
 
+  @doc """
+  Invoked when the channel process is about to exit.
+
+  See `GenServer.terminate/2`.
+  """
   @callback terminate(reason :: :normal | :shutdown | {:shutdown, :left | :closed | term}, Socket.t) ::
               term
 
@@ -348,6 +391,7 @@ defmodule Phoenix.Channel do
   `handle_out/3` callbacks must return one of:
 
       {:noreply, Socket.t} |
+      {:noreply, Socket.t, timeout | :hibernate} |
       {:stop, reason :: term, Socket.t}
 
   """
diff --git a/lib/phoenix/channel/server.ex b/lib/phoenix/channel/server.ex
index 7d4ce3d9d..6a8928a55 100644
--- a/lib/phoenix/channel/server.ex
+++ b/lib/phoenix/channel/server.ex
@@ -33,7 +33,7 @@ defmodule Phoenix.Channel.Server do
           channel: channel,
           join_ref: join_ref,
           assigns: Map.merge(socket.assigns, assigns),
-          private: channel.__socket__(:private)
+          private: Map.merge(channel.__socket__(:private), socket.private)
       }
 
     instrument = %{params: payload, socket: socket}
diff --git a/lib/phoenix/endpoint.ex b/lib/phoenix/endpoint.ex
index 310f922f8..5322a9542 100644
--- a/lib/phoenix/endpoint.ex
+++ b/lib/phoenix/endpoint.ex
@@ -869,6 +869,9 @@ defmodule Phoenix.Endpoint do
         * `:x_headers` - all request headers that have an "x-" prefix
         * `:uri` - a `%URI{}` with information from the conn
 
+      Arbitrary keywords may also appear following the above valid keys, which
+      is useful for passing custom connection information to the socket.
+
       For example:
 
           socket "/socket", AppWeb.UserSocket,
@@ -876,6 +879,14 @@ defmodule Phoenix.Endpoint do
               connect_info: [:peer_data, :x_headers, :uri]
             ]
 
+      With arbitrary keywords:
+
+          socket "/socket", AppWeb.UserSocket,
+            websocket: [
+              connect_info: [:uri, signing_salt: "NZIguRPO"]
+            ]
+
+
   ## Websocket configuration
 
   The following configuration applies only to `:websocket`.
diff --git a/lib/phoenix/endpoint/cowboy2_handler.ex b/lib/phoenix/endpoint/cowboy2_handler.ex
index 3e091d73b..32a1c5490 100644
--- a/lib/phoenix/endpoint/cowboy2_handler.ex
+++ b/lib/phoenix/endpoint/cowboy2_handler.ex
@@ -25,7 +25,7 @@ defmodule Phoenix.Endpoint.Cowboy2Handler do
               {:cowboy_websocket, req, [handler | state], cowboy_opts}
 
             {:error, %{adapter: {@connection, req}}} ->
-              {:error, req}
+              {:ok, req, {handler, opts}}
           end
 
         {:plug, handler, opts} ->
diff --git a/lib/phoenix/socket/transport.ex b/lib/phoenix/socket/transport.ex
index f2219d661..6f00b24c0 100644
--- a/lib/phoenix/socket/transport.ex
+++ b/lib/phoenix/socket/transport.ex
@@ -415,8 +415,10 @@ defmodule Phoenix.Socket.Transport do
         :uri ->
           {:uri, fetch_uri(conn)}
 
+        {key, val} -> {key, val}
+
         _ ->
-          raise ArgumentError, ":connect_info keys are expected to be one of :peer_data, :x_headers, or :uri, got: #{inspect(key)}"
+          raise ArgumentError, ":connect_info keys are expected to be one of :peer_data, :x_headers, or :uri, optionally followed by custom keyword pairs, got: #{inspect(key)}"
       end
     end
   end


Test patch:
diff --git a/installer/test/phx_new_test.exs b/installer/test/phx_new_test.exs
index bc8b9beb1..680f41c1d 100644
--- a/installer/test/phx_new_test.exs
+++ b/installer/test/phx_new_test.exs
@@ -24,7 +24,13 @@ defmodule Mix.Tasks.Phx.NewTest do
       Mix.Tasks.Phx.New.run([@app_name])
 
       assert_file "phx_blog/README.md"
-      assert_file "phx_blog/.formatter.exs", ~r/import_deps: \[:ecto, :phoenix\]/
+
+      assert_file "phx_blog/.formatter.exs", fn file ->
+        assert file =~ "import_deps: [:ecto, :phoenix]"
+        assert file =~ "inputs: [\"*.{ex,exs}\", \"priv/*/seeds.exs\", \"{config,lib,test}/**/*.{ex,exs}\"]"
+        assert file =~ "subdirectories: [\"priv/*/migrations\"]"
+      end
+
       assert_file "phx_blog/mix.exs", fn file ->
         assert file =~ "app: :phx_blog"
         refute file =~ "deps_path: \"../../deps\""
@@ -173,6 +179,12 @@ defmodule Mix.Tasks.Phx.NewTest do
       config = ~r/config :phx_blog, PhxBlog.Repo,/
       refute File.exists?("phx_blog/lib/phx_blog/repo.ex")
 
+      assert_file "phx_blog/.formatter.exs", fn file ->
+        assert file =~ "import_deps: [:phoenix]"
+        assert file =~ "inputs: [\"*.{ex,exs}\", \"{config,lib,test}/**/*.{ex,exs}\"]"
+        refute file =~ "subdirectories:"
+      end
+
       assert_file "phx_blog/mix.exs", &refute(&1 =~ ~r":phoenix_ecto")
 
       assert_file "phx_blog/config/config.exs", fn file ->
diff --git a/installer/test/phx_new_umbrella_test.exs b/installer/test/phx_new_umbrella_test.exs
index 269f9dde8..99dba6b6f 100644
--- a/installer/test/phx_new_umbrella_test.exs
+++ b/installer/test/phx_new_umbrella_test.exs
@@ -76,6 +76,18 @@ defmodule Mix.Tasks.Phx.New.UmbrellaTest do
         assert file =~ ":inet6"
       end
 
+      assert_file app_path(@app, ".formatter.exs"), fn file ->
+        assert file =~ "import_deps: [:ecto]"
+        assert file =~ "inputs: [\"*.{ex,exs}\", \"priv/*/seeds.exs\", \"{config,lib,test}/**/*.{ex,exs}\"]"
+        assert file =~ "subdirectories: [\"priv/*/migrations\"]"
+      end
+
+      assert_file web_path(@app, ".formatter.exs"), fn file ->
+        assert file =~ "inputs: [\"*.{ex,exs}\", \"{config,lib,test}/**/*.{ex,exs}\"]"
+        refute file =~ "import_deps: [:ecto]"
+        refute file =~ "subdirectories:"
+      end
+
       assert_file app_path(@app, "lib/#{@app}/application.ex"), ~r/defmodule PhxUmb.Application do/
       assert_file app_path(@app, "lib/#{@app}/application.ex"), ~r/PhxUmb.Repo/
       assert_file app_path(@app, "lib/#{@app}.ex"), ~r/defmodule PhxUmb do/
diff --git a/lib/phoenix/test/conn_test.ex b/lib/phoenix/test/conn_test.ex
index 2cce7bf99..416030031 100644
--- a/lib/phoenix/test/conn_test.ex
+++ b/lib/phoenix/test/conn_test.ex
@@ -434,7 +434,7 @@ defmodule Phoenix.ConnTest do
       assert redirected_to(conn, 301) =~ "/foo/bar"
       assert redirected_to(conn, :moved_permanently) =~ "/foo/bar"
   """
-  @spec redirected_to(Conn.t, status :: non_neg_integer) :: Conn.t
+  @spec redirected_to(Conn.t, status :: non_neg_integer) :: String.t
   def redirected_to(conn, status \\ 302)
 
   def redirected_to(%Conn{state: :unset}, _status) do
diff --git a/priv/templates/phx.gen.context/test_cases.exs b/priv/templates/phx.gen.context/test_cases.exs
index e83b1252f..92f73323f 100644
--- a/priv/templates/phx.gen.context/test_cases.exs
+++ b/priv/templates/phx.gen.context/test_cases.exs
@@ -36,9 +36,7 @@
 
     test "update_<%= schema.singular %>/2 with valid data updates the <%= schema.singular %>" do
       <%= schema.singular %> = <%= schema.singular %>_fixture()
-      assert {:ok, %<%= inspect schema.alias %>{} = <%= schema.singular %>} = <%= inspect context.alias %>.update_<%= schema.singular %>(<%= schema.singular %>, @update_attrs)
-
-      <%= for {field, value} <- schema.params.update do %>
+      assert {:ok, %<%= inspect schema.alias %>{} = <%= schema.singular %>} = <%= inspect context.alias %>.update_<%= schema.singular %>(<%= schema.singular %>, @update_attrs)<%= for {field, value} <- schema.params.update do %>
       assert <%= schema.singular %>.<%= field %> == <%= Mix.Phoenix.Schema.value(schema, field, value) %><% end %>
     end
 
diff --git a/test/phoenix/integration/websocket_channels_test.exs b/test/phoenix/integration/websocket_channels_test.exs
index 863df89b7..7811eb271 100644
--- a/test/phoenix/integration/websocket_channels_test.exs
+++ b/test/phoenix/integration/websocket_channels_test.exs
@@ -130,6 +130,14 @@ defmodule Phoenix.Integration.WebSocketChannelsTest do
         timeout: 200,
         connect_info: [:x_headers, :peer_data, :uri]
       ]
+
+    socket "/ws/connect_info_custom", UserSocketConnectInfo,
+      websocket: [
+        check_origin: ["//example.com"],
+        timeout: 200,
+        connect_info: [:x_headers, :peer_data, :uri, signing_salt: "salt"]
+      ]
+
   end
 
   setup_all do
@@ -245,6 +253,21 @@ defmodule Phoenix.Integration.WebSocketChannelsTest do
                                       "port" => 80}}}}
       end
 
+      test "transport custom keywords are extracted to the socket connect_info" do
+        {:ok, sock} =
+          WebsocketClient.start_link(
+            self(),
+            "ws://127.0.0.1:#{@port}/ws/connect_info_custom/websocket?vsn=#{@vsn}",
+            @serializer
+          )
+        WebsocketClient.join(sock, lobby(), %{})
+
+        assert_receive %Message{
+          event: "joined",
+          payload: %{"connect_info" => %{"signing_salt" => "salt"}}
+        }
+      end
+
       test "logs user socket connect when enabled" do
         log = capture_log(fn ->
           {:ok, _} = WebsocketClient.start_link(self(), "#{@vsn_path}&logging=enabled", @serializer)