Refactor activity logging

trento-project · Jul 10, 2024 · fb1db88 · fb1db88
1 parent 4196b89
commit fb1db88
Show file tree

Hide file tree

Showing 8 changed files with 81 additions and 216 deletions.
diff --git a/config/config.exs b/config/config.exs
@@ -204,7 +204,6 @@ config :trento, Trento.Infrastructure.SoftwareUpdates.SumaApi,
   executor: Trento.Infrastructure.SoftwareUpdates.Suma.HttpExecutor
 
 config :trento, Trento.ActivityLog.ActivityLogger,
-  adapter: Trento.ActivityLog.Logger.PersistentLogger,
   writer: Trento.Infrastructure.ActivityLog.Logger.DatabasetWriter
 
 config :bodyguard,

diff --git a/lib/trento/activity_logging/activity_logger.ex b/lib/trento/activity_logging/activity_logger.ex
@@ -3,9 +3,57 @@ defmodule Trento.ActivityLog.ActivityLogger do
   ActivityLogger entry point
   """
 
-  @callback log_activity(activity_context :: any()) :: :ok
+  alias Trento.ActivityLog.ActivityCatalog
+  alias Trento.ActivityLog.Logger.ActivityLogWriter
 
-  def log_activity(activity_context), do: adapter().log_activity(activity_context)
+  require Logger
 
-  defp adapter, do: Application.fetch_env!(:trento, __MODULE__)[:adapter]
+  def log_activity(activity_context) do
+    with {:ok, activity_parser} <- get_activity_parser(activity_context),
+         detected_activity <- detect_activity(activity_parser, activity_context),
+         true <- ActivityCatalog.interested?(detected_activity, activity_context) do
+      write_log(%{
+        type: get_activity_type(detected_activity),
+        actor: get_actor(activity_parser, detected_activity, activity_context),
+        metadata: get_metadata(activity_parser, detected_activity, activity_context)
+      })
+    end
+
+    :ok
+  end
+
+  defp get_activity_parser(%Plug.Conn{}),
+    do: {:ok, Trento.ActivityLog.Logger.Parser.PhoenixConnParser}
+
+  defp get_activity_parser(_), do: {:error, :unsupported_activity}
+
+  # defp get_activity_parser(%Commanded.Middleware.Pipeline{}),
+  #   do: {:ok, Trento.ActivityLog.Logger.Parser.CommandedParser}
+
+  defp detect_activity(activity_parser, activity_context),
+    do: activity_parser.detect_activity(activity_context)
+
+  defp get_activity_type(detected_activity),
+    do:
+      detected_activity
+      |> ActivityCatalog.get_activity_type()
+      |> Atom.to_string()
+
+  defp get_actor(activity_parser, detected_activity, activity_context),
+    do: activity_parser.get_activity_actor(detected_activity, activity_context)
+
+  defp get_metadata(activity_parser, detected_activity, activity_context),
+    do: activity_parser.get_activity_metadata(detected_activity, activity_context)
+
+  defp write_log(%{type: activity_type} = entry) do
+    case ActivityLogWriter.write_log(entry) do
+      {:ok, _} ->
+        Logger.info("Logged activity: #{activity_type}")
+
+      {:error, reason} ->
+        Logger.error(
+          "An error occurred while logging activity: #{activity_type}. Reason: #{inspect(reason)}"
+        )
+    end
+  end
 end
diff --git a/lib/trento/activity_logging/logger/persistent_logger.ex b/lib/trento/activity_logging/logger/persistent_logger.ex
diff --git a/lib/trento/infrastructure/activity_log/logger/adapter/noop_logger.ex b/lib/trento/infrastructure/activity_log/logger/adapter/noop_logger.ex
diff --git a/test/support/conn_case.ex b/test/support/conn_case.ex
@@ -17,6 +17,8 @@ defmodule TrentoWeb.ConnCase do
 
   use ExUnit.CaseTemplate
 
+  use Trento.TaskCase
+
   alias Ecto.Adapters.SQL.Sandbox
 
   using do
@@ -35,17 +37,13 @@ defmodule TrentoWeb.ConnCase do
 
   setup tags do
     pid = Sandbox.start_owner!(Trento.Repo, shared: not tags[:async])
-    on_exit(fn -> Sandbox.stop_owner(pid) end)
 
-    stub_activity_logger()
+    on_exit(fn ->
+      wait_for_tasks_completion()
+
+      Sandbox.stop_owner(pid)
+    end)
 
     {:ok, conn: Phoenix.ConnTest.build_conn()}
   end
-
-  defp stub_activity_logger,
-    do:
-      Mox.stub_with(
-        Trento.ActivityLog.ActivityLogger.Mock,
-        Trento.Infrastructure.ActivityLog.Logger.NoopLogger
-      )
 end
diff --git a/test/test_helper.exs b/test/test_helper.exs
@@ -30,17 +30,6 @@ Application.put_env(:trento, Trento.Infrastructure.SoftwareUpdates.Suma,
   auth: Trento.Infrastructure.SoftwareUpdates.Auth.Mock
 )
 
-Mox.defmock(Trento.ActivityLog.ActivityLogger.Mock,
-  for: Trento.ActivityLog.ActivityLogger
-)
-
-activity_log_config =
-  :trento
-  |> Application.fetch_env!(Trento.ActivityLog.ActivityLogger)
-  |> Keyword.put(:adapter, Trento.ActivityLog.ActivityLogger.Mock)
-
-Application.put_env(:trento, Trento.ActivityLog.ActivityLogger, activity_log_config)
-
 Mox.defmock(Trento.Infrastructure.Messaging.Adapter.Mock,
   for: Trento.Infrastructure.Messaging.Adapter.Gen
 )

diff --git a/...tivity_logging/persistent_logger_test.exs → ...activity_logging/activity_logger_test.exs b/...tivity_logging/persistent_logger_test.exs → ...activity_logging/activity_logger_test.exs
@@ -1,7 +1,7 @@
-defmodule Trento.ActivityLog.PersistentLoggerTest do
+defmodule Trento.ActivityLog.ActivityLoggerTest do
   @moduledoc false
 
-  use TrentoWeb.ConnCase, async: false
+  use TrentoWeb.ConnCase, async: true
   use Plug.Test
 
   import Trento.Factory
@@ -11,7 +11,6 @@ defmodule Trento.ActivityLog.PersistentLoggerTest do
   alias Trento.Abilities.Ability
 
   alias Trento.ActivityLog.ActivityLog
-  alias Trento.ActivityLog.Logger.PersistentLogger
   alias Trento.Tags.Tag
 
   alias TrentoWeb.Auth.AccessToken
@@ -80,17 +79,16 @@ defmodule Trento.ActivityLog.PersistentLoggerTest do
         %{credentials: %{username: username} = credentials, expected_metadata: expected_metadata} =
           @scenario
 
-        conn = login(conn, credentials)
+        login(conn, credentials)
 
-        :ok = PersistentLogger.log_activity(conn)
+        wait_for_tasks_completion()
 
         assert [
                  %ActivityLog{
                    type: "login_attempt",
                    actor: ^username,
                    metadata: ^expected_metadata
                  }
-                 | _
                ] = Trento.Repo.all(ActivityLog)
       end
     end
@@ -143,12 +141,11 @@ defmodule Trento.ActivityLog.PersistentLoggerTest do
         resource_id = Faker.UUID.v4()
         tag = Faker.Lorem.word()
 
-        conn =
-          conn
-          |> with_token(user_id)
-          |> tag_resource(resource_id, path_resource, tag)
+        conn
+        |> with_token(user_id)
+        |> tag_resource(resource_id, path_resource, tag)
 
-        :ok = PersistentLogger.log_activity(conn)
+        wait_for_tasks_completion()
 
         assert [
                  %ActivityLog{
@@ -160,7 +157,6 @@ defmodule Trento.ActivityLog.PersistentLoggerTest do
                      "added_tag" => ^tag
                    }
                  }
-                 | _
                ] = Trento.Repo.all(ActivityLog)
       end
     end
@@ -180,12 +176,11 @@ defmodule Trento.ActivityLog.PersistentLoggerTest do
           resource_id: resource_id
         } = insert(:tag, resource_type: expected_resource_type)
 
-        conn =
-          conn
-          |> with_token(user_id)
-          |> untag_resource(resource_id, path_resource, tag)
+        conn
+        |> with_token(user_id)
+        |> untag_resource(resource_id, path_resource, tag)
 
-        :ok = PersistentLogger.log_activity(conn)
+        wait_for_tasks_completion()
 
         assert [
                  %ActivityLog{
@@ -197,7 +192,6 @@ defmodule Trento.ActivityLog.PersistentLoggerTest do
                      "removed_tag" => ^tag
                    }
                  }
-                 | _
                ] = Trento.Repo.all(ActivityLog)
       end
     end