diff --git a/actor b/actor index 23901493b..386e227a8 160000 --- a/actor +++ b/actor @@ -1 +1 @@ -Subproject commit 23901493bbd0731ab22bf70aaed99f0b3ca7642d +Subproject commit 386e227a8a24372f78cfe68525fd9a3f99442bc7 diff --git a/server/src/uds/core/managers/userservice.py b/server/src/uds/core/managers/userservice.py index 00d6a1caf..5ba79271e 100644 --- a/server/src/uds/core/managers/userservice.py +++ b/server/src/uds/core/managers/userservice.py @@ -306,7 +306,7 @@ class UserServiceManager(metaclass=singleton.Singleton): def forced_move_assigned_to_cache_l1(self, user_service: UserService) -> None: """ - Clones the record of a user serviceself. + Clones the record of a user service. For this, the original userservice will ve moved to cache, and a new one will be created to mark it as "REMOVED" @@ -320,6 +320,7 @@ class UserServiceManager(metaclass=singleton.Singleton): user_service_copy.in_use = False user_service_copy.state = State.REMOVED user_service_copy.os_state = State.USABLE + log.log(user_service_copy, types.log.LogLevel.INFO, 'Service moved to cache') # Save the new element. user_service_copy.save() @@ -560,6 +561,7 @@ class UserServiceManager(metaclass=singleton.Singleton): if servicepool.uses_cache: cache: typing.Optional[UserService] = None # Now try to locate 1 from cache already "ready" (must be usable and at level 1) + # First, a cached service that is "fully" ready with transaction.atomic(): caches = typing.cast( list[UserService], @@ -574,6 +576,8 @@ class UserServiceManager(metaclass=singleton.Singleton): if caches: cache = caches[0] # Ensure element is reserved correctly on DB + # Avoid concurrency problems due to cache being assigned to user + # in the middle of this process if ( servicepool.cached_users_services() .select_for_update() @@ -587,6 +591,7 @@ class UserServiceManager(metaclass=singleton.Singleton): # Out of previous atomic if not cache: + # Now try to locate 1 from cache already "ready" (must be usable and at level 1) with transaction.atomic(): caches = typing.cast( list[UserService], @@ -606,9 +611,7 @@ class UserServiceManager(metaclass=singleton.Singleton): cache = None else: cache = None - - # Out of atomic transaction - if cache: + else: # Early assign cache.assign_to(user) @@ -885,7 +888,7 @@ class UserServiceManager(metaclass=singleton.Singleton): userservice: typing.Optional[UserService] = None if kind in 'A': # This is an assigned service - logger.debug('Getting assugned user service %s', uuid_userservice_pool) + logger.debug('Getting assigned user service %s', uuid_userservice_pool) try: userservice = UserService.objects.get(uuid=uuid_userservice_pool, user=user) userservice.service_pool.validate_user(user) diff --git a/server/tests/core/managers/test_userservice.py b/server/tests/core/managers/test_userservice.py index 36c4f6dce..570c49dd6 100644 --- a/server/tests/core/managers/test_userservice.py +++ b/server/tests/core/managers/test_userservice.py @@ -47,7 +47,7 @@ logger = logging.getLogger(__name__) class TestUserserviceManager(UDSTransactionTestCase): manager: UserServiceManager = UserServiceManager.manager() # For convenience debugging - + def test_forced_mode_assigned_to_l1(self) -> None: # Create an user service, we need userservice = services_fixtures.create_db_assigned_userservices()[0] @@ -55,15 +55,15 @@ class TestUserserviceManager(UDSTransactionTestCase): orig_uuid = userservice.uuid orig_src_ip = userservice.src_ip orig_src_hostname = userservice.src_hostname - + self.assertEqual(models.UserService.objects.all().count(), 1) # And uuser service is assigned to an user self.assertIsNotNone(userservice.user) # And cache level is None self.assertEqual(userservice.cache_level, core_types.services.CacheLevel.NONE) - + self.manager.forced_move_assigned_to_cache_l1(userservice) - + # Now, should have 2 user services, one in cache and one in db self.assertEqual(models.UserService.objects.all().count(), 2) # Reload userservice, that should be now in cache @@ -78,8 +78,7 @@ class TestUserserviceManager(UDSTransactionTestCase): # Source ip and hostname should be empty self.assertEqual(userservice.src_ip, '') self.assertEqual(userservice.src_hostname, '') - - + # Look for the created one (that is the assigned, deleted) assigned = models.UserService.objects.exclude(uuid=orig_uuid).get() self.assertEqual(assigned.cache_level, core_types.services.CacheLevel.NONE) @@ -87,12 +86,47 @@ class TestUserserviceManager(UDSTransactionTestCase): self.assertIsNotNone(assigned.user) # Should be removed self.assertEqual(assigned.state, core_types.states.State.REMOVED) - + # unique_id should be same as the original one self.assertEqual(userservice.unique_id, assigned.unique_id) # src_ip and src_hostname should be the original ones self.assertEqual(assigned.src_ip, orig_src_ip) self.assertEqual(assigned.src_hostname, orig_src_hostname) - + def test_release_from_logout(self) -> None: - pass \ No newline at end of file + pass + + def test_get_user_service_with_initial_no_cache(self) -> None: + userservice = services_fixtures.create_db_assigned_userservices()[0] + user = userservice.user + assert user is not None + + # Fix userservice to set it as cache l1 + userservice.cache_level = core_types.services.CacheLevel.L1 + userservice.user = None + userservice.save() + + service_pool = userservice.service_pool + + service_pool.initial_srvs = 1 + service_pool.cache_l1_srvs = 0 + service_pool.cache_l2_srvs = 0 + service_pool.max_srvs = 3 + service_pool.save() + + assigned_info = self.manager.get_user_service_info( + user, + core_types.os.DetectedOsInfo( + core_types.os.KnownOS.LINUX, core_types.os.KnownBrowser.CHROME, '1.0.0' + ), + '1.2.3.4', + f'P{service_pool.uuid}', + service_pool.transports.all()[0].uuid, + ) + + self.assertEqual(assigned_info.userservice.uuid, userservice.uuid) + self.assertEqual(assigned_info.userservice.cache_level, core_types.services.CacheLevel.NONE) + self.assertEqual(assigned_info.userservice.user, user) + self.assertEqual(assigned_info.userservice.src_ip, '1.2.3.4') + self.assertEqual(assigned_info.userservice.src_hostname, '1.2.3.4') + self.assertEqual(assigned_info.transport.uuid, service_pool.transports.all()[0].uuid) diff --git a/server/tests/fixtures/services.py b/server/tests/fixtures/services.py index 0cefe5f0f..4fd5156d2 100644 --- a/server/tests/fixtures/services.py +++ b/server/tests/fixtures/services.py @@ -188,7 +188,7 @@ def create_db_transport(transport_instance: 'Transport|None' = None, **kwargs: t def create_db_userservice( service_pool: models.ServicePool, publication: models.ServicePoolPublication, - user: models.User, + user: 'models.User|None', ) -> models.UserService: user_service: 'models.UserService' = service_pool.userServices.create( friendly_name='user-service-{}'.format(glob['user_service_id']),