Skip to content

Commit b42222f

Browse files
GPortasGPortas
authored
Fix metadata field type display condition in dataverses/{id}/metadatablocks 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
1 parent dddcf29 commit b42222f

File tree

5 files changed

+76
-35
lines changed

5 files changed

+76
-35
lines changed

‎doc/release-notes/10637-fix-dataverse-metadatablocks-api.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
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.
2+

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -412,12 +412,18 @@ public List<DataverseFieldTypeInputLevel> getDataverseFieldTypeInputLevels() {
412412
}
413413

414414
public boolean isDatasetFieldTypeRequiredAsInputLevel(Long datasetFieldTypeId) {
415-
for(DataverseFieldTypeInputLevel dataverseFieldTypeInputLevel : dataverseFieldTypeInputLevels) {
416-
if (dataverseFieldTypeInputLevel.getDatasetFieldType().getId().equals(datasetFieldTypeId) && dataverseFieldTypeInputLevel.isRequired()) {
417-
return true;
418-
}
419-
}
420-
return false;
415+
return dataverseFieldTypeInputLevels.stream()
416+
.anyMatch(inputLevel -> inputLevel.getDatasetFieldType().getId().equals(datasetFieldTypeId) && inputLevel.isRequired());
417+
}
418+
419+
public boolean isDatasetFieldTypeIncludedAsInputLevel(Long datasetFieldTypeId) {
420+
return dataverseFieldTypeInputLevels.stream()
421+
.anyMatch(inputLevel -> inputLevel.getDatasetFieldType().getId().equals(datasetFieldTypeId) && inputLevel.isInclude());
422+
}
423+
424+
public boolean isDatasetFieldTypeInInputLevels(Long datasetFieldTypeId) {
425+
return dataverseFieldTypeInputLevels.stream()
426+
.anyMatch(inputLevel -> inputLevel.getDatasetFieldType().getId().equals(datasetFieldTypeId));
421427
}
422428

