[User management] Add CreateUserResponse and GetUserResponse gRPC res…

…ponse wrappers [DPP-854] (digital-asset#12682) changelog_begin Ledger API Specification: CreateUser and GetUser endpoints of UserManagementService now return the CreateUserResponse or GetUserResponse messages, whereas previously they were returning the User message). changelog_end
TuranDev · Feb 8, 2022 · f2b7902 · f2b7902
1 parent 7547c20
commit f2b7902
Show file tree

Hide file tree

Showing 11 changed files with 66 additions and 35 deletions.
diff --git a/...a/bindings-rxjava/src/main/java/com/daml/ledger/rxjava/grpc/UserManagementClientImpl.java b/...a/bindings-rxjava/src/main/java/com/daml/ledger/rxjava/grpc/UserManagementClientImpl.java
@@ -36,7 +36,7 @@ private Single<User> createUser(
             StubHelper.authenticating(this.serviceFutureStub, maybeToken)
                 .createUser(request.toProto()),
             sequencerFactory)
-        .map(User::fromProto);
+        .map(res -> User.fromProto(res.getUser()));
   }
 
   @Override
@@ -55,7 +55,7 @@ private Single<User> getUser(
             StubHelper.authenticating(this.serviceFutureStub, maybeToken)
                 .getUser(request.toProto()),
             sequencerFactory)
-        .map(User::fromProto);
+        .map(res -> User.fromProto(res.getUser()));
   }
 
   @Override

diff --git a/...rxjava/src/test/scala/com/daml/ledger/rxjava/grpc/helpers/UserManagementServiceImpl.scala b/...rxjava/src/test/scala/com/daml/ledger/rxjava/grpc/helpers/UserManagementServiceImpl.scala
@@ -26,11 +26,11 @@ final class UserManagementServiceImpl extends UserManagementService with FakeAut
     Future.successful(result)
   }
 
-  override def createUser(request: CreateUserRequest): Future[User] =
-    record(request)(User.defaultInstance)
+  override def createUser(request: CreateUserRequest): Future[CreateUserResponse] =
+    record(request)(CreateUserResponse.defaultInstance)
 
-  override def getUser(request: GetUserRequest): Future[User] =
-    record(request)(User.defaultInstance)
+  override def getUser(request: GetUserRequest): Future[GetUserResponse] =
+    record(request)(GetUserResponse.defaultInstance)
 
   override def deleteUser(request: DeleteUserRequest): Future[DeleteUserResponse] =
     record(request)(DeleteUserResponse.defaultInstance)

