From feb758160e29555cefa7ed365aae68e900e0ab1f Mon Sep 17 00:00:00 2001 From: qqmyers Date: Tue, 23 Dec 2025 17:09:32 -0500 Subject: [PATCH 01/12] require embargo reason option --- .../edu/harvard/iq/dataverse/DatasetPage.java | 15 +++++++++++++++ .../edu/harvard/iq/dataverse/FilePage.java | 18 ++++++++++++++++++ .../harvard/iq/dataverse/api/Datasets.java | 10 ++++++++-- .../iq/dataverse/settings/FeatureFlags.java | 6 +++++- src/main/java/propertyFiles/Bundle.properties | 2 ++ .../webapp/file-edit-popup-fragment.xhtml | 19 ++++++++++++++++--- 6 files changed, 64 insertions(+), 6 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java index 20617160a1c..1c70016ade4 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java @@ -160,6 +160,7 @@ import edu.harvard.iq.dataverse.search.SearchFields; import edu.harvard.iq.dataverse.search.SearchUtil; import edu.harvard.iq.dataverse.search.SolrClientService; +import edu.harvard.iq.dataverse.settings.FeatureFlags; import edu.harvard.iq.dataverse.settings.JvmSettings; import edu.harvard.iq.dataverse.util.SignpostingResources; import edu.harvard.iq.dataverse.util.FileMetadataUtil; @@ -6872,4 +6873,18 @@ public void setRequestedCSL(String requestedCSL) { this.requestedCSL = requestedCSL; } + public void validateEmbargoReasonNotBlank(FacesContext context, UIComponent component, Object value) { + if (value == null && FeatureFlags.REQUIRE_EMBARGO_REASON.enabled()) { + throw new ValidatorException( + new FacesMessage(FacesMessage.SEVERITY_ERROR, + BundleUtil.getStringFromBundle("embargo.reason.required"), null) + ); + } + if (value != null && value.toString().trim().isEmpty()) { + throw new ValidatorException( + new FacesMessage(FacesMessage.SEVERITY_ERROR, + BundleUtil.getStringFromBundle("embargo.reason.blank"), null) + ); + } + } } diff --git a/src/main/java/edu/harvard/iq/dataverse/FilePage.java b/src/main/java/edu/harvard/iq/dataverse/FilePage.java index dd1bd56d5bd..f56a1c740fe 100644 --- a/src/main/java/edu/harvard/iq/dataverse/FilePage.java +++ b/src/main/java/edu/harvard/iq/dataverse/FilePage.java @@ -35,6 +35,7 @@ import edu.harvard.iq.dataverse.makedatacount.MakeDataCountLoggingServiceBean; import edu.harvard.iq.dataverse.makedatacount.MakeDataCountLoggingServiceBean.MakeDataCountEntry; import edu.harvard.iq.dataverse.privateurl.PrivateUrlServiceBean; +import edu.harvard.iq.dataverse.settings.FeatureFlags; import edu.harvard.iq.dataverse.settings.JvmSettings; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; import edu.harvard.iq.dataverse.util.BundleUtil; @@ -60,7 +61,9 @@ import jakarta.ejb.EJB; import jakarta.ejb.EJBException; import jakarta.faces.application.FacesMessage; +import jakarta.faces.component.UIComponent; import jakarta.faces.context.FacesContext; +import jakarta.faces.validator.ValidatorException; import jakarta.faces.view.ViewScoped; import jakarta.inject.Inject; import jakarta.inject.Named; @@ -1489,4 +1492,19 @@ public String editFileMetadata(){ return ""; } + public void validateEmbargoReasonNotBlank(FacesContext context, UIComponent component, Object value) { + if (value == null && FeatureFlags.REQUIRE_EMBARGO_REASON.enabled()) { + throw new ValidatorException( + new FacesMessage(FacesMessage.SEVERITY_ERROR, + BundleUtil.getStringFromBundle("embargo.reason.required"), null) + ); + } + if (value != null && value.toString().trim().isEmpty()) { + throw new ValidatorException( + new FacesMessage(FacesMessage.SEVERITY_ERROR, + BundleUtil.getStringFromBundle("embargo.reason.blank"), null) + ); + } + } + } 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 2378388c540..25fe20b0640 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -1513,8 +1513,14 @@ public Response createFileEmbargo(@Context ContainerRequestContext crc, @PathPar return error(Status.BAD_REQUEST, "Date available can not exceed MaxEmbargoDurationInMonths: "+maxEmbargoDurationInMonths); } } - - embargo.setReason(json.getString("reason")); + String reason = null; + if(json.containsKey("reason")) { + reason = json.getString("reason"); + } + if(reason.isBlank() && FeatureFlags.REQUIRE_EMBARGO_REASON.enabled()) { + return error(Status.BAD_REQUEST, "Reason is required for embargoes"); + } + embargo.setReason(reason); List datasetFiles = dataset.getFiles(); List filesToEmbargo = new LinkedList<>(); diff --git a/src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java b/src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java index 2e86fae610e..e1c7e69f7db 100644 --- a/src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java +++ b/src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java @@ -249,7 +249,11 @@ public enum FeatureFlags { * @since Dataverse 6.9 */ ONLY_UPDATE_DATACITE_WHEN_NEEDED("only-update-datacite-when-needed"), - + + /** Require Embargo Reason. By default, adding a reason when embargoing is optional. This + * flag makes a reason required, both in the UI and API. + */ + REQUIRE_EMBARGO_REASON("require-embargo-reason"), ; final String flag; diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index f801c762752..efc1b0a5921 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -41,6 +41,8 @@ embargoed.wasthrough=Was embargoed until embargoed.willbeuntil=Draft: will be embargoed until embargo.date.invalid=Date is outside the allowed range: ({0} to {1}) embargo.date.required=An embargo date is required +embargo.reason.required=An embargo reason is required +embargo.reason.blank=An embargo reason cannot be blank retention.after=Was retained until retention.isfrom=Is retained until retention.willbeafter=Draft: will be retained until diff --git a/src/main/webapp/file-edit-popup-fragment.xhtml b/src/main/webapp/file-edit-popup-fragment.xhtml index 3b1141816c8..e717efc4a60 100644 --- a/src/main/webapp/file-edit-popup-fragment.xhtml +++ b/src/main/webapp/file-edit-popup-fragment.xhtml @@ -123,8 +123,9 @@ maxdate="#{settingsWrapper.maxEmbargoDate}" disabled="#{bean.removeEmbargo}" validator="#{settingsWrapper.validateEmbargoDate}" > -
@@ -133,12 +134,23 @@
-

#{bundle['file.editEmbargoDialog.reason.tip']}

+ + + +
+ +
@@ -153,8 +165,9 @@ - From d2ff816836b4fcfde0a7938f7b4ec21a936577df Mon Sep 17 00:00:00 2001 From: qqmyers Date: Sat, 3 Jan 2026 15:58:59 -0500 Subject: [PATCH 02/12] fix validation --- .../edu/harvard/iq/dataverse/DatasetPage.java | 15 +++++++++++++++ .../java/edu/harvard/iq/dataverse/FilePage.java | 15 +++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java index 1c70016ade4..ebbcd412e24 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java @@ -6874,6 +6874,21 @@ public void setRequestedCSL(String requestedCSL) { } public void validateEmbargoReasonNotBlank(FacesContext context, UIComponent component, Object value) { + + // Skip validation if removing embargo + if (removeEmbargo) { + return; + } + + // Get the source of the current request + String source = context.getExternalContext().getRequestParameterMap() + .get("jakarta.faces.source"); + + // Only validate if the save button triggered this + if (source == null || !source.contains("fileEmbargoPopupSaveButton")) { + return; + } + if (value == null && FeatureFlags.REQUIRE_EMBARGO_REASON.enabled()) { throw new ValidatorException( new FacesMessage(FacesMessage.SEVERITY_ERROR, diff --git a/src/main/java/edu/harvard/iq/dataverse/FilePage.java b/src/main/java/edu/harvard/iq/dataverse/FilePage.java index f56a1c740fe..4a2b8fa7b00 100644 --- a/src/main/java/edu/harvard/iq/dataverse/FilePage.java +++ b/src/main/java/edu/harvard/iq/dataverse/FilePage.java @@ -1493,6 +1493,21 @@ public String editFileMetadata(){ } public void validateEmbargoReasonNotBlank(FacesContext context, UIComponent component, Object value) { + + // Skip validation if removing embargo + if (removeEmbargo) { + return; + } + + // Get the source of the current request + String source = context.getExternalContext().getRequestParameterMap() + .get("jakarta.faces.source"); + + // Only validate if the save button triggered this + if (source == null || !source.contains("fileEmbargoPopupSaveButton")) { + return; + } + if (value == null && FeatureFlags.REQUIRE_EMBARGO_REASON.enabled()) { throw new ValidatorException( new FacesMessage(FacesMessage.SEVERITY_ERROR, From 139e125cd2b209a448e6138aafede3e539d80ca5 Mon Sep 17 00:00:00 2001 From: qqmyers Date: Sat, 3 Jan 2026 15:59:17 -0500 Subject: [PATCH 03/12] update api to also check the optional/not blank case --- src/main/java/edu/harvard/iq/dataverse/api/Datasets.java | 4 +++- 1 file changed, 3 insertions(+), 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 25fe20b0640..7c4d4b0d4cb 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -1517,8 +1517,10 @@ public Response createFileEmbargo(@Context ContainerRequestContext crc, @PathPar if(json.containsKey("reason")) { reason = json.getString("reason"); } - if(reason.isBlank() && FeatureFlags.REQUIRE_EMBARGO_REASON.enabled()) { + if(reason == null && FeatureFlags.REQUIRE_EMBARGO_REASON.enabled()) { return error(Status.BAD_REQUEST, "Reason is required for embargoes"); + } else if(reason.isBlank()) { + return error(Status.BAD_REQUEST, "Reason cannot be blank (whitespace only)"); } embargo.setReason(reason); From 2fdf39d9def9046675866324a5d8abf47cbd26d5 Mon Sep 17 00:00:00 2001 From: qqmyers Date: Sat, 3 Jan 2026 15:59:27 -0500 Subject: [PATCH 04/12] release note --- doc/release-notes/12067-require-embargo-reason.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 doc/release-notes/12067-require-embargo-reason.md diff --git a/doc/release-notes/12067-require-embargo-reason.md b/doc/release-notes/12067-require-embargo-reason.md new file mode 100644 index 00000000000..094d2270703 --- /dev/null +++ b/doc/release-notes/12067-require-embargo-reason.md @@ -0,0 +1,8 @@ +It is now possible to configure Dataverse to require an embargo reason when a user creates an embargo on one or more files. +By default the embargo reason is optional. + +In addition, with this release, if an embargo reason is supplied, it must not be blank. + +New Feature Flag: + +dataverse.feature.require-embargo-reason - default false From d706a7914f355d9e3c6d87bcf8f9da9651ca433d Mon Sep 17 00:00:00 2001 From: qqmyers Date: Sat, 3 Jan 2026 15:59:35 -0500 Subject: [PATCH 05/12] doc updates --- doc/sphinx-guides/source/api/native-api.rst | 2 +- doc/sphinx-guides/source/installation/config.rst | 3 +++ doc/sphinx-guides/source/user/dataset-management.rst | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 6c1720e2b5b..c21f11ba640 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -3607,7 +3607,7 @@ Set an Embargo on Files in a Dataset ``/api/datasets/$dataset-id/files/actions/:set-embargo`` can be used to set an embargo on one or more files in a dataset. Embargoes can be set on files that are only in a draft dataset version (and are not in any previously published version) by anyone who can edit the dataset. The same API call can be used by a superuser to add an embargo to files that have already been released as part of a previously published dataset version. -The API call requires a Json body that includes the embargo's end date (dateAvailable), a short reason (optional), and a list of the fileIds that the embargo should be set on. The dateAvailable must be after the current date and the duration (dateAvailable - today's date) must be less than the value specified by the :ref:`:MaxEmbargoDurationInMonths` setting. All files listed must be in the specified dataset. For example: +The API call requires a Json body that includes the embargo's end date (dateAvailable), a short reason (must not consiste of whitespace only, optional unless Dataverse is configured to make it required), and a list of the fileIds that the embargo should be set on. The dateAvailable must be after the current date and the duration (dateAvailable - today's date) must be less than the value specified by the :ref:`:MaxEmbargoDurationInMonths` setting. All files listed must be in the specified dataset. For example: .. code-block:: bash diff --git a/doc/sphinx-guides/source/installation/config.rst b/doc/sphinx-guides/source/installation/config.rst index 10314aff195..6eb17ccb2ff 100644 --- a/doc/sphinx-guides/source/installation/config.rst +++ b/doc/sphinx-guides/source/installation/config.rst @@ -3938,6 +3938,9 @@ please find all known feature flags below. Any of these flags can be activated u * - only-update-datacite-when-needed - Only contact DataCite to update a DOI after checking to see if DataCite has outdated information (for efficiency, lighter load on DataCite, especially when using file DOIs). - ``Off`` + * - require-embargo-reason + - Make it required to supply a non-blank reason when creating a file embargo. + - ``Off`` **Note:** Feature flags can be set via any `supported MicroProfile Config API source`_, e.g. the environment variable ``DATAVERSE_FEATURE_XXX`` (e.g. ``DATAVERSE_FEATURE_API_SESSION_AUTH=1``). These environment variables can be set in your shell before starting Payara. If you are using :doc:`Docker for development `, you can set them in the `docker compose `_ file. diff --git a/doc/sphinx-guides/source/user/dataset-management.rst b/doc/sphinx-guides/source/user/dataset-management.rst index 22e72a6a210..2529ae22d0e 100755 --- a/doc/sphinx-guides/source/user/dataset-management.rst +++ b/doc/sphinx-guides/source/user/dataset-management.rst @@ -750,7 +750,7 @@ Note that only one Preview URL (normal or with anonymized access) can be configu Embargoes ========= -A Dataverse instance may be configured to support file-level embargoes. Embargoes make file content inaccessible after a dataset version is published - until the embargo end date. +A Dataverse instance may be configured to support file-level embargoes. Embargoes make file content inaccessible after a dataset version is published - until the embargo end date. A reason for the embargo may be supplied when creating the embargo. A reason may be required in some Dataverse instances. This means that file previews and the ability to download files will be blocked. The effect is similar to when a file is restricted except that the embargo will end at the specified date without further action and during the embargo, requests for file access cannot be made. Embargoes of files in a version 1.0 dataset may also affect the date shown in the dataset and file citations. The recommended practice is for the citation to reflect the date on which all embargoes on files in version 1.0 end. (Since Dataverse creates one persistent identifier per dataset and doesn't create new ones for each version, the publication of later versions, with or without embargoed files, does not affect the citation date.) From a275b4b8125561d3efaffcbe47569a77206dc574 Mon Sep 17 00:00:00 2001 From: qqmyers Date: Sun, 4 Jan 2026 12:39:53 -0500 Subject: [PATCH 06/12] fix test --- src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 d4531ec21cf..dc4f47a62ae 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java @@ -5042,7 +5042,7 @@ public void testCitationDate() throws IOException { .body("resource.dates.date[0]", CoreMatchers.equalTo("1999-12-31")) .body("resource.dates.date[1].@dateType", CoreMatchers.equalTo("Updated")) .body("resource.dates.date[1]", CoreMatchers.equalTo(today)) - .body("resource.publicationYear", CoreMatchers.equalTo("2025")); + .body("resource.publicationYear", CoreMatchers.equalTo(currentYear)); Response exportDatasetOaiDc = UtilIT.exportDataset(datasetPid, "oai_dc", apiToken, true); exportDatasetOaiDc.prettyPrint(); From ac6f735b1bdad016deb92906f7a47d2eb788a67b Mon Sep 17 00:00:00 2001 From: qqmyers Date: Mon, 5 Jan 2026 13:14:16 -0500 Subject: [PATCH 07/12] make content-type explicit - catch date parsing error --- doc/sphinx-guides/source/api/native-api.rst | 4 ++-- .../java/edu/harvard/iq/dataverse/api/Datasets.java | 11 ++++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index c21f11ba640..ac2dfe4aa50 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -3607,14 +3607,14 @@ Set an Embargo on Files in a Dataset ``/api/datasets/$dataset-id/files/actions/:set-embargo`` can be used to set an embargo on one or more files in a dataset. Embargoes can be set on files that are only in a draft dataset version (and are not in any previously published version) by anyone who can edit the dataset. The same API call can be used by a superuser to add an embargo to files that have already been released as part of a previously published dataset version. -The API call requires a Json body that includes the embargo's end date (dateAvailable), a short reason (must not consiste of whitespace only, optional unless Dataverse is configured to make it required), and a list of the fileIds that the embargo should be set on. The dateAvailable must be after the current date and the duration (dateAvailable - today's date) must be less than the value specified by the :ref:`:MaxEmbargoDurationInMonths` setting. All files listed must be in the specified dataset. For example: +The API call requires a Json body that includes the embargo's end date (dateAvailable - YYYY-MM-DD format), a short reason (must not consist of whitespace only, optional unless Dataverse is configured to make it required), and a list of the fileIds that the embargo should be set on. The dateAvailable must be after the current date and the duration (dateAvailable - today's date) must be less than the value specified by the :ref:`:MaxEmbargoDurationInMonths` setting. All files listed must be in the specified dataset. For example: .. 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/7U7YBV - export JSON='{"dateAvailable":"2021-10-20", "reason":"Standard project embargo", "fileIds":[300,301,302]}' + export JSON='{"dateAvailable":"2021-01-20", "reason":"Standard project embargo", "fileIds":[300,301,302]}' curl -H "X-Dataverse-key: $API_TOKEN" -H "Content-Type:application/json" "$SERVER_URL/api/datasets/:persistentId/files/actions/:set-embargo?persistentId=$PERSISTENT_IDENTIFIER" -d "$JSON" 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 7c4d4b0d4cb..b3f84bacbf4 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -1438,6 +1438,8 @@ public Response moveDataset(@Context ContainerRequestContext crc, @PathParam("id @POST @AuthRequired @Path("{id}/files/actions/:set-embargo") + @Consumes(MediaType.APPLICATION_JSON) + @Produces(MediaType.APPLICATION_JSON) public Response createFileEmbargo(@Context ContainerRequestContext crc, @PathParam("id") String id, String jsonBody){ // user is authenticated @@ -1496,7 +1498,12 @@ public Response createFileEmbargo(@Context ContainerRequestContext crc, @PathPar LocalDate currentDateTime = LocalDate.now(); - LocalDate dateAvailable = LocalDate.parse(json.getString("dateAvailable")); + LocalDate dateAvailable = null; + try { + dateAvailable = LocalDate.parse(json.getString("dateAvailable")); + } catch (DateTimeParseException e) { + return error(Status.BAD_REQUEST, "Unable to parse dateAvailable"); + } // check :MaxEmbargoDurationInMonths if -1 LocalDate maxEmbargoDateTime = maxEmbargoDurationInMonths != -1 ? LocalDate.now().plusMonths(maxEmbargoDurationInMonths) : null; @@ -1602,6 +1609,8 @@ public Response createFileEmbargo(@Context ContainerRequestContext crc, @PathPar @POST @AuthRequired @Path("{id}/files/actions/:unset-embargo") + @Consumes(MediaType.APPLICATION_JSON) + @Produces(MediaType.APPLICATION_JSON) public Response removeFileEmbargo(@Context ContainerRequestContext crc, @PathParam("id") String id, String jsonBody){ // user is authenticated From be6d1af1d15d92344f99e354949fef7cdee3a70b Mon Sep 17 00:00:00 2001 From: qqmyers Date: Mon, 5 Jan 2026 13:39:47 -0500 Subject: [PATCH 08/12] comment re parse error handling --- src/main/java/edu/harvard/iq/dataverse/api/Datasets.java | 1 + 1 file changed, 1 insertion(+) 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 b3f84bacbf4..6e1e5763ad4 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -1492,6 +1492,7 @@ public Response createFileEmbargo(@Context ContainerRequestContext crc, @PathPar return error(Status.BAD_REQUEST, "No Embargoes allowed"); } + //Any parsing error should be handled via the JsonExceptionsHandler JsonObject json = JsonUtil.getJsonObject(jsonBody); Embargo embargo = new Embargo(); From 7aee23790ce15be86e6628e1a3abdfe6ae747c58 Mon Sep 17 00:00:00 2001 From: qqmyers Date: Mon, 5 Jan 2026 13:40:05 -0500 Subject: [PATCH 09/12] refactor validation --- .../edu/harvard/iq/dataverse/DatasetPage.java | 32 ++----------- .../edu/harvard/iq/dataverse/FilePage.java | 32 ++----------- .../harvard/iq/dataverse/util/FileUtil.java | 46 +++++++++++++++++++ .../webapp/file-edit-popup-fragment.xhtml | 2 +- 4 files changed, 53 insertions(+), 59 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java index ebbcd412e24..8057166c3d8 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java @@ -6873,33 +6873,7 @@ public void setRequestedCSL(String requestedCSL) { this.requestedCSL = requestedCSL; } - public void validateEmbargoReasonNotBlank(FacesContext context, UIComponent component, Object value) { - - // Skip validation if removing embargo - if (removeEmbargo) { - return; - } - - // Get the source of the current request - String source = context.getExternalContext().getRequestParameterMap() - .get("jakarta.faces.source"); - - // Only validate if the save button triggered this - if (source == null || !source.contains("fileEmbargoPopupSaveButton")) { - return; - } - - if (value == null && FeatureFlags.REQUIRE_EMBARGO_REASON.enabled()) { - throw new ValidatorException( - new FacesMessage(FacesMessage.SEVERITY_ERROR, - BundleUtil.getStringFromBundle("embargo.reason.required"), null) - ); - } - if (value != null && value.toString().trim().isEmpty()) { - throw new ValidatorException( - new FacesMessage(FacesMessage.SEVERITY_ERROR, - BundleUtil.getStringFromBundle("embargo.reason.blank"), null) - ); - } + public void validateEmbargoReason(FacesContext context, UIComponent component, Object value) { + FileUtil.validateEmbargoReason(context, component, value, removeEmbargo); } -} +} \ No newline at end of file diff --git a/src/main/java/edu/harvard/iq/dataverse/FilePage.java b/src/main/java/edu/harvard/iq/dataverse/FilePage.java index 4a2b8fa7b00..b08598b2fb8 100644 --- a/src/main/java/edu/harvard/iq/dataverse/FilePage.java +++ b/src/main/java/edu/harvard/iq/dataverse/FilePage.java @@ -1492,34 +1492,8 @@ public String editFileMetadata(){ return ""; } - public void validateEmbargoReasonNotBlank(FacesContext context, UIComponent component, Object value) { - - // Skip validation if removing embargo - if (removeEmbargo) { - return; - } - - // Get the source of the current request - String source = context.getExternalContext().getRequestParameterMap() - .get("jakarta.faces.source"); - - // Only validate if the save button triggered this - if (source == null || !source.contains("fileEmbargoPopupSaveButton")) { - return; - } - - if (value == null && FeatureFlags.REQUIRE_EMBARGO_REASON.enabled()) { - throw new ValidatorException( - new FacesMessage(FacesMessage.SEVERITY_ERROR, - BundleUtil.getStringFromBundle("embargo.reason.required"), null) - ); - } - if (value != null && value.toString().trim().isEmpty()) { - throw new ValidatorException( - new FacesMessage(FacesMessage.SEVERITY_ERROR, - BundleUtil.getStringFromBundle("embargo.reason.blank"), null) - ); - } + public void validateEmbargoReason(FacesContext context, UIComponent component, Object value) { + FileUtil.validateEmbargoReason(context, component, value, removeEmbargo); } - + } diff --git a/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java b/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java index 7502658444a..a7ec3f4b3ce 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java @@ -38,6 +38,7 @@ import edu.harvard.iq.dataverse.ingest.IngestableDataChecker; import edu.harvard.iq.dataverse.license.License; import edu.harvard.iq.dataverse.settings.ConfigCheckService; +import edu.harvard.iq.dataverse.settings.FeatureFlags; import edu.harvard.iq.dataverse.settings.JvmSettings; import edu.harvard.iq.dataverse.util.file.BagItFileHandler; import edu.harvard.iq.dataverse.util.file.CreateDataFileResult; @@ -89,6 +90,10 @@ import jakarta.activation.MimetypesFileTypeMap; import jakarta.ejb.EJBException; import jakarta.enterprise.inject.spi.CDI; +import jakarta.faces.application.FacesMessage; +import jakarta.faces.component.UIComponent; +import jakarta.faces.context.FacesContext; +import jakarta.faces.validator.ValidatorException; import jakarta.json.JsonArray; import jakarta.json.JsonObject; @@ -1836,6 +1841,47 @@ public static boolean isActivelyEmbargoed(List fmdList) { } return false; } + + /** + * Validates that an embargo reason is not blank, and exists when required. + * This method is designed to be called from JSF validator methods. + * + * @param context The FacesContext + * @param component The UIComponent being validated + * @param value The value to validate (embargo reason) + * @param removeEmbargo Whether the embargo is being removed (skips validation if true) + * @param saveButtonId The ID pattern of the save button that should trigger validation + * @throws ValidatorException if validation fails + */ + public static void validateEmbargoReason(FacesContext context, UIComponent component, Object value, + boolean removeEmbargo) { + // Skip validation if removing embargo + if (removeEmbargo) { + return; + } + + // Get the source of the current request + String source = context.getExternalContext().getRequestParameterMap() + .get("jakarta.faces.source"); + + // Only validate if the save button triggered this + if (source == null || !source.contains("fileEmbargoPopupSaveButton")) { + return; + } + + if (value == null && FeatureFlags.REQUIRE_EMBARGO_REASON.enabled()) { + throw new ValidatorException( + new FacesMessage(FacesMessage.SEVERITY_ERROR, + BundleUtil.getStringFromBundle("embargo.reason.required"), null) + ); + } + if (value != null && value.toString().trim().isEmpty()) { + throw new ValidatorException( + new FacesMessage(FacesMessage.SEVERITY_ERROR, + BundleUtil.getStringFromBundle("embargo.reason.blank"), null) + ); + } + } public static boolean isRetentionExpired(DataFile df) { Retention e = df.getRetention(); diff --git a/src/main/webapp/file-edit-popup-fragment.xhtml b/src/main/webapp/file-edit-popup-fragment.xhtml index e717efc4a60..a3b9db00554 100644 --- a/src/main/webapp/file-edit-popup-fragment.xhtml +++ b/src/main/webapp/file-edit-popup-fragment.xhtml @@ -144,7 +144,7 @@
From e2e84830eda17ab8056e82ddf15e6ac25c6a427f Mon Sep 17 00:00:00 2001 From: qqmyers Date: Mon, 5 Jan 2026 14:27:36 -0500 Subject: [PATCH 10/12] add validation tests --- .../FileUtilValidateEmbargoReasonTest.java | 217 ++++++++++++++++++ 1 file changed, 217 insertions(+) create mode 100644 src/test/java/edu/harvard/iq/dataverse/util/FileUtilValidateEmbargoReasonTest.java diff --git a/src/test/java/edu/harvard/iq/dataverse/util/FileUtilValidateEmbargoReasonTest.java b/src/test/java/edu/harvard/iq/dataverse/util/FileUtilValidateEmbargoReasonTest.java new file mode 100644 index 00000000000..f472350704e --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/util/FileUtilValidateEmbargoReasonTest.java @@ -0,0 +1,217 @@ +package edu.harvard.iq.dataverse.util; + +import edu.harvard.iq.dataverse.settings.JvmSettings; +import edu.harvard.iq.dataverse.util.testing.JvmSetting; +import edu.harvard.iq.dataverse.util.testing.LocalJvmSettings; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import jakarta.faces.application.FacesMessage; +import jakarta.faces.component.UIComponent; +import jakarta.faces.context.ExternalContext; +import jakarta.faces.context.FacesContext; +import jakarta.faces.validator.ValidatorException; + +import java.util.HashMap; +import java.util.Map; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +@LocalJvmSettings +public class FileUtilValidateEmbargoReasonTest { + + @Mock + private FacesContext facesContext; + + @Mock + private UIComponent component; + + @Mock + private ExternalContext externalContext; + + private Map requestParameterMap; + private AutoCloseable mocks; + + @BeforeEach + public void setUp() { + mocks = MockitoAnnotations.openMocks(this); + requestParameterMap = new HashMap<>(); + + when(facesContext.getExternalContext()).thenReturn(externalContext); + when(externalContext.getRequestParameterMap()).thenReturn(requestParameterMap); + } + + @AfterEach + public void tearDown() throws Exception { + if (mocks != null) { + mocks.close(); + } + } + + @Test + public void validateEmbargoReason_shouldSkipValidation_whenRemovingEmbargo() { + // Arrange + boolean removeEmbargo = true; + requestParameterMap.put("jakarta.faces.source", "fileEmbargoPopupSaveButton"); + + // Act & Assert - should not throw exception + assertDoesNotThrow(() -> + FileUtil.validateEmbargoReason(facesContext, component, null, removeEmbargo) + ); + } + + @Test + public void validateEmbargoReason_shouldSkipValidation_whenSourceIsNull() { + // Arrange + boolean removeEmbargo = false; + requestParameterMap.put("jakarta.faces.source", null); + + // Act & Assert - should not throw exception + assertDoesNotThrow(() -> + FileUtil.validateEmbargoReason(facesContext, component, null, removeEmbargo) + ); + } + + @Test + public void validateEmbargoReason_shouldSkipValidation_whenSourceIsNotSaveButton() { + // Arrange + boolean removeEmbargo = false; + requestParameterMap.put("jakarta.faces.source", "someOtherButton"); + + // Act & Assert - should not throw exception + assertDoesNotThrow(() -> + FileUtil.validateEmbargoReason(facesContext, component, null, removeEmbargo) + ); + } + + @Test + @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "true", varArgs = "require-embargo-reason") + public void validateEmbargoReason_shouldThrowException_whenReasonIsNullAndFlagEnabled() { + // Arrange + boolean removeEmbargo = false; + requestParameterMap.put("jakarta.faces.source", "fileEmbargoPopupSaveButton"); + + // Act & Assert + ValidatorException exception = assertThrows(ValidatorException.class, () -> + FileUtil.validateEmbargoReason(facesContext, component, null, removeEmbargo) + ); + + assertEquals(FacesMessage.SEVERITY_ERROR, exception.getFacesMessage().getSeverity()); + assertEquals(BundleUtil.getStringFromBundle("embargo.reason.required"), + exception.getFacesMessage().getSummary()); + } + + @Test + @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "false", varArgs = "require-embargo-reason") + public void validateEmbargoReason_shouldNotThrowException_whenReasonIsNullAndFlagDisabled() { + // Arrange + boolean removeEmbargo = false; + requestParameterMap.put("jakarta.faces.source", "fileEmbargoPopupSaveButton"); + + // Act & Assert - should not throw exception + assertDoesNotThrow(() -> + FileUtil.validateEmbargoReason(facesContext, component, null, removeEmbargo) + ); + } + + @ParameterizedTest + @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "false", varArgs = "require-embargo-reason") + @ValueSource(strings = {"", " ", "\t", "\n", " \t\n "}) + public void validateEmbargoReason_shouldThrowException_whenReasonIsBlank(String blankReason) { + // Arrange + boolean removeEmbargo = false; + requestParameterMap.put("jakarta.faces.source", "fileEmbargoPopupSaveButton"); + + // Act & Assert + ValidatorException exception = assertThrows(ValidatorException.class, () -> + FileUtil.validateEmbargoReason(facesContext, component, blankReason, removeEmbargo) + ); + + assertEquals(FacesMessage.SEVERITY_ERROR, exception.getFacesMessage().getSeverity()); + assertEquals(BundleUtil.getStringFromBundle("embargo.reason.blank"), + exception.getFacesMessage().getSummary()); + } + + @Test + @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "true", varArgs = "require-embargo-reason") + public void validateEmbargoReason_shouldHandleComplexScenario_flagEnabledBlankReasonSaveButton() { + // Arrange + boolean removeEmbargo = false; + requestParameterMap.put("jakarta.faces.source", "fileEmbargoPopupSaveButton"); + + // Act & Assert - blank reason should still fail even when flag is disabled + ValidatorException exception = assertThrows(ValidatorException.class, () -> + FileUtil.validateEmbargoReason(facesContext, component, " ", removeEmbargo) + ); + + assertEquals(BundleUtil.getStringFromBundle("embargo.reason.blank"), + exception.getFacesMessage().getSummary()); + } + + @ParameterizedTest + @ValueSource(strings = { + "Valid embargo reason", + " Valid reason with spaces " + }) + public void validateEmbargoReason_shouldNotThrowException_whenReasonIsValid(String validReason) { + // Arrange + boolean removeEmbargo = false; + requestParameterMap.put("jakarta.faces.source", "fileEmbargoPopupSaveButton"); + + // Act & Assert - should not throw exception + assertDoesNotThrow(() -> + FileUtil.validateEmbargoReason(facesContext, component, validReason, removeEmbargo) + ); + } + + @ParameterizedTest + @MethodSource("provideSaveButtonVariations") + public void validateEmbargoReason_shouldValidate_whenSaveButtonTriggersRequest(String buttonId) { + // Arrange + boolean removeEmbargo = false; + requestParameterMap.put("jakarta.faces.source", buttonId); + + // Act & Assert + ValidatorException exception = assertThrows(ValidatorException.class, () -> + FileUtil.validateEmbargoReason(facesContext, component, "", removeEmbargo) + ); + + assertEquals(FacesMessage.SEVERITY_ERROR, exception.getFacesMessage().getSeverity()); + } + + static Stream provideSaveButtonVariations() { + return Stream.of( + Arguments.of("fileEmbargoPopupSaveButton"), + // button in any context + Arguments.of("form:fileEmbargoPopupSaveButton"), + // or any suffix + Arguments.of("fileEmbargoPopupSaveButton:anything") + ); + } + + @Test + @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "true", varArgs = "require-embargo-reason") + public void validateEmbargoReason_shouldHandleComplexScenario_flagEnabledNullReasonSaveButton() { + // Arrange + boolean removeEmbargo = false; + requestParameterMap.put("jakarta.faces.source", "form:fileEmbargoPopupSaveButton"); + + // Act & Assert + ValidatorException exception = assertThrows(ValidatorException.class, () -> + FileUtil.validateEmbargoReason(facesContext, component, null, removeEmbargo) + ); + + assertEquals(BundleUtil.getStringFromBundle("embargo.reason.required"), + exception.getFacesMessage().getSummary()); + } + +} \ No newline at end of file From 676d790be468f521de13a7a10ddcf919413f7361 Mon Sep 17 00:00:00 2001 From: qqmyers Date: Mon, 5 Jan 2026 16:21:05 -0500 Subject: [PATCH 11/12] fix blank test, update deprecated settings call --- src/main/java/edu/harvard/iq/dataverse/api/Datasets.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 6e1e5763ad4..dafa51fea4c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -1482,7 +1482,7 @@ public Response createFileEmbargo(@Context ContainerRequestContext crc, @PathPar // check if embargoes are allowed(:MaxEmbargoDurationInMonths), gets the :MaxEmbargoDurationInMonths setting variable, if 0 or not set(null) return 400 long maxEmbargoDurationInMonths = 0; try { - maxEmbargoDurationInMonths = Long.parseLong(settingsService.get(SettingsServiceBean.Key.MaxEmbargoDurationInMonths.toString())); + maxEmbargoDurationInMonths = Long.parseLong(settingsService.getValueForKey(SettingsServiceBean.Key.MaxEmbargoDurationInMonths)); } catch (NumberFormatException nfe){ if (nfe.getMessage().contains("null")) { return error(Status.BAD_REQUEST, "No Embargoes allowed"); @@ -1527,7 +1527,7 @@ public Response createFileEmbargo(@Context ContainerRequestContext crc, @PathPar } if(reason == null && FeatureFlags.REQUIRE_EMBARGO_REASON.enabled()) { return error(Status.BAD_REQUEST, "Reason is required for embargoes"); - } else if(reason.isBlank()) { + } else if(reason != null && reason.isBlank()) { return error(Status.BAD_REQUEST, "Reason cannot be blank (whitespace only)"); } embargo.setReason(reason); From a6f42d1e30fd54d8a1e0493a5382a521ad8696ae Mon Sep 17 00:00:00 2001 From: qqmyers Date: Mon, 5 Jan 2026 16:24:23 -0500 Subject: [PATCH 12/12] api tests --- .../dataverse/api/DatasetsEmbargoAPITest.java | 246 ++++++++++++++++++ 1 file changed, 246 insertions(+) create mode 100644 src/test/java/edu/harvard/iq/dataverse/api/DatasetsEmbargoAPITest.java diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsEmbargoAPITest.java b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsEmbargoAPITest.java new file mode 100644 index 00000000000..64318e2e223 --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsEmbargoAPITest.java @@ -0,0 +1,246 @@ + +package edu.harvard.iq.dataverse.api; + +import edu.harvard.iq.dataverse.DataFile; +import edu.harvard.iq.dataverse.DataFileServiceBean; +import edu.harvard.iq.dataverse.DatasetServiceBean; +import edu.harvard.iq.dataverse.DatasetVersion; +import edu.harvard.iq.dataverse.Embargo; +import edu.harvard.iq.dataverse.EmbargoServiceBean; +import edu.harvard.iq.dataverse.PermissionServiceBean; +import edu.harvard.iq.dataverse.PermissionServiceBean.StaticPermissionQuery; +import edu.harvard.iq.dataverse.TermsOfUseAndAccess; +import edu.harvard.iq.dataverse.authorization.Permission; +import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; +import edu.harvard.iq.dataverse.engine.command.exception.CommandException; +import edu.harvard.iq.dataverse.settings.JvmSettings; +import edu.harvard.iq.dataverse.settings.SettingsServiceBean; +import edu.harvard.iq.dataverse.util.testing.JvmSetting; +import edu.harvard.iq.dataverse.util.testing.LocalJvmSettings; +import jakarta.json.Json; +import jakarta.json.JsonObject; +import jakarta.json.JsonObjectBuilder; +import jakarta.ws.rs.container.ContainerRequestContext; +import jakarta.ws.rs.core.Response; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import java.time.LocalDate; +import java.util.List; + +import static jakarta.ws.rs.core.Response.Status.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.when; + +@LocalJvmSettings +public class DatasetsEmbargoAPITest { + + + @Mock + private DataFileServiceBean fileService; + + @Mock + private DatasetServiceBean datasetService; + + @Mock + private EmbargoServiceBean embargoService; + + @Mock + private PermissionServiceBean permissionService; + + @Mock + private SettingsServiceBean settingsService; + + @Mock + private ContainerRequestContext crc; + + @Mock + private edu.harvard.iq.dataverse.Dataset dataset; + + @Mock + private edu.harvard.iq.dataverse.DataFile file; + + @Mock + private DatasetVersion datasetVersion; + + @Mock + private TermsOfUseAndAccess termsOfUseAndAccess; + + @Mock + private StaticPermissionQuery permissionQuery; + + @InjectMocks + private Datasets datasetsApi; + + private AuthenticatedUser testUser; + + @BeforeEach + public void setUp() { + MockitoAnnotations.openMocks(this); + + testUser = new AuthenticatedUser(); + testUser.setId(1L); + testUser.setSuperuser(false); + + // Mock the authentication + when(crc.getProperty(ApiConstants.CONTAINER_REQUEST_CONTEXT_USER)) + .thenReturn(testUser); + + // Mock dataset lookup + when(datasetService.find(1L)).thenReturn(dataset); + + // Mock dataset version chain + when(dataset.getLatestVersion()).thenReturn(datasetVersion); + when(dataset.getFiles()).thenReturn(List.of(file)); + when(datasetVersion.getTermsOfUseAndAccess()).thenReturn(termsOfUseAndAccess); + when(datasetVersion.getVersionState()).thenReturn(DatasetVersion.VersionState.DRAFT); + when(termsOfUseAndAccess.getDatasetVersion()).thenReturn(datasetVersion); + + // Mock file lookup + when(fileService.find(2L)).thenReturn(file); + when(fileService.save(any(DataFile.class))).thenAnswer(invocation -> invocation.getArgument(0)); + + + + // Mock permission check + when(permissionService.userOn(eq(testUser), eq(dataset))) + .thenReturn(permissionQuery); + when(permissionQuery.has(Permission.EditDataset)).thenReturn(true); + + // Mock setting + when(settingsService.getValueForKey(SettingsServiceBean.Key.MaxEmbargoDurationInMonths)).thenReturn("12"); + + // Mock embargoService + when(embargoService.merge(any(Embargo.class))).thenAnswer(invocation -> invocation.getArgument(0)); + when(embargoService.save(any(Embargo.class), any(String.class))).thenReturn(1L); + + } + + @Test + @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "true", varArgs = "require-embargo-reason") + public void testCreateFileEmbargo_withReasonRequired_shouldRejectNullReason() { + // Arrange + LocalDate futureDate = LocalDate.now().plusMonths(6); + JsonObjectBuilder embargoJson = Json.createObjectBuilder() + .add("dateAvailable", futureDate.toString()) + .add("fileIds", Json.createArrayBuilder().add(1L)); + + // Act + Response response = datasetsApi.createFileEmbargo(crc, "1", embargoJson.build().toString()); + + // Assert + assertEquals(BAD_REQUEST.getStatusCode(), response.getStatus()); + if (response.hasEntity()) { + Object entity = response.getEntity(); + if (entity instanceof JsonObject) { + JsonObject jsonResponse = (JsonObject) entity; + String message = jsonResponse.getString("message", ""); + assertTrue(message.contains("Reason is required") || message.contains("reason"), + "Expected error message about required reason, got: " + message); + } + } + } + + @Test + @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "true", varArgs = "require-embargo-reason") + public void testCreateFileEmbargo_withReasonRequired_shouldAcceptValidReason() throws CommandException { + // Arrange + LocalDate futureDate = LocalDate.now().plusMonths(6); + JsonObjectBuilder embargoJson = Json.createObjectBuilder() + .add("dateAvailable", futureDate.toString()) + .add("reason", "Valid embargo reason for testing") + .add("fileIds", Json.createArrayBuilder().add(2L)); + + // Act + Response response = datasetsApi.createFileEmbargo(crc, "1", embargoJson.build().toString()); + + // Assert + assertTrue(response.getStatus() == OK.getStatusCode()); + } + + @Test + @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "false", varArgs = "require-embargo-reason") + public void testCreateFileEmbargo_withReasonNotRequired_shouldAcceptNullReason() throws CommandException { + // Arrange + LocalDate futureDate = LocalDate.now().plusMonths(6); + JsonObjectBuilder embargoJson = Json.createObjectBuilder() + .add("dateAvailable", futureDate.toString()) + .add("fileIds", Json.createArrayBuilder().add(2L)); + + // Act + Response response = datasetsApi.createFileEmbargo(crc, "1", embargoJson.build().toString()); + + // Assert + // Should not get BAD_REQUEST for missing reason when flag is disabled + if (response.getStatus() == BAD_REQUEST.getStatusCode() && response.hasEntity()) { + Object entity = response.getEntity(); + if (entity instanceof JsonObject) { + JsonObject jsonResponse = (JsonObject) entity; + String message = jsonResponse.getString("message", ""); + assertTrue(!message.contains("Reason is required"), + "Should not require reason when flag is disabled, got: " + message); + } + } + } + + @ParameterizedTest + @ValueSource(strings = {"", " ", "\t", "\n", " \t\n "}) + @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "false", varArgs = "require-embargo-reason") + public void testCreateFileEmbargo_shouldRejectBlankReason_regardlessOfFlag(String blankReason) { + // Arrange + LocalDate futureDate = LocalDate.now().plusMonths(6); + JsonObjectBuilder embargoJson = Json.createObjectBuilder() + .add("dateAvailable", futureDate.toString()) + .add("reason", blankReason) + .add("fileIds", Json.createArrayBuilder().add(2L)); + + // Act + Response response = datasetsApi.createFileEmbargo(crc, "1", embargoJson.build().toString()); + + // Assert + assertEquals(BAD_REQUEST.getStatusCode(), response.getStatus()); + if (response.hasEntity()) { + Object entity = response.getEntity(); + if (entity instanceof JsonObject) { + JsonObject jsonResponse = (JsonObject) entity; + String message = jsonResponse.getString("message", ""); + assertTrue(message.contains("blank") || message.contains("empty"), + "Expected error message about blank reason, got: " + message); + } + } + } + + @Test + @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "true", varArgs = "require-embargo-reason") + public void testCreateFileEmbargo_withReasonRequired_shouldRejectEmptyString() { + // Arrange + LocalDate futureDate = LocalDate.now().plusMonths(6); + JsonObjectBuilder embargoJson = Json.createObjectBuilder() + .add("dateAvailable", futureDate.toString()) + .add("reason", "") + .add("fileIds", Json.createArrayBuilder().add(2L)); + + // Act + Response response = datasetsApi.createFileEmbargo(crc, "1", embargoJson.build().toString()); + + // Assert + assertEquals(BAD_REQUEST.getStatusCode(), response.getStatus()); + if (response.hasEntity()) { + Object entity = response.getEntity(); + if (entity instanceof JsonObject) { + JsonObject jsonResponse = (JsonObject) entity; + String message = jsonResponse.getString("message", ""); + assertTrue(message.contains("blank") || message.contains("empty") || message.contains("reason"), + "Expected error message about blank/empty reason, got: " + message); + } + } + } +} \ No newline at end of file