Skip to content

Commit 1a58f09

Browse files
TVoldenTVolden
committed
Added field validation and error handling.
1 parent bba66f4 commit 1a58f09

File tree

7 files changed

+100
-39
lines changed

7 files changed

+100
-39
lines changed

‎ocpp-common/src/main/java/eu/chargetime/ocpp/Communicator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ of this software and associated documentation files (the "Software"), to deal
3131
public abstract class Communicator {
3232
protected Transmitter transmitter;
3333

34-
public abstract <T> T unpackPayload(String payload, Class<T> type);
34+
public abstract <T> T unpackPayload(String payload, Class<T> type) throws Exception;
3535
public abstract String packPayload(Object payload);
3636
protected abstract String makeCallResult(String uniqueId, String payload);
3737
protected abstract String makeCall(String uniqueId, String action, String payload);

‎ocpp-common/src/main/java/eu/chargetime/ocpp/Session.java‎

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,23 +71,52 @@ public void close() {
7171
private class CommunicatorEventHandler implements CommunicatorEvents {
7272
@Override
7373
public void onCallResult(String id, String payload) {
74-
events.handleConfirmation(id, communicator.unpackPayload(payload, getConfirmationType(id)));
74+
try {
75+
Confirmation confirmation = communicator.unpackPayload(payload, getConfirmationType(id));
76+
if (confirmation.validate()) {
77+
events.handleConfirmation(id, confirmation);
78+
} else {
79+
communicator.sendCallError(id, "OccurenceConstraintViolation", "Payload for Action is syntactically correct but at least one of the fields violates occurence constraints");
80+
}
81+
}
82+
catch (PropertyConstraintException ex) {
83+
String message = "Field %s violates constraints with value: \"%s\". %s";
84+
communicator.sendCallError(id, "TypeConstraintViolation", String.format(message, ex.getFieldKey(), ex.getFieldValue(), ex.getMessage()));
85+
ex.printStackTrace();
86+
} catch (Exception ex) {
87+
communicator.sendCallError(id, "FormationViolation", "Unable to process action");
88+
ex.printStackTrace();
89+
}
7590
}
7691

7792
@Override
7893
public void onCall(String id, String action, String payload) {
7994
Feature feature = events.findFeatureByAction(action);
8095
if (feature != null) {
81-
CompletableFuture<Confirmation> promise = handleIncomingRequest(communicator.unpackPayload(payload, feature.getRequestType()));
82-
promise.whenComplete(((confirmation, throwable) -> {
83-
if (promise.isCompletedExceptionally()) {
84-
communicator.sendCallError(id, "InternalError", "An internal error occurred and the receiver was not able to process the requested Action successfully");
85-
} else if (confirmation == null) {
86-
communicator.sendCallError(id, "NotSupported", "Requested Action is recognized but not supported by the receiver");
96+
try {
97+
Request request = communicator.unpackPayload(payload, feature.getRequestType());
98+
if (request.validate()) {
99+
CompletableFuture<Confirmation> promise = handleIncomingRequest(request);
100+
promise.whenComplete(((confirmation, throwable) -> {
101+
if (promise.isCompletedExceptionally()) {
102+
communicator.sendCallError(id, "InternalError", "An internal error occurred and the receiver was not able to process the requested Action successfully");
103+
} else if (confirmation == null) {
104+
communicator.sendCallError(id, "NotSupported", "Requested Action is recognized but not supported by the receiver");
105+
} else {
106+
communicator.sendCallResult(id, confirmation);
107+
}
108+
}));
87109
} else {
88-
communicator.sendCallResult(id, confirmation);
110+
communicator.sendCallError(id, "OccurenceConstraintViolation", "Payload for Action is syntactically correct but at least one of the fields violates occurence constraints");
89111
}
90-
}));
112+
} catch (PropertyConstraintException ex) {
113+
String message = "Field %s violates constraints with value: \"%s\". %s";
114+
communicator.sendCallError(id, "TypeConstraintViolation", String.format(message, ex.getFieldKey(), ex.getFieldValue(), ex.getMessage()));
115+
ex.printStackTrace();
116+
} catch (Exception ex) {
117+
communicator.sendCallError(id, "FormationViolation", "Unable to process action");
118+
ex.printStackTrace();
119+
}
91120
} else {
92121
communicator.sendCallError(id, "NotImplemented", "Requested Action is not known by receiver");
93122
}

‎ocpp-common/src/test/java/eu/chargetime/ocpp/test/SessionTest.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@
44
import eu.chargetime.ocpp.feature.Feature;
55
import eu.chargetime.ocpp.model.Confirmation;
66
import eu.chargetime.ocpp.model.Request;
7+
import eu.chargetime.ocpp.model.test.TestRequest;
78
import org.junit.Before;
89
import org.junit.Test;
910
import org.mockito.Mock;
1011
import org.testng.TestException;
1112

12-
import static org.hamcrest.Matchers.*;
13+
import static org.hamcrest.Matchers.equalTo;
1314
import static org.junit.Assert.assertThat;
1415
import static org.mockito.Mockito.*;
1516

@@ -150,16 +151,17 @@ public void onCall_unhandledCallback_callSendCallError() {
150151
}
151152

152153
@Test
153-
public void onCall_handledCallback_callSendCallResult() {
154+
public void onCall_handledCallback_callSendCallResult() throws Exception {
154155
// Given
155156
String someId = "Some id";
156157
Confirmation aConfirmation = new Confirmation() {
157158
@Override
158159
public boolean validate() {
159-
return false;
160+
return true;
160161
}
161162
};
162163
when(sessionEvents.handleRequest(any())).thenReturn(aConfirmation);
164+
when(communicator.unpackPayload(any(), any())).thenReturn(new TestRequest());
163165

164166
// When
165167
eventHandler.onCall(someId, null, null);
@@ -170,10 +172,11 @@ public boolean validate() {
170172
}
171173

172174
@Test
173-
public void onCall_callbackThrowsException_callSendCallResult() {
175+
public void onCall_callbackThrowsException_callSendCallResult() throws Exception {
174176
// Given
175177
String someId = "Some id";
176178
when(sessionEvents.handleRequest(any())).thenThrow(TestException.class);
179+
when(communicator.unpackPayload(any(), any())).thenReturn(new TestRequest());
177180

178181
// When
179182
eventHandler.onCall(someId, null, null);

‎ocpp-test/src/main/java/eu/chargetime/ocpp/model/test/TestConfirmation.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,6 @@ of this software and associated documentation files (the "Software"), to deal
3131
public class TestConfirmation implements Confirmation {
3232
@Override
3333
public boolean validate() {
34-
return false;
34+
return true;
3535
}
3636
}

‎ocpp-test/src/main/java/eu/chargetime/ocpp/model/test/TestRequest.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package eu.chargetime.ocpp.model.test;
2+
3+
import eu.chargetime.ocpp.model.Request;
4+
5+
/**
6+
* ChargeTime.eu - Java-OCA-OCPP
7+
*
8+
* MIT License
9+
*
10+
* Copyright (C) 2016 Thomas Volden <[email protected]>
11+
*
12+
* Permission is hereby granted, free of charge, to any person obtaining a copy
13+
* of this software and associated documentation files (the "Software"), to deal
14+
* in the Software without restriction, including without limitation the rights
15+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
16+
* copies of the Software, and to permit persons to whom the Software is
17+
* furnished to do so, subject to the following conditions:
18+
*
19+
* The above copyright notice and this permission notice shall be included in all
20+
* copies or substantial portions of the Software.
21+
*
22+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
23+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
24+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
25+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
26+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
27+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
28+
* SOFTWARE.
29+
*/
30+
public class TestRequest implements Request {
31+
@Override
32+
public boolean validate() {
33+
return true;
34+
}
35+
}

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

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,9 @@ public JSONCommunicator(Transmitter transmitter) {
5656
}
5757

5858
@Override
59-
public <T> T unpackPayload(String payload, Class<T> type) {
60-
T object = null;
61-
try {
62-
JSONObject json = new JSONObject(payload);
63-
object = parseJSON(json, type);
64-
} catch (Exception ex) {
65-
ex.printStackTrace();
66-
}
67-
return object;
59+
public <T> T unpackPayload(String payload, Class<T> type) throws Exception {
60+
JSONObject json = new JSONObject(payload);
61+
return parseJSON(json, type);
6862
}
6963

7064
@Override
@@ -129,7 +123,7 @@ private Object parseValue(String key, JSONObject json, Method method) throws Exc
129123
}
130124

131125
private Object parseValue(String key, JSONObject json, Class<?> type, Type genericType) throws Exception {
132-
Object output = null;
126+
Object output;
133127

134128
if (type.isArray()) {
135129
output = parseArray(json.getJSONArray(key), type.getComponentType());
@@ -167,7 +161,7 @@ private <T> T[] parseArray(JSONArray array, Class<?> type) throws Exception {
167161
}
168162

169163
private Object parseArrayItem(JSONArray array, int index, Class<?> type) throws Exception {
170-
Object output = null;
164+
Object output;
171165

172166
if (type == String.class) {
173167
output = array.getString(index);

‎ocpp-v1_6/src/test/java/eu/chargetime/ocpp/test/JSONCommunicatorTest.java‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public void setup() {
5858
}
5959

6060
@Test
61-
public void unpack_emptyPayload_returnRequestedType() {
61+
public void unpack_emptyPayload_returnRequestedType() throws Exception {
6262
// Given
6363
String payload = "{}";
6464
Class<?> type = BootNotificationRequest.class;
@@ -71,7 +71,7 @@ public void unpack_emptyPayload_returnRequestedType() {
7171
}
7272

7373
@Test
74-
public void unpack_aStringPayload_returnsTestModelWithAString() {
74+
public void unpack_aStringPayload_returnsTestModelWithAString() throws Exception {
7575
// Given
7676
String aString = "Some string";
7777
String payload = "{\"stringTest\":\"%s\"}";
@@ -84,7 +84,7 @@ public void unpack_aStringPayload_returnsTestModelWithAString() {
8484
}
8585

8686
@Test
87-
public void unpack_aCalendarPayload_returnsTestModelWithACalendar() {
87+
public void unpack_aCalendarPayload_returnsTestModelWithACalendar() throws Exception {
8888
// Given
8989
String aCalendar = "2016-04-28T07:16:11.988Z";
9090
String payload = "{\"calendarTest\":\"%s\"}";
@@ -100,7 +100,7 @@ public void unpack_aCalendarPayload_returnsTestModelWithACalendar() {
100100
}
101101

102102
@Test
103-
public void unpack_anIntegerPayload_returnsTestModelWithAnInteger() {
103+
public void unpack_anIntegerPayload_returnsTestModelWithAnInteger() throws Exception {
104104
// Given
105105
Integer anInteger = 1337;
106106
String payload = "{\"integerTest\":%d}";
@@ -113,7 +113,7 @@ public void unpack_anIntegerPayload_returnsTestModelWithAnInteger() {
113113
}
114114

115115
@Test
116-
public void unpack_aGenericIntPayload_returnsTestModelWithAGenericInt() {
116+
public void unpack_aGenericIntPayload_returnsTestModelWithAGenericInt() throws Exception {
117117
// Given
118118
int anInteger = 1337;
119119
String payload = "{\"intTest\":%d}";
@@ -126,7 +126,7 @@ public void unpack_aGenericIntPayload_returnsTestModelWithAGenericInt() {
126126
}
127127

128128
@Test
129-
public void unpack_aLongPayload_returnsTestModelWithALong() {
129+
public void unpack_aLongPayload_returnsTestModelWithALong() throws Exception {
130130
// Given
131131
Long aLong = 1337L;
132132
String payload = "{\"longTest\":%d}";
@@ -139,7 +139,7 @@ public void unpack_aLongPayload_returnsTestModelWithALong() {
139139
}
140140

141141
@Test
142-
public void unpack_aGenericLongPayload_returnsTestModelWithAGenericLong() {
142+
public void unpack_aGenericLongPayload_returnsTestModelWithAGenericLong() throws Exception {
143143
// Given
144144
long aLong = 1337;
145145
String payload = "{\"genericLongTest\":%d}";
@@ -152,7 +152,7 @@ public void unpack_aGenericLongPayload_returnsTestModelWithAGenericLong() {
152152
}
153153

154154
@Test
155-
public void unpack_aDoublePayload_returnsTestModelWithADouble() {
155+
public void unpack_aDoublePayload_returnsTestModelWithADouble() throws Exception {
156156
// Given
157157
Double aDouble = 13.37D;
158158
String payload = "{\"doubleTest\":%f}";
@@ -165,7 +165,7 @@ public void unpack_aDoublePayload_returnsTestModelWithADouble() {
165165
}
166166

167167
@Test
168-
public void unpack_aGenericDoublePayload_returnsTestModelWithAGenericDouble() {
168+
public void unpack_aGenericDoublePayload_returnsTestModelWithAGenericDouble() throws Exception {
169169
// Given
170170
double aDouble = 13.37;
171171
String payload = "{\"genericDoubleTest\":%f}";
@@ -178,7 +178,7 @@ public void unpack_aGenericDoublePayload_returnsTestModelWithAGenericDouble() {
178178
}
179179

180180
@Test
181-
public void unpack_aBooleanPayload_returnsTestModelWithABoolean() {
181+
public void unpack_aBooleanPayload_returnsTestModelWithABoolean() throws Exception {
182182
// Given
183183
Boolean aBoolean = false;
184184
String payload = "{\"booleanTest\":%b}";
@@ -191,7 +191,7 @@ public void unpack_aBooleanPayload_returnsTestModelWithABoolean() {
191191
}
192192

193193
@Test
194-
public void unpack_aGenericBooleanPayload_returnsTestModelWithAGenericBoolean() {
194+
public void unpack_aGenericBooleanPayload_returnsTestModelWithAGenericBoolean() throws Exception {
195195
// Given
196196
boolean aBoolean = false;
197197
String payload = "{\"genericBooleanTest\":%b}";
@@ -204,7 +204,7 @@ public void unpack_aGenericBooleanPayload_returnsTestModelWithAGenericBoolean()
204204
}
205205

206206
@Test
207-
public void unpack_anObjectPayload_returnsTestModelWithAnObject() {
207+
public void unpack_anObjectPayload_returnsTestModelWithAnObject() throws Exception {
208208
// Given
209209
String payload = "{\"objectTest\":{}}";
210210

@@ -217,7 +217,7 @@ public void unpack_anObjectPayload_returnsTestModelWithAnObject() {
217217

218218

219219
@Test
220-
public void unpack_bootNotificationCallResultPayload_returnBootNotificationConfirmation() {
220+
public void unpack_bootNotificationCallResultPayload_returnBootNotificationConfirmation() throws Exception {
221221
// Given
222222
String currentType = "2016-04-28T07:16:11.988Z";
223223
int interval = 300;

0 commit comments

Comments
 (0)