Skip to content

Commit

Permalink
Fix metadata field type display condition in dataverses/{id}/metadata…
Browse files Browse the repository at this point in the history 
…blocks API endpoint (#10642)

* Fixed: metadata field type display condition in dataverses/{id}/metadatablocks

* Changed: json object builder instantiation

* Added: extended test coverage for testUpdateInputLevels and testFeatureDataverse

* Added: release notes for #10637

* Fixed: JsonPrinter metadata blocks dataset field type isRequired logic

* Refactor: simpler conditions in jsonPrinter

* Refactor: reordered condition in jsonPrinter

* Fixed: displayCondition in jsonPrinter
  • Loading branch information
@GPortas
GPortas authored Jul 2, 2024
1 parent dddcf29 commit b42222f
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 35 deletions.
2 changes: 2 additions & 0 deletions doc/release-notes/10637-fix-dataverse-metadatablocks-api.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
dataverses/{id}/metadatablocks API endpoint has been fixed, since the fields returned for each metadata block when returnDatasetTypes query parameter is set to true was not correct.

18 changes: 12 additions & 6 deletions src/main/java/edu/harvard/iq/dataverse/Dataverse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -412,12 +412,18 @@ public List<DataverseFieldTypeInputLevel> getDataverseFieldTypeInputLevels() {
}

public boolean isDatasetFieldTypeRequiredAsInputLevel(Long datasetFieldTypeId) {
for(DataverseFieldTypeInputLevel dataverseFieldTypeInputLevel : dataverseFieldTypeInputLevels) {
if (dataverseFieldTypeInputLevel.getDatasetFieldType().getId().equals(datasetFieldTypeId) && dataverseFieldTypeInputLevel.isRequired()) {
return true;
}
}
return false;
return dataverseFieldTypeInputLevels.stream()
.anyMatch(inputLevel -> inputLevel.getDatasetFieldType().getId().equals(datasetFieldTypeId) && inputLevel.isRequired());
}

public boolean isDatasetFieldTypeIncludedAsInputLevel(Long datasetFieldTypeId) {
return dataverseFieldTypeInputLevels.stream()
.anyMatch(inputLevel -> inputLevel.getDatasetFieldType().getId().equals(datasetFieldTypeId) && inputLevel.isInclude());
}

public boolean isDatasetFieldTypeInInputLevels(Long datasetFieldTypeId) {
return dataverseFieldTypeInputLevels.stream()
.anyMatch(inputLevel -> inputLevel.getDatasetFieldType().getId().equals(datasetFieldTypeId));
}

public Template getDefaultTemplate() {
Expand Down
18 changes: 13 additions & 5 deletions src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -640,18 +640,26 @@ public static JsonObjectBuilder json(MetadataBlock metadataBlock, boolean printO

JsonObjectBuilder fieldsBuilder = Json.createObjectBuilder();
Set<DatasetFieldType> datasetFieldTypes = new TreeSet<>(metadataBlock.getDatasetFieldTypes());

for (DatasetFieldType datasetFieldType : datasetFieldTypes) {
boolean requiredInOwnerDataverse = ownerDataverse != null && ownerDataverse.isDatasetFieldTypeRequiredAsInputLevel(datasetFieldType.getId());
boolean displayCondition = !printOnlyDisplayedOnCreateDatasetFieldTypes ||
datasetFieldType.isDisplayOnCreate() ||
requiredInOwnerDataverse;
Long datasetFieldTypeId = datasetFieldType.getId();
boolean requiredAsInputLevelInOwnerDataverse = ownerDataverse != null && ownerDataverse.isDatasetFieldTypeRequiredAsInputLevel(datasetFieldTypeId);
boolean includedAsInputLevelInOwnerDataverse = ownerDataverse != null && ownerDataverse.isDatasetFieldTypeIncludedAsInputLevel(datasetFieldTypeId);
boolean isNotInputLevelInOwnerDataverse = ownerDataverse != null && !ownerDataverse.isDatasetFieldTypeInInputLevels(datasetFieldTypeId);

DatasetFieldType parentDatasetFieldType = datasetFieldType.getParentDatasetFieldType();
boolean isRequired = parentDatasetFieldType == null ? datasetFieldType.isRequired() : parentDatasetFieldType.isRequired();

boolean displayCondition = printOnlyDisplayedOnCreateDatasetFieldTypes
? (datasetFieldType.isDisplayOnCreate() || isRequired || requiredAsInputLevelInOwnerDataverse)
: ownerDataverse == null || includedAsInputLevelInOwnerDataverse || isNotInputLevelInOwnerDataverse;

if (displayCondition) {
fieldsBuilder.add(datasetFieldType.getName(), json(datasetFieldType, ownerDataverse));
}
}

jsonObjectBuilder.add("fields", fieldsBuilder);

return jsonObjectBuilder;
}

Expand Down
52 changes: 37 additions & 15 deletions src/test/java/edu/harvard/iq/dataverse/api/DataversesIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -702,8 +702,10 @@ public void testListMetadataBlocks() {
Response setMetadataBlocksResponse = UtilIT.setMetadataBlocks(dataverseAlias, Json.createArrayBuilder().add("citation").add("astrophysics"), apiToken);
setMetadataBlocksResponse.then().assertThat().statusCode(OK.getStatusCode());

String[] testInputLevelNames = {"geographicCoverage", "country"};
Response updateDataverseInputLevelsResponse = UtilIT.updateDataverseInputLevels(dataverseAlias, testInputLevelNames, apiToken);
String[] testInputLevelNames = {"geographicCoverage", "country", "city"};
boolean[] testRequiredInputLevels = {false, true, false};
boolean[] testIncludedInputLevels = {false, true, true};
Response updateDataverseInputLevelsResponse = UtilIT.updateDataverseInputLevels(dataverseAlias, testInputLevelNames, testRequiredInputLevels, testIncludedInputLevels, apiToken);
updateDataverseInputLevelsResponse.then().assertThat().statusCode(OK.getStatusCode());

// Dataverse not found
Expand Down Expand Up @@ -769,6 +771,21 @@ public void testListMetadataBlocks() {
assertThat(expectedAllMetadataBlockDisplayNames, hasItemInArray(actualMetadataBlockDisplayName2));
assertThat(expectedAllMetadataBlockDisplayNames, hasItemInArray(actualMetadataBlockDisplayName3));

// Check dataset fields for the updated input levels are retrieved
int geospatialMetadataBlockIndex = actualMetadataBlockDisplayName1.equals("Geospatial Metadata") ? 0 : actualMetadataBlockDisplayName2.equals("Geospatial Metadata") ? 1 : 2;

// Since the included property of geographicCoverage is set to false, we should retrieve the total number of fields minus one
listMetadataBlocksResponse.then().assertThat()
.body(String.format("data[%d].fields.size()", geospatialMetadataBlockIndex), equalTo(10));

String actualMetadataField1 = listMetadataBlocksResponse.then().extract().path(String.format("data[%d].fields.geographicCoverage.name", geospatialMetadataBlockIndex));
String actualMetadataField2 = listMetadataBlocksResponse.then().extract().path(String.format("data[%d].fields.country.name", geospatialMetadataBlockIndex));
String actualMetadataField3 = listMetadataBlocksResponse.then().extract().path(String.format("data[%d].fields.city.name", geospatialMetadataBlockIndex));

assertNull(actualMetadataField1);
assertNotNull(actualMetadataField2);
assertNotNull(actualMetadataField3);

// Existent dataverse and onlyDisplayedOnCreate=true and returnDatasetFieldTypes=true
listMetadataBlocksResponse = UtilIT.listMetadataBlocks(dataverseAlias, true, true, apiToken);
listMetadataBlocksResponse.then().assertThat().statusCode(OK.getStatusCode());
Expand All @@ -785,16 +802,18 @@ public void testListMetadataBlocks() {
assertThat(expectedOnlyDisplayedOnCreateMetadataBlockDisplayNames, hasItemInArray(actualMetadataBlockDisplayName2));

// Check dataset fields for the updated input levels are retrieved
int geospatialMetadataBlockIndex = actualMetadataBlockDisplayName2.equals("Geospatial Metadata") ? 1 : 0;
geospatialMetadataBlockIndex = actualMetadataBlockDisplayName2.equals("Geospatial Metadata") ? 1 : 0;

listMetadataBlocksResponse.then().assertThat()
.body(String.format("data[%d].fields.size()", geospatialMetadataBlockIndex), equalTo(2));
.body(String.format("data[%d].fields.size()", geospatialMetadataBlockIndex), equalTo(1));

String actualMetadataField1 = listMetadataBlocksResponse.then().extract().path(String.format("data[%d].fields.geographicCoverage.name", geospatialMetadataBlockIndex));
String actualMetadataField2 = listMetadataBlocksResponse.then().extract().path(String.format("data[%d].fields.country.name", geospatialMetadataBlockIndex));
actualMetadataField1 = listMetadataBlocksResponse.then().extract().path(String.format("data[%d].fields.geographicCoverage.name", geospatialMetadataBlockIndex));
actualMetadataField2 = listMetadataBlocksResponse.then().extract().path(String.format("data[%d].fields.country.name", geospatialMetadataBlockIndex));
actualMetadataField3 = listMetadataBlocksResponse.then().extract().path(String.format("data[%d].fields.city.name", geospatialMetadataBlockIndex));

assertNotNull(actualMetadataField1);
assertNull(actualMetadataField1);
assertNotNull(actualMetadataField2);
assertNull(actualMetadataField3);

// User has no permissions on the requested dataverse
Response createSecondUserResponse = UtilIT.createRandomUser();
Expand Down Expand Up @@ -898,12 +917,16 @@ public void testUpdateInputLevels() {

// Update valid input levels
String[] testInputLevelNames = {"geographicCoverage", "country"};
Response updateDataverseInputLevelsResponse = UtilIT.updateDataverseInputLevels(dataverseAlias, testInputLevelNames, apiToken);
boolean[] testRequiredInputLevels = {true, false};
boolean[] testIncludedInputLevels = {true, false};
Response updateDataverseInputLevelsResponse = UtilIT.updateDataverseInputLevels(dataverseAlias, testInputLevelNames, testRequiredInputLevels, testIncludedInputLevels, apiToken);
String actualInputLevelName = updateDataverseInputLevelsResponse.then().extract().path("data.inputLevels[0].datasetFieldTypeName");
int geographicCoverageInputLevelIndex = actualInputLevelName.equals("geographicCoverage") ? 0 : 1;
updateDataverseInputLevelsResponse.then().assertThat()
.body("data.inputLevels[0].required", equalTo(true))
.body("data.inputLevels[0].include", equalTo(true))
.body("data.inputLevels[1].required", equalTo(true))
.body("data.inputLevels[1].include", equalTo(true))
.body(String.format("data.inputLevels[%d].include", geographicCoverageInputLevelIndex), equalTo(true))
.body(String.format("data.inputLevels[%d].required", geographicCoverageInputLevelIndex), equalTo(true))
.body(String.format("data.inputLevels[%d].include", 1 - geographicCoverageInputLevelIndex), equalTo(false))
.body(String.format("data.inputLevels[%d].required", 1 - geographicCoverageInputLevelIndex), equalTo(false))
.statusCode(OK.getStatusCode());
String actualFieldTypeName1 = updateDataverseInputLevelsResponse.then().extract().path("data.inputLevels[0].datasetFieldTypeName");
String actualFieldTypeName2 = updateDataverseInputLevelsResponse.then().extract().path("data.inputLevels[1].datasetFieldTypeName");
Expand All @@ -913,15 +936,14 @@ public void testUpdateInputLevels() {

// Update input levels with an invalid field type name
String[] testInvalidInputLevelNames = {"geographicCoverage", "invalid1"};
updateDataverseInputLevelsResponse = UtilIT.updateDataverseInputLevels(dataverseAlias, testInvalidInputLevelNames, apiToken);
updateDataverseInputLevelsResponse = UtilIT.updateDataverseInputLevels(dataverseAlias, testInvalidInputLevelNames, testRequiredInputLevels, testIncludedInputLevels, apiToken);
updateDataverseInputLevelsResponse.then().assertThat()
.body("message", equalTo("Invalid dataset field type name: invalid1"))
.statusCode(BAD_REQUEST.getStatusCode());

// Update invalid empty input levels
testInputLevelNames = new String[]{};
updateDataverseInputLevelsResponse = UtilIT.updateDataverseInputLevels(dataverseAlias, testInputLevelNames, apiToken);
updateDataverseInputLevelsResponse.prettyPrint();
updateDataverseInputLevelsResponse = UtilIT.updateDataverseInputLevels(dataverseAlias, testInputLevelNames, testRequiredInputLevels, testIncludedInputLevels, apiToken);
updateDataverseInputLevelsResponse.then().assertThat()
.body("message", equalTo("Error while updating dataverse input levels: Input level list cannot be null or empty"))
.statusCode(INTERNAL_SERVER_ERROR.getStatusCode());
Expand Down
21 changes: 12 additions & 9 deletions src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3960,22 +3960,25 @@ static Response requestGlobusUploadPaths(Integer datasetId, JsonObject body, Str
.post("/api/datasets/" + datasetId + "/requestGlobusUploadPaths");
}

static Response updateDataverseInputLevels(String dataverseAlias, String[] inputLevelNames, String apiToken) {
JsonArrayBuilder contactArrayBuilder = Json.createArrayBuilder();
for(String inputLevelName : inputLevelNames) {
contactArrayBuilder.add(Json.createObjectBuilder()
.add("datasetFieldTypeName", inputLevelName)
.add("required", true)
.add("include", true)
);
public static Response updateDataverseInputLevels(String dataverseAlias, String[] inputLevelNames, boolean[] requiredInputLevels, boolean[] includedInputLevels, String apiToken) {
JsonArrayBuilder inputLevelsArrayBuilder = Json.createArrayBuilder();
for (int i = 0; i < inputLevelNames.length; i++) {
inputLevelsArrayBuilder.add(createInputLevelObject(inputLevelNames[i], requiredInputLevels[i], includedInputLevels[i]));
}
return given()
.header(API_TOKEN_HTTP_HEADER, apiToken)
.body(contactArrayBuilder.build().toString())
.body(inputLevelsArrayBuilder.build().toString())
.contentType(ContentType.JSON)
.put("/api/dataverses/" + dataverseAlias + "/inputLevels");
}

private static JsonObjectBuilder createInputLevelObject(String name, boolean required, boolean include) {
return Json.createObjectBuilder()
.add("datasetFieldTypeName", name)
.add("required", required)
.add("include", include);
}

public static Response getOpenAPI(String accept, String format) {
Response response = given()
.header("Accept", accept)
Expand Down

0 comments on commit b42222f

Please sign in to comment.