Gergő Tisza has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/386731 )
Change subject: Fix recreation of deleted list entries ...................................................................... Fix recreation of deleted list entries We have a unique index on list + project + title, so a (soft)deleted entry blocks the creation of another entry with the same project and title in the same list. Instead, let's just repurpose the deleted row. Bug: T179120 Change-Id: Iddbc8af582c93aea182eb356745e34025b67ccae --- M src/ReadingListRepository.php M tests/src/ReadingListRepositoryTest.php 2 files changed, 49 insertions(+), 10 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/ReadingLists refs/changes/31/386731/1 diff --git a/src/ReadingListRepository.php b/src/ReadingListRepository.php index 4bd7e89..c23eb9c 100644 --- a/src/ReadingListRepository.php +++ b/src/ReadingListRepository.php @@ -371,29 +371,56 @@ throw new ReadingListRepositoryException( 'readinglists-db-error-not-own-list', [ $id ] ); } - $this->dbw->insert( + // due to the combination of soft deletion + unique constraint on + // rle_rl_id + rle_project + rle_title, recreation needs special handling + /** @var ReadingListEntryRow $row */ + $row = $this->dbw->selectRow( 'reading_list_entry', + [ 'rle_id', 'rle_deleted' ], [ 'rle_rl_id' => $id, - 'rle_user_id' => $this->userId, 'rle_project' => $project, 'rle_title' => $title, - 'rle_date_created' => $this->dbw->timestamp(), - 'rle_date_updated' => $this->dbw->timestamp(), - 'rle_deleted' => 0, ], __METHOD__, - // throw custom exception for unique constraint on rle_rl_id + rle_project + rle_title - [ 'IGNORE' ] + // lock the row to avoid race conditions with purgeOldDeleted() in the update case + [ 'FOR UPDATE' ] ); - if ( !$this->dbw->affectedRows() ) { + if ( $row === false ) { + $this->dbw->insert( + 'reading_list_entry', + [ + 'rle_rl_id' => $id, + 'rle_user_id' => $this->userId, + 'rle_project' => $project, + 'rle_title' => $title, + 'rle_date_created' => $this->dbw->timestamp(), + 'rle_date_updated' => $this->dbw->timestamp(), + 'rle_deleted' => 0, + ] + ); + } elseif ( $row->rle_deleted ) { + $this->dbw->update( + 'reading_list_entry', + [ + 'rle_date_created' => $this->dbw->timestamp(), + 'rle_date_updated' => $this->dbw->timestamp(), + 'rle_deleted' => 0, + ], + [ + 'rle_id' => $row->rle_id, + ] + ); + } else { throw new ReadingListRepositoryException( 'readinglists-db-error-duplicate-page' ); } + $insertId = $row ? (int)$row->rle_id : $this->dbw->insertId(); $this->logger->info( 'Added entry {entry} for user {user}', [ - 'entry' => $this->dbw->insertId(), + 'entry' => $insertId, 'user' => $this->userId, + 'recreated' => (bool)$row, ] ); - return $this->dbw->insertId(); + return $insertId; } /** diff --git a/tests/src/ReadingListRepositoryTest.php b/tests/src/ReadingListRepositoryTest.php index 4c21b4f..dfb3071 100644 --- a/tests/src/ReadingListRepositoryTest.php +++ b/tests/src/ReadingListRepositoryTest.php @@ -390,6 +390,18 @@ $this->assertTimestampEquals( wfTimestampNow(), $row->rle_date_updated ); $this->assertEquals( 0, $row->rle_deleted ); + // test that deletion + recreation does not trip the unique contstraint + $repository->deleteListEntry( $entryId ); + $entryId2 = $repository->addListEntry( $listId, 'en.wikipedia.org', 'Foo' ); + /** @var ReadingListEntryRow $row */ + $row = $this->db->selectRow( 'reading_list_entry', '*', [ 'rle_id' => $entryId2 ] ); + $this->assertEquals( 1, $row->rle_user_id ); + $this->assertEquals( 'en.wikipedia.org', $row->rle_project ); + $this->assertEquals( 'Foo', $row->rle_title ); + $this->assertTimestampEquals( wfTimestampNow(), $row->rle_date_created ); + $this->assertTimestampEquals( wfTimestampNow(), $row->rle_date_updated ); + $this->assertEquals( 0, $row->rle_deleted ); + $this->assertFailsWith( 'readinglists-db-error-no-such-list', function () use ( $repository ) { $repository->addListEntry( 123, 'en.wikipedia.org', 'A' ); -- To view, visit https://gerrit.wikimedia.org/r/386731 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iddbc8af582c93aea182eb356745e34025b67ccae Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/ReadingLists Gerrit-Branch: master Gerrit-Owner: Gergő Tisza <gti...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits