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

Optional params #11

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

valterss
Copy link

@valterss valterss commented Apr 8, 2019

Add NonStandard option AllowNull to allow serialization of method calls with NULL values in parameters.
It is well supported XML-RPC extension: https://web.archive.org/web/20130120074804/http://ontosys.com/xml-rpc/extensions.php

@LordVeovis
Copy link
Owner

Seems it's the same functionality as #13, but more polished here. I like the AllowNull attribute to explicitely enable the nil non-standard feature.

@valterss
Copy link
Author

valterss commented May 13, 2019

Seems it's the same functionality as #13, but more polished here. I like the AllowNull attribute to explicitely enable the nil non-standard feature.

Python implementation has this approach as well, and I like that this non-standard feature is not enabled by default too.
This is different from #13 as #13 is about deserializing response data which have NULL values. This (#11) allows to send NULL values as parameters of method in the XML-RPC request.
However, I thing #13 could be more polished if deserialization would take XmlRpcMissingMapping(MappingAction.Ignore) attribute into account for data deserialization. For example:

public class MyResult
{
    public string propertyOne;
    [XmlRpcMissingMapping(MappingAction.Ignore)] // don't care if server don't return this field
    public string propertyTwo;
}

Solution could allow NULL (<nil/>) for propertyTwo but does not allow NULL for propertyOne and rise exception as in standard behavior. Of course, if I understand the project correctly.

@LordVeovis LordVeovis self-assigned this May 18, 2019
@LordVeovis LordVeovis added the enhancement New feature or request label May 18, 2019
@alecthegeek
Copy link

Should this be closed?

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

Successfully merging this pull request may close these issues.

3 participants