-
Notifications
You must be signed in to change notification settings - Fork 2
[Port to dtq-dev] Fix OpenAIRE integration: null handling and HTTP client lifecycle #1248
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.
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
Changes from all commits
e1acfa4
7566170
cf659da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,4 +8,5 @@ | |
| on JMockIt Expectations blocks and similar. See https://github.com/checkstyle/checkstyle/issues/3739 --> | ||
| <suppress checks="Indentation" files="src[/\\]test[/\\]java"/> | ||
| <suppress checks="Regexp" files="DSpaceHttpClientFactory\.java"/> | ||
| <suppress checks="Regexp" files="OpenAIRERestConnectorTest\.java"/> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why? but still approve
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test mocks DSpaceHttpClientFactory itself — to do that, it needs to create an HTTP client independently via HttpClientBuilder.create() and inject it into the mock. Using the factory to create the client that the same factory is supposed to return would defeat the purpose of the mock. The checkstyle rule banning HttpClientBuilder.create() is valid for production code, but this test case is a legitimate exception. |
||
| </suppressions> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| /** | ||
| * The contents of this file are subject to the license and copyright | ||
| * detailed in the LICENSE and NOTICE files at the root of the source | ||
| * tree and available online at | ||
| * | ||
| * http://www.dspace.org/license/ | ||
| */ | ||
| package org.dspace.external; | ||
|
|
||
| import static org.junit.Assert.assertTrue; | ||
| import static org.mockito.Mockito.doReturn; | ||
| import static org.mockito.Mockito.spy; | ||
| import static org.mockito.Mockito.when; | ||
|
|
||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.nio.charset.StandardCharsets; | ||
|
|
||
| import eu.openaire.jaxb.model.Response; | ||
| import okhttp3.mockwebserver.MockResponse; | ||
| import okhttp3.mockwebserver.MockWebServer; | ||
| import org.apache.http.client.methods.HttpGet; | ||
| import org.apache.http.impl.client.CloseableHttpClient; | ||
| import org.apache.http.impl.client.HttpClientBuilder; | ||
| import org.dspace.app.client.DSpaceHttpClientFactory; | ||
| import org.junit.Test; | ||
| import org.mockito.MockedStatic; | ||
| import org.mockito.Mockito; | ||
|
|
||
|
|
||
| public class OpenAIRERestConnectorTest { | ||
|
|
||
| @Test | ||
| public void searchProjectByKeywords() { | ||
| try (InputStream is = this.getClass().getResourceAsStream("openaire-projects.xml"); | ||
| MockWebServer mockServer = new MockWebServer()) { | ||
| String projects = new String(is.readAllBytes(), StandardCharsets.UTF_8) | ||
| .replaceAll("( mushroom)", "( DEADBEEF)"); | ||
milanmajchrak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| mockServer.enqueue(new MockResponse().setResponseCode(200).setBody(projects)); | ||
|
|
||
| // setup mocks so we don't have to set whole DSpace kernel etc. | ||
| // still, the idea is to test how the get method behaves | ||
| CloseableHttpClient httpClient = spy(HttpClientBuilder.create().build()); | ||
| doReturn(httpClient.execute(new HttpGet(mockServer.url("").toString()))) | ||
| .when(httpClient).execute(Mockito.any()); | ||
milanmajchrak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| DSpaceHttpClientFactory mock = Mockito.mock(DSpaceHttpClientFactory.class); | ||
| when(mock.build()).thenReturn(httpClient); | ||
|
|
||
| try (MockedStatic<DSpaceHttpClientFactory> mockedFactory = | ||
| Mockito.mockStatic(DSpaceHttpClientFactory.class)) { | ||
| mockedFactory.when(DSpaceHttpClientFactory::getInstance).thenReturn(mock); | ||
| OpenAIRERestConnector connector = new OpenAIRERestConnector(mockServer.url("").toString()); | ||
| Response response = connector.searchProjectByKeywords(0, 10, "keyword"); | ||
| // Basically check it doesn't throw UnmarshallerException and that we are getting our mocked response | ||
| assertTrue("Expected the query to contain the replaced keyword", | ||
| response.getHeader().getQuery().contains("DEADBEEF")); | ||
| } | ||
| } catch (IOException e) { | ||
| e.printStackTrace(); | ||
| } | ||
milanmajchrak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.