diff --git a/bodhi/server/migrations/versions/1c97477e38ee_convert_br_override_notes_to_unicodetext.py b/bodhi/server/migrations/versions/1c97477e38ee_convert_br_override_notes_to_unicodetext.py
new file mode 100644
index 0000000000..8b83faa906
--- /dev/null
+++ b/bodhi/server/migrations/versions/1c97477e38ee_convert_br_override_notes_to_unicodetext.py
@@ -0,0 +1,41 @@
+# Copyright (c) 2020 Mattia Verga
+#
+# This file is part of Bodhi.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+"""
+Convert buildroot override notes to unicodetext.
+
+Revision ID: 1c97477e38ee
+Revises: 325954bac9f7
+Create Date: 2020-06-09 13:03:03.489398
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = '1c97477e38ee'
+down_revision = 'f50dc199039c'
+
+
+def upgrade():
+ """Convert buildrootoverrides notes column to UnicodeText."""
+ op.alter_column("buildroot_overrides", "notes", type_=sa.UnicodeText)
+
+
+def downgrade():
+ """Convert back buildrootoverrides notes column to Unicode."""
+ op.alter_column("buildroot_overrides", "notes", type_=sa.Unicode)
diff --git a/bodhi/server/models.py b/bodhi/server/models.py
index 3f9be95709..c3f41884a6 100644
--- a/bodhi/server/models.py
+++ b/bodhi/server/models.py
@@ -4718,7 +4718,7 @@ class BuildrootOverride(Base):
__include_extras__ = ('nvr',)
__get_by__ = ('build_id',)
- notes = Column(Unicode, nullable=False)
+ notes = Column(UnicodeText, nullable=False)
submission_date = Column(DateTime, default=datetime.utcnow, nullable=False)
expiration_date = Column(DateTime, nullable=False)
diff --git a/bodhi/server/services/overrides.py b/bodhi/server/services/overrides.py
index c54a5bb0c3..1870535151 100644
--- a/bodhi/server/services/overrides.py
+++ b/bodhi/server/services/overrides.py
@@ -33,6 +33,7 @@
from bodhi.server.validators import (
validate_override_builds,
validate_expiration_date,
+ validate_override_notes,
validate_packages,
validate_releases,
validate_username,
@@ -222,6 +223,7 @@ def query_overrides(request):
colander_body_validator,
validate_override_builds,
validate_expiration_date,
+ validate_override_notes,
))
@overrides.post(schema=bodhi.server.schemas.SaveOverrideSchema,
permission='edit',
@@ -231,6 +233,7 @@ def query_overrides(request):
colander_body_validator,
validate_override_builds,
validate_expiration_date,
+ validate_override_notes,
))
def save_override(request):
"""
@@ -269,20 +272,20 @@ def save_override(request):
data['expiration_date'] = max(existing_override.expiration_date,
data['expiration_date'])
- data['notes'] = """{0}
-
-_@{1} ({2})_
-
+ new_notes = f"""{data['notes']}
_____________
-{3}""".format(existing_override.notes, existing_override.submitter.name,
- existing_override.submission_date.strftime("%b %d, %Y"), data['notes'])
+_@{existing_override.submitter.name} ({existing_override.submission_date.strftime('%b %d, %Y')})_
+{existing_override.notes}"""
+ # Truncate notes at 2000 chars
+ if len(new_notes) > 2000:
+ new_notes = new_notes[:1972] + '(...)\n___Notes truncated___'
overrides.append(BuildrootOverride.edit(
request,
edited=build,
submitter=submitter,
submission_date=datetime.now(),
- notes=data['notes'],
+ notes=new_notes,
expiration_date=data['expiration_date'],
expired=None,
))
diff --git a/bodhi/server/templates/override.html b/bodhi/server/templates/override.html
index 3089451b4b..56fcb4ed90 100644
--- a/bodhi/server/templates/override.html
+++ b/bodhi/server/templates/override.html
@@ -137,7 +137,7 @@
Notes
-
+
diff --git a/bodhi/server/validators.py b/bodhi/server/validators.py
index 56f5bfb81a..1fa23d108c 100644
--- a/bodhi/server/validators.py
+++ b/bodhi/server/validators.py
@@ -1206,6 +1206,28 @@ def validate_expiration_date(request, **kwargs):
request.validated['expiration_date'] = expiration_date
+@postschema_validator
+def validate_override_notes(request, **kwargs):
+ """
+ Ensure the override notes is less than 500 chars.
+
+ Args:
+ request (pyramid.request.Request): The current request.
+ kwargs (dict): The kwargs of the related service definition. Unused.
+ """
+ notes = request.validated.get('notes')
+
+ if notes is None:
+ return
+
+ if len(notes) > 2000:
+ request.errors.add('body', 'notes',
+ 'Notes may not contain more than 2000 chars')
+ return
+
+ request.validated['notes'] = notes
+
+
def _get_valid_requirements(request, requirements):
"""
Return a list of valid testcases from taskotron.
diff --git a/bodhi/tests/server/services/test_overrides.py b/bodhi/tests/server/services/test_overrides.py
index af34ab2700..03c2f7746c 100644
--- a/bodhi/tests/server/services/test_overrides.py
+++ b/bodhi/tests/server/services/test_overrides.py
@@ -340,6 +340,7 @@ def test_create_override_for_build_with_test_gating_status_failed(self):
"Cannot create a buildroot override if build's test gating status is failed."
def test_create_duplicate_override(self):
+ """When creating a duplicate override, old notes are appended."""
release = Release.get('F17')
package = RpmPackage(name='not-bodhi')
self.db.add(package)
@@ -366,14 +367,43 @@ def test_create_duplicate_override(self):
data['notes'] = 'new blah blah'
res = self.app.post('/overrides/', data)
o = res.json_body
- new_notes = """blah blah blah
-
-_@guest ({})_
-
+ new_notes = f"""new blah blah
_____________
-new blah blah""".format(datetime.utcnow().strftime("%b %d, %Y"))
+_@guest ({datetime.utcnow().strftime('%b %d, %Y')})_
+blah blah blah"""
assert o['notes'] == new_notes
+ def test_create_duplicate_override_notes_too_long(self):
+ """When notes are too long, truncate the older."""
+ release = Release.get('F17')
+ package = RpmPackage(name='not-bodhi')
+ self.db.add(package)
+ build = RpmBuild(nvr='not-bodhi-2.0-2.fc17', package=package, release=release)
+ self.db.add(build)
+ self.db.flush()
+
+ expiration_date = datetime.utcnow() + timedelta(days=1)
+
+ data = {'nvr': build.nvr, 'notes': 'blah' * 500,
+ 'expiration_date': expiration_date,
+ 'csrf_token': self.get_csrf_token()}
+
+ with fml_testing.mock_sends(override_schemas.BuildrootOverrideTagV1):
+ res = self.app.post('/overrides/', data)
+
+ o = res.json_body
+ assert o['build_id'] == build.id
+ assert o['notes'] == 'blah' * 500
+ assert o['expiration_date'] == expiration_date.strftime("%Y-%m-%d %H:%M:%S")
+ assert o['expired_date'] is None
+
+ # Submit it again
+ data['notes'] = 'new blah blah'
+ res = self.app.post('/overrides/', data)
+ o = res.json_body
+ assert o['notes'].endswith('(...)\n___Notes truncated___')
+ assert len(o['notes']) <= 2000
+
def test_create_override_multiple_nvr(self):
release = Release.get('F17')
package = RpmPackage(name='not-bodhi')
diff --git a/bodhi/tests/server/test_validators.py b/bodhi/tests/server/test_validators.py
index 60748a138a..3abc7da399 100644
--- a/bodhi/tests/server/test_validators.py
+++ b/bodhi/tests/server/test_validators.py
@@ -382,6 +382,35 @@ def test_past(self):
assert request.errors.status == exceptions.HTTPBadRequest.code
+class TestValidateOverrideNotes(BasePyTestCase):
+ """Test the validate_override_notes() function."""
+
+ def test_none(self):
+ """Empty notes should be OK, since we will populate with a default text."""
+ request = mock.Mock()
+ request.errors = Errors()
+ request.validated = {'notes': None}
+
+ validators.validate_override_notes(request)
+
+ assert not len(request.errors)
+
+ def test_length_above_range(self):
+ """We don't allow too verbose notes."""
+ request = mock.Mock()
+ request.errors = Errors()
+ request.validated = {
+ 'notes': 'n' * 2001}
+
+ validators.validate_override_notes(request)
+
+ assert request.errors == [
+ {'location': 'body', 'name': 'notes',
+ 'description': 'Notes may not contain more than 2000 chars'}
+ ]
+ assert request.errors.status == exceptions.HTTPBadRequest.code
+
+
class TestValidateOverrideBuild(BasePyTestCase):
"""Test the validate_override_build() function."""
diff --git a/news/4044.bug b/news/4044.bug
new file mode 100644
index 0000000000..cbd9d7488f
--- /dev/null
+++ b/news/4044.bug
@@ -0,0 +1 @@
+Cap buildroot overrides notes to a maximum of 2k characters and convert the database field to UnicodeText
diff --git a/news/summary.migration b/news/summary.migration
index c7eee8ee10..f7c451e3c5 100644
--- a/news/summary.migration
+++ b/news/summary.migration
@@ -1,3 +1,3 @@
-Migrate relationship between TestCase and Package to TestCase and Build. The migration script will take care of migrate existing data to the new relation.
-The user_id column in comments table has been set to be not nullable.
-
+- Migrate relationship between TestCase and Package to TestCase and Build. The migration script will take care of migrate existing data to the new relation.
+- The user_id column in comments table has been set to be not nullable.
+- The notes column in buildroot_overrides table has been converted to UnicodeText (from Unicode).