423429
public Template getDefaultTemplate() {

‎src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -640,18 +640,26 @@ public static JsonObjectBuilder json(MetadataBlock metadataBlock, boolean printO
640640

641641
JsonObjectBuilder fieldsBuilder = Json.createObjectBuilder();
642642
Set<DatasetFieldType> datasetFieldTypes = new TreeSet<>(metadataBlock.getDatasetFieldTypes());
643+
643644
for (DatasetFieldType datasetFieldType : datasetFieldTypes) {
644-
boolean requiredInOwnerDataverse = ownerDataverse != null && ownerDataverse.isDatasetFieldTypeRequiredAsInputLevel(datasetFieldType.getId());
645-
boolean displayCondition = !printOnlyDisplayedOnCreateDatasetFieldTypes ||
646-
datasetFieldType.isDisplayOnCreate() ||
647-
requiredInOwnerDataverse;
645+
Long datasetFieldTypeId = datasetFieldType.getId();
646+
boolean requiredAsInputLevelInOwnerDataverse = ownerDataverse != null && ownerDataverse.isDatasetFieldTypeRequiredAsInputLevel(datasetFieldTypeId);
647+
boolean includedAsInputLevelInOwnerDataverse = ownerDataverse != null && ownerDataverse.isDatasetFieldTypeIncludedAsInputLevel(datasetFieldTypeId);
648+
boolean isNotInputLevelInOwnerDataverse = ownerDataverse != null && !ownerDataverse.isDatasetFieldTypeInInputLevels(datasetFieldTypeId);
649+
650+
DatasetFieldType parentDatasetFieldType = datasetFieldType.getParentDatasetFieldType();
651+
boolean isRequired = parentDatasetFieldType == null ? datasetFieldType.isRequired() : parentDatasetFieldType.isRequired();
652+
653+
boolean displayCondition = printOnlyDisplayedOnCreateDatasetFieldTypes
654+
? (datasetFieldType.isDisplayOnCreate() || isRequired || requiredAsInputLevelInOwnerDataverse)
655+
: ownerDataverse == null || includedAsInputLevelInOwnerDataverse || isNotInputLevelInOwnerDataverse;
656+
648657
if (displayCondition) {
649658
fieldsBuilder.add(datasetFieldType.getName(), json(datasetFieldType, ownerDataverse));
650659
}
651660
}
652661

653662
jsonObjectBuilder.add("fields", fieldsBuilder);
654-
655663
return jsonObjectBuilder;
656664
}
657665

‎src/test/java/edu/harvard/iq/dataverse/api/DataversesIT.java‎

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -702,8 +702,10 @@ public void testListMetadataBlocks() {
702702
Response setMetadataBlocksResponse = UtilIT.setMetadataBlocks(dataverseAlias, Json.createArrayBuilder().add("citation").add("astrophysics"), apiToken);
703703
setMetadataBlocksResponse.then().assertThat().statusCode(OK.getStatusCode());
704704

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

709711
// Dataverse not found
@@ -769,6 +771,21 @@ public void testListMetadataBlocks() {
769771
assertThat(expectedAllMetadataBlockDisplayNames, hasItemInArray(actualMetadataBlockDisplayName2));
770772
assertThat(expectedAllMetadataBlockDisplayNames, hasItemInArray(actualMetadataBlockDisplayName3));
771773

774+
// Check dataset fields for the updated input levels are retrieved
775+
int geospatialMetadataBlockIndex = actualMetadataBlockDisplayName1.equals("Geospatial Metadata") ? 0 : actualMetadataBlockDisplayName2.equals("Geospatial Metadata") ? 1 : 2;
776+
777+
// Since the included property of geographicCoverage is set to false, we should retrieve the total number of fields minus one
778+
listMetadataBlocksResponse.then().assertThat()
779+
.body(String.format("data[%d].fields.size()", geospatialMetadataBlockIndex), equalTo(10));
780+
781+
String actualMetadataField1 = listMetadataBlocksResponse.then().extract().path(String.format("data[%d].fields.geographicCoverage.name", geospatialMetadataBlockIndex));
782+
String actualMetadataField2 = listMetadataBlocksResponse.then().extract().path(String.format("data[%d].fields.country.name", geospatialMetadataBlockIndex));
783+
String actualMetadataField3 = listMetadataBlocksResponse.then().extract().path(String.format("data[%d].fields.city.name", geospatialMetadataBlockIndex));
784+
785+
assertNull(actualMetadataField1);
786+
assertNotNull(actualMetadataField2);
787+
assertNotNull(actualMetadataField3);
788+
772789
// Existent dataverse and onlyDisplayedOnCreate=true and returnDatasetFieldTypes=true
773790
listMetadataBlocksResponse = UtilIT.listMetadataBlocks(dataverseAlias, true, true, apiToken);
774791
listMetadataBlocksResponse.then().assertThat().statusCode(OK.getStatusCode());
@@ -785,16 +802,18 @@ public void testListMetadataBlocks() {
785802
assertThat(expectedOnlyDisplayedOnCreateMetadataBlockDisplayNames, hasItemInArray(actualMetadataBlockDisplayName2));
786803

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

790807
listMetadataBlocksResponse.then().assertThat()
791-
.body(String.format("data[%d].fields.size()", geospatialMetadataBlockIndex), equalTo(2));
808+
.body(String.format("data[%d].fields.size()", geospatialMetadataBlockIndex), equalTo(1));
792809

793-
String actualMetadataField1 = listMetadataBlocksResponse.then().extract().path(String.format("data[%d].fields.geographicCoverage.name", geospatialMetadataBlockIndex));
794-
String actualMetadataField2 = listMetadataBlocksResponse.then().extract().path(String.format("data[%d].fields.country.name", geospatialMetadataBlockIndex));
810+
actualMetadataField1 = listMetadataBlocksResponse.then().extract().path(String.format("data[%d].fields.geographicCoverage.name", geospatialMetadataBlockIndex));
811+
actualMetadataField2 = listMetadataBlocksResponse.then().extract().path(String.format("data[%d].fields.country.name", geospatialMetadataBlockIndex));
812+
actualMetadataField3 = listMetadataBlocksResponse.then().extract().path(String.format("data[%d].fields.city.name", geospatialMetadataBlockIndex));
795813

796-
assertNotNull(actualMetadataField1);
814+
assertNull(actualMetadataField1);
797815
assertNotNull(actualMetadataField2);
816+
assertNull(actualMetadataField3);
798817

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

899918
// Update valid input levels
900919
String[] testInputLevelNames = {"geographicCoverage", "country"};
901-
Response updateDataverseInputLevelsResponse = UtilIT.updateDataverseInputLevels(dataverseAlias, testInputLevelNames, apiToken);
920+
boolean[] testRequiredInputLevels = {true, false};
921+
boolean[] testIncludedInputLevels = {true, false};
922+
Response updateDataverseInputLevelsResponse = UtilIT.updateDataverseInputLevels(dataverseAlias, testInputLevelNames, testRequiredInputLevels, testIncludedInputLevels, apiToken);
923+
String actualInputLevelName = updateDataverseInputLevelsResponse.then().extract().path("data.inputLevels[0].datasetFieldTypeName");
924+
int geographicCoverageInputLevelIndex = actualInputLevelName.equals("geographicCoverage") ? 0 : 1;
902925
updateDataverseInputLevelsResponse.then().assertThat()
903-
.body("data.inputLevels[0].required", equalTo(true))
904-
.body("data.inputLevels[0].include", equalTo(true))
905-
.body("data.inputLevels[1].required", equalTo(true))
906-
.body("data.inputLevels[1].include", equalTo(true))
926+
.body(String.format("data.inputLevels[%d].include", geographicCoverageInputLevelIndex), equalTo(true))
927+
.body(String.format("data.inputLevels[%d].required", geographicCoverageInputLevelIndex), equalTo(true))
928+
.body(String.format("data.inputLevels[%d].include", 1 - geographicCoverageInputLevelIndex), equalTo(false))
929+
.body(String.format("data.inputLevels[%d].required", 1 - geographicCoverageInputLevelIndex), equalTo(false))
907930
.statusCode(OK.getStatusCode());
908931
String actualFieldTypeName1 = updateDataverseInputLevelsResponse.then().extract().path("data.inputLevels[0].datasetFieldTypeName");
909932
String actualFieldTypeName2 = updateDataverseInputLevelsResponse.then().extract().path("data.inputLevels[1].datasetFieldTypeName");
@@ -913,15 +936,14 @@ public void testUpdateInputLevels() {
913936

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

921944
// Update invalid empty input levels
922945
testInputLevelNames = new String[]{};
923-
updateDataverseInputLevelsResponse = UtilIT.updateDataverseInputLevels(dataverseAlias, testInputLevelNames, apiToken);
924-
updateDataverseInputLevelsResponse.prettyPrint();
946+
updateDataverseInputLevelsResponse = UtilIT.updateDataverseInputLevels(dataverseAlias, testInputLevelNames, testRequiredInputLevels, testIncludedInputLevels, apiToken);
925947
updateDataverseInputLevelsResponse.then().assertThat()
926948
.body("message", equalTo("Error while updating dataverse input levels: Input level list cannot be null or empty"))
927949
.statusCode(INTERNAL_SERVER_ERROR.getStatusCode());

‎src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3960,22 +3960,25 @@ static Response requestGlobusUploadPaths(Integer datasetId, JsonObject body, Str
39603960
.post("/api/datasets/" + datasetId + "/requestGlobusUploadPaths");
39613961
}
39623962

3963-
static Response updateDataverseInputLevels(String dataverseAlias, String[] inputLevelNames, String apiToken) {
3964-
JsonArrayBuilder contactArrayBuilder = Json.createArrayBuilder();
3965-
for(String inputLevelName : inputLevelNames) {
3966-
contactArrayBuilder.add(Json.createObjectBuilder()
3967-
.add("datasetFieldTypeName", inputLevelName)
3968-
.add("required", true)
3969-
.add("include", true)
3970-
);
3963+
public static Response updateDataverseInputLevels(String dataverseAlias, String[] inputLevelNames, boolean[] requiredInputLevels, boolean[] includedInputLevels, String apiToken) {
3964+
JsonArrayBuilder inputLevelsArrayBuilder = Json.createArrayBuilder();
3965+
for (int i = 0; i < inputLevelNames.length; i++) {
3966+
inputLevelsArrayBuilder.add(createInputLevelObject(inputLevelNames[i], requiredInputLevels[i], includedInputLevels[i]));
39713967
}
39723968
return given()
39733969
.header(API_TOKEN_HTTP_HEADER, apiToken)
3974-
.body(contactArrayBuilder.build().toString())
3970+
.body(inputLevelsArrayBuilder.build().toString())
39753971
.contentType(ContentType.JSON)
39763972
.put("/api/dataverses/" + dataverseAlias + "/inputLevels");
39773973
}
39783974

3975+
private static JsonObjectBuilder createInputLevelObject(String name, boolean required, boolean include) {
3976+
return Json.createObjectBuilder()
3977+
.add("datasetFieldTypeName", name)
3978+
.add("required", required)
3979+
.add("include", include);
3980+
}
3981+
39793982
public static Response getOpenAPI(String accept, String format) {
39803983
Response response = given()
39813984
.header("Accept", accept)

0 commit comments

Comments
 (0)