fix: get cloud service client from client factory for every request (#45

) * fix: get cloud service client from client factory for every request * fix: fix some comment strings Co-authored-by: Jason <[email protected]>
aws-greengrass · Jul 8, 2021 · fc43dc7 · fc43dc7
1 parent 1a9fa19
commit fc43dc7
Show file tree

Hide file tree

Showing 10 changed files with 58 additions and 55 deletions.
diff --git a/src/main/java/com/aws/greengrass/certificatemanager/CertificateManager.java b/src/main/java/com/aws/greengrass/certificatemanager/CertificateManager.java
@@ -13,7 +13,7 @@
 import com.aws.greengrass.certificatemanager.certificate.ClientCertificateGenerator;
 import com.aws.greengrass.certificatemanager.certificate.CsrProcessingException;
 import com.aws.greengrass.certificatemanager.certificate.ServerCertificateGenerator;
-import com.aws.greengrass.cisclient.CISClient;
+import com.aws.greengrass.cisclient.ConnectivityInfoProvider;
 import com.aws.greengrass.logging.api.Logger;
 import com.aws.greengrass.logging.impl.LogManager;
 import lombok.NonNull;
@@ -36,7 +36,7 @@ public class CertificateManager {
 
     private final CertificateStore certificateStore;
 
-    private final CISClient cisClient;
+    private final ConnectivityInfoProvider connectivityInfoProvider;
 
     private final CertificateExpiryMonitor certExpiryMonitor;
 
@@ -46,15 +46,15 @@ public class CertificateManager {
      * Constructor.
      *
      * @param certificateStore      Helper class for managing certificate authorities
-     * @param cisClient             CIS Client
+     * @param connectivityInfoProvider             Connectivity Info Provider
      * @param certExpiryMonitor     Certificate Expiry Monitor
      * @param cisShadowMonitor      CIS Shadow Monitor
      */
     @Inject
-    public CertificateManager(CertificateStore certificateStore, CISClient cisClient,
+    public CertificateManager(CertificateStore certificateStore, ConnectivityInfoProvider connectivityInfoProvider,
                               CertificateExpiryMonitor certExpiryMonitor, CISShadowMonitor cisShadowMonitor) {
         this.certificateStore = certificateStore;
-        this.cisClient = cisClient;
+        this.connectivityInfoProvider = connectivityInfoProvider;
         this.certExpiryMonitor = certExpiryMonitor;
         this.cisShadowMonitor = cisShadowMonitor;
     }
@@ -132,7 +132,7 @@ public void subscribeToServerCertificateUpdates(@NonNull String csr, @NonNull Co
             certExpiryMonitor.addToMonitor(certificateGenerator);
             cisShadowMonitor.addToMonitor(certificateGenerator);
 
-            certificateGenerator.generateCertificate(cisClient::getCachedHostAddresses);
+            certificateGenerator.generateCertificate(connectivityInfoProvider::getCachedHostAddresses);
         } catch (KeyStoreException e) {
             logger.atError().setCause(e).log("unable to subscribe to certificate update");
             throw e;

diff --git a/src/main/java/com/aws/greengrass/certificatemanager/certificate/CISShadowMonitor.java b/src/main/java/com/aws/greengrass/certificatemanager/certificate/CISShadowMonitor.java
@@ -5,7 +5,7 @@
 
 package com.aws.greengrass.certificatemanager.certificate;
 
-import com.aws.greengrass.cisclient.CISClient;
+import com.aws.greengrass.cisclient.ConnectivityInfoProvider;
 import com.aws.greengrass.deployment.DeviceConfiguration;
 import com.aws.greengrass.device.exception.CloudServiceInteractionException;
 import com.aws.greengrass.logging.api.Logger;
@@ -65,7 +65,7 @@ public class CISShadowMonitor {
     private final List<CertificateGenerator> monitoredCertificateGenerators = new CopyOnWriteArrayList<>();
     private final ExecutorService executorService;
     private final String shadowName;
-    private final CISClient cisClient;
+    private final ConnectivityInfoProvider connectivityInfoProvider;
     private final AtomicInteger nextVersion = new AtomicInteger(-1);
 
     private final MqttClientConnectionEvents callbacks = new MqttClientConnectionEvents() {
@@ -88,25 +88,27 @@ public void onConnectionResumed(boolean sessionPresent) {
      * @param mqttClient          IoT MQTT client
      * @param executorService     Executor service
      * @param deviceConfiguration Device configuration
-     * @param cisClient           CIS Client
+     * @param connectivityInfoProvider           Connectivity Info Provider
      */
     @Inject
     public CISShadowMonitor(MqttClient mqttClient, ExecutorService executorService,
-                            DeviceConfiguration deviceConfiguration, CISClient cisClient) {
+                            DeviceConfiguration deviceConfiguration,
+                            ConnectivityInfoProvider connectivityInfoProvider) {
         this(mqttClient, null, null, executorService,
-                Coerce.toString(deviceConfiguration.getThingName()) + CIS_SHADOW_SUFFIX, cisClient);
+                Coerce.toString(deviceConfiguration.getThingName()) + CIS_SHADOW_SUFFIX, connectivityInfoProvider);
         this.connection = new WrapperMqttClientConnection(mqttClient);
         this.iotShadowClient = new IotShadowClient(this.connection);
     }
 
     CISShadowMonitor(MqttClient mqttClient, MqttClientConnection connection, IotShadowClient iotShadowClient,
-                     ExecutorService executorService, String shadowName, CISClient cisClient) {
+                     ExecutorService executorService, String shadowName,
+                     ConnectivityInfoProvider connectivityInfoProvider) {
         mqttClient.addToCallbackEvents(callbacks);
         this.connection = connection;
         this.iotShadowClient = iotShadowClient;
         this.executorService = executorService;
         this.shadowName = shadowName;
-        this.cisClient = cisClient;
+        this.connectivityInfoProvider = connectivityInfoProvider;
     }
 
     /**
@@ -211,7 +213,7 @@ private synchronized void handleNewCloudVersion(int newVersion) {
         // to avoid blocking other MQTT subscribers in the Nucleus
         getConnectivityFuture = CompletableFuture.supplyAsync(() -> {
             try {
-                RetryUtils.runWithRetry(GET_CONNECTIVITY_RETRY_CONFIG, cisClient::getConnectivityInfo,
+                RetryUtils.runWithRetry(GET_CONNECTIVITY_RETRY_CONFIG, connectivityInfoProvider::getConnectivityInfo,
                         "get-connectivity", LOGGER);
             } catch (InterruptedException e) {
                 LOGGER.atWarn().cause(e).log("Retry workflow for getting connectivity info interrupted");
@@ -230,7 +232,7 @@ private synchronized void handleNewCloudVersion(int newVersion) {
         getConnectivityFuture.thenAccept((version) -> {
             try {
                 for (CertificateGenerator cg : monitoredCertificateGenerators) {
-                    cg.generateCertificate(cisClient::getCachedHostAddresses);
+                    cg.generateCertificate(connectivityInfoProvider::getCachedHostAddresses);
                 }
                 reportVersion(version);
                 lastVersion = version;

diff --git a/...main/java/com/aws/greengrass/certificatemanager/certificate/CertificateExpiryMonitor.java b/...main/java/com/aws/greengrass/certificatemanager/certificate/CertificateExpiryMonitor.java
@@ -5,7 +5,7 @@
 
 package com.aws.greengrass.certificatemanager.certificate;
 
-import com.aws.greengrass.cisclient.CISClient;
+import com.aws.greengrass.cisclient.ConnectivityInfoProvider;
 import com.aws.greengrass.logging.api.Logger;
 import com.aws.greengrass.logging.impl.LogManager;
 
@@ -27,7 +27,7 @@ public class CertificateExpiryMonitor {
 
     private final ScheduledExecutorService ses;
 
-    private final CISClient cisClient;
+    private final ConnectivityInfoProvider connectivityInfoProvider;
 
     private final Queue<CertificateGenerator> monitoredCertificateGenerators = new
             PriorityBlockingQueue<>(QUEUE_INITIAL_CAPACITY, Comparator.comparing(CertificateGenerator::getExpiryTime));
@@ -37,12 +37,12 @@ public class CertificateExpiryMonitor {
     /**
      * Constructor.
      * @param ses       ScheduledExecutorService to schedule cert expiry checks
-     * @param cisClient CIS Client
+     * @param connectivityInfoProvider Connectivity Info Provider
      */
     @Inject
-    public CertificateExpiryMonitor(ScheduledExecutorService ses, CISClient cisClient) {
+    public CertificateExpiryMonitor(ScheduledExecutorService ses, ConnectivityInfoProvider connectivityInfoProvider) {
         this.ses = ses;
-        this.cisClient = cisClient;
+        this.connectivityInfoProvider = connectivityInfoProvider;
     }
 
     /**
@@ -69,7 +69,7 @@ private void watchForCertExpiryOnce() {
                 break;
             }
             try {
-                cg.generateCertificate(cisClient::getCachedHostAddresses);
+                cg.generateCertificate(connectivityInfoProvider::getCachedHostAddresses);
             } catch (KeyStoreException e) {
                 LOGGER.atError().cause(e).log("Error generating certificate. Will be retried after {} seconds",
                         DEFAULT_CERT_EXPIRY_CHECK_SECONDS);

diff --git a/...n/java/com/aws/greengrass/certificatemanager/certificate/CertificateRequestGenerator.java b/...n/java/com/aws/greengrass/certificatemanager/certificate/CertificateRequestGenerator.java
@@ -28,7 +28,7 @@
 import javax.security.auth.x500.X500Principal;
 
 /**
- * Certificate Request Generator that creates a CSR for given key pair, CIS info and common name.
+ * Certificate Request Generator that creates a CSR for given key pair, connectivity info and common name.
  */
 public final class CertificateRequestGenerator {
     private static final String CSR_COUNTRY = "US";

diff --git a/...m/aws/greengrass/cisclient/CISClient.java → ...s/cisclient/ConnectivityInfoProvider.java b/...m/aws/greengrass/cisclient/CISClient.java → ...s/cisclient/ConnectivityInfoProvider.java
@@ -10,7 +10,6 @@
 import com.aws.greengrass.logging.impl.LogManager;
 import com.aws.greengrass.util.Coerce;
 import com.aws.greengrass.util.GreengrassServiceClientFactory;
-import software.amazon.awssdk.services.greengrassv2data.GreengrassV2DataClient;
 import software.amazon.awssdk.services.greengrassv2data.model.ConnectivityInfo;
 import software.amazon.awssdk.services.greengrassv2data.model.GetConnectivityInfoRequest;
 import software.amazon.awssdk.services.greengrassv2data.model.GetConnectivityInfoResponse;
@@ -24,13 +23,13 @@
 import javax.inject.Inject;
 
 /**
- * Client for retrieving connectivity info from CIS - Connectivity Info Service.
+ * Retrieving connectivity info from CIS - Connectivity Info Service.
  */
-public class CISClient {
-    private static final Logger LOGGER = LogManager.getLogger(CISClient.class);
+public class ConnectivityInfoProvider {
+    private static final Logger LOGGER = LogManager.getLogger(ConnectivityInfoProvider.class);
 
     private final DeviceConfiguration deviceConfiguration;
-    private final GreengrassV2DataClient greengrassV2DataClient;
+    private final GreengrassServiceClientFactory clientFactory;
 
     private volatile List<String> cachedHostAddresses = Collections.emptyList();
 
@@ -41,9 +40,10 @@ public class CISClient {
      * @param clientFactory       factory to get data plane client
      */
     @Inject
-    public CISClient(DeviceConfiguration deviceConfiguration, GreengrassServiceClientFactory clientFactory) {
+    public ConnectivityInfoProvider(DeviceConfiguration deviceConfiguration,
+                                    GreengrassServiceClientFactory clientFactory) {
         this.deviceConfiguration = deviceConfiguration;
-        this.greengrassV2DataClient = clientFactory.getGreengrassV2DataClient();
+        this.clientFactory = clientFactory;
     }
 
     /**
@@ -66,8 +66,8 @@ public List<ConnectivityInfo> getConnectivityInfo() {
         List<ConnectivityInfo> connectivityInfoList = Collections.emptyList();
 
         try {
-            GetConnectivityInfoResponse getConnectivityInfoResponse = greengrassV2DataClient.getConnectivityInfo(
-                    getConnectivityInfoRequest);
+            GetConnectivityInfoResponse getConnectivityInfoResponse = clientFactory.getGreengrassV2DataClient()
+                    .getConnectivityInfo(getConnectivityInfoRequest);
             if (getConnectivityInfoResponse.hasConnectivityInfo()) {
                 // Filter out port and metadata since it is not needed
                 connectivityInfoList = getConnectivityInfoResponse.connectivityInfo();

diff --git a/src/test/java/com/aws/greengrass/certificatemanager/CertificateManagerTest.java b/src/test/java/com/aws/greengrass/certificatemanager/CertificateManagerTest.java
@@ -9,7 +9,7 @@
 import com.aws.greengrass.certificatemanager.certificate.CISShadowMonitor;
 import com.aws.greengrass.certificatemanager.certificate.CertificateStore;
 import com.aws.greengrass.certificatemanager.certificate.CsrProcessingException;
-import com.aws.greengrass.cisclient.CISClient;
+import com.aws.greengrass.cisclient.ConnectivityInfoProvider;
 import com.aws.greengrass.testcommons.testutilities.GGExtension;
 import org.bouncycastle.util.io.pem.PemReader;
 import org.junit.jupiter.api.Assertions;
@@ -89,7 +89,7 @@ public class CertificateManagerTest {
     private static final int TEST_TIME_OUT_SEC = 1;
 
     @Mock
-    CISClient mockCISClient;
+    ConnectivityInfoProvider mockConnectivityInfoProvider;
 
     @Mock
     CertificateExpiryMonitor mockCertExpiryMonitor;
@@ -104,7 +104,7 @@ public class CertificateManagerTest {
 
     @BeforeEach
     void beforeEach() throws KeyStoreException {
-        certificateManager = new CertificateManager(new CertificateStore(tmpPath), mockCISClient, mockCertExpiryMonitor,
+        certificateManager = new CertificateManager(new CertificateStore(tmpPath), mockConnectivityInfoProvider, mockCertExpiryMonitor,
                 mockShadowMonitor);
         certificateManager.update("", CertificateStore.CAType.RSA_2048);
     }

diff --git a/src/test/java/com/aws/greengrass/certificatemanager/certificate/CISShadowMonitorTest.java b/src/test/java/com/aws/greengrass/certificatemanager/certificate/CISShadowMonitorTest.java
@@ -5,7 +5,7 @@
 
 package com.aws.greengrass.certificatemanager.certificate;
 
-import com.aws.greengrass.cisclient.CISClient;
+import com.aws.greengrass.cisclient.ConnectivityInfoProvider;
 import com.aws.greengrass.mqttclient.MqttClient;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -61,7 +61,7 @@ public class CISShadowMonitorTest {
     private ExecutorService mockExecutor;
 
     @Mock
-    private CISClient mockCISClient;
+    private ConnectivityInfoProvider mockConnectivityInfoProvider;
 
     @BeforeEach
     void setup() {
@@ -77,7 +77,8 @@ void setup() {
 
     @Test
     public void GIVEN_CISShadowMonitor_WHEN_connected_THEN_publish_to_get_topic() {
-        new CISShadowMonitor(mockMqttClient, mockConnection, mockShadowClient, mockExecutor, SHADOW_NAME, mockCISClient);
+        new CISShadowMonitor(mockMqttClient, mockConnection, mockShadowClient, mockExecutor, SHADOW_NAME,
+                mockConnectivityInfoProvider);
         ArgumentCaptor<MqttClientConnectionEvents> callbackArgumentCaptor = ArgumentCaptor.forClass(
                 MqttClientConnectionEvents.class);
         verify(mockMqttClient, times(1)).addToCallbackEvents(callbackArgumentCaptor.capture());
@@ -92,7 +93,7 @@ public void GIVEN_CISShadowMonitor_WHEN_connected_THEN_publish_to_get_topic() {
     @Test
     public void GIVEN_CISShadowMonitor_WHEN_start_monitor_THEN_subscribe_and_publish_to_topics() {
         CISShadowMonitor cisShadowMonitor = new CISShadowMonitor(mockMqttClient, mockConnection, mockShadowClient,
-                mockExecutor, SHADOW_NAME, mockCISClient);
+                mockExecutor, SHADOW_NAME, mockConnectivityInfoProvider);
 
         when(mockShadowClient.SubscribeToShadowDeltaUpdatedEvents(any(), any(), any(), any()))
                 .thenReturn(CompletableFuture.completedFuture(0));
@@ -123,7 +124,7 @@ public void GIVEN_CISShadowMonitor_WHEN_start_monitor_THEN_subscribe_and_publish
     @Test
     public void GIVEN_CISShadowMonitor_WHEN_update_delta_version_THEN_cert_generated() throws KeyStoreException {
         CISShadowMonitor cisShadowMonitor = new CISShadowMonitor(mockMqttClient, mockConnection, mockShadowClient,
-                mockExecutor, SHADOW_NAME, mockCISClient);
+                mockExecutor, SHADOW_NAME, mockConnectivityInfoProvider);
         when(mockShadowClient.SubscribeToShadowDeltaUpdatedEvents(any(), any(), any(), any()))
                 .thenReturn(CompletableFuture.completedFuture(0));
         when(mockShadowClient.SubscribeToGetShadowAccepted(any(), any(), any(), any()))
@@ -162,7 +163,7 @@ public void GIVEN_CISShadowMonitor_WHEN_update_delta_version_THEN_cert_generated
     @Test
     public void GIVEN_CISShadowMonitor_WHEN_get_accepted_version_THEN_cert_generated() throws KeyStoreException {
         CISShadowMonitor cisShadowMonitor = new CISShadowMonitor(mockMqttClient, mockConnection, mockShadowClient,
-                mockExecutor, SHADOW_NAME, mockCISClient);
+                mockExecutor, SHADOW_NAME, mockConnectivityInfoProvider);
         when(mockShadowClient.SubscribeToShadowDeltaUpdatedEvents(any(), any(), any(), any()))
                 .thenReturn(CompletableFuture.completedFuture(0));
         when(mockShadowClient.SubscribeToGetShadowAccepted(any(), any(), any(), any()))
@@ -201,7 +202,7 @@ public void GIVEN_CISShadowMonitor_WHEN_get_accepted_version_THEN_cert_generated
     @Test
     public void GIVEN_CISShadowMonitor_WHEN_stop_monitor_THEN_unsubscribe_from_topics() {
         CISShadowMonitor cisShadowMonitor = new CISShadowMonitor(mockMqttClient, mockConnection, mockShadowClient,
-                mockExecutor, SHADOW_NAME, mockCISClient);
+                mockExecutor, SHADOW_NAME, mockConnectivityInfoProvider);
         when(mockShadowClient.SubscribeToShadowDeltaUpdatedEvents(any(), any(), any(), any()))
                 .thenReturn(CompletableFuture.completedFuture(0));
         when(mockShadowClient.SubscribeToGetShadowAccepted(any(), any(), any(), any()))

diff --git a/.../java/com/aws/greengrass/certificatemanager/certificate/CertificateExpiryMonitorTest.java b/.../java/com/aws/greengrass/certificatemanager/certificate/CertificateExpiryMonitorTest.java
@@ -5,7 +5,7 @@
 
 package com.aws.greengrass.certificatemanager.certificate;
 
-import com.aws.greengrass.cisclient.CISClient;
+import com.aws.greengrass.cisclient.ConnectivityInfoProvider;
 import org.bouncycastle.asn1.x500.X500Name;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -41,7 +41,7 @@ public class CertificateExpiryMonitorTest {
     private Consumer<X509Certificate> mockCallback;
 
     @Mock
-    private CISClient mockCISClient;
+    private ConnectivityInfoProvider mockConnectivityInfoProvider;
 
     @TempDir
     Path tmpPath;
@@ -56,7 +56,7 @@ public void GIVEN_certs_added_to_monitor_WHEN_expired_THEN_regenerated() throws
 
         //start cert expiry monitor
         ScheduledExecutorService ses = new ScheduledThreadPoolExecutor(1);
-        CertificateExpiryMonitor certExpiryMonitor = new CertificateExpiryMonitor(ses, mockCISClient);
+        CertificateExpiryMonitor certExpiryMonitor = new CertificateExpiryMonitor(ses, mockConnectivityInfoProvider);
         certExpiryMonitor.startMonitor(TEST_CERT_EXPIRY_CHECK_SECONDS);
 
         //add certs to monitor