Drop the updates.title column.

The updates.title field used to be a space separated list of the
builds found in the update, with a uniqueness constraint. There
were at least two problems with it.

One problem was that the list of builds was not consistently
updated (fedora-infra#1946) and sometimes the title did not reflect the current
set of builds in the update. This was more severe than it may seem,
because it turns out that the CLI was using the title to set the
list of builds when editing updates, instead of using the list of
builds. This means that the CLI could set the list of builds back
to a former set, rather than leaving it at the set it was
currently at. Note that the CLI does not currently support editing
the set of builds on an update (fedora-infra#3037), so it is especially
surprising when this happens.

The second problem is that large updates with many builds end up
generating a very long title and PostgreSQL raises an error
because it refuses to index strings beyond a certain length. It
was found, for example, that release engineering could not create
an update with 300 builds in it. Since we plan to use Bodhi to work
with side tags as part of gating Fedora Rawhide, it will be
important for users to be able to create updates with large
numbers of builds.

The title was also not a particularly useful field. It was possible
to use it as a key to access an update, but that was a poor use for
it since the title changes when the list of builds change, and
because the order of the list of builds was important. This led to
unpredictable and unstable URLs for updates, and Bodhi already had
an update alias field which is stable and is better suited for that
purpose.

This commit drops the field from the database and removes it from
being used to reference updates in the API. It preserves it in API
responses, however, but now generates it dynamically so that the
title is always an accurate list of the builds included in the
update.

fixes fedora-infra#186
fixes fedora-infra#1542
fixes fedora-infra#1946

Signed-off-by: Randy Barlow <[email protected]>

bodhi/client/__init__.py

@@ -421,7 +421,7 @@ def _validate_edit_update(ctx, param, value):
     """
     Validate the update argument given to the updates edit command.
 
-    The update argument can only be update id or update title
+    The update argument can only be an update id.
 
     Args:
         param (basestring): The name of the parameter being validated. Unused.
@@ -435,7 +435,7 @@ def _validate_edit_update(ctx, param, value):
        or re.search(bindings.UPDATE_TITLE_RE, value):
         return value
     else:
-        raise click.BadParameter("Please provide an Update ID or an Update Title")
+        raise click.BadParameter("Please provide an Update ID")
 
 
 @updates.command()
@@ -452,11 +452,11 @@ def edit(user, password, url, debug, openid_api, **kwargs):
     """
     Edit an existing update.
 
-    UPDATE: The title of the update (e.g. FEDORA-2017-f8e0ef2850)
+    UPDATE: The alias of the update (e.g. FEDORA-2017-f8e0ef2850)
     """
     # Developer Docs
     """
-    The update argument can be an update id or the update title.
+    The update argument must be an update id.
 
     Args:
         user (unicode): The username to authenticate as.
@@ -476,23 +476,19 @@ def edit(user, password, url, debug, openid_api, **kwargs):
         if re.search(bindings.UPDATE_ID_RE, kwargs['update']):
             query_param = {'updateid': kwargs['update']}
             resp = client.query(**query_param)
-            title = resp['updates'][0]['title']
         else:
-            # _validate_edit_update() has already ensured that we either got an update ID or an NVR,
-            # so we can assume here that we have an NVR.
-            query_param = {'like': kwargs['update']}
-            resp = client.query(**query_param)
-            title = kwargs['update']
+            raise click.BadParameter(f"{kwargs['update']} is not a valid update id.")
         del(kwargs['update'])
-        kwargs['builds'] = title
-        kwargs['edited'] = title
 
         # Convert list of 'Bug' instances in DB to comma separated bug_ids for parsing.
         former_update = resp['updates'][0].copy()
         if not kwargs['bugs']:
             kwargs['bugs'] = ",".join([str(bug['bug_id']) for bug in former_update['bugs']])
             former_update.pop('bugs', None)
 
+        kwargs['builds'] = [b['nvr'] for b in former_update['builds']]
+        kwargs['edited'] = former_update['alias']
+
         # Replace empty fields with former values from database.
         for field in kwargs:
             if kwargs[field] in (None, '') and field in former_update:
@@ -508,7 +504,6 @@ def edit(user, password, url, debug, openid_api, **kwargs):
 
 @updates.command()
 @click.option('--updateid', help='Query by update ID (eg: FEDORA-2015-0001)')
-@click.option('--title', help='Query by title')
 @click.option('--alias', help='Query by alias')
 @click.option('--approved-since', help='Approved after a specific timestamp')
 @click.option('--approved-before', help='Approved before a specific timestamp')
@@ -594,7 +589,7 @@ def request(update, state, user, password, url, openid_api, **kwargs):
     """
     Change an update's request status.
 
-    UPDATE: The title of the update (e.g. FEDORA-2017-f8e0ef2850)
+    UPDATE: The alias of the update (e.g. FEDORA-2017-f8e0ef2850)
 
     STATE: The state you wish to change the update's request to. Valid options are
     testing, stable, obsolete, unpush, and revoke.
@@ -642,7 +637,7 @@ def comment(update, text, karma, user, password, url, openid_api, **kwargs):
     """
     Comment on an update.
 
-    UPDATE: The title of the update (e.g. FEDORA-2017-f8e0ef2850)
+    UPDATE: The alias of the update (e.g. FEDORA-2017-f8e0ef2850)
 
     TEXT: the comment to be added to the update
     """
@@ -720,7 +715,7 @@ def download(url, **kwargs):
             # *think* that should ever be possible for these opts.
 
             for update in resp.updates:
-                click.echo("Downloading packages from {0}".format(update['title']))
+                click.echo("Downloading packages from {0}".format(update['alias']))
                 for build in update['builds']:
                     # subprocess is icky, but koji module doesn't
                     # expose this in any usable way, and we don't want
@@ -784,13 +779,13 @@ def waive(update, show, test, comment, url, openid_api, **kwargs):
     """
     Show or waive unsatified requirements (ie: missing or failing tests) on an existing update.
 
-    UPDATE: The title of the update (e.g. FEDORA-2017-f8e0ef2850)
+    UPDATE: The alias of the update (e.g. FEDORA-2017-f8e0ef2850)
 
     COMMENT: A comment explaining why the requirements were waived (mandatory with --test)
     """
     # Developer Docs
     """
-    The update argument can be an update id or the update title.
+    The update argument must be an update id..
 
     Args:
         update (unicode): The update who unsatisfied requirements wish to waive.
@@ -1017,7 +1012,7 @@ def print_resp(resp, client, verbose=False, override_hint=True):
                 resp.total, len(resp.updates)))
     elif resp.get('update'):
         click.echo(client.update_str(resp['update']))
