From 982cad0268898851ff87187eaf0d0b7011b1c96a Mon Sep 17 00:00:00 2001
From: Alex S <alex.strizhakov@gmail.com>
Date: Sun, 23 Jun 2019 08:16:16 +0300
Subject: [PATCH] support for config groups

---
 docs/api/admin_api.md                         |  8 +++--
 lib/mix/tasks/pleroma/config.ex               |  6 ++--
 lib/pleroma/config/transfer_task.ex           | 19 ++++++++++--
 .../web/admin_api/admin_api_controller.ex     |  8 ++---
 lib/pleroma/web/admin_api/config.ex           | 29 ++++++++++--------
 .../web/admin_api/views/config_view.ex        |  1 +
 ...20190622151019_add_group_key_to_config.exs | 12 ++++++++
 test/config/transfer_task_test.exs            | 22 ++++++++++++--
 test/support/factory.ex                       |  1 +
 test/tasks/config_test.exs                    | 19 ++++++++----
 .../admin_api/admin_api_controller_test.exs   | 30 +++++++++++++++++--
 test/web/admin_api/config_test.exs            | 20 ++++++-------
 12 files changed, 131 insertions(+), 44 deletions(-)
 create mode 100644 priv/repo/migrations/20190622151019_add_group_key_to_config.exs

diff --git a/docs/api/admin_api.md b/docs/api/admin_api.md
index 63af33821..c05a353d7 100644
--- a/docs/api/admin_api.md
+++ b/docs/api/admin_api.md
@@ -568,8 +568,9 @@ Note: Available `:permission_group` is currently moderator and admin. 404 is ret
 {
   configs: [
     {
+      "group": string,
       "key": string,
-      "value": string or {} or []
+      "value": string or {} or [] or {"tuple": []}
      }
   ]
 }
@@ -597,8 +598,9 @@ Compile time settings (need instance reboot):
 - Method `POST`
 - Params:
   - `configs` => [
+    - `group` (string)
     - `key` (string)
-    - `value` (string, [], {})
+    - `value` (string, [], {} or {"tuple": []})
     - `delete` = true (optional, if parameter must be deleted)
   ]
 
