From 736637a598450a7c2baa2ec6c88f7ac1f6df46f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Oliveira?= <39243097+andrefpoliveira@users.noreply.github.com> Date: Sat, 1 Oct 2022 19:51:39 +0100 Subject: [PATCH] Merging if statements (#7851) --- .../events/AbstractEventPublisher.java | 6 ++-- .../service/jdbi3/CollectionDAO.java | 18 ++++------- .../service/jdbi3/EntityRepository.java | 7 ++--- .../service/jdbi3/TableRepository.java | 20 ++++++------ .../service/jdbi3/UserRepository.java | 8 ++--- .../service/resources/teams/UserResource.java | 24 +++++++------- .../service/security/JwtFilter.java | 8 ++--- .../policyevaluator/CompiledRule.java | 24 ++++++-------- .../util/ElasticSearchClientUtils.java | 31 +++++++++---------- 9 files changed, 64 insertions(+), 82 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/events/AbstractEventPublisher.java b/openmetadata-service/src/main/java/org/openmetadata/service/events/AbstractEventPublisher.java index b4261079763..aa781a17f5a 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/events/AbstractEventPublisher.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/events/AbstractEventPublisher.java @@ -50,10 +50,8 @@ public abstract class AbstractEventPublisher implements EventPublisher { throws Exception { // Ignore events that don't match the webhook event filters ChangeEvent changeEvent = changeEventHolder.get(); - if (!filter.isEmpty()) { - if (!FilterUtil.shouldProcessRequest(changeEvent, filter)) { - return; - } + if (!filter.isEmpty() && !FilterUtil.shouldProcessRequest(changeEvent, filter)) { + return; } // Batch until either the batch has ended or batch size has reached the max size diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java index aecb97deb56..201da5df5ed 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java @@ -967,10 +967,8 @@ public interface CollectionDAO { return listAnnouncementsByEntityLinkBefore( fqnPrefix, toType, limit, before, type, relation, mysqlCondition, postgresCondition); } - if (userName != null) { - if (filterType == FilterType.MENTIONS) { - filterRelation = Relationship.MENTIONED_IN.ordinal(); - } + if (userName != null && filterType == FilterType.MENTIONS) { + filterRelation = Relationship.MENTIONED_IN.ordinal(); } return listThreadsByEntityLinkBefore( fqnPrefix, toType, limit, before, type, status, resolved, relation, userName, teamNames, filterRelation); @@ -1058,10 +1056,8 @@ public interface CollectionDAO { return listAnnouncementsByEntityLinkAfter( fqnPrefix, toType, limit, after, type, relation, mysqlCondition, postgresCondition); } - if (userName != null) { - if (filterType == FilterType.MENTIONS) { - filterRelation = Relationship.MENTIONED_IN.ordinal(); - } + if (userName != null && filterType == FilterType.MENTIONS) { + filterRelation = Relationship.MENTIONED_IN.ordinal(); } return listThreadsByEntityLinkAfter( fqnPrefix, toType, limit, after, type, status, resolved, relation, userName, teamNames, filterRelation); @@ -1146,10 +1142,8 @@ public interface CollectionDAO { } return listCountAnnouncementsByEntityLink(fqnPrefix, toType, type, relation, mysqlCondition, postgresCondition); } - if (userName != null) { - if (filterType == FilterType.MENTIONS) { - filterRelation = Relationship.MENTIONED_IN.ordinal(); - } + if (userName != null && filterType == FilterType.MENTIONS) { + filterRelation = Relationship.MENTIONED_IN.ordinal(); } return listCountThreadsByEntityLink( fqnPrefix, toType, type, status, resolved, relation, userName, teamNames, filterRelation); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java index c8e8fc17eb1..d925d5cb160 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java @@ -1221,11 +1221,10 @@ public abstract class EntityRepository { private void updateOwner() throws JsonProcessingException { EntityReference origOwner = original.getOwner(); EntityReference updatedOwner = updated.getOwner(); - if (operation.isPatch() || updatedOwner != null) { + if ((operation.isPatch() || updatedOwner != null) + && recordChange(FIELD_OWNER, origOwner, updatedOwner, true, entityReferenceMatch)) { // Update owner for all PATCH operations. For PUT operations, ownership can't be removed - if (recordChange(FIELD_OWNER, origOwner, updatedOwner, true, entityReferenceMatch)) { - EntityRepository.this.updateOwner(original, origOwner, updatedOwner); - } + EntityRepository.this.updateOwner(original, origOwner, updatedOwner); } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TableRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TableRepository.java index 522153fad8a..53bdf6abbe1 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TableRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TableRepository.java @@ -1053,22 +1053,22 @@ public class TableRepository extends EntityRepository { private void updateColumnPrecision(Column origColumn, Column updatedColumn) throws JsonProcessingException { String columnField = getColumnField(original, origColumn, "precision"); boolean updated = recordChange(columnField, origColumn.getPrecision(), updatedColumn.getPrecision()); - if (origColumn.getPrecision() != null) { // Previously precision was set - if (updated && updatedColumn.getPrecision() < origColumn.getPrecision()) { - // The precision was reduced. Treat it as backward-incompatible change - majorVersionChange = true; - } + if (origColumn.getPrecision() != null + && updated + && updatedColumn.getPrecision() < origColumn.getPrecision()) { // Previously precision was set + // The precision was reduced. Treat it as backward-incompatible change + majorVersionChange = true; } } private void updateColumnScale(Column origColumn, Column updatedColumn) throws JsonProcessingException { String columnField = getColumnField(original, origColumn, "scale"); boolean updated = recordChange(columnField, origColumn.getScale(), updatedColumn.getScale()); - if (origColumn.getScale() != null) { // Previously scale was set - if (updated && updatedColumn.getScale() < origColumn.getScale()) { - // The scale was reduced. Treat it as backward-incompatible change - majorVersionChange = true; - } + if (origColumn.getScale() != null + && updated + && updatedColumn.getScale() < origColumn.getScale()) { // Previously scale was set + // The scale was reduced. Treat it as backward-incompatible change + majorVersionChange = true; } } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/UserRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/UserRepository.java index c3a0bdde3b4..511dd9c1540 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/UserRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/UserRepository.java @@ -349,10 +349,10 @@ public class UserRepository extends EntityRepository { AuthenticationMechanism updatedAuthMechanism = updated.getAuthenticationMechanism(); if (origAuthMechanism == null && updatedAuthMechanism != null) { recordChange("authenticationMechanism", original.getAuthenticationMechanism(), "new-encrypted-value"); - } else if (origAuthMechanism != null && updatedAuthMechanism != null) { - if (!JsonUtils.areEquals(origAuthMechanism, updatedAuthMechanism)) { - recordChange("authenticationMechanism", "old-encrypted-value", "new-encrypted-value"); - } + } else if (origAuthMechanism != null + && updatedAuthMechanism != null + && !JsonUtils.areEquals(origAuthMechanism, updatedAuthMechanism)) { + recordChange("authenticationMechanism", "old-encrypted-value", "new-encrypted-value"); } } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/teams/UserResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/teams/UserResource.java index f6d3ee65309..c6c3c11f0bd 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/teams/UserResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/teams/UserResource.java @@ -426,11 +426,9 @@ public class UserResource extends EntityResource { .withUsername(securityContext.getUserPrincipal().getName()) .withToken(request.getToken()) .withLogoutTime(logoutTime)); - if (isBasicAuth()) { + if (isBasicAuth() && request.getRefreshToken() != null) { // need to clear the refresh token as well - if (request.getRefreshToken() != null) { - tokenRepository.deleteToken(request.getRefreshToken()); - } + tokenRepository.deleteToken(request.getRefreshToken()); } return Response.status(200).entity("Logout Successful").build(); } @@ -1441,16 +1439,16 @@ public class UserResource extends EntityResource { String botName = create.getBotName(); EntityInterface bot = retrieveBot(botName); // check if the bot user exists - if (!botHasRelationshipWithUser(bot, original)) { + if (!botHasRelationshipWithUser(bot, original) + && original != null + && userHasRelationshipWithAnyBot(original, bot)) { // throw an exception if user already has a relationship with a bot - if (original != null && userHasRelationshipWithAnyBot(original, bot)) { - List userBotRelationship = retrieveBotRelationshipsFor(original); - bot = - Entity.getEntityRepository(Entity.BOT) - .get(null, userBotRelationship.stream().findFirst().orElseThrow().getId(), Fields.EMPTY_FIELDS); - throw new IllegalArgumentException( - String.format("Bot user [%s] is already used by [%s] bot.", user.getName(), bot.getName())); - } + List userBotRelationship = retrieveBotRelationshipsFor(original); + bot = + Entity.getEntityRepository(Entity.BOT) + .get(null, userBotRelationship.stream().findFirst().orElseThrow().getId(), Fields.EMPTY_FIELDS); + throw new IllegalArgumentException( + String.format("Bot user [%s] is already used by [%s] bot.", user.getName(), bot.getName())); } addAuthMechanismToBot(user, create, uriInfo); RestUtil.PutResponse response = dao.createOrUpdate(uriInfo, user); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/JwtFilter.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/JwtFilter.java index d608b88ad2a..ad2736ddf2d 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/JwtFilter.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/JwtFilter.java @@ -201,11 +201,9 @@ public class JwtFilter implements ContainerRequestFilter { } // validate principal domain - if (enforcePrincipalDomain) { - if (!domain.equals(principalDomain)) { - throw new AuthenticationException( - String.format("Not Authorized! Email does not match the principal domain %s", principalDomain)); - } + if (enforcePrincipalDomain && !domain.equals(principalDomain)) { + throw new AuthenticationException( + String.format("Not Authorized! Email does not match the principal domain %s", principalDomain)); } return userName; } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/CompiledRule.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/CompiledRule.java index 0b3a5b2429c..613cf5de568 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/CompiledRule.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/CompiledRule.java @@ -132,11 +132,9 @@ public class CompiledRule extends Rule { Iterator iterator = operationContext.getOperations().listIterator(); while (iterator.hasNext()) { MetadataOperation operation = iterator.next(); - if (matchOperation(operation)) { - if (matchExpression(policyContext, subjectContext, resourceContext)) { - LOG.info("operation {} allowed", operation); - iterator.remove(); - } + if (matchOperation(operation) && matchExpression(policyContext, subjectContext, resourceContext)) { + LOG.info("operation {} allowed", operation); + iterator.remove(); } } } @@ -154,15 +152,13 @@ public class CompiledRule extends Rule { Access access = getAccess(); // Walk through all the operations in the rule and set permissions for (Permission permission : resourcePermission.getPermissions()) { - if (matchOperation(permission.getOperation())) { - if (overrideAccess(access, permission.getAccess())) { - permission - .withAccess(access) - .withRole(policyContext.getRoleName()) - .withPolicy(policyContext.getPolicyName()) - .withRule(this); - LOG.debug("Updated permission {}", permission); - } + if (matchOperation(permission.getOperation()) && overrideAccess(access, permission.getAccess())) { + permission + .withAccess(access) + .withRole(policyContext.getRoleName()) + .withPolicy(policyContext.getPolicyName()) + .withRule(this); + LOG.debug("Updated permission {}", permission); } } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/util/ElasticSearchClientUtils.java b/openmetadata-service/src/main/java/org/openmetadata/service/util/ElasticSearchClientUtils.java index a642cb25e91..918a1d49674 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/util/ElasticSearchClientUtils.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/util/ElasticSearchClientUtils.java @@ -62,22 +62,21 @@ public final class ElasticSearchClientUtils { private static SSLContext createSSLContext(ElasticSearchConfiguration elasticSearchConfiguration) throws KeyStoreException { - if (elasticSearchConfiguration.getScheme().equals("https")) { - if (elasticSearchConfiguration.getTruststorePath() != null - && !elasticSearchConfiguration.getTruststorePath().isEmpty()) { - Path trustStorePath = Paths.get(elasticSearchConfiguration.getTruststorePath()); - KeyStore truststore = KeyStore.getInstance("jks"); - try (InputStream is = Files.newInputStream(trustStorePath)) { - truststore.load(is, elasticSearchConfiguration.getTruststorePassword().toCharArray()); - SSLContextBuilder sslBuilder = SSLContexts.custom().loadTrustMaterial(truststore, null); - return sslBuilder.build(); - } catch (IOException - | NoSuchAlgorithmException - | CertificateException - | KeyStoreException - | KeyManagementException e) { - throw new RuntimeException("Failed to crete SSLContext to for ElasticSearch Client", e); - } + if (elasticSearchConfiguration.getScheme().equals("https") + && elasticSearchConfiguration.getTruststorePath() != null + && !elasticSearchConfiguration.getTruststorePath().isEmpty()) { + Path trustStorePath = Paths.get(elasticSearchConfiguration.getTruststorePath()); + KeyStore truststore = KeyStore.getInstance("jks"); + try (InputStream is = Files.newInputStream(trustStorePath)) { + truststore.load(is, elasticSearchConfiguration.getTruststorePassword().toCharArray()); + SSLContextBuilder sslBuilder = SSLContexts.custom().loadTrustMaterial(truststore, null); + return sslBuilder.build(); + } catch (IOException + | NoSuchAlgorithmException + | CertificateException + | KeyStoreException + | KeyManagementException e) { + throw new RuntimeException("Failed to crete SSLContext to for ElasticSearch Client", e); } } return null;