Skip to content

Commit 4758577

Browse files
julianladischjulianladisch
committed
MODHAADM-105: Unhandled exception when reading PURGE_LOGS_AFTER
https://folio-org.atlassian.net/browse/MODHAADM-105 Parsing the response may fail when doing "new JsonObject(response)", "getJsonArray" or "getJsonObject". The parse error is not handled because the code is in "onSuccess(", therefor the request hangs forever. Fix: Use map, not onSuccess. Avoid Promise that almost always is wrong and can be replaced with Future returning calls. In addition fix CQL and percent encoding.
1 parent 3fb2942 commit 4758577

File tree

6 files changed

+211
-62
lines changed

6 files changed

+211
-62
lines changed

‎pom.xml

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
<properties>
88
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
99
<okapi.version>5.3.0</okapi.version>
10+
<rmb.version>35.3.0</rmb.version>
1011
<vertx.version>4.5.3</vertx.version>
1112
<maven.compiler.source>18</maven.compiler.source>
1213
<maven.compiler.target>18</maven.compiler.target>
@@ -37,6 +38,11 @@
3738
<artifactId>okapi-testing</artifactId>
3839
<version>${okapi.version}</version>
3940
</dependency>
41+
<dependency>
42+
<groupId>org.folio</groupId>
43+
<artifactId>util</artifactId>
44+
<version>${rmb.version}</version>
45+
</dependency>
4046
<dependency>
4147
<groupId>com.ongres.scram</groupId>
4248
<artifactId>client</artifactId>
@@ -158,6 +164,10 @@
158164
<groupId>org.folio.okapi</groupId>
159165
<artifactId>okapi-common</artifactId>
160166
</dependency>
167+
<dependency>
168+
<groupId>org.folio</groupId>
169+
<artifactId>util</artifactId>
170+
</dependency>
161171
<dependency>
162172
<groupId>io.netty</groupId>
163173
<artifactId>netty-transport-native-epoll</artifactId>
@@ -169,16 +179,36 @@
169179
<scope>runtime</scope>
170180
</dependency>
171181
<!-- Test dependencies -->
182+
<dependency>
183+
<groupId>org.junit.jupiter</groupId>
184+
<artifactId>junit-jupiter-api</artifactId>
185+
<scope>test</scope>
186+
</dependency>
172187
<dependency>
173188
<groupId>junit</groupId>
174189
<artifactId>junit</artifactId>
175190
<scope>test</scope>
176191
</dependency>
192+
<dependency>
193+
<groupId>org.junit.jupiter</groupId>
194+
<artifactId>junit-jupiter-engine</artifactId>
195+
<scope>test</scope>
196+
</dependency>
197+
<dependency>
198+
<groupId>org.junit.vintage</groupId>
199+
<artifactId>junit-vintage-engine</artifactId>
200+
<scope>test</scope>
201+
</dependency>
177202
<dependency>
178203
<groupId>io.vertx</groupId>
179204
<artifactId>vertx-unit</artifactId>
180205
<scope>test</scope>
181206
</dependency>
207+
<dependency>
208+
<groupId>io.vertx</groupId>
209+
<artifactId>vertx-junit5</artifactId>
210+
<scope>test</scope>
211+
</dependency>
182212
<dependency>
183213
<groupId>io.rest-assured</groupId>
184214
<artifactId>rest-assured</artifactId>
@@ -194,6 +224,12 @@
194224
<artifactId>mockito-core</artifactId>
195225
<scope>test</scope>
196226
</dependency>
227+
<dependency>
228+
<groupId>org.wiremock</groupId>
229+
<artifactId>wiremock</artifactId>
230+
<version>3.9.2</version>
231+
<scope>test</scope>
232+
</dependency>
197233
</dependencies>
198234
<build>
199235
<pluginManagement>

‎src/main/java/org/folio/harvesteradmin/foliodata/ConfigurationsClient.java

Lines changed: 27 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,38 @@
11
package org.folio.harvesteradmin.foliodata;
22

