Merge pull request #304 from inversify/fix/one-to-many-map-start-dele…

…tion-edge-case

Fix OneToManyMapStar deletion edge case

.changeset/bright-houses-enjoy.md

@@ -0,0 +1,5 @@
+---
+"@inversifyjs/core": patch
+---
+
+Updated `OneToManyMapStar` to fix an issue involving a deletion use case

.changeset/eight-papayas-hide.md

@@ -0,0 +1,5 @@
+---
+"@inversifyjs/container": patch
+---
+
+Updated `ActivationService` and `DeactivationService` to fix an issue involving a service deactivation edge case

packages/container/libraries/core/src/common/models/OneToManyMapStar.spec.ts

@@ -342,7 +342,6 @@ describe(OneToManyMapStar.name, () => {
       let modelFixture: unknown;
       let firstRelationFixture: Required<RelationTest>;
       let secondRelationFixture: Required<RelationTest>;
-      let oneToManyMapStar: OneToManyMapStar<unknown, RelationTest>;
 
       beforeAll(() => {
         modelFixture = Symbol();
@@ -354,21 +353,24 @@ describe(OneToManyMapStar.name, () => {
           bar: 4,
           foo: firstRelationFixture[RelationKey.foo],
         };
-        oneToManyMapStar = new OneToManyMapStar<unknown, RelationTest>({
-          bar: {
-            isOptional: true,
-          },
-          foo: {
-            isOptional: false,
-          },
-        });
-
-        oneToManyMapStar.add(modelFixture, firstRelationFixture);
-        oneToManyMapStar.add(modelFixture, secondRelationFixture);
       });
 
-      describe('when called', () => {
+      describe('when called, with a value asociated to a single model', () => {
+        let oneToManyMapStar: OneToManyMapStar<unknown, RelationTest>;
+
         beforeAll(() => {
+          oneToManyMapStar = new OneToManyMapStar<unknown, RelationTest>({
+            bar: {
+              isOptional: true,
+            },
+            foo: {
+              isOptional: false,
+            },
+          });
+
+          oneToManyMapStar.add(modelFixture, firstRelationFixture);
+          oneToManyMapStar.add(modelFixture, secondRelationFixture);
+
           oneToManyMapStar.removeByRelation(
             RelationKey.bar,
             firstRelationFixture[RelationKey.bar],
@@ -409,6 +411,127 @@ describe(OneToManyMapStar.name, () => {
           });
         });
       });
+
+      describe('when called, with a value asociated to both models', () => {
+        let oneToManyMapStar: OneToManyMapStar<unknown, RelationTest>;
+
+        beforeAll(() => {
+          oneToManyMapStar = new OneToManyMapStar<unknown, RelationTest>({
+            bar: {
+              isOptional: true,
+            },
+            foo: {
+              isOptional: false,
+            },
+          });
+
+          oneToManyMapStar.add(modelFixture, firstRelationFixture);
+          oneToManyMapStar.add(modelFixture, secondRelationFixture);
+
+          oneToManyMapStar.removeByRelation(
+            RelationKey.foo,
+            firstRelationFixture[RelationKey.foo],
+          );
+        });
+
+        describe('when called .get()', () => {
+          let results: {
+            [TKey in RelationKey]-?: Iterable<unknown> | 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<unknown> | undefined;
+            } = {
+              [RelationKey.bar]: undefined,
+              [RelationKey.foo]: undefined,
+            };
+
+            expect(results).toStrictEqual(expectedResults);
+          });
+        });
+      });
+    });
+
+    describe('having a OneToManyMapStart with two models with the same relation', () => {
+      let firstModelFixture: unknown;
+      let secondModelFixture: unknown;
+      let relationFixture: Required<RelationTest>;
+
+      beforeAll(() => {
+        firstModelFixture = Symbol();
+        secondModelFixture = Symbol();
+        relationFixture = {
+          bar: 3,
+          foo: 'foo',
+        };
+      });
+
+      describe('when called', () => {
+        let oneToManyMapStar: OneToManyMapStar<unknown, RelationTest>;
+
+        beforeAll(() => {
+          oneToManyMapStar = new OneToManyMapStar<unknown, RelationTest>({
+            bar: {
+              isOptional: true,
+            },
+            foo: {
+              isOptional: false,
+            },
+          });
+
+          oneToManyMapStar.add(firstModelFixture, relationFixture);
+          oneToManyMapStar.add(secondModelFixture, relationFixture);
+
+          oneToManyMapStar.removeByRelation(
+            RelationKey.bar,
+            relationFixture[RelationKey.bar],
+          );
+        });
+
+        describe('when called .get()', () => {
+          let results: {
+            [TKey in RelationKey]-?: Iterable<unknown> | undefined;
+          };
+
+          beforeAll(() => {
+            results = {
+              [RelationKey.bar]: oneToManyMapStar.get(
+                RelationKey.bar,
+                relationFixture[RelationKey.bar],
+              ),
+              [RelationKey.foo]: oneToManyMapStar.get(
+                RelationKey.foo,
+                relationFixture[RelationKey.foo],
+              ),
+            };
+          });
+
+          it('should return expected results', () => {
+            const expectedResults: {
+              [TKey in RelationKey]-?: Iterable<unknown> | undefined;
+            } = {
+              [RelationKey.bar]: undefined,
+              [RelationKey.foo]: undefined,
+            };
+
+            expect(results).toStrictEqual(expectedResults);
+          });
+        });
+      });
     });
   });
 

