From 31f146b1cdcdc79d1d62429a8237d5f2bb382330 Mon Sep 17 00:00:00 2001 From: jo-pol Date: Tue, 8 Apr 2025 13:16:25 +0200 Subject: [PATCH 001/247] 500->409 for concurrent updates --- .../edu/harvard/iq/dataverse/api/Datasets.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java index 516904f17a6..f754341a36b 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -66,6 +66,7 @@ import jakarta.ws.rs.container.ContainerRequestContext; import jakarta.ws.rs.core.*; import jakarta.ws.rs.core.Response.Status; +import org.apache.commons.lang.exception.ExceptionUtils; import org.apache.commons.lang3.StringUtils; import org.eclipse.microprofile.openapi.annotations.Operation; import org.eclipse.microprofile.openapi.annotations.media.Content; @@ -102,7 +103,6 @@ import edu.harvard.iq.dataverse.engine.command.exception.PermissionException; import edu.harvard.iq.dataverse.dataset.DatasetType; import edu.harvard.iq.dataverse.dataset.DatasetTypeServiceBean; - import static edu.harvard.iq.dataverse.util.json.JsonPrinter.*; import static edu.harvard.iq.dataverse.util.json.NullSafeJsonBuilder.jsonObjectBuilder; @@ -2059,6 +2059,10 @@ public Response createAssignment(@Context ContainerRequestContext crc, RoleAssig return ok( json(execCommand(new AssignRoleCommand(assignee, theRole, dataset, createDataverseRequest(getRequestUser(crc)), privateUrlToken)))); } catch (WrappedResponse ex) { + if (ExceptionUtils.getRootCause(ex).getMessage().contains("duplicate key")) { + // concurrent update + return error(Status.CONFLICT, BundleUtil.getStringFromBundle("datasets.api.grant.role.assignee.has.role.error")); + } List args = Arrays.asList(ex.getMessage()); logger.log(Level.WARNING, BundleUtil.getStringFromBundle("datasets.api.grant.role.cant.create.assignment.error", args)); return ex.getResponse(); @@ -2964,7 +2968,7 @@ public Response getCompareVersions(@Context ContainerRequestContext crc, @PathPa return wr.getResponse(); } } - + @GET @AuthRequired @Path("{id}/versions/compareSummary") @@ -3020,7 +3024,7 @@ public Response getCompareVersionsSummary(@Context ContainerRequestContext crc, return wr.getResponse(); } } - + private JsonObject getDeaccessionJson(DatasetVersion dv) { JsonObjectBuilder compositionBuilder = Json.createObjectBuilder(); @@ -3036,7 +3040,7 @@ private JsonObject getDeaccessionJson(DatasetVersion dv) { JsonObject json = Json.createObjectBuilder() .add("deaccessioned", compositionBuilder) .build(); - + return json; } @@ -4454,7 +4458,7 @@ public Response monitorGlobusDownload(@Context ContainerRequestContext crc, @Pat return badRequest("Error parsing json body"); } - + // Async Call globusService.globusDownload(jsonObject, dataset, authUser); @@ -4957,7 +4961,7 @@ public Response getDatasetVersionInternalCitation(@Context ContainerRequestConte } } - /** + /** * Returns one of the DataCitation.Format types as a raw file download (not wrapped in our ok json) * @param crc * @param datasetId @@ -5533,7 +5537,7 @@ public Response addVersionNote(@Context ContainerRequestContext crc, @PathParam( @Path("{id}/versions/{versionId}/versionNote") public Response deleteVersionNote(@Context ContainerRequestContext crc, @PathParam("id") String datasetId, @PathParam("versionId") String versionId, @Context UriInfo uriInfo, @Context HttpHeaders headers) throws WrappedResponse { if(!FeatureFlags.VERSION_NOTE.enabled()) { - return notFound(BundleUtil.getStringFromBundle("datasets.api.addVersionNote.notEnabled")); + return notFound(BundleUtil.getStringFromBundle("datasets.api.addVersionNote.notEnabled")); } if (!DS_VERSION_DRAFT.equals(versionId)) { AuthenticatedUser user = getRequestAuthenticatedUserOrDie(crc); From 9baf2d52f48b875e8691e4b8325289049ac11080 Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Fri, 18 Apr 2025 14:52:11 -0400 Subject: [PATCH 002/247] File Metadata Update --- .../11392-edit-file-metadata-empty-values.md | 11 ++ doc/sphinx-guides/source/api/changelog.rst | 1 + doc/sphinx-guides/source/api/native-api.rst | 31 +++++ .../edu/harvard/iq/dataverse/api/Files.java | 67 ++++++++++ .../datasetutility/OptionalFileParams.java | 68 +++------- .../edu/harvard/iq/dataverse/api/FilesIT.java | 123 ++++++++++++++++++ .../edu/harvard/iq/dataverse/api/UtilIT.java | 11 +- .../OptionalFileParamsTest.java | 7 +- 8 files changed, 265 insertions(+), 54 deletions(-) create mode 100644 doc/release-notes/11392-edit-file-metadata-empty-values.md diff --git a/doc/release-notes/11392-edit-file-metadata-empty-values.md b/doc/release-notes/11392-edit-file-metadata-empty-values.md new file mode 100644 index 00000000000..66059444e4f --- /dev/null +++ b/doc/release-notes/11392-edit-file-metadata-empty-values.md @@ -0,0 +1,11 @@ +### Edit File Metadata empty values should clear data + +Previously the API POST /files/{id}/metadata would ignore fields with empty values. Now the API updates the fields with the empty values essentially clearing the data. Missing fields will still be ignored. + +This feature also adds a new version of the POST endpoint (/files/{id}/metadata/version/{datasetVersion}) to specify the dataset version to make the file metadata change to. + +datasetVersion can either be the actual ID (12345) or the friendly version (1.0) + +Note that certain fields (i.e. dataFileTags) are not versioned and changes to these will update the published as well as draft versions of the file. + +See also [the guides](https://dataverse-guide--11359.org.readthedocs.build/en/11359/api/native-api.html#updating-file-metadata), #11392, and #11359. diff --git a/doc/sphinx-guides/source/api/changelog.rst b/doc/sphinx-guides/source/api/changelog.rst index 9bcf0cbfaa6..ddef58cd9fa 100644 --- a/doc/sphinx-guides/source/api/changelog.rst +++ b/doc/sphinx-guides/source/api/changelog.rst @@ -11,6 +11,7 @@ v6.7 ---- - An undocumented :doc:`search` parameter called "show_my_data" has been removed. It was never exercised by tests and is believed to be unused. API users should use the :ref:`api-mydata` API instead. +- For POST /api/files/{id}/metadata passing an empty string (“description”:””) or array (“categories”:[]) will no longer be ignored. Empty fields will now clear out the values in the file's metadata. To ignore the fields simply do not include them in the Json string. v6.6 ---- diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 95041ab5304..aba31670aa1 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -4612,6 +4612,10 @@ Updating File Metadata Updates the file metadata for an existing file where ``ID`` is the database id of the file to update or ``PERSISTENT_ID`` is the persistent id (DOI or Handle) of the file. Requires a ``jsonString`` expressing the new metadata. No metadata from the previous version of this file will be persisted, so if you want to update a specific field first get the json with the above command and alter the fields you want. +An extended version of this API will allow for the Dataset Version ID to be specified in order to modify fields of a Datafile in an already published version of the Dataset. + +Note: As of Dataverse 6.7 passing in an empty value for a string field ("description":"") or an empty array for a list ("categories":[]) will clear the data for that field. In prior versions these fields would be ignored. To ignore the fields simply leave them out of the ``jsonString``. + A curl example using an ``ID`` .. code-block:: bash @@ -4656,6 +4660,33 @@ Note: To update the 'tabularTags' property of file metadata, use the 'dataFileTa Also note that dataFileTags are not versioned and changes to these will update the published version of the file. +Extended version of the API: + +This extended version of the API will allow the user to modify fields of a specific version of the file's metadata. + +The Dataset version id can be either the ID of the Dataset version (i.e. 12345) or the Friendly version (i.e. 1.0). + +As noted above the dataFileTags are not versioned and any change to them for this version will also change them in the draft version. It is not recommended to change them with this API. + +.. code-block:: bash + + export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx + export SERVER_URL=https://demo.dataverse.org + export ID=24 + export DATASET_VERSION_ID=1.0 + + curl -H "X-Dataverse-key:$API_TOKEN" -X POST \ + -F 'jsonData={"description":"My description bbb.","provFreeform":"Test prov freeform","categories":["Data"]}' \ + "$SERVER_URL/api/files/$ID/metadata/version/$DATASET_VERSION_ID" + +The fully expanded example above (without environment variables) looks like this: + +.. code-block:: bash + + curl -H "X-Dataverse-key:xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X POST \ + -F 'jsonData={"description":"My description bbb.","provFreeform":"Test prov freeform","categories":["Data"]}' \ + "https://demo.dataverse.org/api/files/:persistentId/metadata/version/1.0?persistentId=doi:10.5072/FK2/AAA000" + .. _EditingVariableMetadata: Updating File Metadata Categories diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Files.java b/src/main/java/edu/harvard/iq/dataverse/api/Files.java index 61a69236f57..e7459c1b325 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -45,6 +45,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import java.util.logging.Logger; @@ -531,7 +532,73 @@ public Response updateFileMetadata(@Context ContainerRequestContext crc, @FormDa .type(MediaType.TEXT_PLAIN) //Our plain text string is already json .build(); } + @POST + @AuthRequired + @Path("{id}/metadata/version/{datasetVersionId}") + public Response updateFileMetadata(@Context ContainerRequestContext crc, @FormDataParam("jsonData") String jsonData, + @PathParam("id") String fileIdOrPersistentId, + @PathParam("datasetVersionId") String datasetVersionId) { + DataverseRequest req; + try { + req = createDataverseRequest(getRequestUser(crc)); + } catch (Exception e) { + return error(BAD_REQUEST, "Error attempting to request information. Maybe a bad API token?"); + } + final DataFile df; + FileMetadata fm = null; + try { + df = execCommand(new GetDataFileCommand(req, findDataFileOrDie(fileIdOrPersistentId))); + fm = df.getFileMetadata(); + for (FileMetadata md : df.getFileMetadatas()) { + if (datasetVersionId.equals(String.valueOf(md.getDatasetVersion().getId())) || + datasetVersionId.equals(md.getDatasetVersion().getFriendlyVersionNumber())) { + fm = md; + } + } + } catch (Exception e) { + return error(BAD_REQUEST, "Error attempting get the requested data file."); + } + + try { + OptionalFileParams optionalFileParams = null; + if (jsonData != null) { + // Load optional params via JSON + optionalFileParams = new OptionalFileParams(jsonData); + } else { + return error(BAD_REQUEST, "Missing Form Data!"); + } + List fmdListMinusCurrentFile = new ArrayList<>(); + for (FileMetadata fileMetadata : df.getFileMetadatas()) { + if (!fileMetadata.getDataFile().equals(df)) { + fmdListMinusCurrentFile.add(fileMetadata); + } + } + jakarta.json.JsonObject jsonObject = JsonUtil.getJsonObject(jsonData); + String incomingLabel = jsonObject.getString("label", null); + String incomingDirectoryLabel = jsonObject.getString("directoryLabel", null); + String existingLabel = fm.getLabel(); + String existingDirectoryLabel = fm.getDirectoryLabel(); + String pathPlusFilename = IngestUtil.getPathAndFileNameToCheck(incomingLabel, incomingDirectoryLabel, existingLabel, existingDirectoryLabel); + if (IngestUtil.conflictsWithExistingFilenames(pathPlusFilename, fmdListMinusCurrentFile)) { + return error(BAD_REQUEST, BundleUtil.getStringFromBundle("files.api.metadata.update.duplicateFile", Arrays.asList(pathPlusFilename))); + } + + optionalFileParams.addOptionalParams(fm); + execCommand(new UpdateDatasetVersionCommand(fm.getDataFile().getOwner(), req, fm)); + + } catch (DataFileTagException ex) { + return error(BAD_REQUEST, ex.getMessage()); + } catch (Exception e) { + return error(Response.Status.INTERNAL_SERVER_ERROR, "Error updating metadata to DataFile: " + e); + } + String jsonString = fm.asGsonObject(true).toString(); + return Response + .status(Response.Status.OK) + .entity("File Metadata update has been completed: " + jsonString) + .type(MediaType.TEXT_PLAIN) //Our plain text string is already json + .build(); + } @GET @AuthRequired @Path("{id}") diff --git a/src/main/java/edu/harvard/iq/dataverse/datasetutility/OptionalFileParams.java b/src/main/java/edu/harvard/iq/dataverse/datasetutility/OptionalFileParams.java index 54844160163..f7df81b6386 100644 --- a/src/main/java/edu/harvard/iq/dataverse/datasetutility/OptionalFileParams.java +++ b/src/main/java/edu/harvard/iq/dataverse/datasetutility/OptionalFileParams.java @@ -194,46 +194,28 @@ public boolean getTabIngest() { return this.tabIngest; } - public boolean hasCategories(){ - if ((categories == null)||(this.categories.isEmpty())){ - return false; - } - return true; + public boolean hasCategories() { + return categories != null; } - public boolean hasFileDataTags(){ - if ((dataFileTags == null)||(this.dataFileTags.isEmpty())){ - return false; - } - return true; + public boolean hasFileDataTags() { + return dataFileTags != null; } public boolean hasDescription(){ - if ((description == null)||(this.description.isEmpty())){ - return false; - } - return true; + return description != null; } - public boolean hasDirectoryLabel(){ - if ((directoryLabel == null)||(this.directoryLabel.isEmpty())){ - return false; - } - return true; + public boolean hasDirectoryLabel() { + return directoryLabel != null; } - public boolean hasLabel(){ - if ((label == null)||(this.label.isEmpty())){ - return false; - } - return true; + public boolean hasLabel() { + return label != null; } - public boolean hasProvFreeform(){ - if ((provFreeForm == null)||(this.provFreeForm.isEmpty())){ - return false; - } - return true; + public boolean hasProvFreeform() { + return provFreeForm != null; } public boolean hasStorageIdentifier() { @@ -245,7 +227,7 @@ public String getStorageIdentifier() { } public boolean hasFileName() { - return ((fileName!=null)&&(!fileName.isEmpty())); + return fileName != null; } public String getFileName() { @@ -253,7 +235,7 @@ public String getFileName() { } public boolean hasMimetype() { - return ((mimeType!=null)&&(!mimeType.isEmpty())); + return mimeType != null; } public String getMimeType() { @@ -266,7 +248,7 @@ public void setCheckSum(String checkSum, ChecksumType type) { } public boolean hasCheckSum() { - return ((checkSumValue!=null)&&(!checkSumValue.isEmpty())); + return checkSumValue != null; } public String getCheckSum() { @@ -294,15 +276,10 @@ public void setFileSize(long fileSize) { * @param tags */ public void setCategories(List newCategories) { - if (newCategories != null) { newCategories = Util.removeDuplicatesNullsEmptyStrings(newCategories); - if (newCategories.isEmpty()) { - newCategories = null; - } + this.categories = newCategories; } - - this.categories = newCategories; } /** @@ -495,27 +472,20 @@ private void addFileDataTags(List potentialTags) throws DataFileTagExcep } potentialTags = Util.removeDuplicatesNullsEmptyStrings(potentialTags); - - if (potentialTags.isEmpty()){ - return; - } - + // Make a new list - this.dataFileTags = new ArrayList<>(); + List newList = new ArrayList<>(); // Add valid potential tags to the list for (String tagToCheck : potentialTags){ if (DataFileTag.isDataFileTag(tagToCheck)){ - this.dataFileTags.add(tagToCheck); + newList.add(tagToCheck); }else{ String errMsg = BundleUtil.getStringFromBundle("file.addreplace.error.invalid_datafile_tag"); throw new DataFileTagException(errMsg + " [" + tagToCheck + "]. Please use one of the following: " + DataFileTag.getListofLabelsAsString()); } } - // Shouldn't happen.... - if (dataFileTags.isEmpty()){ - dataFileTags = null; - } + this.dataFileTags = newList; } private void msg(String s){ diff --git a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java index d7cce94a0b8..de940c30e17 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -1,6 +1,7 @@ package edu.harvard.iq.dataverse.api; import edu.harvard.iq.dataverse.authorization.DataverseRole; +import edu.harvard.iq.dataverse.datasetutility.OptionalFileParams; import io.restassured.RestAssured; import io.restassured.response.Response; @@ -3189,4 +3190,126 @@ public void testFileCitationByVersion() throws IOException { } + // This test handles both updating with empty fields to clear the field and + // update a specific file metadata for dataset version + // Both APIs will be tested /api/files/{id}/metadata and /api/files/{id}/metadata/version/{datasetVersionId} + @Test + public void testUpdateSpecificMetadataVersionAndTestUpdateWithEmptyFields() { + // Create User, Dataverse, and Dataset + Response createUser = UtilIT.createRandomUser(); + createUser.then().assertThat().statusCode(OK.getStatusCode()); + String apiToken = UtilIT.getApiTokenFromResponse(createUser); + + Response createDataverseResponse = UtilIT.createRandomDataverse(apiToken); + createDataverseResponse.then().assertThat().statusCode(CREATED.getStatusCode()); + String dataverseAlias = UtilIT.getAliasFromResponse(createDataverseResponse); + + Response createDatasetResponse = UtilIT.createRandomDatasetViaNativeApi(dataverseAlias, apiToken); + createDatasetResponse.prettyPrint(); + createDatasetResponse.then().assertThat().statusCode(CREATED.getStatusCode()); + Integer datasetId = JsonPath.from(createDatasetResponse.body().asString()).getInt("data.id"); + + // Upload a tab file + JsonObjectBuilder json = Json.createObjectBuilder() + .add(OptionalFileParams.DESCRIPTION_ATTR_NAME, "my description") + .add(OptionalFileParams.DIRECTORY_LABEL_ATTR_NAME, "data/subdir1") + .add(OptionalFileParams.PROVENANCE_FREEFORM_ATTR_NAME, "prov Free Form") + .add(OptionalFileParams.CATEGORIES_ATTR_NAME, Json.createArrayBuilder().add("Data")); + String pathToTestFile = "src/test/resources/tab/test.tab"; + Response uploadFile = UtilIT.uploadFileViaNative(datasetId.toString(), pathToTestFile, json.build(), apiToken); + uploadFile.prettyPrint(); + uploadFile.then().assertThat().statusCode(OK.getStatusCode()); + Long fileId = JsonPath.from(uploadFile.body().asString()).getLong("data.files[0].dataFile.id"); + assertTrue(UtilIT.sleepForLock(datasetId, "Ingest", apiToken, UtilIT.MAXIMUM_INGEST_LOCK_DURATION), "Failed test if Ingest Lock exceeds max duration " + pathToTestFile); + + // Can't add tags until after the file is ingested and determined to be a tabular file + JsonObjectBuilder updateFileJson = Json.createObjectBuilder() + .add(OptionalFileParams.FILE_DATA_TAGS_ATTR_NAME, Json.createArrayBuilder().add("Survey")); + Response updateFileResponse = UtilIT.updateFileMetadata(String.valueOf(fileId), updateFileJson.build().toString(), apiToken); + updateFileResponse.prettyPrint(); + + // Get and verify the FileData + Response getFile = UtilIT.getFileData(String.valueOf(fileId), apiToken); + getFile.prettyPrint(); + getFile.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.description", equalTo("my description")) + .body("data.dataFile.description", equalTo("my description")) + .body("data.directoryLabel", equalTo("data/subdir1")) + .body("data.categories", hasItem("Data")) + .body("data.dataFile.tabularTags", hasItem("Survey")); + + // Publish the Dataverse and Dataset + Response publishResponse = UtilIT.publishDataverseViaNativeApi(dataverseAlias, apiToken); + publishResponse.then().assertThat().statusCode(OK.getStatusCode()); + publishResponse = UtilIT.publishDatasetViaNativeApi(datasetId, "major", apiToken); + publishResponse.then().assertThat().statusCode(OK.getStatusCode()); + + // Create a new DRAFT version + json = Json.createObjectBuilder() + .add(OptionalFileParams.DESCRIPTION_ATTR_NAME, "my 1.1 description"); + Response updateResponse = UtilIT.updateFileMetadata(String.valueOf(fileId), json.build().toString(), apiToken); + updateResponse.then().assertThat().statusCode(OK.getStatusCode()); + + // Publish the draft version as 1.1 + publishResponse = UtilIT.publishDatasetViaNativeApi(datasetId, "minor", apiToken); + publishResponse.then().assertThat().statusCode(OK.getStatusCode()); + publishResponse.prettyPrint(); + + // We now have a 1.0 version and 1.1 version. Now we modify the 1.0 version + String version = "1.0"; + json = Json.createObjectBuilder() + .add(OptionalFileParams.DESCRIPTION_ATTR_NAME, "") + .add(OptionalFileParams.LABEL_ATTR_NAME, "test.tab") + .add(OptionalFileParams.DIRECTORY_LABEL_ATTR_NAME, "") + .add(OptionalFileParams.PROVENANCE_FREEFORM_ATTR_NAME, "") + .add(OptionalFileParams.CATEGORIES_ATTR_NAME, Json.createArrayBuilder()) + .add(OptionalFileParams.FILE_DATA_TAGS_ATTR_NAME, Json.createArrayBuilder()); + + Response updateFile = UtilIT.updateFileMetadata(String.valueOf(fileId), json.build().toString(), apiToken, version); + updateFile.prettyPrint(); + + // Get version 1.0 and see the changes + getFile = UtilIT.getFileData(String.valueOf(fileId), apiToken, version); + getFile.prettyPrint(); + getFile.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.description", equalTo("")) + .body("data.dataFile.description", equalTo("")) + .body("data.directoryLabel", nullValue()) + .body("data.provFreeForm", nullValue()) + .body("data.categories", nullValue()) + .body("data.dataFile.tabularTags", nullValue()); + + // Get the latest version and see the original data unchanged (except description which is why the draft version was created) + getFile = UtilIT.getFileData(String.valueOf(fileId), apiToken); + getFile.prettyPrint(); + getFile.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.description", equalTo("my 1.1 description")) + .body("data.dataFile.description", equalTo("my 1.1 description")) + .body("data.directoryLabel", equalTo("data/subdir1")) + .body("data.categories", hasItem("Data")); + + // Update metadata (creating a draft version) without the version in the path to show that this endpoint also clears the fields + json = Json.createObjectBuilder() + .add(OptionalFileParams.DESCRIPTION_ATTR_NAME, "") + .add(OptionalFileParams.LABEL_ATTR_NAME, "test.tab") + .add(OptionalFileParams.DIRECTORY_LABEL_ATTR_NAME, "") + .add(OptionalFileParams.PROVENANCE_FREEFORM_ATTR_NAME, "") + .add(OptionalFileParams.CATEGORIES_ATTR_NAME, Json.createArrayBuilder()) + .add(OptionalFileParams.FILE_DATA_TAGS_ATTR_NAME, Json.createArrayBuilder()); + updateFile = UtilIT.updateFileMetadata(String.valueOf(fileId), json.build().toString(), apiToken); + updateFile.prettyPrint(); + getFile = UtilIT.getFileData(String.valueOf(fileId), apiToken); + getFile.prettyPrint(); + getFile.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.description", equalTo("")) + .body("data.dataFile.description", equalTo("")) + .body("data.directoryLabel", nullValue()) + .body("data.provFreeForm", nullValue()) + .body("data.categories", nullValue()) + .body("data.dataFile.tabularTags", nullValue()); + } } 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 b9d403c528c..e28ece85a23 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -1088,21 +1088,28 @@ static Response deleteFileApi(Integer fileId, String apiToken) { .header(API_TOKEN_HTTP_HEADER, apiToken) .delete("/api/files/" + fileId); } - + static Response updateFileMetadata(String fileIdOrPersistentId, String jsonAsString, String apiToken) { + return updateFileMetadata(fileIdOrPersistentId, jsonAsString,apiToken, null); + } + static Response updateFileMetadata(String fileIdOrPersistentId, String jsonAsString, String apiToken, String datasetVersionId) { String idInPath = fileIdOrPersistentId; // Assume it's a number. + String versionInPath = ""; String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. if (!NumberUtils.isCreatable(fileIdOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + fileIdOrPersistentId; } + if (datasetVersionId != null) { + versionInPath = "/version/" + datasetVersionId; + } RequestSpecification requestSpecification = given() .header(API_TOKEN_HTTP_HEADER, apiToken); if (jsonAsString != null) { requestSpecification.multiPart("jsonData", jsonAsString); } return requestSpecification - .post("/api/files/" + idInPath + "/metadata" + optionalQueryParam); + .post("/api/files/" + idInPath + "/metadata" + versionInPath + optionalQueryParam); } static Response downloadFile(Integer fileId) { diff --git a/src/test/java/edu/harvard/iq/dataverse/datasetutility/OptionalFileParamsTest.java b/src/test/java/edu/harvard/iq/dataverse/datasetutility/OptionalFileParamsTest.java index c9f251f7e77..cbca33f409d 100644 --- a/src/test/java/edu/harvard/iq/dataverse/datasetutility/OptionalFileParamsTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/datasetutility/OptionalFileParamsTest.java @@ -195,8 +195,9 @@ public void test_09_unusedParamsGood() throws DataFileTagException { assertNull(instance.getDescription()); assertFalse(instance.hasDescription()); - assertNull(instance.getCategories()); - assertFalse(instance.hasCategories()); + assertNotNull(instance.getCategories()); + assertTrue(instance.hasCategories()); + assertTrue(instance.getCategories().isEmpty()); assertNull(instance.getDataFileTags()); assertFalse(instance.hasFileDataTags()); @@ -292,4 +293,4 @@ private void msgt(String s){ print json.dumps(json.dumps(d)) -*/ \ No newline at end of file +*/ From 6d871aabb8c0d5514bdba794384c0ce26df989b4 Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Fri, 18 Apr 2025 17:05:23 -0400 Subject: [PATCH 003/247] new version checking --- .../11392-edit-file-metadata-empty-values.md | 6 +- doc/sphinx-guides/source/api/native-api.rst | 36 +-------- .../edu/harvard/iq/dataverse/api/Files.java | 75 ++----------------- src/main/java/propertyFiles/Bundle.properties | 1 + .../edu/harvard/iq/dataverse/api/FilesIT.java | 69 +++++------------ .../edu/harvard/iq/dataverse/api/UtilIT.java | 4 +- 6 files changed, 33 insertions(+), 158 deletions(-) diff --git a/doc/release-notes/11392-edit-file-metadata-empty-values.md b/doc/release-notes/11392-edit-file-metadata-empty-values.md index 66059444e4f..e42bef13aec 100644 --- a/doc/release-notes/11392-edit-file-metadata-empty-values.md +++ b/doc/release-notes/11392-edit-file-metadata-empty-values.md @@ -2,10 +2,6 @@ Previously the API POST /files/{id}/metadata would ignore fields with empty values. Now the API updates the fields with the empty values essentially clearing the data. Missing fields will still be ignored. -This feature also adds a new version of the POST endpoint (/files/{id}/metadata/version/{datasetVersion}) to specify the dataset version to make the file metadata change to. - -datasetVersion can either be the actual ID (12345) or the friendly version (1.0) - -Note that certain fields (i.e. dataFileTags) are not versioned and changes to these will update the published as well as draft versions of the file. +An optional query parameter was added to ensure the metadata update doesn't overwrite stale data. See also [the guides](https://dataverse-guide--11359.org.readthedocs.build/en/11359/api/native-api.html#updating-file-metadata), #11392, and #11359. diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index aba31670aa1..9bf7b3679bf 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -4612,9 +4612,7 @@ Updating File Metadata Updates the file metadata for an existing file where ``ID`` is the database id of the file to update or ``PERSISTENT_ID`` is the persistent id (DOI or Handle) of the file. Requires a ``jsonString`` expressing the new metadata. No metadata from the previous version of this file will be persisted, so if you want to update a specific field first get the json with the above command and alter the fields you want. -An extended version of this API will allow for the Dataset Version ID to be specified in order to modify fields of a Datafile in an already published version of the Dataset. - -Note: As of Dataverse 6.7 passing in an empty value for a string field ("description":"") or an empty array for a list ("categories":[]) will clear the data for that field. In prior versions these fields would be ignored. To ignore the fields simply leave them out of the ``jsonString``. +Optional Parameter for verifying that the Dataset Version being edited is the latest version can be added &datasetVersionId=12345. This is to prevent stale data from being edited. A curl example using an ``ID`` @@ -4643,10 +4641,11 @@ A curl example using a ``PERSISTENT_ID`` export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx export SERVER_URL=https://demo.dataverse.org export PERSISTENT_ID=doi:10.5072/FK2/AAA000 + export VERSION_ID=12345 curl -H "X-Dataverse-key:$API_TOKEN" -X POST \ -F 'jsonData={"description":"My description bbb.","provFreeform":"Test prov freeform","categories":["Data"],"dataFileTags":["Survey"],"restrict":false}' \ - "$SERVER_URL/api/files/:persistentId/metadata?persistentId=$PERSISTENT_ID" + "$SERVER_URL/api/files/:persistentId/metadata?persistentId=$PERSISTENT_ID&datasetVersionId=$VERSION_ID" The fully expanded example above (without environment variables) looks like this: @@ -4654,39 +4653,12 @@ The fully expanded example above (without environment variables) looks like this curl -H "X-Dataverse-key:xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X POST \ -F 'jsonData={"description":"My description bbb.","provFreeform":"Test prov freeform","categories":["Data"],"dataFileTags":["Survey"],"restrict":false}' \ - "https://demo.dataverse.org/api/files/:persistentId/metadata?persistentId=doi:10.5072/FK2/AAA000" + "https://demo.dataverse.org/api/files/:persistentId/metadata?persistentId=doi:10.5072/FK2/AAA000&datasetVersionId=12345" Note: To update the 'tabularTags' property of file metadata, use the 'dataFileTags' key when making API requests. This property is used to update the 'tabularTags' of the file metadata. Also note that dataFileTags are not versioned and changes to these will update the published version of the file. -Extended version of the API: - -This extended version of the API will allow the user to modify fields of a specific version of the file's metadata. - -The Dataset version id can be either the ID of the Dataset version (i.e. 12345) or the Friendly version (i.e. 1.0). - -As noted above the dataFileTags are not versioned and any change to them for this version will also change them in the draft version. It is not recommended to change them with this API. - -.. code-block:: bash - - export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx - export SERVER_URL=https://demo.dataverse.org - export ID=24 - export DATASET_VERSION_ID=1.0 - - curl -H "X-Dataverse-key:$API_TOKEN" -X POST \ - -F 'jsonData={"description":"My description bbb.","provFreeform":"Test prov freeform","categories":["Data"]}' \ - "$SERVER_URL/api/files/$ID/metadata/version/$DATASET_VERSION_ID" - -The fully expanded example above (without environment variables) looks like this: - -.. code-block:: bash - - curl -H "X-Dataverse-key:xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X POST \ - -F 'jsonData={"description":"My description bbb.","provFreeform":"Test prov freeform","categories":["Data"]}' \ - "https://demo.dataverse.org/api/files/:persistentId/metadata/version/1.0?persistentId=doi:10.5072/FK2/AAA000" - .. _EditingVariableMetadata: Updating File Metadata Categories diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Files.java b/src/main/java/edu/harvard/iq/dataverse/api/Files.java index e7459c1b325..cd106d728b1 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -45,7 +45,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import java.util.logging.Logger; @@ -411,8 +410,8 @@ public Response deleteFileInDataset(@Context ContainerRequestContext crc, @PathP @AuthRequired @Path("{id}/metadata") public Response updateFileMetadata(@Context ContainerRequestContext crc, @FormDataParam("jsonData") String jsonData, - @PathParam("id") String fileIdOrPersistentId - ) throws DataFileTagException, CommandException { + @PathParam("id") String fileIdOrPersistentId, @QueryParam("datasetVersionId") String datasetVersionId + ) throws CommandException { FileMetadata upFmd = null; @@ -430,6 +429,10 @@ public Response updateFileMetadata(@Context ContainerRequestContext crc, @FormDa return error(BAD_REQUEST, "Error attempting get the requested data file."); } + Long latestDatasetVersion = df.getFileMetadata().getDatasetVersion().getId(); + if (datasetVersionId != null && latestDatasetVersion != Long.valueOf(datasetVersionId)) { + return error(Response.Status.BAD_REQUEST, BundleUtil.getStringFromBundle("file.metadata.message.parallelUpdateError")); + } //You shouldn't be trying to edit a datafile that has been replaced List result = em.createNamedQuery("DataFile.findDataFileThatReplacedId", Long.class) @@ -532,73 +535,7 @@ public Response updateFileMetadata(@Context ContainerRequestContext crc, @FormDa .type(MediaType.TEXT_PLAIN) //Our plain text string is already json .build(); } - @POST - @AuthRequired - @Path("{id}/metadata/version/{datasetVersionId}") - public Response updateFileMetadata(@Context ContainerRequestContext crc, @FormDataParam("jsonData") String jsonData, - @PathParam("id") String fileIdOrPersistentId, - @PathParam("datasetVersionId") String datasetVersionId) { - DataverseRequest req; - try { - req = createDataverseRequest(getRequestUser(crc)); - } catch (Exception e) { - return error(BAD_REQUEST, "Error attempting to request information. Maybe a bad API token?"); - } - final DataFile df; - FileMetadata fm = null; - try { - df = execCommand(new GetDataFileCommand(req, findDataFileOrDie(fileIdOrPersistentId))); - fm = df.getFileMetadata(); - for (FileMetadata md : df.getFileMetadatas()) { - if (datasetVersionId.equals(String.valueOf(md.getDatasetVersion().getId())) || - datasetVersionId.equals(md.getDatasetVersion().getFriendlyVersionNumber())) { - fm = md; - } - } - } catch (Exception e) { - return error(BAD_REQUEST, "Error attempting get the requested data file."); - } - try { - OptionalFileParams optionalFileParams = null; - if (jsonData != null) { - // Load optional params via JSON - optionalFileParams = new OptionalFileParams(jsonData); - } else { - return error(BAD_REQUEST, "Missing Form Data!"); - } - List fmdListMinusCurrentFile = new ArrayList<>(); - for (FileMetadata fileMetadata : df.getFileMetadatas()) { - if (!fileMetadata.getDataFile().equals(df)) { - fmdListMinusCurrentFile.add(fileMetadata); - } - } - jakarta.json.JsonObject jsonObject = JsonUtil.getJsonObject(jsonData); - String incomingLabel = jsonObject.getString("label", null); - String incomingDirectoryLabel = jsonObject.getString("directoryLabel", null); - String existingLabel = fm.getLabel(); - String existingDirectoryLabel = fm.getDirectoryLabel(); - String pathPlusFilename = IngestUtil.getPathAndFileNameToCheck(incomingLabel, incomingDirectoryLabel, existingLabel, existingDirectoryLabel); - if (IngestUtil.conflictsWithExistingFilenames(pathPlusFilename, fmdListMinusCurrentFile)) { - return error(BAD_REQUEST, BundleUtil.getStringFromBundle("files.api.metadata.update.duplicateFile", Arrays.asList(pathPlusFilename))); - } - - optionalFileParams.addOptionalParams(fm); - execCommand(new UpdateDatasetVersionCommand(fm.getDataFile().getOwner(), req, fm)); - - } catch (DataFileTagException ex) { - return error(BAD_REQUEST, ex.getMessage()); - } catch (Exception e) { - return error(Response.Status.INTERNAL_SERVER_ERROR, "Error updating metadata to DataFile: " + e); - } - - String jsonString = fm.asGsonObject(true).toString(); - return Response - .status(Response.Status.OK) - .entity("File Metadata update has been completed: " + jsonString) - .type(MediaType.TEXT_PLAIN) //Our plain text string is already json - .build(); - } @GET @AuthRequired @Path("{id}") diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index d7fd94bf65c..18e2406c2a9 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -1945,6 +1945,7 @@ file.metaData.dataFile.dataTab.unf=UNF file.metaData.dataFile.dataTab.variables=Variables file.metaData.dataFile.dataTab.observations=Observations file.metaData.fileAccess=File Access: +file.metadata.message.parallelUpdateError=Changes cannot be saved. This metadata has been edited since this page was opened. To continue, copy your changes, refresh the page to see the recent updates, and re-enter any changes you want to save. file.addDescription=Add file description... file.tags=Tags file.editTags=Edit Tags diff --git a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java index de940c30e17..91246552886 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -3190,11 +3190,8 @@ public void testFileCitationByVersion() throws IOException { } - // This test handles both updating with empty fields to clear the field and - // update a specific file metadata for dataset version - // Both APIs will be tested /api/files/{id}/metadata and /api/files/{id}/metadata/version/{datasetVersionId} @Test - public void testUpdateSpecificMetadataVersionAndTestUpdateWithEmptyFields() { + public void testUpdateWithEmptyFieldsAndVersionCheck() { // Create User, Dataverse, and Dataset Response createUser = UtilIT.createRandomUser(); createUser.then().assertThat().statusCode(OK.getStatusCode()); @@ -3245,53 +3242,12 @@ public void testUpdateSpecificMetadataVersionAndTestUpdateWithEmptyFields() { publishResponse = UtilIT.publishDatasetViaNativeApi(datasetId, "major", apiToken); publishResponse.then().assertThat().statusCode(OK.getStatusCode()); - // Create a new DRAFT version - json = Json.createObjectBuilder() - .add(OptionalFileParams.DESCRIPTION_ATTR_NAME, "my 1.1 description"); - Response updateResponse = UtilIT.updateFileMetadata(String.valueOf(fileId), json.build().toString(), apiToken); - updateResponse.then().assertThat().statusCode(OK.getStatusCode()); - - // Publish the draft version as 1.1 - publishResponse = UtilIT.publishDatasetViaNativeApi(datasetId, "minor", apiToken); - publishResponse.then().assertThat().statusCode(OK.getStatusCode()); - publishResponse.prettyPrint(); - - // We now have a 1.0 version and 1.1 version. Now we modify the 1.0 version - String version = "1.0"; - json = Json.createObjectBuilder() - .add(OptionalFileParams.DESCRIPTION_ATTR_NAME, "") - .add(OptionalFileParams.LABEL_ATTR_NAME, "test.tab") - .add(OptionalFileParams.DIRECTORY_LABEL_ATTR_NAME, "") - .add(OptionalFileParams.PROVENANCE_FREEFORM_ATTR_NAME, "") - .add(OptionalFileParams.CATEGORIES_ATTR_NAME, Json.createArrayBuilder()) - .add(OptionalFileParams.FILE_DATA_TAGS_ATTR_NAME, Json.createArrayBuilder()); - - Response updateFile = UtilIT.updateFileMetadata(String.valueOf(fileId), json.build().toString(), apiToken, version); - updateFile.prettyPrint(); - - // Get version 1.0 and see the changes - getFile = UtilIT.getFileData(String.valueOf(fileId), apiToken, version); - getFile.prettyPrint(); - getFile.then().assertThat() - .statusCode(OK.getStatusCode()) - .body("data.description", equalTo("")) - .body("data.dataFile.description", equalTo("")) - .body("data.directoryLabel", nullValue()) - .body("data.provFreeForm", nullValue()) - .body("data.categories", nullValue()) - .body("data.dataFile.tabularTags", nullValue()); - - // Get the latest version and see the original data unchanged (except description which is why the draft version was created) + // Get the base version getFile = UtilIT.getFileData(String.valueOf(fileId), apiToken); getFile.prettyPrint(); - getFile.then().assertThat() - .statusCode(OK.getStatusCode()) - .body("data.description", equalTo("my 1.1 description")) - .body("data.dataFile.description", equalTo("my 1.1 description")) - .body("data.directoryLabel", equalTo("data/subdir1")) - .body("data.categories", hasItem("Data")); + String datasetVersionId = String.valueOf(JsonPath.from(getFile.body().asString()).getInt("data.datasetVersionId")); - // Update metadata (creating a draft version) without the version in the path to show that this endpoint also clears the fields + // first user updates which creates a new DRAFT version json = Json.createObjectBuilder() .add(OptionalFileParams.DESCRIPTION_ATTR_NAME, "") .add(OptionalFileParams.LABEL_ATTR_NAME, "test.tab") @@ -3299,8 +3255,11 @@ public void testUpdateSpecificMetadataVersionAndTestUpdateWithEmptyFields() { .add(OptionalFileParams.PROVENANCE_FREEFORM_ATTR_NAME, "") .add(OptionalFileParams.CATEGORIES_ATTR_NAME, Json.createArrayBuilder()) .add(OptionalFileParams.FILE_DATA_TAGS_ATTR_NAME, Json.createArrayBuilder()); - updateFile = UtilIT.updateFileMetadata(String.valueOf(fileId), json.build().toString(), apiToken); - updateFile.prettyPrint(); + Response updateResponse = UtilIT.updateFileMetadata(String.valueOf(fileId), json.build().toString(), apiToken, datasetVersionId); + updateResponse.prettyPrint(); + updateResponse.then().assertThat().statusCode(OK.getStatusCode()); + + // Get the latest version getFile = UtilIT.getFileData(String.valueOf(fileId), apiToken); getFile.prettyPrint(); getFile.then().assertThat() @@ -3311,5 +3270,15 @@ public void testUpdateSpecificMetadataVersionAndTestUpdateWithEmptyFields() { .body("data.provFreeForm", nullValue()) .body("data.categories", nullValue()) .body("data.dataFile.tabularTags", nullValue()); + + // Second user updates the base version which should fail since it's already been updated + json = Json.createObjectBuilder() + .add(OptionalFileParams.DESCRIPTION_ATTR_NAME, "my new description"); + updateResponse = UtilIT.updateFileMetadata(String.valueOf(fileId), json.build().toString(), apiToken, datasetVersionId); + updateResponse.prettyPrint(); + updateResponse.then().assertThat() + .body("status", equalTo(ApiConstants.STATUS_ERROR)) + .body("message", equalTo(BundleUtil.getStringFromBundle("file.metadata.message.parallelUpdateError"))) + .statusCode(BAD_REQUEST.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 e28ece85a23..935cc91a82e 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -1101,7 +1101,7 @@ static Response updateFileMetadata(String fileIdOrPersistentId, String jsonAsStr optionalQueryParam = "?persistentId=" + fileIdOrPersistentId; } if (datasetVersionId != null) { - versionInPath = "/version/" + datasetVersionId; + optionalQueryParam = (optionalQueryParam.isEmpty() ? "?" : "&") + "datasetVersionId=" + datasetVersionId; } RequestSpecification requestSpecification = given() .header(API_TOKEN_HTTP_HEADER, apiToken); @@ -1109,7 +1109,7 @@ static Response updateFileMetadata(String fileIdOrPersistentId, String jsonAsStr requestSpecification.multiPart("jsonData", jsonAsString); } return requestSpecification - .post("/api/files/" + idInPath + "/metadata" + versionInPath + optionalQueryParam); + .post("/api/files/" + idInPath + "/metadata" + optionalQueryParam); } static Response downloadFile(Integer fileId) { From fd800fc95d8d57a0192d8bd6ac49d62531dcaa92 Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Fri, 18 Apr 2025 17:08:55 -0400 Subject: [PATCH 004/247] fix test --- src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 935cc91a82e..9a09a5edd9f 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -1094,14 +1094,13 @@ static Response updateFileMetadata(String fileIdOrPersistentId, String jsonAsStr } static Response updateFileMetadata(String fileIdOrPersistentId, String jsonAsString, String apiToken, String datasetVersionId) { String idInPath = fileIdOrPersistentId; // Assume it's a number. - String versionInPath = ""; String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. if (!NumberUtils.isCreatable(fileIdOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + fileIdOrPersistentId; } if (datasetVersionId != null) { - optionalQueryParam = (optionalQueryParam.isEmpty() ? "?" : "&") + "datasetVersionId=" + datasetVersionId; + optionalQueryParam = optionalQueryParam + (optionalQueryParam.isEmpty() ? "?" : "&") + "datasetVersionId=" + datasetVersionId; } RequestSpecification requestSpecification = given() .header(API_TOKEN_HTTP_HEADER, apiToken); From 8dcb065ebd04f64c2516ca05184502b52f8231a4 Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Tue, 22 Apr 2025 09:44:41 -0400 Subject: [PATCH 005/247] fix test --- src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java index 91246552886..81acfe67b6e 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -3255,7 +3255,7 @@ public void testUpdateWithEmptyFieldsAndVersionCheck() { .add(OptionalFileParams.PROVENANCE_FREEFORM_ATTR_NAME, "") .add(OptionalFileParams.CATEGORIES_ATTR_NAME, Json.createArrayBuilder()) .add(OptionalFileParams.FILE_DATA_TAGS_ATTR_NAME, Json.createArrayBuilder()); - Response updateResponse = UtilIT.updateFileMetadata(String.valueOf(fileId), json.build().toString(), apiToken, datasetVersionId); + Response updateResponse = UtilIT.updateFileMetadata(String.valueOf(fileId), json.build().toString(), apiToken); updateResponse.prettyPrint(); updateResponse.then().assertThat().statusCode(OK.getStatusCode()); From 2fce5fdc7498f83945dff07961d152913b96b2dd Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Tue, 22 Apr 2025 13:49:04 -0400 Subject: [PATCH 006/247] add to test --- .../java/edu/harvard/iq/dataverse/api/FilesIT.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java index 81acfe67b6e..60a65418a7a 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -3280,5 +3280,16 @@ public void testUpdateWithEmptyFieldsAndVersionCheck() { .body("status", equalTo(ApiConstants.STATUS_ERROR)) .body("message", equalTo(BundleUtil.getStringFromBundle("file.metadata.message.parallelUpdateError"))) .statusCode(BAD_REQUEST.getStatusCode()); + + // Second user refreshes and updates. Should pass now + getFile = UtilIT.getFileData(String.valueOf(fileId), apiToken); + getFile.prettyPrint(); + getFile.then().assertThat() + .statusCode(OK.getStatusCode()); + datasetVersionId = String.valueOf(JsonPath.from(getFile.body().asString()).getInt("data.datasetVersionId")); + updateResponse = UtilIT.updateFileMetadata(String.valueOf(fileId), json.build().toString(), apiToken, datasetVersionId); + updateResponse.prettyPrint(); + updateResponse.then().assertThat() + .statusCode(OK.getStatusCode()); } } From 26f07b726c2ccd48e395b8d4f9cd0e3b288e4427 Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Tue, 22 Apr 2025 17:13:17 -0400 Subject: [PATCH 007/247] adding info for debugging jenkins test failure --- src/main/java/edu/harvard/iq/dataverse/api/Files.java | 7 +++++-- src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Files.java b/src/main/java/edu/harvard/iq/dataverse/api/Files.java index cd106d728b1..4fd9a8eaf2d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -430,8 +430,11 @@ public Response updateFileMetadata(@Context ContainerRequestContext crc, @FormDa } Long latestDatasetVersion = df.getFileMetadata().getDatasetVersion().getId(); - if (datasetVersionId != null && latestDatasetVersion != Long.valueOf(datasetVersionId)) { - return error(Response.Status.BAD_REQUEST, BundleUtil.getStringFromBundle("file.metadata.message.parallelUpdateError")); + if (datasetVersionId != null && latestDatasetVersion != null && + latestDatasetVersion != Long.valueOf(datasetVersionId)) { + return error(Response.Status.BAD_REQUEST, BundleUtil.getStringFromBundle("file.metadata.message.parallelUpdateError") + + " latestDatasetVersion:" + (latestDatasetVersion != null ? String.valueOf(latestDatasetVersion):"null") + + " datasetVersionId:" + datasetVersionId); } //You shouldn't be trying to edit a datafile that has been replaced diff --git a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java index 60a65418a7a..aca90f95bfe 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -3278,7 +3278,7 @@ public void testUpdateWithEmptyFieldsAndVersionCheck() { updateResponse.prettyPrint(); updateResponse.then().assertThat() .body("status", equalTo(ApiConstants.STATUS_ERROR)) - .body("message", equalTo(BundleUtil.getStringFromBundle("file.metadata.message.parallelUpdateError"))) + .body("message", startsWith(BundleUtil.getStringFromBundle("file.metadata.message.parallelUpdateError"))) .statusCode(BAD_REQUEST.getStatusCode()); // Second user refreshes and updates. Should pass now From 10939ce17d3501d91632efe3ad8b6a5f80b88772 Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Wed, 23 Apr 2025 09:08:09 -0400 Subject: [PATCH 008/247] remove jenkins debug --- src/main/java/edu/harvard/iq/dataverse/api/Files.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Files.java b/src/main/java/edu/harvard/iq/dataverse/api/Files.java index 4fd9a8eaf2d..fcbf6ef21e0 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -429,12 +429,10 @@ public Response updateFileMetadata(@Context ContainerRequestContext crc, @FormDa return error(BAD_REQUEST, "Error attempting get the requested data file."); } - Long latestDatasetVersion = df.getFileMetadata().getDatasetVersion().getId(); + String latestDatasetVersion = String.valueOf(df.getFileMetadata().getDatasetVersion().getId()); if (datasetVersionId != null && latestDatasetVersion != null && - latestDatasetVersion != Long.valueOf(datasetVersionId)) { - return error(Response.Status.BAD_REQUEST, BundleUtil.getStringFromBundle("file.metadata.message.parallelUpdateError") + - " latestDatasetVersion:" + (latestDatasetVersion != null ? String.valueOf(latestDatasetVersion):"null") + - " datasetVersionId:" + datasetVersionId); + !latestDatasetVersion.equals(datasetVersionId)) { + return error(Response.Status.BAD_REQUEST, BundleUtil.getStringFromBundle("file.metadata.message.parallelUpdateError")); } //You shouldn't be trying to edit a datafile that has been replaced From f5ddcff8a466122e2e2a11c7c521e18a21cd9209 Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Thu, 24 Apr 2025 10:43:08 -0400 Subject: [PATCH 009/247] per review comments --- doc/sphinx-guides/source/api/native-api.rst | 6 +++--- .../harvard/iq/dataverse/api/AbstractApiBean.java | 9 +++++++++ .../java/edu/harvard/iq/dataverse/api/Files.java | 12 +++++++----- src/main/java/propertyFiles/Bundle.properties | 2 +- .../java/edu/harvard/iq/dataverse/api/FilesIT.java | 2 +- .../java/edu/harvard/iq/dataverse/api/UtilIT.java | 2 +- 6 files changed, 22 insertions(+), 11 deletions(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 9bf7b3679bf..c397b967be3 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -4612,7 +4612,7 @@ Updating File Metadata Updates the file metadata for an existing file where ``ID`` is the database id of the file to update or ``PERSISTENT_ID`` is the persistent id (DOI or Handle) of the file. Requires a ``jsonString`` expressing the new metadata. No metadata from the previous version of this file will be persisted, so if you want to update a specific field first get the json with the above command and alter the fields you want. -Optional Parameter for verifying that the Dataset Version being edited is the latest version can be added &datasetVersionId=12345. This is to prevent stale data from being edited. +Optional Parameter for verifying that the Dataset Version being edited is the latest version can be added &sourceInternalVersionNumber=12345. This is to prevent stale data from being edited. The value for sourceInternalVersionNumber comes from ``datasetVersionId`` in the response to get $SERVER_URL/api/files/$ID API call A curl example using an ``ID`` @@ -4645,7 +4645,7 @@ A curl example using a ``PERSISTENT_ID`` curl -H "X-Dataverse-key:$API_TOKEN" -X POST \ -F 'jsonData={"description":"My description bbb.","provFreeform":"Test prov freeform","categories":["Data"],"dataFileTags":["Survey"],"restrict":false}' \ - "$SERVER_URL/api/files/:persistentId/metadata?persistentId=$PERSISTENT_ID&datasetVersionId=$VERSION_ID" + "$SERVER_URL/api/files/:persistentId/metadata?persistentId=$PERSISTENT_ID&sourceInternalVersionNumber=$VERSION_ID" The fully expanded example above (without environment variables) looks like this: @@ -4653,7 +4653,7 @@ The fully expanded example above (without environment variables) looks like this curl -H "X-Dataverse-key:xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X POST \ -F 'jsonData={"description":"My description bbb.","provFreeform":"Test prov freeform","categories":["Data"],"dataFileTags":["Survey"],"restrict":false}' \ - "https://demo.dataverse.org/api/files/:persistentId/metadata?persistentId=doi:10.5072/FK2/AAA000&datasetVersionId=12345" + "https://demo.dataverse.org/api/files/:persistentId/metadata?persistentId=doi:10.5072/FK2/AAA000&sourceInternalVersionNumber=12345" Note: To update the 'tabularTags' property of file metadata, use the 'dataFileTags' key when making API requests. This property is used to update the 'tabularTags' of the file metadata. diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index 8841ce3563b..d6391133240 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -451,6 +451,15 @@ protected void validateInternalVersionNumberIsNotOutdated(Dataset dataset, int i } } + protected void validateInternalVersionNumberIsNotOutdated(DataFile dataFile, int internalVersion) throws WrappedResponse { + logger.severe(">>>> internalVersion:"+internalVersion+" dataFile.getFileMetadata().getDatasetVersion().getId():"+dataFile.getFileMetadata().getDatasetVersion().getId()); + if (dataFile.getFileMetadata().getDatasetVersion().getId() > internalVersion) { + throw new WrappedResponse( + badRequest(BundleUtil.getStringFromBundle("abstractApiBean.error.datafileInternalVersionNumberIsOutdated", Collections.singletonList(Integer.toString(internalVersion)))) + ); + } + } + protected DataFile findDataFileOrDie(String id) throws WrappedResponse { DataFile datafile; if (id.equals(PERSISTENT_ID_KEY)) { diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Files.java b/src/main/java/edu/harvard/iq/dataverse/api/Files.java index fcbf6ef21e0..33b380df865 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -410,7 +410,7 @@ public Response deleteFileInDataset(@Context ContainerRequestContext crc, @PathP @AuthRequired @Path("{id}/metadata") public Response updateFileMetadata(@Context ContainerRequestContext crc, @FormDataParam("jsonData") String jsonData, - @PathParam("id") String fileIdOrPersistentId, @QueryParam("datasetVersionId") String datasetVersionId + @PathParam("id") String fileIdOrPersistentId, @QueryParam("sourceInternalVersionNumber") Integer sourceInternalVersionNumber ) throws CommandException { FileMetadata upFmd = null; @@ -429,10 +429,12 @@ public Response updateFileMetadata(@Context ContainerRequestContext crc, @FormDa return error(BAD_REQUEST, "Error attempting get the requested data file."); } - String latestDatasetVersion = String.valueOf(df.getFileMetadata().getDatasetVersion().getId()); - if (datasetVersionId != null && latestDatasetVersion != null && - !latestDatasetVersion.equals(datasetVersionId)) { - return error(Response.Status.BAD_REQUEST, BundleUtil.getStringFromBundle("file.metadata.message.parallelUpdateError")); + if (sourceInternalVersionNumber != null) { + try { + validateInternalVersionNumberIsNotOutdated(df, sourceInternalVersionNumber); + } catch (WrappedResponse wr) { + return wr.getResponse(); + } } //You shouldn't be trying to edit a datafile that has been replaced diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index 18e2406c2a9..09ea8d2e378 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -1945,7 +1945,6 @@ file.metaData.dataFile.dataTab.unf=UNF file.metaData.dataFile.dataTab.variables=Variables file.metaData.dataFile.dataTab.observations=Observations file.metaData.fileAccess=File Access: -file.metadata.message.parallelUpdateError=Changes cannot be saved. This metadata has been edited since this page was opened. To continue, copy your changes, refresh the page to see the recent updates, and re-enter any changes you want to save. file.addDescription=Add file description... file.tags=Tags file.editTags=Edit Tags @@ -3206,3 +3205,4 @@ updateDatasetFieldsCommand.api.processDatasetUpdate.parseError=Error parsing dat #AbstractApiBean.java abstractApiBean.error.datasetInternalVersionNumberIsOutdated=Dataset internal version number {0} is outdated +abstractApiBean.error.datafileInternalVersionNumberIsOutdated=File Metadata internal version number {0} is outdated diff --git a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java index aca90f95bfe..d8d4cf80535 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -3278,7 +3278,7 @@ public void testUpdateWithEmptyFieldsAndVersionCheck() { updateResponse.prettyPrint(); updateResponse.then().assertThat() .body("status", equalTo(ApiConstants.STATUS_ERROR)) - .body("message", startsWith(BundleUtil.getStringFromBundle("file.metadata.message.parallelUpdateError"))) + .body("message", equalTo(BundleUtil.getStringFromBundle("abstractApiBean.error.datafileInternalVersionNumberIsOutdated",Collections.singletonList(datasetVersionId)))) .statusCode(BAD_REQUEST.getStatusCode()); // Second user refreshes and updates. Should pass now 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 9a09a5edd9f..f9e97022d2e 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -1100,7 +1100,7 @@ static Response updateFileMetadata(String fileIdOrPersistentId, String jsonAsStr optionalQueryParam = "?persistentId=" + fileIdOrPersistentId; } if (datasetVersionId != null) { - optionalQueryParam = optionalQueryParam + (optionalQueryParam.isEmpty() ? "?" : "&") + "datasetVersionId=" + datasetVersionId; + optionalQueryParam = optionalQueryParam + (optionalQueryParam.isEmpty() ? "?" : "&") + "sourceInternalVersionNumber=" + datasetVersionId; } RequestSpecification requestSpecification = given() .header(API_TOKEN_HTTP_HEADER, apiToken); From 1615fb91238a08194cf7ea9a760c177cfd2e5ddc Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Thu, 24 Apr 2025 10:44:18 -0400 Subject: [PATCH 010/247] per review comments --- src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index d6391133240..7811eb68244 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -452,7 +452,6 @@ protected void validateInternalVersionNumberIsNotOutdated(Dataset dataset, int i } protected void validateInternalVersionNumberIsNotOutdated(DataFile dataFile, int internalVersion) throws WrappedResponse { - logger.severe(">>>> internalVersion:"+internalVersion+" dataFile.getFileMetadata().getDatasetVersion().getId():"+dataFile.getFileMetadata().getDatasetVersion().getId()); if (dataFile.getFileMetadata().getDatasetVersion().getId() > internalVersion) { throw new WrappedResponse( badRequest(BundleUtil.getStringFromBundle("abstractApiBean.error.datafileInternalVersionNumberIsOutdated", Collections.singletonList(Integer.toString(internalVersion)))) From 8a57fc8d9d5b7a346ea59bd471f42c3322d8acc6 Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Fri, 25 Apr 2025 11:02:11 -0400 Subject: [PATCH 011/247] refactor to use last update timestamp instead of version number --- .../11392-edit-file-metadata-empty-values.md | 2 +- doc/sphinx-guides/source/api/native-api.rst | 8 ++--- .../iq/dataverse/api/AbstractApiBean.java | 15 +++++++-- .../edu/harvard/iq/dataverse/api/Files.java | 6 ++-- .../iq/dataverse/util/json/JsonPrinter.java | 3 +- src/main/java/propertyFiles/Bundle.properties | 2 +- .../edu/harvard/iq/dataverse/api/FilesIT.java | 32 ++++++++++++------- .../edu/harvard/iq/dataverse/api/UtilIT.java | 6 ++-- 8 files changed, 47 insertions(+), 27 deletions(-) diff --git a/doc/release-notes/11392-edit-file-metadata-empty-values.md b/doc/release-notes/11392-edit-file-metadata-empty-values.md index e42bef13aec..652aea63aed 100644 --- a/doc/release-notes/11392-edit-file-metadata-empty-values.md +++ b/doc/release-notes/11392-edit-file-metadata-empty-values.md @@ -2,6 +2,6 @@ Previously the API POST /files/{id}/metadata would ignore fields with empty values. Now the API updates the fields with the empty values essentially clearing the data. Missing fields will still be ignored. -An optional query parameter was added to ensure the metadata update doesn't overwrite stale data. +An optional query parameter (sourceInternalVersionTimestamp) was added to ensure the metadata update doesn't overwrite stale data. See also [the guides](https://dataverse-guide--11359.org.readthedocs.build/en/11359/api/native-api.html#updating-file-metadata), #11392, and #11359. diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index c397b967be3..a27b7ae3269 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -4612,7 +4612,7 @@ Updating File Metadata Updates the file metadata for an existing file where ``ID`` is the database id of the file to update or ``PERSISTENT_ID`` is the persistent id (DOI or Handle) of the file. Requires a ``jsonString`` expressing the new metadata. No metadata from the previous version of this file will be persisted, so if you want to update a specific field first get the json with the above command and alter the fields you want. -Optional Parameter for verifying that the Dataset Version being edited is the latest version can be added &sourceInternalVersionNumber=12345. This is to prevent stale data from being edited. The value for sourceInternalVersionNumber comes from ``datasetVersionId`` in the response to get $SERVER_URL/api/files/$ID API call +Optional Parameter for verifying that the Dataset Version being edited is the latest version can be added &sourceInternalVersionTimestamp=datetime(in format: "yyyy-MM-dd'T'HH:mm:ss'Z'"). This is to prevent stale data from being edited. The value for sourceInternalVersionTimestamp comes from ``lastUpdateTime`` in the response to get $SERVER_URL/api/files/$ID API call A curl example using an ``ID`` @@ -4641,11 +4641,11 @@ A curl example using a ``PERSISTENT_ID`` export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx export SERVER_URL=https://demo.dataverse.org export PERSISTENT_ID=doi:10.5072/FK2/AAA000 - export VERSION_ID=12345 + export UPDATE_TIME=2025-04-25T13:58:28Z curl -H "X-Dataverse-key:$API_TOKEN" -X POST \ -F 'jsonData={"description":"My description bbb.","provFreeform":"Test prov freeform","categories":["Data"],"dataFileTags":["Survey"],"restrict":false}' \ - "$SERVER_URL/api/files/:persistentId/metadata?persistentId=$PERSISTENT_ID&sourceInternalVersionNumber=$VERSION_ID" + "$SERVER_URL/api/files/:persistentId/metadata?persistentId=$PERSISTENT_ID&sourceInternalVersionTimestamp=$UPDATE_TIME" The fully expanded example above (without environment variables) looks like this: @@ -4653,7 +4653,7 @@ The fully expanded example above (without environment variables) looks like this curl -H "X-Dataverse-key:xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X POST \ -F 'jsonData={"description":"My description bbb.","provFreeform":"Test prov freeform","categories":["Data"],"dataFileTags":["Survey"],"restrict":false}' \ - "https://demo.dataverse.org/api/files/:persistentId/metadata?persistentId=doi:10.5072/FK2/AAA000&sourceInternalVersionNumber=12345" + "https://demo.dataverse.org/api/files/:persistentId/metadata?persistentId=doi:10.5072/FK2/AAA000&sourceInternalVersionTimestamp=2025-04-25T13:58:28Z" Note: To update the 'tabularTags' property of file metadata, use the 'dataFileTags' key when making API requests. This property is used to update the 'tabularTags' of the file metadata. diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index 7811eb68244..0a7200662db 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -27,6 +27,7 @@ import edu.harvard.iq.dataverse.search.savedsearch.SavedSearchServiceBean; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; import edu.harvard.iq.dataverse.util.BundleUtil; +import edu.harvard.iq.dataverse.util.DateUtil; import edu.harvard.iq.dataverse.util.FileUtil; import edu.harvard.iq.dataverse.util.SystemConfig; import edu.harvard.iq.dataverse.util.json.JsonParser; @@ -51,6 +52,7 @@ import java.io.InputStream; import java.net.URI; +import java.time.Instant; import java.util.*; import java.util.concurrent.Callable; import java.util.logging.Level; @@ -451,10 +453,17 @@ protected void validateInternalVersionNumberIsNotOutdated(Dataset dataset, int i } } - protected void validateInternalVersionNumberIsNotOutdated(DataFile dataFile, int internalVersion) throws WrappedResponse { - if (dataFile.getFileMetadata().getDatasetVersion().getId() > internalVersion) { + protected void validateInternalTimestampIsNotOutdated(DataFile dataFile, String sourceInternalVersionTimestamp) throws WrappedResponse { + Date date = sourceInternalVersionTimestamp != null ? DateUtil.parseDate(sourceInternalVersionTimestamp, "yyyy-MM-dd'T'HH:mm:ss'Z'") : null; + if (date == null) { throw new WrappedResponse( - badRequest(BundleUtil.getStringFromBundle("abstractApiBean.error.datafileInternalVersionNumberIsOutdated", Collections.singletonList(Integer.toString(internalVersion)))) + badRequest(BundleUtil.getStringFromBundle("jsonparser.error.parsing.date", Collections.singletonList(sourceInternalVersionTimestamp))) + ); + } + Instant instant = date.toInstant(); + if (dataFile.getFileMetadata().getDatasetVersion().getLastUpdateTime().toInstant().getEpochSecond() != instant.getEpochSecond()) { + throw new WrappedResponse( + badRequest(BundleUtil.getStringFromBundle("abstractApiBean.error.datafileInternalVersionTimestampIsOutdated", Collections.singletonList(sourceInternalVersionTimestamp))) ); } } diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Files.java b/src/main/java/edu/harvard/iq/dataverse/api/Files.java index 33b380df865..b38d5375f66 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -410,7 +410,7 @@ public Response deleteFileInDataset(@Context ContainerRequestContext crc, @PathP @AuthRequired @Path("{id}/metadata") public Response updateFileMetadata(@Context ContainerRequestContext crc, @FormDataParam("jsonData") String jsonData, - @PathParam("id") String fileIdOrPersistentId, @QueryParam("sourceInternalVersionNumber") Integer sourceInternalVersionNumber + @PathParam("id") String fileIdOrPersistentId, @QueryParam("sourceInternalVersionTimestamp") String sourceInternalVersionTimestamp ) throws CommandException { FileMetadata upFmd = null; @@ -429,9 +429,9 @@ public Response updateFileMetadata(@Context ContainerRequestContext crc, @FormDa return error(BAD_REQUEST, "Error attempting get the requested data file."); } - if (sourceInternalVersionNumber != null) { + if (sourceInternalVersionTimestamp != null) { try { - validateInternalVersionNumberIsNotOutdated(df, sourceInternalVersionNumber); + validateInternalTimestampIsNotOutdated(df, sourceInternalVersionTimestamp); } catch (WrappedResponse wr) { return wr.getResponse(); } 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 0b5803d75d1..cc5bef0f4a3 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 @@ -873,7 +873,8 @@ public static JsonObjectBuilder json(DataFile df, FileMetadata fileMetadata, boo .add("tabularData", df.isTabularData()) .add("tabularTags", getTabularFileTags(df)) .add("creationDate", df.getCreateDateFormattedYYYYMMDD()) - .add("publicationDate", df.getPublicationDateFormattedYYYYMMDD()); + .add("publicationDate", df.getPublicationDateFormattedYYYYMMDD()) + .add("lastUpdateTime", format(fileMetadata.getDatasetVersion().getLastUpdateTime())); Dataset dfOwner = df.getOwner(); if (dfOwner != null) { builder.add("fileAccessRequest", dfOwner.isFileAccessRequest()); diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index 09ea8d2e378..1036e1a836c 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -3205,4 +3205,4 @@ updateDatasetFieldsCommand.api.processDatasetUpdate.parseError=Error parsing dat #AbstractApiBean.java abstractApiBean.error.datasetInternalVersionNumberIsOutdated=Dataset internal version number {0} is outdated -abstractApiBean.error.datafileInternalVersionNumberIsOutdated=File Metadata internal version number {0} is outdated +abstractApiBean.error.datafileInternalVersionTimestampIsOutdated=File Metadata internal version timestamp {0} is outdated diff --git a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java index d8d4cf80535..de45362ce98 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -5,7 +5,9 @@ import io.restassured.RestAssured; import io.restassured.response.Response; -import java.util.List; +import java.text.ParseException; +import java.text.SimpleDateFormat; +import java.util.*; import java.util.logging.Logger; import edu.harvard.iq.dataverse.api.auth.ApiKeyAuthMechanism; @@ -29,9 +31,6 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.text.MessageFormat; -import java.util.Arrays; -import java.util.Collections; -import java.util.Map; import jakarta.json.Json; import jakarta.json.JsonObjectBuilder; @@ -3191,7 +3190,7 @@ public void testFileCitationByVersion() throws IOException { } @Test - public void testUpdateWithEmptyFieldsAndVersionCheck() { + public void testUpdateWithEmptyFieldsAndVersionCheck() throws InterruptedException { // Create User, Dataverse, and Dataset Response createUser = UtilIT.createRandomUser(); createUser.then().assertThat().statusCode(OK.getStatusCode()); @@ -3245,7 +3244,7 @@ public void testUpdateWithEmptyFieldsAndVersionCheck() { // Get the base version getFile = UtilIT.getFileData(String.valueOf(fileId), apiToken); getFile.prettyPrint(); - String datasetVersionId = String.valueOf(JsonPath.from(getFile.body().asString()).getInt("data.datasetVersionId")); + String lastUpdateTime = String.valueOf(JsonPath.from(getFile.body().asString()).getString("data.dataFile.lastUpdateTime")); // first user updates which creates a new DRAFT version json = Json.createObjectBuilder() @@ -3255,9 +3254,10 @@ public void testUpdateWithEmptyFieldsAndVersionCheck() { .add(OptionalFileParams.PROVENANCE_FREEFORM_ATTR_NAME, "") .add(OptionalFileParams.CATEGORIES_ATTR_NAME, Json.createArrayBuilder()) .add(OptionalFileParams.FILE_DATA_TAGS_ATTR_NAME, Json.createArrayBuilder()); - Response updateResponse = UtilIT.updateFileMetadata(String.valueOf(fileId), json.build().toString(), apiToken); + Response updateResponse = UtilIT.updateFileMetadata(String.valueOf(fileId), json.build().toString(), apiToken, lastUpdateTime); updateResponse.prettyPrint(); updateResponse.then().assertThat().statusCode(OK.getStatusCode()); + Thread.sleep(1500); // Get the latest version getFile = UtilIT.getFileData(String.valueOf(fileId), apiToken); @@ -3270,15 +3270,17 @@ public void testUpdateWithEmptyFieldsAndVersionCheck() { .body("data.provFreeForm", nullValue()) .body("data.categories", nullValue()) .body("data.dataFile.tabularTags", nullValue()); + String latestUpdateTime = String.valueOf(JsonPath.from(getFile.body().asString()).getString("data.dataFile.lastUpdateTime")); + assertTrue(!latestUpdateTime.equalsIgnoreCase(lastUpdateTime)); // Second user updates the base version which should fail since it's already been updated json = Json.createObjectBuilder() .add(OptionalFileParams.DESCRIPTION_ATTR_NAME, "my new description"); - updateResponse = UtilIT.updateFileMetadata(String.valueOf(fileId), json.build().toString(), apiToken, datasetVersionId); + updateResponse = UtilIT.updateFileMetadata(String.valueOf(fileId), json.build().toString(), apiToken, lastUpdateTime); updateResponse.prettyPrint(); updateResponse.then().assertThat() .body("status", equalTo(ApiConstants.STATUS_ERROR)) - .body("message", equalTo(BundleUtil.getStringFromBundle("abstractApiBean.error.datafileInternalVersionNumberIsOutdated",Collections.singletonList(datasetVersionId)))) + .body("message", equalTo(BundleUtil.getStringFromBundle("abstractApiBean.error.datafileInternalVersionTimestampIsOutdated",Collections.singletonList(lastUpdateTime)))) .statusCode(BAD_REQUEST.getStatusCode()); // Second user refreshes and updates. Should pass now @@ -3286,10 +3288,18 @@ public void testUpdateWithEmptyFieldsAndVersionCheck() { getFile.prettyPrint(); getFile.then().assertThat() .statusCode(OK.getStatusCode()); - datasetVersionId = String.valueOf(JsonPath.from(getFile.body().asString()).getInt("data.datasetVersionId")); - updateResponse = UtilIT.updateFileMetadata(String.valueOf(fileId), json.build().toString(), apiToken, datasetVersionId); + lastUpdateTime = String.valueOf(JsonPath.from(getFile.body().asString()).getString("data.dataFile.lastUpdateTime")); + updateResponse = UtilIT.updateFileMetadata(String.valueOf(fileId), json.build().toString(), apiToken, lastUpdateTime); updateResponse.prettyPrint(); updateResponse.then().assertThat() .statusCode(OK.getStatusCode()); + + // Test invalid date + updateResponse = UtilIT.updateFileMetadata(String.valueOf(fileId), json.build().toString(), apiToken, "bad-date"); + updateResponse.prettyPrint(); + updateResponse.then().assertThat() + .body("status", equalTo(ApiConstants.STATUS_ERROR)) + .body("message", equalTo(BundleUtil.getStringFromBundle("jsonparser.error.parsing.date",Collections.singletonList("bad-date")))) + .statusCode(BAD_REQUEST.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 f9e97022d2e..04d0b27d63c 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -1092,15 +1092,15 @@ static Response deleteFileApi(Integer fileId, String apiToken) { static Response updateFileMetadata(String fileIdOrPersistentId, String jsonAsString, String apiToken) { return updateFileMetadata(fileIdOrPersistentId, jsonAsString,apiToken, null); } - static Response updateFileMetadata(String fileIdOrPersistentId, String jsonAsString, String apiToken, String datasetVersionId) { + static Response updateFileMetadata(String fileIdOrPersistentId, String jsonAsString, String apiToken, String sourceInternalVersionTimestamp) { String idInPath = fileIdOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. if (!NumberUtils.isCreatable(fileIdOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + fileIdOrPersistentId; } - if (datasetVersionId != null) { - optionalQueryParam = optionalQueryParam + (optionalQueryParam.isEmpty() ? "?" : "&") + "sourceInternalVersionNumber=" + datasetVersionId; + if (sourceInternalVersionTimestamp != null) { + optionalQueryParam = optionalQueryParam + (optionalQueryParam.isEmpty() ? "?" : "&") + "sourceInternalVersionTimestamp=" + sourceInternalVersionTimestamp; } RequestSpecification requestSpecification = given() .header(API_TOKEN_HTTP_HEADER, apiToken); From 6fedf6e887d09b71618124119e8bac597a9deddd Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Fri, 25 Apr 2025 14:26:05 -0400 Subject: [PATCH 012/247] comment on data/timestamp compare --- src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index 0a7200662db..00dadb1cae4 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -461,6 +461,7 @@ protected void validateInternalTimestampIsNotOutdated(DataFile dataFile, String ); } Instant instant = date.toInstant(); + // granularity is to the second since the json output only returns dates in this format to the second if (dataFile.getFileMetadata().getDatasetVersion().getLastUpdateTime().toInstant().getEpochSecond() != instant.getEpochSecond()) { throw new WrappedResponse( badRequest(BundleUtil.getStringFromBundle("abstractApiBean.error.datafileInternalVersionTimestampIsOutdated", Collections.singletonList(sourceInternalVersionTimestamp))) From eaf49a99bd234cddffe280090781133bbfca50ba Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Tue, 29 Apr 2025 14:50:11 -0400 Subject: [PATCH 013/247] refactor so both datafiles and datasets validate update timestamp the same way --- .../11243-editmetadata-api-extension.md | 4 +++- doc/sphinx-guides/source/api/native-api.rst | 12 ++++++------ .../iq/dataverse/api/AbstractApiBean.java | 18 +++++++----------- .../edu/harvard/iq/dataverse/api/Datasets.java | 8 +++++--- .../edu/harvard/iq/dataverse/api/Files.java | 5 ++--- src/main/java/propertyFiles/Bundle.properties | 3 +-- .../harvard/iq/dataverse/api/DatasetsIT.java | 15 ++++++++------- .../edu/harvard/iq/dataverse/api/FilesIT.java | 2 +- .../edu/harvard/iq/dataverse/api/UtilIT.java | 4 ++-- 9 files changed, 35 insertions(+), 36 deletions(-) diff --git a/doc/release-notes/11243-editmetadata-api-extension.md b/doc/release-notes/11243-editmetadata-api-extension.md index 6f5a2af283b..4aaf9320074 100644 --- a/doc/release-notes/11243-editmetadata-api-extension.md +++ b/doc/release-notes/11243-editmetadata-api-extension.md @@ -1,5 +1,7 @@ ### Edit Dataset Metadata API extension - This endpoint now allows removing fields (by sending empty values), as long as they are not required by the dataset. -- New ``sourceInternalVersionNumber`` optional query parameter, which prevents inconsistencies by managing updates that +- New ``sourceInternalVersionTimestamp`` optional query parameter, which prevents inconsistencies by managing updates that may occur from other users while a dataset is being edited. + +NOTE: This release note was updated to conform to the refactoring of the validation as part of issue #11392 diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index a27b7ae3269..352234d844a 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -2137,26 +2137,26 @@ For these edits your JSON file need only include those dataset fields which you This endpoint also allows removing fields, as long as they are not required by the dataset. To remove a field, send an empty value (``""``) for individual fields. For multiple fields, send an empty array (``[]``). A sample JSON file for removing fields may be downloaded here: :download:`dataset-edit-metadata-delete-fields-sample.json <../_static/api/dataset-edit-metadata-delete-fields-sample.json>` -If another user updates the dataset version metadata before you send the update request, data inconsistencies may occur. To prevent this, you can use the optional ``sourceInternalVersionNumber`` query parameter. This parameter must include the internal version number corresponding to the dataset version being updated. Note that internal version numbers increase sequentially with each version update. +If another user updates the dataset version metadata before you send the update request, data inconsistencies may occur. To prevent this, you can use the optional ``sourceInternalVersionTimestamp`` query parameter. This parameter must include the ``lastUpdateTime`` corresponding to the dataset version being updated. The date must be in this format "yyyy-MM-dd'T'HH:mm:ss'Z'" -If this parameter is provided, the update will proceed only if the internal version number remains unchanged. Otherwise, the request will fail with an error. +If this parameter is provided, the update will proceed only if the ``lastUpdateTime`` remains unchanged. Otherwise, the request will fail with an error. -Example using ``sourceInternalVersionNumber``: +Example using ``sourceInternalVersionTimestamp``: .. code-block:: bash export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx export SERVER_URL=https://demo.dataverse.org export PERSISTENT_IDENTIFIER=doi:10.5072/FK2/BCCP9Z - export SOURCE_INTERNAL_VERSION_NUMBER=5 + export SOURCE_INTERNAL_VERSION_TIMESTAMP=2025-04-25T13:58:28Z - curl -H "X-Dataverse-key: $API_TOKEN" -X PUT "$SERVER_URL/api/datasets/:persistentId/editMetadata?persistentId=$PERSISTENT_IDENTIFIER&replace=true&sourceInternalVersionNumber=$SOURCE_INTERNAL_VERSION_NUMBER" --upload-file dataset-update-metadata.json + curl -H "X-Dataverse-key: $API_TOKEN" -X PUT "$SERVER_URL/api/datasets/:persistentId/editMetadata?persistentId=$PERSISTENT_IDENTIFIER&replace=true&sourceInternalVersionTimestamp=$SOURCE_INTERNAL_VERSION_TIMESTAMP" --upload-file dataset-update-metadata.json The fully expanded example above (without environment variables) looks like this: .. code-block:: bash - curl -H "X-Dataverse-key: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X PUT "https://demo.dataverse.org/api/datasets/:persistentId/editMetadata/?persistentId=doi:10.5072/FK2/BCCP9Z&replace=true&sourceInternalVersionNumber=5" --upload-file dataset-update-metadata.json + curl -H "X-Dataverse-key: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X PUT "https://demo.dataverse.org/api/datasets/:persistentId/editMetadata/?persistentId=doi:10.5072/FK2/BCCP9Z&replace=true&sourceInternalVersionTimestamp=2025-04-25T13:58:28Z" --upload-file dataset-update-metadata.json Delete Dataset Metadata diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index 00dadb1cae4..c53f9f8119c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -445,15 +445,7 @@ public Command handleLatestPublished() { return dsv; } - protected void validateInternalVersionNumberIsNotOutdated(Dataset dataset, int internalVersion) throws WrappedResponse { - if (dataset.getLatestVersion().getVersion() > internalVersion) { - throw new WrappedResponse( - badRequest(BundleUtil.getStringFromBundle("abstractApiBean.error.datasetInternalVersionNumberIsOutdated", Collections.singletonList(Integer.toString(internalVersion)))) - ); - } - } - - protected void validateInternalTimestampIsNotOutdated(DataFile dataFile, String sourceInternalVersionTimestamp) throws WrappedResponse { + protected void validateInternalTimestampIsNotOutdated(DvObject dvObject, String sourceInternalVersionTimestamp) throws WrappedResponse { Date date = sourceInternalVersionTimestamp != null ? DateUtil.parseDate(sourceInternalVersionTimestamp, "yyyy-MM-dd'T'HH:mm:ss'Z'") : null; if (date == null) { throw new WrappedResponse( @@ -461,10 +453,14 @@ protected void validateInternalTimestampIsNotOutdated(DataFile dataFile, String ); } Instant instant = date.toInstant(); + Instant updateTimestamp = + (dvObject instanceof DataFile) ? ((DataFile) dvObject).getFileMetadata().getDatasetVersion().getLastUpdateTime().toInstant() : + (dvObject instanceof Dataset) ? ((Dataset) dvObject).getLatestVersion().getLastUpdateTime().toInstant() : + instant; // granularity is to the second since the json output only returns dates in this format to the second - if (dataFile.getFileMetadata().getDatasetVersion().getLastUpdateTime().toInstant().getEpochSecond() != instant.getEpochSecond()) { + if (updateTimestamp.getEpochSecond() != instant.getEpochSecond()) { throw new WrappedResponse( - badRequest(BundleUtil.getStringFromBundle("abstractApiBean.error.datafileInternalVersionTimestampIsOutdated", Collections.singletonList(sourceInternalVersionTimestamp))) + badRequest(BundleUtil.getStringFromBundle("abstractApiBean.error.internalVersionTimestampIsOutdated", Collections.singletonList(sourceInternalVersionTimestamp))) ); } } diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java index bf04cdd6226..e36b50ad0e0 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -1093,12 +1093,14 @@ private String getCompoundDisplayValue (DatasetFieldCompoundValue dscv){ @PUT @AuthRequired @Path("{id}/editMetadata") - public Response editVersionMetadata(@Context ContainerRequestContext crc, String jsonBody, @PathParam("id") String id, @QueryParam("replace") boolean replaceData, @QueryParam("sourceInternalVersionNumber") Integer sourceInternalVersionNumber) { + public Response editVersionMetadata(@Context ContainerRequestContext crc, String jsonBody, @PathParam("id") String id, + @QueryParam("replace") boolean replaceData, + @QueryParam("sourceInternalVersionTimestamp") String sourceInternalVersionTimestamp) { try { Dataset dataset = findDatasetOrDie(id); - if (sourceInternalVersionNumber != null) { - validateInternalVersionNumberIsNotOutdated(dataset, sourceInternalVersionNumber); + if (sourceInternalVersionTimestamp != null) { + validateInternalTimestampIsNotOutdated(dataset, sourceInternalVersionTimestamp); } JsonObject json = JsonUtil.getJsonObject(jsonBody); diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Files.java b/src/main/java/edu/harvard/iq/dataverse/api/Files.java index b38d5375f66..1b809894e17 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -410,8 +410,7 @@ public Response deleteFileInDataset(@Context ContainerRequestContext crc, @PathP @AuthRequired @Path("{id}/metadata") public Response updateFileMetadata(@Context ContainerRequestContext crc, @FormDataParam("jsonData") String jsonData, - @PathParam("id") String fileIdOrPersistentId, @QueryParam("sourceInternalVersionTimestamp") String sourceInternalVersionTimestamp - ) throws CommandException { + @PathParam("id") String fileIdOrPersistentId, @QueryParam("sourceInternalVersionTimestamp") String sourceInternalVersionTimestamp) { FileMetadata upFmd = null; @@ -526,7 +525,7 @@ public Response updateFileMetadata(@Context ContainerRequestContext crc, @FormDa return error(Response.Status.INTERNAL_SERVER_ERROR, "Error adding metadata to DataFile: " + e); } - } catch (WrappedResponse wr) { + } catch (CommandException | WrappedResponse ex) { return error(BAD_REQUEST, "An error has occurred attempting to update the requested DataFile, likely related to permissions."); } diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index 1036e1a836c..3dae5a8ed15 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -3204,5 +3204,4 @@ datasetFieldValidator.error.emptyRequiredSingleValueForField=Empty required valu updateDatasetFieldsCommand.api.processDatasetUpdate.parseError=Error parsing dataset update: {0} #AbstractApiBean.java -abstractApiBean.error.datasetInternalVersionNumberIsOutdated=Dataset internal version number {0} is outdated -abstractApiBean.error.datafileInternalVersionTimestampIsOutdated=File Metadata internal version timestamp {0} is outdated +abstractApiBean.error.internalVersionTimestampIsOutdated=Internal version timestamp {0} is outdated diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java index b6ff3d9b401..13837bed859 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java @@ -671,25 +671,26 @@ public void testAddUpdateDatasetViaNativeAPI() { """; Response updateMetadataRemoveAlternativeTitles = UtilIT.editVersionMetadataFromJsonStr(datasetPersistentId, jsonString, apiToken); + updateMetadataRemoveAlternativeTitles.prettyPrint(); updateMetadataRemoveAlternativeTitles.then().assertThat() .body("data.metadataBlocks.citation.fields[2].typeName", not(equalTo("alternativeTitle"))) .statusCode(OK.getStatusCode()); - // Test sourceInternalVersionNumber optional query parameter - - Integer internalVersionNumber = updateMetadataRemoveAlternativeTitles.then().extract().path("data.internalVersionNumber"); - assertNotNull(internalVersionNumber); + // Test sourceInternalVersionTimestamp optional query parameter + String sourceInternalVersionTimestamp = updateMetadataRemoveAlternativeTitles.then().extract().path("data.lastUpdateTime"); + assertNotNull(sourceInternalVersionTimestamp); + String oldTimestamp = "2025-04-25T13:58:28Z"; // Case 1 - Pass outdated internal version number - Response updateMetadataWithOutdatedInternalVersionNumber = UtilIT.editVersionMetadataFromJsonStr(datasetPersistentId, jsonString, apiToken, internalVersionNumber - 1); + Response updateMetadataWithOutdatedInternalVersionNumber = UtilIT.editVersionMetadataFromJsonStr(datasetPersistentId, jsonString, apiToken, oldTimestamp); updateMetadataWithOutdatedInternalVersionNumber.then().assertThat() - .body("message", equalTo(BundleUtil.getStringFromBundle("abstractApiBean.error.datasetInternalVersionNumberIsOutdated", Collections.singletonList(Integer.toString(internalVersionNumber - 1))))) + .body("message", equalTo(BundleUtil.getStringFromBundle("abstractApiBean.error.internalVersionTimestampIsOutdated", Collections.singletonList(oldTimestamp)))) .statusCode(BAD_REQUEST.getStatusCode()); // Case 2 - Pass latest internal version number - Response updateMetadataWithLatestInternalVersionNumber = UtilIT.editVersionMetadataFromJsonStr(datasetPersistentId, jsonString, apiToken, internalVersionNumber); + Response updateMetadataWithLatestInternalVersionNumber = UtilIT.editVersionMetadataFromJsonStr(datasetPersistentId, jsonString, apiToken, sourceInternalVersionTimestamp); updateMetadataWithLatestInternalVersionNumber.then().assertThat() .statusCode(OK.getStatusCode()); } diff --git a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java index de45362ce98..f38ca34a283 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -3280,7 +3280,7 @@ public void testUpdateWithEmptyFieldsAndVersionCheck() throws InterruptedExcepti updateResponse.prettyPrint(); updateResponse.then().assertThat() .body("status", equalTo(ApiConstants.STATUS_ERROR)) - .body("message", equalTo(BundleUtil.getStringFromBundle("abstractApiBean.error.datafileInternalVersionTimestampIsOutdated",Collections.singletonList(lastUpdateTime)))) + .body("message", equalTo(BundleUtil.getStringFromBundle("abstractApiBean.error.internalVersionTimestampIsOutdated",Collections.singletonList(lastUpdateTime)))) .statusCode(BAD_REQUEST.getStatusCode()); // Second user refreshes and updates. Should pass now 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 04d0b27d63c..9d85903dde3 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -771,7 +771,7 @@ static Response editVersionMetadataFromJsonStr(String persistentId, String jsonS return editVersionMetadataFromJsonStr(persistentId, jsonString, apiToken, null); } - static Response editVersionMetadataFromJsonStr(String persistentId, String jsonString, String apiToken, Integer sourceInternalVersionNumber) { + static Response editVersionMetadataFromJsonStr(String persistentId, String jsonString, String apiToken, String sourceInternalVersionTimestamp) { return given() .header(API_TOKEN_HTTP_HEADER, apiToken) .body(jsonString) @@ -779,7 +779,7 @@ static Response editVersionMetadataFromJsonStr(String persistentId, String jsonS .put("/api/datasets/:persistentId/editMetadata/?persistentId=" + persistentId + "&replace=true" - + (sourceInternalVersionNumber != null ? "&sourceInternalVersionNumber=" + sourceInternalVersionNumber : "")); + + (sourceInternalVersionTimestamp != null ? "&sourceInternalVersionTimestamp=" + sourceInternalVersionTimestamp : "")); } static Response updateDatasetPIDMetadata(String persistentId, String apiToken) { From 1320d515c98ac8e2013259a198ec92dfee0832c9 Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Wed, 30 Apr 2025 09:19:58 -0400 Subject: [PATCH 014/247] refactor optional qp name from sourceInternalVersionTimestamp to sourceLastUpdateTime --- .../11243-editmetadata-api-extension.md | 2 +- .../11392-edit-file-metadata-empty-values.md | 2 +- doc/sphinx-guides/source/api/native-api.rst | 16 ++++++++-------- .../iq/dataverse/api/AbstractApiBean.java | 8 ++++---- .../edu/harvard/iq/dataverse/api/Datasets.java | 6 +++--- .../java/edu/harvard/iq/dataverse/api/Files.java | 6 +++--- .../edu/harvard/iq/dataverse/api/DatasetsIT.java | 8 ++++---- .../edu/harvard/iq/dataverse/api/UtilIT.java | 10 +++++----- 8 files changed, 29 insertions(+), 29 deletions(-) diff --git a/doc/release-notes/11243-editmetadata-api-extension.md b/doc/release-notes/11243-editmetadata-api-extension.md index 4aaf9320074..3666d8bc30a 100644 --- a/doc/release-notes/11243-editmetadata-api-extension.md +++ b/doc/release-notes/11243-editmetadata-api-extension.md @@ -1,7 +1,7 @@ ### Edit Dataset Metadata API extension - This endpoint now allows removing fields (by sending empty values), as long as they are not required by the dataset. -- New ``sourceInternalVersionTimestamp`` optional query parameter, which prevents inconsistencies by managing updates that +- New ``sourceLastUpdateTime`` optional query parameter, which prevents inconsistencies by managing updates that may occur from other users while a dataset is being edited. NOTE: This release note was updated to conform to the refactoring of the validation as part of issue #11392 diff --git a/doc/release-notes/11392-edit-file-metadata-empty-values.md b/doc/release-notes/11392-edit-file-metadata-empty-values.md index 652aea63aed..5839fa100af 100644 --- a/doc/release-notes/11392-edit-file-metadata-empty-values.md +++ b/doc/release-notes/11392-edit-file-metadata-empty-values.md @@ -2,6 +2,6 @@ Previously the API POST /files/{id}/metadata would ignore fields with empty values. Now the API updates the fields with the empty values essentially clearing the data. Missing fields will still be ignored. -An optional query parameter (sourceInternalVersionTimestamp) was added to ensure the metadata update doesn't overwrite stale data. +An optional query parameter (sourceLastUpdateTime) was added to ensure the metadata update doesn't overwrite stale data. See also [the guides](https://dataverse-guide--11359.org.readthedocs.build/en/11359/api/native-api.html#updating-file-metadata), #11392, and #11359. diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 352234d844a..0f6b49ba079 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -2137,26 +2137,26 @@ For these edits your JSON file need only include those dataset fields which you This endpoint also allows removing fields, as long as they are not required by the dataset. To remove a field, send an empty value (``""``) for individual fields. For multiple fields, send an empty array (``[]``). A sample JSON file for removing fields may be downloaded here: :download:`dataset-edit-metadata-delete-fields-sample.json <../_static/api/dataset-edit-metadata-delete-fields-sample.json>` -If another user updates the dataset version metadata before you send the update request, data inconsistencies may occur. To prevent this, you can use the optional ``sourceInternalVersionTimestamp`` query parameter. This parameter must include the ``lastUpdateTime`` corresponding to the dataset version being updated. The date must be in this format "yyyy-MM-dd'T'HH:mm:ss'Z'" +If another user updates the dataset version metadata before you send the update request, data inconsistencies may occur. To prevent this, you can use the optional ``sourceLastUpdateTime`` query parameter. This parameter must include the ``lastUpdateTime`` corresponding to the dataset version being updated. The date must be in this format "yyyy-MM-dd'T'HH:mm:ss'Z'" If this parameter is provided, the update will proceed only if the ``lastUpdateTime`` remains unchanged. Otherwise, the request will fail with an error. -Example using ``sourceInternalVersionTimestamp``: +Example using ``sourceLastUpdateTime``: .. code-block:: bash export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx export SERVER_URL=https://demo.dataverse.org export PERSISTENT_IDENTIFIER=doi:10.5072/FK2/BCCP9Z - export SOURCE_INTERNAL_VERSION_TIMESTAMP=2025-04-25T13:58:28Z + export SOURCE_LAST_UPDATE_TIME=2025-04-25T13:58:28Z - curl -H "X-Dataverse-key: $API_TOKEN" -X PUT "$SERVER_URL/api/datasets/:persistentId/editMetadata?persistentId=$PERSISTENT_IDENTIFIER&replace=true&sourceInternalVersionTimestamp=$SOURCE_INTERNAL_VERSION_TIMESTAMP" --upload-file dataset-update-metadata.json + curl -H "X-Dataverse-key: $API_TOKEN" -X PUT "$SERVER_URL/api/datasets/:persistentId/editMetadata?persistentId=$PERSISTENT_IDENTIFIER&replace=true&sourceLastUpdateTime=SOURCE_LAST_UPDATE_TIME" --upload-file dataset-update-metadata.json The fully expanded example above (without environment variables) looks like this: .. code-block:: bash - curl -H "X-Dataverse-key: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X PUT "https://demo.dataverse.org/api/datasets/:persistentId/editMetadata/?persistentId=doi:10.5072/FK2/BCCP9Z&replace=true&sourceInternalVersionTimestamp=2025-04-25T13:58:28Z" --upload-file dataset-update-metadata.json + curl -H "X-Dataverse-key: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X PUT "https://demo.dataverse.org/api/datasets/:persistentId/editMetadata/?persistentId=doi:10.5072/FK2/BCCP9Z&replace=true&sourceLastUpdateTime=2025-04-25T13:58:28Z" --upload-file dataset-update-metadata.json Delete Dataset Metadata @@ -4612,7 +4612,7 @@ Updating File Metadata Updates the file metadata for an existing file where ``ID`` is the database id of the file to update or ``PERSISTENT_ID`` is the persistent id (DOI or Handle) of the file. Requires a ``jsonString`` expressing the new metadata. No metadata from the previous version of this file will be persisted, so if you want to update a specific field first get the json with the above command and alter the fields you want. -Optional Parameter for verifying that the Dataset Version being edited is the latest version can be added &sourceInternalVersionTimestamp=datetime(in format: "yyyy-MM-dd'T'HH:mm:ss'Z'"). This is to prevent stale data from being edited. The value for sourceInternalVersionTimestamp comes from ``lastUpdateTime`` in the response to get $SERVER_URL/api/files/$ID API call +Optional Parameter for verifying that the Dataset Version being edited is the latest version can be added &sourceLastUpdateTime=datetime(in format: "yyyy-MM-dd'T'HH:mm:ss'Z'"). This is to prevent stale data from being edited. The value for sourceLastUpdateTime comes from ``lastUpdateTime`` in the response to get $SERVER_URL/api/files/$ID API call A curl example using an ``ID`` @@ -4645,7 +4645,7 @@ A curl example using a ``PERSISTENT_ID`` curl -H "X-Dataverse-key:$API_TOKEN" -X POST \ -F 'jsonData={"description":"My description bbb.","provFreeform":"Test prov freeform","categories":["Data"],"dataFileTags":["Survey"],"restrict":false}' \ - "$SERVER_URL/api/files/:persistentId/metadata?persistentId=$PERSISTENT_ID&sourceInternalVersionTimestamp=$UPDATE_TIME" + "$SERVER_URL/api/files/:persistentId/metadata?persistentId=$PERSISTENT_ID&sourceLastUpdateTime=$UPDATE_TIME" The fully expanded example above (without environment variables) looks like this: @@ -4653,7 +4653,7 @@ The fully expanded example above (without environment variables) looks like this curl -H "X-Dataverse-key:xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X POST \ -F 'jsonData={"description":"My description bbb.","provFreeform":"Test prov freeform","categories":["Data"],"dataFileTags":["Survey"],"restrict":false}' \ - "https://demo.dataverse.org/api/files/:persistentId/metadata?persistentId=doi:10.5072/FK2/AAA000&sourceInternalVersionTimestamp=2025-04-25T13:58:28Z" + "https://demo.dataverse.org/api/files/:persistentId/metadata?persistentId=doi:10.5072/FK2/AAA000&sourceLastUpdateTime=2025-04-25T13:58:28Z" Note: To update the 'tabularTags' property of file metadata, use the 'dataFileTags' key when making API requests. This property is used to update the 'tabularTags' of the file metadata. diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index c53f9f8119c..77c003ed3cb 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -445,11 +445,11 @@ public Command handleLatestPublished() { return dsv; } - protected void validateInternalTimestampIsNotOutdated(DvObject dvObject, String sourceInternalVersionTimestamp) throws WrappedResponse { - Date date = sourceInternalVersionTimestamp != null ? DateUtil.parseDate(sourceInternalVersionTimestamp, "yyyy-MM-dd'T'HH:mm:ss'Z'") : null; + protected void validateInternalTimestampIsNotOutdated(DvObject dvObject, String sourceLastUpdateTime) throws WrappedResponse { + Date date = sourceLastUpdateTime != null ? DateUtil.parseDate(sourceLastUpdateTime, "yyyy-MM-dd'T'HH:mm:ss'Z'") : null; if (date == null) { throw new WrappedResponse( - badRequest(BundleUtil.getStringFromBundle("jsonparser.error.parsing.date", Collections.singletonList(sourceInternalVersionTimestamp))) + badRequest(BundleUtil.getStringFromBundle("jsonparser.error.parsing.date", Collections.singletonList(sourceLastUpdateTime))) ); } Instant instant = date.toInstant(); @@ -460,7 +460,7 @@ protected void validateInternalTimestampIsNotOutdated(DvObject dvObject, String // granularity is to the second since the json output only returns dates in this format to the second if (updateTimestamp.getEpochSecond() != instant.getEpochSecond()) { throw new WrappedResponse( - badRequest(BundleUtil.getStringFromBundle("abstractApiBean.error.internalVersionTimestampIsOutdated", Collections.singletonList(sourceInternalVersionTimestamp))) + badRequest(BundleUtil.getStringFromBundle("abstractApiBean.error.internalVersionTimestampIsOutdated", Collections.singletonList(sourceLastUpdateTime))) ); } } diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java index e36b50ad0e0..bfcf530b57e 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -1095,12 +1095,12 @@ private String getCompoundDisplayValue (DatasetFieldCompoundValue dscv){ @Path("{id}/editMetadata") public Response editVersionMetadata(@Context ContainerRequestContext crc, String jsonBody, @PathParam("id") String id, @QueryParam("replace") boolean replaceData, - @QueryParam("sourceInternalVersionTimestamp") String sourceInternalVersionTimestamp) { + @QueryParam("sourceLastUpdateTime") String sourceLastUpdateTime) { try { Dataset dataset = findDatasetOrDie(id); - if (sourceInternalVersionTimestamp != null) { - validateInternalTimestampIsNotOutdated(dataset, sourceInternalVersionTimestamp); + if (sourceLastUpdateTime != null) { + validateInternalTimestampIsNotOutdated(dataset, sourceLastUpdateTime); } JsonObject json = JsonUtil.getJsonObject(jsonBody); diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Files.java b/src/main/java/edu/harvard/iq/dataverse/api/Files.java index 1b809894e17..5834e7e0008 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -410,7 +410,7 @@ public Response deleteFileInDataset(@Context ContainerRequestContext crc, @PathP @AuthRequired @Path("{id}/metadata") public Response updateFileMetadata(@Context ContainerRequestContext crc, @FormDataParam("jsonData") String jsonData, - @PathParam("id") String fileIdOrPersistentId, @QueryParam("sourceInternalVersionTimestamp") String sourceInternalVersionTimestamp) { + @PathParam("id") String fileIdOrPersistentId, @QueryParam("sourceLastUpdateTime") String sourceLastUpdateTime) { FileMetadata upFmd = null; @@ -428,9 +428,9 @@ public Response updateFileMetadata(@Context ContainerRequestContext crc, @FormDa return error(BAD_REQUEST, "Error attempting get the requested data file."); } - if (sourceInternalVersionTimestamp != null) { + if (sourceLastUpdateTime != null) { try { - validateInternalTimestampIsNotOutdated(df, sourceInternalVersionTimestamp); + validateInternalTimestampIsNotOutdated(df, sourceLastUpdateTime); } catch (WrappedResponse wr) { return wr.getResponse(); } diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java index 13837bed859..162c4b0e125 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java @@ -676,9 +676,9 @@ public void testAddUpdateDatasetViaNativeAPI() { .body("data.metadataBlocks.citation.fields[2].typeName", not(equalTo("alternativeTitle"))) .statusCode(OK.getStatusCode()); - // Test sourceInternalVersionTimestamp optional query parameter - String sourceInternalVersionTimestamp = updateMetadataRemoveAlternativeTitles.then().extract().path("data.lastUpdateTime"); - assertNotNull(sourceInternalVersionTimestamp); + // Test sourceLastUpdateTime optional query parameter + String sourceLastUpdateTime = updateMetadataRemoveAlternativeTitles.then().extract().path("data.lastUpdateTime"); + assertNotNull(sourceLastUpdateTime); String oldTimestamp = "2025-04-25T13:58:28Z"; // Case 1 - Pass outdated internal version number @@ -690,7 +690,7 @@ public void testAddUpdateDatasetViaNativeAPI() { // Case 2 - Pass latest internal version number - Response updateMetadataWithLatestInternalVersionNumber = UtilIT.editVersionMetadataFromJsonStr(datasetPersistentId, jsonString, apiToken, sourceInternalVersionTimestamp); + Response updateMetadataWithLatestInternalVersionNumber = UtilIT.editVersionMetadataFromJsonStr(datasetPersistentId, jsonString, apiToken, sourceLastUpdateTime); updateMetadataWithLatestInternalVersionNumber.then().assertThat() .statusCode(OK.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 9d85903dde3..672b43c11fa 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -771,7 +771,7 @@ static Response editVersionMetadataFromJsonStr(String persistentId, String jsonS return editVersionMetadataFromJsonStr(persistentId, jsonString, apiToken, null); } - static Response editVersionMetadataFromJsonStr(String persistentId, String jsonString, String apiToken, String sourceInternalVersionTimestamp) { + static Response editVersionMetadataFromJsonStr(String persistentId, String jsonString, String apiToken, String sourceLastUpdateTime) { return given() .header(API_TOKEN_HTTP_HEADER, apiToken) .body(jsonString) @@ -779,7 +779,7 @@ static Response editVersionMetadataFromJsonStr(String persistentId, String jsonS .put("/api/datasets/:persistentId/editMetadata/?persistentId=" + persistentId + "&replace=true" - + (sourceInternalVersionTimestamp != null ? "&sourceInternalVersionTimestamp=" + sourceInternalVersionTimestamp : "")); + + (sourceLastUpdateTime != null ? "&sourceLastUpdateTime=" + sourceLastUpdateTime : "")); } static Response updateDatasetPIDMetadata(String persistentId, String apiToken) { @@ -1092,15 +1092,15 @@ static Response deleteFileApi(Integer fileId, String apiToken) { static Response updateFileMetadata(String fileIdOrPersistentId, String jsonAsString, String apiToken) { return updateFileMetadata(fileIdOrPersistentId, jsonAsString,apiToken, null); } - static Response updateFileMetadata(String fileIdOrPersistentId, String jsonAsString, String apiToken, String sourceInternalVersionTimestamp) { + static Response updateFileMetadata(String fileIdOrPersistentId, String jsonAsString, String apiToken, String sourceLastUpdateTime) { String idInPath = fileIdOrPersistentId; // Assume it's a number. String optionalQueryParam = ""; // If idOrPersistentId is a number we'll just put it in the path. if (!NumberUtils.isCreatable(fileIdOrPersistentId)) { idInPath = ":persistentId"; optionalQueryParam = "?persistentId=" + fileIdOrPersistentId; } - if (sourceInternalVersionTimestamp != null) { - optionalQueryParam = optionalQueryParam + (optionalQueryParam.isEmpty() ? "?" : "&") + "sourceInternalVersionTimestamp=" + sourceInternalVersionTimestamp; + if (sourceLastUpdateTime != null) { + optionalQueryParam = optionalQueryParam + (optionalQueryParam.isEmpty() ? "?" : "&") + "sourceLastUpdateTime=" + sourceLastUpdateTime; } RequestSpecification requestSpecification = given() .header(API_TOKEN_HTTP_HEADER, apiToken); From 5d0fe340fe1517605da08cde0c8d8db4c5246feb Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Mon, 5 May 2025 15:43:51 -0400 Subject: [PATCH 015/247] new api for spa to retrieve analytics html --- .../11448-api-endpoint-for-analytics-html.md | 5 ++ doc/sphinx-guides/source/api/native-api.rst | 17 +++++++ .../iq/dataverse/api/CustomizationApi.java | 29 ++++++++++++ .../iq/dataverse/api/CustomizationIT.java | 46 +++++++++++++++++++ .../edu/harvard/iq/dataverse/api/UtilIT.java | 6 +++ 5 files changed, 103 insertions(+) create mode 100644 doc/release-notes/11448-api-endpoint-for-analytics-html.md create mode 100644 src/main/java/edu/harvard/iq/dataverse/api/CustomizationApi.java create mode 100644 src/test/java/edu/harvard/iq/dataverse/api/CustomizationIT.java diff --git a/doc/release-notes/11448-api-endpoint-for-analytics-html.md b/doc/release-notes/11448-api-endpoint-for-analytics-html.md new file mode 100644 index 00000000000..f34f2cf04e2 --- /dev/null +++ b/doc/release-notes/11448-api-endpoint-for-analytics-html.md @@ -0,0 +1,5 @@ +### Feature Request: API endpoint for analytics.html + +New API to get the analytics.html from settings for SPA + +See also [the guides](https://dataverse-guide--11359.org.readthedocs.build/en/11359/installation/config.html#web-analytics-code), #11448. diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index c6bb5d82151..de7ee5c03df 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -7609,3 +7609,20 @@ Parameters: ``per_page`` Number of results returned per page. + +Customization: Web Analytics Code +--------------------------------- + +The Customization API is used to retrieve the analytics-code.html. See also :ref:`web-analytics-code` in the Configuration section of the Installation Guide. + +The content is returned in type="text/html; charset=UTF-8" + +A curl example getting the analytics-code + +.. code-block:: bash + + export SERVER_URL=https://demo.dataverse.org + + curl "$SERVER_URL/api/customization/analytics" + +.. _customization-analytics: diff --git a/src/main/java/edu/harvard/iq/dataverse/api/CustomizationApi.java b/src/main/java/edu/harvard/iq/dataverse/api/CustomizationApi.java new file mode 100644 index 00000000000..9ad586bea81 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/api/CustomizationApi.java @@ -0,0 +1,29 @@ +package edu.harvard.iq.dataverse.api; + +import edu.harvard.iq.dataverse.settings.SettingsServiceBean; +import jakarta.ejb.EJB; +import jakarta.ws.rs.*; +import jakarta.ws.rs.core.Response; + +@Path("customization") +public class CustomizationApi extends AbstractApiBean { + + @EJB + SettingsServiceBean settingsService; + + @GET + @Path("analytics") + @Produces({"text/html; charset=UTF-8"}) + public Response getAnalyticsHTML() { + return getFromSettings(SettingsServiceBean.Key.WebAnalyticsCode, "text/html; charset=UTF-8"); + } + + private Response getFromSettings(SettingsServiceBean.Key key, String type) { + String value = settingsService.getValueForKey(key); + if (value != null) { + return Response.ok(value).type(type).build(); + } else { + return Response.status(Response.Status.NOT_FOUND).build(); + } + } +} diff --git a/src/test/java/edu/harvard/iq/dataverse/api/CustomizationIT.java b/src/test/java/edu/harvard/iq/dataverse/api/CustomizationIT.java new file mode 100644 index 00000000000..2effc36f752 --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/api/CustomizationIT.java @@ -0,0 +1,46 @@ +package edu.harvard.iq.dataverse.api; + +import edu.harvard.iq.dataverse.settings.SettingsServiceBean; +import io.restassured.response.Response; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.CoreMatchers.equalTo; + +public class CustomizationIT { + + @AfterEach + public void after() { + UtilIT.deleteSetting(SettingsServiceBean.Key.WebAnalyticsCode); + } + @Test + public void testGetCustomAnalytics() { + String html = """ + + + """; + + UtilIT.setSetting(SettingsServiceBean.Key.WebAnalyticsCode, html).prettyPrint(); + + Response getResponse = UtilIT.getCustomAnalyticsHTML(); + getResponse.prettyPrint(); + getResponse.then().assertThat() + .statusCode(200) + .body(equalTo(html)); + + UtilIT.deleteSetting(SettingsServiceBean.Key.WebAnalyticsCode).prettyPrint(); + + getResponse = UtilIT.getCustomAnalyticsHTML(); + getResponse.prettyPrint(); + getResponse.then().assertThat() + .statusCode(404); + } +} 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 30a8f3107c2..b7e980bd90f 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -4737,4 +4737,10 @@ public static Response updateDatasetFilesMetadata(String datasetIdOrPersistentId return given().header(API_TOKEN_HTTP_HEADER, apiToken).contentType(ContentType.JSON).body(jsonArray.toString()) .post("/api/datasets/" + idInPath + "/files/metadata" + optionalQueryParam); } + + public static Response getCustomAnalyticsHTML() { + return given() + .contentType("text/html; charset=UTF-8") + .get("/api/customization/analytics"); + } } From 4296254f74b86c4e4532b3d03845a9e3195d4b98 Mon Sep 17 00:00:00 2001 From: jo-pol Date: Tue, 6 May 2025 12:03:43 +0200 Subject: [PATCH 016/247] as requested by review --- src/main/java/edu/harvard/iq/dataverse/api/Datasets.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java index f754341a36b..7b4154cd730 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -66,7 +66,7 @@ import jakarta.ws.rs.container.ContainerRequestContext; import jakarta.ws.rs.core.*; import jakarta.ws.rs.core.Response.Status; -import org.apache.commons.lang.exception.ExceptionUtils; +import org.apache.commons.lang3.exception.ExceptionUtils; import org.apache.commons.lang3.StringUtils; import org.eclipse.microprofile.openapi.annotations.Operation; import org.eclipse.microprofile.openapi.annotations.media.Content; From 173360b5d3772f2c022a5ff559c0ec8439d2c685 Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Tue, 13 May 2025 11:03:14 -0400 Subject: [PATCH 017/247] refactored to move api to /info --- doc/sphinx-guides/source/api/native-api.rst | 17 +++- .../CustomizationFilesServiceBean.java | 91 ++++++++++++++++++ .../dataverse/CustomizationFilesServlet.java | 95 +------------------ .../iq/dataverse/api/CustomizationApi.java | 29 ------ .../edu/harvard/iq/dataverse/api/Info.java | 25 ++++- .../iq/dataverse/api/CustomizationIT.java | 32 +++---- .../edu/harvard/iq/dataverse/api/UtilIT.java | 11 ++- 7 files changed, 148 insertions(+), 152 deletions(-) create mode 100644 src/main/java/edu/harvard/iq/dataverse/CustomizationFilesServiceBean.java delete mode 100644 src/main/java/edu/harvard/iq/dataverse/api/CustomizationApi.java diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index de7ee5c03df..a880ef4f64b 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -7610,19 +7610,28 @@ Parameters: ``per_page`` Number of results returned per page. -Customization: Web Analytics Code ---------------------------------- +Customization File Contents +--------------------------- -The Customization API is used to retrieve the analytics-code.html. See also :ref:`web-analytics-code` in the Configuration section of the Installation Guide. +The Customization API is used to retrieve the analytics-code.html as well as other customization file contents. See also :ref:`web-analytics-code` in the Configuration section of the Installation Guide. The content is returned in type="text/html; charset=UTF-8" +Valid types are "homePage", "header", "footer", "style", "analytics", and "logo". + A curl example getting the analytics-code .. code-block:: bash export SERVER_URL=https://demo.dataverse.org + export TYPE=analytics + + curl -X GET "$SERVER_URL/api/info/settings/customization/$TYPE" + +The fully expanded example above (without environment variables) looks like this: + +.. code-block:: bash - curl "$SERVER_URL/api/customization/analytics" + curl -X GET "https://demo.dataverse.org/api/info/settings/customization/analytics" .. _customization-analytics: diff --git a/src/main/java/edu/harvard/iq/dataverse/CustomizationFilesServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/CustomizationFilesServiceBean.java new file mode 100644 index 00000000000..b12bee4cce1 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/CustomizationFilesServiceBean.java @@ -0,0 +1,91 @@ +package edu.harvard.iq.dataverse; + +import edu.harvard.iq.dataverse.customization.CustomizationConstants; +import edu.harvard.iq.dataverse.settings.SettingsServiceBean; +import jakarta.ejb.EJB; +import jakarta.ejb.Stateless; +import jakarta.inject.Named; +import org.apache.commons.io.IOUtils; + +import java.io.*; +import java.nio.file.Path; +import java.nio.file.Paths; +@Stateless +@Named +public class CustomizationFilesServiceBean { + + @EJB + SettingsServiceBean settingsService; + + public void getContents(PrintWriter writer, String fileType) { + Path physicalPath = Paths.get(getFilePath(fileType)); + FileInputStream inputStream = null; + BufferedReader in = null; + try { + File fileIn = physicalPath.toFile(); + if (fileIn != null) { + inputStream = new FileInputStream(fileIn); + in = new BufferedReader(new InputStreamReader(inputStream)); + StringBuilder responseData = new StringBuilder(); + String line; + while ((line = in.readLine()) != null) { + responseData.append(line); + writer.println(line); + } + inputStream.close(); + } else { + /* + If the file doesn't exist or it is unreadable we don't care + */ + } + + } catch (Exception e) { + /* + If the file doesn't exist, or it is unreadable we don't care + */ + } finally { + IOUtils.closeQuietly(inputStream); + IOUtils.closeQuietly(in); + } + + } + private String getFilePath(String fileTypeParam){ + + String nonNullDefaultIfKeyNotFound = ""; + + if (fileTypeParam.equals(CustomizationConstants.fileTypeHomePage)) { + + // Homepage + return settingsService.getValueForKey(SettingsServiceBean.Key.HomePageCustomizationFile, nonNullDefaultIfKeyNotFound); + + } else if (fileTypeParam.equals(CustomizationConstants.fileTypeHeader)) { + + // Header + return settingsService.getValueForKey(SettingsServiceBean.Key.HeaderCustomizationFile, nonNullDefaultIfKeyNotFound); + + } else if (fileTypeParam.equals(CustomizationConstants.fileTypeFooter)) { + + // Footer + return settingsService.getValueForKey(SettingsServiceBean.Key.FooterCustomizationFile, nonNullDefaultIfKeyNotFound); + + } else if (fileTypeParam.equals(CustomizationConstants.fileTypeStyle)) { + + // Style (css) + return settingsService.getValueForKey(SettingsServiceBean.Key.StyleCustomizationFile, nonNullDefaultIfKeyNotFound); + + } else if (fileTypeParam.equals(CustomizationConstants.fileTypeAnalytics)) { + + // Analytics - appears in head + return settingsService.getValueForKey(SettingsServiceBean.Key.WebAnalyticsCode, nonNullDefaultIfKeyNotFound); + + } else if (fileTypeParam.equals(CustomizationConstants.fileTypeLogo)) { + + // Logo for installation - appears in header + return settingsService.getValueForKey(SettingsServiceBean.Key.LogoCustomizationFile, nonNullDefaultIfKeyNotFound); + } + + + return ""; + } + +} diff --git a/src/main/java/edu/harvard/iq/dataverse/CustomizationFilesServlet.java b/src/main/java/edu/harvard/iq/dataverse/CustomizationFilesServlet.java index 9dd524127d7..0698fd33681 100644 --- a/src/main/java/edu/harvard/iq/dataverse/CustomizationFilesServlet.java +++ b/src/main/java/edu/harvard/iq/dataverse/CustomizationFilesServlet.java @@ -5,23 +5,13 @@ */ package edu.harvard.iq.dataverse; -import edu.harvard.iq.dataverse.customization.CustomizationConstants; -import java.io.BufferedReader; -import java.io.File; -import java.io.FileInputStream; import java.io.IOException; -import java.io.InputStreamReader; -import java.io.PrintWriter; -import java.nio.file.Path; -import java.nio.file.Paths; import jakarta.servlet.ServletException; import jakarta.servlet.annotation.WebServlet; import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import edu.harvard.iq.dataverse.settings.SettingsServiceBean; import jakarta.ejb.EJB; -import org.apache.commons.io.IOUtils; /** * @@ -31,9 +21,8 @@ public class CustomizationFilesServlet extends HttpServlet { @EJB - SettingsServiceBean settingsService; - - + CustomizationFilesServiceBean customizationFilesService; + /** * Processes requests for both HTTP GET and POST * methods. @@ -48,85 +37,7 @@ protected void processRequest(HttpServletRequest request, HttpServletResponse re response.setContentType("text/html;charset=UTF-8"); String customFileType = request.getParameter("customFileType"); - String filePath = getFilePath(customFileType); - - Path physicalPath = Paths.get(filePath); - FileInputStream inputStream = null; - BufferedReader in = null; - try { - File fileIn = physicalPath.toFile(); - if (fileIn != null) { - inputStream = new FileInputStream(fileIn); - - in = new BufferedReader(new InputStreamReader(inputStream)); - String line; - - StringBuilder responseData = new StringBuilder(); - try (PrintWriter out = response.getWriter()) { - - while ((line = in.readLine()) != null) { - responseData.append(line); - out.println(line); - } - } - - inputStream.close(); - - - } else { - /* - If the file doesn't exist or it is unreadable we don't care - */ - } - - } catch (Exception e) { - /* - If the file doesn't exist or it is unreadable we don't care - */ - } finally { - IOUtils.closeQuietly(inputStream); - IOUtils.closeQuietly(in); - } - - } - - private String getFilePath(String fileTypeParam){ - - String nonNullDefaultIfKeyNotFound = ""; - - if (fileTypeParam.equals(CustomizationConstants.fileTypeHomePage)) { - - // Homepage - return settingsService.getValueForKey(SettingsServiceBean.Key.HomePageCustomizationFile, nonNullDefaultIfKeyNotFound); - - } else if (fileTypeParam.equals(CustomizationConstants.fileTypeHeader)) { - - // Header - return settingsService.getValueForKey(SettingsServiceBean.Key.HeaderCustomizationFile, nonNullDefaultIfKeyNotFound); - - } else if (fileTypeParam.equals(CustomizationConstants.fileTypeFooter)) { - - // Footer - return settingsService.getValueForKey(SettingsServiceBean.Key.FooterCustomizationFile, nonNullDefaultIfKeyNotFound); - - } else if (fileTypeParam.equals(CustomizationConstants.fileTypeStyle)) { - - // Style (css) - return settingsService.getValueForKey(SettingsServiceBean.Key.StyleCustomizationFile, nonNullDefaultIfKeyNotFound); - - } else if (fileTypeParam.equals(CustomizationConstants.fileTypeAnalytics)) { - - // Analytics - appears in head - return settingsService.getValueForKey(SettingsServiceBean.Key.WebAnalyticsCode, nonNullDefaultIfKeyNotFound); - - } else if (fileTypeParam.equals(CustomizationConstants.fileTypeLogo)) { - - // Logo for installation - appears in header - return settingsService.getValueForKey(SettingsServiceBean.Key.LogoCustomizationFile, nonNullDefaultIfKeyNotFound); - } - - - return ""; + customizationFilesService.getContents(response.getWriter(), customFileType); } // diff --git a/src/main/java/edu/harvard/iq/dataverse/api/CustomizationApi.java b/src/main/java/edu/harvard/iq/dataverse/api/CustomizationApi.java deleted file mode 100644 index 9ad586bea81..00000000000 --- a/src/main/java/edu/harvard/iq/dataverse/api/CustomizationApi.java +++ /dev/null @@ -1,29 +0,0 @@ -package edu.harvard.iq.dataverse.api; - -import edu.harvard.iq.dataverse.settings.SettingsServiceBean; -import jakarta.ejb.EJB; -import jakarta.ws.rs.*; -import jakarta.ws.rs.core.Response; - -@Path("customization") -public class CustomizationApi extends AbstractApiBean { - - @EJB - SettingsServiceBean settingsService; - - @GET - @Path("analytics") - @Produces({"text/html; charset=UTF-8"}) - public Response getAnalyticsHTML() { - return getFromSettings(SettingsServiceBean.Key.WebAnalyticsCode, "text/html; charset=UTF-8"); - } - - private Response getFromSettings(SettingsServiceBean.Key key, String type) { - String value = settingsService.getValueForKey(key); - if (value != null) { - return Response.ok(value).type(type).build(); - } else { - return Response.status(Response.Status.NOT_FOUND).build(); - } - } -} diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Info.java b/src/main/java/edu/harvard/iq/dataverse/api/Info.java index 7d84681afa5..138e81885a4 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Info.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Info.java @@ -2,6 +2,8 @@ import java.io.FileInputStream; import java.io.InputStream; +import java.io.PrintWriter; +import java.io.StringWriter; import java.net.URL; import java.nio.charset.StandardCharsets; import java.util.Arrays; @@ -9,7 +11,8 @@ import java.util.logging.Level; import java.util.logging.Logger; -import jakarta.ws.rs.Produces; +import edu.harvard.iq.dataverse.CustomizationFilesServiceBean; +import jakarta.ws.rs.*; import org.apache.commons.io.IOUtils; import edu.harvard.iq.dataverse.export.ExportService; @@ -24,9 +27,6 @@ import jakarta.json.Json; import jakarta.json.JsonObjectBuilder; import jakarta.json.JsonValue; -import jakarta.ws.rs.GET; -import jakarta.ws.rs.Path; -import jakarta.ws.rs.QueryParam; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; import org.eclipse.microprofile.openapi.annotations.Operation; @@ -46,6 +46,9 @@ public class Info extends AbstractApiBean { @EJB SystemConfig systemConfig; + @EJB + CustomizationFilesServiceBean customizationFilesService; + private static final Logger logger = Logger.getLogger(Info.class.getCanonicalName()); @GET @@ -139,6 +142,20 @@ public Response getExportFormats() { return ok(responseModel); } + @GET + @Path("settings/customization/{customizationFileType}") + public Response getCustomizationFile(@PathParam("customizationFileType") String customizationFileType) { + StringWriter stringWriter = new StringWriter(); + PrintWriter printWriter = new PrintWriter(stringWriter); + customizationFilesService.getContents(printWriter, customizationFileType); + String contents = stringWriter.toString(); + if (contents != null && !contents.isEmpty()) { + return ok(contents); + } else { + return notFound(customizationFileType + " not set, or unknown."); + } + } + private Response getSettingResponseByKey(SettingsServiceBean.Key key) { String setting = settingsService.getValueForKey(key); if (setting != null) { diff --git a/src/test/java/edu/harvard/iq/dataverse/api/CustomizationIT.java b/src/test/java/edu/harvard/iq/dataverse/api/CustomizationIT.java index 2effc36f752..71b20a81fea 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/CustomizationIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/CustomizationIT.java @@ -5,7 +5,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; -import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.*; public class CustomizationIT { @@ -13,32 +13,26 @@ public class CustomizationIT { public void after() { UtilIT.deleteSetting(SettingsServiceBean.Key.WebAnalyticsCode); } + @Test public void testGetCustomAnalytics() { - String html = """ - - - """; - - UtilIT.setSetting(SettingsServiceBean.Key.WebAnalyticsCode, html).prettyPrint(); - - Response getResponse = UtilIT.getCustomAnalyticsHTML(); + String setting = "./appserver/glassfish/domains/domain1/docroot/index.html"; + UtilIT.setSetting(SettingsServiceBean.Key.WebAnalyticsCode, setting).prettyPrint(); + + Response getResponse = UtilIT.getCustomizationFile("analytics"); getResponse.prettyPrint(); getResponse.then().assertThat() .statusCode(200) - .body(equalTo(html)); + .body(containsString("")); UtilIT.deleteSetting(SettingsServiceBean.Key.WebAnalyticsCode).prettyPrint(); - getResponse = UtilIT.getCustomAnalyticsHTML(); + getResponse = UtilIT.getCustomizationFile("analytics"); + getResponse.prettyPrint(); + getResponse.then().assertThat() + .statusCode(404); + + getResponse = UtilIT.getCustomizationFile("INVALID"); getResponse.prettyPrint(); getResponse.then().assertThat() .statusCode(404); 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 b7e980bd90f..0681bb97bfe 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -4738,9 +4738,12 @@ public static Response updateDatasetFilesMetadata(String datasetIdOrPersistentId .post("/api/datasets/" + idInPath + "/files/metadata" + optionalQueryParam); } - public static Response getCustomAnalyticsHTML() { - return given() - .contentType("text/html; charset=UTF-8") - .get("/api/customization/analytics"); + public static Response getCustomizationFile(String fileType) { + RequestSpecification requestSpec = given(); + + Response resp = requestSpec.contentType("text/html; charset=UTF-8") + .get("/api/info/settings/customization/" + fileType); + + return resp; } } From 246535c835590a43031e1be894d994d012fb4290 Mon Sep 17 00:00:00 2001 From: Vera Clemens Date: Thu, 15 May 2025 11:49:24 +0200 Subject: [PATCH 018/247] feat: allow listing links of a dataset for non-superusers --- .../harvard/iq/dataverse/api/Datasets.java | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java index 0999408a977..5cbc1b60c89 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -2022,20 +2022,23 @@ public Response getCustomTermsTab(@PathParam("id") String id, @PathParam("versio public Response getLinks(@Context ContainerRequestContext crc, @PathParam("id") String idSupplied ) { try { User u = getRequestUser(crc); - if (!u.isSuperuser()) { - return error(Response.Status.FORBIDDEN, "Not a superuser"); - } Dataset dataset = findDatasetOrDie(idSupplied); + if (!dataset.isReleased() && !permissionService.hasPermissionsFor(u, dataset.getOwner(), EnumSet.of(Permission.ViewUnpublishedDataset))) { + return error(Response.Status.FORBIDDEN, "User is not allowed to list the link(s) of this dataset"); + } + long datasetId = dataset.getId(); List dvsThatLinkToThisDatasetId = dataverseSvc.findDataversesThatLinkToThisDatasetId(datasetId); JsonArrayBuilder dataversesThatLinkToThisDatasetIdBuilder = Json.createArrayBuilder(); for (Dataverse dataverse : dvsThatLinkToThisDatasetId) { - JsonObjectBuilder datasetBuilder = Json.createObjectBuilder(); - datasetBuilder.add("id", dataverse.getId()); - datasetBuilder.add("alias", dataverse.getAlias()); - datasetBuilder.add("displayName", dataverse.getDisplayName()); - dataversesThatLinkToThisDatasetIdBuilder.add(datasetBuilder.build()); + if (dataverse.isReleased() || this.permissionService.hasPermissionsFor(u, dataverse, EnumSet.of(Permission.ViewUnpublishedDataverse))) { + JsonObjectBuilder datasetBuilder = Json.createObjectBuilder(); + datasetBuilder.add("id", dataverse.getId()); + datasetBuilder.add("alias", dataverse.getAlias()); + datasetBuilder.add("displayName", dataverse.getDisplayName()); + dataversesThatLinkToThisDatasetIdBuilder.add(datasetBuilder.build()); + } } JsonObjectBuilder response = Json.createObjectBuilder(); response.add("id", datasetId); From 7e8180d6a73e9fcc1aa35bb2ecc2743e365a8b98 Mon Sep 17 00:00:00 2001 From: Vera Clemens Date: Thu, 15 May 2025 11:49:52 +0200 Subject: [PATCH 019/247] test: add test for listing links of a dataset --- .../edu/harvard/iq/dataverse/api/LinkIT.java | 103 +++++++++++++++++- 1 file changed, 99 insertions(+), 4 deletions(-) diff --git a/src/test/java/edu/harvard/iq/dataverse/api/LinkIT.java b/src/test/java/edu/harvard/iq/dataverse/api/LinkIT.java index 2f46960f9a8..7202972141e 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/LinkIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/LinkIT.java @@ -4,13 +4,18 @@ import io.restassured.path.json.JsonPath; import io.restassured.response.Response; +import java.io.StringReader; +import java.util.ArrayList; +import java.util.List; import java.util.logging.Logger; -import static jakarta.ws.rs.core.Response.Status.CREATED; -import static jakarta.ws.rs.core.Response.Status.FORBIDDEN; -import static jakarta.ws.rs.core.Response.Status.OK; + +import static jakarta.ws.rs.core.Response.Status.*; import static org.hamcrest.CoreMatchers.equalTo; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; +import jakarta.json.Json; +import jakarta.json.JsonArray; +import jakarta.json.JsonObject; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -199,4 +204,94 @@ public void testDeepLinks() { } + @Test + public void testListLinks() { + + Response createUser1 = UtilIT.createRandomUser(); + createUser1.prettyPrint(); + createUser1.then().assertThat() + .statusCode(OK.getStatusCode()); + String apiToken1 = UtilIT.getApiTokenFromResponse(createUser1); + + Response createUser2 = UtilIT.createRandomUser(); + createUser2.prettyPrint(); + createUser2.then().assertThat() + .statusCode(OK.getStatusCode()); + String username2 = UtilIT.getUsernameFromResponse(createUser2); + String apiToken2 = UtilIT.getApiTokenFromResponse(createUser2); + + // Create dataverse1 which both user1 and user2 have admin access to + Response createDataverse1 = UtilIT.createRandomDataverse(apiToken1); + createDataverse1.prettyPrint(); + createDataverse1.then().assertThat() + .statusCode(CREATED.getStatusCode()); + String dataverse1Alias = UtilIT.getAliasFromResponse(createDataverse1); + + Response grantUser2AccessOnDataverse = UtilIT.grantRoleOnDataverse(dataverse1Alias, "admin", "@" + username2, apiToken1); + grantUser2AccessOnDataverse.prettyPrint(); + grantUser2AccessOnDataverse.then().assertThat() + .statusCode(OK.getStatusCode()); + + // Create dataset in dataverse1 + Response createDataset = UtilIT.createRandomDatasetViaNativeApi(dataverse1Alias, apiToken1); + createDataset.prettyPrint(); + createDataset.then().assertThat() + .statusCode(CREATED.getStatusCode()); + String datasetPid = JsonPath.from(createDataset.asString()).getString("data.persistentId"); + + // Create another unpublished dataverse2 as user2, and don't grant user1 any permissions on it + // Which means that user1 should not have permission to view this dataverse before it is published + Response createDataverse2 = UtilIT.createRandomDataverse(apiToken2); + createDataverse2.prettyPrint(); + createDataverse2.then().assertThat() + .statusCode(CREATED.getStatusCode()); + String dataverse2Alias = UtilIT.getAliasFromResponse(createDataverse2); + Integer dataverse2Id = UtilIT.getDatasetIdFromResponse(createDataverse2); + + // User1 doesn't have permission to link the dataset to the unpublished dataverse2 + Response tryToLinkToUnpublishedDataverseResponse = UtilIT.linkDataset(datasetPid, dataverse2Alias, apiToken1); + tryToLinkToUnpublishedDataverseResponse.prettyPrint(); + tryToLinkToUnpublishedDataverseResponse.then().assertThat() + .statusCode(UNAUTHORIZED.getStatusCode()); + + // But user2 does have permission to link the dataset to his own unpublished dataverse2 + Response linkDatasetToUnpublishedDataverseResponse = UtilIT.linkDataset(datasetPid, dataverse2Alias, apiToken2); + linkDatasetToUnpublishedDataverseResponse.prettyPrint(); + linkDatasetToUnpublishedDataverseResponse.then().assertThat() + .statusCode(OK.getStatusCode()); + + // User1 has permission to list the links of the dataset, but cannot see the link to the unpublished dataverse2 + Response linkDatasetsResponse = UtilIT.getDatasetLinks(datasetPid, apiToken1); + linkDatasetsResponse.prettyPrint(); + linkDatasetsResponse.then().assertThat() + .statusCode(OK.getStatusCode()); + JsonObject linkDatasets = Json.createReader(new StringReader(linkDatasetsResponse.asString())).readObject(); + JsonArray linksList = linkDatasets.getJsonObject("data").getJsonArray("linked-dataverses"); + assertEquals(0, linksList.size()); + + // User2 has permission to list the links of the dataset and can see the link to the unpublished dataverse2 + Response linkDatasetsResponse2 = UtilIT.getDatasetLinks(datasetPid, apiToken2); + linkDatasetsResponse2.prettyPrint(); + linkDatasetsResponse2.then().assertThat() + .statusCode(OK.getStatusCode()); + JsonObject linkDatasets2 = Json.createReader(new StringReader(linkDatasetsResponse2.asString())).readObject(); + JsonArray linksList2 = linkDatasets2.getJsonObject("data").getJsonArray("linked-dataverses"); + assertEquals(1, linksList2.size()); + assertEquals(dataverse2Id, linksList2.getJsonObject(0).getInt("id")); + + UtilIT.publishDataverseViaNativeApi(dataverse2Alias, apiToken2).then().assertThat() + .statusCode(OK.getStatusCode()); + + // After publishing dataverse2, user1 can now also see the link + Response linkDatasetsResponse3 = UtilIT.getDatasetLinks(datasetPid, apiToken1); + linkDatasetsResponse3.prettyPrint(); + linkDatasetsResponse3.then().assertThat() + .statusCode(OK.getStatusCode()); + JsonObject linkDatasets3 = Json.createReader(new StringReader(linkDatasetsResponse3.asString())).readObject(); + JsonArray linksList3 = linkDatasets3.getJsonObject("data").getJsonArray("linked-dataverses"); + assertEquals(1, linksList3.size()); + assertEquals(dataverse2Id, linksList3.getJsonObject(0).getInt("id")); + + } + } From 1b3fdcc0697ad9ec2c470cf21225d602c05ff731 Mon Sep 17 00:00:00 2001 From: Vera Clemens Date: Thu, 15 May 2025 12:58:59 +0200 Subject: [PATCH 020/247] test: extend test for listing links of a dataset --- .../edu/harvard/iq/dataverse/api/LinkIT.java | 61 ++++++++++++++++++- .../edu/harvard/iq/dataverse/api/UtilIT.java | 10 +-- 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/src/test/java/edu/harvard/iq/dataverse/api/LinkIT.java b/src/test/java/edu/harvard/iq/dataverse/api/LinkIT.java index 7202972141e..691d820b511 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/LinkIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/LinkIT.java @@ -220,19 +220,23 @@ public void testListLinks() { String username2 = UtilIT.getUsernameFromResponse(createUser2); String apiToken2 = UtilIT.getApiTokenFromResponse(createUser2); - // Create dataverse1 which both user1 and user2 have admin access to + // Create and publish dataverse1 which both user1 and user2 have admin access to Response createDataverse1 = UtilIT.createRandomDataverse(apiToken1); createDataverse1.prettyPrint(); createDataverse1.then().assertThat() .statusCode(CREATED.getStatusCode()); String dataverse1Alias = UtilIT.getAliasFromResponse(createDataverse1); + UtilIT.publishDataverseViaNativeApi(dataverse1Alias, apiToken1).then().assertThat() + .statusCode(OK.getStatusCode()); + Response grantUser2AccessOnDataverse = UtilIT.grantRoleOnDataverse(dataverse1Alias, "admin", "@" + username2, apiToken1); grantUser2AccessOnDataverse.prettyPrint(); grantUser2AccessOnDataverse.then().assertThat() .statusCode(OK.getStatusCode()); - // Create dataset in dataverse1 + // Create unpublished dataset in dataverse1 + // Which means that both user1 and user2 have permission to view it Response createDataset = UtilIT.createRandomDatasetViaNativeApi(dataverse1Alias, apiToken1); createDataset.prettyPrint(); createDataset.then().assertThat() @@ -240,7 +244,7 @@ public void testListLinks() { String datasetPid = JsonPath.from(createDataset.asString()).getString("data.persistentId"); // Create another unpublished dataverse2 as user2, and don't grant user1 any permissions on it - // Which means that user1 should not have permission to view this dataverse before it is published + // Which means that user1 doesn't have permission to view this dataverse before it is published Response createDataverse2 = UtilIT.createRandomDataverse(apiToken2); createDataverse2.prettyPrint(); createDataverse2.then().assertThat() @@ -292,6 +296,57 @@ public void testListLinks() { assertEquals(1, linksList3.size()); assertEquals(dataverse2Id, linksList3.getJsonObject(0).getInt("id")); + // Create user3 without permissions on the unpublished dataset + Response createUser3 = UtilIT.createRandomUser(); + createUser3.prettyPrint(); + createUser3.then().assertThat() + .statusCode(OK.getStatusCode()); + String username3 = UtilIT.getUsernameFromResponse(createUser3); + String apiToken3 = UtilIT.getApiTokenFromResponse(createUser3); + + // User3 cannot list the links of the unpublished dataset + Response linkDatasetsResponse4 = UtilIT.getDatasetLinks(datasetPid, apiToken3); + linkDatasetsResponse4.prettyPrint(); + linkDatasetsResponse4.then().assertThat() + .statusCode(FORBIDDEN.getStatusCode()); + + // Grant user3 "member" role on dataverse1, which allows viewing unpublished datasets + Response grantUser3AccessOnDataverse = UtilIT.grantRoleOnDataverse(dataverse1Alias, "member", "@" + username3, apiToken1); + grantUser3AccessOnDataverse.prettyPrint(); + grantUser3AccessOnDataverse.then().assertThat() + .statusCode(OK.getStatusCode()); + + // User 3 can now also list the links + Response linkDatasetsResponse5 = UtilIT.getDatasetLinks(datasetPid, apiToken3); + linkDatasetsResponse5.prettyPrint(); + linkDatasetsResponse5.then().assertThat() + .statusCode(OK.getStatusCode()); + JsonObject linkDatasets5 = Json.createReader(new StringReader(linkDatasetsResponse5.asString())).readObject(); + JsonArray linksList5 = linkDatasets5.getJsonObject("data").getJsonArray("linked-dataverses"); + assertEquals(1, linksList5.size()); + assertEquals(dataverse2Id, linksList5.getJsonObject(0).getInt("id")); + + // Non-authenticated user cannot list the links of the unpublished dataset + Response linkDatasetsResponse6 = UtilIT.getDatasetLinks(datasetPid, null); + linkDatasetsResponse6.prettyPrint(); + linkDatasetsResponse6.then().assertThat() + .statusCode(FORBIDDEN.getStatusCode()); + + Response publishDatasetResponse = UtilIT.publishDatasetViaNativeApi(datasetPid, "major", apiToken1); + publishDatasetResponse.prettyPrint(); + publishDatasetResponse.then().assertThat() + .statusCode(OK.getStatusCode()); + + // After publishing the dataset, non-authenticated user can now also see the link + Response linkDatasetsResponse7 = UtilIT.getDatasetLinks(datasetPid, null); + linkDatasetsResponse7.prettyPrint(); + linkDatasetsResponse7.then().assertThat() + .statusCode(OK.getStatusCode()); + JsonObject linkDatasets7 = Json.createReader(new StringReader(linkDatasetsResponse7.asString())).readObject(); + JsonArray linksList7 = linkDatasets7.getJsonObject("data").getJsonArray("linked-dataverses"); + assertEquals(1, linksList7.size()); + assertEquals(dataverse2Id, linksList7.getJsonObject(0).getInt("id")); + } } 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 bd54da22f43..10bea5102c3 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -2053,10 +2053,12 @@ static Response linkDataset(String datasetToLinkPid, String dataverseAlias, Stri } static Response getDatasetLinks(String datasetToLinkPid, String apiToken) { - Response response = given() - .header(API_TOKEN_HTTP_HEADER, apiToken) - .get("api/datasets/:persistentId/links" + "?persistentId=" + datasetToLinkPid); - return response; + RequestSpecification requestSpecification = given(); + if (apiToken != null) { + requestSpecification = given() + .header(UtilIT.API_TOKEN_HTTP_HEADER, apiToken); + } + return requestSpecification.get("api/datasets/:persistentId/links" + "?persistentId=" + datasetToLinkPid); } static Response createDataverseLink(String linkedDataverseAlias, String linkingDataverseAlias, String apiToken) { From e5fd1339fdaf5bdd9ee67347bc04739fef3ae5cc Mon Sep 17 00:00:00 2001 From: Vera Clemens Date: Thu, 15 May 2025 12:59:49 +0200 Subject: [PATCH 021/247] docs: add release note for #11492 --- doc/release-notes/11492-list-dataset-links.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 doc/release-notes/11492-list-dataset-links.md diff --git a/doc/release-notes/11492-list-dataset-links.md b/doc/release-notes/11492-list-dataset-links.md new file mode 100644 index 00000000000..0a5a6f7a198 --- /dev/null +++ b/doc/release-notes/11492-list-dataset-links.md @@ -0,0 +1 @@ +The [API for listing the collections a dataset has been linked to](https://guides.dataverse.org/en/latest/admin/dataverses-datasets.html#list-collections-that-are-linked-from-a-dataset) (`api/datasets/$linked-dataset-id/links`) is no longer restricted to superusers. For unpublished datasets, users need the "View Unpublished Dataset" permission to access the API. Unpublished collections in the list require the "View Unpublished Dataverse" permission; otherwise, they are hidden. \ No newline at end of file From 97ef00b2e6494525b3c70d30047fe2446ea7e426 Mon Sep 17 00:00:00 2001 From: Vera Clemens Date: Fri, 16 May 2025 16:04:58 +0200 Subject: [PATCH 022/247] fix: fix permission check when listing links of dataset --- src/main/java/edu/harvard/iq/dataverse/api/Datasets.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java index 5cbc1b60c89..725013e5bae 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -2024,7 +2024,7 @@ public Response getLinks(@Context ContainerRequestContext crc, @PathParam("id") User u = getRequestUser(crc); Dataset dataset = findDatasetOrDie(idSupplied); - if (!dataset.isReleased() && !permissionService.hasPermissionsFor(u, dataset.getOwner(), EnumSet.of(Permission.ViewUnpublishedDataset))) { + if (!dataset.isReleased() && !permissionService.hasPermissionsFor(u, dataset, EnumSet.of(Permission.ViewUnpublishedDataset))) { return error(Response.Status.FORBIDDEN, "User is not allowed to list the link(s) of this dataset"); } From 999e426a6e394422f6b8a0d71e79a4565a3db65a Mon Sep 17 00:00:00 2001 From: Vera Clemens Date: Fri, 16 May 2025 16:05:15 +0200 Subject: [PATCH 023/247] test: fix test for listing links of dataset --- .../edu/harvard/iq/dataverse/api/LinkIT.java | 37 ++++++++++++------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/src/test/java/edu/harvard/iq/dataverse/api/LinkIT.java b/src/test/java/edu/harvard/iq/dataverse/api/LinkIT.java index 691d820b511..65b68e62d22 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/LinkIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/LinkIT.java @@ -235,7 +235,7 @@ public void testListLinks() { grantUser2AccessOnDataverse.then().assertThat() .statusCode(OK.getStatusCode()); - // Create unpublished dataset in dataverse1 + // Create and publish dataset in dataverse1 // Which means that both user1 and user2 have permission to view it Response createDataset = UtilIT.createRandomDatasetViaNativeApi(dataverse1Alias, apiToken1); createDataset.prettyPrint(); @@ -243,6 +243,11 @@ public void testListLinks() { .statusCode(CREATED.getStatusCode()); String datasetPid = JsonPath.from(createDataset.asString()).getString("data.persistentId"); + Response publishDatasetResponse = UtilIT.publishDatasetViaNativeApi(datasetPid, "major", apiToken1); + publishDatasetResponse.prettyPrint(); + publishDatasetResponse.then().assertThat() + .statusCode(OK.getStatusCode()); + // Create another unpublished dataverse2 as user2, and don't grant user1 any permissions on it // Which means that user1 doesn't have permission to view this dataverse before it is published Response createDataverse2 = UtilIT.createRandomDataverse(apiToken2); @@ -296,6 +301,13 @@ public void testListLinks() { assertEquals(1, linksList3.size()); assertEquals(dataverse2Id, linksList3.getJsonObject(0).getInt("id")); + // Create another dataset, but don't publish it + Response createDataset2 = UtilIT.createRandomDatasetViaNativeApi(dataverse1Alias, apiToken1); + createDataset2.prettyPrint(); + createDataset2.then().assertThat() + .statusCode(CREATED.getStatusCode()); + String dataset2Pid = JsonPath.from(createDataset2.asString()).getString("data.persistentId"); + // Create user3 without permissions on the unpublished dataset Response createUser3 = UtilIT.createRandomUser(); createUser3.prettyPrint(); @@ -305,7 +317,7 @@ public void testListLinks() { String apiToken3 = UtilIT.getApiTokenFromResponse(createUser3); // User3 cannot list the links of the unpublished dataset - Response linkDatasetsResponse4 = UtilIT.getDatasetLinks(datasetPid, apiToken3); + Response linkDatasetsResponse4 = UtilIT.getDatasetLinks(dataset2Pid, apiToken3); linkDatasetsResponse4.prettyPrint(); linkDatasetsResponse4.then().assertThat() .statusCode(FORBIDDEN.getStatusCode()); @@ -317,35 +329,34 @@ public void testListLinks() { .statusCode(OK.getStatusCode()); // User 3 can now also list the links - Response linkDatasetsResponse5 = UtilIT.getDatasetLinks(datasetPid, apiToken3); + Response linkDatasetsResponse5 = UtilIT.getDatasetLinks(dataset2Pid, apiToken3); linkDatasetsResponse5.prettyPrint(); linkDatasetsResponse5.then().assertThat() .statusCode(OK.getStatusCode()); JsonObject linkDatasets5 = Json.createReader(new StringReader(linkDatasetsResponse5.asString())).readObject(); JsonArray linksList5 = linkDatasets5.getJsonObject("data").getJsonArray("linked-dataverses"); - assertEquals(1, linksList5.size()); - assertEquals(dataverse2Id, linksList5.getJsonObject(0).getInt("id")); + assertEquals(0, linksList5.size()); // Non-authenticated user cannot list the links of the unpublished dataset - Response linkDatasetsResponse6 = UtilIT.getDatasetLinks(datasetPid, null); + Response linkDatasetsResponse6 = UtilIT.getDatasetLinks(dataset2Pid, null); linkDatasetsResponse6.prettyPrint(); linkDatasetsResponse6.then().assertThat() .statusCode(FORBIDDEN.getStatusCode()); - Response publishDatasetResponse = UtilIT.publishDatasetViaNativeApi(datasetPid, "major", apiToken1); - publishDatasetResponse.prettyPrint(); - publishDatasetResponse.then().assertThat() + // Publish dataset2 + Response publishDataset2Response = UtilIT.publishDatasetViaNativeApi(dataset2Pid, "major", apiToken1); + publishDataset2Response.prettyPrint(); + publishDataset2Response.then().assertThat() .statusCode(OK.getStatusCode()); - // After publishing the dataset, non-authenticated user can now also see the link - Response linkDatasetsResponse7 = UtilIT.getDatasetLinks(datasetPid, null); + // After publishing the dataset, non-authenticated user can now also list the links + Response linkDatasetsResponse7 = UtilIT.getDatasetLinks(dataset2Pid, null); linkDatasetsResponse7.prettyPrint(); linkDatasetsResponse7.then().assertThat() .statusCode(OK.getStatusCode()); JsonObject linkDatasets7 = Json.createReader(new StringReader(linkDatasetsResponse7.asString())).readObject(); JsonArray linksList7 = linkDatasets7.getJsonObject("data").getJsonArray("linked-dataverses"); - assertEquals(1, linksList7.size()); - assertEquals(dataverse2Id, linksList7.getJsonObject(0).getInt("id")); + assertEquals(0, linksList7.size()); } From 0cb26a686e1301c8605c78752ca5a4f75f58ef42 Mon Sep 17 00:00:00 2001 From: Vera Clemens Date: Wed, 28 May 2025 15:58:23 +0200 Subject: [PATCH 024/247] feat: split "LinkDataset/Dataverse" permission off of "PublishDataset/Dataverse" --- scripts/api/data/role-curator.json | 3 ++- .../java/edu/harvard/iq/dataverse/DataverseServiceBean.java | 4 ++-- .../edu/harvard/iq/dataverse/authorization/Permission.java | 2 ++ .../command/impl/DeleteDatasetLinkingDataverseCommand.java | 4 ++-- .../iq/dataverse/engine/command/impl/LinkDatasetCommand.java | 2 +- .../dataverse/engine/command/impl/LinkDataverseCommand.java | 4 ++-- src/main/java/propertyFiles/BuiltInRoles.properties | 2 +- src/main/java/propertyFiles/Bundle.properties | 4 ++++ 8 files changed, 16 insertions(+), 9 deletions(-) diff --git a/scripts/api/data/role-curator.json b/scripts/api/data/role-curator.json index 91cb7ec43e2..86f18b2ea6a 100644 --- a/scripts/api/data/role-curator.json +++ b/scripts/api/data/role-curator.json @@ -1,13 +1,14 @@ { "alias":"curator", "name":"Curator", - "description":"For datasets, a person who can edit License + Terms, edit Permissions, and publish datasets.", + "description":"For datasets, a person who can edit License + Terms, edit Permissions, and publish and link datasets.", "permissions":[ "ViewUnpublishedDataset", "EditDataset", "DownloadFile", "DeleteDatasetDraft", "PublishDataset", + "LinkDataset", "ManageDatasetPermissions", "ManageFilePermissions", "AddDataverse", diff --git a/src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java index f89e707cc03..3879b8a3c81 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java @@ -512,7 +512,7 @@ public List filterDataversesForLinking(String query, DataverseRequest for (Dataverse res : results) { if (!remove.contains(res)) { - if (this.permissionService.requestOn(req, res).has(Permission.PublishDataset)) { + if (this.permissionService.requestOn(req, res).has(Permission.LinkDataset)) { dataverseList.add(res); } } @@ -525,7 +525,7 @@ public List filterDataversesForUnLinking(String query, DataverseReque List dataverseList = new ArrayList<>(); if (alreadyLinkeddv_ids != null && !alreadyLinkeddv_ids.isEmpty()) { alreadyLinkeddv_ids.stream().map((testDVId) -> this.find(testDVId)).forEachOrdered((dataverse) -> { - if (this.permissionService.requestOn(req, dataverse).has(Permission.PublishDataset)) { + if (this.permissionService.requestOn(req, dataverse).has(Permission.LinkDataset)) { dataverseList.add(dataverse); } }); diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/Permission.java b/src/main/java/edu/harvard/iq/dataverse/authorization/Permission.java index 32937098118..83dc9965f6f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/Permission.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/Permission.java @@ -48,7 +48,9 @@ public enum Permission implements java.io.Serializable { ManageDatasetPermissions(BundleUtil.getStringFromBundle("permission.managePermissionsDataset"), true, Dataset.class), ManageFilePermissions(BundleUtil.getStringFromBundle("permission.managePermissionsDataFile"), true, DataFile.class), PublishDataverse(BundleUtil.getStringFromBundle("permission.publishDataverse"), true, Dataverse.class), + LinkDataverse(BundleUtil.getStringFromBundle("permission.linkDataverse"), true, Dataverse.class), PublishDataset(BundleUtil.getStringFromBundle("permission.publishDataset"), true, Dataset.class, Dataverse.class), + LinkDataset(BundleUtil.getStringFromBundle("permission.linkDataset"), true, Dataset.class, Dataverse.class), // Delete DeleteDataverse(BundleUtil.getStringFromBundle("permission.deleteDataverse"), true, Dataverse.class), DeleteDatasetDraft(BundleUtil.getStringFromBundle("permission.deleteDataset"), true, Dataset.class); diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DeleteDatasetLinkingDataverseCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DeleteDatasetLinkingDataverseCommand.java index 7f5672c0cd7..adc973df6a9 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DeleteDatasetLinkingDataverseCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DeleteDatasetLinkingDataverseCommand.java @@ -23,14 +23,14 @@ * @author sarahferry */ -@RequiredPermissions( Permission.PublishDataset ) +@RequiredPermissions( Permission.LinkDataset ) public class DeleteDatasetLinkingDataverseCommand extends AbstractCommand{ private final DatasetLinkingDataverse doomed; private final Dataset editedDs; private final boolean index; public DeleteDatasetLinkingDataverseCommand(DataverseRequest aRequest, Dataset editedDs , DatasetLinkingDataverse doomed, boolean index) { - super(aRequest, editedDs); + super(aRequest, doomed.getLinkingDataverse()); this.editedDs = editedDs; this.doomed = doomed; this.index = index; diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/LinkDatasetCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/LinkDatasetCommand.java index aef749d7e26..3cd3520f9f2 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/LinkDatasetCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/LinkDatasetCommand.java @@ -26,7 +26,7 @@ * * @author skraffmiller */ -@RequiredPermissions(Permission.PublishDataset) +@RequiredPermissions(Permission.LinkDataset) public class LinkDatasetCommand extends AbstractCommand { private final Dataset linkedDataset; diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/LinkDataverseCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/LinkDataverseCommand.java index 55fe96556a5..2e1aecc9a84 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/LinkDataverseCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/LinkDataverseCommand.java @@ -31,7 +31,7 @@ * * @author skraffmiller */ -@RequiredPermissions(Permission.PublishDataverse) +@RequiredPermissions(Permission.LinkDataverse) public class LinkDataverseCommand extends AbstractCommand { private final Dataverse linkedDataverse; @@ -47,7 +47,7 @@ public LinkDataverseCommand(DataverseRequest aRequest, Dataverse dataverse, Data public DataverseLinkingDataverse execute(CommandContext ctxt) throws CommandException { if ((!(getUser() instanceof AuthenticatedUser) || !getUser().isSuperuser())) { throw new PermissionException("Link Dataverse can only be called by superusers.", - this, Collections.singleton(Permission.PublishDataverse), linkingDataverse); + this, Collections.singleton(Permission.LinkDataverse), linkingDataverse); } if (linkedDataverse.equals(linkingDataverse)) { throw new IllegalCommandException("Can't link a dataverse to itself", this); diff --git a/src/main/java/propertyFiles/BuiltInRoles.properties b/src/main/java/propertyFiles/BuiltInRoles.properties index 026df600a9c..50dbb1ba80f 100644 --- a/src/main/java/propertyFiles/BuiltInRoles.properties +++ b/src/main/java/propertyFiles/BuiltInRoles.properties @@ -3,7 +3,7 @@ role.admin.description=A person who has all permissions for dataverses, datasets role.contributor.name=Contributor role.contributor.description=For datasets, a person who can edit License + Terms, and then submit them for review. role.curator.name=Curator -role.curator.description=For datasets, a person who can edit License + Terms, edit Permissions, and publish datasets. +role.curator.description=For datasets, a person who can edit License + Terms, edit Permissions, and publish and link datasets. role.dscontributor.name=Dataset Creator role.dscontributor.description=A person who can add datasets within a dataverse. role.fullcontributor.name=Dataverse + Dataset Creator diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index d7fd94bf65c..d9d802b98bb 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -2555,7 +2555,9 @@ permission.addDataverseDataverse=Add a dataverse within another dataverse permission.deleteDataset=Delete a dataset draft permission.deleteDataverse=Delete an unpublished dataverse permission.publishDataset=Publish a dataset +permission.linkDataset=Link a dataset to a dataverse permission.publishDataverse=Publish a dataverse +permission.linkDataverse=Link a dataverse to another dataverse permission.managePermissionsDataFile=Manage permissions for a file permission.managePermissionsDataset=Manage permissions for a dataset permission.managePermissionsDataverse=Manage permissions for a dataverse @@ -2907,7 +2909,9 @@ permission.EditDataset.label=EditDataset permission.ManageDataversePermissions.label=ManageDataversePermissions permission.ManageDatasetPermissions.label=ManageDatasetPermissions permission.PublishDataverse.label=PublishDataverse +permission.LinkDataverse.label=LinkDataverse permission.PublishDataset.label=PublishDataset +permission.LinkDataset.label=LinkDataset permission.DeleteDataverse.label=DeleteDataverse permission.DeleteDatasetDraft.label=DeleteDatasetDraft permission.ManageFilePermissions.label=ManageFilePermissions From 2faca6235960b04e05422dbbf9242f69b36390a6 Mon Sep 17 00:00:00 2001 From: Ludovic DANIEL Date: Wed, 28 May 2025 15:15:02 +0200 Subject: [PATCH 025/247] MakeDataCount MDC API - Adding disconnect() on HttpURLConnection in case of many calls --- .../java/edu/harvard/iq/dataverse/api/MakeDataCountApi.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/MakeDataCountApi.java b/src/main/java/edu/harvard/iq/dataverse/api/MakeDataCountApi.java index ca8f59a71be..79bc2f94094 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/MakeDataCountApi.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/MakeDataCountApi.java @@ -175,11 +175,14 @@ public Response updateCitationsForDataset(@PathParam("id") String id) throws IOE int status = connection.getResponseCode(); if (status != 200) { logger.warning("Failed to get citations from " + url.toString()); + connection.disconnect(); return error(Status.fromStatusCode(status), "Failed to get citations from " + url.toString()); } JsonObject report; try (InputStream inStream = connection.getInputStream()) { report = JsonUtil.getJsonObject(inStream); + } finally { + connection.disconnect(); } JsonObject links = report.getJsonObject("links"); JsonArray data = report.getJsonArray("data"); From d1883619f2ad3f812172a871acf2f8786fe6f54b Mon Sep 17 00:00:00 2001 From: Ludovic DANIEL Date: Wed, 28 May 2025 16:01:40 +0200 Subject: [PATCH 026/247] MakeDataCount MDC API - Use cursor method for pagination to fix infinite loop issue --- .../java/edu/harvard/iq/dataverse/api/MakeDataCountApi.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/MakeDataCountApi.java b/src/main/java/edu/harvard/iq/dataverse/api/MakeDataCountApi.java index 79bc2f94094..562fd7fcb81 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/MakeDataCountApi.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/MakeDataCountApi.java @@ -160,7 +160,7 @@ public Response updateCitationsForDataset(@PathParam("id") String id) throws IOE url = new URI(JvmSettings.DATACITE_REST_API_URL.lookup(pidProvider.getId()) + "/events?doi=" + authorityPlusIdentifier + - "&source=crossref&page[size]=1000").toURL(); + "&source=crossref&page[size]=1000&page[cursor]=1").toURL(); } catch (URISyntaxException e) { //Nominally this means a config error/ bad DATACITE_REST_API_URL for this provider logger.warning("Unable to create URL for " + persistentId + ", pidProvider " + pidProvider.getId()); From a98cc35a18f65e8cbb0fd73cfa935008b966af81 Mon Sep 17 00:00:00 2001 From: Vera Clemens Date: Wed, 28 May 2025 16:01:51 +0200 Subject: [PATCH 027/247] test: extend test of permissions required for linking/unlinking datasets --- .../harvard/iq/dataverse/api/DatasetsIT.java | 38 +++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java index 7a227b02349..fa690687564 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java @@ -2792,13 +2792,19 @@ public void testDcmChecksumValidationMessages() throws IOException, InterruptedE @Test public void testCreateDeleteDatasetLink() { + // Create superuser Response createUser = UtilIT.createRandomUser(); createUser.prettyPrint(); String username = UtilIT.getUsernameFromResponse(createUser); String apiToken = UtilIT.getApiTokenFromResponse(createUser); - Response superuserResponse = UtilIT.makeSuperUser(username); + // Create another user that doesn't have permission to create/delete links + Response createUser2 = UtilIT.createRandomUser(); + createUser2.prettyPrint(); + String username2 = UtilIT.getUsernameFromResponse(createUser2); + String apiToken2 = UtilIT.getApiTokenFromResponse(createUser2); + Response createDataverseResponse = UtilIT.createRandomDataverse(apiToken); createDataverseResponse.prettyPrint(); String dataverseAlias = UtilIT.getAliasFromResponse(createDataverseResponse); @@ -2834,28 +2840,31 @@ public void testCreateDeleteDatasetLink() { publishDatasetForLinking.prettyPrint(); publishTargetDataverse.then().assertThat() .statusCode(OK.getStatusCode()); - - // And link the dataset to this new dataverse: + + // Try to link the dataset to the new dataverse without LinkDataset permissions + createLinkingDatasetResponse = UtilIT.createDatasetLink(datasetId.longValue(), dataverseAlias, apiToken2); + createLinkingDatasetResponse.prettyPrint(); + createLinkingDatasetResponse.then().assertThat() + .body("message", equalTo("User @" + username2 + " is not permitted to perform requested action.")) + .statusCode(UNAUTHORIZED.getStatusCode()); + + // Link the dataset to the new dataverse createLinkingDatasetResponse = UtilIT.createDatasetLink(datasetId.longValue(), dataverseAlias, apiToken); createLinkingDatasetResponse.prettyPrint(); createLinkingDatasetResponse.then().assertThat() .body("data.message", equalTo("Dataset " + datasetId +" linked successfully to " + dataverseAlias)) .statusCode(200); - // Create a new user that doesn't have permission to delete the link - Response createUser2 = UtilIT.createRandomUser(); - createUser2.prettyPrint(); - String username2 = UtilIT.getUsernameFromResponse(createUser2); - String apiToken2 = UtilIT.getApiTokenFromResponse(createUser2); - // Try to delete the link without PublishDataset permissions + // Try to delete the link without LinkDataset permissions Response deleteLinkingDatasetResponse = UtilIT.deleteDatasetLink(datasetId.longValue(), dataverseAlias, apiToken2); deleteLinkingDatasetResponse.prettyPrint(); deleteLinkingDatasetResponse.then().assertThat() .body("message", equalTo("User @" + username2 + " is not permitted to perform requested action.")) .statusCode(UNAUTHORIZED.getStatusCode()); - // Add the Curator role to this user to show that they can delete the link later. (Timing issues if you try to delete right after giving permission) - Response givePermissionResponse = UtilIT.grantRoleOnDataset(datasetPersistentId, "curator", "@" + username2, apiToken); + // Give the user curator rights for the target dataverse to show that they can add and delete the link later + // (Timing issues if you try to add or delete right after giving permission) + Response givePermissionResponse = UtilIT.grantRoleOnDataverse(dataverseAlias, "curator", "@" + username2, apiToken); givePermissionResponse.prettyPrint(); givePermissionResponse.then().assertThat() .statusCode(200); @@ -2868,17 +2877,16 @@ public void testCreateDeleteDatasetLink() { .body("data.message", equalTo("Link from Dataset " + datasetId + " to linked Dataverse " + dataverseAlias + " deleted")) .statusCode(200); - // And re-link the dataset to this new dataverse: - createLinkingDatasetResponse = UtilIT.createDatasetLink(datasetId.longValue(), dataverseAlias, apiToken); + // And now test linking the dataset as user2 with new role as curator (link permissions): + createLinkingDatasetResponse = UtilIT.createDatasetLink(datasetId.longValue(), dataverseAlias, apiToken2); createLinkingDatasetResponse.prettyPrint(); createLinkingDatasetResponse.then().assertThat() .body("data.message", equalTo("Dataset " + datasetId +" linked successfully to " + dataverseAlias)) .statusCode(200); - // And now test deleting it as user2 with new role as curator (Publish permissions): + // And now test deleting it as user2 with new role as curator (link permissions): deleteLinkingDatasetResponse = UtilIT.deleteDatasetLink(datasetId.longValue(), dataverseAlias, apiToken2); deleteLinkingDatasetResponse.prettyPrint(); - deleteLinkingDatasetResponse.then().assertThat() .body("data.message", equalTo("Link from Dataset " + datasetId + " to linked Dataverse " + dataverseAlias + " deleted")) .statusCode(200); From 4f516b2aad7d0c15fefb825f3f7bcc5ed715505d Mon Sep 17 00:00:00 2001 From: Vera Clemens Date: Wed, 28 May 2025 16:02:22 +0200 Subject: [PATCH 028/247] docs: update documentation of linking/unlinking APIs --- doc/sphinx-guides/source/admin/dataverses-datasets.rst | 4 ++-- doc/sphinx-guides/source/user/dataverse-management.rst | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/sphinx-guides/source/admin/dataverses-datasets.rst b/doc/sphinx-guides/source/admin/dataverses-datasets.rst index c6d325a9651..a37819c90e1 100644 --- a/doc/sphinx-guides/source/admin/dataverses-datasets.rst +++ b/doc/sphinx-guides/source/admin/dataverses-datasets.rst @@ -118,7 +118,7 @@ Moves a dataset whose id is passed to a Dataverse collection whose alias is pass Link a Dataset ^^^^^^^^^^^^^^ -Creates a link between a dataset and a Dataverse collection (see the :ref:`dataset-linking` section of the User Guide for more information). :: +Creates a link between a dataset and a Dataverse collection (see the :ref:`dataset-linking` section of the User Guide for more information). Accessible to users with Link Dataset permission on the Dataverse collection. :: curl -H "X-Dataverse-key: $API_TOKEN" -X PUT http://$SERVER/api/datasets/$linked-dataset-id/link/$linking-dataverse-alias @@ -155,7 +155,7 @@ It returns a list in the following format (new format as of v6.4): Unlink a Dataset ^^^^^^^^^^^^^^^^ -Removes a link between a dataset and a Dataverse collection. Accessible to users with Publish Dataset permissions. :: +Removes a link between a dataset and a Dataverse collection. Accessible to users with Link Dataset permission on the Dataverse collection. :: curl -H "X-Dataverse-key: $API_TOKEN" -X DELETE http://$SERVER/api/datasets/$linked-dataset-id/deleteLink/$linking-dataverse-alias diff --git a/doc/sphinx-guides/source/user/dataverse-management.rst b/doc/sphinx-guides/source/user/dataverse-management.rst index ecb1f608c12..bec155e3d32 100755 --- a/doc/sphinx-guides/source/user/dataverse-management.rst +++ b/doc/sphinx-guides/source/user/dataverse-management.rst @@ -215,7 +215,7 @@ Dataset linking allows a Dataverse collection owner to "link" their Dataverse co For example, researchers working on a collaborative study across institutions can each link their own individual institutional Dataverse collections to the one collaborative dataset, making it easier for interested parties from each institution to find the study. -In order to link a dataset, you will need your account to have the "Publish Dataset" permission on the Dataverse collection that is doing the linking. If you created the Dataverse collection then you should have this permission already, but if not then you will need to ask the admin of that Dataverse collection to assign that permission to your account. You do not need any special permissions on the dataset being linked. +In order to link a dataset, you will need your account to have the "Link Dataset" permission on the Dataverse collection that is doing the linking. If you created the Dataverse collection then you should have this permission already, but if not then you will need to ask the admin of that Dataverse collection to assign that permission to your account. You do not need any special permissions on the dataset being linked. To link a dataset to your Dataverse collection, you must navigate to that dataset and click the white "Link" button in the upper-right corner of the dataset page. This will open up a window where you can type in the name of the Dataverse collection that you would like to link the dataset to. Select your Dataverse collection and click the save button. This will establish the link, and the dataset will now appear under your Dataverse collection. From 89a59e2b5d54a5b9a474de410236198c50dc59b0 Mon Sep 17 00:00:00 2001 From: Vera Clemens Date: Wed, 28 May 2025 16:15:26 +0200 Subject: [PATCH 029/247] docs: add release note for #11534 --- doc/release-notes/11534-link-permissions.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 doc/release-notes/11534-link-permissions.md diff --git a/doc/release-notes/11534-link-permissions.md b/doc/release-notes/11534-link-permissions.md new file mode 100644 index 00000000000..29251c1b7d9 --- /dev/null +++ b/doc/release-notes/11534-link-permissions.md @@ -0,0 +1,3 @@ +Linking or unlinking a dataset or dataverse now requires the new "Link Dataset/Dataverse" permission. +Previously, this action was covered by the "Publish Dataset/Dataverse" permission. +Linking and publishing permissions can now be granted separately, allowing for more fine-grained access control. \ No newline at end of file From fd741c96be80477f84aabbdd0fc1edd21f7c3ece Mon Sep 17 00:00:00 2001 From: Vera Clemens Date: Tue, 3 Jun 2025 18:07:38 +0200 Subject: [PATCH 030/247] feat: index datasetCount of dataverses --- conf/solr/schema.xml | 1 + .../iq/dataverse/DatasetServiceBean.java | 2 +- .../iq/dataverse/DataverseServiceBean.java | 30 ++++++++++++++++--- .../command/impl/LinkDatasetCommand.java | 10 +++++++ .../iq/dataverse/search/IndexServiceBean.java | 9 ++++++ .../iq/dataverse/search/SearchFields.java | 1 + .../dataverse/search/SearchServiceBean.java | 2 ++ .../iq/dataverse/search/SolrSearchResult.java | 13 ++++++++ 8 files changed, 63 insertions(+), 5 deletions(-) diff --git a/conf/solr/schema.xml b/conf/solr/schema.xml index 50835957b04..4d87f59df91 100644 --- a/conf/solr/schema.xml +++ b/conf/solr/schema.xml @@ -241,6 +241,7 @@ + - - - + + - + - - + + - - + + - + - + - - + + - - + + - - + + - + @@ -114,13 +138,13 @@ - + - - + + @@ -141,20 +165,20 @@

#{DatasetPage.datasetVersionUI.title.value}

- - - - + + + + - + - + - - + +
@@ -163,10 +187,10 @@
- + - + @@ -184,70 +208,70 @@