From 8c8cc231eab54f1640d9b8169404a14f9a6a1213 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 22 Mar 2023 16:08:46 +0100 Subject: [PATCH 01/35] feat(pid): add dataset version pid configuration code #4499 This commit adds a new scope and setting to the JvmSettings, enabling the configuration of different modes for Dataset Version PIDs. These modes are depicted in VersionPidMode. A test ensures the parsability. In addition, VersionPidMode also contains a fine grained option to change the conduct of Dataverse collections and their datasets for these PIDs. --- .../pidproviders/VersionPidMode.java | 114 ++++++++++++++++++ .../iq/dataverse/settings/JvmSettings.java | 6 + .../pidproviders/VersionPidModeTest.java | 30 +++++ 3 files changed, 150 insertions(+) create mode 100644 src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java create mode 100644 src/test/java/edu/harvard/iq/dataverse/pidproviders/VersionPidModeTest.java diff --git a/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java b/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java new file mode 100644 index 00000000000..3f124720646 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java @@ -0,0 +1,114 @@ +package edu.harvard.iq.dataverse.pidproviders; + +import edu.harvard.iq.dataverse.Dataverse; + +import java.util.Arrays; +import java.util.Objects; +import java.util.Set; + +/** + * Enumlike class to bundle available options for PIDs of Dataset Versions. + * Must be a class to ensure case-insensitive configuration values (not possible/reliable with enum) + */ +public final class VersionPidMode { + + /** + * Means feature is switched off, no version PIDs will be minted + */ + public static final VersionPidMode OFF = new VersionPidMode("OFF"); + + /** + * Means the feature is activated instance wide for all collections and datasets + * (No opt-out so far!) + */ + public static final VersionPidMode GLOBAL = new VersionPidMode("GLOBAL"); + + /** + * Means the feature must be activated per Dataverse Collection (opt-in) + */ + public static final VersionPidMode COLLECTION = new VersionPidMode("COLLECTION"); + + /** + * A collection of conducts for mode {@link #COLLECTION}, used in {@link edu.harvard.iq.dataverse.Dataverse} + * and {@link edu.harvard.iq.dataverse.DataverseServiceBean#wantsDatasetVersionPids(Dataverse)}: + *
    + *
  1. Collection may inherit version pid behaviour from the parent collection(s),
    2. + *
  2. Collection may choose to be actively enabling it
    3. + *
  3. Collection may choose to opt out and skip the minting
    4. + *
