The purpose of these changes was primarily to enhance code readabilit…

…y and maintenance, and secondly to improve performance through caching.

In `userservice.py`, a change was made on how `can_grow_service_pool` method's result is evaluated, from explicitly comparing it with `False` to a pythonic approach of using `not`. This makes the code cleaner and easier to read.

In `service.py` file, a variable name was changed from `services_limit` to `userservices_limit_field`. It appears much more logical namewise as the variable deals with limiting the number of user services.

In the `fixed/service.py` file, the code was refactored for readability and DRY (Don't Repeat Yourself) principle, avoiding code repetition by extracting a recurring pattern into a separate method called `_get_machines_field`.

In `authenticator.py`, a caching mechanism was introduced. If an Authenticator object is already created and no update in values is needed, the existing Authenticator object is returned. This prevents unnecessary creation of new Authenticator objects, thereby improving performance.

In the `managed_object_model.py` file, the instruction to clear the cache was removed. This coincides with the caching strategy introduced in `authenticator.py`.

Lastly, in `OpenGnsys/service.py`, `services_limit` was renamed to `userservices_limit_field`, consistent with the changes in `service.py`, improving code consistency.

server/src/uds/core/managers/userservice.py

@@ -204,7 +204,7 @@ def create_assigned_for(self, service_pool: ServicePool, user: User) -> UserServ
         Creates a new assigned deployed service for the current publication (if any) of service pool and user indicated
         """
         # First, honor concurrent_creation_limit
-        if self.can_grow_service_pool(service_pool) is False:
+        if not self.can_grow_service_pool(service_pool):
             # Cannot create new
             operations_logger.info(
                 'Too many preparing services. Creation of assigned service denied by max preparing services parameter. (login storm with insufficient cache?).'

server/src/uds/core/services/generics/fixed/service.py

@@ -162,6 +162,9 @@ def _assigned_access(self) -> typing.Iterator[set[str]]:
             # If has changed, save it
             if machines != initial_machines:
                 d['vms'] = machines  # Store it
+
+    def _get_machines_field(self) -> gui.MultiChoiceField:
+        return typing.cast(gui.MultiChoiceField, getattr(self, self.alternate_machines_field or 'machines'))
 
     def snapshot_creation(self, userservice_instance: 'FixedUserService') -> None:
         """
@@ -178,7 +181,7 @@ def snapshot_recovery(self, userservice_instance: 'FixedUserService') -> None:
     def unmarshal(self, data: bytes) -> None:
         super().unmarshal(data)
         # Recover userservice
-        self.userservices_limit = len(self.machines.as_list())
+        self.userservices_limit = len(self._get_machines_field().as_list())
 
     @abc.abstractmethod
     def get_name(self, vmid: str) -> str:
@@ -252,7 +255,7 @@ def assign_from_assignables(
         Default implementation NEEDS machines field to be present!!
         """
         fixed_instance = typing.cast('FixedUserService', userservice_instance)
-        machines = typing.cast(gui.MultiChoiceField, getattr(self, self.alternate_machines_field or 'machines' ))
+        machines = self._get_machines_field()
         with self._assigned_access() as assigned_vms:
             if assignable_id not in assigned_vms and assignable_id in machines.as_list():
                 assigned_vms.add(assignable_id)
@@ -264,12 +267,10 @@ def sorted_assignables_list(self) -> list[str]:
         """
         Randomizes the assignation of machines if needed
         """
-        fld_name = self.alternate_machines_field or 'machines'
-        if self.has_field(fld_name) is False:
-            raise ValueError(f'machines field {fld_name} not found')
-        machines = typing.cast(gui.MultiChoiceField, getattr(self, fld_name))
+        machines = self._get_machines_field()
 
         if hasattr(self, 'randomize') and self.randomize.value is True:
+            # Randomize the list
             return random.sample(machines.as_list(), len(machines.as_list()))
 
         return machines.as_list()

server/src/uds/core/services/service.py

@@ -120,11 +120,11 @@ class Service(Module):
     # : Normally set to UNLIMITED. This attribute indicates if the service has some "limitation"
     # : for providing user services. This attribute can be set here or
     # : modified at instance level, core will access always to it using an instance object.
-    # : Note: you can override this value on service instantiation by providing a "services_limit":
-    # :      - If services_limit is an integer, it will be used as userservices_limit
-    # :      - If services_limit is a gui.NumericField, it will be used as userservices_limit (.num() will be called)
-    # :      - If services_limit is a callable, it will be called and the result will be used as userservices_limit
-    # :      - If services_limit is None, userservices_limit will be set to consts.UNLIMITED (as default)
+    # : Note: you can override this value on service instantiation by providing a "userservices_limit_field":
+    # :      - If userservices_limit_field is an integer, it will be used as userservices_limit
+    # :      - If userservices_limit_field is a gui.NumericField, it will be used as userservices_limit (.num() will be called)
+    # :      - If userservices_limit_field is a callable, it will be called and the result will be used as userservices_limit
+    # :      - If userservices_limit_field is None, userservices_limit will be set to consts.UNLIMITED (as default)
     userservices_limit: int = consts.UNLIMITED
 
     # : If this item "has overrided fields", on deployed service edition, defined keys will overwrite defined ones
@@ -278,23 +278,23 @@ def allow_putting_back_to_cache(self) -> bool:
 
     def unmarshal(self, data: bytes) -> None:
         # In fact, we will not unmarshal anything here, but setup maxDeployed
-        # if services_limit exists and it is a gui.NumericField
+        # if userservices_limit_field exists and it is a gui.NumericField
         # Invoke base unmarshal, so "gui fields" gets loaded from data
         super().unmarshal(data)
 
-        if hasattr(self, 'services_limit'):
+        if hasattr(self, 'userservices_limit_field'):
             # Fix self "userservices_limit" value after loading fields
             try:
-                services_limit = getattr(self, 'services_limit', None)
-                if isinstance(services_limit, int):
-                    self.userservices_limit = services_limit
-                elif isinstance(services_limit, gui.NumericField):
-                    self.userservices_limit = services_limit.as_int()
+                userservices_limit_field = getattr(self, 'userservices_limit_field', None)
+                if isinstance(userservices_limit_field, int):
+                    self.userservices_limit = userservices_limit_field
+                elif isinstance(userservices_limit_field, gui.NumericField):
+                    self.userservices_limit = userservices_limit_field.as_int()
                     # For 0 values on userservices_limit field, we will set it to UNLIMITED
                     if self.userservices_limit == 0:
                         self.userservices_limit = consts.UNLIMITED
-                elif callable(services_limit):
-                    self.userservices_limit = typing.cast(collections.abc.Callable[..., int], services_limit)()
+                elif callable(userservices_limit_field):
+                    self.userservices_limit = typing.cast(collections.abc.Callable[..., int], userservices_limit_field)()
                 else:
                     self.userservices_limit = consts.UNLIMITED
             except Exception:
@@ -304,7 +304,7 @@ def unmarshal(self, data: bytes) -> None:
             if self.userservices_limit < 0:
                 self.userservices_limit = consts.UNLIMITED
 
-        # Keep untouched if services_limit is not present
+        # Keep untouched if userservices_limit_field is not present
 
     def mac_generator(self) -> 'UniqueMacGenerator':
         """

server/src/uds/models/authenticator.py

@@ -102,16 +102,22 @@ def get_instance(self, values: ui.gui.ValuesType = None) -> auths.Authenticator:
 
         Raises:
         """
+        if self._cached_instance and values is None:
+            return typing.cast(auths.Authenticator, self._cached_instance)
+
         if not self.id:
             # Return a fake authenticator
             return auths.Authenticator(
                 environment.Environment.environment_for_table_record('fake_auth'), values, uuid=self.uuid
             )
 
-        auType = self.get_type()
+        auth_type = self.get_type()
         env = self.get_environment()
-        auth = auType(env, values, uuid=self.uuid)
+        auth = auth_type(env, values, uuid=self.uuid)
         self.deserialize(auth, values)
+
+        self._cached_instance = auth
+
         return auth
 
     def get_type(self) -> type[auths.Authenticator]:

server/src/uds/models/managed_object_model.py

@@ -88,8 +88,6 @@ def deserialize(self, obj: 'Module', values: typing.Optional[collections.abc.Map
                 self.save(update_fields=['data'])
                 obj.mark_for_upgrade(False)
 
-        self._cached_instance = None  # Ensures returns correct value on get_instance
-
     def get_instance(self, values: typing.Optional[dict[str, str]] = None) -> 'Module':
         """
         Instantiates the object this record contains.

server/src/uds/services/OpenGnsys/service.py

@@ -143,7 +143,7 @@ class OGService(services.Service):
         old_field_name='startIfUnavailable',
     )
 
-    services_limit = fields.services_limit_field()
+    userservices_limit_field = fields.services_limit_field()
 
     prov_uuid = gui.HiddenField(value=None)