mirror/api
1
0
Fork 0
mirror of https://github.com/unraid/api.git synced 2026-01-02 14:40:01 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix: race condition when updating notification types

Browse Source
This commit is contained in:
Pujit Mehrotra
2024-09-24 19:12:34 -04:00
parent 48aca094f5
commit acc847ef1d
1 changed files with 34 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts
View File

@@ -191,6 +191,21 @@ export class NotificationsService {
const unreadPath = join(UNREAD, id);
const archivePath = join(ARCHIVE, id);
/**-----------------------
* Why we use a snapshot
*
* An implicit update to `overview` creates a race condition:
* it might be missing changes from the 'add' event (i.e. incrementing the notification's new category).
*
* So, we use & modify a snapshot of the overview to make sure we're returning accurate
* data to the client.
*------------------------**/
const archiveSnapshot = structuredClone(NotificationsService.overview.archive);
// We expect to only archive 'unread' notifications, but it's possible that the notification
// has already been archived or deleted (e.g. retry logic, spike in network latency).
if (!(await fileExists(unreadPath))) {
this.logger.warn(`[archiveNotification] Could not find notification in unreads: ${id}`);
return NotificationsService.overview;
@@ -199,6 +214,8 @@ export class NotificationsService {
const notification = await this.loadNotificationFile(unreadPath, NotificationType.UNREAD);
await rename(unreadPath, archivePath);
await this.removeFromOverview(notification);
archiveSnapshot.total += 1;
archiveSnapshot[notification.importance.toLowerCase()] += 1;
/**-----------------------
* Event & PubSub logic
@@ -222,14 +239,21 @@ export class NotificationsService {
* to track other stats, we have to update them manually, prior to file deletion.
*------------------------**/
return NotificationsService.overview;
return {
...NotificationsService.overview,
archive: archiveSnapshot,
};
}
public async markAsUnread({ id }: Pick<Notification, 'id'>) {
public async markAsUnread({ id }: Pick<Notification, 'id'>): Promise<NotificationOverview> {
const { UNREAD, ARCHIVE } = this.paths();
const unreadPath = join(UNREAD, id);
const archivePath = join(ARCHIVE, id);
// see `archiveNotification` for why we use a snapshot
// it's b/c of a race condition
const unreadSnapshot = structuredClone(NotificationsService.overview.unread);
if (!(await fileExists(archivePath))) {
this.logger.warn(`[markAsUnread] Could not find notification in archive: ${id}`);
return NotificationsService.overview;
@@ -238,14 +262,16 @@ export class NotificationsService {
const notification = await this.loadNotificationFile(archivePath, NotificationType.ARCHIVE);
await rename(archivePath, unreadPath);
// see `archiveNotification` for why there are 2 ways of updating our overview state,
// and the implications it has for updating notifications.
await this.removeFromOverview(notification);
unreadSnapshot.total += 1;
unreadSnapshot[notification.importance.toLowerCase()] += 1;
// @see `archiveNotification` for why this is commented out, and why there are 2
// different ways of updating the overview & pubsub
//
// await this.addToOverview({ ...notification, type: NotificationType.UNREAD });
return NotificationsService.overview;
return {
...NotificationsService.overview,
unread: unreadSnapshot,
};
}
/**------------------------------------------------------------------------