Skip to content

Commit

Permalink
refactor: removed some code smells from few classes
Browse files Browse the repository at this point in the history
  • Loading branch information
Siddikpatel committed Nov 23, 2024
1 parent a90fb6b commit 5907adf
Show file tree
Hide file tree
Showing 22 changed files with 622 additions and 319 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,8 @@ public class BizConfig extends RefreshableConfig {
new TypeToken<Map<String, Integer>>() {
}.getType();

private final BizDBPropertySource propertySource;

public BizConfig(final BizDBPropertySource propertySource) {
this.propertySource = propertySource;
}

@Override
protected List<RefreshablePropertySource> getRefreshablePropertySources() {
return Collections.singletonList(propertySource);
super(propertySource);
}

public List<String> eurekaServiceUrls() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,17 @@ public Long findReleaseIdFromGrayReleaseRule(String clientAppId, String clientIp
* load gray releases. Because gray release rules actually apply to one more dimension - cluster.
*/
public boolean hasGrayReleaseRule(String clientAppId, String clientIp, String namespaceName) {
return reversedGrayReleaseRuleCache.containsKey(assembleReversedGrayReleaseRuleKey(clientAppId,
namespaceName, clientIp)) || reversedGrayReleaseRuleCache.containsKey
(assembleReversedGrayReleaseRuleKey(clientAppId, namespaceName, GrayReleaseRuleItemDTO
.ALL_IP));

String ruleKeyViaClientIp = assembleReversedGrayReleaseRuleKey(clientAppId,
namespaceName, clientIp);
String ruleKeyViaItemDTO = assembleReversedGrayReleaseRuleKey(clientAppId, namespaceName, GrayReleaseRuleItemDTO
.ALL_IP);

boolean hasScheduleForThisKey = reversedGrayReleaseRuleCache.containsKey(ruleKeyViaClientIp);
boolean hasScheduleForThatKey = reversedGrayReleaseRuleCache.containsKey
(ruleKeyViaItemDTO);

return hasScheduleForThisKey || hasScheduleForThatKey;
}

private void scanGrayReleaseRules() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import org.springframework.stereotype.Service;
import org.springframework.transaction.TransactionStatus;
import org.springframework.transaction.annotation.Transactional;

import java.util.Date;
import java.util.Map;
import java.util.Set;
Expand Down
2 changes: 1 addition & 1 deletion apollo-build-sql-converter/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
<artifactId>spring-boot-starter-jdbc</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
</dependencies>

<profiles>
<profile>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.springframework.core.env.ConfigurableEnvironment;
import org.springframework.util.CollectionUtils;

import java.util.Collections;
import java.util.List;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
Expand All @@ -36,7 +37,7 @@
import javax.annotation.PostConstruct;


public abstract class RefreshableConfig {
public class RefreshableConfig {

private static final Logger logger = LoggerFactory.getLogger(RefreshableConfig.class);

Expand All @@ -49,18 +50,24 @@ public abstract class RefreshableConfig {
@Autowired
private ConfigurableEnvironment environment;

private List<RefreshablePropertySource> propertySources;

/**
* register refreshable property source.
* Notice: The front property source has higher priority.
*/
protected abstract List<RefreshablePropertySource> getRefreshablePropertySources();
private List<RefreshablePropertySource> propertySources;

public RefreshableConfig(RefreshablePropertySource propertySources) {
this.propertySources = Collections.singletonList(propertySources);
}

private List<RefreshablePropertySource> getPropertySources() {
return propertySources;
}

@PostConstruct
public void setup() {

propertySources = getRefreshablePropertySources();
propertySources = getPropertySources();
if (CollectionUtils.isEmpty(propertySources)) {
throw new IllegalStateException("Property sources can not be empty.");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ protected Release findLatestActiveRelease(String appId, String clusterName, Stri

ConfigCacheEntry cacheEntry = configCache.getUnchecked(cacheKey);

//cache is out-dated
if (clientMessages != null && clientMessages.has(messageKey) &&
clientMessages.get(messageKey) > cacheEntry.getNotificationId()) {
// Checking if the cache is out-dated
boolean isCacheOutdated = outdatedCacheCheck(clientMessages, messageKey, cacheEntry);
if (isCacheOutdated) {
//invalidate the cache and try to load from db again
invalidate(cacheKey);
cacheEntry = configCache.getUnchecked(cacheKey);
Expand All @@ -123,6 +123,12 @@ protected Release findLatestActiveRelease(String appId, String clusterName, Stri
return cacheEntry.getRelease();
}

private static boolean outdatedCacheCheck(ApolloNotificationMessages clientMessages, String messageKey, ConfigCacheEntry cacheEntry) {
boolean clientMessagesHasMessageKey = clientMessages != null && clientMessages.has(messageKey);
return clientMessagesHasMessageKey &&
clientMessages.get(messageKey) > cacheEntry.getNotificationId();
}

private void invalidate(String key) {
configCache.invalidate(key);
Tracer.logEvent(TRACER_EVENT_CACHE_INVALIDATE, key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,33 +18,49 @@

import static com.ctrip.framework.apollo.portal.service.SystemRoleManagerService.SYSTEM_PERMISSION_TARGET_ID;

import com.ctrip.framework.apollo.openapi.entity.ConsumerRole;
import com.ctrip.framework.apollo.openapi.repository.ConsumerRoleRepository;
import com.ctrip.framework.apollo.openapi.service.ConsumerRolePermissionService;
import com.ctrip.framework.apollo.openapi.util.ConsumerAuthUtil;
import com.ctrip.framework.apollo.portal.constant.PermissionType;
import com.ctrip.framework.apollo.portal.entity.po.Permission;
import com.ctrip.framework.apollo.portal.entity.po.RolePermission;
import com.ctrip.framework.apollo.portal.repository.PermissionRepository;
import com.ctrip.framework.apollo.portal.repository.RolePermissionRepository;
import com.ctrip.framework.apollo.portal.util.RoleUtils;
import com.nimbusds.oauth2.sdk.util.CollectionUtils;
import org.springframework.stereotype.Component;
import javax.servlet.http.HttpServletRequest;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

@Component
public class ConsumerPermissionValidator {

private final ConsumerRolePermissionService permissionService;
private final ConsumerAuthUtil consumerAuthUtil;
private final RolePermissionRepository rolePermissionRepository;
private final ConsumerRoleRepository consumerRoleRepository;
private final PermissionRepository permissionRepository;

public ConsumerPermissionValidator(final ConsumerRolePermissionService permissionService,
final ConsumerAuthUtil consumerAuthUtil) {
this.permissionService = permissionService;
public ConsumerPermissionValidator(final ConsumerAuthUtil consumerAuthUtil,
final RolePermissionRepository rolePermissionRepository
, final ConsumerRoleRepository consumerRoleRepository,
final PermissionRepository permissionRepository) {
this.consumerAuthUtil = consumerAuthUtil;
this.rolePermissionRepository = rolePermissionRepository;
this.consumerRoleRepository = consumerRoleRepository;
this.permissionRepository = permissionRepository;
}

public boolean hasModifyNamespacePermission(HttpServletRequest request, String appId,
String namespaceName, String env) {
if (hasCreateNamespacePermission(request, appId)) {
return true;
}
return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
return consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
PermissionType.MODIFY_NAMESPACE, RoleUtils.buildNamespaceTargetId(appId, namespaceName))
|| permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
|| consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
PermissionType.MODIFY_NAMESPACE,
RoleUtils.buildNamespaceTargetId(appId, namespaceName, env));

Expand All @@ -55,26 +71,54 @@ public boolean hasReleaseNamespacePermission(HttpServletRequest request, String
if (hasCreateNamespacePermission(request, appId)) {
return true;
}
return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
return consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
PermissionType.RELEASE_NAMESPACE, RoleUtils.buildNamespaceTargetId(appId, namespaceName))
|| permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
|| consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
PermissionType.RELEASE_NAMESPACE,
RoleUtils.buildNamespaceTargetId(appId, namespaceName, env));

}

public boolean hasCreateNamespacePermission(HttpServletRequest request, String appId) {
return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
return consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
PermissionType.CREATE_NAMESPACE, appId);
}

public boolean hasCreateClusterPermission(HttpServletRequest request, String appId) {
return permissionService.consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
return consumerHasPermission(consumerAuthUtil.retrieveConsumerId(request),
PermissionType.CREATE_CLUSTER, appId);
}

public boolean hasCreateApplicationPermission(HttpServletRequest request) {
long consumerId = consumerAuthUtil.retrieveConsumerId(request);
return permissionService.consumerHasPermission(consumerId, PermissionType.CREATE_APPLICATION, SYSTEM_PERMISSION_TARGET_ID);
return consumerHasPermission(consumerId, PermissionType.CREATE_APPLICATION, SYSTEM_PERMISSION_TARGET_ID);
}

public boolean consumerHasPermission(long consumerId, String permissionType, String targetId) {
Permission permission =
permissionRepository.findTopByPermissionTypeAndTargetId(permissionType, targetId);
if (permission == null) {
return false;
}

List<ConsumerRole> consumerRoles = consumerRoleRepository.findByConsumerId(consumerId);
if (CollectionUtils.isEmpty(consumerRoles)) {
return false;
}

Set<Long> roleIds =
consumerRoles.stream().map(ConsumerRole::getRoleId).collect(Collectors.toSet());
List<RolePermission> rolePermissions = rolePermissionRepository.findByRoleIdIn(roleIds);
if (CollectionUtils.isEmpty(rolePermissions)) {
return false;
}

for (RolePermission rolePermission : rolePermissions) {
if (rolePermission.getPermissionId() == permission.getId()) {
return true;
}
}

return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,8 @@ public OpenReleaseDTO createRelease(@PathVariable String appId, @PathVariable St
@PathVariable String namespaceName,
@RequestBody NamespaceReleaseDTO model,
HttpServletRequest request) {
RequestPrecondition.checkArguments(!StringUtils.isContainEmpty(model.getReleasedBy(), model
.getReleaseTitle()),
"Params(releaseTitle and releasedBy) can not be empty");
boolean doesContainEmpty = StringUtils.isContainEmpty(model.getReleasedBy(), model.getReleaseTitle());
RequestPrecondition.checkArguments(!doesContainEmpty, "Params(releaseTitle and releasedBy) can not be empty");

if (userService.findByUserId(model.getReleasedBy()) == null) {
throw BadRequestException.userNotExists(model.getReleasedBy());
Expand All @@ -99,9 +98,8 @@ public OpenReleaseDTO merge(@PathVariable String appId, @PathVariable String env
@PathVariable String clusterName, @PathVariable String namespaceName,
@PathVariable String branchName, @RequestParam(value = "deleteBranch", defaultValue = "true") boolean deleteBranch,
@RequestBody NamespaceReleaseDTO model, HttpServletRequest request) {
RequestPrecondition.checkArguments(!StringUtils.isContainEmpty(model.getReleasedBy(), model
.getReleaseTitle()),
"Params(releaseTitle and releasedBy) can not be empty");
boolean doesContainEmpty = StringUtils.isContainEmpty(model.getReleasedBy(), model.getReleaseTitle());
RequestPrecondition.checkArguments(!doesContainEmpty, "Params(releaseTitle and releasedBy) can not be empty");

if (userService.findByUserId(model.getReleasedBy()) == null) {
throw BadRequestException.userNotExists(model.getReleasedBy());
Expand All @@ -121,20 +119,15 @@ public OpenReleaseDTO createGrayRelease(@PathVariable String appId,
@PathVariable String namespaceName, @PathVariable String branchName,
@RequestBody NamespaceReleaseDTO model,
HttpServletRequest request) {
RequestPrecondition.checkArguments(!StringUtils.isContainEmpty(model.getReleasedBy(), model
.getReleaseTitle()),
"Params(releaseTitle and releasedBy) can not be empty");
boolean doesContainEmpty = StringUtils.isContainEmpty(model.getReleasedBy(), model.getReleaseTitle());
RequestPrecondition.checkArguments(!doesContainEmpty, "Params(releaseTitle and releasedBy) can not be empty");

if (userService.findByUserId(model.getReleasedBy()) == null) {
throw BadRequestException.userNotExists(model.getReleasedBy());
}

NamespaceReleaseModel releaseModel = BeanUtils.transform(NamespaceReleaseModel.class, model);

releaseModel.setAppId(appId);
releaseModel.setEnv(Env.valueOf(env).toString());
releaseModel.setClusterName(branchName);
releaseModel.setNamespaceName(namespaceName);
populateReleaseModel(releaseModel, appId, Env.valueOf(env).toString(), branchName, namespaceName);

return OpenApiBeanUtils.transformFromReleaseDTO(releaseService.publish(releaseModel));
}
Expand All @@ -146,9 +139,9 @@ public OpenReleaseDTO createGrayDelRelease(@PathVariable String appId,
@PathVariable String namespaceName, @PathVariable String branchName,
@RequestBody NamespaceGrayDelReleaseDTO model,
HttpServletRequest request) {
RequestPrecondition.checkArguments(!StringUtils.isContainEmpty(model.getReleasedBy(), model
.getReleaseTitle()),
"Params(releaseTitle and releasedBy) can not be empty");
boolean doesContainEmpty = StringUtils.isContainEmpty(model.getReleasedBy(), model.getReleaseTitle());
RequestPrecondition.checkArguments(!doesContainEmpty, "Params(releaseTitle and releasedBy) can not be empty");

RequestPrecondition.checkArguments(model.getGrayDelKeys() != null,
"Params(grayDelKeys) can not be null");

Expand All @@ -157,19 +150,17 @@ public OpenReleaseDTO createGrayDelRelease(@PathVariable String appId,
}

NamespaceGrayDelReleaseModel releaseModel = BeanUtils.transform(NamespaceGrayDelReleaseModel.class, model);
releaseModel.setAppId(appId);
releaseModel.setEnv(env.toUpperCase());
releaseModel.setClusterName(branchName);
releaseModel.setNamespaceName(namespaceName);
populateReleaseModel(releaseModel, appId, env.toUpperCase(), branchName, namespaceName);

return OpenApiBeanUtils.transformFromReleaseDTO(releaseService.publish(releaseModel, releaseModel.getReleasedBy()));
ReleaseDTO publish = releaseService.publish(releaseModel, releaseModel.getReleasedBy());
return OpenApiBeanUtils.transformFromReleaseDTO(publish);
}

@PutMapping(path = "/releases/{releaseId}/rollback")
public void rollback(@PathVariable String env,
@PathVariable long releaseId, @RequestParam String operator, HttpServletRequest request) {
RequestPrecondition.checkArguments(!StringUtils.isContainEmpty(operator),
"Param operator can not be empty");
boolean doesContainEmpty = StringUtils.isContainEmpty(operator);
RequestPrecondition.checkArguments(!doesContainEmpty, "Param operator can not be empty");

if (userService.findByUserId(operator) == null) {
throw BadRequestException.userNotExists(operator);
Expand All @@ -188,4 +179,11 @@ public void rollback(@PathVariable String env,
this.releaseOpenApiService.rollbackRelease(env, releaseId, operator);
}

private void populateReleaseModel(NamespaceReleaseModel releaseModel, String appId, String env, String branchName, String namespaceName) {
releaseModel.setAppId(appId);
releaseModel.setEnv(env.toUpperCase());
releaseModel.setClusterName(branchName);
releaseModel.setNamespaceName(namespaceName);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package com.ctrip.framework.apollo.portal.component.config;

import com.ctrip.framework.apollo.common.config.RefreshableConfig;
import com.ctrip.framework.apollo.portal.service.PortalDBPropertySource;
import com.google.common.base.Strings;
import org.springframework.stereotype.Component;

/**
* This class handled email related configs for portalConfig class
*/
@Component
public class EmailConfig extends RefreshableConfig {

public EmailConfig(final PortalDBPropertySource portalDBPropertySource) {
super(portalDBPropertySource);
}

public String emailSender() {
String value = getValue(PortalConfigConstants.EMAIL_SENDER, "");
return Strings.isNullOrEmpty(value) ? emailConfigUser() : value;
}

public String emailConfigHost() {
return getValue(PortalConfigConstants.EMAIL_CONFIG_HOST, "");
}

public String emailConfigPassword() {
return getValue(PortalConfigConstants.EMAIL_CONFIG_PASSWORD, "");
}

public String emailTemplateFramework() {
return getValue(PortalConfigConstants.EMAIL_TEMPLATE_FRAMEWORK, "");
}

public String emailReleaseDiffModuleTemplate() {
return getValue(PortalConfigConstants.EMAIL_RELEASE_MODULE_DIFF, "");
}

public String emailRollbackDiffModuleTemplate() {
return getValue(PortalConfigConstants.EMAIL_ROLLBACK_MODULE_DIFF, "");
}

public String emailGrayRulesModuleTemplate() {
return getValue(PortalConfigConstants.EMAIL_RELEASE_MODULE_RULES, "");
}

public String emailConfigUser() {
return getValue(PortalConfigConstants.EMAIL_CONFIG_USER, "");
}

public boolean isEmailEnabled() {
return getBooleanProperty(PortalConfigConstants.EMAIL_ENABLED, false);
}
}
Loading

0 comments on commit 5907adf

Please sign in to comment.