diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/AccessTokenController.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/AccessTokenController.java index c03281df7e..8731b264e9 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/AccessTokenController.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/AccessTokenController.java @@ -18,6 +18,7 @@ package org.apache.dolphinscheduler.api.controller; import org.apache.dolphinscheduler.api.enums.Status; +import org.apache.dolphinscheduler.api.exceptions.ApiException; import org.apache.dolphinscheduler.api.service.AccessTokenService; import org.apache.dolphinscheduler.api.utils.Result; import org.apache.dolphinscheduler.common.Constants; @@ -37,13 +38,14 @@ import springfox.documentation.annotations.ApiIgnore; import java.util.Map; import static org.apache.dolphinscheduler.api.enums.Status.*; + /** * access token controller */ @Api(tags = "ACCESS_TOKEN_TAG", position = 1) @RestController @RequestMapping("/access-token") -public class AccessTokenController extends BaseController{ +public class AccessTokenController extends BaseController { private static final Logger logger = LoggerFactory.getLogger(AccessTokenController.class); @@ -54,140 +56,125 @@ public class AccessTokenController extends BaseController{ /** * create token - * @param loginUser login user - * @param userId token for user id + * + * @param loginUser login user + * @param userId token for user id * @param expireTime expire time for the token - * @param token token + * @param token token * @return create result state code */ @ApiIgnore @PostMapping(value = "/create") @ResponseStatus(HttpStatus.CREATED) + @ApiException(CREATE_ACCESS_TOKEN_ERROR) public Result createToken(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @RequestParam(value = "userId") int userId, - @RequestParam(value = "expireTime") String expireTime, - @RequestParam(value = "token") String token){ + @RequestParam(value = "userId") int userId, + @RequestParam(value = "expireTime") String expireTime, + @RequestParam(value = "token") String token) { logger.info("login user {}, create token , userId : {} , token expire time : {} , token : {}", loginUser.getUserName(), - userId,expireTime,token); - - try { - Map result = accessTokenService.createToken(userId, expireTime, token); - return returnDataList(result); - }catch (Exception e){ - logger.error(CREATE_ACCESS_TOKEN_ERROR.getMsg(),e); - return error(CREATE_ACCESS_TOKEN_ERROR.getCode(), CREATE_ACCESS_TOKEN_ERROR.getMsg()); - } + userId, expireTime, token); + + Map result = accessTokenService.createToken(userId, expireTime, token); + return returnDataList(result); } /** * generate token string - * @param loginUser login user - * @param userId token for user + * + * @param loginUser login user + * @param userId token for user * @param expireTime expire time * @return token string */ @ApiIgnore @PostMapping(value = "/generate") @ResponseStatus(HttpStatus.CREATED) + @ApiException(GENERATE_TOKEN_ERROR) public Result generateToken(@RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @RequestParam(value = "userId") int userId, - @RequestParam(value = "expireTime") String expireTime){ - logger.info("login user {}, generate token , userId : {} , token expire time : {}",loginUser,userId,expireTime); - try { - Map result = accessTokenService.generateToken(userId, expireTime); - return returnDataList(result); - }catch (Exception e){ - logger.error(GENERATE_TOKEN_ERROR.getMsg(),e); - return error(GENERATE_TOKEN_ERROR.getCode(), GENERATE_TOKEN_ERROR.getMsg()); - } + @RequestParam(value = "userId") int userId, + @RequestParam(value = "expireTime") String expireTime) { + logger.info("login user {}, generate token , userId : {} , token expire time : {}", loginUser, userId, expireTime); + Map result = accessTokenService.generateToken(userId, expireTime); + return returnDataList(result); } /** * query access token list paging * * @param loginUser login user - * @param pageNo page number + * @param pageNo page number * @param searchVal search value - * @param pageSize page size + * @param pageSize page size * @return token list of page number and page size */ - @ApiOperation(value = "queryAccessTokenList", notes= "QUERY_ACCESS_TOKEN_LIST_NOTES") + @ApiOperation(value = "queryAccessTokenList", notes = "QUERY_ACCESS_TOKEN_LIST_NOTES") @ApiImplicitParams({ - @ApiImplicitParam(name = "searchVal", value = "SEARCH_VAL", dataType ="String"), + @ApiImplicitParam(name = "searchVal", value = "SEARCH_VAL", dataType = "String"), @ApiImplicitParam(name = "pageNo", value = "PAGE_NO", dataType = "Int", example = "1"), - @ApiImplicitParam(name = "pageSize", value = "PAGE_SIZE", dataType ="Int",example = "20") + @ApiImplicitParam(name = "pageSize", value = "PAGE_SIZE", dataType = "Int", example = "20") }) - @GetMapping(value="/list-paging") + @GetMapping(value = "/list-paging") @ResponseStatus(HttpStatus.OK) + @ApiException(QUERY_ACCESSTOKEN_LIST_PAGING_ERROR) public Result queryAccessTokenList(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @RequestParam("pageNo") Integer pageNo, - @RequestParam(value = "searchVal", required = false) String searchVal, - @RequestParam("pageSize") Integer pageSize){ + @RequestParam("pageNo") Integer pageNo, + @RequestParam(value = "searchVal", required = false) String searchVal, + @RequestParam("pageSize") Integer pageSize) { logger.info("login user {}, list access token paging, pageNo: {}, searchVal: {}, pageSize: {}", - loginUser.getUserName(),pageNo,searchVal,pageSize); - try{ - Map result = checkPageParams(pageNo, pageSize); - if(result.get(Constants.STATUS) != Status.SUCCESS){ - return returnDataListPaging(result); - } - searchVal = ParameterUtils.handleEscapes(searchVal); - result = accessTokenService.queryAccessTokenList(loginUser, searchVal, pageNo, pageSize); + loginUser.getUserName(), pageNo, searchVal, pageSize); + + Map result = checkPageParams(pageNo, pageSize); + if (result.get(Constants.STATUS) != Status.SUCCESS) { return returnDataListPaging(result); - }catch (Exception e){ - logger.error(QUERY_ACCESSTOKEN_LIST_PAGING_ERROR.getMsg(),e); - return error(QUERY_ACCESSTOKEN_LIST_PAGING_ERROR.getCode(),QUERY_ACCESSTOKEN_LIST_PAGING_ERROR.getMsg()); } + searchVal = ParameterUtils.handleEscapes(searchVal); + result = accessTokenService.queryAccessTokenList(loginUser, searchVal, pageNo, pageSize); + return returnDataListPaging(result); } /** * delete access token by id + * * @param loginUser login user - * @param id token id + * @param id token id * @return delete result code */ @ApiIgnore @PostMapping(value = "/delete") @ResponseStatus(HttpStatus.OK) + @ApiException(DELETE_ACCESS_TOKEN_ERROR) public Result delAccessTokenById(@RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @RequestParam(value = "id") int id) { + @RequestParam(value = "id") int id) { logger.info("login user {}, delete access token, id: {},", loginUser.getUserName(), id); - try { - Map result = accessTokenService.delAccessTokenById(loginUser, id); - return returnDataList(result); - }catch (Exception e){ - logger.error(DELETE_ACCESS_TOKEN_ERROR.getMsg(),e); - return error(Status.DELETE_ACCESS_TOKEN_ERROR.getCode(), Status.DELETE_ACCESS_TOKEN_ERROR.getMsg()); - } + Map result = accessTokenService.delAccessTokenById(loginUser, id); + return returnDataList(result); } /** * update token - * @param loginUser login user - * @param id token id - * @param userId token for user + * + * @param loginUser login user + * @param id token id + * @param userId token for user * @param expireTime token expire time - * @param token token string + * @param token token string * @return update result code */ @ApiIgnore @PostMapping(value = "/update") @ResponseStatus(HttpStatus.OK) + @ApiException(UPDATE_ACCESS_TOKEN_ERROR) public Result updateToken(@RequestAttribute(value = Constants.SESSION_USER) User loginUser, @RequestParam(value = "id") int id, @RequestParam(value = "userId") int userId, @RequestParam(value = "expireTime") String expireTime, - @RequestParam(value = "token") String token){ + @RequestParam(value = "token") String token) { logger.info("login user {}, update token , userId : {} , token expire time : {} , token : {}", loginUser.getUserName(), - userId,expireTime,token); - - try { - Map result = accessTokenService.updateToken(id,userId, expireTime, token); - return returnDataList(result); - }catch (Exception e){ - logger.error(UPDATE_ACCESS_TOKEN_ERROR.getMsg(),e); - return error(UPDATE_ACCESS_TOKEN_ERROR.getCode(), UPDATE_ACCESS_TOKEN_ERROR.getMsg()); - } + userId, expireTime, token); + + Map result = accessTokenService.updateToken(id, userId, expireTime, token); + return returnDataList(result); } } diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/enums/Status.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/enums/Status.java index 2cbf22e199..3e5147bd5c 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/enums/Status.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/enums/Status.java @@ -27,6 +27,8 @@ public enum Status { SUCCESS(0, "success", "成功"), + INTERNAL_SERVER_ERROR_ARGS(10000, "Internal Server Error: {0}", "服务端异常: {0}"), + REQUEST_PARAMS_NOT_VALID_ERROR(10001, "request parameter {0} is not valid", "请求参数[{0}]无效"), TASK_TIMEOUT_PARAMS_ERROR(10002, "task timeout parameter is not valid", "任务超时参数无效"), USER_NAME_EXIST(10003, "user name already exists", "用户名已存在"), diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/exceptions/ApiException.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/exceptions/ApiException.java new file mode 100644 index 0000000000..3c094f5294 --- /dev/null +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/exceptions/ApiException.java @@ -0,0 +1,34 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.dolphinscheduler.api.exceptions; + +import org.apache.dolphinscheduler.api.enums.Status; + +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +/** + * controller exception annotation + */ +@Retention(RUNTIME) +@Target(METHOD) +public @interface ApiException { + Status value(); +} diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/exceptions/ApiExceptionHandler.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/exceptions/ApiExceptionHandler.java new file mode 100644 index 0000000000..c00c443bf9 --- /dev/null +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/exceptions/ApiExceptionHandler.java @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.dolphinscheduler.api.exceptions; + +import org.apache.dolphinscheduler.api.enums.Status; +import org.apache.dolphinscheduler.api.utils.Result; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.web.bind.annotation.ControllerAdvice; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.method.HandlerMethod; + +/** + * Exception Handler + */ +@ControllerAdvice +@ResponseBody +public class ApiExceptionHandler { + + private static final Logger logger = LoggerFactory.getLogger(ApiExceptionHandler.class); + + @ExceptionHandler(Exception.class) + public Result exceptionHandler(Exception e, HandlerMethod hm) { + logger.error(e.getMessage(), e); + ApiException ce = hm.getMethodAnnotation(ApiException.class); + if (ce == null) { + return Result.errorWithArgs(Status.INTERNAL_SERVER_ERROR_ARGS, e.getMessage()); + } + Status st = ce.value(); + return Result.error(st); + } + +} diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/AccessTokenService.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/AccessTokenService.java index 897646ba70..5d176961bb 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/AccessTokenService.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/AccessTokenService.java @@ -83,6 +83,9 @@ public class AccessTokenService extends BaseService { public Map createToken(int userId, String expireTime, String token) { Map result = new HashMap<>(5); + if (userId <= 0) { + throw new IllegalArgumentException("User id should not less than or equals to 0."); + } AccessToken accessToken = new AccessToken(); accessToken.setUserId(userId); accessToken.setExpireTime(DateUtils.stringToDate(expireTime)); diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/utils/Result.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/utils/Result.java index 6ab9512286..eacdecf166 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/utils/Result.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/utils/Result.java @@ -16,6 +16,10 @@ */ package org.apache.dolphinscheduler.api.utils; +import org.apache.dolphinscheduler.api.enums.Status; + +import java.text.MessageFormat; + /** * result * @@ -37,13 +41,58 @@ public class Result { */ private T data; - public Result(){} + public Result() { + } - public Result(Integer code , String msg){ + public Result(Integer code, String msg) { this.code = code; this.msg = msg; } + private Result(T data) { + this.code = 0; + this.data = data; + } + + private Result(Status status) { + if (status != null) { + this.code = status.getCode(); + this.msg = status.getMsg(); + } + } + + /** + * Call this function if there is success + * + * @param data data + * @param type + * @return resule + */ + public static Result success(T data) { + return new Result<>(data); + } + + /** + * Call this function if there is any error + * + * @param status status + * @return result + */ + public static Result error(Status status) { + return new Result(status); + } + + /** + * Call this function if there is any error + * + * @param status status + * @param args args + * @return result + */ + public static Result errorWithArgs(Status status, Object... args) { + return new Result(status.getCode(), MessageFormat.format(status.getMsg(), args)); + } + public Integer getCode() { return code; } diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/AccessTokenControllerTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/AccessTokenControllerTest.java index 47946d4af5..a219343371 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/AccessTokenControllerTest.java +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/AccessTokenControllerTest.java @@ -56,6 +56,23 @@ public class AccessTokenControllerTest extends AbstractControllerTest{ logger.info(mvcResult.getResponse().getContentAsString()); } + @Test + public void testExceptionHandler() throws Exception { + MultiValueMap paramsMap = new LinkedMultiValueMap<>(); + paramsMap.add("userId","-1"); + paramsMap.add("expireTime","2019-12-18 00:00:00"); + paramsMap.add("token","507f5aeaaa2093dbdff5d5522ce00510"); + MvcResult mvcResult = mockMvc.perform(post("/access-token/create") + .header("sessionId", sessionId) + .params(paramsMap)) + .andExpect(status().isOk()) + .andExpect(content().contentType(MediaType.APPLICATION_JSON_UTF8)) + .andReturn(); + Result result = JSONUtils.parseObject(mvcResult.getResponse().getContentAsString(), Result.class); + Assert.assertEquals(Status.CREATE_ACCESS_TOKEN_ERROR.getCode(), result.getCode().intValue()); + logger.info(mvcResult.getResponse().getContentAsString()); + } + @Test public void testGenerateToken() throws Exception { MultiValueMap paramsMap = new LinkedMultiValueMap<>(); diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/exceptions/ApiExceptionHandlerTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/exceptions/ApiExceptionHandlerTest.java new file mode 100644 index 0000000000..c0f1b3fd25 --- /dev/null +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/exceptions/ApiExceptionHandlerTest.java @@ -0,0 +1,42 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.dolphinscheduler.api.exceptions; + +import org.apache.dolphinscheduler.api.controller.AccessTokenController; +import org.apache.dolphinscheduler.api.enums.Status; +import org.apache.dolphinscheduler.api.utils.Result; +import org.apache.dolphinscheduler.dao.entity.User; +import org.junit.Assert; +import org.junit.Test; +import org.springframework.web.method.HandlerMethod; + +import java.lang.reflect.Method; + +import static org.junit.Assert.*; + +public class ApiExceptionHandlerTest { + + @Test + public void exceptionHandler() throws NoSuchMethodException { + ApiExceptionHandler handler = new ApiExceptionHandler(); + AccessTokenController controller = new AccessTokenController(); + Method method = controller.getClass().getMethod("createToken", User.class, int.class, String.class, String.class); + HandlerMethod hm = new HandlerMethod(controller, method); + Result result = handler.exceptionHandler(new RuntimeException("test exception"), hm); + Assert.assertEquals(Status.CREATE_ACCESS_TOKEN_ERROR.getCode(),result.getCode().intValue()); + } +} \ No newline at end of file diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/utils/ResultTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/utils/ResultTest.java new file mode 100644 index 0000000000..01fb75cdf7 --- /dev/null +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/utils/ResultTest.java @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.dolphinscheduler.api.utils; + +import org.apache.dolphinscheduler.api.enums.Status; +import org.junit.Assert; +import org.junit.Test; + +import java.util.HashMap; + +import static org.junit.Assert.*; + +public class ResultTest { + + @Test + public void success() { + HashMap map = new HashMap<>(); + map.put("testdata", "test"); + Result ret = Result.success(map); + Assert.assertEquals(Status.SUCCESS.getCode(), ret.getCode().intValue()); + } + + @Test + public void error() { + Result ret = Result.error(Status.ACCESS_TOKEN_NOT_EXIST); + Assert.assertEquals(Status.ACCESS_TOKEN_NOT_EXIST.getCode(), ret.getCode().intValue()); + } + + @Test + public void errorWithArgs() { + Result ret = Result.errorWithArgs(Status.INTERNAL_SERVER_ERROR_ARGS, "test internal server error"); + Assert.assertEquals(Status.INTERNAL_SERVER_ERROR_ARGS.getCode(), ret.getCode().intValue()); + } +} \ No newline at end of file diff --git a/pom.xml b/pom.xml index b05aff08e1..c3e0da581d 100644 --- a/pom.xml +++ b/pom.xml @@ -694,6 +694,7 @@ **/api/dto/resources/visitor/ResourceTreeVisitorTest.java **/api/enums/testGetEnum.java **/api/enums/StatusTest.java + **/api/exceptions/ApiExceptionHandlerTest.java **/api/interceptor/LoginHandlerInterceptorTest.java **/api/security/PasswordAuthenticatorTest.java **/api/security/SecurityConfigTest.java @@ -726,6 +727,7 @@ **/api/utils/FileUtilsTest.java **/api/utils/CheckUtilsTest.java **/api/utils/CheckUtilsTest.java + **/api/utils/ResultTest.java **/common/graph/DAGTest.java **/common/os/OshiTest.java **/common/os/OSUtilsTest.java