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

Support nils values #24

Merged
merged 2 commits into from Oct 27, 2014
Merged

Support nils values #24

merged 2 commits into from Oct 27, 2014

Conversation

anton-ryzhov
Copy link
Contributor

Parse response <item /> as empty string (None now), and <item xsi:nil="true" /> as None.

Send <item xsi:nil="true" /> nodes for None values in the request (it drops it out now).

As usual, tests haven't become worse.

Return empty strings for empty strings rather than `None`
reingart added a commit that referenced this pull request Oct 27, 2014
@reingart reingart merged commit 16f632a into pysimplesoap:master Oct 27, 2014
@vthach
Copy link

vthach commented Oct 28, 2014

The nil did not work for me. I believe the namespace is also needed. Something like this:
< sch:targetName xsi:nil="true" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"/ >

The debug seem to point to this as well:
DEBUG:pysimplesoap.client:<soapenv:Envelope xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/">soapenv:Bodysoapenv:Faultsoapenv:Servercom.ctc.wstx.exc.WstxParsingException: Undeclared namespace prefix "xsi" (for attribute "nil")
at [row,col {unknown-source}]: [5,284]/soapenv:Fault/soapenv:Body/soapenv:Envelope

@anton-ryzhov
Copy link
Contributor Author

Looks like you're right, there may be problem you've mentioned.
But I can't really help you with this because I'm not xml expert and U can't reproduce this bug in my environment.
I'll be great if you fix it by yourself and pullrequest it here. @reingart merges improvements often.

@dotnetdan
Copy link
Contributor

This merge is causing a problematic side effect. When sending a request, all parameter fields that aren't initialized are now being marshalled as xsi:nil="true" - even types that aren't nillable. All other SOAP clients that I've worked with, including WCF, Metro, and SUDS, will not marshal uninitialized parameter fields into the soap request. This keeps the message more compact, and more importantly, the server should be able to assign excluded fields to the default value of their type, instead of being told that it is nil or null.

Also, if I understand correctly, I believe that pull request #22 already fixed this issue without causing the extra problem. Can anybody test and verify this is correct?

@reingart
Copy link
Member

reingart commented Nov 6, 2014

I'm in doubt with this one too, specially in sort_dict that is appending null tags now when before it was specifically banned (BTW, the comment is still there...).

I think that until xsi:nil is correctly parsed from the WSDL, this should be modified to be avoided by default in some way...

Sorry I don't have time to review in more depth, but it would be wise if those that're interested could propose unit tests so we can take further action with more concrete details.

@dotnetdan
Copy link
Contributor

I agree with Mariano. The problem with sort_dict is that it doesn't know if the WSDL has defined that type as nullable. Until this can be properly fixed, does anyone agree that this pull request should be reversed?

@reingart
Copy link
Member

reingart commented Nov 7, 2014

Please @dotnetdan , go ahead
I've added you and @anton-ryzhov to the org so you can fix and push directly (I don't want to be a bottleneck due lack of time)
Thanks both for your contribs!

@anton-ryzhov
Copy link
Contributor Author

About reverting this PR. If some of you faced with bugs with sending requests to server, why you've reverted receiving part of this, a02643d ?

Without it <ax29:fooBar></ax29:fooBar> parsed as None, not empty string. I believe we should redo a02643d and supply better solution instead of d707bbd.

@dotnetdan
Copy link
Contributor

Anton,

Until now, I didn't realize that your original change for pull #24 was about the server - everything I have said up until now was about the client. I am only using the PySimpleSoap client to connect to an existing web service (built in C# and WCF). I do not use the PySimpleSoap server at all.

As soon as I get a chance, I am going to test if I can put back the changes you made to the Unmarshalling logic. I'll follow up on that soon. Let me know if that works for you.

@anton-ryzhov
Copy link
Contributor Author

No-no, there is nothing about PySimpleSoap-server, I don't use it like that ether.

In #24 there is 2 commits, both for PySimpleSoap-client — receiving and sending None/nil values. You've met problems with sending, but also reverted receiving part.

I'm using PySimpleSoap-client with wso2 server and I'm not very involved in all SOAP-related things, I'm just trying to make it work for my case.

I'll test your solution for my case, until that I continue to use my old patch.

@Keoven
Copy link

Keoven commented Dec 12, 2018

Would be great if this is some option that can be turned on -- as the revert here basically caused errors on an existing service I'm connected to. Would this be an option so that I can use the latest version without needing to tag a specific version via commit hash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants