From 58cbbcc1bdea524d90b59ab1d2ce74a376d18b1c Mon Sep 17 00:00:00 2001
From: Sonia Zorba <sonia.zorba@inaf.it>
Date: Fri, 23 Apr 2021 14:23:13 +0200
Subject: [PATCH] PutFileController getEmptyFile() bugfix
---
.../controller/PutFileController.java | 10 +++
.../controller/PutFileControllerTest.java | 89 +++++++++++++------
2 files changed, 74 insertions(+), 25 deletions(-)
diff --git a/src/main/java/it/inaf/ia2/transfer/controller/PutFileController.java b/src/main/java/it/inaf/ia2/transfer/controller/PutFileController.java
index 2bc9e79..f31525d 100644
--- a/src/main/java/it/inaf/ia2/transfer/controller/PutFileController.java
+++ b/src/main/java/it/inaf/ia2/transfer/controller/PutFileController.java
@@ -14,6 +14,8 @@ import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import javax.servlet.http.HttpServletRequest;
import javax.xml.bind.DatatypeConverter;
import net.ivoa.xml.uws.v1.ExecutionPhase;
@@ -176,6 +178,14 @@ public class PutFileController extends FileController {
nameWithoutExtension = fileName;
}
+ Pattern pattern = Pattern.compile("(.*?)-(\\d+)");
+ Matcher matcher = pattern.matcher(nameWithoutExtension);
+ if (matcher.matches()) {
+ nameWithoutExtension = matcher.group(1);
+ int fileIndex = Integer.parseInt(matcher.group(2));
+ index = fileIndex + 1;
+ }
+
String newName = nameWithoutExtension + "-" + index;
if (extension != null) {
newName += "." + extension;
diff --git a/src/test/java/it/inaf/ia2/transfer/controller/PutFileControllerTest.java b/src/test/java/it/inaf/ia2/transfer/controller/PutFileControllerTest.java
index a240884..03f8919 100644
--- a/src/test/java/it/inaf/ia2/transfer/controller/PutFileControllerTest.java
+++ b/src/test/java/it/inaf/ia2/transfer/controller/PutFileControllerTest.java
@@ -11,8 +11,10 @@ import java.util.Optional;
import java.util.UUID;
import net.ivoa.xml.uws.v1.ExecutionPhase;
import org.assertj.core.util.Files;
+import org.junit.jupiter.api.AfterAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
@@ -32,6 +34,7 @@ import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
import org.springframework.test.web.servlet.request.RequestPostProcessor;
import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
+import org.springframework.util.FileSystemUtils;
@SpringBootTest
@AutoConfigureMockMvc
@@ -39,16 +42,28 @@ public class PutFileControllerTest {
@MockBean
private FileDAO fileDao;
-
+
@MockBean
private JobDAO jobDAO;
-
+
@MockBean
private ListOfFilesDAO listOfFilesDAO;
@Autowired
private MockMvc mockMvc;
-
+
+ private static File temporaryDirectory;
+
+ @BeforeAll
+ public static void setUp() {
+ temporaryDirectory = Files.newTemporaryFolder();
+ }
+
+ @AfterAll
+ public static void tearDown() {
+ FileSystemUtils.deleteRecursively(temporaryDirectory);
+ }
+
@Test
public void putGenericFile() throws Exception {
@@ -62,18 +77,26 @@ public class PutFileControllerTest {
.andDo(print())
.andExpect(status().isOk());
- File file = Path.of("/tmp", randomFileName).toFile();
+ File file = Path.of(getTestFilePath(randomFileName)).toFile();
assertTrue(file.exists());
assertEquals("content", Files.contentOf(file, StandardCharsets.UTF_8));
- assertTrue(file.delete());
+ assertTrue(file.delete());
}
-
+
@Test
- public void putGenericFileWithNameConflict() throws Exception {
+ public void putGenericFileWithNameConflictExtension() throws Exception {
+ putGenericFileWithNameConflict("test.txt", "test-1.txt", "test-2.txt");
+ }
+
+ @Test
+ public void putGenericFileWithNameConflictNoExtension() throws Exception {
+ putGenericFileWithNameConflict("test", "test-1", "test-2");
+ }
- String fileName = "pippoFile";
- createBaseFileInfo(fileName);
+ public void putGenericFileWithNameConflict(String name1, String name2, String name3) throws Exception {
+
+ createBaseFileInfo(name1);
MockMultipartFile fakeFile = new MockMultipartFile("file", "test.txt", "text/plain", "content".getBytes());
@@ -82,29 +105,41 @@ public class PutFileControllerTest {
.andDo(print())
.andExpect(status().isOk());
- File file = Path.of("/tmp", fileName).toFile();
+ File file = Path.of(getTestFilePath(name1)).toFile();
assertTrue(file.exists());
assertEquals("content", Files.contentOf(file, StandardCharsets.UTF_8));
-
+
MockMultipartFile fakeFile2 = new MockMultipartFile("file", "test.txt", "text/plain", "content2".getBytes());
-
+
mockMvc.perform(putMultipart("/path/to/test.txt")
.file(fakeFile2))
.andDo(print())
.andExpect(status().isOk());
-
- File file2 = Path.of("/tmp", fileName+"-1").toFile();
- assertTrue(file.exists());
+
+ File file2 = Path.of(getTestFilePath(name2)).toFile();
+ assertTrue(file2.exists());
assertEquals("content2", Files.contentOf(file2, StandardCharsets.UTF_8));
+
+ MockMultipartFile fakeFile3 = new MockMultipartFile("file", "test.txt", "text/plain", "content3".getBytes());
+
+ mockMvc.perform(putMultipart("/path/to/test.txt")
+ .file(fakeFile3))
+ .andDo(print())
+ .andExpect(status().isOk());
+
+ File file3 = Path.of(getTestFilePath(name3)).toFile();
+ assertTrue(file3.exists());
+ assertEquals("content3", Files.contentOf(file3, StandardCharsets.UTF_8));
+
assertTrue(file.delete());
assertTrue(file2.delete());
+ assertTrue(file3.delete());
}
-
@Test
public void putGenericFileWithJobId() throws Exception {
-
+
when(jobDAO.isJobExisting("pippo10")).thenReturn(false);
when(jobDAO.isJobExisting("pippo5")).thenReturn(true);
@@ -112,27 +147,27 @@ public class PutFileControllerTest {
createBaseFileInfo(randomFileName);
MockMultipartFile fakeFile = new MockMultipartFile("file", "test.txt", "text/plain", "content".getBytes());
-
+
// Try with invalid jobid
mockMvc.perform(putMultipart("/path/to/test.txt")
.file(fakeFile).param("jobid", "pippo10"))
.andDo(print())
.andExpect(status().is4xxClientError());
-
+
verify(jobDAO, times(1)).isJobExisting(eq("pippo10"));
-
+
// Retry with valid jobid
mockMvc.perform(putMultipart("/path/to/test.txt")
.file(fakeFile).param("jobid", "pippo5"))
.andDo(print())
.andExpect(status().is2xxSuccessful());
-
+
verify(jobDAO, times(1)).isJobExisting(eq("pippo5"));
verify(jobDAO, times(1)).updateJobPhase(eq(ExecutionPhase.COMPLETED), any());
-
- File file = Path.of("/tmp", randomFileName).toFile();
- assertTrue(file.exists());
+ File file = Path.of(getTestFilePath(randomFileName)).toFile();
+
+ assertTrue(file.exists());
assertEquals("content", Files.contentOf(file, StandardCharsets.UTF_8));
assertTrue(file.delete());
}
@@ -158,7 +193,7 @@ public class PutFileControllerTest {
private FileInfo createBaseFileInfo(String fileName) {
FileInfo fileInfo = new FileInfo();
- fileInfo.setOsPath("/tmp/" + fileName);
+ fileInfo.setOsPath(getTestFilePath(fileName));
fileInfo.setIsPublic(false);
when(fileDao.getFileInfo(any())).thenReturn(Optional.of(fileInfo));
@@ -166,6 +201,10 @@ public class PutFileControllerTest {
return fileInfo;
}
+ private String getTestFilePath(String fileName) {
+ return temporaryDirectory.toPath().resolve(fileName).toFile().getAbsolutePath();
+ }
+
private MockMultipartHttpServletRequestBuilder putMultipart(String uri) {
MockMultipartHttpServletRequestBuilder builder = MockMvcRequestBuilders.multipart(uri);
builder.with(new RequestPostProcessor() {
--
GitLab