diff --git a/dinky-admin/pom.xml b/dinky-admin/pom.xml
index 1b69f59875..b2b1203976 100644
--- a/dinky-admin/pom.xml
+++ b/dinky-admin/pom.xml
@@ -303,6 +303,12 @@
jar
test
+
+ org.apache.flink
+ flink-core
+ ${flink.version}
+ test
+
org.powermock
powermock-module-junit4
diff --git a/dinky-admin/src/main/java/org/dinky/service/resource/impl/ResourceServiceImpl.java b/dinky-admin/src/main/java/org/dinky/service/resource/impl/ResourceServiceImpl.java
index 5f9b5db87f..75a7bf9fe5 100644
--- a/dinky-admin/src/main/java/org/dinky/service/resource/impl/ResourceServiceImpl.java
+++ b/dinky-admin/src/main/java/org/dinky/service/resource/impl/ResourceServiceImpl.java
@@ -144,8 +144,8 @@ public TreeNodeDTO createFolderOrGet(Integer pid, String fileName, String desc)
@Transactional(rollbackFor = Exception.class)
public void rename(Integer id, String fileName, String desc) {
Resources byId = getById(id);
- String sourceFullName = byId.getFullName();
DinkyAssert.checkNull(byId, Status.RESOURCE_DIR_OR_FILE_NOT_EXIST);
+ String sourceFullName = byId.getFullName();
long count = count(new LambdaQueryWrapper()
.eq(Resources::getPid, byId.getPid())
.eq(Resources::getFileName, fileName)
@@ -160,25 +160,10 @@ public void rename(Integer id, String fileName, String desc) {
byId.setFileName(fileName);
byId.setFullName(fullName);
updateById(byId);
- boolean isRunStorageMove = false;
- if (!byId.getIsDirectory()) {
- List list = list(new LambdaQueryWrapper().eq(Resources::getPid, byId.getId()));
- if (CollUtil.isNotEmpty(list)) {
- for (Resources resources : list) {
- resources.setFullName(fullName + "/" + resources.getFileName());
- isRunStorageMove = !resources.getIsDirectory() && !isRunStorageMove;
- }
- updateBatchById(list);
- }
- } else {
- isRunStorageMove = true;
- if (!isExistsChildren(id)) {
- return;
- }
- }
- if (isRunStorageMove) {
- getBaseResourceManager().rename(sourceFullName, fullName);
+ if (sourceFullName.equals(fullName) || (byId.getIsDirectory() && !isExistsChildren(id))) {
+ return;
}
+ getBaseResourceManager().rename(sourceFullName, fullName);
}
@Override
diff --git a/dinky-admin/src/test/java/org/dinky/service/resource/impl/ResourceServiceImplTest.java b/dinky-admin/src/test/java/org/dinky/service/resource/impl/ResourceServiceImplTest.java
new file mode 100644
index 0000000000..3b8b0aff33
--- /dev/null
+++ b/dinky-admin/src/test/java/org/dinky/service/resource/impl/ResourceServiceImplTest.java
@@ -0,0 +1,118 @@
+/*
+ *
+ * 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.dinky.service.resource.impl;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.spy;
+
+import org.dinky.data.model.Resources;
+import org.dinky.data.model.ResourcesModelEnum;
+import org.dinky.data.model.SystemConfiguration;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Collections;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import com.baomidou.mybatisplus.core.conditions.Wrapper;
+
+class ResourceServiceImplTest {
+
+ private final SystemConfiguration configuration = SystemConfiguration.getInstances();
+ private final String originalBasePath =
+ configuration.getResourcesUploadBasePath().getValue();
+ private final ResourcesModelEnum originalModel =
+ configuration.getResourcesModel().getValue();
+
+ @TempDir
+ Path tempDir;
+
+ @AfterEach
+ void restoreConfiguration() {
+ configuration.getResourcesUploadBasePath().setValue(originalBasePath);
+ configuration.getResourcesModel().setValue(originalModel);
+ }
+
+ @Test
+ void renameFileMovesPhysicalResource() throws Exception {
+ configuration.getResourcesUploadBasePath().setValue(tempDir.toString());
+ configuration.getResourcesModel().setValue(ResourcesModelEnum.LOCAL);
+
+ Path oldFile = tempDir.resolve("jars/old.jar");
+ Files.createDirectories(oldFile.getParent());
+ Files.write(oldFile, new byte[] {1});
+
+ Resources resource = Resources.builder()
+ .id(7)
+ .pid(1)
+ .fileName("old.jar")
+ .fullName("jars/old.jar")
+ .description("old")
+ .isDirectory(false)
+ .build();
+ ResourceServiceImpl service = spy(new ResourceServiceImpl());
+ doReturn(resource).when(service).getById(7);
+ doReturn(0L).when(service).count(any(Wrapper.class));
+ doReturn(Collections.emptyList()).when(service).list(any(Wrapper.class));
+ doReturn(true).when(service).updateById(any(Resources.class));
+
+ service.rename(7, "new.jar", "updated");
+
+ assertThat(oldFile).doesNotExist();
+ assertThat(tempDir.resolve("jars/new.jar")).exists();
+ assertThat(resource.getFileName()).isEqualTo("new.jar");
+ assertThat(resource.getFullName()).isEqualTo("jars/new.jar");
+ assertThat(resource.getDescription()).isEqualTo("updated");
+ }
+
+ @Test
+ void updateDescriptionKeepsPhysicalResourceInPlace() throws Exception {
+ configuration.getResourcesUploadBasePath().setValue(tempDir.toString());
+ configuration.getResourcesModel().setValue(ResourcesModelEnum.LOCAL);
+
+ Path file = tempDir.resolve("jars/current.jar");
+ Files.createDirectories(file.getParent());
+ Files.write(file, new byte[] {1});
+
+ Resources resource = Resources.builder()
+ .id(8)
+ .pid(1)
+ .fileName("current.jar")
+ .fullName("jars/current.jar")
+ .description("old")
+ .isDirectory(false)
+ .build();
+ ResourceServiceImpl service = spy(new ResourceServiceImpl());
+ doReturn(resource).when(service).getById(8);
+ doReturn(0L).when(service).count(any(Wrapper.class));
+ doReturn(true).when(service).updateById(any(Resources.class));
+
+ service.rename(8, "current.jar", "updated");
+
+ assertThat(file).exists();
+ assertThat(resource.getFullName()).isEqualTo("jars/current.jar");
+ assertThat(resource.getDescription()).isEqualTo("updated");
+ }
+}
diff --git a/dinky-client/dinky-client-base/src/main/java/org/dinky/resource/impl/HdfsResourceManager.java b/dinky-client/dinky-client-base/src/main/java/org/dinky/resource/impl/HdfsResourceManager.java
index bb6178ffec..281d4e3e7a 100644
--- a/dinky-client/dinky-client-base/src/main/java/org/dinky/resource/impl/HdfsResourceManager.java
+++ b/dinky-client/dinky-client-base/src/main/java/org/dinky/resource/impl/HdfsResourceManager.java
@@ -57,7 +57,10 @@ public void remove(String path) {
@Override
public void rename(String path, String newPath) {
try {
- getHdfs().rename(new Path(getFilePath(path)), new Path(getFilePath(newPath)));
+ boolean renamed = getHdfs().rename(new Path(getFilePath(path)), new Path(getFilePath(newPath)));
+ if (!renamed) {
+ throw new BusException(Status.RESOURCE_FILE_RENAME_FAILED);
+ }
} catch (IOException e) {
throw new BusException(Status.RESOURCE_FILE_RENAME_FAILED, e);
}
diff --git a/dinky-client/dinky-client-base/src/test/java/org/dinky/resource/impl/HdfsResourceManagerTest.java b/dinky-client/dinky-client-base/src/test/java/org/dinky/resource/impl/HdfsResourceManagerTest.java
new file mode 100644
index 0000000000..793603664d
--- /dev/null
+++ b/dinky-client/dinky-client-base/src/test/java/org/dinky/resource/impl/HdfsResourceManagerTest.java
@@ -0,0 +1,54 @@
+/*
+ *
+ * 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.dinky.resource.impl;
+
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import org.dinky.data.enums.Status;
+import org.dinky.data.exception.BusException;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+
+import org.junit.jupiter.api.Test;
+
+class HdfsResourceManagerTest {
+
+ @Test
+ void renameThrowsWhenHdfsRejectsMove() throws Exception {
+ FileSystem fileSystem = mock(FileSystem.class);
+ when(fileSystem.rename(any(Path.class), any(Path.class))).thenReturn(false);
+ HdfsResourceManager resourceManager = new HdfsResourceManager() {
+ @Override
+ public String getFilePath(String path) {
+ return path;
+ }
+ };
+ resourceManager.setHdfs(fileSystem);
+
+ assertThatThrownBy(() -> resourceManager.rename("jars/old.jar", "jars/new.jar"))
+ .isInstanceOf(BusException.class)
+ .extracting("code")
+ .isEqualTo(Status.RESOURCE_FILE_RENAME_FAILED);
+ }
+}