AttributeForwardingDetails are not deleted if associated Attribute is deleted (#822)

* feat: delete ForwardingDetails of associated attribute is deleted

* test: rename ForwardingDetails to AttributeForwardingDetails

* refactor: rename _deleteForwardingDetails

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Author: Milena-Czierlinski

packages/consumption/src/modules/attributes/AttributesController.ts

@@ -809,6 +809,8 @@ export class AttributesController extends ConsumptionBaseController {
         if (!attribute) throw TransportCoreErrors.general.recordNotFound(LocalAttribute, attributeId.toString());
 
         await this.deleteAttributeUnsafe(attributeId);
+        await this._deleteForwardingDetails({ attributeId: attribute.id.toString() });
+
         this.eventBus.publish(new AttributeDeletedEvent(this.identity.address.toString(), attribute));
     }
 
@@ -826,25 +828,28 @@ export class AttributesController extends ConsumptionBaseController {
             "@type": { $in: ["OwnIdentityAttribute", "OwnRelationshipAttribute", "PeerRelationshipAttribute"] }
         })) as (OwnIdentityAttribute | OwnRelationshipAttribute | PeerRelationshipAttribute)[];
         for (const attribute of forwardedAttributes) {
-            await this.removeForwardingDetailsFromAttribute(attribute, peer);
+            await this.deleteForwardingDetailsForAttribute(attribute, peer);
         }
     }
 
