Skip to content

Commit ba0183d

Browse files
kcondonkcondon
authored
Merge pull request #8792 from adaybujeda/8787-improve-error-messages-bagit-handler
BagIt Support - Improve error messages for BagIt handler
2 parents 94e15c1 + d3a15b4 commit ba0183d

15 files changed

Lines changed: 204 additions & 72 deletions

‎src/main/java/edu/harvard/iq/dataverse/EditDataFilesPageHelper.java‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22

33
import edu.harvard.iq.dataverse.util.BundleUtil;
44
import edu.harvard.iq.dataverse.util.file.CreateDataFileResult;
5+
import org.apache.commons.text.StringEscapeUtils;
56

67
import javax.ejb.Stateless;
78
import javax.inject.Inject;
9+
import java.util.Arrays;
810
import java.util.List;
911
import java.util.Optional;
1012
import java.util.stream.Collectors;
@@ -22,6 +24,14 @@ public class EditDataFilesPageHelper {
2224
@Inject
2325
private SettingsWrapper settingsWrapper;
2426

27+
public String consolidateHtmlErrorMessages(List<String> errorMessages) {
28+
if(errorMessages == null || errorMessages.isEmpty()) {
29+
return null;
30+
}
31+
32+
return String.join("</br>", errorMessages);
33+
}
34+
2535
public String getHtmlErrorMessage(CreateDataFileResult createDataFileResult) {
2636
List<String> errors = createDataFileResult.getErrors();
2737
if(errors == null || errors.isEmpty()) {
@@ -33,8 +43,8 @@ public String getHtmlErrorMessage(CreateDataFileResult createDataFileResult) {
3343
return null;
3444
}
3545

36-
String typeMessage = Optional.ofNullable(BundleUtil.getStringFromBundle(createDataFileResult.getBundleKey())).orElse("Error processing file");
37-
String errorsMessage = errors.stream().limit(maxErrorsToShow).map(text -> String.format("<li>%s</li>", text)).collect(Collectors.joining());
38-
return String.format("%s:<br /><ul>%s</ul>", typeMessage, errorsMessage);
46+
String typeMessage = Optional.ofNullable(BundleUtil.getStringFromBundle(createDataFileResult.getBundleKey(), Arrays.asList(createDataFileResult.getFilename()))).orElse("Error processing file");
47+
String errorsMessage = errors.stream().limit(maxErrorsToShow).map(text -> String.format("<li>%s</li>", StringEscapeUtils.escapeHtml4(text))).collect(Collectors.joining());
48+
return String.format("%s<br /><ul>%s</ul>", typeMessage, errorsMessage);
3949
}
4050
}

‎src/main/java/edu/harvard/iq/dataverse/EditDatafilesPage.java‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import java.util.Iterator;
5252
import java.util.List;
5353
import java.util.Map;
54+
import java.util.Optional;
5455
import java.util.logging.Logger;
5556
import javax.ejb.EJB;
5657
import javax.ejb.EJBException;
@@ -1491,7 +1492,7 @@ public void handleDropBoxUpload(ActionEvent event) {
14911492
//datafiles = ingestService.createDataFiles(workingVersion, dropBoxStream, fileName, "application/octet-stream");
14921493
CreateDataFileResult createDataFilesResult = FileUtil.createDataFiles(workingVersion, dropBoxStream, fileName, "application/octet-stream", null, null, systemConfig);
14931494
datafiles = createDataFilesResult.getDataFiles();
1494-
errorMessage = editDataFilesPageHelper.getHtmlErrorMessage(createDataFilesResult);
1495+
Optional.ofNullable(editDataFilesPageHelper.getHtmlErrorMessage(createDataFilesResult)).ifPresent(errorMessage -> errorMessages.add(errorMessage));
14951496

14961497
} catch (IOException ex) {
14971498
this.logger.log(Level.SEVERE, "Error during ingest of DropBox file {0} from link {1}", new Object[]{fileName, fileLink});
@@ -1745,12 +1746,13 @@ public void uploadFinished() {
17451746
uploadedFiles.clear();
17461747
uploadInProgress.setValue(false);
17471748
}
1748-
if(errorMessage != null) {
1749-
FacesContext.getCurrentInstance().addMessage(null, new FacesMessage(FacesMessage.SEVERITY_ERROR, BundleUtil.getStringFromBundle("dataset.file.uploadFailure"), errorMessage));
1750-
PrimeFaces.current().ajax().update(":messagePanel");
1751-
}
1749+
17521750
// refresh the warning message below the upload component, if exists:
17531751
if (uploadComponentId != null) {
1752+
if(!errorMessages.isEmpty()) {
1753+
FacesContext.getCurrentInstance().addMessage(uploadComponentId, new FacesMessage(FacesMessage.SEVERITY_ERROR, BundleUtil.getStringFromBundle("dataset.file.uploadFailure"), editDataFilesPageHelper.consolidateHtmlErrorMessages(errorMessages)));
1754+
}
1755+
17541756
if (uploadWarningMessage != null) {
17551757
if (existingFilesWithDupeContent != null || newlyUploadedFilesWithDupeContent != null) {
17561758
setWarningMessageForAlreadyExistsPopUp(uploadWarningMessage);
@@ -1797,7 +1799,7 @@ public void uploadFinished() {
17971799
multipleDupesNew = false;
17981800
uploadWarningMessage = null;
17991801
uploadSuccessMessage = null;
1800-
errorMessage = null;
1802+
errorMessages = new ArrayList<>();
18011803
}
18021804

18031805
private String warningMessageForFileTypeDifferentPopUp;
@@ -1948,7 +1950,7 @@ private void handleReplaceFileUpload(String fullStorageLocation,
19481950
}
19491951

19501952
private String uploadWarningMessage = null;
1951-
private String errorMessage = null;
1953+
private List<String> errorMessages = new ArrayList<>();
19521954
private String uploadSuccessMessage = null;
19531955
private String uploadComponentId = null;
19541956

@@ -2020,7 +2022,11 @@ public void handleFileUpload(FileUploadEvent event) throws IOException {
20202022
// zip file.
20212023
CreateDataFileResult createDataFilesResult = FileUtil.createDataFiles(workingVersion, uFile.getInputStream(), uFile.getFileName(), uFile.getContentType(), null, null, systemConfig);
20222024
dFileList = createDataFilesResult.getDataFiles();
2023-
errorMessage = editDataFilesPageHelper.getHtmlErrorMessage(createDataFilesResult);
2025+
String createDataFilesError = editDataFilesPageHelper.getHtmlErrorMessage(createDataFilesResult);
2026+
if(createDataFilesError != null) {
2027+
errorMessages.add(createDataFilesError);
2028+
uploadComponentId = event.getComponent().getClientId();
2029+
}
20242030

20252031
} catch (IOException ioex) {
20262032
logger.warning("Failed to process and/or save the file " + uFile.getFileName() + "; " + ioex.getMessage());
@@ -2127,7 +2133,7 @@ public void handleExternalUpload() {
21272133
//datafiles = ingestService.createDataFiles(workingVersion, dropBoxStream, fileName, "application/octet-stream");
21282134
CreateDataFileResult createDataFilesResult = FileUtil.createDataFiles(workingVersion, null, fileName, contentType, fullStorageIdentifier, checksumValue, checksumType, systemConfig);
21292135
datafiles = createDataFilesResult.getDataFiles();
2130-
errorMessage = editDataFilesPageHelper.getHtmlErrorMessage(createDataFilesResult);
2136+
Optional.ofNullable(editDataFilesPageHelper.getHtmlErrorMessage(createDataFilesResult)).ifPresent(errorMessage -> errorMessages.add(errorMessage));
21312137
} catch (IOException ex) {
21322138
logger.log(Level.SEVERE, "Error during ingest of file {0}", new Object[]{fileName});
21332139
}

‎src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -878,7 +878,7 @@ public static CreateDataFileResult createDataFiles(DatasetVersion version, Input
878878
}
879879

880880
datafiles.add(datafile);
881-
return CreateDataFileResult.success(finalType, datafiles);
881+
return CreateDataFileResult.success(fileName, finalType, datafiles);
882882
}
883883

884884
// If it's a ZIP file, we are going to unpack it and create multiple
@@ -1054,7 +1054,7 @@ public static CreateDataFileResult createDataFiles(DatasetVersion version, Input
10541054
logger.warning("Could not remove temp file " + tempFile.getFileName().toString());
10551055
}
10561056
// and return:
1057-
return CreateDataFileResult.success(finalType, datafiles);
1057+
return CreateDataFileResult.success(fileName, finalType, datafiles);
10581058
}
10591059

10601060
} else if (finalType.equalsIgnoreCase(ShapefileHandler.SHAPEFILE_FILE_TYPE)) {
@@ -1070,7 +1070,7 @@ public static CreateDataFileResult createDataFiles(DatasetVersion version, Input
10701070
boolean didProcessWork = shpIngestHelper.processFile();
10711071
if (!(didProcessWork)) {
10721072
logger.severe("Processing of zipped shapefile failed.");
1073-
return CreateDataFileResult.error(finalType);
1073+
return CreateDataFileResult.error(fileName, finalType);
10741074
}
10751075

10761076
try {
@@ -1131,11 +1131,11 @@ public static CreateDataFileResult createDataFiles(DatasetVersion version, Input
11311131
logger.warning("Unable to delete: " + tempFile.toString() + "due to Security Exception: "
11321132
+ se.getMessage());
11331133
}
1134-
return CreateDataFileResult.success(finalType, datafiles);
1134+
return CreateDataFileResult.success(fileName, finalType, datafiles);
11351135
} else {
11361136
logger.severe("No files added from directory of rezipped shapefiles");
11371137
}
1138-
return CreateDataFileResult.error(finalType);
1138+
return CreateDataFileResult.error(fileName, finalType);
11391139

11401140
} else if (finalType.equalsIgnoreCase(BagItFileHandler.FILE_TYPE)) {
11411141
Optional<BagItFileHandler> bagItFileHandler = CDI.current().select(BagItFileHandlerFactory.class).get().getBagItFileHandler();
@@ -1178,10 +1178,10 @@ public static CreateDataFileResult createDataFiles(DatasetVersion version, Input
11781178
}
11791179
datafiles.add(datafile);
11801180

1181-
return CreateDataFileResult.success(finalType, datafiles);
1181+
return CreateDataFileResult.success(fileName, finalType, datafiles);
11821182
}
11831183

1184-
return CreateDataFileResult.error(finalType);
1184+
return CreateDataFileResult.error(fileName, finalType);
11851185
} // end createDataFiles
11861186

11871187

‎src/main/java/edu/harvard/iq/dataverse/util/bagit/BagValidation.java‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@
33
import java.nio.file.Path;
44
import java.util.Collections;
55
import java.util.LinkedHashMap;
6+
import java.util.List;
67
import java.util.Map;
78
import java.util.Optional;
9+
import java.util.stream.Collectors;
10+
import java.util.stream.Stream;
811

912
/**
1013
*
@@ -16,7 +19,7 @@ public class BagValidation {
1619
private final Map<Path, FileValidationResult> fileResults;
1720

1821
public BagValidation(Optional<String> errorMessage) {
19-
this.errorMessage = errorMessage;
22+
this.errorMessage = errorMessage == null ? Optional.empty() : errorMessage;
2023
this.fileResults = new LinkedHashMap<>();
2124
}
2225

@@ -34,6 +37,12 @@ public Map<Path, FileValidationResult> getFileResults() {
3437
return Collections.unmodifiableMap(fileResults);
3538
}
3639

40+
public List<String> getAllErrors() {
41+
Stream<String> mainError = getErrorMessage().stream();
42+
Stream<String> fileErrors = getFileResults().values().stream().filter(result -> result.isError()).map(result -> result.getMessage());
43+
return Stream.concat(mainError, fileErrors).collect(Collectors.toList());
44+
}
45+
3746
public long errors() {
3847
return fileResults.values().stream().filter(result -> result.isError()).count();
3948
}
@@ -76,8 +85,9 @@ public void setSuccess() {
7685
this.status = Status.SUCCESS;
7786
}
7887

79-
public void setError() {
88+
public void setError(String message) {
8089
this.status = Status.ERROR;
90+
this.message = message;
8191
}
8292

8393
public boolean isPending() {
@@ -92,10 +102,6 @@ public boolean isError() {
92102
return status.equals(Status.ERROR);
93103
}
94104

95-
public void setMessage(String message) {
96-
this.message = message;
97-
}
98-
99105
public String getMessage() {
100106
return message;
101107
}

‎src/main/java/edu/harvard/iq/dataverse/util/bagit/BagValidator.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,7 @@ private BagValidation validateChecksums(FileDataProvider fileDataProvider, Manif
118118
FileChecksumValidationJob validationJob = new FileChecksumValidationJob(inputStreamProvider.get(), filePath, fileChecksum, manifestChecksums.getType(), fileValidationResult);
119119
executor.execute(validationJob);
120120
} else {
121-
fileValidationResult.setError();
122-
fileValidationResult.setMessage(getMessage("bagit.validation.file.not.found", filePath, fileDataProvider.getName()));
121+
fileValidationResult.setError(getMessage("bagit.validation.file.not.found", filePath, fileDataProvider.getName()));
123122
}
124123

125124
}
@@ -148,7 +147,8 @@ ExecutorService getExecutorService() {
148147
return Executors.newFixedThreadPool(validatorJobPoolSize);
149148
}
150149

151-
private String getMessage(String propertyKey, Object... parameters){
150+
// Visible for testing
151+
String getMessage(String propertyKey, Object... parameters){
152152
List<String> parameterList = Arrays.stream(parameters).map(param -> param.toString()).collect(Collectors.toList());
153153
return BundleUtil.getStringFromBundle(propertyKey, parameterList);
154154
}

‎src/main/java/edu/harvard/iq/dataverse/util/bagit/FileChecksumValidationJob.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,10 @@ public void run() {
4343
if (fileChecksum.equals(calculatedChecksum)) {
4444
result.setSuccess();
4545
} else {
46-
result.setError();
47-
result.setMessage(getMessage("bagit.checksum.validation.error", filePath, bagChecksumType, fileChecksum, calculatedChecksum));
46+
result.setError(getMessage("bagit.checksum.validation.error", filePath, bagChecksumType, fileChecksum, calculatedChecksum));
4847
}
4948
} catch (Exception e) {
50-
result.setError();
51-
result.setMessage(getMessage("bagit.checksum.validation.exception", filePath, bagChecksumType, e.getMessage()));
49+
result.setError(getMessage("bagit.checksum.validation.exception", filePath, bagChecksumType, e.getMessage()));
5250
logger.log(Level.WARNING, String.format("action=validate-checksum result=error filePath=%s type=%s", filePath, bagChecksumType), e);
5351
} finally {
5452
IOUtils.closeQuietly(inputStream);

‎src/main/java/edu/harvard/iq/dataverse/util/file/BagItFileHandler.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import java.util.List;
2222
import java.util.Optional;
2323
import java.util.logging.Logger;
24-
import java.util.stream.Collectors;
2524

2625
/**
2726
*
@@ -58,25 +57,25 @@ public CreateDataFileResult handleBagItPackage(SystemConfig systemConfig, Datase
5857
try {
5958
List<DataFile> packageDataFiles = processBagItPackage(systemConfig, datasetVersion, uploadedFilename, bagItPackageFile);
6059
if(packageDataFiles.isEmpty()) {
61-
return CreateDataFileResult.error(FILE_TYPE, Collections.emptyList());
60+
return CreateDataFileResult.error(uploadedFilename, FILE_TYPE, Collections.emptyList());
6261
}
6362

6463
BagValidation bagValidation = validateBagItPackage(uploadedFilename, packageDataFiles);
6564
if(bagValidation.success()) {
6665
List<DataFile> finalItems = postProcessor.process(packageDataFiles);
6766
logger.info(String.format("action=handleBagItPackage result=success uploadedFilename=%s file=%s", uploadedFilename, bagItPackageFile.getName()));
68-
return CreateDataFileResult.success(FILE_TYPE, finalItems);
67+
return CreateDataFileResult.success(uploadedFilename, FILE_TYPE, finalItems);
6968
}
7069

7170
// BagIt package has errors
7271
// Capture errors and return to caller
73-
List<String> errors = bagValidation.getFileResults().values().stream().filter(result -> result.isError()).map(result -> result.getMessage()).collect(Collectors.toList());
72+
List<String> errors = bagValidation.getAllErrors();
7473
logger.info(String.format("action=handleBagItPackage result=errors uploadedFilename=%s file=%s errors=%s", uploadedFilename, bagItPackageFile.getName(), errors.size()));
75-
return CreateDataFileResult.error(FILE_TYPE, errors);
74+
return CreateDataFileResult.error(uploadedFilename, FILE_TYPE, errors);
7675

7776
} catch (BagItFileHandlerException e) {
7877
logger.severe(String.format("action=handleBagItPackage result=error uploadedFilename=%s file=%s message=%s", uploadedFilename, bagItPackageFile.getName(), e.getMessage()));
79-
return CreateDataFileResult.error(FILE_TYPE, Arrays.asList(e.getMessage()));
78+
return CreateDataFileResult.error(uploadedFilename, FILE_TYPE, Arrays.asList(e.getMessage()));
8079
} finally {
8180
fileUtil.deleteFile(bagItPackageFile.toPath());
8281
}

‎src/main/java/edu/harvard/iq/dataverse/util/file/CreateDataFileResult.java‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,26 +13,32 @@ public class CreateDataFileResult {
1313

1414
private static final String BUNDLE_KEY_PREFIX = "dataset.file.error";
1515

16+
private final String filename;
1617
private final String type;
1718
private final List<DataFile> dataFiles;
1819
private final List<String> errors;
1920

20-
public CreateDataFileResult(String type, List<DataFile> dataFiles, List<String> errors) {
21+
public CreateDataFileResult(String filename, String type, List<DataFile> dataFiles, List<String> errors) {
22+
this.filename = filename;
2123
this.type = type;
2224
this.dataFiles = dataFiles == null ? null : Collections.unmodifiableList(dataFiles);
2325
this.errors = errors == null ? Collections.emptyList() : Collections.unmodifiableList(errors);
2426
}
2527

26-
public static CreateDataFileResult success(String type, List<DataFile> dataFiles) {
27-
return new CreateDataFileResult(type, dataFiles, null);
28+
public static CreateDataFileResult success(String filename, String type, List<DataFile> dataFiles) {
29+
return new CreateDataFileResult(filename, type, dataFiles, null);
2830
}
2931

30-
public static CreateDataFileResult error(String type) {
31-
return new CreateDataFileResult(type, null, Collections.emptyList());
32+
public static CreateDataFileResult error(String filename, String type) {
33+
return new CreateDataFileResult(filename, type, null, Collections.emptyList());
3234
}
3335

34-
public static CreateDataFileResult error(String type, List<String> errors) {
35-
return new CreateDataFileResult(type, null, errors);
36+
public static CreateDataFileResult error(String filename, String type, List<String> errors) {
37+
return new CreateDataFileResult(filename, type, null, errors);
38+
}
39+
40+
public String getFilename() {
41+
return filename;
3642
}
3743

3844
public String getType() {

‎src/main/java/propertyFiles/Bundle.properties‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2270,12 +2270,12 @@ bagit.sourceOrganization=Dataverse Installation (<Site Url>)
22702270
bagit.sourceOrganizationAddress=<Full address>
22712271
bagit.sourceOrganizationEmail=<Email address>
22722272

2273-
bagit.checksum.validation.error=Invalid checksum. filePath={0} type={1} fileChecksum={2} calculatedChecksum={3}
2274-
bagit.checksum.validation.exception=Error while calculating checksum. filePath={0} type={1} error={2}
2275-
bagit.validation.bag.file.not.found=Invalid bag file: {0}
2276-
bagit.validation.manifest.not.supported=No supported manifest found in: {0} supportedTypes: {1}
2277-
bagit.validation.file.not.found=Manifest declared file: {0} not-found in data provider: {1}
2278-
bagit.validation.exception=Unable to complete checksums for: {0}
2273+
bagit.checksum.validation.error=Invalid checksum for file "{0}". Manifest checksum={2}, calculated checksum={3}, type={1}
2274+
bagit.checksum.validation.exception=Error while calculating checksum for file "{0}". Checksum type={1}, error={2}
2275+
bagit.validation.bag.file.not.found=Invalid BagIt package: "{0}"
2276+
bagit.validation.manifest.not.supported=No supported manifest found in BagIt package. Supported types are: {1}
2277+
bagit.validation.file.not.found=The manifest declared a file, "{0}", that is not found in the BagIt package
2278+
bagit.validation.exception=Unable to complete checksums for BagIt package
22792279

22802280
#Permission.java
22812281
permission.addDataverseDataverse=Add a dataverse within another dataverse
@@ -2348,7 +2348,7 @@ dataset.file.uploadWarning=upload warning
23482348
dataset.file.uploadWorked=upload worked
23492349
dataset.file.upload.popup.explanation.tip=For more information, please refer to the <a href="{0}/{1}/user/dataset-management.html#duplicate-files" title="Duplicate Files - Dataverse User Guide" target="_blank" rel="noopener">Duplicate Files section of the User Guide</a>.
23502350

2351-
dataset.file.error.application/zipped-bagit=BagIt package detected, but errors found. These are the errors found until processing stopped
2351+
dataset.file.error.application/zipped-bagit=BagIt package, "{0}", detected but errors found. These are the errors found until processing stopped:
23522352

23532353
#HarvestingClientsPage.java
23542354
harvest.start.error=Sorry, harvest could not be started for the selected harvesting client configuration (unknown server error).

0 commit comments

Comments
 (0)
⚡