bejancsaba commented on code in PR #6149:
URL: https://github.com/apache/nifi/pull/6149#discussion_r905391497
##########
c2/c2-client-bundle/c2-client-http/src/main/java/org/apache/nifi/c2/client/http/C2HttpClient.java:
##########
@@ -91,6 +92,45 @@ public Optional<C2HeartbeatResponse>
publishHeartbeat(C2Heartbeat heartbeat) {
return serializer.serialize(heartbeat).flatMap(this::sendHeartbeat);
}
+ @Override
+ public Optional<byte[]> retrieveUpdateContent(String flowUpdateUrl) {
Review Comment:
I saw that "final" is heavily utilised in NiFi codebase. I understand the
intention just think that it is more noisy than how useful it is in majority of
the cases. So my question is how strongly you feel about this? If not too
strongly I wouldn't add final everywhere in the c2 module which is pretty self
contained. If this is more like a rule for consistency then I will go ahead and
update.
##########
c2/c2-client-bundle/c2-client-http/src/main/java/org/apache/nifi/c2/client/http/C2HttpClient.java:
##########
@@ -91,6 +92,45 @@ public Optional<C2HeartbeatResponse>
publishHeartbeat(C2Heartbeat heartbeat) {
return serializer.serialize(heartbeat).flatMap(this::sendHeartbeat);
}
+ @Override
+ public Optional<byte[]> retrieveUpdateContent(String flowUpdateUrl) {
+ final Request.Builder requestBuilder = new Request.Builder()
+ .get()
+ .url(flowUpdateUrl);
+ final Request request = requestBuilder.build();
+
+ Optional<ResponseBody> body;
+ try (Response response =
httpClientReference.get().newCall(request).execute()) {
+ int code = response.code();
+ body = Optional.ofNullable(response.body());
+
+ if (code >= HTTP_STATUS_BAD_REQUEST) {
+ StringBuilder messageBuilder = new
StringBuilder(String.format("Configuration retrieval failed: HTTP %d", code));
+ body.map(Object::toString).ifPresent(messageBuilder::append);
+ throw new C2ServerException(messageBuilder.toString());
+ }
+
+ if (!body.isPresent()) {
+ logger.warn("No body returned when pulling a new
configuration");
+ return Optional.empty();
+ }
+
+ return Optional.of(body.get().bytes());
+ } catch (Exception e) {
+ logger.warn("Configuration retrieval failed", e);
+ return Optional.empty();
+ }
Review Comment:
Done
##########
c2/c2-client-bundle/c2-client-http/src/main/java/org/apache/nifi/c2/client/http/C2HttpClient.java:
##########
@@ -91,6 +92,45 @@ public Optional<C2HeartbeatResponse>
publishHeartbeat(C2Heartbeat heartbeat) {
return serializer.serialize(heartbeat).flatMap(this::sendHeartbeat);
}
+ @Override
+ public Optional<byte[]> retrieveUpdateContent(String flowUpdateUrl) {
+ final Request.Builder requestBuilder = new Request.Builder()
+ .get()
+ .url(flowUpdateUrl);
+ final Request request = requestBuilder.build();
+
+ Optional<ResponseBody> body;
+ try (Response response =
httpClientReference.get().newCall(request).execute()) {
+ int code = response.code();
+ body = Optional.ofNullable(response.body());
+
+ if (code >= HTTP_STATUS_BAD_REQUEST) {
+ StringBuilder messageBuilder = new
StringBuilder(String.format("Configuration retrieval failed: HTTP %d", code));
+ body.map(Object::toString).ifPresent(messageBuilder::append);
+ throw new C2ServerException(messageBuilder.toString());
+ }
+
+ if (!body.isPresent()) {
+ logger.warn("No body returned when pulling a new
configuration");
+ return Optional.empty();
+ }
+
+ return Optional.of(body.get().bytes());
+ } catch (Exception e) {
+ logger.warn("Configuration retrieval failed", e);
+ return Optional.empty();
+ }
+ }
+
+ @Override
+ public void acknowledgeOperation(C2OperationAck operationAck) {
+ logger.info("Performing acknowledgement request to {} for operation
{}", clientConfig.getC2AckUrl(), operationAck.getOperationId());
Review Comment:
Changed the log structure as suggested but I would keep the level on info if
that is ok. Operation acknowledgement is super rare and it usually indicates
state / behaviour change on the agent side so can be really helpful in
following what is happening.
##########
c2/c2-client-bundle/c2-client-http/src/test/java/org/apache/nifi/c2/client/http/C2HttpClientTest.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.c2.client.http;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.containing;
+import static com.github.tomakehurst.wiremock.client.WireMock.matching;
+import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.verify;
+import static
org.apache.nifi.c2.client.http.C2HttpClient.HTTP_STATUS_BAD_REQUEST;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.BDDMockito.given;
+
+import com.github.tomakehurst.wiremock.client.WireMock;
+import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
+import com.github.tomakehurst.wiremock.junit5.WireMockTest;
+import java.nio.charset.StandardCharsets;
+import java.util.Optional;
+import org.apache.nifi.c2.client.C2ClientConfig;
+import org.apache.nifi.c2.client.api.C2Serializer;
+import org.apache.nifi.c2.protocol.api.C2Heartbeat;
+import org.apache.nifi.c2.protocol.api.C2HeartbeatResponse;
+import org.apache.nifi.c2.protocol.api.C2OperationAck;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@WireMockTest
+@ExtendWith(MockitoExtension.class)
+public class C2HttpClientTest {
+
+ private static final String HEARTBEAT_PATH = "/c2/heartbeat";
+ private static final String UPDATE_PATH = "/c2/update";
+ private static final String ACK_PATH = "/c2/acknowledge";
+ private static final int HTTP_STATUS_OK = 200;
+
+ @Mock
+ private C2ClientConfig c2ClientConfig;
+
+ @Mock
+ private C2Serializer serializer;
+
+ @InjectMocks
+ private C2HttpClient c2HttpClient;
+
+ @Test
+ void shouldReturnC2HeartbeatResponse(WireMockRuntimeInfo wmRuntimeInfo) {
Review Comment:
Usually there are multiple tests covering the same method for different
cases. I like prefixing it with "should" as it forces you to continue with a
short "explanation" regarding what is being validated here while the method
name can be really vague. Also "test" is implied by the Test annotation above
:) Anyway I understand that the convention is different here so I updated all
tests in this spirit just wanted to give some context why I created them like
it.
##########
c2/c2-client-bundle/c2-client-http/src/test/java/org/apache/nifi/c2/client/http/C2HttpClientTest.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.c2.client.http;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.containing;
+import static com.github.tomakehurst.wiremock.client.WireMock.matching;
+import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.verify;
+import static
org.apache.nifi.c2.client.http.C2HttpClient.HTTP_STATUS_BAD_REQUEST;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.BDDMockito.given;
+
+import com.github.tomakehurst.wiremock.client.WireMock;
+import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
+import com.github.tomakehurst.wiremock.junit5.WireMockTest;
+import java.nio.charset.StandardCharsets;
+import java.util.Optional;
+import org.apache.nifi.c2.client.C2ClientConfig;
+import org.apache.nifi.c2.client.api.C2Serializer;
+import org.apache.nifi.c2.protocol.api.C2Heartbeat;
+import org.apache.nifi.c2.protocol.api.C2HeartbeatResponse;
+import org.apache.nifi.c2.protocol.api.C2OperationAck;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@WireMockTest
+@ExtendWith(MockitoExtension.class)
+public class C2HttpClientTest {
+
+ private static final String HEARTBEAT_PATH = "/c2/heartbeat";
+ private static final String UPDATE_PATH = "/c2/update";
+ private static final String ACK_PATH = "/c2/acknowledge";
+ private static final int HTTP_STATUS_OK = 200;
+
+ @Mock
+ private C2ClientConfig c2ClientConfig;
+
+ @Mock
+ private C2Serializer serializer;
+
+ @InjectMocks
+ private C2HttpClient c2HttpClient;
+
+ @Test
+ void shouldReturnC2HeartbeatResponse(WireMockRuntimeInfo wmRuntimeInfo) {
+ // given
Review Comment:
I like the comments as mostly on the given side there could be multiple
sections that belong together so are worth separating from others by new line
so in my opinion this structure supports readability but this is very
subjective so I updated all tests ass requested to follow conventions.
##########
c2/c2-client-bundle/c2-client-http/src/test/java/org/apache/nifi/c2/client/http/C2HttpClientTest.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.c2.client.http;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.containing;
+import static com.github.tomakehurst.wiremock.client.WireMock.matching;
+import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.verify;
+import static
org.apache.nifi.c2.client.http.C2HttpClient.HTTP_STATUS_BAD_REQUEST;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.BDDMockito.given;
Review Comment:
Updated tests to follow NiFi conventions.
##########
c2/c2-client-bundle/c2-client-http/src/main/java/org/apache/nifi/c2/client/http/C2HttpClient.java:
##########
@@ -91,6 +92,45 @@ public Optional<C2HeartbeatResponse>
publishHeartbeat(C2Heartbeat heartbeat) {
return serializer.serialize(heartbeat).flatMap(this::sendHeartbeat);
}
+ @Override
+ public Optional<byte[]> retrieveUpdateContent(String flowUpdateUrl) {
+ final Request.Builder requestBuilder = new Request.Builder()
+ .get()
+ .url(flowUpdateUrl);
+ final Request request = requestBuilder.build();
+
+ Optional<ResponseBody> body;
+ try (Response response =
httpClientReference.get().newCall(request).execute()) {
+ int code = response.code();
+ body = Optional.ofNullable(response.body());
+
+ if (code >= HTTP_STATUS_BAD_REQUEST) {
Review Comment:
Neat! I was not aware of this function. Iitial implementation was not okhttp
based I think so it made sense then. It is more readable with this. Thanks
##########
c2/c2-client-bundle/c2-client-http/src/test/java/org/apache/nifi/c2/client/http/C2HttpClientTest.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.c2.client.http;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.containing;
+import static com.github.tomakehurst.wiremock.client.WireMock.matching;
+import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.verify;
+import static
org.apache.nifi.c2.client.http.C2HttpClient.HTTP_STATUS_BAD_REQUEST;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.BDDMockito.given;
+
+import com.github.tomakehurst.wiremock.client.WireMock;
+import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
+import com.github.tomakehurst.wiremock.junit5.WireMockTest;
+import java.nio.charset.StandardCharsets;
+import java.util.Optional;
+import org.apache.nifi.c2.client.C2ClientConfig;
+import org.apache.nifi.c2.client.api.C2Serializer;
+import org.apache.nifi.c2.protocol.api.C2Heartbeat;
+import org.apache.nifi.c2.protocol.api.C2HeartbeatResponse;
+import org.apache.nifi.c2.protocol.api.C2OperationAck;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@WireMockTest
+@ExtendWith(MockitoExtension.class)
+public class C2HttpClientTest {
+
+ private static final String HEARTBEAT_PATH = "/c2/heartbeat";
+ private static final String UPDATE_PATH = "/c2/update";
+ private static final String ACK_PATH = "/c2/acknowledge";
+ private static final int HTTP_STATUS_OK = 200;
+
+ @Mock
+ private C2ClientConfig c2ClientConfig;
+
+ @Mock
+ private C2Serializer serializer;
+
+ @InjectMocks
+ private C2HttpClient c2HttpClient;
Review Comment:
Maybe I'm missing something here. We are injecting mocks to the C2HttpClient
(e.g.: C2ClientConfig and C2Serializer) but regardless the calls made are
actual http calls. Anyway I moved to MockWebServer as requested.
##########
c2/c2-client-bundle/c2-client-http/src/main/java/org/apache/nifi/c2/client/http/C2ServerException.java:
##########
@@ -0,0 +1,26 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.c2.client.http;
+
+import java.io.IOException;
+
Review Comment:
Done
##########
c2/c2-client-bundle/c2-client-http/src/test/java/org/apache/nifi/c2/client/http/C2HttpClientTest.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.c2.client.http;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.containing;
+import static com.github.tomakehurst.wiremock.client.WireMock.matching;
+import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.verify;
+import static
org.apache.nifi.c2.client.http.C2HttpClient.HTTP_STATUS_BAD_REQUEST;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.BDDMockito.given;
+
+import com.github.tomakehurst.wiremock.client.WireMock;
+import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
+import com.github.tomakehurst.wiremock.junit5.WireMockTest;
+import java.nio.charset.StandardCharsets;
+import java.util.Optional;
+import org.apache.nifi.c2.client.C2ClientConfig;
+import org.apache.nifi.c2.client.api.C2Serializer;
+import org.apache.nifi.c2.protocol.api.C2Heartbeat;
+import org.apache.nifi.c2.protocol.api.C2HeartbeatResponse;
+import org.apache.nifi.c2.protocol.api.C2OperationAck;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@WireMockTest
+@ExtendWith(MockitoExtension.class)
+public class C2HttpClientTest {
+
+ private static final String HEARTBEAT_PATH = "/c2/heartbeat";
+ private static final String UPDATE_PATH = "/c2/update";
+ private static final String ACK_PATH = "/c2/acknowledge";
+ private static final int HTTP_STATUS_OK = 200;
+
+ @Mock
+ private C2ClientConfig c2ClientConfig;
+
+ @Mock
+ private C2Serializer serializer;
+
+ @InjectMocks
+ private C2HttpClient c2HttpClient;
+
+ @Test
+ void shouldReturnC2HeartbeatResponse(WireMockRuntimeInfo wmRuntimeInfo) {
+ // given
+ C2HeartbeatResponse hbResponse = new C2HeartbeatResponse();
+ stubFor(WireMock.post(HEARTBEAT_PATH)
+ .willReturn(aResponse().withBody("dummyResponseBody")));
+
+
given(serializer.serialize(any(C2Heartbeat.class))).willReturn(Optional.of("Heartbeat"));
+ given(serializer.deserialize(any(),
any())).willReturn(Optional.of(hbResponse));
+
given(c2ClientConfig.getC2Url()).willReturn(wmRuntimeInfo.getHttpBaseUrl() +
HEARTBEAT_PATH);
+
+ // when
+ Optional<C2HeartbeatResponse> response =
c2HttpClient.publishHeartbeat(new C2Heartbeat());
+
+ // then
+ assertTrue(response.isPresent());
+ assertEquals(response.get(), hbResponse);
+ }
+
+ @Test
+ void shouldReturnEmptyInCaseOfCommunicationIssue() {
+ // given
+
given(serializer.serialize(any(C2Heartbeat.class))).willReturn(Optional.of("Heartbeat"));
+
given(c2ClientConfig.getC2Url()).willReturn("http://localhost/incorrectPath";);
+
+ // when
+ Optional<C2HeartbeatResponse> response =
c2HttpClient.publishHeartbeat(new C2Heartbeat());
+
+ // then
+ assertFalse(response.isPresent());
+ }
+
+ @Test
+ void shouldThrowExceptionForInvalidKeystoreFilenameAtInitialization() {
+ // given
+
given(c2ClientConfig.getKeystoreFilename()).willReturn("incorrectKeystoreFilename");
+
+ // when
+ IllegalStateException exception =
assertThrows(IllegalStateException.class, () -> new
C2HttpClient(c2ClientConfig, serializer));
+
+ // then
+ assertEquals("OkHttp TLS configuration failed",
exception.getMessage());
Review Comment:
Agreed, updated
##########
c2/c2-client-bundle/c2-client-service/src/test/java/org/apache/nifi/c2/client/service/C2ClientServiceTest.java:
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.c2.client.service;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import org.apache.nifi.c2.client.api.C2Client;
+import org.apache.nifi.c2.client.service.model.RuntimeInfoWrapper;
+import org.apache.nifi.c2.client.service.operation.C2OperationService;
+import org.apache.nifi.c2.protocol.api.C2Heartbeat;
+import org.apache.nifi.c2.protocol.api.C2HeartbeatResponse;
+import org.apache.nifi.c2.protocol.api.C2Operation;
+import org.apache.nifi.c2.protocol.api.C2OperationAck;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+public class C2ClientServiceTest {
+
+ @Mock
+ private C2Client client;
+
+ @Mock
+ private C2HeartbeatFactory c2HeartbeatFactory;
+
+ @Mock
+ private C2OperationService operationService;
+
+ @InjectMocks
+ private C2ClientService c2ClientService;
+
+ @Test
+ public void shouldSendHeartbeatAndAckWhenOperationPresent() {
Review Comment:
Removed
##########
c2/c2-client-bundle/c2-client-http/pom.xml:
##########
@@ -47,5 +47,32 @@ limitations under the License.
<groupId>com.squareup.okhttp3</groupId>
<artifactId>logging-interceptor</artifactId>
</dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-engine</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-junit-jupiter</artifactId>
+ <scope>test</scope>
+ </dependency>
Review Comment:
Was not aware, removed. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
