From 0fbeafe11f9904b292f3cbbf36a5c583fbb266b6 Mon Sep 17 00:00:00 2001 From: Eugene Pakhomov Date: Wed, 27 Jun 2018 13:16:57 +0200 Subject: [PATCH 1/5] JSONServer fixed to be backward compatible with previous versions of clients Minor clenup issues fixed in WebSocketListener --- .../src/main/java/eu/chargetime/ocpp/JSONServer.java | 8 ++++++-- .../java/eu/chargetime/ocpp/WebSocketListener.java | 11 ++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/JSONServer.java b/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/JSONServer.java index 81b1f46bd..3f9274d99 100644 --- a/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/JSONServer.java +++ b/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/JSONServer.java @@ -63,8 +63,12 @@ public class JSONServer implements IServerAPI { public JSONServer(ServerCoreProfile coreProfile, JSONConfiguration configuration) { featureRepository = new FeatureRepository(); SessionFactory sessionFactory = new SessionFactory(featureRepository); - draftOcppOnly = new Draft_6455(Collections.emptyList(), - Collections.singletonList(new Protocol("ocpp1.6"))); + + ArrayList protocols = new ArrayList<>(); + protocols.add(new Protocol("ocpp1.6")); + protocols.add(new Protocol("")); + draftOcppOnly = new Draft_6455(Collections.emptyList(), protocols); + this.listener = new WebSocketListener(sessionFactory, configuration, draftOcppOnly); server = new Server(this.listener, featureRepository, new PromiseRepository()); featureRepository.addFeatureProfile(coreProfile); diff --git a/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/WebSocketListener.java b/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/WebSocketListener.java index c9847727d..5ac7e67c7 100644 --- a/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/WebSocketListener.java +++ b/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/WebSocketListener.java @@ -107,8 +107,13 @@ public void relay(String message) { public void onClose(WebSocket webSocket, int code, String reason, boolean remote) { logger.debug("On connection close (resource descriptor: {}, code: {}, reason: {}, remote: {})", webSocket.getResourceDescriptor(), code, reason, remote); - sockets.get(webSocket).disconnect(); - sockets.remove(webSocket); + WebSocketReceiver receiver = sockets.get(webSocket); + if(receiver != null) { + receiver.disconnect(); + sockets.remove(webSocket); + } else { + logger.debug("Receiver for socket not found: {}", webSocket); + } } @Override @@ -167,8 +172,8 @@ public void close() { } try { - sockets.clear(); server.stop(TIMEOUT_IN_MILLIS); + sockets.clear(); } catch (InterruptedException e) { // Do second try try { From 7cd50d33c7a219f90cde73e69fad6287a44d66b8 Mon Sep 17 00:00:00 2001 From: Eugene Pakhomov Date: Wed, 27 Jun 2018 13:26:35 +0200 Subject: [PATCH 2/5] Missing imports added --- ocpp-v1_6/src/main/java/eu/chargetime/ocpp/JSONServer.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/JSONServer.java b/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/JSONServer.java index 3f9274d99..f938d944c 100644 --- a/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/JSONServer.java +++ b/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/JSONServer.java @@ -33,12 +33,14 @@ of this software and associated documentation files (the "Software"), to deal import eu.chargetime.ocpp.wss.WssFactoryBuilder; import org.java_websocket.drafts.Draft; import org.java_websocket.drafts.Draft_6455; +import org.java_websocket.protocols.IProtocol; import org.java_websocket.protocols.Protocol; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.net.ssl.SSLContext; import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; import java.util.UUID; import java.util.concurrent.CompletionStage; From 4e6628e784a40e5e2ad163ca450398630e2df434 Mon Sep 17 00:00:00 2001 From: Eugene Pakhomov Date: Wed, 27 Jun 2018 13:53:55 +0200 Subject: [PATCH 3/5] Seems like random integration test failure due to concurrency issues (locally test runs ok) Some logging added and timeouts increased (to be slightly greater than server shutdown timeout) to debug the issue --- .../java/eu/chargetime/ocpp/test/FakeCentralSystem.java | 6 ++++-- .../eu/chargetime/ocpp/test/base/json/JSONBaseSpec.groovy | 4 ++-- .../main/java/eu/chargetime/ocpp/WebSocketTransmitter.java | 3 +++ 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/ocpp-v1_6-test/src/main/java/eu/chargetime/ocpp/test/FakeCentralSystem.java b/ocpp-v1_6-test/src/main/java/eu/chargetime/ocpp/test/FakeCentralSystem.java index 074863ad9..1846bfc48 100644 --- a/ocpp-v1_6-test/src/main/java/eu/chargetime/ocpp/test/FakeCentralSystem.java +++ b/ocpp-v1_6-test/src/main/java/eu/chargetime/ocpp/test/FakeCentralSystem.java @@ -104,14 +104,16 @@ public void clientLost() { } public void started() throws Exception { + final String host = "127.0.0.1"; + if (!isStarted) { int port = 8890; if (server instanceof JSONTestServer) { port = 8887; } - server.open("127.0.0.1", port, dummyHandlers.generateServerEventsHandler()); - logger.info("Server started on port: {}", port); + server.open(host, port, dummyHandlers.generateServerEventsHandler()); + logger.info("Server started on host: {}, port: {}", host, port); isStarted = true; } } diff --git a/ocpp-v1_6-test/src/test/groovy/eu/chargetime/ocpp/test/base/json/JSONBaseSpec.groovy b/ocpp-v1_6-test/src/test/groovy/eu/chargetime/ocpp/test/base/json/JSONBaseSpec.groovy index 4afb46742..5c4d57b05 100644 --- a/ocpp-v1_6-test/src/test/groovy/eu/chargetime/ocpp/test/base/json/JSONBaseSpec.groovy +++ b/ocpp-v1_6-test/src/test/groovy/eu/chargetime/ocpp/test/base/json/JSONBaseSpec.groovy @@ -14,7 +14,7 @@ abstract class JSONBaseSpec extends Specification { FakeChargePoint chargePoint = new FakeChargePoint() def setupSpec() { - def conditions = new PollingConditions(timeout: 10) + def conditions = new PollingConditions(timeout: 11) // When a Central System is running centralSystem.started() @@ -36,7 +36,7 @@ abstract class JSONBaseSpec extends Specification { } def cleanupSpec() { - def conditions = new PollingConditions(timeout: 10) + def conditions = new PollingConditions(timeout: 11) centralSystem.stopped() diff --git a/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/WebSocketTransmitter.java b/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/WebSocketTransmitter.java index 88f021473..d06eee936 100644 --- a/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/WebSocketTransmitter.java +++ b/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/WebSocketTransmitter.java @@ -112,6 +112,9 @@ public void onError(Exception ex) { } configure(); + + logger.debug("Trying to connect to: {}", resource); + try { client.connectBlocking(); closed = false; From 5b35f949b9554e0a674f524012a1b62639c3996c Mon Sep 17 00:00:00 2001 From: Eugene Pakhomov Date: Wed, 27 Jun 2018 18:43:03 +0200 Subject: [PATCH 4/5] Add session UUID to server firmware management handler to be able to tie requests to certain session --- .../ServerFirmwareManagementEventHandler.java | 6 ++++-- .../profile/ServerFirmwareManagementProfile.java | 4 ++-- .../test/ServerFirmwareManagementProfileTest.java | 14 +++++++------- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/feature/profile/ServerFirmwareManagementEventHandler.java b/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/feature/profile/ServerFirmwareManagementEventHandler.java index 9c1f5f0c0..934c23212 100644 --- a/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/feature/profile/ServerFirmwareManagementEventHandler.java +++ b/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/feature/profile/ServerFirmwareManagementEventHandler.java @@ -30,8 +30,10 @@ of this software and associated documentation files (the "Software"), to deal import eu.chargetime.ocpp.model.firmware.FirmwareStatusNotificationConfirmation; import eu.chargetime.ocpp.model.firmware.FirmwareStatusNotificationRequest; +import java.util.UUID; + public interface ServerFirmwareManagementEventHandler { - DiagnosticsStatusNotificationConfirmation handleDiagnosticsStatusNotificationRequest(DiagnosticsStatusNotificationRequest request); + DiagnosticsStatusNotificationConfirmation handleDiagnosticsStatusNotificationRequest(UUID sessionIndex, DiagnosticsStatusNotificationRequest request); - FirmwareStatusNotificationConfirmation handleFirmwareStatusNotificationRequest(FirmwareStatusNotificationRequest request); + FirmwareStatusNotificationConfirmation handleFirmwareStatusNotificationRequest(UUID sessionIndex, FirmwareStatusNotificationRequest request); } diff --git a/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/feature/profile/ServerFirmwareManagementProfile.java b/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/feature/profile/ServerFirmwareManagementProfile.java index cd22f3b73..9d4766904 100644 --- a/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/feature/profile/ServerFirmwareManagementProfile.java +++ b/ocpp-v1_6/src/main/java/eu/chargetime/ocpp/feature/profile/ServerFirmwareManagementProfile.java @@ -58,9 +58,9 @@ public Confirmation handleRequest(UUID sessionIndex, Request request) { Confirmation result = null; if (request instanceof DiagnosticsStatusNotificationRequest) { - result = eventHandler.handleDiagnosticsStatusNotificationRequest((DiagnosticsStatusNotificationRequest) request); + result = eventHandler.handleDiagnosticsStatusNotificationRequest(sessionIndex, (DiagnosticsStatusNotificationRequest) request); } else if (request instanceof FirmwareStatusNotificationRequest) { - result = eventHandler.handleFirmwareStatusNotificationRequest((FirmwareStatusNotificationRequest) request); + result = eventHandler.handleFirmwareStatusNotificationRequest(sessionIndex, (FirmwareStatusNotificationRequest) request); } return result; diff --git a/ocpp-v1_6/src/test/java/eu/chargetime/ocpp/feature/profile/test/ServerFirmwareManagementProfileTest.java b/ocpp-v1_6/src/test/java/eu/chargetime/ocpp/feature/profile/test/ServerFirmwareManagementProfileTest.java index 6c4ce33de..6081db435 100644 --- a/ocpp-v1_6/src/test/java/eu/chargetime/ocpp/feature/profile/test/ServerFirmwareManagementProfileTest.java +++ b/ocpp-v1_6/src/test/java/eu/chargetime/ocpp/feature/profile/test/ServerFirmwareManagementProfileTest.java @@ -47,8 +47,6 @@ of this software and associated documentation files (the "Software"), to deal @RunWith(MockitoJUnitRunner.class) public class ServerFirmwareManagementProfileTest extends ProfileTest { - private static final UUID SESSION_NULL = null; - ServerFirmwareManagementProfile profile; @Mock @@ -99,24 +97,26 @@ public void getFeatureList_containsUpdateFirmwareFeature() { public void handleRequest_aDiagnosticsStatusNotificationRequest_callsHandleDiagnosticsStatusNotificationRequest() { // Given DiagnosticsStatusNotificationRequest request = new DiagnosticsStatusNotificationRequest(); + UUID sessionId = UUID.randomUUID(); // When - profile.handleRequest(SESSION_NULL, request); + profile.handleRequest(sessionId, request); // Then - verify(handler, times(1)).handleDiagnosticsStatusNotificationRequest(eq(request)); + verify(handler, times(1)).handleDiagnosticsStatusNotificationRequest(eq(sessionId), eq(request)); } @Test public void handleRequest_aFirmwareStatusNotificationRequest_callsHandleFirmwareStatusNotificationRequest() { // Given FirmwareStatusNotificationRequest request = new FirmwareStatusNotificationRequest(); + UUID sessionId = UUID.randomUUID(); // When - profile.handleRequest(SESSION_NULL, request); + profile.handleRequest(sessionId, request); // Then - verify(handler, times(1)).handleFirmwareStatusNotificationRequest(eq(request)); + verify(handler, times(1)).handleFirmwareStatusNotificationRequest(eq(sessionId), eq(request)); } -} \ No newline at end of file +} From 6df35bd7c663de5096c8b5be0d9278372c779c5e Mon Sep 17 00:00:00 2001 From: Eugene Pakhomov Date: Wed, 27 Jun 2018 18:50:58 +0200 Subject: [PATCH 5/5] Fixed dummy handlers --- .../src/main/java/eu/chargetime/ocpp/test/DummyHandlers.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ocpp-v1_6-test/src/main/java/eu/chargetime/ocpp/test/DummyHandlers.java b/ocpp-v1_6-test/src/main/java/eu/chargetime/ocpp/test/DummyHandlers.java index 9dfa04f6f..e7bb13286 100644 --- a/ocpp-v1_6-test/src/main/java/eu/chargetime/ocpp/test/DummyHandlers.java +++ b/ocpp-v1_6-test/src/main/java/eu/chargetime/ocpp/test/DummyHandlers.java @@ -133,14 +133,14 @@ public StopTransactionConfirmation handleStopTransactionRequest(UUID sessionInde public ServerFirmwareManagementEventHandler createServerFirmwareManagementEventHandler() { return new ServerFirmwareManagementEventHandler() { @Override - public DiagnosticsStatusNotificationConfirmation handleDiagnosticsStatusNotificationRequest(DiagnosticsStatusNotificationRequest request) { + public DiagnosticsStatusNotificationConfirmation handleDiagnosticsStatusNotificationRequest(UUID sessionId, DiagnosticsStatusNotificationRequest request) { receivedRequest = request; DiagnosticsStatusNotificationConfirmation confirmation = new DiagnosticsStatusNotificationConfirmation(); return failurePoint(confirmation); } @Override - public FirmwareStatusNotificationConfirmation handleFirmwareStatusNotificationRequest(FirmwareStatusNotificationRequest request) { + public FirmwareStatusNotificationConfirmation handleFirmwareStatusNotificationRequest(UUID sessionId, FirmwareStatusNotificationRequest request) { receivedRequest = request; FirmwareStatusNotificationConfirmation confirmation = new FirmwareStatusNotificationConfirmation(); return failurePoint(confirmation);