Mercurial > kallithea
changeset 8648:af7b367f6b5a
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.
| author | Thomas De Schampheleire <thomas.de_schampheleire@nokia.com> |
|---|---|
| date | Mon, 15 Jun 2020 12:37:55 +0200 |
| parents | 9f65a573a298 |
| children | 2589ee18c796 |
| files | kallithea/alembic/versions/f62826179f39_add_unique_constraint_on_.py kallithea/model/db.py |
| diffstat | 2 files changed, 74 insertions(+), 0 deletions(-) [+] |
line wrap: on
line diff
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/kallithea/alembic/versions/f62826179f39_add_unique_constraint_on_.py Mon Jun 15 12:37:55 2020 +0200 @@ -0,0 +1,73 @@ +# 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')
--- a/kallithea/model/db.py Wed Sep 30 13:39:33 2020 +0200 +++ b/kallithea/model/db.py Mon Jun 15 12:37:55 2020 +0200 @@ -2163,6 +2163,7 @@ __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, )