diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 9d407f0f310..02ec9833d6f 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -1528,7 +1528,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: OrganizationMetadataServiceAbstraction, useClass: DefaultOrganizationMetadataService, - deps: [BillingApiServiceAbstraction, ConfigService, PlatformUtilsServiceAbstraction], + deps: [BillingApiServiceAbstraction, PlatformUtilsServiceAbstraction], }), safeProvider({ provide: BillingAccountProfileStateService, diff --git a/libs/common/src/billing/abstractions/billing-api.service.abstraction.ts b/libs/common/src/billing/abstractions/billing-api.service.abstraction.ts index dcb395ef85c..9868a57bd78 100644 --- a/libs/common/src/billing/abstractions/billing-api.service.abstraction.ts +++ b/libs/common/src/billing/abstractions/billing-api.service.abstraction.ts @@ -21,11 +21,7 @@ export abstract class BillingApiServiceAbstraction { organizationId: OrganizationId, ): Promise; - abstract getOrganizationBillingMetadataVNext( - organizationId: OrganizationId, - ): Promise; - - abstract getOrganizationBillingMetadataVNextSelfHost( + abstract getOrganizationBillingMetadataSelfHost( organizationId: OrganizationId, ): Promise; diff --git a/libs/common/src/billing/services/billing-api.service.ts b/libs/common/src/billing/services/billing-api.service.ts index ae6913e545c..834606426db 100644 --- a/libs/common/src/billing/services/billing-api.service.ts +++ b/libs/common/src/billing/services/billing-api.service.ts @@ -36,20 +36,6 @@ export class BillingApiService implements BillingApiServiceAbstraction { async getOrganizationBillingMetadata( organizationId: OrganizationId, - ): Promise { - const r = await this.apiService.send( - "GET", - "/organizations/" + organizationId + "/billing/metadata", - null, - true, - true, - ); - - return new OrganizationBillingMetadataResponse(r); - } - - async getOrganizationBillingMetadataVNext( - organizationId: OrganizationId, ): Promise { const r = await this.apiService.send( "GET", @@ -62,7 +48,7 @@ export class BillingApiService implements BillingApiServiceAbstraction { return new OrganizationBillingMetadataResponse(r); } - async getOrganizationBillingMetadataVNextSelfHost( + async getOrganizationBillingMetadataSelfHost( organizationId: OrganizationId, ): Promise { const r = await this.apiService.send( diff --git a/libs/common/src/billing/services/organization/organization-metadata.service.spec.ts b/libs/common/src/billing/services/organization/organization-metadata.service.spec.ts index a2b012eb161..998356cbc14 100644 --- a/libs/common/src/billing/services/organization/organization-metadata.service.spec.ts +++ b/libs/common/src/billing/services/organization/organization-metadata.service.spec.ts @@ -1,13 +1,11 @@ import { mock } from "jest-mock-extended"; -import { BehaviorSubject, firstValueFrom } from "rxjs"; +import { firstValueFrom } from "rxjs"; import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions"; import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { newGuid } from "@bitwarden/guid"; -import { FeatureFlag } from "../../../enums/feature-flag.enum"; import { OrganizationId } from "../../../types/guid"; import { DefaultOrganizationMetadataService } from "./organization-metadata.service"; @@ -15,9 +13,7 @@ import { DefaultOrganizationMetadataService } from "./organization-metadata.serv describe("DefaultOrganizationMetadataService", () => { let service: DefaultOrganizationMetadataService; let billingApiService: jest.Mocked; - let configService: jest.Mocked; let platformUtilsService: jest.Mocked; - let featureFlagSubject: BehaviorSubject; const mockOrganizationId = newGuid() as OrganizationId; const mockOrganizationId2 = newGuid() as OrganizationId; @@ -34,182 +30,114 @@ describe("DefaultOrganizationMetadataService", () => { beforeEach(() => { billingApiService = mock(); - configService = mock(); platformUtilsService = mock(); - featureFlagSubject = new BehaviorSubject(false); - configService.getFeatureFlag$.mockReturnValue(featureFlagSubject.asObservable()); platformUtilsService.isSelfHost.mockReturnValue(false); - service = new DefaultOrganizationMetadataService( - billingApiService, - configService, - platformUtilsService, - ); + service = new DefaultOrganizationMetadataService(billingApiService, platformUtilsService); }); afterEach(() => { jest.resetAllMocks(); - featureFlagSubject.complete(); }); describe("getOrganizationMetadata$", () => { - describe("feature flag OFF", () => { - beforeEach(() => { - featureFlagSubject.next(false); - }); + it("calls getOrganizationBillingMetadata for cloud-hosted", async () => { + const mockResponse = createMockMetadataResponse(false, 10); + billingApiService.getOrganizationBillingMetadata.mockResolvedValue(mockResponse); - it("calls getOrganizationBillingMetadata when feature flag is off", async () => { - const mockResponse = createMockMetadataResponse(false, 10); - billingApiService.getOrganizationBillingMetadata.mockResolvedValue(mockResponse); + const result = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId)); - const result = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId)); - - expect(configService.getFeatureFlag$).toHaveBeenCalledWith( - FeatureFlag.PM25379_UseNewOrganizationMetadataStructure, - ); - expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledWith( - mockOrganizationId, - ); - expect(billingApiService.getOrganizationBillingMetadataVNext).not.toHaveBeenCalled(); - expect(result).toEqual(mockResponse); - }); - - it("does not cache metadata when feature flag is off", async () => { - const mockResponse1 = createMockMetadataResponse(false, 10); - const mockResponse2 = createMockMetadataResponse(false, 15); - billingApiService.getOrganizationBillingMetadata - .mockResolvedValueOnce(mockResponse1) - .mockResolvedValueOnce(mockResponse2); - - const result1 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId)); - const result2 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId)); - - expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(2); - expect(result1).toEqual(mockResponse1); - expect(result2).toEqual(mockResponse2); - }); + expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledWith( + mockOrganizationId, + ); + expect(result).toEqual(mockResponse); }); - describe("feature flag ON", () => { - beforeEach(() => { - featureFlagSubject.next(true); - }); + it("calls getOrganizationBillingMetadataSelfHost when isSelfHost is true", async () => { + platformUtilsService.isSelfHost.mockReturnValue(true); + const mockResponse = createMockMetadataResponse(true, 25); + billingApiService.getOrganizationBillingMetadataSelfHost.mockResolvedValue(mockResponse); - it("calls getOrganizationBillingMetadataVNext when feature flag is on", async () => { - const mockResponse = createMockMetadataResponse(true, 15); - billingApiService.getOrganizationBillingMetadataVNext.mockResolvedValue(mockResponse); + const result = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId)); - const result = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId)); - - expect(configService.getFeatureFlag$).toHaveBeenCalledWith( - FeatureFlag.PM25379_UseNewOrganizationMetadataStructure, - ); - expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenCalledWith( - mockOrganizationId, - ); - expect(billingApiService.getOrganizationBillingMetadata).not.toHaveBeenCalled(); - expect(result).toEqual(mockResponse); - }); - - it("caches metadata by organization ID when feature flag is on", async () => { - const mockResponse = createMockMetadataResponse(true, 10); - billingApiService.getOrganizationBillingMetadataVNext.mockResolvedValue(mockResponse); - - const result1 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId)); - const result2 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId)); - - expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenCalledTimes(1); - expect(result1).toEqual(mockResponse); - expect(result2).toEqual(mockResponse); - }); - - it("maintains separate cache entries for different organization IDs", async () => { - const mockResponse1 = createMockMetadataResponse(true, 10); - const mockResponse2 = createMockMetadataResponse(false, 20); - billingApiService.getOrganizationBillingMetadataVNext - .mockResolvedValueOnce(mockResponse1) - .mockResolvedValueOnce(mockResponse2); - - const result1 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId)); - const result2 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId2)); - const result3 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId)); - const result4 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId2)); - - expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenCalledTimes(2); - expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenNthCalledWith( - 1, - mockOrganizationId, - ); - expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenNthCalledWith( - 2, - mockOrganizationId2, - ); - expect(result1).toEqual(mockResponse1); - expect(result2).toEqual(mockResponse2); - expect(result3).toEqual(mockResponse1); - expect(result4).toEqual(mockResponse2); - }); - - it("calls getOrganizationBillingMetadataVNextSelfHost when feature flag is on and isSelfHost is true", async () => { - platformUtilsService.isSelfHost.mockReturnValue(true); - const mockResponse = createMockMetadataResponse(true, 25); - billingApiService.getOrganizationBillingMetadataVNextSelfHost.mockResolvedValue( - mockResponse, - ); - - const result = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId)); - - expect(platformUtilsService.isSelfHost).toHaveBeenCalled(); - expect(billingApiService.getOrganizationBillingMetadataVNextSelfHost).toHaveBeenCalledWith( - mockOrganizationId, - ); - expect(billingApiService.getOrganizationBillingMetadataVNext).not.toHaveBeenCalled(); - expect(billingApiService.getOrganizationBillingMetadata).not.toHaveBeenCalled(); - expect(result).toEqual(mockResponse); - }); + expect(platformUtilsService.isSelfHost).toHaveBeenCalled(); + expect(billingApiService.getOrganizationBillingMetadataSelfHost).toHaveBeenCalledWith( + mockOrganizationId, + ); + expect(billingApiService.getOrganizationBillingMetadata).not.toHaveBeenCalled(); + expect(result).toEqual(mockResponse); }); - describe("shareReplay behavior", () => { - beforeEach(() => { - featureFlagSubject.next(true); - }); + it("caches metadata by organization ID", async () => { + const mockResponse = createMockMetadataResponse(true, 10); + billingApiService.getOrganizationBillingMetadata.mockResolvedValue(mockResponse); - it("does not call API multiple times when the same cached observable is subscribed to multiple times", async () => { - const mockResponse = createMockMetadataResponse(true, 10); - billingApiService.getOrganizationBillingMetadataVNext.mockResolvedValue(mockResponse); + const result1 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId)); + const result2 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId)); - const metadata$ = service.getOrganizationMetadata$(mockOrganizationId); + expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(1); + expect(result1).toEqual(mockResponse); + expect(result2).toEqual(mockResponse); + }); - const subscription1Promise = firstValueFrom(metadata$); - const subscription2Promise = firstValueFrom(metadata$); - const subscription3Promise = firstValueFrom(metadata$); + it("maintains separate cache entries for different organization IDs", async () => { + const mockResponse1 = createMockMetadataResponse(true, 10); + const mockResponse2 = createMockMetadataResponse(false, 20); + billingApiService.getOrganizationBillingMetadata + .mockResolvedValueOnce(mockResponse1) + .mockResolvedValueOnce(mockResponse2); - const [result1, result2, result3] = await Promise.all([ - subscription1Promise, - subscription2Promise, - subscription3Promise, - ]); + const result1 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId)); + const result2 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId2)); + const result3 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId)); + const result4 = await firstValueFrom(service.getOrganizationMetadata$(mockOrganizationId2)); - expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenCalledTimes(1); - expect(result1).toEqual(mockResponse); - expect(result2).toEqual(mockResponse); - expect(result3).toEqual(mockResponse); - }); + expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(2); + expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenNthCalledWith( + 1, + mockOrganizationId, + ); + expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenNthCalledWith( + 2, + mockOrganizationId2, + ); + expect(result1).toEqual(mockResponse1); + expect(result2).toEqual(mockResponse2); + expect(result3).toEqual(mockResponse1); + expect(result4).toEqual(mockResponse2); + }); + + it("does not call API multiple times when the same cached observable is subscribed to multiple times", async () => { + const mockResponse = createMockMetadataResponse(true, 10); + billingApiService.getOrganizationBillingMetadata.mockResolvedValue(mockResponse); + + const metadata$ = service.getOrganizationMetadata$(mockOrganizationId); + + const subscription1Promise = firstValueFrom(metadata$); + const subscription2Promise = firstValueFrom(metadata$); + const subscription3Promise = firstValueFrom(metadata$); + + const [result1, result2, result3] = await Promise.all([ + subscription1Promise, + subscription2Promise, + subscription3Promise, + ]); + + expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(1); + expect(result1).toEqual(mockResponse); + expect(result2).toEqual(mockResponse); + expect(result3).toEqual(mockResponse); }); }); describe("refreshMetadataCache", () => { - beforeEach(() => { - featureFlagSubject.next(true); - }); - - it("refreshes cached metadata when called with feature flag on", (done) => { + it("refreshes cached metadata when called", (done) => { const mockResponse1 = createMockMetadataResponse(true, 10); const mockResponse2 = createMockMetadataResponse(true, 20); let invocationCount = 0; - billingApiService.getOrganizationBillingMetadataVNext + billingApiService.getOrganizationBillingMetadata .mockResolvedValueOnce(mockResponse1) .mockResolvedValueOnce(mockResponse2); @@ -221,7 +149,7 @@ describe("DefaultOrganizationMetadataService", () => { expect(result).toEqual(mockResponse1); } else if (invocationCount === 2) { expect(result).toEqual(mockResponse2); - expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenCalledTimes(2); + expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(2); subscription.unsubscribe(); done(); } @@ -234,45 +162,13 @@ describe("DefaultOrganizationMetadataService", () => { }, 10); }); - it("does trigger refresh when feature flag is disabled", async () => { - featureFlagSubject.next(false); - - const mockResponse1 = createMockMetadataResponse(false, 10); - const mockResponse2 = createMockMetadataResponse(false, 20); - let invocationCount = 0; - - billingApiService.getOrganizationBillingMetadata - .mockResolvedValueOnce(mockResponse1) - .mockResolvedValueOnce(mockResponse2); - - const subscription = service.getOrganizationMetadata$(mockOrganizationId).subscribe({ - next: () => { - invocationCount++; - }, - }); - - // wait for initial invocation - await new Promise((resolve) => setTimeout(resolve, 10)); - - expect(invocationCount).toBe(1); - - service.refreshMetadataCache(); - - await new Promise((resolve) => setTimeout(resolve, 10)); - - expect(invocationCount).toBe(2); - expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(2); - - subscription.unsubscribe(); - }); - it("bypasses cache when refreshing metadata", (done) => { const mockResponse1 = createMockMetadataResponse(true, 10); const mockResponse2 = createMockMetadataResponse(true, 20); const mockResponse3 = createMockMetadataResponse(true, 30); let invocationCount = 0; - billingApiService.getOrganizationBillingMetadataVNext + billingApiService.getOrganizationBillingMetadata .mockResolvedValueOnce(mockResponse1) .mockResolvedValueOnce(mockResponse2) .mockResolvedValueOnce(mockResponse3); @@ -289,7 +185,7 @@ describe("DefaultOrganizationMetadataService", () => { service.refreshMetadataCache(); } else if (invocationCount === 3) { expect(result).toEqual(mockResponse3); - expect(billingApiService.getOrganizationBillingMetadataVNext).toHaveBeenCalledTimes(3); + expect(billingApiService.getOrganizationBillingMetadata).toHaveBeenCalledTimes(3); subscription.unsubscribe(); done(); } diff --git a/libs/common/src/billing/services/organization/organization-metadata.service.ts b/libs/common/src/billing/services/organization/organization-metadata.service.ts index 5ce87262c4b..149c4536df4 100644 --- a/libs/common/src/billing/services/organization/organization-metadata.service.ts +++ b/libs/common/src/billing/services/organization/organization-metadata.service.ts @@ -1,10 +1,8 @@ -import { BehaviorSubject, combineLatest, from, Observable, shareReplay, switchMap } from "rxjs"; +import { BehaviorSubject, from, Observable, shareReplay, switchMap } from "rxjs"; import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; -import { FeatureFlag } from "../../../enums/feature-flag.enum"; -import { ConfigService } from "../../../platform/abstractions/config/config.service"; import { OrganizationId } from "../../../types/guid"; import { OrganizationMetadataServiceAbstraction } from "../../abstractions/organization-metadata.service.abstraction"; import { OrganizationBillingMetadataResponse } from "../../models/response/organization-billing-metadata.response"; @@ -17,7 +15,6 @@ export class DefaultOrganizationMetadataService implements OrganizationMetadataS constructor( private billingApiService: BillingApiServiceAbstraction, - private configService: ConfigService, private platformUtilsService: PlatformUtilsService, ) {} private refreshMetadataTrigger = new BehaviorSubject(undefined); @@ -28,50 +25,26 @@ export class DefaultOrganizationMetadataService implements OrganizationMetadataS }; getOrganizationMetadata$(orgId: OrganizationId): Observable { - return combineLatest([ - this.refreshMetadataTrigger, - this.configService.getFeatureFlag$(FeatureFlag.PM25379_UseNewOrganizationMetadataStructure), - ]).pipe( - switchMap(([_, featureFlagEnabled]) => - featureFlagEnabled - ? this.vNextGetOrganizationMetadataInternal$(orgId) - : this.getOrganizationMetadataInternal$(orgId), - ), - ); - } - - private vNextGetOrganizationMetadataInternal$( - orgId: OrganizationId, - ): Observable { - const cacheHit = this.metadataCache.get(orgId); - if (cacheHit) { - return cacheHit; - } - - const result = from(this.fetchMetadata(orgId, true)).pipe( - shareReplay({ bufferSize: 1, refCount: false }), - ); - - this.metadataCache.set(orgId, result); - return result; - } - - private getOrganizationMetadataInternal$( - organizationId: OrganizationId, - ): Observable { - return from(this.fetchMetadata(organizationId, false)).pipe( - shareReplay({ bufferSize: 1, refCount: false }), + return this.refreshMetadataTrigger.pipe( + switchMap(() => { + const cacheHit = this.metadataCache.get(orgId); + if (cacheHit) { + return cacheHit; + } + const result = from(this.fetchMetadata(orgId)).pipe( + shareReplay({ bufferSize: 1, refCount: false }), + ); + this.metadataCache.set(orgId, result); + return result; + }), ); } private async fetchMetadata( organizationId: OrganizationId, - featureFlagEnabled: boolean, ): Promise { - return featureFlagEnabled - ? this.platformUtilsService.isSelfHost() - ? await this.billingApiService.getOrganizationBillingMetadataVNextSelfHost(organizationId) - : await this.billingApiService.getOrganizationBillingMetadataVNext(organizationId) + return this.platformUtilsService.isSelfHost() + ? await this.billingApiService.getOrganizationBillingMetadataSelfHost(organizationId) : await this.billingApiService.getOrganizationBillingMetadata(organizationId); } } diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 71b95ec6057..d252f7dcda5 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -31,7 +31,6 @@ export enum FeatureFlag { /* Billing */ TrialPaymentOptional = "PM-8163-trial-payment", PM24032_NewNavigationPremiumUpgradeButton = "pm-24032-new-navigation-premium-upgrade-button", - PM25379_UseNewOrganizationMetadataStructure = "pm-25379-use-new-organization-metadata-structure", PM26793_FetchPremiumPriceFromPricingService = "pm-26793-fetch-premium-price-from-pricing-service", PM23713_PremiumBadgeOpensNewPremiumUpgradeDialog = "pm-23713-premium-badge-opens-new-premium-upgrade-dialog", PM26462_Milestone_3 = "pm-26462-milestone-3", @@ -149,7 +148,6 @@ export const DefaultFeatureFlagValue = { /* Billing */ [FeatureFlag.TrialPaymentOptional]: FALSE, [FeatureFlag.PM24032_NewNavigationPremiumUpgradeButton]: FALSE, - [FeatureFlag.PM25379_UseNewOrganizationMetadataStructure]: FALSE, [FeatureFlag.PM26793_FetchPremiumPriceFromPricingService]: FALSE, [FeatureFlag.PM23713_PremiumBadgeOpensNewPremiumUpgradeDialog]: FALSE, [FeatureFlag.PM26462_Milestone_3]: FALSE,