diff --git a/src/main/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposerForUpdate.java b/src/main/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposerForUpdate.java index 2e437f5..1c6329e 100644 --- a/src/main/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposerForUpdate.java +++ b/src/main/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposerForUpdate.java @@ -172,13 +172,13 @@ private Map getFilesToReplace(Map pathToFileI } /** - * Creatings a mapping for moving files to a new location. To determine this, the file needs to be unique in the old and the new version, because its checksum is used to locate it. Files that + * Creating a mapping for moving files to a new location. To determine this, the file needs to be unique in the old and the new version, because its checksum is used to locate it. Files that * occur multiple times in either the old or the new version cannot be moved in this way. They will appear to have been deleted in the old version and added in the new. This has the same net * result, except that the "Changes" overview in Dataverse does not record that the file was effectively moved. * * @param pathToFileMetaInLatestVersion map from path to file metadata in the old version * @param pathToFileInfo map from path to file info in the new version (i.e. the deposit). - * @return + * @return a map from old path to new path */ Map getOldToNewPathOfFilesToMove(Map pathToFileMetaInLatestVersion, Map pathToFileInfo) { @@ -199,6 +199,7 @@ Map getOldToNewPathOfFilesToMove(Map pathToFileMetaI return intersects.stream() .map(c -> Map.entry(checksumsToPathNonDuplicatedFilesInLatestVersion.get(c), checksumsToPathNonDuplicatedFilesInDeposit.get(c))) + .filter(entry -> !entry.getKey().equals(entry.getValue())) // filter out files that are not moved (this was not present in the old code) .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); } diff --git a/src/test/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposerFixture.java b/src/test/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposerFixture.java new file mode 100644 index 0000000..0527c22 --- /dev/null +++ b/src/test/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposerFixture.java @@ -0,0 +1,85 @@ +/* + * Copyright (C) 2024 DANS - Data Archiving and Networked Services (info@dans.knaw.nl) + * + * Licensed 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 nl.knaw.dans.dvingest.core.dansbag; + +import nl.knaw.dans.dvingest.core.bagprocessor.DataversePath; +import nl.knaw.dans.dvingest.core.dansbag.deposit.FileInfo; +import nl.knaw.dans.lib.dataverse.model.file.FileMeta; + +import java.nio.file.Path; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.List; +import java.util.Map; + +public class EditFilesComposerFixture { + protected static final Instant inThePast = Instant.parse("2010-01-01T00:00:00Z"); + protected final Instant inTheFuture = Instant.now().plus(1, ChronoUnit.DAYS); + protected EditFilesComposer editFilesComposer; + + /* + * Helper methods to set things up. + */ + protected FileInfo file(String path, String checksum, boolean restricted, String description, List categories) { + var fileMeta = new FileMeta(); + var dataversePath = new DataversePath(path); + fileMeta.setLabel(dataversePath.getLabel()); + fileMeta.setDirectoryLabel(dataversePath.getDirectoryLabel()); + fileMeta.setRestrict(restricted); + if (description != null) { + fileMeta.setDescription(description); + } + if (categories != null) { + fileMeta.setCategories(categories); + } + return new FileInfo(Path.of(path), checksum, false, fileMeta); + } + + private FileInfo sanitizedFile(String path, String sanitizedPath, String checksum, boolean restricted, String description, List categories) { + var fileMeta = new FileMeta(); + var dataversePath = new DataversePath(sanitizedPath); + fileMeta.setLabel(dataversePath.getLabel()); + fileMeta.setDirectoryLabel(dataversePath.getDirectoryLabel()); + fileMeta.setRestrict(restricted); + if (description != null) { + fileMeta.setDescription(description); + } + if (categories != null) { + fileMeta.setCategories(categories); + } + return new FileInfo(Path.of(path), checksum, true, fileMeta); + } + + protected FileInfo sanitizedFile(String path, String sanitizedPath, String checksum) { + return sanitizedFile(path, sanitizedPath, checksum, false, null, null); + } + + protected FileInfo file(String path, String checksum) { + return file(path, checksum, false, null, null); + } + + protected FileInfo file(String path, String checksum, boolean restricted) { + return file(path, checksum, restricted, null, null); + } + + protected FileInfo file(String path, String checksum, boolean restricted, String description) { + return file(path, checksum, restricted, description, null); + } + + protected void add(Map map, FileInfo fileInfo) { + map.put(fileInfo.getPath(), fileInfo); + } +} diff --git a/src/test/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposerForUpdateTest.java b/src/test/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposerForUpdateTest.java index 570e8ce..f469e15 100644 --- a/src/test/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposerForUpdateTest.java +++ b/src/test/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposerForUpdateTest.java @@ -15,5 +15,146 @@ */ package nl.knaw.dans.dvingest.core.dansbag; -public class EditFilesComposerForUpdateTest { +import nl.knaw.dans.dvingest.core.bagprocessor.DataversePath; +import nl.knaw.dans.dvingest.core.dansbag.deposit.FileInfo; +import nl.knaw.dans.dvingest.core.service.DataverseService; +import nl.knaw.dans.lib.dataverse.model.file.Checksum; +import nl.knaw.dans.lib.dataverse.model.file.DataFile; +import nl.knaw.dans.lib.dataverse.model.file.FileMeta; +import org.junit.jupiter.api.Test; + +import java.nio.file.Path; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class EditFilesComposerForUpdateTest extends EditFilesComposerFixture { + private final DataverseService dataverseServiceMock = mock(DataverseService.class); + + private FileMeta fileMeta(String path, String checksum) { + var fileMeta = new FileMeta(); + var dataversePath = new DataversePath(path); + fileMeta.setLabel(dataversePath.getLabel()); + fileMeta.setDirectoryLabel(dataversePath.getDirectoryLabel()); + var dataFile = new DataFile(); + var cs = new Checksum(); + cs.setType("SHA-1"); + cs.setValue(checksum); + dataFile.setChecksum(cs); + dataFile.setFilename(dataversePath.getLabel()); + // No directoryLabel? + fileMeta.setDataFile(dataFile); + return fileMeta; + } + + @Test + public void file_with_same_path_and_different_checksum_is_replaced() throws Exception { + // Given + when(dataverseServiceMock.getFiles(anyString())).thenReturn(List.of(fileMeta("file1.txt", "oldchecksum"))); + Map map = new HashMap<>(); + add(map, file("file1.txt", "newchecksum")); + + editFilesComposer = new EditFilesComposerForUpdate(map, inThePast, "doi:some", null, List.of(), dataverseServiceMock); + + // When + var editFiles = editFilesComposer.composeEditFiles(); + + // Then + assertThat(editFiles.getReplaceFiles()).hasSize(1); + assertThat(editFiles.getReplaceFiles().get(0)).isEqualTo("file1.txt"); + + // The rest is not affected + assertThat(editFiles.getAutoRenameFiles()).isEmpty(); + assertThat(editFiles.getIgnoreFiles()).isEmpty(); + assertThat(editFiles.getUpdateFileMetas()).isEmpty(); + assertThat(editFiles.getAddEmbargoes()).isEmpty(); + assertThat(editFiles.getDeleteFiles()).isEmpty(); + assertThat(editFiles.getMoveFiles()).isEmpty(); + } + + @Test + public void file_with_same_path_and_same_checksum_is_NOT_replaced() throws Exception { + // Given + when(dataverseServiceMock.getFiles(anyString())).thenReturn(List.of(fileMeta("file1.txt", "oldchecksum"))); + Map map = new HashMap<>(); + add(map, file("file1.txt", "oldchecksum")); + + editFilesComposer = new EditFilesComposerForUpdate(map, inThePast, "doi:some", null, List.of(), dataverseServiceMock); + + // When + var editFiles = editFilesComposer.composeEditFiles(); + + // Then + assertThat(editFiles.getReplaceFiles()).isEmpty(); + + // The rest is not affected + assertThat(editFiles.getAutoRenameFiles()).isEmpty(); + assertThat(editFiles.getIgnoreFiles()).isEmpty(); + assertThat(editFiles.getUpdateFileMetas()).isEmpty(); + assertThat(editFiles.getAddEmbargoes()).isEmpty(); + assertThat(editFiles.getDeleteFiles()).isEmpty(); + assertThat(editFiles.getMoveFiles()).isEmpty(); + } + + @Test + public void file_with_different_path_and_same_checksum_is_moved() throws Exception { + // Given + when(dataverseServiceMock.getFiles(anyString())).thenReturn(List.of(fileMeta("path/to/file1.txt", "oldchecksum"))); + Map map = new HashMap<>(); + add(map, file("path/three/file2.txt", "oldchecksum")); + + editFilesComposer = new EditFilesComposerForUpdate(map, inThePast, "doi:some", null, List.of(), dataverseServiceMock); + + // When + var editFiles = editFilesComposer.composeEditFiles(); + + // Then + assertThat(editFiles.getMoveFiles()).hasSize(1); + assertThat(editFiles.getMoveFiles().get(0).getFrom()).isEqualTo("path/to/file1.txt"); + assertThat(editFiles.getMoveFiles().get(0).getTo()).isEqualTo("path/three/file2.txt"); + + // The rest is not affected + assertThat(editFiles.getReplaceFiles()).isEmpty(); + assertThat(editFiles.getAutoRenameFiles()).isEmpty(); + assertThat(editFiles.getIgnoreFiles()).isEmpty(); + assertThat(editFiles.getUpdateFileMetas()).isEmpty(); + assertThat(editFiles.getAddEmbargoes()).isEmpty(); + assertThat(editFiles.getDeleteFiles()).isEmpty(); + } + +// @Test +// public void ambiguous_move_is_implemented_add_delete_and_add() throws Exception { +// // Given +// when(dataverseServiceMock.getFiles(anyString())).thenReturn(List.of(fileMeta("path/to/file1.txt", "oldchecksum"))); +// Map map = new HashMap<>(); +// add(map, file("path/three/file1.txt", "oldchecksum")); +// add(map, file("path/three/file2.txt", "oldchecksum")); +// +// editFilesComposer = new EditFilesComposerForUpdate(map, inThePast, "doi:some", null, List.of(), dataverseServiceMock); +// +// // When +// var editFiles = editFilesComposer.composeEditFiles(); +// +// // Then +// +// assertThat(editFiles.getDeleteFiles()).hasSize(1); +// assertThat(editFiles.getDeleteFiles().get(0)).isEqualTo("path/to/file1.txt"); +// +// // The rest is not affected +// assertThat(editFiles.getReplaceFiles()).isEmpty(); +// assertThat(editFiles.getAutoRenameFiles()).isEmpty(); +// assertThat(editFiles.getIgnoreFiles()).isEmpty(); +// assertThat(editFiles.getUpdateFileMetas()).isEmpty(); +// assertThat(editFiles.getAddEmbargoes()).isEmpty(); +// assertThat(editFiles.getMoveFiles()).isEmpty(); +// } + + + + } diff --git a/src/test/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposerTest.java b/src/test/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposerTest.java index 0aeed29..5688348 100644 --- a/src/test/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposerTest.java +++ b/src/test/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposerTest.java @@ -15,15 +15,12 @@ */ package nl.knaw.dans.dvingest.core.dansbag; -import nl.knaw.dans.dvingest.core.bagprocessor.DataversePath; import nl.knaw.dans.dvingest.core.dansbag.deposit.FileInfo; import nl.knaw.dans.dvingest.core.yaml.AddEmbargo; import nl.knaw.dans.lib.dataverse.model.file.FileMeta; import org.junit.jupiter.api.Test; import java.nio.file.Path; -import java.time.Instant; -import java.time.temporal.ChronoUnit; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -31,67 +28,7 @@ import static org.assertj.core.api.Assertions.assertThat; -public class EditFilesComposerTest { - private static final Instant inThePast = Instant.parse("2010-01-01T00:00:00Z"); - private final Instant inTheFuture = Instant.now().plus(1, ChronoUnit.DAYS); - private EditFilesComposer editFilesComposer; - - /* - * Helper methods to set things up. - */ - private FileInfo file(String path, String checksum, boolean restricted, String description, List categories) { - var fileMeta = new FileMeta(); - var dataversePath = new DataversePath(path); - fileMeta.setLabel(dataversePath.getLabel()); - fileMeta.setDirectoryLabel(dataversePath.getDirectoryLabel()); - fileMeta.setRestrict(restricted); - if (description != null) { - fileMeta.setDescription(description); - } - if (categories != null) { - fileMeta.setCategories(categories); - } - return new FileInfo(Path.of(path), checksum, false, fileMeta); - } - - private FileInfo sanitizedFile(String path, String sanitizedPath, String checksum, boolean restricted, String description, List categories) { - var fileMeta = new FileMeta(); - var dataversePath = new DataversePath(sanitizedPath); - fileMeta.setLabel(dataversePath.getLabel()); - fileMeta.setDirectoryLabel(dataversePath.getDirectoryLabel()); - fileMeta.setRestrict(restricted); - if (description != null) { - fileMeta.setDescription(description); - } - if (categories != null) { - fileMeta.setCategories(categories); - } - return new FileInfo(Path.of(path), checksum, true, fileMeta); - } - - private FileInfo sanitizedFile(String path, String sanitizedPath, String checksum) { - return sanitizedFile(path, sanitizedPath, checksum, false, null, null); - } - - private FileInfo file(String path, String checksum) { - return file(path, checksum, false, null, null); - } - - private FileInfo file(String path, String checksum, boolean restricted) { - return file(path, checksum, restricted, null, null); - } - - private FileInfo file(String path, String checksum, boolean restricted, String description) { - return file(path, checksum, restricted, description, null); - } - - private void add(Map map, FileInfo fileInfo) { - map.put(fileInfo.getPath(), fileInfo); - } - - /* - * Tests - */ +public class EditFilesComposerTest extends EditFilesComposerFixture { @Test public void adding_two_unrestricted_files_leaves_editFiles_empty() {