-
Notifications
You must be signed in to change notification settings - Fork 616
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
Xml/attachments options bugfix, MTOM support for attachments, better Nori options passthrough #944
Conversation
Fix request logging when message containt non-ascii characters (log writing failure occured) Fix using attachments together with 'xml' option (when xml option was provided, attachments option was ignored)
…ons with Savon options
@pcai what about merging this? we use it in production for two years already |
Thanks for your work. Could you describe how to use it? For example get the attachments of a given request? |
@Skulli is far as I remember, there were no changes in public API regarding request/response attachments, except fixing a bug, when both "attachments" and "xml" options were used together in locals. The main feature was support for MTOM attachments encoding: https://www.ibm.com/docs/en/integration-bus/10.0?topic=services-what-is-soap-mtom Request example: client = Savon.client({
multipart: true,
...
})
client.call(:some_operation, {
mtom: true, # new option, default=false
attachments: [...],
}) |
@netcitylife please address the merge conflicts and lack of tests and I can merge. |
# Conflicts: # lib/savon/options.rb
This is working, what can we do to solve these issues which blocks the merge process? |
@@ -101,9 +101,10 @@ def build_request(builder) | |||
request.body = builder.to_s | |||
|
|||
if builder.multipart | |||
request.gzip | |||
type = @locals[:mtom] ? 'application/xop+xml"; start-info="text/xml' : SOAP_REQUEST_TYPE[@globals[:soap_version]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: this MTOM mime type could also be extracted to a constant, so that both branches are harmonious, looking uniform.
Missing in order to merge: tests for the new behaviour. |
rebased, tested, and merged as #1012 |
What kind of change is this?
Bugfixes + new feature
Did you add tests for your changes?
Currently no
Summary of changes
Add MTOM support for SOAP attachments (previously no support)
Fix request logging when message containt non-ascii characters (log writing failure occured)
Fix using attachments together with 'xml' option (when xml option was provided, attachments option was ignored)