Handle concurrent settings saving operations

lib/trento/software_updates.ex

@@ -25,12 +25,14 @@ defmodule Trento.SoftwareUpdates do
 
   @spec get_settings :: {:ok, Settings.t()} | {:error, :settings_not_configured}
   def get_settings do
-    case Repo.one(Settings) do
-      nil ->
-        {:error, :settings_not_configured}
+    settings = Repo.one!(Settings)
 
-      settings ->
+    case has_settings?(settings) do
+      true ->
         {:ok, settings}
+
+      false ->
+        {:error, :settings_not_configured}
     end
   end
 
@@ -39,8 +41,8 @@ defmodule Trento.SoftwareUpdates do
           | {:error, :settings_already_configured}
           | {:error, any()}
   def save_settings(settings_submission, date_service \\ DateService) do
-    with :ok <- ensure_no_settings_configured() do
-      save_new_settings(settings_submission, date_service)
+    with {:ok, :settings_not_configured, settings} <- ensure_no_settings_configured() do
+      save_new_settings(settings, settings_submission, date_service)
     end
   end
 
@@ -56,28 +58,39 @@ defmodule Trento.SoftwareUpdates do
 
   @spec clear_settings :: :ok
   def clear_settings do
-    Repo.delete_all(Settings)
+    Repo.update_all(Settings,
+      set: [
+        url: nil,
+        username: nil,
+        password: nil,
+        ca_cert: nil,
+        ca_uploaded_at: nil,
+        updated_at: DateTime.utc_now()
+      ]
+    )
 
     :ok
   end
 
+  defp has_settings?(%Settings{url: url, username: username, password: password}),
+    do: url != nil and username != nil and password != nil
+
   defp ensure_no_settings_configured do
-    case get_settings() do
-      {:error, :settings_not_configured} ->
-        :ok
+    case has_settings?(settings = Repo.one!(Settings)) do
+      false ->
+        {:ok, :settings_not_configured, settings}
 
-      {:ok, %Settings{} = _} ->
+      true ->
         Logger.error("Error: software updates settings already configured")
-
         {:error, :settings_already_configured}
     end
   end
 
-  defp save_new_settings(settings_submission, date_service) do
+  defp save_new_settings(%Settings{} = settings, settings_submission, date_service) do
     saving_result =
-      %Settings{}
+      settings
       |> Settings.changeset(settings_submission, date_service)
-      |> Repo.insert()
+      |> Repo.update()
 
     case saving_result do
       {:ok, _} = success ->

lib/trento/software_updates/settings.ex

@@ -11,7 +11,7 @@ defmodule Trento.SoftwareUpdates.Settings do
 
   @type t :: %__MODULE__{}
 
-  @primary_key {:id, :binary_id, autogenerate: true}
+  @primary_key {:id, :binary_id, autogenerate: false}
   schema "software_update_settings" do
     field :url, :string
     field :username, :string

priv/repo/migrations/20240214153742_add_uniqueness_to_software_updates_settings.exs

@@ -0,0 +1,28 @@
+defmodule Trento.Repo.Migrations.AddUniquenessToSoftwareUpdatesSettings do
+  use Ecto.Migration
+
+  def change do
+    %Postgrex.Result{rows: rows} =
+      repo().query!("SELECT id FROM software_update_settings;", [], log: false)
+
+    settings_identifier =
+      case rows do
+        [] -> UUID.uuid4()
+        [[binary_uuid | _] | _] -> UUID.binary_to_string!(binary_uuid)
+      end
+
+    alter table(:software_update_settings) do
+      modify :id, :uuid, default: settings_identifier
+    end
+
+    create constraint("software_update_settings", :only_one_record,
+             check: "id ='#{settings_identifier}'"
+           )
+
+    execute "INSERT INTO software_update_settings(id, inserted_at, updated_at) VALUES('#{settings_identifier}', NOW(), NOW()) ON CONFLICT DO NOTHING;"
+  end
+
+  def down do
+    drop constraint("software_update_settings", :only_one_record)
+  end
+end

test/trento/software_updates_test.exs

@@ -20,7 +20,10 @@ defmodule Trento.SoftwareUpdates.SettingsTest do
         username: username,
         password: password
       } =
-        insert(:software_updates_settings, ca_cert: nil, ca_uploaded_at: nil)
+        insert(:software_updates_settings, [ca_cert: nil, ca_uploaded_at: nil],
+          conflict_target: :id,
+          on_conflict: :replace_all
+        )
 
       assert {:ok,
               %Settings{
@@ -42,8 +45,9 @@ defmodule Trento.SoftwareUpdates.SettingsTest do
       } =
         insert(
           :software_updates_settings,
-          ca_cert: Faker.Lorem.sentence(),
-          ca_uploaded_at: DateTime.utc_now()
+          [ca_cert: Faker.Lorem.sentence(), ca_uploaded_at: DateTime.utc_now()],
+          conflict_target: :id,
+          on_conflict: :replace_all
         )
 
       assert {:ok,
@@ -204,7 +208,10 @@ defmodule Trento.SoftwareUpdates.SettingsTest do
     end
 
     test "should validate partial changes to software updates settings" do
-      insert(:software_updates_settings)
+      insert(:software_updates_settings, [],
+        conflict_target: :id,
+        on_conflict: :replace_all
+      )
 
       change_settings_scenarios = [
         %{
@@ -277,8 +284,9 @@ defmodule Trento.SoftwareUpdates.SettingsTest do
       } =
         insert(
           :software_updates_settings,
-          ca_cert: Faker.Lorem.sentence(),
-          ca_uploaded_at: DateTime.utc_now()
+          [ca_cert: Faker.Lorem.sentence(), ca_uploaded_at: DateTime.utc_now()],
+          conflict_target: :id,
+          on_conflict: :replace_all
         )
 
       change_submission = %{
@@ -314,8 +322,9 @@ defmodule Trento.SoftwareUpdates.SettingsTest do
       } =
         insert(
           :software_updates_settings,
-          ca_cert: Faker.Lorem.sentence(),
-          ca_uploaded_at: DateTime.utc_now()
+          [ca_cert: Faker.Lorem.sentence(), ca_uploaded_at: DateTime.utc_now()],
+          conflict_target: :id,
+          on_conflict: :replace_all
         )
 
       change_submission = %{
@@ -344,8 +353,9 @@ defmodule Trento.SoftwareUpdates.SettingsTest do
       } =
         insert(
           :software_updates_settings,
-          ca_cert: Faker.Lorem.sentence(),
-          ca_uploaded_at: DateTime.utc_now()
+          [ca_cert: Faker.Lorem.sentence(), ca_uploaded_at: DateTime.utc_now()],
+          conflict_target: :id,
+          on_conflict: :replace_all
         )
 
       change_submission = %{
@@ -367,8 +377,9 @@ defmodule Trento.SoftwareUpdates.SettingsTest do
     test "should support idempotent sequential clear settings" do
       insert(
         :software_updates_settings,
-        ca_cert: Faker.Lorem.sentence(),
-        ca_uploaded_at: DateTime.utc_now()
+        [ca_cert: Faker.Lorem.sentence(), ca_uploaded_at: DateTime.utc_now()],
+        conflict_target: :id,
+        on_conflict: :replace_all
       )
 
       assert {:ok, _} = SoftwareUpdates.get_settings()