-    elif 'title' in resp:
+    elif resp.get('alias'):
         click.echo(client.update_str(resp))
     elif 'overrides' in resp:
         if len(resp.overrides) == 1:
@@ -1035,7 +1030,7 @@ def print_resp(resp, client, verbose=False, override_hint=True):
         if override_hint:
             _print_override_koji_hint(resp, client)
     elif 'comment' in resp:
-        click.echo('The following comment was added to %s' % resp.comment['update'].title)
+        click.echo('The following comment was added to %s' % resp.comment['update'].alias)
         click.echo(resp.comment.text)
     elif 'compose' in resp:
         click.echo(client.compose_str(resp['compose'], minimal=False))

bodhi/client/bindings.py

@@ -226,7 +226,7 @@ def save(self, **kwargs):
         Save an update.
 
         This entails either creating a new update, or editing an existing one.
-        To edit an existing update, you must specify the update title in
+        To edit an existing update, you must specify the update alias in
         the ``edited`` keyword argument.
 
         Args:
@@ -250,7 +250,7 @@ def save(self, **kwargs):
             stable_karma (int): The upper threshold for marking an update as
                 ``stable``.
             unstable_karma (int): The lower threshold for unpushing an update.
-            edited (basestring): The update title of the existing update that we are
+            edited (basestring): The update alias of the existing update that we are
                 editing.
             severity (basestring): The severity of this update (``urgent``, ``high``,
                 ``medium``, ``low``).
@@ -277,7 +277,7 @@ def request(self, update, request):
         Request an update state change.
 
         Args:
-            update (basestring): The title of the update.
+            update (basestring): The alias of the update.
             request (basestring): The request (``testing``, ``stable``, ``obsolete``,
                 ``unpush``, ``revoke``).
         Returns:
@@ -303,7 +303,7 @@ def waive(self, update, comment, tests=None):
         Waive unsatisfied requirements on an update.
 
         Args:
-            update (basestring): The title of the update.
+            update (basestring): The alias of the update.
             comment (basestring): A comment explaining the waiver.
             tests (tuple(basestring) or None): The list of unsatisfied requirements
                 to waive. If not specified, all unsatisfied requirements of this
