Proactively delete echo_event rows when they become orphaned

When deleting echo_notification or echo_email_batch rows, also delete
the corresponding echo_event rows if no other echo_notification rows
or echo_email_batch rows refer to them.

Bug: T221262
Change-Id: I416ff107bd000a0cfac102408c993f8bec2d0286

includes/EmailBatch.php

@@ -1,5 +1,6 @@
 <?php
 
+use MediaWiki\MediaWikiServices;
 use Wikimedia\Rdbms\IResultWrapper;
 
 /**
@@ -237,19 +238,38 @@ class MWEchoEmailBatch {
 	 * Clear "processed" events in the queue, processed could be: email sent, invalid, users do not want to receive emails
 	 */
 	public function clearProcessedEvent() {
-		$conds = [ 'eeb_user_id' => $this->mUser->getId() ];
+		global $wgUpdateRowsPerQuery;
+		$eventMapper = new EchoEventMapper();
+		$dbFactory = MWEchoDbFactory::newFromDefault();
+		$dbw = $dbFactory->getEchoDb( DB_MASTER );
+		$dbr = $dbFactory->getEchoDb( DB_REPLICA );
+		$lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
+		$ticket = $lbFactory->getEmptyTransactionTicket( __METHOD__ );
+		$domainId = $dbw->getDomainId();
 
-		// there is a processed cutoff point
+		$iterator = new BatchRowIterator( $dbr, 'echo_email_batch', 'eeb_event_id', $wgUpdateRowsPerQuery );
+		$iterator->addConditions( [ 'eeb_user_id' => $this->mUser->getId() ] );
 		if ( $this->lastEvent ) {
-			$conds[] = 'eeb_event_id <= ' . (int)$this->lastEvent;
+			// There is a processed cutoff point
+			$iterator->addConditions( [ 'eeb_event_id <= ' . (int)$this->lastEvent ] );
 		}
+		foreach ( $iterator as $batch ) {
+			$eventIds = [];
+			foreach ( $batch as $row ) {
+				$eventIds[] = $row->eeb_event_id;
+			}
+			$dbw->delete( 'echo_email_batch', [
+				'eeb_user_id' => $this->mUser->getId(),
+				'eeb_event_id' => $eventIds
+			] );
 
-		$dbw = MWEchoDbFactory::newFromDefault()->getEchoDb( DB_MASTER );
-		$dbw->delete(
-			'echo_email_batch',
-			$conds,
-			__METHOD__
-		);
+			// Find out which events are now orphaned, i.e. no longer referenced in echo_email_batch
+			// (besides the rows we just deleted) or in echo_notification, and delete them
+			$eventMapper->deleteOrphanedEvents( $eventIds, $this->mUser->getId(), 'echo_email_batch' );
+
+			$lbFactory->commitAndWaitForReplication(
+				__METHOD__, $ticket, [ 'domain' => $domainId ] );
+		}
 	}
 
 	/**

includes/mapper/EventMapper.php

@@ -181,4 +181,59 @@ class EchoEventMapper extends EchoAbstractMapper {
 		return $data;
 	}
 
+	/**
+	 * Find out which of the given event IDs are orphaned, and delete them.
+	 *
+	 * An event is orphaned if it is not referred to by any rows in the echo_notification or
+	 * echo_email_batch tables. If $ignoreUserId is set, rows for that user are not considered when
+	 * determining orphanhood; if $ignoreUserTable is set, this only applies to that table.
+	 * Use this when you've just recently deleted rows related to this user on the master, so that
+	 * this function won't refuse to delete recently-orphaned events because it still sees the
+	 * recently-deleted rows on the replica.
+	 *
+	 * @param array $eventIds Event IDs to check to see if they have become orphaned
+	 * @param int|null $ignoreUserId Allow events to be deleted if the only referring rows
+	 *  have this user ID
+	 * @param string|null $ignoreUserTable Restrict $ignoreUserId to this table only
+	 *  ('echo_notification' or 'echo_email_batch')
+	 */
+	public function deleteOrphanedEvents( array $eventIds, $ignoreUserId = null, $ignoreUserTable = null ) {
+		$dbw = $this->dbFactory->getEchoDb( DB_MASTER );
+		$dbr = $this->dbFactory->getEchoDb( DB_REPLICA );
+
+		$notifJoinConds = [];
+		$emailJoinConds = [];
+		if ( $ignoreUserId !== null ) {
+			if ( $ignoreUserTable === null || $ignoreUserTable === 'echo_notification' ) {
+				$notifJoinConds[] = 'notification_user != ' . $dbr->addQuotes( $ignoreUserId );
+			}
+			if ( $ignoreUserTable === null || $ignoreUserTable === 'echo_email_batch' ) {
+				$emailJoinConds[] = 'eeb_user_id != ' . $dbr->addQuotes( $ignoreUserId );
+			}
+		}
+		$orphanedEventIds = $dbr->selectFieldValues(
+			[ 'echo_event', 'echo_notification', 'echo_email_batch' ],
+			'event_id',
+			[
+				'event_id' => $eventIds,
+				'notification_timestamp' => null,
+				'eeb_user_id' => null
+			],
+			__METHOD__,
+			[],
+			[
+				'echo_notification' => [ 'LEFT JOIN', array_merge( [
+					'notification_event=event_id'
+				], $notifJoinConds ) ],
+				'echo_email_batch' => [ 'LEFT JOIN', array_merge( [
+					'eeb_event_id=event_id'
+				], $emailJoinConds ) ]
+			]
+		);
+		if ( $orphanedEventIds ) {
+			$dbw->delete( 'echo_event', [ 'event_id' => $orphanedEventIds ] );
+			$dbw->delete( 'echo_target_page', [ 'etp_event' => $orphanedEventIds ] );
+		}
+	}
+
 }