diff --git a/ledger-api/grpc-definitions/com/daml/ledger/api/v1/admin/user_management_service.proto b/ledger-api/grpc-definitions/com/daml/ledger/api/v1/admin/user_management_service.proto
@@ -21,10 +21,10 @@ option csharp_namespace = "Com.Daml.Ledger.Api.V1.Admin";
 service UserManagementService {
 
     // Create a new user, failing if it already exists.
-    rpc CreateUser (CreateUserRequest) returns (User);
+    rpc CreateUser (CreateUserRequest) returns (CreateUserResponse);
 
     // Get the user data of a specific user or the authenticated user.
-    rpc GetUser (GetUserRequest) returns (User);
+    rpc GetUser (GetUserRequest) returns (GetUserResponse);
 
     // Delete an existing user and all its rights.
     rpc DeleteUser (DeleteUserRequest) returns (DeleteUserResponse);
@@ -104,13 +104,23 @@ message CreateUserRequest {
     repeated Right rights = 2;
 }
 
+message CreateUserResponse {
+    // Created user.
+    User user = 1;
+}
+
 // Required authorization: ``HasRight(ParticipantAdmin) OR IsAuthenticatedUser(user_id)``
 message GetUserRequest {
     // The user whose data to retrieve.
     // If set to empty string (the default), then the data for the authenticated user will be retrieved.
     string user_id = 1;
 }
 
+message GetUserResponse {
+    // Retrieved user.
+    User user = 1;
+}
+
 // Required authorization: ``HasRight(ParticipantAdmin)``
 message DeleteUserRequest {
     // The user to delete.

diff --git a/.../scala/com/digitalasset/ledger/api/auth/services/UserManagementServiceAuthorization.scala b/.../scala/com/digitalasset/ledger/api/auth/services/UserManagementServiceAuthorization.scala
@@ -28,10 +28,10 @@ private[daml] final class UserManagementServiceAuthorization(
   private implicit val errorLogger: ContextualizedErrorLogger =
     new DamlContextualizedErrorLogger(logger, loggingContext, None)
 
-  override def createUser(request: CreateUserRequest): Future[User] =
+  override def createUser(request: CreateUserRequest): Future[CreateUserResponse] =
     authorizer.requireAdminClaims(service.createUser)(request)
 
-  override def getUser(request: GetUserRequest): Future[User] =
+  override def getUser(request: GetUserRequest): Future[GetUserResponse] =
     defaultToAuthenticatedUser(request.userId) match {
       case Failure(ex) => Future.failed(ex)
       case Success(Some(userId)) => service.getUser(request.copy(userId = userId))

diff --git a/...t/src/main/scala/com/digitalasset/ledger/client/services/admin/UserManagementClient.scala b/...t/src/main/scala/com/digitalasset/ledger/client/services/admin/UserManagementClient.scala
@@ -30,21 +30,21 @@ final class UserManagementClient(service: UserManagementServiceStub)(implicit
     LedgerClient
       .stub(service, token)
       .createUser(request)
-      .map(fromProtoUser)
+      .map(res => fromProtoUser(res.user.get))
   }
 
   def getUser(userId: UserId, token: Option[String] = None): Future[User] =
     LedgerClient
       .stub(service, token)
       .getUser(proto.GetUserRequest(userId.toString))
-      .map(fromProtoUser)
+      .map(res => fromProtoUser(res.user.get))
 
   /** Retrieve the User information for the user authenticated by the token(s) on the call . */
   def getAuthenticatedUser(token: Option[String] = None): Future[User] =
     LedgerClient
       .stub(service, token)
       .getUser(proto.GetUserRequest())
-      .map(fromProtoUser)
+      .map(res => fromProtoUser(res.user.get))
 
   def deleteUser(userId: UserId, token: Option[String] = None): Future[Unit] =
     LedgerClient

diff --git a/...est-tool/src/main/scala/com/daml/ledger/api/testtool/suites/UserManagementServiceIT.scala b/...est-tool/src/main/scala/com/daml/ledger/api/testtool/suites/UserManagementServiceIT.scala
@@ -13,9 +13,11 @@ import com.daml.ledger.api.testtool.infrastructure.LedgerTestSuite
 import com.daml.ledger.api.testtool.infrastructure.participant.ParticipantTestContext
 import com.daml.ledger.api.v1.admin.user_management_service.{
   CreateUserRequest,
+  CreateUserResponse,
   DeleteUserRequest,
   DeleteUserResponse,
   GetUserRequest,
+  GetUserResponse,
   GrantUserRightsRequest,
   ListUserRightsRequest,
   ListUserRightsResponse,
@@ -100,11 +102,11 @@ final class UserManagementServiceIT extends LedgerTestSuite {
 
     } yield {
       assertTooManyUserRightsError(create1)
-      assertEquals(create2, user1)
+      assertEquals(create2.user, Some(user1))
       assertTooManyUserRightsError(grant1)
       assertEquals(rights1.rights.size, permissionsMaxAndOne.tail.size)
       assertSameElements(rights1.rights, permissionsMaxAndOne.tail)
-      assertEquals(create3, user2)
+      assertEquals(create3.user, Some(user2))
     }
   })
 
@@ -193,7 +195,7 @@ final class UserManagementServiceIT extends LedgerTestSuite {
       get1 <- ledger.userManagement.getUser(GetUserRequest(AdminUserId))
       rights1 <- ledger.userManagement.listUserRights(ListUserRightsRequest(AdminUserId))
     } yield {
-      assertEquals(get1, User(AdminUserId, ""))
+      assertEquals(get1.user, Some(User(AdminUserId, "")))
       assertEquals(rights1, ListUserRightsResponse(Seq(adminPermission)))
     }
   })
@@ -216,9 +218,9 @@ final class UserManagementServiceIT extends LedgerTestSuite {
       res3 <- ledger.userManagement.createUser(CreateUserRequest(Some(user2), Nil))
       res4 <- ledger.userManagement.deleteUser(DeleteUserRequest(userId2))
     } yield {
-      assertEquals(res1, user1)
+      assertEquals(res1, CreateUserResponse(Some(user1)))
       assertUserAlreadyExists(res2)
-      assertEquals(res3, user2)
+      assertEquals(res3, CreateUserResponse(Some(user2)))
       assertEquals(res4, DeleteUserResponse())
     }
   })
@@ -240,7 +242,7 @@ final class UserManagementServiceIT extends LedgerTestSuite {
         .mustFail("retrieving non-existent user")
     } yield {
       assertUserNotFound(res2)
-      assert(res1 == user1)
+      assert(res1 == GetUserResponse(Some(user1)))
     }
   })
 
