Include notebook name in the autosaved notebook path (#748)

* Include notebook name in the autosaved notebook path

* Add test for persisting unsaved notebooks

lib/livebook/session.ex

@@ -71,7 +71,8 @@ defmodule Livebook.Session do
           created_at: DateTime.t(),
           runtime_monitor_ref: reference() | nil,
           autosave_timer_ref: reference() | nil,
-          save_task_pid: pid() | nil
+          save_task_pid: pid() | nil,
+          saved_default_file: FileSystem.File.t() | nil
         }
 
   @typedoc """
@@ -99,6 +100,9 @@ defmodule Livebook.Session do
 
     * `:images` - a map from image name to its binary content, an alternative
       to `:copy_images_from` when the images are in memory
+
+    * `:autosave_path` - a local directory to save notebooks without a file into.
+      Defaults to `Livebook.Config.autosave_path/1`
   """
   @spec start_link(keyword()) :: {:ok, pid} | {:error, any()}
   def start_link(opts) do
@@ -399,7 +403,9 @@ defmodule Livebook.Session do
         created_at: DateTime.utc_now(),
         runtime_monitor_ref: nil,
         autosave_timer_ref: nil,
-        save_task_pid: nil
+        autosave_path: opts[:autosave_path],
+        save_task_pid: nil,
+        saved_default_file: nil
       }
 
       {:ok, state}
@@ -688,9 +694,9 @@ defmodule Livebook.Session do
     {:noreply, handle_operation(state, operation)}
   end
 
-  def handle_info({:save_finished, pid, result}, %{save_task_pid: pid} = state) do
+  def handle_info({:save_finished, pid, result, file, default?}, %{save_task_pid: pid} = state) do
     state = %{state | save_task_pid: nil}
-    {:noreply, handle_save_finished(state, result)}
+    {:noreply, handle_save_finished(state, result, file, default?)}
   end
 
   def handle_info(_message, state), do: {:noreply, state}
@@ -973,7 +979,7 @@ defmodule Livebook.Session do
   end
 
   defp maybe_save_notebook_async(state) do
-    file = notebook_autosave_file(state)
+    {file, default?} = notebook_autosave_file(state)
 
     if file && should_save_notebook?(state) do
       pid = self()
@@ -982,7 +988,7 @@ defmodule Livebook.Session do
       {:ok, pid} =
         Task.start(fn ->
           result = FileSystem.File.write(file, content)
-          send(pid, {:save_finished, self(), result})
+          send(pid, {:save_finished, self(), result, file, default?})
         end)
 
       %{state | save_task_pid: pid}
@@ -992,12 +998,12 @@ defmodule Livebook.Session do
   end
 
   defp maybe_save_notebook_sync(state) do
-    file = notebook_autosave_file(state)
+    {file, default?} = notebook_autosave_file(state)
 
     if file && should_save_notebook?(state) do
       content = LiveMarkdown.Export.notebook_to_markdown(state.data.notebook)
       result = FileSystem.File.write(file, content)
-      handle_save_finished(state, result)
+      handle_save_finished(state, result, file, default?)
     else
       state
     end
@@ -1008,33 +1014,52 @@ defmodule Livebook.Session do
   end
 
   defp notebook_autosave_file(state) do
-    state.data.file || default_notebook_file(state)
+    file = state.data.file || default_notebook_file(state)
+    default? = state.data.file == nil
+    {file, default?}
   end
 
-  defp default_notebook_file(session) do
+  defp default_notebook_file(state) do
-    if path = Livebook.Config.autosave_path() do
+    if path = state.autosave_path || Livebook.Config.autosave_path() do
       dir = path |> FileSystem.Utils.ensure_dir_path() |> FileSystem.File.local()
-      notebook_rel_path = path_with_timestamp(session.session_id, session.created_at)
+      notebook_rel_path = default_notebook_path(state)
       FileSystem.File.resolve(dir, notebook_rel_path)
     end
   end
 
-  defp path_with_timestamp(session_id, date_time) do
+  defp default_notebook_path(state) do
+    title_str =
+      state.data.notebook.name
+      |> String.downcase()
+      |> String.replace(~r/\s+/, "_")
+      |> String.replace(~r/[^\w]/, "")
+
     # We want a random, but deterministic part, so we
-    # use a few characters from the session id, which
+    # use a few trailing characters from the session id,
-    # is random already
+    # which are random already
-    random_str = String.slice(session_id, 0..3)
+    random_str = String.slice(state.session_id, -4..-1)
 
     [date_str, time_str, _] =
-      date_time
+      state.created_at
       |> DateTime.to_iso8601()
       |> String.replace(["-", ":"], "_")
       |> String.split(["T", "."])
 
-    "#{date_str}/#{time_str}_#{random_str}.livemd"
+    "#{date_str}/#{time_str}_#{title_str}_#{random_str}.livemd"
   end
 
-  defp handle_save_finished(state, result) do
+  defp handle_save_finished(state, result, file, default?) do
+    state =
+      if default? do
+        if state.saved_default_file && state.saved_default_file != file do
+          FileSystem.File.remove(state.saved_default_file)
+        end
+
+        %{state | saved_default_file: file}
+      else
+        state
+      end
+
     case result do
       :ok ->
         handle_operation(state, {:mark_as_not_dirty, self()})

lib/livebook/session/data.ex

@@ -165,7 +165,7 @@ defmodule Livebook.Session.Data do
       notebook: notebook,
       origin: nil,
       file: nil,
-      dirty: false,
+      dirty: true,
       section_infos: initial_section_infos(notebook),
       cell_infos: initial_cell_infos(notebook),
       input_values: initial_input_values(notebook),

test/livebook/session_test.exs

@@ -583,6 +583,30 @@ defmodule Livebook.SessionTest do
     assert DateTime.compare(session.created_at, DateTime.utc_now()) == :lt
   end
 
+  @tag :tmp_dir
+  test "session without a file is persisted to autosave path", %{tmp_dir: tmp_dir} do
+    session = start_session(autosave_path: tmp_dir)
+
+    notebook_glob = Path.join(tmp_dir, "**/*.livemd")
+
+    Phoenix.PubSub.subscribe(Livebook.PubSub, "sessions:#{session.id}")
+
+    Session.save(session.pid)
+    assert_receive {:operation, {:mark_as_not_dirty, _}}
+
+    assert [notebook_path] = Path.wildcard(notebook_glob)
+    assert Path.basename(notebook_path) =~ "untitled_notebook"
+
+    # After the name is changed we should save to a different file
+    Session.set_notebook_name(session.pid, "Cat's guide to life")
+
+    Session.save(session.pid)
+    assert_receive {:operation, {:mark_as_not_dirty, _}}
+
+    assert [notebook_path] = Path.wildcard(notebook_glob)
+    assert Path.basename(notebook_path) =~ "cats_guide_to_life"
+  end
+
   defp start_session(opts \\ []) do
     session_id = Utils.random_id()
     {:ok, pid} = Session.start_link(Keyword.merge([id: session_id], opts))