Merge branch 'logger-configuration' into 'develop'

Fix for logger configuration through admin-fe

See merge request pleroma/pleroma!2365

CHANGELOG.md

@@ -22,6 +22,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Support pagination in conversations API
 
 ## [unreleased-patch]
+### Fixed
+- Logger configuration through AdminFE
 
 ## [2.0.2] - 2020-04-08
 ### Added

lib/pleroma/config/transfer_task.ex

@@ -54,10 +54,19 @@ def load_and_update_env(deleted_settings \\ [], restart_pleroma? \\ true) do
           [:pleroma, nil, :prometheus]
         end
 
+      {logger, other} =
+        (Repo.all(ConfigDB) ++ deleted_settings)
+        |> Enum.map(&transform_and_merge/1)
+        |> Enum.split_with(fn {group, _, _, _} -> group in [:logger, :quack] end)
+
+      logger
+      |> Enum.sort()
+      |> Enum.each(&configure/1)
+
       started_applications = Application.started_applications()
 
-      (Repo.all(ConfigDB) ++ deleted_settings)
+      other
-      |> Enum.map(&merge_and_update/1)
+      |> Enum.map(&update/1)
       |> Enum.uniq()
       |> Enum.reject(&(&1 in reject_restart))
       |> maybe_set_pleroma_last()
@@ -81,51 +90,66 @@ defp maybe_set_pleroma_last(apps) do
     end
   end
 
-  defp group_for_restart(:logger, key, _, merged_value) do
+  defp transform_and_merge(%{group: group, key: key, value: value} = setting) do
-    # change logger configuration in runtime, without restart
+    group = ConfigDB.from_string(group)
-    if Keyword.keyword?(merged_value) and
+    key = ConfigDB.from_string(key)
-         key not in [:compile_time_application, :backends, :compile_time_purge_matching] do
+    value = ConfigDB.from_binary(value)
-      Logger.configure_backend(key, merged_value)
-    else
-      Logger.configure([{key, merged_value}])
-    end
-
-    nil
-  end
-
-  defp group_for_restart(group, _, _, _) when group != :pleroma, do: group
-
-  defp group_for_restart(group, key, value, _) do
-    if pleroma_need_restart?(group, key, value), do: group
-  end
-
-  defp merge_and_update(setting) do
-    try do
-      key = ConfigDB.from_string(setting.key)
-      group = ConfigDB.from_string(setting.group)
 
     default = Config.Holder.default_config(group, key)
-      value = ConfigDB.from_binary(setting.value)
 
-      merged_value =
+    merged =
       cond do
         Ecto.get_meta(setting, :state) == :deleted -> default
         can_be_merged?(default, value) -> ConfigDB.merge_group(group, key, default, value)
         true -> value
       end
 
-      :ok = update_env(group, key, merged_value)
+    {group, key, value, merged}
+  end
 
-      group_for_restart(group, key, value, merged_value)
+  # change logger configuration in runtime, without restart
+  defp configure({:quack, key, _, merged}) do
+    Logger.configure_backend(Quack.Logger, [{key, merged}])
+    :ok = update_env(:quack, key, merged)
+  end
+
+  defp configure({_, :backends, _, merged}) do
+    # removing current backends
+    Enum.each(Application.get_env(:logger, :backends), &Logger.remove_backend/1)
+
+    Enum.each(merged, &Logger.add_backend/1)
+
+    :ok = update_env(:logger, :backends, merged)
+  end
+
+  defp configure({group, key, _, merged}) do
+    merged =
+      if key == :console do
+        put_in(merged[:format], merged[:format] <> "\n")
+      else
+        merged
+      end
+
+    backend =
+      if key == :ex_syslogger,
+        do: {ExSyslogger, :ex_syslogger},
+        else: key
+
+    Logger.configure_backend(backend, merged)
+    :ok = update_env(:logger, group, merged)
+  end
+
+  defp update({group, key, value, merged}) do
+    try do
+      :ok = update_env(group, key, merged)
+
+      if group != :pleroma or pleroma_need_restart?(group, key, value), do: group
     rescue
       error ->
         error_msg =
-          "updating env causes error, group: " <>
+          "updating env causes error, group: #{inspect(group)}, key: #{inspect(key)}, value: #{
-            inspect(setting.group) <>
+            inspect(value)
-            " key: " <>
+          } error: #{inspect(error)}"
-            inspect(setting.key) <>
-            " value: " <>
-            inspect(ConfigDB.from_binary(setting.value)) <> " error: " <> inspect(error)
 
         Logger.warn(error_msg)
 
@@ -133,6 +157,9 @@ defp merge_and_update(setting) do
     end
   end
 
+  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)
+
   @spec pleroma_need_restart?(atom(), atom(), any()) :: boolean()
   def pleroma_need_restart?(group, key, value) do
     group_and_key_need_reboot?(group, key) or group_and_subkey_need_reboot?(group, key, value)
@@ -150,9 +177,6 @@ defp group_and_subkey_need_reboot?(group, key, value) do
       end)
   end
 
-  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, env), do: Restarter.Pleroma.restart_after_boot(env)
 
   defp restart(started_applications, app, _) do

test/web/admin_api/admin_api_controller_test.exs

@@ -2273,13 +2273,17 @@ test "saving full setting if value is in full_key_update list", %{conn: conn} do
           value: :erlang.term_to_binary([])
         )
 
+      Pleroma.Config.TransferTask.load_and_update_env([], false)
+
+      assert Application.get_env(:logger, :backends) == []
+
       conn =
         post(conn, "/api/pleroma/admin/config", %{
           configs: [
             %{
               group: config.group,
               key: config.key,
-              value: [":console", %{"tuple" => ["ExSyslogger", ":ex_syslogger"]}]
+              value: [":console"]
             }
           ]
         })
@@ -2290,8 +2294,7 @@ test "saving full setting if value is in full_key_update list", %{conn: conn} do
                    "group" => ":logger",
                    "key" => ":backends",
                    "value" => [
-                     ":console",
+                     ":console"
-                     %{"tuple" => ["ExSyslogger", ":ex_syslogger"]}
                    ],
                    "db" => [":backends"]
                  }
@@ -2299,14 +2302,8 @@ test "saving full setting if value is in full_key_update list", %{conn: conn} do
              }
 
       assert Application.get_env(:logger, :backends) == [
-               :console,
+               :console
-               {ExSyslogger, :ex_syslogger}
              ]
-
-      capture_log(fn ->
-        require Logger
-        Logger.warn("Ooops...")
-      end) =~ "Ooops..."
     end
 
     test "saving full setting if value is not keyword", %{conn: conn} do