Minimize lifespan of param-driven assigns (#2509)

lib/livebook_web/live/session_live.ex

@@ -83,8 +83,7 @@ defmodule LivebookWeb.SessionLive do
            data_view: data_to_view(data),
            autofocus_cell_id: autofocus_cell_id(data.notebook),
            page_title: get_page_title(data.notebook.name),
-           select_secret_ref: nil,
+           action_assigns: %{},
-           select_secret_options: nil,
            allowed_uri_schemes: Livebook.Config.allowed_uri_schemes(),
            starred_files: Livebook.NotebookManager.starred_notebooks() |> starred_files()
          )
@@ -127,35 +126,62 @@ defmodule LivebookWeb.SessionLive do
   defdelegate render(assigns), to: LivebookWeb.SessionLive.Render
 
   @impl true
-  def handle_params(%{"cell_id" => cell_id}, _url, socket)
+  def handle_params(params, url, socket) do
-      when socket.assigns.live_action == :cell_settings do
+    {socket, action_assigns} = handle_params(socket.assigns.live_action, params, url, socket)
+    socket = assign(socket, :action_assigns, action_assigns)
+    {:noreply, socket}
+  end
+
+  defp handle_params(:cell_settings, %{"cell_id" => cell_id}, _url, socket) do
     {:ok, cell, _} = Notebook.fetch_cell_and_section(socket.private.data.notebook, cell_id)
-    {:noreply, assign(socket, cell: cell)}
+    {socket, %{cell: cell}}
   end
 
-  def handle_params(%{}, _url, socket)
+  defp handle_params(:insert_image, %{}, _url, socket) do
-      when socket.assigns.live_action == :insert_image and
+    case pop_in(socket.assigns[:insert_image_metadata]) do
-             not is_map_key(socket.assigns, :insert_image_metadata) do
+      {nil, socket} -> {redirect_to_self(socket), %{}}
-    {:noreply, redirect_to_self(socket)}
+      {metadata, socket} -> {socket, %{insert_image_metadata: metadata}}
-  end
-
-  def handle_params(%{}, _url, socket)
-      when socket.assigns.live_action == :insert_file and
-             not is_map_key(socket.assigns, :insert_file_metadata) do
-    {:noreply, redirect_to_self(socket)}
-  end
-
-  def handle_params(%{"name" => name}, _url, socket)
-      when socket.assigns.live_action == :rename_file_entry do
-    if file_entry = find_file_entry(socket, name) do
-      {:noreply, assign(socket, renaming_file_entry: file_entry)}
-    else
-      {:noreply, redirect_to_self(socket)}
     end
   end
 
-  def handle_params(%{"path_parts" => path_parts}, requested_url, socket)
+  defp handle_params(:insert_file, %{}, _url, socket) do
-      when socket.assigns.live_action == :catch_all do
+    case pop_in(socket.assigns[:insert_file_metadata]) do
+      {nil, socket} -> {redirect_to_self(socket), %{}}
+      {metadata, socket} -> {socket, %{insert_file_metadata: metadata}}
+    end
+  end
+
+  defp handle_params(:rename_file_entry, %{"name" => name}, _url, socket) do
+    if file_entry = find_file_entry(socket, name) do
+      {socket, %{renaming_file_entry: file_entry}}
+    else
+      {redirect_to_self(socket), %{}}
+    end
+  end
+
+  defp handle_params(:export, %{"tab" => tab}, _url, socket) do
+    any_stale_cell? = any_stale_cell?(socket.private.data)
+    {socket, %{tab: tab, any_stale_cell?: any_stale_cell?}}
+  end
+
+  defp handle_params(:add_file_entry, %{"tab" => tab}, _url, socket) do
+    {file_drop_metadata, socket} = pop_in(socket.assigns[:file_drop_metadata])
+    {socket, %{tab: tab, file_drop_metadata: file_drop_metadata}}
+  end
+
+  defp handle_params(:secrets, params, _url, socket) do
+    {select_secret_metadata, socket} = pop_in(socket.assigns[:select_secret_metadata])
+
+    {socket,
+     %{select_secret_metadata: select_secret_metadata, prefill_secret_name: params["secret_name"]}}
+  end
+
+  defp handle_params(live_action, params, _url, socket)
+       when live_action in [:app_settings, :file_settings] do
+    {socket, %{context: params["context"]}}
+  end
+
+  defp handle_params(:catch_all, %{"path_parts" => path_parts}, requested_url, socket) do
     path_parts =
       Enum.map(path_parts, fn
         "__parent__" -> ".."
@@ -163,46 +189,12 @@ defmodule LivebookWeb.SessionLive do
       end)
 
     path = Path.join(path_parts)
