Add NOT NULL constraint to users.role_id and optimize default_role_id

- Add database-level NOT NULL constraint for users.role_id
- Update SystemActor tests to verify NOT NULL constraint enforcement
- Add process dictionary caching for default_role_id/0 to reduce DB queries

lib/accounts/user.ex

@@ -442,6 +442,15 @@ defmodule Mv.Accounts.User do
   It fetches the "Mitglied" role from the database without authorization checks
   (safe during user creation bootstrap phase).
 
+  The result is cached in the process dictionary to avoid repeated database queries
+  during high-volume user creation. The cache is invalidated on application restart.
+
+  ## Performance Note
+
+  This function makes one database query per process (cached in process dictionary).
+  For very high-volume scenarios, consider using a fixed UUID from Application config
+  instead of querying the database.
+
   ## Returns
 
   - UUID of the "Mitglied" role if it exists
@@ -454,9 +463,20 @@ defmodule Mv.Accounts.User do
   """
   @spec default_role_id() :: Ecto.UUID.t() | nil
   def default_role_id do
-    case Mv.Authorization.Role.get_mitglied_role() do
-      {:ok, %Mv.Authorization.Role{id: role_id}} -> role_id
-      _ -> nil
+    # Cache in process dictionary to avoid repeated queries
+    case Process.get({__MODULE__, :default_role_id}) do
+      nil ->
+        role_id =
+          case Mv.Authorization.Role.get_mitglied_role() do
+            {:ok, %Mv.Authorization.Role{id: id}} -> id
+            _ -> nil
+          end
+
+        Process.put({__MODULE__, :default_role_id}, role_id)
+        role_id
+
+      cached_role_id ->
+        cached_role_id
     end
   end
 end

priv/repo/migrations/20260125155125_add_not_null_constraint_to_users_role_id.exs

@@ -0,0 +1,36 @@
+defmodule Mv.Repo.Migrations.AddNotNullConstraintToUsersRoleId do
+  @moduledoc """
+  Adds NOT NULL constraint to users.role_id column.
+
+  This ensures that role_id can never be NULL at the database level,
+  providing an additional safety layer beyond Ash's allow_nil? false.
+
+  Before running this migration, ensure all existing users have a role_id
+  (the previous migration AssignMitgliedRoleToExistingUsers handles this).
+  """
+  use Ecto.Migration
+
+  def up do
+    # First ensure all users have a role_id (safety check)
+    # This should already be done by the previous migration, but we check anyway
+    execute("""
+      UPDATE users
+      SET role_id = (
+        SELECT id FROM roles WHERE name = 'Mitglied' LIMIT 1
+      )
+      WHERE role_id IS NULL
+    """)
+
+    # Now add NOT NULL constraint
+    alter table(:users) do
+      modify :role_id, :uuid, null: false
+    end
+  end
+
+  def down do
+    # Remove NOT NULL constraint (allow NULL again)
+    alter table(:users) do
+      modify :role_id, :uuid, null: true
+    end
+  end
+end

priv/repo/seeds.exs

@@ -5,7 +5,6 @@
 
 alias Mv.Membership
 alias Mv.Accounts
-alias Mv.Authorization
 alias Mv.MembershipFees.MembershipFeeType
 alias Mv.MembershipFees.CycleGenerator
 

priv/resource_snapshots/repo/members/20260125155125.json

@@ -0,0 +1,221 @@
+{
+  "attributes": [
+    {
+      "allow_nil?": false,
+      "default": "fragment(\"uuid_generate_v7()\")",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": true,
+      "references": null,
+      "scale": null,
+      "size": null,
+      "source": "id",
+      "type": "uuid"
+    },
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": false,
+      "references": null,
+      "scale": null,
+      "size": null,
+      "source": "first_name",
+      "type": "text"
+    },
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": false,
+      "references": null,
+      "scale": null,
+      "size": null,
+      "source": "last_name",
+      "type": "text"
+    },
+    {
+      "allow_nil?": false,
+      "default": "nil",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": false,
+      "references": null,
+      "scale": null,
+      "size": null,
+      "source": "email",
+      "type": "text"
+    },
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": false,
+      "references": null,
+      "scale": null,
+      "size": null,
+      "source": "join_date",
+      "type": "date"
+    },
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": false,
+      "references": null,
+      "scale": null,
+      "size": null,
+      "source": "exit_date",
+      "type": "date"
+    },
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": false,
+      "references": null,
+      "scale": null,
+      "size": null,
+      "source": "notes",
+      "type": "text"
+    },
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": false,
+      "references": null,
+      "scale": null,
+      "size": null,
+      "source": "city",
+      "type": "text"
+    },
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": false,
+      "references": null,
+      "scale": null,
+      "size": null,
+      "source": "street",
+      "type": "text"
+    },
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": false,
+      "references": null,
+      "scale": null,
+      "size": null,
+      "source": "house_number",
+      "type": "text"
+    },
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": false,
+      "references": null,
+      "scale": null,
+      "size": null,
+      "source": "postal_code",
+      "type": "text"
+    },
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": false,
+      "references": null,
+      "scale": null,
+      "size": null,
+      "source": "search_vector",
+      "type": "tsvector"
+    },
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": false,
+      "references": null,
+      "scale": null,
+      "size": null,
+      "source": "membership_fee_start_date",
+      "type": "date"
+    },
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": false,
+      "references": {
+        "deferrable": false,
+        "destination_attribute": "id",
+        "destination_attribute_default": null,
+        "destination_attribute_generated": null,
+        "index?": false,
+        "match_type": null,
+        "match_with": null,
+        "multitenancy": {
+          "attribute": null,
+          "global": null,
+          "strategy": null
+        },
+        "name": "members_membership_fee_type_id_fkey",
+        "on_delete": null,
+        "on_update": null,
+        "primary_key?": true,
+        "schema": "public",
+        "table": "membership_fee_types"
+      },
+      "scale": null,
+      "size": null,
+      "source": "membership_fee_type_id",
+      "type": "uuid"
+    }
+  ],
+  "base_filter": null,
+  "check_constraints": [],
+  "custom_indexes": [],
+  "custom_statements": [],
+  "has_create_action": true,
+  "hash": "107B69E0A6FDBE7FAE4B1EABBF3E8C3B1F004B8D96B3759C95071169288968CC",
+  "identities": [
+    {
+      "all_tenants?": false,
+      "base_filter": null,
+      "index_name": "members_unique_email_index",
+      "keys": [
+        {
+          "type": "atom",
+          "value": "email"
+        }
+      ],
+      "name": "unique_email",
+      "nils_distinct?": true,
+      "where": null
+    }
+  ],
+  "multitenancy": {
+    "attribute": null,
+    "global": null,
+    "strategy": null
+  },
+  "repo": "Elixir.Mv.Repo",
+  "schema": null,
+  "table": "members"
+}

priv/resource_snapshots/repo/users/20260125155125.json

@@ -0,0 +1,172 @@
+{
+  "attributes": [
+    {
+      "allow_nil?": false,
+      "default": "fragment(\"gen_random_uuid()\")",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": true,
+      "references": null,
+      "scale": null,
+      "size": null,
+      "source": "id",
+      "type": "uuid"
+    },
+    {
+      "allow_nil?": false,
+      "default": "nil",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": false,
+      "references": null,
+      "scale": null,
+      "size": null,
+      "source": "email",
+      "type": "citext"
+    },
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": false,
+      "references": null,
+      "scale": null,
+      "size": null,
+      "source": "hashed_password",
+      "type": "text"
+    },
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": false,
+      "references": null,
+      "scale": null,
+      "size": null,
+      "source": "oidc_id",
+      "type": "text"
+    },
+    {
+      "allow_nil?": false,
+      "default": "nil",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": false,
+      "references": {
+        "deferrable": false,
+        "destination_attribute": "id",
+        "destination_attribute_default": null,
+        "destination_attribute_generated": null,
+        "index?": false,
+        "match_type": null,
+        "match_with": null,
+        "multitenancy": {
+          "attribute": null,
+          "global": null,
+          "strategy": null
+        },
+        "name": "users_role_id_fkey",
+        "on_delete": "restrict",
+        "on_update": null,
+        "primary_key?": true,
+        "schema": "public",
+        "table": "roles"
+      },
+      "scale": null,
+      "size": null,
+      "source": "role_id",
+      "type": "uuid"
+    },
+    {
+      "allow_nil?": true,
+      "default": "nil",
+      "generated?": false,
+      "precision": null,
+      "primary_key?": false,
+      "references": {
+        "deferrable": false,
+        "destination_attribute": "id",
+        "destination_attribute_default": null,
+        "destination_attribute_generated": null,
+        "index?": false,
+        "match_type": null,
+        "match_with": null,
+        "multitenancy": {
+          "attribute": null,
+          "global": null,
+          "strategy": null
+        },
+        "name": "users_member_id_fkey",
+        "on_delete": "nilify",
+        "on_update": null,
+        "primary_key?": true,
+        "schema": "public",
+        "table": "members"
+      },
+      "scale": null,
+      "size": null,
+      "source": "member_id",
+      "type": "uuid"
+    }
+  ],
+  "base_filter": null,
+  "check_constraints": [],
+  "custom_indexes": [],
+  "custom_statements": [],
+  "has_create_action": true,
+  "hash": "3E8D3C1A8834053B947F08369B81216A0B13019E5FD6FBFB706968FABA49EC06",
+  "identities": [
+    {
+      "all_tenants?": false,
+      "base_filter": null,
+      "index_name": "users_unique_email_index",
+      "keys": [
+        {
+          "type": "atom",
+          "value": "email"
+        }
+      ],
+      "name": "unique_email",
+      "nils_distinct?": true,
+      "where": null
+    },
+    {
+      "all_tenants?": false,
+      "base_filter": null,
+      "index_name": "users_unique_member_index",
+      "keys": [
+        {
+          "type": "atom",
+          "value": "member_id"
+        }
+      ],
+      "name": "unique_member",
+      "nils_distinct?": true,
+      "where": null
+    },
+    {
+      "all_tenants?": false,
+      "base_filter": null,
+      "index_name": "users_unique_oidc_id_index",
+      "keys": [
+        {
+          "type": "atom",
+          "value": "oidc_id"
+        }
+      ],
+      "name": "unique_oidc_id",
+      "nils_distinct?": true,
+      "where": null
+    }
+  ],
+  "multitenancy": {
+    "attribute": null,
+    "global": null,
+    "strategy": null
+  },
+  "repo": "Elixir.Mv.Repo",
+  "schema": null,
+  "table": "users"
+}

test/mv/helpers/system_actor_test.exs

@@ -4,8 +4,6 @@ defmodule Mv.Helpers.SystemActorTest do
   """
   use Mv.DataCase, async: false
 