@@ -330,7 +330,6 @@ def query(self, **kwargs):
         Query bodhi for a list of updates.
 
         Args:
-            title (basestring): The update title.
             alias (basestring): The update alias.
             updateid (basestring): The update ID (eg: FEDORA-2015-0001).
             content_type (basestring): A content type (rpm, module) to limit the query to.
@@ -376,9 +375,6 @@ def query(self, **kwargs):
         Returns:
             munch.Munch: The response from Bodhi describing the query results.
         """
-        if 'title' in kwargs:
-            kwargs['like'] = kwargs['title']
-            del kwargs['title']
         # bodhi1 compat
         if 'limit' in kwargs:
             kwargs['rows_per_page'] = kwargs['limit']
@@ -416,7 +412,7 @@ def get_test_status(self, update):
         Query bodhi for the test status of the specified update..
 
         Args:
-            update (basestring): The title or identifier of the update to
+            update (basestring): The alias of the update to
                 retrieve the test status of.
         Returns:
             munch.Munch: The response from Bodhi describing the query results.
@@ -429,7 +425,7 @@ def comment(self, update, comment, karma=0, email=None):
         Add a comment to an update.
 
         Args:
-            update (basestring): The title of the update comment on.
+            update (basestring): The alias of the update comment on.
             comment (basestring): The text of the comment to add to the update.
             karma (int): The amount of karma to leave. May be -1, 0, or 1. Defaults to 0.
             email (basestring or None): Email address for an anonymous user. If an email address is
@@ -798,16 +794,15 @@ def update_str(self, update, minimal=False):
         update_lines = ['{:=^80}\n'.format('=')]
         update_lines += [
             line + '\n' for line in textwrap.wrap(
-                update['title'].replace(',', ', '),
+                update['title'],
                 width=80,
                 initial_indent=' ' * 5,
                 subsequent_indent=' ' * 5)
         ]
         update_lines.append('{:=^80}\n'.format('='))
 
-        if update['alias']:
-            update_lines.append(
-                line_formatter.format('Update ID', update['alias']))
+        update_lines.append(
+            line_formatter.format('Update ID', update['alias']))
 
         update_lines += [
             line_formatter.format('Content Type', update['content_type']),
@@ -902,12 +897,8 @@ def update_str(self, update, minimal=False):
                     comments_lines[1:])
             ]
 
-        if update['alias']:
-            update_lines.append(
-                '\n  {0}updates/{1}\n'.format(self.base_url, update['alias']))
-        else:
-            update_lines.append(
-                '\n  {0}updates/{1}\n'.format(self.base_url, update['title']))
+        update_lines.append(
+            '\n  {0}updates/{1}\n'.format(self.base_url, update['alias']))
 
         return ''.join(update_lines)
 

bodhi/server/consumers/masher.py

@@ -1,4 +1,4 @@
-# Copyright © 2007-2018 Red Hat, Inc. and others.
+# Copyright © 2007-2019 Red Hat, Inc. and others.
 #
 # This file is part of Bodhi.
 #
@@ -211,14 +211,6 @@ def _get_composes(self, msg):
         with self.db_factory() as db:
             if 'api_version' in msg and msg['api_version'] == 2:
                 composes = [Compose.from_dict(db, c) for c in msg['composes']]
-            elif 'updates' in msg:
-                updates = [db.query(Update).filter(Update.title == t).one() for t in msg['updates']]
-                composes = Compose.from_updates(updates)
-                for c in composes:
-                    db.add(c)
-                    # This flush is necessary so the compose finds its updates, which gives it a
-                    # content_type when it is serialized later.
-                    db.flush()
             else:
                 raise ValueError('Unable to process fedmsg: {}'.format(msg))
 
@@ -383,7 +375,7 @@ def work(self):
         notifications.publish(
             topic="mashtask.mashing",
             msg=dict(repo=self.id,
-                     updates=[u.title for u in self.compose.updates],
+                     updates=[' '.join([b.nvr for b in u.builds]) for u in self.compose.updates],
                      agent=self.agent,
                      ctype=self.ctype.value),
             force=True,
@@ -469,7 +461,7 @@ def perform_gating(self):
         for update in self.compose.updates:
             result, reason = update.check_requirements(self.db, config)
             if not result:
-                self.log.warning("%s failed gating: %s" % (update.title, reason))
+                self.log.warning("%s failed gating: %s" % (update.alias, reason))
                 self.eject_from_mash(update, reason)
         # We may have removed some updates from this compose above, and do we don't want future
         # reads on self.compose.updates to see those, so let's mark that attribute expired so
@@ -486,7 +478,7 @@ def eject_from_mash(self, update, reason):
                 comment on the update, in a log message, and in a fedmsg.
         """
         update.locked = False
