Merge pull request #123 from yiming187/portal_exception

Enhance exception process in portal

apollo-adminservice/src/test/java/com/ctrip/apollo/adminservice/AllTests.java

@@ -5,11 +5,14 @@ import org.junit.runners.Suite;
 import org.junit.runners.Suite.SuiteClasses;
 
 import com.ctrip.apollo.adminservice.controller.AppControllerTest;
+import com.ctrip.apollo.adminservice.controller.ControllerExceptionTest;
+import com.ctrip.apollo.adminservice.controller.ControllerIntegrationExceptionTest;
 import com.ctrip.apollo.adminservice.controller.ItemSetControllerTest;
 import com.ctrip.apollo.adminservice.controller.ReleaseControllerTest;
 
 @RunWith(Suite.class)
-@SuiteClasses({AppControllerTest.class, ReleaseControllerTest.class, ItemSetControllerTest.class})
+@SuiteClasses({AppControllerTest.class, ReleaseControllerTest.class, ItemSetControllerTest.class,
+    ControllerExceptionTest.class, ControllerIntegrationExceptionTest.class})
 public class AllTests {
 
 }

apollo-adminservice/src/test/java/com/ctrip/apollo/adminservice/controller/AbstractControllerTest.java

@@ -18,7 +18,7 @@ import com.ctrip.apollo.AdminServiceTestConfiguration;
 @WebIntegrationTest(randomPort = true)
 public abstract class AbstractControllerTest {
 
-  RestTemplate restTemplate = new TestRestTemplate("user", "");
+  RestTemplate restTemplate = new TestRestTemplate("apollo", "");
 
   @PostConstruct
   private void postConstruct() {

apollo-adminservice/src/test/java/com/ctrip/apollo/adminservice/controller/AppControllerTest.java

@@ -53,7 +53,6 @@ public class AppControllerTest extends AbstractControllerTest {
     App savedApp = appRepository.findOne(first.getId());
     Assert.assertEquals(dto.getAppId(), savedApp.getAppId());
     Assert.assertNotNull(savedApp.getDataChangeCreatedTime());
-    Assert.assertNull(savedApp.getDataChangeLastModifiedTime());
 
     response = restTemplate.postForEntity(getBaseAppUrl(), dto, AppDTO.class);
     AppDTO second = response.getBody();
@@ -82,8 +81,7 @@ public class AppControllerTest extends AbstractControllerTest {
   @Test(expected = HttpClientErrorException.class)
   @Sql(scripts = "/controller/cleanup.sql", executionPhase = ExecutionPhase.AFTER_TEST_METHOD)
   public void testFindNotExist() {
-    ResponseEntity<AppDTO> result =
-        restTemplate.getForEntity(getBaseAppUrl() + "notExists", AppDTO.class);
+    restTemplate.getForEntity(getBaseAppUrl() + "notExists", AppDTO.class);
   }
 
   @Test

apollo-adminservice/src/test/java/com/ctrip/apollo/adminservice/controller/ControllerIntegrationExceptionTest.java

@@ -11,7 +11,6 @@ import org.junit.Test;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.http.ResponseEntity;
 import org.springframework.test.context.jdbc.Sql;
 import org.springframework.test.context.jdbc.Sql.ExecutionPhase;
 import org.springframework.test.util.ReflectionTestUtils;

apollo-common/src/main/java/com/ctrip/apollo/common/controller/GlobalDefaultExceptionHandler.java

@@ -7,10 +7,12 @@ import org.springframework.web.HttpMediaTypeException;
 import org.springframework.web.HttpRequestMethodNotSupportedException;
 import org.springframework.web.bind.annotation.ControllerAdvice;
 import org.springframework.web.bind.annotation.ExceptionHandler;
+import org.springframework.web.client.HttpStatusCodeException;
 
 import com.ctrip.apollo.core.exception.AbstractBaseException;
 import com.ctrip.apollo.core.exception.BadRequestException;
 import com.ctrip.apollo.core.exception.NotFoundException;
+import com.google.gson.Gson;
 
 import java.time.LocalDateTime;
 import java.time.format.DateTimeFormatter;
@@ -27,6 +29,9 @@ import static org.springframework.http.MediaType.APPLICATION_JSON;
 
 @ControllerAdvice
 public class GlobalDefaultExceptionHandler {
+
+  private Gson gson = new Gson();
+
   @ExceptionHandler(Exception.class)
   public ResponseEntity<Map<String, Object>> exception(HttpServletRequest request, Exception ex) {
     return handleError(request, INTERNAL_SERVER_ERROR, ex);
@@ -40,6 +45,10 @@ public class GlobalDefaultExceptionHandler {
   private ResponseEntity<Map<String, Object>> handleError(HttpServletRequest request,
       HttpStatus status, Throwable ex, String message) {
     ex = resolveError(ex);
+    if (ex.getCause() instanceof HttpStatusCodeException) {
+      return restTemplateException(request, (HttpStatusCodeException) ex.getCause());
+    }
+
     Map<String, Object> errorAttributes = new LinkedHashMap<>();
     errorAttributes.put("status", status.value());
     errorAttributes.put("message", message);
@@ -73,6 +82,16 @@ public class GlobalDefaultExceptionHandler {
     return handleError(request, BAD_REQUEST, ex);
   }
 
+  @ExceptionHandler(HttpStatusCodeException.class)
+  public ResponseEntity<Map<String, Object>> restTemplateException(HttpServletRequest request,
+      HttpStatusCodeException ex) {
+    @SuppressWarnings("unchecked")
+    Map<String, Object> errorAttributes = gson.fromJson(ex.getResponseBodyAsString(), Map.class);
+    HttpHeaders headers = new HttpHeaders();
+    headers.setContentType(APPLICATION_JSON);
+    return new ResponseEntity<>(errorAttributes, headers, ex.getStatusCode());
+  }
+
   private Throwable resolveError(Throwable ex) {
     while (ex instanceof ServletException && ex.getCause() != null) {
       ex = ((ServletException) ex).getCause();

apollo-common/src/main/java/com/ctrip/apollo/common/utils/ExceptionUtils.java

+package com.ctrip.apollo.common.utils;
+
+import java.util.Map;
+
+import org.springframework.web.client.HttpStatusCodeException;
+
+import com.google.common.base.MoreObjects;
+import com.google.gson.Gson;
+
+public final class ExceptionUtils {
+
+  private static Gson gson = new Gson();
+
+  public static String toString(HttpStatusCodeException e) {
+    @SuppressWarnings("unchecked")
+    Map<String, Object> errorAttributes = gson.fromJson(e.getResponseBodyAsString(), Map.class);
+    if (errorAttributes != null) {
+      return MoreObjects.toStringHelper(HttpStatusCodeException.class)
+          .add("status", errorAttributes.get("status"))
+          .add("message", errorAttributes.get("message"))
+          .add("timestamp", errorAttributes.get("timestamp"))
+          .add("exception", errorAttributes.get("exception"))
+          .add("errorCode", errorAttributes.get("errorCode"))
+          .add("stackTrace", errorAttributes.get("stackTrace")).toString();
+    }
+    return "";
+  }
+}

apollo-core/src/main/java/com/ctrip/apollo/core/exception/AbstractBaseException.java

@@ -17,6 +17,10 @@ public abstract class AbstractBaseException extends RuntimeException{
     super(str);
   }
   
+  public AbstractBaseException(String str, Exception e){
+    super(str,e);
+  }
+  
   public AbstractBaseException(String str, String errorCode){
     super(str);
     this.setErrorCode(errorCode);

apollo-core/src/main/java/com/ctrip/apollo/core/exception/ServiceException.java

@@ -11,4 +11,7 @@ public class ServiceException extends AbstractBaseException {
     super(str);
   }
 
+  public ServiceException(String str, Exception e) {
+    super(str, e);
+  }
 }

apollo-portal/src/main/java/com/ctrip/apollo/portal/api/API.java

@@ -7,7 +7,6 @@ import org.springframework.web.client.RestTemplate;
 
 import com.ctrip.apollo.common.auth.RestTemplateFactory;
 import com.ctrip.apollo.core.enums.Env;
-import com.ctrip.apollo.core.exception.ServiceException;
 import com.ctrip.apollo.portal.service.ServiceLocator;
 
 public class API {
@@ -26,14 +25,7 @@ public class API {
   }
 
   public String getAdminServiceHost(Env env) {
-    // 本地测试用
-    // return "http://localhost:8090";
-    try {
-      return serviceLocator.getAdminService(env).getHomepageUrl();
-    } catch (ServiceException e) {
-      e.printStackTrace();
-    }
-    return "";
+    return serviceLocator.getAdminService(env).getHomepageUrl();
   }
 
 }

apollo-portal/src/main/java/com/ctrip/apollo/portal/api/AdminServiceAPI.java

@@ -8,40 +8,34 @@ import com.ctrip.apollo.core.dto.ItemChangeSets;
 import com.ctrip.apollo.core.dto.ItemDTO;
 import com.ctrip.apollo.core.dto.NamespaceDTO;
 import com.ctrip.apollo.core.dto.ReleaseDTO;
-import com.ctrip.apollo.core.exception.ServiceException;
-import com.ctrip.apollo.core.utils.StringUtils;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import org.springframework.http.HttpEntity;
-import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.stereotype.Service;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
-import org.springframework.web.client.HttpClientErrorException;
 
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
 
 
 @Service
 public class AdminServiceAPI {
 
-  private static final Logger logger = LoggerFactory.getLogger(AdminServiceAPI.class);
-
   @Service
   public static class AppAPI extends API {
 
     public static String APP_API = "/apps";
 
     public List<AppDTO> getApps(Env env) {
-      return Arrays.asList(restTemplate.getForObject(getAdminServiceHost(env) + APP_API, AppDTO[].class));
+      AppDTO[] appDTOs =
+          restTemplate.getForObject(getAdminServiceHost(env) + APP_API, AppDTO[].class);
+      return Arrays.asList(appDTOs);
     }
 
     public AppDTO save(Env env, AppDTO app) {
-      return restTemplate.postForEntity(getAdminServiceHost(env) + APP_API, app, AppDTO.class).getBody();
+      return restTemplate.postForEntity(getAdminServiceHost(env) + APP_API, app, AppDTO.class)
+          .getBody();
     }
   }
 
@@ -49,31 +43,19 @@ public class AdminServiceAPI {
   @Service
   public static class NamespaceAPI extends API {
 
-    public List<NamespaceDTO> findNamespaceByCluster(String appId, Env env,
-                                                     String clusterName) {
-      if (StringUtils.isContainEmpty(appId, clusterName)) {
-        return null;
-      }
-
+    public List<NamespaceDTO> findNamespaceByCluster(String appId, Env env, String clusterName) {
       NamespaceDTO[] namespaceDTOs = restTemplate.getForObject(
-          getAdminServiceHost(env) + String.format("apps/%s/clusters/%s/namespaces", appId, clusterName),
+          getAdminServiceHost(env)
+              + String.format("apps/%s/clusters/%s/namespaces", appId, clusterName),
           NamespaceDTO[].class);
-
-      if (namespaceDTOs == null){
-        return Collections.emptyList();
-      }else {
-        return Arrays.asList(namespaceDTOs);
-      }
+      return Arrays.asList(namespaceDTOs);
     }
 
-    public NamespaceDTO loadNamespace(String appId, Env env,
-                                      String clusterName, String namespaceName) {
-      if (StringUtils.isContainEmpty(appId, clusterName, namespaceName)) {
-        return null;
-      }
-      return restTemplate.getForObject(getAdminServiceHost(env) +
-                                       String.format("apps/%s/clusters/%s/namespaces/%s", appId, clusterName,
-                                                     namespaceName), NamespaceDTO.class);
+    public NamespaceDTO loadNamespace(String appId, Env env, String clusterName,
+        String namespaceName) {
+      return restTemplate.getForObject(getAdminServiceHost(env)
+          + String.format("apps/%s/clusters/%s/namespaces/%s", appId, clusterName, namespaceName),
+          NamespaceDTO.class);
     }
   }
 
@@ -81,93 +63,59 @@ public class AdminServiceAPI {
   public static class ItemAPI extends API {
 
     public List<ItemDTO> findItems(String appId, Env env, String clusterName, String namespace) {
-      if (StringUtils.isContainEmpty(appId, clusterName, namespace)) {
-        return Collections.emptyList();
-      }
-
-      ItemDTO[] itemDTOs = restTemplate.getForObject(getAdminServiceHost(env) + String
-                                                         .format("apps/%s/clusters/%s/namespaces/%s/items", appId,
-                                                                 clusterName, namespace),
-                                                     ItemDTO[].class);
-
-      if (itemDTOs == null) {
-        return Collections.emptyList();
-      } else {
-        return Arrays.asList(itemDTOs);
-      }
+      ItemDTO[] itemDTOs =
+          restTemplate
+              .getForObject(
+                  getAdminServiceHost(env) + String.format(
+                      "apps/%s/clusters/%s/namespaces/%s/items", appId, clusterName, namespace),
+                  ItemDTO[].class);
+      return Arrays.asList(itemDTOs);
     }
 
     public void updateItems(String appId, Env env, String clusterName, String namespace,
-                            ItemChangeSets changeSets) {
-      if (StringUtils.isContainEmpty(appId, clusterName, namespace)) {
-        return;
-      }
-      restTemplate.postForEntity(getAdminServiceHost(env) + String.format("apps/%s/clusters/%s/namespaces/%s/itemset",
-                                                                          appId, clusterName, namespace), changeSets,
-                                 Void.class);
+        ItemChangeSets changeSets) {
+      restTemplate.postForEntity(getAdminServiceHost(env) + String
+          .format("apps/%s/clusters/%s/namespaces/%s/itemset", appId, clusterName, namespace),
+          changeSets, Void.class);
     }
-
-
   }
 
   @Service
   public static class ClusterAPI extends API {
 
     public List<ClusterDTO> findClustersByApp(String appId, Env env) {
-      if (StringUtils.isContainEmpty(appId)) {
-        return null;
-      }
-
-      ClusterDTO[] clusterDTOs = restTemplate.getForObject(getAdminServiceHost(env) + String.format("apps/%s/clusters", appId),
-                                ClusterDTO[].class);
-
-      if (clusterDTOs == null){
-        return Collections.emptyList();
-      }else {
-        return Arrays.asList(clusterDTOs);
-      }
+      ClusterDTO[] clusterDTOs = restTemplate.getForObject(
+          getAdminServiceHost(env) + String.format("apps/%s/clusters", appId), ClusterDTO[].class);
+      return Arrays.asList(clusterDTOs);
     }
   }
 
   @Service
   public static class ReleaseAPI extends API {
 
-    public ReleaseDTO loadLatestRelease(String appId, Env env, String clusterName, String namespace) {
-      if (StringUtils.isContainEmpty(appId, clusterName, namespace)) {
-        return null;
-      }
-      try {
-        ReleaseDTO releaseDTO = restTemplate.getForObject(getAdminServiceHost(env) + String
-            .format("apps/%s/clusters/%s/namespaces/%s/releases/latest", appId,
-                    clusterName, namespace), ReleaseDTO.class);
-        return releaseDTO;
-      } catch (HttpClientErrorException e) {
-        logger.warn(
-            " call [ReleaseAPI.loadLatestRelease] and return not fount exception.app id:{}, env:{}, clusterName:{}, namespace:{}",
-            appId, env, clusterName, namespace);
-        return null;
-      }
+    public ReleaseDTO loadLatestRelease(String appId, Env env, String clusterName,
+        String namespace) {
+      ReleaseDTO releaseDTO = restTemplate.getForObject(
+          getAdminServiceHost(env) + String.format(
+              "apps/%s/clusters/%s/namespaces/%s/releases/latest", appId, clusterName, namespace),
+          ReleaseDTO.class);
+      return releaseDTO;
     }
 
-    public ReleaseDTO release(String appId, Env env, String clusterName, String namespace, String releaseBy,
-                              String comment) {
+    public ReleaseDTO release(String appId, Env env, String clusterName, String namespace,
+        String releaseBy, String comment) {
       MultiValueMap<String, String> parameters = new LinkedMultiValueMap<String, String>();
       parameters.add("name", releaseBy);
       parameters.add("comment", comment);
       HttpEntity<MultiValueMap<String, String>> entity =
           new HttpEntity<MultiValueMap<String, String>>(parameters, null);
-      ResponseEntity<ReleaseDTO> response = restTemplate.postForEntity(getAdminServiceHost(env) + String.
-                                       format("apps/%s/clusters/%s/namespaces/%s/releases", appId, clusterName, namespace),
-                                   entity, ReleaseDTO.class);
-
-      if (response != null && response.getStatusCode() == HttpStatus.OK){
-        return response.getBody();
-      }else {
-        logger.error("createRelease fail.id:{}, env:{}, clusterName:{}, namespace:{},releaseBy{}",
-                     appId, env, clusterName, namespace, releaseBy);
-        throw new ServiceException(" call create createRelease api error.");
-      }
+      ResponseEntity<ReleaseDTO> response =
+          restTemplate
+              .postForEntity(
+                  getAdminServiceHost(env) + String.format(
+                      "apps/%s/clusters/%s/namespaces/%s/releases", appId, clusterName, namespace),
+                  entity, ReleaseDTO.class);
+      return response.getBody();
     }
   }
-
 }

apollo-portal/src/main/java/com/ctrip/apollo/portal/service/AppService.java

@@ -6,10 +6,11 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
+import org.springframework.web.client.HttpStatusCodeException;
 
-import com.ctrip.apollo.core.enums.Env;
+import com.ctrip.apollo.common.utils.ExceptionUtils;
 import com.ctrip.apollo.core.dto.AppDTO;
-import com.ctrip.apollo.core.exception.ServiceException;
+import com.ctrip.apollo.core.enums.Env;
 import com.ctrip.apollo.portal.PortalSettings;
 import com.ctrip.apollo.portal.api.AdminServiceAPI;
 import com.ctrip.apollo.portal.entity.ClusterNavTree;
@@ -49,9 +50,9 @@ public class AppService {
     for (Env env : envs) {
       try {
         appAPI.save(env, app);
-      } catch (Exception e) {
-        logger.error("oops! save app error. app id:{}", app.getAppId(), e);
-        throw new ServiceException("call service error.");
+      } catch (HttpStatusCodeException e) {
+        logger.error(ExceptionUtils.toString(e));
+        throw e;
       }
     }
   }

apollo-portal/src/test/java/com/ctrip/apollo/portal/AbstractPortalTest.java

 package com.ctrip.apollo.portal;
 
+import javax.annotation.PostConstruct;
+
 import org.junit.runner.RunWith;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.test.SpringApplicationConfiguration;
+import org.springframework.boot.test.TestRestTemplate;
+import org.springframework.boot.test.WebIntegrationTest;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.web.client.DefaultResponseErrorHandler;
+import org.springframework.web.client.RestTemplate;
 
 import com.ctrip.apollo.PortalApplication;
 
 @RunWith(SpringJUnit4ClassRunner.class)
 @SpringApplicationConfiguration(classes = PortalApplication.class)
+@WebIntegrationTest(randomPort = true)
 public abstract class AbstractPortalTest {
 
+  RestTemplate restTemplate = new TestRestTemplate("apollo", "");
+
+  @PostConstruct
+  private void postConstruct() {
+    restTemplate.setErrorHandler(new DefaultResponseErrorHandler());
+  }
+
+  @Value("${local.server.port}")
+  int port;
 }

apollo-portal/src/test/java/com/ctrip/apollo/portal/AppServiceTest.java

@@ -22,7 +22,7 @@ import static org.junit.Assert.assertEquals;
 import static org.mockito.Mockito.when;
 
 @RunWith(MockitoJUnitRunner.class)
-public class AppServiceTest extends AbstractPortalTest{
+public class AppServiceTest {
 
   @Mock
   private PortalSettings settings;

apollo-portal/src/test/java/com/ctrip/apollo/portal/ConfigServiceTest.java

@@ -26,7 +26,7 @@ import static org.junit.Assert.assertEquals;
 import static org.mockito.Mockito.when;
 
 @RunWith(MockitoJUnitRunner.class)
-public class ConfigServiceTest extends AbstractPortalTest{
+public class ConfigServiceTest {
 
   @Mock
   private AdminServiceAPI.NamespaceAPI namespaceAPI;

apollo-portal/src/test/java/com/ctrip/apollo/portal/ServiceExceptionTest.java

+package com.ctrip.apollo.portal;
+
+
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.when;
+
+import java.nio.charset.Charset;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.http.HttpStatus;
+import org.springframework.test.util.ReflectionTestUtils;
+import org.springframework.web.client.HttpServerErrorException;
+import org.springframework.web.client.HttpStatusCodeException;
+
+import com.ctrip.apollo.common.utils.ExceptionUtils;
+import com.ctrip.apollo.core.dto.AppDTO;
+import com.ctrip.apollo.core.enums.Env;
+import com.ctrip.apollo.core.exception.ServiceException;
+import com.ctrip.apollo.portal.api.AdminServiceAPI;
+import com.ctrip.apollo.portal.service.AppService;
+import com.google.gson.Gson;
+
+public class ServiceExceptionTest extends AbstractPortalTest {
+
+  @Autowired
+  private AppService appService;
+
+  @Mock
+  private AdminServiceAPI.AppAPI appAPI;
+
+  @Before
+  public void setUp() {
+    MockitoAnnotations.initMocks(this);
+    ReflectionTestUtils.setField(appService, "appAPI", appAPI);
+  }
+
+  private String getBaseAppUrl() {
+    return "http://localhost:" + port + "/apps/";
+  }
+
+  @Test
+  public void testAdminServiceException() {
+    Map<String, Object> errorAttributes = new LinkedHashMap<>();
+    errorAttributes.put("status", 500);
+    errorAttributes.put("message", "admin server error");
+    errorAttributes.put("timestamp",
+        LocalDateTime.now().format(DateTimeFormatter.ISO_LOCAL_DATE_TIME));
+    errorAttributes.put("exception", ServiceException.class.getName());
+    errorAttributes.put("errorCode", "8848");
+
+    HttpStatusCodeException adminException =
+        new HttpServerErrorException(HttpStatus.INTERNAL_SERVER_ERROR, "admin server error",
+            new Gson().toJson(errorAttributes).getBytes(), Charset.defaultCharset());
+    when(appAPI.save(any(Env.class), any(AppDTO.class))).thenThrow(adminException);
+
+    AppDTO dto = generateSampleDTOData();
+    try {
+      restTemplate.postForEntity(getBaseAppUrl(), dto, AppDTO.class);
+    } catch (HttpStatusCodeException e) {
+      @SuppressWarnings("unchecked")
+      Map<String, String> attr = new Gson().fromJson(e.getResponseBodyAsString(), Map.class);
+      System.out.println(ExceptionUtils.toString(e));
+      Assert.assertEquals("admin server error", attr.get("message"));
+      Assert.assertEquals("8848", attr.get("errorCode"));
+    }
+  }
+
+  private AppDTO generateSampleDTOData() {
+    AppDTO dto = new AppDTO();
+    dto.setAppId("someAppId");
+    dto.setName("someName");
+    dto.setOwnerName("someOwner");
+    dto.setOwnerEmail("someOwner@ctrip.com");
+    return dto;
+  }
+}