diff --git a/Phabricator/config.py b/Phabricator/config.py --- a/Phabricator/config.py +++ b/Phabricator/config.py @@ -70,6 +70,20 @@ )) ) +conf.registerChannelValue( + Phabricator, 'announce', + registry.Boolean(False, _( + """Determines whether Phabricator's feed will be announced + on the channel.""" + )) +) + +conf.registerChannelValue( + Phabricator.announce, 'interval', + registry.PositiveInteger(60, _( + """The interval between two queries to Phabricator's feed API.""" + )) +) # vim:set shiftwidth=4 tabstop=4 expandtab textwidth=79: diff --git a/Phabricator/plugin.py b/Phabricator/plugin.py --- a/Phabricator/plugin.py +++ b/Phabricator/plugin.py @@ -28,15 +28,18 @@ ### -from collections import defaultdict import re import time +import threading +from collections import defaultdict, namedtuple import phabricator import supybot.utils as utils from supybot.commands import * +import supybot.world as world import supybot.plugins as plugins +import supybot.ircmsgs as ircmsgs import supybot.ircutils as ircutils import supybot.callbacks as callbacks try: @@ -47,6 +50,8 @@ # without the i18n module _ = lambda x: x +feed_announce = namedtuple('feed_announce', 'fetch_time max_epoch') +cache_entry = namedtuple('phid_cache_entry', 'data expiry') class Phabricator(callbacks.PluginRegexp): """Integration with the Phabricator development collaboration tools""" @@ -63,7 +68,12 @@ self.default_conduit = None self._conduits = {} - self._phid_cache = defaultdict(lambda: (None, 0)) + self._last_feed_announces = defaultdict( + lambda: feed_announce(0, None)) + self._phid_object_cache = defaultdict( + lambda: cache_entry(None, 0)) + self._phid_transaction_cache = defaultdict( + lambda: cache_entry(None, 0)) host = self.registryValue('phabricatorURI') token = self.registryValue('phabricatorConduitToken') if host and token: @@ -97,18 +107,57 @@ return self.conduit_for_host_token(host, token) def get_object_by_phid(self, recipient, phid, skip_cache=False): + objs = self.get_objects_by_phid(recipient, [phid], skip_cache) + return objs.get(phid) + + def get_objects_by_phid(self, recipient, phids, skip_cache=False): + objects = {} if not skip_cache: - object, timeout = self._phid_cache[recipient, phid] - if timeout > time.time(): - return object - - r = self.conduit(recipient).phid.query(phids=[phid]) - object = r.response.get(phid) - self._phid_cache[recipient, phid] = ( - object, - time.time() + self.phid_cache_expiry - ) - return object + for phid in phids: + obj, timeout = self._phid_object_cache[recipient, phid] + if timeout > time.time(): + objects[phid] = obj + if set(objects) == set(phids): + # If we already got all the phids we need in the cache, + # no need to make a query + return objects + + # Else, make a single request for all the objects + # (even those in the cache; there's no harm in refreshing them) + r = self.conduit(recipient).phid.query(phids=phids) + for (phid, obj) in r.items(): + self._phid_object_cache[recipient, phid] = cache_entry( + obj, + time.time() + self.phid_cache_expiry + ) + return dict(r) + + def get_transactions_by_phid(self, recipient, transactions_phids, + object_phid, skip_cache=False): + transactions = {} + if not skip_cache: + for phid in transactions_phids: + trans, timeout = self._phid_transaction_cache[recipient, + object_phid, phid] + if timeout > time.time(): + transactions[phid] = trans + if set(transactions) == set(transactions_phids): + # If we already got all the phids we need in the cache, + # no need to make a query + return transactions + + # Else, make a single request for all the transactions + # (even those in the cache; there's no harm in refreshing them) + r = self.conduit(recipient).transaction.search( + objectIdentifier=object_phid, + constraints={'phids': transactions_phids}) + for trans in r['data']: + self._phid_transaction_cache[recipient, object_phid, trans['phid']] = cache_entry( + trans, + time.time() + self.phid_cache_expiry + ) + transactions[trans['phid']] = trans + return dict(r) def get_commit_author_info(self, recipient, commit, type='author'): if not commit['%sPHID' % type]: @@ -370,6 +419,68 @@ to=recipient, ) + def __call__(self, irc, msg): + super().__call__(irc, msg) + threading.Thread(target=self._update_feeds).start() + + def _update_feeds(self): + """Goes through all channels, and trigger an update on the ones + which have an associated feed.""" + for irc in world.ircs: + for channel in irc.state.channels: + if self.registryValue('announce', channel): + self._update_feed_if_needed(irc, channel) + + def _update_feed_if_needed(self, irc, channel): + """Triggers an update if the channel's feed should be updated.""" + recipient_key = (irc.network, channel) + previous_announce = self._last_feed_announces[recipient_key] + current_time = time.time() + interval = self.registryValue('announce.interval', channel) + if previous_announce.fetch_time + interval < current_time: + max_epoch = self._update_feed(irc, channel, + previous_announce.max_epoch) + self._last_feed_announces[recipient_key] = feed_announce( + fetch_time=current_time, + max_epoch=max_epoch) + + def _update_feed(self, irc, channel, after_epoch): + """Send updates of a feed to a channel, and returns its max epoch.""" + conduit = self.conduit(channel) + stories = conduit.feed.query(view='data') + + objects = self.get_objects_by_phid(channel, + [story['data']['objectPHID'] for story in stories.values()]) + + after_epoch = after_epoch or 0 + for story in stories.values(): + if (after_epoch > 0 and # Don't announce on the first run + story['epoch'] > after_epoch): + self._announce_story(irc, channel, story, + objects[story['data']['objectPHID']]) + max_epoch = max(story['epoch'] for story in stories.values()) + return max(after_epoch, max_epoch) + + def _announce_story(self, irc, channel, story, obj): + """Send a story to a channel.""" + transactions = self.get_transactions_by_phid(channel, + list(story['data']['transactionPHIDs']), obj['phid']) + actions = defaultdict(lambda: []) + for trans in transactions['data']: + actions[self.get_user_by_phid(channel, trans['authorPHID'])] \ + .append(trans['type']) + + parts = [] + for (author, author_actions) in actions.items(): + parts.append('{} from {}'.format('+'.join(author_actions), author)) + msg = format('%s; on %s %u', + ', '.join(parts), + obj['fullName'], + obj['uri'], + ) + irc.queueMsg(ircmsgs.privmsg(channel, msg)) + + Class = Phabricator diff --git a/Phabricator/test.py b/Phabricator/test.py --- a/Phabricator/test.py +++ b/Phabricator/test.py @@ -30,9 +30,231 @@ from supybot.test import * +ENTRY1 = { + 'authorPHID': 'PHID-USER-fozivtfr457sc7smrhtv', + 'chronologicalKey': '6607424649221995233', + 'class': 'PhabricatorApplicationTransactionFeedStory', + 'data': {'objectPHID': 'PHID-TASK-lwuvnwjjnenqsyan73om', + 'transactionPHIDs': {'PHID-XACT-TASK-cgdt45mjjymbxpk': 'PHID-XACT-TASK-cgdt45mjjymbxpk'}}, + 'epoch': 1538410933} -class PhabricatorTestCase(PluginTestCase): +ENTRY2 = { + 'authorPHID': 'PHID-USER-jyszzzys2aaakr2q2ijx', + 'chronologicalKey': '6607410428317095759', + 'class': 'PhabricatorApplicationTransactionFeedStory', + 'data': {'objectPHID': 'PHID-DREV-jaamseb4cyq2glp3ekmr', + 'transactionPHIDs': {'PHID-XACT-DREV-lo5bwt2qii6dbud': 'PHID-XACT-DREV-lo5bwt2qii6dbud', + 'PHID-XACT-DREV-q4kayyycsd4izyj': 'PHID-XACT-DREV-q4kayyycsd4izyj'}}, + 'epoch': 1538407622} + +ENTRY3 = { + 'authorPHID': 'PHID-USER-jyszzzys2aaakr2q2ijx', + 'chronologicalKey': '6607737853965443787', + 'class': 'PhabricatorApplicationTransactionFeedStory', + 'data': {'objectPHID': 'PHID-DREV-ypvg646gtojpnfcszoiv', + 'transactionPHIDs': {'PHID-XACT-DREV-5wapkzlcy3bhup4': 'PHID-XACT-DREV-5wapkzlcy3bhup4', + 'PHID-XACT-DREV-b7dsk7hobiqoxn6': 'PHID-XACT-DREV-b7dsk7hobiqoxn6'}}, + 'epoch': 1538483857} + +FEED1 = { + 'phid-stry-zuicf6fhi4esag22cmw2': ENTRY2, + } + +FEED2 = { + 'phid-stry-zuafozdksyhxqixmqej3': ENTRY1, + 'phid-stry-zuicf6fhi4esag22cmw2': ENTRY2, + } + +FEED3 = { + 'phid-stry-zuafozdksyhxqixmqej3': ENTRY1, + 'phid-stry-zuicf6fhi4esag22cmw2': ENTRY2, + 'phid-stry-zzphs53tpt6j7equbkp7': ENTRY3, + } + +QUERY_PHID = { +'PHID-DREV-jaamseb4cyq2glp3ekmr': {'fullName': 'D454: Provide Sphinx targets ' + 'in docs/images/ (see D453).', + 'name': 'D454', + 'phid': 'PHID-DREV-jaamseb4cyq2glp3ekmr', + 'status': 'open', + 'type': 'DREV', + 'typeName': 'Differential Revision', + 'uri': + 'https://forge.softwareheritage.org/D454'}, +'PHID-TASK-lwuvnwjjnenqsyan73om': {'fullName': 'T611: support for external ' + 'definitions in the ' + 'svn/subversion loader', + 'name': 'T611', + 'phid': 'PHID-TASK-lwuvnwjjnenqsyan73om', + 'status': 'open', + 'type': 'TASK', + 'typeName': 'Maniphest Task', + 'uri': + 'https://forge.softwareheritage.org/T611'}, +'PHID-USER-fozivtfr457sc7smrhtv': {'fullName': 'ardumont (Antoine R. Dumont)', + 'name': 'ardumont', + 'phid': 'PHID-USER-fozivtfr457sc7smrhtv', + 'status': 'open', + 'type': 'USER', + 'typeName': 'User', + 'uri': + 'https://forge.softwareheritage.org/p/ardumont/'}, +'PHID-DREV-ypvg646gtojpnfcszoiv': {'fullName': "D453: Make 'make' in " + "swh-*/docs/ should run 'make' " + 'in swh-*/docs/images/', + 'name': 'D453', + 'phid': 'PHID-DREV-ypvg646gtojpnfcszoiv', + 'status': 'open', + 'type': 'DREV', + 'typeName': 'Differential Revision', + 'uri': + 'https://forge.softwareheritage.org/D453'}, +'PHID-USER-jyszzzys2aaakr2q2ijx': {'fullName': 'vlorentz (vlorentz)', + 'name': 'vlorentz', + 'phid': 'PHID-USER-jyszzzys2aaakr2q2ijx', + 'status': 'open', + 'type': 'USER', + 'typeName': 'User', + 'uri': + 'https://forge.softwareheritage.org/p/vlorentz/'}, +} + +SEARCH_TRANSACTION = { + ('PHID-TASK-lwuvnwjjnenqsyan73om', ('PHID-XACT-TASK-cgdt45mjjymbxpk',)): + {'cursor': {'after': None, 'before': None, 'limit': 100}, + 'data': [{'authorPHID': 'PHID-USER-fozivtfr457sc7smrhtv', + 'comments': [{'authorPHID': 'PHID-USER-fozivtfr457sc7smrhtv', + 'content': {'raw': 'foobar1\nspam\negg'}, + 'dateCreated': 1538410933, + 'dateModified': 1538410933, + 'id': 2413, + 'phid': 'PHID-XCMT-6sqdj7jbbgpituemximf', + 'removed': False, + 'version': 1}], + 'dateCreated': 1538410933, + 'dateModified': 1538410933, + 'fields': {}, + 'id': 22700, + 'objectPHID': 'PHID-TASK-lwuvnwjjnenqsyan73om', + 'phid': 'PHID-XACT-TASK-cgdt45mjjymbxpk', + 'type': 'comment'}]}, + ('PHID-DREV-ypvg646gtojpnfcszoiv', ('PHID-XACT-DREV-5wapkzlcy3bhup4', + 'PHID-XACT-DREV-b7dsk7hobiqoxn6')): +{'cursor': {'after': None, 'before': None, 'limit': 100}, + 'data': [{'authorPHID': 'PHID-USER-jyszzzys2aaakr2q2ijx', + 'comments': [{'authorPHID': 'PHID-USER-jyszzzys2aaakr2q2ijx', + 'content': {'raw': ''}, + 'dateCreated': 1538483876, + 'dateModified': 1538483876, + 'id': 2255, + 'phid': 'PHID-XCMT-gvlxm7ka3qse5ycpvh5n', + 'removed': True, + 'version': 2}, + {'authorPHID': 'PHID-USER-jyszzzys2aaakr2q2ijx', + 'content': {'raw': ''}, + 'dateCreated': 1538483857, + 'dateModified': 1538483857, + 'id': 2254, + 'phid': 'PHID-XCMT-gsnm5pxmy4tb7mpr3vcg', + 'removed': False, + 'version': 1}], + 'dateCreated': 1538483857, + 'dateModified': 1538483876, + 'fields': {}, + 'id': 8824, + 'objectPHID': 'PHID-DREV-ypvg646gtojpnfcszoiv', + 'phid': 'PHID-XACT-DREV-5wapkzlcy3bhup4', + 'type': 'comment'}, + {'authorPHID': 'PHID-USER-jyszzzys2aaakr2q2ijx', + 'comments': [], + 'dateCreated': 1538483857, + 'dateModified': 1538483857, + 'fields': {'commitPHIDs': [], + 'new': 'PHID-DIFF-qdape3nidx5nk3tq6vw5', + 'old': 'PHID-DIFF-ymevkypj7damk72gisvq'}, + 'id': 8823, + 'objectPHID': 'PHID-DREV-ypvg646gtojpnfcszoiv', + 'phid': 'PHID-XACT-DREV-b7dsk7hobiqoxn6', + 'type': 'update'}]} + } + + +class PhabricatorTestCase(ChannelPluginTestCase): plugins = ('Phabricator',) + config = { + 'supybot.plugins.Phabricator.announce.interval': 1, + } + + def testAnnounce(self): + nb_mock_calls = 0 + + class MockConduit: + class feed: + @classmethod + def query(cls, *, view): + nonlocal nb_mock_calls + nb_mock_calls += 1 + self.assertEqual(view, 'data') + return current_feed + + class phid: + @classmethod + def query(cls, *, phids): + return {phid: QUERY_PHID[phid] for phid in phids} + + class transaction: + @classmethod + def search(cls, *, objectIdentifier, constraints): + self.assertEqual(set(constraints), {'phids'}) + return SEARCH_TRANSACTION[objectIdentifier, + tuple(sorted(constraints['phids']))] + + def mock_get_conduit(*args): + return MockConduit + self.irc.getCallback('Phabricator').conduit_for_host_token = \ + mock_get_conduit + + # Check there are no announce the first time. + current_feed = FEED1 + self.assertNotError('config channel plugins.Phabricator.announce True') + m = self.getMsg(' ', timeout=0.1) + self.assertEqual(nb_mock_calls, 1, m) + self.assertIs(m, None) + time.sleep(1.1) + m = self.getMsg(' ', timeout=0.1) + self.assertEqual(nb_mock_calls, 2, m) + self.assertIs(m, None) + + # A new story, that should be announced only after the time interval. + current_feed = FEED2 + m = self.getMsg(' ', timeout=0.1) + self.assertEqual(nb_mock_calls, 2, m) + self.assertIs(m, None) + time.sleep(1.1) + m = self.getMsg(' ', timeout=0.1) + self.assertEqual(nb_mock_calls, 3, m) + self.assertIsNot(m, None) + self.assertEqual(m.args[1], + 'comment from ardumont; on T611: support for external ' + 'definitions in the svn/subversion loader ' + '', m) + + # Another new story. + current_feed = FEED3 + m = self.getMsg(' ', timeout=0.1) + self.assertEqual(nb_mock_calls, 3, m) + self.assertIs(m, None) + time.sleep(1.1) + m = self.getMsg(' ', timeout=0.1) + self.assertEqual(nb_mock_calls, 4, m) + self.assertIsNot(m, None) + self.assertIn(m.args[1], + "comment+update from vlorentz; on D453: Make 'make' in " + "swh-*/docs/ should run 'make' in swh-*/docs/images/ " + "", + m) + + # vim:set shiftwidth=4 tabstop=4 expandtab textwidth=79: