From e00c66714590948ef917909779772155e20a3c96 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sun, 6 Dec 2020 18:02:30 +0300 Subject: [PATCH] [#3174] Refactoring: ConfigDB fetching functions, ConfigDB tests. Minor fixes. --- lib/mix/tasks/pleroma/config.ex | 22 +- lib/pleroma/config_db.ex | 8 +- test/mix/tasks/pleroma/config_test.exs | 241 ++++++------------ .../controllers/config_controller_test.exs | 4 +- .../controllers/relay_controller_test.exs | 2 +- 5 files changed, 96 insertions(+), 181 deletions(-) diff --git a/lib/mix/tasks/pleroma/config.ex b/lib/mix/tasks/pleroma/config.ex index 25f1ca05d..b5d802948 100644 --- a/lib/mix/tasks/pleroma/config.ex +++ b/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) - |> Enum.each(&dump/1) + |> ConfigDB.get_by_group_and_key(key) + |> 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 diff --git a/lib/pleroma/config_db.ex b/lib/pleroma/config_db.ex index 2c3c0cb5c..8e8bb732f 100644 --- a/lib/pleroma/config_db.ex +++ b/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()] - def get_all_by_group_and_key(group, key) do - from(c in ConfigDB, where: c.group == ^group and c.key == ^key) |> Repo.all() + @spec get_by_group_and_key(atom() | String.t(), atom() | String.t()) :: t() | nil + def get_by_group_and_key(group, key) do + 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 diff --git a/test/mix/tasks/pleroma/config_test.exs b/test/mix/tasks/pleroma/config_test.exs index 3658b3179..1ea9f5790 100644 --- a/test/mix/tasks/pleroma/config_test.exs +++ b/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 - Mix.Tasks.Pleroma.Config.migrate_to_db("config/not_existance_config_file.exs") + defp config_records do + ConfigDB + |> Repo.all() + |> Enum.sort() + end - assert_receive {:mix_shell, :info, - [ - "To migrate settings, you must define custom settings in config/not_existance_config_file.exs." - ]}, - 15 + defp insert_config_record(group, key, value) do + insert(:config, + group: group, + key: key, + 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) - on_exit(fn -> Application.put_env(:quack, :level, initial) end) + clear_config([:quack, :level]) 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"]]) - assert Repo.aggregate(ConfigDB, :count, :id) == 1 + insert_config_record(:pleroma, :first_setting, key: "value", key2: ["Activity"]) + 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, key: :setting_second, value: [key: "value2", key2: [Repo]]) - insert(:config, group: :quack, key: :level, value: :info) + insert_config_record(:pleroma, :setting_first, key: "value", key2: ["Activity"]) + insert_config_record(:pleroma, :setting_second, key: "value2", key2: [Repo]) + 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, - group: :pleroma, - key: :instance, - value: [ - name: "Pleroma Test" - ] + insert_config_record(:pleroma, :instance, name: "Pleroma Test") + + insert_config_record(:web_push_encryption, :vapid_details, + subject: "mailto:administrator@example.com", + public_key: + "BOsPL-_KjNnjj_RMvLeR3dTOrcndi4TbMR0cu56gLGfGaT5m1gXxSfRHOcC4Dd78ycQL1gdhtx13qgKHmTM5xAI", + private_key: "Ism6FNdS31nLCA94EfVbJbDdJXCxAZ8cZiB1JQPN_t4" ) - insert(:config, - 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, - group: :pleroma, - key: :instance, - value: [ - name: "Pleroma Test" - ] - ) + insert_config_record(:pleroma, :instance, name: "Pleroma Test") + insert_config_record(:pleroma, Pleroma.Captcha, enabled: false) - insert(:config, - group: :pleroma, - key: Pleroma.Captcha, - value: [ - enabled: false - ] - ) - - Mix.Tasks.Pleroma.Config.run(["dump", "pleroma", "Pleroma.Captcha"]) + MixTask.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, - group: :pleroma, - key: :instance, - value: [ - name: "Pleroma Test" - ] - ) + insert_config_record(:pleroma, :instance, name: "Pleroma Test") + insert_config_record(:pleroma, Pleroma.Captcha, enabled: false) - insert(:config, - group: :pleroma, - key: Pleroma.Captcha, - value: [ - enabled: false - ] - ) - - Mix.Tasks.Pleroma.Config.run(["dump"]) + MixTask.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, - group: :pleroma, - key: :instance, - value: [ - name: "Pleroma Test" - ] - ) + insert_config_record(:pleroma, :instance, name: "Pleroma Test") - Mix.Tasks.Pleroma.Config.run(["dump"]) + MixTask.run(["dump"]) - assert_receive {:mix_shell, :error, - [ - "ConfigDB not enabled. Please check the value of :configurable_from_database in your configuration." - ]} + 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, - group: :pleroma, - key: :instance, - value: [ - name: "Pleroma Test" - ] - ) + MixTask.run(["delete", "--force", "pleroma"]) - _config_before = Repo.all(ConfigDB) - - 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 + assert [%ConfigDB{group: :pleroma2, key: :key2}] = config_records() end test "deletes specified key" do - insert(:config, - group: :pleroma, - key: :instance, - value: [ - name: "Pleroma Test" - ] - ) + MixTask.run(["delete", "--force", "pleroma", "Pleroma.Captcha"]) - insert(:config, - group: :pleroma, - key: Pleroma.Captcha, - value: [ - 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 + assert [ + %ConfigDB{group: :pleroma, key: :instance}, + %ConfigDB{group: :pleroma2, key: :key2} + ] = config_records() end test "resets entire config" do - insert(:config, - group: :pleroma, - key: :instance, - value: [ - name: "Pleroma Test" - ] - ) + MixTask.run(["reset", "--force"]) - _config_before = Repo.all(ConfigDB) - - 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 == [] + assert config_records() == [] end end end diff --git a/test/pleroma/web/admin_api/controllers/config_controller_test.exs b/test/pleroma/web/admin_api/controllers/config_controller_test.exs index 4e897455f..276e827d1 100644 --- a/test/pleroma/web/admin_api/controllers/config_controller_test.exs +++ b/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") diff --git a/test/pleroma/web/admin_api/controllers/relay_controller_test.exs b/test/pleroma/web/admin_api/controllers/relay_controller_test.exs index b4c5e7567..379067a62 100644 --- a/test/pleroma/web/admin_api/controllers/relay_controller_test.exs +++ b/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