diff --git a/dspace-api/src/main/java/org/dspace/identifier/ClarinCommunityDOIIdentifierProvider.java b/dspace-api/src/main/java/org/dspace/identifier/ClarinCommunityDOIIdentifierProvider.java index 9176d861c9f9..dcd92341d6f2 100644 --- a/dspace-api/src/main/java/org/dspace/identifier/ClarinCommunityDOIIdentifierProvider.java +++ b/dspace-api/src/main/java/org/dspace/identifier/ClarinCommunityDOIIdentifierProvider.java @@ -21,14 +21,12 @@ import org.dspace.content.MetadataValue; import org.dspace.content.logic.Filter; import org.dspace.core.Context; -import org.dspace.identifier.doi.ClarinDataCiteConnector; -import org.dspace.identifier.doi.DOIConnector; -import org.springframework.beans.factory.annotation.Autowired; /** * DOI identifier provider for CLARIN communities. - * It uses the ClarinDataCiteConnector to connect to DataCite and requires the DOI prefix - * and the list of CLARIN community IDs to be configured. + * It requires the DOI prefix and the list of CLARIN community IDs to be configured. + * The DOI connector (a ClarinDataCiteConnector) is configured per provider in Spring, including its + * credentials and DOI prefix — the connector's prefix must match this provider's prefix. * * @author Milan Kuchtiak (kuchtiak@ufal.mff.cuni.cz) */ @@ -36,8 +34,6 @@ public class ClarinCommunityDOIIdentifierProvider extends DOIIdentifierProvider private static final Logger log = LogManager.getLogger(ClarinCommunityDOIIdentifierProvider.class); - protected DOIConnector connector; - private String doiPrefix; private String namespaceSeparator; @@ -45,40 +41,12 @@ public class ClarinCommunityDOIIdentifierProvider extends DOIIdentifierProvider private Set communities = new HashSet<>(); public void init() { - if (!(this.connector instanceof ClarinDataCiteConnector)) { - throw new IllegalStateException("ClarinCommunityDOIIdentifierProvider requires ClarinDataCiteConnector"); - } if (doiPrefix == null) { throw new IllegalStateException("No DOI prefix configured for ClarinCommunityDOIIdentifierProvider"); } if (namespaceSeparator == null) { namespaceSeparator = ""; } - - ClarinDataCiteConnector dataCiteConnector = (ClarinDataCiteConnector) this.connector; - - dataCiteConnector.setDoiPrefix(doiPrefix); - - String username = this.configurationService.getProperty("identifier.doi." + doiPrefix + ".user"); - if (username == null) { - log.error("No username configured for DOI prefix '{}'", doiPrefix); - throw new IllegalStateException("No username configured for DOI prefix '" + doiPrefix + "'"); - } - dataCiteConnector.setUsername(username); - - String password = this.configurationService.getProperty("identifier.doi." + doiPrefix + ".password"); - if (password == null) { - log.error("No password configured for DOI prefix '{}'", doiPrefix); - throw new IllegalStateException("No password configured for DOI prefix '" + doiPrefix + "'"); - } - dataCiteConnector.setPassword(password); - } - - @Override - @Autowired(required = true) - public void setDOIConnector(DOIConnector connector) { - super.setDOIConnector(connector); - this.connector = connector; } /** diff --git a/dspace-api/src/main/java/org/dspace/identifier/ClarinDOIIdentifierProvider.java b/dspace-api/src/main/java/org/dspace/identifier/ClarinDOIIdentifierProvider.java index 000a2f30cd0e..ed957adb6bb5 100644 --- a/dspace-api/src/main/java/org/dspace/identifier/ClarinDOIIdentifierProvider.java +++ b/dspace-api/src/main/java/org/dspace/identifier/ClarinDOIIdentifierProvider.java @@ -8,11 +8,11 @@ package org.dspace.identifier; import java.sql.SQLException; -import java.util.Collections; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -40,7 +40,16 @@ public class ClarinDOIIdentifierProvider extends DOIIdentifierProvider { private List providers; - protected final SimpleCache simpleCache = new SimpleCache<>(5); + /** + * Cache of the matched provider per collection, keyed by collection UUID. The matched provider is + * a function of the collection's community ancestry (not of the item), so the cache is shared by all + * items of a collection and is naturally bounded by the number of collections. An empty Optional + * records that the collection is in no configured community, so negative lookups are cached too. + * Entries live for the JVM lifetime: a collection moved to another community keeps its old provider + * until restart. + */ + protected final Map> collectionProviderCache = + new ConcurrentHashMap<>(); public void setProviders(List providers) { this.providers = providers; @@ -135,6 +144,26 @@ public void checkMintable(Context context, Filter filter, DSpaceObject dso) } } + /** + * Delete the given DOI online (at the registration agency) using the community provider whose + * configured prefix matches the DOI. + *

+ * Unlike the item-based operations above (register/reserve/mint/...), this method deliberately fails + * loud when no provider matches the DOI's prefix: the lookup here is by DOI prefix alone, there is no + * DSpaceObject available at delete time, so there is no item context in which "not in a configured + * community" would be an expected, skippable state. + *

+ * The only caller is DOIOrganiser's {@code --delete-doi} CLI path, which catches + * {@link DOIIdentifierException} and reports the failure to the operator. The delegated + * {@code deleteOnline} already throws {@link DOIIdentifierException} for other failure modes; + * silently skipping an unknown prefix would leave the DOI row stuck in {@code TO_BE_DELETED} + * with no visible error. + * + * @param context the DSpace context + * @param identifier the DOI to delete online + * @throws DOIIdentifierException if no configured provider matches the DOI's prefix, or if the + * delegated online deletion fails + */ @Override public void deleteOnline(Context context, String identifier) throws DOIIdentifierException { getProviderForIdentifier(identifier).deleteOnline(context, identifier); @@ -208,21 +237,11 @@ public void updateMetadataOnline(Context context, DSpaceObject dso, String ident private ClarinCommunityDOIIdentifierProvider getProviderForItem(Context context, Item item) { - if (simpleCache.containsKey(item.getID())) { - return simpleCache.get(item.getID()); - } - // Check communities of item.getCollections() - this will only see collections if the item is archived for (Collection collection : item.getCollections()) { - try { - for (ClarinCommunityDOIIdentifierProvider provider : providers) { - if (isCollectionInProvider(provider, collection)) { - simpleCache.put(item.getID(), provider); - return provider; - } - } - } catch (SQLException e) { - log.error("Error while determining DOI provider for item {}", item.getID(), e); + ClarinCommunityDOIIdentifierProvider provider = getProviderForCollection(context, collection); + if (provider != null) { + return provider; } } @@ -233,21 +252,7 @@ private ClarinCommunityDOIIdentifierProvider getProviderForItem(Context context, if (parent instanceof Collection) { log.debug("Got parent DSO for item: " + parent.getID().toString()); log.debug("Parent DSO handle: " + parent.getHandle()); - try { - // Now iterate communities of this parent collection - for (ClarinCommunityDOIIdentifierProvider provider : providers) { - if (isCollectionInProvider(provider, (Collection) parent)) { - simpleCache.put(item.getID(), provider); - return provider; - } - } - // parent collection is not in any of the configured communities, - // cache this fact to avoid future lookups - simpleCache.put(item.getID(), null); - } catch (SQLException e) { - log.error("Error while determining DOI provider for item {} from the parent collection", - item.getID(), e); - } + return getProviderForCollection(context, (Collection) parent); } else { log.debug("Parent DSO is null or is not a Collection..."); } @@ -258,8 +263,34 @@ private ClarinCommunityDOIIdentifierProvider getProviderForItem(Context context, return null; } + private ClarinCommunityDOIIdentifierProvider getProviderForCollection(Context context, Collection collection) { + Optional cached = collectionProviderCache.get(collection.getID()); + if (cached != null) { + return cached.orElse(null); + } + ClarinCommunityDOIIdentifierProvider match = null; + try { + for (ClarinCommunityDOIIdentifierProvider provider : providers) { + if (isCollectionInProvider(provider, collection)) { + match = provider; + break; + } + } + } catch (SQLException e) { + // don't cache on error, so a transient DB problem doesn't pin a wrong (negative) result + log.error("Error while determining DOI provider for collection {}", collection.getID(), e); + return null; + } + collectionProviderCache.put(collection.getID(), Optional.ofNullable(match)); + return match; + } + private boolean isCollectionInProvider(ClarinCommunityDOIIdentifierProvider provider, Collection collection) throws SQLException { + // Only the direct parent communities of the collection are considered, mirroring the semantics of + // the per-community handle prefixes (lr.pid.community.configurations): a provider configured with a + // top-level community does NOT cover its sub-communities — list each sub-community explicitly in the + // provider's "communities" property if items under it should get DOIs List parentCommunities = collection.getCommunities(); for (Community parentCommunity : parentCommunities) { if (provider.getCommunities().contains(parentCommunity.getID().toString())) { @@ -291,33 +322,4 @@ private void logInfoNonConfigurableEntity(DSpaceObject dso, String actionName) { log.info("Item {} is not in a configured CLARIN community, skipping DOI {}", dso.getID(), actionName); } - protected static class SimpleCache { - private final Map cache; - - public SimpleCache(int maxCapacity) { - this.cache = Collections.synchronizedMap(new LinkedHashMap(maxCapacity, 0.75f, true) { - @Override - protected boolean removeEldestEntry(Map.Entry eldest) { - return size() > maxCapacity; - } - }); - } - - public V get(K key) { - return cache.get(key); - } - - public void put(K key, V value) { - cache.put(key, value); - } - - public boolean containsKey(K key) { - return cache.containsKey(key); - } - - public void clear() { - cache.clear(); - } - } - } \ No newline at end of file diff --git a/dspace-api/src/main/java/org/dspace/identifier/doi/DataCiteConnector.java b/dspace-api/src/main/java/org/dspace/identifier/doi/DataCiteConnector.java index b875f9cc01a1..7dbceab1196b 100644 --- a/dspace-api/src/main/java/org/dspace/identifier/doi/DataCiteConnector.java +++ b/dspace-api/src/main/java/org/dspace/identifier/doi/DataCiteConnector.java @@ -250,10 +250,7 @@ protected String getPassword() { } protected String getDoiPrefix() { - if (this.doiPrefix == null) { - this.doiPrefix = configurationService.getProperty(CFG_PREFIX); - } - return this.doiPrefix; + return this.doiPrefix != null ? this.doiPrefix : configurationService.getProperty(CFG_PREFIX); } @Override diff --git a/dspace-api/src/test/java/org/dspace/identifier/ClarinDOIIdentifierProviderIT.java b/dspace-api/src/test/java/org/dspace/identifier/ClarinDOIIdentifierProviderIT.java index 8f4349eb5422..e74dcddfeae6 100644 --- a/dspace-api/src/test/java/org/dspace/identifier/ClarinDOIIdentifierProviderIT.java +++ b/dspace-api/src/test/java/org/dspace/identifier/ClarinDOIIdentifierProviderIT.java @@ -165,11 +165,46 @@ public void testMint() throws IdentifierException, SQLException, AuthorizeExcept assertTrue(doi4.startsWith("doi:10.1/1-")); // check if the DOI for itemV2 is different from the one for item1 assertFalse(doi4.startsWith(doi1)); - checkDoi(doi2, DOIIdentifierProvider.MINTED); + checkDoi(doi4, DOIIdentifierProvider.MINTED); // check that the old DOI identifier was removed from itemV2 assertEquals(0, getDoiMetadata(itemV2).size()); } + @Test + public void testMintInSubCommunity() throws IdentifierException, SQLException { + context.turnOffAuthorisationSystem(); + // parentCommunity is configured for provider 10.1; the sub-community itself is not configured + Community subCommunity = CommunityBuilder.createSubCommunity(context, parentCommunity) + .withName("Sub Community 1") + .build(); + Collection subCollection = CollectionBuilder.createCollection(context, subCommunity) + .withName("Sub Collection") + .build(); + Item subItem = ItemBuilder.createItem(context, subCollection) + .withTitle("Sub Item") + .build(); + context.restoreAuthSystemState(); + + // matching mirrors the per-community handle prefixes (lr.pid.community.configurations): only the + // direct parent communities of the owning collection count, so the top-level community's provider + // does not cover the sub-community and the item gets no DOI + assertNull(provider.mint(context, subItem)); + + // listing the sub-community explicitly in a provider's communities is the supported configuration + ClarinCommunityDOIIdentifierProvider subCommunityProvider = + createCommunityProvider("10.3", "3-", Set.of(subCommunity.getID().toString())); + ClarinDOIIdentifierProvider subCommunityDispatcher = new ClarinDOIIdentifierProvider(); + subCommunityDispatcher.setProviders(List.of(subCommunityProvider)); + subCommunityDispatcher.itemService = itemService; + subCommunityDispatcher.doiService = doiService; + subCommunityDispatcher.contentServiceFactory = ContentServiceFactory.getInstance(); + + String doi = subCommunityDispatcher.mint(context, subItem); + assertNotNull(doi); + assertTrue(doi.startsWith("doi:10.3/3-")); + checkDoi(doi, DOIIdentifierProvider.MINTED); + } + @Test public void testCheckMintable() throws IdentifierException { provider.checkMintable(context, item1); @@ -281,9 +316,6 @@ public void testOnlineOperations() throws IdentifierException, SQLException { private ClarinCommunityDOIIdentifierProvider createCommunityProvider(String doiPrefix, String namespaceSeparator, Set communityIds) { - configurationService.setProperty("identifier.doi." + doiPrefix + ".user", "test_user"); - configurationService.setProperty("identifier.doi." + doiPrefix + ".password", "password"); - ClarinDataCiteConnector connector = mock(ClarinDataCiteConnector.class); ClarinCommunityDOIIdentifierProvider communityProvider = new ClarinCommunityDOIIdentifierProvider(); diff --git a/dspace/config/spring/api/identifier-service.xml b/dspace/config/spring/api/identifier-service.xml index 004e32fd38f0..46fab55a2ea4 100644 --- a/dspace/config/spring/api/identifier-service.xml +++ b/dspace/config/spring/api/identifier-service.xml @@ -123,12 +123,21 @@ that allows you to configure multiple community specific DOI providers. Each provider will only mint DOIs for items in the configured communities. + An item belongs to a community when that community is a direct parent of the item's owning + collection - the same semantics as the per-community handle prefixes + (lr.pid.community.configurations). A provider configured with a top-level community does not + cover its sub-communities; list each sub-community explicitly in the "communities" property. + Credentials, for DOI connectors need to be configured, per each DOI prefix, in local.cfg, e.g.: identifier.doi.10.83111.user = identifier.doi.10.83111.password = identifier.doi.10.83222.user = identifier.doi.10.83222.password = + + The ${...} placeholders on the connector beans below resolve against the DSpace configuration, + so the credentials stay in local.cfg. Each connector's doiPrefix must match the doiPrefix of the + provider it is wired into. -->