Skip to content

Commit b93a138

Browse files
TVoldenTVolden
authored
Merge pull request #59 from eupakhomov/0.5_sonar_complains_fixed_tests_added
Tests added, critical Sonar complains resolved
2 parents 48d0dfd + 84f123f commit b93a138

7 files changed

+349
-19
lines changed

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

+9-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
import eu.chargetime.ocpp.model.CallMessage;
66
import eu.chargetime.ocpp.model.CallResultMessage;
77
import eu.chargetime.ocpp.model.Message;
8+
import org.slf4j.Logger;
9+
import org.slf4j.LoggerFactory;
810

911
import java.lang.reflect.Type;
1012
import java.text.ParseException;
@@ -46,6 +48,8 @@ of this software and associated documentation files (the "Software"), to deal
4648
*/
4749
public class JSONCommunicator extends Communicator {
4850

51+
private static final Logger logger = LoggerFactory.getLogger(JSONCommunicator.class);
52+
4953
private static final int INDEX_MESSAGEID = 0;
5054
private static final int TYPENUMBER_CALL = 2;
5155
private static final int INDEX_CALL_ACTION = 2;
@@ -138,7 +142,7 @@ protected Object makeCallError(String uniqueId, String action, String errorCode,
138142

139143
@Override
140144
protected Message parse(Object json) {
141-
Message message = null;
145+
Message message;
142146
JsonParser parser = new JsonParser();
143147
JsonArray array = parser.parse(json.toString()).getAsJsonArray();
144148

@@ -154,7 +158,11 @@ protected Message parse(Object json) {
154158
((CallErrorMessage) message).setErrorCode(array.get(INDEX_CALLERROR_ERRORCODE).getAsString());
155159
((CallErrorMessage) message).setErrorDescription(array.get(INDEX_CALLERROR_DESCRIPTION).getAsString());
156160
((CallErrorMessage) message).setRawPayload(array.get(INDEX_CALLERROR_PAYLOAD).toString());
161+
} else {
162+
logger.error("Unknown message type of message: {}", json.toString());
163+
throw new IllegalArgumentException("Unknown message type");
157164
}
165+
158166
message.setId(array.get(INDEX_UNIQUEID).getAsString());
159167

160168
return message;

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

+14-15
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
package eu.chargetime.ocpp;/*
1+
package eu.chargetime.ocpp;
2+
3+
/*
24
ChargeTime.eu - Java-OCA-OCPP
35
46
MIT License
@@ -63,9 +65,9 @@ public <T> T unpackPayload(Object payload, Class<T> type) {
6365
T output = null;
6466
try {
6567
Document input = (Document) payload;
66-
input = setNamespace(input, "urn://Ocpp/Cs/2015/10/");
68+
setNamespace(input, "urn://Ocpp/Cs/2015/10/");
6769
Unmarshaller unmarshaller = JAXBContext.newInstance(type).createUnmarshaller();
68-
JAXBElement<T> jaxbElement = (JAXBElement<T>) unmarshaller.unmarshal(input, type);
70+
JAXBElement<T> jaxbElement = unmarshaller.unmarshal(input, type);
6971
output = jaxbElement.getValue();
7072
} catch (JAXBException e) {
7173
logger.warn("unpackPayload() failed", e);
@@ -82,17 +84,14 @@ public Object packPayload(Object payload) {
8284
factory.setNamespaceAware(false);
8385
document = factory.newDocumentBuilder().newDocument();
8486
marshaller.marshal(payload, document);
85-
document = setNamespace(document, hostInfo.getNamespace());
86-
} catch (JAXBException e) {
87+
setNamespace(document, hostInfo.getNamespace());
88+
} catch (JAXBException | ParserConfigurationException e) {
8789
logger.warn("packPayload() failed", e);
88-
} catch (ParserConfigurationException e) {
89-
logger.warn("packPayload() failed", e);
9090
}
9191
return document;
9292
}
9393

94-
private Document setNamespace(Document document, String namespace) {
95-
Document output = document;
94+
private void setNamespace(Document document, String namespace) {
9695
Element orgElement = document.getDocumentElement();
9796
Element newElement = document.createElementNS(namespace, orgElement.getNodeName());
9897

@@ -101,8 +100,7 @@ private Document setNamespace(Document document, String namespace) {
101100
appendChildNS(document, newElement, childNodes.item(i), namespace);
102101
}
103102

104-
output.replaceChild(newElement, orgElement);
105-
return output;
103+
document.replaceChild(newElement, orgElement);
106104
}
107105

108106
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
173171
createMessageHeader(uniqueId, action, isResponse, message);
174172

175173
if (isResponse) {
176-
payload = setNamespace(payload, hostInfo.isClient() ? SOAPHostInfo.NAMESPACE_CHARGEBOX : SOAPHostInfo.NAMESPACE_CENTRALSYSTEM);
174+
setNamespace(payload, hostInfo.isClient() ? SOAPHostInfo.NAMESPACE_CHARGEBOX : SOAPHostInfo.NAMESPACE_CENTRALSYSTEM);
177175
}
178176

179177
message.getSOAPBody().addDocument(payload);
@@ -260,7 +258,8 @@ public Message parseMessage() {
260258

261259
String relatesTo = getElementValue(HEADER_RELATESTO);
262260
String action = getElementValue(HEADER_ACTION);
263-
if (relatesTo != null && !"".equals(relatesTo) && action.endsWith("Response")) {
261+
262+
if (relatesTo != null && !relatesTo.isEmpty() && action != null && action.endsWith("Response")) {
264263
if (soapMessage.getSOAPBody().hasFault())
265264
output = parseError();
266265
else
@@ -269,7 +268,7 @@ public Message parseMessage() {
269268
output = parseCall();
270269
}
271270

272-
if (action != null && !"".equals(action))
271+
if (action != null && !action.isEmpty())
273272
output.setAction(action.substring(1));
274273

275274
if (!soapMessage.getSOAPBody().hasFault())
@@ -302,7 +301,7 @@ private CallErrorMessage parseError() {
302301
message.setErrorDescription(fault.getFaultReasonTexts().next().toString());
303302

304303
} catch (SOAPException e) {
305-
e.printStackTrace();
304+
logger.error("Parse error", e);
306305
}
307306

308307
return message;

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

+2
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ public BaseWssFactoryBuilder sslContext(SSLContext sslContext) {
5959

6060
@Override
6161
public WebSocketServerFactory build() {
62+
verify();
63+
6264
return ciphers == null
6365
? new DefaultSSLWebSocketServerFactory(sslContext)
6466
: new CustomSSLWebSocketServerFactory(sslContext, ciphers);

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

+37-3
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,14 @@ public class BaseWssSocketBuilder implements WssSocketBuilder {
4040

4141
public static final int DEFAULT_WSS_PORT = 443;
4242
private Proxy proxy = Proxy.NO_PROXY;
43+
private SocketFactory socketFactory = Socket::new;
4344
private SSLSocketFactory sslSocketFactory;
4445
private boolean tcpNoDelay;
4546
private boolean reuseAddr;
4647
private boolean autoClose = true;
4748
private URI uri;
49+
private InetSocketAddressFactory inetSocketAddressFactory =
50+
(host, port) -> new InetSocketAddress(host, port);
4851

4952
// 0 for infinite timeout
5053
private int connectionTimeout = 0;
@@ -65,6 +68,16 @@ public BaseWssSocketBuilder sslSocketFactory(SSLSocketFactory sslSocketFactory)
6568
return this;
6669
}
6770

71+
public BaseWssSocketBuilder socketFactory(SocketFactory socketFactory) {
72+
this.socketFactory = socketFactory;
73+
return this;
74+
}
75+
76+
public BaseWssSocketBuilder inetSocketAddressFactory(InetSocketAddressFactory inetSocketAddressFactory) {
77+
this.inetSocketAddressFactory = inetSocketAddressFactory;
78+
return this;
79+
}
80+
6881
public BaseWssSocketBuilder tcpNoDelay(boolean tcpNoDelay) {
6982
this.tcpNoDelay = tcpNoDelay;
7083
return this;
@@ -93,22 +106,43 @@ public BaseWssSocketBuilder connectionTimeout(int connectionTimeout) {
93106

94107
@Override
95108
public Socket build() throws IOException {
96-
Socket socket = new Socket(proxy);
109+
verify(true);
110+
111+
Socket socket = socketFactory.getSocket(proxy);
97112
socket.setTcpNoDelay(tcpNoDelay);
98113
socket.setReuseAddress(reuseAddr);
99114

100-
if( !socket.isBound() ) {
101-
socket.connect(new InetSocketAddress(uri.getHost(), getPort(uri)), connectionTimeout);
115+
if(!socket.isBound()) {
116+
socket.connect(inetSocketAddressFactory.getInetSocketAddress(uri.getHost(), getPort(uri)),
117+
connectionTimeout);
102118
}
103119

104120
return sslSocketFactory.createSocket(socket, uri.getHost(), getPort(uri), autoClose);
105121
}
106122

107123
@Override
108124
public void verify() {
125+
verify(false);
126+
}
127+
128+
public interface SocketFactory {
129+
Socket getSocket(Proxy proxy);
130+
}
131+
132+
public interface InetSocketAddressFactory {
133+
InetSocketAddress getInetSocketAddress(String host, int port);
134+
}
135+
136+
private void verify(boolean complete) {
109137
if(sslSocketFactory == null) {
110138
throw new IllegalStateException("sslSocketFactory must be set");
111139
}
140+
141+
if(complete) {
142+
if(uri == null) {
143+
throw new IllegalStateException("uri must be set");
144+
}
145+
}
112146
}
113147

114148
private int getPort(URI uri) {

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

+65
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package eu.chargetime.ocpp.wss;
2+
3+
import org.java_websocket.WebSocketServerFactory;
4+
import org.java_websocket.server.DefaultSSLWebSocketServerFactory;
5+
import org.junit.Test;
6+
7+
import javax.net.ssl.SSLContext;
8+
9+
import java.util.List;
10+
11+
import static org.hamcrest.CoreMatchers.any;
12+
import static org.hamcrest.CoreMatchers.instanceOf;
13+
import static org.hamcrest.CoreMatchers.is;
14+
import static org.junit.Assert.*;
15+
import static org.mockito.Mockito.mock;
16+
17+
/**
18+
* Test for {@link BaseWssFactoryBuilder}
19+
*/
20+
public class BaseWssFactoryBuilderTest {
21+
22+
@Test
23+
public void builder_returnsBuilder() {
24+
BaseWssFactoryBuilder builder = BaseWssFactoryBuilder.builder();
25+
assertThat(builder, is(any(BaseWssFactoryBuilder.class)));
26+
}
27+
28+
@Test
29+
public void builder_withSSLContextSet_throwsNoException() {
30+
SSLContext sslContext = mock(SSLContext.class);
31+
BaseWssFactoryBuilder.builder().sslContext(sslContext).verify();
32+
}
33+
34+
@Test
35+
public void builder_builtWithCiphers_returnsCustomSSLWebSocketServerFactory() {
36+
SSLContext sslContext = mock(SSLContext.class);
37+
List<String> cihpers = mock(List.class);
38+
WebSocketServerFactory factory = BaseWssFactoryBuilder.builder()
39+
.sslContext(sslContext)
40+
.ciphers(cihpers)
41+
.build();
42+
43+
assertThat(factory, is(instanceOf(CustomSSLWebSocketServerFactory.class)));
44+
}
45+
46+
@Test
47+
public void builder_builtWithoutCiphers_returnsDefaultSSLWebSocketServerFactory() {
48+
SSLContext sslContext = mock(SSLContext.class);
49+
WebSocketServerFactory factory = BaseWssFactoryBuilder.builder()
50+
.sslContext(sslContext)
51+
.build();
52+
53+
assertThat(factory, is(instanceOf(DefaultSSLWebSocketServerFactory.class)));
54+
}
55+
56+
@Test(expected = IllegalStateException.class)
57+
public void builder_withoutSSLContextSet_failsBuildWithException() {
58+
BaseWssFactoryBuilder.builder().build();
59+
}
60+
61+
@Test(expected = IllegalStateException.class)
62+
public void builder_withoutSSLContextSet_failsVerificationWithException() {
63+
BaseWssFactoryBuilder.builder().verify();
64+
}
65+
}

0 commit comments

Comments
 (0)