From e69986169095796f2845c4f859234d96f91bf9ff Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Sat, 18 Jan 2020 12:25:56 +0300 Subject: [PATCH] full update for some subkeys --- config/config.exs | 1 - lib/pleroma/config/config_db.ex | 65 +++++++++++++++++-- lib/pleroma/config/transfer_task.ex | 2 +- .../web/admin_api/admin_api_controller.ex | 12 +++- test/config/config_db_test.exs | 34 ++++++++++ test/config/transfer_task_test.exs | 36 +++++++++- .../admin_api/admin_api_controller_test.exs | 37 ++++++++++- 7 files changed, 174 insertions(+), 13 deletions(-) diff --git a/config/config.exs b/config/config.exs index 0a8f1465c..12105e672 100644 --- a/config/config.exs +++ b/config/config.exs @@ -112,7 +112,6 @@ 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", diff --git a/lib/pleroma/config/config_db.ex b/lib/pleroma/config/config_db.ex index 93b02370c..85a049f8a 100644 --- a/lib/pleroma/config/config_db.ex +++ b/lib/pleroma/config/config_db.ex @@ -69,7 +69,7 @@ def get_db_keys(%ConfigDB{} = config) 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 @@ def get_db_keys(value, key) 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 @@ def update_or_create(params) 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 @@ def transform(entity) when is_binary(entity) or is_map(entity) or is_list(entity 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) diff --git a/lib/pleroma/config/transfer_task.ex b/lib/pleroma/config/transfer_task.ex index 6e651c48b..9b571d5d9 100644 --- a/lib/pleroma/config/transfer_task.ex +++ b/lib/pleroma/config/transfer_task.ex @@ -42,7 +42,7 @@ defp update_env(setting) 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 diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index 7572a6b65..cc658dc0e 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -834,10 +834,20 @@ def config_show(conn, _params) 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 diff --git a/test/config/config_db_test.exs b/test/config/config_db_test.exs index 7668bc547..19619620e 100644 --- a/test/config/config_db_test.exs +++ b/test/config/config_db_test.exs @@ -155,6 +155,40 @@ test "full update if value is not keyword" 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 diff --git a/test/config/transfer_task_test.exs b/test/config/transfer_task_test.exs index 37bea20a3..20dc06863 100644 --- a/test/config/transfer_task_test.exs +++ b/test/config/transfer_task_test.exs @@ -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 @@ test "transfer config values from db to env" 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 @@ test "transfer config values for 1 group and some keys" 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 @@ test "transfer config values for 1 group and some keys" 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 @@ test "non existing atom" 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 diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index 2a0261b0e..cfa6207c2 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -1916,9 +1916,10 @@ test "merged default setting with db settings", %{conn: conn} 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 @@ test "merged default setting with db settings", %{conn: conn} 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