chore: fix sonarcloud issues in v2.x.x (#3710)

* Catch IllegalArgumentException when message key is not found Signed-off-by: nx673747 <nafi.xhafa@broadcom.com> * Do not use 'Error' JavaScript identifier as a function name Signed-off-by: nx673747 <nafi.xhafa@broadcom.com> * update method to create invalid payload exception Signed-off-by: nx673747 <nafi.xhafa@broadcom.com> * remove redundant @qualifier annotations when defining a bean Signed-off-by: nx673747 <nafi.xhafa@broadcom.com> * adding unit tests for invalid key message Signed-off-by: nx673747 <nafi.xhafa@broadcom.com> --------- Signed-off-by: nx673747 <nafi.xhafa@broadcom.com> Co-authored-by: nx673747 <nafi.xhafa@broadcom.com>
zowe · Aug 30, 2024 · 12257f2 · 12257f2
1 parent b6f2461
commit 12257f2
Show file tree

Hide file tree

Showing 11 changed files with 93 additions and 51 deletions.
diff --git a/apiml-common/src/main/java/org/zowe/apiml/product/web/HttpConfig.java b/apiml-common/src/main/java/org/zowe/apiml/product/web/HttpConfig.java
@@ -21,7 +21,6 @@
 import org.apache.http.conn.socket.PlainConnectionSocketFactory;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.eclipse.jetty.util.ssl.SslContextFactory;
-import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -204,7 +203,6 @@ public void run() {
     }
 
     @Bean
-    @Qualifier("publicKeyCertificatesBase64")
     public Set<String> publicKeyCertificatesBase64() {
         return publicKeyCertificatesBase64;
     }
@@ -218,7 +216,6 @@ private void setTruststore(SslContextFactory sslContextFactory) {
     }
 
     @Bean
-    @Qualifier("jettyClientSslContextFactory")
     public SslContextFactory.Client jettyClientSslContextFactory() {
         SslContextFactory.Client sslContextFactory = new SslContextFactory.Client();
         sslContextFactory.setProtocol(protocol);
@@ -241,7 +238,6 @@ public SslContextFactory.Client jettyClientSslContextFactory() {
      */
     @Bean
     @Primary
-    @Qualifier("restTemplateWithKeystore")
     public RestTemplate restTemplateWithKeystore() {
         HttpComponentsClientHttpRequestFactory factory = new HttpComponentsClientHttpRequestFactory(secureHttpClient);
         factory.setReadTimeout(readTimeout);
@@ -257,7 +253,6 @@ public RestTemplate restTemplateWithKeystore() {
      * @return default RestTemplate, which doesn't use certificate from keystore
      */
     @Bean
-    @Qualifier("restTemplateWithoutKeystore")
     public RestTemplate restTemplateWithoutKeystore() {
         HttpComponentsClientHttpRequestFactory factory = new HttpComponentsClientHttpRequestFactory(secureHttpClientWithoutKeystore);
         factory.setReadTimeout(readTimeout);
@@ -268,9 +263,8 @@ public RestTemplate restTemplateWithoutKeystore() {
     /**
      * @return HttpClient which use a certificate to authenticate
      */
-    @Bean
+    @Bean("secureHttpClientWithKeystore")
     @Primary
-    @Qualifier("secureHttpClientWithKeystore")
     public CloseableHttpClient secureHttpClient() {
         return secureHttpClient;
     }
@@ -279,7 +273,6 @@ public CloseableHttpClient secureHttpClient() {
      * @return HttpClient, which doesn't use a certificate to authenticate
      */
     @Bean
-    @Qualifier("secureHttpClientWithoutKeystore")
     public CloseableHttpClient secureHttpClientWithoutKeystore() {
         return secureHttpClientWithoutKeystore;
     }

diff --git a/...mmon/src/main/java/org/zowe/apiml/security/common/auth/saf/SafResourceAccessEndpoint.java b/...mmon/src/main/java/org/zowe/apiml/security/common/auth/saf/SafResourceAccessEndpoint.java
@@ -29,7 +29,7 @@ public class SafResourceAccessEndpoint implements SafResourceAccessVerifying {
 
     private static final String URL_VARIABLE_SUFFIX = "/{entity}/{level}";
 
-    @Value("${apiml.security.authorization.endpoint.url:http://localhost:8542/saf-auth}")
+    @Value("${apiml.security.authorization.endpoint.url:#{'http://localhost:8542/saf-auth'}}")
     private String endpointUrl;
 
     private final RestTemplate restTemplate;

diff --git a/caching-service/src/main/java/org/zowe/apiml/caching/api/CachingController.java b/caching-service/src/main/java/org/zowe/apiml/caching/api/CachingController.java
@@ -313,23 +313,22 @@ private void keyNotInCache() {
         throw new StorageException(Messages.KEY_NOT_PROVIDED.getKey(), Messages.KEY_NOT_PROVIDED.getStatus());
     }
 
-    private void invalidPayload(String keyValue, String message) {
-        throw new StorageException(Messages.INVALID_PAYLOAD.getKey(), Messages.INVALID_PAYLOAD.getStatus(),
+    private StorageException invalidPayloadException(String keyValue, String message) {
+        return new StorageException(Messages.INVALID_PAYLOAD.getKey(), Messages.INVALID_PAYLOAD.getStatus(),
             keyValue, message);
     }
 
     private void checkForInvalidPayload(KeyValue keyValue) {
         if (keyValue == null) {
-            invalidPayload(null, "No KeyValue provided in the payload");
+            throw invalidPayloadException(null, "No KeyValue provided in the payload");
         }
 
         if (keyValue.getValue() == null) {
-            invalidPayload(keyValue.toString(), "No value provided in the payload");
+            throw invalidPayloadException(keyValue.toString(), "No value provided in the payload");
         }
 
-        String key = keyValue.getKey();
-        if (key == null) {
-            invalidPayload(keyValue.toString(), "No key provided in the payload");
+        if (keyValue.getKey() == null) {
+            throw invalidPayloadException(keyValue.toString(), "No key provided in the payload");
         }
     }
 

diff --git a/...ay-service/src/main/java/org/zowe/apiml/cloudgatewayservice/config/ConnectionsConfig.java b/...ay-service/src/main/java/org/zowe/apiml/cloudgatewayservice/config/ConnectionsConfig.java
@@ -233,8 +233,7 @@ SslContext sslContext(boolean setKeystore) {
         }
     }
 
-    @Bean
-    @Qualifier("primaryApimlEurekaJerseyClient")
+    @Bean("primaryApimlEurekaJerseyClient")
     EurekaJerseyClient getEurekaJerseyClient() {
         return factory().createEurekaJerseyClientBuilder(eurekaServerUrl, serviceId).build();
     }

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
@@ -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
@@ -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
@@ -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
@@ -10,12 +10,14 @@
 
 package org.zowe.apiml.message.log;
 
+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;
@@ -37,6 +39,39 @@ void testEmpty() {
         assertNull(apimlLogger.log("someKey"));
     }
 
+    @Test
+    void when_nullMessageService_return_nullMessage() {
+        ApimlLogger apimlLogger = ApimlLogger.empty();
+
+        assertNull(ReflectionTestUtils.getField(apimlLogger, "messageService"));
+        assertNull(apimlLogger.log("org.zowe.apiml.common.invalidMessageKey"));
+    }
+
+    @Test
+    void when_invalidKey_return_invalidKeyMessage() {
+        ApimlLogger apimlLogger = new ApimlLogger(ApimlLoggerTest.class, null);
+
+        Logger logger = mock(Logger.class);
+        ReflectionTestUtils.setField(apimlLogger, "logger", logger);
+
+        Marker marker = (Marker) ReflectionTestUtils.getField(apimlLogger, "marker");
+
+        assertNull(ReflectionTestUtils.getField(apimlLogger, "messageService"));
+
+        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]);
+    }
+
     @Test
     void testLogLevel() {
         ApimlLogger apimlLogger = new ApimlLogger(ApimlLoggerTest.class, null);
@@ -46,20 +81,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());

diff --git a/...i/frontend/src/components/Error/Error.jsx → ...d/src/components/Error/ErrorComponent.jsx b/...i/frontend/src/components/Error/Error.jsx → ...d/src/components/Error/ErrorComponent.jsx
@@ -26,12 +26,12 @@ const ErrorTypography = withStyles(() => ({
     },
 }))(Typography);
 
-function Error(props) {
+function ErrorComponent(props) {
     return (
         <ErrorTypography {...props}>
             <CustomErrorIcon id="erroricon" {...props} /> {props.text}
         </ErrorTypography>
     );
 }
 
-export default Error;
+export default ErrorComponent;
diff --git a/...ntend/src/components/Error/Error.test.jsx → .../components/Error/ErrorComponent.test.jsx b/...ntend/src/components/Error/Error.test.jsx → .../components/Error/ErrorComponent.test.jsx
@@ -13,20 +13,20 @@
 import React from 'react';
 import { shallow } from 'enzyme';
 
-import Error from './Error';
+import ErrorComponent from './ErrorComponent';
 
 describe('>>> Error component tests', () => {
     it('should display a styled Typography', () => {
         const errorText = 'my error';
-        const sample = shallow(<Error text={errorText} />);
+        const sample = shallow(<ErrorComponent text={errorText} />);
 
         const ErrorTypography = sample.find('WithStyles(WithStyles(ForwardRef(Typography)))');
         expect(ErrorTypography).toExist();
         expect(ErrorTypography.text().trim()).toEqual(errorText);
     });
 
     it('should display an element with id erroricon', () => {
-        const sample = shallow(<Error />);
+        const sample = shallow(<ErrorComponent />);
         expect(sample.find('[id="erroricon"]')).toExist();
     });
 });
diff --git a/metrics-service-ui/frontend/src/components/Login/Login.jsx b/metrics-service-ui/frontend/src/components/Login/Login.jsx
@@ -16,7 +16,7 @@ import './LoginWebflow.css';
 import LoginBackground from '../../assets/images/login_background.jpg';
 import Spinner from '../Spinner/Spinner';
 import MetricsIconButton from '../Icons/MetricsIconButton';
-import Error from '../Error/Error';
+import ErrorComponent from '../Error/ErrorComponent';
 
 const useStyles = makeStyles((theme) => ({
     root: {
@@ -76,7 +76,7 @@ const LoginError = withStyles(() => ({
     root: {
         marginLeft: 20,
     },
-}))(Error);
+}))(ErrorComponent);
 
 function unexpectedError(error) {
     return (