-    {:noreply, handle_relative_path(socket, path, requested_url)}
+    socket = handle_relative_path(socket, path, requested_url)
+    {socket, %{}}
   end
 
-  def handle_params(%{"tab" => tab}, _url, socket) when socket.assigns.live_action == :export do
+  defp handle_params(_live_action, _params, _url, socket) do
-    any_stale_cell? = any_stale_cell?(socket.private.data)
+    {socket, %{}}
-    {:noreply, assign(socket, tab: tab, any_stale_cell?: any_stale_cell?)}
-  end
-
-  def handle_params(%{"tab" => tab} = params, _url, socket)
-      when socket.assigns.live_action == :add_file_entry do
-    file_drop_metadata =
-      if(params["file_drop"] == "true", do: socket.assigns[:file_drop_metadata])
-
-    {:noreply, assign(socket, tab: tab, file_drop_metadata: file_drop_metadata)}
-  end
-
-  def handle_params(params, _url, socket)
-      when socket.assigns.live_action == :secrets do
-    socket =
-      if params["preselect_name"] do
-        assign(socket, prefill_secret_name: params["preselect_name"])
-      else
-        # Erase any previously stored reference
-        assign(socket,
-          prefill_secret_name: params["secret_name"],
-          select_secret_ref: nil,
-          select_secret_options: nil
-        )
-      end
-
-    {:noreply, socket}
-  end
-
-  def handle_params(params, _url, socket)
-      when socket.assigns.live_action in [:app_settings, :file_settings] do
-    {:noreply, assign(socket, route_params: Map.take(params, ["context"]))}
-  end
-
-  def handle_params(_params, _url, socket) do
-    {:noreply, socket}
   end
 
   @impl true
@@ -695,13 +687,15 @@ defmodule LivebookWeb.SessionLive do
       ) do
     socket =
       assign(socket,
-        select_secret_ref: select_secret_ref,
+        select_secret_metadata: %{
-        select_secret_options: select_secret_options
+          ref: select_secret_ref,
+          options: select_secret_options
+        }
       )
 
     {:noreply,
      push_patch(socket,
-       to: ~p"/sessions/#{socket.assigns.session.id}/secrets?preselect_name=#{preselect_name}"
+       to: ~p"/sessions/#{socket.assigns.session.id}/secrets?secret_name=#{preselect_name}"
      )}
   end
 
@@ -792,7 +786,7 @@ defmodule LivebookWeb.SessionLive do
 
   def handle_event("insert_file_action", %{"idx" => idx}, socket) do
     %{section_id: section_id, cell_id: cell_id, file_entry: file_entry, handlers: handlers} =
-      socket.assigns.insert_file_metadata
+      socket.assigns.action_assigns.insert_file_metadata
 
     handler = Enum.fetch!(handlers, idx)
     source = String.replace(handler.definition.source, "{{NAME}}", file_entry.name)
@@ -834,9 +828,7 @@ defmodule LivebookWeb.SessionLive do
       {:noreply,
        socket
        |> assign(file_drop_metadata: %{section_id: section_id, cell_id: cell_id})
-       |> push_patch(
+       |> push_patch(to: ~p"/sessions/#{socket.assigns.session.id}/add-file/upload")
-         to: ~p"/sessions/#{socket.assigns.session.id}/add-file/upload?file_drop=true"
-       )
        |> push_event("finish_file_drop", %{})}
     else
       reason = "To see the available options, you need a connected runtime."
@@ -847,8 +839,7 @@ defmodule LivebookWeb.SessionLive do
   def handle_event("handle_file_drop", %{}, socket) do
     {:noreply,
      socket
-     |> assign(file_drop_metadata: nil)
+     |> push_patch(to: ~p"/sessions/#{socket.assigns.session.id}/add-file/upload")
-     |> push_patch(to: ~p"/sessions/#{socket.assigns.session.id}/add-file/upload?file_drop=true")
      |> push_event("finish_file_drop", %{})}
   end
 
@@ -1004,7 +995,7 @@ defmodule LivebookWeb.SessionLive do
   end
 
   def handle_info({:file_entry_uploaded, file_entry}, socket) do
-    case socket.assigns.file_drop_metadata do
+    case socket.assigns.action_assigns[:file_drop_metadata] do
       %{section_id: section_id, cell_id: cell_id} ->
         {:noreply,
          socket
@@ -1019,7 +1010,7 @@ defmodule LivebookWeb.SessionLive do
          |> push_patch(to: ~p"/sessions/#{socket.assigns.session.id}/insert-file")}
 
       nil ->
-        {:noreply, socket}
+        {:noreply, push_patch(socket, to: ~p"/sessions/#{socket.assigns.session.id}")}
     end
   end
 

lib/livebook_web/live/session_live/add_file_entry_upload_component.ex

@@ -120,7 +120,7 @@ defmodule LivebookWeb.SessionLive.AddFileEntryUploadComponent do
         file_entry = %{name: data.name, type: :attachment}
         Livebook.Session.add_file_entries(socket.assigns.session.pid, [file_entry])
         send(self(), {:file_entry_uploaded, file_entry})
-        {:noreply, push_patch(socket, to: ~p"/sessions/#{socket.assigns.session.id}")}
+        {:noreply, socket}
 
       {:error, changeset} ->
         {:noreply, assign(socket, changeset: changeset)}

lib/livebook_web/live/session_live/render.ex

@@ -72,7 +72,7 @@ defmodule LivebookWeb.SessionLive.Render do
         session={@session}
         file={@data_view.file}
         hub={@data_view.hub}
-        context={@route_params["context"]}
+        context={@action_assigns.context}
         persist_outputs={@data_view.persist_outputs}
         autosave_interval_s={@data_view.autosave_interval_s}
       />
@@ -90,7 +90,7 @@ defmodule LivebookWeb.SessionLive.Render do
         id="app-settings"
         session={@session}
         settings={@data_view.app_settings}
-        context={@route_params["context"]}
+        context={@action_assigns.context}
         deployed_app_slug={@data_view.deployed_app_slug}
       />
     </.modal>
@@ -127,7 +127,7 @@ defmodule LivebookWeb.SessionLive.Render do
         session={@session}
         hub={@data_view.hub}
         file_entries={@data_view.file_entries}
-        tab={@tab}
+        tab={@action_assigns.tab}
       />
     </.modal>
 
@@ -142,7 +142,7 @@ defmodule LivebookWeb.SessionLive.Render do
         module={LivebookWeb.SessionLive.RenameFileEntryComponent}
         id="rename-file-entry"
         session={@session}
-        file_entry={@renaming_file_entry}
+        file_entry={@action_assigns.renaming_file_entry}
       />
     </.modal>
 
@@ -168,11 +168,11 @@ defmodule LivebookWeb.SessionLive.Render do
       patch={@self_path}
     >
       <.live_component
-        module={settings_component_for(@cell)}
+        module={settings_component_for(@action_assigns.cell)}
         id="cell-settings"
         session={@session}
         return_to={@self_path}
-        cell={@cell}
+        cell={@action_assigns.cell}
       />
     </.modal>
 
@@ -188,7 +188,7 @@ defmodule LivebookWeb.SessionLive.Render do
         id="insert-image"
         session={@session}
         return_to={@self_path}
