[#3174] Refactoring: ConfigDB fetching functions, ConfigDB tests.

Minor fixes.

lib/mix/tasks/pleroma/config.ex

@@ -5,6 +5,7 @@
 defmodule Mix.Tasks.Pleroma.Config do
   use Mix.Task
 
+  import Ecto.Query
   import Mix.Pleroma
 
   alias Pleroma.ConfigDB
@@ -48,7 +49,7 @@ def run(["dump"]) do
       unless settings == [] do
         shell_info("#{header}")
 
-        settings |> Enum.each(&dump(&1))
+        Enum.each(settings, &dump(&1))
       else
         shell_error("No settings in ConfigDB.")
       end
@@ -63,8 +64,8 @@ def run(["dump", group, key]) do
       key = maybe_atomize(key)
 
       group
-      |> ConfigDB.get_all_by_group_and_key(key)
+      |> ConfigDB.get_by_group_and_key(key)
-      |> Enum.each(&dump/1)
+      |> dump()
     end)
   end
 
@@ -84,10 +85,9 @@ def run(["groups"]) do
 
       groups =
         ConfigDB
+        |> distinct([c], true)
+        |> select([c], c.group)
         |> Repo.all()
-        |> Enum.map(fn x -> x.group end)
-        |> Enum.sort()
-        |> Enum.uniq()
 
       if length(groups) > 0 do
         shell_info("The following configuration groups are set in ConfigDB:\r\n")
@@ -295,12 +295,14 @@ defp delete(config, true) do
 
   defp delete(_config, _), do: :ok
 
-  defp dump(%Pleroma.ConfigDB{} = config) do
+  defp dump(%ConfigDB{} = config) do
     value = inspect(config.value, limit: :infinity)
 
     shell_info("config #{inspect(config.group)}, #{inspect(config.key)}, #{value}\r\n\r\n")
   end
 
+  defp dump(_), do: :noop
+
   defp dump_group(group) when is_atom(group) do
     group
     |> ConfigDB.get_all_by_group()
@@ -316,7 +318,7 @@ defp group_exists?(group) do
   defp maybe_atomize(arg) when is_atom(arg), do: arg
 
   defp maybe_atomize(arg) when is_binary(arg) do
-    if Pleroma.ConfigDB.module_name?(arg) do
+    if ConfigDB.module_name?(arg) do
       String.to_existing_atom("Elixir." <> arg)
     else
       String.to_atom(arg)
@@ -336,14 +338,14 @@ defp check_configdb(callback) do
 
   defp delete_key(group, key) do
     check_configdb(fn ->
-      Pleroma.ConfigDB.delete(%{group: group, key: key})
+      ConfigDB.delete(%{group: group, key: key})
     end)
   end
 
   defp delete_group(group) do
     check_configdb(fn ->
       group
-      |> Pleroma.ConfigDB.get_all_by_group()
+      |> ConfigDB.get_all_by_group()
       |> Enum.each(&ConfigDB.delete/1)
     end)
   end

lib/pleroma/config_db.ex

@@ -46,13 +46,13 @@ def get_all_by_group(group) do
     from(c in ConfigDB, where: c.group == ^group) |> Repo.all()
   end
 
-  @spec get_all_by_group_and_key(atom() | String.t(), atom() | String.t()) :: [t()]
+  @spec get_by_group_and_key(atom() | String.t(), atom() | String.t()) :: t() | nil
-  def get_all_by_group_and_key(group, key) do
+  def get_by_group_and_key(group, key) do
-    from(c in ConfigDB, where: c.group == ^group and c.key == ^key) |> Repo.all()
+    get_by_params(%{group: group, key: key})
   end
 
   @spec get_by_params(map()) :: ConfigDB.t() | nil
-  def get_by_params(params), do: Repo.get_by(ConfigDB, params)
+  def get_by_params(%{group: _, key: _} = params), do: Repo.get_by(ConfigDB, params)
 
   @spec changeset(ConfigDB.t(), map()) :: Changeset.t()
   def changeset(config, params \\ %{}) do

test/mix/tasks/pleroma/config_test.exs

@@ -7,6 +7,7 @@ defmodule Mix.Tasks.Pleroma.ConfigTest do
 
   import Pleroma.Factory
 
+  alias Mix.Tasks.Pleroma.Config, as: MixTask
   alias Pleroma.ConfigDB
   alias Pleroma.Repo
 