+ */ + public enum CollectionConduct { + INHERIT("inherit"), + ACTIVE("active"), + SKIP("skip"); + + private final String name; + + CollectionConduct(String name) { + this.name = name; + } + + @Override + public String toString() { + return this.name; + } + + public static CollectionConduct findBy(String name) { + return Arrays.stream(CollectionConduct.values()) + .filter(cs -> cs.name.equalsIgnoreCase(name)) + .findFirst() + .orElse(null); + } + } + + + // Init as unmodifiable set + public static final Set values; + static { + values = Set.of(OFF, GLOBAL, COLLECTION); + } + + private final String mode; + + // Hide the no-arg constructor - no one shall get fancy ideas of extension. + private VersionPidMode() { + this.mode = ""; + } + + // Hide the constructor - no one shall get fancy ideas of extension. + private VersionPidMode(String mode) { + this.mode = mode; + } + + /** + * Used to enable auto-conversion for MicroProfile Config. + * Comparison is done case-insensitive to enable all variants of writing the config value. + * + * Note that a non-matching value will return null, which will trigger a {@link java.util.NoSuchElementException} + * for the conversion. + * + * @see MicroProfile Config Spec for Autoconverts + * + * @param mode The mode to lookup + * @return A matching constant or null if not matching. + */ + public static VersionPidMode of(String mode) { + return values.stream() + .filter(m -> m.mode.equalsIgnoreCase(mode)) + .findAny() + .orElse(null); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof VersionPidMode)) return false; + VersionPidMode that = (VersionPidMode) o; + return mode.equalsIgnoreCase(that.mode); + } + + @Override + public int hashCode() { + return Objects.hash(mode); + } +} diff --git a/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java b/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java index 370bd631dd9..b6a18dc6b9d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java +++ b/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java @@ -74,6 +74,12 @@ public enum JvmSettings { // Avoids adding flag entries twice. FEATURE_FLAG(SCOPE_FLAGS), + // PID SETTINGS + SCOPE_PID(PREFIX, "pid"), + + SCOPE_PID_VERSIONS(SCOPE_PID, "version"), + PID_VERSIONS_MODE(SCOPE_PID_VERSIONS, "mode"), + ; private static final String SCOPE_SEPARATOR = "."; diff --git a/src/test/java/edu/harvard/iq/dataverse/pidproviders/VersionPidModeTest.java b/src/test/java/edu/harvard/iq/dataverse/pidproviders/VersionPidModeTest.java new file mode 100644 index 00000000000..f6ad1018a86 --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/pidproviders/VersionPidModeTest.java @@ -0,0 +1,30 @@ +package edu.harvard.iq.dataverse.pidproviders; + +import edu.harvard.iq.dataverse.settings.JvmSettings; +import edu.harvard.iq.dataverse.util.testing.JvmSetting; +import org.junit.jupiter.api.Test; + +import java.util.NoSuchElementException; + +import static org.junit.jupiter.api.Assertions.*; +class VersionPidModeTest { + + @Test + @JvmSetting(key = JvmSettings.PID_VERSIONS_MODE, value = "collection") + void setToValidValue() { + assertEquals(VersionPidMode.COLLECTION, JvmSettings.PID_VERSIONS_MODE.lookup(VersionPidMode.class)); + } + + @Test + @JvmSetting(key = JvmSettings.PID_VERSIONS_MODE, value = "GloBal") + void setToOtherValidValue() { + assertEquals(VersionPidMode.GLOBAL, JvmSettings.PID_VERSIONS_MODE.lookup(VersionPidMode.class)); + } + + @Test + @JvmSetting(key = JvmSettings.PID_VERSIONS_MODE, value = "foobar") + void setToInvalidValue() { + assertThrows(NoSuchElementException.class, () -> JvmSettings.PID_VERSIONS_MODE.lookup(VersionPidMode.class)); + } + +} \ No newline at end of file From d4a3ed38915380e9b199eb1ae93c91bd82830a20 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 22 Mar 2023 16:14:46 +0100 Subject: [PATCH 02/35] refactor(model): switch JPA constraints from Hibernate internal to JSR official Also removing unused import of @Transient --- src/main/java/edu/harvard/iq/dataverse/Dataverse.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java index bc8716b6129..9f98abb3b7e 100644 --- a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java +++ b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java @@ -30,14 +30,13 @@ import javax.persistence.OneToOne; import javax.persistence.OrderBy; import javax.persistence.Table; -import javax.persistence.Transient; +import javax.validation.constraints.NotBlank; +import javax.validation.constraints.NotEmpty; import javax.validation.constraints.NotNull; import javax.validation.constraints.Pattern; import javax.validation.constraints.Size; import org.apache.commons.lang3.StringUtils; -import org.hibernate.validator.constraints.NotBlank; -import org.hibernate.validator.constraints.NotEmpty; /** * @@ -182,8 +181,7 @@ public void setDefaultContributorRole(DataverseRole defaultContributorRole) { private boolean facetRoot; // By default, themeRoot should be true, as new dataverses should start with the default theme private boolean themeRoot = true; - private boolean templateRoot; - + private boolean templateRoot; @OneToOne(mappedBy = "dataverse",cascade={ CascadeType.REMOVE, CascadeType.MERGE,CascadeType.PERSIST}, orphanRemoval=true) private DataverseTheme dataverseTheme; From 3cb82b43fb8e4416a9abecfe21e667fde198580a Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 22 Mar 2023 16:17:38 +0100 Subject: [PATCH 03/35] feat(model): add dataset version PID conduct to Dataverse collection JPA model Enable the database model to carry the configured conduct of a collection. Also enable JSON parser and printer to marshal the setting. --- .../edu/harvard/iq/dataverse/Dataverse.java | 22 ++++++++++++++++++- .../iq/dataverse/util/json/JsonParser.java | 6 +++++ .../iq/dataverse/util/json/JsonPrinter.java | 3 ++- .../V5.13.0.1__4499-dataset-version-pids.sql | 1 + 4 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql diff --git a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java index 9f98abb3b7e..d020bd48394 100644 --- a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java +++ b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java @@ -2,7 +2,7 @@ import edu.harvard.iq.dataverse.harvest.client.HarvestingClient; import edu.harvard.iq.dataverse.authorization.DataverseRole; -import edu.harvard.iq.dataverse.dataaccess.DataAccess; +import edu.harvard.iq.dataverse.pidproviders.VersionPidMode.CollectionConduct; import edu.harvard.iq.dataverse.search.savedsearch.SavedSearch; import edu.harvard.iq.dataverse.util.BundleUtil; import edu.harvard.iq.dataverse.util.SystemConfig; @@ -589,6 +589,26 @@ public void setCitationDatasetFieldTypes(List citationDatasetF } + + /** + * Indicate if this Dataverse Collection wants to publicize PIDs for each (major) {@link DatasetVersion} + * for any {@link Dataset} in it. + * + * @see edu.harvard.iq.dataverse.pidproviders.VersionPidMode#COLLECTION + * @see CollectionConduct + */ + @Enumerated(EnumType.STRING) + private CollectionConduct datasetVersionPidConduct = CollectionConduct.INHERIT; + + public void setDatasetVersionPidConduct(CollectionConduct conduct) { + this.datasetVersionPidConduct = conduct; + } + + public CollectionConduct getDatasetVersionPidConduct() { + return this.datasetVersionPidConduct; + } + + public List getDataverseFacets() { return getDataverseFacets(false); 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 22e2c6c8d78..1ef6bec9f3e 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 @@ -28,6 +28,8 @@ import edu.harvard.iq.dataverse.harvest.client.HarvestingClient; import edu.harvard.iq.dataverse.license.License; import edu.harvard.iq.dataverse.license.LicenseServiceBean; +import edu.harvard.iq.dataverse.pidproviders.VersionPidMode; +import edu.harvard.iq.dataverse.pidproviders.VersionPidMode.CollectionConduct; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; import edu.harvard.iq.dataverse.util.BundleUtil; import edu.harvard.iq.dataverse.workflow.Workflow; @@ -119,6 +121,10 @@ public Dataverse parseDataverse(JsonObject jobj) throws JsonParseException { dv.setPermissionRoot(jobj.getBoolean("permissionRoot", false)); dv.setFacetRoot(jobj.getBoolean("facetRoot", false)); dv.setAffiliation(jobj.getString("affiliation", null)); + dv.setDatasetVersionPidConduct( + CollectionConduct.findBy( + jobj.getString("versionPidsConduct", CollectionConduct.INHERIT.toString()) + )); if (jobj.containsKey("dataverseContacts")) { JsonArray dvContacts = jobj.getJsonArray("dataverseContacts"); 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 6ec4209336d..ccbcabd6f3a 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 @@ -274,7 +274,8 @@ public static JsonObjectBuilder json(Dataverse dv, Boolean hideEmail) { .add("id", dv.getId()) .add("alias", dv.getAlias()) .add("name", dv.getName()) - .add("affiliation", dv.getAffiliation()); + .add("affiliation", dv.getAffiliation()) + .add("versionPidsConduct", dv.getDatasetVersionPidConduct().toString()); if(!hideEmail) { bld.add("dataverseContacts", JsonPrinter.json(dv.getDataverseContacts())); } diff --git a/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql b/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql new file mode 100644 index 00000000000..bea4739c02a --- /dev/null +++ b/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql @@ -0,0 +1 @@ +ALTER TABLE dataverse ADD COLUMN IF NOT EXISTS datasetVersionPidConduct varchar(16); From b4644a0fac8a4dfb3661c50fa2032399ea0aac1a Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 22 Mar 2023 16:44:48 +0100 Subject: [PATCH 04/35] feat(collections): add business logic to determine dataset version pid conduct #4499 This commit adds a public method DataverseServiceBean.wantsDatasetVersionPids() that will determine how to deal with a dataset version (which belongs to a dataset that lives within a collection) in terms of "should a PID be registered/updated?". The background is: when a dataset is published, there will be the context of the owning Dataverse collection. It's important to take into account the configured conduct for the collection in the decision how to go ahead with a version's PID. --- .../iq/dataverse/DataverseServiceBean.java | 40 ++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java index e092f209acd..56939fb7837 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java @@ -15,9 +15,11 @@ import edu.harvard.iq.dataverse.batch.util.LoggingUtil; import edu.harvard.iq.dataverse.dataaccess.ImageThumbConverter; import edu.harvard.iq.dataverse.engine.command.DataverseRequest; +import edu.harvard.iq.dataverse.pidproviders.VersionPidMode; import edu.harvard.iq.dataverse.search.IndexServiceBean; import edu.harvard.iq.dataverse.search.SolrIndexServiceBean; import edu.harvard.iq.dataverse.search.SolrSearchResult; +import edu.harvard.iq.dataverse.settings.JvmSettings; import edu.harvard.iq.dataverse.util.StringUtil; import edu.harvard.iq.dataverse.util.SystemConfig; import java.io.File; @@ -30,7 +32,6 @@ import java.util.Map; import java.util.logging.Logger; import java.util.Properties; -import java.util.concurrent.Future; import javax.ejb.EJB; import javax.ejb.Stateless; import javax.inject.Inject; @@ -927,6 +928,41 @@ public List getDatasetTitlesWithinDataverse(Long dataverseId) { return em.createNativeQuery(cqString).getResultList(); } - + /** + * Check if a given Dataverse Collection has been configured to generate PIDs for any new version of a dataset + * contained in it. + * @param dataverse The collection to analyse + * @return true if enabled, false if disabled + * @throws java.util.NoSuchElementException When no or invalid configuration for version PID mode is given + */ + public boolean wantsDatasetVersionPids(Dataverse collection) { + VersionPidMode vpm = JvmSettings.PID_VERSIONS_MODE.lookup(VersionPidMode.class); + + if (vpm.equals(VersionPidMode.GLOBAL)) { + return true; + } else if (vpm.equals(VersionPidMode.OFF)) { + return false; + } + // now mode = collection's choice - ask the collection and if necessary all ancestors what to do + return askForVersionPidConduct(collection); + } + + /** + * Recursively scan all ancestors if someone defines a proper conduct for version PIDs. + * @param collection The collection to ask for advice + * @return true if someone in the hierarchy has state "ACTIVE", false otherwise + */ + private boolean askForVersionPidConduct(Dataverse collection) { + // Null safety here also matched the case where even root doesn't know, defaulting to save cycles. + if (collection == null) { + return false; + } + switch (collection.getDatasetVersionPidConduct()) { + case SKIP: return false; + case ACTIVE: return true; + case INHERIT: return askForVersionPidConduct(collection.getOwner()); + } + return false; + } } From b6eb3be301d3c47308dc1b27ccc2c30921777521 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 22 Mar 2023 16:46:49 +0100 Subject: [PATCH 05/35] feat(pid): add initial extension points for dataset version PIDs These are placeholders for now, to be filled with actual code. --- .../iq/dataverse/GlobalIdServiceBean.java | 7 +++++++ .../impl/FinalizeDatasetPublicationCommand.java | 17 +++++++++++++++++ .../command/impl/RegisterDvObjectCommand.java | 8 ++++++++ .../impl/UpdateDvObjectPIDMetadataCommand.java | 8 ++++++++ 4 files changed, 40 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java index 4ff3d6dc9ac..5be446b178e 100644 --- a/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java @@ -43,6 +43,13 @@ public interface GlobalIdServiceBean { boolean publicizeIdentifier(DvObject studyIn); + /** + * Create and publish a PID for a given DatasetVersion. + * @param datasetVersion The version to publish + * @return true if successful, false otherwise + */ + //boolean publicizeIdentifier(DatasetVersion datasetVersion); + String generateDatasetIdentifier(Dataset dataset); String generateDataFileIdentifier(DataFile datafile); boolean isGlobalIdUnique(GlobalId globalId); diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/FinalizeDatasetPublicationCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/FinalizeDatasetPublicationCommand.java index cb46b36eb53..d3374902069 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/FinalizeDatasetPublicationCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/FinalizeDatasetPublicationCommand.java @@ -103,6 +103,8 @@ public Dataset execute(CommandContext ctxt) throws CommandException { // sure we exit cleanly: registerExternalIdentifier(theDataset, ctxt, false); + + // TODO: try to register an identifier for the version as well if necessary } catch (CommandException comEx) { logger.warning("Failed to reserve the identifier "+theDataset.getGlobalId().asString()+"; notifying the user(s), unlocking the dataset"); // Send failure notification to the user: @@ -372,8 +374,10 @@ private void publicizeExternalIdentifier(Dataset dataset, CommandContext ctxt) t try { String currentGlobalIdProtocol = ctxt.settings().getValueForKey(SettingsServiceBean.Key.Protocol, ""); String currentGlobalAuthority = ctxt.settings().getValueForKey(SettingsServiceBean.Key.Authority, ""); + String dataFilePIDFormat = ctxt.settings().getValueForKey(SettingsServiceBean.Key.DataFilePIDFormat, "DEPENDENT"); boolean isFilePIDsEnabled = ctxt.systemConfig().isFilePIDsEnabled(); + // We will skip trying to register the global identifiers for datafiles // if "dependent" file-level identifiers are requested, AND the naming // protocol, or the authority of the dataset global id is different from @@ -399,6 +403,19 @@ private void publicizeExternalIdentifier(Dataset dataset, CommandContext ctxt) t df.setIdentifierRegistered(true); } } + + // Publish a PID for this dataset version if activated + // -> At this point, we have a latest version that is not in "released" state yet. + // -> + // TODO: might throw a NoSuchElementException. Adapt error message? + if (ctxt.dataverses().wantsDatasetVersionPids(dataset.getOwner())) { + // TODO: check result like above + // TODO: which version to pass? is at time of calling the latest version already released? + // TODO: maybe we should check for this being a *new* major version before handing it over... (this is not meant to be at discretion of the id service!) + //idServiceBean.publicizeIdentifier(dataset.getLatestVersion()); + } + + // Publish the Dataset PID (after the version because we want to include the version in metadata updates) if (!idServiceBean.publicizeIdentifier(dataset)) { throw new Exception(); } diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/RegisterDvObjectCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/RegisterDvObjectCommand.java index 299d1a925f4..7929b5b51ff 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/RegisterDvObjectCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/RegisterDvObjectCommand.java @@ -3,6 +3,7 @@ import edu.harvard.iq.dataverse.AlternativePersistentIdentifier; import edu.harvard.iq.dataverse.DataFile; import edu.harvard.iq.dataverse.Dataset; +import edu.harvard.iq.dataverse.Dataverse; import edu.harvard.iq.dataverse.DvObject; import edu.harvard.iq.dataverse.GlobalId; import edu.harvard.iq.dataverse.engine.command.AbstractVoidCommand; @@ -83,6 +84,13 @@ protected void executeImpl(CommandContext ctxt) throws CommandException { target.setGlobalIdCreateTime(new Timestamp(new Date().getTime())); } if (target.isReleased()) { + // If this is a dataset, release a new dataset version first before, to be included in the dataset + if (target.isInstanceofDataset() && + ctxt.dataverses().wantsDatasetVersionPids((Dataverse) target.getOwner())) { + // TODO: publicize dataset version as well + } + + // Now publicize the actual object idServiceBean.publicizeIdentifier(target); } if (idServiceBean.registerWhenPublished() && target.isReleased()) { diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/UpdateDvObjectPIDMetadataCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/UpdateDvObjectPIDMetadataCommand.java index 7e37241563c..bcda9aae507 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/UpdateDvObjectPIDMetadataCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/UpdateDvObjectPIDMetadataCommand.java @@ -2,6 +2,7 @@ import edu.harvard.iq.dataverse.DataFile; import edu.harvard.iq.dataverse.Dataset; +import edu.harvard.iq.dataverse.Dataverse; import edu.harvard.iq.dataverse.GlobalIdServiceBean; import edu.harvard.iq.dataverse.authorization.Permission; import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; @@ -48,7 +49,14 @@ protected void executeImpl(CommandContext ctxt) throws CommandException { } GlobalIdServiceBean idServiceBean = GlobalIdServiceBean.getBean(target.getProtocol(), ctxt); try { + // First publicize new dataset version if enabled, so it can be included in the update of the dataset + if (ctxt.dataverses().wantsDatasetVersionPids(target.getOwner())) { + // TODO: publicize dataset version as well + } + + // Publicize the dataset Boolean doiRetString = idServiceBean.publicizeIdentifier(target); + if (doiRetString) { target.setGlobalIdCreateTime(new Timestamp(new Date().getTime())); ctxt.em().merge(target); From 45dd5deb5c90d7f2da03688ac964ddf8c4cae106 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 22 Mar 2023 16:47:39 +0100 Subject: [PATCH 06/35] feat(api): add stub API endpoints for version pid conduct in collections These are placeholders for now, to be extended with real code. --- .../edu/harvard/iq/dataverse/api/Dataverses.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) 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 8ebeefec405..c348b9ce11b 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Dataverses.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Dataverses.java @@ -67,6 +67,7 @@ import edu.harvard.iq.dataverse.engine.command.impl.UpdateDataverseMetadataBlocksCommand; import edu.harvard.iq.dataverse.engine.command.impl.UpdateExplicitGroupCommand; import edu.harvard.iq.dataverse.engine.command.impl.UpdateMetadataBlockFacetsCommand; +import edu.harvard.iq.dataverse.pidproviders.VersionPidMode; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; import edu.harvard.iq.dataverse.util.BundleUtil; import edu.harvard.iq.dataverse.util.ConstraintViolationUtil; @@ -1328,5 +1329,19 @@ public Response linkDataverse(@Context ContainerRequestContext crc, @PathParam(" return ex.getResponse(); } } + + @GET + @AuthRequired + @Path("{id}/versionPidState") + public Response getVersionPidsState(@Context ContainerRequestContext crc, @PathParam("id") String identifier) { + return error(Status.NOT_FOUND, "Not implemented yet."); + } + @POST + @AuthRequired + @Path("{id}/versionPidState") + public Response setVersionPidsState(@Context ContainerRequestContext crc, @PathParam("id") String identifier, String stringState) { + return error(Status.NOT_FOUND, "Not implemented yet."); + } + } From a503718958bffef7db0848730c2a7634c02c7da1 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 24 Mar 2023 18:41:03 +0100 Subject: [PATCH 07/35] fix(model): make Dataverse.datasetVersionPidConduct require value Making it NON NULL in database and Bean Validation. --- src/main/java/edu/harvard/iq/dataverse/Dataverse.java | 2 ++ .../db/migration/V5.13.0.1__4499-dataset-version-pids.sql | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java index d020bd48394..3ee68d17696 100644 --- a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java +++ b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java @@ -597,6 +597,8 @@ public void setCitationDatasetFieldTypes(List citationDatasetF * @see edu.harvard.iq.dataverse.pidproviders.VersionPidMode#COLLECTION * @see CollectionConduct */ + @NotNull + @Column(nullable = false) @Enumerated(EnumType.STRING) private CollectionConduct datasetVersionPidConduct = CollectionConduct.INHERIT; diff --git a/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql b/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql index bea4739c02a..ef9d9fd335a 100644 --- a/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql +++ b/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql @@ -1 +1 @@ -ALTER TABLE dataverse ADD COLUMN IF NOT EXISTS datasetVersionPidConduct varchar(16); +ALTER TABLE dataverse ADD COLUMN IF NOT EXISTS datasetVersionPidConduct varchar(16) NOT NULL; From f340e12bbd4fca15921cd964e442ae55b2af77b0 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 24 Mar 2023 18:42:58 +0100 Subject: [PATCH 08/35] refactor(api): simplify ConstraintViolationExceptionHandler and make reusable Removing the unnecessary transformation via the ValidationError class and at the same time making it available to use from API endpoints to create nicely formatted JSON (error) responses. --- .../ConstraintViolationExceptionHandler.java | 73 +++++++++---------- 1 file changed, 34 insertions(+), 39 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ConstraintViolationExceptionHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ConstraintViolationExceptionHandler.java index 4cbf31d1d2c..7eb59bb268b 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ConstraintViolationExceptionHandler.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/ConstraintViolationExceptionHandler.java @@ -1,64 +1,59 @@ package edu.harvard.iq.dataverse.api.errorhandlers; -import edu.harvard.iq.dataverse.util.json.JsonPrinter; - import javax.json.Json; import javax.json.JsonArray; import javax.json.JsonArrayBuilder; -import javax.json.JsonObject; import javax.validation.ConstraintViolation; import javax.validation.ConstraintViolationException; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.ext.ExceptionMapper; import javax.ws.rs.ext.Provider; -import java.util.List; -import java.util.stream.Collectors; +import java.util.Set; @Provider public class ConstraintViolationExceptionHandler implements ExceptionMapper { - public class ValidationError { - private String path; - private String message; - - public String getPath() { return path; } - public void setPath(String path) { this.path = path; } - public String getMessage() { return message; } - public void setMessage(String message) { this.message = message; } - } - @Override public Response toResponse(ConstraintViolationException exception) { + JsonArrayBuilder builder = Json.createArrayBuilder(); - List errors = exception.getConstraintViolations().stream() - .map(this::toValidationError) - .collect(Collectors.toList()); + // This hack and code duplication is necessary as there is no way to make createResponse(Set>) + // also accept Set>. So we split by creating the JsonArray before the Response. + exception.getConstraintViolations().forEach(violation -> builder.add( + Json.createObjectBuilder() + .add("path", violation.getPropertyPath().toString()) + .add("message", violation.getMessage()) + .add("invalidValue", violation.getInvalidValue() == null ? "null" : violation.getInvalidValue().toString()))); - return Response.status(Response.Status.BAD_REQUEST) - .entity( Json.createObjectBuilder() - .add("status", "ERROR") - .add("code", Response.Status.BAD_REQUEST.getStatusCode()) - .add("message", "JPA validation constraints failed persistence. See list of violations for details.") - .add("violations", toJsonArray(errors)) - .build()) - .type(MediaType.APPLICATION_JSON_TYPE).build(); + return createResponse(builder.build()); } - private ValidationError toValidationError(ConstraintViolation constraintViolation) { - ValidationError error = new ValidationError(); - error.setPath(constraintViolation.getPropertyPath().toString()); - error.setMessage(constraintViolation.getMessage()); - return error; + /** + * Create a nice JSON based response from a set of constraint violations. + * @param violations The violations (will be transformed to JSON objects) + * @return A {@link Response} for JAX-RS, containing a JSON based description of the problems + */ + public static Response createResponse(Set> violations) { + JsonArrayBuilder builder = Json.createArrayBuilder(); + + violations.forEach(violation -> builder.add( + Json.createObjectBuilder() + .add("path", violation.getPropertyPath().toString()) + .add("message", violation.getMessage()) + .add("invalidValue", violation.getInvalidValue() == null ? "null" : violation.getInvalidValue().toString()))); + + return createResponse(builder.build()); } - private JsonArray toJsonArray(List list) { - JsonArrayBuilder builder = Json.createArrayBuilder(); - list.stream() - .forEach(error -> builder.add( - Json.createObjectBuilder() - .add("path", error.getPath()) - .add("message", error.getMessage()))); - return builder.build(); + private static Response createResponse(JsonArray violations) { + return Response.status(Response.Status.BAD_REQUEST) + .entity( Json.createObjectBuilder() + .add("status", "ERROR") + .add("code", Response.Status.BAD_REQUEST.getStatusCode()) + .add("message", "JPA validation constraints failed persistence. See list of violations for details.") + .add("violations", violations) + .build()) + .type(MediaType.APPLICATION_JSON_TYPE).build(); } } \ No newline at end of file From db272fab35ffdb77c6398fe7499b9902328d88ae Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 24 Mar 2023 18:50:39 +0100 Subject: [PATCH 09/35] feat(api): make some Dataverse collection attributes changeable For a first set of attributes (name, alias, description and, most important for the PR about #4499, the dataset version PID conduct), make an endpoint available that allows changes via simple PUT HTTP commands. --- .../harvard/iq/dataverse/api/Dataverses.java | 105 +++++++++++++++--- .../pidproviders/VersionPidMode.java | 6 + 2 files changed, 94 insertions(+), 17 deletions(-) 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 c348b9ce11b..848b0179671 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Dataverses.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Dataverses.java @@ -12,6 +12,7 @@ import edu.harvard.iq.dataverse.api.auth.AuthRequired; import edu.harvard.iq.dataverse.api.datadeposit.SwordServiceBean; import edu.harvard.iq.dataverse.api.dto.DataverseMetadataBlockFacetDTO; +import edu.harvard.iq.dataverse.api.errorhandlers.ConstraintViolationExceptionHandler; import edu.harvard.iq.dataverse.authorization.DataverseRole; import edu.harvard.iq.dataverse.DvObject; import edu.harvard.iq.dataverse.GlobalId; @@ -67,7 +68,7 @@ import edu.harvard.iq.dataverse.engine.command.impl.UpdateDataverseMetadataBlocksCommand; import edu.harvard.iq.dataverse.engine.command.impl.UpdateExplicitGroupCommand; import edu.harvard.iq.dataverse.engine.command.impl.UpdateMetadataBlockFacetsCommand; -import edu.harvard.iq.dataverse.pidproviders.VersionPidMode; +import edu.harvard.iq.dataverse.pidproviders.VersionPidMode.CollectionConduct; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; import edu.harvard.iq.dataverse.util.BundleUtil; import edu.harvard.iq.dataverse.util.ConstraintViolationUtil; @@ -76,17 +77,23 @@ import edu.harvard.iq.dataverse.util.json.JSONLDUtil; import edu.harvard.iq.dataverse.util.json.JsonParseException; +import edu.harvard.iq.dataverse.util.json.JsonPrinter; + import static edu.harvard.iq.dataverse.util.json.JsonPrinter.brief; import java.io.StringReader; import java.util.Collections; import java.util.LinkedList; import java.util.List; +import java.util.Set; import java.util.TreeSet; import java.util.logging.Level; import java.util.logging.Logger; +import javax.annotation.Resource; import javax.ejb.EJB; +import javax.ejb.EJBContext; import javax.ejb.EJBException; import javax.ejb.Stateless; +import javax.inject.Inject; import javax.json.Json; import javax.json.JsonArrayBuilder; import javax.json.JsonNumber; @@ -96,7 +103,10 @@ import javax.json.JsonValue; import javax.json.JsonValue.ValueType; import javax.json.stream.JsonParsingException; +import javax.validation.ConstraintViolation; import javax.validation.ConstraintViolationException; +import javax.validation.Validator; +import javax.validation.constraints.NotNull; import javax.ws.rs.BadRequestException; import javax.ws.rs.Consumes; import javax.ws.rs.DELETE; @@ -160,7 +170,13 @@ public class Dataverses extends AbstractApiBean { @EJB SwordServiceBean swordService; - + + @Inject + Validator validator; + + @Resource + EJBContext ejbCtxt; + @POST @AuthRequired public Response addRoot(@Context ContainerRequestContext crc, String body) { @@ -575,7 +591,76 @@ public Response deleteDataverse(@Context ContainerRequestContext crc, @PathParam return ok("Dataverse " + idtf + " deleted"); }, getRequestUser(crc)); } - + + /** + * Endpoint to change attributes of a Dataverse collection. + * + * @apiNote Example curl command: + * curl -X PUT -d "test" http://localhost:8080/api/dataverses/$ALIAS/attribute/alias + * to change the alias of the collection named $ALIAS to "test". + */ + @PUT + @AuthRequired + @Path("{identifier}/attribute/{attribute}") + public Response updateAttribute(@Context ContainerRequestContext crc, @PathParam("identifier") String identifier, + @PathParam("attribute") String attribute, @NotNull String value) { + try { + Dataverse collection = findDataverseOrDie(identifier); + User user = getRequestUser(crc); + DataverseRequest dvRequest = createDataverseRequest(user); + + // TODO: The cases below use hard coded strings, because we have no place for definitions of those! + // They are taken from util.json.JsonParser / util.json.JsonPrinter. This shall be changed. + // This also should be extended to more attributes, like the type, theme, contacts, some booleans, etc. + switch (attribute) { + case "alias": + collection.setAlias(value); + break; + case "name": + collection.setName(value); + break; + case "description": + collection.setDescription(value); + break; + case "affiliation": + collection.setAffiliation(value); + break; + case "versionPidsConduct": + CollectionConduct conduct = CollectionConduct.findBy(value); + if (conduct == null) { + return badRequest("'" + value + "' is not one of [" + + String.join(",", CollectionConduct.asList()) + "]"); + } + collection.setDatasetVersionPidConduct(conduct); + break; + default: + return badRequest("'" + attribute + "' is not a supported attribute"); + } + + // Validate now to avoid hubbub in Command Engine + Set> violations = validator.validate(collection); + if (!violations.isEmpty()) { + // TODO: This is an ugly hack to avoid the EJB transaction this endpoint method is automatically + // wrapped into (because this an EJB bean) trying to persist our changes from above to the + // database on it's on (which would obviously fail!) Usually, we would throw an exception + // to trigger the rollback, but that would cause noisy logs about them from the EJB container. + ejbCtxt.setRollbackOnly(); + return ConstraintViolationExceptionHandler.createResponse(violations); + } + + // Off to persistence layer + execCommand(new UpdateDataverseCommand(collection, null, null, dvRequest, null)); + + // Also return modified collection to user + return ok("Update successful", JsonPrinter.json(collection)); + + // TODO: This is an anti-pattern, necessary due to this bean being an EJB, causing very noisy and unnecessary + // logging by the EJB container for bubbling exceptions. (It would be handled by the error handlers.) + } catch (WrappedResponse e) { + return e.getResponse(); + } + } + @DELETE @AuthRequired @Path("{linkingDataverseId}/deleteLink/{linkedDataverseId}") @@ -1330,18 +1415,4 @@ public Response linkDataverse(@Context ContainerRequestContext crc, @PathParam(" } } - @GET - @AuthRequired - @Path("{id}/versionPidState") - public Response getVersionPidsState(@Context ContainerRequestContext crc, @PathParam("id") String identifier) { - return error(Status.NOT_FOUND, "Not implemented yet."); - } - - @POST - @AuthRequired - @Path("{id}/versionPidState") - public Response setVersionPidsState(@Context ContainerRequestContext crc, @PathParam("id") String identifier, String stringState) { - return error(Status.NOT_FOUND, "Not implemented yet."); - } - } diff --git a/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java b/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java index 3f124720646..099454eab52 100644 --- a/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java +++ b/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java @@ -3,8 +3,10 @@ import edu.harvard.iq.dataverse.Dataverse; import java.util.Arrays; +import java.util.List; import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; /** * Enumlike class to bundle available options for PIDs of Dataset Versions. @@ -59,6 +61,10 @@ public static CollectionConduct findBy(String name) { .findFirst() .orElse(null); } + + public static List asList() { + return values.stream().map(Object::toString).collect(Collectors.toList()); + } } From 8a37a50dc7cf877d77f098eb7a1e729acc69c480 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 29 Mar 2023 17:52:38 +0200 Subject: [PATCH 10/35] fix(model): add missing default for collection version pid conduct in DB migration --- .../db/migration/V5.13.0.1__4499-dataset-version-pids.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql b/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql index ef9d9fd335a..6105fd09ac2 100644 --- a/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql +++ b/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql @@ -1 +1 @@ -ALTER TABLE dataverse ADD COLUMN IF NOT EXISTS datasetVersionPidConduct varchar(16) NOT NULL; +ALTER TABLE dataverse ADD COLUMN IF NOT EXISTS datasetVersionPidConduct varchar(16) NOT NULL DEFAULT 'inherit'; From 2dc83b7ab0653a7aea88600bcaea892e8aa30480 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 29 Mar 2023 17:54:08 +0200 Subject: [PATCH 11/35] feat(model): add PID field to DatasetVersion model and include in DB migration --- .../harvard/iq/dataverse/DatasetVersion.java | 20 +++++++++++++++++++ .../V5.13.0.1__4499-dataset-version-pids.sql | 5 +++++ 2 files changed, 25 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java b/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java index a043d110473..92a71d79ff5 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java @@ -128,6 +128,18 @@ public enum VersionState { @Column(length = VERSION_NOTE_MAX_LENGTH) private String versionNote; + /** + * A (globally) unique persisten identifier for this version. + * The version PID will always be dependent on the protocol and authority of the containing dataset. + * This identifier may contain {@link edu.harvard.iq.dataverse.settings.SettingsServiceBean.Key#Shoulder} if + * configured and some more unique characters, also depending on the admin's choice to make version PIDs dependent + * on the dataset PID. + * + * The PID may be null (feature disabled, old entry, etc), but if present, it must be unique. + */ + @Column(unique = true) + private String persistentIdentifier; + /* * @todo versionState should never be null so when we are ready, uncomment * the `nullable = false` below. @@ -232,6 +244,14 @@ public Long getVersion() { public void setVersion(Long version) { } + + public String getPersistentIdentifier() { + return this.persistentIdentifier; + } + + public void setPersistentIdentifier(String identifier) { + this.persistentIdentifier = identifier; + } public String getDataverseSiteUrl() { return dataverseSiteUrl; diff --git a/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql b/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql index 6105fd09ac2..a0e71279b21 100644 --- a/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql +++ b/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql @@ -1 +1,6 @@ ALTER TABLE dataverse ADD COLUMN IF NOT EXISTS datasetVersionPidConduct varchar(16) NOT NULL DEFAULT 'inherit'; + +ALTER TABLE datasetVersion ADD COLUMN IF NOT EXISTS persistentIdentifier varchar(255); +/* As Postgres does not support "if not exists" when adding a constraint, must remove first to make this not bail */ +ALTER TABLE datasetVersion DROP CONSTRAINT IF EXISTS datasetversion_persistentidentifier_key; +ALTER TABLE datasetVersion ADD CONSTRAINT datasetversion_persistentidentifier_key UNIQUE(persistentIdentifier); From 2d71f55939cd27c1a49d5b2f5c07ecb4af68d477 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 19 Apr 2023 18:21:34 +0200 Subject: [PATCH 12/35] feat(pid): add version pid generation styles Adding the generations styles as a class again to enable auto-conversion via MicroProfile Config API. Also adding a JvmSettings entry to enable lookup of the configured style. --- .../pidproviders/VersionPidMode.java | 36 +++++++++++++++++++ .../iq/dataverse/settings/JvmSettings.java | 1 + 2 files changed, 37 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java b/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java index 099454eab52..94d1ab6040a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java +++ b/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java @@ -67,6 +67,42 @@ public static List asList() { } } + /** + * Defining a list of styles how to generate the version PIDs. + * This is not extensible from the outside on purpose. + * This is a class to enable auto-conversion via MicroProfile Config + * also with lowercase setting values. + */ + public static final class GenStyle { + + public static final GenStyle DATASET = new GenStyle("DATASET"); + public static final GenStyle SUFFIX = new GenStyle("SUFFIX"); + + // Init as unmodifiable set + public static final Set values; + static { + values = Set.of(DATASET, SUFFIX); + } + + private final String style; + + GenStyle(String style) { + this.style = style; + } + + @Override + public String toString() { + return this.style; + } + + public static GenStyle of(String style) { + return values.stream() + .filter(m -> m.style.equalsIgnoreCase(style)) + .findAny() + .orElse(null); + } + } + // Init as unmodifiable set public static final Set values; diff --git a/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java b/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java index 1060f2c7222..3606247faff 100644 --- a/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java +++ b/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java @@ -109,6 +109,7 @@ public enum JvmSettings { // VERSION PID SETTINGS SCOPE_PID_VERSIONS(SCOPE_PID, "version"), PID_VERSIONS_MODE(SCOPE_PID_VERSIONS, "mode"), + PID_VERSIONS_STYLE(SCOPE_PID_VERSIONS, "style"), ; From 7acbfb79ea4030d3a6dd0adca45b0fe2c64427e9 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 19 Apr 2023 18:24:49 +0200 Subject: [PATCH 13/35] feat(pid): add version identifier methods to PID provider interface #4499 This commit adds the methods necessary to be implemented by any provider that supports version PIDs. By default, the interface will ensure that exceptions are thrown about unsupported actions, when the methods are not implemented. Extensive JavaDoc describes details. --- .../iq/dataverse/GlobalIdServiceBean.java | 45 +++++++++++++++++-- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java index 5be446b178e..b73e4433138 100644 --- a/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java @@ -44,14 +44,53 @@ public interface GlobalIdServiceBean { boolean publicizeIdentifier(DvObject studyIn); /** - * Create and publish a PID for a given DatasetVersion. + * Publish a PID for a given {@link DatasetVersion}. + * + * @apiNote This method is meant to be called when a new version of a dataset is being published. + * * @param datasetVersion The version to publish - * @return true if successful, false otherwise + * @return true if successful, false otherwise (or when datasetVersion is null) + * @throws IllegalArgumentException When a provider does not support generating these PIDs, does not allow their + * generation due to configuration or doesn't like the current look of the version + * (i.e. not being a new major version or not having an identifier set). */ - //boolean publicizeIdentifier(DatasetVersion datasetVersion); + default boolean publicizeIdentifier(final DatasetVersion datasetVersion) { + throw new IllegalArgumentException("This provider does not (yet) support publishing versions."); + } String generateDatasetIdentifier(Dataset dataset); String generateDataFileIdentifier(DataFile datafile); + + /** + * Generate a new PID for a {@link DatasetVersion}. + * + * Note that the generation of this identifier depends on configuration by a sysadmin and concrete + * implementation for a given PID provider (it might be limited by its capabilities). + * + * @implNote This method is meant to be implemented free of side effects. + * + * @param datasetVersion The version of a dataset to create a PID for + * @return An "identifier", meant to be used for {@link GlobalId}, retrievable via {@link GlobalId#getIdentifier()}. + * Must not be null. + * @throws IllegalArgumentException When a provider does not support generating these PIDs, does not allow their + * generation due to configuration or doesn't like the current look of the version + * (i.e. not being a new major version). + */ + default String generateDatasetVersionIdentifier(final DatasetVersion datasetVersion) { + throw new IllegalArgumentException("This provider does not (yet) support publishing versions."); + } + + /** + * Retrieve the character that is inserted as a delimiter between the dataset identifier + * and the version number. Defaults to a dot ".", but in case a PID system does not support + * this character, the provider can override it. + * + * @return The delimiter character, defaulting to '.' + */ + default char getVersionSuffixDelimiter() { + return '.'; + } + boolean isGlobalIdUnique(GlobalId globalId); String getUrlPrefix(); From 5a6dc683173ca1965f04b487a07140d0882ba3a3 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 19 Apr 2023 18:42:50 +0200 Subject: [PATCH 14/35] feat(pid): add default generator for version PIDs Adding a default generator saves implementing the logic multiple times. This generator understands the two known styles. It also ensures the business logic that no released versions receive an identifier and only major versions. --- .../AbstractGlobalIdServiceBean.java | 31 ++++ .../AbstractGlobalIdServiceBeanTest.java | 159 ++++++++++++++++++ 2 files changed, 190 insertions(+) create mode 100644 src/test/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBeanTest.java diff --git a/src/main/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBean.java index 479438f3f45..8e56632100d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBean.java @@ -1,5 +1,7 @@ package edu.harvard.iq.dataverse; +import edu.harvard.iq.dataverse.pidproviders.VersionPidMode.GenStyle; +import edu.harvard.iq.dataverse.settings.JvmSettings; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; import edu.harvard.iq.dataverse.util.SystemConfig; import java.io.InputStream; @@ -155,6 +157,35 @@ public DvObject generateIdentifier(DvObject dvObject) { return dvObject; } + /** + * Generate an identifier for a given dataset version, depending on the chosen (configured) generation style. + * (See also {@link GenStyle} for available styles.) + * + * @param datasetVersion The version of a dataset to create a PID for + * @return The identifier (will never be null) + * @throws NoSuchElementException When no style has been configured for the instance + * @throws IllegalArgumentException If the style configured is not supported by this generator, the version is + * already released or a minor version. + */ + @Override + public String generateDatasetVersionIdentifier(DatasetVersion datasetVersion) { + // HINT: this default business logic might be made configurable later + // or can be overridden by a provider implementing custom logic. + if (datasetVersion.isReleased() || datasetVersion.getMinorVersionNumber() > 0) { + throw new IllegalArgumentException("Released or draft minor versions are not allowed"); + } + + GenStyle style = JvmSettings.PID_VERSIONS_STYLE.lookup(GenStyle.class); + + if (style == GenStyle.DATASET) { + return generateDatasetIdentifier(datasetVersion.getDataset()); + } else if (style == GenStyle.SUFFIX) { + return datasetVersion.getDataset().getIdentifier() + getVersionSuffixDelimiter() + datasetVersion.getVersionNumber(); + } + + throw new IllegalArgumentException("No supported version PID generation style configured"); + } + //ToDo just send the DvObject.DType public String generateDatasetIdentifier(Dataset dataset) { //ToDo - track these in the bean diff --git a/src/test/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBeanTest.java b/src/test/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBeanTest.java new file mode 100644 index 00000000000..3c25d1dbbe3 --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBeanTest.java @@ -0,0 +1,159 @@ +package edu.harvard.iq.dataverse; + +import edu.harvard.iq.dataverse.mocks.MocksFactory; +import edu.harvard.iq.dataverse.settings.JvmSettings; +import edu.harvard.iq.dataverse.util.testing.JvmSetting; +import org.apache.commons.lang3.RandomStringUtils; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeFalse; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +class AbstractGlobalIdServiceBeanTest { + + private static final int idLen = 8; + + @Test + @JvmSetting(key = JvmSettings.PID_VERSIONS_STYLE, value = "dataset") + void generateDatasetVersionIdentifierWithStyleDataset() { + // given + TestIdService sut = new TestIdService(); + Dataset dataset = MocksFactory.makeDataset(); + String datasetIdentifier = dataset.getIdentifier(); + + // We assume that this code will be called with a dataset version that is about to be published + // and thus is not in status released. The random generated dataset is assumed to have the version number + // "1", so we can compare things properly. + assumeFalse(dataset.getLatestVersion().isReleased()); + assumeTrue(dataset.getLatestVersion().getVersionNumber() == 1); + assumeTrue(dataset.getLatestVersion().getMinorVersionNumber() == 0); + + // when + String versionIdentifier = sut.generateDatasetVersionIdentifier(dataset.getLatestVersion()); + + // then + assertNotEquals(datasetIdentifier, versionIdentifier); + assertFalse(versionIdentifier.contains(".")); + assertEquals(idLen, versionIdentifier.length()); + } + + @Test + @JvmSetting(key = JvmSettings.PID_VERSIONS_STYLE, value = "suffix") + void generateDatasetVersionIdentifierWithStyleSuffix() { + // given + TestIdService sut = new TestIdService(); + Dataset dataset = MocksFactory.makeDataset(); + String datasetIdentifier = dataset.getIdentifier(); + + // We assume that this code will be called with a dataset version that is about to be published + // and thus is not in status released. The random generated dataset is assumed to have the version number + // "1", so we can compare things properly. + assumeFalse(dataset.getLatestVersion().isReleased()); + assumeTrue(dataset.getLatestVersion().getVersionNumber() == 1); + assumeTrue(dataset.getLatestVersion().getMinorVersionNumber() == 0); + + // when + String versionIdentifier = sut.generateDatasetVersionIdentifier(dataset.getLatestVersion()); + + // then + assertNotEquals(datasetIdentifier, versionIdentifier); + assertTrue(versionIdentifier.contains(".")); + assertEquals(versionIdentifier, datasetIdentifier + ".1"); + } + + @Test + void generateDatasetVersionIdentifierFailsWithMinorVersion() { + // given + TestIdService sut = new TestIdService(); + Dataset dataset = MocksFactory.makeDataset(); + + assumeFalse(dataset.getLatestVersion().isReleased()); + + // Set to non-allowed combination + dataset.getLatestVersion().setMinorVersionNumber(1L); + + // when & then (split retrieval to avoid ambiguous lambda exceptions) + DatasetVersion version = dataset.getLatestVersion(); + assertThrows(IllegalArgumentException.class, () -> sut.generateDatasetVersionIdentifier(version)); + } + + @Test + void generateDatasetVersionIdentifierFailsWithReleasedVersion() { + // given + TestIdService sut = new TestIdService(); + Dataset dataset = MocksFactory.makeDataset(); + String datasetIdentifier = dataset.getIdentifier(); + + assumeTrue(dataset.getLatestVersion().getMinorVersionNumber() == 0); + + // Set to non-allowed combination + dataset.getLatestVersion().setVersionState(DatasetVersion.VersionState.RELEASED); + + // when & then (split retrieval to avoid ambiguous lambda exceptions) + DatasetVersion version = dataset.getLatestVersion(); + assertThrows(IllegalArgumentException.class, () -> sut.generateDatasetVersionIdentifier(version)); + } + + + static class TestIdService extends AbstractGlobalIdServiceBean { + + @Override + public boolean alreadyExists(GlobalId globalId) throws Exception { + return false; + } + + @Override + public boolean registerWhenPublished() { + return false; + } + + @Override + public List getProviderInformation() { + return null; + } + + @Override + public String createIdentifier(DvObject dvo) throws Throwable { + return RandomStringUtils.randomAlphanumeric(idLen).toUpperCase(); + } + + @Override + public String generateDatasetIdentifier(Dataset dataset) { + return RandomStringUtils.randomAlphanumeric(idLen).toUpperCase(); + } + + @Override + public Map getIdentifierMetadata(DvObject dvo) { + return null; + } + + @Override + public String modifyIdentifierTargetURL(DvObject dvo) throws Exception { + return null; + } + + @Override + public void deleteIdentifier(DvObject dvo) throws Exception { + + } + + @Override + public boolean publicizeIdentifier(DvObject studyIn) { + return false; + } + + @Override + public String getUrlPrefix() { + return null; + } + } + +} \ No newline at end of file From 4ca88b952d94781a2176cff545d91f1728bd105d Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 20 Apr 2023 19:35:10 +0200 Subject: [PATCH 15/35] feat(pid): check dataset has identifier for version pid by suffix In (the unlikely) case the dataset owning a dataset version will ever have no identifier set already when trying to create a suffix style identifier for the version, this new check will ensure proper failure and avoid adding the suffix to a null or empty string. Includes tests. --- .../AbstractGlobalIdServiceBean.java | 10 ++++++++-- .../AbstractGlobalIdServiceBeanTest.java | 20 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBean.java index 8e56632100d..bbf433abc67 100644 --- a/src/main/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBean.java @@ -165,7 +165,8 @@ public DvObject generateIdentifier(DvObject dvObject) { * @return The identifier (will never be null) * @throws NoSuchElementException When no style has been configured for the instance * @throws IllegalArgumentException If the style configured is not supported by this generator, the version is - * already released or a minor version. + * already released or a minor version, or if the owning dataset has no identifier + * while creating a suffix style identifier. */ @Override public String generateDatasetVersionIdentifier(DatasetVersion datasetVersion) { @@ -180,7 +181,12 @@ public String generateDatasetVersionIdentifier(DatasetVersion datasetVersion) { if (style == GenStyle.DATASET) { return generateDatasetIdentifier(datasetVersion.getDataset()); } else if (style == GenStyle.SUFFIX) { - return datasetVersion.getDataset().getIdentifier() + getVersionSuffixDelimiter() + datasetVersion.getVersionNumber(); + String datasetIdentifier = datasetVersion.getDataset().getIdentifier(); + if (datasetIdentifier == null || datasetIdentifier.isEmpty()) { + throw new IllegalArgumentException("Dataset must not have empty identifier when creating dataset version identifier by suffix"); + } + + return datasetIdentifier + getVersionSuffixDelimiter() + datasetVersion.getVersionNumber(); } throw new IllegalArgumentException("No supported version PID generation style configured"); diff --git a/src/test/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBeanTest.java b/src/test/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBeanTest.java index 3c25d1dbbe3..a652fd0db1b 100644 --- a/src/test/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBeanTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBeanTest.java @@ -5,6 +5,8 @@ import edu.harvard.iq.dataverse.util.testing.JvmSetting; import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.NullAndEmptySource; import java.util.List; import java.util.Map; @@ -102,6 +104,24 @@ void generateDatasetVersionIdentifierFailsWithReleasedVersion() { assertThrows(IllegalArgumentException.class, () -> sut.generateDatasetVersionIdentifier(version)); } + @ParameterizedTest + @JvmSetting(key = JvmSettings.PID_VERSIONS_STYLE, value = "suffix") + @NullAndEmptySource + void generateDatasetVersionIdentifierFailsWithSuffixStyleAndEmptyDatasetIdentifier(String pid) { + // given + TestIdService sut = new TestIdService(); + Dataset dataset = MocksFactory.makeDataset(); + + assumeTrue(dataset.getLatestVersion().getMinorVersionNumber() == 0); + + // Set to non-allowed combination + dataset.setIdentifier(pid); + + // when & then (split retrieval to avoid ambiguous lambda exceptions) + DatasetVersion version = dataset.getLatestVersion(); + assertThrows(IllegalArgumentException.class, () -> sut.generateDatasetVersionIdentifier(version)); + } + static class TestIdService extends AbstractGlobalIdServiceBean { From 5288bef9b27a77167d2dc077d80facbc7854f650 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 20 Apr 2023 19:39:49 +0200 Subject: [PATCH 16/35] feat(pid): enable version PID generation when new version is created Any new version that is created by executing the CreateDatasetVersionCommand will need to have an identifier generated at that precise moment. Depending on the activation status, the generation is triggered. As the global PID service needs to retrieved from the context of the command engine, the context is added as a parameter to the method. --- .../impl/AbstractCreateDatasetCommand.java | 5 ++++- .../impl/CreateDatasetVersionCommand.java | 16 ++++++++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AbstractCreateDatasetCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AbstractCreateDatasetCommand.java index 8f477a66424..daf5d7da55f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AbstractCreateDatasetCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AbstractCreateDatasetCommand.java @@ -81,7 +81,7 @@ public Dataset execute(CommandContext ctxt) throws CommandException { DatasetVersion dsv = getVersionToPersist(theDataset); // This re-uses the state setup logic of CreateDatasetVersionCommand, but // without persisting the new version, or altering its files. - new CreateDatasetVersionCommand(getRequest(), theDataset, dsv).prepareDatasetAndVersion(); + new CreateDatasetVersionCommand(getRequest(), theDataset, dsv).prepareDatasetAndVersion(ctxt); theDataset.setCreator((AuthenticatedUser) getRequest().getUser()); @@ -104,6 +104,9 @@ public Dataset execute(CommandContext ctxt) throws CommandException { String driverId = theDataset.getEffectiveStorageDriverId(); theDataset.setStorageIdentifier(driverId + DataAccess.SEPARATOR + theDataset.getAuthorityForFileStorage() + "/" + theDataset.getIdentifierForFileStorage()); } + + // TODO: why is this here? At the very start of this command execution we generate an identifier - why should + // it be gone now and if so - why would we simply create a new one? Wouldn't this be a major flaw? if (theDataset.getIdentifier()==null) { theDataset.setIdentifier(idServiceBean.generateDatasetIdentifier(theDataset)); } diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CreateDatasetVersionCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CreateDatasetVersionCommand.java index 72439d4ba4a..2b4975f4dca 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CreateDatasetVersionCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CreateDatasetVersionCommand.java @@ -4,6 +4,7 @@ import edu.harvard.iq.dataverse.DatasetVersion; import edu.harvard.iq.dataverse.DatasetVersion.VersionState; import edu.harvard.iq.dataverse.FileMetadata; +import edu.harvard.iq.dataverse.GlobalIdServiceBean; import edu.harvard.iq.dataverse.authorization.Permission; import edu.harvard.iq.dataverse.engine.command.CommandContext; import edu.harvard.iq.dataverse.engine.command.DataverseRequest; @@ -57,7 +58,7 @@ public DatasetVersion execute(CommandContext ctxt) throws CommandException { //because it includes validation and we need the validation //to happen after file metdata is added to return a //good wrapped response if the TOA/Request Access not in compliance - prepareDatasetAndVersion(); + prepareDatasetAndVersion(ctxt); // TODO make async // ctxt.index().indexDataset(dataset); @@ -72,7 +73,7 @@ public DatasetVersion execute(CommandContext ctxt) throws CommandException { * * @throws CommandException */ - public void prepareDatasetAndVersion() throws CommandException { + public void prepareDatasetAndVersion(CommandContext ctxt) throws CommandException { newVersion.setDataset(dataset); newVersion.setDatasetFields(newVersion.initDatasetFields()); newVersion.setCreateTime(getTimestamp()); @@ -83,6 +84,17 @@ public void prepareDatasetAndVersion() throws CommandException { //had been stripped from the dataset fields prior to validation validateOrDie(newVersion, false); DatasetFieldUtil.tidyUpFields(newVersion.getDatasetFields(), true); + + /* If the owning Dataverse collection or global configuration requires it, we will try to generate + * a persistent identifier for this new dataset version. + * This implies when the generation style is "suffix", the dataset itself must already have an identifier. + * This is currently the case, as this method is called by the code that prepares new datasets + * and the identifier is generated before this. This (might) result in throwing an illegal argument exception. + */ + GlobalIdServiceBean globalIdServiceBean = GlobalIdServiceBean.getBean(dataset.getProtocol(), ctxt); + if (globalIdServiceBean != null && ctxt.dataverses().wantsDatasetVersionPids(dataset.getOwner())) { + newVersion.setPersistentIdentifier(globalIdServiceBean.generateDatasetVersionIdentifier(newVersion)); + } final List currentVersions = dataset.getVersions(); ArrayList dsvs = new ArrayList<>(currentVersions.size()); From 37bcecc3e437bf3e85480e4d16562d6298b5e1ae Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Apr 2023 10:55:16 +0200 Subject: [PATCH 17/35] feat(pid): add global id generation to versions and print To enable printing the persistent identifier of a dataset version in JSON exports, the version needs to hand over a GlobalId composed of protocol, authority and identifier. Adding this getter to the model and printing null-safe in JsonPrinter now. --- .../harvard/iq/dataverse/DatasetVersion.java | 26 +++++++++++++++++++ .../iq/dataverse/util/json/JsonPrinter.java | 2 ++ 2 files changed, 28 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java b/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java index 92a71d79ff5..faafc219584 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java @@ -140,6 +140,14 @@ public enum VersionState { @Column(unique = true) private String persistentIdentifier; + /** + * Caching the {@link GlobalId} in a transient field saves retrievals and reformatting. Its value will + * be based on {@link #persistentIdentifier}, and details from {@link #dataset} like protocol, authority and + * shoulder. + */ + @Transient + private GlobalId globalId; + /* * @todo versionState should never be null so when we are ready, uncomment * the `nullable = false` below. @@ -252,6 +260,24 @@ public String getPersistentIdentifier() { public void setPersistentIdentifier(String identifier) { this.persistentIdentifier = identifier; } + + /** + * Create a {@link GlobalId} from {@link #persistentIdentifier} and the owning {@link #dataset} + * details of protocol and authority. This method is not free of side effects: it will cache + * the generated value in a transient instance variable if not yet initialized. + * + * @return The global id for this version or null if no PID, protocol or authority present. + */ + public GlobalId getGlobalId() { + if (this.globalId == null && this.getPersistentIdentifier() != null && + this.dataset.getProtocol() != null && this.dataset.getAuthority() != null) { + this.globalId = PidUtil.parseAsGlobalID( + this.dataset.getProtocol(), + this.dataset.getAuthority(), + this.getPersistentIdentifier()); + } + return this.globalId; + } public String getDataverseSiteUrl() { return dataverseSiteUrl; 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 a1f9ff0155c..5c926aa114d 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 @@ -361,6 +361,8 @@ public static JsonObjectBuilder json(DatasetVersion dsv) { JsonObjectBuilder bld = jsonObjectBuilder() .add("id", dsv.getId()).add("datasetId", dsv.getDataset().getId()) .add("datasetPersistentId", dsv.getDataset().getGlobalId().asString()) + // This might be null if this version does not have a version, the builder will silently omit it then. + .add("persistentId", dsv.getGlobalId().asString()) .add("storageIdentifier", dsv.getDataset().getStorageIdentifier()) .add("versionNumber", dsv.getVersionNumber()).add("versionMinorNumber", dsv.getMinorVersionNumber()) .add("versionState", dsv.getVersionState().name()).add("versionNote", dsv.getVersionNote()) From 2885dab20a6a8f5e0cb4b8a0f90c0e331064709e Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Apr 2023 11:06:56 +0200 Subject: [PATCH 18/35] fix(pid,sql): correct SQL default for version pid conduct in migration script --- .../db/migration/V5.13.0.1__4499-dataset-version-pids.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql b/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql index a0e71279b21..8267db2d2f5 100644 --- a/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql +++ b/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql @@ -1,4 +1,4 @@ -ALTER TABLE dataverse ADD COLUMN IF NOT EXISTS datasetVersionPidConduct varchar(16) NOT NULL DEFAULT 'inherit'; +ALTER TABLE dataverse ADD COLUMN IF NOT EXISTS datasetVersionPidConduct varchar(16) NOT NULL DEFAULT 'INHERIT'; ALTER TABLE datasetVersion ADD COLUMN IF NOT EXISTS persistentIdentifier varchar(255); /* As Postgres does not support "if not exists" when adding a constraint, must remove first to make this not bail */ From e8d0023fae78f1210218d72c9df63c0b87204c8c Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Apr 2023 16:45:58 +0200 Subject: [PATCH 19/35] Revert "feat(pid): enable version PID generation when new version is created" This reverts commit 5288bef9b27a77167d2dc077d80facbc7854f650. After investigation, the timing to wire the generation in like here is ill advised. This command is executed when a *new* dataset is about to be created and all of the details in the DatasetVersion are still "null"! (No status yet, no version numbers, etc) As we will never know a-priori if a version is going to be major or minor (that depends on users' actions and someone making a choice when hitting "publish"), there is no reason to pre-register a version PID that might never see the light of day. A version PID for a minor version will carry the PID of it's major revision, as it is a simple update. This will require changing the logic some more. --- .../impl/AbstractCreateDatasetCommand.java | 5 +---- .../impl/CreateDatasetVersionCommand.java | 16 ++-------------- 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AbstractCreateDatasetCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AbstractCreateDatasetCommand.java index daf5d7da55f..8f477a66424 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AbstractCreateDatasetCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AbstractCreateDatasetCommand.java @@ -81,7 +81,7 @@ public Dataset execute(CommandContext ctxt) throws CommandException { DatasetVersion dsv = getVersionToPersist(theDataset); // This re-uses the state setup logic of CreateDatasetVersionCommand, but // without persisting the new version, or altering its files. - new CreateDatasetVersionCommand(getRequest(), theDataset, dsv).prepareDatasetAndVersion(ctxt); + new CreateDatasetVersionCommand(getRequest(), theDataset, dsv).prepareDatasetAndVersion(); theDataset.setCreator((AuthenticatedUser) getRequest().getUser()); @@ -104,9 +104,6 @@ public Dataset execute(CommandContext ctxt) throws CommandException { String driverId = theDataset.getEffectiveStorageDriverId(); theDataset.setStorageIdentifier(driverId + DataAccess.SEPARATOR + theDataset.getAuthorityForFileStorage() + "/" + theDataset.getIdentifierForFileStorage()); } - - // TODO: why is this here? At the very start of this command execution we generate an identifier - why should - // it be gone now and if so - why would we simply create a new one? Wouldn't this be a major flaw? if (theDataset.getIdentifier()==null) { theDataset.setIdentifier(idServiceBean.generateDatasetIdentifier(theDataset)); } diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CreateDatasetVersionCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CreateDatasetVersionCommand.java index 2b4975f4dca..72439d4ba4a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CreateDatasetVersionCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CreateDatasetVersionCommand.java @@ -4,7 +4,6 @@ import edu.harvard.iq.dataverse.DatasetVersion; import edu.harvard.iq.dataverse.DatasetVersion.VersionState; import edu.harvard.iq.dataverse.FileMetadata; -import edu.harvard.iq.dataverse.GlobalIdServiceBean; import edu.harvard.iq.dataverse.authorization.Permission; import edu.harvard.iq.dataverse.engine.command.CommandContext; import edu.harvard.iq.dataverse.engine.command.DataverseRequest; @@ -58,7 +57,7 @@ public DatasetVersion execute(CommandContext ctxt) throws CommandException { //because it includes validation and we need the validation //to happen after file metdata is added to return a //good wrapped response if the TOA/Request Access not in compliance - prepareDatasetAndVersion(ctxt); + prepareDatasetAndVersion(); // TODO make async // ctxt.index().indexDataset(dataset); @@ -73,7 +72,7 @@ public DatasetVersion execute(CommandContext ctxt) throws CommandException { * * @throws CommandException */ - public void prepareDatasetAndVersion(CommandContext ctxt) throws CommandException { + public void prepareDatasetAndVersion() throws CommandException { newVersion.setDataset(dataset); newVersion.setDatasetFields(newVersion.initDatasetFields()); newVersion.setCreateTime(getTimestamp()); @@ -84,17 +83,6 @@ public void prepareDatasetAndVersion(CommandContext ctxt) throws CommandExceptio //had been stripped from the dataset fields prior to validation validateOrDie(newVersion, false); DatasetFieldUtil.tidyUpFields(newVersion.getDatasetFields(), true); - - /* If the owning Dataverse collection or global configuration requires it, we will try to generate - * a persistent identifier for this new dataset version. - * This implies when the generation style is "suffix", the dataset itself must already have an identifier. - * This is currently the case, as this method is called by the code that prepares new datasets - * and the identifier is generated before this. This (might) result in throwing an illegal argument exception. - */ - GlobalIdServiceBean globalIdServiceBean = GlobalIdServiceBean.getBean(dataset.getProtocol(), ctxt); - if (globalIdServiceBean != null && ctxt.dataverses().wantsDatasetVersionPids(dataset.getOwner())) { - newVersion.setPersistentIdentifier(globalIdServiceBean.generateDatasetVersionIdentifier(newVersion)); - } final List currentVersions = dataset.getVersions(); ArrayList dsvs = new ArrayList<>(currentVersions.size()); From 1eada8954c2b34f9780d9b877f736527e675fdde Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 26 Apr 2023 17:28:39 +0200 Subject: [PATCH 20/35] refactor(pid): replace exception for GlobalIdServiceBean.createIdentifier() So far, the method to create identifiers was defined in the interface to be throwing any Exception or other Throwable it likes to throw. This is a dark antipattern and should be avoided at all cost. With this change, the interface defines the method to throw IOExceptions, which is a much better description of what kind of errors to expect. Per definition, an I/O exception "signals that an I/O exception of some sort has occurred. This class is the general class of exceptions produced by failed or interrupted I/O operations." Which is exactly what happens if for some reason the communication between us and the PID provider goes sideways. It also adds Javadoc to the interface. Up until now, it was no where documented what the method will or has to return and what this method should and should not do when implemented for a provider. --- .../iq/dataverse/DOIDataCiteServiceBean.java | 2 +- .../iq/dataverse/DOIEZIdServiceBean.java | 6 +++-- .../iq/dataverse/GlobalIdServiceBean.java | 14 +++++++++-- .../iq/dataverse/HandlenetServiceBean.java | 25 +++++++++++-------- .../FakePidProviderServiceBean.java | 4 ++- .../PermaLinkPidProviderServiceBean.java | 3 ++- .../pidproviders/UnmanagedDOIServiceBean.java | 2 +- .../UnmanagedHandlenetServiceBean.java | 4 ++- .../AbstractGlobalIdServiceBeanTest.java | 3 ++- 9 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DOIDataCiteServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/DOIDataCiteServiceBean.java index c5d4faa0569..dc75b95374f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DOIDataCiteServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/DOIDataCiteServiceBean.java @@ -63,7 +63,7 @@ public boolean alreadyExists(GlobalId pid) { @Override - public String createIdentifier(DvObject dvObject) throws Exception { + public String createIdentifier(DvObject dvObject) throws IOException { logger.log(Level.FINE,"createIdentifier"); if(dvObject.getIdentifier() == null || dvObject.getIdentifier().isEmpty() ){ dvObject = generateIdentifier(dvObject); diff --git a/src/main/java/edu/harvard/iq/dataverse/DOIEZIdServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/DOIEZIdServiceBean.java index 5776aca8c8a..914b8af816d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DOIEZIdServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/DOIEZIdServiceBean.java @@ -3,6 +3,8 @@ import edu.harvard.iq.dataverse.settings.JvmSettings; import edu.ucsb.nceas.ezid.EZIDException; import edu.ucsb.nceas.ezid.EZIDService; + +import java.io.IOException; import java.util.*; import java.util.logging.Level; import java.util.logging.Logger; @@ -240,7 +242,7 @@ public List getProviderInformation(){ } @Override - public String createIdentifier(DvObject dvObject) throws Throwable { + public String createIdentifier(DvObject dvObject) throws IOException { logger.log(Level.FINE, "createIdentifier"); if(dvObject.getIdentifier() == null || dvObject.getIdentifier().isEmpty() ){ dvObject = generateIdentifier(dvObject); @@ -265,7 +267,7 @@ public String createIdentifier(DvObject dvObject) throws Throwable { logger.log(Level.WARNING, "cause", e.getCause()); logger.log(Level.WARNING, "message {0}", e.getMessage()); logger.log(Level.WARNING, "identifier: ", identifier); - throw e; + throw new IOException(e); } } diff --git a/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java index b73e4433138..38ac55cf704 100644 --- a/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java @@ -6,6 +6,7 @@ import edu.harvard.iq.dataverse.pidproviders.PidUtil; import edu.harvard.iq.dataverse.settings.SettingsServiceBean.Key; +import java.io.IOException; import java.util.*; import java.util.function.Function; import java.util.logging.Level; @@ -24,8 +25,17 @@ public interface GlobalIdServiceBean { boolean isConfigured(); List getProviderInformation(); - - String createIdentifier(DvObject dvo) throws Throwable; + + /** + * Let the PID provider create the identifier for a dataset or datafile (collections are not yet supported) by + * linking it to the target URL. This might involve registering metadata of the object alongside. + * The identifier is not yet to be published - this step is done by {@link #publicizeIdentifier(DatasetVersion)}. + * + * @param dvo The object to create the identifier for + * @return Some response or nothing at all (null). Up to the provider. + * @throws IOException If creation fails. May contain wrapped root causes. + */ + String createIdentifier(DvObject dvo) throws IOException; Map getIdentifierMetadata(DvObject dvo); diff --git a/src/main/java/edu/harvard/iq/dataverse/HandlenetServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/HandlenetServiceBean.java index 9ac4d5e29ae..e85408fd350 100644 --- a/src/main/java/edu/harvard/iq/dataverse/HandlenetServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/HandlenetServiceBean.java @@ -25,6 +25,7 @@ import java.io.File; import java.io.FileInputStream; +import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.*; import java.util.logging.Level; @@ -140,11 +141,17 @@ public void reRegisterHandle(DvObject dvObject) { } else { // Create a new handle from scratch: logger.log(Level.INFO, "Handle {0} not registered. Registering (creating) from scratch.", handle); - registerNewHandle(dvObject); + try { + registerNewHandle(dvObject); + // HINT: This is a hack. Before switch to throwing exceptions, the method above would have returned an + // exception, which would have been ignored. We're doing the same now by catching and handling it. + } catch (IOException e) { + logger.log(Level.WARNING, "RegisterNewHandle failed.", e); + } } } - public Throwable registerNewHandle(DvObject dvObject) { + public void registerNewHandle(DvObject dvObject) throws IOException { logger.log(Level.FINE,"registerNewHandle"); String handlePrefix = dvObject.getAuthority(); String handle = getDvObjectHandle(dvObject); @@ -180,14 +187,12 @@ public Throwable registerNewHandle(DvObject dvObject) { AbstractResponse response = resolver.processRequest(req); if (response.responseCode == AbstractMessage.RC_SUCCESS) { logger.log(Level.INFO, "Success! Response: \n{0}", response); - return null; } else { logger.log(Level.WARNING, "RegisterNewHandle failed. Error response: {0}", response); - return new Exception("registerNewHandle failed: " + response); + throw new IOException("registerNewHandle failed: " + response); } - } catch (Throwable t) { - logger.log(Level.WARNING, "registerNewHandle failed", t); - return t; + } catch (HandleException t) { + throw new IOException("registerNewHandle failed", t); } } @@ -388,10 +393,8 @@ public List getProviderInformation(){ @Override - public String createIdentifier(DvObject dvObject) throws Throwable { - Throwable result = registerNewHandle(dvObject); - if (result != null) - throw result; + public String createIdentifier(DvObject dvObject) throws IOException { + registerNewHandle(dvObject); // TODO get exceptions from under the carpet return getDvObjectHandle(dvObject); diff --git a/src/main/java/edu/harvard/iq/dataverse/pidproviders/FakePidProviderServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/pidproviders/FakePidProviderServiceBean.java index 68dd853d4de..99da3b8b762 100644 --- a/src/main/java/edu/harvard/iq/dataverse/pidproviders/FakePidProviderServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/pidproviders/FakePidProviderServiceBean.java @@ -3,6 +3,8 @@ import edu.harvard.iq.dataverse.DOIServiceBean; import edu.harvard.iq.dataverse.DvObject; import edu.harvard.iq.dataverse.GlobalId; + +import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -43,7 +45,7 @@ public List getProviderInformation() { } @Override - public String createIdentifier(DvObject dvo) throws Throwable { + public String createIdentifier(DvObject dvo) throws IOException { return "fakeIdentifier"; } diff --git a/src/main/java/edu/harvard/iq/dataverse/pidproviders/PermaLinkPidProviderServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/pidproviders/PermaLinkPidProviderServiceBean.java index 957522b7728..b5dfebd96f5 100644 --- a/src/main/java/edu/harvard/iq/dataverse/pidproviders/PermaLinkPidProviderServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/pidproviders/PermaLinkPidProviderServiceBean.java @@ -9,6 +9,7 @@ import edu.harvard.iq.dataverse.settings.SettingsServiceBean.Key; import edu.harvard.iq.dataverse.util.SystemConfig; +import java.io.IOException; import java.lang.StackWalker.StackFrame; import java.util.ArrayList; import java.util.HashMap; @@ -84,7 +85,7 @@ public List getProviderInformation() { } @Override - public String createIdentifier(DvObject dvo) throws Throwable { + public String createIdentifier(DvObject dvo) throws IOException { //Call external resolver and send landing URL? //FWIW: Return value appears to only be used in RegisterDvObjectCommand where success requires finding the dvo identifier in this string. (Also logged a couple places). return(dvo.getGlobalId().asString()); diff --git a/src/main/java/edu/harvard/iq/dataverse/pidproviders/UnmanagedDOIServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/pidproviders/UnmanagedDOIServiceBean.java index 088992fd3ec..5e3203a5156 100644 --- a/src/main/java/edu/harvard/iq/dataverse/pidproviders/UnmanagedDOIServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/pidproviders/UnmanagedDOIServiceBean.java @@ -48,7 +48,7 @@ public boolean alreadyExists(GlobalId pid) { } @Override - public String createIdentifier(DvObject dvObject) throws Exception { + public String createIdentifier(DvObject dvObject) throws IOException { throw new NotImplementedException(); } diff --git a/src/main/java/edu/harvard/iq/dataverse/pidproviders/UnmanagedHandlenetServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/pidproviders/UnmanagedHandlenetServiceBean.java index c467b8672ee..0890c021279 100644 --- a/src/main/java/edu/harvard/iq/dataverse/pidproviders/UnmanagedHandlenetServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/pidproviders/UnmanagedHandlenetServiceBean.java @@ -4,6 +4,8 @@ import edu.harvard.iq.dataverse.DvObject; import edu.harvard.iq.dataverse.GlobalId; import edu.harvard.iq.dataverse.HandlenetServiceBean; + +import java.io.IOException; import java.util.*; import java.util.logging.Level; import java.util.logging.Logger; @@ -60,7 +62,7 @@ public List getProviderInformation() { } @Override - public String createIdentifier(DvObject dvObject) throws Throwable { + public String createIdentifier(DvObject dvObject) throws IOException { throw new NotImplementedException(); } diff --git a/src/test/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBeanTest.java b/src/test/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBeanTest.java index a652fd0db1b..6cab8051123 100644 --- a/src/test/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBeanTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBeanTest.java @@ -8,6 +8,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.NullAndEmptySource; +import java.io.IOException; import java.util.List; import java.util.Map; @@ -141,7 +142,7 @@ public List getProviderInformation() { } @Override - public String createIdentifier(DvObject dvo) throws Throwable { + public String createIdentifier(DvObject dvo) throws IOException { return RandomStringUtils.randomAlphanumeric(idLen).toUpperCase(); } From 909b9f5fb7c27211d7b9b032dbbc6e1ddf91bb9b Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 2 May 2023 16:35:43 +0200 Subject: [PATCH 21/35] feat(pid): make DatasetVersion know about identifier registration status When setting an identifier for a particular dataset version, we might reuse a formerly used one, e.g. if it's a minor version. To know in the provider if some version's identifier has been registered already, we set this property. --- .../harvard/iq/dataverse/DatasetVersion.java | 30 +++++++++++++++++++ .../V5.13.0.1__4499-dataset-version-pids.sql | 2 ++ 2 files changed, 32 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java b/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java index faafc219584..d82581a0eb0 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java @@ -148,6 +148,12 @@ public enum VersionState { @Transient private GlobalId globalId; + /** + * Saving in the database if this identifier has been registered before to avoid + * re-registration (which would probably fail) and switch to modification + */ + private boolean identifierRegistered = false; + /* * @todo versionState should never be null so when we are ready, uncomment * the `nullable = false` below. @@ -278,6 +284,30 @@ public GlobalId getGlobalId() { } return this.globalId; } + + /** + * Check the status of the version identifier - has it been registered? + * @return True if registered, false otherwise. + */ + public boolean isIdentifierRegistered() { + return this.identifierRegistered; + } + + /** + * Set registration as done. + */ + public void setIdentifierRegistered() { + this.identifierRegistered = true; + } + + /** + * Overwrite the registration status with a specific value + * @param status The new status of the registration + */ + public void setIdentifierRegistered(boolean status) { + this.identifierRegistered = status; + } + public String getDataverseSiteUrl() { return dataverseSiteUrl; diff --git a/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql b/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql index 8267db2d2f5..1087ce62d99 100644 --- a/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql +++ b/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql @@ -4,3 +4,5 @@ ALTER TABLE datasetVersion ADD COLUMN IF NOT EXISTS persistentIdentifier varchar /* As Postgres does not support "if not exists" when adding a constraint, must remove first to make this not bail */ ALTER TABLE datasetVersion DROP CONSTRAINT IF EXISTS datasetversion_persistentidentifier_key; ALTER TABLE datasetVersion ADD CONSTRAINT datasetversion_persistentidentifier_key UNIQUE(persistentIdentifier); + +ALTER TABLE datasetVersion ADD COLUMN IF NOT EXISTS identifierRegistered bool NOT NULL DEFAULT false; From 4ecaa09552dcd381f835dd6dacea3bc5f7849a25 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 2 May 2023 16:52:42 +0200 Subject: [PATCH 22/35] refactor(cmd): add NotImplementedException By adding a NotImplementedException, a subclass of UnsupportedOperationException, we can describe methods not yet implemented. This status might change in the future (or not). The important part about this introduction: this runtime exception is flagged as an "application exception", which makes the EJB exception inspection not handle it as a system exception (which is the default), resulting in rolling back transactions. The idea is to enable handling perfectly fine situations within the command engine when some other component of Dataverse - e.g. the EJB of a PID provider - throws this exception. This way we can catch the exception and deal e.g. with dataset unlocks. --- .../exception/NotImplementedException.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 src/main/java/edu/harvard/iq/dataverse/engine/command/exception/NotImplementedException.java diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/exception/NotImplementedException.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/exception/NotImplementedException.java new file mode 100644 index 00000000000..1879d113bd1 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/exception/NotImplementedException.java @@ -0,0 +1,23 @@ +package edu.harvard.iq.dataverse.engine.command.exception; + +import javax.ejb.ApplicationException; + + +/** + * This is a copycat of the Apache Commons Lang3 exception but with an EJB annotation to make this runtime ("unchecked") + * exception not fail on the remote end of an EJB, but catchable in the client (the commands committed to the engine). + * By using this exception, the transaction is not yet rolled back, and we can react to events in other components + * within the command (it might not be necessary to rollback, etc). + * + * @apiNote This exception should be added to a future Dataverse Java API module + */ +@ApplicationException +public class NotImplementedException extends UnsupportedOperationException { + public NotImplementedException() { + super(); + } + + public NotImplementedException(String message) { + super(message); + } +} From 4eac492d302fea4b83b8552881794d8be5e977c6 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 2 May 2023 16:53:48 +0200 Subject: [PATCH 23/35] fix(pid): simple typo in javadoc of GlobalIdServiceBean.createIdentifier() --- src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java index 38ac55cf704..a0d7e47e7be 100644 --- a/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java @@ -29,7 +29,7 @@ public interface GlobalIdServiceBean { /** * Let the PID provider create the identifier for a dataset or datafile (collections are not yet supported) by * linking it to the target URL. This might involve registering metadata of the object alongside. - * The identifier is not yet to be published - this step is done by {@link #publicizeIdentifier(DatasetVersion)}. + * The identifier is not yet to be published - this step is done by {@link #publicizeIdentifier(DvObject)}. * * @param dvo The object to create the identifier for * @return Some response or nothing at all (null). Up to the provider. From 6b891aa8308c1f8eada7fece5f2a6eb542678da1 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 2 May 2023 16:56:43 +0200 Subject: [PATCH 24/35] feat(pid): restructure version identifier related methods in GlobalIdServiceBean With the addition of NotImplementedException we can better express what's going on without rollback exceptions in EJB. The methods that will reach out to some provider now throw checked IOExceptions, as the communication might go sideways and the command engine needs to act accordingly. (This is the same behaviour now as for the normal object methods) Also extending JavaDoc descriptions and expectations a bit. --- .../iq/dataverse/GlobalIdServiceBean.java | 37 +++++++++++-------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java index a0d7e47e7be..0f1f3ab5a21 100644 --- a/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java @@ -2,6 +2,7 @@ import static edu.harvard.iq.dataverse.GlobalIdServiceBean.logger; import edu.harvard.iq.dataverse.engine.command.CommandContext; +import edu.harvard.iq.dataverse.engine.command.exception.NotImplementedException; import edu.harvard.iq.dataverse.pidproviders.PermaLinkPidProviderServiceBean; import edu.harvard.iq.dataverse.pidproviders.PidUtil; import edu.harvard.iq.dataverse.settings.SettingsServiceBean.Key; @@ -54,40 +55,46 @@ public interface GlobalIdServiceBean { boolean publicizeIdentifier(DvObject studyIn); /** - * Publish a PID for a given {@link DatasetVersion}. + * Publish a PID for a given {@link DatasetVersion} and make it findable and resolvable. * - * @apiNote This method is meant to be called when a new version of a dataset is being published. + * @apiNote This method is meant to be called when a new version identifier is about to be published which is + * already created - either by calling {@link #createIdentifier(DatasetVersion)} before or knowing this + * will be an update of an existing one. * * @param datasetVersion The version to publish * @return true if successful, false otherwise (or when datasetVersion is null) - * @throws IllegalArgumentException When a provider does not support generating these PIDs, does not allow their - * generation due to configuration or doesn't like the current look of the version - * (i.e. not being a new major version or not having an identifier set). + * @throws IOException In case the communication with the provider failed for some reason. + * @throws NotImplementedException When a provider does not support PIDs for versions */ - default boolean publicizeIdentifier(final DatasetVersion datasetVersion) { - throw new IllegalArgumentException("This provider does not (yet) support publishing versions."); + default boolean publicizeIdentifier(final DatasetVersion datasetVersion) throws IOException { + throw new NotImplementedException("This provider does not (yet) support publishing versions."); } String generateDatasetIdentifier(Dataset dataset); String generateDataFileIdentifier(DataFile datafile); /** - * Generate a new PID for a {@link DatasetVersion}. - * + * Generate a PID for a {@link DatasetVersion}. * Note that the generation of this identifier depends on configuration by a sysadmin and concrete * implementation for a given PID provider (it might be limited by its capabilities). + * The provider may return an existing PID of a former version, e.g. to reuse an existing identifier + * in case of minor version updates. * - * @implNote This method is meant to be implemented free of side effects. + * @implNote This method is meant to be implemented free of side effects (not manipulating the version). + * Take care not to throw other exception than those documented here to avoid EJB exception handling + * kicking in. * * @param datasetVersion The version of a dataset to create a PID for * @return An "identifier", meant to be used for {@link GlobalId}, retrievable via {@link GlobalId#getIdentifier()}. * Must not be null. - * @throws IllegalArgumentException When a provider does not support generating these PIDs, does not allow their - * generation due to configuration or doesn't like the current look of the version - * (i.e. not being a new major version). + * @throws NotImplementedException When a provider does not support generating PIDs for version. + * @throws IllegalArgumentException When a provider does not allow their generation due to configuration or doesn't + * like the current look of the version (i.e. not being a new major version). + * Note: this is made a checked exception to make it a business exception, + * avoiding EJB exception handling and handling inside command engine. */ - default String generateDatasetVersionIdentifier(final DatasetVersion datasetVersion) { - throw new IllegalArgumentException("This provider does not (yet) support publishing versions."); + default String generateDatasetVersionIdentifier(final DatasetVersion datasetVersion) throws IllegalArgumentException { + throw new NotImplementedException("This provider does not (yet) support publishing versions."); } /** From 3c18c73833e310e5ba08822080212f506afdedf4 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 2 May 2023 17:33:48 +0200 Subject: [PATCH 25/35] feat(pid): restructure default version identifier generation Now returning the same identifier for a minor version that has been assigned to the adjacent major version. As before, this can be changed by a provider. The tests have been changed accordingly. --- .../AbstractGlobalIdServiceBean.java | 43 +++++++++++-------- .../AbstractGlobalIdServiceBeanTest.java | 22 +++++++--- 2 files changed, 41 insertions(+), 24 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBean.java index bbf433abc67..0875bab1315 100644 --- a/src/main/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBean.java @@ -163,33 +163,42 @@ public DvObject generateIdentifier(DvObject dvObject) { * * @param datasetVersion The version of a dataset to create a PID for * @return The identifier (will never be null) - * @throws NoSuchElementException When no style has been configured for the instance * @throws IllegalArgumentException If the style configured is not supported by this generator, the version is * already released or a minor version, or if the owning dataset has no identifier * while creating a suffix style identifier. */ @Override - public String generateDatasetVersionIdentifier(DatasetVersion datasetVersion) { - // HINT: this default business logic might be made configurable later - // or can be overridden by a provider implementing custom logic. - if (datasetVersion.isReleased() || datasetVersion.getMinorVersionNumber() > 0) { - throw new IllegalArgumentException("Released or draft minor versions are not allowed"); + public String generateDatasetVersionIdentifier(final DatasetVersion datasetVersion) throws IllegalArgumentException { + if (datasetVersion == null || datasetVersion.isReleased()) { + throw new IllegalArgumentException("Version may not be null or released"); } - GenStyle style = JvmSettings.PID_VERSIONS_STYLE.lookup(GenStyle.class); + // If this is a minor version update, reuse the identifier of the last released version + if (datasetVersion.getMinorVersionNumber() > 0) { + return datasetVersion.getDataset().getReleasedVersion().getPersistentIdentifier(); + } - if (style == GenStyle.DATASET) { - return generateDatasetIdentifier(datasetVersion.getDataset()); - } else if (style == GenStyle.SUFFIX) { - String datasetIdentifier = datasetVersion.getDataset().getIdentifier(); - if (datasetIdentifier == null || datasetIdentifier.isEmpty()) { - throw new IllegalArgumentException("Dataset must not have empty identifier when creating dataset version identifier by suffix"); - } + try { + GenStyle style = JvmSettings.PID_VERSIONS_STYLE.lookup(GenStyle.class); + + if (style == GenStyle.DATASET) { + return generateDatasetIdentifier(datasetVersion.getDataset()); + + } else if (style == GenStyle.SUFFIX) { + String datasetIdentifier = datasetVersion.getDataset().getIdentifier(); + if (datasetIdentifier == null || datasetIdentifier.isEmpty()) { + throw new IllegalArgumentException("Dataset must not have empty identifier when creating dataset version identifier by suffix"); + } + + return datasetIdentifier + getVersionSuffixDelimiter() + datasetVersion.getVersionNumber(); - return datasetIdentifier + getVersionSuffixDelimiter() + datasetVersion.getVersionNumber(); + // Nothing appropriate found - bail out + } else { + throw new IllegalArgumentException("No supported version PID generation style configured"); + } + } catch (NoSuchElementException e) { + throw new IllegalArgumentException("No supported version PID generation style configured", e); } - - throw new IllegalArgumentException("No supported version PID generation style configured"); } //ToDo just send the DvObject.DType diff --git a/src/test/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBeanTest.java b/src/test/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBeanTest.java index 6cab8051123..c89def770bc 100644 --- a/src/test/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBeanTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/AbstractGlobalIdServiceBeanTest.java @@ -9,6 +9,7 @@ import org.junit.jupiter.params.provider.NullAndEmptySource; import java.io.IOException; +import java.util.Date; import java.util.List; import java.util.Map; @@ -73,19 +74,26 @@ void generateDatasetVersionIdentifierWithStyleSuffix() { } @Test - void generateDatasetVersionIdentifierFailsWithMinorVersion() { + void generateDatasetVersionIdentifierReturnsReleasedIdWithMinorVersion() { // given TestIdService sut = new TestIdService(); Dataset dataset = MocksFactory.makeDataset(); - + + // Release the first version + dataset.getLatestVersion().setReleaseTime(new Date()); + dataset.getLatestVersion().setVersionState(DatasetVersion.VersionState.RELEASED); + dataset.getLatestVersion().setPersistentIdentifier("test"); + assumeTrue(dataset.getLatestVersion().isReleased()); + + // Add a new minor version + DatasetVersion newVersion = dataset.getOrCreateEditVersion(); assumeFalse(dataset.getLatestVersion().isReleased()); - - // Set to non-allowed combination dataset.getLatestVersion().setMinorVersionNumber(1L); + assumeTrue(newVersion.getMinorVersionNumber() == 1); - // when & then (split retrieval to avoid ambiguous lambda exceptions) - DatasetVersion version = dataset.getLatestVersion(); - assertThrows(IllegalArgumentException.class, () -> sut.generateDatasetVersionIdentifier(version)); + // when + String identifier = sut.generateDatasetVersionIdentifier(dataset.getLatestVersion()); + assertEquals("test", identifier); } @Test From 3c1433efe47c5ad10d4402c378b927a38a8cd2bc Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 2 May 2023 17:36:11 +0200 Subject: [PATCH 26/35] feat(pid): add create identifier interface in GlobalIdServiceBean for versions Identifiers for versions are done in two steps: 1. create it (if not existing) and 2. make it findable. The second step is done by GlobalIdServiceBean.publicizeIdentifier, and this commit now adds the interface method for the first step. --- .../iq/dataverse/GlobalIdServiceBean.java | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java index 0f1f3ab5a21..89b537e17f4 100644 --- a/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/GlobalIdServiceBean.java @@ -37,7 +37,25 @@ public interface GlobalIdServiceBean { * @throws IOException If creation fails. May contain wrapped root causes. */ String createIdentifier(DvObject dvo) throws IOException; - + + /** + * Let the PID provider create the identifier for a dataset version like it does in {@link #createIdentifier(DvObject)}. + * The identifier is not yet to be published - this step is done by {@link #publicizeIdentifier(DatasetVersion)}. + * Note that the provider needs to decide if minor versions get their own identifiers as well. If minor versions + * are to be updates of majors, this may be done in {@link #publicizeIdentifier(DatasetVersion)} as well. + * + * @implNote This method is expected to create a new identifier, not overwrite an existing one, and not make + * an identifier findable yet (this is the job of {@link #publicizeIdentifier(DatasetVersion)}). + * + * @param datasetVersion The version to be registered at the provider + * @return Some response or nothing at all (null). Up to the provider. + * @throws IOException If creation fails. May contain wrapped root causes. + * @throws NotImplementedException If version registration is not supported by the provider + */ + default String createIdentifier(DatasetVersion datasetVersion) throws IOException { + throw new NotImplementedException("This provider does not (yet) support creating identifiers for versions"); + } + Map getIdentifierMetadata(DvObject dvo); String modifyIdentifierTargetURL(DvObject dvo) throws Exception; From afc23d133b82e18f0c1510731dbbdcf19c44ed3b Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 2 May 2023 17:36:45 +0200 Subject: [PATCH 27/35] feat(pid): add fake version identifier methods to FakePidProviderServiceBean --- .../pidproviders/FakePidProviderServiceBean.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/pidproviders/FakePidProviderServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/pidproviders/FakePidProviderServiceBean.java index 99da3b8b762..f872af53e19 100644 --- a/src/main/java/edu/harvard/iq/dataverse/pidproviders/FakePidProviderServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/pidproviders/FakePidProviderServiceBean.java @@ -1,6 +1,7 @@ package edu.harvard.iq.dataverse.pidproviders; import edu.harvard.iq.dataverse.DOIServiceBean; +import edu.harvard.iq.dataverse.DatasetVersion; import edu.harvard.iq.dataverse.DvObject; import edu.harvard.iq.dataverse.GlobalId; @@ -74,5 +75,15 @@ public boolean publicizeIdentifier(DvObject studyIn) { protected String getProviderKeyName() { return "FAKE"; } - + + + @Override + public String createIdentifier(DatasetVersion datasetVersion) throws IOException { + return "fakeVersionIdentifier"; + } + + @Override + public boolean publicizeIdentifier(DatasetVersion datasetVersion) throws IOException { + return true; + } } From d0bf0fdd86be125c17511fe8a201112e254aa3ec Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 2 May 2023 18:01:01 +0200 Subject: [PATCH 28/35] feat(pid,cmd): create and publish version identifiers on dataset publication With this commit, a first draft to create and release a version identifier from the command engine is added. FinalizeDatasetPublicationCommand is used after someone hit publish in the UI and currently takes care of the creation and publishing for dataset identifiers and files, now extended to look after version PIDs as well. --- .../FinalizeDatasetPublicationCommand.java | 60 +++++++++++++------ 1 file changed, 42 insertions(+), 18 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/FinalizeDatasetPublicationCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/FinalizeDatasetPublicationCommand.java index d3374902069..7c3c3002776 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/FinalizeDatasetPublicationCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/FinalizeDatasetPublicationCommand.java @@ -7,6 +7,7 @@ import edu.harvard.iq.dataverse.DatasetFieldConstant; import edu.harvard.iq.dataverse.DatasetLock; import static edu.harvard.iq.dataverse.DatasetVersion.VersionState.*; +import edu.harvard.iq.dataverse.DatasetVersion; import edu.harvard.iq.dataverse.DatasetVersionUser; import edu.harvard.iq.dataverse.Dataverse; import edu.harvard.iq.dataverse.DvObject; @@ -18,6 +19,7 @@ import edu.harvard.iq.dataverse.engine.command.DataverseRequest; 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.NotImplementedException; import edu.harvard.iq.dataverse.export.ExportService; import edu.harvard.iq.dataverse.privateurl.PrivateUrl; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; @@ -27,6 +29,7 @@ import java.sql.Timestamp; import java.util.Date; import java.util.List; +import java.util.Objects; import java.util.logging.Level; import java.util.logging.Logger; import edu.harvard.iq.dataverse.GlobalIdServiceBean; @@ -37,9 +40,6 @@ import java.util.concurrent.Future; import org.apache.solr.client.solrj.SolrServerException; -import javax.ejb.EJB; -import javax.inject.Inject; - /** * @@ -101,10 +101,7 @@ public Dataset execute(CommandContext ctxt) throws CommandException { try { // This can potentially throw a CommandException, so let's make // sure we exit cleanly: - - registerExternalIdentifier(theDataset, ctxt, false); - - // TODO: try to register an identifier for the version as well if necessary + registerExternalIdentifier(theDataset, ctxt, false); } catch (CommandException comEx) { logger.warning("Failed to reserve the identifier "+theDataset.getGlobalId().asString()+"; notifying the user(s), unlocking the dataset"); // Send failure notification to the user: @@ -182,10 +179,42 @@ public Dataset execute(CommandContext ctxt) throws CommandException { ctxt.engine().submit(new DeletePrivateUrlCommand(getRequest(), theDataset)); } - if (theDataset.getLatestVersion().getVersionState() != RELEASED) { + if (theDataset.getLatestVersion().getVersionState() != RELEASED) { // some imported datasets may already be released. - if (!datasetExternallyReleased) { + /* If activated, now is the right moment to create an identifier for this version, as the + * metadata has been all updated. In PublishDatasetCommand (which comes before this command), + * the numbers have already been adapted, so we know if this will be a major or minor version. + * The identifier for the dataset has definitely been created (or this would have failed above), + * so we can include it in any metadata records for this version. + * + * Note: although it may seem wrong to trigger registering any version, it is a (configurable?) + * decision in the provider's realm if minor versions are just updates of a major or their own. + */ + if (ctxt.dataverses().wantsDatasetVersionPids(theDataset.getOwner())) { + DatasetVersion version = theDataset.getLatestVersion(); + GlobalIdServiceBean idServiceBean = GlobalIdServiceBean.getBean(theDataset.getProtocol(), ctxt); + Objects.requireNonNull(idServiceBean, "Could not retrieve PID provider"); + + try { + // Generate an identifier for the version + String identifier = idServiceBean.generateDatasetVersionIdentifier(version); + version.setPersistentIdentifier(identifier); + + // Try to register the identifier with the provider + idServiceBean.createIdentifier(version); + version.setIdentifierRegistered(); + } catch (IOException | NotImplementedException e) { + logger.warning("Failed to register the version identifier "+version.getGlobalId().asString()+"; notifying the user(s), unlocking the dataset"); + + // Send failure notification to the user: + notifyUsersDatasetPublishStatus(ctxt, theDataset, UserNotification.Type.PUBLISHFAILED_PIDREG); + + ctxt.datasets().removeDatasetLocks(theDataset, DatasetLock.Reason.finalizePublication); + throw new CommandException(BundleUtil.getStringFromBundle("dataset.publish.error", idServiceBean.getProviderInformation()), this); + } + } + publicizeExternalIdentifier(theDataset, ctxt); // Will throw a CommandException, unless successful. // This will end the execution of the command, but the method @@ -404,18 +433,13 @@ private void publicizeExternalIdentifier(Dataset dataset, CommandContext ctxt) t } } - // Publish a PID for this dataset version if activated - // -> At this point, we have a latest version that is not in "released" state yet. - // -> - // TODO: might throw a NoSuchElementException. Adapt error message? + // Publish a PID for this dataset version (if activated). Leave details to the provider. if (ctxt.dataverses().wantsDatasetVersionPids(dataset.getOwner())) { - // TODO: check result like above - // TODO: which version to pass? is at time of calling the latest version already released? - // TODO: maybe we should check for this being a *new* major version before handing it over... (this is not meant to be at discretion of the id service!) - //idServiceBean.publicizeIdentifier(dataset.getLatestVersion()); + DatasetVersion version = dataset.getLatestVersion(); + idServiceBean.publicizeIdentifier(version); } - // Publish the Dataset PID (after the version because we want to include the version in metadata updates) + // Publish the Dataset PID (after the version, because we want to include the version in metadata updates) if (!idServiceBean.publicizeIdentifier(dataset)) { throw new Exception(); } From 288e987102cbe0302d6e413c01cfaabba0cc30e0 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 2 May 2023 18:04:36 +0200 Subject: [PATCH 29/35] fix(pid): make DatasetVersion.persistentIdentifier non-unique in database As minor versions might carry the identifier of their major version, we cannot set a unique constraint on the column. Removing this from the model as well as the Flyway migration. --- src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java | 7 ++++--- .../db/migration/V5.13.0.1__4499-dataset-version-pids.sql | 3 --- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java b/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java index d82581a0eb0..cffa046138b 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java @@ -129,15 +129,16 @@ public enum VersionState { private String versionNote; /** - * A (globally) unique persisten identifier for this version. + * A (globally) unique persistent identifier for this version. * The version PID will always be dependent on the protocol and authority of the containing dataset. * This identifier may contain {@link edu.harvard.iq.dataverse.settings.SettingsServiceBean.Key#Shoulder} if * configured and some more unique characters, also depending on the admin's choice to make version PIDs dependent * on the dataset PID. * - * The PID may be null (feature disabled, old entry, etc), but if present, it must be unique. + * The PID may be null (feature disabled, old entry, etc.). It might not be unique, as minor versions by default + * carry the identifier of their adjacent major version. */ - @Column(unique = true) + @Column private String persistentIdentifier; /** diff --git a/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql b/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql index 1087ce62d99..a5dccb57542 100644 --- a/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql +++ b/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql @@ -1,8 +1,5 @@ ALTER TABLE dataverse ADD COLUMN IF NOT EXISTS datasetVersionPidConduct varchar(16) NOT NULL DEFAULT 'INHERIT'; ALTER TABLE datasetVersion ADD COLUMN IF NOT EXISTS persistentIdentifier varchar(255); -/* As Postgres does not support "if not exists" when adding a constraint, must remove first to make this not bail */ -ALTER TABLE datasetVersion DROP CONSTRAINT IF EXISTS datasetversion_persistentidentifier_key; -ALTER TABLE datasetVersion ADD CONSTRAINT datasetversion_persistentidentifier_key UNIQUE(persistentIdentifier); ALTER TABLE datasetVersion ADD COLUMN IF NOT EXISTS identifierRegistered bool NOT NULL DEFAULT false; From 877c728b6480bf952fc357864268119dba51fa14 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 14 Jun 2023 10:42:44 +0200 Subject: [PATCH 30/35] fix(db): rename version PID db migration because of conflict and not being included in next release --- ...-version-pids.sql => V5.14.0.1__4499-dataset-version-pids.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/main/resources/db/migration/{V5.13.0.1__4499-dataset-version-pids.sql => V5.14.0.1__4499-dataset-version-pids.sql} (100%) diff --git a/src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql b/src/main/resources/db/migration/V5.14.0.1__4499-dataset-version-pids.sql similarity index 100% rename from src/main/resources/db/migration/V5.13.0.1__4499-dataset-version-pids.sql rename to src/main/resources/db/migration/V5.14.0.1__4499-dataset-version-pids.sql From c17cf27abfd2723cd5cbd61e941901f920648e3b Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 14 Jun 2023 12:58:18 +0200 Subject: [PATCH 31/35] refactor(pid): align collection conduct to be more aligned to JPA Instead of providing an initialized value and always saving a value, make the model itself return "inherit" as default when the DB value is null. This is better for backward compat and searches. --- .../java/edu/harvard/iq/dataverse/Dataverse.java | 13 ++++++++----- .../V5.14.0.1__4499-dataset-version-pids.sql | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java index 3ee68d17696..25aab8ba2db 100644 --- a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java +++ b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java @@ -594,20 +594,23 @@ public void setCitationDatasetFieldTypes(List citationDatasetF * Indicate if this Dataverse Collection wants to publicize PIDs for each (major) {@link DatasetVersion} * for any {@link Dataset} in it. * - * @see edu.harvard.iq.dataverse.pidproviders.VersionPidMode#COLLECTION + * @see edu.harvard.iq.dataverse.pidproviders.VersionPidMode#ALLOW_MAJOR + * @see edu.harvard.iq.dataverse.pidproviders.VersionPidMode#ALLOW_MINOR * @see CollectionConduct */ - @NotNull - @Column(nullable = false) @Enumerated(EnumType.STRING) - private CollectionConduct datasetVersionPidConduct = CollectionConduct.INHERIT; + private CollectionConduct datasetVersionPidConduct; public void setDatasetVersionPidConduct(CollectionConduct conduct) { this.datasetVersionPidConduct = conduct; } + /** + * Retrieve the version PID conduct mode for this collection + * @return One of {@link CollectionConduct}. Never null, defaults to {@link CollectionConduct#INHERIT}. + */ public CollectionConduct getDatasetVersionPidConduct() { - return this.datasetVersionPidConduct; + return this.datasetVersionPidConduct != null ? this.datasetVersionPidConduct : CollectionConduct.INHERIT; } diff --git a/src/main/resources/db/migration/V5.14.0.1__4499-dataset-version-pids.sql b/src/main/resources/db/migration/V5.14.0.1__4499-dataset-version-pids.sql index a5dccb57542..36a335160da 100644 --- a/src/main/resources/db/migration/V5.14.0.1__4499-dataset-version-pids.sql +++ b/src/main/resources/db/migration/V5.14.0.1__4499-dataset-version-pids.sql @@ -1,4 +1,4 @@ -ALTER TABLE dataverse ADD COLUMN IF NOT EXISTS datasetVersionPidConduct varchar(16) NOT NULL DEFAULT 'INHERIT'; +ALTER TABLE dataverse ADD COLUMN IF NOT EXISTS datasetVersionPidConduct varchar(16); ALTER TABLE datasetVersion ADD COLUMN IF NOT EXISTS persistentIdentifier varchar(255); From 77c9e5a344b18af6ffacf519956118bd64c7e0a8 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 14 Jun 2023 13:01:41 +0200 Subject: [PATCH 32/35] refactor(pid): align global and collection modes to review feedback See https://github.com/IQSS/dataverse/pull/9462#issuecomment-1532079229 See https://github.com/IQSS/dataverse/pull/9462#issuecomment-1543521998 --- .../pidproviders/VersionPidMode.java | 27 ++++++++++--------- .../pidproviders/VersionPidModeTest.java | 8 +++--- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java b/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java index 94d1ab6040a..96d8c0c8ed3 100644 --- a/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java +++ b/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java @@ -17,32 +17,35 @@ public final class VersionPidMode { /** * Means feature is switched off, no version PIDs will be minted */ - public static final VersionPidMode OFF = new VersionPidMode("OFF"); + public static final VersionPidMode OFF = new VersionPidMode("off"); /** - * Means the feature is activated instance wide for all collections and datasets - * (No opt-out so far!) + * Means the feature is activated, but instance wide for all collections and datasets + * only major versions can have a PID. Enabling for minor versions is prohibited. */ - public static final VersionPidMode GLOBAL = new VersionPidMode("GLOBAL"); + public static final VersionPidMode ALLOW_MAJOR = new VersionPidMode("allow-major"); /** - * Means the feature must be activated per Dataverse Collection (opt-in) + * Means the feature is activated and any collection may go for PIDs assigned to major and/or minor versions. */ - public static final VersionPidMode COLLECTION = new VersionPidMode("COLLECTION"); + public static final VersionPidMode ALLOW_MINOR = new VersionPidMode("allow-minor"); /** - * A collection of conducts for mode {@link #COLLECTION}, used in {@link edu.harvard.iq.dataverse.Dataverse} + * A collection of conducts for Dataverse collections, used in {@link edu.harvard.iq.dataverse.Dataverse} * and {@link edu.harvard.iq.dataverse.DataverseServiceBean#wantsDatasetVersionPids(Dataverse)}: *
    *
  1. Collection may inherit version pid behaviour from the parent collection(s),
    2. - *
  2. Collection may choose to be actively enabling it
    3. - *
  3. Collection may choose to opt out and skip the minting
    4. + *
  4. collection may choose to opt out and skip the minting,
    5. + *
  5. collection may choose to activate it for major versions only, or
    6. + *
  6. collection may choose to activate it for major and minor versions.
    7. *
