Remove user-management error cases from scenario-service proto. (#12460)

* Remove user-management error cases from scenario-service proto. changelog_begin changelog_end * remove Error.UserManagement * remove UserManagememtError * simp * meh... done over by scala's poor excuse for a type system
digital-asset · Jan 19, 2022 · 6fbaac4 · 6fbaac4
1 parent 1f1e8bf
commit 6fbaac4
Show file tree

Hide file tree

Showing 9 changed files with 70 additions and 129 deletions.
diff --git a/compiler/scenario-service/client/src/DA/Daml/LF/PrettyScenario.hs b/compiler/scenario-service/client/src/DA/Daml/LF/PrettyScenario.hs
@@ -354,11 +354,6 @@ prettyScenarioErrorError (Just err) =  do
     ScenarioErrorErrorScenarioPartyAlreadyExists name ->
       pure $ "Tried to allocate a party that already exists: " <-> ltext name
 
-    ScenarioErrorErrorScenarioUserNotFound userId ->
-      pure $ "User not found: " <-> ltext userId
-    ScenarioErrorErrorScenarioUserAlreadyExists userId ->
-      pure $ "User already exists: " <-> ltext userId
-
     ScenarioErrorErrorScenarioContractNotVisible ScenarioError_ContractNotVisible{..} ->
       pure $ vcat
         [ "Attempt to fetch or exercise a contract not visible to the reading parties."

diff --git a/compiler/scenario-service/protos/scenario_service.proto b/compiler/scenario-service/protos/scenario_service.proto
@@ -279,8 +279,6 @@ message ScenarioError {
     Empty ComparableValueError = 29;
     ContractIdInContractKey contract_id_in_contract_key = 30;
     Empty ValueExceedsMaxNesting = 31;
-    string scenario_user_not_found = 33;
-    string scenario_user_already_exists = 34;
   }
 }
 

diff --git a/...cenario-service/server/src/main/scala/com/digitalasset/daml/lf/scenario/Conversions.scala b/...cenario-service/server/src/main/scala/com/digitalasset/daml/lf/scenario/Conversions.scala
@@ -249,14 +249,6 @@ final class Conversions(
 
       case Error.PartyAlreadyExists(party) =>
         builder.setScenarioPartyAlreadyExists(party)
-
-      case Error.UserManagement(err) =>
-        err match {
-          case Error.UserManagementError.UserNotFound(userId) =>
-            builder.setScenarioUserNotFound(userId)
-          case Error.UserManagementError.UserExists(userId) =>
-            builder.setScenarioUserAlreadyExists(userId)
-        }
     }
     builder.build
   }

diff --git a/daml-lf/scenario-interpreter/src/main/scala/com/digitalasset/daml/lf/Error.scala b/daml-lf/scenario-interpreter/src/main/scala/com/digitalasset/daml/lf/Error.scala
@@ -4,7 +4,7 @@
 package com.daml.lf
 package scenario
 
-import com.daml.lf.data.Ref.{Identifier, Party, UserId}
+import com.daml.lf.data.Ref.{Identifier, Party}
 import com.daml.lf.data.Time
 import com.daml.lf.ledger.EventId
 import com.daml.lf.speedy.SError.SError
@@ -74,13 +74,4 @@ object Error {
 
   /** Tried to allocate a party that already exists. */
   final case class PartyAlreadyExists(name: String) extends Error
-
-  final case class UserManagement(error: UserManagementError) extends Error
-
-  sealed abstract class UserManagementError extends Product with Serializable
-
-  object UserManagementError {
-    final case class UserNotFound(userId: UserId) extends UserManagementError
-    final case class UserExists(userId: UserId) extends UserManagementError
-  }
 }
diff --git a/daml-lf/scenario-interpreter/src/main/scala/com/digitalasset/daml/lf/Pretty.scala b/daml-lf/scenario-interpreter/src/main/scala/com/digitalasset/daml/lf/Pretty.scala
@@ -66,15 +66,6 @@ private[lf] object Pretty {
 
       case Error.PartyAlreadyExists(party) =>
         text(s"Error: Tried to allocate a party that already exists: $party")
-
-      case Error.UserManagement(err) => prettyError(err)
     }
 
