From f8edbafccfd72ace71c73e35a0e18ab4612601da Mon Sep 17 00:00:00 2001 From: Suresh Srinivas Date: Sat, 6 Aug 2022 12:50:23 -0700 Subject: [PATCH] Sonar flagged issues clean up (#6622) --- .../catalog/ResourceRegistry.java | 1 - .../catalog/events/ChangeEventHandler.java | 55 ++++++-------- .../catalog/jdbi3/CollectionDAO.java | 30 +++----- .../catalog/jdbi3/TableRepository.java | 28 ++++--- .../catalog/jdbi3/TeamRepository.java | 6 ++ .../jdbi3/TestDefinitionRepository.java | 3 +- .../catalog/resources/teams/UserResource.java | 16 ++-- .../catalog/security/DefaultAuthorizer.java | 6 +- .../security/policyevaluator/PolicyCache.java | 4 +- .../security/policyevaluator/RoleCache.java | 7 +- .../policyevaluator/SubjectCache.java | 6 +- .../catalog/socket/HeaderRequestWrapper.java | 2 +- .../catalog/socket/SocketAddressFilter.java | 5 +- .../catalog/socket/WebSocketManager.java | 73 ++++++------------- .../catalog/util/ChangeEventParser.java | 10 +-- .../openmetadata/catalog/util/RestUtil.java | 2 +- .../AirflowConfigValidationImpl.java | 1 + .../catalog/CatalogApplicationTest.java | 4 +- 18 files changed, 114 insertions(+), 145 deletions(-) diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/ResourceRegistry.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/ResourceRegistry.java index afc44665547..9de96cfb177 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/ResourceRegistry.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/ResourceRegistry.java @@ -7,7 +7,6 @@ import java.util.List; import org.openmetadata.catalog.type.ResourceDescriptor; public class ResourceRegistry { - private static final ResourceRegistry registry = new ResourceRegistry(); private static final List RESOURCE_DESCRIPTORS = new ArrayList<>(); private ResourceRegistry() {} diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/events/ChangeEventHandler.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/events/ChangeEventHandler.java index 8f5670e5782..9a33e62be7b 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/events/ChangeEventHandler.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/events/ChangeEventHandler.java @@ -118,7 +118,7 @@ public class ChangeEventHandler implements EventHandler { EntityLink about = EntityLink.parse(thread.getAbout()); feedDao.create(thread, entity.getId(), owner, about); String jsonThread = mapper.writeValueAsString(thread); - WebSocketManager.getInstance().broadCastMessageToAll(WebSocketManager.feedBroadcastChannel, jsonThread); + WebSocketManager.getInstance().broadCastMessageToAll(WebSocketManager.FEED_BROADCAST_CHANNEL, jsonThread); } } } @@ -140,23 +140,23 @@ public class ChangeEventHandler implements EventHandler { if (thread.getPostsCount() == 0) { List assignees = thread.getTask().getAssignees(); assignees.forEach( - (e) -> { + e -> { if (Entity.USER.equals(e.getType())) { WebSocketManager.getInstance() - .sendToOne(e.getId(), WebSocketManager.taskBroadcastChannel, jsonThread); + .sendToOne(e.getId(), WebSocketManager.TASK_BROADCAST_CHANNEL, jsonThread); } else if (Entity.TEAM.equals(e.getType())) { // fetch all that are there in the team List records = dao.relationshipDAO() .findTo(e.getId().toString(), TEAM, Relationship.HAS.ordinal(), Entity.USER); WebSocketManager.getInstance() - .sendToManyWithString(records, WebSocketManager.taskBroadcastChannel, jsonThread); + .sendToManyWithString(records, WebSocketManager.TASK_BROADCAST_CHANNEL, jsonThread); } }); - return; } + break; case Conversation: - WebSocketManager.getInstance().broadCastMessageToAll(WebSocketManager.feedBroadcastChannel, jsonThread); + WebSocketManager.getInstance().broadCastMessageToAll(WebSocketManager.FEED_BROADCAST_CHANNEL, jsonThread); List mentions; if (thread.getPostsCount() == 0) { mentions = MessageParser.getEntityLinks(thread.getMessage()); @@ -165,29 +165,25 @@ public class ChangeEventHandler implements EventHandler { mentions = MessageParser.getEntityLinks(latestPost.getMessage()); } mentions.forEach( - (entityLink) -> { + entityLink -> { String fqn = entityLink.getEntityFQN(); - switch (entityLink.getEntityType()) { - case USER: - User user = dao.userDAO().findEntityByName(fqn); - WebSocketManager.getInstance() - .sendToOne(user.getId(), WebSocketManager.mentionChannel, jsonThread); - break; - case TEAM: - Team team = dao.teamDAO().findEntityByName(fqn); - // fetch all that are there in the team - List records = - dao.relationshipDAO() - .findTo(team.getId().toString(), TEAM, Relationship.HAS.ordinal(), Entity.USER); - WebSocketManager.getInstance() - .sendToManyWithString(records, WebSocketManager.mentionChannel, jsonThread); - break; + if (USER.equals(entityLink.getEntityType())) { + User user = dao.userDAO().findEntityByName(fqn); + WebSocketManager.getInstance() + .sendToOne(user.getId(), WebSocketManager.MENTION_CHANNEL, jsonThread); + } else if (TEAM.equals(entityLink.getEntityType())) { + Team team = dao.teamDAO().findEntityByName(fqn); + // fetch all that are there in the team + List records = + dao.relationshipDAO().findTo(team.getId().toString(), TEAM, Relationship.HAS.ordinal(), USER); + WebSocketManager.getInstance() + .sendToManyWithString(records, WebSocketManager.MENTION_CHANNEL, jsonThread); } }); - return; + break; case Announcement: default: - return; + break; } } catch (JsonProcessingException e) { throw new RuntimeException(e); @@ -234,13 +230,10 @@ public class ChangeEventHandler implements EventHandler { String entityType = entityReference.getType(); String entityFQN = entityReference.getFullyQualifiedName(); EventType eventType = null; - switch (changeType) { - case RestUtil.ENTITY_UPDATED: - eventType = ENTITY_UPDATED; - break; - case RestUtil.ENTITY_SOFT_DELETED: - eventType = ENTITY_SOFT_DELETED; - break; + if (RestUtil.ENTITY_UPDATED.equals(changeType)) { + eventType = ENTITY_UPDATED; + } else if (RestUtil.ENTITY_SOFT_DELETED.equals(changeType)) { + eventType = ENTITY_SOFT_DELETED; } return getChangeEvent(eventType, entityType, entityInterface) diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java index 2cc442855a8..3e53f1e07ac 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java @@ -2130,14 +2130,12 @@ public interface CollectionDAO { @Override default int listCount(ListFilter filter) { String team = filter.getQueryParam("team"); - Boolean isAdmin = null; - Boolean isBot = null; String isBotStr = filter.getQueryParam("isBot"); String isAdminStr = filter.getQueryParam("isAdmin"); String mySqlCondition = filter.getCondition("ue"); String postgresCondition = filter.getCondition("ue"); if (isAdminStr != null) { - isAdmin = Boolean.parseBoolean(isAdminStr); + boolean isAdmin = Boolean.parseBoolean(isAdminStr); if (isAdmin) { mySqlCondition = String.format("%s AND JSON_EXTRACT(ue.json, '$.isAdmin') = TRUE ", mySqlCondition); postgresCondition = String.format("%s AND ((ue.json#>'{isAdmin}')::boolean) = TRUE ", postgresCondition); @@ -2153,7 +2151,7 @@ public interface CollectionDAO { } } if (isBotStr != null) { - isBot = Boolean.parseBoolean(isBotStr); + boolean isBot = Boolean.parseBoolean(isBotStr); if (isBot) { mySqlCondition = String.format("%s AND JSON_EXTRACT(ue.json, '$.isBot') = TRUE ", mySqlCondition); postgresCondition = String.format("%s AND ((ue.json#>'{isBot}')::boolean) = TRUE ", postgresCondition); @@ -2167,7 +2165,7 @@ public interface CollectionDAO { "%s AND ue.json#>'{isBot}' IS NULL OR ((ue.json#>'{isBot}')::boolean) = FALSE ", postgresCondition); } } - if (team == null && isAdmin == null && isBot == null) { + if (team == null && isAdminStr == null && isBotStr == null) { return EntityDAO.super.listCount(filter); } return listCount( @@ -2177,14 +2175,12 @@ public interface CollectionDAO { @Override default List listBefore(ListFilter filter, int limit, String before) { String team = filter.getQueryParam("team"); - Boolean isAdmin = null; - Boolean isBot = null; String isBotStr = filter.getQueryParam("isBot"); String isAdminStr = filter.getQueryParam("isAdmin"); String mySqlCondition = filter.getCondition("ue"); String postgresCondition = filter.getCondition("ue"); if (isAdminStr != null) { - isAdmin = Boolean.parseBoolean(isAdminStr); + boolean isAdmin = Boolean.parseBoolean(isAdminStr); if (isAdmin) { mySqlCondition = String.format("%s AND JSON_EXTRACT(ue.json, '$.isAdmin') = TRUE ", mySqlCondition); postgresCondition = String.format("%s AND ((ue.json#>'{isAdmin}')::boolean) = TRUE ", postgresCondition); @@ -2200,7 +2196,7 @@ public interface CollectionDAO { } } if (isBotStr != null) { - isBot = Boolean.parseBoolean(isBotStr); + boolean isBot = Boolean.parseBoolean(isBotStr); if (isBot) { mySqlCondition = String.format("%s AND JSON_EXTRACT(ue.json, '$.isBot') = TRUE ", mySqlCondition); postgresCondition = String.format("%s AND ((ue.json#>'{isBot}')::boolean) = TRUE ", postgresCondition); @@ -2214,7 +2210,7 @@ public interface CollectionDAO { "%s AND ue.json#>'{isBot}' IS NULL OR ((ue.json#>'{isBot}')::boolean) = FALSE ", postgresCondition); } } - if (team == null && isAdmin == null && isBot == null) { + if (team == null && isAdminStr == null && isBotStr == null) { return EntityDAO.super.listBefore(filter, limit, before); } return listBefore( @@ -2231,14 +2227,12 @@ public interface CollectionDAO { @Override default List listAfter(ListFilter filter, int limit, String after) { String team = filter.getQueryParam("team"); - Boolean isAdmin = null; - Boolean isBot = null; String isBotStr = filter.getQueryParam("isBot"); String isAdminStr = filter.getQueryParam("isAdmin"); String mySqlCondition = filter.getCondition("ue"); String postgresCondition = filter.getCondition("ue"); if (isAdminStr != null) { - isAdmin = Boolean.parseBoolean(isAdminStr); + boolean isAdmin = Boolean.parseBoolean(isAdminStr); if (isAdmin) { mySqlCondition = String.format("%s AND JSON_EXTRACT(ue.json, '$.isAdmin') = TRUE ", mySqlCondition); postgresCondition = String.format("%s AND ((ue.json#>'{isAdmin}')::boolean) = TRUE ", postgresCondition); @@ -2254,7 +2248,7 @@ public interface CollectionDAO { } } if (isBotStr != null) { - isBot = Boolean.parseBoolean(isBotStr); + boolean isBot = Boolean.parseBoolean(isBotStr); if (isBot) { mySqlCondition = String.format("%s AND JSON_EXTRACT(ue.json, '$.isBot') = TRUE ", mySqlCondition); postgresCondition = String.format("%s AND ((ue.json#>'{isBot}')::boolean) = TRUE ", postgresCondition); @@ -2268,7 +2262,7 @@ public interface CollectionDAO { "%s AND ue.json#>'{isBot}' IS NULL OR ((ue.json#>'{isBot}')::boolean) = FALSE ", postgresCondition); } } - if (team == null && isAdmin == null && isBot == null) { + if (team == null && isAdminStr == null && isBotStr == null) { return EntityDAO.super.listAfter(filter, limit, after); } return listAfter( @@ -2703,7 +2697,7 @@ public interface CollectionDAO { if (entityFqn != null) { condition = String.format("%s AND entityFqn='%s' ", condition, entityFqn); } - if (startTs != null & endTs != null) { + if (startTs != null && endTs != null) { condition = String.format( "%s AND timestamp BETWEEN %d and %d ", condition, Long.parseLong(startTs), Long.parseLong(endTs)); @@ -2737,7 +2731,7 @@ public interface CollectionDAO { if (entityFqn != null) { condition = String.format("%s AND entityFqn='%s' ", condition, entityFqn); } - if (startTs != null & endTs != null) { + if (startTs != null && endTs != null) { condition = String.format( "%s AND timestamp BETWEEN %d and %d ", condition, Long.parseLong(startTs), Long.parseLong(endTs)); @@ -2769,7 +2763,7 @@ public interface CollectionDAO { if (entityFqn != null) { condition = String.format("%s AND entityFqn='%s' ", condition, entityFqn); } - if (startTs != null & endTs != null) { + if (startTs != null && endTs != null) { condition = String.format( "%s AND timestamp BETWEEN %d and %d ", condition, Long.parseLong(startTs), Long.parseLong(endTs)); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TableRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TableRepository.java index d021b89a49c..5809ee84bb3 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TableRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TableRepository.java @@ -98,6 +98,9 @@ public class TableRepository extends EntityRepository { public static final String FIELD_RELATION_COLUMN_TYPE = "table.columns.column"; public static final String FIELD_RELATION_TABLE_TYPE = "table"; + public static final String TABLE_PROFILE_EXTENSION = "table.tableProfile"; + public static final String TABLE_SAMPLE_DATA_EXTENSION = "table.sampleData"; + public static final String TABLE_PROFILER_CONFIG_EXTENSION = "table.tableProfilerConfig"; public TableRepository(CollectionDAO daoCollection) { super( @@ -208,7 +211,7 @@ public class TableRepository extends EntityRepository
{ daoCollection .entityExtensionDAO() - .insert(tableId.toString(), "table.sampleData", "tableData", JsonUtils.pojoToJson(tableData)); + .insert(tableId.toString(), TABLE_SAMPLE_DATA_EXTENSION, "tableData", JsonUtils.pojoToJson(tableData)); setFields(table, Fields.EMPTY_FIELDS); return table.withSampleData(tableData); } @@ -216,7 +219,7 @@ public class TableRepository extends EntityRepository
{ @Transaction public TableProfilerConfig getTableProfilerConfig(Table table) throws IOException { return JsonUtils.readValue( - daoCollection.entityExtensionDAO().getExtension(table.getId().toString(), "table.tableProfilerConfig"), + daoCollection.entityExtensionDAO().getExtension(table.getId().toString(), TABLE_PROFILER_CONFIG_EXTENSION), TableProfilerConfig.class); } @@ -242,7 +245,7 @@ public class TableRepository extends EntityRepository
{ .entityExtensionDAO() .insert( tableId.toString(), - "table.tableProfilerConfig", + TABLE_PROFILER_CONFIG_EXTENSION, "tableProfilerConfig", JsonUtils.pojoToJson(tableProfilerConfig)); setFields(table, Fields.EMPTY_FIELDS); @@ -254,7 +257,7 @@ public class TableRepository extends EntityRepository
{ // Validate the request content Table table = dao.findEntityById(tableId); - daoCollection.entityExtensionDAO().delete(tableId.toString(), "table.tableProfilerConfig"); + daoCollection.entityExtensionDAO().delete(tableId.toString(), TABLE_PROFILER_CONFIG_EXTENSION); setFields(table, Fields.EMPTY_FIELDS); return table; } @@ -273,14 +276,14 @@ public class TableRepository extends EntityRepository
{ JsonUtils.readValue( daoCollection .entityExtensionTimeSeriesDao() - .getExtensionAtTimestamp(tableId.toString(), "table.tableProfile", tableProfile.getTimestamp()), + .getExtensionAtTimestamp(tableId.toString(), TABLE_PROFILE_EXTENSION, tableProfile.getTimestamp()), TableProfile.class); if (storedTableProfile != null) { daoCollection .entityExtensionTimeSeriesDao() .update( tableId.toString(), - "table.tableProfile", + TABLE_PROFILE_EXTENSION, JsonUtils.pojoToJson(tableProfile), tableProfile.getTimestamp()); } else { @@ -289,7 +292,7 @@ public class TableRepository extends EntityRepository
{ .insert( tableId.toString(), table.getFullyQualifiedName(), - "table.tableProfile", + TABLE_PROFILE_EXTENSION, "tableProfile", JsonUtils.pojoToJson(tableProfile)); setFields(table, Fields.EMPTY_FIELDS); @@ -305,12 +308,12 @@ public class TableRepository extends EntityRepository
{ JsonUtils.readValue( daoCollection .entityExtensionTimeSeriesDao() - .getExtensionAtTimestamp(tableId.toString(), "table.tableProfile", timestamp), + .getExtensionAtTimestamp(tableId.toString(), TABLE_PROFILE_EXTENSION, timestamp), TableProfile.class); if (storedTableProfile != null) { daoCollection .entityExtensionTimeSeriesDao() - .deleteAtTimestamp(tableId.toString(), "table.tableProfile", timestamp); + .deleteAtTimestamp(tableId.toString(), TABLE_PROFILE_EXTENSION, timestamp); table.setTableProfile(storedTableProfile); return table; } @@ -991,12 +994,15 @@ public class TableRepository extends EntityRepository
{ private TableData getSampleData(Table table) throws IOException { return JsonUtils.readValue( - daoCollection.entityExtensionDAO().getExtension(table.getId().toString(), "table.sampleData"), TableData.class); + daoCollection.entityExtensionDAO().getExtension(table.getId().toString(), TABLE_SAMPLE_DATA_EXTENSION), + TableData.class); } private TableProfile getTableProfile(Table table) throws IOException { return JsonUtils.readValue( - daoCollection.entityExtensionTimeSeriesDao().getLatestExtension(table.getId().toString(), "table.tableProfile"), + daoCollection + .entityExtensionTimeSeriesDao() + .getLatestExtension(table.getId().toString(), TABLE_PROFILE_EXTENSION), TableProfile.class); } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TeamRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TeamRepository.java index ee2e8c8eaaa..6b0eb592e1c 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TeamRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TeamRepository.java @@ -223,6 +223,11 @@ public class TeamRepository extends EntityRepository { } List children = getTeams(childrenRefs); switch (team.getTeamType()) { + case GROUP: + if (!children.isEmpty()) { + throw new IllegalArgumentException(CatalogExceptionMessage.createGroup()); + } + break; case DEPARTMENT: validateChildren(team, children, DEPARTMENT); break; @@ -249,6 +254,7 @@ public class TeamRepository extends EntityRepository { } List parents = getTeams(parentRefs); switch (team.getTeamType()) { + case GROUP: case DEPARTMENT: validateParents(team, parents, DEPARTMENT, DIVISION, BUSINESS_UNIT, ORGANIZATION); break; diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TestDefinitionRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TestDefinitionRepository.java index 922a841fd68..fd6fd01f39f 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TestDefinitionRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TestDefinitionRepository.java @@ -3,6 +3,7 @@ package org.openmetadata.catalog.jdbi3; import static org.openmetadata.catalog.Entity.TEST_DEFINITION; import java.io.IOException; +import org.openmetadata.catalog.Entity; import org.openmetadata.catalog.resources.dqtests.TestDefinitionResource; import org.openmetadata.catalog.tests.TestDefinition; import org.openmetadata.catalog.type.EntityReference; @@ -25,7 +26,7 @@ public class TestDefinitionRepository extends EntityRepository { @Override public TestDefinition setFields(TestDefinition entity, EntityUtil.Fields fields) throws IOException { - entity.setOwner(fields.contains("owner") ? getOwner(entity) : null); + entity.setOwner(fields.contains(Entity.FIELD_OWNER) ? getOwner(entity) : null); return entity; } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/teams/UserResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/teams/UserResource.java index 1b71681fee5..d27961a3073 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/teams/UserResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/teams/UserResource.java @@ -392,9 +392,7 @@ public class UserResource extends EntityResource { throws IOException { User user = dao.get(uriInfo, id, Fields.EMPTY_FIELDS); - if (!user.getIsBot()) { - throw new IllegalArgumentException("Generating JWT token is only supported for bot users"); - } + authorizeGenerateJWT(user); authorizer.authorizeAdmin(securityContext, false); JWTAuthMechanism jwtAuthMechanism = jwtTokenGenerator.generateJWTToken(user, generateTokenRequest.getJWTTokenExpiry()); @@ -427,9 +425,7 @@ public class UserResource extends EntityResource { throws IOException { User user = dao.get(uriInfo, id, Fields.EMPTY_FIELDS); - if (!user.getIsBot()) { - throw new IllegalArgumentException("Generating JWT token is only supported for bot users"); - } + authorizeGenerateJWT(user); authorizer.authorizeAdmin(securityContext, false); JWTAuthMechanism jwtAuthMechanism = new JWTAuthMechanism().withJWTToken(StringUtils.EMPTY); AuthenticationMechanism authenticationMechanism = @@ -460,7 +456,7 @@ public class UserResource extends EntityResource { throws IOException { User user = dao.get(uriInfo, id, new Fields(List.of("authenticationMechanism"))); - if (!user.getIsBot()) { + if (!Boolean.TRUE.equals(user.getIsBot())) { throw new IllegalArgumentException("JWT token is only supported for bot users"); } authorizer.authorizeAdmin(securityContext, false); @@ -567,4 +563,10 @@ public class UserResource extends EntityResource { .withTeams(EntityUtil.toEntityReferences(create.getTeams(), Entity.TEAM)) .withRoles(EntityUtil.toEntityReferences(create.getRoles(), Entity.ROLE)); } + + private void authorizeGenerateJWT(User user) { + if (!Boolean.TRUE.equals(user.getIsBot())) { + throw new IllegalArgumentException("Generating JWT token is only supported for bot users"); + } + } } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/DefaultAuthorizer.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/DefaultAuthorizer.java index 0d1cbdc0873..fa16b2d7f91 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/DefaultAuthorizer.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/DefaultAuthorizer.java @@ -59,9 +59,9 @@ public class DefaultAuthorizer implements Authorizer { this.testUsers = new HashSet<>(config.getTestPrincipals()); this.principalDomain = config.getPrincipalDomain(); - SubjectCache.getInstance().initialize(); - PolicyCache.getInstance().initialize(); - RoleCache.getInstance().initialize(); + SubjectCache.initialize(); + PolicyCache.initialize(); + RoleCache.initialize(); LOG.debug("Admin users: {}", adminUsers); initializeUsers(); } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/PolicyCache.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/PolicyCache.java index c102c99da1b..21f526e0b13 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/PolicyCache.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/PolicyCache.java @@ -47,7 +47,7 @@ public class PolicyCache { } /** To be called during application startup by Default Authorizer */ - public void initialize() { + public static void initialize() { if (!INITIALIZED) { POLICY_CACHE = CacheBuilder.newBuilder().maximumSize(100).build(new PolicyLoader()); POLICY_REPOSITORY = Entity.getEntityRepository(Entity.POLICY); @@ -88,7 +88,7 @@ public class PolicyCache { return rules; } - public void cleanUp() { + public static void cleanUp() { POLICY_CACHE.cleanUp(); INITIALIZED = false; } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/RoleCache.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/RoleCache.java index 311e0091efe..2a64134b901 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/RoleCache.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/RoleCache.java @@ -31,7 +31,7 @@ import org.openmetadata.catalog.util.EntityUtil.Fields; /** Subject context used for Access Control Policies */ @Slf4j public class RoleCache { - private static RoleCache INSTANCE = new RoleCache(); + private static final RoleCache INSTANCE = new RoleCache(); private static volatile boolean INITIALIZED = false; protected static LoadingCache ROLE_CACHE; private static EntityRepository ROLE_REPOSITORY; @@ -42,7 +42,7 @@ public class RoleCache { } /** To be called only once during the application start from DefaultAuthorizer */ - public void initialize() { + public static void initialize() { if (!INITIALIZED) { ROLE_CACHE = CacheBuilder.newBuilder().maximumSize(100).build(new RoleLoader()); ROLE_REPOSITORY = Entity.getEntityRepository(Entity.ROLE); @@ -68,9 +68,6 @@ public class RoleCache { } static class RoleLoader extends CacheLoader { - private static final EntityRepository ROLE_REPOSITORY = Entity.getEntityRepository(Entity.ROLE); - private static final Fields FIELDS = ROLE_REPOSITORY.getFields("policies"); - @Override public Role load(@CheckForNull UUID roleId) throws IOException { Role role = ROLE_REPOSITORY.get(null, roleId.toString(), FIELDS); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/SubjectCache.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/SubjectCache.java index 79644c5ea22..3b0683a7a14 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/SubjectCache.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/security/policyevaluator/SubjectCache.java @@ -34,7 +34,7 @@ import org.openmetadata.catalog.util.EntityUtil.Fields; /** Subject context used for Access Control Policies */ @Slf4j public class SubjectCache { - private static SubjectCache INSTANCE = new SubjectCache(); + private static final SubjectCache INSTANCE = new SubjectCache(); private static volatile boolean INITIALIZED = false; protected static LoadingCache USER_CACHE; @@ -46,7 +46,7 @@ public class SubjectCache { protected static Fields TEAM_FIELDS; // Expected to be called only once from the DefaultAuthorizer - public void initialize() { + public static void initialize() { if (!INITIALIZED) { USER_CACHE = CacheBuilder.newBuilder().maximumSize(1000).expireAfterAccess(1, TimeUnit.MINUTES).build(new UserLoader()); @@ -80,7 +80,7 @@ public class SubjectCache { } } - public void cleanUp() { + public static void cleanUp() { USER_CACHE.invalidateAll(); TEAM_CACHE.invalidateAll(); INITIALIZED = false; diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/socket/HeaderRequestWrapper.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/socket/HeaderRequestWrapper.java index 62f1e217392..c34dd9027ee 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/socket/HeaderRequestWrapper.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/socket/HeaderRequestWrapper.java @@ -27,7 +27,7 @@ public class HeaderRequestWrapper extends HttpServletRequestWrapper { super(request); } - private Map headerMap = new HashMap<>(); + private final Map headerMap = new HashMap<>(); public void addHeader(String name, String value) { headerMap.put(name, value); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/socket/SocketAddressFilter.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/socket/SocketAddressFilter.java index 117e3c36ced..ff848e0a70e 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/socket/SocketAddressFilter.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/socket/SocketAddressFilter.java @@ -26,14 +26,13 @@ import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; +import lombok.extern.slf4j.Slf4j; import org.openmetadata.catalog.security.AuthenticationConfiguration; import org.openmetadata.catalog.security.AuthorizerConfiguration; import org.openmetadata.catalog.security.JwtFilter; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +@Slf4j public class SocketAddressFilter implements Filter { - private static final Logger LOG = LoggerFactory.getLogger(SocketAddressFilter.class); private JwtFilter jwtFilter; private final boolean enableSecureSocketConnection; diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/socket/WebSocketManager.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/socket/WebSocketManager.java index f617fd0794e..2ebcbb05138 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/socket/WebSocketManager.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/socket/WebSocketManager.java @@ -1,5 +1,7 @@ package org.openmetadata.catalog.socket; +import static org.openmetadata.common.utils.CommonUtil.listOrEmpty; + import io.socket.engineio.server.EngineIoServer; import io.socket.engineio.server.EngineIoServerOptions; import io.socket.socketio.server.SocketIoNamespace; @@ -7,61 +9,46 @@ import io.socket.socketio.server.SocketIoServer; import io.socket.socketio.server.SocketIoSocket; import java.util.*; import java.util.concurrent.ConcurrentHashMap; +import lombok.Getter; +import lombok.extern.slf4j.Slf4j; import org.openmetadata.catalog.jdbi3.CollectionDAO.EntityRelationshipRecord; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +@Slf4j public class WebSocketManager { - private static final Logger LOG = LoggerFactory.getLogger(WebSocketManager.class); private static WebSocketManager INSTANCE; - private final EngineIoServer mEngineIoServer; - private final SocketIoServer mSocketIoServer; - public static final String feedBroadcastChannel = "activityFeed"; - public static final String taskBroadcastChannel = "taskChannel"; - public static final String mentionChannel = "mentionChannel"; + @Getter private final EngineIoServer engineIoServer; + @Getter private final SocketIoServer socketIoServer; + public static final String FEED_BROADCAST_CHANNEL = "activityFeed"; + public static final String TASK_BROADCAST_CHANNEL = "taskChannel"; + public static final String MENTION_CHANNEL = "mentionChannel"; private final Map> activityFeedEndpoints = new ConcurrentHashMap<>(); private WebSocketManager(EngineIoServerOptions eiOptions) { - mEngineIoServer = new EngineIoServer(eiOptions); - mSocketIoServer = new SocketIoServer(mEngineIoServer); + engineIoServer = new EngineIoServer(eiOptions); + socketIoServer = new SocketIoServer(engineIoServer); initializeHandlers(); } private void initializeHandlers() { - SocketIoNamespace ns = mSocketIoServer.namespace("/"); + SocketIoNamespace ns = socketIoServer.namespace("/"); // On Connection ns.on( "connection", args -> { SocketIoSocket socket = (SocketIoSocket) args[0]; - final String userId; - String tempId; - try { - tempId = socket.getInitialHeaders().get("UserId").get(0); - } catch (Exception ex) { - tempId = socket.getInitialQuery().get("userId"); - } - userId = tempId; + List remoteAddress = socket.getInitialHeaders().get("RemoteAddress"); + Map> initialHeaders = socket.getInitialHeaders(); + List userIdHeaders = listOrEmpty(initialHeaders.get("UserId")); + String userId = userIdHeaders.isEmpty() ? socket.getInitialQuery().get("userId") : userIdHeaders.get(0); if (userId != null && !userId.equals("")) { - LOG.info( - "Client :" - + userId - + "with Remote Address :" - + socket.getInitialHeaders().get("RemoteAddress") - + "connected." - + socket.getInitialQuery()); + LOG.info("Client : {} with Remote Address:{} connected {} ", userId, remoteAddress, initialHeaders); // On Socket Disconnect socket.on( "disconnect", args1 -> { - LOG.info( - "Client from:" - + userId - + "with Remote Address :" - + socket.getInitialHeaders().get("RemoteAddress") - + " disconnected."); + LOG.info("Client from: {} with Remote Address:{} disconnected.", userId, remoteAddress); UUID id = UUID.fromString(userId); Map allUserConnection = activityFeedEndpoints.get(id); allUserConnection.remove(socket.getId()); @@ -71,20 +58,14 @@ public class WebSocketManager { "connect_error", args1 -> LOG.error( - "Connection ERROR for user:" - + userId - + "with Remote Address :" - + socket.getInitialHeaders().get("RemoteAddress") - + " disconnected.")); + "Connection ERROR for user:{} with Remote Address:{} disconnected", userId, remoteAddress)); socket.on( "connect_failed", args1 -> LOG.error( - "Connection failed ERROR for user:" - + userId - + "with Remote Address :" - + socket.getInitialHeaders().get("RemoteAddress") - + " disconnected.")); + "Connection failed ERROR for user: {} with Remote Address: {} disconnected", + userId, + remoteAddress)); UUID id = UUID.fromString(userId); Map userSocketConnections; @@ -101,14 +82,6 @@ public class WebSocketManager { return INSTANCE; } - public SocketIoServer getSocketIoServer() { - return mSocketIoServer; - } - - public EngineIoServer getEngineIoServer() { - return mEngineIoServer; - } - public Map> getActivityFeedEndpoints() { return activityFeedEndpoints; } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/ChangeEventParser.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/ChangeEventParser.java index 2c3b74ef70c..3780b46dbc3 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/ChangeEventParser.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/ChangeEventParser.java @@ -49,7 +49,6 @@ import org.openmetadata.catalog.type.FieldChange; public final class ChangeEventParser { public static final String FEED_ADD_MARKER = ""; public static final String FEED_REMOVE_MARKER = ""; - public static final String SLACK_STRIKE_MARKER = "~%s~ "; public static final String FEED_BOLD = "**%s**"; public static final String SLACK_BOLD = "*%s* "; public static final String FEED_SPAN_ADD = ""; @@ -94,12 +93,12 @@ public final class ChangeEventParser { Map messages = getFormattedMessages(PUBLISH_TO.SLACK, event.getChangeDescription(), (EntityInterface) event.getEntity()); List attachmentList = new ArrayList<>(); - for (var entryset : messages.entrySet()) { + for (var entry : messages.entrySet()) { SlackAttachment attachment = new SlackAttachment(); List mark = new ArrayList<>(); mark.add("text"); attachment.setMarkdownIn(mark); - attachment.setText(entryset.getValue()); + attachment.setText(entry.getValue()); attachmentList.add(attachment); } slackMessage.setAttachments(attachmentList.toArray(new SlackAttachment[0])); @@ -352,9 +351,8 @@ public final class ChangeEventParser { if (oldValue == null || oldValue.toString().isEmpty()) { return String.format( - "Updated " + (publishTo == PUBLISH_TO.FEED ? FEED_BOLD : SLACK_BOLD) + " to %s", - updatedField, - getFieldValue(newValue)); + "Updated %s to %s", + publishTo == PUBLISH_TO.FEED ? FEED_BOLD : SLACK_BOLD, updatedField, getFieldValue(newValue)); } else if (updatedField.contains("tags") || updatedField.contains(FIELD_OWNER)) { return getPlainTextUpdateMessage(publishTo, updatedField, getFieldValue(oldValue), getFieldValue(newValue)); } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/RestUtil.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/RestUtil.java index 6e4909c6699..f171fa36fb4 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/RestUtil.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/RestUtil.java @@ -78,7 +78,7 @@ public final class RestUtil { } public static String replaceSpaces(String s) { - s = s.replaceAll(" ", "%20"); + s = s.replace(" ", "%20"); return s; } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/validators/AirflowConfigValidationImpl.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/validators/AirflowConfigValidationImpl.java index 0991f997da7..4b5b4fed1b8 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/validators/AirflowConfigValidationImpl.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/validators/AirflowConfigValidationImpl.java @@ -71,6 +71,7 @@ public class AirflowConfigValidationImpl implements ConstraintValidator