+ * Note that the instance wide configuration will limit available choices. */ public enum CollectionConduct { INHERIT("inherit"), - ACTIVE("active"), - SKIP("skip"); + SKIP("skip"), + MAJOR("major"), + MINOR("minor"); private final String name; @@ -107,7 +110,7 @@ public static GenStyle of(String style) { // Init as unmodifiable set public static final Set values; static { - values = Set.of(OFF, GLOBAL, COLLECTION); + values = Set.of(OFF, ALLOW_MAJOR, ALLOW_MINOR); } private final String mode; diff --git a/src/test/java/edu/harvard/iq/dataverse/pidproviders/VersionPidModeTest.java b/src/test/java/edu/harvard/iq/dataverse/pidproviders/VersionPidModeTest.java index f6ad1018a86..37b57348906 100644 --- a/src/test/java/edu/harvard/iq/dataverse/pidproviders/VersionPidModeTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/pidproviders/VersionPidModeTest.java @@ -10,15 +10,15 @@ class VersionPidModeTest { @Test - @JvmSetting(key = JvmSettings.PID_VERSIONS_MODE, value = "collection") + @JvmSetting(key = JvmSettings.PID_VERSIONS_MODE, value = "allow-minor") void setToValidValue() { - assertEquals(VersionPidMode.COLLECTION, JvmSettings.PID_VERSIONS_MODE.lookup(VersionPidMode.class)); + assertEquals(VersionPidMode.ALLOW_MINOR, JvmSettings.PID_VERSIONS_MODE.lookup(VersionPidMode.class)); } @Test - @JvmSetting(key = JvmSettings.PID_VERSIONS_MODE, value = "GloBal") + @JvmSetting(key = JvmSettings.PID_VERSIONS_MODE, value = "ALLOW-major") void setToOtherValidValue() { - assertEquals(VersionPidMode.GLOBAL, JvmSettings.PID_VERSIONS_MODE.lookup(VersionPidMode.class)); + assertEquals(VersionPidMode.ALLOW_MAJOR, JvmSettings.PID_VERSIONS_MODE.lookup(VersionPidMode.class)); } @Test From 0b51e1b1afb2b7635cf705c1bc07404b36c43a9a Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 14 Jun 2023 18:53:25 +0200 Subject: [PATCH 33/35] feat(pid): update DataverseServiceBean.wantsDatasetVersionPids() calls with info about minor or major version --- .../command/impl/FinalizeDatasetPublicationCommand.java | 8 +++++--- .../engine/command/impl/RegisterDvObjectCommand.java | 4 +++- .../command/impl/UpdateDvObjectPIDMetadataCommand.java | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/FinalizeDatasetPublicationCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/FinalizeDatasetPublicationCommand.java index f06748c2e8b..9d69acf3813 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/FinalizeDatasetPublicationCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/FinalizeDatasetPublicationCommand.java @@ -191,8 +191,9 @@ public Dataset execute(CommandContext ctxt) throws CommandException { * Note: although it may seem wrong to trigger registering any version, it is a (configurable?) * decision in the provider's realm if minor versions are just updates of a major or their own. */ - if (ctxt.dataverses().wantsDatasetVersionPids(theDataset.getOwner())) { - DatasetVersion version = theDataset.getLatestVersion(); + DatasetVersion version = theDataset.getLatestVersion(); + + if (ctxt.dataverses().wantsDatasetVersionPids(theDataset.getOwner(), version.getMinorVersionNumber() > 0)) { GlobalIdServiceBean idServiceBean = GlobalIdServiceBean.getBean(theDataset.getProtocol(), ctxt); Objects.requireNonNull(idServiceBean, "Could not retrieve PID provider"); @@ -434,7 +435,8 @@ private void publicizeExternalIdentifier(Dataset dataset, CommandContext ctxt) t } // Publish a PID for this dataset version (if activated). Leave details to the provider. - if (ctxt.dataverses().wantsDatasetVersionPids(dataset.getOwner())) { + + if (ctxt.dataverses().wantsDatasetVersionPids(dataset.getOwner(), dataset.getLatestVersion().getMinorVersionNumber() > 0)) { DatasetVersion version = dataset.getLatestVersion(); idServiceBean.publicizeIdentifier(version); } diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/RegisterDvObjectCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/RegisterDvObjectCommand.java index 7929b5b51ff..9584573f2c3 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/RegisterDvObjectCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/RegisterDvObjectCommand.java @@ -86,7 +86,9 @@ protected void executeImpl(CommandContext ctxt) throws CommandException { if (target.isReleased()) { // If this is a dataset, release a new dataset version first before, to be included in the dataset if (target.isInstanceofDataset() && - ctxt.dataverses().wantsDatasetVersionPids((Dataverse) target.getOwner())) { + ctxt.dataverses().wantsDatasetVersionPids( + (Dataverse) target.getOwner(), + ((Dataset) target).getLatestVersion().getMinorVersionNumber() > 0)) { // TODO: publicize dataset version as well } diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/UpdateDvObjectPIDMetadataCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/UpdateDvObjectPIDMetadataCommand.java index bcda9aae507..7353a383c8d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/UpdateDvObjectPIDMetadataCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/UpdateDvObjectPIDMetadataCommand.java @@ -50,7 +50,7 @@ protected void executeImpl(CommandContext ctxt) throws CommandException { GlobalIdServiceBean idServiceBean = GlobalIdServiceBean.getBean(target.getProtocol(), ctxt); try { // First publicize new dataset version if enabled, so it can be included in the update of the dataset - if (ctxt.dataverses().wantsDatasetVersionPids(target.getOwner())) { + if (ctxt.dataverses().wantsDatasetVersionPids(target.getOwner(), target.getLatestVersion().getMinorVersionNumber() > 0)) { // TODO: publicize dataset version as well } From fa8d544e86d8ccf314d0695f177153248765033a Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 14 Jun 2023 19:01:48 +0200 Subject: [PATCH 34/35] feat(pid): change business logic for DataverseServiceBean.wantsDatasetVersionPids - Incorporate feedback from first review - Switch to model where admin sets limits but the collections can override but not exceed the limit - Use admin settings as defaults if collection chain does not provide a choice now - Also add unit testing for business logic See also https://github.com/IQSS/dataverse/pull/9462#issuecomment-1532079229 See also https://github.com/IQSS/dataverse/pull/9462#issuecomment-1543521998 --- .../iq/dataverse/DataverseServiceBean.java | 72 +++++++---- .../dataverse/DataverseServiceBeanTest.java | 119 ++++++++++++++++++ 2 files changed, 164 insertions(+), 27 deletions(-) create mode 100644 src/test/java/edu/harvard/iq/dataverse/DataverseServiceBeanTest.java diff --git a/src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java index 56939fb7837..7fd53cf6a0d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java @@ -30,6 +30,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.logging.Level; import java.util.logging.Logger; import java.util.Properties; import javax.ejb.EJB; @@ -930,39 +932,55 @@ public List getDatasetTitlesWithinDataverse(Long dataverseId) { } /** - * Check if a given Dataverse Collection has been configured to generate PIDs for any new version of a dataset - * contained in it. - * @param dataverse The collection to analyse + * Check if a given Dataverse Collection has been configured to generate PIDs for a new version of a dataset + * contained in it. Will also respect the global version PID settings by an admin via + * {@link JvmSettings#PID_VERSIONS_MODE}. + * + * @param collection The collection to check. May not be null (will throw NPE). + * @param willBeMinorVersion Will the {@link DatasetVersion} to receive the PID be a minor version? + * * @return true if enabled, false if disabled * @throws java.util.NoSuchElementException When no or invalid configuration for version PID mode is given */ - public boolean wantsDatasetVersionPids(Dataverse collection) { - VersionPidMode vpm = JvmSettings.PID_VERSIONS_MODE.lookup(VersionPidMode.class); + public boolean wantsDatasetVersionPids(final Dataverse collection, boolean willBeMinorVersion) { + Objects.requireNonNull(collection, "Collection parameter must not be null"); - if (vpm.equals(VersionPidMode.GLOBAL)) { - return true; - } else if (vpm.equals(VersionPidMode.OFF)) { - return false; - } - // now mode = collection's choice - ask the collection and if necessary all ancestors what to do - return askForVersionPidConduct(collection); - } - - /** - * Recursively scan all ancestors if someone defines a proper conduct for version PIDs. - * @param collection The collection to ask for advice - * @return true if someone in the hierarchy has state "ACTIVE", false otherwise - */ - private boolean askForVersionPidConduct(Dataverse collection) { - // Null safety here also matched the case where even root doesn't know, defaulting to save cycles. - if (collection == null) { + // Deactivated by admin globally or no PID for minor version allowed? + VersionPidMode vpm = JvmSettings.PID_VERSIONS_MODE.lookup(VersionPidMode.class); + if (VersionPidMode.OFF.equals(vpm) || ( willBeMinorVersion && VersionPidMode.ALLOW_MAJOR.equals(vpm) )) { return false; } - switch (collection.getDatasetVersionPidConduct()) { - case SKIP: return false; - case ACTIVE: return true; - case INHERIT: return askForVersionPidConduct(collection.getOwner()); + + // Check the collection itself; and potentially it's ancestors + Dataverse c = collection; + while (c != null) { + // Note: the default behavior is INHERIT for the model class + switch (c.getDatasetVersionPidConduct()) { + case SKIP: + logger.log(Level.FINE, "Collection {0} makes {1} skip version PIDs", new String[]{c.getAlias(), collection.getAlias()}); + return false; + case MAJOR: + logger.log(Level.FINE, "Collection {0} allows its sub {1} PIDs for major versions", new String[]{c.getAlias(), collection.getAlias()}); + return !willBeMinorVersion; + case MINOR: + if (vpm.equals(VersionPidMode.ALLOW_MINOR)) { + logger.log(Level.FINE, "Collection {0} allows its sub {1} PIDs for minor versions", new String[]{c.getAlias(), collection.getAlias()}); + return true; + } else { + // In some cases, an admin might have switched the setting after someone already activated it. + // The collection's conduct mode should be updated - we will still cap it as admin says no. + logger.log(Level.INFO, "Collection {0} allows its sub {1} PIDs for minor versions, which is disabled globally. Please update conduct mode of {0}.", new String[]{c.getAlias(), collection.getAlias()}); + return !willBeMinorVersion; + } + case INHERIT: + // Note: root dataverse has no owner, which will break the loop condition + c = c.getOwner(); + } } - return false; + + // If the root dataverse did also not have a policy set, use what the admin configured. + // Note: one could argue we should just return true here, as the below boolean expression is just the + // negation of the one at the top and the collections didn't intervene. But better safe than sorry... + return VersionPidMode.ALLOW_MINOR.equals(vpm) || (!willBeMinorVersion && VersionPidMode.ALLOW_MAJOR.equals(vpm)); } } diff --git a/src/test/java/edu/harvard/iq/dataverse/DataverseServiceBeanTest.java b/src/test/java/edu/harvard/iq/dataverse/DataverseServiceBeanTest.java new file mode 100644 index 00000000000..d503c6800bb --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/DataverseServiceBeanTest.java @@ -0,0 +1,119 @@ +package edu.harvard.iq.dataverse; + +import edu.harvard.iq.dataverse.mocks.MocksFactory; +import edu.harvard.iq.dataverse.settings.JvmSettings; +import edu.harvard.iq.dataverse.util.testing.JvmSetting; +import org.junit.jupiter.api.BeforeAll; +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 java.util.stream.Stream; + +import static edu.harvard.iq.dataverse.pidproviders.VersionPidMode.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class DataverseServiceBeanTest { + + DataverseServiceBean dataverseServiceBean = new DataverseServiceBean(); + + private static final Dataverse root = MocksFactory.makeDataverse(); + private static final Dataverse intermediate = MocksFactory.makeDataverse(); + private static final Dataverse child = MocksFactory.makeDataverse(); + + @BeforeAll + static void setup() { + intermediate.setOwner(root); + child.setOwner(intermediate); + } + + static Stream versionPidCombinationsForAdminMajor() { + return Stream.of( + Arguments.of(false, CollectionConduct.SKIP, false), + Arguments.of(false, CollectionConduct.SKIP, true), + Arguments.of(true, CollectionConduct.MAJOR, false), + Arguments.of(false, CollectionConduct.MAJOR, true), + Arguments.of(true, CollectionConduct.MINOR, false), + Arguments.of(false, CollectionConduct.MINOR, true) + ); + } + + @ParameterizedTest + @MethodSource("versionPidCombinationsForAdminMajor") + @JvmSetting(key = JvmSettings.PID_VERSIONS_MODE, value = "allow-major") + void versionPidCollectionConductModesWithAdminMajorOnly(boolean expected, CollectionConduct rootConduct, boolean willBeMinor) { + // given + root.setDatasetVersionPidConduct(rootConduct); + + // when + assertEquals(expected, dataverseServiceBean.wantsDatasetVersionPids(child, willBeMinor)); + } + + static Stream versionPidCombinationsForAdminMinor() { + return Stream.of( + Arguments.of(false, CollectionConduct.SKIP, false), + Arguments.of(false, CollectionConduct.SKIP, true), + Arguments.of(true, CollectionConduct.MAJOR, false), + Arguments.of(false, CollectionConduct.MAJOR, true), + Arguments.of(true, CollectionConduct.MINOR, false), + Arguments.of(true, CollectionConduct.MINOR, true) + ); + } + + @ParameterizedTest + @MethodSource("versionPidCombinationsForAdminMinor") + @JvmSetting(key = JvmSettings.PID_VERSIONS_MODE, value = "allow-minor") + void versionPidCollectionConductModesWithAdminMinor(boolean expected, CollectionConduct rootConduct, boolean willBeMinor) { + // given + root.setDatasetVersionPidConduct(rootConduct); + + // when + assertEquals(expected, dataverseServiceBean.wantsDatasetVersionPids(child, willBeMinor)); + } + + @Test + void versionPidCollectionMayNotBeNull() { + assertThrows(NullPointerException.class, () -> dataverseServiceBean.wantsDatasetVersionPids(null, false)); + } + + @Test + @JvmSetting(key = JvmSettings.PID_VERSIONS_MODE, value = "off") + void versionPidCollectionAdminDisabled() { + assertFalse(dataverseServiceBean.wantsDatasetVersionPids(child, false)); + assertFalse(dataverseServiceBean.wantsDatasetVersionPids(child, true)); + } + + @Test + @JvmSetting(key = JvmSettings.PID_VERSIONS_MODE, value = "allow-major") + void versionPidCollectionAdminMajorOnly() { + assertFalse(dataverseServiceBean.wantsDatasetVersionPids(child, true)); + } + + @Test + @JvmSetting(key = JvmSettings.PID_VERSIONS_MODE, value = "allow-major") + void versionPidNoCollectionConductButAdminMajorOnly() { + // given + Dataverse collection = MocksFactory.makeDataverse(); + collection.setDatasetVersionPidConduct(CollectionConduct.INHERIT); + + // when & then + assertTrue(dataverseServiceBean.wantsDatasetVersionPids(collection, false)); + assertFalse(dataverseServiceBean.wantsDatasetVersionPids(collection, true)); + } + + @Test + @JvmSetting(key = JvmSettings.PID_VERSIONS_MODE, value = "allow-minor") + void versionPidNoCollectionConductButAdminMinor() { + // given + Dataverse collection = MocksFactory.makeDataverse(); + collection.setDatasetVersionPidConduct(CollectionConduct.INHERIT); + + // when & then + assertTrue(dataverseServiceBean.wantsDatasetVersionPids(collection, false)); + assertTrue(dataverseServiceBean.wantsDatasetVersionPids(collection, true)); + } +} \ No newline at end of file From d8f85342cb7d916b06f85e0dfca5f0008d8d1a13 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Mon, 19 Jun 2023 11:35:08 +0200 Subject: [PATCH 35/35] docs(pid): fix Javadoc in VersionPidMode --- .../edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java b/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java index 96d8c0c8ed3..71fb5474d18 100644 --- a/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java +++ b/src/main/java/edu/harvard/iq/dataverse/pidproviders/VersionPidMode.java @@ -32,7 +32,7 @@ public final class VersionPidMode { /** * A collection of conducts for Dataverse collections, used in {@link edu.harvard.iq.dataverse.Dataverse} - * and {@link edu.harvard.iq.dataverse.DataverseServiceBean#wantsDatasetVersionPids(Dataverse)}: + * and {@link edu.harvard.iq.dataverse.DataverseServiceBean#wantsDatasetVersionPids(Dataverse, boolean)}: *
    *
  1. Collection may inherit version pid behaviour from the parent collection(s),
    2. *
  2. collection may choose to opt out and skip the minting,