Repository: phoenixframework/phoenix
Pull request #4950 (2022-09-16 15:04:53)

Add @flash assign


Patch:
diff --git a/lib/phoenix/controller.ex b/lib/phoenix/controller.ex
index d1a096f48..a424c074e 100644
--- a/lib/phoenix/controller.ex
+++ b/lib/phoenix/controller.ex
@@ -1328,14 +1328,14 @@ defmodule Phoenix.Controller do
   Fetches the flash storage.
   """
   def fetch_flash(conn, _opts \\ []) do
-    if Map.get(conn.private, :phoenix_flash) do
+    if Map.get(conn.assigns, :flash) do
       conn
     else
       session_flash = get_session(conn, "phoenix_flash")
       conn = persist_flash(conn, session_flash || %{})
 
       register_before_send conn, fn conn ->
-        flash = conn.private.phoenix_flash
+        flash = conn.assigns.flash
         flash_size = map_size(flash)
 
         cond do
@@ -1396,8 +1396,9 @@ defmodule Phoenix.Controller do
       %{"info" => "Welcome Back!"}
 
   """
+  @deprecated "get_flash/1 is deprecated. Use the @flash assign provided by the :fetch_flash plug"
   def get_flash(conn) do
-    Map.get(conn.private, :phoenix_flash) ||
+    Map.get(conn.assigns, :flash) ||
       raise ArgumentError, message: "flash not fetched, call fetch_flash/2"
   end
 
@@ -1411,6 +1412,7 @@ defmodule Phoenix.Controller do
       "Welcome Back!"
 
   """
+  @deprecated "get_flash/2 is deprecated. Use Phoenix.Flash.get(@flash, key) instead"
   def get_flash(conn, key) do
     get_flash(conn)[flash_key(key)]
   end
@@ -1447,7 +1449,7 @@ defmodule Phoenix.Controller do
   defp flash_key(atom) when is_atom(atom), do: Atom.to_string(atom)
 
   defp persist_flash(conn, value) do
-    put_private(conn, :phoenix_flash, value)
+    assign(conn, :flash, value)
   end
 
   @doc """
diff --git a/lib/phoenix/flash.ex b/lib/phoenix/flash.ex
new file mode 100644
index 000000000..794153fad
--- /dev/null
+++ b/lib/phoenix/flash.ex
@@ -0,0 +1,27 @@
+defmodule Phoenix.Flash do
+  @moduledoc """
+  Provides shared flash access.
+  """
+
+  @doc """
+  Gets the key from the map of flash data.
+
+  ## Examples
+
+      <div id="info"><%= Phoenix.Flash.get(@flash, :info) %></div>
+      <div id="error"><%= Phoenix.Flash.get(@flash, :error) %></div>
+  """
+  def get(%mod{}, key) when is_atom(key) or is_binary(key) do
+    raise ArgumentError, """
+    expected a map of flash data, but got a %#{inspect(mod)}{}
+
+    Use the @flash assign set by the :fetch_flash plug instead:
+
+        <%= Phoenix.Flash.get(@flash, :#{key}) %>
+    """
+  end
+
+  def get(%{} = flash, key) when is_atom(key) or is_binary(key) do
+    Map.get(flash, to_string(key))
+  end
+end


Test patch:
diff --git a/lib/phoenix/test/conn_test.ex b/lib/phoenix/test/conn_test.ex
index 7aa5d5f3c..df7c3b7f4 100644
--- a/lib/phoenix/test/conn_test.ex
+++ b/lib/phoenix/test/conn_test.ex
@@ -274,13 +274,17 @@ defmodule Phoenix.ConnTest do
   Gets the whole flash storage.
   """
   @spec get_flash(Conn.t) :: map
-  defdelegate get_flash(conn), to: Phoenix.Controller
+  @deprecated "get_flash/1 is deprecated. Use conn.assigns.flash instead"
+  def get_flash(conn), do: conn.assigns.flash
 
   @doc """
   Gets the given key from the flash storage.
   """
   @spec get_flash(Conn.t, term) :: term
-  defdelegate get_flash(conn, key), to: Phoenix.Controller
+  @deprecated "get_flash/2 is deprecated. Use Phoenix.Flash.get/2 instead"
+  def get_flash(conn, key) do
+    Phoenix.Flash.get(conn.assigns.flash, key)
+  end
 
   @doc """
   Puts the given value under key in the flash storage.
