GET /v1/roles?:roleId?fields=users should return users (#2308)

catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/RoleRepository.java

@@ -19,7 +19,10 @@ import java.io.IOException;
 import java.net.URI;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 import java.util.UUID;
+import java.util.stream.Collectors;
+import lombok.NonNull;
 import lombok.extern.slf4j.Slf4j;
 import org.jdbi.v3.sqlobject.transaction.Transaction;
 import org.openmetadata.catalog.Entity;
@@ -56,10 +59,11 @@ public class RoleRepository extends EntityRepository<Role> {
   @Override
   public Role setFields(Role role, Fields fields) throws IOException {
     role.setPolicy(fields.contains("policy") ? getPolicyForRole(role) : null);
+    role.setUsers(fields.contains("users") ? getUsersForRole(role) : null);
     return role;
   }
 
-  private EntityReference getPolicyForRole(Role role) throws IOException {
+  private EntityReference getPolicyForRole(@NonNull Role role) throws IOException {
     List<String> result =
         daoCollection
             .relationshipDAO()
@@ -79,6 +83,29 @@ public class RoleRepository extends EntityRepository<Role> {
     return Entity.getEntityReference(Entity.POLICY, UUID.fromString(result.get(0)));
   }
 
+  private List<EntityReference> getUsersForRole(@NonNull Role role) {
+    List<EntityReference> entityReferences =
+        daoCollection
+            .relationshipDAO()
+            .findFromEntity(
+                role.getId().toString(),
+                Entity.ROLE,
+                Relationship.HAS.ordinal(),
+                Entity.USER,
+                toBoolean(toInclude(role)));
+    return Optional.ofNullable(entityReferences).orElse(Collections.emptyList()).stream()
+        .map(
+            ref -> {
+              try {
+                return Entity.getEntityReference(Entity.USER, ref.getId());
+              } catch (IOException e) {
+                LOG.warn("Could not get entity reference for user {}", ref.getId());
+              }
+              return ref;
+            })
+        .collect(Collectors.toList());
+  }
+
   @Override
   public void restorePatchAttributes(Role original, Role updated) {
     // Patch can't make changes to following fields. Ignore the changes

catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/teams/RoleResource.java

@@ -27,6 +27,7 @@ import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.security.GeneralSecurityException;
 import java.text.ParseException;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
 import java.util.UUID;
@@ -77,6 +78,7 @@ public class RoleResource {
 
   public static Role addHref(UriInfo uriInfo, Role role) {
     Entity.withHref(uriInfo, role.getPolicy());
+    Entity.withHref(uriInfo, role.getUsers());
     return role;
   }
 
@@ -101,8 +103,8 @@ public class RoleResource {
     }
   }
 
-  public static final String FIELDS = "policy";
+  public static final String FIELDS = "policy,users";
-  public static final List<String> FIELD_LIST = List.of(FIELDS);
+  public static final List<String> FIELD_LIST = Arrays.asList(FIELDS.replace(" ", "").split(","));
 
   @GET
   @Valid
@@ -154,6 +156,7 @@ public class RoleResource {
     } else { // Forward paging or first page
       roles = dao.listAfter(uriInfo, fields, null, limitParam, after, include);
     }
+    roles.getData().forEach(role -> addHref(uriInfo, role));
     return roles;
   }
 
@@ -312,6 +315,7 @@ public class RoleResource {
     SecurityUtil.checkAdminOrBotRole(authorizer, securityContext);
     Role role = getRole(createRole, securityContext);
     RestUtil.PutResponse<Role> response = dao.createOrUpdate(uriInfo, role);
+    addHref(uriInfo, response.getEntity());
     return response.toResponse();
   }
 
@@ -341,6 +345,7 @@ public class RoleResource {
     SecurityUtil.checkAdminOrBotRole(authorizer, securityContext);
     PatchResponse<Role> response =
         dao.patch(uriInfo, UUID.fromString(id), securityContext.getUserPrincipal().getName(), patch);
+    addHref(uriInfo, response.getEntity());
     return response.toResponse();
   }
 

catalog-rest-service/src/main/resources/json/schema/entity/teams/role.json

@@ -55,6 +55,10 @@
     "policy" : {
       "description": "Policy that is attached to this role.",
       "$ref": "../../type/entityReference.json"
+    },
+    "users" : {
+      "description": "Users that have this role assigned.",
+      "$ref": "../../type/entityReference.json#/definitions/entityReferenceList"
     }
   },
   "required" : [

catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java

@@ -357,6 +357,9 @@ public abstract class EntityResourceTest<T> extends CatalogApplicationTest {
   // Get interface to access all common entity attributes
   public abstract EntityInterface<T> getEntityInterface(T entity);
 
+  // Do some preparation work right before calling validateGetWithDifferentFields.
+  protected void prepareGetWithDifferentFields(T entity) throws HttpResponseException {};
+
   // Get an entity by ID and name with different fields. See TableResourceTest for example.
   public abstract void validateGetWithDifferentFields(T entity, boolean byName) throws HttpResponseException;
 
@@ -526,6 +529,7 @@ public abstract class EntityResourceTest<T> extends CatalogApplicationTest {
   void get_entityWithDifferentFields_200_OK(TestInfo test) throws IOException, URISyntaxException {
     Object create = createRequest(getEntityName(test), "description", "displayName", USER_OWNER1);
     T entity = createAndCheckEntity(create, adminAuthHeaders());
+    prepareGetWithDifferentFields(entity);
     validateGetWithDifferentFields(entity, false);
     validateGetWithDifferentFields(entity, true);
   }

catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/RoleResourceTest.java

@@ -25,6 +25,7 @@ import static org.openmetadata.catalog.util.TestUtils.assertResponse;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import java.io.IOException;
 import java.net.URISyntaxException;
+import java.util.List;
 import java.util.Map;
 import java.util.UUID;
 import javax.json.JsonPatch;
@@ -164,17 +165,36 @@ public class RoleResourceTest extends EntityResourceTest<Role> {
     assertEquals(expectedDisplayName, role.getDisplayName());
   }
 
+  @Override
+  protected void prepareGetWithDifferentFields(Role role) throws HttpResponseException {
+    // Assign two arbitrary users this role for testing.
+    UserResourceTest.createUser(
+        UserResourceTest.create(role.getName() + "user1").withRoles(List.of(role.getId())), adminAuthHeaders());
+    UserResourceTest.createUser(
+        UserResourceTest.create(role.getName() + "user2").withRoles(List.of(role.getId())), adminAuthHeaders());
+  };
+
   /** Validate returned fields GET .../roles/{id}?fields="..." or GET .../roles/name/{name}?fields="..." */
   @Override
   public void validateGetWithDifferentFields(Role expectedRole, boolean byName) throws HttpResponseException {
     String updatedBy = TestUtils.getPrincipal(adminAuthHeaders());
-    // Role does not have any supported additional fields yet.
+
     // .../roles
-    Role getRole =
+    Role role =
         byName
             ? getRoleByName(expectedRole.getName(), null, adminAuthHeaders())
             : getRole(expectedRole.getId(), null, adminAuthHeaders());
-    validateRole(getRole, expectedRole.getDescription(), expectedRole.getDisplayName(), updatedBy);
+    validateRole(role, expectedRole.getDescription(), expectedRole.getDisplayName(), updatedBy);
+
+    // .../roles?fields=policy,users
+    String fields = "policy,users";
+    role =
+        byName
+            ? getRoleByName(expectedRole.getName(), fields, adminAuthHeaders())
+            : getRole(expectedRole.getId(), fields, adminAuthHeaders());
+    validateRole(role, expectedRole.getDescription(), expectedRole.getDisplayName(), updatedBy);
+    TestUtils.validateEntityReference(role.getPolicy());
+    TestUtils.validateEntityReference(role.getUsers());
   }
 
   private Role patchRole(UUID roleId, String originalJson, Role updated, Map<String, String> authHeaders)

catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java

@@ -506,7 +506,7 @@ public class UserResourceTest extends EntityResourceTest<User> {
     return create(getEntityName(test, index));
   }
 
-  public CreateUser create(String entityName) {
+  public static CreateUser create(String entityName) {
     // user part of the email should be less than 64 in length
     String emailUser = entityName == null || entityName.isEmpty() ? UUID.randomUUID().toString() : entityName;
     emailUser = emailUser.length() > 64 ? emailUser.substring(0, 64) : emailUser;

ingestion-core/src/metadata/_version.py

@@ -7,5 +7,5 @@ Provides metadata version information.
 
 from incremental import Version
 
-__version__ = Version("metadata", 0, 8, 0, dev=5)
+__version__ = Version("metadata", 0, 8, 0, dev=6)
 __all__ = ["__version__"]