From 4b5ea87b3dd6e99937d2304f7b12eaf9dc44429d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Sun, 5 Dec 2021 14:58:19 +0100 Subject: [PATCH] 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 | 63 ++++++++++++++++++++++++---------- lib/livebook/session/data.ex | 2 +- test/livebook/session_test.exs | 24 +++++++++++++ 3 files changed, 69 insertions(+), 20 deletions(-) diff --git a/lib/livebook/session.ex b/lib/livebook/session.ex index 5020fcffd..74d5103f7 100644 --- a/lib/livebook/session.ex +++ b/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 - if path = Livebook.Config.autosave_path() do + defp default_notebook_file(state) 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 - # is random already - random_str = String.slice(session_id, 0..3) + # use a few trailing characters from the session id, + # which are random already + 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()}) diff --git a/lib/livebook/session/data.ex b/lib/livebook/session/data.ex index 84cb86254..ad8b6fd96 100644 --- a/lib/livebook/session/data.ex +++ b/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), diff --git a/test/livebook/session_test.exs b/test/livebook/session_test.exs index 6d0581b6a..db2cb4dca 100644 --- a/test/livebook/session_test.exs +++ b/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))