diff --git a/test/phoenix/controller/flash_test.exs b/test/phoenix/controller/flash_test.exs
index 302f13a32..210214e80 100644
--- a/test/phoenix/controller/flash_test.exs
+++ b/test/phoenix/controller/flash_test.exs
@@ -4,6 +4,8 @@ defmodule Phoenix.Controller.FlashTest do
 
   import Phoenix.Controller
 
+  alias Phoenix.Flash
+
   setup do
     Logger.disable(self())
     :ok
@@ -24,23 +26,27 @@ defmodule Phoenix.Controller.FlashTest do
   end
 
   test "does not fetch flash twice" do
-    assert conn(:get, "/")
-           |> with_session()
-           |> put_session("phoenix_flash", %{"foo" => "bar"})
-           |> fetch_flash()
-           |> put_session("phoenix_flash", %{"foo" => "baz"})
-           |> fetch_flash()
-           |> get_flash() == %{"foo" => "bar"}
+    expected_flash = %{"foo" => "bar"}
+    conn =
+      conn(:get, "/")
+      |> with_session()
+      |> put_session("phoenix_flash", expected_flash)
+      |> fetch_flash()
+      |> put_session("phoenix_flash", %{"foo" => "baz"})
+      |> fetch_flash()
+
+    assert conn.assigns.flash == expected_flash
+    assert conn.assigns.flash == expected_flash
   end
 
   test "flash is persisted when status is a redirect" do
     for status <- 300..308 do
       conn = conn(:get, "/") |> with_session |> fetch_flash()
                              |> put_flash(:notice, "elixir") |> send_resp(status, "ok")
-      assert get_flash(conn, :notice) == "elixir"
+      assert Flash.get(conn.assigns.flash, :notice) == "elixir"
       assert get_resp_header(conn, "set-cookie") != []
       conn = conn(:get, "/") |> recycle_cookies(conn) |> with_session |> fetch_flash()
-      assert get_flash(conn, :notice) == "elixir"
+      assert Flash.get(conn.assigns.flash, :notice) == "elixir"
     end
   end
 
@@ -48,10 +54,10 @@ defmodule Phoenix.Controller.FlashTest do
     for status <- [299, 309, 200, 404] do
       conn = conn(:get, "/") |> with_session |> fetch_flash()
                              |> put_flash(:notice, "elixir") |> send_resp(status, "ok")
-      assert get_flash(conn, :notice) == "elixir"
+      assert Flash.get(conn.assigns.flash, :notice) == "elixir"
       assert get_resp_header(conn, "set-cookie") != []
       conn = conn(:get, "/") |> recycle_cookies(conn) |> with_session |> fetch_flash()
-      assert get_flash(conn, :notice) == nil
+      assert Flash.get(conn.assigns.flash, :notice) == nil
     end
   end
 
@@ -85,27 +91,21 @@ defmodule Phoenix.Controller.FlashTest do
     assert ["_app=" <> _] = get_resp_header(conn, "set-cookie")
   end
 
-  test "get_flash/1 raises ArgumentError when flash not previously fetched" do
-    assert_raise ArgumentError, fn ->
-      conn(:get, "/") |> with_session |> get_flash()
-    end
-  end
-
-  test "get_flash/1 returns the map of messages" do
+  test "flash assigns contains the map of messages" do
     conn = conn(:get, "/") |> with_session |> fetch_flash([]) |> put_flash(:notice, "hi")
-    assert get_flash(conn) == %{"notice" => "hi"}
+    assert conn.assigns.flash == %{"notice" => "hi"}
   end
 
-  test "get_flash/2 returns the message by key" do
+  test "Flash.get/2 returns the message by key" do
     conn = conn(:get, "/") |> with_session |> fetch_flash([]) |> put_flash(:notice, "hi")