@@ -22,29 +23,41 @@ defmodule Mix.Tasks.Pleroma.ConfigTest do
     :ok
   end
 
-  test "error if file with custom settings doesn't exist" do
+  defp config_records do
-    Mix.Tasks.Pleroma.Config.migrate_to_db("config/not_existance_config_file.exs")
+    ConfigDB
+    |> Repo.all()
+    |> Enum.sort()
+  end
 
-    assert_receive {:mix_shell, :info,
+  defp insert_config_record(group, key, value) do
-                    [
+    insert(:config,
-                      "To migrate settings, you must define custom settings in config/not_existance_config_file.exs."
+      group: group,
-                    ]},
+      key: key,
-                   15
+      value: value
+    )
+  end
+
+  test "error if file with custom settings doesn't exist" do
+    MixTask.migrate_to_db("config/non_existent_config_file.exs")
+
+    msg =
+      "To migrate settings, you must define custom settings in config/non_existent_config_file.exs."
+
+    assert_receive {:mix_shell, :info, [^msg]}, 15
   end
 
   describe "migrate_to_db/1" do
     setup do
       clear_config(:configurable_from_database, true)
-      initial = Application.get_env(:quack, :level)
+      clear_config([:quack, :level])
-      on_exit(fn -> Application.put_env(:quack, :level, initial) end)
     end
 
     @tag capture_log: true
     test "config migration refused when deprecated settings are found" do
       clear_config([:media_proxy, :whitelist], ["domain_without_scheme.com"])
-      assert Repo.all(ConfigDB) == []
+      assert config_records() == []
 
-      Mix.Tasks.Pleroma.Config.migrate_to_db("test/fixtures/config/temp.secret.exs")
+      MixTask.migrate_to_db("test/fixtures/config/temp.secret.exs")
 
       assert_received {:mix_shell, :error, [message]}
 
@@ -53,9 +66,9 @@ test "config migration refused when deprecated settings are found" do
     end
 
     test "filtered settings are migrated to db" do
-      assert Repo.all(ConfigDB) == []
+      assert config_records() == []
 
-      Mix.Tasks.Pleroma.Config.migrate_to_db("test/fixtures/config/temp.secret.exs")
+      MixTask.migrate_to_db("test/fixtures/config/temp.secret.exs")
 
       config1 = ConfigDB.get_by_params(%{group: ":pleroma", key: ":first_setting"})
       config2 = ConfigDB.get_by_params(%{group: ":pleroma", key: ":second_setting"})
@@ -70,17 +83,17 @@ test "filtered settings are migrated to db" do
     end
 
     test "config table is truncated before migration" do
-      insert(:config, key: :first_setting, value: [key: "value", key2: ["Activity"]])
+      insert_config_record(:pleroma, :first_setting, key: "value", key2: ["Activity"])
-      assert Repo.aggregate(ConfigDB, :count, :id) == 1
+      assert length(config_records()) == 1
 
-      Mix.Tasks.Pleroma.Config.migrate_to_db("test/fixtures/config/temp.secret.exs")
+      MixTask.migrate_to_db("test/fixtures/config/temp.secret.exs")
 
       config = ConfigDB.get_by_params(%{group: ":pleroma", key: ":first_setting"})
       assert config.value == [key: "value", key2: [Repo]]
     end
   end
 
-  describe "with deletion temp file" do
+  describe "with deletion of temp file" do
     setup do
       clear_config(:configurable_from_database, true)
       temp_file = "config/temp.exported_from_db.secret.exs"
@@ -93,13 +106,13 @@ test "config table is truncated before migration" do
     end
 
     test "settings are migrated to file and deleted from db", %{temp_file: temp_file} do
-      insert(:config, key: :setting_first, value: [key: "value", key2: ["Activity"]])
+      insert_config_record(:pleroma, :setting_first, key: "value", key2: ["Activity"])
-      insert(:config, key: :setting_second, value: [key: "value2", key2: [Repo]])
+      insert_config_record(:pleroma, :setting_second, key: "value2", key2: [Repo])
-      insert(:config, group: :quack, key: :level, value: :info)
+      insert_config_record(:quack, :level, :info)
 
-      Mix.Tasks.Pleroma.Config.run(["migrate_from_db", "--env", "temp", "-d"])
+      MixTask.run(["migrate_from_db", "--env", "temp", "-d"])
 
