From 24fa31939b09a18a3e9fe7f6757364f7127da3b6 Mon Sep 17 00:00:00 2001 From: jin Date: Fri, 20 Mar 2020 17:43:44 +0800 Subject: [PATCH] feat:optimizing code (#2245) * feat:optimizing code * add licence and format code --- .../api/controller/AlertGroupController.java | 24 ++++++--- .../api/service/AlertGroupService.java | 43 ++++++--------- .../api/service/UserAlertGroupService.java | 38 +++++++++++++ .../api/service/AlertGroupServiceTest.java | 47 +++++++++------- .../service/UserAlertGroupServiceTest.java | 53 +++++++++++++++++++ pom.xml | 1 + 6 files changed, 151 insertions(+), 55 deletions(-) create mode 100644 dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UserAlertGroupService.java create mode 100644 dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/UserAlertGroupServiceTest.java diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/AlertGroupController.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/AlertGroupController.java index e9bffa510b..140434ee43 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/AlertGroupController.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/AlertGroupController.java @@ -93,11 +93,11 @@ public class AlertGroupController extends BaseController{ public Result list(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser) { logger.info("login user {}, query all alertGroup", loginUser.getUserName()); - try{ + try { HashMap result = alertGroupService.queryAlertgroup(); return returnDataList(result); - }catch (Exception e){ - logger.error(Status.QUERY_ALL_ALERTGROUP_ERROR.getMsg(),e); + } catch (Exception e) { + logger.error(Status.QUERY_ALL_ALERTGROUP_ERROR.getMsg(), e); return error(Status.QUERY_ALL_ALERTGROUP_ERROR.getCode(), Status.QUERY_ALL_ALERTGROUP_ERROR.getMsg()); } } @@ -214,12 +214,20 @@ public class AlertGroupController extends BaseController{ @GetMapping(value = "/verify-group-name") @ResponseStatus(HttpStatus.OK) public Result verifyGroupName(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser, - @RequestParam(value ="groupName") String groupName - ) { - logger.info("login user {}, verfiy group name: {}", - loginUser.getUserName(),groupName); + @RequestParam(value ="groupName") String groupName) { + logger.info("login user {}, verify group name: {}", loginUser.getUserName(), groupName); - return alertGroupService.verifyGroupName(loginUser, groupName); + boolean exist= alertGroupService.existGroupName(groupName); + Result result = new Result(); + if (exist) { + logger.error("group {} has exist, can't create again.", groupName); + result.setCode(Status.ALERT_GROUP_EXIST.getCode()); + result.setMsg(Status.ALERT_GROUP_EXIST.getMsg()); + } else { + result.setCode(Status.SUCCESS.getCode()); + result.setMsg(Status.SUCCESS.getMsg()); + } + return result; } /** diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/AlertGroupService.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/AlertGroupService.java index 70310b6331..001a10d08a 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/AlertGroupService.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/AlertGroupService.java @@ -16,17 +16,17 @@ */ package org.apache.dolphinscheduler.api.service; +import java.util.*; import org.apache.dolphinscheduler.api.enums.Status; import org.apache.dolphinscheduler.api.utils.PageInfo; -import org.apache.dolphinscheduler.api.utils.Result; import org.apache.dolphinscheduler.common.Constants; import org.apache.dolphinscheduler.common.enums.AlertType; +import org.apache.dolphinscheduler.common.utils.CollectionUtils; import org.apache.dolphinscheduler.common.utils.StringUtils; import org.apache.dolphinscheduler.dao.entity.AlertGroup; import org.apache.dolphinscheduler.dao.entity.User; import org.apache.dolphinscheduler.dao.entity.UserAlertGroup; import org.apache.dolphinscheduler.dao.mapper.AlertGroupMapper; -import org.apache.dolphinscheduler.dao.mapper.UserAlertGroupMapper; import com.baomidou.mybatisplus.core.metadata.IPage; import com.baomidou.mybatisplus.extension.plugins.pagination.Page; import org.slf4j.Logger; @@ -35,11 +35,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; -import java.util.Date; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - /** * alert group service */ @@ -52,8 +47,7 @@ public class AlertGroupService extends BaseService{ private AlertGroupMapper alertGroupMapper; @Autowired - private UserAlertGroupMapper userAlertGroupMapper; - + private UserAlertGroupService userAlertGroupService; /** * query alert group list * @@ -122,7 +116,7 @@ public class AlertGroupService extends BaseService{ alertGroup.setCreateTime(now); alertGroup.setUpdateTime(now); - // insert + // insert int insert = alertGroupMapper.insert(alertGroup); if (insert > 0) { @@ -199,7 +193,7 @@ public class AlertGroupService extends BaseService{ return result; } - userAlertGroupMapper.deleteByAlertgroupId(id); + userAlertGroupService.deleteByAlertGroupId(id); alertGroupMapper.deleteById(id); putMsg(result, Status.SUCCESS); return result; @@ -223,22 +217,26 @@ public class AlertGroupService extends BaseService{ return result; } - userAlertGroupMapper.deleteByAlertgroupId(alertgroupId); + userAlertGroupService.deleteByAlertGroupId(alertgroupId); if (StringUtils.isEmpty(userIds)) { putMsg(result, Status.SUCCESS); return result; } String[] userIdsArr = userIds.split(","); - + Date now = new Date(); + List alertGroups = new ArrayList<>(userIds.length()); for (String userId : userIdsArr) { - Date now = new Date(); UserAlertGroup userAlertGroup = new UserAlertGroup(); userAlertGroup.setAlertgroupId(alertgroupId); userAlertGroup.setUserId(Integer.parseInt(userId)); userAlertGroup.setCreateTime(now); userAlertGroup.setUpdateTime(now); - userAlertGroupMapper.insert(userAlertGroup); + alertGroups.add(userAlertGroup); + } + + if (CollectionUtils.isNotEmpty(alertGroups)) { + userAlertGroupService.saveBatch(alertGroups); } putMsg(result, Status.SUCCESS); @@ -248,22 +246,11 @@ public class AlertGroupService extends BaseService{ /** * verify group name exists * - * @param loginUser login user * @param groupName group name * @return check result code */ - public Result verifyGroupName(User loginUser, String groupName) { - Result result = new Result(); + public boolean existGroupName(String groupName) { List alertGroup = alertGroupMapper.queryByGroupName(groupName); - if (alertGroup != null && alertGroup.size() > 0) { - logger.error("group {} has exist, can't create again.", groupName); - result.setCode(Status.ALERT_GROUP_EXIST.getCode()); - result.setMsg(Status.ALERT_GROUP_EXIST.getMsg()); - } else { - result.setCode(Status.SUCCESS.getCode()); - result.setMsg(Status.SUCCESS.getMsg()); - } - - return result; + return CollectionUtils.isNotEmpty(alertGroup); } } diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UserAlertGroupService.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UserAlertGroupService.java new file mode 100644 index 0000000000..502185709f --- /dev/null +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UserAlertGroupService.java @@ -0,0 +1,38 @@ +/* + * 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.service; + +import com.baomidou.mybatisplus.extension.service.impl.ServiceImpl; +import org.apache.dolphinscheduler.dao.entity.UserAlertGroup; +import org.apache.dolphinscheduler.dao.mapper.UserAlertGroupMapper; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; + +/** + * + */ +@Service +public class UserAlertGroupService extends ServiceImpl { + + @Autowired + private UserAlertGroupMapper userAlertGroupMapper; + + boolean deleteByAlertGroupId(Integer groupId) { + return userAlertGroupMapper.deleteByAlertgroupId(groupId) >= 1; + } + +} diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AlertGroupServiceTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AlertGroupServiceTest.java index 4a31902af9..ab7dac4d60 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AlertGroupServiceTest.java +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AlertGroupServiceTest.java @@ -18,9 +18,12 @@ package org.apache.dolphinscheduler.api.service; import com.baomidou.mybatisplus.core.metadata.IPage; import com.baomidou.mybatisplus.extension.plugins.pagination.Page; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import org.apache.dolphinscheduler.api.enums.Status; import org.apache.dolphinscheduler.api.utils.PageInfo; -import org.apache.dolphinscheduler.api.utils.Result; import org.apache.dolphinscheduler.common.Constants; import org.apache.dolphinscheduler.common.enums.AlertType; import org.apache.dolphinscheduler.common.enums.UserType; @@ -31,9 +34,12 @@ import org.apache.dolphinscheduler.dao.mapper.AlertGroupMapper; import org.apache.dolphinscheduler.dao.mapper.UserAlertGroupMapper; import org.junit.After; import org.junit.Assert; +import static org.junit.Assert.assertEquals; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import static org.mockito.ArgumentMatchers.*; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Mockito; @@ -41,14 +47,6 @@ import org.mockito.junit.MockitoJUnitRunner; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; - @RunWith(MockitoJUnitRunner.class) public class AlertGroupServiceTest { @@ -60,6 +58,8 @@ public class AlertGroupServiceTest { private AlertGroupMapper alertGroupMapper; @Mock private UserAlertGroupMapper userAlertGroupMapper; + @Mock + UserAlertGroupService userAlertGroupService; private String groupName = "AlertGroupServiceTest"; @@ -160,25 +160,34 @@ public class AlertGroupServiceTest { } + @Test - public void testGrantUser(){ + public void testGrantUser() { + + Integer groupId = 1; + + ArgumentCaptor groupArgument = ArgumentCaptor.forClass(Integer.class); + + Mockito.when(userAlertGroupService.deleteByAlertGroupId(anyInt())).thenReturn(true); + + Map result = alertGroupService.grantUser(getLoginUser(), groupId, "123,321"); + Mockito.verify(userAlertGroupService).deleteByAlertGroupId(groupArgument.capture()); - Map result = alertGroupService.grantUser(getLoginUser(),1,"123,321"); logger.info(result.toString()); - Assert.assertEquals(Status.SUCCESS,result.get(Constants.STATUS)); + assertEquals(groupArgument.getValue(), groupId); + assertEquals(Status.SUCCESS, result.get(Constants.STATUS)); } + @Test - public void testVerifyGroupName(){ + public void testVerifyGroupName() { //group name not exist - Result result = alertGroupService.verifyGroupName(getLoginUser(), groupName); - logger.info(result.toString()); - Assert.assertEquals(Status.SUCCESS.getMsg(),result.getMsg()); + boolean result = alertGroupService.existGroupName(groupName); + Assert.assertFalse(result); Mockito.when(alertGroupMapper.queryByGroupName(groupName)).thenReturn(getList()); //group name exist - result = alertGroupService.verifyGroupName(getLoginUser(), groupName); - logger.info(result.toString()); - Assert.assertEquals(Status.ALERT_GROUP_EXIST.getMsg(),result.getMsg()); + result = alertGroupService.existGroupName(groupName); + Assert.assertTrue(result); } diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/UserAlertGroupServiceTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/UserAlertGroupServiceTest.java new file mode 100644 index 0000000000..24b1d5a98b --- /dev/null +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/UserAlertGroupServiceTest.java @@ -0,0 +1,53 @@ +/* + * 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.service; + +import org.apache.dolphinscheduler.dao.mapper.UserAlertGroupMapper; +import static org.junit.Assert.assertEquals; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +/** + * + */ +@RunWith(MockitoJUnitRunner.class) +public class UserAlertGroupServiceTest { + + @InjectMocks + UserAlertGroupService userAlertGroupService; + + @Mock + UserAlertGroupMapper userAlertGroupMapper; + + @Test + public void deleteByAlertGroupId() { + + Integer groupId = 1; + userAlertGroupService.deleteByAlertGroupId(groupId); + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Integer.class); + + Mockito.verify(userAlertGroupMapper).deleteByAlertgroupId(argumentCaptor.capture()); + assertEquals(argumentCaptor.getValue(), groupId); + + } + +} \ No newline at end of file diff --git a/pom.xml b/pom.xml index 9331de3a68..6c864e77f1 100644 --- a/pom.xml +++ b/pom.xml @@ -704,6 +704,7 @@ **/api/service/TenantServiceTest.java **/api/service/WorkerGroupServiceTest.java **/api/service/AlertGroupServiceTest.java + **/api/service/UserAlertGroupServiceTest.java **/api/service/ProjectServiceTest.java **/api/service/ProcessDefinitionServiceTest.java **/api/service/UdfFuncServiceTest.java