fix : mediator add to alias should not store duplicated alias (#19)

* add to alias is a set, and not duplicates fixed ussing mongo addtoset * Added tests and Updated the reposne to nochnage in case of the noupdate was done --------- Co-authored-by: Shailesh Patil <[email protected]> Signed-off-by: Shailesh Patil <[email protected]> Signed-off-by: Shailesh Patil <[email protected]>
hyperledger · May 1, 2024 · a9cdd03 · a9cdd03
1 parent d9c6926
commit a9cdd03
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 10 deletions.
diff --git a/mediator/src/main/scala/io/iohk/atala/mediator/db/UserAccountRepo.scala b/mediator/src/main/scala/io/iohk/atala/mediator/db/UserAccountRepo.scala
@@ -66,11 +66,11 @@ class UserAccountRepo(reactiveMongoApi: ReactiveMongoApi)(using ec: ExecutionCon
 
   }
 
-  def addAlias(owner: DIDSubject, newAlias: DIDSubject): ZIO[Any, StorageError, Either[String, Unit]] = {
+  def addAlias(owner: DIDSubject, newAlias: DIDSubject): ZIO[Any, StorageError, Either[String, Int]] = {
     def selector: BSONDocument = BSONDocument("did" -> owner)
 
     def update: BSONDocument = BSONDocument(
-      "$push" -> BSONDocument(
+      "$addToSet" -> BSONDocument(
         "alias" -> newAlias
       )
     )
@@ -85,11 +85,11 @@ class UserAccountRepo(reactiveMongoApi: ReactiveMongoApi)(using ec: ExecutionCon
         )
         .tapError(err => ZIO.logError(s"addAlias :  ${err.getMessage}"))
         .mapError(ex => StorageThrowable(ex))
-    } yield Right(())
+    } yield Right(result.nModified)
 
   }
 
-  def removeAlias(owner: DIDSubject, newAlias: DIDSubject): ZIO[Any, StorageError, Either[String, Unit]] = {
+  def removeAlias(owner: DIDSubject, newAlias: DIDSubject): ZIO[Any, StorageError, Either[String, Int]] = {
     def selector: BSONDocument = BSONDocument("did" -> owner)
     def update: BSONDocument = BSONDocument(
       "$pull" -> BSONDocument(
@@ -107,7 +107,7 @@ class UserAccountRepo(reactiveMongoApi: ReactiveMongoApi)(using ec: ExecutionCon
         )
         .tapError(err => ZIO.logError(s"removeAlias :  ${err.getMessage}"))
         .mapError(ex => StorageThrowable(ex))
-    } yield Right(())
+    } yield Right(result.nModified)
 
   }
 

diff --git a/mediator/src/main/scala/io/iohk/atala/mediator/protocols/MediatorCoordinationExecuter.scala b/mediator/src/main/scala/io/iohk/atala/mediator/protocols/MediatorCoordinationExecuter.scala
@@ -63,11 +63,13 @@ object MediatorCoordinationExecuter extends ProtocolExecuterWithServices[Protoco
             case (fromto, KeylistAction.add) =>
               repo.addAlias(m.from.toDID, fromto.toDID).map {
                 case Left(value)     => (fromto, KeylistAction.add, KeylistResult.server_error)
+                case Right(0)        => (fromto, KeylistAction.add, KeylistResult.no_change)
                 case Right(newState) => (fromto, KeylistAction.add, KeylistResult.success)
               }
             case (fromto, KeylistAction.remove) =>
               repo.removeAlias(m.from.toDID, fromto.toDID).map {
                 case Left(value)     => (fromto, KeylistAction.remove, KeylistResult.server_error)
+                case Right(0)        => (fromto, KeylistAction.remove, KeylistResult.no_change)
                 case Right(newState) => (fromto, KeylistAction.remove, KeylistResult.success)
               }
           }

diff --git a/mediator/src/test/scala/io/iohk/atala/mediator/db/UserAccountRepoSpec.scala b/mediator/src/test/scala/io/iohk/atala/mediator/db/UserAccountRepoSpec.scala
@@ -61,27 +61,53 @@ object UserAccountRepoSpec extends ZIOSpecDefault with AccountStubSetup {
         assertTrue(result.isEmpty)
       }
     },
-    test("Add alias to existing Did Account return right") {
+    test("Add alias to existing Did Account return right nModified value 1") {
       for {
         userAccount <- ZIO.service[UserAccountRepo]
         result <- userAccount.addAlias(DIDSubject(alice), DIDSubject(bob))
         didAccount <- userAccount.getDidAccount(DIDSubject(alice))
       } yield {
         assertTrue(result.isRight)
-        assertTrue(result == Right(()))
+        assertTrue(result == Right((1)))
         assertTrue(didAccount.isDefined)
         val alias: Seq[String] = didAccount.map(_.alias.map(_.did)).getOrElse(Seq.empty)
         assertTrue(alias == Seq(alice, bob))
       }
     },
-    test("Remove alias to existing Did Account should return right") {
+    test("Add same alias to existing Did Account return right with nModified value 0") {
+      for {
+        userAccount <- ZIO.service[UserAccountRepo]
+        result <- userAccount.addAlias(DIDSubject(alice), DIDSubject(alice))
+        didAccount <- userAccount.getDidAccount(DIDSubject(alice))
+      } yield {
+        assertTrue(result.isRight)
+        assertTrue(result == Right(0))
+        assertTrue(didAccount.isDefined)
+        val alias: Seq[String] = didAccount.map(_.alias.map(_.did)).getOrElse(Seq.empty)
+        assertTrue(alias == Seq(alice, bob))
+      }
+    },
+    test("Remove alias to existing Did Account should return right with nModified value 1 ") {
+      for {
+        userAccount <- ZIO.service[UserAccountRepo]
+        result <- userAccount.removeAlias(DIDSubject(alice), DIDSubject(bob))
+        didAccount <- userAccount.getDidAccount(DIDSubject(alice))
+      } yield {
+        assertTrue(result.isRight)
+        assertTrue(result == Right(1))
+        assertTrue(didAccount.isDefined)
+        val alias: Seq[String] = didAccount.map(_.alias.map(_.did)).getOrElse(Seq.empty)
+        assertTrue(alias == Seq(alice))
+      }
+    },
+    test("Remove alias to unknown or unregister alias Did  should return right with noModified value 0") {
       for {
         userAccount <- ZIO.service[UserAccountRepo]
         result <- userAccount.removeAlias(DIDSubject(alice), DIDSubject(bob))
         didAccount <- userAccount.getDidAccount(DIDSubject(alice))
       } yield {
         assertTrue(result.isRight)
-        assertTrue(result == Right(()))
+        assertTrue(result == Right(0))
         assertTrue(didAccount.isDefined)
         val alias: Seq[String] = didAccount.map(_.alias.map(_.did)).getOrElse(Seq.empty)
         assertTrue(alias == Seq(alice))
@@ -98,7 +124,7 @@ object UserAccountRepoSpec extends ZIOSpecDefault with AccountStubSetup {
         didAccount <- userAccount.getDidAccount(DIDSubject(alice))
       } yield {
         assertTrue(result.isRight)
-        assertTrue(result == Right(()))
+        assertTrue(result == Right(1))
         assertTrue(msgAdded.writeErrors == Nil)
         assertTrue(msgAdded.n == 1)
         assertTrue(addedToInbox == 1)