3+
import static org.folio.util.StringUtil.cqlEncode;
4+
35
import io.vertx.core.Future;
4-
import io.vertx.core.json.JsonArray;
56
import io.vertx.core.json.JsonObject;
67
import io.vertx.ext.web.RoutingContext;
7-
import io.vertx.reactivex.core.Promise;
88
import org.apache.logging.log4j.LogManager;
99
import org.apache.logging.log4j.Logger;
10+
import org.folio.util.PercentCodec;
1011

11-
import java.net.URLEncoder;
12-
import java.nio.charset.StandardCharsets;
13-
14-
12+
/**
13+
* Run GET /configurations/entries HTTP request to get configuration entry from mod-configuration.
14+
*/
1515
public class ConfigurationsClient {
16-
public static final String CONFIGURATIONS_PATH = "/configurations/entries";
17-
public static final String RECORDS = "configs";
18-
19-
protected static final Logger logger =
20-
LogManager.getLogger(ConfigurationsClient.class);
21-
22-
public static Future<String> getStringValue (RoutingContext routingContext, String moduleName, String configName) {
23-
String query = "module==" + moduleName + " and configName==" + configName + " and enabled=true";
24-
Promise<String> promise = Promise.promise();
25-
Folio.okapiClient(routingContext).get(CONFIGURATIONS_PATH +
26-
"?query=(" + URLEncoder.encode(query, StandardCharsets.UTF_8) +")")
27-
.onSuccess(response -> {
28-
JsonObject json = new JsonObject(response);
29-
JsonArray entries = json.getJsonArray(RECORDS);
30-
if (entries.isEmpty()) {
31-
promise.complete(null);
32-
33-
} else {
34-
JsonObject entry = entries.getJsonObject(0);
35-
promise.complete(entry.getString("value"));
36-
}
37-
}).onFailure(response -> {
38-
logger.info("Could not obtain settings by module " + moduleName + " and config " + configName + ": " + response.getMessage());
39-
promise.complete(null);
40-
});
41-
return promise.future();
42-
}
16+
public static final String CONFIGURATIONS_PATH = "/configurations/entries";
17+
public static final String RECORDS = "configs";
18+
19+
protected static final Logger logger =
20+
LogManager.getLogger(ConfigurationsClient.class);
21+
22+
/**
23+
* Get the configuration value from mod-configuration for the module and configName.
24+
*/
25+
public static Future<String> getStringValue(RoutingContext routingContext,
26+
String moduleName, String configName) {
27+
28+
var cql = "module==" + cqlEncode(moduleName) + " and configName==" + cqlEncode(configName)
29+
+ " and enabled=true";
30+
return Folio.okapiClient(routingContext)
31+
.get(CONFIGURATIONS_PATH + "?query=" + PercentCodec.encode(cql))
32+
.map(response ->
33+
new JsonObject(response).getJsonArray(RECORDS).getJsonObject(0).getString("value"))
34+
.onFailure(e -> logger.error("Could not obtain settings by module " + moduleName
35+
+ " and config " + configName + ": " + e.getMessage()));
36+
}
4337

4438
}

‎src/main/java/org/folio/harvesteradmin/foliodata/SettingsClient.java

Lines changed: 24 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,38 @@
11
package org.folio.harvesteradmin.foliodata;
22

3+
import static org.folio.util.StringUtil.cqlEncode;
4+
35
import io.vertx.core.Future;
4-
import io.vertx.core.json.JsonArray;
56
import io.vertx.core.json.JsonObject;
67
import io.vertx.ext.web.RoutingContext;
7-
import io.vertx.reactivex.core.Promise;
88
import org.apache.logging.log4j.LogManager;
99
import org.apache.logging.log4j.Logger;
10+
import org.folio.util.PercentCodec;
1011

