MENDELU/ORCID Authority by milanmajchrak · Pull Request #1292 · dataquest-dev/DSpace

milanmajchrak · 2026-03-20T15:03:11Z

…or, and supporting changes

Cherry-picked from:

ufal/orcid-cherry-pick (16e5d91): SimpleORCIDAuthority, CachingOrcidRestConnector, ExpandedSearchConverter, enable-orcid.cfg, datacite registry, ehcache config
vsb-tuo/orcid (fb0508d): Enable ORCID authority in dspace.cfg, uncomment Orcidv3AuthorityValue bean
zcu-pub-add-orcid-identifier (509c6a1): Register dc.identifier.orcid metadata type
vsb/orcid-authority-fix (e8552d0): Fix LazyInitializationException - remove context.abort() in SimpleORCIDAuthority.resolveLocalLabel(), add safety net in SolrServiceImpl.updateIndex()

Additional changes:

Add findByAuthorityAndLanguage to MetadataValueService/DAO (required by SimpleORCIDAuthority local DB label fallback)
Fix Bundle.getBitstreams() lazy loading in HibernateDBConnection.uncacheEntity() using session.contains() guard

Problem description

Analysis

(Write here, if there is needed describe some specific problem. Erase it, when it is not needed.)

Problems

(Write here, if some unexpected problems occur during solving issues. Erase it, when it is not needed.)

Manual Testing (if applicable)

Added to testing scenarios

Copilot review

Requested review from Copilot

…or, and supporting changes Cherry-picked from: - ufal/orcid-cherry-pick (16e5d91): SimpleORCIDAuthority, CachingOrcidRestConnector, ExpandedSearchConverter, enable-orcid.cfg, datacite registry, ehcache config - vsb-tuo/orcid (fb0508d): Enable ORCID authority in dspace.cfg, uncomment Orcidv3AuthorityValue bean - zcu-pub-add-orcid-identifier (509c6a1): Register dc.identifier.orcid metadata type - vsb/orcid-authority-fix (e8552d0): Fix LazyInitializationException - remove context.abort() in SimpleORCIDAuthority.resolveLocalLabel(), add safety net in SolrServiceImpl.updateIndex() Additional changes: - Add findByAuthorityAndLanguage to MetadataValueService/DAO (required by SimpleORCIDAuthority local DB label fallback) - Fix Bundle.getBitstreams() lazy loading in HibernateDBConnection.uncacheEntity() using session.contains() guard

Copilot

Pull request overview

Adds ORCID-based authority control support (including a caching ORCID REST connector) plus related configuration/registry updates and a couple of stability fixes (Solr indexing error handling, Hibernate uncache guard).

Changes:

Introduce SimpleORCIDAuthority and CachingOrcidRestConnector (with Ehcache-backed label caching) and enable related Spring/config wiring.
Extend metadata registry/services (new dc.identifier.orcid, new findByAuthorityAndLanguage() DAO/service API, and a new DataCite registry file).
Add resilience fixes in indexing (SolrServiceImpl) and Hibernate entity uncache (HibernateDBConnection).

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
dspace/config/spring/api/orcid-authority-services.xml	Enables ORCID authority types and registers the caching ORCID connector bean
dspace/config/registries/dublin-core-types.xml	Registers `dc.identifier.orcid`
dspace/config/registries/datacite.xml	Adds a DataCite metadata registry file
dspace/config/features/enable-orcid.cfg	Feature toggle/config to enable ORCID authority for `dc.contributor.author`
dspace/config/ehcache.xml	Adds ORCID cache template + `orcid-labels` cache alias
dspace/config/dspace.cfg	Includes the ORCID feature config by default
dspace-api/src/main/java/org/dspace/external/CachingOrcidRestConnector.java	New connector using `@Cacheable` to cache ORCID labels
dspace-api/src/main/java/org/dspace/external/provider/orcid/xml/ExpandedSearchConverter.java	Converts ORCID expanded-search XML into internal result objects
dspace-api/src/main/java/org/dspace/external/provider/orcid/xml/CacheLogger.java	Ehcache event listener to debug ORCID cache activity
dspace-api/src/main/java/org/dspace/content/authority/SimpleORCIDAuthority.java	New `ChoiceAuthority` implementation backed by ORCID API + local DB fallback
dspace-api/src/main/java/org/dspace/content/service/MetadataValueService.java	Adds `findByAuthorityAndLanguage()` to service interface
dspace-api/src/main/java/org/dspace/content/MetadataValueServiceImpl.java	Implements the new `findByAuthorityAndLanguage()` service method
dspace-api/src/main/java/org/dspace/content/dao/MetadataValueDAO.java	Adds `findByAuthorityAndLanguage()` to DAO interface
dspace-api/src/main/java/org/dspace/content/dao/impl/MetadataValueDAOImpl.java	Implements `findByAuthorityAndLanguage()` query
dspace-api/src/main/java/org/dspace/discovery/SolrServiceImpl.java	Adds per-object runtime exception handling during bulk indexing
dspace-api/src/main/java/org/dspace/core/HibernateDBConnection.java	Avoids triggering Bundle bitstreams lazy-load when bundle is detached
dspace-api/src/test/java/org/dspace/external/CachingOrcidRestConnectorTest.java	Adds tests for connector + caching behavior
dspace-api/src/test/data/dspaceFolder/config/spring/api/orcid-authority-services.xml	Test Spring wiring for ORCID authority components

Copilot · 2026-03-20T15:11:27Z

dspace-api/src/main/java/org/dspace/external/CachingOrcidRestConnector.java

