From 5cacb988b99347b228a30743fbcf310c9479b3f9 Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 6 Dec 2019 15:12:56 +0300 Subject: [PATCH] partially settings update --- docs/API/admin_api.md | 35 +++++++ lib/pleroma/web/admin_api/config.ex | 49 ++++++++-- test/support/factory.ex | 8 +- .../admin_api/admin_api_controller_test.exs | 55 ++++++++++- test/web/admin_api/config_test.exs | 94 ++++++++++++++++--- 5 files changed, 218 insertions(+), 23 deletions(-) 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 @@ def update(%Config{} = config, %{value: value}) 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 @@ def delete(params) 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 @@ defp do_convert(entity) when is_binary(entity), do: entity 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 @@ def registration_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 @@ test "with settings in db", %{conn: conn} 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 @@ test "save config setting without key", %{conn: conn} 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 @@ test "update/1" 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"} - ] + params = [ + %{group: "pleroma", key: key2, value: "another_value"}, + %{group: config.group, key: config.key, value: "new_value"} + ] - assert Repo.all(Config) |> length() == 1 + assert Repo.all(Config) |> length() == 1 - Enum.each(params, &Config.update_or_create(&1)) + Enum.each(params, &Config.update_or_create(&1)) - assert Repo.all(Config) |> length() == 2 + assert Repo.all(Config) |> length() == 2 - config1 = Config.get_by_params(%{group: config.group, key: config.key}) - config2 = Config.get_by_params(%{group: "pleroma", key: key2}) + config1 = Config.get_by_params(%{group: config.group, key: config.key}) + config2 = Config.get_by_params(%{group: "pleroma", key: key2}) - assert config1.value == Config.transform("new_value") - assert config2.value == Config.transform("another_value") + 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)) + + {:ok, _config} = + Config.update_or_create(%{ + group: config.group, + key: config.key, + value: [key1: :val1, key3: :val3] + }) + + 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