Skip to content

Commit

Permalink
Added saving the NBN to deposit.properties onSuccess + fixes some reg…
Browse files Browse the repository at this point in the history
…ression introduced after changing to explicit addUnrestrictedFiles.
  • Loading branch information
janvanmansum committed Dec 9, 2024
1 parent 1ece89b commit 18c7d93
Show file tree
Hide file tree
Showing 10 changed files with 95 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@

import java.nio.file.Path;

/**
* Proxy for dd-validate-dans-bag service.
*/
public interface ValidateDansBagService {

ValidateOkDto validate(Path bag);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/nl/knaw/dans/dvingest/core/DepositTask.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public enum Status {
public DepositTask(DataverseIngestDeposit dataverseIngestDeposit, Path outputDir, boolean onlyConvertDansDeposit, ValidateDansBagService validateDansBagService, DataverseService dataverseService, UtilityServices utilityServices,
DansBagMappingService dansBagMappingService,
YamlService yamlService) {
this.deposit = dansBagMappingService == null ? dataverseIngestDeposit : new DansDepositSupport(dataverseIngestDeposit, validateDansBagService, dansBagMappingService, yamlService);
this.deposit = dansBagMappingService == null ? dataverseIngestDeposit : new DansDepositSupport(dataverseIngestDeposit, validateDansBagService, dansBagMappingService, dataverseService, yamlService);
this.dataverseService = dataverseService;
this.onlyConvertDansDeposit = onlyConvertDansDeposit;
this.utilityServices = utilityServices;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
import java.io.IOException;
import java.util.UUID;


/**
* Processes a bag, creating and/or editing a dataset version in Dataverse.
*/
@Slf4j
public class BagProcessor {
private final DatasetVersionCreator datasetVersionCreator;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import org.apache.tika.utils.StringUtils;

/**
* A filepath in Dataverse is a combination of file label and directory label. This class converts between a regular representation of a path and the Dataverse representation.
* A filepath in Dataverse is a combination of file label and directory label. This class converts between a regular representation of a filepath and the Dataverse representation.
*/
@Value
public class DataversePath {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@

@Slf4j

/**
* Edits files in a dataset in Dataverse, based on the edit-files.yml file, which has been read and parsed into an EditFiles object.
*/
public class FilesEditor {
private final UUID depositId;
private final Path dataDir;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import nl.knaw.dans.dvingest.core.dansbag.deposit.DansBagDeposit;
import nl.knaw.dans.dvingest.core.dansbag.exception.InvalidDepositException;
import nl.knaw.dans.dvingest.core.dansbag.exception.RejectedDepositException;
import nl.knaw.dans.dvingest.core.service.DataverseService;
import nl.knaw.dans.dvingest.core.service.YamlService;
import nl.knaw.dans.lib.dataverse.DataverseException;

Expand All @@ -41,16 +42,18 @@ public class DansDepositSupport implements Deposit {

private final ValidateDansBagService validateDansBagService;
private final DansBagMappingService dansBagMappingService;
private final DataverseService dataverseService;
private final YamlService yamlService;
private final DataverseIngestDeposit ingestDataverseIngestDeposit;
private final boolean isDansDeposit;

private DansBagDeposit dansDeposit;

public DansDepositSupport(DataverseIngestDeposit dataverseIngestDeposit, ValidateDansBagService validateDansBagService, DansBagMappingService dansBagMappingService, YamlService yamlService) {
public DansDepositSupport(DataverseIngestDeposit dataverseIngestDeposit, ValidateDansBagService validateDansBagService, DansBagMappingService dansBagMappingService, DataverseService dataverseService, YamlService yamlService) {
this.validateDansBagService = validateDansBagService;
this.ingestDataverseIngestDeposit = dataverseIngestDeposit;
this.dansBagMappingService = dansBagMappingService;
this.dataverseService = dataverseService;
this.yamlService = yamlService;
try {
this.isDansDeposit = dataverseIngestDeposit.getBags().get(0).looksLikeDansBag();
Expand Down Expand Up @@ -107,12 +110,18 @@ public void onSuccess(@NonNull String pid, String message) {
var bag = ingestDataverseIngestDeposit.getBags().get(0);
var action = bag.getUpdateState().getAction();
if (action.startsWith("publish")) {
ingestDataverseIngestDeposit.updateProperties(Map.of(
"state.label", "PUBLISHED",
"state.description", "The dataset is published",
"identifier.doi", pid
)
);
try {
var nbn = dataverseService.getDatasetUrnNbn(pid);
ingestDataverseIngestDeposit.updateProperties(Map.of(
"state.label", "PUBLISHED",
"state.description", "The dataset is published",
"identifier.doi", pid,
"identifier.urn", nbn
)
);
} catch (IOException | DataverseException e) {
throw new RuntimeException("Error getting URN:NBN", e); // Cancelling the "success"
}
}
else if (action.equals("submit-for-review")) {
ingestDataverseIngestDeposit.updateProperties(Map.of(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public EditFiles composeEditFiles() {

editFiles.setAutoRenameFiles(getAutoRenamedFiles(renamedFiles));
editFiles.setAddRestrictedFiles(getRestrictedFilesToAdd(pathFileInfoMap));
editFiles.setAddUnrestrictedFiles(getUnrestrictedFilesToAdd(pathFileInfoMap));
editFiles.setUpdateFileMetas(getUpdatedFileMetas(pathFileInfoMap));

var filePathsToEmbargo = getEmbargoedFiles(pathFileInfoMap, dateAvailable);
Expand Down Expand Up @@ -95,20 +96,27 @@ protected List<String> getFilesToIgnore(Map<Path, FileInfo> files) {
* @param files the file infos found in files.xml
* @return a list of file paths that should be added as restricted files
*/
protected List<String> getRestrictedFilesToAdd(Map<Path, FileInfo> files) {
private List<String> getRestrictedFilesToAdd(Map<Path, FileInfo> files) {
return files.entrySet().stream()
.filter(entry -> entry.getValue().getMetadata().getRestricted())
.map(entry -> entry.getKey().toString())
.toList();
}

private List<String> getUnrestrictedFilesToAdd(Map<Path, FileInfo> files) {
return files.entrySet().stream()
.filter(entry -> !entry.getValue().getMetadata().getRestricted())
.map(entry -> entry.getKey().toString())
.toList();
}

/**
* Get the files that have metadata that should be updated.
*
* @param files the file infos found in files.xml
* @return a list of FileMetas that should be updated
*/
protected List<FileMeta> getUpdatedFileMetas(Map<Path, FileInfo> files) {
private List<FileMeta> getUpdatedFileMetas(Map<Path, FileInfo> files) {
return files.values().stream()
.map(FileInfo::getMetadata)
.filter(this::hasAttributes)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ public interface DataverseService {

void deleteFile(int id) throws DataverseException, IOException;

String getDatasetUrnNbn(String datasetId) throws IOException, DataverseException;

void waitForState(String persistentId, String state) throws DataverseException;

void updateMetadata(String targetDatasetPid, DatasetVersion datasetMetadata) throws DataverseException, IOException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import nl.knaw.dans.lib.dataverse.model.dataset.License;
import nl.knaw.dans.lib.dataverse.model.dataset.MetadataBlockSummary;
import nl.knaw.dans.lib.dataverse.model.dataset.MetadataField;
import nl.knaw.dans.lib.dataverse.model.dataset.PrimitiveSingleValueField;
import nl.knaw.dans.lib.dataverse.model.dataset.UpdateType;
import nl.knaw.dans.lib.dataverse.model.file.FileMeta;
import nl.knaw.dans.lib.dataverse.model.search.DatasetResultItem;
Expand Down Expand Up @@ -191,6 +192,20 @@ public List<String> findDoiByMetadataField(String key, String value) throws IOEx
.collect(Collectors.toList());
}

@Override
public String getDatasetUrnNbn(String pid) throws IOException, DataverseException {
var dataset = dataverseClient.dataset(pid);
var version = dataset.getVersion();
var data = version.getData();
var metadata = data.getMetadataBlocks().get("dansDataVaultMetadata");

return metadata.getFields().stream()
.filter(f -> f.getTypeName().equals("dansNbn"))
.map(f -> (PrimitiveSingleValueField) f)
.map(PrimitiveSingleValueField::getValue)
.findFirst().orElseThrow(() -> new IllegalStateException("No URN:NBN found in dataset"));
}

// TODO: move this to dans-dataverse-client-lib; it is similar to awaitLockState.
public void waitForState(String datasetId, String expectedState) {
var numberOfTimesTried = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
public class EditFilesComposerTest extends EditFilesComposerFixture {

@Test
public void adding_two_unrestricted_files_leaves_editFiles_empty() {
public void adding_two_unrestricted_files_adds_them_to_addUnrestrictedFiles() {
// Given
Map<Path, FileInfo> map = new HashMap<>();
add(map, file("file1.txt", "checksum1"));
Expand All @@ -41,8 +41,12 @@ public void adding_two_unrestricted_files_leaves_editFiles_empty() {
// When
var editFiles = editFilesComposer.composeEditFiles();

var unrestrictedFiles = editFiles.getAddUnrestrictedFiles();
assertThat(unrestrictedFiles).hasSize(2);
assertThat(unrestrictedFiles).contains("file1.txt", "file2.txt");

// Then
assertEmptyFieldsExcept(editFiles);
assertEmptyFieldsExcept(editFiles, "addUnrestrictedFiles");
}

@Test
Expand All @@ -62,7 +66,11 @@ public void adding_two_restricted_files_adds_them_to_addRestrictedFiles() {
assertThat(addRestrictedFiles).hasSize(2);
assertThat(addRestrictedFiles).contains("file1.txt", "file2.txt");

assertEmptyFieldsExcept(editFiles, "addRestrictedFiles");
var unrestrictedFiles = editFiles.getAddUnrestrictedFiles();
assertThat(unrestrictedFiles).hasSize(1);
assertThat(unrestrictedFiles).contains("file3.txt");

assertEmptyFieldsExcept(editFiles, "addRestrictedFiles", "addUnrestrictedFiles");
}

@Test
Expand All @@ -82,7 +90,11 @@ public void setting_description_adds_it_to_updateFileMetas() {
assertThat(updateFileMetas).hasSize(2);
assertThat(updateFileMetas).extracting(FileMeta::getDescription).contains("description1", "description2");

assertEmptyFieldsExcept(editFiles, "updateFileMetas");
var unrestrictedFiles = editFiles.getAddUnrestrictedFiles();
assertThat(unrestrictedFiles).hasSize(3);
assertThat(unrestrictedFiles).contains("file1.txt", "file2.txt", "file3.txt");

assertEmptyFieldsExcept(editFiles, "updateFileMetas", "addUnrestrictedFiles");
}

@Test
Expand All @@ -102,7 +114,11 @@ public void setting_categories_adds_them_to_updateFileMetas() {
assertThat(updateFileMetas).hasSize(2);
assertThat(updateFileMetas).extracting(FileMeta::getCategories).contains(List.of("category1"), List.of("category2"));

assertEmptyFieldsExcept(editFiles, "updateFileMetas");
var unrestrictedFiles = editFiles.getAddUnrestrictedFiles();
assertThat(unrestrictedFiles).hasSize(3);
assertThat(unrestrictedFiles).contains("file1.txt", "file2.txt", "file3.txt");

assertEmptyFieldsExcept(editFiles, "updateFileMetas", "addUnrestrictedFiles");
}

@Test
Expand All @@ -121,7 +137,11 @@ 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"));

assertEmptyFieldsExcept(editFiles, "addEmbargoes");
var unrestrictedFiles = editFiles.getAddUnrestrictedFiles();
assertThat(unrestrictedFiles).hasSize(2);
assertThat(unrestrictedFiles).contains("file1.txt", "file2.txt");

assertEmptyFieldsExcept(editFiles, "addEmbargoes", "addUnrestrictedFiles");
}

@Test
Expand All @@ -141,7 +161,11 @@ 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");

assertEmptyFieldsExcept(editFiles, "autoRenameFiles");
var unrestrictedFiles = editFiles.getAddUnrestrictedFiles();
assertThat(unrestrictedFiles).hasSize(2);
assertThat(unrestrictedFiles).contains("file1.txt", "file2.txt");

assertEmptyFieldsExcept(editFiles, "autoRenameFiles", "addUnrestrictedFiles");
}

@Test
Expand All @@ -155,8 +179,12 @@ public void fileExclusionPattern_ignores_files_matching_pattern() {
// When
var editFiles = editFilesComposer.composeEditFiles();

var addUnrestrictedFiles = editFiles.getAddUnrestrictedFiles();
assertThat(addUnrestrictedFiles).hasSize(1);
assertThat(addUnrestrictedFiles).contains("file2.txt");

// Then
assertEmptyFieldsExcept(editFiles);
assertEmptyFieldsExcept(editFiles, "addUnrestrictedFiles");
}

@Test
Expand All @@ -176,7 +204,11 @@ 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"));

assertEmptyFieldsExcept(editFiles, "addEmbargoes");
var unrestrictedFiles = editFiles.getAddUnrestrictedFiles();
assertThat(unrestrictedFiles).hasSize(3);
assertThat(unrestrictedFiles).contains("file1.txt", "file2.txt", "subdir/file3.txt");

assertEmptyFieldsExcept(editFiles, "addEmbargoes", "addUnrestrictedFiles");
}

}

0 comments on commit 18c7d93

Please sign in to comment.