From 7845117fadcdaa4417d18c24d5bd213083eb13ec Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 21:55:52 +0100 Subject: [PATCH] refactor: improve error handling and documentation in PermissionSets - Add explicit ArgumentError for invalid permission set names with helpful message - Soften performance claim in documentation (intended to be constant-time) - Add tests for error handling - Improve maintainability with guard clause for invalid inputs --- lib/mv/authorization/permission_sets.ex | 11 +++++++-- .../mv/authorization/permission_sets_test.exs | 23 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/lib/mv/authorization/permission_sets.ex b/lib/mv/authorization/permission_sets.ex index 6139f7f..f9197de 100644 --- a/lib/mv/authorization/permission_sets.ex +++ b/lib/mv/authorization/permission_sets.ex @@ -38,7 +38,9 @@ defmodule Mv.Authorization.PermissionSets do ## Performance - All functions are pure and compile-time. Permission lookups are < 1 microsecond. + All functions are pure and intended to be constant-time. Permission lookups + are very fast (typically < 1 microsecond in practice) as they are simple + pattern matches and map lookups with no database queries or external calls. """ @type scope :: :own | :linked | :all @@ -81,10 +83,15 @@ defmodule Mv.Authorization.PermissionSets do true iex> PermissionSets.get_permissions(:invalid) - ** (FunctionClauseError) no function clause matching + ** (ArgumentError) invalid permission set: :invalid. Must be one of: [:own_data, :read_only, :normal_user, :admin] """ @spec get_permissions(atom()) :: permission_set() + def get_permissions(set) when set not in [:own_data, :read_only, :normal_user, :admin] do + raise ArgumentError, + "invalid permission set: #{inspect(set)}. Must be one of: #{inspect(all_permission_sets())}" + end + def get_permissions(:own_data) do %{ resources: [ diff --git a/test/mv/authorization/permission_sets_test.exs b/test/mv/authorization/permission_sets_test.exs index 06e2110..de960a9 100644 --- a/test/mv/authorization/permission_sets_test.exs +++ b/test/mv/authorization/permission_sets_test.exs @@ -567,4 +567,27 @@ defmodule Mv.Authorization.PermissionSetsTest do {:error, :invalid_permission_set} end end + + describe "get_permissions/1 - error handling" do + test "raises ArgumentError for invalid permission set with helpful message" do + assert_raise ArgumentError, + ~r/invalid permission set: :invalid\. Must be one of:/, + fn -> + PermissionSets.get_permissions(:invalid) + end + end + + test "error message includes all valid permission sets" do + error = + assert_raise ArgumentError, fn -> + PermissionSets.get_permissions(:unknown) + end + + error_message = Exception.message(error) + assert error_message =~ "own_data" + assert error_message =~ "read_only" + assert error_message =~ "normal_user" + assert error_message =~ "admin" + end + end end