packages/container/libraries/core/src/common/models/OneToManyMapStar.ts

@@ -40,7 +40,7 @@ export class OneToManyMapStar<TModel, TRelation extends object>
     for (const relationKey of Reflect.ownKeys(
       relation,
     ) as (keyof TRelation)[]) {
-      this.#buildOrGetRelationModelSet(relationKey, relation[relationKey]).push(
+      this.#buildOrGetRelationModels(relationKey, relation[relationKey]).push(
         model,
       );
     }
@@ -93,7 +93,9 @@ export class OneToManyMapStar<TModel, TRelation extends object>
       return;
     }
 
-    for (const model of models) {
+    const uniqueModelsSet: Set<TModel> = new Set(models);
+
+    for (const model of uniqueModelsSet) {
       const relations: TRelation[] | undefined =
         this.#modelToRelationMap.get(model);
 
@@ -124,20 +126,20 @@ export class OneToManyMapStar<TModel, TRelation extends object>
     return relations;
   }
 
-  #buildOrGetRelationModelSet<TKey extends keyof TRelation>(
+  #buildOrGetRelationModels<TKey extends keyof TRelation>(
     relationKey: TKey,
     relationValue: TRelation[TKey],
   ): TModel[] {
-    let modelSet: TModel[] | undefined =
+    let models: TModel[] | undefined =
       this.#relationToModelsMaps[relationKey].get(relationValue);
 
-    if (modelSet === undefined) {
-      modelSet = [];
+    if (models === undefined) {
+      models = [];
 
-      this.#relationToModelsMaps[relationKey].set(relationValue, modelSet);
+      this.#relationToModelsMaps[relationKey].set(relationValue, models);
     }
 
-    return modelSet;
+    return models;
   }
 
   #pushEntriesIntoMap<TKey, TValue>(
@@ -166,17 +168,17 @@ export class OneToManyMapStar<TModel, TRelation extends object>
     relationKey: TKey,
     relationValue: TRelation[TKey],
   ): void {
-    const modelSet: TModel[] | undefined =
+    const relationModels: TModel[] | undefined =
       this.#relationToModelsMaps[relationKey].get(relationValue);
 
-    if (modelSet !== undefined) {
-      const index: number = modelSet.indexOf(model);
+    if (relationModels !== undefined) {
+      const index: number = relationModels.indexOf(model);
 
       if (index !== NOT_FOUND_INDEX) {
-        modelSet.splice(index, 1);
+        relationModels.splice(index, 1);
       }
 
-      if (modelSet.length === 0) {
+      if (relationModels.length === 0) {
         this.#relationToModelsMaps[relationKey].delete(relationValue);
       }
     }