From 7a2940673b4d4c93c80cf9fd96007fb66493c27a Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 28 Oct 2025 19:01:25 -0400 Subject: [PATCH] fix(dirt): add subscription cleanup to AllActivitiesService Critical fix for production code quality and memory leak prevention. Adds takeUntil pattern to all subscriptions to comply with ADR-0003 (Observable Data Services) requirements. **Subscription Cleanup (ADR-0003 Compliance):** - Add takeUntil pattern to AllActivitiesService subscriptions - Add _destroy$ Subject and destroy() method - Prevents memory leaks by properly unsubscribing from observables - Follows Observable Data Services ADR requirements Changes: - Import Subject and takeUntil from rxjs - Add private _destroy$ Subject for cleanup coordination - Apply takeUntil(this._destroy$) to all 3 subscriptions: - enrichedReportData$ subscription - criticalReportResults$ subscription - newApplications$ subscription - Add destroy() method for proper resource cleanup This ensures proper resource cleanup and follows Bitwarden's architectural decision records for observable management. Related to PM-27284 --- .../services/view/all-activities.service.ts | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/view/all-activities.service.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/view/all-activities.service.ts index 7de8ab223de..4fa8a6779b9 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/view/all-activities.service.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/view/all-activities.service.ts @@ -1,4 +1,5 @@ -import { BehaviorSubject } from "rxjs"; +import { BehaviorSubject, Subject } from "rxjs"; +import { takeUntil } from "rxjs/operators"; import { ApplicationHealthReportDetailEnriched } from "../../models"; import { OrganizationReportSummary } from "../../models/report-models"; @@ -11,6 +12,8 @@ export class AllActivitiesService { /// Going forward, this class can be simplified by using the RiskInsightsDataService /// as it contains the application summary data. + private _destroy$ = new Subject(); + private reportSummarySubject$ = new BehaviorSubject({ totalMemberCount: 0, totalCriticalMemberCount: 0, @@ -40,7 +43,7 @@ export class AllActivitiesService { constructor(private dataService: RiskInsightsDataService) { // All application summary changes - this.dataService.enrichedReportData$.subscribe((report) => { + this.dataService.enrichedReportData$.pipe(takeUntil(this._destroy$)).subscribe((report) => { if (report) { this.setAllAppsReportSummary(report.summaryData); this.setAllAppsReportDetails(report.reportData); @@ -48,14 +51,14 @@ export class AllActivitiesService { }); // Critical application summary changes - this.dataService.criticalReportResults$.subscribe((report) => { + this.dataService.criticalReportResults$.pipe(takeUntil(this._destroy$)).subscribe((report) => { if (report) { this.setCriticalAppsReportSummary(report.summaryData); } }); // New applications changes (from orchestrator's reactive pipeline) - this.dataService.newApplications$.subscribe((newApps) => { + this.dataService.newApplications$.pipe(takeUntil(this._destroy$)).subscribe((newApps) => { this.setNewApplications(newApps); }); } @@ -108,4 +111,14 @@ export class AllActivitiesService { newApplications: newApps, }); } + + /** + * Cleanup method to prevent memory leaks. + * Should be called when the service is no longer needed. + * Complies with ADR-0003 (Observable Data Services) requirements. + */ + destroy(): void { + this._destroy$.next(); + this._destroy$.complete(); + } }