Merge branch 'need-reboot-flag' into 'develop'

need_reboot flag for adminFE

See merge request pleroma/pleroma!2188

docs/API/admin_api.md

@@ -682,6 +682,8 @@ Note: Available `:permission_group` is currently moderator and admin. 404 is ret
 
 ### Get list of merged default settings with saved in database.
 
+*If `need_reboot` flag exists in response, instance must be restarted, so reboot time settings can take effect.*
+
 **Only works when configuration from database is enabled.**
 
 - Params:
@@ -692,20 +694,24 @@ Note: Available `:permission_group` is currently moderator and admin. 404 is ret
 
 ```json
 {
-  configs: [
+  "configs": [
     {
       "group": ":pleroma",
       "key": "Pleroma.Upload",
       "value": []
      }
-  ]
+  ],
+  "need_reboot": true
 }
 ```
+ need_reboot - *optional*, if were changed reboot time settings.
 
 ## `POST /api/pleroma/admin/config`
 
 ### Update config settings
 
+*If `need_reboot` flag exists in response, instance must be restarted, so reboot time settings can take effect.*
+
 **Only works when configuration from database is enabled.**
 
 Some modifications are necessary to save the config settings correctly:
@@ -793,7 +799,7 @@ config :quack,
 ```
 ```json
 {
-  configs: [
+  "configs": [
     {"group": ":quack", "key": ":level", "value": ":debug"},
     {"group": ":quack", "key": ":meta", "value": [":all"]},
     ...
@@ -804,7 +810,7 @@ config :quack,
 
 ```json
 {
-  configs: [
+  "configs": [
     {
       "group": ":pleroma",
       "key": "Pleroma.Upload",
@@ -836,15 +842,17 @@ config :quack,
     - 400 Bad Request `"To use this endpoint you need to enable configuration from database."`
 ```json
 {
-  configs: [
+  "configs": [
     {
       "group": ":pleroma",
       "key": "Pleroma.Upload",
       "value": [...]
      }
-  ]
+  ],
+  "need_reboot": true
 }
 ```
+need_reboot - *optional*, if were changed reboot time settings.
 
 ## ` GET /api/pleroma/admin/config/descriptions`
 

lib/pleroma/config/transfer_task.ex

@@ -146,9 +146,7 @@ defp group_and_subkey_need_reboot?(group, key, value) do
   defp update_env(group, key, nil), do: Application.delete_env(group, key)
   defp update_env(group, key, value), do: Application.put_env(group, key, value)
 
-  defp restart(_, :pleroma, :test), do: Logger.warn("pleroma restarted")
-
-  defp restart(_, :pleroma, _), do: send(Restarter.Pleroma, :after_boot)
+  defp restart(_, :pleroma, env), do: Restarter.Pleroma.restart_after_boot(env)
 
   defp restart(started_applications, app, _) do
     with {^app, _, _} <- List.keyfind(started_applications, app, 0),

lib/pleroma/web/admin_api/admin_api_controller.ex

@@ -8,6 +8,7 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do
   import Pleroma.Web.ControllerHelper, only: [json_response: 3]
 
   alias Pleroma.Activity
+  alias Pleroma.Config
   alias Pleroma.ConfigDB
   alias Pleroma.ModerationLog
   alias Pleroma.Plugs.OAuthScopesPlug
@@ -570,8 +571,8 @@ def relay_unfollow(%{assigns: %{user: admin}} = conn, %{"relay_url" => target})
   @doc "Sends registration invite via email"
   def email_invite(%{assigns: %{user: user}} = conn, %{"email" => email} = params) do
     with true <-
-           Pleroma.Config.get([:instance, :invites_enabled]) &&
-             !Pleroma.Config.get([:instance, :registrations_open]),
+           Config.get([:instance, :invites_enabled]) &&
+             !Config.get([:instance, :registrations_open]),
          {:ok, invite_token} <- UserInviteToken.create_invite(),
          email <-
            Pleroma.Emails.UserEmail.user_invitation_email(
@@ -808,7 +809,7 @@ def config_show(conn, _params) do
       configs = ConfigDB.get_all_as_keyword()
 
       merged =
-        Pleroma.Config.Holder.config()
+        Config.Holder.config()
         |> ConfigDB.merge(configs)
         |> Enum.map(fn {group, values} ->
           Enum.map(values, fn {key, value} ->
@@ -838,7 +839,16 @@ def config_show(conn, _params) do
         end)
         |> List.flatten()
 
-      json(conn, %{configs: merged})
+      response = %{configs: merged}
+
+      response =
+        if Restarter.Pleroma.need_reboot?() do
+          Map.put(response, :need_reboot, true)
+        else
+          response
+        end
+
+      json(conn, response)
     end
   end
 
@@ -863,20 +873,26 @@ def config_update(conn, %{"configs" => configs}) do
           Ecto.get_meta(config, :state) == :deleted
         end)
 
-      Pleroma.Config.TransferTask.load_and_update_env(deleted, false)
+      Config.TransferTask.load_and_update_env(deleted, false)
 
       need_reboot? =
+        Restarter.Pleroma.need_reboot?() ||
           Enum.any?(updated, fn config ->
             group = ConfigDB.from_string(config.group)
             key = ConfigDB.from_string(config.key)
             value = ConfigDB.from_binary(config.value)
-          Pleroma.Config.TransferTask.pleroma_need_restart?(group, key, value)
+            Config.TransferTask.pleroma_need_restart?(group, key, value)
           end)
 
       response = %{configs: updated}
 
       response =
-        if need_reboot?, do: Map.put(response, :need_reboot, need_reboot?), else: response
+        if need_reboot? do
+          Restarter.Pleroma.need_reboot()
+          Map.put(response, :need_reboot, need_reboot?)
+        else
+          response
+        end
 
       conn
       |> put_view(ConfigView)
@@ -886,18 +902,14 @@ def config_update(conn, %{"configs" => configs}) do
 
   def restart(conn, _params) do
     with :ok <- configurable_from_database(conn) do
-      if Pleroma.Config.get(:env) == :test do
-        Logger.warn("pleroma restarted")
-      else
-        send(Restarter.Pleroma, {:restart, 50})
-      end
+      Restarter.Pleroma.restart(Config.get(:env), 50)
 
       json(conn, %{})
     end
   end
 
   defp configurable_from_database(conn) do
-    if Pleroma.Config.get(:configurable_from_database) do
+    if Config.get(:configurable_from_database) do
       :ok
     else
       errors(

restarter/lib/pleroma.ex

@@ -1,26 +1,72 @@
 defmodule Restarter.Pleroma do
   use GenServer
 
+  require Logger
+
   def start_link(_) do
     GenServer.start_link(__MODULE__, [], name: __MODULE__)
   end
 
-  def init(_), do: {:ok, %{}}
+  def init(_), do: {:ok, %{need_reboot?: false}}
 
-  def handle_info(:after_boot, %{after_boot: true} = state), do: {:noreply, state}
+  def need_reboot? do
+    GenServer.call(__MODULE__, :need_reboot?)
+  end
 
-  def handle_info(:after_boot, state) do
-    restart(:pleroma)
+  def need_reboot do
+    GenServer.cast(__MODULE__, :need_reboot)
+  end
+
+  def refresh do
+    GenServer.cast(__MODULE__, :refresh)
+  end
+
+  def restart(env, delay) do
+    GenServer.cast(__MODULE__, {:restart, env, delay})
+  end
+
+  def restart_after_boot(env) do
+    GenServer.cast(__MODULE__, {:after_boot, env})
+  end
+
+  def handle_call(:need_reboot?, _from, state) do
+    {:reply, state[:need_reboot?], state}
+  end
+
+  def handle_cast(:refresh, _state) do
+    {:noreply, %{need_reboot?: false}}
+  end
+
+  def handle_cast(:need_reboot, %{need_reboot?: true} = state), do: {:noreply, state}
+
+  def handle_cast(:need_reboot, state) do
+    {:noreply, Map.put(state, :need_reboot?, true)}
+  end
+
+  def handle_cast({:restart, :test, _}, state) do
+    Logger.warn("pleroma restarted")
+    {:noreply, Map.put(state, :need_reboot?, false)}
+  end
+
+  def handle_cast({:restart, _, delay}, state) do
+    Process.sleep(delay)
+    do_restart(:pleroma)
+    {:noreply, Map.put(state, :need_reboot?, false)}
+  end
+
+  def handle_cast({:after_boot, _}, %{after_boot: true} = state), do: {:noreply, state}
+
+  def handle_cast({:after_boot, :test}, state) do
+    Logger.warn("pleroma restarted")
     {:noreply, Map.put(state, :after_boot, true)}
   end
 
-  def handle_info({:restart, delay}, state) do
-    Process.sleep(delay)
-    restart(:pleroma)
-    {:noreply, state}
+  def handle_cast({:after_boot, _}, state) do
+    do_restart(:pleroma)
+    {:noreply, Map.put(state, :after_boot, true)}
   end
 
-  defp restart(app) do
+  defp do_restart(app) do
     :ok = Application.ensure_started(app)
     :ok = Application.stop(app)
     :ok = Application.start(app)

test/config/transfer_task_test.exs

@@ -109,6 +109,10 @@ test "transfer config values with full subkey update" do
   end
 
   describe "pleroma restart" do
+    setup do
+      on_exit(fn -> Restarter.Pleroma.refresh() end)
+    end
+
     test "don't restart if no reboot time settings were changed" do
       emoji = Application.get_env(:pleroma, :emoji)
       on_exit(fn -> Application.put_env(:pleroma, :emoji, emoji) end)
@@ -125,7 +129,7 @@ test "don't restart if no reboot time settings were changed" do
              )
     end
 
-    test "restart pleroma on reboot time key" do
+    test "on reboot time key" do
       chat = Application.get_env(:pleroma, :chat)
       on_exit(fn -> Application.put_env(:pleroma, :chat, chat) end)
 
@@ -138,7 +142,7 @@ test "restart pleroma on reboot time key" do
       assert capture_log(fn -> TransferTask.start_link([]) end) =~ "pleroma restarted"
     end
 
-    test "restart pleroma on reboot time subkey" do
+    test "on reboot time subkey" do
       captcha = Application.get_env(:pleroma, Pleroma.Captcha)
       on_exit(fn -> Application.put_env(:pleroma, Pleroma.Captcha, captcha) end)
 

test/web/admin_api/admin_api_controller_test.exs

@@ -6,7 +6,11 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do
   use Pleroma.Web.ConnCase
   use Oban.Testing, repo: Pleroma.Repo
 
+  import Pleroma.Factory
+  import ExUnit.CaptureLog
+
   alias Pleroma.Activity
+  alias Pleroma.Config
   alias Pleroma.ConfigDB
   alias Pleroma.HTML
   alias Pleroma.ModerationLog
@@ -19,7 +23,6 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do
   alias Pleroma.Web.CommonAPI
   alias Pleroma.Web.MastodonAPI.StatusView
   alias Pleroma.Web.MediaProxy
-  import Pleroma.Factory
 
   setup_all do
     Tesla.Mock.mock_global(fn env -> apply(HttpRequestMock, :request, [env]) end)
@@ -41,7 +44,7 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do
 
   describe "with [:auth, :enforce_oauth_admin_scope_usage]," do
     clear_config([:auth, :enforce_oauth_admin_scope_usage]) do
-      Pleroma.Config.put([:auth, :enforce_oauth_admin_scope_usage], true)
+      Config.put([:auth, :enforce_oauth_admin_scope_usage], true)
     end
 
     test "GET /api/pleroma/admin/users/:nickname requires admin:read:accounts or broader scope",
@@ -91,7 +94,7 @@ test "GET /api/pleroma/admin/users/:nickname requires admin:read:accounts or bro
 
   describe "unless [:auth, :enforce_oauth_admin_scope_usage]," do
     clear_config([:auth, :enforce_oauth_admin_scope_usage]) do
-      Pleroma.Config.put([:auth, :enforce_oauth_admin_scope_usage], false)
+      Config.put([:auth, :enforce_oauth_admin_scope_usage], false)
     end
 
     test "GET /api/pleroma/admin/users/:nickname requires " <>
@@ -579,11 +582,11 @@ test "/:right DELETE, can remove from a permission group (multiple)", %{
 
   describe "POST /api/pleroma/admin/email_invite, with valid config" do
     clear_config([:instance, :registrations_open]) do
-      Pleroma.Config.put([:instance, :registrations_open], false)
+      Config.put([:instance, :registrations_open], false)
     end
 
     clear_config([:instance, :invites_enabled]) do
-      Pleroma.Config.put([:instance, :invites_enabled], true)
+      Config.put([:instance, :invites_enabled], true)
     end
 
     test "sends invitation and returns 204", %{admin: admin, conn: conn} do
@@ -602,8 +605,8 @@ test "sends invitation and returns 204", %{admin: admin, conn: conn} do
       assert token_record
       refute token_record.used
 
-      notify_email = Pleroma.Config.get([:instance, :notify_email])
-      instance_name = Pleroma.Config.get([:instance, :name])
+      notify_email = Config.get([:instance, :notify_email])
+      instance_name = Config.get([:instance, :name])
 
       email =
         Pleroma.Emails.UserEmail.user_invitation_email(
@@ -639,8 +642,8 @@ test "it returns 403 if requested by a non-admin" do
     clear_config([:instance, :invites_enabled])
 
     test "it returns 500 if `invites_enabled` is not enabled", %{conn: conn} do
-      Pleroma.Config.put([:instance, :registrations_open], false)
-      Pleroma.Config.put([:instance, :invites_enabled], false)
+      Config.put([:instance, :registrations_open], false)
+      Config.put([:instance, :invites_enabled], false)
 
       conn = post(conn, "/api/pleroma/admin/users/email_invite?email=foo@bar.com&name=JD")
 
@@ -648,8 +651,8 @@ test "it returns 500 if `invites_enabled` is not enabled", %{conn: conn} do
     end
 
     test "it returns 500 if `registrations_open` is enabled", %{conn: conn} do
-      Pleroma.Config.put([:instance, :registrations_open], true)
-      Pleroma.Config.put([:instance, :invites_enabled], true)
+      Config.put([:instance, :registrations_open], true)
+      Config.put([:instance, :invites_enabled], true)
 
       conn = post(conn, "/api/pleroma/admin/users/email_invite?email=foo@bar.com&name=JD")
 
@@ -1886,13 +1889,13 @@ test "returns error when status is not exist", %{conn: conn} do
 
   describe "GET /api/pleroma/admin/config" do
     clear_config(:configurable_from_database) do
-      Pleroma.Config.put(:configurable_from_database, true)
+      Config.put(:configurable_from_database, true)
     end
 
     test "when configuration from database is off", %{conn: conn} do
-      initial = Pleroma.Config.get(:configurable_from_database)
-      Pleroma.Config.put(:configurable_from_database, false)
-      on_exit(fn -> Pleroma.Config.put(:configurable_from_database, initial) end)
+      initial = Config.get(:configurable_from_database)
+      Config.put(:configurable_from_database, false)
+      on_exit(fn -> Config.put(:configurable_from_database, initial) end)
       conn = get(conn, "/api/pleroma/admin/config")
 
       assert json_response(conn, 400) ==
@@ -2036,11 +2039,12 @@ test "POST /api/pleroma/admin/config error", %{conn: conn} do
         Application.delete_env(:pleroma, Pleroma.Captcha.NotReal)
         Application.put_env(:pleroma, :http, http)
         Application.put_env(:tesla, :adapter, Tesla.Mock)
+        Restarter.Pleroma.refresh()
       end)
     end
 
     clear_config(:configurable_from_database) do
-      Pleroma.Config.put(:configurable_from_database, true)
+      Config.put(:configurable_from_database, true)
     end
 
     @tag capture_log: true
@@ -2249,11 +2253,10 @@ test "saving config with partial update", %{conn: conn} do
     end
 
     test "saving config which need pleroma reboot", %{conn: conn} do
-      chat = Pleroma.Config.get(:chat)
-      on_exit(fn -> Pleroma.Config.put(:chat, chat) end)
+      chat = Config.get(:chat)
+      on_exit(fn -> Config.put(:chat, chat) end)
 
-      conn =
-        post(
+      assert post(
                conn,
                "/api/pleroma/admin/config",
                %{
@@ -2262,8 +2265,7 @@ test "saving config which need pleroma reboot", %{conn: conn} do
                  ]
                }
              )
-
-      assert json_response(conn, 200) == %{
+             |> json_response(200) == %{
                "configs" => [
                  %{
                    "db" => [":enabled"],
@@ -2274,6 +2276,80 @@ test "saving config which need pleroma reboot", %{conn: conn} do
                ],
                "need_reboot" => true
              }
+
+      configs =
+        conn
+        |> get("/api/pleroma/admin/config")
+        |> json_response(200)
+
+      assert configs["need_reboot"]
+
+      capture_log(fn ->
+        assert conn |> get("/api/pleroma/admin/restart") |> json_response(200) == %{}
+      end) =~ "pleroma restarted"
+
+      configs =
+        conn
+        |> get("/api/pleroma/admin/config")
+        |> json_response(200)
+
+      refute Map.has_key?(configs, "need_reboot")
+    end
+
+    test "update setting which need reboot, don't change reboot flag until reboot", %{conn: conn} do
+      chat = Config.get(:chat)
+      on_exit(fn -> Config.put(:chat, chat) end)
+
+      assert post(
+               conn,
+               "/api/pleroma/admin/config",
+               %{
+                 configs: [
+                   %{group: ":pleroma", key: ":chat", value: [%{"tuple" => [":enabled", true]}]}
+                 ]
+               }
+             )
+             |> json_response(200) == %{
+               "configs" => [
+                 %{
+                   "db" => [":enabled"],
+                   "group" => ":pleroma",
+                   "key" => ":chat",
+                   "value" => [%{"tuple" => [":enabled", true]}]
+                 }
+               ],
+               "need_reboot" => true
+             }
+
+      assert post(conn, "/api/pleroma/admin/config", %{
+               configs: [
+                 %{group: ":pleroma", key: ":key1", value: [%{"tuple" => [":key3", 3]}]}
+               ]
+             })
+             |> json_response(200) == %{
+               "configs" => [
+                 %{
+                   "group" => ":pleroma",
+                   "key" => ":key1",
+                   "value" => [
+                     %{"tuple" => [":key3", 3]}
+                   ],
+                   "db" => [":key3"]
+                 }
+               ],
+               "need_reboot" => true
+             }
+
+      capture_log(fn ->
+        assert conn |> get("/api/pleroma/admin/restart") |> json_response(200) == %{}
+      end) =~ "pleroma restarted"
+
+      configs =
+        conn
+        |> get("/api/pleroma/admin/config")
+        |> json_response(200)
+
+      refute Map.has_key?(configs, "need_reboot")
     end
 
     test "saving config with nested merge", %{conn: conn} do
@@ -2410,7 +2486,7 @@ test "saving full setting if value is in full_key_update list", %{conn: conn} do
                {ExSyslogger, :ex_syslogger}
              ]
 
-      ExUnit.CaptureLog.capture_log(fn ->
+      capture_log(fn ->
         require Logger
         Logger.warn("Ooops...")
       end) =~ "Ooops..."
@@ -2543,7 +2619,7 @@ test "common config example", %{conn: conn} do
         })
 
       assert Application.get_env(:tesla, :adapter) == Tesla.Adapter.Httpc
-      assert Pleroma.Config.get([Pleroma.Captcha.NotReal, :name]) == "Pleroma"
+      assert Config.get([Pleroma.Captcha.NotReal, :name]) == "Pleroma"
 
       assert json_response(conn, 200) == %{
                "configs" => [
@@ -2979,13 +3055,15 @@ test "proxy tuple ip", %{conn: conn} do
 
   describe "GET /api/pleroma/admin/restart" do
     clear_config(:configurable_from_database) do
-      Pleroma.Config.put(:configurable_from_database, true)
+      Config.put(:configurable_from_database, true)
     end
 
     test "pleroma restarts", %{conn: conn} do
-      ExUnit.CaptureLog.capture_log(fn ->
+      capture_log(fn ->
         assert conn |> get("/api/pleroma/admin/restart") |> json_response(200) == %{}
       end) =~ "pleroma restarted"
+
+      refute Restarter.Pleroma.need_reboot?()
     end
   end