Feature/ocpp security extensions #196
Conversation
…ed security for OCPP 1.6-J'
There was a problem hiding this comment.
This was an impressive change set. I have tried to compare the code to the white paper specification from OCA's homepage. I may have missed something, but I did my best.
Some general notes: I'm very thankful that you decided to add unit tests. I would have loved to have some behavior tests with some fake client/servers actually sending request/confirmations. I don't know if the deserializer can actually instantiate the requests/confirmations without an empty default constructor? Have you tested this?
Also, you seem to make boundary tests, but only test the negative scenarios, I think I found a bug that would have been caught if you also made positive scenarios (testing the very limits).
Overall a very impressive change set, thank you for that. I only have some specification breaking issues I need you to fix before I can approve. Please see the comments for more information and please don't hesitate to ask if I'm writing jiberish again :)
ocpp-v1_6/src/main/java/eu/chargetime/ocpp/model/securityext/InstallCertificateRequest.java
Show resolved
Hide resolved
...1_6/src/main/java/eu/chargetime/ocpp/model/securityext/SecurityEventNotificationRequest.java
Show resolved
Hide resolved
ocpp-v1_6/src/main/java/eu/chargetime/ocpp/model/securityext/types/FirmwareType.java
Outdated
Show resolved
Hide resolved
ocpp-v1_6/src/main/java/eu/chargetime/ocpp/model/securityext/types/LogParametersType.java
Outdated
Show resolved
Hide resolved
ocpp-v1_6/src/main/java/eu/chargetime/ocpp/model/validation/IdentifierStringValidationRule.java
Show resolved
Hide resolved
...t/java/eu/chargetime/ocpp/feature/profile/test/securityext/ClientSecurityExtProfileTest.java
Show resolved
Hide resolved
...est/java/eu/chargetime/ocpp/model/securityext/test/SecurityEventNotificationRequestTest.java
Show resolved
Hide resolved
ocpp-v1_6/src/test/java/eu/chargetime/ocpp/model/securityext/test/types/FirmwareTypeTest.java
Show resolved
Hide resolved
ocpp-v1_6/src/test/java/eu/chargetime/ocpp/model/securityext/test/types/FirmwareTypeTest.java
Outdated
Show resolved
Hide resolved
...1_6/src/test/java/eu/chargetime/ocpp/model/securityext/test/types/LogParametersTypeTest.java
Show resolved
Hide resolved
|
Hi @m-oben, Everything looked fine, I'm merging it with master. Thank you for this awesome contribution. I really appreciate it. Sincerely, |
Resolves #90