From 66591aa69aa2a2b212325e936fb2eea8f601dea4 Mon Sep 17 00:00:00 2001 From: Scott Ming Date: Sat, 25 May 2024 15:08:30 +0800 Subject: [PATCH] Try integrate proxy instead of multiple `maybe_xxx` method --- .../lib/lexical/remote_control.ex | 4 ++ .../lib/lexical/remote_control/api.ex | 17 ++----- .../lib/lexical/remote_control/application.ex | 1 - .../lib/lexical/remote_control/build.ex | 17 ------- .../lexical/remote_control/code_mod/format.ex | 12 +---- .../lexical/remote_control/code_mod/rename.ex | 5 +- .../lexical/remote_control/commands/rename.ex | 50 +++++++++++-------- .../remote_control/commands/rename_test.exs | 31 ++++++++---- apps/server/lib/lexical/server/state.ex | 6 ++- 9 files changed, 66 insertions(+), 77 deletions(-) diff --git a/apps/remote_control/lib/lexical/remote_control.ex b/apps/remote_control/lib/lexical/remote_control.ex index e8b776b25..ca4990aea 100644 --- a/apps/remote_control/lib/lexical/remote_control.ex +++ b/apps/remote_control/lib/lexical/remote_control.ex @@ -62,6 +62,10 @@ defmodule Lexical.RemoteControl do defdelegate workspace_symbols(query), to: CodeIntelligence.Symbols, as: :for_workspace + defdelegate maybe_update_rename_progress(triggered_message), + to: RemoteControl.Commands.Rename, + as: :update_progress + def start_link(%Project{} = project) do :ok = ensure_epmd_started() start_net_kernel(project) diff --git a/apps/remote_control/lib/lexical/remote_control/api.ex b/apps/remote_control/lib/lexical/remote_control/api.ex index 76a1ee6c8..01857b564 100644 --- a/apps/remote_control/lib/lexical/remote_control/api.ex +++ b/apps/remote_control/lib/lexical/remote_control/api.ex @@ -7,6 +7,7 @@ defmodule Lexical.RemoteControl.Api do alias Lexical.Project alias Lexical.RemoteControl alias Lexical.RemoteControl.CodeIntelligence + alias Lexical.RemoteControl.CodeMod require Logger @@ -18,18 +19,6 @@ defmodule Lexical.RemoteControl.Api do RemoteControl.call(project, RemoteControl, :compile_document, [document]) end - def maybe_schedule_compile(project, triggered_message) do - RemoteControl.call(project, Build, :maybe_schedule_compile, [triggered_message]) - end - - def maybe_compile_document(project, document, updated_message) do - RemoteControl.call(project, Build, :maybe_compile_document, [ - project, - document, - updated_message - ]) - end - def expand_alias( %Project{} = project, segments_or_module, @@ -89,6 +78,10 @@ defmodule Lexical.RemoteControl.Api do ]) end + def maybe_update_rename_progress(project, updated_message) do + RemoteControl.call(project, RemoteControl, :maybe_update_rename_progress, [updated_message]) + end + def complete(%Project{} = project, %Env{} = env) do Logger.info("Completion for #{inspect(env.position)}") RemoteControl.call(project, RemoteControl, :complete, [env]) diff --git a/apps/remote_control/lib/lexical/remote_control/application.ex b/apps/remote_control/lib/lexical/remote_control/application.ex index e52b684d0..d44109a92 100644 --- a/apps/remote_control/lib/lexical/remote_control/application.ex +++ b/apps/remote_control/lib/lexical/remote_control/application.ex @@ -15,7 +15,6 @@ defmodule Lexical.RemoteControl.Application do [ RemoteControl.Api.Proxy, {RemoteControl.Commands.Reindex, nil}, - RemoteControl.Commands.Rename, RemoteControl.Module.Loader, {RemoteControl.Dispatch, progress: true}, RemoteControl.ModuleMappings, diff --git a/apps/remote_control/lib/lexical/remote_control/build.ex b/apps/remote_control/lib/lexical/remote_control/build.ex index 6160a011c..2d01d61fb 100644 --- a/apps/remote_control/lib/lexical/remote_control/build.ex +++ b/apps/remote_control/lib/lexical/remote_control/build.ex @@ -4,7 +4,6 @@ defmodule Lexical.RemoteControl.Build do alias Lexical.RemoteControl alias Lexical.RemoteControl.Build.Document.Compilers.HEEx alias Lexical.RemoteControl.Build.State - alias Lexical.RemoteControl.Commands.Rename alias Lexical.VM.Versions require Logger @@ -36,22 +35,6 @@ defmodule Lexical.RemoteControl.Build do :ok end - def maybe_schedule_compile(message) do - if Rename.in_progress?() do - Rename.update_progress(message) - else - GenServer.cast(__MODULE__, {:compile, false}) - end - end - - def maybe_compile_document(%Project{} = project, %Document{} = document, message) do - if Rename.in_progress?() do - Rename.update_progress(message) - else - compile_document(project, document) - end - end - # this is for testing def force_compile_document(%Project{} = project, %Document{} = document) do with false <- Path.absname(document.path) == "mix.exs", diff --git a/apps/remote_control/lib/lexical/remote_control/code_mod/format.ex b/apps/remote_control/lib/lexical/remote_control/code_mod/format.ex index 270a5b97b..287ac5c31 100644 --- a/apps/remote_control/lib/lexical/remote_control/code_mod/format.ex +++ b/apps/remote_control/lib/lexical/remote_control/code_mod/format.ex @@ -5,7 +5,6 @@ defmodule Lexical.RemoteControl.CodeMod.Format do alias Lexical.RemoteControl alias Lexical.RemoteControl.Build alias Lexical.RemoteControl.CodeMod.Diff - alias Lexical.RemoteControl.Commands.Rename require Logger @@ -100,22 +99,13 @@ defmodule Lexical.RemoteControl.CodeMod.Format do def edits(%Document{} = document) do project = RemoteControl.get_project() - with :ok <- ensure_not_renaming(), - :ok <- Build.compile_document(project, document), + with :ok <- Build.compile_document(project, document), {:ok, formatted} <- do_format(project, document) do edits = Diff.diff(document, formatted) {:ok, Changes.new(document, edits)} end end - defp ensure_not_renaming do - if Rename.in_progress?() do - {:error, :rename_in_progress} - else - :ok - end - end - defp do_format(%Project{} = project, %Document{} = document) do project_path = Project.project_path(project) diff --git a/apps/remote_control/lib/lexical/remote_control/code_mod/rename.ex b/apps/remote_control/lib/lexical/remote_control/code_mod/rename.ex index 153d84283..5e9f23e50 100644 --- a/apps/remote_control/lib/lexical/remote_control/code_mod/rename.ex +++ b/apps/remote_control/lib/lexical/remote_control/code_mod/rename.ex @@ -34,10 +34,7 @@ defmodule Lexical.RemoteControl.CodeMod.Rename do progress_notification_functions = Progress.begin_percent("Renaming", Enum.count(uri_with_expected_operation)) - Commands.Rename.set_rename_progress( - uri_with_expected_operation, - progress_notification_functions - ) + Commands.Rename.start_link([uri_with_expected_operation, progress_notification_functions]) end defp uri_with_expected_operation(client_name, document_changes_list) diff --git a/apps/remote_control/lib/lexical/remote_control/commands/rename.ex b/apps/remote_control/lib/lexical/remote_control/commands/rename.ex index c3db66288..3c1288b0b 100644 --- a/apps/remote_control/lib/lexical/remote_control/commands/rename.ex +++ b/apps/remote_control/lib/lexical/remote_control/commands/rename.ex @@ -60,30 +60,17 @@ defmodule Lexical.RemoteControl.Commands.Rename do end end + alias Lexical.RemoteControl.Api.Proxy use GenServer - def start_link(_) do - GenServer.start_link(__MODULE__, %State{}, name: __MODULE__) + def start_link([uri_with_expected_operation, progress_functions]) do + state = State.new(uri_with_expected_operation, progress_functions) + GenServer.start_link(__MODULE__, state, name: __MODULE__) end @impl true def init(state) do - {:ok, state} - end - - @spec set_rename_progress( - %{Lexical.uri() => Messages.file_changed() | Messages.file_saved()}, - {function(), function()} - ) :: :ok - def set_rename_progress(uri_with_expected_operation, progress_functions) do - GenServer.cast( - __MODULE__, - {:set_rename_progress, uri_with_expected_operation, progress_functions} - ) - end - - def in_progress? do - GenServer.call(__MODULE__, :in_progress?) + {:ok, state, {:continue, :start_buffering}} end @spec update_progress(Messages.file_changed() | Messages.file_saved()) :: :ok @@ -92,7 +79,25 @@ defmodule Lexical.RemoteControl.Commands.Rename do # Instead, it should call this function to synchronously update the status, # thus preventing failures due to latency issues. def update_progress(message) do - GenServer.cast(__MODULE__, {:update_progress, message}) + if in_progress?() do + GenServer.cast(__MODULE__, {:update_progress, message}) + else + :ok + end + end + + def in_progress? do + if Process.whereis(__MODULE__) do + GenServer.call(__MODULE__, :in_progress?) + else + false + end + end + + @impl true + def handle_continue(:start_buffering, state) do + Proxy.start_buffering(self()) + {:noreply, state} end @impl true @@ -103,7 +108,12 @@ defmodule Lexical.RemoteControl.Commands.Rename do @impl true def handle_cast({:update_progress, message}, state) do new_state = State.update_progress(state, message) - {:noreply, new_state} + + if State.in_progress?(new_state) do + {:noreply, new_state} + else + {:stop, :normal, new_state} + end end @impl true diff --git a/apps/remote_control/test/lexical/remote_control/commands/rename_test.exs b/apps/remote_control/test/lexical/remote_control/commands/rename_test.exs index ed19b9ef8..c77670949 100644 --- a/apps/remote_control/test/lexical/remote_control/commands/rename_test.exs +++ b/apps/remote_control/test/lexical/remote_control/commands/rename_test.exs @@ -1,4 +1,5 @@ defmodule Lexical.RemoteControl.Commands.RenameTest do + alias Lexical.RemoteControl.Api.Proxy alias Lexical.RemoteControl.Commands.Rename import Lexical.RemoteControl.Api.Messages @@ -7,13 +8,13 @@ defmodule Lexical.RemoteControl.Commands.RenameTest do use Patch setup do - start_supervised!(Rename) pid = self() progress_funcs = {fn delta, message -> update_progress(pid, delta, message) end, fn -> complete_progress(pid) end} + patch(Proxy, :start_buffering, :ok) %{progress_funcs: progress_funcs} end @@ -21,22 +22,28 @@ defmodule Lexical.RemoteControl.Commands.RenameTest do progress_funcs: progress_funcs } do uri = "file://file.ex" - assert :ok = Rename.set_rename_progress(%{uri => file_changed(uri: uri)}, progress_funcs) + init_opts = [%{uri => file_changed(uri: uri)}, progress_funcs] + pid = start_supervised!({Rename, init_opts}) + assert Rename.in_progress?() + assert_called(Proxy.start_buffering(^pid)) end - test "it should mark the `in_progress` as false when a rename is done", %{ - progress_funcs: progress_funcs - } do + test "it should mark the `in_progress` as false and shutdown the process when a rename is done", + %{ + progress_funcs: progress_funcs + } do file_uri = "file://file.ex" + init_opts = [%{file_uri => file_saved(uri: file_uri)}, progress_funcs] + start_supervised!({Rename, init_opts}) - Rename.set_rename_progress(%{file_uri => file_saved(uri: file_uri)}, progress_funcs) Rename.update_progress(file_saved(uri: file_uri)) assert_receive {:update_progress, 1, ""} assert_receive :complete_progress refute Rename.in_progress?() + refute Process.whereis(Rename) end test "it should still in progress if there are files yet to be saved.", %{ @@ -45,19 +52,23 @@ defmodule Lexical.RemoteControl.Commands.RenameTest do uri1 = "file://file1.ex" uri2 = "file://file2.ex" - Rename.set_rename_progress( + init_opts = [ %{uri1 => file_changed(uri: uri1), uri2 => file_saved(uri: uri2)}, progress_funcs - ) + ] - Rename.update_progress(file_changed(uri: uri1)) + start_supervised!({Rename, init_opts}) + Rename.update_progress(file_changed(uri: uri1)) assert_receive {:update_progress, 1, ""} - refute_receive :complete_progress assert Rename.in_progress?() end + test "it should return `:ok` when updating the progress if the process is not alive" do + assert :ok = Rename.update_progress(file_changed(uri: "file://file.ex")) + end + defp update_progress(pid, delta, message) do send(pid, {:update_progress, delta, message}) end diff --git a/apps/server/lib/lexical/server/state.ex b/apps/server/lib/lexical/server/state.ex index f3f0c3f6e..5b638b0f7 100644 --- a/apps/server/lib/lexical/server/state.ex +++ b/apps/server/lib/lexical/server/state.ex @@ -162,8 +162,9 @@ defmodule Lexical.Server.State do to_version: updated_source.version ) + Api.maybe_update_rename_progress(state.configuration.project, updated_message) + Api.compile_document(state.configuration.project, updated_source) Api.broadcast(project, updated_message) - Api.maybe_compile_document(state.configuration.project, updated_source, updated_message) {:ok, state} error -> @@ -213,7 +214,8 @@ defmodule Lexical.Server.State do case Document.Store.save(uri) do :ok -> - Api.maybe_schedule_compile(state.configuration.project, file_saved(uri: uri)) + Api.maybe_update_rename_progress(state.configuration.project, file_saved(uri: uri)) + Api.schedule_compile(state.configuration.project, false) {:ok, state}