diff --git a/reviewboard/accounts/backends.py b/reviewboard/accounts/backends.py index 2aff9d64854d3d4a8a16ecc0dde0d32f8889f408..6b65f542b2e232a6050d06852111ea2ace086a35 100644 --- a/reviewboard/accounts/backends.py +++ b/reviewboard/accounts/backends.py @@ -174,6 +174,7 @@ class StandardAuthBackend(AuthBackend, ModelBackend): _VALID_LOCAL_SITE_PERMISSIONS = [ 'hostingsvcs.change_hostingserviceaccount', 'hostingsvcs.create_hostingserviceaccount', + 'integrations.change_configuredintegration', 'reviews.add_group', 'reviews.can_change_status', 'reviews.can_edit_reviewrequest', diff --git a/reviewboard/extensions/hooks.py b/reviewboard/extensions/hooks.py index 1a7f87c48ff6f4f6b3f9420c19a2c9a5acaf0260..cc8e6cb627fc7800653926be41878ba4addf27a4 100644 --- a/reviewboard/extensions/hooks.py +++ b/reviewboard/extensions/hooks.py @@ -1,5 +1,8 @@ from __future__ import unicode_literals +import uuid +import logging + from django.utils import six from djblets.extensions.hooks import (DataGridColumnsHook, ExtensionHook, ExtensionHookPoint, SignalHook, @@ -18,6 +21,9 @@ from reviewboard.datagrids.grids import (DashboardDataGrid, UserPageReviewRequestDataGrid) from reviewboard.hostingsvcs.service import (register_hosting_service, unregister_hosting_service) +from reviewboard.integrations.integration import (register_integration, + unregister_integration) +from reviewboard.integrations.manager import get_integration_manager from reviewboard.reviews.fields import (get_review_request_fieldset, register_review_request_fieldset, unregister_review_request_fieldset) @@ -544,6 +550,72 @@ class UserPageSidebarItemsHook(DataGridSidebarItemsHook): extension, UserPageReviewRequestDataGrid, item_classes) +@six.add_metaclass(ExtensionHookPoint) +class IntegrationHook(ExtensionHook): + """Allows extensions to register and unregister service integrations.""" + + def __init__(self, extension, integration): + super(IntegrationHook, self).__init__(extension) + self.integration = integration + self.integration.static_path = extension.id + self.manager = get_integration_manager() + + register_integration(self.integration) + + for config in self.manager.get_config_instances(self.integration. + integration_id): + self.manager.reload_config(config) + + def shutdown(self): + super(IntegrationHook, self).shutdown() + unregister_integration(self.integration) + + for config in self.manager.get_config_instances(self.integration. + integration_id): + self.manager.shutdown_config(config.pk) + + +@six.add_metaclass(ExtensionHookPoint) +class IntegrationSignalHook(ExtensionHook): + """Connects Integration to a Django signal.""" + + def __init__(self, integration, signal, callback, sender=None, + sandbox_errors=True): + self.integration = integration + self.integration.hooks.add(self) + self.__class__.add_hook(self) + self.initialized = True + + self.signal = signal + self.callback = callback + self.dispatch_uid = uuid.uuid1() + self.sender = sender + self.sandbox_errors = sandbox_errors + + signal.connect(self._wrap_callback, sender=self.sender, weak=False, + dispatch_uid=self.dispatch_uid) + + def shutdown(self): + super(IntegrationSignalHook, self).shutdown() + self.signal.disconnect(dispatch_uid=self.dispatch_uid, + sender=self.sender) + + def _wrap_callback(self, **kwargs): + """Wraps a callback function, passing extra parameters and sandboxing. + + This will call the callback with an integration= keyword argument, + and sandbox any errors (if sandbox_errors is True). + """ + try: + self.callback(integration=self.integration, **kwargs) + except Exception as e: + logging.error('Error when calling %r from IntegrationSignalHook:' + ' %s', self.callback, e, exc_info=1) + + if not self.sandbox_errors: + raise + + __all__ = [ 'AccountPageFormsHook', 'AccountPagesHook', @@ -561,6 +633,8 @@ __all__ = [ 'HeaderActionHook', 'HeaderDropdownActionHook', 'HostingServiceHook', + 'IntegrationHook', + 'IntegrationSignalHook', 'NavigationBarHook', 'ReviewRequestActionHook', 'ReviewRequestApprovalHook', diff --git a/reviewboard/extensions/tests.py b/reviewboard/extensions/tests.py index 68f2e59a21da04bb08dad96b8bb7ce9ac3961e24..fce6a5f44840cc8efd7dc959d903b0baf2d6aff2 100644 --- a/reviewboard/extensions/tests.py +++ b/reviewboard/extensions/tests.py @@ -16,6 +16,7 @@ from reviewboard.extensions.hooks import (AdminWidgetHook, HeaderActionHook, HeaderDropdownActionHook, HostingServiceHook, + IntegrationHook, NavigationBarHook, ReviewRequestActionHook, ReviewRequestApprovalHook, @@ -24,6 +25,9 @@ from reviewboard.extensions.hooks import (AdminWidgetHook, WebAPICapabilitiesHook) from reviewboard.hostingsvcs.service import (get_hosting_service, HostingService) +from reviewboard.integrations.integration import (Integration, + get_integration, + get_integrations) from reviewboard.testing.testcase import TestCase from reviewboard.reviews.models.review_request import ReviewRequest from reviewboard.reviews.fields import (BaseReviewRequestField, @@ -222,13 +226,13 @@ class HostingServiceHookTests(TestCase): self.extension.shutdown() def test_register(self): - """Testing HostingServiceHook initializing""" + """Testing HostingServiceHook registration""" HostingServiceHook(extension=self.extension, service_cls=TestService) self.assertNotEqual(None, get_hosting_service(TestService.name)) def test_unregister(self): - """Testing HostingServiceHook uninitializing""" + """Testing HostingServiceHook unregistration""" hook = HostingServiceHook(extension=self.extension, service_cls=TestService) @@ -420,6 +424,41 @@ class SandboxExtension(Extension): super(SandboxExtension, self).__init__(*args, **kwargs) +class TestIntegration(Integration): + integration_id = 'TestIntegration' + + +class IntegrationHookTest(TestCase): + """Testing integration hook.""" + + def setUp(self): + super(IntegrationHookTest, self).setUp() + + manager = ExtensionManager('') + self.extension = DummyExtension(extension_manager=manager) + self.extension.id = "DummyExtension" + + def tearDown(self): + super(IntegrationHookTest, self).tearDown() + + self.extension.shutdown() + + def test_register(self): + """Testing IntegrationHook initializing""" + IntegrationHook(self.extension, TestIntegration) + + self.assertIn(TestIntegration, get_integrations()) + self.assertEqual(TestIntegration, + get_integration(TestIntegration.integration_id)) + + def test_unregister(self): + """Testing IntegrationHook unitializing""" + hook = IntegrationHook(self.extension, TestIntegration) + + hook.shutdown() + self.assertNotIn(TestIntegration, get_integrations()) + + class ReviewRequestApprovalTestHook(ReviewRequestApprovalHook): def is_approved(self, review_request, prev_approved, prev_failure): raise Exception diff --git a/reviewboard/integrations/__init__.py b/reviewboard/integrations/__init__.py new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/reviewboard/integrations/integration.py b/reviewboard/integrations/integration.py new file mode 100644 index 0000000000000000000000000000000000000000..c2c1ca7d83f2d638e5c6c7a02cb2cd70c8c2991c --- /dev/null +++ b/reviewboard/integrations/integration.py @@ -0,0 +1,129 @@ +from __future__ import unicode_literals + +import logging + + +class Integration(object): + """Base class for an intergration. + + Integration provided by an extension must be subclass of this + class. This provides the setting for setting up, authenticating + and shutting down of the integration. + """ + + #: The unique identifier of the integration. + #: + #: This identifier will be use in retrieving and registering of the + #: integration, and thus will have to be unique among the integrations. + integration_id = None + + #: The display name of the integration. + name = None + + #: A short description on the functionalities of the integration. + description = None + + #: Flag is set to True if the integration allow LocalSite configuration. + #: + #: If this is set to False, the services provided by the integration will + #: be global, and no LocalSite configuration will be allowed. + allows_local_sites = False + + #: Flag is set to True if the integration support repositories. + supports_repositories = False + + #: Flag is set to True if the integration needs authentication. + needs_authentication = False + + #: The path for the icon file of the integration. + #: + #: The icon file should be placed in the "static" folder within the + #: extension package. The given path will then be the relative path of the + #: icon within the "static" folder. + icon_path = None + + #: The form class of the integration. + #: + #: The given form class should be a subclass of the + #: IntegrationSettingsForm. + config_form = None + + #: The config template for the integration. + #: + #: The given template should extends IntegrationConfigTemplate. + config_template = None + + def __init__(self, config): + if not self.integration_id: + self.integration_id = '%s.%s' % ( + [self.__module__, self.__class__.__name__]) + + self.config = config + self.local_site = config.local_site + self.hooks = set() + + def initialize(self): + """Initialize the integration. + + This provides custom initialization for the subclass. All the processes + that are required in initializing the service of the integration + should be in this method. This will allow the integration to be toggled + without the need to recreate a new instance of the object. + """ + pass + + def shutdown(self): + """Shut down the integration. + + Subclasses should override this to shut down the integration and + deregister all the services provided by the integration. All the + required processes should be in this method. This will allow the + integration to be toggled without the need to recreate a new + instance of the object. + """ + self.shutdown_hooks() + + def shutdown_hooks(self): + """Shuts down all hooks for the integration.""" + for hook in self.hooks: + if hook.initialized: + hook.shutdown() + + self.hooks = set() + + def get_authentication_url(self): + """Returns the authentication URL for the integration.""" + raise NotImplementedError + + +_integrations = {} + + +def register_integration(integration): + """Register a given integration.""" + if integration.integration_id in _integrations: + raise KeyError('"%s" is already a registered integration' + % integration.integration_id) + + _integrations[integration.integration_id] = integration + + +def unregister_integration(integration): + """Unregister a given integration.""" + try: + del _integrations[integration.integration_id] + except KeyError: + logging.error('Failed to unregister unknown integration' + ' "%s"' % integration.name) + raise KeyError('"%s" is not a registered integration' + % integration.integration_id) + + +def get_integrations(): + """Returns the list of integrations.""" + return list(_integrations.values()) + + +def get_integration(integration_id): + """Returns the integration with the given integration ID.""" + return _integrations.get(integration_id) diff --git a/reviewboard/integrations/manager.py b/reviewboard/integrations/manager.py new file mode 100644 index 0000000000000000000000000000000000000000..372c1e77bbe0f941d96916468039ff8868825938 --- /dev/null +++ b/reviewboard/integrations/manager.py @@ -0,0 +1,131 @@ +from __future__ import unicode_literals + +import logging + +from six import itervalues + +from reviewboard.integrations.models import ConfiguredIntegration + + +class IntegrationManager(object): + """A manager for all integrations.""" + + def __init__(self): + self._config_instances = {} + self._load_configs() + + def initialize_config(self, config_id): + """Register the configured integration. + + Look for the config instance and do a proper initialization on its + integration instance. + """ + config = self.get_config_instance(config_id) + + if config.integration and config.is_enabled: + config.integration.shutdown() + config.integration.local_site = config.local_site + config.integration.initialize() + + def shutdown_config(self, config_id): + """Unregister the configured integration. + + Look for the config instance and do a proper shutdown on its + integration instance. + """ + config_instance = self.get_config_instance(config_id) + + if config_instance.integration: + config_instance.integration.shutdown() + + def reload_config(self, config): + """Update the configured integration.""" + self._config_instances[config.pk] = config + + if config.is_enabled: + self.initialize_config(config.pk) + else: + self.shutdown_config(config.pk) + + def delete_config(self, config_id): + """Delete the configurated integration. + + Shutdown the config before deleting the configured integration. + """ + self.shutdown_config(config_id) + ConfiguredIntegration.objects.filter(pk=config_id).delete() + + try: + del self._config_instances[config_id] + except: + logging.error("Config instance %s was already deleted" % config_id) + + def disable_config(self, config_id): + """Disable a configured integration.""" + self._toggle_config(config_id, False) + + def enable_config(self, config_id): + """Enable a configured integration.""" + self._toggle_config(config_id, True) + + def create_config(self, config): + """Create a new configured integration.""" + config.save() + self.reload_config(config) + + def get_config_instances(self, integration_id=None): + """Returns all configured integration instances. + + If an optional ``integration_id`` parameter is given, only instances + belonging to paramter will be returned. + """ + configs = itervalues(self._config_instances) + + if integration_id: + configs = filter(lambda config: config.integration_id == + integration_id, configs) + + return list(configs) + + def get_config_instance(self, config_id): + """Returns the specfic configured integration instance.""" + try: + return self._config_instances[config_id] + except (KeyError, AttributeError): + raise KeyError('This configuration is not registered.') + + def _load_configs(self): + """Load and cache all configured integrations.""" + configs = ConfiguredIntegration.objects.all() + + for config in configs: + self._config_instances[config.pk] = config + + def _toggle_config(self, config_id, is_enabled): + """Toggle a configured integration. + + Update the configuration of configured integration object and + the state of its integration instance. + """ + config = self.get_config_instance(config_id) + config.is_enabled = is_enabled + config.save(update_fields=['is_enabled']) + + self.reload_config(config) + + +_integration_manager = None + + +def get_integration_manager(): + """Returns the integration manager. + + Instead of creating an integration manager directly, this method should be + called to ensure that there is only one instance of the manager. + """ + global _integration_manager + + if not _integration_manager: + _integration_manager = IntegrationManager() + + return _integration_manager diff --git a/reviewboard/integrations/models.py b/reviewboard/integrations/models.py new file mode 100644 index 0000000000000000000000000000000000000000..2787ace65f82fc1d045367137669ff1502e532bf --- /dev/null +++ b/reviewboard/integrations/models.py @@ -0,0 +1,53 @@ +from __future__ import unicode_literals + +from django.db import models +from django.utils.functional import cached_property +from django.utils.translation import ugettext_lazy as _ +from djblets.db.fields import JSONField + +from reviewboard.integrations.integration import get_integration +from reviewboard.site.models import LocalSite + + +class ConfiguredIntegration(models.Model): + """Configuration of an integration.""" + + integration_id = models.CharField(max_length=255) + description = models.CharField(max_length=255, blank=True) + is_enabled = models.BooleanField(default=False) + configuration = JSONField(blank=True) + local_site = models.ForeignKey(LocalSite, + related_name='integration_configurations', + verbose_name=_('Local site'), + blank=True, + null=True) + + @cached_property + def integration(self): + """The integration instance for this configuration.""" + cls = get_integration(self.integration_id) + + if cls: + return cls(self) + else: + return None + + def is_accessible_by(self, user): + """Returns whether or not the user has access to the integration. + + The integration can be access by the user if it is set to global or if + the user has access to the LocalSite. + """ + if self.local_site and not self.local_site.is_accessible_by(user): + return False + + return True + + def is_mutable_by(self, user): + """Returns whether the user can modify this configuration. + + The integration can be change by an administrator of the global site + with the proper permission or the administrator of the LocalSite. + """ + return user.has_perm('integrations.change_configuredintegration', + self.local_site) diff --git a/reviewboard/integrations/tests.py b/reviewboard/integrations/tests.py new file mode 100644 index 0000000000000000000000000000000000000000..d42312ad38c4acc7bf3498af937c1e878c67298d --- /dev/null +++ b/reviewboard/integrations/tests.py @@ -0,0 +1,108 @@ +from __future__ import unicode_literals + +from reviewboard.extensions.base import Extension +from reviewboard.integrations.integration import (Integration, + register_integration, + unregister_integration) +from reviewboard.integrations.manager import IntegrationManager +from reviewboard.integrations.models import ConfiguredIntegration +from reviewboard.testing.testcase import TestCase + + +class TestExtension(Extension): + id = 'TestExtension' + + +class TestIntegration(Integration): + integration_id = 'TestIntegration' + name = 'Test Integration' + description = 'Add test integration for your review.' + static_path = "test" + extension = TestExtension + + def initialize(self): + self.init = True + + def shutdown(self): + self.init = False + + +class IntegrationManagerTest(TestCase): + """Testing Integration Manager.""" + + def setUp(self): + register_integration(TestIntegration) + self.manager = IntegrationManager() + + def tearDown(self): + super(IntegrationManagerTest, self).tearDown() + unregister_integration(TestIntegration) + + def test_start_with_existing_configs(self): + """Testing integration manager initializing with existing configured + integrations. + """ + config1 = self.create_configured_integration( + integration_id='TestIntegration', + ) + config2 = self.create_configured_integration( + integration_id='TestIntegration', + is_enabled=True + ) + self.manager.reload_config(config1) + self.manager.reload_config(config2) + + configs = self.manager.get_config_instances() + config = ConfiguredIntegration.objects.get(pk=1) + + self.assertEqual(len(configs), 2) + self.assertIn(config, configs) + + def test_start_with_empty_configs(self): + """Testing integration manager initializing with no configured + integrations. + """ + configs = self.manager.get_config_instances() + self.assertEqual(len(configs), 0) + + def test_shutdown_unknown_config(self): + """Testing integration manager unregistering nonexistent configured + integration. + """ + self.assertRaises(KeyError, self.manager.shutdown_config, 3) + + def test_delete_config(self): + """Testing integration manager deleting configured integration.""" + config1 = self.create_configured_integration( + integration_id='TestIntegration', + ) + + self.manager.reload_config(config1) + self.manager.delete_config(1) + + configs = self.manager.get_config_instances() + self.assertEqual(len(configs), 0) + + def test_delete_unknown_config(self): + """Testing integration manager deleting unknown config""" + self.assertRaises(KeyError, self.manager.delete_config, 3) + + def test_toggle_config(self): + """Testing integration manager toggling of configured integration.""" + config1 = self.create_configured_integration( + integration_id='TestIntegration' + ) + self.manager.reload_config(config1) + + config = ConfiguredIntegration.objects.get(pk=1) + self.assertFalse(config.is_enabled) + + self.manager._toggle_config(1, True) + config = ConfiguredIntegration.objects.get(pk=1) + self.assertTrue(config.is_enabled) + self.assertTrue(self.manager.get_config_instance(1).is_enabled) + + self.manager._toggle_config(1, False) + config = ConfiguredIntegration.objects.get(pk=1) + self.assertFalse(config.is_enabled) + self.assertFalse(self.manager.get_config_instance(1).is_enabled) diff --git a/reviewboard/settings.py b/reviewboard/settings.py index 3ed1675741dd4213fe9a276c3cf73cabb333e830..c291523b546135c7966d8e5c434c51e07224b265 100644 --- a/reviewboard/settings.py +++ b/reviewboard/settings.py @@ -167,6 +167,7 @@ RB_BUILTIN_APPS = [ 'reviewboard.diffviewer', 'reviewboard.extensions', 'reviewboard.hostingsvcs', + 'reviewboard.integrations', 'reviewboard.notifications', 'reviewboard.reviews', 'reviewboard.scmtools', diff --git a/reviewboard/testing/testcase.py b/reviewboard/testing/testcase.py index d81c844954b9531d27e535b2d6fd10e41916d65f..08734cf8faf4e00f13c044f9086a844778f32520 100644 --- a/reviewboard/testing/testcase.py +++ b/reviewboard/testing/testcase.py @@ -20,6 +20,7 @@ from reviewboard.accounts.models import ReviewRequestVisit from reviewboard.attachments.models import FileAttachment from reviewboard.diffviewer.differ import DiffCompatVersion from reviewboard.diffviewer.models import DiffSet, DiffSetHistory, FileDiff +from reviewboard.integrations.models import ConfiguredIntegration from reviewboard.reviews.models import (Comment, FileAttachmentComment, Group, Review, ReviewRequest, ReviewRequestDraft, Screenshot, @@ -277,6 +278,23 @@ class TestCase(DjbletsTestCase): status=status, diff=diff) + def create_configured_integration(self, integration_id, is_enabled=False, + configuration={}, + description="Test description", + local_site=None): + """Create a configured integration for testing. + + The configured integration is populated with the default data that can + be overridden by the caller. It may optionally be attached to a + LocalSite. + """ + return ConfiguredIntegration.objects.create( + integration_id=integration_id, + is_enabled=is_enabled, + configuration={}, + description=description, + local_site=local_site) + def create_repository(self, with_local_site=False, name='Test Repo', tool_name='Git', path=None, local_site=None, **kwargs):