Skip to content

Commit

Permalink
Refactoring the message splicing of internal Exceptions (#4571)
Browse files Browse the repository at this point in the history
* add tech-support-qq-4.png

* Update README.md

* Enhance the user experience in the scenario of submitting duplicate keys

* Modify the key-value conflict exception prompt, adjust the code style

* refactor(apollo): Refactoring the message splicing of internal Exceptions

Co-authored-by: Jason Song <[email protected]>
  • Loading branch information
klboke and nobodyiam authored Sep 18, 2022
1 parent d6fd359 commit 70b70c3
Show file tree
Hide file tree
Showing 22 changed files with 42 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public PageDTO<InstanceDTO> getByRelease(@RequestParam("releaseId") long release
Pageable pageable) {
Release release = releaseService.findOne(releaseId);
if (release == null) {
throw new NotFoundException(String.format("release not found for %s", releaseId));
throw new NotFoundException("release not found for %s", releaseId);
}
Page<InstanceConfig> instanceConfigsPage = instanceService.findActiveInstanceConfigsByReleaseKey
(release.getReleaseKey(), pageable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public void update(App app) {

App managedApp = appRepository.findByAppId(appId);
if (managedApp == null) {
throw new BadRequestException(String.format("App not exists. AppId = %s", appId));
throw new BadRequestException("App not exists. AppId = %s", appId);
}

managedApp.setName(app.getName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,8 @@ private Namespace findNamespaceByAppIdAndClusterNameAndNamespaceName(String appI
String namespaceName) {
Namespace namespace = namespaceService.findOne(appId, clusterName, namespaceName);
if (namespace == null) {
throw new NotFoundException(String.format("namespace not found for appId:%s clusterName:%s namespaceName:%s",
appId, clusterName, namespaceName));
throw new NotFoundException("namespace not found for appId:%s clusterName:%s namespaceName:%s",
appId, clusterName, namespaceName);
}
return namespace;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public ItemChangeSets updateSet(String appId, String clusterName,
Namespace namespace = namespaceService.findOne(appId, clusterName, namespaceName);

if (namespace == null) {
throw new NotFoundException(String.format("Namespace %s not found", namespaceName));
throw new NotFoundException("Namespace %s not found", namespaceName);
}

String operator = changeSet.getDataChangeLastModifiedBy();
Expand Down Expand Up @@ -112,7 +112,7 @@ private void doUpdateItems(List<ItemDTO> toUpdateItems, Namespace namespace, Str

Item managedItem = itemService.findOne(entity.getId());
if (managedItem == null) {
throw new NotFoundException(String.format("item not found.(key=%s)", entity.getKey()));
throw new NotFoundException("item not found.(key=%s)", entity.getKey());
}
if (managedItem.getNamespaceId() != namespace.getId()) {
throw new BadRequestException("Invalid request, item and namespace do not match!");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,11 +473,11 @@ public Release rollback(long releaseId, String operator) {
PageRequest page = PageRequest.of(0, 2);
List<Release> twoLatestActiveReleases = findActiveReleases(appId, clusterName, namespaceName, page);
if (twoLatestActiveReleases == null || twoLatestActiveReleases.size() < 2) {
throw new BadRequestException(String.format(
throw new BadRequestException(
"Can't rollback namespace(appId=%s, clusterName=%s, namespaceName=%s) because there is only one active release",
appId,
clusterName,
namespaceName));
namespaceName);
}

release.setAbandoned(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ public abstract class AbstractApolloHttpException extends RuntimeException{

/**
* When args not empty, use {@link com.google.common.base.Strings#lenientFormat(String, Object...)}
* to replace %s in msgtpl with args to set the error message. Otherwise, use msgtpl
* to replace %s in msgTpl with args to set the error message. Otherwise, use msgTpl
* to set the error message. e.g.:
* <pre>{@code new NotFoundException("... %s ... %s ... %s", "str", 0, 0.1)}</pre>
* If the number of '%s' in `msgtpl` does not match args length, the '%s' string will be printed.
* If the number of '%s' in `msgTpl` does not match args length, the '%s' string will be printed.
*/
public AbstractApolloHttpException(String msgtpl, Object... args){
super(args == null || args.length == 0 ? msgtpl : Strings.lenientFormat(msgtpl, args));
public AbstractApolloHttpException(String msgTpl, Object... args){
super(args == null || args.length == 0 ? msgTpl : Strings.lenientFormat(msgTpl, args));
}

public AbstractApolloHttpException(String msg, Exception e){
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ public class NotFoundException extends AbstractApolloHttpException {
/**
* @see AbstractApolloHttpException#AbstractApolloHttpException(String, Object...)
*/
public NotFoundException(String msgtpl, Object... args) {
super(msgtpl, args);
public NotFoundException(String msgTpl, Object... args) {
super(msgTpl, args);
setHttpStatus(HttpStatus.NOT_FOUND);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public Consumer createConsumer(Consumer consumer) {
String ownerName = consumer.getOwnerName();
UserInfo owner = userService.findByUserId(ownerName);
if (owner == null) {
throw new BadRequestException(String.format("User does not exist. UserId = %s", ownerName));
throw new BadRequestException("User does not exist. UserId = %s", ownerName);
}
consumer.setOwnerEmail(owner.getEmail());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ public OpenClusterDTO createCluster(@PathVariable String appId, @PathVariable St
@Valid @RequestBody OpenClusterDTO cluster, HttpServletRequest request) {

if (!Objects.equals(appId, cluster.getAppId())) {
throw new BadRequestException(String.format(
"AppId not equal. AppId in path = %s, AppId in payload = %s", appId, cluster.getAppId()));
throw new BadRequestException(
"AppId not equal. AppId in path = %s, AppId in payload = %s", appId, cluster.getAppId());
}

String clusterName = cluster.getName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public OpenItemDTO createItem(@PathVariable String appId, @PathVariable String e
}

if (!StringUtils.isEmpty(item.getComment()) && item.getComment().length() > ITEM_COMMENT_MAX_LENGTH) {
throw new BadRequestException(String.format("Comment length should not exceed %s characters", ITEM_COMMENT_MAX_LENGTH));
throw new BadRequestException("Comment length should not exceed %s characters", ITEM_COMMENT_MAX_LENGTH);
}

return this.itemOpenApiService.createItem(appId, env, clusterName, namespaceName, item);
Expand All @@ -118,7 +118,7 @@ public void updateItem(@PathVariable String appId, @PathVariable String env,
}

if (!StringUtils.isEmpty(item.getComment()) && item.getComment().length() > ITEM_COMMENT_MAX_LENGTH) {
throw new BadRequestException(String.format("Comment length should not exceed %s characters", ITEM_COMMENT_MAX_LENGTH));
throw new BadRequestException("Comment length should not exceed %s characters", ITEM_COMMENT_MAX_LENGTH);
}

if (createIfNotExists) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,25 +56,25 @@ public OpenAppNamespaceDTO createNamespace(@PathVariable String appId,
HttpServletRequest request) {

if (!Objects.equals(appId, appNamespaceDTO.getAppId())) {
throw new BadRequestException(String.format("AppId not equal. AppId in path = %s, AppId in payload = %s", appId,
appNamespaceDTO.getAppId()));
throw new BadRequestException("AppId not equal. AppId in path = %s, AppId in payload = %s", appId,
appNamespaceDTO.getAppId());
}
RequestPrecondition.checkArgumentsNotEmpty(appNamespaceDTO.getAppId(), appNamespaceDTO.getName(),
appNamespaceDTO.getFormat(), appNamespaceDTO.getDataChangeCreatedBy());

if (!InputValidator.isValidAppNamespace(appNamespaceDTO.getName())) {
throw new BadRequestException(String.format("Invalid Namespace format: %s",
throw new BadRequestException("Invalid Namespace format: %s",
InputValidator.INVALID_CLUSTER_NAMESPACE_MESSAGE + " & "
+ InputValidator.INVALID_NAMESPACE_NAMESPACE_MESSAGE));
+ InputValidator.INVALID_NAMESPACE_NAMESPACE_MESSAGE);
}

if (!ConfigFileFormat.isValidFormat(appNamespaceDTO.getFormat())) {
throw new BadRequestException(String.format("Invalid namespace format. format = %s", appNamespaceDTO.getFormat()));
throw new BadRequestException("Invalid namespace format. format = %s", appNamespaceDTO.getFormat());
}

String operator = appNamespaceDTO.getDataChangeCreatedBy();
if (userService.findByUserId(operator) == null) {
throw new BadRequestException(String.format("Illegal user. user = %s", operator));
throw new BadRequestException("Illegal user. user = %s", operator);
}

return this.namespaceOpenApiService.createAppNamespace(appNamespaceDTO);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public ItemChangeSets resolve(long namespaceId, String configText, List<ItemDTO>
String[] newItems = configText.split(ITEM_SEPARATOR);
Set<String> repeatKeys = new HashSet<>();
if (isHasRepeatKey(newItems, repeatKeys)) {
throw new BadRequestException(String.format("Config text has repeated keys: %s, please check your input.", repeatKeys));
throw new BadRequestException("Config text has repeated keys: %s, please check your input.", repeatKeys);
}

ItemChangeSets changeSets = new ItemChangeSets();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public List<ConsumerRole> assignNamespaceRoleToConsumer(@PathVariable String tok
continue;
}
if (Env.UNKNOWN.equals(Env.transformEnv(env))) {
throw new BadRequestException(String.format("env: %s is illegal", env));
throw new BadRequestException("env: %s is illegal", env);
}
envList.add(env);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public ReleaseDTO merge(@PathVariable String appId, @PathVariable String env,
@RequestBody NamespaceReleaseModel model) {

if (model.isEmergencyPublish() && !portalConfig.isEmergencyPublishAllowed(Env.valueOf(env))) {
throw new BadRequestException(String.format("Env: %s is not supported emergency publish now", env));
throw new BadRequestException("Env: %s is not supported emergency publish now", env);
}

ReleaseDTO createdRelease = namespaceBranchService.merge(appId, Env.valueOf(env), clusterName, namespaceName, branchName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,8 @@ public AppNamespace createAppNamespace(@PathVariable String appId,
@RequestParam(defaultValue = "true") boolean appendNamespacePrefix,
@Valid @RequestBody AppNamespace appNamespace) {
if (!InputValidator.isValidAppNamespace(appNamespace.getName())) {
throw new BadRequestException(String.format("Invalid Namespace format: %s",
InputValidator.INVALID_CLUSTER_NAMESPACE_MESSAGE + " & " + InputValidator.INVALID_NAMESPACE_NAMESPACE_MESSAGE));
throw new BadRequestException("Invalid Namespace format: %s",
InputValidator.INVALID_CLUSTER_NAMESPACE_MESSAGE + " & " + InputValidator.INVALID_NAMESPACE_NAMESPACE_MESSAGE);
}

AppNamespace createdAppNamespace = appNamespaceService.createAppNamespaceInLocal(appNamespace, appendNamespacePrefix);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ public ResponseEntity<Void> removeAppRoleFromUser(@PathVariable String appId, @P

private void checkUserExists(String userId) {
if (userService.findByUserId(userId) == null) {
throw new BadRequestException(String.format("User %s does not exist!", userId));
throw new BadRequestException("User %s does not exist!", userId);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public ReleaseDTO createRelease(@PathVariable String appId,
model.setNamespaceName(namespaceName);

if (model.isEmergencyPublish() && !portalConfig.isEmergencyPublishAllowed(Env.valueOf(env))) {
throw new BadRequestException(String.format("Env: %s is not supported emergency publish now", env));
throw new BadRequestException("Env: %s is not supported emergency publish now", env);
}

ReleaseDTO createdRelease = releaseService.publish(model);
Expand Down Expand Up @@ -110,7 +110,7 @@ public ReleaseDTO createGrayRelease(@PathVariable String appId,
model.setNamespaceName(namespaceName);

if (model.isEmergencyPublish() && !portalConfig.isEmergencyPublishAllowed(Env.valueOf(env))) {
throw new BadRequestException(String.format("Env: %s is not supported emergency publish now", env));
throw new BadRequestException("Env: %s is not supported emergency publish now", env);
}

ReleaseDTO createdRelease = releaseService.publish(model);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public List<AppNamespace> findAll() {
@Transactional
public void createDefaultAppNamespace(String appId) {
if (!isAppNamespaceNameUnique(appId, ConfigConsts.NAMESPACE_APPLICATION)) {
throw new BadRequestException(String.format("App already has application namespace. AppId = %s", appId));
throw new BadRequestException("App already has application namespace. AppId = %s", appId);
}

AppNamespace appNs = new AppNamespace();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public App createAppInLocal(App app) {
App managedApp = appRepository.findByAppId(appId);

if (managedApp != null) {
throw new BadRequestException(String.format("App already exists. AppId = %s", appId));
throw new BadRequestException("App already exists. AppId = %s", appId);
}

UserInfo owner = userService.findByUserId(app.getOwnerName());
Expand Down Expand Up @@ -178,7 +178,7 @@ public App updateAppInLocal(App app) {

App managedApp = appRepository.findByAppId(appId);
if (managedApp == null) {
throw new BadRequestException(String.format("App not exists. AppId = %s", appId));
throw new BadRequestException("App not exists. AppId = %s", appId);
}

managedApp.setName(app.getName());
Expand All @@ -188,7 +188,7 @@ public App updateAppInLocal(App app) {
String ownerName = app.getOwnerName();
UserInfo owner = userService.findByUserId(ownerName);
if (owner == null) {
throw new BadRequestException(String.format("App's owner not exists. owner = %s", ownerName));
throw new BadRequestException("App's owner not exists. owner = %s", ownerName);
}
managedApp.setOwnerName(owner.getUserId());
managedApp.setOwnerEmail(owner.getEmail());
Expand All @@ -209,7 +209,7 @@ public EnvClusterInfo createEnvNavNode(Env env, String appId) {
public App deleteAppInLocal(String appId) {
App managedApp = appRepository.findByAppId(appId);
if (managedApp == null) {
throw new BadRequestException(String.format("App not exists. AppId = %s", appId));
throw new BadRequestException("App not exists. AppId = %s", appId);
}
String operator = userInfoHolder.getUser().getUserId();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public List<ClusterDTO> findClusters(Env env, String appId) {

public ClusterDTO createCluster(Env env, ClusterDTO cluster) {
if (!clusterAPI.isClusterUnique(cluster.getAppId(), env, cluster.getName())) {
throw new BadRequestException(String.format("cluster %s already exists.", cluster.getName()));
throw new BadRequestException("cluster %s already exists.", cluster.getName());
}
ClusterDTO clusterDTO = clusterAPI.create(env, cluster);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,9 @@ private long getNamespaceId(NamespaceIdentifier namespaceIdentifier) {
namespaceDTO = namespaceAPI.loadNamespace(appId, env, clusterName, namespaceName);
} catch (HttpClientErrorException e) {
if (e.getStatusCode() == HttpStatus.NOT_FOUND) {
throw new BadRequestException(String.format(
throw new BadRequestException(
"namespace not exist. appId:%s, env:%s, clusterName:%s, namespaceName:%s", appId, env, clusterName,
namespaceName));
namespaceName);
}
throw e;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ public NamespaceDTO loadNamespaceBaseInfo(String appId, Env env, String clusterN
String namespaceName) {
NamespaceDTO namespace = namespaceAPI.loadNamespace(appId, env, clusterName, namespaceName);
if (namespace == null) {
throw new BadRequestException(String.format("Namespace: %s not exist.", namespaceName));
throw new BadRequestException("Namespace: %s not exist.", namespaceName);
}
return namespace;
}
Expand Down

0 comments on commit 70b70c3

Please sign in to comment.