Skip to content

Commit 1c3f8ae

Browse files
committed
Added fail fast validation on the setters for idTag. Covered it with unit test.
1 parent 5d92c40 commit 1c3f8ae

File tree

8 files changed

+106
-28
lines changed

8 files changed

+106
-28
lines changed

ocpp-v1_6-test/src/test/groovy/eu/chargetime/ocpp/test/core/soap/SOAPStartTransactionSpec.groovy

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,14 @@ class SOAPStartTransactionSpec extends Specification {
1919

2020
def setup() {
2121
chargePoint.connect()
22-
chargePoint.sendBootNotification("VendorX", "SingleSocketCharger")
2322
}
2423

2524
def cleanup() {
2625
chargePoint.disconnect()
2726
}
2827

2928
def "Charge point sends StartTransaction request and receives a response"() {
30-
def conditions = new PollingConditions(timeout: 2)
29+
def conditions = new PollingConditions(timeout: 10)
3130
when:
3231
chargePoint.sendStartTransactionRequest()
3332

ocpp-v1_6/src/main/java/eu/chargetime/ocpp/model/core/RemoteStartTransactionRequest.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,7 @@ public class RemoteStartTransactionRequest implements Request {
4646
@Override
4747
public boolean validate() {
4848
boolean valid = true;
49-
if (valid &= idTag != null)
50-
valid &= ModelUtil.validate(idTag, 20);
49+
valid &= ModelUtil.validate(idTag, 20);
5150

5251
if (chargingProfile != null) {
5352
valid &= chargingProfile.validate();
@@ -93,10 +92,14 @@ public String getIdTag() {
9392
/**
9493
* Required. The identifier that Charge Point must use to start a transaction.
9594
*
96-
* @param idTag an IdToken
95+
* @param idTag a String with max length 20
96+
* @throws PropertyConstraintException field isn't filled out correct.
9797
*/
9898
@XmlElement
99-
public void setIdTag(String idTag) {
99+
public void setIdTag(String idTag) throws PropertyConstraintException {
100+
if (!ModelUtil.validate(idTag, 20))
101+
throw new PropertyConstraintException("idTag", idTag, "Exceeded limit");
102+
100103
this.idTag = idTag;
101104
}
102105

ocpp-v1_6/src/main/java/eu/chargetime/ocpp/model/core/StartTransactionRequest.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,7 @@ public class StartTransactionRequest implements Request {
4949
public boolean validate() {
5050
boolean valid = true;
5151
valid &= connectorId != null && connectorId > 0;
52-
if (valid &= idTag != null)
53-
valid &= ModelUtil.validate(idTag, 20);
52+
valid &= ModelUtil.validate(idTag, 20);
5453
valid &= meterStart != null;
5554
valid &= timestamp != null;
5655
return valid;
@@ -91,10 +90,14 @@ public String getIdTag() {
9190
/**
9291
* Required. This contains the identifier for which a transaction has to be started.
9392
*
94-
* @param idTag the IdToken.
93+
* @param idTag a String with max length 20
94+
* @throws PropertyConstraintException field isn't filled out correct.
9595
*/
9696
@XmlElement
97-
public void setIdTag(String idTag) {
97+
public void setIdTag(String idTag) throws PropertyConstraintException {
98+
if (!ModelUtil.validate(idTag, 20))
99+
throw new PropertyConstraintException("idTag", idTag, "Exceeded limit");
100+
98101
this.idTag = idTag;
99102
}
100103

ocpp-v1_6/src/main/java/eu/chargetime/ocpp/model/core/StopTransactionRequest.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package eu.chargetime.ocpp.model.core;
22

3+
import eu.chargetime.ocpp.PropertyConstraintException;
34
import eu.chargetime.ocpp.model.Request;
5+
import eu.chargetime.ocpp.utilities.ModelUtil;
46

57
import javax.xml.bind.annotation.XmlElement;
68
import javax.xml.bind.annotation.XmlRootElement;
@@ -72,10 +74,14 @@ public String getIdTag() {
7274
* It is optional because a Charge Point may terminate charging without the presence of an idTag, e.g. in case of a reset.
7375
* A Charge Point SHALL send the idTag if known.
7476
*
75-
* @param idTag the IdToken.
77+
* @param idTag a String with max length 20
78+
* @throws PropertyConstraintException field isn't filled out correct.
7679
*/
7780
@XmlElement
78-
public void setIdTag(String idTag) {
81+
public void setIdTag(String idTag) throws PropertyConstraintException {
82+
if (idTag != null && !ModelUtil.validate(idTag, 20))
83+
throw new PropertyConstraintException("idTag", idTag, "Exceeded limit");
84+
7985
this.idTag = idTag;
8086
}
8187

ocpp-v1_6/src/test/java/eu/chargetime/ocpp/model/test/RemoteStartTransactionRequestTest.java

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import eu.chargetime.ocpp.PropertyConstraintException;
44
import eu.chargetime.ocpp.model.core.ChargingProfile;
55
import eu.chargetime.ocpp.model.core.RemoteStartTransactionRequest;
6+
import eu.chargetime.ocpp.utilities.TestUtilities;
67
import org.junit.Assert;
78
import org.junit.Before;
89
import org.junit.Test;
@@ -37,7 +38,7 @@
3738
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
3839
* SOFTWARE.
3940
*/
40-
public class RemoteStartTransactionRequestTest {
41+
public class RemoteStartTransactionRequestTest extends TestUtilities {
4142
RemoteStartTransactionRequest request;
4243

4344
@Before
@@ -75,9 +76,9 @@ public void setConnectorId_positiveInteger_connectorIdIsSet() throws Exception {
7576
}
7677

7778
@Test
78-
public void setIdTag_someIdToken_idTagIsSet() throws Exception {
79+
public void setIdTag_string20_idTagIsSet() throws Exception {
7980
// Given
80-
String idTag = "xxx";
81+
String idTag = aString(20);
8182

8283
// When
8384
request.setIdTag(idTag);
@@ -86,6 +87,40 @@ public void setIdTag_someIdToken_idTagIsSet() throws Exception {
8687
assertThat(request.getIdTag(), equalTo(idTag));
8788
}
8889

90+
@Test
91+
public void setIdTag_nullValue_throwsPropertyConstraintException() {
92+
// Given
93+
String nullValue = null;
94+
95+
try {
96+
// When
97+
request.setIdTag(nullValue);
98+
99+
Assert.fail("Expected PropertyConstraintException");
100+
} catch (PropertyConstraintException ex) {
101+
// Then
102+
assertThat(ex.getFieldKey(), equalTo("idTag"));
103+
assertThat(ex.getFieldValue(), equalTo(nullValue));
104+
}
105+
}
106+
107+
@Test
108+
public void setIdTag_exceeds20Chars_throwsPropertyConstraintException() {
109+
// Given
110+
String longString = aString(21);
111+
112+
try {
113+
// When
114+
request.setIdTag(longString);
115+
116+
Assert.fail("Expected PropertyConstraintException");
117+
} catch (PropertyConstraintException ex) {
118+
// Then
119+
assertThat(ex.getFieldKey(), equalTo("idTag"));
120+
assertThat(ex.getFieldValue(), equalTo(longString));
121+
}
122+
}
123+
89124
@Test
90125
public void setChargingProfile_someChargingProfile_chargingProfileIsSet() throws Exception {
91126
// Given

ocpp-v1_6/src/test/java/eu/chargetime/ocpp/model/test/StartTransactionRequestTest.java

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import eu.chargetime.ocpp.PropertyConstraintException;
44
import eu.chargetime.ocpp.model.core.StartTransactionRequest;
5+
import eu.chargetime.ocpp.utilities.TestUtilities;
56
import org.junit.Assert;
67
import org.junit.Before;
78
import org.junit.Test;
@@ -11,7 +12,6 @@
1112
import static org.hamcrest.CoreMatchers.equalTo;
1213
import static org.hamcrest.CoreMatchers.is;
1314
import static org.junit.Assert.assertThat;
14-
import static org.mockito.Mockito.*;
1515

1616
/*
1717
* ChargeTime.eu - Java-OCA-OCPP
@@ -38,7 +38,7 @@
3838
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
3939
* SOFTWARE.
4040
*/
41-
public class StartTransactionRequestTest {
41+
public class StartTransactionRequestTest extends TestUtilities {
4242
StartTransactionRequest request;
4343

4444
@Before
@@ -64,27 +64,61 @@ public void setConnectorId_zeroInteger_throwsPropertyConstraintException() {
6464
}
6565

6666
@Test
67-
public void setConnectorId_positiveInteger_connectorIdIsSet() throws Exception {
67+
public void setIdTag_nullValue_throwsPropertyConstraintException() {
6868
// Given
69-
Integer positive = 42;
69+
String nullValue = null;
70+
71+
try {
72+
// When
73+
request.setIdTag(nullValue);
74+
75+
Assert.fail("Expected PropertyConstraintException");
76+
} catch (PropertyConstraintException ex) {
77+
// Then
78+
assertThat(ex.getFieldKey(), equalTo("idTag"));
79+
assertThat(ex.getFieldValue(), equalTo(nullValue));
80+
}
81+
}
82+
83+
@Test
84+
public void setIdTag_exceeds20Chars_throwsPropertyConstraintException() {
85+
// Given
86+
String longString = aString(21);
87+
88+
try {
89+
// When
90+
request.setIdTag(longString);
91+
92+
Assert.fail("Expected PropertyConstraintException");
93+
} catch (PropertyConstraintException ex) {
94+
// Then
95+
assertThat(ex.getFieldKey(), equalTo("idTag"));
96+
assertThat(ex.getFieldValue(), equalTo(longString));
97+
}
98+
}
99+
100+
@Test
101+
public void setIdTag_string20_isSet() throws Exception {
102+
// Given
103+
String validString = aString(20);
70104

71105
// When
72-
request.setConnectorId(positive);
106+
request.setIdTag(validString);
73107

74108
// Then
75-
assertThat(request.getConnectorId(), equalTo(positive));
109+
assertThat(request.getIdTag(), equalTo(validString));
76110
}
77111

78112
@Test
79-
public void setIdTag_aIdToken_idTagIsSet() {
113+
public void setConnectorId_positiveInteger_connectorIdIsSet() throws Exception {
80114
// Given
81-
String idTag = "xxx";
115+
Integer positive = 42;
82116

83117
// When
84-
request.setIdTag(idTag);
118+
request.setConnectorId(positive);
85119

86120
// Then
87-
assertThat(request.getIdTag(), equalTo(idTag));
121+
assertThat(request.getConnectorId(), equalTo(positive));
88122
}
89123

90124
@Test

ocpp-v1_6/src/test/java/eu/chargetime/ocpp/model/test/StopTransactionRequestTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public void setUp() throws Exception {
4949
}
5050

5151
@Test
52-
public void setIdTag_anIdToken_idTagIsSet() {
52+
public void setIdTag_anIdToken_idTagIsSet() throws Exception {
5353
// Given
5454
String idTag = "xxx";
5555

ocpp-v1_6/src/test/java/eu/chargetime/ocpp/test/TimeoutSessionTest.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@
99
import org.junit.Test;
1010
import org.junit.runner.RunWith;
1111
import org.mockito.Mock;
12-
import org.mockito.Mockito;
13-
import org.mockito.MockitoAnnotations;
1412
import org.mockito.runners.MockitoJUnitRunner;
1513

1614
import static org.mockito.Matchers.any;

0 commit comments

Comments
 (0)