review:

extend HttpPushSpecificConfigTest and test parallelism and omitRequestBody; throw ConfigException.WrongType in case value type is not correct; minor code formatting; Signed-off-by: Stefan Maute <stefan.maute@bosch.io>
eclipse-ditto · Feb 9, 2022 · d3296eb · d3296eb
1 parent 2fafd1a
commit d3296eb
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 11 deletions.
diff --git a/...ava/org/eclipse/ditto/connectivity/service/messaging/httppush/DefaultHttpPushFactory.java b/...ava/org/eclipse/ditto/connectivity/service/messaging/httppush/DefaultHttpPushFactory.java
@@ -135,6 +135,7 @@ public HttpRequest newRequest(final HttpPublishTarget httpPublishTarget) {
         if (passwordSeparatorLocation >= 0) {
             final String username = userInfo.substring(0, passwordSeparatorLocation);
             final String password = userInfo.substring(Math.min(userInfo.length(), passwordSeparatorLocation + 1));
+
             return request.addCredentials(HttpCredentials.createBasicHttpCredentials(username, password));
         } else {
             return request;
@@ -154,6 +155,7 @@ private static String determineBaseUri(final Uri baseUri) {
         } else {
             baseUriStrToUse = baseUriStr;
         }
+
         return baseUriStrToUse;
     }
 
@@ -166,6 +168,7 @@ private static String determineHttpPath(final HttpPublishTarget httpPublishTarge
         } else {
             pathWithQueryToUse = PATH_DELIMITER + pathWithQuery;
         }
+
         return pathWithQueryToUse;
     }
 
@@ -232,6 +235,7 @@ private ConnectionPoolSettings getConnectionPoolSettings(final ActorSystem syste
     private static <T> Pair<Try<HttpResponse>, T> onRequestTimeout(final Pair<HttpRequest, T> requestPair) {
         final Try<HttpResponse> failure =
                 new Failure<>(new TimeoutException("Request timed out: " + requestPair.first().getUri()));
+
         return Pair.create(failure, requestPair.second());
     }
 

diff --git a/...in/java/org/eclipse/ditto/connectivity/service/messaging/httppush/HttpPublisherActor.java b/...in/java/org/eclipse/ditto/connectivity/service/messaging/httppush/HttpPublisherActor.java
@@ -204,6 +204,7 @@ private static Uri setPathAndQuery(final Uri uri, @Nullable final String path, @
         if (query != null) {
             newUri = newUri.rawQueryString(query);
         }
+
         return newUri;
     }
 
@@ -221,6 +222,7 @@ private static Pair<Iterable<HttpHeader>, ContentType> getHttpHeadersPair(
                 }
             }
         }
+
         return Pair.create(headers, contentType);
     }
 
