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 diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 84c1db6d338..11a49c93e47 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -3609,14 +3609,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 (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 - 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/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.) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java index 20617160a1c..8057166c3d8 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,7 @@ public void setRequestedCSL(String requestedCSL) { this.requestedCSL = requestedCSL; } -} + 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 dd1bd56d5bd..b08598b2fb8 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,8 @@ public String editFileMetadata(){ return ""; } + 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/api/Datasets.java b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java index c15efb4c651..ec1c7abd272 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -1445,6 +1445,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 @@ -1487,7 +1489,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"); @@ -1497,13 +1499,19 @@ 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(); 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; @@ -1520,8 +1528,16 @@ 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 == null && FeatureFlags.REQUIRE_EMBARGO_REASON.enabled()) { + return error(Status.BAD_REQUEST, "Reason is required for embargoes"); + } else if(reason != null && reason.isBlank()) { + return error(Status.BAD_REQUEST, "Reason cannot be blank (whitespace only)"); + } + embargo.setReason(reason); List datasetFiles = dataset.getFiles(); List filesToEmbargo = new LinkedList<>(); @@ -1601,6 +1617,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 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/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/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..a3b9db00554 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 @@ - 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 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