Open api spex object refactor (#2780)

* Add struct?: false to all openapispex schemas

* Refactored controllers according to the new openapispex

* Bump openapispex to 3.19.1

* fix e2e tests

lib/trento/activity_log.ex

@@ -13,29 +13,32 @@ defmodule Trento.ActivityLog do
   alias Trento.Repo
 
   @spec get_settings() ::
-          {:ok, Settings.t()} | {:error, :activity_log_settings_not_configured}
+          {:ok, Settings.t()} | {:error, :not_found}
   def get_settings do
     case Repo.one(Settings.base_query()) do
       %Settings{} = settings -> {:ok, settings}
-      nil -> {:error, :activity_log_settings_not_configured}
+      nil -> {:error, :not_found}
     end
   end
 
   @spec change_retention_period(integer(), RetentionPeriodUnit.t()) ::
           {:ok, Settings.t()}
           | {:error, :activity_log_settings_not_configured}
-          | {:error, any()}
   def change_retention_period(value, unit) do
-    with {:ok, settings} <- get_settings() do
-      settings
-      |> Settings.changeset(%{
-        retention_time: %{
-          value: value,
-          unit: unit
-        }
-      })
-      |> Repo.update()
-      |> log_error("Error while updating activity log retention period")
+    case get_settings() do
+      {:ok, settings} ->
+        settings
+        |> Settings.changeset(%{
+          retention_time: %{
+            value: value,
+            unit: unit
+          }
+        })
+        |> Repo.update()
+        |> log_error("Error while updating activity log retention period")
+
+      {:error, :not_found} ->
+        {:error, :activity_log_settings_not_configured}
     end
   end
 

lib/trento_web/controllers/v1/cluster_controller.ex

@@ -79,8 +79,8 @@ defmodule TrentoWeb.V1.ClusterController do
       unprocessable_entity: OpenApiSpex.JsonErrorResponse.response()
     ]
 
-  def select_checks(%{body_params: body_params} = conn, %{cluster_id: cluster_id}) do
-    %{checks: checks} = body_params
+  def select_checks(conn, %{cluster_id: cluster_id}) do
+    %{checks: checks} = OpenApiSpex.body_params(conn)
 
     with :ok <- Clusters.select_checks(cluster_id, checks) do
       conn

lib/trento_web/controllers/v1/discovery_controller.ex

@@ -24,12 +24,16 @@ defmodule TrentoWeb.V1.DiscoveryController do
         conn,
         _
       ) do
-    body_params = Map.get(conn, :body_params)
+    %{
+      agent_id: agent_id,
+      discovery_type: discovery_type,
+      payload: payload
+    } = OpenApiSpex.body_params(conn)
 
     event = %{
-      "agent_id" => body_params.agent_id,
-      "discovery_type" => body_params.discovery_type,
-      "payload" => body_params.payload
+      "agent_id" => agent_id,
+      "discovery_type" => discovery_type,
+      "payload" => payload
     }
 
     with :ok <- Discovery.handle(event) do

lib/trento_web/controllers/v1/host_controller.ex

@@ -109,8 +109,8 @@ defmodule TrentoWeb.V1.HostController do
       unprocessable_entity: OpenApiSpex.JsonErrorResponse.response()
     ]
 
-  def select_checks(%{body_params: body_params} = conn, %{id: host_id}) do
-    %{checks: checks} = body_params
+  def select_checks(conn, %{id: host_id}) do
+    %{checks: checks} = OpenApiSpex.body_params(conn)
 
     with :ok <- Hosts.select_checks(host_id, checks) do
       conn

lib/trento_web/controllers/v1/profile_controller.ex

@@ -34,8 +34,9 @@ defmodule TrentoWeb.V1.ProfileController do
       forbidden: Schema.Forbidden.response()
     ]
 
-  def update(%{body_params: profile_params} = conn, _) do
+  def update(conn, _) do
     %User{} = user = Pow.Plug.current_user(conn)
+    profile_params = OpenApiSpex.body_params(conn)
 
     with {:ok, %User{} = updated_user} <- Users.update_user_profile(user, profile_params) do
       render(conn, "profile.json", user: updated_user)
@@ -90,9 +91,10 @@ defmodule TrentoWeb.V1.ProfileController do
       forbidden: Schema.Forbidden.response()
     ]
 
