full update for some subkeys
authorAlexander Strizhakov <alex.strizhakov@gmail.com>
Sat, 18 Jan 2020 09:25:56 +0000 (12:25 +0300)
committerAlexander Strizhakov <alex.strizhakov@gmail.com>
Sat, 18 Jan 2020 09:25:56 +0000 (12:25 +0300)
config/config.exs
lib/pleroma/config/config_db.ex
lib/pleroma/config/transfer_task.ex
lib/pleroma/web/admin_api/admin_api_controller.ex
test/config/config_db_test.exs
test/config/transfer_task_test.exs
test/web/admin_api/admin_api_controller_test.exs

index 0a8f1465c9c6937f274ed1e16639ef0662e561e0..12105e672907948fc23ca3c5edc8eb60f9c8c9ee 100644 (file)
@@ -112,7 +112,6 @@ config :pleroma, :emoji,
   shortcode_globs: ["/emoji/custom/**/*.png"],
   pack_extensions: [".png", ".gif"],
   groups: [
-    # Put groups that have higher priority than defaults here. Example in `docs/config/custom_emoji.md`
     Custom: ["/emoji/*.png", "/emoji/**/*.png"]
   ],
   default_manifest: "https://git.pleroma.social/pleroma/emoji-index/raw/master/index.json",
index 93b02370caa42e57cb647eb9d233f2b24b8deee1..85a049f8a00d8907d20f1ffe3d00eae2a9f0b534 100644 (file)
@@ -69,7 +69,7 @@ defmodule Pleroma.ConfigDB do
     |> get_db_keys(config.key)
   end
 
-  @spec get_db_keys(keyword() | any()) :: [String.t()]
+  @spec get_db_keys(keyword(), any()) :: [String.t()]
   def get_db_keys(value, key) do
     if Keyword.keyword?(value) do
       value |> Keyword.keys() |> Enum.map(&convert(&1))
@@ -78,9 +78,57 @@ defmodule Pleroma.ConfigDB do
     end
   end
 
