fix: UserAccountRepo.createOrFindDidAccount (#69)

Second mediation request cause failure PRISM Mediator with DatabaseException Fix UserAccountRepo.newDidAccount For ATL-5408
hyperledger · Aug 1, 2023 · 3526f0a · 3526f0a
1 parent 3db00c2
commit 3526f0a
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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))