Added SOAP 1.2 Support using the DataContractSerializer #877
Conversation
- Updated the MetaWCFBodyWritter constructor (DataContractSerializer) to accept the SoapBindings as a parameter so we can establish which version of SOAP to use. Created supporting methods as per the MetaBodyWriter class. - Added the missing WS-Addressing 1.0 WSDL Binding Namespace in the Namespaces. - Moved parts of the MetaMessage outside the check for BasicAuthentication, as I believe they need to be always present. - Updated SoapEndpointMiddleware to use the dynamic binding names instead of the hard coded HttpBinding. - Updated all relevant tests to test using both serializers. Also moved the hard coded vales for the Binding/Port to constants.
|
Thank you for your contribution! |
I'm sorry but I cannot find the comment. I looked through all the files and I cannot see any changes/comments. As I am new to Github would you be kind enough to point me to the right direction? |
| @@ -233,11 +233,11 @@ private async Task ProcessMeta(HttpContext httpContext, bool showDocumentation) | |||
| { | |||
| var baseUrl = httpContext.Request.Scheme + "://" + httpContext.Request.Host + httpContext.Request.PathBase + httpContext.Request.Path; | |||
| var xmlNamespaceManager = GetXmlNamespaceManager(null); | |||
| var bindingName = "BasicHttpBinding_" + _service.GeneralContract.Name; | |||
| var bindingName = _options.EncoderOptions[0].BindingName + "_" + _service.GeneralContract.Name; | |||
There was a problem hiding this comment.
Since this variable will be set here, even if EncoderOptions.BindingName is null, I don't think the fallback, to use "BasicHttpBinding" as default name, works.
Could you verify that the meta document is the same as before this PR, by default?
There was a problem hiding this comment.
Would you be happy to read from the EncoderOptions but if they are empty we use the deault value?
Something like this:
var bindingName = !string.IsNullOrWhiteSpace(_options.EncoderOptions[0].BindingName) ? options.EncoderOptions[0].BindingName : "BasicHttpBinding" + _service.GeneralContract.Name;
There was a problem hiding this comment.
I will recheck the Meta body and get back to you.
There was a problem hiding this comment.
Yep, your suggested change for binding name seems correct.
Just make sure that if one doesn't set any options the binding name etc. stays the same. Then I'm happy.
There was a problem hiding this comment.
I just updated the PR. I updated how we handle a default Binding value if none is present. I updated the relavent tests. Checked the metadata for SOAP 1.1 before and after the PR and it seems correct. I removed couple of hard coded sections (PolicyReference, UsingAddressing) as I wrongly assumed that they were always needed.
Sorry, I forgot to actually finish my code review 🤦♂️ I just want you to make sure that the default values stay the same. Wouldn't want to introduce breaking changes. |
- Restored the default value for a Binding if none is present in the SoapEndpointMiddleware class. - Updated relevant tests. - Restored the check around BasicAuthentication in MetaMessage class as it was modified incorrectly and removed the "UsingAddressing" tag. - Removed the hard coded "PolicyReference" section in the MetaWCFBodyWriter class. - Manually checked MetaData before and after the changes when using SOAP 1.1. Also MetaData after adding SOAP 1.2 support seems to be correct.
|
Thanks! |
Thank you for approving and merging. Is there any chance to bring the next release out sooner rather than later? I obviously do not know what other work is being done in the background, but I do need an official release to be consumed in my current work project. Thank you |
Fixes #825