kallithea Changeset - af7b367f6b5a

Changeset - af7b367f6b5a

Parent rev.

Child rev.

[Not reviewed]

default

0 1 1

Thomas De Schampheleire - 5 years ago 2020-06-15 12:37:55
thomas.de_schampheleire@nokia.com

db: introduce constraint ensuring no duplicate (reviewer, pullrequest) combinations

A reviewer should only be added once to a review.

Previously, this was not ensured by the database itself, although that the
controller would try to not add duplicate reviewers. But there was no hard
guarantee: e.g. simultaneous adding of the same reviewer to the same review
by a review owner and admin, a framework bug that sends the same request
twice, ... could still trigger duplicate addition. Additionally, code
changes (e.g. a new API) could introduce bugs at the controller level.

Existing production databases were found to contain such duplicate entries.
Nevertheless, as the code displaying reviewers in a pull request filtered
out duplicates, this never showed in the UI, and never was a 'real' problem.

Add a UniqueConstraint in the database to prevent such entries, with a
database migration step that will first find and remove existing duplicates.

2 files changed with 74 insertions and 0 deletions:

kallithea/alembic/versions/f62826179f39_add_unique_constraint_on_.py

kallithea/model/db.py

0 comments (0 inline, 0 general)

kallithea/alembic/versions/f62826179f39_add_unique_constraint_on_.py

➞

Show inline comments

@@ new file 100644 @@
 # 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 3 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, see <http://www.gnu.org/licenses/>.
 """add unique constraint on PullRequestReviewer
 Revision ID: f62826179f39
 Revises: a0a1bf09c143
 Create Date: 2020-06-15 12:30:37.420321
 """
 # The following opaque hexadecimal identifiers ("revisions") are used
 # by Alembic to track this migration script and its relations to others.
 revision = 'f62826179f39'
 down_revision = 'a0a1bf09c143'
 branch_labels = None
 depends_on = None
 import sqlalchemy as sa
 from alembic import op
 from kallithea.model.db import PullRequestReviewer
 def upgrade():
     session = sa.orm.session.Session(bind=op.get_bind())
     # there may be existing duplicates in the database, remove them first
     seen = set()
     # duplicate_values contains one copy of each duplicated pair
     duplicate_values = (
         session
         .query(PullRequestReviewer.pull_request_id, PullRequestReviewer.user_id)
         .group_by(PullRequestReviewer.pull_request_id, PullRequestReviewer.user_id)
         .having(sa.func.count(PullRequestReviewer.pull_request_reviewers_id) > 1)
+    )
     for pull_request_id, user_id in duplicate_values:
         # duplicate_occurrences contains all db records of the duplicate_value
         # currently being processed
         duplicate_occurrences = (
             session
             .query(PullRequestReviewer)
             .filter(PullRequestReviewer.pull_request_id == pull_request_id)
             .filter(PullRequestReviewer.user_id == user_id)
+        )
         for prr in duplicate_occurrences:
             if (pull_request_id, user_id) in seen:
                 session.delete(prr)
             else:
                 seen.add((pull_request_id, user_id))
     session.commit()
     # after deleting all duplicates, add the unique constraint
     with op.batch_alter_table('pull_request_reviewers', schema=None) as batch_op:
         batch_op.create_unique_constraint(batch_op.f('uq_pull_request_reviewers_pull_request_id'), ['pull_request_id', 'user_id'])
 def downgrade():
     with op.batch_alter_table('pull_request_reviewers', schema=None) as batch_op:
         batch_op.drop_constraint(batch_op.f('uq_pull_request_reviewers_pull_request_id'), type_='unique')

kallithea/model/db.py

➞

Show inline comments

@@ @@ -2163,6 +2163,7 @@ class PullRequestReviewer(Base, BaseDbMo @@
     __tablename__ = 'pull_request_reviewers'
     __table_args__ = (
         Index('pull_request_reviewers_user_id_idx', 'user_id'),
         UniqueConstraint('pull_request_id', 'user_id'),
         _table_args_default_dict,
+    )

0 comments (0 inline, 0 general)