diff --git a/db_patches/patch-drop-notification_bundle_base.sql b/db_patches/patch-drop-notification_bundle_base.sql new file mode 100644 index 000000000..0da891c3f --- /dev/null +++ b/db_patches/patch-drop-notification_bundle_base.sql @@ -0,0 +1,5 @@ +-- Drop unused field notification_bundle_base and the indexes that contain it +DROP INDEX /*i*/echo_notification_user_base_read_timestamp ON /*_*/echo_notification; +DROP INDEX /*i*/echo_notification_user_base_timestamp ON /*_*/echo_notification; +DROP INDEX /*i*/echo_notification_user_hash_base_timestamp ON /*_*/echo_notification; +ALTER TABLE /*_*/echo_notification DROP notification_bundle_base; diff --git a/echo.sql b/echo.sql index d69364989..ae65f7e7d 100644 --- a/echo.sql +++ b/echo.sql @@ -41,8 +41,6 @@ CREATE TABLE /*_*/echo_notification ( notification_timestamp binary(14) not null, -- Timestamp when the user read the notification, or null if unread notification_read_timestamp binary(14) null, - -- No longer used, should be removed (T143763) - notification_bundle_base boolean not null default 1, -- Hash for bundling together similar notifications. Notifications that can be bundled together -- will have the same hash notification_bundle_hash varchar(32) binary not null, @@ -53,14 +51,8 @@ CREATE TABLE /*_*/echo_notification ( -- Index to get a user's notifications in chronological order CREATE INDEX /*i*/echo_user_timestamp ON /*_*/echo_notification (notification_user,notification_timestamp); --- No longer used, should be removed -CREATE INDEX /*i*/echo_notification_user_base_read_timestamp ON /*_*/echo_notification (notification_user, notification_bundle_base, notification_read_timestamp); --- No longer used, should be removed -CREATE INDEX /*i*/echo_notification_user_base_timestamp ON /*_*/echo_notification (notification_user, notification_bundle_base, notification_timestamp, notification_event); -- Used by NotificationMapper::fetchNewestByUserBundleHash() CREATE INDEX /*i*/echo_notification_user_hash_timestamp ON /*_*/echo_notification (notification_user, notification_bundle_hash, notification_timestamp); --- No longer used, should be removed -CREATE INDEX /*i*/echo_notification_user_hash_base_timestamp ON /*_*/echo_notification (notification_user, notification_bundle_display_hash, notification_bundle_base, notification_timestamp); -- Used to get all notifications for a given event CREATE INDEX /*i*/echo_notification_event ON /*_*/echo_notification (notification_event); -- Used to get read/unread notifications for a user diff --git a/extension.json b/extension.json index e7c6df3e1..97b979961 100644 --- a/extension.json +++ b/extension.json @@ -917,7 +917,6 @@ "ApiEchoNotifications": "includes/api/ApiEchoNotifications.php", "ApiEchoNotificationsTest": "tests/phpunit/api/ApiEchoNotificationsTest.php", "ApiEchoUnreadNotificationPages": "includes/api/ApiEchoUnreadNotificationPages.php", - "BackfillReadBundles": "maintenance/backfillReadBundles.php", "BackfillUnreadWikis": "maintenance/backfillUnreadWikis.php", "Bundleable": "includes/Bundleable.php", "Bundler": "includes/Bundler.php", diff --git a/includes/EchoHooks.php b/includes/EchoHooks.php index e2a21268c..43e2a2f05 100644 --- a/includes/EchoHooks.php +++ b/includes/EchoHooks.php @@ -192,7 +192,7 @@ class EchoHooks { "$dir/db_patches/patch-drop-echo_target_page-etp_user.sql" ); } - $updater->addExtensionField( 'echo_notification', 'notification_bundle_base', + $updater->addExtensionField( 'echo_notification', 'notification_bundle_hash', "$dir/db_patches/patch-notification-bundling-field.sql" ); // This index was renamed twice, first from type_page to event_type and // later from event_type to echo_event_type @@ -202,8 +202,10 @@ class EchoHooks { } $updater->dropExtensionTable( 'echo_subscription', "$dir/db_patches/patch-drop-echo_subscription.sql" ); - $updater->dropExtensionField( 'echo_event', 'event_timestamp', - "$dir/db_patches/patch-drop-echo_event-event_timestamp.sql" ); + if ( $updater->getDB()->getType() !== 'sqlite' ) { + $updater->dropExtensionField( 'echo_event', 'event_timestamp', + "$dir/db_patches/patch-drop-echo_event-event_timestamp.sql" ); + } $updater->addExtensionField( 'echo_email_batch', 'eeb_event_hash', "$dir/db_patches/patch-email_batch-new-field.sql" ); $updater->addExtensionField( 'echo_event', 'event_page_id', @@ -233,6 +235,10 @@ class EchoHooks { "$dir/db_patches/patch-drop-echo_event-event_page_namespace.sql" ); $updater->dropExtensionField( 'echo_event', 'event_page_title', "$dir/db_patches/patch-drop-echo_event-event_page_title.sql" ); + if ( $updater->getDB()->getType() !== 'sqlite' ) { + $updater->dropExtensionField( 'echo_notification', 'notification_bundle_base', + "$dir/db_patches/patch-drop-notification_bundle_base.sql" ); + } } /** diff --git a/includes/api/ApiEchoNotifications.php b/includes/api/ApiEchoNotifications.php index ae17a127d..f5ca9c700 100644 --- a/includes/api/ApiEchoNotifications.php +++ b/includes/api/ApiEchoNotifications.php @@ -454,7 +454,6 @@ class ApiEchoNotifications extends ApiQueryBase { $row->notification_user = $user->getId(); $row->notification_timestamp = $maxTimestamp; $row->notification_read_timestamp = null; - $row->notification_bundle_base = 1; $row->notification_bundle_hash = md5( 'bogus' ); $row->notification_bundle_display_hash = md5( 'also-bogus' ); diff --git a/includes/mapper/NotificationMapper.php b/includes/mapper/NotificationMapper.php index a70b906f4..683a78b95 100644 --- a/includes/mapper/NotificationMapper.php +++ b/includes/mapper/NotificationMapper.php @@ -23,24 +23,6 @@ class EchoNotificationMapper extends EchoAbstractMapper { $dbw, __METHOD__, function ( IDatabase $dbw, $fname ) use ( $row, $listeners ) { - // Reset the bundle base if this notification has a display hash - // the result of this operation is that all previous notifications - // with the same display hash are set to non-base because new record - // is becoming the bundle base - if ( $row['notification_bundle_display_hash'] ) { - $dbw->update( - 'echo_notification', - [ 'notification_bundle_base' => 0 ], - [ - 'notification_user' => $row['notification_user'], - 'notification_bundle_display_hash' => - $row['notification_bundle_display_hash'], - 'notification_bundle_base' => 1 - ], - $fname - ); - } - $row['notification_timestamp'] = $dbw->timestamp( $row['notification_timestamp'] ); $dbw->insert( 'echo_notification', $row, $fname ); diff --git a/includes/model/Notification.php b/includes/model/Notification.php index 43e1e34f0..3b252e78c 100644 --- a/includes/model/Notification.php +++ b/includes/model/Notification.php @@ -30,13 +30,6 @@ class EchoNotification extends EchoAbstractEntity implements Bundleable { */ protected $readTimestamp; - /** - * Determine whether this is a bundle base. Default is 1, - * which means it's a bundle base - * @var int - */ - protected $bundleBase = 1; - /** * The hash used to determine if a set of event could be bundled * @var string @@ -176,7 +169,6 @@ class EchoNotification extends EchoAbstractEntity implements Bundleable { if ( $row->notification_read_timestamp ) { $notification->readTimestamp = wfTimestamp( TS_MW, $row->notification_read_timestamp ); } - $notification->bundleBase = $row->notification_bundle_base; $notification->bundleHash = $row->notification_bundle_hash; $notification->bundleDisplayHash = $row->notification_bundle_display_hash; @@ -193,7 +185,6 @@ class EchoNotification extends EchoAbstractEntity implements Bundleable { 'notification_user' => $this->user->getId(), 'notification_timestamp' => $this->timestamp, 'notification_read_timestamp' => $this->readTimestamp, - 'notification_bundle_base' => $this->bundleBase, 'notification_bundle_hash' => $this->bundleHash, 'notification_bundle_display_hash' => $this->bundleDisplayHash ]; @@ -235,14 +226,6 @@ class EchoNotification extends EchoAbstractEntity implements Bundleable { return $this->getReadTimestamp() !== null; } - /** - * Getter method - * @return int Notification bundle base - */ - public function getBundleBase() { - return $this->bundleBase; - } - /** * Getter method * @return string|null Notification bundle hash @@ -316,7 +299,6 @@ class EchoNotification extends EchoAbstractEntity implements Bundleable { 'notification_user', 'notification_timestamp', 'notification_read_timestamp', - 'notification_bundle_base', 'notification_bundle_hash', 'notification_bundle_display_hash', ] ); diff --git a/maintenance/backfillReadBundles.php b/maintenance/backfillReadBundles.php deleted file mode 100644 index 04be37971..000000000 --- a/maintenance/backfillReadBundles.php +++ /dev/null @@ -1,76 +0,0 @@ -mDescription = "Backfill echo_notification.notification_read_timestamp for bundles"; - - $this->setBatchSize( 300 ); - - $this->requireExtension( 'Echo' ); - } - - public function execute() { - $dbFactory = MWEchoDbFactory::newFromDefault(); - $dbw = $dbFactory->getEchoDb( DB_MASTER ); - $dbr = $dbFactory->getEchoDb( DB_REPLICA ); - $iterator = new BatchRowIterator( - $dbr, - 'echo_notification', - [ 'notification_user', 'notification_event' ], - $this->mBatchSize - ); - $iterator->setFetchColumns( [ 'notification_bundle_display_hash', 'notification_read_timestamp' ] ); - - $unreadNonBase = $dbr->selectSQLText( - 'echo_notification', - 'notification_bundle_display_hash', - [ - 'notification_bundle_base' => 0, - 'notification_read_timestamp IS NULL', - "notification_bundle_display_hash <> ''", - ] - ); - - $iterator->addConditions( [ - 'notification_bundle_base' => 1, - 'notification_read_timestamp IS NOT NULL', - "notification_bundle_display_hash IN ( $unreadNonBase )", - ] ); - - $processed = 0; - foreach ( $iterator as $batch ) { - foreach ( $batch as $row ) { - $userId = $row->notification_user; - $displayHash = $row->notification_bundle_display_hash; - $readTimestamp = $row->notification_read_timestamp; - - $dbw->update( - 'echo_notification', - [ 'notification_read_timestamp' => $readTimestamp ], - [ - 'notification_user' => $userId, - 'notification_bundle_display_hash' => $displayHash, - 'notification_bundle_base' => 0, - 'notification_read_timestamp IS NULL', - ] - ); - - $processed += $dbw->affectedRows(); - } - - $this->output( "Updated $processed notifications.\n" ); - $dbFactory->waitForSlaves(); - } - } -} - -$maintClass = BackfillReadBundles::class; -require_once RUN_MAINTENANCE_IF_MAIN; diff --git a/tests/phpunit/mapper/NotificationMapperTest.php b/tests/phpunit/mapper/NotificationMapperTest.php index e12fae09a..e9fe6a787 100644 --- a/tests/phpunit/mapper/NotificationMapperTest.php +++ b/tests/phpunit/mapper/NotificationMapperTest.php @@ -31,7 +31,6 @@ class EchoNotificationMapperTest extends MediaWikiTestCase { 'notification_user' => 1, 'notification_timestamp' => '20140615101010', 'notification_read_timestamp' => '', - 'notification_bundle_base' => 1, 'notification_bundle_hash' => 'testhash', 'notification_bundle_display_hash' => 'testdisplayhash' ] @@ -69,7 +68,6 @@ class EchoNotificationMapperTest extends MediaWikiTestCase { 'notification_user' => 1, 'notification_timestamp' => '20140615101010', 'notification_read_timestamp' => '20140616101010', - 'notification_bundle_base' => 1, 'notification_bundle_hash' => 'testhash', 'notification_bundle_display_hash' => 'testdisplayhash' ] @@ -123,7 +121,6 @@ class EchoNotificationMapperTest extends MediaWikiTestCase { 'notification_user' => 1, 'notification_timestamp' => '20140615101010', 'notification_read_timestamp' => '20140616101010', - 'notification_bundle_base' => 1, 'notification_bundle_hash' => 'testhash', 'notification_bundle_display_hash' => 'testdisplayhash' ]; @@ -151,7 +148,6 @@ class EchoNotificationMapperTest extends MediaWikiTestCase { 'notification_user' => 1, 'notification_timestamp' => '20140615101010', 'notification_read_timestamp' => '20140616101010', - 'notification_bundle_base' => 1, 'notification_bundle_hash' => 'testhash', 'notification_bundle_display_hash' => 'testdisplayhash' ]; diff --git a/tests/phpunit/model/NotificationTest.php b/tests/phpunit/model/NotificationTest.php index 6546fcd70..264f96c35 100644 --- a/tests/phpunit/model/NotificationTest.php +++ b/tests/phpunit/model/NotificationTest.php @@ -54,7 +54,6 @@ class EchoNotificationTest extends MediaWikiTestCase { 'notification_event' => 1, 'notification_timestamp' => time(), 'notification_read_timestamp' => '', - 'notification_bundle_base' => 1, 'notification_bundle_hash' => 'testhash', 'notification_bundle_display_hash' => 'testdisplayhash' ];