@@ -289,7 +291,7 @@ final class UserManagementServiceIT extends LedgerTestSuite {
       def filterUsers(users: Iterable[User]) = users.filter(u => u.id == userId1 || u.id == userId2)
 
       assertSameElements(filterUsers(res1.users), Seq(user1))
-      assertEquals(res2, user2)
+      assertEquals(res2.user, Some(user2))
       assertSameElements(
         filterUsers(res3.users),
         Set(user1, user2),
@@ -497,7 +499,7 @@ final class UserManagementServiceIT extends LedgerTestSuite {
       res6 <- ledger.userManagement
         .listUserRights(ListUserRightsRequest(userId1))
     } yield {
-      assertEquals(res1, user1)
+      assertEquals(res1.user, Some(user1))
       assertEquals(res2, ListUserRightsResponse(Seq.empty))
       assertSameElements(
         res3.newlyGrantedRights.toSet,

diff --git a/...ation-api/src/main/scala/platform/apiserver/services/admin/ApiUserManagementService.scala b/...ation-api/src/main/scala/platform/apiserver/services/admin/ApiUserManagementService.scala
@@ -13,6 +13,7 @@ import com.daml.error.{
   ErrorCodesVersionSwitcher,
 }
 import com.daml.ledger.api.domain._
+import com.daml.ledger.api.v1.admin.user_management_service.{CreateUserResponse, GetUserResponse}
 import com.daml.ledger.api.v1.admin.{user_management_service => proto}
 import com.daml.ledger.participant.state.index.v2.UserManagementStore
 import com.daml.lf.data.Ref
@@ -52,7 +53,7 @@ private[apiserver] final class ApiUserManagementService(
   override def bindService(): ServerServiceDefinition =
     proto.UserManagementServiceGrpc.bindService(this, executionContext)
 
-  override def createUser(request: proto.CreateUserRequest): Future[proto.User] =
+  override def createUser(request: proto.CreateUserRequest): Future[CreateUserResponse] =
     withValidation {
       for {
         pUser <- requirePresence(request.user, "user")
@@ -67,17 +68,17 @@ private[apiserver] final class ApiUserManagementService(
           rights = pRights,
         )
         .flatMap(handleResult("creating user"))
-        .map(_ => request.user.get)
+        .map(_ => CreateUserResponse(Some(request.user.get)))
     }
 
-  override def getUser(request: proto.GetUserRequest): Future[proto.User] =
+  override def getUser(request: proto.GetUserRequest): Future[GetUserResponse] =
     withValidation(
       requireUserId(request.userId, "user_id")
     )(userId =>
       userManagementStore
         .getUser(userId)
         .flatMap(handleResult("getting user"))
-        .map(toProtoUser)
+        .map(u => GetUserResponse(Some(toProtoUser(u))))
     )
 
   override def deleteUser(request: proto.DeleteUserRequest): Future[proto.DeleteUserResponse] =

diff --git a/.../src/test/it/scala/com/daml/ledger/platform/sandbox/auth/GetAuthenticatedUserAuthIT.scala b/.../src/test/it/scala/com/daml/ledger/platform/sandbox/auth/GetAuthenticatedUserAuthIT.scala
@@ -6,6 +6,7 @@ import java.util.UUID
 
 import com.daml.ledger.api.v1.admin.user_management_service.{
   GetUserRequest,
+  GetUserResponse,
   User,
   UserManagementServiceGrpc,
 }
@@ -23,7 +24,7 @@ class GetAuthenticatedUserAuthIT extends ServiceCallAuthTests {
     stub(UserManagementServiceGrpc.stub(channel), token).getUser(GetUserRequest())
 
   private def expectUser(token: Option[String], expectedUser: User): Future[Assertion] =
-    serviceCallWithToken(token).map(assertResult(expectedUser)(_))
+    serviceCallWithToken(token).map(assertResult(GetUserResponse(Some(expectedUser)))(_))
 
   private def getUser(token: Option[String], userId: String) =
     stub(UserManagementServiceGrpc.stub(channel), token).getUser(GetUserRequest(userId))
@@ -54,7 +55,7 @@ class GetAuthenticatedUserAuthIT extends ServiceCallAuthTests {
       aliceRetrieved1 <- getUser(aliceToken, "")
       // user accesses its own user record with specifying the id
       aliceRetrieved2 <- getUser(aliceToken, alice.id)
-
-    } yield assertResult((alice, alice))((aliceRetrieved1, aliceRetrieved2))
+      expected = GetUserResponse(Some(alice))
+    } yield assertResult((expected, expected))((aliceRetrieved1, aliceRetrieved2))
   }
 }
diff --git a/...rc/test/it/scala/com/daml/ledger/platform/sandbox/auth/GetUserWithGivenUserIdAuthIT.scala b/...rc/test/it/scala/com/daml/ledger/platform/sandbox/auth/GetUserWithGivenUserIdAuthIT.scala
@@ -18,7 +18,9 @@ class GetUserWithGivenUserIdAuthIT extends AdminServiceCallAuthTests {
     val testId = UUID.randomUUID().toString
 
     def getUser(userId: String): Future[User] =
-      stub(UserManagementServiceGrpc.stub(channel), token).getUser(GetUserRequest(userId))
+      stub(UserManagementServiceGrpc.stub(channel), token)
+        .getUser(GetUserRequest(userId))
+        .map(_.user.get)
 
     for {
       // create a normal users

diff --git a/...sandbox-on-x/src/test/lib/scala/com/daml/platform/sandbox/auth/ServiceCallAuthTests.scala b/...sandbox-on-x/src/test/lib/scala/com/daml/platform/sandbox/auth/ServiceCallAuthTests.scala
@@ -156,6 +156,6 @@ trait ServiceCallAuthTests
     val req = proto.CreateUserRequest(Some(proto.User(userId)), rights)
     stub(proto.UserManagementServiceGrpc.stub(channel), canReadAsAdminStandardJWT)
       .createUser(req)
-      .map((_, userToken))
+      .map(res => (res.user.get, userToken))
   }
 }
diff --git a/templates/create-daml-app-test-resources/index.test.ts b/templates/create-daml-app-test-resources/index.test.ts
@@ -232,7 +232,10 @@ const follow = async (page: Page, userToFollow: string) => {
 }
 
 // LOGIN_TEST_BEGIN
-test('log in as a new user, log out and log back in', async () => {
+// FIXME Enable this test once the integration test can be run against a version of Canton that supports the new interface for creating a user
+// FIXME The change was introduced as part of https://github.com/digital-asset/daml/pull/12682
+// FIXME The ticket tracking this issue is https://github.com/digital-asset/daml/issues/12808
+test.skip('log in as a new user, log out and log back in', async () => {
   const [user, party] = await getParty();
 
   // Log in as a new user.
@@ -266,7 +269,10 @@ test('log in as a new user, log out and log back in', async () => {
 // - while the user that is followed is logged out
 // These are all successful cases.
 
-test('log in as three different users and start following each other', async () => {
+// FIXME Enable this test once the integration test can be run against a version of Canton that supports the new interface for creating a user
+// FIXME The change was introduced as part of https://github.com/digital-asset/daml/pull/12682
+// FIXME The ticket tracking this issue is https://github.com/digital-asset/daml/issues/12808
+test.skip('log in as three different users and start following each other', async () => {
   const [user1, party1] = await getParty();
   const [user2, party2] = await getParty();
   const [user3, party3] = await getParty();
@@ -354,7 +360,10 @@ test('log in as three different users and start following each other', async ()
   await page3.close();
 }, 60_000);
 
-test('error when following self', async () => {
+// FIXME Enable this test once the integration test can be run against a version of Canton that supports the new interface for creating a user
+// FIXME The change was introduced as part of https://github.com/digital-asset/daml/pull/12682
+// FIXME The ticket tracking this issue is https://github.com/digital-asset/daml/issues/12808
+test.skip('error when following self', async () => {
   const [user, party] = await getParty();
   const page = await newUiPage();
 
@@ -369,7 +378,10 @@ test('error when following self', async () => {
   await page.close();
 });
 
-test('error when adding a user that you are already following', async () => {
+// FIXME Enable this test once the integration test can be run against a version of Canton that supports the new interface for creating a user
+// FIXME The change was introduced as part of https://github.com/digital-asset/daml/pull/12682
+// FIXME The ticket tracking this issue is https://github.com/digital-asset/daml/issues/12808
+test.skip('error when adding a user that you are already following', async () => {
   const [user1, party1] = await getParty();
   const [user2, party2] = await getParty();
   const page = await newUiPage();
@@ -424,7 +436,10 @@ test('error on non-existent user id', async () => {
     await page.close();
 }, 40_000);
 
-test('error on user with no primary party', async () => {
+// FIXME Enable this test once the integration test can be run against a version of Canton that supports the new interface for creating a user
+// FIXME The change was introduced as part of https://github.com/digital-asset/daml/pull/12682
+// FIXME The ticket tracking this issue is https://github.com/digital-asset/daml/issues/12808
+test.skip('error on user with no primary party', async () => {
     const invalidUser = "noprimary";
     // TODO replace with daml-ledger once it exposes the create user endpoint.
   const grpcurlUserArgs = [