+  @full_subkey_update [
+    {:pleroma, :assets, :mascots},
+    {:pleroma, :emoji, :groups},
+    {:pleroma, :workers, :retries},
+    {:pleroma, :mrf_subchain, :match_actor},
+    {:pleroma, :mrf_keyword, :replace}
+  ]
+
+  @spec deep_merge(atom(), atom(), keyword(), keyword()) :: keyword()
+  def deep_merge(group, key, old_value, new_value) do
+    old_keys =
+      old_value
+      |> Keyword.keys()
+      |> MapSet.new()
+
+    new_keys =
+      new_value
+      |> Keyword.keys()
+      |> MapSet.new()
+
+    intersect_keys = old_keys |> MapSet.intersection(new_keys) |> MapSet.to_list()
+
+    subkeys = sub_key_full_update(group, key, intersect_keys)
+
+    merged_value = DeepMerge.deep_merge(old_value, new_value)
+
+    Enum.reduce(subkeys, merged_value, fn subkey, acc ->
+      Keyword.put(acc, subkey, new_value[subkey])
+    end)
+  end
+
+  @spec sub_key_full_update?(atom(), atom(), [Keyword.key()]) :: boolean()
+  def sub_key_full_update?(group, key, subkeys) do
+    Enum.any?(@full_subkey_update, fn {g, k, subkey} ->
+      g == group and k == key and subkey in subkeys
+    end)
+  end
+
+  defp sub_key_full_update(group, key, subkeys) do
+    Enum.map(@full_subkey_update, fn
+      {g, k, subkey} when g == group and k == key ->
+        if subkey in subkeys, do: subkey, else: []
+
+      _ ->
+        []
+    end)
+    |> List.flatten()
+  end
+
   @full_key_update [
     {:pleroma, :ecto_repos},
-    {:pleroma, :assets},
     {:quack, :meta},
     {:mime, :types},
     {:cors_plug, [:max_age, :methods, :expose, :headers]},
@@ -114,8 +162,14 @@ defmodule Pleroma.ConfigDB do
          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 <- DeepMerge.deep_merge(old_value, transformed_value) do
-      ConfigDB.update(config, %{value: new_value, transformed?: true})
+         new_value <-
+           deep_merge(
+             ConfigDB.from_string(config.group),
+             ConfigDB.from_string(config.key),
+             old_value,
+             transformed_value
+           ) do
+      ConfigDB.update(config, %{value: new_value})
     else
       {reason, false, config} when reason in [:partial_update, :can_be_merged] ->
         ConfigDB.update(config, params)
@@ -236,6 +290,9 @@ defmodule Pleroma.ConfigDB do
 
   def transform(entity), do: to_binary(entity)
 
+  @spec transform_with_out_binary(any()) :: any()
+  def transform_with_out_binary(entity), do: do_transform(entity)
+
   @spec to_binary(any()) :: binary()
   def to_binary(entity), do: :erlang.term_to_binary(entity)
 
index 6e651c48ba0e72293f9cf8349205f95840c19421..9b571d5d9b06f77b5fbca91f20b12e299c476000 100644 (file)
@@ -42,7 +42,7 @@ defmodule Pleroma.Config.TransferTask do
 
         merged_value =
           if can_be_merged?(default, value) do
-            DeepMerge.deep_merge(default, value)
+            ConfigDB.deep_merge(group, key, default, value)
           else
             value
           end
index 7572a6b658a1c6c3314fc403f777e15aee7f4821..cc658dc0e334f83789239f48434c7a11b37f16e2 100644 (file)
@@ -834,10 +834,20 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do
                   ConfigDB.get_db_keys(value, key)
                 end
 
+              db_value = configs[group][key]
+
+              merged_value =
+                if !is_nil(db_value) and Keyword.keyword?(db_value) and
+                     ConfigDB.sub_key_full_update?(group, key, Keyword.keys(db_value)) do
+                  ConfigDB.deep_merge(group, key, value, db_value)
+                else
+                  value
+                end
+
               setting = %{
                 group: ConfigDB.convert(group),
                 key: ConfigDB.convert(key),
-                value: ConfigDB.convert(value)
+                value: ConfigDB.convert(merged_value)
               }
 
               if db, do: Map.put(setting, :db, db), else: setting
index 7668bc5473fd270a7e0d9ffa9bb1f07fbcebc0c8..19619620eadf20fc7bcf8df73fd80d64120cc1f5 100644 (file)
@@ -155,6 +155,40 @@ defmodule Pleroma.ConfigDBTest do
 
       assert ConfigDB.from_binary(updated.value) == Tesla.Adapter.Httpc
     end
+
+    test "only full update for some subkeys" do
+      config1 =
+        insert(:config,
+          key: ":emoji",
+          value: ConfigDB.to_binary(groups: [a: 1, b: 2], key: [a: 1])
+        )
+
+      config2 =
+        insert(:config,
+          key: ":assets",
+          value: ConfigDB.to_binary(mascots: [a: 1, b: 2], key: [a: 1])
+        )
+
+      {:ok, _config} =
+        ConfigDB.update_or_create(%{
+          group: config1.group,
+          key: config1.key,
+          value: [groups: [c: 3, d: 4], key: [b: 2]]
+        })
+
+      {:ok, _config} =
+        ConfigDB.update_or_create(%{
+          group: config2.group,
+          key: config2.key,
+          value: [mascots: [c: 3, d: 4], key: [b: 2]]
+        })
+
+      updated1 = ConfigDB.get_by_params(%{group: config1.group, key: config1.key})
+      updated2 = ConfigDB.get_by_params(%{group: config2.group, key: config2.key})
+
+      assert ConfigDB.from_binary(updated1.value) == [groups: [c: 3, d: 4], key: [a: 1, b: 2]]
+      assert ConfigDB.from_binary(updated2.value) == [mascots: [c: 3, d: 4], key: [a: 1, b: 2]]
+    end
   end
 
   test "delete/1" do
index 37bea20a3439f5d49f1cb1344889fa1125210ac7..20dc068636dc570db401ef72bef976103f4c98e6 100644 (file)
@@ -5,6 +5,7 @@
 defmodule Pleroma.Config.TransferTaskTest do
   use Pleroma.DataCase
 
+  alias Pleroma.Config.TransferTask
   alias Pleroma.ConfigDB
 
   clear_config(:configurable_from_database) do
@@ -34,7 +35,7 @@ defmodule Pleroma.Config.TransferTaskTest do
       value: [:test_value1, :test_value2]
     })
 
