From 065d051dc162f27728c7c43c624dbfab80ad8a15 Mon Sep 17 00:00:00 2001 From: Matt Date: Thu, 20 Jan 2022 08:10:10 -0800 Subject: [PATCH] GET /v1/roles?:roleId?fields=users should return users (#2308) --- .../catalog/jdbi3/RoleRepository.java | 29 ++++++++++++++++++- .../catalog/resources/teams/RoleResource.java | 9 ++++-- .../json/schema/entity/teams/role.json | 4 +++ .../catalog/resources/EntityResourceTest.java | 4 +++ .../resources/teams/RoleResourceTest.java | 26 +++++++++++++++-- .../resources/teams/UserResourceTest.java | 2 +- ingestion-core/src/metadata/_version.py | 2 +- 7 files changed, 68 insertions(+), 8 deletions(-) diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/RoleRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/RoleRepository.java index a7436669114..1ac9c8bf89f 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/RoleRepository.java +++ b/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 { @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 result = daoCollection .relationshipDAO() @@ -79,6 +83,29 @@ public class RoleRepository extends EntityRepository { return Entity.getEntityReference(Entity.POLICY, UUID.fromString(result.get(0))); } + private List getUsersForRole(@NonNull Role role) { + List 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 diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/teams/RoleResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/teams/RoleResource.java index 991a6c919f3..8cf544f0867 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/teams/RoleResource.java +++ b/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 List FIELD_LIST = List.of(FIELDS); + public static final String FIELDS = "policy,users"; + public static final List 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 response = dao.createOrUpdate(uriInfo, role); + addHref(uriInfo, response.getEntity()); return response.toResponse(); } @@ -341,6 +345,7 @@ public class RoleResource { SecurityUtil.checkAdminOrBotRole(authorizer, securityContext); PatchResponse response = dao.patch(uriInfo, UUID.fromString(id), securityContext.getUserPrincipal().getName(), patch); + addHref(uriInfo, response.getEntity()); return response.toResponse(); } diff --git a/catalog-rest-service/src/main/resources/json/schema/entity/teams/role.json b/catalog-rest-service/src/main/resources/json/schema/entity/teams/role.json index 0b29eae99e6..e43e476d9d4 100644 --- a/catalog-rest-service/src/main/resources/json/schema/entity/teams/role.json +++ b/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" : [ diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java index 9b1a95b2722..9f9b1b3f05b 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java @@ -357,6 +357,9 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { // Get interface to access all common entity attributes public abstract EntityInterface 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 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); } diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/RoleResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/RoleResourceTest.java index 56d255b9663..0c55a6fb773 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/RoleResourceTest.java +++ b/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 { 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 authHeaders) diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java index 8c9f168d7c4..65760e6ae97 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java @@ -506,7 +506,7 @@ public class UserResourceTest extends EntityResourceTest { 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; diff --git a/ingestion-core/src/metadata/_version.py b/ingestion-core/src/metadata/_version.py index 22a3302be6d..cda0c695648 100644 --- a/ingestion-core/src/metadata/_version.py +++ b/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__"]