From 9e4d8d709d19ba2f75dcf754ea43600a9b5f8a6e Mon Sep 17 00:00:00 2001 From: Matt Date: Sun, 20 Feb 2022 11:27:32 -0800 Subject: [PATCH] Support swapping default roles (#2868) * Support swapping default roles Pseudocode: ``` A. patchRole(role1, default=True): B. set role1.default = True for all users: add role1 to user.roles C. for role in roles if role != role1: set role.default = False for all users: delete role from user.roles ``` This ensures that changeDescription for the role(s) and user(s) are updated accordingly. Potential optimization: Adding role1 and removing role from user.roles could be considered/implemented as one change. However, increases code complexity. * Set DataConsumer as default role * Fix tests * Fix code smell --- .../mysql/v004__create_db_connection_info.sql | 6 +- .../exception/CatalogExceptionMessage.java | 4 + .../catalog/jdbi3/EntityRepository.java | 6 +- .../catalog/jdbi3/RoleRepository.java | 121 +++++++++++++++--- .../catalog/jdbi3/UserRepository.java | 2 +- .../catalog/jdbi3/WebhookRepository.java | 7 +- .../json/data/role/DataConsumer.json | 3 +- .../catalog/resources/EntityResourceTest.java | 40 +++++- .../resources/teams/RoleResourceTest.java | 52 +++++++- .../resources/teams/UserResourceTest.java | 82 ++++++++---- ingestion-core/src/metadata/_version.py | 2 +- 11 files changed, 267 insertions(+), 58 deletions(-) diff --git a/bootstrap/sql/mysql/v004__create_db_connection_info.sql b/bootstrap/sql/mysql/v004__create_db_connection_info.sql index b0175c5c0b4..67c729ea44a 100644 --- a/bootstrap/sql/mysql/v004__create_db_connection_info.sql +++ b/bootstrap/sql/mysql/v004__create_db_connection_info.sql @@ -42,11 +42,15 @@ CREATE TABLE IF NOT EXISTS glossary_term_entity ( ); --- Set default as false for all existing roles, to avoid unintended manipulation of roles during migration. +-- Set default as false for all existing roles except DataConsumer, to avoid unintended manipulation of roles during migration. UPDATE role_entity SET json = JSON_SET(json, '$.default', FALSE); +UPDATE role_entity +SET json = JSON_SET(json, '$.default', TRUE) +WHERE name = 'DataConsumer'; + ALTER TABLE role_entity ADD COLUMN `default` BOOLEAN GENERATED ALWAYS AS (JSON_EXTRACT(json, '$.default')), ADD INDEX(`default`); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/exception/CatalogExceptionMessage.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/exception/CatalogExceptionMessage.java index a5b0effa454..daec69a8996 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/exception/CatalogExceptionMessage.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/exception/CatalogExceptionMessage.java @@ -29,6 +29,10 @@ public final class CatalogExceptionMessage { return entityNotFound(entityType, id.toString()); } + public static String entitiesNotFound(String entityType) { + return String.format("%s instances not found", entityType); + } + public static String readOnlyAttribute(String entityType, String attribute) { return String.format("%s attribute %s can't be modified", entityType, attribute); } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/EntityRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/EntityRepository.java index 21db11179bb..a9300e68a3a 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/EntityRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/EntityRepository.java @@ -908,12 +908,12 @@ public abstract class EntityRepository { this.operation = operation; } - public final void update() throws IOException { + public final void update() throws IOException, ParseException { update(false); } /** Compare original and updated entities and perform updates. Update the entity version and track changes. */ - public final void update(boolean allowEdits) throws IOException { + public final void update(boolean allowEdits) throws IOException, ParseException { if (operation.isDelete()) { // DELETE Operation updateDeleted(); } else { // PUT or PATCH operations @@ -930,7 +930,7 @@ public abstract class EntityRepository { storeUpdate(); } - public void entitySpecificUpdate() throws IOException { + public void entitySpecificUpdate() throws IOException, ParseException { // Default implementation. Override this to add any entity specific field updates } 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 d5d0f416421..bc206611f68 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 @@ -16,12 +16,15 @@ package org.openmetadata.catalog.jdbi3; import static org.openmetadata.catalog.util.EntityUtil.toBoolean; import java.io.IOException; -import java.io.UnsupportedEncodingException; import java.net.URI; import java.security.GeneralSecurityException; +import java.text.ParseException; +import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; import javax.ws.rs.core.UriInfo; @@ -31,10 +34,13 @@ import org.jdbi.v3.sqlobject.transaction.Transaction; import org.openmetadata.catalog.Entity; import org.openmetadata.catalog.entity.policies.Policy; import org.openmetadata.catalog.entity.teams.Role; +import org.openmetadata.catalog.entity.teams.User; +import org.openmetadata.catalog.exception.CatalogExceptionMessage; import org.openmetadata.catalog.exception.EntityNotFoundException; import org.openmetadata.catalog.resources.teams.RoleResource; import org.openmetadata.catalog.type.ChangeDescription; import org.openmetadata.catalog.type.EntityReference; +import org.openmetadata.catalog.type.Include; import org.openmetadata.catalog.type.PolicyType; import org.openmetadata.catalog.type.Relationship; import org.openmetadata.catalog.util.EntityInterface; @@ -180,24 +186,16 @@ public class RoleRepository extends EntityRepository { } public ResultList getDefaultRolesResultList(UriInfo uriInfo, Fields fields) - throws GeneralSecurityException, UnsupportedEncodingException { + throws GeneralSecurityException, IOException { List roles = getDefaultRoles(uriInfo, fields); return new ResultList<>(roles, null, null, roles.size()); } - private List getDefaultRoles(UriInfo uriInfo, Fields fields) { - List roles = - daoCollection.roleDAO().getDefaultRoles().stream() - .map( - json -> { - try { - return withHref(uriInfo, setFields(JsonUtils.readValue(json, Role.class), fields)); - } catch (IOException e) { - LOG.warn("Could not parse Role from json {}", json); - } - return null; - }) - .collect(Collectors.toList()); + private List getDefaultRoles(UriInfo uriInfo, Fields fields) throws IOException { + List roles = new ArrayList<>(); + for (String roleJson : daoCollection.roleDAO().getDefaultRoles()) { + roles.add(withHref(uriInfo, setFields(JsonUtils.readValue(roleJson, Role.class), fields))); + } if (roles.size() > 1) { LOG.warn( "{} roles {}, are registered as default. There SHOULD be only one role marked as default.", @@ -340,8 +338,97 @@ public class RoleRepository extends EntityRepository { } @Override - public void entitySpecificUpdate() throws IOException { - recordChange("default", original.getEntity().getDefault(), updated.getEntity().getDefault()); + public void entitySpecificUpdate() throws IOException, ParseException { + updateDefault(original.getEntity(), updated.getEntity()); + } + + private void updateDefault(Role origRole, Role updatedRole) throws IOException, ParseException { + long startTime = System.nanoTime(); + if (Boolean.FALSE.equals(origRole.getDefault()) && Boolean.TRUE.equals(updatedRole.getDefault())) { + setDefaultToTrue(updatedRole); + } + if (Boolean.TRUE.equals(origRole.getDefault()) && Boolean.FALSE.equals(updatedRole.getDefault())) { + setDefaultToFalse(updatedRole); + } + recordChange("default", origRole.getDefault(), updatedRole.getDefault()); + LOG.debug( + "Took {} ns to update {} role field default from {} to {}", + System.nanoTime() - startTime, + updatedRole.getName(), + origRole.getDefault(), + updatedRole.getDefault()); + } + + private void setDefaultToTrue(Role role) throws IOException, ParseException { + List defaultRoles = getDefaultRoles(null, ROLE_PATCH_FIELDS); + EntityRepository roleRepository = Entity.getEntityRepository(Entity.ROLE); + // Set default=FALSE for all existing default roles. + for (Role defaultRole : defaultRoles) { + if (defaultRole.getId().equals(role.getId())) { + // Skip the current role which is being set with default=TRUE. + continue; + } + Role origDefaultRole = roleRepository.get(null, defaultRole.getId().toString(), ROLE_PATCH_FIELDS); + Role updatedDefaultRole = roleRepository.get(null, defaultRole.getId().toString(), ROLE_PATCH_FIELDS); + updatedDefaultRole = updatedDefaultRole.withDefault(false); + new RoleUpdater(origDefaultRole, updatedDefaultRole, Operation.PATCH).update(); + } + LOG.info("Creating 'user --- has ---> role' relationship for {} role", role.getName()); + updateUsers(new RoleEntityInterface(role).getEntityReference(), null); + } + + private void setDefaultToFalse(Role role) { + LOG.info("Deleting 'user --- has ---> role' relationship for {} role", role.getName()); + updateUsers(null, new RoleEntityInterface(role).getEntityReference()); + } + + private List getAllUsers() { + EntityRepository userRepository = Entity.getEntityRepository(Entity.USER); + try { + return userRepository + .listAfter(null, UserRepository.USER_UPDATE_FIELDS, null, Integer.MAX_VALUE - 1, null, Include.ALL) + .getData(); + } catch (GeneralSecurityException | IOException | ParseException e) { + throw EntityNotFoundException.byMessage(CatalogExceptionMessage.entitiesNotFound(Entity.USER)); + } + } + + private void updateUsers(EntityReference addRole, EntityReference removeRole) { + List users = getAllUsers(); + if (users.isEmpty()) { + return; + } + EntityRepository userRepository = Entity.getEntityRepository(Entity.USER); + users + .parallelStream() + .forEach( + user -> { + try { + updateUser(userRepository, user, addRole, removeRole); + } catch (IOException | ParseException e) { + throw new RuntimeException(e); + } + }); + } + + private void updateUser( + EntityRepository userRepository, User user, EntityReference addRole, EntityReference removeRole) + throws IOException, ParseException { + User origUser = userRepository.get(null, user.getId().toString(), UserRepository.USER_PATCH_FIELDS); + User updatedUser = userRepository.get(null, user.getId().toString(), UserRepository.USER_PATCH_FIELDS); + List roles = updatedUser.getRoles(); + if (roles == null) { + roles = new ArrayList<>(); + } + Set rolesSet = new HashSet<>(roles); + if (addRole != null) { + rolesSet.add(addRole); + } + if (removeRole != null) { + rolesSet.remove(removeRole); + } + updatedUser.setRoles(new ArrayList<>(rolesSet)); + Entity.getEntityRepository(Entity.USER).getUpdater(origUser, updatedUser, Operation.PATCH).update(); } } } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/UserRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/UserRepository.java index ce32c6bc444..cf80f7af608 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/UserRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/UserRepository.java @@ -78,7 +78,7 @@ public class UserRepository extends EntityRepository { daoCollection.roleDAO().getDefaultRolesIds().stream().map(UUID::fromString).collect(Collectors.toSet()); defaultRoleIds.removeAll(existingRoleIds); - if (rolesRef == null || rolesRef.size() == 0) { + if (rolesRef == null || rolesRef.isEmpty()) { rolesRef = new ArrayList<>(); } for (UUID roleId : defaultRoleIds) { diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/WebhookRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/WebhookRepository.java index 1abddfaa838..27e7406db81 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/WebhookRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/WebhookRepository.java @@ -417,21 +417,22 @@ public class WebhookRepository extends EntityRepository { webhook.getEventFilters().forEach(f -> filter.put(f.getEventType(), f.getEntities())); } - private void setErrorStatus(Long attemptTime, Integer statusCode, String reason) throws IOException { + private void setErrorStatus(Long attemptTime, Integer statusCode, String reason) + throws IOException, ParseException { if (!attemptTime.equals(webhook.getFailureDetails().getLastFailedAt())) { setStatus(Status.FAILED, attemptTime, statusCode, reason, null); } throw new RuntimeException(reason); } - private void setAwaitingRetry(Long attemptTime, int statusCode, String reason) throws IOException { + private void setAwaitingRetry(Long attemptTime, int statusCode, String reason) throws IOException, ParseException { if (!attemptTime.equals(webhook.getFailureDetails().getLastFailedAt())) { setStatus(Status.AWAITING_RETRY, attemptTime, statusCode, reason, attemptTime + currentBackoffTime); } } private void setStatus(Status status, Long attemptTime, Integer statusCode, String reason, Long timestamp) - throws IOException { + throws IOException, ParseException { Webhook stored = daoCollection.webhookDAO().findEntityById(webhook.getId()); webhook.setStatus(status); webhook diff --git a/catalog-rest-service/src/main/resources/json/data/role/DataConsumer.json b/catalog-rest-service/src/main/resources/json/data/role/DataConsumer.json index ad2ab255583..9ab680860ec 100644 --- a/catalog-rest-service/src/main/resources/json/data/role/DataConsumer.json +++ b/catalog-rest-service/src/main/resources/json/data/role/DataConsumer.json @@ -2,5 +2,6 @@ "name": "DataConsumer", "displayName": "Data Consumer", "description": "Users with Data Consumer role use different data assets for their day to day work.", - "deleted": false + "deleted": false, + "default": true } 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 91299064f93..a8a657f3913 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 @@ -1372,6 +1372,18 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { } public final T createAndCheckEntity(K create, Map authHeaders) throws IOException { + return createAndCheckEntity(create, authHeaders, create); + } + + /** + * Helper function to create an entity, submit POST API request and validate response. + * + * @param create entity to be created + * @param authHeaders auth headers to be used for the PATCH API request + * @param created expected response from POST API after entity has been created + * @return entity response from the POST API + */ + public final T createAndCheckEntity(K create, Map authHeaders, K created) throws IOException { // Validate an entity that is created has all the information set in create request String updatedBy = TestUtils.getPrincipal(authHeaders); T entity = createEntity(create, authHeaders); @@ -1379,12 +1391,12 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { assertEquals(updatedBy, entityInterface.getUpdatedBy()); assertEquals(0.1, entityInterface.getVersion()); // First version of the entity - validateCreatedEntity(entity, create, authHeaders); + validateCreatedEntity(entity, created, authHeaders); // GET the entity created and ensure it has all the information set in create request T getEntity = getEntity(entityInterface.getId(), authHeaders); assertEquals(0.1, entityInterface.getVersion()); // First version of the entity - validateCreatedEntity(getEntity, create, authHeaders); + validateCreatedEntity(getEntity, created, authHeaders); // TODO GET the entity by name @@ -1465,10 +1477,32 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { UpdateType updateType, ChangeDescription expectedChange) throws IOException { + return patchEntityAndCheck(updated, originalJson, authHeaders, updateType, expectedChange, updated); + } + + /** + * Helper function to generate JSON PATCH, submit PATCH API request and validate response. + * + * @param updated entity to compare with response from PATCH API + * @param originalJson JSON representation of entity before the update + * @param authHeaders auth headers to be used for the PATCH API request + * @param updateType type of update, see {@link TestUtils.UpdateType} + * @param expectedChange change description that is expected from the PATCH API response + * @param update entity used to diff against originalJson to generate JSON PATCH for PATCH API test + * @return entity response from the PATCH API + */ + protected final T patchEntityAndCheck( + T updated, + String originalJson, + Map authHeaders, + UpdateType updateType, + ChangeDescription expectedChange, + T update) + throws IOException { EntityInterface entityInterface = getEntityInterface(updated); // Validate information returned in patch response has the updates - T returned = patchEntity(entityInterface.getId(), originalJson, updated, authHeaders); + T returned = patchEntity(entityInterface.getId(), originalJson, update, authHeaders); entityInterface = getEntityInterface(returned); compareEntities(updated, returned, authHeaders); 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 4d26e805f90..dbe5722bb7a 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 @@ -15,6 +15,8 @@ package org.openmetadata.catalog.resources.teams; import static javax.ws.rs.core.Response.Status.FORBIDDEN; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.fail; import static org.openmetadata.catalog.util.TestUtils.ADMIN_AUTH_HEADERS; import static org.openmetadata.catalog.util.TestUtils.TEST_AUTH_HEADERS; import static org.openmetadata.catalog.util.TestUtils.UpdateType.MINOR_UPDATE; @@ -36,6 +38,7 @@ import org.openmetadata.catalog.Entity; import org.openmetadata.catalog.api.teams.CreateRole; import org.openmetadata.catalog.entity.policies.Policy; import org.openmetadata.catalog.entity.teams.Role; +import org.openmetadata.catalog.entity.teams.User; import org.openmetadata.catalog.jdbi3.RoleRepository.RoleEntityInterface; import org.openmetadata.catalog.resources.EntityResourceTest; import org.openmetadata.catalog.resources.policies.PolicyResource; @@ -56,11 +59,47 @@ public class RoleResourceTest extends EntityResourceTest { } @Test - void get_queryDefaultRole(TestInfo test) throws IOException { - Role defaultRole = createRolesAndSetDefault(test, 7); + void defaultRole(TestInfo test) throws IOException { + // Check if there is a default role in the beginning. DataConsumer is a default role. List defaultRoles = getDefaultRoles(); assertEquals(1, defaultRoles.size()); - assertEquals(defaultRole.getId(), defaultRoles.get(0).getId()); + Role prevDefaultRole = defaultRoles.get(0); + + // Update the default role and check the following: + // - the default role has changed + // - the previous default role is no longer set as default + // - all users have roles set correctly (only current default role should exist) + for (int i = 0; i < 2; i++) { // Run two iterations for this test. + Role defaultRole = createRolesAndSetDefault(test, 7, i * 10); + defaultRoles = getDefaultRoles(); + assertEquals(1, defaultRoles.size()); + assertEquals(defaultRole.getId(), defaultRoles.get(0).getId()); + assertNotEquals(defaultRole.getId(), prevDefaultRole.getId()); + + List users = new UserResourceTest().listEntities(Map.of("fields", "roles"), ADMIN_AUTH_HEADERS).getData(); + for (User user : users) { + boolean defaultRoleSet = false, prevDefaultRoleExists = false; + + for (EntityReference role : user.getRoles()) { + if (role.getId().equals(defaultRole.getId())) { + defaultRoleSet = true; + } + if (role.getId().equals(prevDefaultRole.getId())) { + prevDefaultRoleExists = true; + } + } + if (!defaultRoleSet) { + fail(String.format("Default role %s was not set for user %s", defaultRole.getName(), user.getName())); + } + if (prevDefaultRoleExists) { + fail( + String.format( + "Previous default role %s has not been removed for user %s", + prevDefaultRole.getName(), user.getName())); + } + } + prevDefaultRole = defaultRole; + } } public List getDefaultRoles() throws HttpResponseException { @@ -72,17 +111,18 @@ public class RoleResourceTest extends EntityResourceTest { * * @return the default role */ - public Role createRolesAndSetDefault(TestInfo test, @Positive int numberOfRoles) throws IOException { + public Role createRolesAndSetDefault(TestInfo test, @Positive int numberOfRoles, @Positive int offset) + throws IOException { // Create a set of roles. for (int i = 0; i < numberOfRoles; i++) { - CreateRole create = createRequest(test, i + 1); + CreateRole create = createRequest(test, offset + i + 1); createAndCheckRole(create, ADMIN_AUTH_HEADERS); } // Set one of the roles as default. Role role = getEntityByName( - getEntityName(test, new Random().nextInt(numberOfRoles)), + getEntityName(test, offset + new Random().nextInt(numberOfRoles)), Collections.emptyMap(), RoleResource.FIELDS, ADMIN_AUTH_HEADERS); 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 337b930ec4a..d15efa96979 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 @@ -101,19 +101,15 @@ public class UserResourceTest extends EntityResourceTest { @Order(Integer.MAX_VALUE) // Run this test last to avoid side effects of default role creation to fail other tests. @Test void post_userWithDefaultRole(TestInfo test) throws IOException { - // Given no default role has been set, when a user is created, then no role should be assigned. - CreateUser create = createRequest(test, 1); - createUserAndCheckRoles(create, Collections.emptyList()); - RoleResourceTest roleResourceTest = new RoleResourceTest(); - Role defaultRole = roleResourceTest.createRolesAndSetDefault(test, 7); List roles = roleResourceTest.listEntities(Collections.emptyMap(), ADMIN_AUTH_HEADERS).getData(); UUID nonDefaultRoleId = roles.stream().filter(role -> !role.getDefault()).findAny().orElseThrow().getId(); - UUID defaultRoleId = defaultRole.getId(); + UUID defaultRoleId = + roles.stream().filter(Role::getDefault).findAny().orElseThrow().getId(); // DataConsumer is default role. // Given a default role has been set, when a user is created without any roles, then the default role should be // assigned. - create = createRequest(test, 2); + CreateUser create = createRequest(test, 2); createUserAndCheckRoles(create, Arrays.asList(defaultRoleId)); // Given a default role has been set, when a user is created with a non default role, then the default role should @@ -244,12 +240,24 @@ public class UserResourceTest extends EntityResourceTest { RoleResourceTest roleResourceTest = new RoleResourceTest(); Role role1 = roleResourceTest.createEntity(roleResourceTest.createRequest(test, 1), ADMIN_AUTH_HEADERS); Role role2 = roleResourceTest.createEntity(roleResourceTest.createRequest(test, 2), ADMIN_AUTH_HEADERS); + UUID defaultRoleId = + roleResourceTest.listEntities(Collections.emptyMap(), ADMIN_AUTH_HEADERS).getData().stream() + .filter(Role::getDefault) + .findAny() + .orElseThrow() + .getId(); // DataConsumer is default role. + EntityReference defaultRoleRef = + new RoleEntityInterface(roleResourceTest.getEntity(defaultRoleId, RoleResource.FIELDS, ADMIN_AUTH_HEADERS)) + .getEntityReference(); + List roles = Arrays.asList(role1.getId(), role2.getId()); CreateUser create = createRequest(test).withRoles(roles); - User user = createAndCheckEntity(create, ADMIN_AUTH_HEADERS); + List createdRoles = Arrays.asList(role1.getId(), role2.getId(), defaultRoleRef.getId()); + CreateUser created = createRequest(test).withRoles(createdRoles); + User user = createAndCheckEntity(create, ADMIN_AUTH_HEADERS, created); // Ensure User has relationship to these roles - String[] expectedRoles = roles.stream().map(UUID::toString).sorted().toArray(String[]::new); + String[] expectedRoles = createdRoles.stream().map(UUID::toString).sorted().toArray(String[]::new); List roleReferences = user.getRoles(); String[] actualRoles = roleReferences.stream().map(ref -> ref.getId().toString()).sorted().toArray(String[]::new); assertArrayEquals(expectedRoles, actualRoles); @@ -391,11 +399,16 @@ public class UserResourceTest extends EntityResourceTest { Profile profile = new Profile().withImages(new ImageList().withImage(URI.create("http://image.com"))); RoleResourceTest roleResourceTest = new RoleResourceTest(); + List roles = roleResourceTest.listEntities(Collections.emptyMap(), ADMIN_AUTH_HEADERS).getData(); + UUID defaultRoleId = + roles.stream().filter(Role::getDefault).findAny().orElseThrow().getId(); // DataConsumer is default role. + EntityReference defaultRoleRef = + new RoleEntityInterface(roleResourceTest.getEntity(defaultRoleId, RoleResource.FIELDS, ADMIN_AUTH_HEADERS)) + .getEntityReference(); EntityReference role1 = new RoleEntityInterface( roleResourceTest.createEntity(roleResourceTest.createRequest(test, 1), ADMIN_AUTH_HEADERS)) .getEntityReference(); - List roles1 = Arrays.asList(role1); // // Add previously absent attributes @@ -403,22 +416,31 @@ public class UserResourceTest extends EntityResourceTest { String origJson = JsonUtils.pojoToJson(user); String timezone = "America/Los_Angeles"; - user.withRoles(roles1) + user.withRoles(Arrays.asList(role1, defaultRoleRef)) .withTeams(teams) .withTimezone(timezone) .withDisplayName("displayName") .withProfile(profile) .withIsBot(false) .withIsAdmin(false); + User update = + JsonUtils.readValue(origJson, User.class) + .withRoles(Arrays.asList(role1)) + .withTeams(teams) + .withTimezone(timezone) + .withDisplayName("displayName") + .withProfile(profile) + .withIsBot(false) + .withIsAdmin(false); ChangeDescription change = getChangeDescription(user.getVersion()); - change.getFieldsAdded().add(new FieldChange().withName("roles").withNewValue(roles1)); + change.getFieldsAdded().add(new FieldChange().withName("roles").withNewValue(Arrays.asList(role1))); change.getFieldsAdded().add(new FieldChange().withName("teams").withNewValue(teams)); change.getFieldsAdded().add(new FieldChange().withName("timezone").withNewValue(timezone)); change.getFieldsAdded().add(new FieldChange().withName("displayName").withNewValue("displayName")); change.getFieldsAdded().add(new FieldChange().withName("profile").withNewValue(profile)); change.getFieldsAdded().add(new FieldChange().withName("isBot").withNewValue(false)); - user = patchEntityAndCheck(user, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change); + user = patchEntityAndCheck(user, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change, update); // // Replace the attributes @@ -431,20 +453,28 @@ public class UserResourceTest extends EntityResourceTest { new RoleEntityInterface( roleResourceTest.createEntity(roleResourceTest.createRequest(test, 2), ADMIN_AUTH_HEADERS)) .getEntityReference(); - List roles2 = Arrays.asList(role2); origJson = JsonUtils.pojoToJson(user); - user.withRoles(roles2) + user.withRoles(Arrays.asList(role2, defaultRoleRef)) .withTeams(teams1) .withTimezone(timezone1) .withDisplayName("displayName1") .withProfile(profile1) .withIsBot(true) .withIsAdmin(false); + update = + JsonUtils.readValue(origJson, User.class) + .withRoles(Arrays.asList(role2)) + .withTeams(teams1) + .withTimezone(timezone1) + .withDisplayName("displayName1") + .withProfile(profile1) + .withIsBot(true) + .withIsAdmin(false); change = getChangeDescription(user.getVersion()); - change.getFieldsDeleted().add(new FieldChange().withName("roles").withOldValue(roles1)); - change.getFieldsAdded().add(new FieldChange().withName("roles").withNewValue(roles2)); + change.getFieldsDeleted().add(new FieldChange().withName("roles").withOldValue(Arrays.asList(role1))); + change.getFieldsAdded().add(new FieldChange().withName("roles").withNewValue(Arrays.asList(role2))); change.getFieldsDeleted().add(new FieldChange().withName("teams").withOldValue(List.of(team2))); change.getFieldsAdded().add(new FieldChange().withName("teams").withNewValue(List.of(team3))); change @@ -455,30 +485,38 @@ public class UserResourceTest extends EntityResourceTest { .add(new FieldChange().withName("displayName").withOldValue("displayName").withNewValue("displayName1")); change.getFieldsUpdated().add(new FieldChange().withName("profile").withOldValue(profile).withNewValue(profile1)); change.getFieldsUpdated().add(new FieldChange().withName("isBot").withOldValue(false).withNewValue(true)); - - user = patchEntityAndCheck(user, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change); + user = patchEntityAndCheck(user, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change, update); // // Remove the attributes // origJson = JsonUtils.pojoToJson(user); - user.withRoles(null) + user.withRoles(Arrays.asList(defaultRoleRef)) .withTeams(null) .withTimezone(null) .withDisplayName(null) .withProfile(null) .withIsBot(null) .withIsAdmin(false); + update = + JsonUtils.readValue(origJson, User.class) + .withRoles(null) + .withTeams(null) + .withTimezone(null) + .withDisplayName(null) + .withProfile(null) + .withIsBot(null) + .withIsAdmin(false); // Note non-empty display field is not deleted change = getChangeDescription(user.getVersion()); - change.getFieldsDeleted().add(new FieldChange().withName("roles").withOldValue(roles2)); + change.getFieldsDeleted().add(new FieldChange().withName("roles").withOldValue(Arrays.asList(role2))); change.getFieldsDeleted().add(new FieldChange().withName("teams").withOldValue(teams1)); change.getFieldsDeleted().add(new FieldChange().withName("timezone").withOldValue(timezone1)); change.getFieldsDeleted().add(new FieldChange().withName("displayName").withOldValue("displayName1")); change.getFieldsDeleted().add(new FieldChange().withName("profile").withOldValue(profile1)); change.getFieldsDeleted().add(new FieldChange().withName("isBot").withOldValue(true).withNewValue(null)); - patchEntityAndCheck(user, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change); + patchEntityAndCheck(user, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change, update); } @Test diff --git a/ingestion-core/src/metadata/_version.py b/ingestion-core/src/metadata/_version.py index 74cf1be929c..e39e68bcc81 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, 9, 0, dev=10) +__version__ = Version("metadata", 0, 9, 0, dev=11) __all__ = ["__version__"]