From a8f09e62667d5a6245cf35594b865db339df623e Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Thu, 31 Oct 2024 14:32:57 -0400 Subject: [PATCH 1/9] Published datasets should contain files --- ...published-datasets-should-contain-files.md | 9 +++ doc/sphinx-guides/source/api/native-api.rst | 1 + .../edu/harvard/iq/dataverse/Dataverse.java | 11 +++- .../harvard/iq/dataverse/api/Dataverses.java | 2 +- .../command/impl/PublishDatasetCommand.java | 20 +++++- .../impl/UpdateDataverseAttributeCommand.java | 33 ++++++---- .../iq/dataverse/util/json/JsonParser.java | 3 + .../iq/dataverse/util/json/JsonPrinter.java | 3 + src/main/java/propertyFiles/Bundle.properties | 1 + src/main/resources/db/migration/V6.5.0.1.sql | 2 + .../harvard/iq/dataverse/api/DatasetsIT.java | 66 +++++++++++++++++-- 11 files changed, 132 insertions(+), 19 deletions(-) create mode 100644 doc/release-notes/10981-published-datasets-should-contain-files.md create mode 100644 src/main/resources/db/migration/V6.5.0.1.sql diff --git a/doc/release-notes/10981-published-datasets-should-contain-files.md b/doc/release-notes/10981-published-datasets-should-contain-files.md new file mode 100644 index 00000000000..73c76744164 --- /dev/null +++ b/doc/release-notes/10981-published-datasets-should-contain-files.md @@ -0,0 +1,9 @@ +## Feature: Prevent publishing Datasets without files +A new attribute was added to Collections in order to control the publishing of Datasets without files. Once set, the publishing of a Dataset within a Collection or Collection's hierarchy, without files, will be blocked for all non Admin users. +In order to configure a Collection to block publishing an Admin must set the attribute "requireFilesToPublishDataset" to true. +Any Collection created under a Collection with this attribute will also be bound by this blocking. Setting this attribute on the Root Dataverse will essentially block the publishing of Datasets without files for the entire installation. +```shell +curl -X PUT -H "X-Dataverse-key:$API_TOKEN" "$SERVER_URL/api/dataverses/$ID/attribute/requireFilesToPublishDataset?value=true" +``` + +See also #10981. diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index f8b8620f121..8fab30a884b 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -1005,6 +1005,7 @@ The following attributes are supported: * ``description`` Description * ``affiliation`` Affiliation * ``filePIDsEnabled`` ("true" or "false") Restricted to use by superusers and only when the :ref:`:AllowEnablingFilePIDsPerCollection <:AllowEnablingFilePIDsPerCollection>` setting is true. Enables or disables registration of file-level PIDs in datasets within the collection (overriding the instance-wide setting). +* ``requireFilesToPublishDataset`` ("true" or "false") Dataset needs files in order to be published. Restricted to use by Administrators. If any TRUE found in the ownership tree publishing will be blocked. Publishing by an Administrator will not be blocked. .. _collection-storage-quotas: diff --git a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java index 86e2e0207c1..3bbf02fd611 100644 --- a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java +++ b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java @@ -602,7 +602,16 @@ public List getCitationDatasetFieldTypes() { public void setCitationDatasetFieldTypes(List citationDatasetFieldTypes) { this.citationDatasetFieldTypes = citationDatasetFieldTypes; } - + + @Column(nullable = true) + private Boolean requireFilesToPublishDataset; + public Boolean getRequireFilesToPublishDataset() { + return requireFilesToPublishDataset; + } + public void setRequireFilesToPublishDataset(boolean requireFilesToPublishDataset) { + this.requireFilesToPublishDataset = requireFilesToPublishDataset; + } + /** * @Note: this setting is Nullable, with {@code null} indicating that the * desired behavior is not explicitly configured for this specific collection. diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Dataverses.java b/src/main/java/edu/harvard/iq/dataverse/api/Dataverses.java index 2be6b1e51c2..f549003f70b 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Dataverses.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Dataverses.java @@ -631,7 +631,7 @@ public Response updateAttribute(@Context ContainerRequestContext crc, @PathParam } private Object formatAttributeValue(String attribute, String value) throws WrappedResponse { - if (attribute.equals("filePIDsEnabled")) { + if (List.of("filePIDsEnabled","requireFilesToPublishDataset").contains(attribute)) { return parseBooleanOrDie(value); } return value; diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java index 1ac41105237..dfe2bb44b20 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java @@ -2,6 +2,7 @@ import edu.harvard.iq.dataverse.Dataset; import edu.harvard.iq.dataverse.DatasetLock; +import edu.harvard.iq.dataverse.Dataverse; import edu.harvard.iq.dataverse.authorization.Permission; import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; import edu.harvard.iq.dataverse.engine.command.CommandContext; @@ -9,8 +10,11 @@ import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; import edu.harvard.iq.dataverse.engine.command.exception.CommandException; import edu.harvard.iq.dataverse.engine.command.exception.IllegalCommandException; +import edu.harvard.iq.dataverse.util.BundleUtil; import edu.harvard.iq.dataverse.workflow.Workflow; import edu.harvard.iq.dataverse.workflow.WorkflowContext.TriggerType; + +import java.util.List; import java.util.Optional; import java.util.logging.Logger; import static java.util.stream.Collectors.joining; @@ -218,9 +222,23 @@ private void verifyCommandArguments(CommandContext ctxt) throws IllegalCommandEx if (minorRelease && !getDataset().getLatestVersion().isMinorUpdate()) { throw new IllegalCommandException("Cannot release as minor version. Re-try as major release.", this); } + + if (getDataset().getFiles().isEmpty() && requiresFilesToPublishDataset()) { + throw new IllegalCommandException(BundleUtil.getStringFromBundle("dataset.mayNotPublish.FilesRequired"), this); + } } } - + private boolean requiresFilesToPublishDataset() { + if (!getUser().isSuperuser()) { + List owners = getDataset().getOwner().getOwners(); + for(Dataverse owner : owners) { + if (owner.getRequireFilesToPublishDataset() != null && owner.getRequireFilesToPublishDataset()) { + return true; + } + } + } + return false; + } @Override public boolean onSuccess(CommandContext ctxt, Object r) { diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/UpdateDataverseAttributeCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/UpdateDataverseAttributeCommand.java index 57ac20fcee6..ab12d8eea26 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/UpdateDataverseAttributeCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/UpdateDataverseAttributeCommand.java @@ -24,7 +24,7 @@ public class UpdateDataverseAttributeCommand extends AbstractCommand private static final String ATTRIBUTE_DESCRIPTION = "description"; private static final String ATTRIBUTE_AFFILIATION = "affiliation"; private static final String ATTRIBUTE_FILE_PIDS_ENABLED = "filePIDsEnabled"; - + private static final String ATTRIBUTE_REQUIRE_FILES_TO_PUBLISH_DATASET = "requireFilesToPublishDataset"; private final Dataverse dataverse; private final String attributeName; private final Object attributeValue; @@ -45,8 +45,9 @@ public Dataverse execute(CommandContext ctxt) throws CommandException { case ATTRIBUTE_AFFILIATION: setStringAttribute(attributeName, attributeValue); break; + case ATTRIBUTE_REQUIRE_FILES_TO_PUBLISH_DATASET: case ATTRIBUTE_FILE_PIDS_ENABLED: - setBooleanAttributeForFilePIDs(ctxt); + setBooleanAttribute(ctxt, true); break; default: throw new IllegalCommandException("'" + attributeName + "' is not a supported attribute", this); @@ -86,25 +87,33 @@ private void setStringAttribute(String attributeName, Object attributeValue) thr } /** - * Helper method to handle the "filePIDsEnabled" boolean attribute. + * Helper method to handle boolean attributes. * * @param ctxt The command context. + * @param adminOnly True if this attribute can only be modified by an Administrator * @throws PermissionException if the user doesn't have permission to modify this attribute. */ - private void setBooleanAttributeForFilePIDs(CommandContext ctxt) throws CommandException { - if (!getRequest().getUser().isSuperuser()) { + private void setBooleanAttribute(CommandContext ctxt, boolean adminOnly) throws CommandException { + if (adminOnly && !getRequest().getUser().isSuperuser()) { throw new PermissionException("You must be a superuser to change this setting", this, Collections.singleton(Permission.EditDataset), dataverse); } - if (!ctxt.settings().isTrueForKey(SettingsServiceBean.Key.AllowEnablingFilePIDsPerCollection, false)) { - throw new PermissionException("Changing File PID policy per collection is not enabled on this server", - this, Collections.singleton(Permission.EditDataset), dataverse); - } if (!(attributeValue instanceof Boolean)) { - throw new IllegalCommandException("'" + ATTRIBUTE_FILE_PIDS_ENABLED + "' requires a boolean value", this); + throw new IllegalCommandException("'" + attributeName + "' requires a boolean value", this); + } + switch (attributeName) { + case ATTRIBUTE_FILE_PIDS_ENABLED: + if (!ctxt.settings().isTrueForKey(SettingsServiceBean.Key.AllowEnablingFilePIDsPerCollection, false)) { + throw new PermissionException("Changing File PID policy per collection is not enabled on this server", + this, Collections.singleton(Permission.EditDataset), dataverse); + } + dataverse.setFilePIDsEnabled((Boolean) attributeValue); + case ATTRIBUTE_REQUIRE_FILES_TO_PUBLISH_DATASET: + dataverse.setRequireFilesToPublishDataset((Boolean) attributeValue); + break; + default: + throw new IllegalCommandException("Unsupported boolean attribute: " + attributeName, this); } - - dataverse.setFilePIDsEnabled((Boolean) attributeValue); } } diff --git a/src/main/java/edu/harvard/iq/dataverse/util/json/JsonParser.java b/src/main/java/edu/harvard/iq/dataverse/util/json/JsonParser.java index 2f01c9bc2f2..50caf2c6732 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/json/JsonParser.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/json/JsonParser.java @@ -160,6 +160,9 @@ public Dataverse parseDataverse(JsonObject jobj) throws JsonParseException { if (jobj.containsKey("filePIDsEnabled")) { dv.setFilePIDsEnabled(jobj.getBoolean("filePIDsEnabled")); } + if (jobj.containsKey("requireFilesToPublishDataset")) { + dv.setRequireFilesToPublishDataset(jobj.getBoolean("requireFilesToPublishDataset")); + } /* We decided that subject is not user set, but gotten from the subject of the dataverse's datasets - leavig this code in for now, in case we need to go back to it at some point 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 1bdee48b14d..8f5f97512aa 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 @@ -292,6 +292,9 @@ public static JsonObjectBuilder json(Dataverse dv, Boolean hideEmail, Boolean re if (dv.getFilePIDsEnabled() != null) { bld.add("filePIDsEnabled", dv.getFilePIDsEnabled()); } + if (dv.getRequireFilesToPublishDataset() != null) { + bld.add("requireFilesToPublishDataset", dv.getRequireFilesToPublishDataset()); + } bld.add("isReleased", dv.isReleased()); List inputLevels = dv.getDataverseFieldTypeInputLevels(); diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index 149e6a7e828..ebc09a1d731 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -1542,6 +1542,7 @@ dataset.mayNotPublish.administrator= This dataset cannot be published until {0} dataset.mayNotPublish.both= This dataset cannot be published until {0} is published. Would you like to publish both right now? dataset.mayNotPublish.twoGenerations= This dataset cannot be published until {0} and {1} are published. dataset.mayNotBePublished.both.button=Yes, Publish Both +dataset.mayNotPublish.FilesRequired=This dataset cannot be published without uploaded files. dataset.viewVersion.unpublished=View Unpublished Version dataset.viewVersion.published=View Published Version dataset.link.title=Link Dataset diff --git a/src/main/resources/db/migration/V6.5.0.1.sql b/src/main/resources/db/migration/V6.5.0.1.sql new file mode 100644 index 00000000000..661924b54af --- /dev/null +++ b/src/main/resources/db/migration/V6.5.0.1.sql @@ -0,0 +1,2 @@ +-- files are required to publish datasets +ALTER TABLE dataverse ADD COLUMN IF NOT EXISTS requirefilestopublishdataset bool; 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 93f1024ae7a..0f24e6b73a9 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java @@ -16,16 +16,14 @@ import edu.harvard.iq.dataverse.util.SystemConfig; import edu.harvard.iq.dataverse.util.json.JSONLDUtil; import edu.harvard.iq.dataverse.util.json.JsonUtil; +import edu.harvard.iq.dataverse.util.json.NullSafeJsonBuilder; import io.restassured.RestAssured; import io.restassured.http.ContentType; import io.restassured.parsing.Parser; import io.restassured.path.json.JsonPath; import io.restassured.path.xml.XmlPath; import io.restassured.response.Response; -import jakarta.json.Json; -import jakarta.json.JsonArray; -import jakarta.json.JsonObject; -import jakarta.json.JsonObjectBuilder; +import jakarta.json.*; import jakarta.ws.rs.core.Response.Status; import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.StringUtils; @@ -5168,4 +5166,64 @@ public void testGetCanDownloadAtLeastOneFile() { Response getUserPermissionsOnDatasetInvalidIdResponse = UtilIT.getCanDownloadAtLeastOneFile("testInvalidId", DS_VERSION_LATEST, secondUserApiToken); getUserPermissionsOnDatasetInvalidIdResponse.then().assertThat().statusCode(BAD_REQUEST.getStatusCode()); } + + @Test + public void testRequireFilesToPublishDatasets() { + // Create superuser and regular user + Response createUserResponse = UtilIT.createRandomUser(); + createUserResponse.then().assertThat().statusCode(OK.getStatusCode()); + String usernameAdmin = UtilIT.getUsernameFromResponse(createUserResponse); + String apiTokenAdmin = UtilIT.getApiTokenFromResponse(createUserResponse); + Response makeSuperUser = UtilIT.makeSuperUser(usernameAdmin); + assertEquals(200, makeSuperUser.getStatusCode()); + + createUserResponse = UtilIT.createRandomUser(); + createUserResponse.then().assertThat().statusCode(OK.getStatusCode()); + String apiToken = UtilIT.getApiTokenFromResponse(createUserResponse); + + // Create and publish a top level Dataverse (under root) with a requireFilesToPublishDataset set to true + Response createDataverseResponse = UtilIT.createRandomDataverse(apiToken); + String ownerAlias = UtilIT.getAliasFromResponse(createDataverseResponse); + // Only admin can set this attribute + Response setDataverseAttributeResponse = UtilIT.setCollectionAttribute(ownerAlias, "requireFilesToPublishDataset", "true", apiToken); + setDataverseAttributeResponse.prettyPrint(); + setDataverseAttributeResponse.then().assertThat().statusCode(UNAUTHORIZED.getStatusCode()); + setDataverseAttributeResponse = UtilIT.setCollectionAttribute(ownerAlias, "requireFilesToPublishDataset", "true", apiTokenAdmin); + setDataverseAttributeResponse.prettyPrint(); + setDataverseAttributeResponse.then().assertThat().statusCode(OK.getStatusCode()); + setDataverseAttributeResponse.then().assertThat().body("data.requireFilesToPublishDataset",equalTo(true)); + Response publishDataverseResponse = UtilIT.publishDataverseViaNativeApi(ownerAlias, apiTokenAdmin); + publishDataverseResponse.prettyPrint(); + publishDataverseResponse.then().assertThat().statusCode(OK.getStatusCode()); + + // Create and publish a new Dataverse under the above Dataverse with requireFilesToPublishDataset not set (default null) + String alias = "dv2-" + UtilIT.getRandomIdentifier(); + createDataverseResponse = UtilIT.createSubDataverse(alias, null, apiToken, ownerAlias); + createDataverseResponse.prettyPrint(); + createDataverseResponse.then().assertThat().statusCode(CREATED.getStatusCode()); + publishDataverseResponse = UtilIT.publishDataverseViaNativeApi(alias, apiToken); + publishDataverseResponse.then().assertThat().statusCode(OK.getStatusCode()); + + // Create a Dataset under the 2nd level Dataverse + Response createDatasetResponse = UtilIT.createRandomDatasetViaNativeApi(alias, apiToken); + createDatasetResponse.then().assertThat().statusCode(CREATED.getStatusCode()); + Integer id = UtilIT.getDatasetIdFromResponse(createDatasetResponse); + + // Try to publish with no files (minimum is 1 file from the top level Dataverse) + Response publishDatasetResponse = UtilIT.publishDatasetViaNativeApi(String.valueOf(id), "major", apiToken); + publishDatasetResponse.prettyPrint(); + publishDatasetResponse.then().assertThat().statusCode(FORBIDDEN.getStatusCode()); + publishDatasetResponse.then().assertThat().body("message", containsString( + BundleUtil.getStringFromBundle("dataset.mayNotPublish.FilesRequired") + )); + + // Upload 1 file and try to publish again + String pathToFile = "src/main/webapp/resources/images/dataverseproject.png"; + Response uploadResponse = UtilIT.uploadFileViaNative(String.valueOf(id), pathToFile, apiToken); + uploadResponse.then().assertThat().statusCode(OK.getStatusCode()); + + publishDatasetResponse = UtilIT.publishDatasetViaNativeApi(String.valueOf(id), "major", apiToken); + publishDatasetResponse.prettyPrint(); + publishDatasetResponse.then().assertThat().statusCode(OK.getStatusCode()); + } } From 1159134aa8b3cbb987a5e9f3d566de9914f17ff7 Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Thu, 31 Oct 2024 14:42:51 -0400 Subject: [PATCH 2/9] cosmetic fixes --- src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 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 0f24e6b73a9..3bc15bfc363 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java @@ -16,14 +16,16 @@ import edu.harvard.iq.dataverse.util.SystemConfig; import edu.harvard.iq.dataverse.util.json.JSONLDUtil; import edu.harvard.iq.dataverse.util.json.JsonUtil; -import edu.harvard.iq.dataverse.util.json.NullSafeJsonBuilder; import io.restassured.RestAssured; import io.restassured.http.ContentType; import io.restassured.parsing.Parser; import io.restassured.path.json.JsonPath; import io.restassured.path.xml.XmlPath; import io.restassured.response.Response; -import jakarta.json.*; +import jakarta.json.Json; +import jakarta.json.JsonArray; +import jakarta.json.JsonObject; +import jakarta.json.JsonObjectBuilder; import jakarta.ws.rs.core.Response.Status; import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.StringUtils; From c0c4c89aee8e9dd5acfb979dbc0c36f1803da57a Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Thu, 31 Oct 2024 15:56:01 -0400 Subject: [PATCH 3/9] change Admin to superuser in docs --- .../10981-published-datasets-should-contain-files.md | 4 ++-- doc/sphinx-guides/source/api/native-api.rst | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/release-notes/10981-published-datasets-should-contain-files.md b/doc/release-notes/10981-published-datasets-should-contain-files.md index 73c76744164..74c932f853a 100644 --- a/doc/release-notes/10981-published-datasets-should-contain-files.md +++ b/doc/release-notes/10981-published-datasets-should-contain-files.md @@ -1,6 +1,6 @@ ## Feature: Prevent publishing Datasets without files -A new attribute was added to Collections in order to control the publishing of Datasets without files. Once set, the publishing of a Dataset within a Collection or Collection's hierarchy, without files, will be blocked for all non Admin users. -In order to configure a Collection to block publishing an Admin must set the attribute "requireFilesToPublishDataset" to true. +A new attribute was added to Collections in order to control the publishing of Datasets without files. Once set, the publishing of a Dataset within a Collection or Collection's hierarchy, without files, will be blocked for all non superusers. +In order to configure a Collection to block publishing a superuser must set the attribute "requireFilesToPublishDataset" to true. Any Collection created under a Collection with this attribute will also be bound by this blocking. Setting this attribute on the Root Dataverse will essentially block the publishing of Datasets without files for the entire installation. ```shell curl -X PUT -H "X-Dataverse-key:$API_TOKEN" "$SERVER_URL/api/dataverses/$ID/attribute/requireFilesToPublishDataset?value=true" diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 8fab30a884b..d174d4a87cd 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -1005,7 +1005,7 @@ The following attributes are supported: * ``description`` Description * ``affiliation`` Affiliation * ``filePIDsEnabled`` ("true" or "false") Restricted to use by superusers and only when the :ref:`:AllowEnablingFilePIDsPerCollection <:AllowEnablingFilePIDsPerCollection>` setting is true. Enables or disables registration of file-level PIDs in datasets within the collection (overriding the instance-wide setting). -* ``requireFilesToPublishDataset`` ("true" or "false") Dataset needs files in order to be published. Restricted to use by Administrators. If any TRUE found in the ownership tree publishing will be blocked. Publishing by an Administrator will not be blocked. +* ``requireFilesToPublishDataset`` ("true" or "false") Dataset needs files in order to be published. Restricted to use by superusers. If any TRUE found in the ownership tree publishing will be blocked. Publishing by a superusers will not be blocked. .. _collection-storage-quotas: From 85af55f8f1a6dfe0165a28d425cdcc6f51f6ea2d Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Fri, 1 Nov 2024 10:45:06 -0400 Subject: [PATCH 4/9] change how hierarchy works --- .../10981-published-datasets-should-contain-files.md | 8 +++++--- doc/sphinx-guides/source/api/native-api.rst | 2 +- src/main/java/edu/harvard/iq/dataverse/Dataverse.java | 9 +++++++++ .../engine/command/impl/PublishDatasetCommand.java | 9 +++++---- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/doc/release-notes/10981-published-datasets-should-contain-files.md b/doc/release-notes/10981-published-datasets-should-contain-files.md index 74c932f853a..964e7ff1937 100644 --- a/doc/release-notes/10981-published-datasets-should-contain-files.md +++ b/doc/release-notes/10981-published-datasets-should-contain-files.md @@ -1,7 +1,9 @@ ## Feature: Prevent publishing Datasets without files -A new attribute was added to Collections in order to control the publishing of Datasets without files. Once set, the publishing of a Dataset within a Collection or Collection's hierarchy, without files, will be blocked for all non superusers. -In order to configure a Collection to block publishing a superuser must set the attribute "requireFilesToPublishDataset" to true. -Any Collection created under a Collection with this attribute will also be bound by this blocking. Setting this attribute on the Root Dataverse will essentially block the publishing of Datasets without files for the entire installation. +A new attribute was added to Collections in order to control the publishing of Datasets without files. +Once set to "True", the publishing of a Dataset within a Collection, without files, will be blocked for all non superusers. +In order to configure a Collection to block publishing a superuser must set the attribute "requireFilesToPublishDataset" to "True". +The collection's hierarchy will be checked if the collection's "requireFilesToPublishDataset" attribute is not set explicitly to "True" or "False". + ```shell curl -X PUT -H "X-Dataverse-key:$API_TOKEN" "$SERVER_URL/api/dataverses/$ID/attribute/requireFilesToPublishDataset?value=true" ``` diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index d174d4a87cd..8d73dd9dd24 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -1005,7 +1005,7 @@ The following attributes are supported: * ``description`` Description * ``affiliation`` Affiliation * ``filePIDsEnabled`` ("true" or "false") Restricted to use by superusers and only when the :ref:`:AllowEnablingFilePIDsPerCollection <:AllowEnablingFilePIDsPerCollection>` setting is true. Enables or disables registration of file-level PIDs in datasets within the collection (overriding the instance-wide setting). -* ``requireFilesToPublishDataset`` ("true" or "false") Dataset needs files in order to be published. Restricted to use by superusers. If any TRUE found in the ownership tree publishing will be blocked. Publishing by a superusers will not be blocked. +* ``requireFilesToPublishDataset`` ("true" or "false") Restricted to use by superusers. Defines if Dataset needs files in order to be published. If not set the determination will be made through inheritance by checking the owners of this collection. Publishing by a superusers will not be blocked. .. _collection-storage-quotas: diff --git a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java index 3bbf02fd611..5b6fbdee6ba 100644 --- a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java +++ b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java @@ -605,6 +605,15 @@ public void setCitationDatasetFieldTypes(List citationDatasetF @Column(nullable = true) private Boolean requireFilesToPublishDataset; + /** + * Specifies whether the existance of files in a dataset is required when publishing + * @return {@code Boolean.TRUE} if explicitly enabled, {@code Boolean.FALSE} if explicitly disabled. + * {@code null} indicates that the behavior is not explicitly defined, in which + * case the behavior should follow the explicit configuration of the first + * direct ancestor collection. + * @Note: If present, this configuration therefore by default applies to all + * the sub-collections, unless explicitly overwritten there. + */ public Boolean getRequireFilesToPublishDataset() { return requireFilesToPublishDataset; } diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java index dfe2bb44b20..50800f72271 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java @@ -230,11 +230,12 @@ private void verifyCommandArguments(CommandContext ctxt) throws IllegalCommandEx } private boolean requiresFilesToPublishDataset() { if (!getUser().isSuperuser()) { - List owners = getDataset().getOwner().getOwners(); - for(Dataverse owner : owners) { - if (owner.getRequireFilesToPublishDataset() != null && owner.getRequireFilesToPublishDataset()) { - return true; + Dataverse parent = getDataset().getOwner(); + while (parent != null) { + if (parent.getRequireFilesToPublishDataset() != null) { + return parent.getRequireFilesToPublishDataset(); } + parent = parent.getOwner(); } } return false; From 3f599cfa1935a5b421597590463284ba4b1dacef Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Thu, 21 Nov 2024 10:29:17 -0500 Subject: [PATCH 5/9] address review comments --- .../edu/harvard/iq/dataverse/Dataverse.java | 11 +++++++++++ .../command/impl/PublishDatasetCommand.java | 19 +++++++------------ 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java index 5b6fbdee6ba..f5d935ad353 100644 --- a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java +++ b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java @@ -791,6 +791,17 @@ public List getOwners() { return owners; } + public boolean getEffectiveRequireFilesToPublishDataset() { + Dataverse dv = this; + while (dv != null) { + if (dv.getRequireFilesToPublishDataset() != null) { + return dv.getRequireFilesToPublishDataset(); + } + dv = dv.getOwner(); + } + return false; + } + @Override public boolean equals(Object object) { // TODO: Warning - this method won't work in the case the id fields are not set diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java index 50800f72271..54223ac63b6 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java @@ -14,7 +14,6 @@ import edu.harvard.iq.dataverse.workflow.Workflow; import edu.harvard.iq.dataverse.workflow.WorkflowContext.TriggerType; -import java.util.List; import java.util.Optional; import java.util.logging.Logger; import static java.util.stream.Collectors.joining; @@ -223,22 +222,18 @@ private void verifyCommandArguments(CommandContext ctxt) throws IllegalCommandEx throw new IllegalCommandException("Cannot release as minor version. Re-try as major release.", this); } - if (getDataset().getFiles().isEmpty() && requiresFilesToPublishDataset()) { + if (getDataset().getFiles().isEmpty() && getEffectiveRequireFilesToPublishDataset()) { throw new IllegalCommandException(BundleUtil.getStringFromBundle("dataset.mayNotPublish.FilesRequired"), this); } } } - private boolean requiresFilesToPublishDataset() { - if (!getUser().isSuperuser()) { - Dataverse parent = getDataset().getOwner(); - while (parent != null) { - if (parent.getRequireFilesToPublishDataset() != null) { - return parent.getRequireFilesToPublishDataset(); - } - parent = parent.getOwner(); - } + private boolean getEffectiveRequireFilesToPublishDataset() { + if (getUser().isSuperuser()) { + return false; + } else { + Dataverse dv = getDataset().getOwner(); + return dv != null && dv.getEffectiveRequireFilesToPublishDataset(); } - return false; } @Override From a244f18eabff62070adf25f5cfc06a09b1b09859 Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Thu, 21 Nov 2024 10:43:30 -0500 Subject: [PATCH 6/9] address review comments --- src/main/java/edu/harvard/iq/dataverse/Dataverse.java | 2 +- .../dataverse/engine/command/impl/PublishDatasetCommand.java | 2 +- .../java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java | 5 +---- .../resources/db/migration/{V6.5.0.1.sql => V6.4.0.3.sql} | 0 src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java | 2 +- 5 files changed, 4 insertions(+), 7 deletions(-) rename src/main/resources/db/migration/{V6.5.0.1.sql => V6.4.0.3.sql} (100%) diff --git a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java index f5d935ad353..312f4aa13f1 100644 --- a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java +++ b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java @@ -791,7 +791,7 @@ public List getOwners() { return owners; } - public boolean getEffectiveRequireFilesToPublishDataset() { + public boolean getEffectiveRequiresFilesToPublishDataset() { Dataverse dv = this; while (dv != null) { if (dv.getRequireFilesToPublishDataset() != null) { diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java index 54223ac63b6..eccc69b95c6 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java @@ -232,7 +232,7 @@ private boolean getEffectiveRequireFilesToPublishDataset() { return false; } else { Dataverse dv = getDataset().getOwner(); - return dv != null && dv.getEffectiveRequireFilesToPublishDataset(); + return dv != null && dv.getEffectiveRequiresFilesToPublishDataset(); } } 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 fb4ef516f9b..dd8971d67a1 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 @@ -17,7 +17,6 @@ import edu.harvard.iq.dataverse.authorization.users.User; import edu.harvard.iq.dataverse.branding.BrandingUtil; import edu.harvard.iq.dataverse.dataaccess.DataAccess; -import edu.harvard.iq.dataverse.dataset.DatasetType; import edu.harvard.iq.dataverse.dataset.DatasetUtil; import edu.harvard.iq.dataverse.datavariable.CategoryMetadata; import edu.harvard.iq.dataverse.datavariable.DataVariable; @@ -294,9 +293,7 @@ public static JsonObjectBuilder json(Dataverse dv, Boolean hideEmail, Boolean re if (dv.getFilePIDsEnabled() != null) { bld.add("filePIDsEnabled", dv.getFilePIDsEnabled()); } - if (dv.getRequireFilesToPublishDataset() != null) { - bld.add("requireFilesToPublishDataset", dv.getRequireFilesToPublishDataset()); - } + bld.add("effectiveRequiresFilesToPublishDataset", dv.getEffectiveRequiresFilesToPublishDataset()); bld.add("isReleased", dv.isReleased()); List inputLevels = dv.getDataverseFieldTypeInputLevels(); diff --git a/src/main/resources/db/migration/V6.5.0.1.sql b/src/main/resources/db/migration/V6.4.0.3.sql similarity index 100% rename from src/main/resources/db/migration/V6.5.0.1.sql rename to src/main/resources/db/migration/V6.4.0.3.sql 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 1be1e498ccd..a33d077dc07 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java @@ -5193,7 +5193,7 @@ public void testRequireFilesToPublishDatasets() { setDataverseAttributeResponse = UtilIT.setCollectionAttribute(ownerAlias, "requireFilesToPublishDataset", "true", apiTokenAdmin); setDataverseAttributeResponse.prettyPrint(); setDataverseAttributeResponse.then().assertThat().statusCode(OK.getStatusCode()); - setDataverseAttributeResponse.then().assertThat().body("data.requireFilesToPublishDataset",equalTo(true)); + setDataverseAttributeResponse.then().assertThat().body("data.effectiveRequiresFilesToPublishDataset",equalTo(true)); Response publishDataverseResponse = UtilIT.publishDataverseViaNativeApi(ownerAlias, apiTokenAdmin); publishDataverseResponse.prettyPrint(); publishDataverseResponse.then().assertThat().statusCode(OK.getStatusCode()); From 5dce13b847be29ed0c32483cadba2b0ffe41523d Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Thu, 21 Nov 2024 10:50:31 -0500 Subject: [PATCH 7/9] address review comments --- .../dataverse/engine/command/impl/PublishDatasetCommand.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java index eccc69b95c6..2c32b1e8954 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/PublishDatasetCommand.java @@ -222,12 +222,12 @@ private void verifyCommandArguments(CommandContext ctxt) throws IllegalCommandEx throw new IllegalCommandException("Cannot release as minor version. Re-try as major release.", this); } - if (getDataset().getFiles().isEmpty() && getEffectiveRequireFilesToPublishDataset()) { + if (getDataset().getFiles().isEmpty() && getEffectiveRequiresFilesToPublishDataset()) { throw new IllegalCommandException(BundleUtil.getStringFromBundle("dataset.mayNotPublish.FilesRequired"), this); } } } - private boolean getEffectiveRequireFilesToPublishDataset() { + private boolean getEffectiveRequiresFilesToPublishDataset() { if (getUser().isSuperuser()) { return false; } else { From 6186a6e1147bb48324670927afb124024d29c23c Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Thu, 21 Nov 2024 13:41:02 -0500 Subject: [PATCH 8/9] address review comments --- src/main/java/propertyFiles/Bundle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index c82f9ab248d..30c1e96ee78 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -1542,7 +1542,7 @@ dataset.mayNotPublish.administrator= This dataset cannot be published until {0} dataset.mayNotPublish.both= This dataset cannot be published until {0} is published. Would you like to publish both right now? dataset.mayNotPublish.twoGenerations= This dataset cannot be published until {0} and {1} are published. dataset.mayNotBePublished.both.button=Yes, Publish Both -dataset.mayNotPublish.FilesRequired=This dataset cannot be published without uploaded files. +dataset.mayNotPublish.FilesRequired=Published datasets should contain at least one data file. dataset.viewVersion.unpublished=View Unpublished Version dataset.viewVersion.published=View Published Version dataset.link.title=Link Dataset From d12d9f6bc11829b41330ba85aaeef5b3081b1140 Mon Sep 17 00:00:00 2001 From: Steven Winship <39765413+stevenwinship@users.noreply.github.com> Date: Mon, 25 Nov 2024 10:16:18 -0500 Subject: [PATCH 9/9] fix bad merge --- 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 65e984de919..1b2d7e9a431 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java @@ -5323,7 +5323,7 @@ public void testRequireFilesToPublishDatasets() { setDataverseAttributeResponse = UtilIT.setCollectionAttribute(ownerAlias, "requireFilesToPublishDataset", "true", apiTokenAdmin); setDataverseAttributeResponse.prettyPrint(); setDataverseAttributeResponse.then().assertThat().statusCode(OK.getStatusCode()); - setDataverseAttributeResponse.then().assertThat().body("data.requireFilesToPublishDataset", equalTo(true)); + setDataverseAttributeResponse.then().assertThat().body("data.effectiveRequiresFilesToPublishDataset", equalTo(true)); Response publishDataverseResponse = UtilIT.publishDataverseViaNativeApi(ownerAlias, apiTokenAdmin); publishDataverseResponse.prettyPrint(); publishDataverseResponse.then().assertThat().statusCode(OK.getStatusCode());