-      assert Repo.all(ConfigDB) == []
+      assert config_records() == []
 
       file = File.read!(temp_file)
       assert file =~ "config :pleroma, :setting_first,"
@@ -169,9 +182,9 @@ test "load a settings with large values and pass to file", %{temp_file: temp_fil
         ]
       )
 
-      Mix.Tasks.Pleroma.Config.run(["migrate_from_db", "--env", "temp", "-d"])
+      MixTask.run(["migrate_from_db", "--env", "temp", "-d"])
 
-      assert Repo.all(ConfigDB) == []
+      assert config_records() == []
       assert File.exists?(temp_file)
       {:ok, file} = File.read(temp_file)
 
@@ -191,26 +204,16 @@ test "load a settings with large values and pass to file", %{temp_file: temp_fil
     setup do: clear_config(:configurable_from_database, true)
 
     test "dumping a specific group" do
-      insert(:config,
+      insert_config_record(:pleroma, :instance, name: "Pleroma Test")
-        group: :pleroma,
-        key: :instance,
-        value: [
-          name: "Pleroma Test"
-        ]
-      )
 
-      insert(:config,
+      insert_config_record(:web_push_encryption, :vapid_details,
-        group: :web_push_encryption,
-        key: :vapid_details,
-        value: [
         subject: "mailto:administrator@example.com",
         public_key:
           "BOsPL-_KjNnjj_RMvLeR3dTOrcndi4TbMR0cu56gLGfGaT5m1gXxSfRHOcC4Dd78ycQL1gdhtx13qgKHmTM5xAI",
         private_key: "Ism6FNdS31nLCA94EfVbJbDdJXCxAZ8cZiB1JQPN_t4"
-        ]
       )
 
-      Mix.Tasks.Pleroma.Config.run(["dump", "pleroma"])
+      MixTask.run(["dump", "pleroma"])
 
       assert_receive {:mix_shell, :info,
                       ["config :pleroma, :instance, [name: \"Pleroma Test\"]\r\n\r\n"]}
@@ -225,23 +228,10 @@ test "dumping a specific group" do
     end
 
     test "dumping a specific key in a group" do
-      insert(:config,
+      insert_config_record(:pleroma, :instance, name: "Pleroma Test")
-        group: :pleroma,
+      insert_config_record(:pleroma, Pleroma.Captcha, enabled: false)
-        key: :instance,
-        value: [
-          name: "Pleroma Test"
-        ]
-      )
 
-      insert(:config,
+      MixTask.run(["dump", "pleroma", "Pleroma.Captcha"])
-        group: :pleroma,
-        key: Pleroma.Captcha,
-        value: [
-          enabled: false
-        ]
-      )
-
-      Mix.Tasks.Pleroma.Config.run(["dump", "pleroma", "Pleroma.Captcha"])
 
       refute_receive {:mix_shell, :info,
                       ["config :pleroma, :instance, [name: \"Pleroma Test\"]\r\n\r\n"]}
@@ -251,23 +241,10 @@ test "dumping a specific key in a group" do
     end
 
     test "dumps all configuration successfully" do
-      insert(:config,
+      insert_config_record(:pleroma, :instance, name: "Pleroma Test")
-        group: :pleroma,
+      insert_config_record(:pleroma, Pleroma.Captcha, enabled: false)
-        key: :instance,
-        value: [
-          name: "Pleroma Test"
-        ]
-      )
 
-      insert(:config,
+      MixTask.run(["dump"])
-        group: :pleroma,
-        key: Pleroma.Captcha,
-        value: [
-          enabled: false
-        ]
-      )
-
-      Mix.Tasks.Pleroma.Config.run(["dump"])
 
       assert_receive {:mix_shell, :info,
                       ["config :pleroma, :instance, [name: \"Pleroma Test\"]\r\n\r\n"]}
@@ -281,115 +258,49 @@ test "dumps all configuration successfully" do
     test "refuses to dump" do
       clear_config(:configurable_from_database, false)
 
-      insert(:config,
+      insert_config_record(:pleroma, :instance, name: "Pleroma Test")
-        group: :pleroma,
-        key: :instance,
-        value: [
-          name: "Pleroma Test"
-        ]
-      )
 
-      Mix.Tasks.Pleroma.Config.run(["dump"])
+      MixTask.run(["dump"])
 
-      assert_receive {:mix_shell, :error,
+      msg =
-                      [
         "ConfigDB not enabled. Please check the value of :configurable_from_database in your configuration."
-                      ]}
+
+      assert_receive {:mix_shell, :error, [^msg]}
     end
   end
 
   describe "destructive operations" do
     setup do: clear_config(:configurable_from_database, true)
 
+    setup do
+      insert_config_record(:pleroma, :instance, name: "Pleroma Test")
+      insert_config_record(:pleroma, Pleroma.Captcha, enabled: false)
+      insert_config_record(:pleroma2, :key2, z: 1)
+
+      assert length(config_records()) == 3
+
+      :ok
+    end
+
     test "deletes group of settings" do
-      insert(:config,
+      MixTask.run(["delete", "--force", "pleroma"])
-        group: :pleroma,
-        key: :instance,
-        value: [
-          name: "Pleroma Test"
-        ]
-      )
 
-      _config_before = Repo.all(ConfigDB)
+      assert [%ConfigDB{group: :pleroma2, key: :key2}] = config_records()
-
-      assert config_before = [
-               %Pleroma.ConfigDB{
-                 group: :pleroma,
-                 key: :instance,
-                 value: [name: "Pleroma Test"]
-               }
-             ]
-
-      Mix.Tasks.Pleroma.Config.run(["delete", "--force", "pleroma"])
-
-      config_after = Repo.all(ConfigDB)
-
-      refute config_after == config_before
     end
 
     test "deletes specified key" do
-      insert(:config,
+      MixTask.run(["delete", "--force", "pleroma", "Pleroma.Captcha"])
-        group: :pleroma,
-        key: :instance,
-        value: [
-          name: "Pleroma Test"
-        ]
-      )
 
-      insert(:config,
+      assert [
-        group: :pleroma,
+               %ConfigDB{group: :pleroma, key: :instance},
-        key: Pleroma.Captcha,
+               %ConfigDB{group: :pleroma2, key: :key2}
-        value: [
+             ] = config_records()
-          enabled: false
-        ]
-      )
-
-      _config_before = Repo.all(ConfigDB)
-
-      assert config_before = [
-               %Pleroma.ConfigDB{
-                 group: :pleroma,
-                 key: :instance,
-                 value: [name: "Pleroma Test"]
-               },
-               %Pleroma.ConfigDB{
-                 group: :pleroma,
-                 key: Pleroma.Captcha,
-                 value: [enabled: false]
-               }
-             ]
-
-      Mix.Tasks.Pleroma.Config.run(["delete", "--force", "pleroma", "Pleroma.Captcha"])
-
-      config_after = Repo.all(ConfigDB)
-
-      refute config_after == config_before
     end
 
     test "resets entire config" do
-      insert(:config,
+      MixTask.run(["reset", "--force"])
-        group: :pleroma,
-        key: :instance,
-        value: [
-          name: "Pleroma Test"
-        ]
-      )
 
-      _config_before = Repo.all(ConfigDB)
+      assert config_records() == []
-
-      assert config_before = [
-               %Pleroma.ConfigDB{
-                 group: :pleroma,
-                 key: :instance,
-                 value: [name: "Pleroma Test"]
-               }
-             ]
-
-      Mix.Tasks.Pleroma.Config.run(["reset", "--force"])
-
-      config_after = Repo.all(ConfigDB)
-
-      assert config_after == []
     end
   end
 end

test/pleroma/web/admin_api/controllers/config_controller_test.exs

@@ -162,7 +162,9 @@ test "with valid `admin_token` query parameter, skips OAuth scopes check" do
     end
   end
 
-  test "POST /api/pleroma/admin/config error", %{conn: conn} do
+  test "POST /api/pleroma/admin/config with configdb disabled", %{conn: conn} do
+    clear_config(:configurable_from_database, false)
+
     conn =
       conn
       |> put_req_header("content-type", "application/json")

test/pleroma/web/admin_api/controllers/relay_controller_test.exs

@@ -60,7 +60,7 @@ test "GET /relay", %{conn: conn} do
 
       conn = get(conn, "/api/pleroma/admin/relay")
 
-      assert json_response_and_validate_schema(conn, 200)["relays"] == [
+      assert json_response_and_validate_schema(conn, 200)["relays"] |> Enum.sort() == [
                %{
                  "actor" => "http://mastodon.example.org/users/admin",
                  "followed_back" => true