From c1eafc9a2c52c7e5182345dcad3072870bdb40da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Sun, 21 Feb 2021 19:17:14 +0100 Subject: [PATCH] Save notebook and notify clients when session is stopped (#45) --- lib/live_book/session.ex | 18 ++++++++++++++---- lib/live_book/session_supervisor.ex | 4 ++-- lib/live_book_web/live/session_live.ex | 7 +++++++ test/live_book/session_supervisor_test.exs | 2 ++ test/live_book/session_test.exs | 22 ++++++++++++++++++++++ 5 files changed, 47 insertions(+), 6 deletions(-) diff --git a/lib/live_book/session.ex b/lib/live_book/session.ex index 9e22b787b..b414a508c 100644 --- a/lib/live_book/session.ex +++ b/lib/live_book/session.ex @@ -215,11 +215,14 @@ defmodule LiveBook.Session do end @doc """ - Synchronously stops the server. + Asynchronously sends a close request to the server. + + This results in saving the file and broadcasting + a :closed message to the session topic. """ - @spec stop(id()) :: :ok - def stop(session_id) do - GenServer.stop(name(session_id)) + @spec close(id()) :: :ok + def close(session_id) do + GenServer.cast(name(session_id), :close) end ## Callbacks @@ -377,6 +380,13 @@ defmodule LiveBook.Session do {:noreply, maybe_save_notebook(state)} end + def handle_cast(:close, state) do + maybe_save_notebook(state) + broadcast_message(state.session_id, :session_closed) + + {:stop, :shutdown, state} + end + @impl true def handle_info({:DOWN, ref, :process, _, _}, %{runtime_monitor_ref: ref} = state) do broadcast_info(state.session_id, "runtime node terminated unexpectedly") diff --git a/lib/live_book/session_supervisor.ex b/lib/live_book/session_supervisor.ex index 31b84f52a..4e41eb032 100644 --- a/lib/live_book/session_supervisor.ex +++ b/lib/live_book/session_supervisor.ex @@ -50,13 +50,13 @@ defmodule LiveBook.SessionSupervisor do end @doc """ - Synchronously stops a session process identified by the given id. + Asynchronously stops a session process identified by the given id. Broadcasts `{:session_delete, id}` message under the `"sessions"` topic. """ @spec delete_session(Session.id()) :: :ok def delete_session(id) do - Session.stop(id) + Session.close(id) broadcast_sessions_message({:session_deleted, id}) :ok end diff --git a/lib/live_book_web/live/session_live.ex b/lib/live_book_web/live/session_live.ex index d1019f1f5..0162aafd5 100644 --- a/lib/live_book_web/live/session_live.ex +++ b/lib/live_book_web/live/session_live.ex @@ -382,6 +382,13 @@ defmodule LiveBookWeb.SessionLive do {:noreply, put_flash(socket, :info, message)} end + def handle_info(:session_closed, socket) do + {:noreply, + socket + |> put_flash(:info, "Session has been closed") + |> push_redirect(to: Routes.home_path(socket, :page))} + end + def handle_info(_message, socket), do: {:noreply, socket} defp after_operation(socket, _prev_socket, {:insert_section, _index, section_id}) do diff --git a/test/live_book/session_supervisor_test.exs b/test/live_book/session_supervisor_test.exs index d40122b03..ba1e58d0e 100644 --- a/test/live_book/session_supervisor_test.exs +++ b/test/live_book/session_supervisor_test.exs @@ -23,9 +23,11 @@ defmodule LiveBook.SessionSupervisorTest do test "stops the session process identified by the given id" do {:ok, id} = SessionSupervisor.create_session() {:ok, pid} = SessionSupervisor.get_session_pid(id) + ref = Process.monitor(pid) SessionSupervisor.delete_session(id) + assert_receive {:DOWN, ^ref, :process, _, _} refute has_child_with_pid?(SessionSupervisor, pid) end diff --git a/test/live_book/session_test.exs b/test/live_book/session_test.exs index 29d30f5bc..616d5763b 100644 --- a/test/live_book/session_test.exs +++ b/test/live_book/session_test.exs @@ -187,6 +187,28 @@ defmodule LiveBook.SessionTest do end end + describe "close/1" do + @tag :tmp_dir + test "saves the notebook and notifies subscribers once the session is closed", + %{session_id: session_id, tmp_dir: tmp_dir} do + path = Path.join(tmp_dir, "notebook.livemd") + Session.set_path(session_id, path) + # Perform a change, so the notebook is dirty + Session.set_notebook_name(session_id, "My notebook") + + Phoenix.PubSub.subscribe(LiveBook.PubSub, "sessions:#{session_id}") + + refute File.exists?(path) + + Process.flag(:trap_exit, true) + Session.close(session_id) + + assert_receive :session_closed + assert File.exists?(path) + assert File.read!(path) =~ "My notebook" + end + end + describe "start_link/1" do @tag :tmp_dir test "fails if the given path is already in use", %{tmp_dir: tmp_dir} do