From 28d648457f7972f8bf5f3374ee1831d8f2796905 Mon Sep 17 00:00:00 2001 From: Jan van Mansum Date: Sun, 8 Dec 2024 21:21:23 +0100 Subject: [PATCH] Made ignoring files the default instead of adding unrestricted. --- .../FileUploadInclusionPredicate.java | 6 +- .../core/dansbag/DansBagMappingService.java | 2 +- .../dansbag/DansBagMappingServiceImpl.java | 9 +- .../core/dansbag/DansDepositConverter.java | 3 +- .../core/dansbag/DansDepositSupport.java | 2 +- .../core/dansbag/EditFilesComposer.java | 1 - .../dansbag/EditFilesComposerForUpdate.java | 31 ++-- .../dans/dvingest/core/yaml/EditFiles.java | 2 +- .../FileUploadInclusionPredicateTest.java | 26 --- .../dansbag/DansDepositConverterTest.java | 2 +- .../dansbag/EditFilesComposerFixture.java | 46 +++++ .../EditFilesComposerForUpdateTest.java | 160 ++++++++++-------- .../core/dansbag/EditFilesComposerTest.java | 66 +------- 13 files changed, 176 insertions(+), 180 deletions(-) diff --git a/src/main/java/nl/knaw/dans/dvingest/core/bagprocessor/FileUploadInclusionPredicate.java b/src/main/java/nl/knaw/dans/dvingest/core/bagprocessor/FileUploadInclusionPredicate.java index dc38ff9..ea2bda9 100644 --- a/src/main/java/nl/knaw/dans/dvingest/core/bagprocessor/FileUploadInclusionPredicate.java +++ b/src/main/java/nl/knaw/dans/dvingest/core/bagprocessor/FileUploadInclusionPredicate.java @@ -31,7 +31,7 @@ public class FileUploadInclusionPredicate implements Predicate { @Override public boolean evaluate(File file) { return editFiles != null && (restrictedFiles ? isRestricted(file) : notRestricted(file)) - && notReplaced(file) && notIgnored(file); + && notReplaced(file); } private boolean notReplaced(File file) { @@ -45,8 +45,4 @@ private boolean isRestricted(File file) { private boolean notRestricted(File file) { return editFiles.getAddUnrestrictedFiles().contains(dataDir.relativize(file.toPath()).toString()); } - - private boolean notIgnored(File file) { - return !editFiles.getIgnoreFiles().contains(dataDir.relativize(file.toPath()).toString()); - } } \ No newline at end of file diff --git a/src/main/java/nl/knaw/dans/dvingest/core/dansbag/DansBagMappingService.java b/src/main/java/nl/knaw/dans/dvingest/core/dansbag/DansBagMappingService.java index 725256b..029526f 100644 --- a/src/main/java/nl/knaw/dans/dvingest/core/dansbag/DansBagMappingService.java +++ b/src/main/java/nl/knaw/dans/dvingest/core/dansbag/DansBagMappingService.java @@ -32,7 +32,7 @@ public interface DansBagMappingService { Dataset getDatasetMetadataFromDansDeposit(DansBagDeposit dansDeposit); - EditFiles getEditFilesFromDansDeposit(DansBagDeposit dansDeposit); + EditFiles getEditFilesFromDansDeposit(DansBagDeposit dansDeposit, String updatesDataset); EditPermissions getEditPermissionsFromDansDeposit(DansBagDeposit dansDeposit); diff --git a/src/main/java/nl/knaw/dans/dvingest/core/dansbag/DansBagMappingServiceImpl.java b/src/main/java/nl/knaw/dans/dvingest/core/dansbag/DansBagMappingServiceImpl.java index 795c42f..b291dc2 100644 --- a/src/main/java/nl/knaw/dans/dvingest/core/dansbag/DansBagMappingServiceImpl.java +++ b/src/main/java/nl/knaw/dans/dvingest/core/dansbag/DansBagMappingServiceImpl.java @@ -141,10 +141,15 @@ public Dataset getDatasetMetadataFromDansDeposit(DansBagDeposit dansDeposit) { } @Override - public EditFiles getEditFilesFromDansDeposit(DansBagDeposit dansDeposit) { + public EditFiles getEditFilesFromDansDeposit(DansBagDeposit dansDeposit, String updatesDataset) { var files = getFileInfo(dansDeposit); var dateAvailable = getDateAvailable(dansDeposit); - return new EditFilesComposer(files, dateAvailable, fileExclusionPattern, embargoExclusions).composeEditFiles(); + if (updatesDataset == null) { + return new EditFilesComposer(files, dateAvailable, fileExclusionPattern, embargoExclusions).composeEditFiles(); + } + else { + return new EditFilesComposerForUpdate(files, dateAvailable, updatesDataset, fileExclusionPattern, embargoExclusions, dataverseService).composeEditFiles(); + } } @Override diff --git a/src/main/java/nl/knaw/dans/dvingest/core/dansbag/DansDepositConverter.java b/src/main/java/nl/knaw/dans/dvingest/core/dansbag/DansDepositConverter.java index 28bb67c..85f38b7 100644 --- a/src/main/java/nl/knaw/dans/dvingest/core/dansbag/DansDepositConverter.java +++ b/src/main/java/nl/knaw/dans/dvingest/core/dansbag/DansDepositConverter.java @@ -27,6 +27,7 @@ @AllArgsConstructor public class DansDepositConverter { private final DansBagDeposit dansDeposit; + private final String updatesDataset; private final DansBagMappingService mappingService; private final YamlService yamlService; @@ -37,7 +38,7 @@ public void run() throws IOException { var dataset = mappingService.getDatasetMetadataFromDansDeposit(dansDeposit); yamlService.writeYaml(dataset, dansDeposit.getBagDir().resolve("dataset.yml")); - var editFiles = mappingService.getEditFilesFromDansDeposit(dansDeposit); + var editFiles = mappingService.getEditFilesFromDansDeposit(dansDeposit, updatesDataset); yamlService.writeYaml(new EditFilesRoot(editFiles), dansDeposit.getBagDir().resolve("edit-files.yml")); var editPermissions = mappingService.getEditPermissionsFromDansDeposit(dansDeposit); diff --git a/src/main/java/nl/knaw/dans/dvingest/core/dansbag/DansDepositSupport.java b/src/main/java/nl/knaw/dans/dvingest/core/dansbag/DansDepositSupport.java index 79d7efd..3e5bb0d 100644 --- a/src/main/java/nl/knaw/dans/dvingest/core/dansbag/DansDepositSupport.java +++ b/src/main/java/nl/knaw/dans/dvingest/core/dansbag/DansDepositSupport.java @@ -70,7 +70,7 @@ public boolean convertDansDepositIfNeeded() { ingestDataverseIngestDeposit.updateProperties(Map.of("updatesDataset", updatesDataset)); } dansDeposit = dansBagMappingService.readDansDeposit(ingestDataverseIngestDeposit.getLocation()); - new DansDepositConverter(dansDeposit, dansBagMappingService, yamlService).run(); + new DansDepositConverter(dansDeposit, updatesDataset, dansBagMappingService, yamlService).run(); log.info("Conversion successful"); return true; } diff --git a/src/main/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposer.java b/src/main/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposer.java index 36cf08e..9f7500e 100644 --- a/src/main/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposer.java +++ b/src/main/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposer.java @@ -58,7 +58,6 @@ public EditFiles composeEditFiles() { var ignoredFiles = getFilesToIgnore(pathFileInfoMap); var editFiles = new EditFiles(); - editFiles.setIgnoreFiles(ignoredFiles); pathFileInfoMap = removeIgnoredFiles(pathFileInfoMap, ignoredFiles); editFiles.setAutoRenameFiles(getAutoRenamedFiles(renamedFiles)); 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 1c6329e..04fb3f6 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 @@ -100,7 +100,7 @@ public EditFiles composeEditFiles() { var filesToReplace = getFilesToReplace(pathFileInfoMap, fileReplacementCandidates); log.debug("filesToReplace = {}", filesToReplace); // TODO: check if we can do away with the IDs - editFiles.setReplaceFiles(filesToReplace.values().stream().map(FileInfo::getPath).map(Path::toString).collect(Collectors.toList())); + editFiles.setReplaceFiles(filesToReplace.values().stream().map(FileInfo::getPath).map(Path::toString).toList()); /* * To find the files to delete we start from the paths in the deposit payload. In principle, these paths are remaining, so should NOT be deleted. @@ -124,7 +124,7 @@ public EditFiles composeEditFiles() { * After the movements have been performed, which paths are occupied? We start from the paths of the latest version (pathToFileMetaInLatestVersion.keySet) * * The old paths of the moved files (oldToNewPathMovedFiles.keySet) are no longer occupied, so they must be subtracted. This is important in the case where - * a deposit renames/moves a file (leaving the checksum unchanges) but provides a new file for the vacated path. + * a deposit renames/moves a file (leaving the checksum unchanged) but provides a new file for the vacated path. * * The paths of the deleted files (pathsToDelete) are no longer occupied, so must be subtracted. (It is not strictly necessary for the calculation * of pathsToAdd, but done anyway to keep the logic consistent.) @@ -144,21 +144,20 @@ public EditFiles composeEditFiles() { .toList().stream() .filter(f -> f.getMetadata().getRestricted()) .toList(); - editFiles.setAddRestrictedFiles(restrictedFilesToAdd.stream().map(FileInfo::getPath).map(Path::toString).collect(Collectors.toList())); + editFiles.setAddRestrictedFiles(restrictedFilesToAdd.stream().map(FileInfo::getPath).map(Path::toString).toList()); - // todo: set ignoredFiles + var unrestrictedFilesToAdd = pathsToAdd.stream() + .map(pathFileInfoMap::get) + .toList().stream() + .filter(f -> !f.getMetadata().getRestricted()) + .toList(); + editFiles.setAddUnrestrictedFiles(unrestrictedFilesToAdd.stream().map(FileInfo::getPath).map(Path::toString).toList()); // todo: embargoes return editFiles; } - private Set getFileDeletions(Set pathsToDelete, Map pathToFileInfoInLatestVersion) { - return pathsToDelete.stream() - .map(p -> pathToFileInfoInLatestVersion.get(p).getDataFile().getId()) - .collect(Collectors.toSet()); - } - private Map getFilesToReplace(Map pathToFileInfo, Map fileReplacementCandidates) { var intersection = SetUtils.intersection(pathToFileInfo.keySet(), fileReplacementCandidates.keySet()); @@ -172,9 +171,9 @@ private Map getFilesToReplace(Map pathToFileI } /** - * 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. + * 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). @@ -206,11 +205,11 @@ Map getOldToNewPathOfFilesToMove(Map pathToFileMetaI private Map getChecksumsToPathOfNonDuplicateFiles(Map pathToChecksum) { // inverse map first - var inversed = pathToChecksum.entrySet().stream() + var inverse = pathToChecksum.entrySet().stream() .collect(Collectors.groupingBy(Map.Entry::getValue, Collectors.mapping(Map.Entry::getKey, Collectors.toList()))); - // filter out items with 0 or more than 1 items - return inversed.entrySet().stream() + // filter out items with 0 or more than 1 item + return inverse.entrySet().stream() .filter(item -> item.getValue().size() == 1) .collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().get(0))); } diff --git a/src/main/java/nl/knaw/dans/dvingest/core/yaml/EditFiles.java b/src/main/java/nl/knaw/dans/dvingest/core/yaml/EditFiles.java index 9f78eac..0aa3961 100644 --- a/src/main/java/nl/knaw/dans/dvingest/core/yaml/EditFiles.java +++ b/src/main/java/nl/knaw/dans/dvingest/core/yaml/EditFiles.java @@ -27,7 +27,7 @@ public class EditFiles { private List addUnrestrictedFiles = List.of(); private List addRestrictedFiles = List.of(); private List moveFiles = List.of(); - private List ignoreFiles = List.of(); +// private List ignoreFiles = List.of(); private List autoRenameFiles = List.of(); private List updateFileMetas = List.of(); private List addEmbargoes = List.of(); diff --git a/src/test/java/nl/knaw/dans/dvingest/core/bagprocessor/FileUploadInclusionPredicateTest.java b/src/test/java/nl/knaw/dans/dvingest/core/bagprocessor/FileUploadInclusionPredicateTest.java index 5dccae1..b2f6631 100644 --- a/src/test/java/nl/knaw/dans/dvingest/core/bagprocessor/FileUploadInclusionPredicateTest.java +++ b/src/test/java/nl/knaw/dans/dvingest/core/bagprocessor/FileUploadInclusionPredicateTest.java @@ -33,32 +33,6 @@ public void setUp() { editFiles = new EditFiles(); } - @Test - public void ignored_files_are_skipped_for_unrestricted_file_upload() throws Exception { - // Given - editFiles.setIgnoreFiles(List.of("file1")); - var fileUploadInclusionPredicate = new FileUploadInclusionPredicate(editFiles, dataDir.toPath(), false); - - // When - var result = fileUploadInclusionPredicate.evaluate(new File("dataDir/file1")); - - // Then - assertThat(result).isFalse(); - } - - @Test - public void ignored_files_are_skipped_for_restricted_file_upload() throws Exception { - // Given - editFiles.setIgnoreFiles(List.of("file1")); - var fileUploadInclusionPredicate = new FileUploadInclusionPredicate(editFiles, dataDir.toPath(), true); - - // When - var result = fileUploadInclusionPredicate.evaluate(new File("dataDir/file1")); - - // Then - assertThat(result).isFalse(); - } - @Test public void restricted_files_are_included_for_restricted_file_upload() throws Exception { // Given diff --git a/src/test/java/nl/knaw/dans/dvingest/core/dansbag/DansDepositConverterTest.java b/src/test/java/nl/knaw/dans/dvingest/core/dansbag/DansDepositConverterTest.java index 3c3cd58..9b1f370 100644 --- a/src/test/java/nl/knaw/dans/dvingest/core/dansbag/DansDepositConverterTest.java +++ b/src/test/java/nl/knaw/dans/dvingest/core/dansbag/DansDepositConverterTest.java @@ -44,7 +44,7 @@ public void run_converts_dans_sword_all_mappings_example_to_dataverse_ingest_dep Mockito.when(dataverseServiceMock.getUserById(Mockito.anyString())).thenReturn(Optional.of(authenticatedUser)); // When - new DansDepositConverter(deposit, mappingService, yamlService).run(); + new DansDepositConverter(deposit, null, mappingService, yamlService).run(); // Then assertThat(deposit.getBagDir().resolve("dataset.yml")).exists(); 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 index 0527c22..91ee33a 100644 --- a/src/test/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposerFixture.java +++ b/src/test/java/nl/knaw/dans/dvingest/core/dansbag/EditFilesComposerFixture.java @@ -17,6 +17,9 @@ import nl.knaw.dans.dvingest.core.bagprocessor.DataversePath; import nl.knaw.dans.dvingest.core.dansbag.deposit.FileInfo; +import nl.knaw.dans.dvingest.core.yaml.EditFiles; +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 java.nio.file.Path; @@ -25,6 +28,8 @@ import java.util.List; import java.util.Map; +import static org.assertj.core.api.Assertions.assertThat; + 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); @@ -82,4 +87,45 @@ protected FileInfo file(String path, String checksum, boolean restricted, String protected void add(Map map, FileInfo fileInfo) { map.put(fileInfo.getPath(), fileInfo); } + + protected void assertEmptyFieldsExcept(EditFiles editFiles, String... except) { + List exceptList = List.of(except); + if (!exceptList.contains("addRestrictedFiles")) { + assertThat(editFiles.getAddRestrictedFiles()).as("expected addRestrictedFiles to remain empty").isEmpty(); + } + if (!exceptList.contains("addUnrestrictedFiles")) { + assertThat(editFiles.getAddUnrestrictedFiles()).as("expected addUnrestrictedFiles to remain empty").isEmpty(); + } + if (!exceptList.contains("autoRenameFiles")) { + assertThat(editFiles.getAutoRenameFiles()).as("expected autoRenameFiles to remain empty").isEmpty(); + } + if (!exceptList.contains("updateFileMetas")) { + assertThat(editFiles.getUpdateFileMetas()).as("expected updateFileMetas to remain empty").isEmpty(); + } + if (!exceptList.contains("addEmbargoes")) { + assertThat(editFiles.getAddEmbargoes()).as("expected addEmbargoes to remain empty").isEmpty(); + } + if (!exceptList.contains("deleteFiles")) { + assertThat(editFiles.getDeleteFiles()).as("expected deleteFiles to remain empty").isEmpty(); + } + if (!exceptList.contains("moveFiles")) { + assertThat(editFiles.getMoveFiles()).as("expected moveFiles to remain empty").isEmpty(); + } + } + + protected 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; + } } 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 f469e15..a6ff5ce 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,12 +15,8 @@ */ 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.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; @@ -36,22 +32,6 @@ 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 @@ -68,13 +48,7 @@ public void file_with_same_path_and_different_checksum_is_replaced() throws Exce 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(); + assertEmptyFieldsExcept(editFiles, "replaceFiles"); } @Test @@ -92,13 +66,7 @@ public void file_with_same_path_and_same_checksum_is_NOT_replaced() throws Excep // 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(); + assertEmptyFieldsExcept(editFiles); } @Test @@ -118,43 +86,101 @@ public void file_with_different_path_and_same_checksum_is_moved() throws Excepti 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(); + assertEmptyFieldsExcept(editFiles, "moveFiles"); } -// @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(); -// } + @Test + public void unrestricted_file_with_different_path_and_different_checksum_is_added() throws Exception { + // Given + when(dataverseServiceMock.getFiles(anyString())).thenReturn(List.of(fileMeta("path/to/file1.txt", "oldchecksum"))); + Map map = new HashMap<>(); + add(map, file("path/to/file1.txt", "oldchecksum")); // Confirming that the file is to remain in the dataset + add(map, file("path/three/file2.txt", "newchecksum")); + editFilesComposer = new EditFilesComposerForUpdate(map, inThePast, "doi:some", null, List.of(), dataverseServiceMock); + + // When + var editFiles = editFilesComposer.composeEditFiles(); + + // Then + assertThat(editFiles.getAddUnrestrictedFiles()).hasSize(1); + assertThat(editFiles.getAddUnrestrictedFiles()).contains("path/three/file2.txt"); + + assertEmptyFieldsExcept(editFiles, "addUnrestrictedFiles"); + } + + @Test + public void restricted_file_with_different_path_and_different_checksum_is_added() throws Exception { + // Given + when(dataverseServiceMock.getFiles(anyString())).thenReturn(List.of(fileMeta("path/to/file1.txt", "oldchecksum"))); + Map map = new HashMap<>(); + add(map, file("path/to/file1.txt", "oldchecksum")); // Confirming that the file is to remain in the dataset + add(map, file("path/three/file2.txt", "newchecksum", true)); + editFilesComposer = new EditFilesComposerForUpdate(map, inThePast, "doi:some", null, List.of(), dataverseServiceMock); + + // When + var editFiles = editFilesComposer.composeEditFiles(); + // Then + assertThat(editFiles.getAddRestrictedFiles()).hasSize(1); + assertThat(editFiles.getAddRestrictedFiles()).contains("path/three/file2.txt"); + assertEmptyFieldsExcept(editFiles, "addRestrictedFiles"); + } + + @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"); + assertThat(editFiles.getAddUnrestrictedFiles()).hasSize(2); + assertThat(editFiles.getAddUnrestrictedFiles()).contains("path/three/file1.txt", "path/three/file2.txt"); + + assertEmptyFieldsExcept(editFiles, "deleteFiles", "addUnrestrictedFiles"); + } + + @Test + public void file_not_replaced_nor_in_current_deposit_is_deleted() throws Exception { + // Given + when(dataverseServiceMock.getFiles(anyString())).thenReturn(List.of(fileMeta("path/to/file1.txt", "oldchecksum"))); + Map map = new HashMap<>(); + + 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"); + + assertEmptyFieldsExcept(editFiles, "deleteFiles"); + } + + @Test + public void file_with_same_path_and_checksum_is_not_touched() throws Exception { + // Given + when(dataverseServiceMock.getFiles(anyString())).thenReturn(List.of(fileMeta("path/to/file1.txt", "oldchecksum"))); + Map map = new HashMap<>(); + add(map, file("path/to/file1.txt", "oldchecksum")); + + editFilesComposer = new EditFilesComposerForUpdate(map, inThePast, "doi:some", null, List.of(), dataverseServiceMock); + + // When + var editFiles = editFilesComposer.composeEditFiles(); + + // Then + assertEmptyFieldsExcept(editFiles); + } } 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 5688348..ba6c810 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 @@ -42,13 +42,7 @@ public void adding_two_unrestricted_files_leaves_editFiles_empty() { var editFiles = editFilesComposer.composeEditFiles(); // Then - assertThat(editFiles.getAddRestrictedFiles()).isEmpty(); - assertThat(editFiles.getAutoRenameFiles()).isEmpty(); - assertThat(editFiles.getIgnoreFiles()).isEmpty(); - assertThat(editFiles.getUpdateFileMetas()).isEmpty(); - assertThat(editFiles.getAddEmbargoes()).isEmpty(); - assertThat(editFiles.getDeleteFiles()).isEmpty(); - assertThat(editFiles.getMoveFiles()).isEmpty(); + assertEmptyFieldsExcept(editFiles); } @Test @@ -68,13 +62,7 @@ public void adding_two_restricted_files_adds_them_to_addRestrictedFiles() { assertThat(addRestrictedFiles).hasSize(2); assertThat(addRestrictedFiles).contains("file1.txt", "file2.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(); + assertEmptyFieldsExcept(editFiles, "addRestrictedFiles"); } @Test @@ -94,13 +82,7 @@ public void setting_description_adds_it_to_updateFileMetas() { assertThat(updateFileMetas).hasSize(2); assertThat(updateFileMetas).extracting(FileMeta::getDescription).contains("description1", "description2"); - // The rest is not affected - assertThat(editFiles.getAddRestrictedFiles()).isEmpty(); - assertThat(editFiles.getAutoRenameFiles()).isEmpty(); - assertThat(editFiles.getIgnoreFiles()).isEmpty(); - assertThat(editFiles.getAddEmbargoes()).isEmpty(); - assertThat(editFiles.getDeleteFiles()).isEmpty(); - assertThat(editFiles.getMoveFiles()).isEmpty(); + assertEmptyFieldsExcept(editFiles, "updateFileMetas"); } @Test @@ -120,13 +102,7 @@ public void setting_categories_adds_them_to_updateFileMetas() { assertThat(updateFileMetas).hasSize(2); assertThat(updateFileMetas).extracting(FileMeta::getCategories).contains(List.of("category1"), List.of("category2")); - // The rest is not affected - assertThat(editFiles.getAddRestrictedFiles()).isEmpty(); - assertThat(editFiles.getAutoRenameFiles()).isEmpty(); - assertThat(editFiles.getIgnoreFiles()).isEmpty(); - assertThat(editFiles.getAddEmbargoes()).isEmpty(); - assertThat(editFiles.getDeleteFiles()).isEmpty(); - assertThat(editFiles.getMoveFiles()).isEmpty(); + assertEmptyFieldsExcept(editFiles, "updateFileMetas"); } @Test @@ -145,13 +121,7 @@ public void setting_dateAvailable_to_future_date_embargoes_all_files() { assertThat(addEmbargoes).hasSize(1); // There is only one embargo, covering all files assertThat(addEmbargoes).extracting(AddEmbargo::getFilePaths).containsExactly(List.of("file1.txt", "file2.txt")); - // The rest is not affected - assertThat(editFiles.getAddRestrictedFiles()).isEmpty(); - assertThat(editFiles.getAutoRenameFiles()).isEmpty(); - assertThat(editFiles.getIgnoreFiles()).isEmpty(); - assertThat(editFiles.getUpdateFileMetas()).isEmpty(); - assertThat(editFiles.getDeleteFiles()).isEmpty(); - assertThat(editFiles.getMoveFiles()).isEmpty(); + assertEmptyFieldsExcept(editFiles, "addEmbargoes"); } @Test @@ -171,13 +141,7 @@ public void sanitized_files_are_add_to_autoRenamedFiles() { assertThat(autoRenameFiles.get(0).getFrom()).isEqualTo("file1.txt"); assertThat(autoRenameFiles.get(0).getTo()).isEqualTo("file1_sanitized.txt"); - // The rest is not affected - assertThat(editFiles.getAddRestrictedFiles()).isEmpty(); - assertThat(editFiles.getIgnoreFiles()).isEmpty(); - assertThat(editFiles.getUpdateFileMetas()).isEmpty(); - assertThat(editFiles.getAddEmbargoes()).isEmpty(); - assertThat(editFiles.getDeleteFiles()).isEmpty(); - assertThat(editFiles.getMoveFiles()).isEmpty(); + assertEmptyFieldsExcept(editFiles, "autoRenameFiles"); } @Test @@ -192,15 +156,7 @@ public void fileExclusionPattern_ignores_files_matching_pattern() { var editFiles = editFilesComposer.composeEditFiles(); // Then - assertThat(editFiles.getIgnoreFiles()).containsExactly("file1.txt"); - - // The rest is not affected - assertThat(editFiles.getAddRestrictedFiles()).isEmpty(); - assertThat(editFiles.getAutoRenameFiles()).isEmpty(); - assertThat(editFiles.getUpdateFileMetas()).isEmpty(); - assertThat(editFiles.getAddEmbargoes()).isEmpty(); - assertThat(editFiles.getDeleteFiles()).isEmpty(); - assertThat(editFiles.getMoveFiles()).isEmpty(); + assertEmptyFieldsExcept(editFiles); } @Test @@ -220,13 +176,7 @@ public void embargoExclusions_ignores_files_matching_filepaths() { assertThat(addEmbargoes).hasSize(1); // There is only one embargo, covering all files assertThat(addEmbargoes).extracting(AddEmbargo::getFilePaths).containsExactly(List.of("file1.txt")); - // The rest is not affected - assertThat(editFiles.getAddRestrictedFiles()).isEmpty(); - assertThat(editFiles.getAutoRenameFiles()).isEmpty(); - assertThat(editFiles.getIgnoreFiles()).isEmpty(); - assertThat(editFiles.getUpdateFileMetas()).isEmpty(); - assertThat(editFiles.getDeleteFiles()).isEmpty(); - assertThat(editFiles.getMoveFiles()).isEmpty(); + assertEmptyFieldsExcept(editFiles, "addEmbargoes"); } }