From c346802d24ab74902e2dbde99b18defa2c44cf36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Pintos=20L=C3=B3pez?= Date: Tue, 14 Jan 2025 00:33:26 +0100 Subject: [PATCH 1/3] fix(core): update OneToManyMapStar to allow duplicated models --- .changeset/five-mice-exist.md | 5 + .changeset/tender-laws-kick.md | 5 + .../services/ActivationsService.spec.ts | 6 +- .../binding/services/ActivationsService.ts | 2 +- .../binding/services/BindingService.spec.ts | 12 +- .../src/binding/services/BindingService.ts | 2 +- .../services/DeactivationsService.spec.ts | 6 +- .../binding/services/DeactivationsService.ts | 2 +- .../common/models/OneToManyMapStar.spec.ts | 133 +++++++++++++++++- .../src/common/models/OneToManyMapStar.ts | 64 ++++++--- .../models/__mocks__/OneToManyMapStar.ts | 9 +- 11 files changed, 200 insertions(+), 46 deletions(-) create mode 100644 .changeset/five-mice-exist.md create mode 100644 .changeset/tender-laws-kick.md diff --git a/.changeset/five-mice-exist.md b/.changeset/five-mice-exist.md new file mode 100644 index 00000000..1725f139 --- /dev/null +++ b/.changeset/five-mice-exist.md @@ -0,0 +1,5 @@ +--- +"@inversifyjs/core": patch +--- + +Updated `DeactivationService` to allow duplicated deactivations diff --git a/.changeset/tender-laws-kick.md b/.changeset/tender-laws-kick.md new file mode 100644 index 00000000..44b6ff36 --- /dev/null +++ b/.changeset/tender-laws-kick.md @@ -0,0 +1,5 @@ +--- +"@inversifyjs/core": patch +--- + +Updated `ActivationService` to allow duplicated activations diff --git a/packages/container/libraries/core/src/binding/services/ActivationsService.spec.ts b/packages/container/libraries/core/src/binding/services/ActivationsService.spec.ts index 7ef46f10..e923ad80 100644 --- a/packages/container/libraries/core/src/binding/services/ActivationsService.spec.ts +++ b/packages/container/libraries/core/src/binding/services/ActivationsService.spec.ts @@ -65,9 +65,9 @@ describe(ActivationsService.name, () => { jest.clearAllMocks(); }); - it('should call activationMaps.set()', () => { - expect(activationMapsMock.set).toHaveBeenCalledTimes(1); - expect(activationMapsMock.set).toHaveBeenCalledWith( + it('should call activationMaps.add()', () => { + expect(activationMapsMock.add).toHaveBeenCalledTimes(1); + expect(activationMapsMock.add).toHaveBeenCalledWith( activationFixture, relationFixture, ); diff --git a/packages/container/libraries/core/src/binding/services/ActivationsService.ts b/packages/container/libraries/core/src/binding/services/ActivationsService.ts index d3198943..41ad7957 100644 --- a/packages/container/libraries/core/src/binding/services/ActivationsService.ts +++ b/packages/container/libraries/core/src/binding/services/ActivationsService.ts @@ -54,7 +54,7 @@ export class ActivationsService implements Cloneable { activation: BindingActivation, relation: BindingActivationRelation, ): void { - this.#activationMaps.set(activation, relation); + this.#activationMaps.add(activation, relation); } public clone(): ActivationsService { diff --git a/packages/container/libraries/core/src/binding/services/BindingService.spec.ts b/packages/container/libraries/core/src/binding/services/BindingService.spec.ts index 80f4c992..a7d10005 100644 --- a/packages/container/libraries/core/src/binding/services/BindingService.spec.ts +++ b/packages/container/libraries/core/src/binding/services/BindingService.spec.ts @@ -359,13 +359,13 @@ describe(BindingService.name, () => { jest.clearAllMocks(); }); - it('should call bindingMaps.set()', () => { + it('should call bindingMaps.add()', () => { const expectedRelation: BindingRelation = { serviceId: bindingFixture.serviceIdentifier, }; - expect(bindingMapsMock.set).toHaveBeenCalledTimes(1); - expect(bindingMapsMock.set).toHaveBeenCalledWith( + expect(bindingMapsMock.add).toHaveBeenCalledTimes(1); + expect(bindingMapsMock.add).toHaveBeenCalledWith( bindingFixture, expectedRelation, ); @@ -395,14 +395,14 @@ describe(BindingService.name, () => { jest.clearAllMocks(); }); - it('should call bindingMaps.set()', () => { + it('should call bindingMaps.add()', () => { const expectedRelation: BindingRelation = { moduleId: bindingFixture.moduleId as number, serviceId: bindingFixture.serviceIdentifier, }; - expect(bindingMapsMock.set).toHaveBeenCalledTimes(1); - expect(bindingMapsMock.set).toHaveBeenCalledWith( + expect(bindingMapsMock.add).toHaveBeenCalledTimes(1); + expect(bindingMapsMock.add).toHaveBeenCalledWith( bindingFixture, expectedRelation, ); diff --git a/packages/container/libraries/core/src/binding/services/BindingService.ts b/packages/container/libraries/core/src/binding/services/BindingService.ts index b3c6bb9c..94745482 100644 --- a/packages/container/libraries/core/src/binding/services/BindingService.ts +++ b/packages/container/libraries/core/src/binding/services/BindingService.ts @@ -100,6 +100,6 @@ export class BindingService implements Cloneable { relation[BindingRelationKind.moduleId] = binding.moduleId; } - this.#bindingMaps.set(binding as Binding, relation); + this.#bindingMaps.add(binding as Binding, relation); } } diff --git a/packages/container/libraries/core/src/binding/services/DeactivationsService.spec.ts b/packages/container/libraries/core/src/binding/services/DeactivationsService.spec.ts index 4d8bf5bf..76aad43e 100644 --- a/packages/container/libraries/core/src/binding/services/DeactivationsService.spec.ts +++ b/packages/container/libraries/core/src/binding/services/DeactivationsService.spec.ts @@ -66,9 +66,9 @@ describe(DeactivationsService.name, () => { jest.clearAllMocks(); }); - it('should call deactivationMaps.set()', () => { - expect(deactivationMapsMock.set).toHaveBeenCalledTimes(1); - expect(deactivationMapsMock.set).toHaveBeenCalledWith( + it('should call deactivationMaps.add()', () => { + expect(deactivationMapsMock.add).toHaveBeenCalledTimes(1); + expect(deactivationMapsMock.add).toHaveBeenCalledWith( deactivationFixture, relationFixture, ); diff --git a/packages/container/libraries/core/src/binding/services/DeactivationsService.ts b/packages/container/libraries/core/src/binding/services/DeactivationsService.ts index 0bfe8aa5..09bfc8bd 100644 --- a/packages/container/libraries/core/src/binding/services/DeactivationsService.ts +++ b/packages/container/libraries/core/src/binding/services/DeactivationsService.ts @@ -54,7 +54,7 @@ export class DeactivationsService implements Cloneable { deactivation: BindingDeactivation, relation: BindingDeactivationRelation, ): void { - this.#deactivationMaps.set(deactivation, relation); + this.#deactivationMaps.add(deactivation, relation); } public clone(): DeactivationsService { diff --git a/packages/container/libraries/core/src/common/models/OneToManyMapStar.spec.ts b/packages/container/libraries/core/src/common/models/OneToManyMapStar.spec.ts index 3f542d9b..a083a262 100644 --- a/packages/container/libraries/core/src/common/models/OneToManyMapStar.spec.ts +++ b/packages/container/libraries/core/src/common/models/OneToManyMapStar.spec.ts @@ -26,7 +26,7 @@ describe(OneToManyMapStar.name, () => { }, }); - oneToManyMapStar.set(Symbol(), { + oneToManyMapStar.add(Symbol(), { [RelationKey.bar]: 2, [RelationKey.foo]: 'foo-value-fixture', }); @@ -67,7 +67,7 @@ describe(OneToManyMapStar.name, () => { }, }); - oneToManyMapStar.set(modelFixture, { + oneToManyMapStar.add(modelFixture, { [relationKeyFixture]: relationValueFixture, }); }); @@ -125,6 +125,54 @@ describe(OneToManyMapStar.name, () => { }); }); }); + + describe('having a OneToManyMapStartSpec with twice a model', () => { + let modelFixture: unknown; + let relationKeyFixture: RelationKey.foo; + let relationValueFixture: string; + + let oneToManyMapStar: OneToManyMapStar; + + beforeAll(() => { + modelFixture = Symbol(); + relationKeyFixture = RelationKey.foo; + relationValueFixture = 'value-fixture'; + + oneToManyMapStar = new OneToManyMapStar({ + bar: { + isOptional: true, + }, + foo: { + isOptional: false, + }, + }); + + oneToManyMapStar.add(modelFixture, { + [relationKeyFixture]: relationValueFixture, + }); + + oneToManyMapStar.add(modelFixture, { + [relationKeyFixture]: relationValueFixture, + }); + }); + + describe('when called', () => { + let result: unknown; + + beforeAll(() => { + result = [ + ...(oneToManyMapStar.get( + relationKeyFixture, + relationValueFixture, + ) ?? []), + ]; + }); + + it('should return expected result', () => { + expect(result).toStrictEqual([modelFixture, modelFixture]); + }); + }); + }); }); describe('.getAllKeys', () => { @@ -150,7 +198,7 @@ describe(OneToManyMapStar.name, () => { }, }); - oneToManyMapStar.set(modelFixture, relationFixture); + oneToManyMapStar.add(modelFixture, relationFixture); }); describe('when called', () => { @@ -168,7 +216,7 @@ describe(OneToManyMapStar.name, () => { }); describe('.removeByRelation', () => { - describe('having a OneToManyMapStart with a no models', () => { + describe('having a OneToManyMapStart with no models', () => { let relationFixture: Required; let oneToManyMapStar: OneToManyMapStar; @@ -247,7 +295,7 @@ describe(OneToManyMapStar.name, () => { }, }); - oneToManyMapStar.set(modelFixture, relationFixture); + oneToManyMapStar.add(modelFixture, relationFixture); }); describe('when called', () => { @@ -289,6 +337,79 @@ describe(OneToManyMapStar.name, () => { }); }); }); + + describe('having a OneToManyMapStart with twice a model with different relations', () => { + let modelFixture: unknown; + let firstRelationFixture: Required; + let secondRelationFixture: Required; + let oneToManyMapStar: OneToManyMapStar; + + beforeAll(() => { + modelFixture = Symbol(); + firstRelationFixture = { + bar: 3, + foo: 'foo', + }; + secondRelationFixture = { + bar: 4, + foo: firstRelationFixture[RelationKey.foo], + }; + oneToManyMapStar = new OneToManyMapStar({ + bar: { + isOptional: true, + }, + foo: { + isOptional: false, + }, + }); + + oneToManyMapStar.add(modelFixture, firstRelationFixture); + oneToManyMapStar.add(modelFixture, secondRelationFixture); + }); + + describe('when called', () => { + beforeAll(() => { + oneToManyMapStar.removeByRelation( + RelationKey.bar, + firstRelationFixture[RelationKey.bar], + ); + }); + + describe('when called .get()', () => { + let results: { + [TKey in RelationKey]-?: Iterable | undefined; + }; + + beforeAll(() => { + results = { + [RelationKey.bar]: [ + ...(oneToManyMapStar.get( + RelationKey.bar, + firstRelationFixture[RelationKey.bar], + ) ?? []), + ], + [RelationKey.foo]: [ + ...(oneToManyMapStar.get( + RelationKey.foo, + firstRelationFixture[RelationKey.foo], + ) ?? []), + ], + }; + }); + + it('should return expected results', () => { + const expectedResults: { + [TKey in RelationKey]-?: Iterable | undefined; + } = { + [RelationKey.bar]: [], + [RelationKey.foo]: [modelFixture], + }; + + expect(results).toStrictEqual(expectedResults); + }); + }); + }); + }); }); describe('.set', () => { @@ -314,7 +435,7 @@ describe(OneToManyMapStar.name, () => { describe('when called', () => { beforeAll(() => { - oneToManyMapStar.set(modelFixture, relationFixture); + oneToManyMapStar.add(modelFixture, relationFixture); }); describe('when called .get() with relation values', () => { diff --git a/packages/container/libraries/core/src/common/models/OneToManyMapStar.ts b/packages/container/libraries/core/src/common/models/OneToManyMapStar.ts index d723a695..4a8f3eaf 100644 --- a/packages/container/libraries/core/src/common/models/OneToManyMapStar.ts +++ b/packages/container/libraries/core/src/common/models/OneToManyMapStar.ts @@ -1,5 +1,7 @@ import { Cloneable } from './Cloneable'; +const NOT_FOUND_INDEX: number = -1; + export type OneToManyMapStartSpec = { [TKey in keyof TRelation]: { isOptional: undefined extends TRelation[TKey] ? true : false; @@ -7,7 +9,7 @@ export type OneToManyMapStartSpec = { }; type RelationToModelMap = { - [TKey in keyof TRelation]-?: Map>; + [TKey in keyof TRelation]-?: Map; }; /** @@ -16,7 +18,7 @@ type RelationToModelMap = { export class OneToManyMapStar implements Cloneable> { - readonly #modelToRelationMap: Map; + readonly #modelToRelationMap: Map; readonly #relationToModelsMaps: RelationToModelMap; readonly #spec: OneToManyMapStartSpec; @@ -32,6 +34,18 @@ export class OneToManyMapStar this.#spec = spec; } + public add(model: TModel, relation: TRelation): void { + this.#buildOrGetModelArray(model).push(relation); + + for (const relationKey of Reflect.ownKeys( + relation, + ) as (keyof TRelation)[]) { + this.#buildOrGetRelationModelSet(relationKey, relation[relationKey]).push( + model, + ); + } + } + public clone(): OneToManyMapStar { const properties: (keyof TRelation)[] = Reflect.ownKeys( this.#spec, @@ -60,7 +74,7 @@ export class OneToManyMapStar key: TKey, value: Required[TKey], ): Iterable | undefined { - return this.#relationToModelsMaps[key].get(value)?.values(); + return this.#relationToModelsMaps[key].get(value); } public getAllKeys( @@ -80,39 +94,45 @@ export class OneToManyMapStar } for (const model of models) { - const relation: TRelation | undefined = + const relations: TRelation[] | undefined = this.#modelToRelationMap.get(model); - if (relation === undefined) { + if (relations === undefined) { throw new Error('Expecting model relation, none found'); } - this.#removeModelFromRelationMaps(model, relation); + for (const relation of relations) { + if (relation[key] === value) { + this.#removeModelFromRelationMaps(model, relation); + } + } + this.#modelToRelationMap.delete(model); } } - public set(model: TModel, relation: TRelation): void { - this.#modelToRelationMap.set(model, relation); + #buildOrGetModelArray(model: TModel): TRelation[] { + let relations: TRelation[] | undefined = + this.#modelToRelationMap.get(model); - for (const relationKey of Reflect.ownKeys( - relation, - ) as (keyof TRelation)[]) { - this.#buildOrGetRelationModelSet(relationKey, relation[relationKey]).add( - model, - ); + if (relations === undefined) { + relations = []; + + this.#modelToRelationMap.set(model, relations); } + + return relations; } #buildOrGetRelationModelSet( relationKey: TKey, relationValue: TRelation[TKey], - ): Set { - let modelSet: Set | undefined = + ): TModel[] { + let modelSet: TModel[] | undefined = this.#relationToModelsMaps[relationKey].get(relationValue); if (modelSet === undefined) { - modelSet = new Set(); + modelSet = []; this.#relationToModelsMaps[relationKey].set(relationValue, modelSet); } @@ -146,13 +166,17 @@ export class OneToManyMapStar relationKey: TKey, relationValue: TRelation[TKey], ): void { - const modelSet: Set | undefined = + const modelSet: TModel[] | undefined = this.#relationToModelsMaps[relationKey].get(relationValue); if (modelSet !== undefined) { - modelSet.delete(model); + const index: number = modelSet.indexOf(model); + + if (index !== NOT_FOUND_INDEX) { + modelSet.splice(index, 1); + } - if (modelSet.size === 0) { + if (modelSet.length === 0) { this.#relationToModelsMaps[relationKey].delete(relationValue); } } diff --git a/packages/container/libraries/core/src/common/models/__mocks__/OneToManyMapStar.ts b/packages/container/libraries/core/src/common/models/__mocks__/OneToManyMapStar.ts index 0f39f81b..c074e247 100644 --- a/packages/container/libraries/core/src/common/models/__mocks__/OneToManyMapStar.ts +++ b/packages/container/libraries/core/src/common/models/__mocks__/OneToManyMapStar.ts @@ -1,5 +1,7 @@ import { jest } from '@jest/globals'; +// eslint-disable-next-line @typescript-eslint/no-explicit-any +const addMock: jest.Mock = jest.fn(); // eslint-disable-next-line @typescript-eslint/no-explicit-any const cloneMock: jest.Mock = jest.fn(); // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -8,10 +10,9 @@ const getMock: jest.Mock = jest.fn(); const getAllKeysMock: jest.Mock = jest.fn(); // eslint-disable-next-line @typescript-eslint/no-explicit-any const removeByRelationMock: jest.Mock = jest.fn(); -// eslint-disable-next-line @typescript-eslint/no-explicit-any -const setMock: jest.Mock = jest.fn(); export class OneToManyMapStar { + public readonly add: (model: TModel, relation: TRelation) => void; public readonly clone: () => OneToManyMapStar; public readonly get: ( key: TKey, @@ -27,13 +28,11 @@ export class OneToManyMapStar { value: Required[TKey], ) => void; - public readonly set: (model: TModel, relation: TRelation) => void; - constructor() { + this.add = addMock; this.clone = cloneMock; this.get = getMock; this.getAllKeys = getAllKeysMock; this.removeByRelation = removeByRelationMock; - this.set = setMock; } } From b23d421a31f2ceb22ec87e08fd12957a340930ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Pintos=20L=C3=B3pez?= Date: Tue, 14 Jan 2025 00:34:30 +0100 Subject: [PATCH 2/3] test(core): rename test to be an integration test --- ...ec.ts => classMetadataKindIsARuntimeDiscriminator.int.spec.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename packages/container/libraries/core/src/issues/{classMetadataKindIsARuntimeDiscriminator.spec.ts => classMetadataKindIsARuntimeDiscriminator.int.spec.ts} (100%) diff --git a/packages/container/libraries/core/src/issues/classMetadataKindIsARuntimeDiscriminator.spec.ts b/packages/container/libraries/core/src/issues/classMetadataKindIsARuntimeDiscriminator.int.spec.ts similarity index 100% rename from packages/container/libraries/core/src/issues/classMetadataKindIsARuntimeDiscriminator.spec.ts rename to packages/container/libraries/core/src/issues/classMetadataKindIsARuntimeDiscriminator.int.spec.ts From fa923195c57b7a551a9677db92653fef08c3b870 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Pintos=20L=C3=B3pez?= Date: Tue, 14 Jan 2025 00:35:14 +0100 Subject: [PATCH 3/3] test(e2e-tests): add activation rule --- .../features/container/activation.feature | 23 ++++++++++++++++ .../step-definitions/givenDefinitions.ts | 13 ++++++++++ .../step-definitions/whenDefinitions.ts | 26 +++++++++++++++++++ .../step-definitions/thenDefinitions.ts | 11 ++++++++ 4 files changed, 73 insertions(+) create mode 100644 packages/container/tools/e2e-tests/features/container/activation.feature diff --git a/packages/container/tools/e2e-tests/features/container/activation.feature b/packages/container/tools/e2e-tests/features/container/activation.feature new file mode 100644 index 00000000..21bcb6c3 --- /dev/null +++ b/packages/container/tools/e2e-tests/features/container/activation.feature @@ -0,0 +1,23 @@ +Feature: Activation + + A container can register service activation that are invoked when the service is resolved. + + Background: Having a container + Given a container + + Rule: an activation is invoked when its asociated service is resolved + + Scenario: A registered activation is invoked when the service is resolved + Given a sword type binding as "sword" + When "sword" binding is bound to container + And a weapon upgrade activation is registered for sword type + And container gets a sword type value + Then value is a sword with improved damage + + Scenario: An activation registered twice is invoked when the service is resolved + Given a sword type binding as "sword" + When "sword" binding is bound to container + And a weapon upgrade activation is registered for sword type + And a weapon upgrade activation is registered for sword type + And container gets a sword type value + Then value is a sword with improved damage (x2) diff --git a/packages/container/tools/e2e-tests/src/binding/step-definitions/givenDefinitions.ts b/packages/container/tools/e2e-tests/src/binding/step-definitions/givenDefinitions.ts index 7c1113ab..bb88795f 100644 --- a/packages/container/tools/e2e-tests/src/binding/step-definitions/givenDefinitions.ts +++ b/packages/container/tools/e2e-tests/src/binding/step-definitions/givenDefinitions.ts @@ -199,6 +199,19 @@ Given( }, ); +Given( + 'a {warriorRelatedType} type binding as {string}', + function (warriorRelatedType: Newable, bindingAlias: string): void { + givenTypeBinding.bind(this)( + warriorRelatedType, + undefined, + undefined, + undefined, + bindingAlias, + ); + }, +); + Given( 'a {warriorRelatedType} type binding as {string} in {bindingScope} scope', function ( diff --git a/packages/container/tools/e2e-tests/src/binding/step-definitions/whenDefinitions.ts b/packages/container/tools/e2e-tests/src/binding/step-definitions/whenDefinitions.ts index 1ab4e75a..a052e4f0 100644 --- a/packages/container/tools/e2e-tests/src/binding/step-definitions/whenDefinitions.ts +++ b/packages/container/tools/e2e-tests/src/binding/step-definitions/whenDefinitions.ts @@ -1,10 +1,25 @@ import { When } from '@cucumber/cucumber'; +import { Newable, ServiceIdentifier } from '@inversifyjs/common'; +import { BindingActivation } from '@inversifyjs/core'; import { defaultAlias } from '../../common/models/defaultAlias'; import { InversifyWorld } from '../../common/models/InversifyWorld'; import { getContainerOrFail } from '../../container/calculations/getContainerOrFail'; import { getBindingOrFail } from '../calculations/getContainerOrFail'; +function whenActivationIsRegistered( + this: InversifyWorld, + activation: BindingActivation, + serviceIdentifier: ServiceIdentifier, + containerAlias?: string, +): void { + const parsedContainerAlias: string = containerAlias ?? defaultAlias; + + getContainerOrFail + .bind(this)(parsedContainerAlias) + .onActivation(serviceIdentifier, activation); +} + function whenBindingIsBound( this: InversifyWorld, bindingAlias?: string, @@ -28,3 +43,14 @@ When( whenBindingIsBound.bind(this)(bindingAlias); }, ); + +When( + 'a(n) {activation} activation is registered for {warriorRelatedType} type', + function (activation: BindingActivation, warriorRelatedType: Newable): void { + whenActivationIsRegistered.bind(this)( + activation, + warriorRelatedType, + undefined, + ); + }, +); diff --git a/packages/container/tools/e2e-tests/src/container/step-definitions/thenDefinitions.ts b/packages/container/tools/e2e-tests/src/container/step-definitions/thenDefinitions.ts index 4c38413a..1e8b264d 100644 --- a/packages/container/tools/e2e-tests/src/container/step-definitions/thenDefinitions.ts +++ b/packages/container/tools/e2e-tests/src/container/step-definitions/thenDefinitions.ts @@ -122,3 +122,14 @@ Then( assert.equal(value.damage, 12); }, ); + +Then( + 'value is a sword with improved damage \\(x{int}\\)', + function (times: number): void { + const value: unknown = + getContainerGetRequestOrFail.bind(this)(defaultAlias); + + assert.ok(value instanceof Sword); + assert.equal(value.damage, 10 + 2 * times); + }, +);