Za api proxy (#752)

* Use mfa record for proxied broadcasts * Add sequence number to proxied mfa's to track order --------- Co-authored-by: Zach Allaun <[email protected]>
lexical-lsp · May 23, 2024 · 5bb014a · 5bb014a
1 parent 65da237
commit 5bb014a
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 81 deletions.
diff --git a/apps/remote_control/lib/lexical/remote_control/api/proxy.ex b/apps/remote_control/lib/lexical/remote_control/api/proxy.ex
@@ -57,8 +57,8 @@ defmodule Lexical.RemoteControl.Api.Proxy do
   end
 
   def broadcast(message) do
-    message = message(body: message)
-    :gen_statem.call(__MODULE__, buffer(contents: message))
+    mfa = to_mfa(RemoteControl.Dispatch.broadcast(message))
+    :gen_statem.call(__MODULE__, buffer(contents: mfa))
   end
 
   def schedule_compile(force? \\ false) do
@@ -148,11 +148,6 @@ defmodule Lexical.RemoteControl.Api.Proxy do
     {:keep_state, state, [{:reply, from, return}]}
   end
 
-  def buffering({:call, from}, buffer(contents: message() = message), %State{} = state) do
-    state = State.add_message(state, message)
-    {:keep_state, state, [{:reply, from, :ok}]}
-  end
-
   def buffering({:call, from}, drop(return: return), %State{} = state) do
     {:keep_state, state, [{:reply, from, return}]}
   end
@@ -174,8 +169,4 @@ defmodule Lexical.RemoteControl.Api.Proxy do
   defp apply(mfa(module: module, function: function, arguments: arguments)) do
     apply(module, function, arguments)
   end
-
-  defp apply(message(body: message)) do
-    RemoteControl.Dispatch.broadcast(message)
-  end
 end
diff --git a/apps/remote_control/lib/lexical/remote_control/api/proxy/records.ex b/apps/remote_control/lib/lexical/remote_control/api/proxy/records.ex
@@ -3,11 +3,15 @@ defmodule Lexical.RemoteControl.Api.Proxy.Records do
 
   import Record
 
-  defrecord :message, body: nil
-  defrecord :mfa, module: nil, function: nil, arguments: []
+  defrecord :mfa, module: nil, function: nil, arguments: [], seq: nil
 
   def mfa(module, function, arguments) do
-    mfa(module: module, function: function, arguments: arguments)
+    mfa(
+      module: module,
+      function: function,
+      arguments: arguments,
+      seq: System.unique_integer([:monotonic])
+    )
   end
 
   defmacro to_mfa(ast) do
@@ -28,9 +32,6 @@ defmodule Lexical.RemoteControl.Api.Proxy.Records do
             })
     end
 
-    quote do
-      require unquote(__MODULE__)
-      mfa(module: unquote(m), function: unquote(f), arguments: unquote(a))
-    end
+    quote(do: unquote(__MODULE__).mfa(unquote(m), unquote(f), unquote(a)))
   end
 end
diff --git a/apps/remote_control/lib/lexical/remote_control/api/proxy/state.ex b/apps/remote_control/lib/lexical/remote_control/api/proxy/state.ex
@@ -1,14 +1,13 @@
 defmodule Lexical.RemoteControl.Api.Proxy.State do
   alias Lexical.Document
+  alias Lexical.RemoteControl
   alias Lexical.RemoteControl.Api
   alias Lexical.RemoteControl.Build
   alias Lexical.RemoteControl.Commands
 
   import Api.Messages
   import Api.Proxy.Records
-  import Record
 
-  defrecord :indexed, index: nil, value: nil
   defstruct initiator_pid: nil, buffer: []
 
   def new(initiator_pid) do
@@ -19,34 +18,27 @@ defmodule Lexical.RemoteControl.Api.Proxy.State do
     %__MODULE__{state | buffer: [mfa_record | state.buffer]}
   end
 
-  def add_message(%__MODULE__{} = state, message() = message_record) do
-    %__MODULE__{state | buffer: [message_record | state.buffer]}
-  end
-
   def flush(%__MODULE__{} = state) do
-    {commands, messages} =
+    {messages, commands} =
       state.buffer
       |> Enum.reverse()
-      |> Enum.with_index()
-      |> Enum.map(fn {item, index} -> indexed(index: index, value: item) end)
-      |> Enum.split_with(fn indexed(value: value) -> match?(mfa(), value) end)
+      |> Enum.split_with(fn value ->
+        match?(mfa(module: RemoteControl.Dispatch, function: :broadcast), value)
+      end)
 
     {project_compile, document_compiles, reindex} = collapse_commands(commands)
 
     all_commands = [project_compile | Map.values(document_compiles)]
 
     all_commands
     |> Enum.concat(collapse_messages(messages, project_compile, document_compiles))