-  def confirm_totp_enrollment(%{body_params: body_params} = conn, _) do
+  def confirm_totp_enrollment(conn, _) do
     %User{} = user = Pow.Plug.current_user(conn)
-    totp_code = Map.get(body_params, :totp_code)
+
+    %{totp_code: totp_code} = OpenApiSpex.body_params(conn)
 
     with {:ok, %User{totp_enabled_at: totp_enabled_at}} <-
            Users.confirm_totp_enrollment(user, totp_code) do

lib/trento_web/controllers/v1/settings_controller.ex

@@ -87,8 +87,8 @@ defmodule TrentoWeb.V1.SettingsController do
       not_found: Schema.NotFound.response()
     ]
 
-  def update_api_key_settings(%{body_params: body_params} = conn, _) do
-    %{expire_at: expire_at} = body_params
+  def update_api_key_settings(conn, _) do
+    %{expire_at: expire_at} = OpenApiSpex.body_params(conn)
 
     with {:ok, updated_settings} <- Settings.update_api_key_settings(expire_at) do
       api_key =
@@ -116,9 +116,12 @@ defmodule TrentoWeb.V1.SettingsController do
       unprocessable_entity: Schema.UnprocessableEntity.response()
     ]
 
-  def update_activity_log_settings(%{body_params: body_params} = conn, _) do
+  def update_activity_log_settings(
+        conn,
+        _
+      ) do
     %{retention_time: %{value: retention_period, unit: retention_period_unit}} =
-      body_params
+      OpenApiSpex.body_params(conn)
 
     with {:ok, updated_settings} <-
            ActivityLog.change_retention_period(retention_period, retention_period_unit) do
@@ -139,18 +142,10 @@ defmodule TrentoWeb.V1.SettingsController do
     ]
 
   def get_activity_log_settings(conn, _) do
-    case ActivityLog.get_settings() do
-      {:ok, settings} ->
-        render(conn, "activity_log_settings.json", %{
-          activity_log_settings: settings
-        })
-
-      _ ->
-        # Here we rebind the error returned by the ActivityLog.get_settings/0 function
-        # to a "not_found" in order to disambiguate from a similar error being returned by the
-        # ActivityLog.change_retention_period/2 function. This error gets picked up by the FallbackController
-        # and leads to dispatching to the appropriate error response.
-        {:error, :not_found}
+    with {:ok, settings} <- ActivityLog.get_settings() do
+      render(conn, "activity_log_settings.json", %{
+        activity_log_settings: settings
+      })
     end
   end
 

lib/trento_web/controllers/v1/suma_credentials_controller.ex

@@ -49,10 +49,10 @@ defmodule TrentoWeb.V1.SUMACredentialsController do
     ]
 
   @spec create(Plug.Conn.t(), any) :: Plug.Conn.t()
-  def create(%{body_params: body_params} = conn, _) do
-    attrs = decode_body(body_params)
+  def create(conn, _) do
+    settings_params = OpenApiSpex.body_params(conn)
 
-    with {:ok, saved_settings} <- SoftwareUpdates.save_settings(attrs) do
+    with {:ok, saved_settings} <- SoftwareUpdates.save_settings(settings_params) do
       conn
       |> put_status(:created)
       |> render("suma_credentials.json", %{settings: saved_settings})
@@ -72,10 +72,10 @@ defmodule TrentoWeb.V1.SUMACredentialsController do
     ]
 
   @spec update(Plug.Conn.t(), any) :: Plug.Conn.t()
-  def update(%{body_params: body_params} = conn, _) do
-    attrs = decode_body(body_params)
+  def update(conn, _) do
+    update_settings_paylod = OpenApiSpex.body_params(conn)
 
-    with {:ok, saved_settings} <- SoftwareUpdates.change_settings(attrs) do
+    with {:ok, saved_settings} <- SoftwareUpdates.change_settings(update_settings_paylod) do
       conn
       |> put_status(:ok)
       |> render("suma_credentials.json", %{settings: saved_settings})
@@ -116,7 +116,4 @@ defmodule TrentoWeb.V1.SUMACredentialsController do
   end
 
   def get_policy_resource(_), do: Trento.SoftwareUpdates.Settings
-
-  defp decode_body(body) when is_struct(body), do: Map.from_struct(body)
-  defp decode_body(body), do: body
 end

lib/trento_web/controllers/v1/tags_controller.ex

@@ -52,7 +52,7 @@ defmodule TrentoWeb.V1.TagsController do
         } = conn,
         %{id: id}
       ) do
