Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
kukel opened this issue Sep 7, 2021 · 6 comments
Open

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

kukel opened this issue Sep 7, 2021 · 6 comments
Assignees

Comments

@kukel
Copy link
Contributor

kukel commented Sep 7, 2021

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 phax self-assigned this Sep 8, 2021
@phax phax added enhancement Profile Peppol Peppol AS4 related labels Sep 8, 2021
@phax
Copy link
Owner

phax commented Sep 8, 2021

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
Copy link
Contributor Author

kukel commented Sep 8, 2021

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

@phax
Copy link
Owner

phax commented Sep 8, 2021

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

@kukel
Copy link
Contributor Author

kukel commented Sep 8, 2021

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

@kukel
Copy link
Contributor Author

kukel commented Sep 8, 2021

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
Copy link

stale bot commented Apr 16, 2022

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.

@stale stale bot added the wontfix label Apr 16, 2022
@phax phax added pinned and removed wontfix labels Apr 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants