Skip to content

Commit d32fae8

Browse files
Micheleboychukvins01-4science
Micheleboychuk
authored and
vins01-4science
committed
Merged in task/dspace-cris-2024_02_x/DSC-2848 (pull request DSpace#5803)
DSC-2848: [2024] Improve OpenPolicyFinderService & OpenPolicyFinderAuthority Approved-by: Vincenzo Mecca
2 parents 8d6b43a + 419f41b commit d32fae8

7 files changed

Lines changed: 222 additions & 30 deletions

File tree

‎dspace-api/src/main/java/org/dspace/app/openpolicyfinder/OpenPolicyFinderService.java‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,9 @@ public OpenPolicyFinderPublisherResponse performPublisherRequest(String type, St
144144
sleepBetweenTimeouts));
145145

146146
try (CloseableHttpClient client = DSpaceHttpClientFactory.getInstance().buildWithoutAutomaticRetries(5)) {
147-
Thread.sleep(sleepBetweenTimeouts);
148-
147+
if (numberOfTries > 1) {
148+
Thread.sleep(sleepBetweenTimeouts);
149+
}
149150
// Construct a default HTTP method (first result)
150151
method = constructHttpGet(type, field, predicate, value, start, limit);
151152

@@ -160,6 +161,7 @@ public OpenPolicyFinderPublisherResponse performPublisherRequest(String type, St
160161
+ statusCode);
161162
String errorBody = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8);
162163
log.error("Error from Open Policy Finder HTTP request: " + errorBody);
164+
continue;
163165
}
164166

165167
HttpEntity responseBody = response.getEntity();
@@ -251,7 +253,9 @@ public OpenPolicyFinderResponse performRequest(String type, String field, String
251253
sleepBetweenTimeouts));
252254