+        log.debug("getLabel: " + orcid);
+        // in theory, we could use orcid.org/v3.0/<ORCID>/personal-details, but didn't want to write another converter
+        ExpandedSearchConverter.Results search = search("orcid:" + orcid, 0, 1);
+        if (search.isOk() && search.numFound() > 0) {


This condition can throw IndexOutOfBoundsException: numFound may be > 0 while results() is empty (e.g., if results were filtered out due to missing/blank ORCID iDs). Check that results() is non-empty before accessing index 0 (or base the condition on results().isEmpty()).

Suggested change

if (search.isOk() && search.numFound() > 0) {

if (search.isOk() && !search.results().isEmpty()) {

Copilot · 2026-03-20T15:11:28Z

dspace-api/src/test/java/org/dspace/external/CachingOrcidRestConnectorTest.java

+    @Test(expected = RuntimeException.class)
+    public void getAccessToken_badUrl() {
+        String accessToken = sut.getAccessToken("secret","id", "http://example.com");
+        assertNull("Expecting accessToken to be null", accessToken);
+    }
+
+    @Test(expected = RuntimeException.class)
+    public void getAccessToken_badParams() {
+        //expect an exception to be thrown
+        sut.getAccessToken(null, null, null);
+    }
+
+    @Test(expected = RuntimeException.class)
+    public void getAccessToken() {
+        String accessToken = sut.getAccessToken("DEAD", "BEEF", "https://sandbox.orcid.org/oauth/token");
+        assertNotNull("Expecting accessToken to be not null", accessToken);
+    }


Using @test(expected=RuntimeException.class) means assertions after the call will never run if the exception is thrown as expected, so these tests don't actually validate the returned value. Either remove the expected exception and assert on the result, or use an explicit try/catch (or Assert.assertThrows pattern) to both verify the exception and assert anything else you need.

Copilot · 2026-03-20T15:11:28Z

dspace-api/src/main/java/org/dspace/content/dao/impl/MetadataValueDAOImpl.java

+        queryString.append("SELECT m FROM MetadataValue m WHERE m.authority = :authority");
+
+        if (language != null) {
+            queryString.append(" AND m.language = :language");
+        }
+
+        queryString.append(" ORDER BY m.place ASC");
+
+        Query query = createQuery(context, queryString.toString());
+        query.setParameter("authority", authority);
+
+        if (language != null) {
+            query.setParameter("language", language);
+        }
+


When language is null, this query currently returns all languages for the authority (no language predicate). Callers (e.g. local-label fallback) appear to use language=null to mean "language IS NULL"; returning all languages can pick an arbitrary label and break locale fallback. Update the query to explicitly filter for NULL language when the parameter is null (and consider treating blank as null too).

Copilot · 2026-03-20T15:11:28Z

dspace/config/spring/api/orcid-authority-services.xml

    <bean name="AuthorityTypes" class="org.dspace.authority.AuthorityTypes">
        <property name="types">
            <list>
-                <!-- <bean class="org.dspace.authority.orcid.Orcidv3AuthorityValue"/> -->
+                <bean class="org.dspace.authority.orcid.Orcidv3AuthorityValue"/>
                <bean class="org.dspace.authority.PersonAuthorityValue"/>


Orcidv3AuthorityValue.newInstance() looks up a bean named "AuthoritySource" (Orcidv3SolrAuthorityImpl). In this config the OrcidSource/AuthoritySource bean is still commented out, so enabling Orcidv3AuthorityValue here will lead to a null service lookup / runtime failure when the authority tries to instantiate/query. Either uncomment and configure the OrcidSource bean (and its dependencies) or keep Orcidv3AuthorityValue disabled here.

Copilot · 2026-03-20T15:11:28Z

dspace-api/src/main/java/org/dspace/external/provider/orcid/xml/ExpandedSearchConverter.java

+        } catch (SAXException | URISyntaxException e) {
+            log.error(e);
+        }


Logging only the exception object here will generally omit the stack trace. Log a message and pass the exception as the throwable parameter so troubleshooting has full context.

Copilot · 2026-03-20T15:11:29Z

dspace-api/src/test/java/org/dspace/external/CachingOrcidRestConnectorTest.java

+    @Test
+    public void getLabel() {
+        sut = Mockito.spy(sut);
+        sut.setApiURL("https://pub.sandbox.orcid.org/v3.0");
+        //Mock the CachingOrcidRestConnector so that getAccessToken returns sandboxToken
+        doReturn(sandboxToken).when(sut).getAccessToken(Mockito.anyString(), Mockito.anyString(), Mockito.anyString());
+
+        String label = sut.getLabel(orcid);
+        assertEquals(expectedLabel, label);
+    }
+    @Test
+    public void search() {
+        sut = Mockito.spy(sut);
+        sut.setApiURL("https://pub.sandbox.orcid.org/v3.0");
+        //Mock the CachingOrcidRestConnector so that getAccessToken returns sandboxToken
+        doReturn(sandboxToken).when(sut).getAccessToken(Mockito.anyString(), Mockito.anyString(), Mockito.anyString());
+
+        ExpandedSearchConverter.Results search = sut.search("joh", 0, 1);
+        //Should match all Johns also, because edismax with wildcard
+        assertTrue(search.numFound() > 1000);
+    }


These tests call the real ORCID sandbox endpoints (getLabel/search) which makes the unit test suite dependent on external network access and remote service stability/data. This is likely to be flaky or fail in CI environments without outbound access. Consider mocking the HTTP client / response bodies (e.g., via an injectable HttpClient wrapper) so tests are deterministic and offline.

Copilot · 2026-03-20T15:11:29Z

dspace/config/features/enable-orcid.cfg

@@ -0,0 +1,24 @@
+## ORCID authority (https://wiki.lyrasis.org/display/DSDOC7x/ORCID+Authority) is bit cumbersome to use


Grammar nit: "is bit cumbersome" should be "is a bit cumbersome".

Suggested change

## ORCID authority (https://wiki.lyrasis.org/display/DSDOC7x/ORCID+Authority) is bit cumbersome to use

## ORCID authority (https://wiki.lyrasis.org/display/DSDOC7x/ORCID+Authority) is a bit cumbersome to use

Copilot · 2026-03-20T15:11:29Z

dspace/config/registries/datacite.xml

+        <name>datacite</name>
+        <namespace>http://datacite.org/schema/kernel-4</namespace>
+    </dc-schema>
+    <dc-type>
+        <schema>datacite</schema>
+        <element>relation</element>
+        <qualifier>IsCitedBy</qualifier>
+        <scope_note>IsCitedBy</scope_note>
+    </dc-type>
+    <dc-type>
+        <schema>datacite</schema>
+        <element>relation</element>
+        <qualifier>IsReferencedBy</qualifier>
+        <scope_note>IsReferencedBy</scope_note>
+    </dc-type>
+    <dc-type>
+        <schema>datacite</schema>
+        <element>relation</element>
+        <qualifier>IsSupplementTo</qualifier>
+        <scope_note>IsSupplementTo</scope_note>
+    </dc-type>
+    <dc-type>
+        <schema>datacite</schema>
+        <element>relation</element>
+        <qualifier>Cites</qualifier>
+        <scope_note>Cites</scope_note>
+    </dc-type>
+    <dc-type>
+        <schema>datacite</schema>
+        <element>relation</element>
+        <qualifier>References</qualifier>
+        <scope_note>References</scope_note>
+    </dc-type>
+    <dc-type>
+        <schema>datacite</schema>
+        <element>relation</element>
+        <qualifier>IsSupplementedBy</qualifier>
+        <scope_note>IsSupplementedBy</scope_note>
+    </dc-type>
+    <dc-type>
+        <schema>datacite</schema>
+        <element>relation</element>
+        <qualifier>IsContinuedBy</qualifier>
+        <scope_note>IsContinuedBy</scope_note>
+    </dc-type>
+    <dc-type>
+        <schema>datacite</schema>
+        <element>relation</element>
+        <qualifier>Continues</qualifier>
+        <scope_note>Continues</scope_note>
+    </dc-type>
+    <dc-type>
+        <schema>datacite</schema>
+        <element>relation</element>
+        <qualifier>IsDescribedBy</qualifier>
+        <scope_note>IsDescribedBy</scope_note>


This new registry file defines the same schema name/namespace (datacite / kernel-4) that already exists in dspace/config/registries/datacite-types.xml. Loading both will likely create duplicate/conflicting metadata fields. Additionally, qualifiers here are capitalized (e.g. "IsReferencedBy") while the existing registry uses lowercase (e.g. "isReferencedBy"), which would create distinct fields due to case sensitivity. Consider extending the existing datacite-types.xml instead of introducing a parallel schema file, and align qualifier casing with the established registry.

Suggested change

<name>datacite</name>

<namespace>http://datacite.org/schema/kernel-4</namespace>

</dc-schema>

<dc-type>

<schema>datacite</schema>

<element>relation</element>

<qualifier>IsCitedBy</qualifier>

<scope_note>IsCitedBy</scope_note>

</dc-type>

<dc-type>

<schema>datacite</schema>

<element>relation</element>

<qualifier>IsReferencedBy</qualifier>

<scope_note>IsReferencedBy</scope_note>

</dc-type>

<dc-type>

<schema>datacite</schema>

<element>relation</element>

<qualifier>IsSupplementTo</qualifier>

<scope_note>IsSupplementTo</scope_note>

</dc-type>

<dc-type>

<schema>datacite</schema>

<element>relation</element>

<qualifier>Cites</qualifier>

<scope_note>Cites</scope_note>

</dc-type>

<dc-type>

<schema>datacite</schema>

<element>relation</element>

<qualifier>References</qualifier>

<scope_note>References</scope_note>

</dc-type>

<dc-type>

<schema>datacite</schema>

<element>relation</element>

<qualifier>IsSupplementedBy</qualifier>

<scope_note>IsSupplementedBy</scope_note>

</dc-type>

<dc-type>

<schema>datacite</schema>

<element>relation</element>

<qualifier>IsContinuedBy</qualifier>

<scope_note>IsContinuedBy</scope_note>

</dc-type>

<dc-type>

<schema>datacite</schema>

<element>relation</element>

<qualifier>Continues</qualifier>

<scope_note>Continues</scope_note>

</dc-type>

<dc-type>

<schema>datacite</schema>

<element>relation</element>

<qualifier>IsDescribedBy</qualifier>

<scope_note>IsDescribedBy</scope_note>

<name>datacite-relations</name>

<namespace>http://datacite.org/schema/kernel-4</namespace>

</dc-schema>

<dc-type>

<schema>datacite-relations</schema>

<element>relation</element>

<qualifier>isCitedBy</qualifier>

<scope_note>isCitedBy</scope_note>

</dc-type>

<dc-type>

<schema>datacite-relations</schema>

<element>relation</element>

<qualifier>isReferencedBy</qualifier>

<scope_note>isReferencedBy</scope_note>

</dc-type>

<dc-type>

<schema>datacite-relations</schema>

<element>relation</element>

<qualifier>isSupplementTo</qualifier>

<scope_note>isSupplementTo</scope_note>

</dc-type>

<dc-type>

<schema>datacite-relations</schema>

<element>relation</element>

<qualifier>cites</qualifier>

<scope_note>cites</scope_note>

</dc-type>

<dc-type>

<schema>datacite-relations</schema>

<element>relation</element>

<qualifier>references</qualifier>

<scope_note>references</scope_note>

</dc-type>

<dc-type>

<schema>datacite-relations</schema>

<element>relation</element>

<qualifier>isSupplementedBy</qualifier>

<scope_note>isSupplementedBy</scope_note>

</dc-type>

<dc-type>

<schema>datacite-relations</schema>

<element>relation</element>

<qualifier>isContinuedBy</qualifier>

<scope_note>isContinuedBy</scope_note>

</dc-type>

<dc-type>

<schema>datacite-relations</schema>

<element>relation</element>

<qualifier>continues</qualifier>

<scope_note>continues</scope_note>

</dc-type>

<dc-type>

<schema>datacite-relations</schema>

<element>relation</element>

<qualifier>isDescribedBy</qualifier>

<scope_note>isDescribedBy</scope_note>

Copilot · 2026-03-20T15:11:30Z

dspace-api/src/main/java/org/dspace/external/CachingOrcidRestConnector.java

+            String authenticationParameters =
+                    String.format("client_id=%s&client_secret=%s&scope=/read-public&grant_type=client_credentials",
+                            clientId, clientSecret);
+
+            HttpRequest request = HttpRequest.newBuilder()
+                    .uri(java.net.URI.create(OAUTHUrl))
+                    .POST(HttpRequest.BodyPublishers.ofString(authenticationParameters))
+                    .timeout(Duration.ofSeconds(5))
+                    .header("Accept", "application/json")
+                    .header("Content-Type", "application/x-www-form-urlencoded")
+                    .build();


The form body for the token request is built via String.format without URL-encoding clientId/clientSecret. If either contains reserved characters (e.g. '&', '=' or '+'), the request will be malformed and authentication can fail. Build the POST body using proper form encoding (encode each value, or use a standard form-encoding utility) similar to OrcidClientImpl which uses UrlEncodedFormEntity.

Copilot · 2026-03-20T15:11:30Z

dspace-api/src/main/java/org/dspace/content/authority/SimpleORCIDAuthority.java

+
+        return key;
+    }
+    }


There is an extra closing brace here which prematurely ends the class/method block and will cause compilation to fail. Remove the stray '}' so the braces properly match the method/class structure.

Suggested change

}

Copilot AI review requested due to automatic review settings March 20, 2026 15:03

Copilot started reviewing on behalf of milanmajchrak March 20, 2026 15:03 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MENDELU/ORCID Authority#1292

MENDELU/ORCID Authority#1292
milanmajchrak wants to merge 1 commit intocustomer/mendelufrom
mendelu/ordic-redirect

milanmajchrak commented Mar 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	if (search.isOk() && search.numFound() > 0) {
	if (search.isOk() && !search.results().isEmpty()) {

		@@ -0,0 +1,24 @@
		## ORCID authority (https://wiki.lyrasis.org/display/DSDOC7x/ORCID+Authority) is bit cumbersome to use

Conversation

milanmajchrak commented Mar 20, 2026

Problem description

Analysis

Problems

Manual Testing (if applicable)

Copilot review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants