Tests added, critical Sonar complains resolved

ocpp-v1_6/src/main/java/eu/chargetime/ocpp/JSONCommunicator.java

@@ -5,6 +5,8 @@
 import eu.chargetime.ocpp.model.CallMessage;
 import eu.chargetime.ocpp.model.CallResultMessage;
 import eu.chargetime.ocpp.model.Message;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.lang.reflect.Type;
 import java.text.ParseException;
@@ -46,6 +48,8 @@ of this software and associated documentation files (the "Software"), to deal
  */
 public class JSONCommunicator extends Communicator {
 
+    private static final Logger logger = LoggerFactory.getLogger(JSONCommunicator.class);
+
     private static final int INDEX_MESSAGEID = 0;
     private static final int TYPENUMBER_CALL = 2;
     private static final int INDEX_CALL_ACTION = 2;
@@ -138,7 +142,7 @@ protected Object makeCallError(String uniqueId, String action, String errorCode,
 
     @Override
     protected Message parse(Object json) {
-        Message message = null;
+        Message message;
         JsonParser parser = new JsonParser();
         JsonArray array = parser.parse(json.toString()).getAsJsonArray();
 
@@ -154,7 +158,11 @@ protected Message parse(Object json) {
             ((CallErrorMessage) message).setErrorCode(array.get(INDEX_CALLERROR_ERRORCODE).getAsString());
             ((CallErrorMessage) message).setErrorDescription(array.get(INDEX_CALLERROR_DESCRIPTION).getAsString());
             ((CallErrorMessage) message).setRawPayload(array.get(INDEX_CALLERROR_PAYLOAD).toString());
+        } else {
+            logger.error("Unknown message type of message: {}", json.toString());
+            throw new IllegalArgumentException("Unknown message type");
         }
+
         message.setId(array.get(INDEX_UNIQUEID).getAsString());
 
         return message;

ocpp-v1_6/src/main/java/eu/chargetime/ocpp/SOAPCommunicator.java

@@ -1,4 +1,6 @@
-package eu.chargetime.ocpp;/*
+package eu.chargetime.ocpp;
+
+/*
     ChargeTime.eu - Java-OCA-OCPP
 
     MIT License
@@ -63,9 +65,9 @@ public <T> T unpackPayload(Object payload, Class<T> type) {
         T output = null;
         try {
             Document input = (Document) payload;
-            input = setNamespace(input, "urn://Ocpp/Cs/2015/10/");
+            setNamespace(input, "urn://Ocpp/Cs/2015/10/");
             Unmarshaller unmarshaller = JAXBContext.newInstance(type).createUnmarshaller();
-            JAXBElement<T> jaxbElement = (JAXBElement<T>) unmarshaller.unmarshal(input, type);
+            JAXBElement<T> jaxbElement = unmarshaller.unmarshal(input, type);
             output = jaxbElement.getValue();
         } catch (JAXBException e) {
         	 logger.warn("unpackPayload() failed", e);
@@ -82,17 +84,14 @@ public Object packPayload(Object payload) {
             factory.setNamespaceAware(false);
             document = factory.newDocumentBuilder().newDocument();
             marshaller.marshal(payload, document);
-            document = setNamespace(document, hostInfo.getNamespace());
-        } catch (JAXBException e) {
+            setNamespace(document, hostInfo.getNamespace());
+        } catch (JAXBException | ParserConfigurationException e) {
         	logger.warn("packPayload() failed", e);
-        } catch (ParserConfigurationException e) {
-       	 	logger.warn("packPayload() failed", e);
         }
         return document;
     }
 
-    private Document setNamespace(Document document, String namespace) {
-        Document output = document;
+    private void setNamespace(Document document, String namespace) {
         Element orgElement = document.getDocumentElement();
         Element newElement = document.createElementNS(namespace, orgElement.getNodeName());
 
@@ -101,8 +100,7 @@ private Document setNamespace(Document document, String namespace) {
             appendChildNS(document, newElement, childNodes.item(i), namespace);
         }
 
-        output.replaceChild(newElement, orgElement);
-        return output;
+        document.replaceChild(newElement, orgElement);
     }
 
     private void appendChildNS(Document doc, Node destination, Node child, String namespace) {
@@ -173,7 +171,7 @@ private Object createMessage(String uniqueId, String action, Document payload, b
             createMessageHeader(uniqueId, action, isResponse, message);
 
             if (isResponse) {
-                payload = setNamespace(payload, hostInfo.isClient() ? SOAPHostInfo.NAMESPACE_CHARGEBOX : SOAPHostInfo.NAMESPACE_CENTRALSYSTEM);
+                setNamespace(payload, hostInfo.isClient() ? SOAPHostInfo.NAMESPACE_CHARGEBOX : SOAPHostInfo.NAMESPACE_CENTRALSYSTEM);
             }
 
             message.getSOAPBody().addDocument(payload);
@@ -260,7 +258,8 @@ public Message parseMessage() {
 
                 String relatesTo = getElementValue(HEADER_RELATESTO);
                 String action = getElementValue(HEADER_ACTION);
-                if (relatesTo != null && !"".equals(relatesTo) && action.endsWith("Response")) {
+
+                if (relatesTo != null && !relatesTo.isEmpty() && action != null && action.endsWith("Response")) {
                     if (soapMessage.getSOAPBody().hasFault())
                         output = parseError();
                     else
@@ -269,7 +268,7 @@ public Message parseMessage() {
                     output = parseCall();
                 }
 
-                if (action != null && !"".equals(action))
+                if (action != null && !action.isEmpty())
                     output.setAction(action.substring(1));
 
                 if (!soapMessage.getSOAPBody().hasFault())
@@ -302,7 +301,7 @@ private CallErrorMessage parseError() {
                     message.setErrorDescription(fault.getFaultReasonTexts().next().toString());
 
             } catch (SOAPException e) {
-                e.printStackTrace();
+                logger.error("Parse error", e);
             }
 
             return message;

ocpp-v1_6/src/main/java/eu/chargetime/ocpp/wss/BaseWssFactoryBuilder.java

@@ -59,6 +59,8 @@ public BaseWssFactoryBuilder sslContext(SSLContext sslContext) {
 
     @Override
     public WebSocketServerFactory build() {
+        verify();
+
         return ciphers == null
                 ? new DefaultSSLWebSocketServerFactory(sslContext)
                 : new CustomSSLWebSocketServerFactory(sslContext, ciphers);

ocpp-v1_6/src/main/java/eu/chargetime/ocpp/wss/BaseWssSocketBuilder.java

@@ -40,11 +40,14 @@ public class BaseWssSocketBuilder implements WssSocketBuilder {
 
     public static final int DEFAULT_WSS_PORT = 443;
     private Proxy proxy = Proxy.NO_PROXY;
+    private SocketFactory socketFactory = Socket::new;
     private SSLSocketFactory sslSocketFactory;
     private boolean tcpNoDelay;
     private boolean reuseAddr;
     private boolean autoClose = true;
     private URI uri;
+    private InetSocketAddressFactory inetSocketAddressFactory =
+            (host, port) -> new InetSocketAddress(host, port);
 
     // 0 for infinite timeout
     private int connectionTimeout = 0;
@@ -65,6 +68,16 @@ public BaseWssSocketBuilder sslSocketFactory(SSLSocketFactory sslSocketFactory)
         return this;
     }
 
+    public BaseWssSocketBuilder socketFactory(SocketFactory socketFactory) {
+        this.socketFactory = socketFactory;
+        return this;
+    }
+
+    public BaseWssSocketBuilder inetSocketAddressFactory(InetSocketAddressFactory inetSocketAddressFactory) {
+        this.inetSocketAddressFactory = inetSocketAddressFactory;
+        return this;
+    }
+
     public BaseWssSocketBuilder tcpNoDelay(boolean tcpNoDelay) {
         this.tcpNoDelay = tcpNoDelay;
         return this;
@@ -93,22 +106,43 @@ public BaseWssSocketBuilder connectionTimeout(int connectionTimeout) {
 
     @Override
     public Socket build() throws IOException {
-        Socket socket = new Socket(proxy);
+        verify(true);
+
+        Socket socket = socketFactory.getSocket(proxy);
         socket.setTcpNoDelay(tcpNoDelay);
         socket.setReuseAddress(reuseAddr);
 
-        if( !socket.isBound() ) {
-            socket.connect(new InetSocketAddress(uri.getHost(), getPort(uri)), connectionTimeout);
+        if(!socket.isBound()) {
+            socket.connect(inetSocketAddressFactory.getInetSocketAddress(uri.getHost(), getPort(uri)),
+                    connectionTimeout);
         }
 
         return sslSocketFactory.createSocket(socket, uri.getHost(), getPort(uri), autoClose);
     }
 
     @Override
     public void verify() {
+        verify(false);
+    }
+
+    public interface SocketFactory {
+        Socket getSocket(Proxy proxy);
+    }
+
+    public interface InetSocketAddressFactory {
+        InetSocketAddress getInetSocketAddress(String host, int port);
+    }
+
+    private void verify(boolean complete) {
         if(sslSocketFactory == null) {
             throw new IllegalStateException("sslSocketFactory must be set");
         }
+
+        if(complete) {
+            if(uri == null) {
+                throw new IllegalStateException("uri must be set");
+            }
+        }
     }
 
     private int getPort(URI uri) {

ocpp-v1_6/src/test/java/eu/chargetime/ocpp/wss/BaseWssFactoryBuilderTest.java

@@ -0,0 +1,65 @@
+package eu.chargetime.ocpp.wss;
+
+import org.java_websocket.WebSocketServerFactory;
+import org.java_websocket.server.DefaultSSLWebSocketServerFactory;
+import org.junit.Test;
+
+import javax.net.ssl.SSLContext;
+
+import java.util.List;
+
+import static org.hamcrest.CoreMatchers.any;
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.mock;
+
+/**
+ * Test for {@link BaseWssFactoryBuilder}
+ */
+public class BaseWssFactoryBuilderTest {
+
+    @Test
+    public void builder_returnsBuilder() {
+        BaseWssFactoryBuilder builder = BaseWssFactoryBuilder.builder();
+        assertThat(builder, is(any(BaseWssFactoryBuilder.class)));
+    }
+
+    @Test
+    public void builder_withSSLContextSet_throwsNoException() {
+        SSLContext sslContext = mock(SSLContext.class);
+        BaseWssFactoryBuilder.builder().sslContext(sslContext).verify();
+    }
+
+    @Test
+    public void builder_builtWithCiphers_returnsCustomSSLWebSocketServerFactory() {
+        SSLContext sslContext = mock(SSLContext.class);
+        List<String> cihpers = mock(List.class);
+        WebSocketServerFactory factory = BaseWssFactoryBuilder.builder()
+                .sslContext(sslContext)
+                .ciphers(cihpers)
+                .build();
+
+        assertThat(factory, is(instanceOf(CustomSSLWebSocketServerFactory.class)));
+    }
+
+    @Test
+    public void builder_builtWithoutCiphers_returnsDefaultSSLWebSocketServerFactory() {
+        SSLContext sslContext = mock(SSLContext.class);
+        WebSocketServerFactory factory = BaseWssFactoryBuilder.builder()
+                .sslContext(sslContext)
+                .build();
+
+        assertThat(factory, is(instanceOf(DefaultSSLWebSocketServerFactory.class)));
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void builder_withoutSSLContextSet_failsBuildWithException() {
+        BaseWssFactoryBuilder.builder().build();
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void builder_withoutSSLContextSet_failsVerificationWithException() {
+        BaseWssFactoryBuilder.builder().verify();
+    }
+}