@@ -255,7 +257,6 @@ private static byte[] getBytePayload(final ExternalMessage message) {
 
     private static CompletionStage<JsonValue> getResponseBody(final HttpResponse response, final int maxBytes,
             final Materializer materializer) {
-
         return response.entity()
                 .withSizeLimit(maxBytes)
                 .toStrict(READ_BODY_TIMEOUT_MS, materializer)
@@ -302,7 +303,6 @@ private static Uri stripUserInfo(final Uri requestUri) {
                 (duration, infoProvider) -> connectionLogger.success(infoProvider,
                         "HTTP request took <{0}> ms.", duration.toMillis());
 
-
         final Flow<Pair<HttpRequest, HttpPushContext>, Pair<HttpRequest, HttpPushContext>, NotUsed> oauthFlow =
                 ClientCredentialsFlowVisitor.eval(getContext().getSystem(), config, connection);
 
@@ -449,8 +449,7 @@ public void onResponse(final Try<HttpResponse> tryResponse) {
                             response.entity().getContentType());
 
                     toCommandResponseOrAcknowledgement(signal, autoAckTarget, response, maxTotalMessageSize,
-                            ackSizeQuota,
-                            targetAuthorizationContext)
+                            ackSizeQuota, targetAuthorizationContext)
                             .thenAccept(resultFuture::complete)
                             .exceptionally(e -> {
                                 resultFuture.completeExceptionally(e);
@@ -528,7 +527,6 @@ private CompletionStage<SendResult> toCommandResponseOrAcknowledgement(final Sig
                     // There is an issued ack declared but its not live-response => handle response as acknowledgement.
                     result = Acknowledgement.of(autoAckLabel.get(), entityId, httpStatus, mergedDittoHeaders, body);
                 }
-
             } else {
                 // No Acks declared as issued acks => Handle response either as live response or as acknowledgement
                 // or as fallback build a response for local diagnostics.
@@ -590,6 +588,7 @@ private static Optional<SignalWithEntityId<?>> tryToGetAsLiveCommandWithEntityId
         } else {
             result = null;
         }
+
         return Optional.ofNullable(result);
     }
 

diff --git a/.../main/java/org/eclipse/ditto/connectivity/service/messaging/httppush/HttpPushFactory.java b/.../main/java/org/eclipse/ditto/connectivity/service/messaging/httppush/HttpPushFactory.java
@@ -91,4 +91,5 @@ static HttpPushFactory of(final Connection connection, final HttpPushConfig http
             final ConnectionLogger connectionLogger, final Supplier<SshTunnelState> tunnelConfigSupplier) {
         return DefaultHttpPushFactory.of(connection, httpPushConfig, connectionLogger, tunnelConfigSupplier);
     }
+
 }
diff --git a/...ava/org/eclipse/ditto/connectivity/service/messaging/httppush/HttpPushSpecificConfig.java b/...ava/org/eclipse/ditto/connectivity/service/messaging/httppush/HttpPushSpecificConfig.java
@@ -26,6 +26,7 @@
 import com.typesafe.config.Config;
 import com.typesafe.config.ConfigException;
 import com.typesafe.config.ConfigFactory;
+import com.typesafe.config.ConfigValueType;
 
 /**
  * Class providing access to HTTP push specific configuration.
@@ -34,9 +35,7 @@
 public final class HttpPushSpecificConfig {
 
     static final String IDLE_TIMEOUT = "idleTimeout";
-
     static final String PARALLELISM = "parallelism";
-
     static final String OMIT_REQUEST_BODY = "omitRequestBody";
 
     private final Config specificConfig;
@@ -66,6 +65,7 @@ private static Map<String, Object> toDefaultConfig(final HttpPushConfig httpConf
         defaultMap.put(IDLE_TIMEOUT, httpConfig.getRequestTimeout());
         defaultMap.put(PARALLELISM, 1);
         defaultMap.put(OMIT_REQUEST_BODY, httpConfig.getOmitRequestBodyMethods());
+
         return defaultMap;
     }
 
@@ -90,7 +90,11 @@ public List<String> omitRequestBody() {
         try {
             return specificConfig.getStringList(OMIT_REQUEST_BODY);
         } catch (final ConfigException.WrongType e) {
-            return List.of(specificConfig.getString(OMIT_REQUEST_BODY).split(","));
+            final var configValue = specificConfig.getValue(OMIT_REQUEST_BODY);
+            if (ConfigValueType.STRING == configValue.valueType()) {
+                return List.of(specificConfig.getString(OMIT_REQUEST_BODY).split(","));
+            }
+            throw e;
         }
     }
 

diff --git a/...t/java/org/eclipse/ditto/connectivity/service/messaging/httppush/HttpPushFactoryTest.java b/...t/java/org/eclipse/ditto/connectivity/service/messaging/httppush/HttpPushFactoryTest.java
@@ -386,7 +386,6 @@ private static Connection createHttpPushConnection(final ServerBinding binding)
                 ConnectivityStatus.OPEN,
                 "http://127.0.0.1:" + binding.localAddress().getPort())
                 .targets(singletonList(AbstractBaseClientActorTest.HTTP_TARGET))
-                .specificConfig(Map.of())
                 .build();
     }
 
@@ -398,6 +397,7 @@ private static HttpProxyConfig getEnabledProxyConfig(final ServerBinding binding
                 "  username = \"username\"\n" +
                 "  password = \"password\"\n" +
                 "}");
+
         return DefaultHttpProxyConfig.ofProxy(config);
     }
 }
diff --git a/...org/eclipse/ditto/connectivity/service/messaging/httppush/HttpPushSpecificConfigTest.java b/...org/eclipse/ditto/connectivity/service/messaging/httppush/HttpPushSpecificConfigTest.java
@@ -16,16 +16,22 @@
 import static org.mockito.Mockito.when;
 
 import java.time.Duration;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 
 import org.eclipse.ditto.connectivity.model.Connection;
 import org.eclipse.ditto.connectivity.service.config.HttpPushConfig;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.Mockito;
 
+/**
+ * Unit test for {@link HttpPushSpecificConfig}.
+ */
 public final class HttpPushSpecificConfigTest {
 
     private HttpPushConfig httpConfig;
@@ -34,27 +40,38 @@ public final class HttpPushSpecificConfigTest {
     @Before
     public void setup() {
         httpConfig = Mockito.mock(HttpPushConfig.class);
-        when(httpConfig.getRequestTimeout()).thenReturn(Duration.ofSeconds(2));
         connection = Mockito.mock(Connection.class);
     }
 
     @Test
     public void parseHttpSpecificConfig() {
+        final String omitBodyRequest = "OPTIONS,DELETE";
         final Map<String, String> configuredSpecificConfig = new HashMap<>();
         configuredSpecificConfig.put(HttpPushSpecificConfig.IDLE_TIMEOUT, "3s");
+        configuredSpecificConfig.put(HttpPushSpecificConfig.PARALLELISM, "2");
+        configuredSpecificConfig.put(HttpPushSpecificConfig.OMIT_REQUEST_BODY, omitBodyRequest);
 
+        when(httpConfig.getRequestTimeout()).thenReturn(Duration.ofSeconds(2));
         when(connection.getSpecificConfig()).thenReturn(configuredSpecificConfig);
         final var specificConfig = HttpPushSpecificConfig.fromConnection(connection, httpConfig);
 
         assertThat(specificConfig.idleTimeout()).isEqualTo(Duration.ofSeconds(3));
+        assertThat(specificConfig.parallelism()).isEqualTo(2);
+        assertThat(specificConfig.omitRequestBody())
+                .isEqualTo(Arrays.stream(omitBodyRequest.split(",")).collect(Collectors.toList()));
     }
 
     @Test
     public void defaultConfig() {
+        final List<String> expectedOmittedRequestBody = List.of("GET", "DELETE");
         when(connection.getSpecificConfig()).thenReturn(Collections.emptyMap());
+        when(httpConfig.getRequestTimeout()).thenReturn(Duration.ofSeconds(60));
+        when(httpConfig.getOmitRequestBodyMethods()).thenReturn(expectedOmittedRequestBody);
         final HttpPushSpecificConfig specificConfig = HttpPushSpecificConfig.fromConnection(connection, httpConfig);
 
-        assertThat(specificConfig.idleTimeout()).isEqualTo(Duration.ofSeconds(2));
+        assertThat(specificConfig.idleTimeout()).isEqualTo(Duration.ofSeconds(60));
+        assertThat(specificConfig.parallelism()).isEqualTo(1);
+        assertThat(specificConfig.omitRequestBody()).isEqualTo(expectedOmittedRequestBody);
     }
 
 }