-    |> Enum.filter(&match?(indexed(), &1))
-    |> Enum.sort_by(fn
-      indexed(index: index) ->
-        index
-    end)
-    |> Enum.map(fn indexed(value: message) -> message end)
+    |> Enum.filter(&match?(mfa(), &1))
+    |> Enum.sort_by(fn mfa(seq: seq) -> seq end)
     |> then(fn commands ->
-      case reindex do
-        indexed(value: value) -> commands ++ [value]
-        _ -> commands
+      if reindex do
+        commands ++ [reindex]
+      else
+        commands
       end
     end)
   end
@@ -64,16 +56,16 @@ defmodule Lexical.RemoteControl.Api.Proxy.State do
       |> Enum.reduce(
         initial_state,
         fn
-          indexed(value: mfa(module: Build, function: :schedule_compile)) = indexed, acc ->
-            Map.update(acc, :project_compiles, [indexed], &[indexed | &1])
+          mfa(module: Build, function: :schedule_compile) = mfa, acc ->
+            Map.update(acc, :project_compiles, [mfa], &[mfa | &1])
 
-          indexed(value: mfa(module: Build, function: :compile_document) = mfa) = indexed, acc ->
+          mfa(module: Build, function: :compile_document) = mfa, acc ->
             mfa(arguments: [_, document]) = mfa
             uri = document.uri
-            put_in(acc, [:document_compiles, uri], indexed)
+            put_in(acc, [:document_compiles, uri], mfa)
 
-          indexed(value: mfa(module: Commands.Reindex)) = indexed, acc ->
-            Map.put(acc, :reindex, indexed)
+          mfa(module: Commands.Reindex) = mfa, acc ->
+            Map.put(acc, :reindex, mfa)
 
           _, acc ->
             acc
@@ -88,14 +80,14 @@ defmodule Lexical.RemoteControl.Api.Proxy.State do
 
     project_compile =
       Enum.reduce(project_compiles, nil, fn
-        indexed(value: mfa(arguments: [_, true])) = indexed, _ ->
-          indexed
+        mfa(arguments: [_, true]) = mfa, _ ->
+          mfa
 
-        indexed(value: mfa(arguments: [true])) = indexed, _ ->
-          indexed
+        mfa(arguments: [true]) = mfa, _ ->
+          mfa
 
-        indexed() = indexed, nil ->
-          indexed
+        mfa() = mfa, nil ->
+          mfa
 
         _, acc ->
           acc
@@ -123,17 +115,17 @@ defmodule Lexical.RemoteControl.Api.Proxy.State do
     # 4. Progress messages should still be sent to dispatch, even when buffering
 
     Enum.filter(messages, fn
-      indexed(value: message(body: file_compile_requested())) ->
+      mfa(arguments: [file_compile_requested()]) ->
         false
 
-      indexed(value: message(body: project_compile_requested())) ->
+      mfa(arguments: [project_compile_requested()]) ->
         false
 
-      indexed(value: message(body: file_diagnostics(uri: uri))) ->
+      mfa(arguments: [file_diagnostics(uri: uri)]) ->
         not (Map.has_key?(document_compiles, uri) or
                match?(project_compile_requested(), project_compile))
 
-      indexed(value: message(body: body)) ->
+      mfa(arguments: [body]) ->
         case fetch_uri(body) do
           {:ok, uri} ->
             Document.Store.open?(uri)

diff --git a/apps/remote_control/test/lexical/remote_control/api/proxy/state_test.exs b/apps/remote_control/test/lexical/remote_control/api/proxy/state_test.exs
@@ -1,5 +1,6 @@
 defmodule Lexical.RemoteControl.Api.Proxy.StateTest do
   alias Lexical.Document
+  alias Lexical.RemoteControl
   alias Lexical.RemoteControl.Api.Messages
   alias Lexical.RemoteControl.Api.Proxy
   alias Lexical.RemoteControl.Api.Proxy.State
@@ -31,13 +32,7 @@ defmodule Lexical.RemoteControl.Api.Proxy.StateTest do
 
   def add_to_state_and_flush(messages) do
     messages
-    |> Enum.reduce(State.new(self()), fn
-      mfa() = mfa, state ->
-        State.add_mfa(state, mfa)
-
-      message() = message, state ->
-        State.add_message(state, message)
-    end)
+    |> Enum.reduce(State.new(self()), &State.add_mfa(&2, &1))
     |> State.flush()
   end
 
@@ -50,7 +45,7 @@ defmodule Lexical.RemoteControl.Api.Proxy.StateTest do
         ]
         |> add_to_state_and_flush()
 
-      assert [to_mfa(Build.schedule_compile(project))] == flushed_messages
+      assert [{:mfa, Build, :schedule_compile, [^project], _}] = flushed_messages
     end
 
     test "force project compilation takes precedence", %{project: project} do
@@ -62,18 +57,18 @@ defmodule Lexical.RemoteControl.Api.Proxy.StateTest do
         ]
         |> add_to_state_and_flush()
 
-      assert [{:mfa, Build, :schedule_compile, [project, true]}] == flushed_messages
+      assert [{:mfa, Build, :schedule_compile, [^project, true], _}] = flushed_messages
     end
 
     test "a project compilation removes all document compilations", %{project: project} do
-      flushed_messaes =
+      flushed_messages =
         [
           to_mfa(Build.compile_document(project, document())),
           to_mfa(Build.schedule_compile(project))
         ]
         |> add_to_state_and_flush()
 
-      assert flushed_messaes == [to_mfa(Build.schedule_compile(project))]
+      assert [{:mfa, Build, :schedule_compile, [^project], _}] = flushed_messages
     end
 
     test "documents that aren't open are removed", %{project: project} do
@@ -99,7 +94,7 @@ defmodule Lexical.RemoteControl.Api.Proxy.StateTest do
         ]
         |> add_to_state_and_flush()
 
