[Notifications] cron cleanup (#3580212) · Issues · project / canvas

[Notifications] cron cleanup

>>> [!note] Migrated issue   Reported by: [justafish](https://www.drupal.org/user/161058) Related to !830 >>> <p>Implement a cron hook that performs two distinct cleanup operations:</p> <ol> <li><strong>Purge stale processing notifications:</strong> Processing notifications that have been active for more than 30 minutes are deleted and replaced with error notifications so the user is informed that the operation timed out. This only affects <code>processing</code>-type notifications.</li> <li><strong>Delete expired notifications:</strong> All notifications (of any type) and their associated read entries older than 30 days are removed. Notifications are deleted first; read entries are only deleted after that succeeds.</li> </ol> <h2>Dependencies</h2> <p>Ticket 1 (notification storage and service) must be complete.</p> <h2>Acceptance criteria</h2> <h3>Purge stale processing (<code>purgeStaleProcessing()</code>)</h3> <ul> <li>Processing notifications older than 30 minutes are deleted.</li> <li>For each purged processing notification, an error notification is created with the same key, a title of "Operation timed out", and the original notification's message.</li> <li>Processing notifications younger than 30 minutes are not affected.</li> <li>Non-processing notifications are never affected by the purge.</li> </ul> <h3>Delete expired (<code>deleteExpired()</code>)</h3> <ul> <li>All notifications of any type older than 30 days are deleted.</li> <li>Read entries older than 30 days are deleted.</li> <li>Notifications are deleted first; read entries are only deleted after the notification delete succeeds.</li> <li>A single pre-computed cutoff timestamp is used for both deletes to ensure consistency.</li> </ul> <h3>Cron hook</h3> <ul> <li><code>hook_cron</code> is implemented in the Canvas module (via a Hook class).</li> <li>The cron hook calls <code>purgeStaleProcessing()</code> first, then <code>deleteExpired()</code>.</li> <li>Kernel tests cover all purge and cleanup scenarios.</li> </ul> <h2>Implementation plan</h2> <h3>Hook class</h3> <p>Create <code>src/Hook/NotificationCronHook.php</code>:</p> <pre><pre>&lt;?php<br><br>declare(strict_types=1);<br><br>namespace Drupal\canvas\Hook;<br><br>use Drupal\canvas\Notification\NotificationService;<br>use Drupal\Core\Hook\Attribute\Hook;<br><br>final class NotificationCronHook {<br><br>  public function __construct(<br>    private readonly NotificationService $notificationService,<br>  ) {}<br><br>  #[Hook('cron')]<br>  public function __invoke(): void {<br>    // Purge stale processing notifications (&gt; 30 min), replacing each with an<br>    // error notification.<br>    $this->notificationService->purgeStaleProcessing();<br>    // Delete all expired notifications and read entries (&gt; 30 days).<br>    $this->notificationService->deleteExpired();<br>  }<br>}</pre></pre><h3><code>purgeStaleProcessing()</code> — processing notifications only</h3> <p>This method targets only <code>type = 'processing'</code> notifications. It uses the <code>idx_type_timestamp</code> index for efficient queries.</p> <p>Select stale processing notifications:</p> <pre><pre>SELECT * FROM canvas_notifications<br>WHERE type = 'processing' AND timestamp &lt; :threshold</pre></pre><p>For each stale notification, call <code>$this->create()</code> with:</p> <ul> <li><code>type</code> = <code>'error'</code></li> <li><code>key</code> = original notification's <code>notification_key</code></li> <li><code>title</code> = <code>'Operation timed out'</code></li> <li><code>message</code> = original notification's <code>message</code></li> </ul> <p>Delete all stale processing notifications in a single query:</p> <pre><pre>DELETE FROM canvas_notifications<br>WHERE type = 'processing' AND timestamp &lt; :threshold</pre></pre><p>Non-processing notifications are never touched by this method.</p> <h3><code>deleteExpired()</code> — all notification types</h3> <p>Removes all data older than 30 days regardless of type. Notifications are deleted first; read entries are only deleted after that succeeds, to prevent read entries from being removed while the notifications they reference still exist. Both deletes use the same pre-computed cutoff to ensure consistency.</p> <pre><pre>public function deleteExpired(): void {<br>  $cutoff = (int) (microtime(true) * 1000) - self::RETENTION_MS;<br>  $this->database->delete(self::TABLE_NOTIFICATIONS)<br>    ->condition('timestamp', $cutoff, '&lt;')<br>    ->execute();<br>  $this->database->delete(self::TABLE_READS)<br>    ->condition('timestamp', $cutoff, '&lt;')<br>    ->execute();<br>}</pre></pre><p>Both queries use the <code>idx_timestamp</code> index.</p> <h3>Constants</h3> <p>Both are class constants on <code>NotificationService</code>, not configuration values. They can be adjusted later if needed.</p> <ul> <li><code>PROCESSING_TIMEOUT_MS = 1800000</code> (30 minutes in milliseconds)</li> <li><code>RETENTION_MS = 2592000000</code> (30 days in milliseconds)</li> </ul> <h2>Tests required</h2> <p>Kernel test: <code>tests/src/Kernel/Notification/NotificationCronHookTest.php</code></p> <h3>Purge stale processing tests</h3> <table> <thead> <tr> <th>Test method</th> <th>Validates</th> </tr> </thead> <tbody> <tr> <td><code>testPurgesStaleProcessingNotifications</code></td> <td>Processing notification > 30 min old is deleted</td> </tr> <tr> <td><code>testCreatesErrorForPurgedProcessing</code></td> <td>Error row created with same key and message</td> </tr> <tr> <td><code>testLeavesRecentProcessingAlone</code></td> <td>Processing notification < 30 min old is untouched</td> </tr> <tr> <td><code>testPurgeDoesNotAffectNonProcessingTypes</code></td> <td><code>info</code>, <code>warning</code>, <code>error</code>, and <code>success</code> notifications are untouched by purge</td> </tr> <tr> <td><code>testPurgeHandlesMultipleStaleNotifications</code></td> <td>Multiple stale processing entries are all purged and replaced</td> </tr> </tbody> </table> <h3>Delete expired tests</h3> <table> <thead> <tr> <th>Test method</th> <th>Validates</th> </tr> </thead> <tbody> <tr> <td><code>testDeletesExpiredNotificationsOfAllTypes</code></td> <td>Notifications of every type older than 30 days are removed</td> </tr> <tr> <td><code>testDeletesExpiredReadEntries</code></td> <td>Read entries older than 30 days are removed</td> </tr> <tr> <td><code>testDeleteExpiredNotificationsBeforeReads</code></td> <td>Notifications are deleted before read entries</td> </tr> <tr> <td><code>testDeleteExpiredUsesConsistentCutoff</code></td> <td>Both deletes use the same cutoff timestamp</td> </tr> <tr> <td><code>testDeleteExpiredLeavesRecentNotifications</code></td> <td>Notifications younger than 30 days are untouched</td> </tr> </tbody> </table> <h2>Manual testing steps</h2> <h3>Purge stale processing</h3> <ol> <li> <p>Create a processing notification with a timestamp 31 minutes in the past:</p> <pre><pre>ddev drush php:eval "<br>  \$service = \Drupal::service('Drupal\canvas\Notification\NotificationService');<br>  \$service->create([<br>    'type' => 'processing',<br>    'key' => 'test_sync',<br>    'title' => 'Test sync in progress',<br>    'message' => 'Syncing test data...',<br>    'timestamp' => (int)(microtime(true) * 1000) - 1860000,<br>  ]);<br>"</pre></pre></li> <li> <p>Verify the processing notification exists:</p> <pre>ddev drush sqlq "SELECT id, type, notification_key, title FROM canvas_notifications;"</pre></li> <li> <p>Run cron:</p> <pre>ddev drush cron</pre></li> <li> <p>Verify the processing notification was deleted:</p> <pre>ddev drush sqlq "SELECT COUNT(*) FROM canvas_notifications WHERE type = 'processing' AND notification_key = 'test_sync';"</pre><p>Expected: <code>0</code>.</p> </li> <li> <p>Verify the replacement error notification was created:</p> <pre>ddev drush sqlq "SELECT type, notification_key, title FROM canvas_notifications WHERE notification_key = 'test_sync';"</pre><p>Expected: one row with <code>type = error</code>, <code>notification_key = test_sync</code>, <code>title = Operation timed out</code>.</p> </li> <li>Create a processing notification with a recent timestamp (within 30 minutes). Run cron again. Verify it was NOT deleted. Expected: <code>1</code>.</li> <li>Create a non-processing notification (e.g. <code>info</code>). Run cron. Verify it remains. Expected: <code>1</code>.</li> </ol> <h3>Delete expired</h3> <ol> <li> <p>Insert a notification with a timestamp 31 days in the past:</p> <pre><pre>ddev drush php:eval "<br>  \$service = \Drupal::service('Drupal\canvas\Notification\NotificationService');<br>  \$service->create([<br>    'type' => 'info',<br>    'title' => 'Old notification',<br>    'message' => 'This should be cleaned up.',<br>    'timestamp' => (int)(microtime(true) * 1000) - 2678400000,<br>  ]);<br>"</pre></pre></li> <li> <p>Run cron. Verify the old notification was deleted:</p> <pre>ddev drush sqlq "SELECT COUNT(*) FROM canvas_notifications WHERE title = 'Old notification';"</pre><p>Expected: <code>0</code>.</p> </li> <li> <p>Verify that recent notifications (< 30 days old) still exist:</p> <pre>ddev drush sqlq "SELECT COUNT(*) FROM canvas_notifications;"</pre><p>Expected: matches however many recent notifications you created in earlier steps.</p> </li> </ol> > Related issue: [Issue #3573776](https://www.drupal.org/node/3573776)

issue