From e67cb0ff436865bf19e62705cc9381de4d068c89 Mon Sep 17 00:00:00 2001 From: Nafi Xhafa <164854562+nxhafa@users.noreply.github.com> Date: Mon, 2 Sep 2024 10:59:32 +0200 Subject: [PATCH] chore: fix Medium and High issues in SonarCloud report for branch v3.x.x (#3712) * fix medium and high issues in SonarCloud report for branch v3.x.x Signed-off-by: nx673747 * adding unit test for invalid key message Signed-off-by: nx673747 * structuring test class with Nested inner class Signed-off-by: nx673747 --------- Signed-off-by: nx673747 Co-authored-by: nx673747 --- .../message/core/AbstractMessageService.java | 5 +- .../org/zowe/apiml/message/core/Message.java | 18 +++++- .../zowe/apiml/message/log/ApimlLogger.java | 25 ++++---- .../apiml/message/log/ApimlLoggerTest.java | 57 +++++++++++++++---- .../config/ComponentsConfiguration.java | 6 +- 5 files changed, 81 insertions(+), 30 deletions(-) diff --git a/common-service-core/src/main/java/org/zowe/apiml/message/core/AbstractMessageService.java b/common-service-core/src/main/java/org/zowe/apiml/message/core/AbstractMessageService.java index 4d02e01ab2..49da1c50eb 100644 --- a/common-service-core/src/main/java/org/zowe/apiml/message/core/AbstractMessageService.java +++ b/common-service-core/src/main/java/org/zowe/apiml/message/core/AbstractMessageService.java @@ -133,8 +133,7 @@ private MessageTemplate validateMessageTemplate(String key) { * @return {@link MessageTemplate} */ private MessageTemplate getInvalidMessageTemplate() { - String text = "Internal error: Invalid message key '%s' provided. No default message found. " + - "Please contact support of further assistance."; - return new MessageTemplate(Message.INVALID_KEY_MESSAGE, "ZWEAM102", MessageType.ERROR, text); + return new MessageTemplate(Message.INVALID_KEY_MESSAGE, "ZWEAM102", MessageType.ERROR, + Message.INVALID_KEY_MESSAGE_TEXT); } } diff --git a/common-service-core/src/main/java/org/zowe/apiml/message/core/Message.java b/common-service-core/src/main/java/org/zowe/apiml/message/core/Message.java index 80c6c830e5..a8a94de080 100644 --- a/common-service-core/src/main/java/org/zowe/apiml/message/core/Message.java +++ b/common-service-core/src/main/java/org/zowe/apiml/message/core/Message.java @@ -25,6 +25,8 @@ */ public final class Message { public static final String INVALID_KEY_MESSAGE = "org.zowe.apiml.common.invalidMessageKey"; + public static final String INVALID_KEY_MESSAGE_TEXT = "Internal error: Invalid message key '%s' provided. " + + "No default message found. Please contact support of further assistance."; public static final String INVALID_MESSAGE_TEXT_FORMAT = "org.zowe.apiml.common.invalidMessageTextFormat"; private final String requestedKey; @@ -62,14 +64,26 @@ public static Message of(String requestedKey, return new Message(requestedKey, messageTemplate, messageParameters); } + /** + * Returns an {@link MessageType#ERROR} {@link Message} object indicating that the specified key is invalid. + * The {@link MessageTemplate} for this message has key {@link #INVALID_KEY_MESSAGE}. + * + * @param requestedKey the invalid key. + * @return {@link Message} + */ + public static Message invalidKeyMessage(String requestedKey) { + return new Message(requestedKey, new MessageTemplate(Message.INVALID_KEY_MESSAGE, "ZWEAM102", + MessageType.ERROR, Message.INVALID_KEY_MESSAGE_TEXT), new Object[]{requestedKey}); + } + /** * Validate the message text and parameters returning them as formatted String. * * @param messageText the message text. * @param messageParameters the object containing the message parameters. - * @throws MissingFormatArgumentException when the amount of parameters is less than required. - * @throws IllegalFormatConversionException when format is not valid. * @return a formatted String + * @throws MissingFormatArgumentException when the amount of parameters is less than required. + * @throws IllegalFormatConversionException when format is not valid. */ private static String validateMessageTextFormat(String messageText, Object[] messageParameters) { return String.format(messageText, messageParameters); diff --git a/common-service-core/src/main/java/org/zowe/apiml/message/log/ApimlLogger.java b/common-service-core/src/main/java/org/zowe/apiml/message/log/ApimlLogger.java index faf091d7ab..b50de9af92 100644 --- a/common-service-core/src/main/java/org/zowe/apiml/message/log/ApimlLogger.java +++ b/common-service-core/src/main/java/org/zowe/apiml/message/log/ApimlLogger.java @@ -67,16 +67,19 @@ public static ApimlLogger empty() { * @param parameters for message */ public Message log(String key, Object... parameters) { - ObjectUtil.requireNotNull(key, "key can't be null"); - ObjectUtil.requireNotNull(parameters, "parameters can't be null"); + Message message; + if (key == null) { + message = Message.invalidKeyMessage(null); + } else { + ObjectUtil.requireNotNull(parameters, "parameters can't be null"); - if (messageService != null) { - Message message = messageService.createMessage(key, parameters); - log(message); - return message; + if (messageService == null) { + return null; + } + message = messageService.createMessage(key, parameters); } - - return null; + log(message); + return message; } /** @@ -94,9 +97,9 @@ public void log(Message message) { /** * Method which allows to log text in its level type. * - * @param messageType type of the message - * @param text text for message - * @param arguments arguments for message text + * @param messageType type of the message + * @param text text for message + * @param arguments arguments for message text * @throws IllegalArgumentException when parameters are null */ @SuppressWarnings("squid:S2629") diff --git a/common-service-core/src/test/java/org/zowe/apiml/message/log/ApimlLoggerTest.java b/common-service-core/src/test/java/org/zowe/apiml/message/log/ApimlLoggerTest.java index 5f2adb4ede..8c835579ee 100644 --- a/common-service-core/src/test/java/org/zowe/apiml/message/log/ApimlLoggerTest.java +++ b/common-service-core/src/test/java/org/zowe/apiml/message/log/ApimlLoggerTest.java @@ -10,12 +10,15 @@ package org.zowe.apiml.message.log; +import org.junit.jupiter.api.Nested; +import org.zowe.apiml.message.core.Message; import org.zowe.apiml.message.core.MessageType; import org.junit.jupiter.api.Test; import org.slf4j.Logger; import org.slf4j.Marker; import org.springframework.test.util.ReflectionTestUtils; +import org.zowe.apiml.message.template.MessageTemplate; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; @@ -46,20 +49,20 @@ void testLogLevel() { Marker marker = (Marker) ReflectionTestUtils.getField(apimlLogger, "marker"); - apimlLogger.log(MessageType.TRACE, "traceLog", new Object[] {"param1"}); - verify(logger, times(1)).trace(marker, "traceLog", new Object[] {"param1"}); + apimlLogger.log(MessageType.TRACE, "traceLog", new Object[]{"param1"}); + verify(logger, times(1)).trace(marker, "traceLog", new Object[]{"param1"}); - apimlLogger.log(MessageType.DEBUG, "debugLog", new Object[] {"param2"}); - verify(logger, times(1)).debug(marker, "debugLog", new Object[] {"param2"}); + apimlLogger.log(MessageType.DEBUG, "debugLog", new Object[]{"param2"}); + verify(logger, times(1)).debug(marker, "debugLog", new Object[]{"param2"}); - apimlLogger.log(MessageType.INFO, "infoLog", new Object[] {"param3"}); - verify(logger, times(1)).info(marker, "infoLog", new Object[] {"param3"}); + apimlLogger.log(MessageType.INFO, "infoLog", new Object[]{"param3"}); + verify(logger, times(1)).info(marker, "infoLog", new Object[]{"param3"}); - apimlLogger.log(MessageType.WARNING, "warningLog", new Object[] {"param4"}); - verify(logger, times(1)).warn(marker, "warningLog", new Object[] {"param4"}); + apimlLogger.log(MessageType.WARNING, "warningLog", new Object[]{"param4"}); + verify(logger, times(1)).warn(marker, "warningLog", new Object[]{"param4"}); - apimlLogger.log(MessageType.ERROR, "errorLog", new Object[] {"param5"}); - verify(logger, times(1)).error(marker, "errorLog", new Object[] {"param5"}); + apimlLogger.log(MessageType.ERROR, "errorLog", new Object[]{"param5"}); + verify(logger, times(1)).error(marker, "errorLog", new Object[]{"param5"}); verify(logger, times(1)).trace((Marker) any(), anyString(), (Object[]) any()); verify(logger, times(1)).debug((Marker) any(), anyString(), (Object[]) any()); @@ -68,4 +71,38 @@ void testLogLevel() { verify(logger, times(1)).error((Marker) any(), anyString(), (Object[]) any()); } + @Nested + class GivenNullMessageService { + ApimlLogger apimlLogger = ApimlLogger.empty(); + + @Test + void when_nullMessageService_return_nullMessage() { + assertNull(ReflectionTestUtils.getField(apimlLogger, "messageService")); + assertNull(apimlLogger.log("org.zowe.apiml.common.invalidMessageKey")); + } + + @Test + void when_nullKey_return_invalidKeyMessage() { + assertNull(ReflectionTestUtils.getField(apimlLogger, "messageService")); + + Logger logger = mock(Logger.class); + ReflectionTestUtils.setField(apimlLogger, "logger", logger); + + Marker marker = (Marker) ReflectionTestUtils.getField(apimlLogger, "marker"); + + Message message = apimlLogger.log(null, new Object[]{}); + MessageTemplate messageTemplate = (MessageTemplate) ReflectionTestUtils.getField(message, "messageTemplate"); + String invalidKeyMessageText = "Internal error: Invalid message key '%s' provided. " + + "No default message found. Please contact support of further assistance."; + assertNull(ReflectionTestUtils.getField(message, "requestedKey")); + assertEquals("org.zowe.apiml.common.invalidMessageKey", messageTemplate.getKey()); + assertEquals("ZWEAM102", messageTemplate.getNumber()); + assertEquals(MessageType.ERROR, messageTemplate.getType()); + assertEquals(invalidKeyMessageText, messageTemplate.getText()); + + verify(logger, times(1)).error(marker, "ZWEAM102E Internal error: Invalid message key " + + "'null' provided. No default message found. Please contact support of further assistance.", new Object[0]); + } + } + } diff --git a/zaas-service/src/main/java/org/zowe/apiml/zaas/security/config/ComponentsConfiguration.java b/zaas-service/src/main/java/org/zowe/apiml/zaas/security/config/ComponentsConfiguration.java index ecd7c8f8af..981731fcc0 100644 --- a/zaas-service/src/main/java/org/zowe/apiml/zaas/security/config/ComponentsConfiguration.java +++ b/zaas-service/src/main/java/org/zowe/apiml/zaas/security/config/ComponentsConfiguration.java @@ -72,8 +72,7 @@ public Providers loginProviders( * Implementation of AuthSourceService interface which uses client certificate as an authentication source. * This bean performs the mapping between common name from the client certificate and the mainframe user ID. */ - @Bean - @Qualifier("x509MFAuthSourceService") + @Bean("x509MFAuthSourceService") public X509AuthSourceService getX509MFAuthSourceService(@Qualifier("x509Mapper") AuthenticationMapper mapper, TokenCreationService tokenCreationService, AuthenticationService authenticationService) { return new X509AuthSourceService(mapper, tokenCreationService, authenticationService); } @@ -83,8 +82,7 @@ public X509AuthSourceService getX509MFAuthSourceService(@Qualifier("x509Mapper") * This bean does not perform the mapping between common name from the client certificate and the mainframe user ID. * It treats client name from certificate as user ID and uses X509CommonNameUserMapper for validation. */ - @Bean - @Qualifier("x509CNAuthSourceService") + @Bean("x509CNAuthSourceService") public X509AuthSourceService getX509CNAuthSourceService(TokenCreationService tokenCreationService, AuthenticationService authenticationService) { return new X509CNAuthSourceService(new X509CommonNameUserMapper(), tokenCreationService, authenticationService); }