From a322a9060aaec543c73165291e91afe04c200a0f Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Feb 09 2017 15:54:10 +0000 Subject: [PATCH 1/45] Add a CachedFunction class This class is a wrapper for a function that will cache its results until a relevant message is emitted on the bus. --- diff --git a/hubs/hinting.py b/hubs/hinting.py deleted file mode 100755 index b1a8c2f..0000000 --- a/hubs/hinting.py +++ /dev/null @@ -1,37 +0,0 @@ -from __future__ import unicode_literals - -import decorator - -import fedmsg.config - -import logging -log = logging.getLogger('hubs.hinting') - - -def hint(topics=None, categories=None, usernames=None, ubiquitous=False): - topics = topics or [] - categories = categories or [] - default_usernames = lambda x: [] - usernames = usernames or default_usernames - ubiquitous = ubiquitous - - @decorator.decorator - def wrapper(fn, *args, **kwargs): - return fn(*args, **kwargs) - - def wrapper_wrapper(fn): - wrapped = wrapper(fn) - wrapped.hints = dict( - topics=topics, - categories=categories, - usernames_function=usernames, - ubiquitous=ubiquitous, - ) - return wrapped - - return wrapper_wrapper - - -def prefixed(topic, prefix='org.fedoraproject'): - config = fedmsg.config.load_config() # This is memoized for us. - return '.'.join([prefix, config['environment'], topic]) diff --git a/hubs/tests/__init__.py b/hubs/tests/__init__.py index 2394c1e..d761381 100644 --- a/hubs/tests/__init__.py +++ b/hubs/tests/__init__.py @@ -35,7 +35,7 @@ class APPTest(unittest.TestCase): self.app = hubs.app.app.test_client() self.app.testing = True self.session = hubs.app.session - from hubs.widgets.base import cache + from hubs.widgets.caching import cache cache.configure(backend='dogpile.cache.null', replace_existing_backend=True) self.populate() diff --git a/hubs/tests/test_widget_caching.py b/hubs/tests/test_widget_caching.py new file mode 100644 index 0000000..967db0e --- /dev/null +++ b/hubs/tests/test_widget_caching.py @@ -0,0 +1,73 @@ +from __future__ import unicode_literals + +from hubs.widgets.caching import cache, CachedFunction + +from mock import Mock +from hubs.tests import APPTest +from hubs.models import Hub, Widget + + +class DummyFunction(CachedFunction): + + def __init__(self, *args, **kwargs): + super(DummyFunction, self).__init__(*args, **kwargs) + self.execute_mock = Mock() + + def execute(self, *args, **kwargs): + return self.execute_mock(*args, **kwargs) + + +class CachedFunctionTest(APPTest): + + def setUp(self): + super(CachedFunctionTest, self).setUp() + # Use a memory backend, not the default null backend, or we can't test + # anything. + cache.configure(backend='dogpile.cache.memory', + replace_existing_backend=True) + self.w_instance = Widget.query.filter( + Hub.name == "ralph", + Widget.plugin == "about", + ).one() + self.fn = DummyFunction(self.w_instance) + cache.delete(self.fn.get_cache_key()) + + def tearDown(self): + cache.delete(self.fn.get_cache_key()) + super(CachedFunctionTest, self).tearDown() + + def test_get_cache_key(self): + self.assertEqual( + self.fn.get_cache_key(), + b"%d|DummyFunction" % self.w_instance.idx + ) + + def test_result_cached(self): + self.fn.execute_mock.return_value = "testing" + result = self.fn() + self.fn.execute_mock.assert_called_once_with() + self.assertEqual(result, "testing") + result = self.fn() + self.assertEqual(result, "testing") + # Check it hasn't been called a second time. + self.fn.execute_mock.assert_called_once() + + def test_is_cached(self): + key = self.fn.get_cache_key() + self.assertFalse(self.fn.is_cached()) + cache.set(key, "testing is_cached") + self.assertTrue(self.fn.is_cached()) + + def test_invalidate(self): + key = self.fn.get_cache_key() + cache.set(key, "testing invalidate") + self.fn.invalidate() + self.assertFalse(self.fn.is_cached()) + + def test_rebuild(self): + key = self.fn.get_cache_key() + cache.set(key, "testing rebuild") + self.fn.execute_mock.return_value = "testing" + self.fn.rebuild() + self.fn.execute_mock.assert_called_once() + self.assertEqual(cache.get(key), "testing") diff --git a/hubs/widgets/caching.py b/hubs/widgets/caching.py new file mode 100644 index 0000000..2eded91 --- /dev/null +++ b/hubs/widgets/caching.py @@ -0,0 +1,147 @@ +""" +Attributes: + cache (dogpile.cache.region.CacheRegion): The cache where function results + will be stored. It is configured with the `fedora-hubs.cache` key in + fedmsg configuration. +""" +from __future__ import unicode_literals + +import datetime +import dogpile +import dogpile.cache +import fedmsg.config +import logging + +log = logging.getLogger(__name__) + + +def _get_cache(): + config = fedmsg.config.load_config() + cache_defaults = { + "backend": "dogpile.cache.dbm", + "expiration_time": 1, # Expire every 1 second, for development + "arguments": { + "filename": "/var/tmp/fedora-hubs-cache.db", + }, + } + cache = dogpile.cache.make_region() + cache.configure(**config.get('fedora-hubs.cache', cache_defaults)) + return cache + +cache = _get_cache() + + +class CachedFunction(object): + """ + A function that has automatic caching and invalidation features. + + This class is a wrapper for a function that will cache its results until a + relevant message is emitted on the bus. + + To use this class, you must subclass it and implement a couple methods. + It is instantiated by passing a widget instance (database record) as only + argument, which lets it access the `.config` property and act accordingly. + + You must implement the `execute()` method, which must return a + JSON-serializable value. + + You must also implement the `should_invalidate()` method, which takes the + bus message as only argument, and returns `True` if the cache should be + rebuild, `False` otherwise. + + To call the function, instanciate the class and execute it. You may also + call the `.get_data()` method. + + Args: + instance (hubs.models.Widget): The widget instance. + """ + + # invalidate_filter = {} + + def __init__(self, instance): + self.instance = instance + + def execute(self): + """ + The function to cache. + + This is the main method, it must be implemented. + + Returns: + dict or list: A JSON-serializable value that will be cached. + """ + raise NotImplementedError + + def get_cache_key(self): + return "|".join([ + str(self.instance.idx), + self.__class__.__name__, + ]).encode('utf-8') + + def get_data(self): + key = self.get_cache_key() + # log.debug("Accessing cache key %s", key) + return cache.get_or_create( + key, lambda: self.execute()) + + __call__ = get_data + + # def _get_topic(self, message): + # return message['topic'] + + # def _get_category(self, message): + # return message['topic'].split('.')[3] + + # def pass_filter(self, message): + # topic_filter = self.invalidate_filter.get("topic") + # if topic_filter and self._get_topic(message) not in topic_filter: + # return False + # category_filter = self.invalidate_filter.get("category") + # if category_filter and \ + # self._get_category(message) not in category_filter: + # return False + # return True + + def should_invalidate(self, message): + """ + Tell the cache invalidator if the received message should invalidate + this function's cache. + + Args: + message (dict): The recieved bus message. + + Returns: + bool: If `True`, the cache will be rebuilt. + """ + # if not self._pass_filter(message): + # return False + return True + + def is_cached(self): + """ + Return a boolean indicating if the function's result is currently in the cache. + + Returns: + bool: Whether the cache currently contains the function result. + """ + result = cache.get(self.get_cache_key(), ignore_expiration=True) + return not isinstance(result, dogpile.cache.api.NoValue) + + def invalidate(self): + """ + Invalidate this function's cache. + """ + key = self.get_cache_key() + if not self.is_cached(): + log.debug("Not deleting cache key %s. It is absent.", key) + return + log.debug("Deleting cache key %s.", key) + cache.delete(key) + + def rebuild(self): + """ + Rebuild this function's cache. + """ + self.invalidate() + self.instance.hub.last_refreshed = datetime.datetime.utcnow() + self.get_data() From 81125b1f916295de027e835048eeb6534a00e764 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Feb 09 2017 15:54:10 +0000 Subject: [PATCH 2/45] Add a class for widget-specific views --- diff --git a/hubs/widgets/chrome.py b/hubs/widgets/chrome.py old mode 100755 new mode 100644 index b869094..cc9b4ca --- a/hubs/widgets/chrome.py +++ b/hubs/widgets/chrome.py @@ -1,6 +1,6 @@ from __future__ import unicode_literals -from hubs.widgets.base import wraps +from functools import wraps from hubs.widgets import templating _panel_template = templating.environment.get_template( @@ -9,8 +9,9 @@ _panel_heading_template = templating.environment.get_template( 'templates/panel_heading.html') +# FIXME: replace with template block extension def panel(title=None, klass="panel-default", key=None, footer_template=None): - def decorator(func): + def decorator(view, func): @wraps(func) def inner(*args, **kwargs): heading = '' @@ -18,7 +19,8 @@ def panel(title=None, klass="panel-default", key=None, footer_template=None): if title: heading = _panel_heading_template.render(title=title) if footer_template: - footer = footer_template.render(**kwargs) + footer = templating.environment.get_template( + footer_template).render(**kwargs) content = func(*args, **kwargs) if key and not kwargs.get(key): return content diff --git a/hubs/widgets/view.py b/hubs/widgets/view.py new file mode 100644 index 0000000..f6f486a --- /dev/null +++ b/hubs/widgets/view.py @@ -0,0 +1,111 @@ +from __future__ import unicode_literals, absolute_import + +import flask +from flask.views import View + + +class WidgetView(View): + """ + This class is the skeleton of a widget-specific view. + + You must subclass this class to use it in your view: + + - the `name` attribute must be defined + - the `template_name` attribute must be defined, or the `get_template()` + method must be implemented + - the `get_context()` method must be implemented + + Every widget must have a view with the name "`root`" registered to `['/']`, + which will be the main entry point for the widget. + + The resulting endpoint will be composed using the widget name and the view + name as `_`, for example `meetings_root`. + Remember that when you want to reverse the URL with `url_for`. + + When reversing the URL, you need to pass the ``hub`` and ``idx`` kwargs, + which are respectively the hub name (`Hub.name`) and the widget instance + (the database record) primary key (`Widget.idx`). + + Attributes: + name (str): The view name. It will be used to compose the URL endpoint, + following the "`_`" convention. + url_rules (list): A list of URL rules that this view must be registered + for (like the `rule` parameter of Flask's `add_url_rule()`). + template_name (str): The template name to use for this view. It will be + looked for in the widget's template environment. + chrome (function): An optional chrome template wrapper. + + This class is Flask-specific. If another framework were to be switched to, + it would have to be re-implemented. + """ + + name = None + url_rules = [] + template_name = None + chrome = None + + def __init__(self, widget): + """ + Args: + widget (hubs.widgets.base.Widget): the widget that uses this view. + """ + self.widget = widget + if self.name is None: + raise NotImplementedError + + def get_context(self, instance, *args, **kwargs): + """ + Return the template context for this view. + + Args: + instance (hubs.models.Widget): the widget instance. + """ + raise NotImplementedError + + def get_template(self): + """ + Return the template object, looking for `template_name` in the widget's + template environment. + """ + if self.template_name is None: + return NotImplementedError + tpl_env = self.widget.get_template_environment() + template = tpl_env.get_template(self.template_name) + return template + + def get_extra_context(self, instance, *args, **kwargs): + """ + Add extra data to the template context. + """ + # Put source links in all API results + return { + 'source_url': flask.url_for( + 'widget_source', name=self.widget.name), + 'widget_url': flask.url_for( + '%s_root' % self.widget.name, + hub=instance.hub.name, idx=instance.idx), + 'edit_url': flask.url_for( + 'widget_edit', hub=instance.hub.name, idx=instance.idx), + 'widget': self.widget, + 'widget_instance': instance, + } + + def _get_instance(self, *args, **kwargs): + from hubs.views.utils import get_widget_instance + hubname = kwargs.pop("hub") + widgetidx = kwargs.pop("idx") + return get_widget_instance(hubname, widgetidx) + + def dispatch_request(self, *args, **kwargs): + """ + The `dispatch_request` method that subclasses of `flask.views.View` + must implement. It binds the other methods together. + """ + instance = self._get_instance(*args, **kwargs) + context = self.get_context(instance, *args, **kwargs) + context.update(self.get_extra_context(instance, *args, **kwargs)) + template = self.get_template() + if self.chrome is None: + return self.get_template().render(**context) + else: + return self.chrome(template.render)(**context) From b96f6eee158de1599ab70a74b2422ca7dbb8962f Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Feb 09 2017 15:54:10 +0000 Subject: [PATCH 3/45] Create a base class for widgets --- diff --git a/hubs/tests/test_widget_base.py b/hubs/tests/test_widget_base.py new file mode 100644 index 0000000..fd6b3b3 --- /dev/null +++ b/hubs/tests/test_widget_base.py @@ -0,0 +1,110 @@ +from __future__ import unicode_literals + +from hubs.widgets.base import Widget +from hubs.widgets.caching import CachedFunction +from hubs.widgets.view import WidgetView + +from mock import Mock +from hubs.tests import APPTest +#import hubs.models + + +class TestingWidget(Widget): + name = "testing" + +class TestingView(WidgetView): + name = "root" + +class TestingFunction(CachedFunction): + pass + + +class WidgetTest(APPTest): + + def setUp(self): + super(WidgetTest, self).setUp() + # Backup the URL map + self._old_url_map = ( + self.app.application.url_map._rules[:], + self.app.application.url_map._rules_by_endpoint.copy() + ) + + def tearDown(self): + # Restore the URL map + self.app.application.url_map._rules = self._old_url_map[0] + self.app.application.url_map._rules_by_endpoint = self._old_url_map[1] + super(WidgetTest, self).tearDown() + + def test_validate(self): + class LocalTestWidget(Widget): + views = {} + def get_views(self): + return self.views + test_widget = LocalTestWidget() + self.assertRaisesRegexp( + AttributeError, '.* "name" .*', test_widget.validate) + test_widget.name = "învalid næme" + self.assertRaisesRegexp( + AttributeError, '^Invalid widget name: ', test_widget.validate) + test_widget.name = "localtest" + self.assertRaisesRegexp( + AttributeError, '.* "position" .*', test_widget.validate) + test_widget.position = "invalid" + self.assertRaisesRegexp( + AttributeError, '.* "position" .*', test_widget.validate) + test_widget.position = "both" + self.assertRaisesRegexp( + AttributeError, '.* "root" .*', test_widget.validate) + test_widget.views["root"] = Mock() + test_widget.views["root"].url_rules = ["/invalid"] + self.assertRaisesRegexp( + AttributeError, '.* "/" .*', test_widget.validate) + test_widget.views["root"].url_rules = ["/"] + try: + test_widget.validate() + except AttributeError as e: + self.fail(e) + + def test_list_views(self): + testing_widget = TestingWidget() + self.assertEqual( + testing_widget.get_views(), + {"root": TestingView} + ) + + def test_list_functions(self): + testing_widget = TestingWidget() + self.assertEqual( + testing_widget.get_cached_functions(), + {"TestingFunction": TestingFunction} + ) + + def test_list_views(self): + class TestView1(WidgetView): + name = "root" + url_rules = ["/", "/test-1/"] + class TestView2(WidgetView): + name = "test2" + url_rules = ["/test-2"] + testing_widget = TestingWidget() + testing_widget.get_views = Mock() + testing_widget.get_views.return_value = { + "root": TestView1(testing_widget), + "test2": TestView2(testing_widget), + } + testing_widget.register_routes(self.app.application) + # TestView1 + self.assertIn("testing_root", + self.app.application.url_map._rules_by_endpoint) + self.assertIn("testing_root", self.app.application.view_functions) + rules = list(self.app.application.url_map.iter_rules(endpoint="testing_root")) + self.assertEqual(len(rules), 2) + self.assertEqual(rules[0].rule, "//w/testing//") + self.assertEqual(rules[1].rule, "//w/testing//test-1/") + # TestView2 + self.assertIn("testing_test2", + self.app.application.url_map._rules_by_endpoint) + self.assertIn("testing_test2", self.app.application.view_functions) + rules = list(self.app.application.url_map.iter_rules(endpoint="testing_test2")) + self.assertEqual(len(rules), 1) + self.assertEqual(rules[0].rule, "//w/testing//test-2") diff --git a/hubs/widgets/base.py b/hubs/widgets/base.py old mode 100755 new mode 100644 index 581532e..ecccf46 --- a/hubs/widgets/base.py +++ b/hubs/widgets/base.py @@ -1,117 +1,129 @@ -from __future__ import unicode_literals +from __future__ import unicode_literals, absolute_import import collections -import datetime -import functools -import hashlib -import json -import sys +import logging +import re -import dogpile.cache -import flask -import six.moves.urllib_parse +from importlib import import_module +from .caching import CachedFunction +from .view import WidgetView -import fedmsg.config -import logging log = logging.getLogger(__name__) -config = fedmsg.config.load_config() -cache_defaults = { - "backend": "dogpile.cache.dbm", - "expiration_time": 1, # Expire every 1 second, for development - "arguments": { - "filename": "/var/tmp/fedora-hubs-cache.db", - }, -} -cache = dogpile.cache.make_region() -cache.configure(**config.get('fedora-hubs.cache', cache_defaults)) - - Argument = collections.namedtuple( - 'Argument', ('name', 'default', 'validator', 'help')) - - -def argument(name, default, validator, help): - def decorator(func): - @wraps(func) - def inner(*args, **kwargs): - return func(*args, **kwargs) - - inner.widget_arguments.append(Argument(name, default, validator, help)) - return inner - return decorator - - -def AGPLv3(name): - def decorator(func): - @wraps(func) - def inner(session, widget, *args, **kwargs): - result = func(session, widget, *args, **kwargs) - result['source_url'] = flask.url_for('widget_source', name=name) - result['widget_url'] = flask.url_for( - 'widget_render', hub=widget.hub.name, idx=widget.idx) - result['edit_url'] = flask.url_for( - 'widget_edit', hub=widget.hub.name, idx=widget.idx) - result['widget'] = widget - return result - - return inner - return decorator - - -def smartcache(func): - @wraps(func) - def inner(session, widget, *args, **kwargs): - key = cache_key_generator(widget, *args, **kwargs) - creator = lambda: func(session, widget, *args, **kwargs) - #log.debug("Accessing cache key %s", key) - return cache.get_or_create(key, creator) + 'Argument', ('name', 'label', 'default', 'validator', 'help')) - return inner - -def invalidate_cache(widget, *args, **kwargs): - key = cache_key_generator(widget, *args, **kwargs) - - value = cache.get(key, ignore_expiration=True) - if isinstance(value, dogpile.cache.api.NoValue): - log.debug("Not deleting cache key %s. It is absent.", key) - return - - widget.hub.last_refreshed = datetime.datetime.utcnow() - log.debug("Deleting cache key %s.", key) - cache.delete(key) - - -def cache_key_generator(widget, *args, **kwargs): - return "|".join([ - str(widget.idx), - json.dumps(args), - json.dumps(kwargs), - ]).encode('utf-8') - - -def wraps(original): - @functools.wraps(original) - def decorator(subsequent): - subsequent = functools.wraps(original)(subsequent) - subsequent.widget_arguments = getattr(original, 'widget_arguments', []) - return subsequent - return decorator - - -def widget_route(**options): - """Register a view for the current widget. - - This decorator can be used to expose a specific function below a widget's - URL endpoint. Refer to the "Widget-specific views" section in the - documentation to learn how to construct the corresponding URL. +class Widget(object): + """ + The main widget class, you must subclass it to create a widget. + + Args: + name (str): The widget name. It will not be displayed in the UI, but + will appear in some URLs, so be careful to only use simple, + URL-compatible characters. + position (str): The position of the widget in the rendered page. It + should be one of the following values: 'left', 'right', or 'both'. + arguments (list): A list of dictionaries that describe a widget's + configuration. See `hubs.widgets.base.Argument`. """ - def decorator(func): - options["view_func_name"] = func.__name__ - mdict = sys.modules[func.__module__].__dict__ - mroutes = mdict.setdefault('ROUTES', []) - mroutes.append(options) - return func - return decorator + + name = None + position = None + arguments = [] + + def validate(self): + """Ensure that the widget has the bits it needs.""" + if self.name is None: + raise AttributeError('Widgets must have a "name" attribute') + if not re.match('^[\w_.-]+$', self.name): + raise AttributeError( + 'Invalid widget name: %r. ' % self.name + + 'Please use URL-compatible characters.') + if self.position not in ['left', 'right', 'both']: + raise AttributeError( + '%r\'s "position" is not: `left`, `right` or `both`' + % self.name) + root_view = self.get_views().get("root") + if root_view is None: + raise AttributeError( + 'Widgets must have a "root" view, please refer to the ' + 'documentation.') + if "/" not in root_view.url_rules: + raise AttributeError( + 'The root view must be registered for the "/" URL rule.') + + def get_arguments(self): + return [Argument(**arg) for arg in self.arguments] + + def get_template_environment(self): + """ + Get the template environment that widget-specific views will use. + """ + # We may have widget-specific template environment one day + # (when widget templates are stored in the widget's directory + # instead of a common widget template directory). + # When that happens, it should be cached in a Widget attribute. + from hubs.widgets.templating import environment + return environment + + def _get_local_subclasses(self, parent_class): + """ + Returns the subclasses of the `parent_class` defined in this widget's + Python module. + """ + result = [] + widget_module = import_module(self.__module__) + for objname in dir(widget_module): + obj = getattr(widget_module, objname) + if type(obj) != type(object): + continue # we only look for classes + if not issubclass(obj, parent_class): + continue # we only want subclasses of the parent_class + if obj.__module__ != self.__module__: + # we don't want locally imported classes (like the + # parent_class itself) + continue + result.append(obj) + return result + + def get_views(self): + """ + Returns: + dict: A dictionary of the widget-specific views, indexed by their + `name` attribute. + """ + return dict([ + (view.name, view) for view in + self._get_local_subclasses(WidgetView) + ]) + + def register_routes(self, app): + """ + Register the widget-specific views in the web framework. + + Args: + app (flask.Flask): The Flask application to register the views + with. + + This function is Flask-specific. + """ + for view_name, view_class in self.get_views().items(): + endpoint = "%s_%s" % (self.name, view_name) + view_func = view_class.as_view(endpoint, self) + for url_rule in view_class.url_rules: + rule = "//w/%s//%s" % ( + self.name, url_rule.lstrip("/")) + app.add_url_rule(rule, view_func=view_func) + + def get_cached_functions(self): + """ + Returns: + dict: A dictionary of the widget's cached functions, indexed by + their class name. + """ + result = {} + for fn_class in self._get_local_subclasses(CachedFunction): + result[fn_class.__name__] = fn_class + return result From c178b3e690b989153cbc0572c0c78b30eba211dd Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Feb 09 2017 15:54:10 +0000 Subject: [PATCH 4/45] Create a widget registry class --- diff --git a/hubs/app.py b/hubs/app.py old mode 100755 new mode 100644 index 60981cf..0e81a8d --- a/hubs/app.py +++ b/hubs/app.py @@ -104,5 +104,12 @@ def check_auth(): else: flask.g.auth = munch.Munch(logged_in=False) +# Register widgets +import hubs.widgets +hubs.widgets.registry.register_list(app.config["WIDGETS"]) +# Register routes import hubs.views + +# Add widget-specific routes +hubs.widgets.registry.register_routes(app) diff --git a/hubs/default_config.py b/hubs/default_config.py old mode 100755 new mode 100644 index e540ad9..be038ad --- a/hubs/default_config.py +++ b/hubs/default_config.py @@ -32,3 +32,29 @@ OIDC_SCOPES = [ # TODO - instead of 'develop', use the version from pkg_resources to figure out # the right tag to link people to. AGPL ftw. SOURCE_URL = 'https://pagure.io/fedora-hubs/blob/develop/f' # /hubs/widgets/badges.py' + + +WIDGETS = [ + 'hubs.widgets.dummy.Dummy', + 'hubs.widgets.stats.Stats', + 'hubs.widgets.rules.Rules', + 'hubs.widgets.sticky.Sticky', + 'hubs.widgets.about.About', + 'hubs.widgets.badges.Badges', + 'hubs.widgets.library.Library', + 'hubs.widgets.linechart.Linechart', + 'hubs.widgets.fedmsgstats.FedmsgStats', + 'hubs.widgets.feed.Feed', + 'hubs.widgets.subscriptions.Subscriptions', + 'hubs.widgets.meetings.Meetings', + 'hubs.widgets.pagure_pr.PagurePRs', + 'hubs.widgets.github_pr.GitHubPRs', + 'hubs.widgets.pagureissues.PagureIssues', + 'hubs.widgets.githubissues.GitHubIssues', + 'hubs.widgets.bugzilla.Bugzilla', + 'hubs.widgets.fhosted.FedoraHosted', + 'hubs.widgets.memberships.Memberships', + 'hubs.widgets.contact.Contact', + 'hubs.widgets.workflow.pendingacls.PendingACLs', + 'hubs.widgets.workflow.updates2stable.Updates2Stable', + ] diff --git a/hubs/tests/test_widget_routes.py b/hubs/tests/test_widget_routes.py deleted file mode 100644 index 7eae714..0000000 --- a/hubs/tests/test_widget_routes.py +++ /dev/null @@ -1,70 +0,0 @@ -from __future__ import unicode_literals - -import hubs.widgets - -from mock import Mock -from hubs.models import Hub, Widget -from hubs.tests import APPTest - -class WidgetRoutesTest(APPTest): - - def setUp(self): - super(WidgetRoutesTest, self).setUp() - # Backup the URL map - self._old_url_map = ( - self.app.application.url_map._rules[:], - self.app.application.url_map._rules_by_endpoint.copy() - ) - - def tearDown(self): - for module in hubs.widgets.registry.values(): - if hasattr(module, 'ROUTES'): - delattr(module, 'ROUTES') - # Restore the URL map - self.app.application.url_map._rules = self._old_url_map[0] - self.app.application.url_map._rules_by_endpoint = self._old_url_map[1] - super(WidgetRoutesTest, self).tearDown() - - def test_routes_variable(self): - calls = [] - def mock_view(*args, **kw): - calls.append({'args': args, 'kw': kw}) - return '' - mock_view.__module__ = 'hubs.widgets.contact' - hubs.widgets.contact.mock_view = mock_view - hubs.widgets.contact.ROUTES = [{ - 'rule': 'dummy-url', - 'endpoint': 'dummy-endpoint', - 'view_func_name': 'mock_view', - }] - hubs.views._load_widget_views() - added_rules = list( - self.app.application.url_map.iter_rules( - endpoint='contact_dummy-endpoint') - ) - self.assertEqual(len(added_rules), 1) - self.assertEqual( - added_rules[0].rule, - '///widget/dummy-url' - ) - widget = self.session.query(Widget).filter_by( - plugin="contact").first() - self.app.get('/%s/%s/widget/dummy-url' - % (widget.hub.name, widget.idx)) - self.assertEqual(len(calls), 1) - # Check arguments: session and widget - self.assertEqual(calls[0]["args"][0].__class__, self.session.__class__) - self.assertEqual(calls[0]["args"][1], widget) - - def test_decorator(self): - def mock_view(*args, **kw): - return '' - mock_view.__module__ = 'hubs.widgets.feed' - hubs.widgets.feed.mock_view = hubs.widgets.base.widget_route( - rule='dummy-url', endpoint='dummy-endpoint')(mock_view) - self.assertEqual( - hubs.widgets.feed.ROUTES, - [{'view_func_name': 'mock_view', - 'endpoint': 'dummy-endpoint', - 'rule': 'dummy-url'}] - ) diff --git a/hubs/views/__init__.py b/hubs/views/__init__.py index 54f6bd1..1c97db4 100644 --- a/hubs/views/__init__.py +++ b/hubs/views/__init__.py @@ -6,45 +6,3 @@ from .widget import * from .user import * from .api import * from .plus_plus import * - -# -# Add widget-specific routes -# - -import flask -import functools -import hubs.models -import hubs.widgets -from sqlalchemy.orm.exc import NoResultFound -from hubs.app import app, session -from .utils import get_widget - -def _widget_view_decorator(func): - """ - This internal decorator will edit the view function arguments. - - It will: - - remove the hub name and the widget primary key - - add the database session and the widget instance - """ - @functools.wraps(func) - def inner(*args, **kwargs): - hubname = kwargs.pop("hub") - widgetidx = kwargs.pop("idx") - widget = get_widget(hubname, widgetidx) - return func(session, widget, *args, **kwargs) - return inner - -def _load_widget_views(): - for name, module in hubs.widgets.registry.items(): - for params in getattr(module, 'ROUTES', []): - params["rule"] = "///widget/" \ - + params["rule"].lstrip("/") - if not params.get("endpoint"): - params["endpoint"] = params["view_func_name"] - params["endpoint"] = "%s_%s" % (name, params["endpoint"]) - params["view_func"] = _widget_view_decorator( - getattr(module, params.pop("view_func_name"))) - app.add_url_rule(**params) - -_load_widget_views() diff --git a/hubs/widgets/__init__.py b/hubs/widgets/__init__.py old mode 100755 new mode 100644 index 95253e7..6d5f257 --- a/hubs/widgets/__init__.py +++ b/hubs/widgets/__init__.py @@ -1,132 +1,10 @@ +""" +Attributes: + registry (dict): This dictionary is a registry of available widgets in + Fedora Hubs. +""" from __future__ import unicode_literals -from hubs.widgets import dummy -from hubs.widgets import stats -from hubs.widgets import rules -from hubs.widgets import sticky -from hubs.widgets import about -from hubs.widgets import badges -from hubs.widgets import library -from hubs.widgets import linechart -from hubs.widgets import fedmsgstats -from hubs.widgets import feed -from hubs.widgets import subscriptions -from hubs.widgets import meetings -from hubs.widgets import pagure_pr -from hubs.widgets import github_pr -from hubs.widgets import pagureissues -from hubs.widgets import githubissues -from hubs.widgets import bugzilla -from hubs.widgets import fhosted -from hubs.widgets import memberships -from hubs.widgets import contact +from .registry import WidgetRegistry -from hubs.widgets.workflow import pendingacls -from hubs.widgets.workflow import updates2stable - -from hubs.widgets.base import AGPLv3, smartcache - -registry = { - 'dummy': dummy, - 'stats': stats, - 'rules': rules, - 'sticky': sticky, - 'about': about, - 'badges': badges, - 'library': library, - 'linechart': linechart, - 'fedmsgstats': fedmsgstats, - 'feed': feed, - 'subscriptions': subscriptions, - 'meetings': meetings, - 'pagure_pr': pagure_pr, - 'github_pr': github_pr, - 'pagureissues': pagureissues, - 'githubissues': githubissues, - 'bugzilla': bugzilla, - 'fedorahosted': fhosted, - 'memberships': memberships, - 'contact': contact, - - 'workflow.pendingacls': pendingacls, - 'workflow.updates2stable': updates2stable, -} - - -def validate_registry(registry): - """ Ensure that the widgets in the registry have the bits they need. - - - Check that a template is available and has a render callable. - - Look for a data function, etc.. - """ - for name, module in registry.items(): - if not hasattr(module, 'template'): - raise AttributeError('%r has no "template"' % module) - if not hasattr(module.template, 'render'): - raise AttributeError('%r\'s template has no "render"' % module) - if not callable(module.template.render): - raise TypeError('%r\'s template.render not callable' % module) - - if not hasattr(module, 'position'): - raise AttributeError('%r has not "position" function' % module) - if module.position not in ['left', 'right', 'both']: - raise TypeError( - '%r\'s "position" is not: `left`, `right` or `both`' - % module) - - if not hasattr(module, 'data'): - raise AttributeError('%r has not "data" function' % module) - if not callable(module.data): - raise TypeError('%r\'s "data" is not callable' % module) - - if hasattr(module, 'chrome'): - if not callable(module.chrome): - raise TypeError('%r\'s "chrome" is not callable' % module) - - -def prepare_registry(registry): - """ Do things ahead of time that we can to the registry. - - - Wrap a cache layer around the data functions. - - Wrap any chrome around the render functions. - """ - for name, module in registry.items(): - # Wrap chrome around the render function - module.render = module.template.render - if hasattr(module, 'chrome'): - module.render = module.chrome(module.render) - - # Put source links in all API results - module.data = AGPLv3(name)(module.data) - - # Wrap the data functions in a cache layer to be invalidated by fedmsg - # TODO -- we could just do this with a decorator to be explicit.. - module.data = smartcache(module.data) - - -validate_registry(registry) -prepare_registry(registry) - - -def get_site_vars(): - import flask - return dict( - session=flask.app.session, - g=flask.g, - url_for=flask.url_for, - ) - - -def render(module, session, widget, *args, **kwargs): - """ Main API entry point. - - Call this to render a widget into HTML - """ - # The API returns exactly this data. Shared cache - data = module.data(session, widget, *args, **kwargs) - - # Also expose some site-level info to the widget here at render-time - data.update(get_site_vars()) - - # Use the API data to fill out a template, and potentially decorate it. - return module.render(**data) +registry = WidgetRegistry() diff --git a/hubs/widgets/registry.py b/hubs/widgets/registry.py new file mode 100644 index 0000000..006eced --- /dev/null +++ b/hubs/widgets/registry.py @@ -0,0 +1,68 @@ +from __future__ import unicode_literals + +from importlib import import_module +from six.moves import UserDict +from hubs.widgets.base import Widget + + +class WidgetRegistry(UserDict): + """ + The widget registry. + + It behaves like a dictionary where widget names are keys and widget class + instances are values. There are additional methods to register widgets and + widget-specific routes. + """ + + def register_list(self, widget_list): + """ + Register a list of widget class paths. + + Args: + widget_list (list): List of Python paths to widget classes that + must be registered. + """ + for widget_path in widget_list: + self.register(widget_path) + + def register(self, widget_path): + """ + Register a widget. + + Args: + widget_path (str): Python path to the subclass of + `hubs.widgets.base.Widget`. + """ + mod_path, _, cls_name = widget_path.rpartition('.') + mod = import_module(mod_path) + try: + widget_class = getattr(mod, cls_name) + except AttributeError: + raise ValueError("Can't find widget %s" % widget_path) + if not issubclass(widget_class, Widget): + raise ValueError( + "Widget %s must be a subclass of %s.Widget" + % (widget_path, Widget.__module__)) + widget = widget_class() + if not widget.name: + prefix = 'hubs.widgets.' + if widget_path.startswith(prefix): + widget.name = widget_path[len(prefix):] + else: + widget.name = widget_path + widget.validate() + self.data[widget.name] = widget + + def register_routes(self, app): + """ + Register widget-specific routes with the provided app. + + This is not done automatically to allow widgets to be registered + without instantiating the Flask application. + + Args: + app (flask.Flask): The Flask application to register the views + with. + """ + for widget in self.values(): + widget.register_routes(app) diff --git a/populate.py b/populate.py index d4b262d..503b0e9 100755 --- a/populate.py +++ b/populate.py @@ -6,12 +6,24 @@ from __future__ import unicode_literals import json import hubs.models +import hubs.widgets import fedmsg.config fedmsg_config = fedmsg.config.load_config() session = hubs.models.init(fedmsg_config['hubs.sqlalchemy.uri'], True, True) +# Register widgets we will use +hubs.widgets.registry.register_list([ + "hubs.widgets.contact.Contact", + "hubs.widgets.stats.Stats", + "hubs.widgets.rules.Rules", + "hubs.widgets.meetings.Meetings", + "hubs.widgets.about.About", + "hubs.widgets.sticky.Sticky", + "hubs.widgets.dummy.Dummy", + ]) + users = ['mrichard', 'duffy', 'ryanlerch', 'gnokii', 'nask0', 'abompard', 'decause', 'ralph', 'lmacken', 'croberts', 'mattdm', 'pravins', 'keekri', 'linuxmodder', 'bee2502', 'jflory7'] From eb85faa6512dcc5a63cbdd22909b9da85ccf503a Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Feb 09 2017 15:54:10 +0000 Subject: [PATCH 5/45] Adapt the models to the new widget class --- diff --git a/hubs/models.py b/hubs/models.py old mode 100755 new mode 100644 index 63fbcb1..d322437 --- a/hubs/models.py +++ b/hubs/models.py @@ -30,6 +30,7 @@ import os import random import bleach +import dogpile import sqlalchemy as sa from sqlalchemy import create_engine from sqlalchemy.ext.declarative import declarative_base @@ -292,10 +293,11 @@ class Hub(BASE): def _config_default(context): - plugin_name = context.current_parameters['plugin'] - plugin = hubs.widgets.registry[plugin_name] - arguments = getattr(plugin.data, 'widget_arguments', []) - return json.dumps(dict([(arg.name, arg.default) for arg in arguments])) + widget_name = context.current_parameters['plugin'] + widget = hubs.widgets.registry[widget_name] + return json.dumps(dict([ + (arg.name, arg.default) for arg in widget.get_arguments() + ])) class Widget(BASE): @@ -334,8 +336,11 @@ class Widget(BASE): def __json__(self): session = object_session(self) module = hubs.widgets.registry[self.plugin] - data = module.data(session, self, **self.config) + root_view = module.get_views()["root"](module) + data = root_view.get_context(self) + data.update(root_view.get_extra_context(self)) data.pop('widget', None) + data.pop('widget_instance', None) return { 'id': self.idx, # TODO -- use flask.url_for to get the url for this widget @@ -355,11 +360,6 @@ class Widget(BASE): def module(self): return hubs.widgets.registry[self.plugin] - def render(self): - session = object_session(self) - render = hubs.widgets.render - return render(self.module, session, self, **self.config) - class User(BASE): __tablename__ = 'users' From 57569e8d718edf8733eba6b4f4c242f7cbce111d Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Feb 09 2017 15:54:10 +0000 Subject: [PATCH 6/45] Rename get_widget to get_widget_instance To make the distinction clearer betweed widgets (subclasses of hubs.widgets.base.Widget) and widget instances (database records, instances of hubs.models.Widget). --- diff --git a/hubs/views/utils.py b/hubs/views/utils.py index e2d4bd2..df05992 100644 --- a/hubs/views/utils.py +++ b/hubs/views/utils.py @@ -20,7 +20,7 @@ def get_hub(name, session=None): flask.abort(404) -def get_widget(hub, idx, session=None): +def get_widget_instance(hub, idx, session=None): """ Utility shorthand to get a widget and 404 if not found. """ if session is None: session = flask.g.db diff --git a/hubs/views/widget.py b/hubs/views/widget.py index f413985..613e47d 100644 --- a/hubs/views/widget.py +++ b/hubs/views/widget.py @@ -3,14 +3,14 @@ from __future__ import unicode_literals, absolute_import import datetime import flask -from hubs.app import app, session -from .utils import get_widget +from hubs.app import app +from .utils import get_widget_instance @app.route('///') @app.route('//') def widget_render(hub, idx): - widget = get_widget(hub, idx) + widget = get_widget_instance(hub, idx) return widget.render() # , edit=False) # was blocking all widgets from working, sorry! @@ -18,7 +18,7 @@ def widget_render(hub, idx): @app.route('///json') @app.route('///json/') def widget_json(hub, idx): - widget = get_widget(hub, idx) + widget = get_widget_instance(hub, idx) response = flask.jsonify(widget.__json__()) # TODO -- modify headers with response.headers['X-fedora-hubs-wat'] = 'foo' return response @@ -34,7 +34,7 @@ def widget_edit(hub, idx): def widget_edit_get(hub, idx): - widget = get_widget(hub, idx) + widget = get_widget_instance(hub, idx) return flask.render_template( 'edit.html', hub=hub, @@ -44,7 +44,7 @@ def widget_edit_get(hub, idx): def widget_edit_post(hub, idx): - widget = get_widget(hub, idx) + widget = get_widget_instance(hub, idx) error = False config = {} for arg in widget.module.data.widget_arguments: @@ -80,7 +80,7 @@ def widget_edit_post(hub, idx): @app.route('///delete', methods=['POST']) def widget_edit_delete(hub, idx): ''' Remove a widget from a hub. ''' - widget = get_widget(hub, idx) + widget = get_widget_instance(hub, idx) flask.g.db.delete(widget) try: flask.g.db.commit() From d505f7b6be1f007151764f84c075cde45d61447a Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Feb 09 2017 15:58:10 +0000 Subject: [PATCH 7/45] Adapt views and templates to the new widget class --- diff --git a/hubs/templates/add_widget.html b/hubs/templates/add_widget.html index 422cb84..76fc4ac 100644 --- a/hubs/templates/add_widget.html +++ b/hubs/templates/add_widget.html @@ -8,7 +8,7 @@ {% endif %} + {% else %} +

Nothing to configure

{% endfor %} - {% else %} -

Nothing to configure

- {% endif %} {% endif %}