Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed some code smells in apollo-portal module #2 #3974

Merged
merged 1 commit into from
Sep 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@
import com.ctrip.framework.apollo.portal.service.RolePermissionService;
import com.ctrip.framework.apollo.portal.spi.UserInfoHolder;
import com.ctrip.framework.apollo.portal.util.RoleUtils;
import com.google.common.base.Strings;
import com.google.common.collect.Sets;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.data.domain.Pageable;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.util.CollectionUtils;
import org.springframework.util.StringUtils;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
Expand Down Expand Up @@ -97,7 +97,7 @@ public AppController(

@GetMapping
public List<App> findApps(@RequestParam(value = "appIds", required = false) String appIds) {
if (StringUtils.isEmpty(appIds)) {
if (Strings.isNullOrEmpty(appIds)) {
return appService.findAll();
}
return appService.findByAppIds(Sets.newHashSet(appIds.split(",")));
Expand All @@ -106,7 +106,7 @@ public List<App> findApps(@RequestParam(value = "appIds", required = false) Stri
@GetMapping("/search/by-appid-or-name")
public PageDTO<App> searchByAppIdOrAppName(@RequestParam(value = "query", required = false) String query,
Pageable pageable) {
if (StringUtils.isEmpty(query)) {
if (Strings.isNullOrEmpty(query)) {
return appService.findAll(pageable);
}
return appService.searchByAppIdOrAppName(query, pageable);
Expand Down Expand Up @@ -173,7 +173,7 @@ public MultiResponseEntity<EnvClusterInfo> nav(@PathVariable String appId) {
response.addResponseEntity(RichResponseEntity.ok(appService.createEnvNavNode(env, appId)));
} catch (Exception e) {
response.addResponseEntity(RichResponseEntity.error(HttpStatus.INTERNAL_SERVER_ERROR,
"load env:" + env.name() + " cluster error." + e
"load env:" + env.getName() + " cluster error." + e
.getMessage()));
}
}
Expand All @@ -184,7 +184,8 @@ public MultiResponseEntity<EnvClusterInfo> nav(@PathVariable String appId) {
public ResponseEntity<Void> create(@PathVariable String env, @Valid @RequestBody App app) {
appService.createAppInRemote(Env.valueOf(env), app);

roleInitializationService.initNamespaceSpecificEnvRoles(app.getAppId(), ConfigConsts.NAMESPACE_APPLICATION, env, userInfoHolder.getUser().getUserId());
roleInitializationService.initNamespaceSpecificEnvRoles(app.getAppId(), ConfigConsts.NAMESPACE_APPLICATION,
env, userInfoHolder.getUser().getUserId());

return ResponseEntity.ok().build();
}
Expand Down Expand Up @@ -228,7 +229,6 @@ public MultiResponseEntity<String> findMissEnvs(@PathVariable String appId) {
}

return response;

}

private App transformToApp(AppModel appModel) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ public void importConfigFile(@PathVariable String appId, @PathVariable String en
// check file
ConfigFileUtils.check(file);
final String format = ConfigFileUtils.getFormat(file.getOriginalFilename());
final String standardFilename = ConfigFileUtils.toFilename(appId, clusterName, namespaceName, ConfigFileFormat.fromString(format));
final String standardFilename = ConfigFileUtils.toFilename(appId, clusterName, namespaceName,
ConfigFileFormat.fromString(format));
configsImportService.importOneConfigFromFile(env, standardFilename, file.getInputStream());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -215,17 +215,14 @@ private void schedulePeriodicRefresh() {
ScheduledExecutorService scheduledExecutorService =
Executors.newScheduledThreadPool(1, ApolloThreadFactory.create("MetaServiceLocator", true));

scheduledExecutorService.scheduleAtFixedRate(new Runnable() {
@Override
public void run() {
try {
for (String metaServerAddresses : selectedMetaServerAddressCache.keySet()) {
updateMetaServerAddresses(metaServerAddresses);
}
} catch (Throwable ex) {
logger.warn(String.format("Refreshing meta server address failed, will retry in %d seconds",
REFRESH_INTERVAL_IN_SECOND), ex);
scheduledExecutorService.scheduleAtFixedRate(() -> {
try {
for (String metaServerAddresses : selectedMetaServerAddressCache.keySet()) {
updateMetaServerAddresses(metaServerAddresses);
}
} catch (Throwable ex) {
logger.warn(String.format("Refreshing meta server address failed, will retry in %d seconds",
REFRESH_INTERVAL_IN_SECOND), ex);
}
}, REFRESH_INTERVAL_IN_SECOND, REFRESH_INTERVAL_IN_SECOND, TimeUnit.SECONDS);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public void onConfigPublish(ConfigPublishEvent event) {

private class ConfigPublishNotifyTask implements Runnable {

private ConfigPublishEvent.ConfigPublishInfo publishInfo;
private final ConfigPublishEvent.ConfigPublishInfo publishInfo;

ConfigPublishNotifyTask(ConfigPublishEvent.ConfigPublishInfo publishInfo) {
this.publishInfo = publishInfo;
Expand Down Expand Up @@ -131,7 +131,7 @@ private ReleaseHistoryBO getReleaseHistory() {
/**
* webhook send
*
* @param releaseHistory
* @param releaseHistory {@link ReleaseHistoryBO}
*/
private void sendPublishWebHook(ReleaseHistoryBO releaseHistory) {
Env env = publishInfo.getEnv();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
@Component
public class CreationListener {

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

private final PortalSettings portalSettings;
private final AdminServiceAPI.AppAPI appAPI;
Expand All @@ -56,7 +56,7 @@ public void onAppCreationEvent(AppCreationEvent event) {
try {
appAPI.createApp(env, appDTO);
} catch (Throwable e) {
logger.error("Create app failed. appId = {}, env = {})", appDTO.getAppId(), env, e);
LOGGER.error("Create app failed. appId = {}, env = {})", appDTO.getAppId(), env, e);
Tracer.logError(String.format("Create app failed. appId = %s, env = %s", appDTO.getAppId(), env), e);
}
}
Expand All @@ -70,7 +70,7 @@ public void onAppNamespaceCreationEvent(AppNamespaceCreationEvent event) {
try {
namespaceAPI.createAppNamespace(env, appNamespace);
} catch (Throwable e) {
logger.error("Create appNamespace failed. appId = {}, env = {}", appNamespace.getAppId(), env, e);
LOGGER.error("Create appNamespace failed. appId = {}, env = {}", appNamespace.getAppId(), env, e);
Tracer.logError(String.format("Create appNamespace failed. appId = %s, env = %s", appNamespace.getAppId(), env), e);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,8 @@ public AppService(

public List<App> findAll() {
Iterable<App> apps = appRepository.findAll();
if (apps == null) {
return Collections.emptyList();
}
return Lists.newArrayList((apps));

return Lists.newArrayList(apps);
}

public PageDTO<App> findAll(Pageable pageable) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,11 @@
import com.ctrip.framework.apollo.portal.repository.FavoriteRepository;
import com.ctrip.framework.apollo.portal.spi.UserInfoHolder;
import com.ctrip.framework.apollo.portal.spi.UserService;
import java.util.Collections;
import com.google.common.base.Strings;
import org.springframework.data.domain.Pageable;
import org.springframework.stereotype.Service;
import org.springframework.util.StringUtils;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Objects;

Expand Down Expand Up @@ -77,8 +76,8 @@ public Favorite addFavorite(Favorite favorite) {


public List<Favorite> search(String userId, String appId, Pageable page) {
boolean isUserIdEmpty = StringUtils.isEmpty(userId);
boolean isAppIdEmpty = StringUtils.isEmpty(appId);
boolean isUserIdEmpty = Strings.isNullOrEmpty(userId);
boolean isAppIdEmpty = Strings.isNullOrEmpty(appId);

if (isAppIdEmpty && isUserIdEmpty) {
throw new BadRequestException("user id and app id can't be empty at the same time");
Expand Down Expand Up @@ -106,7 +105,6 @@ public List<Favorite> search(String userId, String appId, Pageable page) {
return Collections.singletonList(favoriteRepository.findByUserIdAndAppId(userId, appId));
}


public void deleteFavorite(long favoriteId) {
Favorite favorite = favoriteRepository.findById(favoriteId).orElse(null);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,9 @@ public void revokeItem(String appId, Env env, String clusterName, String namespa

updateItems(appId, env, clusterName, namespaceName, changeSets);

Tracer.logEvent(TracerEventType.MODIFY_NAMESPACE_BY_TEXT,
String.format("%s+%s+%s+%s", appId, env, clusterName, namespaceName));
Tracer.logEvent(TracerEventType.MODIFY_NAMESPACE, String.format("%s+%s+%s+%s", appId, env, clusterName, namespaceName));
String formatStr = String.format("%s+%s+%s+%s", appId, env, clusterName, namespaceName);
Tracer.logEvent(TracerEventType.MODIFY_NAMESPACE_BY_TEXT, formatStr);
Tracer.logEvent(TracerEventType.MODIFY_NAMESPACE, formatStr);
}

public List<ItemDiffs> compare(List<NamespaceIdentifier> comparedNamespaces, List<ItemDTO> sourceItems) {
Expand All @@ -252,7 +252,7 @@ private long getNamespaceId(NamespaceIdentifier namespaceIdentifier) {
String clusterName = namespaceIdentifier.getClusterName();
String namespaceName = namespaceIdentifier.getNamespaceName();
Env env = namespaceIdentifier.getEnv();
NamespaceDTO namespaceDTO = null;
NamespaceDTO namespaceDTO;
try {
namespaceDTO = namespaceAPI.loadNamespace(appId, env, clusterName, namespaceName);
} catch (HttpClientErrorException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
@Service
public class NamespaceService {

private Logger logger = LoggerFactory.getLogger(NamespaceService.class);
private static final Logger LOGGER = LoggerFactory.getLogger(NamespaceService.class);
private static final Gson GSON = new Gson();

private final PortalConfig portalConfig;
Expand Down Expand Up @@ -168,7 +168,7 @@ public List<NamespaceBO> findNamespaceBOs(String appId, Env env, String clusterN
namespaceBO = transformNamespace2BO(env, namespace);
namespaceBOs.add(namespaceBO);
} catch (Exception e) {
logger.error("parse namespace error. app id:{}, env:{}, clusterName:{}, namespace:{}",
LOGGER.error("parse namespace error. app id:{}, env:{}, clusterName:{}, namespace:{}",
appId, env, clusterName, namespace.getNamespaceName(), e);
throw e;
}
Expand Down Expand Up @@ -298,7 +298,7 @@ private void fillAppNamespaceProperties(NamespaceBO namespace) {
final boolean isPublic;
if (appNamespace == null) {
//dirty data
logger.warn("Dirty data, cannot find appNamespace by namespaceName [{}], appId = {}, cluster = {}, set it format to {}, make public", namespaceName, appId, clusterName, ConfigFileFormat.Properties.getValue());
LOGGER.warn("Dirty data, cannot find appNamespace by namespaceName [{}], appId = {}, cluster = {}, set it format to {}, make public", namespaceName, appId, clusterName, ConfigFileFormat.Properties.getValue());
format = ConfigFileFormat.Properties.getValue();
isPublic = true; // set to true, because public namespace allowed to delete by user
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import com.ctrip.framework.apollo.openapi.filter.ConsumerAuthenticationFilter;
import com.ctrip.framework.apollo.openapi.util.ConsumerAuditUtil;
import com.ctrip.framework.apollo.openapi.util.ConsumerAuthUtil;

import org.springframework.boot.web.servlet.FilterRegistrationBean;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
Expand All @@ -28,9 +27,11 @@
public class AuthFilterConfiguration {

@Bean
public FilterRegistrationBean openApiAuthenticationFilter(ConsumerAuthUtil consumerAuthUtil,
ConsumerAuditUtil consumerAuditUtil) {
FilterRegistrationBean openApiFilter = new FilterRegistrationBean();
public FilterRegistrationBean<ConsumerAuthenticationFilter> openApiAuthenticationFilter(
ConsumerAuthUtil consumerAuthUtil,
ConsumerAuditUtil consumerAuditUtil) {

FilterRegistrationBean<ConsumerAuthenticationFilter> openApiFilter = new FilterRegistrationBean<>();

openApiFilter.setFilter(new ConsumerAuthenticationFilter(consumerAuthUtil, consumerAuditUtil));
openApiFilter.addUrlPatterns("/openapi/*");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public void send(Email email) {

static class HTMLDataSource implements DataSource {

private String html;
private final String html;

HTMLDataSource(String htmlString) {
html = htmlString;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,11 @@ public Set<UserInfo> queryUsersWithRole(String roleName) {

List<UserRole> userRoles = userRoleRepository.findByRoleId(role.getId());

Set<UserInfo> users = userRoles.stream().map(userRole -> {
return userRoles.stream().map(userRole -> {
UserInfo userInfo = new UserInfo();
userInfo.setUserId(userRole.getUserId());
return userInfo;
}).collect(Collectors.toSet());

return users;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public class DefaultSsoHeartbeatHandler implements SsoHeartbeatHandler {
public void doHeartbeat(HttpServletRequest request, HttpServletResponse response) {
try {
response.sendRedirect("default_sso_heartbeat.html");
} catch (IOException e) {
} catch (IOException ignore) {
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,16 @@ public class FilterLdapByGroupUserSearch extends FilterBasedLdapUserSearch {

private static final Logger logger = LoggerFactory.getLogger(FilterLdapByGroupUserSearch.class);
private static final String MEMBER_UID_ATTR_NAME = "memberUid";
private String searchBase;
private String groupBase;
private String groupSearch;
private String rdnKey;
private String groupMembershipAttrName;
private String loginIdAttrName;
private final String searchBase;
private final String groupBase;
private final String groupSearch;
private final String rdnKey;
private final String groupMembershipAttrName;
private final String loginIdAttrName;

private final SearchControls searchControls = new SearchControls();

private BaseLdapPathContextSource contextSource;

private final BaseLdapPathContextSource contextSource;

public FilterLdapByGroupUserSearch(String searchBase, String searchFilter,
String groupBase, BaseLdapPathContextSource contextSource, String groupSearch,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public class LdapUserService implements UserService {
/**
* 用户信息Mapper
*/
private ContextMapper<UserInfo> ldapUserInfoMapper = (ctx) -> {
private final ContextMapper<UserInfo> ldapUserInfoMapper = (ctx) -> {
DirContextAdapter contextAdapter = (DirContextAdapter) ctx;
UserInfo userInfo = new UserInfo();
userInfo.setUserId(contextAdapter.getStringAttribute(loginIdAttrName));
Expand Down Expand Up @@ -317,9 +317,7 @@ public List<UserInfo> findByUserIds(List<String> userIds) {
return Collections.emptyList();
}
if (StringUtils.isNotBlank(groupSearch)) {
List<UserInfo> userListByGroup = searchUserInfoByGroup(groupBase, groupSearch, null,
userIds);
return userListByGroup;
return searchUserInfoByGroup(groupBase, groupSearch, null, userIds);
}
ContainerCriteria criteria = query().where(loginIdAttrName).is(userIds.get(0));
userIds.stream().skip(1).forEach(userId -> criteria.or(loginIdAttrName).is(userId));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,9 @@
import com.ctrip.framework.apollo.portal.entity.po.UserPO;
import com.ctrip.framework.apollo.portal.repository.UserRepository;
import java.util.ArrayList;
import java.util.Base64;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.ThreadLocalRandom;
import java.util.stream.Collectors;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
Expand Down