Merge branch 'haaggarw/LoggerCleanUpChanges' into 'master'

Clean up

See merge request !3

provider/wks-azure/src/main/java/org/opengroup/osdu/wks/provider/azure/credentials/JwtTokenGenerator.java

 package org.opengroup.osdu.wks.provider.azure.credentials;
 
-import org.opengroup.osdu.azure.logging.Slf4JLogger;
 import org.opengroup.osdu.azure.util.AzureServicePrincipal;
 import org.opengroup.osdu.core.common.model.tenant.TenantInfo;
 import org.opengroup.osdu.wks.exceptions.ApplicationException;
@@ -25,9 +24,6 @@ public class JwtTokenGenerator implements UserCredential {
     @Autowired
     private AzureServicePrincipal azureServicePrincipal;
 
-    @Autowired
-    private Slf4JLogger logger;
-
     @Override
     public String getIdToken(TenantInfo tenant) throws ApplicationException {
         // Azure service principal will have access to all data partitions

provider/wks-azure/src/main/java/org/opengroup/osdu/wks/provider/azure/di/AzureBootstrapConfig.java

@@ -10,6 +10,8 @@ import lombok.Getter;
 import org.opengroup.osdu.azure.util.AzureServicePrincipal;
 import org.opengroup.osdu.core.common.model.http.AppException;
 import org.opengroup.osdu.wks.provider.azure.utils.MDCContextMap;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -20,6 +22,8 @@ import javax.inject.Named;
 @Getter
 public class AzureBootstrapConfig {
 
+    private final static Logger LOGGER = LoggerFactory.getLogger(AzureBootstrapConfig.class);
+
     @Value("${azure.storage.account-name}")
     private String storageAccount;
 
@@ -56,9 +60,12 @@ public class AzureBootstrapConfig {
     @Value("${executor-n-threads}")
     private String nThreads;
 
-    @Value("${max_concurrent_calls}")
+    @Value("${max-concurrent-calls}")
     private String maxConcurrentCalls;
 
+    @Value("${max-lock-renew}")
+    private String maxLockRenewDurationInSeconds;
+
     @Bean
     @Named("STORAGE_ACCOUNT_NAME")
     public String storageAccount() {
@@ -113,6 +120,7 @@ public class AzureBootstrapConfig {
         try {
             subscriptionClient = new SubscriptionClient(connectionStringBuilder, ReceiveMode.PEEKLOCK);
         } catch (InterruptedException | ServiceBusException e) {
+            LOGGER.error("Unexpected error creating Subscription Client", e);
             throw new AppException(500, "Server Error", "Unexpected error creating Subscription Client", e);
         }
 
@@ -127,11 +135,13 @@ public class AzureBootstrapConfig {
     String getKeyVaultSecret(SecretClient kv, String secretName) {
         KeyVaultSecret secret = kv.getSecret(secretName);
         if (secret == null) {
+            LOGGER.error("No secret found with name {}", secretName);
             throw new IllegalStateException(String.format("No secret found with name %s", secretName));
         }
 
         String secretValue = secret.getValue();
         if (secretValue == null) {
+            LOGGER.error("Secret unexpectedly missing from KeyVault response for secret with name {}", secretName);
             throw new IllegalStateException(String.format(
                     "Secret unexpectedly missing from KeyVault response for secret with name %s", secretName));
         }

provider/wks-azure/src/main/java/org/opengroup/osdu/wks/provider/azure/pubsub/MessageHandler.java

@@ -4,7 +4,6 @@ import com.microsoft.azure.servicebus.ExceptionPhase;
 import com.microsoft.azure.servicebus.IMessage;
 import com.microsoft.azure.servicebus.IMessageHandler;
 import com.microsoft.azure.servicebus.SubscriptionClient;
-import org.opengroup.osdu.wks.provider.azure.di.AzureBootstrapConfig;
 
 import java.util.concurrent.CompletableFuture;
 import org.slf4j.Logger;
@@ -15,12 +14,10 @@ public class MessageHandler implements IMessageHandler {
     private final static Logger LOGGER = LoggerFactory.getLogger(MessageHandler.class);
     private final SubscriptionClient receiveClient;
     private final ProcessWKSTransform processWKSTransform;
-    private final AzureBootstrapConfig azureBootstrapConfig;
 
-    public MessageHandler(SubscriptionClient client, ProcessWKSTransform processWKSTransform, AzureBootstrapConfig azureBootstrapConfig) {
+    public MessageHandler(SubscriptionClient client, ProcessWKSTransform processWKSTransform) {
         this.receiveClient = client;
         this.processWKSTransform = processWKSTransform;
-        this.azureBootstrapConfig = azureBootstrapConfig;
     }
 
     @Override

provider/wks-azure/src/main/java/org/opengroup/osdu/wks/provider/azure/pubsub/ProcessWKSTransform.java

@@ -8,7 +8,6 @@ import org.opengroup.osdu.core.common.model.http.DpsHeaders;
 import org.opengroup.osdu.wks.exceptions.ApplicationException;
 import org.opengroup.osdu.wks.exceptions.BadRequestException;
 import org.opengroup.osdu.wks.model.RawRecordDetails;
-import org.opengroup.osdu.wks.provider.azure.di.AzureBootstrapConfig;
 import org.opengroup.osdu.wks.provider.azure.utils.MDCContextMap;
 import org.opengroup.osdu.wks.service.WKSService;
 import org.slf4j.Logger;
@@ -18,8 +17,6 @@ import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.HttpStatus;
 import org.springframework.stereotype.Component;
 
-import java.util.Collections;
-
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 @Component
@@ -31,9 +28,6 @@ public class ProcessWKSTransform {
     @Autowired
     private WKSService wKSService;
 
-    @Autowired
-    private AzureBootstrapConfig azureBootstrapConfig;
-
     @Autowired
     private MDCContextMap mdcContextMap;
 
@@ -46,11 +40,11 @@ public class ProcessWKSTransform {
             RawRecordDetails[] rawRecordDetails = retrieveDataFromMessage(message);
             wKSService.transform(rawRecordDetails, dataPartitionId, correlationId);
         } catch (BadRequestException e) {
-            LOGGER.error(String.format("Bad Request Reason: %s, pubsub message id: %s", e.getErrorMsg(),
-                    message.getMessageId()));
+            LOGGER.error("Bad Request Reason: {}, pubsub message id: {}", e.getErrorMsg(),
+                    message.getMessageId());
         } catch (ApplicationException e) {
-            LOGGER.error(String.format("Application Error Reason: %s, pubsub message id: %s", e.getErrorMsg(),
-                    message.getMessageId()));
+            LOGGER.error("Application Error Reason: {}, pubsub message id: {}", e.getErrorMsg(),
+                    message.getMessageId());
         }
 
         MDC.clear();

provider/wks-azure/src/main/java/org/opengroup/osdu/wks/provider/azure/pubsub/SubscriptionManagerImpl.java

@@ -39,15 +39,14 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
 
     private void registerMessageHandler(SubscriptionClient subscriptionClient, ExecutorService executorService) {
         try {
-            MessageHandler messageHandler = new MessageHandler(subscriptionClient, processWKSTransform, azureBootstrapConfig);
-
+            MessageHandler messageHandler = new MessageHandler(subscriptionClient, processWKSTransform);
             subscriptionClient.registerMessageHandler(
                     messageHandler,
-                    new MessageHandlerOptions(Integer.parseUnsignedInt(azureBootstrapConfig.getMaxConcurrentCalls()), false, Duration.ofMinutes(2)),
+                    new MessageHandlerOptions(Integer.parseUnsignedInt(azureBootstrapConfig.getMaxConcurrentCalls()), false, Duration.ofSeconds(Integer.parseUnsignedInt(azureBootstrapConfig.getMaxLockRenewDurationInSeconds()))),
                     executorService);
 
         } catch (InterruptedException | ServiceBusException e) {
-            LOGGER.error(String.format("Error registering message handler %s", e.getMessage()));
+            LOGGER.error("Error registering message handler {}", e.getMessage());
         }
     }
 

provider/wks-azure/src/main/java/org/opengroup/osdu/wks/provider/azure/storage/MappingStoreImpl.java

@@ -4,7 +4,6 @@ import com.fasterxml.jackson.databind.ObjectMapper;
 import org.opengroup.osdu.azure.blobstorage.BlobStore;
 import org.opengroup.osdu.core.common.model.http.DpsHeaders;
 import org.opengroup.osdu.wks.model.MappingsModel;
-import org.opengroup.osdu.wks.provider.azure.di.AzureBootstrapConfig;
 import org.opengroup.osdu.wks.provider.interfaces.MappingStore;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -20,9 +19,6 @@ public class MappingStoreImpl implements MappingStore {
     @Autowired
     private BlobStore blobStore;
 
-    @Autowired
-    private AzureBootstrapConfig azureBootstrapConfig;
-
     @Override
     public MappingsModel getMapping(final String fileName) {
         MappingsModel mappings = null;
@@ -32,7 +28,7 @@ public class MappingStoreImpl implements MappingStore {
             ObjectMapper mapper = new ObjectMapper();
             mappings = mapper.readValue(content, MappingsModel.class);
         } catch (Exception e) {
-            LOGGER.error(String.format("Error while processing mappings from blob store %s", e.getMessage()));
+            LOGGER.error("Error while processing mappings from blob store {}", e.getMessage());
         }
         return mappings;
     }

provider/wks-azure/src/main/resources/application.properties

@@ -14,8 +14,6 @@
 # limitations under the License.
 #
 
-service.domain.name=${service_domain_name}
-
 azure-client-id=${AZURE_CLIENT_ID}
 azure-client-secret=${AZURE_CLIENT_SECRET}
 azure-tenant-id=${AZURE_TENANT_ID}
@@ -51,5 +49,8 @@ STORAGE_API=${storage_service_endpoint}
 # Specifies the number of threads to be created on the thread pool
 executor-n-threads=${executor_n_threads}
 
-# Specified the maximum number of concurrent calls to the callback the message pump should initiate
-max-concurrent-calls=${max_concurrent_calls}
\ No newline at end of file
+# Specifies the maximum number of concurrent calls to the callback the message pump should initiate
+max-concurrent-calls=${max_concurrent_calls}
+
+# Specifies the maximum duration in seconds within which the lock will be renewed automatically
+max-lock-renew=${max_lock_renew_duration_seconds}
\ No newline at end of file

provider/wks-azure/src/test/java/org/opengroup/osdu/wks/provider/azure/pubsub/ProcessWKSTransformTest.java

@@ -26,7 +26,6 @@ import org.opengroup.osdu.core.common.model.http.DpsHeaders;
 import org.opengroup.osdu.wks.exceptions.ApplicationException;
 import org.opengroup.osdu.wks.exceptions.BadRequestException;
 import org.opengroup.osdu.wks.model.RawRecordDetails;
-import org.opengroup.osdu.wks.provider.azure.di.AzureBootstrapConfig;
 import org.opengroup.osdu.wks.provider.azure.utils.MDCContextMap;
 import org.opengroup.osdu.wks.service.WKSService;
 
@@ -62,9 +61,6 @@ public class ProcessWKSTransformTest {
     @Mock
     private Message message;
 
-    @Mock
-    private AzureBootstrapConfig azureBootstrapConfig;
-
     @Mock
     private MDCContextMap mdcContextMap;
 

provider/wks-azure/src/test/java/org/opengroup/osdu/wks/provider/azure/pubsub/SubscriptionManagerImplTest.java

@@ -10,12 +10,9 @@ import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.opengroup.osdu.wks.provider.azure.di.AzureBootstrapConfig;
 
-import java.util.Collections;
-
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doThrow;
-import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -23,6 +20,7 @@ import static org.mockito.Mockito.when;
 @ExtendWith(MockitoExtension.class)
 public class SubscriptionManagerImplTest {
 
+    private static final String maxLockRenewDuration = "60";
     private static final String maxConcurrentCalls = "1";
     private static final String nThreads = "2";
     private static final String errorMessage = "some-error";
@@ -46,6 +44,7 @@ public class SubscriptionManagerImplTest {
     public void init() {
         when(azureBootstrapConfig.getMaxConcurrentCalls()).thenReturn(maxConcurrentCalls);
         when(azureBootstrapConfig.getNThreads()).thenReturn(nThreads);
+        when(azureBootstrapConfig.getMaxLockRenewDurationInSeconds()).thenReturn(maxLockRenewDuration);
     }
 
     @Test
@@ -58,6 +57,7 @@ public class SubscriptionManagerImplTest {
 
         verify(azureBootstrapConfig, times(1)).getMaxConcurrentCalls();
         verify(azureBootstrapConfig, times(1)).getNThreads();
+        verify(azureBootstrapConfig, times(1)).getMaxLockRenewDurationInSeconds();
     }
 
     @Test
@@ -70,5 +70,6 @@ public class SubscriptionManagerImplTest {
 
         verify(azureBootstrapConfig, times(1)).getMaxConcurrentCalls();
         verify(azureBootstrapConfig, times(1)).getNThreads();
+        verify(azureBootstrapConfig, times(1)).getMaxLockRenewDurationInSeconds();
     }
 }