bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2026-01-28 07:13:29 +00:00
Code Issues Releases Wiki Activity

fix: topological sort not using injection token as key

Browse Source
This commit is contained in:
Andreas Coroiu
2026-01-21 09:59:59 +01:00
parent 5dfaf03219
commit a5fb35b282
5 changed files with 208 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
apps/desktop/src/app/services/services.module.ts
View File

@@ -564,8 +564,8 @@ const safeProviders: SafeProvider[] = [
}),
initializableProvider(InitService),
initializableProvider(SdkLoadService),
// initializableProvider(flagEnabled("sdk") ? DefaultSdkLoadService : NoopSdkLoadService),
initializableProvider(SshAgentService),
initializableProvider(NativeMessagingService),
];
@NgModule({

17
libs/angular/src/platform/abstractions/decentralized-init.service.ts
View File

@@ -1,32 +1,35 @@
import { InjectionToken } from "@angular/core";
import { Dependency, Initializable } from "@bitwarden/common/platform/abstractions/initializable";
import { Dependency } from "@bitwarden/common/platform/abstractions/initializable";
import { SafeProvider } from "../utils/safe-provider";
/**
* Multi-provider token for registering services that need initialization.
* Multi-provider token for registering service classes that need initialization.
* Register the service class/token (not the instance) and Angular's Injector will resolve them.
* Services register themselves by adding to their library's provider bundle:
*
* @example
* ```typescript
* export const VAULT_PROVIDERS = [
* { provide: INIT_SERVICES, useExisting: SyncService, multi: true },
* { provide: INIT_SERVICES, useExisting: VaultTimeoutService, multi: true },
* { provide: INIT_SERVICES, useValue: SyncService, multi: true },
* { provide: INIT_SERVICES, useValue: VaultTimeoutService, multi: true },
* ];
* ```
*
* Note: Use useValue (not useExisting) to register the class token itself.
*/
export const INIT_SERVICES = new InjectionToken<Initializable[]>("INIT_SERVICES");
export const INIT_SERVICES = new InjectionToken<Dependency[]>("INIT_SERVICES");
/**
* Helper function to create a type-safe provider for an Initializable service.
*
* @param type The Initializable service class
* @param ctor The Initializable service class/token to register
*/
export function initializableProvider<T extends Dependency>(ctor: T) {
return {
provide: INIT_SERVICES,
useExisting: ctor,
useValue: ctor,
multi: true,
} as SafeProvider;
}

16
libs/angular/src/platform/services/decentralized-init.service.example.ts
View File

@@ -9,9 +9,11 @@
* This is NOT production code - it's a reference example.
*/
import { Injectable, Type } from "@angular/core";
import { Injectable } from "@angular/core";
import { Initializable, INIT_SERVICES } from "../abstractions/decentralized-init.service";
import { Initializable, Dependency } from "@bitwarden/common/platform/abstractions/initializable";
import { INIT_SERVICES } from "../abstractions/decentralized-init.service";
// ============================================================================
// STEP 1: Make your services implement Initializable
@@ -23,7 +25,7 @@ import { Initializable, INIT_SERVICES } from "../abstractions/decentralized-init
*/
@Injectable({ providedIn: "root" })
export class ExampleConfigService implements Initializable {
dependencies: Type<Initializable>[] = []; // No dependencies
dependencies: Dependency[] = []; // No dependencies
async init(): Promise<void> {
// Load config, etc.
@@ -78,9 +80,9 @@ export class ExampleSyncService implements Initializable {
export const EXAMPLE_LIBRARY_PROVIDERS = [
// The multi-provider registration prevents tree-shaking
// while providedIn: 'root' handles the actual service instantiation
{ provide: INIT_SERVICES, useExisting: ExampleConfigService, multi: true },
{ provide: INIT_SERVICES, useExisting: ExampleDatabaseService, multi: true },
{ provide: INIT_SERVICES, useExisting: ExampleSyncService, multi: true },
{ provide: INIT_SERVICES, useValue: ExampleConfigService, multi: true },
{ provide: INIT_SERVICES, useValue: ExampleDatabaseService, multi: true },
{ provide: INIT_SERVICES, useValue: ExampleSyncService, multi: true },
];
// ============================================================================
@@ -151,5 +153,5 @@ export const EXAMPLE_LIBRARY_PROVIDERS = [
* If a service declares a dependency that isn't registered:
* "ServiceA depends on ServiceB, but ServiceB is not registered in INIT_SERVICES.
* Make sure to add it to your providers array:
* { provide: INIT_SERVICES, useExisting: ServiceB, multi: true }"
* { provide: INIT_SERVICES, useValue: ServiceB, multi: true }"
*/

192
libs/angular/src/platform/services/default-decentralized-init.service.spec.ts
View File

@@ -1,12 +1,12 @@
import { Type } from "@angular/core";
import { Injector } from "@angular/core";
import { Initializable } from "../abstractions/decentralized-init.service";
import { Dependency, Initializable } from "@bitwarden/common/platform/abstractions/initializable";
import { DefaultDecentralizedInitService } from "./default-decentralized-init.service";
// Test service implementations
class TestService implements Initializable {
dependencies: Type<Initializable>[] = [];
dependencies: Dependency[] = [];
initCalled = false;
init(): Promise<void> | void {
@@ -23,6 +23,19 @@ function createTrackingService(name: string, executionOrder: string[]) {
};
}
// Helper to create a mock Injector that maps tokens to instances
function createMockInjector(tokenMap: Map<Dependency, Initializable>): Injector {
return {
get: (token: Dependency) => {
const instance = tokenMap.get(token);
if (!instance) {
throw new Error(`No provider for ${token.name}!`);
}
return instance;
},
} as Injector;
}
describe("DefaultDecentralizedInitService", () => {
let executionOrder: string[];
@@ -34,7 +47,8 @@ describe("DefaultDecentralizedInitService", () => {
describe("given no registered services", () => {
it("completes without error when called", async () => {
// Arrange
const sut = new DefaultDecentralizedInitService([]);
const mockInjector = createMockInjector(new Map());
const sut = new DefaultDecentralizedInitService([], mockInjector);
// Act & Assert
await expect(sut.init()).resolves.not.toThrow();
@@ -45,7 +59,9 @@ describe("DefaultDecentralizedInitService", () => {
it("initializes a single service when called", async () => {
// Arrange
const service = new TestService();
const sut = new DefaultDecentralizedInitService([service]);
const tokenMap = new Map([[TestService, service]]);
const mockInjector = createMockInjector(tokenMap);
const sut = new DefaultDecentralizedInitService([TestService], mockInjector);
// Act
await sut.init();
@@ -56,10 +72,24 @@ describe("DefaultDecentralizedInitService", () => {
it("initializes all services when called with multiple independent services", async () => {
// Arrange
const service1 = new TestService();
const service2 = new TestService();
const service3 = new TestService();
const sut = new DefaultDecentralizedInitService([service1, service2, service3]);
class Service1 extends TestService {}
class Service2 extends TestService {}
class Service3 extends TestService {}
const service1 = new Service1();
const service2 = new Service2();
const service3 = new Service3();
const tokenMap = new Map([
[Service1, service1],
[Service2, service2],
[Service3, service3],
]);
const mockInjector = createMockInjector(tokenMap);
const sut = new DefaultDecentralizedInitService(
[Service1, Service2, Service3],
mockInjector,
);
// Act
await sut.init();
@@ -81,7 +111,12 @@ describe("DefaultDecentralizedInitService", () => {
const serviceB = new ServiceB();
serviceB.dependencies = [ServiceA];
const sut = new DefaultDecentralizedInitService([serviceB, serviceA]);
const tokenMap = new Map([
[ServiceA, serviceA],
[ServiceB, serviceB],
]);
const mockInjector = createMockInjector(tokenMap);
const sut = new DefaultDecentralizedInitService([ServiceB, ServiceA], mockInjector);
// Act
await sut.init();
@@ -107,7 +142,17 @@ describe("DefaultDecentralizedInitService", () => {
serviceC.dependencies = [ServiceA, ServiceB];
serviceD.dependencies = [ServiceC];
const sut = new DefaultDecentralizedInitService([serviceD, serviceB, serviceC, serviceA]);
const tokenMap = new Map([
[ServiceA, serviceA],
[ServiceB, serviceB],
[ServiceC, serviceC],
[ServiceD, serviceD],
]);
const mockInjector = createMockInjector(tokenMap);
const sut = new DefaultDecentralizedInitService(
[ServiceD, ServiceB, ServiceC, ServiceA],
mockInjector,
);
// Act
await sut.init();
@@ -139,7 +184,17 @@ describe("DefaultDecentralizedInitService", () => {
serviceC.dependencies = [ServiceA];
serviceD.dependencies = [ServiceB, ServiceC];
const sut = new DefaultDecentralizedInitService([serviceD, serviceC, serviceB, serviceA]);
const tokenMap = new Map([
[ServiceA, serviceA],
[ServiceB, serviceB],
[ServiceC, serviceC],
[ServiceD, serviceD],
]);
const mockInjector = createMockInjector(tokenMap);
const sut = new DefaultDecentralizedInitService(
[ServiceD, ServiceC, ServiceB, ServiceA],
mockInjector,
);
// Act
await sut.init();
@@ -166,7 +221,9 @@ describe("DefaultDecentralizedInitService", () => {
}
const service = new CountingService();
const sut = new DefaultDecentralizedInitService([service]);
const tokenMap = new Map([[CountingService, service]]);
const mockInjector = createMockInjector(tokenMap);
const sut = new DefaultDecentralizedInitService([CountingService], mockInjector);
// Act
await sut.init();
@@ -174,6 +231,60 @@ describe("DefaultDecentralizedInitService", () => {
// Assert
expect(initCount).toBe(1);
});
it("resolves dependencies by abstract parent class when called", async () => {
// Arrange - Simulates Angular pattern:
// { provide: AbstractService, useClass: ConcreteService }
// { provide: INIT_SERVICES, useValue: AbstractService, multi: true }
abstract class AbstractBaseService extends TestService {
abstract someMethod(): void;
}
class ConcreteImplementation extends AbstractBaseService {
someMethod(): void {
executionOrder.push("method");
}
async init(): Promise<void> {
executionOrder.push("concrete");
await super.init();
}
}
class DependentService extends TestService {
// References the abstract class, not the concrete implementation
dependencies = [AbstractBaseService as Dependency];
async init(): Promise<void> {
executionOrder.push("dependent");
await super.init();
}
}
// Register using the abstract token (what's in INIT_SERVICES)
// Angular DI provides the concrete implementation
const concreteService = new ConcreteImplementation();
const dependentService = new DependentService();
const tokenMap = new Map<Dependency, Initializable>([
[AbstractBaseService as Dependency, concreteService], // Token points to concrete instance
[DependentService as Dependency, dependentService],
]);
const mockInjector = createMockInjector(tokenMap);
const sut = new DefaultDecentralizedInitService(
[DependentService as Dependency, AbstractBaseService as Dependency],
mockInjector,
);
// Act
await sut.init();
// Assert
// Should resolve AbstractBaseService token to ConcreteImplementation instance
expect(executionOrder).toEqual(["concrete", "dependent"]);
expect(concreteService.initCalled).toBe(true);
expect(dependentService.initCalled).toBe(true);
});
});
describe("given services with circular dependencies", () => {
@@ -185,10 +296,15 @@ describe("DefaultDecentralizedInitService", () => {
const serviceA = new ServiceA();
const serviceB = new ServiceB();
serviceA.dependencies = [ServiceB as Type<Initializable>];
serviceB.dependencies = [ServiceA as Type<Initializable>];
serviceA.dependencies = [ServiceB as Dependency];
serviceB.dependencies = [ServiceA as Dependency];
const sut = new DefaultDecentralizedInitService([serviceA, serviceB]);
const tokenMap = new Map([
[ServiceA, serviceA],
[ServiceB, serviceB],
]);
const mockInjector = createMockInjector(tokenMap);
const sut = new DefaultDecentralizedInitService([ServiceA, ServiceB], mockInjector);
// Act & Assert
await expect(sut.init()).rejects.toThrow(/Circular dependency detected/);
@@ -204,11 +320,20 @@ describe("DefaultDecentralizedInitService", () => {
const serviceB = new ServiceB();
const serviceC = new ServiceC();
serviceA.dependencies = [ServiceB as Type<Initializable>];
serviceB.dependencies = [ServiceC as Type<Initializable>];
serviceC.dependencies = [ServiceA as Type<Initializable>];
serviceA.dependencies = [ServiceB as Dependency];
serviceB.dependencies = [ServiceC as Dependency];
serviceC.dependencies = [ServiceA as Dependency];
const sut = new DefaultDecentralizedInitService([serviceA, serviceB, serviceC]);
const tokenMap = new Map([
[ServiceA, serviceA],
[ServiceB, serviceB],
[ServiceC, serviceC],
]);
const mockInjector = createMockInjector(tokenMap);
const sut = new DefaultDecentralizedInitService(
[ServiceA, ServiceB, ServiceC],
mockInjector,
);
// Act & Assert
await expect(sut.init()).rejects.toThrow(/Circular dependency detected/);
@@ -224,7 +349,10 @@ describe("DefaultDecentralizedInitService", () => {
}
const serviceB = new ServiceB();
const sut = new DefaultDecentralizedInitService([serviceB]);
const tokenMap = new Map([[ServiceB, serviceB]]);
// Note: ServiceA is not in the tokenMap
const mockInjector = createMockInjector(tokenMap);
const sut = new DefaultDecentralizedInitService([ServiceB], mockInjector);
// Act & Assert
await expect(sut.init()).rejects.toThrow(/not registered in INIT_SERVICES/);
@@ -238,11 +366,13 @@ describe("DefaultDecentralizedInitService", () => {
}
const myService = new MyService();
const sut = new DefaultDecentralizedInitService([myService]);
const tokenMap = new Map([[MyService, myService]]);
const mockInjector = createMockInjector(tokenMap);
const sut = new DefaultDecentralizedInitService([MyService], mockInjector);
// Act & Assert
await expect(sut.init()).rejects.toThrow("MyService depends on MyDependency");
await expect(sut.init()).rejects.toThrow("useExisting: MyDependency");
await expect(sut.init()).rejects.toThrow("useValue: MyDependency");
});
});
@@ -256,7 +386,9 @@ describe("DefaultDecentralizedInitService", () => {
}
const service = new FailingService();
const sut = new DefaultDecentralizedInitService([service]);
const tokenMap = new Map([[FailingService, service]]);
const mockInjector = createMockInjector(tokenMap);
const sut = new DefaultDecentralizedInitService([FailingService], mockInjector);
// Act & Assert
await expect(sut.init()).rejects.toThrow(/Failed to initialize FailingService/);
@@ -275,7 +407,9 @@ describe("DefaultDecentralizedInitService", () => {
}
const service = new SyncService();
const sut = new DefaultDecentralizedInitService([service]);
const tokenMap = new Map([[SyncService, service]]);
const mockInjector = createMockInjector(tokenMap);
const sut = new DefaultDecentralizedInitService([SyncService], mockInjector);
// Act
await sut.init();
@@ -305,7 +439,13 @@ describe("DefaultDecentralizedInitService", () => {
const syncService = new SyncService();
const asyncService = new AsyncService();
const sut = new DefaultDecentralizedInitService([asyncService, syncService]);
const tokenMap = new Map([
[SyncService, syncService],
[AsyncService, asyncService],
]);
const mockInjector = createMockInjector(tokenMap);
const sut = new DefaultDecentralizedInitService([AsyncService, SyncService], mockInjector);
// Act
await sut.init();

32
libs/angular/src/platform/services/default-decentralized-init.service.ts
View File

@@ -1,4 +1,4 @@
import { Inject, Injectable } from "@angular/core";
import { Inject, Injectable, Injector } from "@angular/core";
import { Dependency, Initializable } from "@bitwarden/common/platform/abstractions/initializable";
@@ -12,7 +12,8 @@ import {
* to execute initialization in dependency order.
*
* This service:
* - Discovers all registered Initializable services via the INIT_SERVICES token
* - Collects registered service tokens via the INIT_SERVICES token
* - Resolves tokens to instances using Angular's Injector
* - Builds a dependency graph from each service's dependencies property
* - Performs topological sort to determine execution order
* - Detects circular dependencies and throws clear errors
@@ -20,14 +21,22 @@ import {
*/
@Injectable()
export class DefaultDecentralizedInitService implements DecentralizedInitService {
constructor(@Inject(INIT_SERVICES) private initServices: Initializable[]) {}
constructor(
@Inject(INIT_SERVICES) private initServiceTokens: Dependency[],
private injector: Injector,
) {}
async init(): Promise<void> {
if (!this.initServices || this.initServices.length === 0) {
if (!this.initServiceTokens || this.initServiceTokens.length === 0) {
return;
}
const sorted = this.topologicalSort(this.initServices);
// Resolve all tokens to instances using Angular's Injector
const services: Initializable[] = this.initServiceTokens.map((token) =>
this.injector.get(token),
);
const sorted = this.topologicalSort(services, this.initServiceTokens);
for (const service of sorted) {
try {
@@ -42,14 +51,17 @@ export class DefaultDecentralizedInitService implements DecentralizedInitService
* Performs topological sort on services based on their declared dependencies.
* Returns services in an order where all dependencies come before dependents.
*
* @param services The resolved service instances
* @param tokens The tokens used to register these services (parallel array)
* @throws Error if circular dependencies are detected
* @throws Error if a dependency is declared but not registered
*/
private topologicalSort(services: Initializable[]): Initializable[] {
// Build a map from constructor to instance for quick lookup
private topologicalSort(services: Initializable[], tokens: Dependency[]): Initializable[] {
// Build a map from token to instance
// This uses the exact tokens that were registered, so abstract classes work correctly
const instanceMap = new Map<Dependency, Initializable>();
for (const service of services) {
instanceMap.set(service.constructor as Dependency, service);
for (let i = 0; i < services.length; i++) {
instanceMap.set(tokens[i], services[i]);
}
const sorted: Initializable[] = [];
@@ -79,7 +91,7 @@ export class DefaultDecentralizedInitService implements DecentralizedInitService
throw new Error(
`${service.constructor.name} depends on ${depClass.name}, but ${depClass.name} is not registered in INIT_SERVICES. ` +
`Make sure to add it to your providers array:\n` +
`{ provide: INIT_SERVICES, useExisting: ${depClass.name}, multi: true }`,
`{ provide: INIT_SERVICES, useValue: ${depClass.name}, multi: true }`,
);
}