From a9cdd0370325219bc92cc2b47de00fcbaa77f12f Mon Sep 17 00:00:00 2001 From: Shailesh Patil Date: Tue, 27 Jun 2023 11:12:56 +0100 Subject: [PATCH] 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 Signed-off-by: Shailesh Patil Signed-off-by: Shailesh Patil --- .../atala/mediator/db/UserAccountRepo.scala | 10 +++--- .../MediatorCoordinationExecuter.scala | 2 ++ .../mediator/db/UserAccountRepoSpec.scala | 36 ++++++++++++++++--- 3 files changed, 38 insertions(+), 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 index f7dee615..94cbc422 100644 --- 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 index c107e1c6..acbe2892 100644 --- 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 index d04a116b..038e7362 100644 --- 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)