phax / phase4

phase4 - AS4 client and server for integration into existing systems. Specific support for Peppol and CEF eDelivery built-in.
Apache License 2.0
154 stars 47 forks source link

Make SBDHBuilder#finishFields is public so it can be used directly #61

Open kukel opened 3 years ago

kukel commented 3 years ago

SBDHBuilder#finishFields is protected but it is required when you want to process a already populated SBDH document. Since there is no real other reason to extend Phase4PeppolSender, making this public would be a nice thing to have.

phax commented 3 years ago

Well, I was looking at it and I am not conviced yet. As the finishFields is called internally from AbstractAS4MessageBuilder.sendMessage, I don't see the reason why I should make this public. Can you please elaborate on the specific use case?

You can still derive from class SBDHBuilder and extend the visibility of finishFields yourself. But I really miss the reason why someone from the outside would call this....

kukel commented 3 years ago

Because it failed sending if I did not explicitly call it. Will try to create a unit test to show.

phax commented 3 years ago

That is weird. That is done explicitly in a base class. Are you overriding some classes and not calling the supermethod?

kukel commented 3 years ago

Nope, working on a MainPhase4PeppolHelgerSBDH now ;-)... Few minutes

kukel commented 3 years ago

Took some time, our Test SMP underwent some unexpected maintenance. This issue is semi valid.

When I use .payload(bytes[]), which I did, it fails and the .finishFields() is/seems needed (and some more). Using .payloadAndMetadata(PeppolSBDHDocument) and marshalling the bytes/inputstream to PeppolSBDHDocument first.

But this could be optimized I think since it is then made into bytes again. Why not pass on the bytes and in .payload(bytes[]) marshall for the meta data AND set the bytes. Saves a relatively expensive operation.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.