From: Alexander <alex.strizhakov@gmail.com>
Date: Fri, 6 Dec 2019 12:12:56 +0000 (+0300)
Subject: partially settings update
X-Git-Url: https://git.squeep.com/?a=commitdiff_plain;h=5cacb988b99347b228a30743fbcf310c9479b3f9;p=akkoma

partially settings update
---

diff --git a/docs/API/admin_api.md b/docs/API/admin_api.md
index 851c526d6..dff12db56 100644
--- a/docs/API/admin_api.md
+++ b/docs/API/admin_api.md
@@ -764,6 +764,41 @@ Most of the settings will be applied in `runtime`, this means that you don't nee
 [subkey2: val2] \\ value after deletion
 ```
 
+*Most of the settings can be partially updated through merge old values with new values, except settings value of which is list or is not keyword.*
+
+Example of setting without keyword in value:
+```elixir
+config :tesla, :adapter, Tesla.Adapter.Hackney
+```
+
+List of settings which have list in value:
+```elixir
+@full_key_update [
+    {:pleroma, :ecto_repos},
+    {:quack, :meta},
+    {:mime, :types},
+    {:cors_plug, [:max_age, :methods, :expose, :headers]},
+    {:auto_linker, :opts},
+    {:swarm, :node_blacklist}
+  ]
+```
+
+*Settings without explicit key must be sended in separate config object params.*
+```elixir
+config :quack,
+  level: :debug,
+  meta: [:all],
+  ...
+```
+```json
+{
+  configs: [
+    {"group": ":quack", "key": ":level", "value": ":debug"},
+    {"group": ":quack", "key": ":meta", "value": [":all"]},
+    ...
+  ]
+}
+```
 - Request:
 
 ```json
diff --git a/lib/pleroma/web/admin_api/config.ex b/lib/pleroma/web/admin_api/config.ex
index a74acfbc6..e141a13da 100644
--- a/lib/pleroma/web/admin_api/config.ex
+++ b/lib/pleroma/web/admin_api/config.ex
@@ -46,14 +46,48 @@ defmodule Pleroma.Web.AdminAPI.Config do
     |> Repo.update()
   end
 
+  @full_key_update [
+    {:pleroma, :ecto_repos},
+    {:quack, :meta},
+    {:mime, :types},
+    {:cors_plug, [:max_age, :methods, :expose, :headers]},
+    {:auto_linker, :opts},
+    {:swarm, :node_blacklist}
+  ]
+
+  defp only_full_update?(%Config{} = config) do
+    config_group = Config.from_string(config.group)
+    config_key = Config.from_string(config.key)
+
+    Enum.any?(@full_key_update, fn
+      {group, key} when is_list(key) ->
+        config_group == group and config_key in key
+
+      {group, key} ->
+        config_group == group and config_key == key
+    end)
+  end
+
+  defp can_be_partially_updated?(%Config{} = config), do: not only_full_update?(config)
+
   @spec update_or_create(map()) :: {:ok, Config.t()} | {:error, Changeset.t()}
   def update_or_create(params) do
     search_opts = Map.take(params, [:group, :key])
 
-    with %Config{} = config <- Config.get_by_params(search_opts) do
-      Config.update(config, params)
+    with %Config{} = config <- Config.get_by_params(search_opts),
+         {:partial_update, true, config} <-
+           {:partial_update, can_be_partially_updated?(config), config},
+         old_value <- from_binary(config.value),
+         transformed_value <- do_transform(params[:value]),
+         {:can_be_merged, true, config} <- {:can_be_merged, is_list(transformed_value), config},
+         new_value <- Keyword.merge(old_value, transformed_value) do
+      Config.update(config, %{value: new_value, transformed?: true})
     else
-      nil -> Config.create(params)
+      {reason, false, config} when reason in [:partial_update, :can_be_merged] ->
+        Config.update(config, params)
+
+      nil ->
+        Config.create(params)
     end
   end
 
@@ -63,7 +97,7 @@ defmodule Pleroma.Web.AdminAPI.Config do
 
     with %Config{} = config <- Config.get_by_params(search_opts),
          {config, sub_keys} when is_list(sub_keys) <- {config, params[:subkeys]},
