From 897021eaccf930e86e8ddca33ad83a68eda867c7 Mon Sep 17 00:00:00 2001 From: Ajit Mehrotra Date: Wed, 3 Dec 2025 17:27:25 -0500 Subject: [PATCH] fix(notifications api): implement async generator for improve filtering logic Problem this solution addresses: Basically, when users filtered by alert, warning, or info, results were being paginated first, then filtered by the requested importance, so filtered notifications were not working properly in some (a lot) of cases. - added a new async generator method to load notifications in batches, enhancing performance and error handling. - refactored the notification loading logic to utilize the generator, improving readability and maintainability. - updated filtering logic to streamline the process of matching notifications based on importance and type. --- .../notifications/notifications.service.ts | 69 ++++++++++++++----- 1 file changed, 52 insertions(+), 17 deletions(-) diff --git a/api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts b/api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts index 8daa13a98..bf2dd022d 100644 --- a/api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts +++ b/api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts @@ -682,6 +682,29 @@ export class NotificationsService { .map(({ path }) => path); } + private async *getNotificationsGenerator( + files: string[], + type: NotificationType + ): AsyncGenerator<{ success: true; value: Notification } | { success: false; reason: unknown }> { + const BATCH_SIZE = 10; + for (let i = 0; i < files.length; i += BATCH_SIZE) { + const batch = files.slice(i, i + BATCH_SIZE); + const promises = batch.map(async (file) => { + try { + const value = await this.loadNotificationFile(file, type); + return { success: true, value } as const; + } catch (reason) { + return { success: false, reason } as const; + } + }); + + const results = await Promise.all(promises); + for (const res of results) { + yield res; + } + } + } + /** * Given a an array of files, reads and filters all the files in the directory, * and attempts to parse each file as a Notification. @@ -699,27 +722,39 @@ export class NotificationsService { filters: Partial ): Promise<[Notification[], unknown[]]> { const { importance, type, offset = 0, limit = files.length } = filters; - - const fileReads = files - .slice(offset, limit + offset) - .map((file) => this.loadNotificationFile(file, type ?? NotificationType.UNREAD)); - const results = await Promise.allSettled(fileReads); + const notifications: Notification[] = []; + const errors: unknown[] = []; + let skipped = 0; // if the filter is defined & truthy, tests if the actual value matches the filter const passesFilter = (actual: T, filter?: unknown) => !filter || actual === filter; + const matches = (n: Notification) => + passesFilter(n.importance, importance) && + passesFilter(n.type, type ?? NotificationType.UNREAD); - return [ - results - .filter(isFulfilled) - .map((result) => result.value) - .filter( - (notification) => - passesFilter(notification.importance, importance) && - passesFilter(notification.type, type) - ) - .sort(this.sortLatestFirst), - results.filter(isRejected).map((result) => result.reason), - ]; + const generator = this.getNotificationsGenerator(files, type ?? NotificationType.UNREAD); + + for await (const result of generator) { + if (!result.success) { + errors.push(result.reason); + continue; + } + + const notification = result.value; + + if (matches(notification)) { + if (skipped < offset) { + skipped++; + } else { + notifications.push(notification); + if (notifications.length >= limit) { + break; + } + } + } + } + + return [notifications.sort(this.sortLatestFirst), errors]; } /**