From b42222fd35006c24e57eccece81d3496dcac31b8 Mon Sep 17 00:00:00 2001 From: Guillermo Portas Date: Tue, 2 Jul 2024 15:59:19 +0100 Subject: [PATCH] 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 --- .../10637-fix-dataverse-metadatablocks-api.md | 2 + .../edu/harvard/iq/dataverse/Dataverse.java | 18 ++++--- .../iq/dataverse/util/json/JsonPrinter.java | 18 +++++-- .../iq/dataverse/api/DataversesIT.java | 52 +++++++++++++------ .../edu/harvard/iq/dataverse/api/UtilIT.java | 21 ++++---- 5 files changed, 76 insertions(+), 35 deletions(-) create mode 100644 doc/release-notes/10637-fix-dataverse-metadatablocks-api.md diff --git a/doc/release-notes/10637-fix-dataverse-metadatablocks-api.md b/doc/release-notes/10637-fix-dataverse-metadatablocks-api.md new file mode 100644 index 00000000000..c8c9c4fa66f --- /dev/null +++ b/doc/release-notes/10637-fix-dataverse-metadatablocks-api.md @@ -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. + diff --git a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java index 78b1827c798..978c716e058 100644 --- a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java +++ b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java @@ -412,12 +412,18 @@ public List 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() { diff --git a/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java b/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java index 95f14b79ece..c72dfc1d127 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java @@ -640,18 +640,26 @@ public static JsonObjectBuilder json(MetadataBlock metadataBlock, boolean printO JsonObjectBuilder fieldsBuilder = Json.createObjectBuilder(); Set 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; } diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DataversesIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DataversesIT.java index b072a803aa4..79cc46cfa79 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DataversesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DataversesIT.java @@ -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 @@ -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()); @@ -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(); @@ -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"); @@ -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()); diff --git a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java index 257610dbc32..0216859b869 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -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)