-    Pleroma.Config.TransferTask.start_link([])
+    TransferTask.start_link([])
 
     assert Application.get_env(:pleroma, :test_key) == [live: 2, com: 3]
     assert Application.get_env(:idna, :test_key) == [live: 15, com: 35]
@@ -63,7 +64,7 @@ defmodule Pleroma.Config.TransferTaskTest do
       value: [:none]
     })
 
-    Pleroma.Config.TransferTask.start_link([])
+    TransferTask.start_link([])
 
     assert Application.get_env(:quack, :level) == :info
     assert Application.get_env(:quack, :meta) == [:none]
@@ -76,6 +77,35 @@ defmodule Pleroma.Config.TransferTaskTest do
     end)
   end
 
+  test "transfer config values with full subkey update" do
+    emoji = Application.get_env(:pleroma, :emoji)
+    assets = Application.get_env(:pleroma, :assets)
+
+    ConfigDB.create(%{
+      group: ":pleroma",
+      key: ":emoji",
+      value: [groups: [a: 1, b: 2]]
+    })
+
+    ConfigDB.create(%{
+      group: ":pleroma",
+      key: ":assets",
+      value: [mascots: [a: 1, b: 2]]
+    })
+
+    TransferTask.start_link([])
+
+    emoji_env = Application.get_env(:pleroma, :emoji)
+    assert emoji_env[:groups] == [a: 1, b: 2]
+    assets_env = Application.get_env(:pleroma, :assets)
+    assert assets_env[:mascots] == [a: 1, b: 2]
+
+    on_exit(fn ->
+      Application.put_env(:pleroma, :emoji, emoji)
+      Application.put_env(:pleroma, :assets, assets)
+    end)
+  end
+
   test "non existing atom" do
     ConfigDB.create(%{
       group: ":pleroma",
@@ -84,7 +114,7 @@ defmodule Pleroma.Config.TransferTaskTest do
     })
 
     assert ExUnit.CaptureLog.capture_log(fn ->
-             Pleroma.Config.TransferTask.start_link([])
+             TransferTask.start_link([])
            end) =~
              "updating env causes error, group: \":pleroma\", key: \":undefined_atom_key\", value: [live: 2, com: 3], error: %ArgumentError{message: \"argument error\"}"
   end
index 2a0261b0e002547c614ee05cd684f51c8434f9c4..cfa6207c24061e3ed79b374057b2cc366bd20be7 100644 (file)
@@ -1916,9 +1916,10 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do
           value: ConfigDB.to_binary(k1: :v1, k2: :v2)
         )
 
-      conn = get(conn, "/api/pleroma/admin/config")
-
-      %{"configs" => configs} = json_response(conn, 200)
+      %{"configs" => configs} =
+        conn
+        |> get("/api/pleroma/admin/config")
+        |> json_response(200)
 
       assert length(configs) > 3
 
@@ -1945,6 +1946,36 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do
                ]
       end)
     end
+
+    test "subkeys with full update right merge", %{conn: conn} do
+      config1 =
+        insert(:config,
+          key: ":emoji",
+          value: ConfigDB.to_binary(groups: [a: 1, b: 2], key: [a: 1])
+        )
+
+      config2 =
+        insert(:config,
+          key: ":assets",
+          value: ConfigDB.to_binary(mascots: [a: 1, b: 2], key: [a: 1])
+        )
+
+      %{"configs" => configs} =
+        conn
+        |> get("/api/pleroma/admin/config")
+        |> json_response(200)
+
+      [%{"key" => ":emoji", "value" => emoji_val}, %{"key" => ":assets", "value" => assets_val}] =
+        Enum.filter(configs, fn %{"group" => group, "key" => key} ->
+          group == ":pleroma" and key in [config1.key, config2.key]
+        end)
+
+      emoji_val = ConfigDB.transform_with_out_binary(emoji_val)
+      assets_val = ConfigDB.transform_with_out_binary(assets_val)
+
+      assert emoji_val[:groups] == [a: 1, b: 2]
+      assert assets_val[:mascots] == [a: 1, b: 2]
+    end
   end
 
   test "POST /api/pleroma/admin/config error", %{conn: conn} do