Skip to content

Commit

Permalink
validate FileMetadatas along with dataset #3405
Browse files Browse the repository at this point in the history
This lets us return a "bad request" rather than a 500 error.
  • Loading branch information
pdurbin committed Oct 24, 2016
1 parent 8e4b749 commit 4e59b22
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 4 deletions.
22 changes: 22 additions & 0 deletions src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@
uniqueConstraints = @UniqueConstraint(columnNames = {"dataset_id,versionnumber,minorversionnumber"}))
public class DatasetVersion implements Serializable {

private static final Logger logger = Logger.getLogger(DatasetVersion.class.getCanonicalName());

/**
* Convenience comparator to compare dataset versions by their version number.
* The draft version is considered the latest.
Expand Down Expand Up @@ -1034,6 +1036,26 @@ public Set<ConstraintViolation> validate() {
}
}
}
List<FileMetadata> dsvfileMetadatas = this.getFileMetadatas();
if (dsvfileMetadatas != null) {
for (FileMetadata fileMetadata : dsvfileMetadatas) {
Set<ConstraintViolation<FileMetadata>> constraintViolations = validator.validate(fileMetadata);
if (constraintViolations.size() > 0) {
// currently only support one message
ConstraintViolation<FileMetadata> violation = constraintViolations.iterator().next();
/**
* @todo How can we expose this more detailed message
* containing the invalid value to the user?
*/
String message = "Constraint violation found in FileMetadata. "
+ violation.getMessage() + " "
+ "The invalid value is \"" + violation.getInvalidValue().toString() + "\".";
logger.info(message);
returnSet.add(violation);
break; // currently only support one message, so we can break out of the loop after the first constraint violation
}
}
}
return returnSet;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ public Dataset execute(CommandContext ctxt) throws CommandException {
String validationFailedString = "Validation failed:";
for (ConstraintViolation constraintViolation : constraintViolations) {
validationFailedString += " " + constraintViolation.getMessage();
validationFailedString += " Invalid value: '" + constraintViolation.getInvalidValue() + "'.";
}
throw new IllegalCommandException(validationFailedString, this);
}
Expand Down
13 changes: 9 additions & 4 deletions src/test/java/edu/harvard/iq/dataverse/api/FileMetadataIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,10 @@
import java.util.UUID;

import static com.jayway.restassured.RestAssured.given;
import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.fail;
import static org.hamcrest.CoreMatchers.equalTo;

/**
* FileMetadata Integration Tests
Expand Down Expand Up @@ -138,15 +140,18 @@ public void testJsonParserWithDirectoryLabels() {
try {

// try to create a dataset with directory labels that contain both leading and trailing file separators
Response resp = given()
Response shouldFailDueToLeadingAndTrailingSeparators = given()
.header(keyString, token)
.body(IOUtils.toString(classLoader.getResourceAsStream(
"json/complete-dataset-with-files-invalid-directory-labels.json")))
.contentType("application/json")
.post("/api/dataverses/" + testName + "/datasets");
// this request should fail since a @Pattern constraint exists in FileMetadata to prevent this
assertEquals(resp.getStatusCode(), 500);

shouldFailDueToLeadingAndTrailingSeparators.prettyPrint();
shouldFailDueToLeadingAndTrailingSeparators.then().assertThat()
// Note that the JSON under test actually exercises leading too but only the first (trailing) is exercised here.
.body("message", equalTo("Validation failed: Directory Name cannot contain leading or trailing file separators. Invalid value: 'data/subdir1/'."))
.statusCode(BAD_REQUEST.getStatusCode());

// create dataset and set id
dsId = given()
.header(keyString, token)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import java.util.List;

import static edu.harvard.iq.dataverse.mocks.MocksFactory.makeDataset;
import java.util.Set;
import javax.validation.ConstraintViolation;

/**
* Tests against IngestServiceBean helper methods.
Expand Down Expand Up @@ -367,4 +369,40 @@ public void testCheckForDuplicateFileNamesTabular() throws Exception {
assertEquals(file2NameAltered, true);
}

@Test
public void testDirectoryLabels() {

DatasetVersion datasetVersion = new DatasetVersion();
FileMetadata fileMetadata = new FileMetadata();
fileMetadata.setLabel("foo.png");
fileMetadata.setDirectoryLabel("/has/leading/slash");
datasetVersion.getFileMetadatas().add(fileMetadata);

Set<ConstraintViolation> violations1 = datasetVersion.validate();
assertEquals(1, violations1.size());
ConstraintViolation violation1 = violations1.iterator().next();
assertEquals("Directory Name cannot contain leading or trailing file separators.", violation1.getMessage());

// reset
datasetVersion.setFileMetadatas(new ArrayList<>());
Set<ConstraintViolation> violations2 = datasetVersion.validate();
assertEquals(0, violations2.size());

fileMetadata.setDirectoryLabel("has/trailing/slash/");
datasetVersion.getFileMetadatas().add(fileMetadata);
Set<ConstraintViolation> violations3 = datasetVersion.validate();
assertEquals(1, violations3.size());
assertEquals("Directory Name cannot contain leading or trailing file separators.", violations3.iterator().next().getMessage());

// reset
datasetVersion.setFileMetadatas(new ArrayList<>());
Set<ConstraintViolation> violations4 = datasetVersion.validate();
assertEquals(0, violations4.size());

fileMetadata.setDirectoryLabel("just/right");
datasetVersion.getFileMetadatas().add(fileMetadata);
Set<ConstraintViolation> violations5 = datasetVersion.validate();
assertEquals(0, violations5.size());
}

}

0 comments on commit 4e59b22

Please sign in to comment.