@@ -608,6 +610,7 @@ Compile time settings (need instance reboot):
 {
   configs: [
     {
+      "group": "pleroma",
       "key": "Pleroma.Upload",
       "value": {
         "uploader": "Pleroma.Uploaders.Local",
@@ -636,6 +639,7 @@ Compile time settings (need instance reboot):
 {
   configs: [
     {
+      "group": string,
       "key": string,
       "value": string or {} or [] or {"tuple": []}
      }
diff --git a/lib/mix/tasks/pleroma/config.ex b/lib/mix/tasks/pleroma/config.ex
index cc5425362..4ed2c9789 100644
--- a/lib/mix/tasks/pleroma/config.ex
+++ b/lib/mix/tasks/pleroma/config.ex
@@ -24,7 +24,7 @@ def run(["migrate_to_db"]) do
       |> Enum.reject(fn {k, _v} -> k in [Pleroma.Repo, :env] end)
       |> Enum.each(fn {k, v} ->
         key = to_string(k) |> String.replace("Elixir.", "")
-        {:ok, _} = Config.update_or_create(%{key: key, value: v})
+        {:ok, _} = Config.update_or_create(%{group: "pleroma", key: key, value: v})
         Mix.shell().info("#{key} is migrated.")
       end)
 
@@ -51,7 +51,9 @@ def run(["migrate_from_db", env]) do
 
         IO.write(
           file,
-          "config :pleroma, #{config.key}#{mark} #{inspect(Config.from_binary(config.value))}\r\n"
+          "config :#{config.group}, #{config.key}#{mark} #{
+            inspect(Config.from_binary(config.value))
+          }\r\n"
         )
 
         {:ok, _} = Repo.delete(config)
diff --git a/lib/pleroma/config/transfer_task.ex b/lib/pleroma/config/transfer_task.ex
index a8cbfa52a..cf880aa22 100644
--- a/lib/pleroma/config/transfer_task.ex
+++ b/lib/pleroma/config/transfer_task.ex
@@ -11,8 +11,17 @@ def start_link do
   def load_and_update_env do
     if Pleroma.Config.get([:instance, :dynamic_configuration]) and
          Ecto.Adapters.SQL.table_exists?(Pleroma.Repo, "config") do
-      Pleroma.Repo.all(Config)
-      |> Enum.each(&update_env(&1))
+      for_restart =
+        Pleroma.Repo.all(Config)
+        |> Enum.map(&update_env(&1))
+
+      # We need to restart applications for loaded settings take effect
+      for_restart
+      |> Enum.reject(&(&1 in [:pleroma, :ok]))
+      |> Enum.each(fn app ->
+        Application.stop(app)
+        :ok = Application.start(app)
+      end)
     end
   end
 
@@ -25,11 +34,15 @@ defp update_env(setting) do
           setting.key
         end
 
+      group = String.to_existing_atom(setting.group)
+
       Application.put_env(
-        :pleroma,
+        group,
         String.to_existing_atom(key),
         Config.from_binary(setting.value)
       )
+
+      group
     rescue
       e ->
         require Logger
diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex
index 03dfdca82..953a22ea0 100644
--- a/lib/pleroma/web/admin_api/admin_api_controller.ex
+++ b/lib/pleroma/web/admin_api/admin_api_controller.ex
@@ -377,12 +377,12 @@ def config_update(conn, %{"configs" => configs}) do
       if Pleroma.Config.get([:instance, :dynamic_configuration]) do
         updated =
           Enum.map(configs, fn
-            %{"key" => key, "value" => value} ->
-              {:ok, config} = Config.update_or_create(%{key: key, value: value})
+            %{"group" => group, "key" => key, "value" => value} ->
+              {:ok, config} = Config.update_or_create(%{group: group, key: key, value: value})
               config
 
-            %{"key" => key, "delete" => "true"} ->
-              {:ok, _} = Config.delete(key)
+            %{"group" => group, "key" => key, "delete" => "true"} ->
+              {:ok, _} = Config.delete(%{group: group, key: key})
               nil
           end)
           |> Enum.reject(&is_nil(&1))
diff --git a/lib/pleroma/web/admin_api/config.ex b/lib/pleroma/web/admin_api/config.ex
index 2e149bf25..8b9b658a9 100644
--- a/lib/pleroma/web/admin_api/config.ex
+++ b/lib/pleroma/web/admin_api/config.ex
@@ -12,26 +12,27 @@ defmodule Pleroma.Web.AdminAPI.Config do
 
   schema "config" do
     field(:key, :string)
+    field(:group, :string)
     field(:value, :binary)
 
     timestamps()
   end
 
-  @spec get_by_key(String.t()) :: Config.t() | nil
-  def get_by_key(key), do: Repo.get_by(Config, key: key)
+  @spec get_by_params(map()) :: Config.t() | nil
+  def get_by_params(params), do: Repo.get_by(Config, params)
 
   @spec changeset(Config.t(), map()) :: Changeset.t()
   def changeset(config, params \\ %{}) do
     config
-    |> cast(params, [:key, :value])
-    |> validate_required([:key, :value])
-    |> unique_constraint(:key)
+    |> cast(params, [:key, :group, :value])
+    |> validate_required([:key, :group, :value])
+    |> unique_constraint(:key, name: :config_group_key_index)
   end
 
   @spec create(map()) :: {:ok, Config.t()} | {:error, Changeset.t()}
-  def create(%{key: key, value: value}) do
+  def create(params) do
     %Config{}
-    |> changeset(%{key: key, value: transform(value)})
+    |> changeset(Map.put(params, :value, transform(params[:value])))
     |> Repo.insert()
   end
 
@@ -43,20 +44,20 @@ def update(%Config{} = config, %{value: value}) do
   end
 
   @spec update_or_create(map()) :: {:ok, Config.t()} | {:error, Changeset.t()}
-  def update_or_create(%{key: key} = params) do
-    with %Config{} = config <- Config.get_by_key(key) do
+  def update_or_create(params) do
+    with %Config{} = config <- Config.get_by_params(Map.take(params, [:group, :key])) do
       Config.update(config, params)
     else
       nil -> Config.create(params)
     end
   end
 
-  @spec delete(String.t()) :: {:ok, Config.t()} | {:error, Changeset.t()}
-  def delete(key) do
-    with %Config{} = config <- Config.get_by_key(key) do
+  @spec delete(map()) :: {:ok, Config.t()} | {:error, Changeset.t()}
+  def delete(params) do
+    with %Config{} = config <- Config.get_by_params(params) do
       Repo.delete(config)
     else
-      nil -> {:error, "Config with key #{key} not found"}
+      nil -> {:error, "Config with params #{inspect(params)} not found"}
     end
   end
 
@@ -90,6 +91,8 @@ defp do_convert(value) when is_atom(value) do
   end
 
   @spec transform(any()) :: binary()
+  def transform(%{"tuple" => _} = entity), do: :erlang.term_to_binary(do_transform(entity))
+
   def transform(entity) when is_map(entity) do
     tuples =
       for {k, v} <- entity,
diff --git a/lib/pleroma/web/admin_api/views/config_view.ex b/lib/pleroma/web/admin_api/views/config_view.ex
index c8560033e..3ccc9ca46 100644
--- a/lib/pleroma/web/admin_api/views/config_view.ex
+++ b/lib/pleroma/web/admin_api/views/config_view.ex
@@ -10,6 +10,7 @@ def render("index.json", %{configs: configs}) do
   def render("show.json", %{config: config}) do
     %{
       key: config.key,
+      group: config.group,
       value: Pleroma.Web.AdminAPI.Config.from_binary_to_map(config.value)
     }
   end
diff --git a/priv/repo/migrations/20190622151019_add_group_key_to_config.exs b/priv/repo/migrations/20190622151019_add_group_key_to_config.exs
new file mode 100644
index 000000000..d7a3785d0
--- /dev/null
+++ b/priv/repo/migrations/20190622151019_add_group_key_to_config.exs
@@ -0,0 +1,12 @@
+defmodule Pleroma.Repo.Migrations.AddGroupKeyToConfig do
+  use Ecto.Migration
+
+  def change do
+    alter table("config") do
+      add(:group, :string)
+    end
+
+    drop(unique_index("config", :key))
+    create(unique_index("config", [:group, :key]))
+  end
+end
diff --git a/test/config/transfer_task_test.exs b/test/config/transfer_task_test.exs
index 9b8a8dd45..c0e433263 100644
--- a/test/config/transfer_task_test.exs
+++ b/test/config/transfer_task_test.exs
@@ -13,19 +13,37 @@ defmodule Pleroma.Config.TransferTaskTest do
 
   test "transfer config values from db to env" do
     refute Application.get_env(:pleroma, :test_key)
-    Pleroma.Web.AdminAPI.Config.create(%{key: "test_key", value: [live: 2, com: 3]})
+    refute Application.get_env(:idna, :test_key)
+
+    Pleroma.Web.AdminAPI.Config.create(%{
+      group: "pleroma",
+      key: "test_key",
+      value: [live: 2, com: 3]
+    })
+
+    Pleroma.Web.AdminAPI.Config.create(%{
+      group: "idna",
+      key: "test_key",
+      value: [live: 15, com: 35]
+    })
 
     Pleroma.Config.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]
 
     on_exit(fn ->
       Application.delete_env(:pleroma, :test_key)
+      Application.delete_env(:idna, :test_key)
     end)
   end
 
   test "non existing atom" do
-    Pleroma.Web.AdminAPI.Config.create(%{key: "undefined_atom_key", value: [live: 2, com: 3]})
+    Pleroma.Web.AdminAPI.Config.create(%{
+      group: "pleroma",
+      key: "undefined_atom_key",
+      value: [live: 2, com: 3]
+    })
 
     assert ExUnit.CaptureLog.capture_log(fn ->
              Pleroma.Config.TransferTask.start_link()
diff --git a/test/support/factory.ex b/test/support/factory.ex
index 5be34660e..c2812e8f7 100644
--- a/test/support/factory.ex
+++ b/test/support/factory.ex
@@ -314,6 +314,7 @@ def registration_factory do
   def config_factory do
     %Pleroma.Web.AdminAPI.Config{
       key: sequence(:key, &"some_key_#{&1}"),
+      group: "pleroma",
       value:
         sequence(
           :value,
diff --git a/test/tasks/config_test.exs b/test/tasks/config_test.exs
index d448b0444..9c9a31bf4 100644
--- a/test/tasks/config_test.exs
+++ b/test/tasks/config_test.exs
@@ -30,17 +30,26 @@ test "settings are migrated to db" do
 
     Mix.Tasks.Pleroma.Config.run(["migrate_to_db"])
 
-    first_db = Config.get_by_key("first_setting")
-    second_db = Config.get_by_key("second_setting")
-    refute Config.get_by_key("Pleroma.Repo")
+    first_db = Config.get_by_params(%{group: "pleroma", key: "first_setting"})
+    second_db = Config.get_by_params(%{group: "pleroma", key: "second_setting"})
+    refute Config.get_by_params(%{group: "pleroma", key: "Pleroma.Repo"})
 
     assert Config.from_binary(first_db.value) == [key: "value", key2: [Pleroma.Repo]]
     assert Config.from_binary(second_db.value) == [key: "value2", key2: [Pleroma.Activity]]
   end
 
   test "settings are migrated to file and deleted from db", %{temp_file: temp_file} do
-    Config.create(%{key: "setting_first", value: [key: "value", key2: [Pleroma.Activity]]})
-    Config.create(%{key: "setting_second", value: [key: "valu2", key2: [Pleroma.Repo]]})
+    Config.create(%{
+      group: "pleroma",
+      key: "setting_first",
+      value: [key: "value", key2: [Pleroma.Activity]]
+    })
+
+    Config.create(%{
+      group: "pleroma",
+      key: "setting_second",
+      value: [key: "valu2", key2: [Pleroma.Repo]]
+    })
 
     Mix.Tasks.Pleroma.Config.run(["migrate_from_db", "temp"])
 
diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs
index 49889d6d7..4278ac59d 100644
--- a/test/web/admin_api/admin_api_controller_test.exs
+++ b/test/web/admin_api/admin_api_controller_test.exs
@@ -1363,8 +1363,9 @@ test "create new config setting in db", %{conn: conn} do
       conn =
         post(conn, "/api/pleroma/admin/config", %{
           configs: [
-            %{key: "key1", value: "value1"},
+            %{group: "pleroma", key: "key1", value: "value1"},
             %{
+              group: "pleroma",
               key: "key2",
               value: %{
                 "nested_1" => "nested_value1",
@@ -1375,6 +1376,7 @@ test "create new config setting in db", %{conn: conn} do
               }
             },
             %{
+              group: "pleroma",
               key: "key3",
               value: [
                 %{"nested_3" => ":nested_3", "nested_33" => "nested_33"},
@@ -1382,8 +1384,14 @@ test "create new config setting in db", %{conn: conn} do
               ]
             },
             %{
+              group: "pleroma",
               key: "key4",
               value: %{"nested_5" => ":upload", "endpoint" => "https://example.com"}
+            },
+            %{
+              group: "idna",
+              key: "key5",
+              value: %{"tuple" => ["string", "Pleroma.Captcha.NotReal", []]}
             }
           ]
         })
@@ -1391,10 +1399,12 @@ test "create new config setting in db", %{conn: conn} do
       assert json_response(conn, 200) == %{
                "configs" => [
                  %{
+                   "group" => "pleroma",
                    "key" => "key1",
                    "value" => "value1"
                  },
                  %{
+                   "group" => "pleroma",
                    "key" => "key2",
                    "value" => [
                      %{"nested_1" => "nested_value1"},
@@ -1407,6 +1417,7 @@ test "create new config setting in db", %{conn: conn} do
                    ]
                  },
                  %{
+                   "group" => "pleroma",
                    "key" => "key3",
                    "value" => [
                      [%{"nested_3" => "nested_3"}, %{"nested_33" => "nested_33"}],
@@ -1414,8 +1425,14 @@ test "create new config setting in db", %{conn: conn} do
                    ]
                  },
                  %{
+                   "group" => "pleroma",
                    "key" => "key4",
                    "value" => [%{"endpoint" => "https://example.com"}, %{"nested_5" => "upload"}]
+                 },
+                 %{
+                   "group" => "idna",
+                   "key" => "key5",
+                   "value" => %{"tuple" => ["string", "Pleroma.Captcha.NotReal", []]}
                  }
                ]
              }
@@ -1439,6 +1456,8 @@ test "create new config setting in db", %{conn: conn} do
                endpoint: "https://example.com",
                nested_5: :upload
              ]
+
+      assert Application.get_env(:idna, :key5) == {"string", Pleroma.Captcha.NotReal, []}
     end
 
     test "update config setting & delete", %{conn: conn} do
@@ -1448,14 +1467,15 @@ test "update config setting & delete", %{conn: conn} do
       conn =
         post(conn, "/api/pleroma/admin/config", %{
           configs: [
-            %{key: config1.key, value: "another_value"},
-            %{key: config2.key, delete: "true"}
+            %{group: config1.group, key: config1.key, value: "another_value"},
+            %{group: config2.group, key: config2.key, delete: "true"}
           ]
         })
 
       assert json_response(conn, 200) == %{
                "configs" => [
                  %{
+                   "group" => "pleroma",
                    "key" => config1.key,
                    "value" => "another_value"
                  }
@@ -1471,6 +1491,7 @@ test "common config example", %{conn: conn} do
         post(conn, "/api/pleroma/admin/config", %{
           configs: [
             %{
+              "group" => "pleroma",
               "key" => "Pleroma.Captcha.NotReal",
               "value" => %{
                 "enabled" => ":false",
@@ -1484,6 +1505,7 @@ test "common config example", %{conn: conn} do
       assert json_response(conn, 200) == %{
                "configs" => [
                  %{
+                   "group" => "pleroma",
                    "key" => "Pleroma.Captcha.NotReal",
                    "value" => [
                      %{"enabled" => false},
@@ -1500,6 +1522,7 @@ test "tuples with more than two values", %{conn: conn} do
         post(conn, "/api/pleroma/admin/config", %{
           configs: [
             %{
+              "group" => "pleroma",
               "key" => "Pleroma.Web.Endpoint.NotReal",
               "value" => [
                 %{
@@ -1557,6 +1580,7 @@ test "tuples with more than two values", %{conn: conn} do
       assert json_response(conn, 200) == %{
                "configs" => [
                  %{
+                   "group" => "pleroma",
                    "key" => "Pleroma.Web.Endpoint.NotReal",
                    "value" => [
                      %{
diff --git a/test/web/admin_api/config_test.exs b/test/web/admin_api/config_test.exs
index 39050c276..10cb3b68a 100644
--- a/test/web/admin_api/config_test.exs
+++ b/test/web/admin_api/config_test.exs
@@ -7,18 +7,18 @@ test "get_by_key/1" do
     config = insert(:config)
     insert(:config)
 
-    assert config == Config.get_by_key(config.key)
+    assert config == Config.get_by_params(%{group: config.group, key: config.key})
   end
 
   test "create/1" do
-    {:ok, config} = Config.create(%{key: "some_key", value: "some_value"})
-    assert config == Config.get_by_key("some_key")
+    {:ok, config} = Config.create(%{group: "pleroma", key: "some_key", value: "some_value"})
+    assert config == Config.get_by_params(%{group: "pleroma", key: "some_key"})
   end
 
   test "update/1" do
     config = insert(:config)
     {:ok, updated} = Config.update(config, %{value: "some_value"})
-    loaded = Config.get_by_key(config.key)
+    loaded = Config.get_by_params(%{group: config.group, key: config.key})
     assert loaded == updated
   end
 
@@ -27,8 +27,8 @@ test "update_or_create/1" do
     key2 = "another_key"
 
     params = [
-      %{key: key2, value: "another_value"},
-      %{key: config.key, value: "new_value"}
+      %{group: "pleroma", key: key2, value: "another_value"},
+      %{group: config.group, key: config.key, value: "new_value"}
     ]
 
     assert Repo.all(Config) |> length() == 1
@@ -37,8 +37,8 @@ test "update_or_create/1" do
 
     assert Repo.all(Config) |> length() == 2
 
-    config1 = Config.get_by_key(config.key)
-    config2 = Config.get_by_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")
@@ -46,8 +46,8 @@ test "update_or_create/1" do
 
   test "delete/1" do
     config = insert(:config)
-    {:ok, _} = Config.delete(config.key)
-    refute Config.get_by_key(config.key)
+    {:ok, _} = Config.delete(%{key: config.key, group: config.group})
+    refute Config.get_by_params(%{key: config.key, group: config.group})
   end
 
   describe "transform/1" do