-      assert flushed_messages == [to_mfa(Build.compile_document(project, document))]
+      assert [{:mfa, Build, :compile_document, [^project, ^document], _}] = flushed_messages
     end
 
     test "there can only be one reindex", %{project: project} do
@@ -110,33 +105,36 @@ defmodule Lexical.RemoteControl.Api.Proxy.StateTest do
         ]
         |> add_to_state_and_flush()
 
-      assert [mfa(module: Commands.Reindex, function: :perform)] = flushed_messages
+      assert [{:mfa, Commands.Reindex, :perform, [^project], _}] = flushed_messages
     end
 
     test "a reindex is the last thing", %{project: project} do
+      other = open_document("file:///other.uri")
+      third = open_document("file:///third.uri")
+
       flushed_messages =
         [
           to_mfa(Commands.Reindex.perform()),
-          to_mfa(Build.compile_document(project, open_document("file:///other.uri"))),
-          to_mfa(Build.compile_document(project, open_document("file:///third.uri")))
+          to_mfa(Build.compile_document(project, other)),
+          to_mfa(Build.compile_document(project, third))
         ]
         |> add_to_state_and_flush()
 
-      assert flushed_messages == [
-               to_mfa(Build.compile_document(project, document("file:///other.uri"))),
-               to_mfa(Build.compile_document(project, document("file:///third.uri"))),
-               to_mfa(Commands.Reindex.perform())
-             ]
+      assert [
+               {:mfa, Build, :compile_document, [^project, ^other], _},
+               {:mfa, Build, :compile_document, [^project, ^third], _},
+               {:mfa, Commands.Reindex, :perform, [], _}
+             ] = flushed_messages
     end
   end
 
-  defp wrap_with_messages(messages) do
+  defp wrap_broadcasts(messages) do
     Enum.map(messages, fn
       mfa() = mfa ->
         mfa
 
       message ->
-        message(body: message)
+        mfa(module: RemoteControl.Dispatch, function: :broadcast, arguments: [message])
     end)
   end
 
@@ -150,7 +148,7 @@ defmodule Lexical.RemoteControl.Api.Proxy.StateTest do
           file_compiled(uri: @default_uri),
           file_deleted(uri: @default_uri)
         ]
-        |> wrap_with_messages()
+        |> wrap_broadcasts()
         |> add_to_state_and_flush()
 
       assert flushed_messages == []
@@ -166,7 +164,7 @@ defmodule Lexical.RemoteControl.Api.Proxy.StateTest do
           file_compiled(uri: uri),
           file_deleted(uri: uri)
         ]
-        |> wrap_with_messages()
+        |> wrap_broadcasts()
 
       flushed_messages = add_to_state_and_flush(orig_messages)
 
@@ -183,25 +181,25 @@ defmodule Lexical.RemoteControl.Api.Proxy.StateTest do
           to_mfa(Build.compile_document(project, document)),
           file_diagnostics(uri: @default_uri)
         ]
-        |> wrap_with_messages()
+        |> wrap_broadcasts()
         |> add_to_state_and_flush()
 
-      assert flushed_messages == [to_mfa(Build.compile_document(project, document))]
+      assert [{:mfa, Build, :compile_document, [^project, ^document], _}] = flushed_messages
     end
 
     test "file compiles are removed" do
       document = open_document()
 
       assert [] ==
                [file_compile_requested(uri: document.uri)]
-               |> wrap_with_messages()
+               |> wrap_broadcasts()
                |> add_to_state_and_flush()
     end
 
     test "project compiles are removed" do
       assert [] ==
                [project_compile_requested()]
-               |> wrap_with_messages()
+               |> wrap_broadcasts()
                |> add_to_state_and_flush()
     end
   end