fix client state handling (#178)

edgedb · Oct 31, 2023 · b68bda9 · b68bda9
1 parent cf1cbd1
commit b68bda9
Show file tree

Hide file tree

Showing 7 changed files with 73 additions and 45 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - support for EdgeDB binary protocol `2.0`.
 - new `EdgeDB.MultiRange` type to represent multiranges from `EdgeDB 4.0`.
 
+### Fixed
+- client state handling in `EdgeDB.with_config/2`/`EdgeDB.without_config/2`,
+    `EdgeDB.with_globals/2`/`EdgeDB.without_globals/2` and
+    `EdgeDB.with_module_aliases/2`/`EdgeDB.without_module_aliases/2`.
+
 ## [0.6.1] - 2023-07-07
 
 [Compare with 0.6.0](https://github.com/edgedb/edgedb-elixir/compare/v0.6.0...v0.6.1)

diff --git a/lib/edgedb.ex b/lib/edgedb.ex
@@ -697,7 +697,7 @@ defmodule EdgeDB do
 
   This is equivalent to using the `configure session set` command.
   """
-  @spec with_config(client(), %{atom() => term()}) :: client()
+  @spec with_config(client(), EdgeDB.Client.State.config()) :: client()
   def with_config(client, config \\ %{}) do
     client
     |> to_client()
@@ -709,7 +709,7 @@ defmodule EdgeDB do
 
   This is equivalent to using the `configure session reset` command.
   """
-  @spec without_config(client(), list(atom())) :: client()
+  @spec without_config(client(), list(EdgeDB.Client.State.config_key())) :: client()
   def without_config(client, config_keys \\ []) do
     client
     |> to_client()

diff --git a/lib/edgedb/client.ex b/lib/edgedb/client.ex
@@ -139,14 +139,14 @@ defmodule EdgeDB.Client do
   end
 
   @doc false
-  @spec with_config(t(), %{atom() => term()}) :: t()
+  @spec with_config(t(), State.config()) :: t()
   def with_config(client, config \\ %{}) do
     client = to_client(client)
     %__MODULE__{client | state: State.with_config(client.state, config)}
   end
 
   @doc false
-  @spec without_config(t(), list(atom())) :: t()
+  @spec without_config(t(), list(State.config_key())) :: t()
   def without_config(client, config_keys \\ []) do
     client = to_client(client)
     %__MODULE__{client | state: State.without_config(client.state, config_keys)}

diff --git a/lib/edgedb/client/state.ex b/lib/edgedb/client/state.ex
@@ -23,13 +23,34 @@ defmodule EdgeDB.Client.State do
             config: %{},
             globals: %{}
 
+  @typedoc since: "0.7.0"
+  @typedoc """
+  Keys that EdgeDB accepts for changing client behaviour configuration.
+
+  The meaning and acceptable values can be found in the
+    [docs](https://www.edgedb.com/docs/stdlib/cfg#client-connections).
+  """
+  @type config_key() ::
+          :allow_user_specified_id
+          | :session_idle_timeout
+          | :session_idle_transaction_timeout
+          | :query_execution_timeout
+
+  @typedoc since: "0.7.0"
+  @typedoc """
+  Config to be passed to `EdgeDB.with_config/2`.
+  """
+  @type config() ::
+          %{config_key() => term()}
+          | list({config_key(), term()})
+
   @typedoc """
   State for the client is an execution context that affects the execution of EdgeQL commands.
   """
   @opaque t() :: %__MODULE__{
             module: String.t() | nil,
             aliases: %{String.t() => String.t()},
-            config: %{String.t() => term()},
+            config: %{config_key() => term()},
             globals: %{String.t() => term()}
           }
 
@@ -61,25 +82,17 @@ defmodule EdgeDB.Client.State do
   """
   @spec without_module_aliases(t(), list(String.t())) :: t()
   def without_module_aliases(%__MODULE__{} = state, aliases \\ []) do
-    new_aliases =
-      case aliases do
-        [] ->
-          %{}
-
-        aliases ->
-          Enum.reduce(aliases, state.aliases, &Map.delete(&2, &1))
-      end
-
-    %__MODULE__{state | aliases: new_aliases}
+    %__MODULE__{state | aliases: Map.drop(state.aliases, aliases)}
   end
 
   @doc """
   Returns an `EdgeDB.Client.State` with adjusted session config.
 
   This is equivalent to using the `configure session set` command.
   """
-  @spec with_config(t(), %{atom() => term()}) :: t()
+  @spec with_config(t(), config()) :: t()
   def with_config(%__MODULE__{} = state, config \\ %{}) do
+    config = Enum.into(config, %{})
     %__MODULE__{state | config: Map.merge(state.config, config)}
   end
 
@@ -88,18 +101,9 @@ defmodule EdgeDB.Client.State do
 
   This is equivalent to using the `configure session reset` command.
   """
-  @spec without_config(t(), list(atom())) :: t()
+  @spec without_config(t(), list(config_key())) :: t()
   def without_config(%__MODULE__{} = state, config_keys \\ []) do
-    new_config =
-      case config_keys do
-        [] ->
-          %{}
-
-        config_keys ->
-          Enum.reduce(config_keys, state.config, &Map.delete(&2, &1))
-      end
-
-    %__MODULE__{state | config: new_config}
+    %__MODULE__{state | config: Map.drop(state.config, config_keys)}
   end
 
   @doc """
@@ -133,12 +137,12 @@ defmodule EdgeDB.Client.State do
     new_globals =
       case global_names do
         [] ->
-          %{}
+          state.globals
 
         global_names ->
           Enum.reduce(
             global_names,
-            state.config,
+            state.globals,
             &Map.delete(&2, resolve_name(state.aliases, module, &1))
           )
       end

diff --git a/pages/rst/api/api.rst b/pages/rst/api/api.rst
@@ -580,7 +580,7 @@ See ``EdgeDB.with_default_module/2``, ``EdgeDB.with_module_aliases/2``/``EdgeDB.
 
 .. code:: elixir
 
-   @spec EdgeDB.with_config(client(), %{required(atom()) => term()}) :: client()
+   @spec EdgeDB.with_config(client(), EdgeDB.Client.State.config()) :: client()
 
 Returns client with adjusted session config.
 
@@ -646,7 +646,7 @@ See ``EdgeDB.transaction_option/0`` for supported options.
 
 .. code:: elixir
 
-   @spec EdgeDB.without_config(client(), [atom()]) :: client()
+   @spec EdgeDB.without_config(client(), [EdgeDB.Client.State.config_key()]) :: client()
 
 Returns client without specified session config.
 
@@ -786,6 +786,30 @@ See ``EdgeDB.with_client_state/2``, ``EdgeDB.with_default_module/2``, ``EdgeDB.w
 Types
 ~~~~~
 
+*type* ``EdgeDB.Client.State.config/0``
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+.. code:: elixir
+
+   @type EdgeDB.Client.State.config() :: %{required(config_key()) => term()} | [{config_key(), term()}]
+
+Config to be passed to ``EdgeDB.with_config/2``.
+
+*type* ``EdgeDB.Client.State.config_key/0``
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+.. code:: elixir
+
+   @type EdgeDB.Client.State.config_key() ::
+     :allow_user_specified_id
+     | :session_idle_timeout
+     | :session_idle_transaction_timeout
+     | :query_execution_timeout
+
+Keys that EdgeDB accepts for changing client behaviour configuration.
+
+The meaning and acceptable values can be found in the `docs`_.
+
 *type* ``EdgeDB.Client.State.t/0``
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
@@ -805,7 +829,7 @@ Functions
 
 .. code:: elixir
 
-   @spec EdgeDB.Client.State.with_config(t(), %{required(atom()) => term()}) :: t()
+   @spec EdgeDB.Client.State.with_config(t(), config()) :: t()
 
 Returns an ``EdgeDB.Client.State`` with adjusted session config.
 
@@ -849,7 +873,7 @@ This is equivalent to using the ``set alias`` command.
 
 .. code:: elixir
 
-   @spec EdgeDB.Client.State.without_config(t(), [atom()]) :: t()
+   @spec EdgeDB.Client.State.without_config(t(), [config_key()]) :: t()
 
 Returns an ``EdgeDB.Client.State`` without specified session config.
 
@@ -939,3 +963,4 @@ Wrap a connection in a transaction.
 .. _the EdgeQL transaction statement: https://www.edgedb.com/docs/reference/edgeql/tx_start#statement::start-transaction
 .. _retrying transactions: %60EdgeDB.transaction/3%60
 .. _RFC: https://github.com/edgedb/rfcs/blob/master/text/1004-transactions-api.rst
+.. _docs: https://www.edgedb.com/docs/stdlib/cfg#client-connections
diff --git a/test/edgedb/api_test.exs b/test/edgedb/api_test.exs
@@ -420,13 +420,7 @@ defmodule Tests.EdgeDB.APITest do
     skip_before(version: 2, scope: :describe)
 
     setup %{client: client} do
-      %{
-        client:
-          EdgeDB.without_module_aliases(client, %{
-            "schema_alias" => "schema",
-            "cfg_alias" => "cfg"
-          })
-      }
+      %{client: EdgeDB.with_module_aliases(client, %{"cfg_alias" => "cfg"})}
     end
 
     test "removes aliases from passed to EdgeDB", %{client: client} do
@@ -446,7 +440,7 @@ defmodule Tests.EdgeDB.APITest do
       duration = Timex.Duration.from_microseconds(175_507_600_000)
 
       %{
-        client: EdgeDB.with_config(client, %{query_execution_timeout: duration}),
+        client: EdgeDB.with_config(client, query_execution_timeout: duration),
         duration: duration
       }
     end
@@ -471,7 +465,7 @@ defmodule Tests.EdgeDB.APITest do
       # 48:45:07:6
       duration = Timex.Duration.from_microseconds(175_507_600_000)
 
-      %{client: EdgeDB.with_config(client, %{query_execution_timeout: duration})}
+      %{client: EdgeDB.with_config(client, query_execution_timeout: duration)}
     end
 
     test "removes config keys from passed to EdgeDB", %{client: client} do
@@ -529,7 +523,7 @@ defmodule Tests.EdgeDB.APITest do
         |> EdgeDB.Client.State.with_default_module("schema")
         |> EdgeDB.Client.State.with_module_aliases(%{"math_alias" => "math", "cfg_alias" => "cfg"})
         |> EdgeDB.Client.State.with_globals(%{"default::current_user" => current_user})
-        |> EdgeDB.Client.State.with_config(%{query_execution_timeout: duration})
+        |> EdgeDB.Client.State.with_config(query_execution_timeout: duration)
 
       %{
         client: EdgeDB.with_client_state(client, state),

diff --git a/test/edgedb/connection/state_test.exs b/test/edgedb/connection/state_test.exs
@@ -15,7 +15,7 @@ defmodule Tests.EdgeDB.Connection.StateTest do
         |> EdgeDB.Client.State.with_default_module("schema")
         |> EdgeDB.Client.State.with_module_aliases(%{"math_alias" => "math", "cfg_alias" => "cfg"})
         |> EdgeDB.Client.State.with_globals(%{"default::current_user" => current_user})
-        |> EdgeDB.Client.State.with_config(%{query_execution_timeout: duration})
+        |> EdgeDB.Client.State.with_config(query_execution_timeout: duration)
 
       %{state: state, current_user: current_user, duration: duration}
     end
@@ -67,7 +67,7 @@ defmodule Tests.EdgeDB.Connection.StateTest do
         |> EdgeDB.Client.State.with_default_module("schema")
         |> EdgeDB.Client.State.with_module_aliases(%{"math_alias" => "math", "cfg_alias" => "cfg"})
         |> EdgeDB.Client.State.with_globals(%{"default::current_user" => current_user})
-        |> EdgeDB.Client.State.with_config(%{query_execution_timeout: duration})
+        |> EdgeDB.Client.State.with_config(query_execution_timeout: duration)
 
       Application.put_env(:edgedb, :client_state, state)
       on_exit(fn -> Application.delete_env(:edgedb, :client_state) end)