From 44770a876797394e544c69c31c0a87609bc25586 Mon Sep 17 00:00:00 2001 From: lgcareer <18610854716@163.com> Date: Thu, 23 Apr 2020 14:24:22 +0800 Subject: [PATCH] [fix #2442] It will give a error tip that resource not exist exwhen update resources. (#2483) * fix #2442 and remove unavailable code * revert verifyResourceName method * Add ServiceException * add ServiceExceptionTest * update ServiceExceptionTest * add ServiceExceptionTest in pom --- .../api/exceptions/ServiceException.java | 56 ++++++++++++++++++ .../api/service/ResourcesService.java | 58 +++++-------------- .../api/exceptions/ServiceExceptionTest.java | 46 +++++++++++++++ pom.xml | 1 + 4 files changed, 119 insertions(+), 42 deletions(-) create mode 100644 dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/exceptions/ServiceException.java create mode 100644 dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/exceptions/ServiceExceptionTest.java diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/exceptions/ServiceException.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/exceptions/ServiceException.java new file mode 100644 index 0000000000..5669e6c3df --- /dev/null +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/exceptions/ServiceException.java @@ -0,0 +1,56 @@ +/* + * 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; + + +/** + * service exception + */ +public class ServiceException extends RuntimeException { + + /** + * code + */ + private Integer code; + + public ServiceException() { + } + + public ServiceException(Status status) { + super(status.getMsg()); + this.code = status.getCode(); + } + + public ServiceException(Integer code,String message) { + super(message); + this.code = code; + } + + public ServiceException(String message) { + super(message); + } + + public Integer getCode() { + return this.code; + } + + public void setCode(Integer code) { + this.code = code; + } +} \ No newline at end of file diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ResourcesService.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ResourcesService.java index c98b7c31b9..a2af47b2d5 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ResourcesService.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ResourcesService.java @@ -26,6 +26,7 @@ import org.apache.dolphinscheduler.api.dto.resources.filter.ResourceFilter; import org.apache.dolphinscheduler.api.dto.resources.visitor.ResourceTreeVisitor; import org.apache.dolphinscheduler.api.dto.resources.visitor.Visitor; import org.apache.dolphinscheduler.api.enums.Status; +import org.apache.dolphinscheduler.api.exceptions.ServiceException; import org.apache.dolphinscheduler.api.utils.PageInfo; import org.apache.dolphinscheduler.api.utils.Result; import org.apache.dolphinscheduler.common.Constants; @@ -234,9 +235,6 @@ public class ResourcesService extends BaseService { } Date now = new Date(); - - - Resource resource = new Resource(pid,name,fullName,false,desc,file.getOriginalFilename(),loginUser.getId(),type,file.getSize(),now,now); try { @@ -342,7 +340,6 @@ public class ResourcesService extends BaseService { String originResourceName = resource.getAlias(); if (!resource.isDirectory()) { //get the file suffix - String suffix = originResourceName.substring(originResourceName.lastIndexOf(".")); //if the name without suffix then add it ,else use the origin name @@ -352,7 +349,7 @@ public class ResourcesService extends BaseService { } // updateResource data - List childrenResource = listAllChildren(resource); + List childrenResource = listAllChildren(resource,false); String oldFullName = resource.getFullName(); Date now = new Date(); @@ -385,16 +382,16 @@ public class ResourcesService extends BaseService { result.setData(resultMap); } catch (Exception e) { logger.error(Status.UPDATE_RESOURCE_ERROR.getMsg(), e); - throw new RuntimeException(Status.UPDATE_RESOURCE_ERROR.getMsg()); + throw new ServiceException(Status.UPDATE_RESOURCE_ERROR); } // if name unchanged, return directly without moving on HDFS if (originResourceName.equals(name)) { return result; } - // get file hdfs path - // delete hdfs file by type + // get the path of origin file in hdfs String originHdfsFileName = HadoopUtils.getHdfsFileName(resource.getType(),tenantCode,originFullName); + // get the path of dest file in hdfs String destHdfsFileName = HadoopUtils.getHdfsFileName(resource.getType(),tenantCode,fullName); try { @@ -408,6 +405,7 @@ public class ResourcesService extends BaseService { } catch (Exception e) { logger.error(MessageFormat.format("hdfs copy {0} -> {1} fail", originHdfsFileName, destHdfsFileName), e); putMsg(result,Status.HDFS_COPY_FAIL); + throw new ServiceException(Status.HDFS_COPY_FAIL); } return result; @@ -542,34 +540,6 @@ public class ResourcesService extends BaseService { return result; } - /** - * get all resources - * @param loginUser login user - * @return all resource set - */ - /*private Set getAllResources(User loginUser, ResourceType type) { - int userId = loginUser.getId(); - boolean listChildren = true; - if(isAdmin(loginUser)){ - userId = 0; - listChildren = false; - } - List resourceList = resourcesMapper.queryResourceListAuthored(userId, type.ordinal()); - Set allResourceList = new HashSet<>(resourceList); - if (listChildren) { - Set authorizedIds = new HashSet<>(); - List authorizedDirecoty = resourceList.stream().filter(t->t.getUserId() != loginUser.getId() && t.isDirectory()).collect(Collectors.toList()); - if (CollectionUtils.isNotEmpty(authorizedDirecoty)) { - for(Resource resource : authorizedDirecoty){ - authorizedIds.addAll(listAllChildren(resource)); - } - List childrenResources = resourcesMapper.listResourceByIds(authorizedIds.toArray(new Integer[authorizedIds.size()])); - allResourceList.addAll(childrenResources); - } - } - return allResourceList; - }*/ - /** * query resource list * @@ -580,8 +550,11 @@ public class ResourcesService extends BaseService { public Map queryResourceJarList(User loginUser, ResourceType type) { Map result = new HashMap<>(5); - - List allResourceList = resourcesMapper.queryResourceListAuthored(loginUser.getId(), type.ordinal(),0); + int userId = loginUser.getId(); + if(isAdmin(loginUser)){ + userId = 0; + } + List allResourceList = resourcesMapper.queryResourceListAuthored(userId, type.ordinal(),0); List resources = new ResourceFilter(".jar",new ArrayList<>(allResourceList)).filter(); Visitor resourceTreeVisitor = new ResourceTreeVisitor(resources); result.put(Constants.DATA_LIST, resourceTreeVisitor.visit().getChildren()); @@ -631,7 +604,7 @@ public class ResourcesService extends BaseService { Map> resourceProcessMap = ResourceProcessDefinitionUtils.getResourceProcessDefinitionMap(list); Set resourceIdSet = resourceProcessMap.keySet(); // get all children of the resource - List allChildren = listAllChildren(resource); + List allChildren = listAllChildren(resource,true); Integer[] needDeleteResourceIdArray = allChildren.toArray(new Integer[allChildren.size()]); //if resource type is UDF,need check whether it is bound by UDF functon @@ -1193,12 +1166,13 @@ public class ResourcesService extends BaseService { /** * list all children id - * @param resource resource + * @param resource resource + * @param containSelf whether add self to children list * @return all children id */ - List listAllChildren(Resource resource){ + List listAllChildren(Resource resource,boolean containSelf){ List childList = new ArrayList<>(); - if (resource.getId() != -1) { + if (resource.getId() != -1 && containSelf) { childList.add(resource.getId()); } diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/exceptions/ServiceExceptionTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/exceptions/ServiceExceptionTest.java new file mode 100644 index 0000000000..a574253d1d --- /dev/null +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/exceptions/ServiceExceptionTest.java @@ -0,0 +1,46 @@ +/* + * 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.junit.Assert; +import org.junit.Test; + +public class ServiceExceptionTest { + @Test + public void getCodeTest(){ + ServiceException serviceException = new ServiceException(); + Assert.assertNull(serviceException.getCode()); + + serviceException = new ServiceException(Status.ALERT_GROUP_EXIST); + Assert.assertNotNull(serviceException.getCode()); + + serviceException = new ServiceException(10012, "alarm group already exists"); + Assert.assertNotNull(serviceException.getCode()); + } + @Test + public void getMessageTest(){ + ServiceException serviceException = new ServiceException(); + Assert.assertNull(serviceException.getMessage()); + + serviceException = new ServiceException(Status.ALERT_GROUP_EXIST); + Assert.assertNotNull(serviceException.getMessage()); + + serviceException = new ServiceException(10012, "alarm group already exists"); + Assert.assertNotNull(serviceException.getMessage()); + } +} diff --git a/pom.xml b/pom.xml index 053652fc92..8f81e2aea9 100644 --- a/pom.xml +++ b/pom.xml @@ -695,6 +695,7 @@ **/api/enums/testGetEnum.java **/api/enums/StatusTest.java **/api/exceptions/ApiExceptionHandlerTest.java + **/api/exceptions/ServiceExceptionTest.java **/api/interceptor/LoginHandlerInterceptorTest.java **/api/security/PasswordAuthenticatorTest.java **/api/security/SecurityConfigTest.java