-  import Ecto.Query
-
   alias Mv.Helpers.SystemActor
   alias Mv.Authorization
   alias Mv.Accounts
@@ -267,18 +265,10 @@ defmodule Mv.Helpers.SystemActorTest do
   end
 
   describe "edge cases" do
-    test "raises error if admin user has no role", %{admin_user: admin_user} do
-      # Remove role from admin user by directly setting role_id to NULL in database
-      # (We can't use Ash because allow_nil? false prevents setting role_id to nil)
-      # Convert UUID to binary format for Postgrex
-      admin_user_id = Ecto.UUID.cast!(admin_user.id)
-
-      Mv.Repo.update_all(
-        from(u in "users", where: u.id == type(^admin_user_id, :binary_id)),
-        set: [role_id: nil]
-      )
-
-      # Delete system user to force fallback
+    test "raises error if admin user has invalid role (role loading fails)", %{
+      admin_user: admin_user
+    } do
+      # Delete system user to force fallback to admin user
       system_actor = SystemActor.get_system_actor()
 
       case Accounts.User
@@ -291,12 +281,29 @@ defmodule Mv.Helpers.SystemActorTest do
           :ok
       end
 
-      SystemActor.invalidate_cache()
+      # Test that NOT NULL + FK constraints prevent setting role_id to NULL
+      # We verify this by attempting to set role_id to NULL and expecting a constraint violation
+      admin_user_id = Ecto.UUID.cast!(admin_user.id)
+      admin_user_id_binary = Ecto.UUID.dump!(admin_user_id)
 