253255
try (CloseableHttpClient client = DSpaceHttpClientFactory.getInstance().buildWithoutAutomaticRetries(5)) {
254-
Thread.sleep(sleepBetweenTimeouts);
256+
if (numberOfTries > 1) {
257+
Thread.sleep(sleepBetweenTimeouts);
258+
}
255259

256260
// Construct a default HTTP method (first result)
257261
method = constructHttpGet(type, field, predicate, value, start, limit);
@@ -260,14 +264,15 @@ public OpenPolicyFinderResponse performRequest(String type, String field, String
260264
try (CloseableHttpResponse response = client.execute(method)) {
261265
int statusCode = response.getStatusLine().getStatusCode();
262266

263-
log.debug(response.getStatusLine().getStatusCode() + ": "
264-
+ response.getStatusLine().getReasonPhrase());
267+
log.debug(statusCode
268+
+ ": " + response.getStatusLine().getReasonPhrase());
265269

266270
if (statusCode != HttpStatus.SC_OK) {
267271
opfResponse = new OpenPolicyFinderResponse("Open Policy Finder return not OK status: "
268272
+ statusCode);
269273
String errorBody = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8);
270274
log.error("Error from Open Policy Finder HTTP request: " + errorBody);
275+
continue;
271276
}
272277

273278
HttpEntity responseBody = response.getEntity();

‎dspace-api/src/main/java/org/dspace/content/authority/ItemAuthority.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public class ItemAuthority implements ChoiceAuthority, LinkableEntityAuthority {
5858
final static String CHOICES_EXTERNALSOURCE_PREFIX = "choises.externalsource.";
5959

6060
/** the name assigned to the specific instance by the PluginService, @see {@link NameAwarePlugin} **/
61-
private String authorityName;
61+
protected String authorityName;
6262

6363
protected DSpace dspace = new DSpace();
6464

@@ -78,7 +78,7 @@ public class ItemAuthority implements ChoiceAuthority, LinkableEntityAuthority {
7878
private ExternalDataService externalDataService = ExternalServiceFactory.getInstance().getExternalDataService();
7979

8080
// map of field key to presentation type
81-
protected Map<String, String> externalSource = new HashMap<String, String>();
81+
protected Map<String, String> externalSource = new HashMap<>();
8282

8383
public static final String DEFAULT = "local";
8484

@@ -274,7 +274,6 @@ public Map<String, String> getExternalSource() {
274274
} else {
275275
log.warn("Skipping invalid external source authority configuration property: " + sourceIdentifier +
276276
" does not exist");
277-
continue;
278277
}
279278
}
280279
}
@@ -288,7 +287,7 @@ public Map<String, String> getExtra(String key, String locale) {
288287
SolrClient solr = searchService.getSolrSearchCore().getSolr();
289288
if (Objects.isNull(solr)) {
290289
log.error("unable to find solr instance");
291-
return new HashMap<String, String>();
290+
return new HashMap<>();
292291
}
293292

294293

@@ -305,7 +304,7 @@ public Map<String, String> getExtra(String key, String locale) {
305304
List<Choice> choiceList = getChoiceListFromQueryResults(queryResponse.getResults(), key, false);
306305
if (choiceList.isEmpty()) {
307306
log.warn("No documents found for key=" + key);
308-
return new HashMap<String, String>();
307+
return new HashMap<>();
309308
}
310309

311310
return choiceList.iterator().next().extras;
@@ -314,7 +313,7 @@ public Map<String, String> getExtra(String key, String locale) {
314313
log.error(e.getMessage(), e);
315314
}
316315

317-
return new HashMap<String, String>();
316+
return new HashMap<>();
318317
}
319318

320319
protected int calculateConfidence(Choice[] choices) {

‎dspace-api/src/main/java/org/dspace/content/authority/OpenPolicyFinderAuthority.java‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import org.dspace.app.openpolicyfinder.OpenPolicyFinderService;
2222
import org.dspace.app.openpolicyfinder.v2.OpenPolicyFinderJournal;
2323
import org.dspace.app.openpolicyfinder.v2.OpenPolicyFinderResponse;
24-
import org.dspace.core.NameAwarePlugin;
2524
import org.dspace.services.ConfigurationService;
2625
import org.dspace.services.factory.DSpaceServicesFactory;
2726
import org.dspace.utils.DSpace;
@@ -41,12 +40,6 @@ public class OpenPolicyFinderAuthority extends ItemAuthority {
4140
private static final String PREDICATE_EQUALS = "equals";
4241
private static final String PREDICATE_CONTAINS_WORD = "contains word";
4342

44-
/**
45-
* the name assigned to the specific instance by the PluginService, @see
46-
* {@link NameAwarePlugin}
47-
**/
48-
private String authorityName;
49-
5043
private ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService();
5144

5245
private DSpace dspace = new DSpace();

‎dspace-api/src/main/java/org/dspace/content/authority/RorOrgUnitAuthority.java‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,6 @@ public class RorOrgUnitAuthority extends ItemAuthority {
3737
DSpaceServicesFactory.getInstance().getConfigurationService();
3838
private final PluginService pluginService = CoreServiceFactory.getInstance().getPluginService();
3939

40-
private String authorityName;
41-
4240
@Override
4341
public Choices getMatches(String text, int start, int limit, String locale) {
4442

@@ -187,14 +185,4 @@ public String getPrimaryLinkedEntityType() {
187185

188186
return null;
189187
}
190-
191-
@Override
192-
public void setPluginInstanceName(String name) {
193-
authorityName = name;
194-
}
195-
196-
@Override
197-
public String getPluginInstanceName() {
198-
return authorityName;
199-
}
200188
}

‎dspace-api/src/test/java/org/dspace/content/authority/ItemAuthorityTest.java‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/**
2+
* The contents of this file are subject to the license and copyright
3+
* detailed in the LICENSE and NOTICE files at the root of the source
4+
* tree and available online at
5+
*
6+
* http://www.dspace.org/license/
7+
*/
8+
package org.dspace.content.authority;
9+
10+
import static org.junit.Assert.assertEquals;
11+
12+
import org.dspace.AbstractUnitTest;
13+
import org.dspace.services.ConfigurationService;
14+
import org.dspace.services.factory.DSpaceServicesFactory;
15+
import org.junit.Before;
16+
import org.junit.Test;
17+
18+
/**
19+
* Unit tests for ItemAuthority refactored code (DSC-2848)
20+
* Tests getSource() method, protected fields, and inheritance.
21+
*/
22+
public class ItemAuthorityTest extends AbstractUnitTest {
23+
24+
private ConfigurationService configurationService;
25+
private ItemAuthority itemAuthority;
26+
27+
@Before
28+
public void setUp() {
29+
configurationService = DSpaceServicesFactory.getInstance().getConfigurationService();
30+
itemAuthority = new ItemAuthority();
31+
itemAuthority.setPluginInstanceName("testAuthority");
32+
}
33+
34+
/**
35+
* Test that getSource() returns configured value when set.
36+
*/
37+
@Test
38+
public void testGetSourceWithConfiguredValue() {
39+
configurationService.setProperty("cris.ItemAuthority.testAuthority.source", "openpolicyfinder");
40+
String source = itemAuthority.getSource();
41+
assertEquals("Source should match configured value", "openpolicyfinder", source);
42+
}
43+
44+
/**
45+
* Test that getSource() returns default "local" when not configured.
46+
*/
47+
@Test
48+
public void testGetSourceWithDefaultValue() {
49+
String source = itemAuthority.getSource();
50+
assertEquals("Source should default to 'local'", "local", source);
51+
}
52+
53+
/**
54+
* Test that authorityName is accessible to subclasses (protected field).
55+
*/
56+
@Test
57+
public void testAuthorityNameIsProtected() {
58+
itemAuthority.setPluginInstanceName("testProtectedAccess");
59+
assertEquals("Authority name should be accessible", "testProtectedAccess",
60+
itemAuthority.getPluginInstanceName());
61+
// Verify the field is protected by checking subclass access
62+
OpenPolicyFinderAuthority subAuthority = new OpenPolicyFinderAuthority();
63+
subAuthority.setPluginInstanceName("subAuthority");
64+
assertEquals("Subclass should access protected authorityName", "subAuthority",
65+
subAuthority.getPluginInstanceName());
66+
}
67+
}

‎dspace-api/src/test/java/org/dspace/content/authority/OpenPolicyFinderAuthorityTest.java‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/**
2+
* The contents of this file are subject to the license and copyright
3+
* detailed in the LICENSE and NOTICE files at the root of the source
4+
* tree and available online at
5+
*
6+
* http://www.dspace.org/license/
7+
*/
8+
package org.dspace.content.authority;
9+
10+
import static org.junit.Assert.assertEquals;
11+
import static org.junit.Assert.assertNotNull;
12+
13+
import org.dspace.AbstractUnitTest;
14+
import org.dspace.services.ConfigurationService;
15+
import org.dspace.services.factory.DSpaceServicesFactory;
16+
import org.junit.Before;
17+
import org.junit.Test;
18+
19+
/**
20+
* Unit tests for OpenPolicyFinderAuthority refactoring (DSC-2848)
21+
* Validates:
22+
* - No duplicate authorityName field (inherits from ItemAuthority)
23+
* - getSource() works via inheritance from ItemAuthority
24+
* - No unused imports (validated by compile)
25+
*/
26+
public class OpenPolicyFinderAuthorityTest extends AbstractUnitTest {
27+
28+
private ConfigurationService configurationService;
29+
private OpenPolicyFinderAuthority authority;
30+
31+
@Before
32+
public void setUp() {
33+
configurationService = DSpaceServicesFactory.getInstance().getConfigurationService();
34+
authority = new OpenPolicyFinderAuthority();
35+
authority.setPluginInstanceName("testOPFAuthority");
36+
}
37+
38+
/**
39+
* Test that getSource() returns configured value via inheritance from ItemAuthority.
40+
*/
41+
@Test
42+
public void testGetSourceInheritance() {
43+
// Test with configured value
44+
configurationService.setProperty("cris.ItemAuthority.testOPFAuthority.source", "openpolicyfinder");
45+
String source = authority.getSource();
46+
assertEquals("getSource() should return configured value via inheritance", "openpolicyfinder", source);
47+
48+
// Test default value
49+
configurationService.setProperty("cris.ItemAuthority.testOPFAuthority.source", null);
50+
source = authority.getSource();
51+
assertEquals("getSource() should return default 'local'", "local", source);
52+
}
53+
54+
/**
55+
* Test that authorityName is inherited from ItemAuthority (no duplicate field).
56+
*/
57+
@Test
58+
public void testAuthorityNameInheritance() {
59+
authority.setPluginInstanceName("testName");
60+
assertEquals("authorityName should be inherited from ItemAuthority",
61+
"testName", authority.getPluginInstanceName());
62+
}
63+
64+
/**
65+
* Test that convertToChoice uses getSource() correctly.
66+
* Note: This is an integration test that requires mock OpenPolicyFinderService.
67+
*/
68+
@Test
69+
public void testConvertToChoiceUsesGetSource() {
70+
// Verify that the method exists and returns non-null source
71+
String source = authority.getSource();
72+
assertNotNull("getSource() should not return null", source);
73+
}
74+
}

‎dspace-api/src/test/java/org/dspace/content/authority/RorOrgUnitAuthorityTest.java‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
/**
2+
* The contents of this file are subject to the license and copyright
3+
* detailed in the LICENSE and NOTICE files at the root of the source
4+
* tree and available online at
5+
*
6+
* http://www.dspace.org/license/
7+
*/
8+
package org.dspace.content.authority;
9+
10+
import static org.junit.Assert.assertEquals;
11+
import static org.junit.Assert.assertNull;
12+
13+
import org.dspace.AbstractUnitTest;
14+
import org.dspace.services.ConfigurationService;
15+
import org.dspace.services.factory.DSpaceServicesFactory;
16+
import org.junit.Before;
17+
import org.junit.Test;
18+
19+
public class RorOrgUnitAuthorityTest extends AbstractUnitTest {
20+
21+
private RorOrgUnitAuthority authority;
22+
private ConfigurationService configurationService;
23+
24+
@Before
25+
public void setUp() throws Exception {
26+
authority = new RorOrgUnitAuthority();
27+
authority.setPluginInstanceName("RorOrgUnitAuthority");
28+
configurationService = DSpaceServicesFactory.getInstance().getConfigurationService();
29+
}
30+
31+
@Test
32+
public void testGetSourceDefault() {
33+
String source = authority.getSource();
34+
assertEquals("local", source);
35+
}
36+
37+
@Test
38+
public void testGetSourceConfigured() {
39+
configurationService.setProperty("cris.ItemAuthority.RorOrgUnitAuthority.source", "ror");
40+
String source = authority.getSource();
41+
assertEquals("ror", source);
42+
configurationService.setProperty("cris.ItemAuthority.RorOrgUnitAuthority.source", null);
43+
}
44+
45+
@Test
46+
public void testAuthorityNameInheritance() {
47+
authority.setPluginInstanceName("testRorAuthority");
48+
assertEquals("testRorAuthority", authority.getPluginInstanceName());
49+
}
50+
51+
@Test
52+
public void testGetLinkedEntityTypes() {
53+
String[] entityTypes = authority.getLinkedEntityTypes();
54+
// May return null or array depending on configuration
55+
// Just verify the method doesn't throw an exception
56+
if (entityTypes != null) {
57+
assertEquals("RorOrgUnitAuthority", authority.getPluginInstanceName());
58+
}
59+
}
60+
61+
@Test
62+
public void testGetPrimaryLinkedEntityTypeDefault() {
63+
String primaryType = authority.getPrimaryLinkedEntityType();
64+
assertNull(primaryType);
65+
}
66+
}

0 commit comments

Comments
 (0)