Pcc3 api fixes#47
Conversation
…own when the request type is a RetrieveResultObj
…d interface is is agreed for CD3
…se in which the information consumer is not allowed to pull on the given transactionId. It is a responsibility of the serviceRegistry to conduct such check.
… a bad request is reported to the client rather than internal server error
…vant when calling a secom controller incorrectly, we'd want to return 404 and not a 400 as was the case
|
Merge conflicts locally by rebasing on v2 before PR |
|
Thanks Jakob, I will have a look! |
nvasta
left a comment
nvasta
left a comment
There was a problem hiding this comment.
I can understand the changes but there are some points that need to be cleared out.
We also now have a third (unfortunately) core-springboot package with native Springboot support. That is for Springboot 4 and we should if possible also upgrade that one to match these changes,
The ServiceInstanceStatusEnum for example hasn't been included in there.
| envelopeSignatureCertificate, | ||
| envelopeRootCertificateThumbprint, | ||
| envelopeSignatureTime | ||
| envelopeSignatureTime, |
There was a problem hiding this comment.
I don't think the comma is required
| } | ||
|
|
||
|
|
||
| try { |
There was a problem hiding this comment.
Is there a specific use case we are trying to catch here?
I could agree with the try catch, but I think if included it should extend in the whole operation, i.e. it should include the validation checks also taking place in the envelope.
|
|
||
| if (pathInfo != null) { | ||
| // Handle retrieveResult requests including /transactionId | ||
| if (pathInfo.equals(RETRIEVE_RESULT_INTERFACE_PATH) |
There was a problem hiding this comment.
Isn't the retrieve results now a POST, why isn't it included in the switch statement?
| Response.Status responseStatus; | ||
| ResponseObject responseObject = new ResponseObject(); | ||
| jakarta.ws.rs.core.Response.Status responseStatus; | ||
| EncryptionKeyResponseObject encryptionKeyResponseObject = new EncryptionKeyResponseObject(); |
There was a problem hiding this comment.
Why are we returning an EncryptionKeyResponseObject ? A specific ResponseObject has now been introduced to match the SECOM v2.0 OpenAPI.
| if (ex instanceof SecomNotAuthorisedException) { | ||
| responseStatus = Response.Status.UNAUTHORIZED; | ||
| encryptionKeyResponseObject.setMessage("Not authorized to requested information"); | ||
| return Response.status(responseStatus) |
There was a problem hiding this comment.
No need for this, let's follow the same way all other interfaces are written and return one response in the end.
| @@ -62,7 +62,6 @@ public Object[] getAttributeArray() { | |||
| envelopeSignatureCertificate, | |||
| envelopeRootCertificateThumbprint, | |||
| envelopeSignatureTime, | |||
| @@ -62,7 +62,6 @@ public Object[] getAttributeArray() { | |||
| envelopeSignatureCertificate, | |||
| envelopeRootCertificateThumbprint, | |||
| envelopeSignatureTime, | |||
| envelopeSignatureCertificate, | ||
| envelopeRootCertificateThumbprint, | ||
| envelopeSignatureTime | ||
| envelopeSignatureTime, |
| envelopeSignatureCertificate, | ||
| envelopeRootCertificateThumbprint, | ||
| envelopeSignatureTime | ||
| envelopeSignatureTime, |
| @Override | ||
| public Object[] getAttributeArray() { | ||
| return new Object[] { | ||
| return new Object[]{ |
There was a problem hiding this comment.
Why are we removing the blank space?
2 participants
Implements changes of May 2026 in IEC 63173-2 PCC3.