From 0d65583e2777cebe8ab7993212373f440b62a163 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Jan 19 2018 13:48:30 +0000 Subject: [PATCH 1/3] Move the get_fedmsg_config function --- diff --git a/check-queue-length.py b/check-queue-length.py index d6d90e9..3673515 100755 --- a/check-queue-length.py +++ b/check-queue-length.py @@ -17,7 +17,7 @@ from __future__ import unicode_literals, print_function import retask -from hubs.utils import get_fedmsg_config +from hubs.utils.fedmsg import get_fedmsg_config config = get_fedmsg_config() diff --git a/create-group-from-fas.py b/create-group-from-fas.py index 34c515e..7458518 100755 --- a/create-group-from-fas.py +++ b/create-group-from-fas.py @@ -9,7 +9,7 @@ from fedora.client import AppError import hubs.app import hubs.database import hubs.models -from hubs.utils import get_fedmsg_config +from hubs.utils.fedmsg import get_fedmsg_config from hubs.utils.fas import FASClient diff --git a/hubs/app.py b/hubs/app.py index a596e63..ef00df2 100644 --- a/hubs/app.py +++ b/hubs/app.py @@ -10,7 +10,8 @@ import munch from flask_oidc import OpenIDConnect import hubs.models -from hubs.utils import get_fedmsg_config, username2avatar, hub2groupavatar +from hubs.utils import username2avatar, hub2groupavatar +from hubs.utils.fedmsg import get_fedmsg_config app = flask.Flask(__name__) diff --git a/hubs/backend/consumer.py b/hubs/backend/consumer.py index 080f919..7082bbb 100644 --- a/hubs/backend/consumer.py +++ b/hubs/backend/consumer.py @@ -9,7 +9,7 @@ import fedmsg.meta import retask.task import retask.queue -from hubs.utils import get_fedmsg_config +from hubs.utils.fedmsg import get_fedmsg_config log = logging.getLogger("hubs") diff --git a/hubs/database.py b/hubs/database.py index 03d75de..3d4c049 100644 --- a/hubs/database.py +++ b/hubs/database.py @@ -32,7 +32,7 @@ from sqlalchemy.orm import sessionmaker from sqlalchemy.orm import scoped_session from sqlalchemy.schema import MetaData -from hubs.utils import get_fedmsg_config +from hubs.utils.fedmsg import get_fedmsg_config log = logging.getLogger(__name__) diff --git a/hubs/feed.py b/hubs/feed.py index a7b6476..4610679 100644 --- a/hubs/feed.py +++ b/hubs/feed.py @@ -11,7 +11,8 @@ import pymongo from fedmsg.encoding import loads, dumps from hubs.models import Hub, User, HubConfig, Association -from hubs.utils import get_fedmsg_config, pagure +from hubs.utils import pagure +from hubs.utils.fedmsg import get_fedmsg_config log = logging.getLogger(__name__) diff --git a/hubs/migrations/env.py b/hubs/migrations/env.py index 05a85ce..fdcf778 100644 --- a/hubs/migrations/env.py +++ b/hubs/migrations/env.py @@ -16,7 +16,7 @@ except ImportError: sys.path.append(os.getcwd()) from hubs.database import BASE -from hubs.utils import get_fedmsg_config +from hubs.utils.fedmsg import get_fedmsg_config # This is the Alembic Config object, which provides diff --git a/hubs/utils/__init__.py b/hubs/utils/__init__.py index 3caf9e4..7db4aff 100755 --- a/hubs/utils/__init__.py +++ b/hubs/utils/__init__.py @@ -1,29 +1,10 @@ from __future__ import unicode_literals -import os from hashlib import sha256, md5 import humanize -import fedmsg.config -import fedmsg.meta from six.moves.urllib_parse import urlencode -from hubs.default_fedmsg_config import config as default_fedmsg_config - - -def get_fedmsg_config(): - try: - filenames = [os.environ["FEDMSG_CONFIG"]] - except KeyError: - filenames = None - fedmsg_config = fedmsg.config.load_config(filenames=filenames) - # Only add default values if they don't exist yet. - for key, value in default_fedmsg_config.items(): - if key not in fedmsg_config: - fedmsg_config[key] = value - # Meta processors - fedmsg.meta.make_processors(**fedmsg_config) - return fedmsg_config def username2avatar(username, s=312): query = urlencode([('s', s), ('d', 'retro')]) diff --git a/hubs/utils/cache.py b/hubs/utils/cache.py index ab618a2..f087b64 100644 --- a/hubs/utils/cache.py +++ b/hubs/utils/cache.py @@ -10,7 +10,7 @@ from __future__ import unicode_literals import dogpile import dogpile.cache -from hubs.utils import get_fedmsg_config +from hubs.utils.fedmsg import get_fedmsg_config def _get_cache(): diff --git a/hubs/utils/fas.py b/hubs/utils/fas.py index 1d1099e..a992ebf 100644 --- a/hubs/utils/fas.py +++ b/hubs/utils/fas.py @@ -12,7 +12,7 @@ from six.moves.email_mime_text import MIMEText from hubs.database import Session from hubs.models import Hub, User, Association -from hubs.utils import get_fedmsg_config +from hubs.utils.fedmsg import get_fedmsg_config log = logging.getLogger(__name__) diff --git a/hubs/utils/fedmsg.py b/hubs/utils/fedmsg.py new file mode 100644 index 0000000..30544b0 --- /dev/null +++ b/hubs/utils/fedmsg.py @@ -0,0 +1,31 @@ +from __future__ import absolute_import, unicode_literals + +import os + +import fedmsg +import fedmsg.config +import fedmsg.meta + +from hubs.default_fedmsg_config import config as default_fedmsg_config + + +TOPIC_PREFIX = "org.fedoraproject.hubs" + + +def get_fedmsg_config(): + try: + filenames = [os.environ["FEDMSG_CONFIG"]] + except KeyError: + filenames = None + fedmsg_config = fedmsg.config.load_config(filenames=filenames) + # Only add default values if they don't exist yet. + for key, value in default_fedmsg_config.items(): + if key not in fedmsg_config: + fedmsg_config[key] = value + # Meta processors + fedmsg.meta.make_processors(**fedmsg_config) + return fedmsg_config + + +def publish(topic, msg): + return fedmsg.publish(topic=topic, modname='hubs', msg=msg) diff --git a/hubs/widgets/github_pr/__init__.py b/hubs/widgets/github_pr/__init__.py index e638778..84c76e7 100644 --- a/hubs/widgets/github_pr/__init__.py +++ b/hubs/widgets/github_pr/__init__.py @@ -2,7 +2,8 @@ from __future__ import unicode_literals import logging -from hubs.utils import get_fedmsg_config, validators +from hubs.utils import validators +from hubs.utils.fedmsg import get_fedmsg_config from hubs.utils.github import github_repos, github_pulls from hubs.widgets.base import Widget from hubs.widgets.view import RootWidgetView diff --git a/hubs/widgets/my_hubs/__init__.py b/hubs/widgets/my_hubs/__init__.py index 6ab1387..13f9322 100644 --- a/hubs/widgets/my_hubs/__init__.py +++ b/hubs/widgets/my_hubs/__init__.py @@ -1,7 +1,7 @@ from __future__ import unicode_literals import hubs.models -from hubs.utils import get_fedmsg_config +from hubs.utils.fedmsg import get_fedmsg_config from hubs.widgets.base import Widget from hubs.widgets.view import RootWidgetView diff --git a/populate-from-fas.py b/populate-from-fas.py index 303b3bd..b339f1c 100755 --- a/populate-from-fas.py +++ b/populate-from-fas.py @@ -11,7 +11,7 @@ import fedora.client.fas2 import hubs.database import hubs.models -from hubs.utils import get_fedmsg_config +from hubs.utils.fedmsg import get_fedmsg_config from hubs.utils.fas import FASClient diff --git a/populate.py b/populate.py index 240047e..74ecd55 100755 --- a/populate.py +++ b/populate.py @@ -8,7 +8,7 @@ import hubs.app import hubs.database import hubs.models import hubs.widgets -from hubs.utils import get_fedmsg_config +from hubs.utils.fedmsg import get_fedmsg_config fedmsg_config = get_fedmsg_config() diff --git a/sync-group-from-fas.py b/sync-group-from-fas.py index b9664ae..113c6a0 100755 --- a/sync-group-from-fas.py +++ b/sync-group-from-fas.py @@ -9,7 +9,7 @@ from fedora.client import AppError import hubs.app import hubs.database from hubs.models import Hub -from hubs.utils import get_fedmsg_config +from hubs.utils.fedmsg import get_fedmsg_config from hubs.utils.fas import sync_team_hub From 49c00a462953a796f5ab9140efa31f71afce4347 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Jan 19 2018 13:48:30 +0000 Subject: [PATCH 2/3] Use fedmsg to notify changes in Hubs --- diff --git a/ansible/roles/hubs/handlers/main.yml b/ansible/roles/hubs/handlers/main.yml index 7f7235b..f71ee8f 100644 --- a/ansible/roles/hubs/handlers/main.yml +++ b/ansible/roles/hubs/handlers/main.yml @@ -6,6 +6,11 @@ listen: "hubs configuration change" when: not hubs_dev_mode +- name: restart the hubs-specific fedmsg-relay + service: name=hubs-fedmsg-relay state=restarted + listen: "hubs configuration change" + when: not hubs_dev_mode + - name: restart hubs triage service: name=hubs-triage@* state=restarted listen: "hubs configuration change" diff --git a/ansible/roles/hubs/tasks/main.yml b/ansible/roles/hubs/tasks/main.yml index e65bba1..da40241 100644 --- a/ansible/roles/hubs/tasks/main.yml +++ b/ansible/roles/hubs/tasks/main.yml @@ -12,6 +12,7 @@ - redis - mongodb-server - fedmsg-hub + - fedmsg-relay - python-virtualenv - python3-flask-oidc @@ -159,8 +160,11 @@ # Services -- name: Disable the system-wide fedmsg-hub - service: name=fedmsg-hub state=stopped enabled=no +- name: Disable the system-wide fedmsg daemons + service: name={{ item }} state=stopped enabled=no + with_items: + - fedmsg-hub + - fedmsg-relay # Include mode-specific tasks diff --git a/ansible/roles/hubs/tasks/prod.yml b/ansible/roles/hubs/tasks/prod.yml index 0969ffd..0cc4894 100644 --- a/ansible/roles/hubs/tasks/prod.yml +++ b/ansible/roles/hubs/tasks/prod.yml @@ -7,6 +7,7 @@ - hubs-worker@ - hubs-sse - hubs-fedmsg-hub + - hubs-fedmsg-relay register: service_installed - name: reload systemd @@ -22,5 +23,6 @@ - hubs-worker@2 - hubs-sse - hubs-fedmsg-hub + - hubs-fedmsg-relay - include_tasks: webserver.yml diff --git a/ansible/roles/hubs/templates/fedmsg_config b/ansible/roles/hubs/templates/fedmsg_config index 9df3e04..a5c6803 100644 --- a/ansible/roles/hubs/templates/fedmsg_config +++ b/ansible/roles/hubs/templates/fedmsg_config @@ -20,6 +20,9 @@ config = { 'hubs.consumer.enabled': True, 'hubs.redis.triage-queue-name': 'fedora-hubs-triage-queue', + # Use fedmsg-relay to publish messages + 'active': True, + # FAS credentials #'fas_credentials': { # 'username': '{{ hubs_fas_username }}', diff --git a/ansible/roles/hubs/templates/honcho-procfile b/ansible/roles/hubs/templates/honcho-procfile index ee8a451..feba397 100644 --- a/ansible/roles/hubs/templates/honcho-procfile +++ b/ansible/roles/hubs/templates/honcho-procfile @@ -3,4 +3,5 @@ triage: {{ hubs_venv_dir }}/bin/fedora-hubs-triage worker: {{ hubs_venv_dir }}/bin/fedora-hubs-worker sse: {{ hubs_venv_dir }}/bin/python /usr/bin/twistd -l - --pidfile= -ny {{ hubs_code_dir }}/hubs/backend/sse_server.tac fedmsg_hub: {{ hubs_venv_dir }}/bin/python /usr/bin/fedmsg-hub +fedmsg_relay: {{ hubs_venv_dir }}/bin/python /usr/bin/fedmsg-relay js_build: cd {{ hubs_code_dir }}/hubs/static/client && npm run dev diff --git a/ansible/roles/hubs/templates/hubs-fedmsg-relay.service b/ansible/roles/hubs/templates/hubs-fedmsg-relay.service new file mode 100644 index 0000000..d8fe0ca --- /dev/null +++ b/ansible/roles/hubs/templates/hubs-fedmsg-relay.service @@ -0,0 +1,14 @@ +[Unit] +Description=Hubs-specific fedmsg processing relay +After=network.target +Documentation=https://fedmsg.readthedocs.org/ + +[Service] +ExecStart={{ hubs_venv_dir }}/bin/python /usr/bin/fedmsg-relay +Type=simple +User=fedmsg +Group=fedmsg +Restart=on-failure + +[Install] +WantedBy=multi-user.target diff --git a/docs/dev-guide.rst b/docs/dev-guide.rst index db78bae..4f4a691 100644 --- a/docs/dev-guide.rst +++ b/docs/dev-guide.rst @@ -281,7 +281,7 @@ flexibility. To get this working, you're going to set up: Start with some required packages:: - $ sudo dnf install postgresql-server python-datanommer-consumer datanommer-commands fedmsg-hub npm + $ sudo dnf install postgresql-server python-datanommer-consumer datanommer-commands fedmsg-hub fedmsg-relay npm And there are some support libraries you'll also need:: diff --git a/fedmsg.d/hubs.py b/fedmsg.d/hubs.py new file mode 100644 index 0000000..98685ba --- /dev/null +++ b/fedmsg.d/hubs.py @@ -0,0 +1,4 @@ +config = { + # Use fedmsg-relay to publish messages + 'active': True, +} diff --git a/hubs/backend/triage.py b/hubs/backend/triage.py index d296c03..d76f2db 100755 --- a/hubs/backend/triage.py +++ b/hubs/backend/triage.py @@ -90,6 +90,50 @@ def triage(msg): 'username': msg["msg"]["user"], })) + # Handle Hubs changes + if topic.endswith('.hubs.hub.created'): + hub = hubs.models.Hub.query.get(msg["msg"]["hub_id"]) + if hub is not None: + yield retask.task.Task(json.dumps({ + 'type': 'sync-team-hub', + 'hub': hub.id, + 'created': True, + })) + if topic.endswith('.hubs.user.created'): + username = msg["msg"]["username"] + user = hubs.models.User.query.get(username) + if user is not None: + yield retask.task.Task(json.dumps({ + 'type': 'sync-user', + 'username': username, + 'created': True, + })) + if topic.endswith('.hubs.widget.updated'): + widget = hubs.models.Widget.query.get(msg["msg"]["widget_id"]) + if widget is not None: + widget_will_reload = False + cached_functions = widget.module.get_cached_functions() + for fn_name, fn_class in cached_functions.items(): + fn = fn_class(widget) + if fn.should_invalidate_on_widget_config_change( + msg["msg"]["changed_keys"]): + yield retask.task.Task(json.dumps({ + 'type': 'widget-cache', + 'idx': widget.idx, + 'hub': widget.hub.id, + 'fn_name': fn_name, + 'msg_id': msg['msg_id'], + })) + widget_will_reload = True + if not widget_will_reload: + # Reload the widget nonetheless because the config + # change may impact rendering. + yield retask.task.Task(json.dumps({ + 'type': 'widget-update', + 'idx': widget.idx, + 'hub': widget.hub.id, + })) + # Store the list of concerned hubs to check later in the # should_invalidate() method of Feed widgets. msg["_hubs"] = hubs.feed.get_hubs_for_msg(msg) diff --git a/hubs/models/hub.py b/hubs/models/hub.py index 3f9a12f..ce6db70 100644 --- a/hubs/models/hub.py +++ b/hubs/models/hub.py @@ -36,6 +36,7 @@ from hubs.defaults import ( add_group_widgets, add_user_widgets, add_stream_widgets, ) from hubs.utils import username2avatar +from hubs.utils.fedmsg import publish from hubs.signals import hub_created from .association import Association from .constants import ROLES, HUB_TYPES @@ -233,41 +234,28 @@ class Hub(ObjectAuthzMixin, BASE): self.subscribe(user, role='owner') elif self.hub_type == "team": add_group_widgets(self) - flask.g.task_queue.enqueue( - "sync-team-hub", - hub=self.id, - created=True, - ) + publish("hub.created", { + "hub_name": self.name, + "hub_type": self.hub_type, + "hub_id": self.id + }) elif self.hub_type == "stream": add_stream_widgets(self) def on_updated(self, old_config): - for widget_instance in self.widgets: - if not widget_instance.enabled: - continue - widget = widget_instance.module - new_config = self.config.to_dict() - will_reload = False - cached_functions = widget.get_cached_functions() - for fn_name, fn_class in cached_functions.items(): - fn = fn_class(widget_instance) - if fn.should_invalidate_on_hub_config_change(old_config): - flask.g.task_queue.enqueue( - "widget-cache", - idx=widget_instance.idx, - hub=self.name, - fn_name=fn_name, - ) - will_reload = True - if not will_reload: - # Reload the widget if it has asked for it. - if widget.should_reload_on_hub_config_change( - old_config, new_config): - flask.g.task_queue.enqueue( - "widget-update", - idx=widget_instance.idx, - hub=self.name, - ) + new_config = self.config.to_dict() + changed = [ + key for key in new_config.keys() + if new_config.get(key) != old_config.get(key) + ] + # Notify but don't send the config values on the bus, there + # may be private stuff there. + publish("hub.updated", { + "hub_name": self.name, + "hub_type": self.hub_type, + "hub_id": self.id, + "changed_keys": changed, + }) def _get_auth_user_access_level(self, user): # overridden to handle user and stream hubs. diff --git a/hubs/models/user.py b/hubs/models/user.py index dcd519a..2378a77 100644 --- a/hubs/models/user.py +++ b/hubs/models/user.py @@ -32,6 +32,7 @@ from sqlalchemy.orm import relation from hubs.database import BASE, Session from hubs.utils import username2avatar +from hubs.utils.fedmsg import publish from hubs.signals import user_created from .savednotification import SavedNotification @@ -150,8 +151,6 @@ class User(BASE): Hub.create_user_hub(self.username, self.fullname) if Hub.by_name(self.username, "stream") is None: Hub.create_stream_hub(self.username) - flask.g.task_queue.enqueue( - "sync-user", - username=self.username, - created=True, - ) + publish("user.created", { + "username": self.username, + }) diff --git a/hubs/models/widget.py b/hubs/models/widget.py index 10a637a..1bf4cef 100644 --- a/hubs/models/widget.py +++ b/hubs/models/widget.py @@ -27,12 +27,12 @@ import json import logging from collections import defaultdict -import flask import sqlalchemy as sa import hubs.widgets from hubs.authz import ObjectAuthzMixin from hubs.database import BASE +from hubs.utils.fedmsg import publish log = logging.getLogger(__name__) @@ -100,26 +100,18 @@ class Widget(ObjectAuthzMixin, BASE): self._config = json.dumps(config) def on_updated(self, old_config): - will_reload = False - cached_functions = self.module.get_cached_functions() - for fn_name, fn_class in cached_functions.items(): - fn = fn_class(self) - if fn.should_invalidate_on_widget_config_change(old_config): - flask.g.task_queue.enqueue( - "widget-cache", - idx=self.idx, - hub=self.hub.name, - fn_name=fn_name, - ) - will_reload = True - if not will_reload: - # Reload the widget nonetheless because the config - # change may impact rendering. - flask.g.task_queue.enqueue( - "widget-update", - idx=self.idx, - hub=self.hub.name, - ) + new_config = self.config + changed = [ + key for key in new_config.keys() + if new_config.get(key) != old_config.get(key) + ] + publish("widget.updated", { + "hub_name": self.hub.name, + "hub_type": self.hub.hub_type, + "hub_id": self.hub.id, + "widget_id": self.idx, + "changed_keys": changed, + }) def _get_auth_access_level(self, user): return self.hub._get_auth_access_level(user) diff --git a/hubs/widgets/base.py b/hubs/widgets/base.py index cb8990d..5997082 100644 --- a/hubs/widgets/base.py +++ b/hubs/widgets/base.py @@ -223,22 +223,6 @@ class Widget(object): result[fn_class.__name__] = fn_class return result - def should_reload_on_hub_config_change(self, old_config, new_config): - """Return whether the widget should be reloaded when the hub - configuration changes. - - By default it wil follow ``reload_on_hub_config_change``. Overload this - method to do a more fine-grained analysis. - - Args: - old_config (dict): the hub's old configuration. - new_config (dict): the hub's new configuration. - - Returns: - bool: Whether the widget should be reloaded. - """ - return self.reload_on_hub_config_change - @property def display_title(self): """The title of the widget box in the UI.""" diff --git a/hubs/widgets/caching.py b/hubs/widgets/caching.py index ce60d30..5bbf790 100644 --- a/hubs/widgets/caching.py +++ b/hubs/widgets/caching.py @@ -41,13 +41,9 @@ class CachedFunction(object): retrieves a lot of raw data from an external service, and config-dependant filtering is done in the view calling the function. - invalidate_on_hub_config_change (bool): ``True`` if the cached result - depends on the hub configuration, ``False`` otherwise. Defaults - to ``False``. """ invalidate_on_widget_config_change = True - invalidate_on_hub_config_change = False def __init__(self, instance): self.instance = instance @@ -102,36 +98,21 @@ class CachedFunction(object): """ raise NotImplementedError - def should_invalidate_on_widget_config_change(self, old_config): + def should_invalidate_on_widget_config_change(self, changed): """Return whether the function's cache should be invalidated when the widget configuration changes. - By default it wil follow ``invalidate_on_widget_config_change``. + By default it will follow ``invalidate_on_widget_config_change``. Overload this method to do a more fine-grained analysis. Args: - old_config (dict): the widget's old configuration. + changed (list): The list of configuration keys that changed. Returns: bool: Whether the function's cache should be invalidated. """ return self.invalidate_on_widget_config_change - def should_invalidate_on_hub_config_change(self, old_config): - """Return whether the function's cache should be invalidated when the - hub configuration changes. - - By default it wil follow ``invalidate_on_hub_config_change``. - Overload this method to do a more fine-grained analysis. - - Args: - old_config (dict): the hub's old configuration. - - Returns: - bool: Whether the function's cache should be invalidated. - """ - return self.invalidate_on_hub_config_change - def is_cached(self): """ Return a boolean indicating if the function's result is currently in diff --git a/hubs/widgets/meetings/__init__.py b/hubs/widgets/meetings/__init__.py index 12b9041..7f9ce27 100644 --- a/hubs/widgets/meetings/__init__.py +++ b/hubs/widgets/meetings/__init__.py @@ -51,7 +51,6 @@ class BaseView(RootWidgetView): class GetMeetings(CachedFunction): TOPIC = ".fedocal.calendar." - invalidate_on_hub_config_change = True def execute(self): calendar = self.instance.hub.config.get("calendar") @@ -82,6 +81,12 @@ class GetMeetings(CachedFunction): return meetings def should_invalidate(self, message): + # Hub update + if message["topic"].endswith('.hubs.hub.updated'): + if "calendar" not in message["msg"]["changed_keys"]: + return False + return message["msg"]["hub_id"] == self.instance.hub.id + # Calendar update if self.TOPIC not in message["topic"]: return False try: @@ -90,10 +95,6 @@ class GetMeetings(CachedFunction): return False return (calendar == self.instance.hub.config.get("calendar")) - def should_invalidate_on_hub_config_change(self, old_config): - new_config = self.instance.hub.config - return old_config.get("calendar") != new_config.get("calendar") - def next_meeting(meetings): now = datetime.datetime.utcnow() From 67b7f3ab4d49d6eae3d640e811efccba87ddd341 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Jan 19 2018 13:48:30 +0000 Subject: [PATCH 3/3] Publish messages on subscription and unsubscription --- diff --git a/hubs/models/hub.py b/hubs/models/hub.py index ce6db70..4cd13c4 100644 --- a/hubs/models/hub.py +++ b/hubs/models/hub.py @@ -130,29 +130,39 @@ class Hub(ObjectAuthzMixin, BASE): return [assoc.user for assoc in self.associations if assoc.role == 'stargazer'] + def publish(self, topic, extra_msg=None): + msg = { + "hub_name": self.name, + "hub_type": self.hub_type, + "hub_id": self.id, + } + if extra_msg: + msg.update(extra_msg) + publish(topic, msg) + def subscribe(self, user, role='subscriber'): """ Subscribe a user to this hub. """ # TODO -- add logic here to manage not adding the user multiple - # times, doing different roles, etc.. publish a fedmsg message, - # etc... + # times, doing different roles, etc... session = object_session(self) session.add(Association(user=user, hub=self, role=role)) - session.flush() + session.commit() + self.publish("user.role.added", { + "username": user.username, + "role": role, + }) # Members & owners are subscribers by default too (#474) if self.hub_type == "team" and role in ( "member", "pending-member", "owner", "pending-owner"): existing_subscriber = Association.query.filter_by( hub=self, user=user, role="subscriber") if existing_subscriber.count() == 0: - session.add(Association( - hub=self, user=user, role="subscriber")) - session.commit() + self.subscribe(user, "subscriber") def unsubscribe(self, user, role='subscriber'): """ Unsubscribe a user to this hub. """ # TODO -- add logic here to manage not adding the user multiple - # times, doing different roles, etc.. publish a fedmsg message, - # etc... + # times, doing different roles, etc... session = object_session(self) association = Association.query.filter_by( hub=self, user=user, role=role).first() @@ -162,23 +172,28 @@ class Hub(ObjectAuthzMixin, BASE): # When stepping down from an owner, turn into a member. is_member = bool(Association.query.filter_by( hub=self, user=user, role="member").count()) - if is_member: - session.delete(association) - else: + if not is_member: association.role = 'member' - else: - session.delete(association) - session.flush() + session.commit() + self.publish("user.role.changed", { + "username": user.username, + "old_role": "owner", + "role": "member", + }) + return + session.delete(association) + session.commit() + self.publish("user.role.removed", { + "username": user.username, + "role": role, + }) # Members are subscribers by default too, unsubscribe them when they # leave. (#474) if role in ("member", "pending-member"): existing_subscriber = Association.query.filter_by( hub=self, user=user, role="subscriber") - try: - session.delete(existing_subscriber.one()) - except sa.orm.exc.NoResultFound: - pass - session.commit() + if existing_subscriber.count() > 0: + self.unsubscribe(user, "subscriber") @classmethod def by_name(cls, name, hub_type): @@ -234,11 +249,7 @@ class Hub(ObjectAuthzMixin, BASE): self.subscribe(user, role='owner') elif self.hub_type == "team": add_group_widgets(self) - publish("hub.created", { - "hub_name": self.name, - "hub_type": self.hub_type, - "hub_id": self.id - }) + self.publish("hub.created") elif self.hub_type == "stream": add_stream_widgets(self) @@ -250,10 +261,7 @@ class Hub(ObjectAuthzMixin, BASE): ] # Notify but don't send the config values on the bus, there # may be private stuff there. - publish("hub.updated", { - "hub_name": self.name, - "hub_type": self.hub_type, - "hub_id": self.id, + self.publish("hub.updated", { "changed_keys": changed, })