-        text = '%s ejected from the push because %r' % (update.title, reason)
+        text = '%s ejected from the push because %r' % (update.alias, reason)
         log.warning(text)
         update.comment(self.db, text, author=u'bodhi')
         # Remove the pending tag as well
@@ -721,7 +713,7 @@ def modify_bugs(self):
         """Mark bugs on each Update as modified."""
         self.log.info('Updating bugs')
         for update in self.compose.updates:
-            self.log.debug('Modifying bugs for %s', update.title)
+            self.log.debug('Modifying bugs for %s', update.alias)
             update.modify_bugs()
 
     @checkpoint
@@ -766,7 +758,7 @@ def send_testing_digest(self):
                     maildata += u' %3i  %s   %s\n' % (
                         update.days_in_testing,
                         update.abs_url(),
-                        update.title)
+                        update.alias)
                 maildata += '\n\n'
 
             critpath_updates = self.get_unapproved_critpath_updates(prefix)
@@ -776,7 +768,7 @@ def send_testing_digest(self):
                     maildata += u' %3i  %s   %s\n' % (
                         update.days_in_testing,
                         update.abs_url(),
-                        update.title)
+                        update.alias)
                 maildata += '\n\n'
 
             maildata += testhead % prefix

bodhi/server/consumers/updates.py

@@ -1,4 +1,4 @@
-# Copyright 2015-2018 Red Hat Inc., and others.
+# Copyright 2015-2019 Red Hat Inc., and others.
 #
 # This file is part of Bodhi.
 #
@@ -111,11 +111,6 @@ def consume(self, message):
         # https://github.com/fedora-infra/bodhi/issues/458
         time.sleep(1)
 
-        if not alias:
-            log.error("Update Handler got update with no "
-                      "alias %s." % pprint.pformat(msg))
-            return
-
         with self.db_factory() as session:
             update = Update.get(alias)
             if not update:
@@ -202,7 +197,7 @@ def work_on_bugs(self, session, update, bugs):
 
                 log.info("Commenting on %r" % bug.bug_id)
                 comment = config['initial_bug_msg'] % (
-                    update.title, update.release.long_name, update.abs_url())
+                    update.alias, update.release.long_name, update.abs_url())
 
                 log.info("Modifying %r" % bug.bug_id)
                 bug.modified(update, comment)

bodhi/server/mail.py

@@ -50,7 +50,7 @@
 %(email)s has deleted the %(package)s update for %(release)s\n\n%(updatestr)s
 """,
         'fields': lambda agent, x: {
-            'package': x.title,
+            'package': x.alias,
             'email': agent,
             'release': '%s %s' % (x.release.long_name, x.status),
             'updatestr': str(x)
@@ -62,7 +62,7 @@
 %(email)s has edited the %(package)s update for %(release)s\n\n%(updatestr)s
 """,
         'fields': lambda agent, x: {
-            'package': x.title,
+            'package': x.alias,
             'email': agent,
             'release': '%s %s' % (x.release.long_name, x.status),
             'updatestr': str(x)
@@ -74,7 +74,7 @@
 %(package)s has been successfully pushed for %(release)s.\n\n%(updatestr)s
 """,
         'fields': lambda agent, x: {
-            'package': x.title,
+            'package': x.alias,
             'release': '%s %s' % (x.release.long_name, x.status),
             'updatestr': str(x)
         }
@@ -187,7 +187,7 @@
 %(updatestr)s
 """,
         'fields': lambda agent, x: {
-            'package': x.title,
+            'package': x.alias,
             'comment': x.comments[-1],
             'updatestr': str(x)
         }
@@ -214,7 +214,7 @@
 %(updatestr)s
 """,
         'fields': lambda agent, x: {
-            'package': x.title,
+            'package': x.alias,
             'stablekarma': x.stable_karma,
             'updatestr': str(x)
         }
@@ -232,7 +232,7 @@
     https://admin.fedoraproject.org/updates/approve/%(package)s
 """,
         'fields': lambda agent, x: {
-            'package': x.title,
+            'package': x.alias,
             'submitter': agent,
             'updatestr': str(x)
         }