-    assert get_flash(conn, :notice) == "hi"
-    assert get_flash(conn, "notice") == "hi"
+    assert Flash.get(conn.assigns.flash, :notice) == "hi"
+    assert Flash.get(conn.assigns.flash, "notice") == "hi"
   end
 
-  test "get_flash/2 returns nil for missing key" do
+  test "Flash.get/2 returns nil for missing key" do
     conn = conn(:get, "/") |> with_session |> fetch_flash([])
-    assert get_flash(conn, :notice) == nil
-    assert get_flash(conn, "notice") == nil
+    assert Flash.get(conn.assigns.flash, :notice) == nil
+    assert Flash.get(conn.assigns.flash, "notice") == nil
   end
 
   test "put_flash/3 raises ArgumentError when flash not previously fetched" do
@@ -114,18 +114,24 @@ defmodule Phoenix.Controller.FlashTest do
     end
   end
 
-  test "put_flash/3 adds the key/message pair to the flash" do
+  test "put_flash/3 adds the key/message pair to the flash and updates assigns" do
     conn =
       conn(:get, "/")
       |> with_session
       |> fetch_flash([])
+
+    assert conn.assigns.flash == %{}
+
+    conn =
+      conn
       |> put_flash(:error, "oh noes!")
       |> put_flash(:notice, "false alarm!")
 
-    assert get_flash(conn, :error) == "oh noes!"
-    assert get_flash(conn, "error") == "oh noes!"
-    assert get_flash(conn, :notice) == "false alarm!"
-    assert get_flash(conn, "notice") == "false alarm!"
+    assert conn.assigns.flash == %{"error" => "oh noes!", "notice" => "false alarm!"}
+    assert Flash.get(conn.assigns.flash, :error) == "oh noes!"
+    assert Flash.get(conn.assigns.flash, "error") == "oh noes!"
+    assert Flash.get(conn.assigns.flash, :notice) == "false alarm!"
+    assert Flash.get(conn.assigns.flash, "notice") == "false alarm!"
   end
 
   test "clear_flash/1 clears the flash messages" do
@@ -136,9 +142,9 @@ defmodule Phoenix.Controller.FlashTest do
       |> put_flash(:error, "oh noes!")
       |> put_flash(:notice, "false alarm!")
 
-    refute get_flash(conn) == %{}
+    refute conn.assigns.flash == %{}
     conn = clear_flash(conn)
-    assert get_flash(conn) == %{}
+    assert conn.assigns.flash == %{}
   end
 
   test "merge_flash/2 adds kv-pairs to the flash" do
@@ -148,10 +154,10 @@ defmodule Phoenix.Controller.FlashTest do
       |> fetch_flash([])
       |> merge_flash(error: "oh noes!", notice: "false alarm!")
 
-    assert get_flash(conn, :error) == "oh noes!"
-    assert get_flash(conn, "error") == "oh noes!"
-    assert get_flash(conn, :notice) == "false alarm!"
-    assert get_flash(conn, "notice") == "false alarm!"
+    assert Flash.get(conn.assigns.flash, :error) == "oh noes!"
+    assert Flash.get(conn.assigns.flash, "error") == "oh noes!"
+    assert Flash.get(conn.assigns.flash, :notice) == "false alarm!"
+    assert Flash.get(conn.assigns.flash, "notice") == "false alarm!"
   end
 
   test "fetch_flash/2 raises ArgumentError when session not previously fetched" do
@@ -159,4 +165,18 @@ defmodule Phoenix.Controller.FlashTest do
       conn(:get, "/") |> fetch_flash([])
     end
   end
+
+  describe "Flash" do
+    test "get/2" do
+      assert Flash.get(%{}, :info) == nil
+      assert Flash.get(%{"info" => "hi"}, :info) == "hi"
+      assert Flash.get(%{"info" => "hi", "error" => "ohno"}, :error) == "ohno"
+    end
+
+    test "invalid access" do
+      assert_raise ArgumentError, ~r/expected a map of flash data, but got a %Plug.Conn{}/, fn ->
+        Flash.get(%Plug.Conn{}, :info)
+      end
+    end
+  end
 end