Evaluate Release.create_automatic_updates for automatic updates.

This makes the automatic update consumer compare the tags in tagged
builds to the candidate tags of releases which have automatic updates
enabled. Previously, this was defined in the instance configuration
which would have required restarting the instance after changes.

Signed-off-by: Nils Philippsen <nils@redhat.com>

bodhi/server/consumers/automatic_updates.py

@@ -24,7 +24,6 @@
 
 import logging
 import re
-import typing
 
 import fedora_messaging
 
@@ -67,53 +66,6 @@ def __init__(self, db_factory: transactional_session_maker = None):
         else:
             self.db_factory = db_factory
 
-    @classmethod
-    def _read_configuration(cls):
-        """Extract release <-> tags information from the configuration once."""
-        if getattr(cls, '_configuration_read', False):
-            return
-
-        cls._rels_to_tags = {}
-        cls._tags_to_rels = {}
-
-        for k, v in config.items():
-            m = cls._auto_update_cfg_key_re.match(k)
-
-            if not m:
-                continue
-
-            rname = m.group('releasename')
-            subkey = m.group('subkey')
-
-            if subkey != 'from_tags':
-                continue
-
-            tags = []
-
-            for tag in (x.strip() for x in v.split(',')):
-                if tag in cls._tags_to_rels:
-                    log.warning(f"Tag used twice, ignoring release/tag pairing: {rname!r}, {tag!r}")
-                    continue
-                tags.append(tag)
-                cls._tags_to_rels[tag] = rname
-
-            if tags:
-                cls._rels_to_tags[rname] = tags
-
-        cls._configuration_read = True
-
-    @property
-    def rels_to_tags(self) -> typing.Dict[str, typing.List[str]]:
-        """Map release names to tag names."""
-        self._read_configuration()
-        return self._rels_to_tags
-
-    @property
-    def tags_to_rels(self) -> typing.Dict[str, str]:
-        """Map tag names to release names."""
-        self._read_configuration()
-        return self._tags_to_rels
-
     def __call__(self, message: fedora_messaging.api.Message) -> None:
         """Create updates from appropriately tagged builds.
 
@@ -126,25 +78,27 @@ def __call__(self, message: fedora_messaging.api.Message) -> None:
             log.debug("Ignoring message without 'msg'.")
             return
 
-        btag = msg.get('tag')
-
-        if btag not in self.tags_to_rels:
-            log.debug(f"Ignoring build being tagged into {btag!r}.")
-            return
-
         missing = []
-        for mandatory in ('build_id', 'name', 'version', 'release'):
+        for mandatory in ('tag', 'build_id', 'name', 'version', 'release'):
             if mandatory not in msg:
                 missing.append(mandatory)
         if missing:
             raise BodhiException(f"Received incomplete tag message. Missing: {', '.join(missing)}")
 
+        btag = msg['tag']
         bname = msg['name']
         bversion = msg['version']
         brelease = msg['release']
         bnvr = f'{bname}-{bversion}-{brelease}'
 
         with self.db_factory() as dbsession:
+            rel = dbsession.query(Release).filter_by(create_automatic_updates=True,
+                                                     candidate_tag=btag).first()
+            if not rel:
+                log.debug(f"Ignoring build being tagged into {btag!r}, no release configured for "
+                          "automatic updates for it found.")
+                return
+
             koji = buildsys.get_session()
 
             kbuildinfo = koji.getBuild(bnvr)
@@ -161,14 +115,6 @@ def __call__(self, message: fedora_messaging.api.Message) -> None:
                 'nvr': kbuildinfo['nvr'].rsplit('-', 2),
             }
 
-            rname = self._tags_to_rels[btag]
-
-            rel = Release.get(rname)
-
-            if not rel:
-                raise BodhiException(f"Would create automatic update for {bnvr}, but release"
-                                     f" {rname!r} is missing.")
-
             bcls = ContentType.infer_content_class(Build, kbuildinfo)
             build = bcls.get(bnvr)
             if build:

bodhi/tests/server/__init__.py

@@ -105,7 +105,9 @@ def populate(db):
         pending_testing_tag='f17-updates-testing-pending',
         pending_stable_tag='f17-updates-pending',
         override_tag='f17-override',
-        branch='f17', state=ReleaseState.current)
+        branch='f17', state=ReleaseState.current,
+        create_automatic_updates=True,
+    )
     db.add(release)
     db.flush()
     # This mock will help us generate a consistent update alias.

bodhi/tests/server/base.py

@@ -260,7 +260,7 @@ def create_update(self, build_nvrs, release_name='F17'):
         """
         return create_update(self.db, build_nvrs, release_name)
 
-    def create_release(self, version):
+    def create_release(self, version, create_automatic_updates=False):
         """
         Create and return a :class:`Release` with the given version.
 
@@ -279,7 +279,9 @@ def create_release(self, version):
             pending_testing_tag='f{}-updates-testing-pending'.format(version),
             pending_stable_tag='f{}-updates-pending'.format(version),
             override_tag='f{}-override'.format(version),
-            branch='f{}'.format(version), state=models.ReleaseState.current)
+            branch='f{}'.format(version), state=models.ReleaseState.current,
+            create_automatic_updates=create_automatic_updates,
+        )
         self.db.add(release)
         models.Release._all_releases = None
         models.Release._tag_cache = None

bodhi/tests/server/consumers/test_automatic_updates.py

@@ -17,17 +17,15 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 """These are tests for the bodhi.server.consumers.automatic_updates module."""
 
-from contextlib import contextmanager
 from copy import deepcopy
 from unittest import mock
 import logging
-import re
 
 from fedora_messaging.api import Message
 import _pytest.logging
 import pytest
 
-from bodhi.server.consumers.automatic_updates import AutomaticUpdateHandler, config
+from bodhi.server.consumers.automatic_updates import AutomaticUpdateHandler
 from bodhi.server.exceptions import BodhiException
 from bodhi.server.models import Build, Update, UpdateType, User
 from bodhi.tests.server import base
@@ -46,44 +44,15 @@ def messages(self):
     _pytest.logging.LogCaptureFixture = MonkeyLogCaptureFixture
 
 
-@contextmanager
-def tweaked_configuration():
-    """Context manager for handling tweaked configuration.
-
-    AutomaticUpdateHandler caches some items of the configuration in the class.
-    Subsequently tweaked configuration is ineffective unless forced to be
-    re-read, which is what this context manager does, both for the code run in
-    context and any subsequent use of AutomaticUpdateHandler objects.
-
-    Usage:
-
-    with tweaked_configuration(), ...:
-        # tweak the configuration
-        ...
-
-        # do the things
-        ...
-
-    # original configuration is re-read
-    """
-    # force reading in mock configuration
-    del AutomaticUpdateHandler._configuration_read
-
-    # do the things in context
-    yield
-
-    # re-read real configuration for subsequent tests
-    del AutomaticUpdateHandler._configuration_read
-    AutomaticUpdateHandler._read_configuration()
-
-
 class TestAutomaticUpdateHandler(base.BasePyTestCase):
     """Test the automatic update handler."""
 
     def setup_method(self, method):
         """Set up environment for each test."""
         super().setup_method(method)
 
+        self.release = self.create_release('47', create_automatic_updates=True)
+
         msg = {
             'build_id': 442562,
             'name': 'colord',
@@ -140,49 +109,6 @@ def test___init___without_db_factory(self, transactional_session_maker):
         assert handler.db_factory is transactional_session_maker.return_value
         transactional_session_maker.assert_called_once_with()
 
-    def test_release_configuration_is_cached(self):
-        """Assert that release/tag configuration is only read once."""
-        # access configuration properties once
-        rels_to_tags = self.handler.rels_to_tags
-        tags_to_rels = self.handler.tags_to_rels
-
-        # assert the dicts aren't recreated
-        assert rels_to_tags is self.handler.rels_to_tags
-        assert tags_to_rels is self.handler.tags_to_rels
-
-    def test_wonky_configuration(self, caplog):
-        """Assert that the code copes well with 'wonky' configuration.
-
-        Things that should be handled:
-        - unknown subkeys to *.autoupdates should be ignored
-        - tags defined to create automatic updates for more than one release
-          should only apply to the first release
-        """
-        caplog.set_level(logging.DEBUG)
-
-        mock_config = {
-            'release.autoupdates.hahaha': 'boo',
-            'release.autoupdates.from_tags': 'tag1, tag2',
-            'otherrelease.autoupdates.from_tags': 'tag2, tag3',
-            'yetanotherrelease.autoupdates.from_tags': 'tag3',
-        }
-
-        with tweaked_configuration(), mock.patch.dict(config, values=mock_config, clear=True):
-            assert 'release' in self.handler.rels_to_tags
-            assert 'tag1' in self.handler.rels_to_tags['release']
-            assert 'tag2' in self.handler.rels_to_tags['release']
-
-            assert 'otherrelease' in self.handler.rels_to_tags
-            assert 'tag2' not in self.handler.rels_to_tags['otherrelease']
-            assert 'tag3' in self.handler.rels_to_tags['otherrelease']
-
-            assert 'yetanotherrelease' not in self.handler.rels_to_tags
-
-        assert ("Tag used twice, ignoring release/tag pairing: 'otherrelease', 'tag2'"
-                in caplog.messages)
-        assert ("Tag used twice, ignoring release/tag pairing: 'yetanotherrelease', 'tag3'"
-                in caplog.messages)
-
     # Test robustness: malformed messages, unknown koji builds, incomplete
     # buildinfo, release missing from the DB
 
@@ -231,20 +157,6 @@ def test_incomplete_koji_buildinfo_owner(self, caplog):
         assert(f"Koji build {sample_nvr} without owner name, creating update anonymously."
                in caplog.messages)
 
-    def test_missing_release(self):
-        """Test configured release missing from DB."""
-        mock_config = dict(config)
-        mock_config['missingrelease.autoupdates.from_tags'] = mock_config.pop(
-            'f17.autoupdates.from_tags'
-        )
-
-        with tweaked_configuration(), mock.patch.dict(
-            config, values=mock_config, clear=True
-        ), pytest.raises(BodhiException,
-                         match=re.compile(r"Would create automatic update for .*, but release "
-                                          r"'.*' is missing.")):
-            self.handler(self.sample_message)
-
     def test_missing_user(self, caplog):
         """Test Koji build user missing from DB."""
         caplog.set_level(logging.DEBUG)
@@ -289,7 +201,8 @@ def test_ignored_tag(self, caplog):
         msg.body['msg']['tag'] = bogus_tag
         self.handler(msg)
 
-        assert f"Ignoring build being tagged into '{bogus_tag}'." in caplog.messages
+        assert any(x.startswith(f"Ignoring build being tagged into '{bogus_tag}'")
+                   for x in caplog.messages)
 
     def test_duplicate_message(self, caplog):
         """Assert that duplicate messages ignore existing build/update."""