-    public async removeForwardingDetailsFromAttribute<T extends OwnIdentityAttribute | OwnRelationshipAttribute | PeerRelationshipAttribute>(
-        attribute: T,
-        peer: CoreAddress
-    ): Promise<T> {
-        const existingForwardingDetailsDocs = await this.forwardingDetails.find({ attributeId: attribute.id.toString(), peer: peer.toString() });
-        const existingForwardingDetails = existingForwardingDetailsDocs.map((obj) => AttributeForwardingDetails.from(obj));
+    public async deleteForwardingDetailsForAttribute(attribute: OwnIdentityAttribute | OwnRelationshipAttribute | PeerRelationshipAttribute, peer?: CoreAddress): Promise<void> {
+        const query: any = { attributeId: attribute.id.toString() };
+        if (peer) query["peer"] = peer.toString();
 
-        for (const entry of existingForwardingDetails) {
-            await this.forwardingDetails.delete(entry);
-        }
+        await this._deleteForwardingDetails(query);
 
         await this.updateNumberOfForwards(attribute);
 
         this.eventBus.publish(new AttributeForwardingDetailsChangedEvent(this.identity.address.toString(), attribute));
-        return attribute;
+    }
+
+    private async _deleteForwardingDetails(query: any): Promise<void> {
+        const existingForwardingDetailsDocs = await this.forwardingDetails.find(query);
+        const existingForwardingDetails = existingForwardingDetailsDocs.map((obj) => AttributeForwardingDetails.from(obj));
+
+        for (const entry of existingForwardingDetails) {
+            await this.forwardingDetails.delete(entry);
+        }
     }
 
     public async executeFullAttributeDeletionProcess(attribute: LocalAttribute): Promise<void> {
@@ -1566,7 +1571,6 @@ export class AttributesController extends ConsumptionBaseController {
 
     public async getForwardingDetailsForAttribute(attribute: LocalAttribute, query: any = {}): Promise<AttributeForwardingDetails[]> {
         const docs = await this.forwardingDetails.find({ ...query, attributeId: attribute.id.toString() });
-
         return docs.map((doc) => AttributeForwardingDetails.from(doc));
     }
 

packages/consumption/test/modules/attributes/AttributesController.test.ts

@@ -327,8 +327,8 @@ describe("AttributesController", function () {
         });
     });
 
-    describe("change ForwardingDetails of Attributes", function () {
-        test("should allow to add ForwardingDetails to an OwnIdentityAttribute", async function () {
+    describe("change AttributeForwardingDetails of Attributes", function () {
+        test("should allow to add AttributeForwardingDetails to an OwnIdentityAttribute", async function () {
             const attributeParams = {
                 content: IdentityAttribute.from({
                     value: {
@@ -350,7 +350,7 @@ describe("AttributesController", function () {
             expect(isForwarded).toBe(true);
         });
 
-        test("should allow to add ForwardingDetails to an OwnRelationshipAttribute", async function () {
+        test("should allow to add AttributeForwardingDetails to an OwnRelationshipAttribute", async function () {
             const thirdPartyAddress = CoreAddress.from("thirdPartyAdress");
             const peerAddress = CoreAddress.from("peerAddress");
 
@@ -381,7 +381,7 @@ describe("AttributesController", function () {
             expect(isForwarded).toBe(true);
         });
 
-        test("should allow to add ForwardingDetails to a PeerRelationshipAttribute", async function () {
+        test("should allow to add AttributeForwardingDetails to a PeerRelationshipAttribute", async function () {
             const thirdPartyAddress = CoreAddress.from("thirdPartyAdress");
             const peerAddress = CoreAddress.from("peerAddress");
 
@@ -412,7 +412,7 @@ describe("AttributesController", function () {
             expect(isForwarded).toBe(true);
         });
 
-        test("should publish an event adding ForwardingDetails to an Attribute", async function () {
+        test("should publish an event adding AttributeForwardingDetails to an Attribute", async function () {
             const attributeParams = {
                 content: IdentityAttribute.from({
                     value: {
@@ -431,7 +431,7 @@ describe("AttributesController", function () {
             mockEventBus.expectLastPublishedEvent(AttributeForwardingDetailsChangedEvent, forwardedAttribute);
         });
 
-        test("should throw trying to add ForwardingDetails to an Attribute if it is already forwarded", async function () {
+        test("should throw trying to add AttributeForwardingDetails to an Attribute if it is already forwarded", async function () {
             const attributeParams = {
                 content: IdentityAttribute.from({
                     value: {
@@ -449,7 +449,7 @@ describe("AttributesController", function () {
             );
         });
 
-        test("should allow to add ForwardingDetails to an Attribute if it is already forwarded but DeletedByRecipient", async function () {
+        test("should allow to add AttributeForwardingDetails to an Attribute if it is already forwarded but DeletedByRecipient", async function () {
             const attributeParams = {
                 content: IdentityAttribute.from({
                     value: {
@@ -472,7 +472,7 @@ describe("AttributesController", function () {
             expect(updatedAttribute.numberOfForwards).toBe(2);
         });
 
-        test("should updated ForwardingDetails of an Attribute if it is already forwarded but ToBeDeletedByRecipient", async function () {
+        test("should updated AttributeForwardingDetails of an Attribute if it is already forwarded but ToBeDeletedByRecipient", async function () {
             const attributeParams = {
                 content: IdentityAttribute.from({
                     value: {
@@ -498,7 +498,7 @@ describe("AttributesController", function () {
             expect(forwardingDetails[0].deletionInfo).toBeUndefined();
         });
 
-        test("should remove ForwardingDetails from an Attribute", async function () {
+        test("should delete AttributeForwardingDetails for an Attribute", async function () {
             const attributeParams = {
                 content: IdentityAttribute.from({
                     value: {
@@ -516,12 +516,13 @@ describe("AttributesController", function () {
             const isForwarded = await consumptionController.attributes.isAttributeForwardedToPeer(forwardedAttribute, peer);
             expect(isForwarded).toBe(true);
 
-            const updatedAttribute = await consumptionController.attributes.removeForwardingDetailsFromAttribute(forwardedAttribute, peer);
-            const updatedIsForwarded = await consumptionController.attributes.isAttributeForwardedToPeer(updatedAttribute, peer);
+            await consumptionController.attributes.deleteForwardingDetailsForAttribute(forwardedAttribute, peer);
+            const updatedAttribute = await consumptionController.attributes.getLocalAttribute(forwardedAttribute.id);
+            const updatedIsForwarded = await consumptionController.attributes.isAttributeForwardedToPeer(updatedAttribute!, peer);
             expect(updatedIsForwarded).toBe(false);
         });
 
-        test("should publish an event when removing ForwardingDetails from an Attribute", async function () {
+        test("should publish an event when removing AttributeForwardingDetails from an Attribute", async function () {
             const attributeParams = {
                 content: IdentityAttribute.from({
                     value: {
@@ -537,11 +538,12 @@ describe("AttributesController", function () {
             const forwardedAttribute = await consumptionController.attributes.addForwardingDetailsToAttribute(attribute, peer, CoreId.from("aSourceReferenceId"));
             mockEventBus.clearPublishedEvents();
 
-            const updatedAttribute = await consumptionController.attributes.removeForwardingDetailsFromAttribute(forwardedAttribute, peer);
+            await consumptionController.attributes.deleteForwardingDetailsForAttribute(forwardedAttribute, peer);
+            const updatedAttribute = (await consumptionController.attributes.getLocalAttribute(forwardedAttribute.id)) as OwnIdentityAttribute;
             mockEventBus.expectLastPublishedEvent(AttributeForwardingDetailsChangedEvent, updatedAttribute);
         });
 
-        test("should not change Attribute trying to remove ForwardingDetails from an Attribute without ForwardingDetails", async function () {
+        test("should not change Attribute trying to remove AttributeForwardingDetails from an Attribute without AttributeForwardingDetails", async function () {
             const attributeParams = {
                 content: IdentityAttribute.from({
                     value: {
@@ -554,7 +556,9 @@ describe("AttributesController", function () {
             const attributeWithoutForwardingDetails = await consumptionController.attributes.createOwnIdentityAttribute(attributeParams);
 
             const peer = CoreAddress.from("address");
-            const unchangedAttribute = await consumptionController.attributes.removeForwardingDetailsFromAttribute(attributeWithoutForwardingDetails, peer);
+            await consumptionController.attributes.deleteForwardingDetailsForAttribute(attributeWithoutForwardingDetails, peer);
+
+            const unchangedAttribute = await consumptionController.attributes.getLocalAttribute(attributeWithoutForwardingDetails.id);
             expect(unchangedAttribute).toStrictEqual(attributeWithoutForwardingDetails);
         });
     });
@@ -1044,6 +1048,26 @@ describe("AttributesController", function () {
             expect(peerAttribute).toBeUndefined();
         });
 
+        test("should delete AttributeForwardingDetails when deleting an Attribute", async function () {
+            const attribute = await consumptionController.attributes.createOwnIdentityAttribute({
+                content: IdentityAttribute.from({
+                    value: EMailAddress.from({
+                        value: "my@email.address"
+                    }),
+                    owner: consumptionController.accountController.identity.address
+                })
+            });
+            await consumptionController.attributes.addForwardingDetailsToAttribute(attribute, CoreAddress.from("aPeer"), CoreId.from("aSourceReference"));
+
+            const forwardingDetailsBeforeAttributeDeletion = await consumptionController.attributes.getForwardingDetailsForAttribute(attribute);
+            expect(forwardingDetailsBeforeAttributeDeletion).toHaveLength(1);
+
+            await consumptionController.attributes.deleteAttribute(attribute.id);
+
+            const forwardingDetailsAfterAttributeDeletion = await consumptionController.attributes.getForwardingDetailsForAttribute(attribute);
+            expect(forwardingDetailsAfterAttributeDeletion).toHaveLength(0);
+        });
+
         describe("should validate and execute full Attribute deletion process", function () {
             let predecessorOwnIdentityAttribute: OwnIdentityAttribute;
             let successorOwnIdentityAttribute: OwnIdentityAttribute;

packages/runtime/src/useCases/consumption/attributes/DeleteSharedAttributesForRejectedOrRevokedRelationship.ts

@@ -56,7 +56,7 @@ export class DeleteSharedAttributesForRejectedOrRevokedRelationshipUseCase exten
         )) as (OwnIdentityAttribute | OwnRelationshipAttribute | PeerRelationshipAttribute)[];
 
         for (const attribute of forwardedAttributes) {
-            await this.attributesController.removeForwardingDetailsFromAttribute(attribute, relationship.peer.address);
+            await this.attributesController.deleteForwardingDetailsForAttribute(attribute, relationship.peer.address);
         }
 
         await this.accountController.syncDatawallet();