diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Files.java b/src/main/java/edu/harvard/iq/dataverse/api/Files.java index 11616b76566..7759761f35e 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -390,18 +390,21 @@ public Response updateFileMetadata(@FormDataParam("jsonData") String jsonData, JsonReader jsonReader = Json.createReader(new StringReader(jsonData)); javax.json.JsonObject jsonObject = jsonReader.readObject(); - String label = jsonObject.getString("label", null); - String directoryLabel = jsonObject.getString("directoryLabel", null); - String path = ""; - if (directoryLabel != null) { - path = directoryLabel + "/"; - } - if (label == null) { - label = df.getFileMetadata().getLabel(); + String incomingLabel = jsonObject.getString("label", null); + String incomingDirectoryLabel = jsonObject.getString("directoryLabel", null); + String existingLabel = df.getFileMetadata().getLabel(); + String existingDirectoryLabel = df.getFileMetadata().getDirectoryLabel(); + String pathPlusFilename = IngestUtil.getPathAndFileNameToCheck(incomingLabel, incomingDirectoryLabel, existingLabel, existingDirectoryLabel); + // We remove the current file from the list we'll check for duplicates. + // Instead, the current file is passed in as pathPlusFilename. + List fmdListMinusCurrentFile = new ArrayList<>(); + for (FileMetadata fileMetadata : fmdList) { + if (!fileMetadata.equals(df.getFileMetadata())) { + fmdListMinusCurrentFile.add(fileMetadata); + } } - if (IngestUtil.conflictsWithExistingFilenames(label, directoryLabel, fmdList, df)) { - String incomingPathPlusFileName = path + label; - return error(BAD_REQUEST, BundleUtil.getStringFromBundle("files.api.metadata.update.duplicateFile", Arrays.asList(incomingPathPlusFileName))); + if (IngestUtil.conflictsWithExistingFilenames(pathPlusFilename, fmdListMinusCurrentFile)) { + return error(BAD_REQUEST, BundleUtil.getStringFromBundle("files.api.metadata.update.duplicateFile", Arrays.asList(pathPlusFilename))); } optionalFileParams.addOptionalParams(upFmd); diff --git a/src/main/java/edu/harvard/iq/dataverse/ingest/IngestUtil.java b/src/main/java/edu/harvard/iq/dataverse/ingest/IngestUtil.java index 910d34e7b3a..0be5a58a790 100644 --- a/src/main/java/edu/harvard/iq/dataverse/ingest/IngestUtil.java +++ b/src/main/java/edu/harvard/iq/dataverse/ingest/IngestUtil.java @@ -101,38 +101,35 @@ public static String duplicateFilenameCheck(FileMetadata fileMetadata, Set fileMetadatas, DataFile dataFile) { + public static boolean conflictsWithExistingFilenames(String pathPlusFilename, List fileMetadatas) { List filePathsAndNames = getPathsAndFileNames(fileMetadatas); - if (label != null || directoryLabel != null) { - String path = ""; - if (directoryLabel != null) { - path = directoryLabel + "/"; - } - if (label == null) { - label = dataFile.getFileMetadata().getLabel(); - } - String incomingPathPlusFileName = path + label; - logger.fine(filePathsAndNames.toString()); - logger.fine("incomingPathName: " + incomingPathPlusFileName); - if (filePathsAndNames.contains(incomingPathPlusFileName)) { - return true; - } - } - return false; + return filePathsAndNames.contains(pathPlusFilename); } /** diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DuplicateFilesIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DuplicateFilesIT.java index deab5ee0fba..6227e96fdfa 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DuplicateFilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DuplicateFilesIT.java @@ -15,6 +15,7 @@ import static javax.ws.rs.core.Response.Status.CREATED; import static javax.ws.rs.core.Response.Status.OK; import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.nullValue; import org.junit.BeforeClass; import org.junit.Test; @@ -263,4 +264,233 @@ public void moveFileToDirectoryContainingSameFileName() throws IOException { } + /** + * In this test we make sure that other changes in the absence of label and + * directoryLabel go through, such as changing a file description. + */ + @Test + public void modifyFileDescription() throws IOException { + + Response createUser = UtilIT.createRandomUser(); + createUser.prettyPrint(); + createUser.then().assertThat() + .statusCode(OK.getStatusCode()); + String username = UtilIT.getUsernameFromResponse(createUser); + String apiToken = UtilIT.getApiTokenFromResponse(createUser); + + Response createDataverseResponse = UtilIT.createRandomDataverse(apiToken); + createDataverseResponse.prettyPrint(); + createDataverseResponse.then().assertThat() + .statusCode(CREATED.getStatusCode()); + + String dataverseAlias = UtilIT.getAliasFromResponse(createDataverseResponse); + + Response createDataset = UtilIT.createRandomDatasetViaNativeApi(dataverseAlias, apiToken); + createDataset.prettyPrint(); + createDataset.then().assertThat() + .statusCode(CREATED.getStatusCode()); + + Integer datasetId = UtilIT.getDatasetIdFromResponse(createDataset); + + Path pathtoReadme1 = Paths.get(Files.createTempDirectory(null) + File.separator + "README.md"); + Files.write(pathtoReadme1, "File 1".getBytes()); + System.out.println("README: " + pathtoReadme1); + + Response uploadReadme1 = UtilIT.uploadFileViaNative(datasetId.toString(), pathtoReadme1.toString(), apiToken); + uploadReadme1.prettyPrint(); + uploadReadme1.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.files[0].label", equalTo("README.md")); + + Integer idOfReadme1 = JsonPath.from(uploadReadme1.getBody().asString()).getInt("data.files[0].dataFile.id"); + System.out.println("id: " + idOfReadme1); + + JsonObjectBuilder updateFileMetadata = Json.createObjectBuilder() + .add("description", "This file is awesome."); + Response updateFileMetadataResponse = UtilIT.updateFileMetadata(String.valueOf(idOfReadme1), updateFileMetadata.build().toString(), apiToken); + updateFileMetadataResponse.prettyPrint(); + updateFileMetadataResponse.then().statusCode(OK.getStatusCode()); + + } + + /** + * In this test we make sure that that if you keep the label the same, you + * are still able to change other metadata such as file description. + */ + @Test + public void modifyFileDescriptionSameLabel() throws IOException { + + Response createUser = UtilIT.createRandomUser(); + createUser.prettyPrint(); + createUser.then().assertThat() + .statusCode(OK.getStatusCode()); + String username = UtilIT.getUsernameFromResponse(createUser); + String apiToken = UtilIT.getApiTokenFromResponse(createUser); + + Response createDataverseResponse = UtilIT.createRandomDataverse(apiToken); + createDataverseResponse.prettyPrint(); + createDataverseResponse.then().assertThat() + .statusCode(CREATED.getStatusCode()); + + String dataverseAlias = UtilIT.getAliasFromResponse(createDataverseResponse); + + Response createDataset = UtilIT.createRandomDatasetViaNativeApi(dataverseAlias, apiToken); + createDataset.prettyPrint(); + createDataset.then().assertThat() + .statusCode(CREATED.getStatusCode()); + + Integer datasetId = UtilIT.getDatasetIdFromResponse(createDataset); + + Path pathtoReadme1 = Paths.get(Files.createTempDirectory(null) + File.separator + "README.md"); + Files.write(pathtoReadme1, "File 1".getBytes()); + System.out.println("README: " + pathtoReadme1); + + JsonObjectBuilder json1 = Json.createObjectBuilder() + .add("directoryLabel", "code"); + + Response uploadReadme1 = UtilIT.uploadFileViaNative(datasetId.toString(), pathtoReadme1.toString(), json1.build(), apiToken); + uploadReadme1.prettyPrint(); + uploadReadme1.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.files[0].label", equalTo("README.md")); + + Integer idOfReadme1 = JsonPath.from(uploadReadme1.getBody().asString()).getInt("data.files[0].dataFile.id"); + System.out.println("id: " + idOfReadme1); + + JsonObjectBuilder updateFileMetadata = Json.createObjectBuilder() + .add("label", "README.md") + .add("directoryLabel", "code") + .add("description", "This file is awesome."); + Response updateFileMetadataResponse = UtilIT.updateFileMetadata(String.valueOf(idOfReadme1), updateFileMetadata.build().toString(), apiToken); + updateFileMetadataResponse.prettyPrint(); + updateFileMetadataResponse.then().statusCode(OK.getStatusCode()); + + } + + /** + * What if the directory for the file exists and you pass the filename + * (label) while changing the description? This should be allowed. + */ + @Test + public void existingDirectoryPassLabelChangeDescription() throws IOException { + + Response createUser = UtilIT.createRandomUser(); + createUser.prettyPrint(); + createUser.then().assertThat() + .statusCode(OK.getStatusCode()); + String username = UtilIT.getUsernameFromResponse(createUser); + String apiToken = UtilIT.getApiTokenFromResponse(createUser); + + Response createDataverseResponse = UtilIT.createRandomDataverse(apiToken); + createDataverseResponse.prettyPrint(); + createDataverseResponse.then().assertThat() + .statusCode(CREATED.getStatusCode()); + + String dataverseAlias = UtilIT.getAliasFromResponse(createDataverseResponse); + + Response createDataset = UtilIT.createRandomDatasetViaNativeApi(dataverseAlias, apiToken); + createDataset.prettyPrint(); + createDataset.then().assertThat() + .statusCode(CREATED.getStatusCode()); + + Integer datasetId = UtilIT.getDatasetIdFromResponse(createDataset); + + Path pathToFile = Paths.get(Files.createTempDirectory(null) + File.separator + "label"); + Files.write(pathToFile, "File 1".getBytes()); + System.out.println("file: " + pathToFile); + + JsonObjectBuilder json1 = Json.createObjectBuilder() + .add("directory", "code"); + + Response uploadFile = UtilIT.uploadFileViaNative(datasetId.toString(), pathToFile.toString(), json1.build(), apiToken); + uploadFile.prettyPrint(); + uploadFile.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.files[0].label", equalTo("label")); + + Integer idOfFile = JsonPath.from(uploadFile.getBody().asString()).getInt("data.files[0].dataFile.id"); + System.out.println("id: " + idOfFile); + + JsonObjectBuilder updateFileMetadata = Json.createObjectBuilder() + .add("label", "label") + .add("description", "This file is awesome."); + Response updateFileMetadataResponse = UtilIT.updateFileMetadata(String.valueOf(idOfFile), updateFileMetadata.build().toString(), apiToken); + updateFileMetadataResponse.prettyPrint(); + updateFileMetadataResponse.then().statusCode(OK.getStatusCode()); + + } + + /** + * This test is for the following scenario. + * + * What if the database has null for the directoryLabel? What if you pass in + * directory as “” (because you don’t realize you can just not pass it). + * when it check and compares, the old directory is null. so will that mean + * labelChange = true and it will fail even though you didn’t really change + * the directory? + * + * While "labelChange" does end up being true, + * IngestUtil.conflictsWithExistingFilenames returns false, so the change is + * allowed to go through. The description is allowed to be changed and the + * directoryLabel remains null even though the user passed in an empty + * string, which is what we want. + */ + @Test + public void existingDirectoryNullPassEmptyStringChangeDescription() throws IOException { + + Response createUser = UtilIT.createRandomUser(); + createUser.prettyPrint(); + createUser.then().assertThat() + .statusCode(OK.getStatusCode()); + String username = UtilIT.getUsernameFromResponse(createUser); + String apiToken = UtilIT.getApiTokenFromResponse(createUser); + + Response createDataverseResponse = UtilIT.createRandomDataverse(apiToken); + createDataverseResponse.prettyPrint(); + createDataverseResponse.then().assertThat() + .statusCode(CREATED.getStatusCode()); + + String dataverseAlias = UtilIT.getAliasFromResponse(createDataverseResponse); + + Response createDataset = UtilIT.createRandomDatasetViaNativeApi(dataverseAlias, apiToken); + createDataset.prettyPrint(); + createDataset.then().assertThat() + .statusCode(CREATED.getStatusCode()); + + Integer datasetId = UtilIT.getDatasetIdFromResponse(createDataset); + + Path pathToFile = Paths.get(Files.createTempDirectory(null) + File.separator + "file1.txt"); + Files.write(pathToFile, "File 1".getBytes()); + System.out.println("file: " + pathToFile); + + JsonObjectBuilder json1 = Json.createObjectBuilder() + .add("description", "This is my file."); + + Response uploadFile = UtilIT.uploadFileViaNative(datasetId.toString(), pathToFile.toString(), json1.build(), apiToken); + uploadFile.prettyPrint(); + uploadFile.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.files[0].label", equalTo("file1.txt")); + + Integer idOfFile = JsonPath.from(uploadFile.getBody().asString()).getInt("data.files[0].dataFile.id"); + System.out.println("id: " + idOfFile); + + JsonObjectBuilder updateFileMetadata = Json.createObjectBuilder() + // It doesn't make sense to pass "" as a directoryLabel. + .add("directoryLabel", "") + .add("description", "This file is awesome."); + Response updateFileMetadataResponse = UtilIT.updateFileMetadata(String.valueOf(idOfFile), updateFileMetadata.build().toString(), apiToken); + updateFileMetadataResponse.prettyPrint(); + updateFileMetadataResponse.then().statusCode(OK.getStatusCode()); + + Response datasetJson = UtilIT.nativeGet(datasetId, apiToken); + datasetJson.prettyPrint(); + datasetJson.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.latestVersion.files[0].label", equalTo("file1.txt")) + .body("data.latestVersion.files[0].description", equalTo("This file is awesome.")) + // Even though we tried to set directoryValue to "" above, it's correctly null in the database. + .body("data.latestVersion.files[0].directoryLabel", nullValue()); + } + } diff --git a/src/test/java/edu/harvard/iq/dataverse/ingest/IngestUtilTest.java b/src/test/java/edu/harvard/iq/dataverse/ingest/IngestUtilTest.java index 9fc5be4481f..abc773971c2 100644 --- a/src/test/java/edu/harvard/iq/dataverse/ingest/IngestUtilTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/ingest/IngestUtilTest.java @@ -11,6 +11,7 @@ import java.sql.Timestamp; import java.text.SimpleDateFormat; import java.util.ArrayList; +import java.util.Arrays; import java.util.Date; import java.util.List; import java.util.Set; @@ -611,4 +612,25 @@ public void testUnfUtil() { assertEquals("UNF:6:FWBO/a1GcxDnM3fNLdzrHw==", datasetUnfValue); } + @Test + public void testPathPlusFilename() { + String incomingLabel = "incomingLabel"; + String incomingDirectoryLabel = "incomingDirectoryLabel"; + String existingLabel = "existingLabel"; + String existingDirectoryLabel = "existingDirectoryLabel"; + String pathPlusFilename = IngestUtil.getPathAndFileNameToCheck(incomingLabel, incomingDirectoryLabel, existingLabel, existingDirectoryLabel); + assertEquals("incomingDirectoryLabel/incomingLabel", pathPlusFilename); + } + + @Test + public void renameFileToSameName() { + String pathPlusFilename = "README.md"; + FileMetadata file1 = new FileMetadata(); + file1.setLabel("README.md"); + FileMetadata file2 = new FileMetadata(); + file2.setLabel("README2.md"); + List fileMetadatas = Arrays.asList(file1, file2); + assertTrue(IngestUtil.conflictsWithExistingFilenames(pathPlusFilename, fileMetadatas)); + } + }