From e4995a713f1e8e7b895ecbdac91404b80884677e Mon Sep 17 00:00:00 2001 From: Fabio Pinheiro Date: Tue, 1 Aug 2023 12:59:46 +0100 Subject: [PATCH] fix: UserAccountRepo.createOrFindDidAccount (#69) Second mediation request cause failure PRISM Mediator with DatabaseException Fix UserAccountRepo.newDidAccount For ATL-5408 Signed-off-by: Fabio Pinheiro Signed-off-by: Shailesh Patil --- .../atala/mediator/db/BsonImplicits.scala | 7 ++- .../atala/mediator/db/UserAccountRepo.scala | 47 +++++++++++++++---- .../MediatorCoordinationExecuter.scala | 12 +++-- .../mediator/db/UserAccountRepoSpec.scala | 22 ++++++--- 4 files changed, 67 insertions(+), 21 deletions(-) diff --git a/mediator/src/main/scala/io/iohk/atala/mediator/db/BsonImplicits.scala b/mediator/src/main/scala/io/iohk/atala/mediator/db/BsonImplicits.scala index 3976f093..29a2a9e0 100644 --- a/mediator/src/main/scala/io/iohk/atala/mediator/db/BsonImplicits.scala +++ b/mediator/src/main/scala/io/iohk/atala/mediator/db/BsonImplicits.scala @@ -20,7 +20,12 @@ given BSONReader[DIDSubject] with { given BSONWriter[DID] with { import DID.* - def writeTry(obj: DID): Try[BSONValue] = Try(BSONString(obj.string)) + def writeTry(obj: DID): Try[BSONValue] = { + println("_" * 100) + println(obj.did) + println("^" * 100) + Try(BSONString(obj.did)) + } } given BSONReader[DID] with { 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 d78b3d1b..4753fc6a 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 @@ -29,20 +29,49 @@ class UserAccountRepo(reactiveMongoApi: ReactiveMongoApi)(using ec: ExecutionCon .tapError(err => ZIO.logError(s"Couldn't get collection ${err.getMessage}")) .mapError(ex => StorageCollection(ex)) - def newDidAccount(did: DIDSubject): IO[StorageError, WriteResult] = { - val value = DidAccount( - did = did, - alias = Seq(did), - messagesRef = Seq.empty + /** create or return account for a DIDSubject */ + def createOrFindDidAccount(did: DIDSubject): IO[StorageError, Either[String, DidAccount]] = { + def projection: Option[BSONDocument] = None + def selectorConditionToInsert = BSONDocument( + Seq( + "$or" -> BSONArray( + BSONDocument(Seq("did" -> BSONString(did.did))), + BSONDocument(Seq("alias" -> BSONString(did.did))) // TODO test + ) + ) ) + for { _ <- ZIO.logInfo("newDidAccount") coll <- collection - result <- ZIO - .fromFuture(implicit ec => coll.insert.one(value)) - .tapError(err => ZIO.logError(s"Insert newDidAccount : ${err.getMessage}")) + findR <- ZIO // TODO this should be atomic + .fromFuture(implicit ec => + coll + .find(selectorConditionToInsert, projection) + .cursor[DidAccount]() + .collect[Seq](1, Cursor.FailOnError[Seq[DidAccount]]()) // Just one + .map(_.headOption) + ) + .tapError(err => ZIO.logError(s"Insert newDidAccount (check condition setp): ${err.getMessage}")) .mapError(ex => StorageThrowable(ex)) - + result <- findR match + case Some(data) if data.did != did => ZIO.left("Fail found document: " + data) + case Some(old) => ZIO.right(old) + case None => + val value = DidAccount( + did = did, + alias = Seq(did), + messagesRef = Seq.empty + ) + ZIO + .fromFuture(implicit ec => coll.insert.one(value)) + .map(e => + e.n match + case 1 => Right(value) + case _ => Left("Fail to inserte: " + e.toString()) + ) + .tapError(err => ZIO.logError(s"Insert newDidAccount : ${err.getMessage}")) + .mapError(ex => StorageThrowable(ex)) } yield result } 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 ddcfe424..d04db5af 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 @@ -11,6 +11,7 @@ import io.iohk.atala.mediator.actions.* import io.iohk.atala.mediator.db.UserAccountRepo import zio.* import zio.json.* +import io.iohk.atala.mediator.db.DidAccount object MediatorCoordinationExecuter extends ProtocolExecuterWithServices[ProtocolExecuter.Services & UserAccountRepo] { override def suportedPIURI: Seq[PIURI] = Seq( @@ -50,10 +51,13 @@ object MediatorCoordinationExecuter extends ProtocolExecuterWithServices[Protoco for { _ <- ZIO.logInfo("MediateRequest") repo <- ZIO.service[UserAccountRepo] - result <- repo.newDidAccount(m.from.asDIDURL.toDID) - reply = result.n match - case 1 => m.makeRespondMediateGrant.toPlaintextMessage - case _ => m.makeRespondMediateDeny.toPlaintextMessage + result: Either[String, DidAccount] <- repo.createOrFindDidAccount(m.from.asDIDURL.toDID) + reply <- result match + case Left(errorStr) => + ZIO.log(s"MediateDeny: $errorStr") *> ZIO.succeed(m.makeRespondMediateDeny.toPlaintextMessage) + case Right(value) => + ZIO.log(s"MediateGrant: $value") *> + ZIO.succeed(m.makeRespondMediateGrant.toPlaintextMessage) } yield SyncReplyOnly(reply) case m: KeylistUpdate => for { 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 8854d0c9..f5693e54 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 @@ -30,18 +30,17 @@ object UserAccountRepoSpec extends ZIOSpecDefault with AccountStubSetup { userAccount <- ZIO.service[UserAccountRepo] col <- userAccount.collection _ = col.indexesManager.create(index) - result <- userAccount.newDidAccount(DIDSubject(alice)) + result <- userAccount.createOrFindDidAccount(DIDSubject(alice)) } yield { - assertTrue(result.writeErrors == Nil) - assertTrue(result.n == 1) + assertTrue(result.isRight) } }, - test("insert same Did should fail") { + test("insert same Did should NOT fail") { for { userAccount <- ZIO.service[UserAccountRepo] - result <- userAccount.newDidAccount(DIDSubject(alice)).exit + result <- userAccount.createOrFindDidAccount(DIDSubject(alice)) } yield { - assert(result)(fails(isSubtype[StorageError](anything))) + assertTrue(result.isRight) } }, test("Get Did Account") { @@ -74,6 +73,14 @@ object UserAccountRepoSpec extends ZIOSpecDefault with AccountStubSetup { assertTrue(alias == Seq(alice, bob)) } }, + test("insert/create a UserAccount for a DID that is used as a alias should fail") { + for { + userAccount <- ZIO.service[UserAccountRepo] + result <- userAccount.createOrFindDidAccount(DIDSubject(bob)) + } yield { + assertTrue(result.isLeft) + } + }, test("Add same alias to existing Did Account return right with nModified value 0") { for { userAccount <- ZIO.service[UserAccountRepo] @@ -97,10 +104,11 @@ object UserAccountRepoSpec extends ZIOSpecDefault with AccountStubSetup { 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") { + 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))