diff --git a/djblets/avatars/registry.py b/djblets/avatars/registry.py index 6f0b9d300bfa85ff5eb58e4081c14dc8fed6906e..183df23460d1db8c1923dedebe059dda0e73dd7c 100644 --- a/djblets/avatars/registry.py +++ b/djblets/avatars/registry.py @@ -3,6 +3,7 @@ from __future__ import unicode_literals import logging +import warnings from django.utils.translation import ugettext_lazy as _ @@ -65,6 +66,9 @@ AVATAR_SERVICE_DEFAULT_ERRORS.update({ }) +logger = logging.getLogger(__name__) + + class AvatarServiceRegistry(Registry): """A registry for avatar services. @@ -72,6 +76,16 @@ class AvatarServiceRegistry(Registry): :py:mod:`djblets.avatars.services.gravatar` for an example). The registries are saved to the database and require the use of the :py:mod:`djblets.siteconfig` app. + + .. versionchanged:: 1.0.3 + + The avatar configuration is now retrieved from and immediately written + to the current :py:class:`~djblets.siteconfig.models.SiteConfiguration`, + in order to ensure that the list of enabled avatar services and the + default service are never stale. This differs from 1.0.0 through 1.0.2, + where the callers could make change to the local state without ever + risking it being written to the database (which is generally not the + desired behavior anyway). """ #: The key name for the list of enabled services. @@ -102,8 +116,6 @@ class AvatarServiceRegistry(Registry): """Initialize the avatar service registry.""" super(AvatarServiceRegistry, self).__init__() - self._enabled_services = set() - self._default_service_id = None self._instance_cache = {} def get_avatar_service(self, avatar_service_id): @@ -118,34 +130,40 @@ class AvatarServiceRegistry(Registry): Returns: djblets.avatars.services.base.AvatarService: - The requested avatar service. + The requested avatar service, or ``None`` if not found. Raises: - djblets.avatars.errors.AvatarServiceNotFoundError: - Raised if the avatar service cannot be found. - djblets.avatars.errors.DisabledServiceError: - Raised if the requested service is disabled. + The requested service is disabled. """ - if avatar_service_id not in self._instance_cache: + service = self._instance_cache.get(avatar_service_id) + + if service is None: try: service_cls = self.get('avatar_service_id', avatar_service_id) except self.lookup_error_class: service_cls = None - if service_cls: - if not self.is_enabled(service_cls): - raise DisabledServiceError(self.format_error( - DISABLED_SERVICE, - service_id=avatar_service_id)) - - service = self._instance_cache[avatar_service_id] = \ - service_cls(self.settings_manager_class) - else: + if not service_cls: return None else: - service = self._instance_cache[avatar_service_id] - assert self.is_enabled(type(service)) + service_cls = type(service) + + # It's important to note that if the service instance was in the + # cache before, that doesn't mean it should be considered enabled. + # Another SiteConfiguration instance on another process/server + # may have disabled it, and our SiteConfiguration may have picked + # that up since this was added to the cache. + if not self.is_enabled(service_cls): + self._instance_cache.pop(avatar_service_id, None) + + raise DisabledServiceError(self.format_error( + DISABLED_SERVICE, + service_id=avatar_service_id)) + + if service is None: + service = self._instance_cache[avatar_service_id] = \ + service_cls(self.settings_manager_class) return service @@ -155,10 +173,8 @@ class AvatarServiceRegistry(Registry): Yields: tuple: - djblets.avatars.forms.AvatarServiceConfigForm: - The enabled services that have configuration forms. + The enabled service instances that have configuration forms. """ - self.populate() return ( self.get_avatar_service(service.avatar_service_id) for service in self.enabled_services @@ -174,11 +190,10 @@ class AvatarServiceRegistry(Registry): The set of enabled avatar services, as :py:class:`djblets.avatars.service.AvatarService` instances. """ - self.populate() - + siteconfig = SiteConfiguration.objects.get_current() services = set() - for service_id in self._enabled_services: + for service_id in siteconfig.get(self.ENABLED_SERVICES_KEY): try: service = self.get('avatar_service_id', service_id) except self.lookup_error_class: @@ -196,6 +211,9 @@ class AvatarServiceRegistry(Registry): If the default service would be disabled by setting the set of enabled services, the default service will be set to ``None``. + This is equivalent to setting :py:attr:`enabled_services` with + ``save=True``. It's considered deprecated in favor of that method. + Args: services (set): The set of services to set as enabled. Each element must be a @@ -206,60 +224,100 @@ class AvatarServiceRegistry(Registry): This exception is raised when an unknown avatar service is enabled. """ - self.populate() + warnings.warn('Setting AvatarServiceRegistry.enabled_services is ' + 'deprecated in favor of calling ' + 'AvatarServiceRegistry.set_enabled_services.', + DeprecationWarning) - if not isinstance(services, set): - services = set(services) + self.set_enabled_services(services, save=True) + + def set_enabled_services(self, services, save=True): + """Set the enabled services. + + If the default service would be disabled by setting the set of enabled + services, the default service will be set to ``None``. + + Args: + services (set): + The set of services to set as enabled. Each element must be a + subclass of :py:class:`~djblets.avatars.service.AvatarService`. + + save (bool, optional): + Whether to save the settings after setting the list of + enabled services. The default is to immediately save. + + Raises: + djblets.avatars.errors.AvatarServiceNotFoundError: + This exception is raised when an unknown avatar service is + enabled. + """ + new_service_ids = set() for service in services: - if service not in self: + if service in self: + new_service_ids.add(service.avatar_service_id) + else: raise self.lookup_error_class(self.format_error( UNKNOWN_SERVICE_ENABLED, service_id=service.avatar_service_id)) - new_service_ids = { - service.avatar_service_id - for service in services - } + siteconfig = SiteConfiguration.objects.get_current() + cur_service_ids = \ + set(siteconfig.get(self.ENABLED_SERVICES_KEY)) - to_enable = new_service_ids - self._enabled_services - to_disable = self._enabled_services - new_service_ids + if new_service_ids == cur_service_ids: + return - for service_id in to_disable: - try: - service = self.get('avatar_service_id', service_id) - except self.lookup_error_class: - service = None + to_enable = new_service_ids - cur_service_ids + to_disable = cur_service_ids - new_service_ids - if service is not None: - self.disable_service(service, save=False) + for service_id in to_disable: + self.disable_service_by_id(service_id, save=False) for service_id in to_enable: - self.enable_service(self.get('avatar_service_id', service_id), - save=False) + self.enable_service_by_id(service_id, save=False) default_service = self.default_service if (default_service is not None and not self.is_enabled(type(default_service))): - self.set_default_service(None) + self.set_default_service(None, save=False) - self.save() + if save: + self.save() @property def default_service(self): - """Return the default avatar service. + """The default avatar service. Returns: djblets.avatars.services.AvatarService: The default avatar service, or ``None`` if there isn't one. """ - self.populate() + siteconfig = SiteConfiguration.objects.get_current() + default_service_id = siteconfig.get(self.DEFAULT_SERVICE_KEY) - if self._default_service_id is None: + if default_service_id is None: return None - return self.get_avatar_service(self._default_service_id) + enabled_service_ids = siteconfig.get(self.ENABLED_SERVICES_KEY) + + if default_service_id not in enabled_service_ids: + # The settings listed a default service that was no longer in + # the list of enabled services. Warn about this, fix it, and + # save the value in settings. + logger.error(self.format_error(DISABLED_SERVICE_DEFAULT, + service_id=default_service_id)) + + # Set this manually in the SiteConfiguration instead of using + # disable_service_by_id, to avoid infinitely recursing into this + # property and that method, and also because it's just not + # necessary to go through everything that function does. + siteconfig.set(self.DEFAULT_SERVICE_KEY, None) + + return None + + return self.get_avatar_service(default_service_id) def set_default_service(self, service, save=True): """Set the default avatar service. @@ -279,10 +337,8 @@ class AvatarServiceRegistry(Registry): djblets.avatars.errors.DisabledServiceError: Raised if the service is not enabled. """ - self.populate() - if service is None: - self._default_service_id = None + default_service_id = None elif service not in self: raise self.lookup_error_class(self.format_error( UNKNOWN_SERVICE_DEFAULT, service_id=service.avatar_service_id)) @@ -291,10 +347,15 @@ class AvatarServiceRegistry(Registry): DISABLED_SERVICE_DEFAULT, service_id=service.avatar_service_id)) else: - self._default_service_id = service.avatar_service_id + default_service_id = service.avatar_service_id - if save: - self.save() + siteconfig = SiteConfiguration.objects.get_current() + + if default_service_id != siteconfig.get(self.DEFAULT_SERVICE_KEY): + siteconfig.set(self.DEFAULT_SERVICE_KEY, default_service_id) + + if save: + self.save() def has_service(self, service_id): """Return whether or not the avatar service ID is registered. @@ -362,13 +423,24 @@ class AvatarServiceRegistry(Registry): """ if (self.default_service is not None and self.default_service.avatar_service_id == service_id): - self.set_default_service(None) + self.set_default_service(None, save=save) - self._enabled_services.discard(service_id) self._instance_cache.pop(service_id, None) - if save: - self.save() + siteconfig = SiteConfiguration.objects.get_current() + enabled_service_ids = list(siteconfig.get(self.ENABLED_SERVICES_KEY)) + + try: + enabled_service_ids.remove(service_id) + except ValueError: + # This wasn't enabled in the stored site configuration, so don't + # try saving now. We're done. + pass + finally: + siteconfig.set(self.ENABLED_SERVICES_KEY, enabled_service_ids) + + if save: + self.save() def enable_service(self, service, save=True): """Enable an avatar service. @@ -390,10 +462,33 @@ class AvatarServiceRegistry(Registry): raise self.lookup_error_class(self.format_error( UNKNOWN_SERVICE_ENABLED, service_id=service.avatar_service_id)) - self._enabled_services.add(service.avatar_service_id) + self.enable_service_by_id(service.avatar_service_id, save=save) - if save: - self.save() + def enable_service_by_id(self, service_id, save=True): + """Enable an avatar service. + + Args: + service_id (unicode): + The ID of the service to enable. Callers must take care to + ensure this matches a service stored in the registry. + + save (bool, optional): + Whether or not the avatar service registry will be saved to the + database after enabling the service. This defaults to ``True``. + + Raises: + djblets.avatars.errors.AvatarServiceNotFoundError: + This is raised if the service is not registered. + """ + siteconfig = SiteConfiguration.objects.get_current() + enabled_service_ids = siteconfig.get(self.ENABLED_SERVICES_KEY) + + if service_id not in enabled_service_ids: + siteconfig.set(self.ENABLED_SERVICES_KEY, + enabled_service_ids + [service_id]) + + if save: + self.save() def is_enabled(self, service): """Return whether or not the given avatar service is enabled. @@ -404,10 +499,16 @@ class AvatarServiceRegistry(Registry): :py:class:`~djblets.avatars.service.AvatarService`. Returns: - bool: Whether or not the service ID is registered. + bool: + Whether or not the service ID is registered. """ - return (service in self and - service.avatar_service_id in self._enabled_services) + if service not in self: + return + + siteconfig = SiteConfiguration.objects.get_current() + enabled_service_ids = siteconfig.get(self.ENABLED_SERVICES_KEY) + + return service.avatar_service_id in enabled_service_ids def unregister(self, service): """Unregister an avatar service. @@ -443,42 +544,13 @@ class AvatarServiceRegistry(Registry): if self.populated: return - super(AvatarServiceRegistry, self).populate() - siteconfig = SiteConfiguration.objects.get_current() - needs_save = False - - enabled_service_ids = set( - siteconfig.get(self.ENABLED_SERVICES_KEY) or []) + siteconfig.add_defaults({ + self.ENABLED_SERVICES_KEY: [], + self.DEFAULT_SERVICE_KEY: None, + }) - for avatar_service_id in enabled_service_ids: - if not self.has_service(avatar_service_id): - logging.error(self.format_error( - UNKNOWN_SERVICE_ENABLED, service_id=avatar_service_id)) - - default_service_id = siteconfig.get(self.DEFAULT_SERVICE_KEY) - - if default_service_id is not None: - if not self.has_service(default_service_id): - # Warn about the service not being in the registry, but leave - # the default ID alone, in case it's later registered (by an - # extension or other code). - logging.error(self.format_error(UNKNOWN_SERVICE_DEFAULT, - service_id=default_service_id)) - elif default_service_id not in enabled_service_ids: - # The settings listed a default service that was no longer in - # the list of enabled services. Warn about this, fix it, and - # save the value in settings. - logging.error(self.format_error(DISABLED_SERVICE_DEFAULT, - service_id=default_service_id)) - default_service_id = None - needs_save = True - - self._default_service_id = default_service_id - self._enabled_services = enabled_service_ids - - if needs_save: - self.save() + super(AvatarServiceRegistry, self).populate() def get_defaults(self): """Yield the default avatar services. @@ -491,26 +563,21 @@ class AvatarServiceRegistry(Registry): yield service_class def save(self): - """Save the list of enabled avatar services to the database.""" + """Save the avatar configuration to the database. + + As the avatar configuration is stored in the + :py:class:`~djblets.siteconfig.models.SiteConfiguration`, this method + will save any pending configuration, synchronizing it to all other + processes/servers. + + If there are pending avatar configuration changes (due to passing + ``save=False`` to some methods), and there's a separate call to + :py:meth:`SiteConfiguration.save() + ` without calling + this method, the new avatar configuration will still be saved. + """ siteconfig = SiteConfiguration.objects.get_current() - dirty = False - - enabled_services = self._enabled_services - default_service = self._default_service_id - - stored_enabled_services = \ - siteconfig.get(self.ENABLED_SERVICES_KEY) or [] - - if enabled_services != set(stored_enabled_services): - siteconfig.set(self.ENABLED_SERVICES_KEY, list(enabled_services)) - dirty = True - - if default_service != siteconfig.get(self.DEFAULT_SERVICE_KEY): - siteconfig.set(self.DEFAULT_SERVICE_KEY, default_service) - dirty = True - - if dirty: - siteconfig.save() + siteconfig.save() def for_user(self, user, service_id=None): """Return the requested avatar service for the given user. diff --git a/djblets/avatars/tests.py b/djblets/avatars/tests.py index ae3d2cf2cc349d1319bedc9fe136c798b9d5623f..47f5a0a3627af67ce7e991de7c09b1128334f29d 100644 --- a/djblets/avatars/tests.py +++ b/djblets/avatars/tests.py @@ -2,7 +2,6 @@ from __future__ import unicode_literals -import logging import uuid from django.contrib.auth.models import User @@ -302,25 +301,89 @@ class AvatarServiceRegistryTests(SpyAgency, TestCase): def setUp(self): super(AvatarServiceRegistryTests, self).setUp() - site = Site.objects.get_current() - self.siteconfig = SiteConfiguration.objects.create(site=site) - def tearDown(self): - super(AvatarServiceRegistryTests, self).tearDown() - self.siteconfig.delete() + self.siteconfig = SiteConfiguration.objects.create( + site=Site.objects.get_current()) def test_enable_service(self): """Testing AvatarServiceRegistry.enable_service""" registry = AvatarServiceRegistry() registry.register(DummyAvatarService) self.assertFalse(registry.is_enabled(DummyAvatarService)) - self.assertSetEqual(set(registry.enabled_services), set()) + self.assertEqual(set(registry.enabled_services), set()) registry.enable_service(DummyAvatarService) self.assertTrue(registry.is_enabled(DummyAvatarService)) - self.assertSetEqual( - registry.enabled_services, - {DummyAvatarService}) + self.assertEqual(registry.enabled_services, {DummyAvatarService}) + + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + [DummyAvatarService.avatar_service_id]) + + def test_enable_service_with_save_false(self): + """Testing AvatarServiceRegistry.enable_service with save=False""" + registry = AvatarServiceRegistry() + registry.register(DummyAvatarService) + self.assertFalse(registry.is_enabled(DummyAvatarService)) + self.assertEqual(set(registry.enabled_services), set()) + + registry.enable_service(DummyAvatarService, save=False) + self.assertTrue(registry.is_enabled(DummyAvatarService)) + self.assertEqual(registry.enabled_services, {DummyAvatarService}) + + # Make sure we've saved this in our cached siteconfig... + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + [DummyAvatarService.avatar_service_id]) + + # ... but not in the database. + siteconfig = SiteConfiguration.objects.get(pk=siteconfig.pk) + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + []) + + def test_enable_service_by_id(self): + """Testing AvatarServiceRegistry.enable_service_by_id""" + registry = AvatarServiceRegistry() + registry.register(DummyAvatarService) + self.assertFalse(registry.is_enabled(DummyAvatarService)) + self.assertEqual(set(registry.enabled_services), set()) + + registry.enable_service_by_id(DummyAvatarService.avatar_service_id) + self.assertTrue(registry.is_enabled(DummyAvatarService)) + self.assertEqual(registry.enabled_services, {DummyAvatarService}) + + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + [DummyAvatarService.avatar_service_id]) + + def test_enable_service_by_id_with_save_false(self): + """Testing AvatarServiceRegistry.enable_service_by_id with save=False + """ + registry = AvatarServiceRegistry() + registry.register(DummyAvatarService) + self.assertFalse(registry.is_enabled(DummyAvatarService)) + self.assertEqual(set(registry.enabled_services), set()) + + registry.enable_service_by_id(DummyAvatarService.avatar_service_id, + save=False) + self.assertTrue(registry.is_enabled(DummyAvatarService)) + self.assertEqual(registry.enabled_services, {DummyAvatarService}) + + # Make sure we've saved this in our cached siteconfig... + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + [DummyAvatarService.avatar_service_id]) + + # ... but not in the database. + siteconfig = SiteConfiguration.objects.get(pk=siteconfig.pk) + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + []) def test_disable_service(self): """Testing AvatarServiceRegistry.disable_service""" @@ -329,26 +392,130 @@ class AvatarServiceRegistryTests(SpyAgency, TestCase): registry.enable_service(GravatarService) self.assertTrue(registry.is_enabled(GravatarService)) - self.assertSetEqual(set(registry.enabled_services), - {GravatarService}) + self.assertEqual(set(registry.enabled_services), {GravatarService}) registry.disable_service(GravatarService) self.assertFalse(registry.is_enabled(GravatarService)) - self.assertSetEqual(set(registry.enabled_services), set()) + self.assertEqual(set(registry.enabled_services), set()) + + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + []) + + def test_disable_service_with_save_false(self): + """Testing AvatarServiceRegistry.disable_service with save=False""" + registry = AvatarServiceRegistry() + self.assertFalse(registry.is_enabled(GravatarService)) + registry.enable_service(GravatarService) + + self.assertTrue(registry.is_enabled(GravatarService)) + self.assertEqual(set(registry.enabled_services), {GravatarService}) + + registry.disable_service(GravatarService, save=False) + self.assertFalse(registry.is_enabled(GravatarService)) + self.assertEqual(set(registry.enabled_services), set()) + + # Make sure we've saved this in our cached siteconfig... + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + []) + + # ... but not in the database. + siteconfig = SiteConfiguration.objects.get(pk=siteconfig.pk) + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + [GravatarService.avatar_service_id]) + + def test_disable_service_by_id(self): + """Testing AvatarServiceRegistry.disable_service_by_id""" + registry = AvatarServiceRegistry() + self.assertFalse(registry.is_enabled(GravatarService)) + registry.enable_service(GravatarService) + + self.assertTrue(registry.is_enabled(GravatarService)) + self.assertEqual(set(registry.enabled_services), {GravatarService}) + + registry.disable_service_by_id(GravatarService.avatar_service_id) + self.assertFalse(registry.is_enabled(GravatarService)) + self.assertEqual(set(registry.enabled_services), set()) + + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + []) + + def test_disable_service_by_id_with_save_false(self): + """Testing AvatarServiceRegistry.disable_service_by_id with save=False + """ + registry = AvatarServiceRegistry() + self.assertFalse(registry.is_enabled(GravatarService)) + registry.enable_service(GravatarService) + + self.assertTrue(registry.is_enabled(GravatarService)) + self.assertEqual(set(registry.enabled_services), {GravatarService}) + + registry.disable_service_by_id(GravatarService.avatar_service_id, + save=False) + self.assertFalse(registry.is_enabled(GravatarService)) + self.assertEqual(set(registry.enabled_services), set()) + + # Make sure we've saved this in our cached siteconfig... + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + []) + + # ... but not in the database. + siteconfig = SiteConfiguration.objects.get(pk=siteconfig.pk) + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + [GravatarService.avatar_service_id]) def test_set_enabled_services(self): - """Testing AvatarServiceRegistry.enabled_services setter""" + """Testing AvatarServiceRegistry.set_enabled_services""" registry = AvatarServiceRegistry() registry.register(DummyAvatarService) - registry.enabled_services = [DummyAvatarService, GravatarService] + registry.set_enabled_services([DummyAvatarService, GravatarService]) + + self.assertEqual(registry.enabled_services, + {DummyAvatarService, GravatarService}) + + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + [DummyAvatarService.avatar_service_id, + GravatarService.avatar_service_id]) + + def test_set_enabled_services_with_save_false(self): + """Testing AvatarServiceRegistry.set_enabled_services with save=False + """ + registry = AvatarServiceRegistry() + registry.register(DummyAvatarService) + registry.set_enabled_services([DummyAvatarService, GravatarService], + save=False) self.assertEqual(registry.enabled_services, {DummyAvatarService, GravatarService}) + # Make sure we've saved this in our cached siteconfig... + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + set(siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY)), + {DummyAvatarService.avatar_service_id, + GravatarService.avatar_service_id}) + + # ... but not in the database. + siteconfig = SiteConfiguration.objects.get(pk=siteconfig.pk) + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + []) + def test_get_enabled_services_populated(self): - """Testing AvatarServiceRegistry.enabled_services getter calls + """Testing AvatarServiceRegistry.enabled_services property calls populate() """ class TestRegistry(AvatarServiceRegistry): @@ -364,6 +531,10 @@ class AvatarServiceRegistryTests(SpyAgency, TestCase): def get_defaults(self): yield DummyAvatarService + self.siteconfig.set(AvatarServiceRegistry.ENABLED_SERVICES_KEY, + [DummyAvatarService.avatar_service_id]) + self.siteconfig.save() + registry = TestRegistry() self.assertFalse(registry.populated) @@ -372,35 +543,57 @@ class AvatarServiceRegistryTests(SpyAgency, TestCase): self.assertSetEqual(enabled_services, {DummyAvatarService}) def test_set_enabled_services_invalid_service(self): - """Testing AvatarServiceRegistry.enabled_services setter with an + """Testing AvatarServiceRegistry.set_enabled_services with an unregistered service """ registry = AvatarServiceRegistry() with self.assertRaises(ItemLookupError): - registry.enabled_services = [DummyAvatarService, GravatarService] + registry.set_enabled_services([DummyAvatarService, + GravatarService]) self.assertEqual(registry.enabled_services, set()) + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + []) + def test_default_service(self): """Testing AvatarServiceRegistry.default_service""" registry = AvatarServiceRegistry() registry.register(DummyAvatarService) - registry.enabled_services = [DummyAvatarService, GravatarService] - + registry.set_enabled_services([DummyAvatarService, GravatarService]) self.assertIsNone(registry.default_service) + # Set the default backend to the dummy backend. registry.set_default_service(DummyAvatarService) self.assertIsInstance(registry.default_service, DummyAvatarService) + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.DEFAULT_SERVICE_KEY), + DummyAvatarService.avatar_service_id) + + # Set the default backend to the Gravatar backend. registry.set_default_service(GravatarService) self.assertIsInstance(registry.default_service, GravatarService) + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.DEFAULT_SERVICE_KEY), + GravatarService.avatar_service_id) + + # Remove the default backend. registry.set_default_service(None) self.assertIsNone(registry.default_service) - def test_default_service_after_service_reregisterd(self): + siteconfig = SiteConfiguration.objects.get_current() + self.assertIsNone( + siteconfig.get(AvatarServiceRegistry.DEFAULT_SERVICE_KEY)) + + def test_default_service_after_service_reregistered(self): """Testing AvatarServiceRegistry.default_service after service registered after previously unregistered """ @@ -413,48 +606,45 @@ class AvatarServiceRegistryTests(SpyAgency, TestCase): registry = AvatarServiceRegistry() registry.populate() - self.assertEqual(registry._default_service_id, - DummyAvatarService.avatar_service_id) self.assertIsNone(registry.default_service) + # Make sure it's still saved in siteconfig. We'd only unset it if + # wasn't in the list of stoerd enabled service IDs, since we might be + # running without extensions loaded. + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.DEFAULT_SERVICE_KEY), + DummyAvatarService.avatar_service_id) + registry.register(DummyAvatarService) self.assertIsInstance(registry.default_service, DummyAvatarService) - def test_set_default_service_invalid(self): - """Testing AvatarServiceRegistry.set_default_service setter with an - unregistered service - """ - registry = AvatarServiceRegistry() - - self.assertIsNone(registry.default_service) - - with self.assertRaises(ItemLookupError): - registry.set_default_service(DummyAvatarService) + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.DEFAULT_SERVICE_KEY), + DummyAvatarService.avatar_service_id) - self.assertIsNone(registry.default_service) + def test_set_default_service(self): + """Testing AvatarServiceRegistry.set_default_service""" + self.siteconfig.set(AvatarServiceRegistry.ENABLED_SERVICES_KEY, + [GravatarService.avatar_service_id]) + self.siteconfig.save() - def test_set_default_service_disabled(self): - """Testing AvatarServiceRegistry.set_default_service setter with a - disabled service - """ registry = AvatarServiceRegistry() - gravatar_service = registry.get('avatar_service_id', - GravatarService.avatar_service_id) - self.assertIsNone(registry.default_service) - with self.assertRaises(DisabledServiceError): - registry.set_default_service(gravatar_service) + registry.set_default_service(GravatarService) - self.assertIsNone(registry.default_service) + self.assertIsInstance(registry.default_service, GravatarService) - def test_populate(self): - """Testing AvatarServiceRegistry.populate with site configuration - settings - """ - self.spy_on(logging.error) + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.DEFAULT_SERVICE_KEY), + GravatarService.avatar_service_id) + def test_set_default_service_with_save_false(self): + """Testing AvatarServiceRegistry.set_default_service with save=False""" self.siteconfig.set(AvatarServiceRegistry.ENABLED_SERVICES_KEY, [GravatarService.avatar_service_id]) self.siteconfig.set(AvatarServiceRegistry.DEFAULT_SERVICE_KEY, @@ -462,119 +652,105 @@ class AvatarServiceRegistryTests(SpyAgency, TestCase): self.siteconfig.save() registry = AvatarServiceRegistry() - registry.populate() - - self.assertTrue(registry.populated) - self.assertIsInstance(registry.default_service, GravatarService) - self.assertEqual(registry.enabled_services, {GravatarService}) - - self.assertFalse(logging.error.spy.called) - - def test_populate_invalid_default(self): - """Testing AvatarServiceRegistry.populate with an invalid default - registry in the site configuration - """ - self.spy_on(logging.error) - - self.siteconfig.set(AvatarServiceRegistry.DEFAULT_SERVICE_KEY, - DummyAvatarService.avatar_service_id) - self.siteconfig.save() - - registry = AvatarServiceRegistry() - registry.populate() + registry.set_default_service(None, save=False) self.assertIsNone(registry.default_service) - self.assertEqual(registry.enabled_services, set()) - self.assertTrue(logging.error.spy.called) - - # Check that the old invalid default is still recorded, in case a - # backend (perhaps from an extension) is just temporarily disabled. + # Make sure we've saved this in our cached siteconfig... siteconfig = SiteConfiguration.objects.get_current() + self.assertIsNone( + siteconfig.get(AvatarServiceRegistry.DEFAULT_SERVICE_KEY)) + + # ... but not in the database. + siteconfig = SiteConfiguration.objects.get(pk=siteconfig.pk) self.assertEqual( siteconfig.get(AvatarServiceRegistry.DEFAULT_SERVICE_KEY), - DummyAvatarService.avatar_service_id) + GravatarService.avatar_service_id) - def test_populate_disabled_default(self): - """Testing AvatarServiceRegistry.populate with a disabled default - registry in the site configuration + def test_set_default_service_invalid(self): + """Testing AvatarServiceRegistry.set_default_service with an + unregistered service """ - self.spy_on(logging.error) - + self.siteconfig.set(AvatarServiceRegistry.ENABLED_SERVICES_KEY, + [GravatarService.avatar_service_id]) self.siteconfig.set(AvatarServiceRegistry.DEFAULT_SERVICE_KEY, GravatarService.avatar_service_id) self.siteconfig.save() registry = AvatarServiceRegistry() - registry.populate() + self.assertIsInstance(registry.default_service, GravatarService) - self.assertIn(GravatarService, registry) - self.assertIsNone(registry.default_service, None) - self.assertEqual(registry.enabled_services, set()) + with self.assertRaises(ItemLookupError): + registry.set_default_service(DummyAvatarService) - self.assertTrue(logging.error.spy.called) + self.assertIsInstance(registry.default_service, GravatarService) siteconfig = SiteConfiguration.objects.get_current() - self.assertIsNone( - siteconfig.get(AvatarServiceRegistry.DEFAULT_SERVICE_KEY)) + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.DEFAULT_SERVICE_KEY), + GravatarService.avatar_service_id) - def test_populate_invalid_enabled_services(self): - """Testing AvatarServiceRegistry.populate with an unregistered + def test_set_default_service_disabled(self): + """Testing AvatarServiceRegistry.set_default_service with a disabled service """ - self.spy_on(logging.error) - self.siteconfig.set(AvatarServiceRegistry.ENABLED_SERVICES_KEY, - [DummyAvatarService.avatar_service_id]) - self.siteconfig.save() - registry = AvatarServiceRegistry() - registry.populate() + gravatar_service = registry.get('avatar_service_id', + GravatarService.avatar_service_id) self.assertIsNone(registry.default_service) - self.assertEqual(registry.enabled_services, set()) - self.assertEqual(registry._enabled_services, - {DummyAvatarService.avatar_service_id}) - self.assertTrue(logging.error.spy.called) + with self.assertRaises(DisabledServiceError): + registry.set_default_service(gravatar_service) + + self.assertIsNone(registry.default_service) - # Check that the old enabled backend is still recorded, in case a - # backend (perhaps from an extension) is just temporarily disabled. siteconfig = SiteConfiguration.objects.get_current() - self.assertEqual( - siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), - [DummyAvatarService.avatar_service_id]) + self.assertIsNone( + siteconfig.get(AvatarServiceRegistry.DEFAULT_SERVICE_KEY)) - def test_populate_custom_services(self): - """Testing AvatarServiceRegistry.populate for subclasses with custom - default registrations + def test_populate(self): + """Testing AvatarServiceRegistry.populate with site configuration + settings """ - class TestRegistry(AvatarServiceRegistry): - settings_manager_class = DummySettingsManager(None, {}) - default_avatar_service_classes = [DummyAvatarService, - GravatarService] - - self.spy_on(logging.error) self.siteconfig.set(AvatarServiceRegistry.ENABLED_SERVICES_KEY, - [DummyAvatarService.avatar_service_id]) + [GravatarService.avatar_service_id]) self.siteconfig.set(AvatarServiceRegistry.DEFAULT_SERVICE_KEY, - DummyAvatarService.avatar_service_id) + GravatarService.avatar_service_id) self.siteconfig.save() - registry = TestRegistry() - self.assertIsInstance(registry.default_service, DummyAvatarService) - self.assertFalse(logging.error.spy.called) + registry = AvatarServiceRegistry() + registry.populate() + + self.assertTrue(registry.populated) + self.assertIsInstance(registry.default_service, GravatarService) + self.assertEqual(registry.enabled_services, {GravatarService}) def test_unregister(self): """Testing AvatarServiceRegistry.unregister""" + self.siteconfig.set(AvatarServiceRegistry.DEFAULT_SERVICE_KEY, + GravatarService.avatar_service_id) + self.siteconfig.set(AvatarServiceRegistry.ENABLED_SERVICES_KEY, + [GravatarService.avatar_service_id]) + self.siteconfig.save() + registry = AvatarServiceRegistry() gravatar_service = registry.get('avatar_service_id', GravatarService.avatar_service_id) - registry.enable_service(GravatarService) + self.assertEqual(registry.enabled_services, {gravatar_service}) - self.assertSetEqual(registry.enabled_services, {gravatar_service}) registry.unregister(gravatar_service) - self.assertSetEqual(registry.enabled_services, set()) + self.assertEqual(registry.enabled_services, set()) + self.assertIsNone(registry.default_service) + + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + [GravatarService.avatar_service_id]) + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.DEFAULT_SERVICE_KEY), + GravatarService.avatar_service_id) def test_unregister_register_keeps_enabled(self): """Testing AvatarServiceRegistry.unregister followed by register keeps @@ -586,12 +762,17 @@ class AvatarServiceRegistryTests(SpyAgency, TestCase): registry.enable_service(GravatarService) - self.assertSetEqual(registry.enabled_services, {gravatar_service}) + self.assertEqual(registry.enabled_services, {gravatar_service}) registry.unregister(gravatar_service) - self.assertSetEqual(registry.enabled_services, set()) + self.assertEqual(registry.enabled_services, set()) registry.register(gravatar_service) - self.assertSetEqual(registry.enabled_services, {gravatar_service}) + self.assertEqual(registry.enabled_services, {gravatar_service}) + + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + [GravatarService.avatar_service_id]) def test_disable_default(self): """Testing AvatarServiceRegistry.disable_service unsets the default @@ -604,20 +785,33 @@ class AvatarServiceRegistryTests(SpyAgency, TestCase): registry.disable_service(GravatarService) self.assertIsNone(registry.default_service) + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + []) + self.assertIsNone( + siteconfig.get(AvatarServiceRegistry.DEFAULT_SERVICE_KEY)) + def test_disable_default_from_setter(self): - """Testing AvatarServiceRegistry.enabled_services setter unsets the - default services + """Testing AvatarServiceRegistry.set_enabled_services unsets the + default services when removed """ registry = AvatarServiceRegistry() registry.enable_service(GravatarService) registry.set_default_service(GravatarService) - registry.enabled_services = [] + registry.set_enabled_services([]) self.assertIsNone(registry.default_service) + siteconfig = SiteConfiguration.objects.get_current() + self.assertEqual( + siteconfig.get(AvatarServiceRegistry.ENABLED_SERVICES_KEY), + []) + self.assertIsNone( + siteconfig.get(AvatarServiceRegistry.DEFAULT_SERVICE_KEY)) + def test_for_user(self): """Testing AvatarServiceRegistry.for_user""" - class DummyAvatarServiceRegistry(AvatarServiceRegistry): settings_manager_class = DummySettingsManager('dummy', {}) default_avatar_service_classes = [ diff --git a/djblets/siteconfig/tests.py b/djblets/siteconfig/tests.py index c0fbc18847615b8ecd8952e0ad8112ca9746c99d..5c69454729a7c3e2c9a554b8ea8a9dbcb06ef06a 100644 --- a/djblets/siteconfig/tests.py +++ b/djblets/siteconfig/tests.py @@ -207,18 +207,18 @@ class SiteConfigurationTests(SiteConfigTestCase): def test_add_defaults(self): """Testing SiteConfiguration.add_defaults""" - defaults = { + self.siteconfig.add_defaults({ 'valid_key_1': 'valid_parameter_1', 'valid_key_2': 'valid_parameter_2', 'valid_key_3': 'valid_parameter_3', - } - - self.siteconfig.add_defaults(defaults) + }) - self.assertEqual(defaults, - self.siteconfig.get_defaults()) self.assertEqual(self.siteconfig.get('valid_key_1'), 'valid_parameter_1') + self.assertEqual(self.siteconfig.get('valid_key_2'), + 'valid_parameter_2') + self.assertEqual(self.siteconfig.get('valid_key_3'), + 'valid_parameter_3') def test_add_default(self): """Testing SiteConfiguration.add_default""" @@ -229,22 +229,20 @@ class SiteConfigurationTests(SiteConfigTestCase): def test_get_defaults(self): """Testing SiteConfiguration.get_defaults""" - defaults = { + self.siteconfig.add_defaults({ 'valid_key_1': 'valid_parameter_1', 'valid_key_2': 'valid_parameter_2', 'valid_key_3': 'valid_parameter_3', - } - - self.siteconfig.add_defaults(defaults) + }) self.siteconfig.add_default('valid_key_1', 'valid_new_parameter_1') - self.assertEqual( - self.siteconfig.get_defaults(), - { - 'valid_key_1': 'valid_new_parameter_1', - 'valid_key_2': 'valid_parameter_2', - 'valid_key_3': 'valid_parameter_3', - }) + siteconfig_defaults = self.siteconfig.get_defaults() + self.assertEqual(siteconfig_defaults['valid_key_1'], + 'valid_new_parameter_1') + self.assertEqual(siteconfig_defaults['valid_key_2'], + 'valid_parameter_2') + self.assertEqual(siteconfig_defaults['valid_key_3'], + 'valid_parameter_3') class SiteConfigurationManagerTests(SiteConfigTestCase):