Skip to content

Pr1370 review #1390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kosarko wants to merge 6 commits into
base: issue_#1181_doi_configuration_per_community
Choose a base branch
from
Open

Pr1370 review #1390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e048b7a
resolve review comment - verify status of freshly minted version DOI …
kosarko Jul 16, 2026
4d0e302
resolve review comment - document why deleteOnline fails loud for an …
kosarko Jul 16, 2026
c652159
resolve review comment - make getDoiPrefix stateless to avoid lazy-in…
kosarko Jul 16, 2026
bffc5fa
resolve review comment - key the provider cache by collection instead…
kosarko Jul 16, 2026
92c5d4e
resolve review comment - document that DOI providers match direct par…
kosarko Jul 16, 2026
4e42b92
resolve review comment - configure connector credentials and prefix i…
kosarko Jul 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 3 additions & 35 deletions dspace-api/src/main/java/org/dspace/identifier/ClarinCommunityDOIIdentifierProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,64 +21,32 @@
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)
*/
public class ClarinCommunityDOIIdentifierProvider extends DOIIdentifierProvider {

private static final Logger log = LogManager.getLogger(ClarinCommunityDOIIdentifierProvider.class);

protected DOIConnector connector;

private String doiPrefix;

private String namespaceSeparator;

private Set<String> 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;
}

/**
Expand Down
122 changes: 62 additions & 60 deletions dspace-api/src/main/java/org/dspace/identifier/ClarinDOIIdentifierProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -40,7 +40,16 @@ public class ClarinDOIIdentifierProvider extends DOIIdentifierProvider {

private List<ClarinCommunityDOIIdentifierProvider> providers;

protected final SimpleCache<UUID, ClarinCommunityDOIIdentifierProvider> 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<UUID, Optional<ClarinCommunityDOIIdentifierProvider>> collectionProviderCache =
new ConcurrentHashMap<>();

public void setProviders(List<ClarinCommunityDOIIdentifierProvider> providers) {
this.providers = providers;
Expand Down Expand Up @@ -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.
* <p>
* 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.
* <p>
* 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);
Expand Down Expand Up @@ -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;
}
}

Expand All @@ -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...");
}
Expand All @@ -258,8 +263,34 @@ private ClarinCommunityDOIIdentifierProvider getProviderForItem(Context context,
return null;
}

private ClarinCommunityDOIIdentifierProvider getProviderForCollection(Context context, Collection collection) {
Optional<ClarinCommunityDOIIdentifierProvider> 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<Community> parentCommunities = collection.getCommunities();
for (Community parentCommunity : parentCommunities) {
if (provider.getCommunities().contains(parentCommunity.getID().toString())) {
Expand Down Expand Up @@ -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<K, V> {

@kuchtiak-ufal kuchtiak-ufal Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SimpleCache was limited to 5 items, and is replaced by unlimited ConcurrentHashMap.
My concern is that It may occupy lot of memory.

private final Map<K, V> cache;

public SimpleCache(int maxCapacity) {
this.cache = Collections.synchronizedMap(new LinkedHashMap<K, V>(maxCapacity, 0.75f, true) {
@Override
protected boolean removeEldestEntry(Map.Entry<K, V> 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();
}
}

}
5 changes: 1 addition & 4 deletions dspace-api/src/main/java/org/dspace/identifier/doi/DataCiteConnector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
40 changes: 36 additions & 4 deletions dspace-api/src/test/java/org/dspace/identifier/ClarinDOIIdentifierProviderIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -281,9 +316,6 @@ public void testOnlineOperations() throws IdentifierException, SQLException {
private ClarinCommunityDOIIdentifierProvider createCommunityProvider(String doiPrefix,
String namespaceSeparator,
Set<String> 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();
Expand Down
21 changes: 19 additions & 2 deletions dspace/config/spring/api/identifier-service.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = <username>
identifier.doi.10.83111.password = <password>
identifier.doi.10.83222.user = <username>
identifier.doi.10.83222.password = <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.
-->
<!-- Uncomment to enable community specific DOI providers
<bean id="org.dspace.identifier.DOIIdentifierProvider" class="org.dspace.identifier.ClarinDOIIdentifierProvider">
Expand All @@ -146,7 +155,11 @@
abstract="true">
<property name="configurationService" ref="org.dspace.services.ConfigurationService" />
<property name="DOIConnector">
<bean parent = "org.dspace.identifier.doi.DOIConnector" />
<bean parent = "org.dspace.identifier.doi.DOIConnector">
<property name="username" value="${identifier.doi.10.83111.user}" />
<property name="password" value="${identifier.doi.10.83111.password}" />
<property name="doiPrefix" value="10.83111" />

@kuchtiak-ufal kuchtiak-ufal Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid the same doiPrefix to be declared twice ?

I'd leave the doiPrefix to be the property of Provider, and username, password properties of DataCiteConnector,
and use the init() method to pass the doiPrefix to connector.

BTW, after this change, it doesn't matter what format of username/password properties will be used for connector. It could be whatever format. What's mentioned here is just a convention.

</bean>
</property>
<property name="doiPrefix" value="10.83111" />
<property name="namespaceSeparator" value="clarin.1." />
Expand All @@ -164,7 +177,11 @@
abstract="true">
<property name="configurationService" ref="org.dspace.services.ConfigurationService" />
<property name="DOIConnector">
<bean parent = "org.dspace.identifier.doi.DOIConnector" />
<bean parent = "org.dspace.identifier.doi.DOIConnector">
<property name="username" value="${identifier.doi.10.83222.user}" />
<property name="password" value="${identifier.doi.10.83222.password}" />
<property name="doiPrefix" value="10.83222" />
</bean>
</property>
<property name="doiPrefix" value="10.83222" />
<property name="namespaceSeparator" value="clarin.2." />
Expand Down
Loading