From fea734ca703b686701b87c8c4c4969deb05d1f92 Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 6 Dec 2019 17:50:53 +0300 Subject: [PATCH] errors on endpoints --- docs/API/admin_api.md | 21 ++-- .../web/admin_api/admin_api_controller.ex | 106 +++++++++--------- test/tasks/config_test.exs | 4 +- .../admin_api/admin_api_controller_test.exs | 67 ++++++++--- 4 files changed, 118 insertions(+), 80 deletions(-) diff --git a/docs/API/admin_api.md b/docs/API/admin_api.md index dff12db56..98af8e8f3 100644 --- a/docs/API/admin_api.md +++ b/docs/API/admin_api.md @@ -665,19 +665,6 @@ Note: Available `:permission_group` is currently moderator and admin. 404 is ret - 404 Not Found `"Not found"` - On success: 200 OK `{}` -## `GET /api/pleroma/admin/config/migrate_to_db` - -### Run mix task pleroma.config migrate_to_db - -Copies `pleroma` environment settings to the database. - -- Params: none -- Response: - -```json -{} -``` - ## `GET /api/pleroma/admin/config/migrate_from_db` ### Run mix task pleroma.config migrate_from_db @@ -686,6 +673,8 @@ Copies all settings from database to `config/{env}.exported_from_db.secret.exs` - Params: none - Response: + - On failure: + - 400 Bad Request `"To use this endpoint you need to enable dynamic configuration."` ```json {} @@ -699,6 +688,9 @@ Copies all settings from database to `config/{env}.exported_from_db.secret.exs` - Params: none - Response: + - On failure: + - 400 Bad Request `"To use this endpoint you need to enable dynamic configuration."` + - 400 Bad Request `"To use dynamic configuration migrate your settings to database."` ```json { @@ -831,7 +823,8 @@ config :quack, ``` - Response: - + - On failure: + - 400 Bad Request `"To use this endpoint you need to enable dynamic configuration."` ```json { configs: [ diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index 376f88061..23dcbedba 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -778,71 +778,77 @@ def list_log(conn, params) do |> render("index.json", %{log: log}) end - def migrate_to_db(conn, _params) do - Mix.Tasks.Pleroma.Config.run(["migrate_to_db"]) - json(conn, %{}) - end - - def migrate_from_db(conn, _params) do - Mix.Tasks.Pleroma.Config.run([ - "migrate_from_db", - "--env", - to_string(Pleroma.Config.get(:env)), - "-d" - ]) - - json(conn, %{}) - end - def config_descriptions(conn, _params) do conn |> Plug.Conn.put_resp_content_type("application/json") |> Plug.Conn.send_resp(200, @descriptions_json) end - def config_show(conn, _params) do - configs = Pleroma.Repo.all(Config) + def migrate_from_db(conn, _params) do + with :ok <- check_dynamic_configuration(conn) do + Mix.Tasks.Pleroma.Config.run([ + "migrate_from_db", + "--env", + to_string(Pleroma.Config.get(:env)), + "-d" + ]) - conn - |> put_view(ConfigView) - |> render("index.json", %{configs: configs}) + json(conn, %{}) + end + end + + def config_show(conn, _params) do + with :ok <- check_dynamic_configuration(conn) do + configs = Pleroma.Repo.all(Config) + + if configs == [] do + errors(conn, {:error, "To use dynamic configuration migrate your settings to database."}) + else + conn + |> put_view(ConfigView) + |> render("index.json", %{configs: configs}) + end + end end def config_update(conn, %{"configs" => configs}) do - updated = - if Pleroma.Config.get([:instance, :dynamic_configuration]) do - updated = - Enum.map(configs, fn - %{"group" => group, "key" => key, "delete" => "true"} = params -> - with {:ok, config} <- - Config.delete(%{group: group, key: key, subkeys: params["subkeys"]}) do - config - end + with :ok <- check_dynamic_configuration(conn) do + updated = + Enum.map(configs, fn + %{"group" => group, "key" => key, "delete" => "true"} = params -> + with {:ok, config} <- + Config.delete(%{group: group, key: key, subkeys: params["subkeys"]}) do + config + end - %{"group" => group, "key" => key, "value" => value} -> - with {:ok, config} <- - Config.update_or_create(%{group: group, key: key, value: value}) do - config - end - end) - |> Enum.reject(&is_nil(&1)) + %{"group" => group, "key" => key, "value" => value} -> + with {:ok, config} <- + Config.update_or_create(%{group: group, key: key, value: value}) do + config + end + end) + |> Enum.reject(&is_nil(&1)) - Pleroma.Config.TransferTask.load_and_update_env() + Pleroma.Config.TransferTask.load_and_update_env() - Mix.Tasks.Pleroma.Config.run([ - "migrate_from_db", - "--env", - to_string(Pleroma.Config.get(:env)) - ]) + Mix.Tasks.Pleroma.Config.run([ + "migrate_from_db", + "--env", + to_string(Pleroma.Config.get(:env)) + ]) - updated - else - [] - end + conn + |> put_view(ConfigView) + |> render("index.json", %{configs: updated}) + end + end - conn - |> put_view(ConfigView) - |> render("index.json", %{configs: updated}) + defp check_dynamic_configuration(conn) do + if Pleroma.Config.get([:instance, :dynamic_configuration]) do + :ok + else + errors(conn, {:error, "To use this endpoint you need to enable dynamic configuration."}) + end end def reload_emoji(conn, _params) do diff --git a/test/tasks/config_test.exs b/test/tasks/config_test.exs index dfe3904ca..74451a9e8 100644 --- a/test/tasks/config_test.exs +++ b/test/tasks/config_test.exs @@ -24,8 +24,8 @@ defmodule Mix.Tasks.Pleroma.ConfigTest do end test "settings are migrated to db" do - initial = Application.get_all_env(:quack) - on_exit(fn -> Application.put_all_env([{:quack, initial}]) end) + initial = Application.get_env(:quack, :level) + on_exit(fn -> Application.put_env(:quack, :level, initial) end) assert Repo.all(Config) == [] Application.put_env(:pleroma, :first_setting, key: "value", key2: [Repo]) diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index e2e10d3f8..41d2c4212 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -1929,16 +1929,31 @@ test "returns error when status is not exist", %{conn: conn} do end describe "GET /api/pleroma/admin/config" do + clear_config([:instance, :dynamic_configuration]) do + Pleroma.Config.put([:instance, :dynamic_configuration], true) + end + setup %{conn: conn} do admin = insert(:user, is_admin: true) %{conn: assign(conn, :user, admin)} end + test "when dynamic configuration is off", %{conn: conn} do + initial = Pleroma.Config.get([:instance, :dynamic_configuration]) + Pleroma.Config.put([:instance, :dynamic_configuration], false) + on_exit(fn -> Pleroma.Config.put([:instance, :dynamic_configuration], initial) end) + conn = get(conn, "/api/pleroma/admin/config") + + assert json_response(conn, 400) == + "To use this endpoint you need to enable dynamic configuration." + end + test "without any settings in db", %{conn: conn} do conn = get(conn, "/api/pleroma/admin/config") - assert json_response(conn, 200) == %{"configs" => []} + assert json_response(conn, 400) == + "To use dynamic configuration migrate your settings to database." end test "with settings in db", %{conn: conn} do @@ -1966,6 +1981,18 @@ test "with settings in db", %{conn: conn} do end end + test "POST /api/pleroma/admin/config error" do + admin = insert(:user, is_admin: true) + + conn = + build_conn() + |> assign(:user, admin) + |> post("/api/pleroma/admin/config", %{"configs" => []}) + + assert json_response(conn, 400) == + "To use this endpoint you need to enable dynamic configuration." + end + describe "POST /api/pleroma/admin/config" do setup %{conn: conn} do admin = insert(:user, is_admin: true) @@ -2101,8 +2128,15 @@ test "create new config setting in db", %{conn: conn} do end test "save config setting without key", %{conn: conn} do - initial = Application.get_all_env(:quack) - on_exit(fn -> Application.put_all_env([{:quack, initial}]) end) + level = Application.get_env(:quack, :level) + meta = Application.get_env(:quack, :meta) + webhook_url = Application.get_env(:quack, :webhook_url) + + on_exit(fn -> + Application.put_env(:quack, :level, level) + Application.put_env(:quack, :meta, meta) + Application.put_env(:quack, :webhook_url, webhook_url) + end) conn = post(conn, "/api/pleroma/admin/config", %{ @@ -2640,16 +2674,13 @@ test "delete part of settings by atom subkeys", %{conn: conn} do setup %{conn: conn} do admin = insert(:user, is_admin: true) - temp_file = "config/test.exported_from_db.secret.exs" - Mix.shell(Mix.Shell.Quiet) on_exit(fn -> Mix.shell(Mix.Shell.IO) - :ok = File.rm(temp_file) end) - %{conn: assign(conn, :user, admin), admin: admin} + %{conn: assign(conn, :user, admin)} end clear_config([:instance, :dynamic_configuration]) do @@ -2660,20 +2691,28 @@ test "delete part of settings by atom subkeys", %{conn: conn} do Pleroma.Config.put([:feed, :post_title], %{max_length: 100, omission: "…"}) end - test "transfer settings to DB and to file", %{conn: conn, admin: admin} do + test "transfer settings to DB and to file", %{conn: conn} do + on_exit(fn -> :ok = File.rm("config/test.exported_from_db.secret.exs") end) assert Pleroma.Repo.all(Pleroma.Web.AdminAPI.Config) == [] - conn = get(conn, "/api/pleroma/admin/config/migrate_to_db") - assert json_response(conn, 200) == %{} + Mix.Tasks.Pleroma.Config.run(["migrate_to_db"]) assert Pleroma.Repo.all(Pleroma.Web.AdminAPI.Config) > 0 - conn = - build_conn() - |> assign(:user, admin) - |> get("/api/pleroma/admin/config/migrate_from_db") + conn = get(conn, "/api/pleroma/admin/config/migrate_from_db") assert json_response(conn, 200) == %{} assert Pleroma.Repo.all(Pleroma.Web.AdminAPI.Config) == [] end + + test "returns error if dynamic configuration is off", %{conn: conn} do + initial = Pleroma.Config.get([:instance, :dynamic_configuration]) + on_exit(fn -> Pleroma.Config.put([:instance, :dynamic_configuration], initial) end) + Pleroma.Config.put([:instance, :dynamic_configuration], false) + + conn = get(conn, "/api/pleroma/admin/config/migrate_from_db") + + assert json_response(conn, 400) == + "To use this endpoint you need to enable dynamic configuration." + end end describe "GET /api/pleroma/admin/users/:nickname/statuses" do