includes/mapper/NotificationMapper.php

@@ -334,33 +334,43 @@ class EchoNotificationMapper extends EchoAbstractMapper {
 	 */
 	public function deleteByUserEventOffset( User $user, $eventId ) {
 		global $wgUpdateRowsPerQuery;
+		$eventMapper = new EchoEventMapper( $this->dbFactory );
 		$userId = $user->getId();
 		$dbw = $this->dbFactory->getEchoDb( DB_MASTER );
+		$dbr = $this->dbFactory->getEchoDb( DB_REPLICA );
 		$lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
 		$ticket = $lbFactory->getEmptyTransactionTicket( __METHOD__ );
 		$domainId = $dbw->getDomainId();
 
-		$idsToDelete = $dbw->selectFieldValues(
+		$iterator = new BatchRowIterator(
+			$dbr,
 			'echo_notification',
 			'notification_event',
-			[
-				'notification_user' => $userId,
-				'notification_event < ' . (int)$eventId
-			],
-			__METHOD__
+			$wgUpdateRowsPerQuery
 		);
-		if ( !$idsToDelete ) {
-			return true;
-		}
-		foreach ( array_chunk( $idsToDelete, $wgUpdateRowsPerQuery ) as $batch ) {
+		$iterator->addConditions( [
+			'notification_user' => $userId,
+			'notification_event < ' . (int)$eventId
+		] );
+
+		foreach ( $iterator as $batch ) {
+			$eventIds = [];
+			foreach ( $batch as $row ) {
+				$eventIds[] = $row->notification_event;
+			}
 			$dbw->delete(
 				'echo_notification',
 				[
 					'notification_user' => $userId,
-					'notification_event' => $batch,
+					'notification_event' => $eventIds,
 				],
 				__METHOD__
 			);
+
+			// Find out which events are now orphaned, i.e. no longer referenced in echo_notifications
+			// (besides the rows we just deleted) or in echo_email_batch, and delete them
+			$eventMapper->deleteOrphanedEvents( $eventIds, $userId, 'echo_notification' );
+
 			$lbFactory->commitAndWaitForReplication(
 				__METHOD__, $ticket, [ 'domain' => $domainId ] );
 		}

tests/phpunit/mapper/NotificationMapperTest.php

@@ -129,10 +129,27 @@ class EchoNotificationMapperTest extends MediaWikiTestCase {
 	public function testDeleteByUserEventOffset() {
 		$this->setMwGlobals( [ 'wgUpdateRowsPerQuery' => 4 ] );
 		$mockDb = $this->getMock( IDatabase::class );
-		$mockDb->expects( $this->any() )
-			->method( 'selectFieldValues' )
-			->will( $this->returnValue( [ 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 ] ) );
+		$makeResultRows = function ( $eventIds ) {
+			return new ArrayIterator( array_map( function ( $eventId ) {
+				return (object)[ 'notification_event' => $eventId ];
+			}, $eventIds ) );
+		};
+		$mockDb->expects( $this->exactly( 4 ) )
+			->method( 'select' )
+			->willReturnOnConsecutiveCalls(
+				$this->returnValue( $makeResultRows( [ 1, 2, 3, 5 ] ) ),
+				$this->returnValue( $makeResultRows( [ 8, 13, 21, 34 ] ) ),
+				$this->returnValue( $makeResultRows( [ 55, 89 ] ) ),
+				$this->returnValue( $makeResultRows( [] ) )
+			);
 		$mockDb->expects( $this->exactly( 3 ) )
+			->method( 'selectFieldValues' )
+			->willReturnOnConsecutiveCalls(
+				$this->returnValue( [] ),
+				$this->returnValue( [ 13, 21 ] ),
+				$this->returnValue( [ 55 ] )
+			);
+		$mockDb->expects( $this->exactly( 7 ) )
 			->method( 'delete' )
 			->withConsecutive(
 				[
@@ -145,10 +162,30 @@ class EchoNotificationMapperTest extends MediaWikiTestCase {
 					$this->equalTo( [ 'notification_user' => 1, 'notification_event' => [ 8, 13, 21, 34 ] ] ),
 					$this->anything()
 				],
+				[
+					$this->equalTo( 'echo_event' ),
+					$this->equalTo( [ 'event_id' => [ 13, 21 ] ] ),
+					$this->anything()
+				],
+				[
+					$this->equalTo( 'echo_target_page' ),
+					$this->equalTo( [ 'etp_event' => [ 13, 21 ] ] ),
+					$this->anything()
+				],
 				[
 					$this->equalTo( 'echo_notification' ),
 					$this->equalTo( [ 'notification_user' => 1, 'notification_event' => [ 55, 89 ] ] ),
 					$this->anything()
+				],
+				[
+					$this->equalTo( 'echo_event' ),
+					$this->equalTo( [ 'event_id' => [ 55 ] ] ),
+					$this->anything()
+				],
+				[
+					$this->equalTo( 'echo_target_page' ),
+					$this->equalTo( [ 'etp_event' => [ 55 ] ] ),
+					$this->anything()
 				]
 			)
 			->willReturn( true );