-    %{value: value} = Map.get(conn, :body_params)
+    %{value: value} = OpenApiSpex.body_params(conn)
 
     with {:ok, %Tag{value: value}} <- Tags.add_tag(value, id, resource_type) do
       conn

lib/trento_web/openapi/v1/schema/ability.ex

@@ -7,37 +7,43 @@ defmodule TrentoWeb.OpenApi.V1.Schema.Ability do
   defmodule AbilityItem do
     @moduledoc false
 
-    OpenApiSpex.schema(%{
-      title: "Ability",
-      description: "Ability entry",
-      type: :object,
-      additionalProperties: false,
-      properties: %{
-        id: %Schema{type: :integer, description: "Ability ID", nullable: false},
-        name: %Schema{type: :string, description: "Ability name", nullable: false},
-        resource: %Schema{
-          type: :string,
-          description: "Resource attached to ability",
-          nullable: false
+    OpenApiSpex.schema(
+      %{
+        title: "Ability",
+        description: "Ability entry",
+        type: :object,
+        additionalProperties: false,
+        properties: %{
+          id: %Schema{type: :integer, description: "Ability ID", nullable: false},
+          name: %Schema{type: :string, description: "Ability name", nullable: false},
+          resource: %Schema{
+            type: :string,
+            description: "Resource attached to ability",
+            nullable: false
+          },
+          label: %Schema{
+            type: :string,
+            description: "Description of the ability",
+            nullable: false
+          }
         },
-        label: %Schema{
-          type: :string,
-          description: "Description of the ability",
-          nullable: false
-        }
+        required: [:id, :name, :resource]
       },
-      required: [:id, :name, :resource]
-    })
+      struct?: false
+    )
   end
 
   defmodule AbilityCollection do
     @moduledoc false
 
-    OpenApiSpex.schema(%{
-      title: "AbilityCollection",
-      description: "A collection of abilities in the system",
-      type: :array,
-      items: AbilityItem
-    })
+    OpenApiSpex.schema(
+      %{
+        title: "AbilityCollection",
+        description: "A collection of abilities in the system",
+        type: :array,
+        items: AbilityItem
+      },
+      struct?: false
+    )
   end
 end

lib/trento_web/openapi/v1/schema/activity_log.ex

@@ -4,37 +4,40 @@ defmodule TrentoWeb.OpenApi.V1.Schema.ActivityLog do
   require OpenApiSpex
   alias OpenApiSpex.Schema
 
-  OpenApiSpex.schema(%{
-    title: "ActivityLog",
-    description: "Activity Log for the current installation.",
-    type: :array,
-    items: %Schema{
-      title: "ActivityLogEntry",
-      type: :object,
-      additionalProperties: false,
-      properties: %{
-        id: %Schema{
-          type: :string,
-          description: "Identifier of Activity Log entry.",
-          format: :uuid
+  OpenApiSpex.schema(
+    %{
+      title: "ActivityLog",
+      description: "Activity Log for the current installation.",
+      type: :array,
+      items: %Schema{
+        title: "ActivityLogEntry",
+        type: :object,
+        additionalProperties: false,
+        properties: %{
+          id: %Schema{
+            type: :string,
+            description: "Identifier of Activity Log entry.",
+            format: :uuid
+          },
+          type: %Schema{
+            type: :string,
+            description: "Type of Activity Log entry."
+          },
+          actor: %Schema{
+            type: :string,
+            description: "Actor causing an Activity Log entry. E.g. System or a specific user."
+          },
+          metadata: %Schema{
+            type: :object
+          },
+          occurred_on: %Schema{
+            type: :string,
+            description: "Timestamp upon Activity Log entry insertion."
+          }
         },
-        type: %Schema{
-          type: :string,
-          description: "Type of Activity Log entry."
-        },
-        actor: %Schema{
-          type: :string,
-          description: "Actor causing an Activity Log entry. E.g. System or a specific user."
-        },
-        metadata: %Schema{
-          type: :object
-        },
-        occurred_on: %Schema{
-          type: :string,
-          description: "Timestamp upon Activity Log entry insertion."
-        }
-      },
-      required: [:id, :type, :actor, :metadata, :occurred_on]
-    }
-  })
+        required: [:id, :type, :actor, :metadata, :occurred_on]
+      }
+    },
+    struct?: false
+  )
 end