11-
import java.net.URLEncoder;
12-
import java.nio.charset.StandardCharsets;
13-
12+
/**
13+
* Run GET /settings/entries HTTP request to get a setting from mod-settings.
14+
*/
1415
public class SettingsClient {
15-
public static final String SETTINGS_PATH = "/settings/entries";
16-
public static final String RECORDS = "items";
16+
public static final String SETTINGS_PATH = "/settings/entries";
17+
public static final String RECORDS = "items";
1718

18-
protected static final Logger logger =
19-
LogManager.getLogger(SettingsClient.class);
19+
protected static final Logger logger =
20+
LogManager.getLogger(SettingsClient.class);
2021

21-
public static Future<String> getStringValue (RoutingContext routingContext, String scope, String key) {
22-
String query = "scope==" + scope + " and key==" + key;
23-
Promise<String> promise = Promise.promise();
24-
Folio.okapiClient(routingContext).get(SETTINGS_PATH +
25-
"?query=(" + URLEncoder.encode(query, StandardCharsets.UTF_8) +")")
26-
.onSuccess(response -> {
27-
JsonObject json = new JsonObject(response);
28-
JsonArray entries = json.getJsonArray(RECORDS);
29-
if (entries.isEmpty()) {
30-
promise.complete(null);
22+
/**
23+
* Get the settings value from mod-settings for the scope and key.
24+
*/
25+
public static Future<String> getStringValue(RoutingContext routingContext,
26+
String scope, String key) {
3127

32-
} else {
33-
JsonObject entry = entries.getJsonObject(0);
34-
promise.complete(entry.getString("value"));
35-
}
36-
}).onFailure(response -> {
37-
logger.error("Could not obtain settings by scope " + scope + " and key " + key + ": " + response.getMessage());
38-
promise.complete(null);
39-
});
40-
return promise.future();
41-
}
28+
var cql = "scope==" + cqlEncode(scope) + " and key==" + cqlEncode(key);
29+
return Folio.okapiClient(routingContext)
30+
.get(SETTINGS_PATH + "?query=" + PercentCodec.encode(cql))
31+
.map(response ->
32+
new JsonObject(response).getJsonArray(RECORDS).getJsonObject(0).getString("value"))
33+
.onFailure(e ->
34+
logger.error("Could not obtain settings by scope " + scope
35+
+ " and key " + key + ": " + e.getMessage()));
36+
}
4237

4338
}

