Update code based on PR comments.

src/primaite/simulator/core.py

@@ -21,7 +21,7 @@ class ActionPermissionValidator(ABC):
 
     @abstractmethod
     def __call__(self, request: List[str], context: Dict) -> bool:
-        """TODO."""
+        """Use the request and context paramters to decide whether the action should be permitted."""
         pass
 
 
@@ -52,6 +52,10 @@ class Action:
         turning it off, then the SimComponent should have a turn_off(self) method that does not need to accept any args.
         Then, this Action will be given something like ``func = lambda request, context: self.turn_off()``.
 
+        ``validator`` is an instance of a subclass of `ActionPermissionValidator`. This is essentially a callable that
+        accepts `request` and `context` and returns a boolean to represent whether the permission is granted to perform
+        the action.
+
         :param func: Function that performs the request.
         :type func: Callable[[List[str], Dict], None]
         :param validator: Function that checks if the request is authenticated given the context.
@@ -62,14 +66,28 @@ class Action:
 
 
 class ActionManager:
-    """TODO."""
+    """
+    ActionManager is used by `SimComponent` instances to keep track of actions.
+
+    Its main purpose is to be a lookup from action name to action function and corresponding validation function. This
+    class is responsible for providing a consistent API for processing actions as well as helpful error messages.
+    """
 
     def __init__(self) -> None:
-        """TODO."""
+        """Initialise ActionManager with an empty action lookup."""
         self.actions: Dict[str, Action] = {}
 
     def process_request(self, request: List[str], context: Dict) -> None:
-        """Process action request."""
+        """Process an action request.
+
+        :param request: A list of strings which specify what action to take. The first string must be one of the allowed
+            actions, i.e. it must be a key of self.actions. The subsequent strings in the list are passed as parameters
+            to the action function.
+        :type request: List[str]
+        :param context: Dictionary of additional information necessary to process or validate the request.
+        :type context: Dict
+        :raises RuntimeError: If the request parameter does not have a valid action identifier as the first item.
+        """
         action_key = request[0]
 
         if action_key not in self.actions:
@@ -90,6 +108,18 @@ class ActionManager:
         action.func(action_options, context)
 
     def add_action(self, name: str, action: Action) -> None:
+        """Add an action to this action manager.
+
+        :param name: The string associated to this action.
+        :type name: str
+        :param action: Action object.
+        :type action: Action
+        """
+        if name in self.actions:
+            msg = f"Attempted to register an action but the action name {name} is already taken."
+            _LOGGER.error(msg)
+            raise RuntimeError(msg)
+
         self.actions[name] = action
 
 

src/primaite/simulator/domain/account.py

@@ -11,25 +11,25 @@ _LOGGER = getLogger(__name__)
 class AccountType(Enum):
     """Whether the account is intended for a user to log in or for a service to use."""
 
-    service = 1
+    SERVICE = 1
     "Service accounts are used to grant permissions to software on nodes to perform actions"
-    user = 2
+    USER = 2
     "User accounts are used to allow agents to log in and perform actions"
 
 
 class AccountStatus(Enum):
     """Whether the account is active."""
 
-    enabled = 1
+    ENABLED = 1
-    disabled = 2
+    DISABLED = 2
 
 
 class PasswordPolicyLevel(Enum):
     """Complexity requirements for account passwords."""
 
-    low = 1
+    LOW = 1
-    medium = 2
+    MEDIUM = 2
-    high = 3
+    HIGH = 3
 
 
 class Account(SimComponent):
@@ -47,7 +47,7 @@ class Account(SimComponent):
     "Account password."
     account_type: AccountType
     "Account Type, currently this can be service account (used by apps) or user account."
-    status: AccountStatus = AccountStatus.disabled
+    status: AccountStatus = AccountStatus.DISABLED
 
     def describe_state(self) -> Dict:
         """Describe state for agent observations."""
@@ -55,11 +55,11 @@ class Account(SimComponent):
 
     def enable(self):
         """Set the status to enabled."""
-        self.status = AccountStatus.enabled
+        self.status = AccountStatus.ENABLED
 
     def disable(self):
         """Set the status to disabled."""
-        self.status = AccountStatus.disabled
+        self.status = AccountStatus.DISABLED
 
     def log_on(self) -> None:
         """TODO."""

src/primaite/simulator/domain/controller.py

@@ -47,7 +47,11 @@ class GroupMembershipValidator(ActionPermissionValidator):
     """Permit actions based on group membership."""
 
     def __init__(self, allowed_groups: List[AccountGroup]) -> None:
-        """TODO."""
+        """Store a list of groups that should be granted permission.
+
+        :param allowed_groups: List of AccountGroups that are permitted to perform some action.
+        :type allowed_groups: List[AccountGroup]
+        """
         self.allowed_groups = allowed_groups
 
     def __call__(self, request: List[str], context: Dict) -> bool:
@@ -64,7 +68,7 @@ class DomainController(SimComponent):
     """Main object for controlling the domain."""
 
     # owned objects
-    accounts: List[Account] = []
+    accounts: Dict[str, Account] = {}
     groups: Final[List[AccountGroup]] = list(AccountGroup)
 
     domain_group_membership: Dict[Literal[AccountGroup.domain_admin, AccountGroup.domain_user], List[Account]] = {}
@@ -73,10 +77,10 @@ class DomainController(SimComponent):
     ] = {}
 
     # references to non-owned objects. Not sure if all are needed here.
-    nodes: List[temp_node] = []
+    nodes: Dict[str, temp_node] = {}
-    applications: List[temp_application] = []
+    applications: Dict[str, temp_application] = {}
-    folders: List[temp_folder] = []
+    folders: List[temp_folder] = {}
-    files: List[temp_file] = []
+    files: List[temp_file] = {}
 
     def _register_account(self, account: Account) -> None:
         """TODO."""