-  def prettyError(err: Error.UserManagementError) = err match {
-    case Error.UserManagementError.UserNotFound(userId) =>
-      text(s"Error: User with id $userId does not exist")
-    case Error.UserManagementError.UserExists(userId) =>
-      text(s"Error: Tried to create a user id $userId but such a user already exists")
-  }
-
 }
diff --git a/daml-script/runner/BUILD.bazel b/daml-script/runner/BUILD.bazel
@@ -69,7 +69,6 @@ da_scala_test(
     deps = [
         ":script-runner-lib",
         "//daml-lf/data",
-        "//daml-lf/scenario-interpreter",
         "//language-support/scala/bindings",
         "//language-support/scala/bindings-akka",
     ],

diff --git a/...main/scala/com/digitalasset/daml/lf/engine/script/ledgerinteraction/IdeLedgerClient.scala b/...main/scala/com/digitalasset/daml/lf/engine/script/ledgerinteraction/IdeLedgerClient.scala
@@ -58,9 +58,6 @@ class IdeLedgerClient(
 
   private val userManagementStore = new ide.UserManagementStore()
 
-  private def handleUserManagement[T](r: ide.UserManagementStore.Result[T]): Future[T] =
-    r.fold(err => Future.failed(scenario.Error.UserManagement(err)), Future.successful(_))
-
   override def query(parties: OneAnd[Set, Ref.Party], templateId: Identifier)(implicit
       ec: ExecutionContext,
       mat: Materializer,
@@ -323,37 +320,28 @@ class IdeLedgerClient(
       esf: ExecutionSequencerFactory,
       mat: Materializer,
   ): Future[Option[Unit]] =
-    userManagementStore.createUser(user, rights.toSet) match {
-      case Left(scenario.Error.UserManagementError.UserExists(_)) => Future.successful(None)
-      case a => handleUserManagement(a).map(Some(_))
-    }
+    Future.successful(userManagementStore.createUser(user, rights.toSet))
 
   override def getUser(id: UserId)(implicit
       ec: ExecutionContext,
       esf: ExecutionSequencerFactory,
       mat: Materializer,
   ): Future[Option[User]] =
-    userManagementStore.getUser(id) match {
-      case Left(scenario.Error.UserManagementError.UserNotFound(_)) => Future.successful(None)
-      case a => handleUserManagement(a).map(Some(_))
-    }
+    Future.successful(userManagementStore.getUser(id))
 
   override def deleteUser(id: UserId)(implicit
       ec: ExecutionContext,
       esf: ExecutionSequencerFactory,
       mat: Materializer,
   ): Future[Option[Unit]] =
-    userManagementStore.deleteUser(id) match {
-      case Left(scenario.Error.UserManagementError.UserNotFound(_)) => Future.successful(None)
-      case a => handleUserManagement(a).map(Some(_))
-    }
+    Future.successful(userManagementStore.deleteUser(id))
 
   override def listUsers()(implicit
       ec: ExecutionContext,
       esf: ExecutionSequencerFactory,
       mat: Materializer,
   ): Future[List[User]] =
-    handleUserManagement(userManagementStore.listUsers())
+    Future.successful(userManagementStore.listUsers())
 
   override def grantUserRights(
       id: UserId,
@@ -363,10 +351,7 @@ class IdeLedgerClient(
       esf: ExecutionSequencerFactory,
       mat: Materializer,
   ): Future[Option[List[UserRight]]] =
-    userManagementStore.grantRights(id, rights.toSet) match {
-      case Left(scenario.Error.UserManagementError.UserNotFound(_)) => Future.successful(None)
-      case a => handleUserManagement(a).map(_.toList).map(Some(_))
-    }
+    Future.successful(userManagementStore.grantRights(id, rights.toSet).map(_.toList))
 
   override def revokeUserRights(
       id: UserId,
@@ -376,18 +361,12 @@ class IdeLedgerClient(
       esf: ExecutionSequencerFactory,
       mat: Materializer,
   ): Future[Option[List[UserRight]]] =
-    userManagementStore.revokeRights(id, rights.toSet) match {
-      case Left(scenario.Error.UserManagementError.UserNotFound(_)) => Future.successful(None)
-      case a => handleUserManagement(a).map(_.toList).map(Some(_))
-    }
+    Future.successful(userManagementStore.revokeRights(id, rights.toSet).map(_.toList))
 
   override def listUserRights(id: UserId)(implicit
       ec: ExecutionContext,
       esf: ExecutionSequencerFactory,
       mat: Materializer,
   ): Future[Option[List[UserRight]]] =
-    userManagementStore.listUserRights(id) match {
-      case Left(scenario.Error.UserManagementError.UserNotFound(_)) => Future.successful(None)
-      case a => handleUserManagement(a).map(_.toList).map(Some(_))
-    }
+    Future.successful(userManagementStore.listUserRights(id).map(_.toList))
 }
diff --git a/...la/com/digitalasset/daml/lf/engine/script/ledgerinteraction/ide/UserManagementStore.scala b/...la/com/digitalasset/daml/lf/engine/script/ledgerinteraction/ide/UserManagementStore.scala
@@ -4,7 +4,6 @@
 package com.daml.lf.engine.script.ledgerinteraction.ide
 
 import com.daml.lf.data.Ref.UserId
-import com.daml.lf.scenario.Error.UserManagementError
 import com.daml.ledger.api.domain.{User, UserRight}
 
 import scala.collection.mutable
@@ -15,55 +14,54 @@ import scala.collection.mutable
 // #11896. While it would be nice to not have to duplicate this for now this seems like the
 // simpler option than trying to reuse participant code in the script service.
 private[ledgerinteraction] class UserManagementStore {
-  import UserManagementError._
   import UserManagementStore._
 
-  def createUser(user: User, rights: Set[UserRight]): Result[Unit] =
+  def createUser(user: User, rights: Set[UserRight]): Option[Unit] =
     putIfAbsent(UserInfo(user, rights)) match {
-      case Some(_) => Left(UserExists(user.id))
-      case None => Right(())
+      case Some(_) => None
+      case None => Some(())
     }
 
-  def getUser(id: UserId): Result[User] =
+  def getUser(id: UserId): Option[User] =
     lookup(id) match {
-      case Some(userInfo) => Right(userInfo.user)
-      case None => Left(UserNotFound(id))
+      case Some(userInfo) => Some(userInfo.user)
+      case None => None
     }
 
-  def deleteUser(id: UserId): Result[Unit] =
+  def deleteUser(id: UserId): Option[Unit] =
     dropExisting(id) match {
-      case Some(_) => Right(())
-      case None => Left(UserNotFound(id))
+      case Some(_) => Some(())
+      case None => None
     }
 
-  def grantRights(id: UserId, granted: Set[UserRight]): Result[Set[UserRight]] =
+  def grantRights(id: UserId, granted: Set[UserRight]): Option[Set[UserRight]] =
     lookup(id) match {
       case Some(userInfo) =>
         val newlyGranted = granted.diff(userInfo.rights)
         replaceInfo(userInfo, userInfo.copy(rights = userInfo.rights ++ newlyGranted))
-        Right(newlyGranted)
+        Some(newlyGranted)
       case None =>
-        Left(UserNotFound(id))
+        None
     }
 
-  def revokeRights(id: UserId, revoked: Set[UserRight]): Result[Set[UserRight]] =
+  def revokeRights(id: UserId, revoked: Set[UserRight]): Option[Set[UserRight]] =
     lookup(id) match {
       case Some(userInfo) =>
         val effectivelyRevoked = revoked.intersect(userInfo.rights)
         replaceInfo(userInfo, userInfo.copy(rights = userInfo.rights -- effectivelyRevoked))
-        Right(effectivelyRevoked)
+        Some(effectivelyRevoked)
       case None =>
-        Left(UserNotFound(id))
+        None
     }
 
-  def listUserRights(id: UserId): Result[Set[UserRight]] =
+  def listUserRights(id: UserId): Option[Set[UserRight]] =
     lookup(id) match {
-      case Some(userInfo) => Right(userInfo.rights)
-      case None => Left(UserNotFound(id))
+      case Some(userInfo) => Some(userInfo.rights)
+      case None => None
     }
 
-  def listUsers(): Result[List[User]] =
-    Right(state.values.map(_.user).toList)
+  def listUsers(): List[User] =
+    state.values.map(_.user).toList
 
   private val state: mutable.Map[UserId, UserInfo] = mutable.Map()
   private def lookup(id: UserId) = state.get(id)
@@ -89,5 +87,4 @@ object UserManagementStore {
   case class UserInfo(user: User, rights: Set[UserRight]) {
     def toStateEntry: (UserId, UserInfo) = user.id -> this
   }
-  type Result[T] = Either[UserManagementError, T]
 }