-        insert_image_metadata={@insert_image_metadata}
+        insert_image_metadata={@action_assigns.insert_image_metadata}
       />
     </.modal>
 
@@ -204,7 +204,7 @@ defmodule LivebookWeb.SessionLive.Render do
         id="insert-file"
         session={@session}
         return_to={@self_path}
-        insert_file_metadata={@insert_file_metadata}
+        insert_file_metadata={@action_assigns.insert_file_metadata}
       />
     </.modal>
 
@@ -223,8 +223,8 @@ defmodule LivebookWeb.SessionLive.Render do
         module={LivebookWeb.SessionLive.ExportComponent}
         id="export"
         session={@session}
-        tab={@tab}
+        tab={@action_assigns.tab}
-        any_stale_cell?={@any_stale_cell?}
+        any_stale_cell?={@action_assigns.any_stale_cell?}
       />
     </.modal>
 
@@ -248,7 +248,7 @@ defmodule LivebookWeb.SessionLive.Render do
       :if={@live_action == :secrets}
       id="secrets-modal"
       show
-      width={if(@select_secret_ref, do: :large, else: :medium)}
+      width={if(@action_assigns.select_secret_metadata, do: :large, else: :medium)}
       patch={@self_path}
     >
       <.live_component
@@ -258,9 +258,8 @@ defmodule LivebookWeb.SessionLive.Render do
         secrets={@data_view.secrets}
         hub_secrets={@data_view.hub_secrets}
         hub={@data_view.hub}
-        prefill_secret_name={@prefill_secret_name}
+        select_secret_metadata={@action_assigns.select_secret_metadata}
-        select_secret_ref={@select_secret_ref}
+        prefill_secret_name={@action_assigns.prefill_secret_name}
-        select_secret_options={@select_secret_options}
         return_to={@self_path}
       />
     </.modal>

lib/livebook_web/live/session_live/secrets_component.ex

@@ -15,7 +15,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
   def update(assigns, socket) do
     socket = assign(socket, assigns)
 
-    secret_name = socket.assigns[:prefill_secret_name]
+    secret_name = socket.assigns.prefill_secret_name
 
     socket =
       socket
@@ -48,7 +48,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
         hub={@hub}
       />
       <div class="flex flex-columns gap-4">
-        <div :if={@select_secret_ref} class="basis-1/2 grow-0 pr-4 border-r">
+        <div :if={@select_secret_metadata} class="basis-1/2 grow-0 pr-4 border-r">
           <div class="flex flex-col space-y-4">
             <p class="text-gray-800">
               Choose a secret
@@ -98,7 +98,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
           class="basis-1/2 grow"
         >
           <div class="flex flex-col space-y-4">
-            <p :if={@select_secret_ref} class="text-gray-700">
+            <p :if={@select_secret_metadata} class="text-gray-700">
               Add new secret
             </p>
             <.text_field
@@ -258,14 +258,17 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
      |> push_secret_selected(secret_name)}
   end
 
-  defp push_secret_selected(%{assigns: %{select_secret_ref: nil}} = socket, _), do: socket
+  defp push_secret_selected(%{assigns: %{select_secret_metadata: nil}} = socket, _), do: socket
 
-  defp push_secret_selected(%{assigns: %{select_secret_ref: ref}} = socket, secret_name) do
+  defp push_secret_selected(
+         %{assigns: %{select_secret_metadata: %{ref: ref}}} = socket,
+         secret_name
+       ) do
     push_event(socket, "secret_selected", %{select_secret_ref: ref, secret_name: secret_name})
   end
 
-  defp title(%{assigns: %{select_secret_ref: nil}}), do: "Add secret"
+  defp title(%{assigns: %{select_secret_metadata: nil}}), do: "Add secret"
-  defp title(%{assigns: %{select_secret_options: %{"title" => title}}}), do: title
+  defp title(%{assigns: %{select_secret_metadata: %{options: %{"title" => title}}}}), do: title
   defp title(_), do: "Select secret"
 
   defp set_secret(socket, %Secret{hub_id: nil} = secret) do