‎src/test/java/org/folio/harvesteradmin/foliodata/ClientTestBase.java

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
package org.folio.harvesteradmin.foliodata;
2+
3+
import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl;
4+
import static com.github.tomakehurst.wiremock.client.WireMock.get;
5+
import static com.github.tomakehurst.wiremock.client.WireMock.ok;
6+
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
7+
import static org.hamcrest.MatcherAssert.assertThat;
8+
import static org.hamcrest.Matchers.containsString;
9+
import static org.hamcrest.Matchers.instanceOf;
10+
import static org.hamcrest.Matchers.is;
11+
import static org.junit.jupiter.api.Assertions.assertThrows;
12+
import static org.mockito.Mockito.mock;
13+
import static org.mockito.Mockito.when;
14+
15+
import java.util.concurrent.ExecutionException;
16+
17+
import org.junit.jupiter.api.Test;
18+
import org.junit.jupiter.api.Timeout;
19+
import org.junit.jupiter.api.extension.ExtendWith;
20+
import com.github.tomakehurst.wiremock.client.WireMock;
21+
import com.github.tomakehurst.wiremock.junit5.WireMockTest;
22+
23+
import io.vertx.core.Future;
24+
import io.vertx.core.Vertx;
25+
import io.vertx.core.http.HttpServerRequest;
26+
import io.vertx.core.http.impl.headers.HeadersMultiMap;
27+
import io.vertx.ext.web.RoutingContext;
28+
import io.vertx.junit5.VertxExtension;
29+
30+
@ExtendWith(VertxExtension.class)
31+
@Timeout(value = 5)
32+
@WireMockTest(httpPort=9130)
33+
abstract class ClientTestBase {
34+
35+
@Test
36+
void success(Vertx vertx) {
37+
var routingContext = routingContext(vertx);
38+
assertThat(getStringValue(routingContext), is("val"));
39+
}
40+
41+
@Test
42+
void emptyList(Vertx vertx) {
43+
var routingContext = routingContext(vertx);
44+
stubFor(get(anyUrl()).willReturn(ok("""
45+
{ "items": [], "configs": [] }
46+
""")));
47+
var e = assertThrows(RuntimeException.class, () -> getStringValue(routingContext));
48+
assertThat(e.getCause().getCause(), is(instanceOf(IndexOutOfBoundsException.class)));
49+
}
50+
51+
@Test
52+
void emptyResponse(Vertx vertx) {
53+
var routingContext = routingContext(vertx);
54+
stubFor(get(anyUrl()).willReturn(ok()));
55+
var e = assertThrows(RuntimeException.class, () -> getStringValue(routingContext));
56+
assertThat(e.getCause().getCause(), is(instanceOf(NullPointerException.class)));
57+
}
58+
59+
@Test
60+
void notFound(Vertx vertx) {
61+
var routingContext = routingContext(vertx);
62+
stubFor(get(anyUrl()).willReturn(WireMock.notFound()));
63+
var e = assertThrows(RuntimeException.class, () -> getStringValue(routingContext));
64+
assertThat(e.getMessage(), containsString("404"));
65+
}
66+
67+
abstract Future<String> getStringValueFuture(RoutingContext routingContext, String key1, String key2);
68+
69+
String getStringValue(RoutingContext routingContext) {
70+
try {
71+
return getStringValueFuture(routingContext, "mod-x", "lookup")
72+
.toCompletionStage().toCompletableFuture()
73+
.get();
74+
} catch (InterruptedException | ExecutionException e) {
75+
throw new RuntimeException(e);
76+
}
77+
}
78+
79+
RoutingContext routingContext(Vertx vertx) {
80+
stubFor(get("/settings/entries?query=scope%3D%3D%22mod-x%22%20and%20key%3D%3D%22lookup%22")
81+
.willReturn(ok("""
82+
{ "items": [ { "value": "val" } ] }
83+
""")));
84+
stubFor(get("/configurations/entries?query=module%3D%3D%22mod-x%22%20and"
85+
+ "%20configName%3D%3D%22lookup%22%20and%20enabled%3Dtrue")
86+
.willReturn(ok("""
87+
{ "configs": [ { "value": "val" } ] }
88+
""")));
89+
var request = mock(HttpServerRequest.class);
90+
when(request.headers()).thenReturn(HeadersMultiMap.headers()
91+
.add("X-Okapi-Url", "http://localhost:9130"));
92+
when(request.getHeader("X-Okapi-Url")).thenReturn("http://localhost:9130");
93+
var routingContext = mock(RoutingContext.class);
94+
when(routingContext.vertx()).thenReturn(vertx);
95+
when(routingContext.request()).thenReturn(request);
96+
return routingContext;
97+
}
98+
99+
100+
}

‎src/test/java/org/folio/harvesteradmin/foliodata/ConfigurationsClientTest.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package org.folio.harvesteradmin.foliodata;
2+
3+
import io.vertx.core.Future;
4+
import io.vertx.ext.web.RoutingContext;
5+
6+
class ConfigurationsClientTest extends ClientTestBase {
7+
8+
Future<String> getStringValueFuture(RoutingContext routingContext, String key1, String key2) {
9+
return ConfigurationsClient.getStringValue(routingContext, key1, key2);
10+
}
11+
12+
}

‎src/test/java/org/folio/harvesteradmin/foliodata/SettingsClientTest.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package org.folio.harvesteradmin.foliodata;
2+
3+
import io.vertx.core.Future;
4+
import io.vertx.ext.web.RoutingContext;
5+
6+
class SettingsClientTest extends ClientTestBase {
7+
8+
Future<String> getStringValueFuture(RoutingContext routingContext, String key1, String key2) {
9+
return SettingsClient.getStringValue(routingContext, key1, key2);
10+
}
11+
12+
}

0 commit comments

Comments
 (0)