-         old_value <- :erlang.binary_to_term(config.value),
+         old_value <- from_binary(config.value),
          keys <- Enum.map(sub_keys, &do_transform_string(&1)),
          new_value <- Keyword.drop(old_value, keys) do
       Config.update(config, %{value: new_value})
@@ -129,10 +163,13 @@ defmodule Pleroma.Web.AdminAPI.Config do
   def transform(entity) when is_binary(entity) or is_map(entity) or is_list(entity) do
     entity
     |> do_transform()
-    |> :erlang.term_to_binary()
+    |> to_binary()
   end
 
-  def transform(entity), do: :erlang.term_to_binary(entity)
+  def transform(entity), do: to_binary(entity)
+
+  @spec to_binary(any()) :: binary()
+  def to_binary(entity), do: :erlang.term_to_binary(entity)
 
   defp do_transform(%Regex{} = entity), do: entity
 
diff --git a/test/support/factory.ex b/test/support/factory.ex
index a7aa54f73..c16cbc9d7 100644
--- a/test/support/factory.ex
+++ b/test/support/factory.ex
@@ -377,7 +377,13 @@ defmodule Pleroma.Factory do
 
   def config_factory do
     %Pleroma.Web.AdminAPI.Config{
-      key: sequence(:key, &":some_key_#{&1}"),
+      key:
+        sequence(:key, fn key ->
+          # Atom dynamic registration hack in tests
+          "some_key_#{key}"
+          |> String.to_atom()
+          |> inspect()
+        end),
       group: ":pleroma",
       value:
         sequence(
diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs
index 1372edcab..e2e10d3f8 100644
--- a/test/web/admin_api/admin_api_controller_test.exs
+++ b/test/web/admin_api/admin_api_controller_test.exs
@@ -1979,6 +1979,7 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do
         Application.delete_env(:pleroma, :keyaa2)
         Application.delete_env(:pleroma, Pleroma.Web.Endpoint.NotReal)
         Application.delete_env(:pleroma, Pleroma.Captcha.NotReal)
+        Application.put_env(:tesla, :adapter, Tesla.Mock)
         :ok = File.rm("config/test.exported_from_db.secret.exs")
       end)
 
@@ -2141,14 +2142,64 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do
       assert Application.get_env(:quack, :webhook_url) == "https://hooks.slack.com/services/KEY"
     end
 
+    test "saving config with partial update", %{conn: conn} do
+      config = insert(:config, key: ":key1", value: :erlang.term_to_binary(key1: 1, key2: 2))
+
+      conn =
+        post(conn, "/api/pleroma/admin/config", %{
+          configs: [
+            %{group: config.group, key: config.key, value: [%{"tuple" => [":key3", 3]}]}
+          ]
+        })
+
+      assert json_response(conn, 200) == %{
+               "configs" => [
+                 %{
+                   "group" => ":pleroma",
+                   "key" => ":key1",
+                   "value" => [
+                     %{"tuple" => [":key1", 1]},
+                     %{"tuple" => [":key2", 2]},
+                     %{"tuple" => [":key3", 3]}
+                   ]
+                 }
+               ]
+             }
+    end
+
+    test "saving full setting if value is not keyword", %{conn: conn} do
+      config =
+        insert(:config,
+          group: ":tesla",
+          key: ":adapter",
+          value: :erlang.term_to_binary(Tesla.Adapter.Hackey)
+        )
+
+      conn =
+        post(conn, "/api/pleroma/admin/config", %{
+          configs: [
+            %{group: config.group, key: config.key, value: "Tesla.Adapter.Httpc"}
+          ]
+        })
+
+      assert json_response(conn, 200) == %{
+               "configs" => [
+                 %{
+                   "group" => ":tesla",
+                   "key" => ":adapter",
+                   "value" => "Tesla.Adapter.Httpc"
+                 }
+               ]
+             }
+    end
+
     test "update config setting & delete", %{conn: conn} do
       config1 = insert(:config, key: ":keyaa1")
       config2 = insert(:config, key: ":keyaa2")
 
       insert(:config,
         group: "ueberauth",
-        key: "Ueberauth.Strategy.Microsoft.OAuth",
-        value: :erlang.term_to_binary([])
+        key: "Ueberauth.Strategy.Microsoft.OAuth"
       )
 
       conn =
diff --git a/test/web/admin_api/config_test.exs b/test/web/admin_api/config_test.exs
index bff31bb85..c37eff092 100644
--- a/test/web/admin_api/config_test.exs
+++ b/test/web/admin_api/config_test.exs
@@ -26,26 +26,92 @@ defmodule Pleroma.Web.AdminAPI.ConfigTest do
     assert loaded == updated
   end
 
-  test "update_or_create/1" do
-    config = insert(:config)
-    key2 = "another_key"
+  describe "update_or_create/1" do
+    test "common" do
+      config = insert(:config)
+      key2 = "another_key"
+
+      params = [
+        %{group: "pleroma", key: key2, value: "another_value"},
+        %{group: config.group, key: config.key, value: "new_value"}
+      ]
+
+      assert Repo.all(Config) |> length() == 1
 
-    params = [
-      %{group: "pleroma", key: key2, value: "another_value"},
-      %{group: config.group, key: config.key, value: "new_value"}
-    ]
+      Enum.each(params, &Config.update_or_create(&1))
 
-    assert Repo.all(Config) |> length() == 1
+      assert Repo.all(Config) |> length() == 2
 
-    Enum.each(params, &Config.update_or_create(&1))
+      config1 = Config.get_by_params(%{group: config.group, key: config.key})
+      config2 = Config.get_by_params(%{group: "pleroma", key: key2})
 
-    assert Repo.all(Config) |> length() == 2
+      assert config1.value == Config.transform("new_value")
+      assert config2.value == Config.transform("another_value")
+    end
+
+    test "partial update" do
+      config = insert(:config, value: Config.to_binary(key1: "val1", key2: :val2))
 
-    config1 = Config.get_by_params(%{group: config.group, key: config.key})
-    config2 = Config.get_by_params(%{group: "pleroma", key: key2})
+      {:ok, _config} =
+        Config.update_or_create(%{
+          group: config.group,
+          key: config.key,
+          value: [key1: :val1, key3: :val3]
+        })
 
-    assert config1.value == Config.transform("new_value")
-    assert config2.value == Config.transform("another_value")
+      updated = Config.get_by_params(%{group: config.group, key: config.key})
+
+      value = Config.from_binary(updated.value)
+      assert length(value) == 3
+      assert value[:key1] == :val1
+      assert value[:key2] == :val2
+      assert value[:key3] == :val3
+    end
+
+    test "only full update for some keys" do
+      config1 = insert(:config, key: ":ecto_repos", value: Config.to_binary(repo: Pleroma.Repo))
+      config2 = insert(:config, group: ":cors_plug", key: ":max_age", value: Config.to_binary(18))
+
+      {:ok, _config} =
+        Config.update_or_create(%{
+          group: config1.group,
+          key: config1.key,
+          value: [another_repo: [Pleroma.Repo]]
+        })
+
+      {:ok, _config} =
+        Config.update_or_create(%{
+          group: config2.group,
+          key: config2.key,
+          value: 777
+        })
+
+      updated1 = Config.get_by_params(%{group: config1.group, key: config1.key})
+      updated2 = Config.get_by_params(%{group: config2.group, key: config2.key})
+
+      assert Config.from_binary(updated1.value) == [another_repo: [Pleroma.Repo]]
+      assert Config.from_binary(updated2.value) == 777
+    end
+
+    test "full update if value is not keyword" do
+      config =
+        insert(:config,
+          group: ":tesla",
+          key: ":adapter",
+          value: Config.to_binary(Tesla.Adapter.Hackney)
+        )
+
+      {:ok, _config} =
+        Config.update_or_create(%{
+          group: config.group,
+          key: config.key,
+          value: Tesla.Adapter.Httpc
+        })
+
+      updated = Config.get_by_params(%{group: config.group, key: config.key})
+
+      assert Config.from_binary(updated.value) == Tesla.Adapter.Httpc
+    end
   end
 
   test "delete/1" do