Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open api spex object refactor #2780

Merged
merged 4 commits into from
Jul 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 16 additions & 13 deletions lib/trento/activity_log.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions lib/trento_web/controllers/v1/cluster_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 8 additions & 4 deletions lib/trento_web/controllers/v1/discovery_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lib/trento_web/controllers/v1/host_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions lib/trento_web/controllers/v1/profile_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
27 changes: 11 additions & 16 deletions lib/trento_web/controllers/v1/settings_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down
15 changes: 6 additions & 9 deletions lib/trento_web/controllers/v1/suma_credentials_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand All @@ -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})
Expand Down Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the unique real place where we are forced to decode to map?
As far as I see, we can get the remaining fields exactly the same way without the struct? field

defp decode_body(body), do: body
end
2 changes: 1 addition & 1 deletion lib/trento_web/controllers/v1/tags_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
56 changes: 31 additions & 25 deletions lib/trento_web/openapi/v1/schema/ability.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
67 changes: 35 additions & 32 deletions lib/trento_web/openapi/v1/schema/activity_log.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading