From b190089458b1c23fab797d2a015188cae5c297e3 Mon Sep 17 00:00:00 2001 From: Roman Heinrich Date: Sat, 11 Nov 2023 09:45:13 +0100 Subject: [PATCH 1/3] Chore: get rid of the test folder --- lib/surrealix/util.ex | 2 +- lib/test_helper.exs | 54 ++++++++++++++++++++++++++++++++++++++++++ mix.exs | 2 +- test/test_helper.exs | 55 ------------------------------------------- 4 files changed, 56 insertions(+), 57 deletions(-) delete mode 100644 test/test_helper.exs diff --git a/lib/surrealix/util.ex b/lib/surrealix/util.ex index a762700..5bd912c 100644 --- a/lib/surrealix/util.ex +++ b/lib/surrealix/util.ex @@ -1,6 +1,6 @@ defmodule Surrealix.Util do @moduledoc """ - Some generic utility functions + Generic utility functions """ @doc """ diff --git a/lib/test_helper.exs b/lib/test_helper.exs index 869559e..6cd8f2f 100644 --- a/lib/test_helper.exs +++ b/lib/test_helper.exs @@ -1 +1,55 @@ ExUnit.start() +Mneme.start(restart: true) + +defmodule MnemeDefaults do + defmacro __using__(_) do + quote do + use Mneme, action: :accept, default_pattern: :last, force_update: false + end + end +end + +defmodule TestSupport do + use ExUnit.Case + + def extract_res({:ok, res}) do + Map.get(res, "result") + end + + def extract_res({:ok, res}, index) do + extract_res({:ok, res}) |> Enum.at(index) |> Map.get("result") + end + + def extract_res_list({:ok, res}) do + result = Map.get(res, "result") + + result + |> Enum.map(fn x -> + {:ok, Map.get(x, "result")} + end) + end + + def setup_surrealix(_context) do + db = db_name() + # NOT start_link(), so we can cleanup after test exits! + # {:ok, pid} = Surrealix.start(debug: [:trace]) ## this is for debugging! + {:ok, pid} = Surrealix.start() + Surrealix.signin(pid, %{user: "root", pass: "root"}) + Surrealix.use(pid, "test", db) + + on_exit(:drop_db, fn -> + _res = Surrealix.query(pid, "remove database #{db};") + Surrealix.stop(pid) + end) + + %{pid: pid} + end + + def db_name() do + rand = + :crypto.strong_rand_bytes(6) + |> Base.encode64() + + "test_#{Regex.replace(~r/\W/, rand, "")}" + end +end diff --git a/mix.exs b/mix.exs index a80f879..482c611 100644 --- a/mix.exs +++ b/mix.exs @@ -13,7 +13,7 @@ defmodule Surrealix.MixProject do deps: deps(), package: package(), description: description(), - test_paths: ["test", "lib"], + test_paths: ["lib"], test_pattern: "*_test.exs" ] end diff --git a/test/test_helper.exs b/test/test_helper.exs deleted file mode 100644 index 6cd8f2f..0000000 --- a/test/test_helper.exs +++ /dev/null @@ -1,55 +0,0 @@ -ExUnit.start() -Mneme.start(restart: true) - -defmodule MnemeDefaults do - defmacro __using__(_) do - quote do - use Mneme, action: :accept, default_pattern: :last, force_update: false - end - end -end - -defmodule TestSupport do - use ExUnit.Case - - def extract_res({:ok, res}) do - Map.get(res, "result") - end - - def extract_res({:ok, res}, index) do - extract_res({:ok, res}) |> Enum.at(index) |> Map.get("result") - end - - def extract_res_list({:ok, res}) do - result = Map.get(res, "result") - - result - |> Enum.map(fn x -> - {:ok, Map.get(x, "result")} - end) - end - - def setup_surrealix(_context) do - db = db_name() - # NOT start_link(), so we can cleanup after test exits! - # {:ok, pid} = Surrealix.start(debug: [:trace]) ## this is for debugging! - {:ok, pid} = Surrealix.start() - Surrealix.signin(pid, %{user: "root", pass: "root"}) - Surrealix.use(pid, "test", db) - - on_exit(:drop_db, fn -> - _res = Surrealix.query(pid, "remove database #{db};") - Surrealix.stop(pid) - end) - - %{pid: pid} - end - - def db_name() do - rand = - :crypto.strong_rand_bytes(6) - |> Base.encode64() - - "test_#{Regex.replace(~r/\W/, rand, "")}" - end -end From c87fe9b3853d090cb622a2478595b99a213d7fa9 Mon Sep 17 00:00:00 2001 From: Roman Heinrich Date: Sat, 11 Nov 2023 10:36:08 +0100 Subject: [PATCH 2/3] Feat: simpler callback registration directly on SocketState Problem: - current :telemetry inspired way to handle live query callbacks makes handling reconnections much harder, than it should be - also we have quite some modules just for this feature Solution: - just store the callback function directly in SocketState struct - reconnection becomes much simpler, since all the info is directly there, and we also do not have to deal with garbage cleanup on process termination in a manual way --- README.md | 8 ++-- lib/surrealix/api.ex | 4 +- lib/surrealix/application.ex | 4 +- lib/surrealix/attach_error.ex | 9 ---- lib/surrealix/dispatch.ex | 44 ------------------ lib/surrealix/dispatch_test.exs | 50 --------------------- lib/surrealix/handler_table.ex | 69 ----------------------------- lib/surrealix/socket.ex | 10 +++-- lib/surrealix/socket_state.ex | 22 +++++---- lib/surrealix/socket_state_test.exs | 52 ++++++++++++++++------ lib/surrealix_test.exs | 2 +- lib/test_helper.exs | 1 + 12 files changed, 67 insertions(+), 208 deletions(-) delete mode 100644 lib/surrealix/attach_error.ex delete mode 100644 lib/surrealix/dispatch.ex delete mode 100644 lib/surrealix/dispatch_test.exs delete mode 100644 lib/surrealix/handler_table.ex diff --git a/README.md b/README.md index 1c4dee2..0af87c7 100644 --- a/README.md +++ b/README.md @@ -33,13 +33,13 @@ Surrealix.query(pid, "SELECT * FROM type::table($table);", %{table: "person"}) ```elixir ## Example with live query callbacks -Surrealix.live_query(pid, "LIVE DIFF SELECT * FROM user;", fn event, data, config -> - IO.inspect({event, data, config}, label: "callback") +Surrealix.live_query(pid, "LIVE SELECT * FROM user;", fn data, query_id -> + IO.inspect({data, query_id}, label: "callback") end) ## Example with live query with DIFF -Surrealix.live_query(pid, "LIVE SELECT DIFF FROM user;", fn event, data, config -> - IO.inspect({event, data, config}, label: "callback") +Surrealix.live_query(pid, "LIVE SELECT DIFF FROM user;", fn data, query_id -> + IO.inspect({data, query_id}, label: "callback") end) diff --git a/lib/surrealix/api.ex b/lib/surrealix/api.ex index 1375069..cb4c09b 100644 --- a/lib/surrealix/api.ex +++ b/lib/surrealix/api.ex @@ -34,9 +34,7 @@ defmodule Surrealix.Api do with {:sql_live_check, true} <- {:sql_live_check, Util.is_live_query_stmt(sql)}, {:ok, res} <- query(pid, sql, vars), %{"result" => [%{"result" => lq_id}]} <- res do - event = [:live_query, lq_id] - :ok = Surrealix.Dispatch.attach("#{lq_id}_main", event, callback) - :ok = WebSockex.cast(pid, {:register_lq, sql, lq_id}) + :ok = WebSockex.cast(pid, {:register_lq, sql, lq_id, callback}) {:ok, res} else {:sql_live_check, false} -> {:error, "Not a live query: `#{sql}`!"} diff --git a/lib/surrealix/application.ex b/lib/surrealix/application.ex index ad9f6f1..c0b830a 100644 --- a/lib/surrealix/application.ex +++ b/lib/surrealix/application.ex @@ -5,9 +5,7 @@ defmodule Surrealix.Application do @impl true def start(_type, _args) do - children = [ - {Surrealix.HandlerTable, []} - ] + children = [] opts = [strategy: :one_for_one, name: Surrealix.Supervisor] Supervisor.start_link(children, opts) diff --git a/lib/surrealix/attach_error.ex b/lib/surrealix/attach_error.ex deleted file mode 100644 index f7e5a91..0000000 --- a/lib/surrealix/attach_error.ex +++ /dev/null @@ -1,9 +0,0 @@ -defmodule Surrealix.AttachError do - @moduledoc false - defexception [:message] - - @impl true - def exception(msg) do - %__MODULE__{message: msg} - end -end diff --git a/lib/surrealix/dispatch.ex b/lib/surrealix/dispatch.ex deleted file mode 100644 index b11ed10..0000000 --- a/lib/surrealix/dispatch.ex +++ /dev/null @@ -1,44 +0,0 @@ -defmodule Surrealix.Dispatch do - @moduledoc """ - Handles callback registration (for only when using live queries). - Based on ideas here: https://github.com/keathley/sync_dispatch - - which itself is based on :telemetry (https://hex.pm/packages/telemetry) implementation. - """ - - alias Surrealix.HandlerTable - - @type handler_function :: (list(atom()), term(), term() -> any()) - - @doc """ - Executes an event and any handlers that are attached to the event name. `data` - can be any term(). - """ - @spec execute(list(atom()), term()) :: :ok | no_return() - def execute(event, data) do - handlers = HandlerTable.handlers_for_event(event) - - for {handler, event, config} <- handlers do - handler.(event, data, config) - end - - :ok - end - - @doc """ - Attaches a function to an event. The provided id is used for idempotence. - Different handlers should use unique handler ids. - """ - @spec attach(term(), list(atom()), handler_function(), term()) :: - :ok | {:error, %Surrealix.AttachError{}} - def attach(id, event, fun, config \\ nil) do - HandlerTable.insert(id, event, fun, config) - end - - def remove_by_id(id) do - HandlerTable.remove_by_id(id) - end - - def remove_by_event(id) do - HandlerTable.remove_by_event(id) - end -end diff --git a/lib/surrealix/dispatch_test.exs b/lib/surrealix/dispatch_test.exs deleted file mode 100644 index 17bc87c..0000000 --- a/lib/surrealix/dispatch_test.exs +++ /dev/null @@ -1,50 +0,0 @@ -defmodule Surrealix.DispatchTest do - # cant be async, uses a global gen ETS table! - use ExUnit.Case - alias Surrealix.Dispatch - - setup do - Surrealix.HandlerTable.start_link([]) - Surrealix.HandlerTable.delete_all() - - :ok - end - - test "dispatches events" do - us = self() - - Dispatch.attach("handler-one", [:event, :prefix], fn event, data, config -> - send(us, {:first, event, data, config}) - end) - - Dispatch.attach( - "handler-two", - [:event, :prefix], - fn event, data, config -> - send(us, {:second, event, data, config}) - end, - :two - ) - - Dispatch.execute([:event, :prefix], %{test: true}) - - assert_receive {:first, event, data, config} - assert event == [:event, :prefix] - assert data == %{test: true} - assert config == nil - - assert_receive {:second, event, data, config} - assert event == [:event, :prefix] - assert data == %{test: true} - assert config == :two - end - - test "re-using event ids produces an error" do - assert :ok = Dispatch.attach("handler", [:event, :prefix], fn _, _, _ -> nil end) - - assert {:error, error} = - Dispatch.attach("handler", [:event, :prefix], fn _, _, _ -> nil end) - - assert match?(%Surrealix.AttachError{}, error) - end -end diff --git a/lib/surrealix/handler_table.ex b/lib/surrealix/handler_table.ex deleted file mode 100644 index b630d37..0000000 --- a/lib/surrealix/handler_table.ex +++ /dev/null @@ -1,69 +0,0 @@ -defmodule Surrealix.HandlerTable do - @moduledoc """ - Wrapper around ETS to store callbacks for live queries. Modelled after :telemetry. - """ - use GenServer - - @table __MODULE__ - @table_opts [ - :duplicate_bag, - :public, - :named_table, - {:keypos, 2}, - {:read_concurrency, true} - ] - - def start_link(args) do - GenServer.start_link(__MODULE__, args, name: __MODULE__) - end - - def init(_args) do - _ = :ets.new(@table, @table_opts) - {:ok, %{}} - end - - def handlers_for_event(event) do - case :ets.lookup(@table, event) do - [] -> - [] - - list -> - for {_id, event, f, config} <- list do - {f, event, config} - end - end - end - - def insert(id, events, fun, config) do - case :ets.match(@table, {id, :_, :_, :_}) do - [] -> - :ets.insert(@table, {id, events, fun, config}) - :ok - - [_res] -> - msg = """ - A handler with the id: #{id} has already been added. - """ - - error = %Surrealix.AttachError{message: msg} - {:error, error} - end - end - - def remove_by_id(id) do - :ets.match_delete(@table, {id, :_, :_, :_}) - end - - def remove_by_event(event) do - :ets.match_delete(@table, {:_, event, :_, :_}) - end - - # Debug / dev functions ############# - def delete_all() do - :ets.delete_all_objects(@table) - end - - def all do - :ets.tab2list(@table) - end -end diff --git a/lib/surrealix/socket.ex b/lib/surrealix/socket.ex index de38b85..db4fade 100644 --- a/lib/surrealix/socket.ex +++ b/lib/surrealix/socket.ex @@ -52,8 +52,8 @@ defmodule Surrealix.Socket do exit(:normal) end - def handle_cast({:register_lq, sql, query_id}, state) do - state = SocketState.add_lq(state, sql, query_id) + def handle_cast({:register_lq, sql, query_id, callback}, state) do + state = SocketState.add_lq(state, sql, query_id, callback) {:ok, state} end @@ -73,7 +73,11 @@ defmodule Surrealix.Socket do if is_nil(task) do # No registered task for this ID, must be a live query update lq_id = get_in(json, ["result", "id"]) - Surrealix.Dispatch.execute([:live_query, lq_id], json) + lq_item = SocketState.get_lq(state, lq_id) + + if(!is_nil(lq_item)) do + lq_item.callback.(json, lq_id) + end else if Process.alive?(task.pid) do Process.send(task.pid, {:ok, json, id}, []) diff --git a/lib/surrealix/socket_state.ex b/lib/surrealix/socket_state.ex index 841975e..3e446d3 100644 --- a/lib/surrealix/socket_state.ex +++ b/lib/surrealix/socket_state.ex @@ -41,9 +41,9 @@ defmodule Surrealix.SocketState do @doc """ Register a SQL statement for a particular LiveQuery ID """ - def add_lq(state = %SocketState{}, sql, query_id) do - lq_sql = MapSet.put(state.lq_sql, sql) - item = %{sql: sql, query_id: query_id} + def add_lq(state = %SocketState{}, sql, query_id, callback) do + lq_sql = MapSet.put(state.lq_sql, {sql, callback}) + item = %{sql: sql, query_id: query_id, callback: callback} state |> put_in([:lq_running, query_id], item) @@ -65,7 +65,7 @@ defmodule Surrealix.SocketState do {item, state} = pop_in(state, [:lq_running, query_id]) if item do - lq_sql = MapSet.delete(state.lq_sql, item.sql) + lq_sql = MapSet.delete(state.lq_sql, {item.sql, item.callback}) state |> Map.put(:lq_sql, lq_sql) @@ -78,12 +78,16 @@ defmodule Surrealix.SocketState do Remove a LiveQuery by SQL """ def delete_lq_by_sql(state = %SocketState{}, sql) do - lq_running = - Enum.reject(state.lq_running, fn {_id, value} -> Map.get(value, :sql) == sql end) - |> Map.new() + found = Enum.find(state.lq_running, fn {_id, value} -> Map.get(value, :sql) == sql end) - lq_sql = MapSet.delete(state.lq_sql, sql) - Map.put(state, :lq_running, lq_running) |> Map.put(:lq_sql, lq_sql) + if !is_nil(found) do + {key, item} = found + lq_running = Map.delete(state.lq_running, key) + lq_sql = MapSet.delete(state.lq_sql, {item.sql, item.callback}) + Map.put(state, :lq_running, lq_running) |> Map.put(:lq_sql, lq_sql) + else + state + end end @doc """ diff --git a/lib/surrealix/socket_state_test.exs b/lib/surrealix/socket_state_test.exs index 81f2cb3..f692917 100644 --- a/lib/surrealix/socket_state_test.exs +++ b/lib/surrealix/socket_state_test.exs @@ -3,6 +3,8 @@ defmodule Surrealix.SocketStateTest do alias Surrealix.SocketState + def dummy_callback(), do: fn -> nil end + describe "tasks" do test "add_task / get_task / delete_task" do state = SocketState.new() @@ -24,9 +26,11 @@ defmodule Surrealix.SocketStateTest do describe "lq" do test "add_lq / get_lq" do state = SocketState.new() - state = state |> SocketState.add_lq("select * from person", "11-22") + cb = dummy_callback() + state = state |> SocketState.add_lq("select * from person", "11-22", cb) assert SocketState.get_lq(state, "11-22") == %{ + callback: cb, query_id: "11-22", sql: "select * from person" } @@ -34,30 +38,52 @@ defmodule Surrealix.SocketStateTest do test "all_lq" do state = SocketState.new() - state = state |> SocketState.add_lq("select * from person", "11-22") - state = state |> SocketState.add_lq("select * from user", "11-23") - assert SocketState.all_lq(state) == ["select * from person", "select * from user"] + cb = dummy_callback() + state = state |> SocketState.add_lq("select * from person", "11-22", cb) + state = state |> SocketState.add_lq("select * from user", "11-23", cb) + + assert SocketState.all_lq(state) == [ + {"select * from person", cb}, + {"select * from user", cb} + ] end test "delete_lq_by_id" do state = SocketState.new() - state = state |> SocketState.add_lq("select * from person", "11-22") - state = state |> SocketState.add_lq("select * from user", "11-23") - assert SocketState.all_lq(state) == ["select * from person", "select * from user"] + cb = dummy_callback() + state = state |> SocketState.add_lq("select * from person", "11-22", cb) + state = state |> SocketState.add_lq("select * from user", "11-23", cb) + + assert SocketState.all_lq(state) == [ + {"select * from person", cb}, + {"select * from user", cb} + ] + state = state |> SocketState.delete_lq_by_id("11-23") - assert SocketState.all_lq(state) == ["select * from person"] + assert SocketState.all_lq(state) == [{"select * from person", cb}] assert SocketState.get_lq(state, "11-23") == nil end test "delete_lq_by_sql" do state = SocketState.new() - state = state |> SocketState.add_lq("select * from person", "11-22") - state = state |> SocketState.add_lq("select * from user", "11-23") - assert SocketState.all_lq(state) == ["select * from person", "select * from user"] + cb = dummy_callback() + state = state |> SocketState.add_lq("select * from person", "11-22", cb) + state = state |> SocketState.add_lq("select * from user", "11-23", cb) + + assert SocketState.all_lq(state) == [ + {"select * from person", cb}, + {"select * from user", cb} + ] + state = SocketState.delete_lq_by_sql(state, "select * from person") - assert SocketState.all_lq(state) == ["select * from user"] + assert SocketState.all_lq(state) == [{"select * from user", cb}] assert SocketState.get_lq(state, "11-22") == nil - assert SocketState.get_lq(state, "11-23") == %{query_id: "11-23", sql: "select * from user"} + + assert SocketState.get_lq(state, "11-23") == %{ + query_id: "11-23", + sql: "select * from user", + callback: cb + } end end end diff --git a/lib/surrealix_test.exs b/lib/surrealix_test.exs index 8c9074e..9b2fae8 100644 --- a/lib/surrealix_test.exs +++ b/lib/surrealix_test.exs @@ -36,7 +36,7 @@ defmodule Surrealix.Test do testpid = self() {:ok, _} = - Surrealix.live_query(pid, "LIVE SELECT * FROM user;", fn _event, data, _config -> + Surrealix.live_query(pid, "LIVE SELECT * FROM user;", fn data, _query_id -> send(testpid, {:lq, data}) end) diff --git a/lib/test_helper.exs b/lib/test_helper.exs index 6cd8f2f..bc0154c 100644 --- a/lib/test_helper.exs +++ b/lib/test_helper.exs @@ -1,3 +1,4 @@ +# ExUnit.start(trace: true) ## -> verbose output, great for debugging! ExUnit.start() Mneme.start(restart: true) From 0b387bf74331243e10b5994f115d3c5d86a98f31 Mon Sep 17 00:00:00 2001 From: Roman Heinrich Date: Sat, 11 Nov 2023 10:39:41 +0100 Subject: [PATCH 3/3] Feat: activate Github Actions on pull requests --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 29eed59..0fb71f8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,6 +3,7 @@ name: CI push: branches: - main + pull_request: jobs: full-tests: name: Full Tests