Skip to content

Commit d4e9c93

Browse files
kosarkoCopilotmilanmajchrak
committed
[Port to dtq-dev] Fix OpenAIRE integration: null handling and HTTP client lifecycle (#1248)
* Fix OpenAIRE integration: null handling and HTTP client lifecycle (ufal#1330) * Add test for OpenAIRE connector * Initial plan * Add null check for OpenAIRE response to prevent NullPointerException Co-authored-by: kosarko <1842385+kosarko@users.noreply.github.com> * Fix HTTP client lifecycle to prevent premature connection closure Co-authored-by: kosarko <1842385+kosarko@users.noreply.github.com> * Keep the try with resources but copy the response into an in memory stream and return that * license:check --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kosarko <1842385+kosarko@users.noreply.github.com> (cherry picked from commit 02984db) * Handle NumberFormatException in OpenAIREFundingDataProvider.getNumberOfResults and use explicit UTF-8 charset in OpenAIRERestConnectorTest --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: kosarko <1842385+kosarko@users.noreply.github.com> Co-authored-by: milanmajchrak <milan.majchrak@dataquest.sk>
1 parent a3e6ecd commit d4e9c93

5 files changed

Lines changed: 101 additions & 3 deletions

File tree

checkstyle-suppressions.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,5 @@
88
on JMockIt Expectations blocks and similar. See https://github.com/checkstyle/checkstyle/issues/3739 -->
99
<suppress checks="Indentation" files="src[/\\]test[/\\]java"/>
1010
<suppress checks="Regexp" files="DSpaceHttpClientFactory\.java"/>
11+
<suppress checks="Regexp" files="OpenAIRERestConnectorTest\.java"/>
1112
</suppressions>

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,11 @@ public InputStream get(String file, String accessToken) {
206206
break;
207207
}
208208

209-
// do not close this httpClient
210-
result = getResponse.getEntity().getContent();
209+
// the client will be closed, we need to copy the response stream to a new one that we can return
210+
try (InputStream is = getResponse.getEntity().getContent()) {
211+
byte[] bytes = is.readAllBytes();
212+
result = new java.io.ByteArrayInputStream(bytes);
213+
}
211214
}
212215
} catch (MalformedURLException e1) {
213216
getGotError(e1, url + '/' + file);

dspace-api/src/main/java/org/dspace/external/provider/impl/OpenaireFundingDataProvider.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,19 @@ public int getNumberOfResults(String query) {
169169
String encodedQuery = encodeValue(query);
170170

171171
Response projectResponse = connector.searchProjectByKeywords(0, 0, encodedQuery);
172-
return Integer.parseInt(projectResponse.getHeader().getTotal());
172+
if (projectResponse == null || projectResponse.getHeader() == null) {
173+
return 0;
174+
}
175+
String total = projectResponse.getHeader().getTotal();
176+
if (StringUtils.isBlank(total)) {
177+
return 0;
178+
}
179+
try {
180+
return Integer.parseInt(total);
181+
} catch (NumberFormatException e) {
182+
log.error("Failed to parse search result count from OpenAIRE: {}", e.getMessage());
183+
return 0;
184+
}
173185
}
174186

175187
/**
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
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.external;
9+
10+
import static org.junit.Assert.assertTrue;
11+
import static org.mockito.Mockito.doReturn;
12+
import static org.mockito.Mockito.spy;
13+
import static org.mockito.Mockito.when;
14+
15+
import java.io.IOException;
16+
import java.io.InputStream;
17+
import java.nio.charset.StandardCharsets;
18+
19+
import eu.openaire.jaxb.model.Response;
20+
import okhttp3.mockwebserver.MockResponse;
21+
import okhttp3.mockwebserver.MockWebServer;
22+
import org.apache.http.client.methods.HttpGet;
23+
import org.apache.http.impl.client.CloseableHttpClient;
24+
import org.apache.http.impl.client.HttpClientBuilder;
25+
import org.dspace.app.client.DSpaceHttpClientFactory;
26+
import org.junit.Test;
27+
import org.mockito.MockedStatic;
28+
import org.mockito.Mockito;
29+
30+
31+
public class OpenAIRERestConnectorTest {
32+
33+
@Test
34+
public void searchProjectByKeywords() {
35+
try (InputStream is = this.getClass().getResourceAsStream("openaire-projects.xml");
36+
MockWebServer mockServer = new MockWebServer()) {
37+
String projects = new String(is.readAllBytes(), StandardCharsets.UTF_8)
38+
.replaceAll("( mushroom)", "( DEADBEEF)");
39+
mockServer.enqueue(new MockResponse().setResponseCode(200).setBody(projects));
40+
41+
// setup mocks so we don't have to set whole DSpace kernel etc.
42+
// still, the idea is to test how the get method behaves
43+
CloseableHttpClient httpClient = spy(HttpClientBuilder.create().build());
44+
doReturn(httpClient.execute(new HttpGet(mockServer.url("").toString())))
45+
.when(httpClient).execute(Mockito.any());
46+
47+
DSpaceHttpClientFactory mock = Mockito.mock(DSpaceHttpClientFactory.class);
48+
when(mock.build()).thenReturn(httpClient);
49+
50+
try (MockedStatic<DSpaceHttpClientFactory> mockedFactory =
51+
Mockito.mockStatic(DSpaceHttpClientFactory.class)) {
52+
mockedFactory.when(DSpaceHttpClientFactory::getInstance).thenReturn(mock);
53+
OpenAIRERestConnector connector = new OpenAIRERestConnector(mockServer.url("").toString());
54+
Response response = connector.searchProjectByKeywords(0, 10, "keyword");
55+
// Basically check it doesn't throw UnmarshallerException and that we are getting our mocked response
56+
assertTrue("Expected the query to contain the replaced keyword",
57+
response.getHeader().getQuery().contains("DEADBEEF"));
58+
}
59+
} catch (IOException e) {
60+
e.printStackTrace();
61+
}
62+
}
63+
}

dspace-api/src/test/java/org/dspace/external/provider/impl/OpenaireFundingDataProviderTest.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414
import java.util.List;
1515
import java.util.Optional;
1616

17+
import eu.openaire.jaxb.model.Response;
1718
import org.dspace.AbstractDSpaceTest;
19+
import org.dspace.external.OpenAIRERestConnector;
1820
import org.dspace.external.factory.ExternalServiceFactory;
1921
import org.dspace.external.model.ExternalDataObject;
2022
import org.dspace.external.provider.ExternalDataProvider;
@@ -102,4 +104,21 @@ public void testGetDataObjectWInvalidId() {
102104

103105
assertTrue("openaireFunding.getExternalDataObject.notExists:WRONGID", result.isEmpty());
104106
}
107+
108+
@Test
109+
public void testGetNumberOfResultsWhenResponseIsNull() {
110+
// Create a mock connector that returns null
111+
OpenAIREFundingDataProvider provider = new OpenAIREFundingDataProvider();
112+
provider.setSourceIdentifier("test");
113+
provider.setConnector(new OpenAIRERestConnector("test") {
114+
@Override
115+
public Response searchProjectByKeywords(int page, int size, String... keywords) {
116+
return null;
117+
}
118+
});
119+
120+
// Should return 0 when response is null, not throw NullPointerException
121+
int result = provider.getNumberOfResults("test");
122+
assertEquals("Should return 0 when response is null", 0, result);
123+
}
105124
}

0 commit comments

Comments
 (0)