Commit d7a2736a authored by harshit aggarwal's avatar harshit aggarwal
Browse files

PR comments

parent d8078b82
Pipeline #6071 failed with stage
in 13 minutes and 15 seconds
......@@ -6,15 +6,17 @@ import org.opengroup.osdu.core.common.model.tenant.TenantInfo;
import org.opengroup.osdu.wks.exceptions.ApplicationException;
import org.opengroup.osdu.wks.provider.azure.di.AzureBootstrapConfig;
import org.opengroup.osdu.wks.provider.interfaces.UserCredential;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Configuration;
import java.io.UnsupportedEncodingException;
import java.util.Collections;
@Configuration
public class JwtTokenGenerator implements UserCredential {
private final static Logger LOGGER = LoggerFactory.getLogger(JwtTokenGenerator.class);
private static final String BEARER = "Bearer ";
@Autowired
......@@ -41,7 +43,7 @@ public class JwtTokenGenerator implements UserCredential {
}
catch (UnsupportedEncodingException e) {
logger.error(azureBootstrapConfig.getLogPrefix(), e.getMessage(), Collections.emptyMap());
LOGGER.error(e.getMessage());
throw new ApplicationException(e.getMessage(), e.getCause());
}
......
......@@ -59,9 +59,6 @@ public class AzureBootstrapConfig {
@Value("${max_concurrent_calls}")
private String maxConcurrentCalls;
@Value("${LOG_PREFIX}")
private String logPrefix;
@Bean
@Named("STORAGE_ACCOUNT_NAME")
public String storageAccount() {
......@@ -99,13 +96,11 @@ public class AzureBootstrapConfig {
}
@Bean
@Named("MDC_CONTEXT_MAP")
public MDCContextMap mdcContextMap() {
return new MDCContextMap();
}
@Bean
@Named("SUBSCRIPTION_CLIENT")
public SubscriptionClient subscriptionClient() {
String entityPath = serviceBusTopic + "/subscriptions/" + serviceBusTopicSubscription;
ConnectionStringBuilder connectionStringBuilder = new ConnectionStringBuilder(
......@@ -125,7 +120,6 @@ public class AzureBootstrapConfig {
}
@Bean
@Named("AZURE_SERVICE_PRINCIPAL")
public AzureServicePrincipal getAzureServicePrincipal() {
return new AzureServicePrincipal();
}
......
......@@ -4,23 +4,22 @@ 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.azure.logging.Slf4JLogger;
import org.opengroup.osdu.wks.provider.azure.di.AzureBootstrapConfig;
import java.util.Collections;
import java.util.concurrent.CompletableFuture;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class MessageHandler implements IMessageHandler {
private final static Logger LOGGER = LoggerFactory.getLogger(MessageHandler.class);
private final SubscriptionClient receiveClient;
private final ProcessWKSTransform processWKSTransform;
private final Slf4JLogger logger;
private final AzureBootstrapConfig azureBootstrapConfig;
public MessageHandler(SubscriptionClient client, ProcessWKSTransform processWKSTransform, Slf4JLogger logger, AzureBootstrapConfig azureBootstrapConfig) {
public MessageHandler(SubscriptionClient client, ProcessWKSTransform processWKSTransform, AzureBootstrapConfig azureBootstrapConfig) {
this.receiveClient = client;
this.processWKSTransform = processWKSTransform;
this.logger = logger;
this.azureBootstrapConfig = azureBootstrapConfig;
}
......@@ -32,7 +31,7 @@ public class MessageHandler implements IMessageHandler {
@Override
public void notifyException(Throwable throwable, ExceptionPhase exceptionPhase) {
logger.error(azureBootstrapConfig.getLogPrefix(),exceptionPhase + "-" + throwable.getMessage(), Collections.emptyMap());
LOGGER.error(exceptionPhase + "-" + throwable.getMessage());
}
}
......@@ -4,7 +4,6 @@ import com.google.gson.Gson;
import com.google.gson.JsonElement;
import com.google.gson.JsonParser;
import com.microsoft.azure.servicebus.IMessage;
import org.opengroup.osdu.azure.logging.Slf4JLogger;
import org.opengroup.osdu.core.common.model.http.DpsHeaders;
import org.opengroup.osdu.wks.exceptions.ApplicationException;
import org.opengroup.osdu.wks.exceptions.BadRequestException;
......@@ -12,6 +11,8 @@ 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;
import org.slf4j.LoggerFactory;
import org.slf4j.MDC;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;
......@@ -24,14 +25,12 @@ import static java.nio.charset.StandardCharsets.UTF_8;
@Component
public class ProcessWKSTransform {
private final static Logger LOGGER = LoggerFactory.getLogger(ProcessWKSTransform.class);
private static final String DATA = "data";
@Autowired
private WKSService wKSService;
@Autowired
private Slf4JLogger logger;
@Autowired
private AzureBootstrapConfig azureBootstrapConfig;
......@@ -47,11 +46,11 @@ public class ProcessWKSTransform {
RawRecordDetails[] rawRecordDetails = retrieveDataFromMessage(message);
wKSService.transform(rawRecordDetails, dataPartitionId, correlationId);
} catch (BadRequestException e) {
logger.error(azureBootstrapConfig.getLogPrefix(), String.format("Bad Request Reason: %s, pubsub message id: %s", e.getErrorMsg(),
message.getMessageId()), Collections.emptyMap());
LOGGER.error(String.format("Bad Request Reason: %s, pubsub message id: %s", e.getErrorMsg(),
message.getMessageId()));
} catch (ApplicationException e) {
logger.error(azureBootstrapConfig.getLogPrefix(), String.format("Application Error Reason: %s, pubsub message id: %s", e.getErrorMsg(),
message.getMessageId()), Collections.emptyMap());
LOGGER.error(String.format("Application Error Reason: %s, pubsub message id: %s", e.getErrorMsg(),
message.getMessageId()));
}
MDC.clear();
......
......@@ -3,21 +3,19 @@ package org.opengroup.osdu.wks.provider.azure.pubsub;
import com.microsoft.azure.servicebus.MessageHandlerOptions;
import com.microsoft.azure.servicebus.SubscriptionClient;
import com.microsoft.azure.servicebus.primitives.ServiceBusException;
import org.opengroup.osdu.azure.logging.Slf4JLogger;
import org.opengroup.osdu.wks.provider.azure.di.AzureBootstrapConfig;
import org.opengroup.osdu.wks.provider.interfaces.SubscriptionManager;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
import java.time.Duration;
import java.util.Collections;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
@Component
public class SubscriptionManagerImpl implements SubscriptionManager {
@Autowired
private Slf4JLogger logger;
private final static Logger LOGGER = LoggerFactory.getLogger(SubscriptionManagerImpl.class);
@Autowired
private SubscriptionClientFactory subscriptionClientFactory;
......@@ -41,7 +39,7 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
private void registerMessageHandler(SubscriptionClient subscriptionClient, ExecutorService executorService) {
try {
MessageHandler messageHandler = new MessageHandler(subscriptionClient, processWKSTransform, logger, azureBootstrapConfig);
MessageHandler messageHandler = new MessageHandler(subscriptionClient, processWKSTransform, azureBootstrapConfig);
subscriptionClient.registerMessageHandler(
messageHandler,
......@@ -49,7 +47,7 @@ public class SubscriptionManagerImpl implements SubscriptionManager {
executorService);
} catch (InterruptedException | ServiceBusException e) {
logger.error(azureBootstrapConfig.getLogPrefix(), String.format("Error registering message handler %s", e.getMessage()), Collections.emptyMap());
LOGGER.error(String.format("Error registering message handler %s", e.getMessage()));
}
}
......
......@@ -2,27 +2,24 @@ package org.opengroup.osdu.wks.provider.azure.storage;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.opengroup.osdu.azure.blobstorage.BlobStore;
import org.opengroup.osdu.azure.logging.Slf4JLogger;
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;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
import java.util.Collections;
@Component
public class MappingStoreImpl implements MappingStore {
private final static Logger LOGGER = LoggerFactory.getLogger(MappingStoreImpl.class);
private static final String JSON_EXTENSION = ".json";
@Autowired
private BlobStore blobStore;
@Autowired
private Slf4JLogger logger;
@Autowired
private AzureBootstrapConfig azureBootstrapConfig;
......@@ -35,7 +32,7 @@ public class MappingStoreImpl implements MappingStore {
ObjectMapper mapper = new ObjectMapper();
mappings = mapper.readValue(content, MappingsModel.class);
} catch (Exception e) {
logger.error(azureBootstrapConfig.getLogPrefix(), String.format("Error while processing mappings from blob store %s", e.getMessage()), Collections.emptyMap());
LOGGER.error(String.format("Error while processing mappings from blob store %s", e.getMessage()));
}
return mappings;
}
......
......@@ -13,7 +13,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
LOG_PREFIX=wks
service.domain.name=${service_domain_name}
......@@ -53,9 +52,4 @@ STORAGE_API=${storage_service_endpoint}
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}
#logging configuration
logging.transaction.enabled=true
logging.slf4jlogger.enabled=true
#logging.mdccontext.enabled=true
\ No newline at end of file
max-concurrent-calls=${max_concurrent_calls}
\ No newline at end of file
......@@ -6,14 +6,12 @@ import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
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;
import org.opengroup.osdu.wks.provider.azure.di.AzureBootstrapConfig;
import java.io.UnsupportedEncodingException;
import java.util.Collections;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
......@@ -31,7 +29,6 @@ public class JwtTokenGeneratorTest {
private static final String clientSecret = "client-secret";
private static final String tenantID = "tenant-id";
private static final String appResourceId = "app-resource-id";
private static final String logPrefix = "log-prefix";
@InjectMocks
private JwtTokenGenerator jwtTokenGenerator;
......@@ -42,9 +39,6 @@ public class JwtTokenGeneratorTest {
@Mock
private AzureBootstrapConfig azureBootstrapConfig;
@Mock
private Slf4JLogger logger;
@BeforeEach
public void init() {
when(azureBootstrapConfig.getClientId()).thenReturn(clientId);
......@@ -73,7 +67,6 @@ public class JwtTokenGeneratorTest {
public void shouldThrowApplicationException() throws UnsupportedEncodingException {
doThrow(UnsupportedEncodingException.class).when(azureServicePrincipal).getIdToken(clientId, clientSecret, tenantID, appResourceId);
when(azureBootstrapConfig.getLogPrefix()).thenReturn(logPrefix);
ApplicationException exception = assertThrows(ApplicationException.class, () -> {
jwtTokenGenerator.getIdToken(new TenantInfo());
......@@ -81,7 +74,6 @@ public class JwtTokenGeneratorTest {
assertNotNull(exception);
verify(azureServicePrincipal, times(1)).getIdToken(clientId, clientSecret, tenantID, appResourceId);
verify(logger, times(1)).error(logPrefix, null, Collections.emptyMap());
verify(azureBootstrapConfig, times(1)).getClientId();
verify(azureBootstrapConfig, times(1)).getClientSecret();
verify(azureBootstrapConfig, times(1)).getTenantId();
......
......@@ -22,7 +22,6 @@ import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.opengroup.osdu.azure.logging.Slf4JLogger;
import org.opengroup.osdu.core.common.model.http.DpsHeaders;
import org.opengroup.osdu.wks.exceptions.ApplicationException;
import org.opengroup.osdu.wks.exceptions.BadRequestException;
......@@ -52,7 +51,6 @@ public class ProcessWKSTransformTest {
private static final String emptyMessage = "{}";
private static final String validMessage = "{\"message\":{\"data\":[{\"id\":\"opendes:at:wellbore-b3Blbm\",\"kind\":\"opendes:at:wellbore:1.0.0\",\"op\":\"create\"}],\"account-id\":\"opendes\",\"data-partition-id\":\"opendes\",\"correlation-id\":\"908fcf8d-30c5-4c74-a0ae-ab47b48b7a85\"}}";
private static final String noDataMessage = "{\"message\":{\"data\":[],\"account-id\":\"opendes\",\"data-partition-id\":\"opendes\",\"correlation-id\":\"908fcf8d-30c5-4c74-a0ae-ab47b48b7a85\"}}";
private static final String logPrefix = "log-prefix";
private static final String messageId = "message-id";
@InjectMocks
......@@ -67,9 +65,6 @@ public class ProcessWKSTransformTest {
@Mock
private AzureBootstrapConfig azureBootstrapConfig;
@Mock
private Slf4JLogger logger;
@Mock
private MDCContextMap mdcContextMap;
......@@ -78,7 +73,6 @@ public class ProcessWKSTransformTest {
when(message.getProperties()).thenReturn(getMessageProperties());
lenient().when(message.getMessageId()).thenReturn(messageId);
lenient().when(azureBootstrapConfig.getLogPrefix()).thenReturn(logPrefix);
}
@Test
......@@ -92,8 +86,6 @@ public class ProcessWKSTransformTest {
verify(message, times(2)).getProperties();
verify(message, times(1)).getMessageBody();
verify(message, times(1)).getMessageId();
verify(azureBootstrapConfig, times(1)).getLogPrefix();
verify(logger, times(1)).error(logPrefix, "Bad Request Reason: Invalid record change message, message object not found" + ", pubsub message id: " + messageId, Collections.emptyMap());
}
@Test
......@@ -107,8 +99,6 @@ public class ProcessWKSTransformTest {
verify(message, times(2)).getProperties();
verify(message, times(1)).getMessageBody();
verify(message, times(1)).getMessageId();
verify(azureBootstrapConfig, times(1)).getLogPrefix();
verify(logger, times(1)).error(logPrefix, "Bad Request Reason: Invalid record change message, message data not found" + ", pubsub message id: " + messageId, Collections.emptyMap());
}
@Test
......@@ -123,8 +113,6 @@ public class ProcessWKSTransformTest {
verify(message, times(2)).getProperties();
verify(message, times(1)).getMessageBody();
verify(message, times(1)).getMessageId();
verify(azureBootstrapConfig, times(1)).getLogPrefix();
verify(logger, times(1)).error(logPrefix, "Application Error Reason: null" + ", pubsub message id: " + messageId, Collections.emptyMap());
}
@Test
......
......@@ -8,7 +8,6 @@ import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.opengroup.osdu.azure.logging.Slf4JLogger;
import org.opengroup.osdu.wks.provider.azure.di.AzureBootstrapConfig;
import java.util.Collections;
......@@ -27,14 +26,10 @@ public class SubscriptionManagerImplTest {
private static final String maxConcurrentCalls = "1";
private static final String nThreads = "2";
private static final String errorMessage = "some-error";
private static final String logPrefix = "log-prefix";
@InjectMocks
private SubscriptionManagerImpl subscriptionManager;
@Mock
private Slf4JLogger logger;
@Mock
private SubscriptionClientFactory subscriptionClientFactory;
......@@ -63,7 +58,6 @@ public class SubscriptionManagerImplTest {
verify(azureBootstrapConfig, times(1)).getMaxConcurrentCalls();
verify(azureBootstrapConfig, times(1)).getNThreads();
verify(logger, never()).error("", "", Collections.emptyMap());
}
@Test
......@@ -71,13 +65,10 @@ public class SubscriptionManagerImplTest {
doThrow(new InterruptedException(errorMessage)).when(subscriptionClient).registerMessageHandler(any(), any(), any());
when(subscriptionClientFactory.getSubscriptionClient()).thenReturn(subscriptionClient);
when(azureBootstrapConfig.getLogPrefix()).thenReturn(logPrefix);
subscriptionManager.subscribeRecordsChangeEvent();
verify(azureBootstrapConfig, times(1)).getMaxConcurrentCalls();
verify(azureBootstrapConfig, times(1)).getNThreads();
verify(azureBootstrapConfig, times(1)).getLogPrefix();
verify(logger, times(1)).error(logPrefix, "Error registering message handler " + errorMessage, Collections.emptyMap());
}
}
......@@ -8,7 +8,6 @@ import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.opengroup.osdu.azure.blobstorage.BlobStore;
import org.opengroup.osdu.azure.logging.Slf4JLogger;
import org.opengroup.osdu.core.common.model.http.DpsHeaders;
import org.opengroup.osdu.wks.model.AttributeMappingModel;
import org.opengroup.osdu.wks.model.MappingsModel;
......@@ -16,8 +15,6 @@ import org.opengroup.osdu.wks.provider.azure.di.AzureBootstrapConfig;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@ExtendWith(MockitoExtension.class)
......@@ -28,7 +25,6 @@ public class MappingStoreImplTest {
private static final String wksSchemaKind = "opendes;wks:wellbore:1.0.0";
private static final String rawAttribute = "well_id";
private static final String wksAttribute = "wks_id";
private static final String logPrefix = "log-prefix";
@InjectMocks
private MappingStoreImpl mappingStore;
......@@ -36,9 +32,6 @@ public class MappingStoreImplTest {
@Mock
private BlobStore blobStore;
@Mock
private Slf4JLogger logger;
@Mock
private AzureBootstrapConfig azureBootstrapConfig;
......@@ -64,10 +57,8 @@ public class MappingStoreImplTest {
public void shouldReturnNullWhenMappingIsInvalid() {
String content = "{\n" + " \"name\": \"invalid_json\"\n" + "}";
when(blobStore.readFromBlob(DpsHeaders.DATA_PARTITION_ID, fileName + JSON_EXTENSION)).thenReturn(content);
when(azureBootstrapConfig.getLogPrefix()).thenReturn(logPrefix);
MappingsModel mappings = mappingStore.getMapping(fileName);
assertNull(mappings);
verify(azureBootstrapConfig, times(1)).getLogPrefix();
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment