Skip to content

Commit ddc56a2

Browse files
committed
improve impl and ws logging
1 parent a39594b commit ddc56a2

7 files changed

Lines changed: 147 additions & 100 deletions

File tree

src/main/java/de/rwth/idsg/steve/SteveConfiguration.java

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -14,27 +14,27 @@ public enum SteveConfiguration {
1414
CONFIG;
1515

1616
// Root mapping for Spring
17-
private String springMapping = "/";
17+
private final String springMapping = "/";
1818
// Web frontend
19-
private String springManagerMapping = "/manager/*";
19+
private final String springManagerMapping = "/manager/*";
2020
// Mapping for CXF SOAP services
21-
private String cxfMapping = "/services/*";
21+
private final String cxfMapping = "/services/*";
2222
// Dummy service path
23-
private String routerEndpointPath = "/CentralSystemService";
23+
private final String routerEndpointPath = "/CentralSystemService";
2424
// Time zone for the application and database connections
25-
private String timeZoneId = "UTC"; // or ZoneId.systemDefault().getId();
25+
private final String timeZoneId = "UTC"; // or ZoneId.systemDefault().getId();
2626

2727
// -------------------------------------------------------------------------
2828
// main.properties
2929
// -------------------------------------------------------------------------
3030

31-
private String contextPath;
32-
private String steveVersion;
33-
private ApplicationProfile profile;
34-
private Ocpp ocpp;
35-
private Auth auth;
36-
private DB db;
37-
private Jetty jetty;
31+
private final String contextPath;
32+
private final String steveVersion;
33+
private final ApplicationProfile profile;
34+
private final Ocpp ocpp;
35+
private final Auth auth;
36+
private final DB db;
37+
private final Jetty jetty;
3838

3939
SteveConfiguration() {
4040
PropertiesFileLoader p = new PropertiesFileLoader("main.properties");
@@ -103,43 +103,43 @@ private void validate() {
103103
// Jetty configuration
104104
@Builder @Getter
105105
public static class Jetty {
106-
private String serverHost;
107-
private boolean gzipEnabled;
106+
private final String serverHost;
107+
private final boolean gzipEnabled;
108108

109109
// HTTP
110-
private boolean httpEnabled;
111-
private int httpPort;
110+
private final boolean httpEnabled;
111+
private final int httpPort;
112112

113113
// HTTPS
114-
private boolean httpsEnabled;
115-
private int httpsPort;
116-
private String keyStorePath;
117-
private String keyStorePassword;
114+
private final boolean httpsEnabled;
115+
private final int httpsPort;
116+
private final String keyStorePath;
117+
private final String keyStorePassword;
118118
}
119119

120120
// Database configuration
121121
@Builder @Getter
122122
public static class DB {
123-
private String ip;
124-
private int port;
125-
private String schema;
126-
private String userName;
127-
private String password;
128-
private boolean sqlLogging;
123+
private final String ip;
124+
private final int port;
125+
private final String schema;
126+
private final String userName;
127+
private final String password;
128+
private final boolean sqlLogging;
129129
}
130130

131131
// Credentials for Web interface access
132132
@Builder @Getter
133133
public static class Auth {
134-
private String userName;
135-
private String password;
134+
private final String userName;
135+
private final String password;
136136
}
137137

138138
// OCPP-related configuration
139139
@Builder @Getter
140140
public static class Ocpp {
141-
private boolean autoRegisterUnknownStations;
142-
private WsSessionSelectStrategyEnum wsSessionSelectStrategy;
141+
private final boolean autoRegisterUnknownStations;
142+
private final WsSessionSelectStrategyEnum wsSessionSelectStrategy;
143143
}
144144

145145
}

src/main/java/de/rwth/idsg/steve/ocpp/ws/AbstractWebSocketEndpoint.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ private void handleTextMessage(WebSocketSession session, TextMessage webSocketMe
8282
String incomingString = webSocketMessage.getPayload();
8383
String chargeBoxId = getChargeBoxId(session);
8484

85-
log.info("[chargeBoxId={}, sessionId={}] Received message: {}", chargeBoxId, session.getId(), incomingString);
85+
WebSocketLogger.receivedText(chargeBoxId, session, incomingString);
8686

8787
CommunicationContext context = new CommunicationContext(session, chargeBoxId);
8888
context.setIncomingString(incomingString);
@@ -91,15 +91,17 @@ private void handleTextMessage(WebSocketSession session, TextMessage webSocketMe
9191
}
9292

9393
private void handlePongMessage(WebSocketSession session) {
94-
log.debug("[id={}] Received pong message", session.getId());
94+
WebSocketLogger.receivedPong(getChargeBoxId(session), session);
9595

9696
// TODO: Not sure about the following. Should update DB? Should call directly repo?
9797
ocppServerRepository.updateChargeboxHeartbeat(getChargeBoxId(session), DateTime.now());
9898
}
9999

100100
@Override
101101
public void afterConnectionEstablished(WebSocketSession session) throws Exception {
102-
log.info("New connection established: {}", session);
102+
String chargeBoxId = getChargeBoxId(session);
103+
104+
WebSocketLogger.connected(chargeBoxId, session);
103105

104106
// Just to keep the connection alive, such that the servers do not close
105107
// the connection because of a idle timeout, we ping-pong at fixed intervals.
@@ -109,8 +111,6 @@ public void afterConnectionEstablished(WebSocketSession session) throws Exceptio
109111
WebSocketConfiguration.PING_INTERVAL,
110112
TimeUnit.MINUTES);
111113

112-
String chargeBoxId = getChargeBoxId(session);
113-
114114
futureResponseContextStore.addSession(session);
115115

116116
int sizeBeforeAdd;
@@ -129,10 +129,10 @@ public void afterConnectionEstablished(WebSocketSession session) throws Exceptio
129129

130130
@Override
131131
public void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) throws Exception {
132-
log.warn("[id={}] Connection was closed, status: {}", session.getId(), closeStatus);
133-
134132
String chargeBoxId = getChargeBoxId(session);
135133

134+
WebSocketLogger.closed(chargeBoxId, session, closeStatus);
135+
136136
futureResponseContextStore.removeSession(session);
137137

138138
int sizeAfterRemove;

src/main/java/de/rwth/idsg/steve/ocpp/ws/FutureResponseContextStoreImpl.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,10 @@ public class FutureResponseContextStoreImpl implements FutureResponseContextStor
2626

2727
@Override
2828
public void addSession(WebSocketSession session) {
29-
Map<String, FutureResponseContext> contextMap = lookupTable.get(session);
30-
if (contextMap == null) {
31-
log.debug("Creating new store for sessionId '{}'", session.getId());
32-
lookupTable.put(session, new ConcurrentHashMap<>());
33-
}
29+
lookupTable.computeIfAbsent(session, webSocketSession -> {
30+
log.debug("Creating new store for sessionId '{}'", webSocketSession.getId());
31+
return new ConcurrentHashMap<>();
32+
});
3433
}
3534

3635
@Override

src/main/java/de/rwth/idsg/steve/ocpp/ws/SessionContextStore.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@
1515
public interface SessionContextStore {
1616
void add(String chargeBoxId, WebSocketSession session, ScheduledFuture pingSchedule);
1717
void remove(String chargeBoxId, WebSocketSession session);
18+
WebSocketSession getSession(String chargeBoxId);
1819
int getSize(String chargeBoxId);
20+
int getNumberOfChargeBoxes();
1921
List<String> getChargeBoxIdList();
2022
Map<String, Deque<SessionContext>> getACopy();
21-
int getNumberOfChargeBoxes();
22-
WebSocketSession getSession(String chargeBoxId);
2323
}
Lines changed: 69 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package de.rwth.idsg.steve.ocpp.ws;
22

3-
import com.google.common.base.Strings;
43
import com.google.common.collect.ImmutableMap;
4+
import com.google.common.util.concurrent.Striped;
55
import de.rwth.idsg.steve.SteveException;
66
import de.rwth.idsg.steve.ocpp.ws.custom.WsSessionSelectStrategy;
77
import de.rwth.idsg.steve.ocpp.ws.data.SessionContext;
@@ -17,6 +17,7 @@
1717
import java.util.NoSuchElementException;
1818
import java.util.concurrent.ConcurrentHashMap;
1919
import java.util.concurrent.ScheduledFuture;
20+
import java.util.concurrent.locks.Lock;
2021

2122
/**
2223
* @author Sevket Goekay <goekay@dbis.rwth-aachen.de>
@@ -31,6 +32,8 @@ public class SessionContextStoreImpl implements SessionContextStore {
3132
*/
3233
private final ConcurrentHashMap<String, Deque<SessionContext>> lookupTable = new ConcurrentHashMap<>();
3334

35+
private final Striped<Lock> locks = Striped.lock(16);
36+
3437
private final WsSessionSelectStrategy wsSessionSelectStrategy;
3538

3639
public SessionContextStoreImpl(WsSessionSelectStrategy wsSessionSelectStrategy) {
@@ -39,48 +42,77 @@ public SessionContextStoreImpl(WsSessionSelectStrategy wsSessionSelectStrategy)
3942

4043
@Override
4144
public void add(String chargeBoxId, WebSocketSession session, ScheduledFuture pingSchedule) {
42-
SessionContext context = new SessionContext(session, pingSchedule, DateTime.now());
45+
Lock l = locks.get(chargeBoxId);
46+
l.lock();
47+
try {
48+
SessionContext context = new SessionContext(session, pingSchedule, DateTime.now());
4349

44-
Deque<SessionContext> endpointDeque = lookupTable.computeIfAbsent(chargeBoxId, str -> new ArrayDeque<>());
45-
endpointDeque.addLast(context); // Adding at the end
50+
Deque<SessionContext> endpointDeque = lookupTable.computeIfAbsent(chargeBoxId, str -> new ArrayDeque<>());
51+
endpointDeque.addLast(context); // Adding at the end
4652

47-
log.debug("A new SessionContext is stored for chargeBoxId '{}'. Store size: {}",
48-
chargeBoxId, endpointDeque.size());
53+
log.debug("A new SessionContext is stored for chargeBoxId '{}'. Store size: {}",
54+
chargeBoxId, endpointDeque.size());
55+
} finally {
56+
l.unlock();
57+
}
4958
}
5059

5160
@Override
5261
public void remove(String chargeBoxId, WebSocketSession session) {
53-
Deque<SessionContext> endpointDeque = lookupTable.get(chargeBoxId);
54-
if (endpointDeque == null) {
55-
log.debug("No session context to remove for chargeBoxId '{}'", chargeBoxId);
56-
return;
57-
}
62+
Lock l = locks.get(chargeBoxId);
63+
l.lock();
64+
try {
65+
Deque<SessionContext> endpointDeque = lookupTable.get(chargeBoxId);
66+
if (endpointDeque == null) {
67+
log.debug("No session context to remove for chargeBoxId '{}'", chargeBoxId);
68+
return;
69+
}
5870

59-
// Prevent "java.util.ConcurrentModificationException: null"
60-
// Reason: Cannot modify the set (remove the item) we are iterating
61-
// Solution: Iterate the set, find the item, remove the item after the for-loop
62-
//
63-
SessionContext toRemove = null;
64-
for (SessionContext context : endpointDeque) {
65-
if (context.getSession().getId().equals(session.getId())) {
66-
toRemove = context;
67-
break;
71+
// Prevent "java.util.ConcurrentModificationException: null"
72+
// Reason: Cannot modify the set (remove the item) we are iterating
73+
// Solution: Iterate the set, find the item, remove the item after the for-loop
74+
//
75+
SessionContext toRemove = null;
76+
for (SessionContext context : endpointDeque) {
77+
if (context.getSession().getId().equals(session.getId())) {
78+
toRemove = context;
79+
break;
80+
}
6881
}
69-
}
7082

71-
if (toRemove != null) {
72-
// 1. Cancel the ping task
73-
toRemove.getPingSchedule().cancel(true);
74-
// 2. Delete from collection
75-
if (endpointDeque.remove(toRemove)) {
76-
log.debug("A SessionContext is removed for chargeBoxId '{}'. Store size: {}",
77-
chargeBoxId, endpointDeque.size());
83+
if (toRemove != null) {
84+
// 1. Cancel the ping task
85+
toRemove.getPingSchedule().cancel(true);
86+
// 2. Delete from collection
87+
if (endpointDeque.remove(toRemove)) {
88+
log.debug("A SessionContext is removed for chargeBoxId '{}'. Store size: {}",
89+
chargeBoxId, endpointDeque.size());
90+
}
91+
// 3. Delete empty collection from lookup table in order to correctly calculate
92+
// the number of connected chargeboxes with getNumberOfChargeBoxes()
93+
if (endpointDeque.size() == 0) {
94+
lookupTable.remove(chargeBoxId);
95+
}
7896
}
79-
// 3. Delete empty collection from lookup table in order to correctly calculate
80-
// the number of connected chargeboxes with getNumberOfChargeBoxes()
81-
if (endpointDeque.size() == 0) {
82-
lookupTable.remove(chargeBoxId);
97+
} finally {
98+
l.unlock();
99+
}
100+
}
101+
102+
@Override
103+
public WebSocketSession getSession(String chargeBoxId) {
104+
Lock l = locks.get(chargeBoxId);
105+
l.lock();
106+
try {
107+
Deque<SessionContext> endpointDeque = lookupTable.get(chargeBoxId);
108+
if (endpointDeque == null) {
109+
throw new NoSuchElementException();
83110
}
111+
return wsSessionSelectStrategy.getSession(endpointDeque);
112+
} catch (NoSuchElementException e) {
113+
throw new SteveException("No session context for chargeBoxId '%s'", chargeBoxId, e);
114+
} finally {
115+
l.unlock();
84116
}
85117
}
86118

@@ -94,6 +126,11 @@ public int getSize(String chargeBoxId) {
94126
}
95127
}
96128

129+
@Override
130+
public int getNumberOfChargeBoxes() {
131+
return lookupTable.size();
132+
}
133+
97134
@Override
98135
public List<String> getChargeBoxIdList() {
99136
return Collections.list(lookupTable.keys());
@@ -103,27 +140,4 @@ public List<String> getChargeBoxIdList() {
103140
public Map<String, Deque<SessionContext>> getACopy() {
104141
return ImmutableMap.copyOf(lookupTable);
105142
}
106-
107-
@Override
108-
public int getNumberOfChargeBoxes() {
109-
return lookupTable.size();
110-
}
111-
112-
@Override
113-
public WebSocketSession getSession(String chargeBoxId) {
114-
if (Strings.isNullOrEmpty(chargeBoxId)) {
115-
throw new SteveException("Invalid chargeBoxId (null or empty)");
116-
}
117-
118-
try {
119-
Deque<SessionContext> endpointDeque = lookupTable.get(chargeBoxId);
120-
if (endpointDeque == null) {
121-
throw new NoSuchElementException();
122-
}
123-
return wsSessionSelectStrategy.getSession(endpointDeque);
124-
} catch (NoSuchElementException e) {
125-
throw new SteveException("No session context for chargeBoxId '%s'", chargeBoxId, e);
126-
}
127-
}
128-
129143
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package de.rwth.idsg.steve.ocpp.ws;
2+
3+
import lombok.extern.slf4j.Slf4j;
4+
import org.springframework.web.socket.CloseStatus;
5+
import org.springframework.web.socket.WebSocketSession;
6+
7+
/**
8+
* @author Sevket Goekay <goekay@dbis.rwth-aachen.de>
9+
* @since 10.05.2018
10+
*/
11+
@Slf4j
12+
public class WebSocketLogger {
13+
14+
public static void connected(String chargeBoxId, WebSocketSession session) {
15+
log.info("[chargeBoxId={}, sessionId={}] Connection is established", chargeBoxId, session.getId());
16+
}
17+
18+
public static void closed(String chargeBoxId, WebSocketSession session, CloseStatus closeStatus) {
19+
log.warn("[chargeBoxId={}, sessionId={}] Connection is closed, status: {}", chargeBoxId, session.getId(), closeStatus);
20+
}
21+
22+
public static void receivedPong(String chargeBoxId, WebSocketSession session) {
23+
log.debug("[chargeBoxId={}, sessionId={}] Received pong message", chargeBoxId, session.getId());
24+
}
25+
26+
public static void receivedText(String chargeBoxId, WebSocketSession session, String msg) {
27+
log.info("[chargeBoxId={}, sessionId={}] Received: {}", chargeBoxId, session.getId(), msg);
28+
}
29+
30+
public static void sending(String chargeBoxId, WebSocketSession session, String msg) {
31+
log.info("[chargeBoxId={}, sessionId={}] Sending: {}", chargeBoxId, session.getId(), msg);
32+
}
33+
}

0 commit comments

Comments
 (0)