-      # Should raise error because admin user has no role
-      assert_raise RuntimeError, ~r/System actor must have a role assigned/, fn ->
-        SystemActor.get_system_actor()
-      end
+      # Attempting to set role_id to NULL should fail due to NOT NULL constraint
+      assert_raise Postgrex.Error,
+                   ~r/null value in column.*role_id.*violates not-null constraint/i,
+                   fn ->
+                     Ecto.Adapters.SQL.query!(
+                       Mv.Repo,
+                       """
+                         UPDATE users
+                         SET role_id = NULL
+                         WHERE id = $1::uuid
+                       """,
+                       [admin_user_id_binary]
+                     )
+                   end
+
+      # Note: With NOT NULL + FK constraints, we can't test the "no role" case directly
+      # because the database prevents it. This is the desired behavior - the constraints
+      # guarantee that role_id is always valid.
     end
 
     test "handles concurrent calls without race conditions" do
@@ -372,23 +379,32 @@ defmodule Mv.Helpers.SystemActorTest do
       end
     end
 
-    test "raises error if system user has no role", %{system_user: system_user} do
-      # Remove role from system user by directly setting role_id to NULL in database
-      # (We can't use Ash because allow_nil? false prevents setting role_id to nil)
-      # Convert UUID to binary format for Postgrex
+    test "raises error if system user has invalid role (role loading fails)", %{
+      system_user: system_user
+    } do
+      # Test that NOT NULL + FK constraints prevent setting role_id to NULL
+      # We verify this by attempting to set role_id to NULL and expecting a constraint violation
       system_user_id = Ecto.UUID.cast!(system_user.id)
+      system_user_id_binary = Ecto.UUID.dump!(system_user_id)
 
-      Mv.Repo.update_all(
-        from(u in "users", where: u.id == type(^system_user_id, :binary_id)),
-        set: [role_id: nil]
-      )
+      # Attempting to set role_id to NULL should fail due to NOT NULL constraint
+      assert_raise Postgrex.Error,
+                   ~r/null value in column.*role_id.*violates not-null constraint/i,
+                   fn ->
+                     Ecto.Adapters.SQL.query!(
+                       Mv.Repo,
+                       """
+                         UPDATE users
+                         SET role_id = NULL
+                         WHERE id = $1::uuid
+                       """,
+                       [system_user_id_binary]
+                     )
+                   end
 
-      SystemActor.invalidate_cache()
-
-      # Should raise error because system user has no role
-      assert_raise RuntimeError, ~r/System actor must have a role assigned/, fn ->
-        SystemActor.get_system_actor()
-      end
+      # Note: With NOT NULL + FK constraints, we can't test the "no role" case directly
+      # because the database prevents it. This is the desired behavior - the constraints
+      # guarantee that role_id is always valid.
     end
   end
 end