Fixups from review

Author: nickanderson

promise-types/appstreams/appstreams.py

@@ -54,38 +54,37 @@ def __init__(self, **kwargs):
         )
 
     def _validate_state(self, value):
-        if value not in (
+        accepted = (
             "enabled",
             "disabled",
             "installed",
             "removed",
             "default",
             "reset",
-        ):
-            raise ValidationError(
-                "State attribute must be 'enabled', 'disabled', 'installed', "
-                "'removed', 'default', or 'reset'"
-            )
+        )
+        if value not in accepted:
+            accepted_str = "', '".join(accepted)
+            raise ValidationError(f"State attribute must be '{accepted_str}'")
 
     def _validate_module_name(self, name):
         # Validate module name to prevent injection
-        if not re.match(r"^[a-zA-Z0-9_.-]+$", name):
+        if not re.fullmatch(r"[a-zA-Z0-9_.-]+", name):
             raise ValidationError(
                 f"Invalid module name: {name}. Only alphanumeric, underscore, "
                 f"dot, and dash characters are allowed."
             )
 
     def _validate_stream_name(self, stream):
         # Validate stream name to prevent injection
-        if stream and not re.match(r"^[a-zA-Z0-9_.-]+$", stream):
+        if stream and not re.fullmatch(r"[a-zA-Z0-9_.-]+", stream):
             raise ValidationError(
                 f"Invalid stream name: {stream}. Only alphanumeric, underscore, "
                 f"dot, and dash characters are allowed."
             )
 
     def _validate_profile_name(self, profile):
         # Validate profile name to prevent injection
-        if profile and not re.match(r"^[a-zA-Z0-9_.-]+$", profile):
+        if profile and not re.fullmatch(r"[a-zA-Z0-9_.-]+", profile):
             raise ValidationError(
                 f"Invalid profile name: {profile}. Only alphanumeric, underscore, "
                 f"dot, and dash characters are allowed."

promise-types/appstreams/test_appstreams_logic.py

@@ -110,7 +110,8 @@ def test_install_profile_repaired(module, mock_base, mock_mpc):
     # Initial state: enabled but not fully installed with profile
     mock_mpc.getModuleState.return_value = mock_mpc.ModuleState_ENABLED
     mock_mpc.getEnabledStream.return_value = "12"
-    mock_mpc.getInstalledProfiles.return_value = []
+    # First call (pre-install check) returns [], second call (post-install verify) returns ["common"]
+    mock_mpc.getInstalledProfiles.side_effect = [[], ["common"]]
 
     # helper for _get_profile_packages
     # It queries module, gets stream, gets profiles, gets content
@@ -132,7 +133,11 @@ def test_install_profile_repaired(module, mock_base, mock_mpc):
 
 def test_remove_module_repaired(module, mock_base, mock_mpc):
     """Test removing a module using 'removed' state (REPAIRED)"""
-    mock_mpc.getModuleState.return_value = mock_mpc.ModuleState_INSTALLED
+    # First call (current state check) returns INSTALLED, second (post-remove verify) returns DEFAULT
+    mock_mpc.getModuleState.side_effect = [
+        mock_mpc.ModuleState_INSTALLED,
+        mock_mpc.ModuleState_DEFAULT,
+    ]
     mock_mpc.getEnabledStream.return_value = "12"
     mock_mpc.getInstalledProfiles.return_value = ["common"]
 
@@ -167,7 +172,12 @@ def test_install_profile_idempotency_success(module, mock_base, mock_mpc):
 
 def test_reset_module_repaired(module, mock_base, mock_mpc):
     """Test resetting a module to default state (REPAIRED)"""
-    mock_mpc.getModuleState.return_value = mock_mpc.ModuleState_ENABLED
+    # evaluate_promise calls _get_module_state first, then _reset_module calls it twice more
+    mock_mpc.getModuleState.side_effect = [
+        mock_mpc.ModuleState_ENABLED,  # evaluate_promise current-state check
+        mock_mpc.ModuleState_ENABLED,  # _reset_module early-exit check
+        mock_mpc.ModuleState_DEFAULT,  # _reset_module post-reset verification
+    ]
 
     result = module.evaluate_promise("nodejs", {"state": "default", "stream": "12"}, {})