1
\$\begingroup\$

I'm consuming an ASMX service with SOAP messages. Since Xamarin doesn't support SOAP 1.2, I'm writing the Web requests manually. I'm sending a user/password to retrieve the user ID.

A successful response may look like this:

<soap:Envelope xmlns:soap="http://www.w3.org/2003/05/soap-envelope" xmlns:ns="link-to-namespace">
  <soap:Body>
    <ns:User>
      <ns:UserId>123</ns:UserId>
    </ns:User>
  </soap:Body>
</soap:Envelope>

An error response may look like this:

<soap:Envelope xmlns:soap="http://www.w3.org/2003/05/soap-envelope">
  <soap:Body>
    <soap:Fault>
      <soap:Code>
        <soap:Value>InvalidUsernameAndPassword</soap:Value>
      </soap:Code>
    </soap:Fault>
  </soap:Body>
</soap:Envelope>

What's the best way to parse the response? And how to check if there is an error?

Currently I just look for the presense of the <Fault> element.

WebService class handles the request and returns the SOAP envelope as an XDocument:

class WebService
{
    // Returns the SOAP envelope as an XDocument.
    public XDocument Invoke(string soapAction, string method, Dictionary<string, string> parameters)
    {

UserApi class acts as a proxy towards the service.

class UserApi
{
    // Sends user/pwd to server to retrieve user id.
    public string GetUserId(string username, string password)
    {
        var soapAction = "link-to-soap-action";
        var method = "GetUserId";
        var parameters = new Dictionary<string, string>
        {
            { "Username", username },
            { "Password", password}
        };
        var service = new WebService();
        var result = service.Invoke(soapAction, method, parameters);

        XNamespace nsSoap = "http://www.w3.org/2003/05/soap-envelope";
        XNamespace ns = "link-to-namespace";

        // Look for the "Fault" element in the response. If present, there was an error.
        var fault = result.Root.Element("{" + nsSoap + "}Body").Element("{" + ns_soap + "}Fault");
        if (error != null)
        {
            var codeWithNs = error.Element("{" + nsSoap + "}Code").Element("{" + ns_soap + "}Value").Value;
            var codeSplit = codeWithNs.Split(':');
            var code = codeSplit.Length == 2 ? codeSplit[1] : codeSplit[0];
            if (code == "InvalidUsernameOrPassword")
            {
                throw new InvalidUsernameOrPasswordException();
            }
            else
            {
                throw new Exception("Something went wrong.");
            }
        }

        return userId = result.Descendants(ns + "UserId").First().Value;
    }

    public async Task<string> GetUserIdAsync(string username, string password)
    {
        return await Task.Run(() =>
        {
            return GetUserId(username, password);
        });
    }
}

Currently it works, at least under optimal conditions. But my solution looks dense and fragile. If I should go with deserialization, here's some pseudo code of what I'm thinking:

try
{
    var serializer = new XmlSerializer(typeof(UserEnvelope));
    var user = serializer.Deserialize(response);
}
catch (InvalidOperationException)
{
    var serializer = new XmlSerializer(typeof(ErrorEnvelope));
    var error = serializer.Deserialize(response);
    if (error.Code == "InvalidUsernameOrPassword")
    ...etc
}
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. \$\endgroup\$ – Dannnno Mar 1 '18 at 19:52
2
\$\begingroup\$

Spot the inconsistency:

        var fault = result.Root.Element("{" + nsSoap + "}Body").Element("{" + ns_soap + "}Fault");
...
            var codeWithNs = error.Element("{" + nsSoap + "}Code").Element("{" + ns_soap + "}Value").Value;
...
        return userId = result.Descendants(ns + "UserId").First().Value;

Ok, there are two inconsistencies. Firstly, nsSoap vs ns_soap. I suspect that that difference is due to refactoring the code before posting in, in which case please don't do that again: we want to review code which compiles and works.

Secondly, "{" + nsSoap + "}Body" vs ns + "UserId". The simple concatenation is far more readable.


        var service = new WebService();
        var result = service.Invoke(soapAction, method, parameters);

        XNamespace nsSoap = "http://www.w3.org/2003/05/soap-envelope";
        XNamespace ns = "link-to-namespace";

        // Look for the "Fault" element in the response. If present, there was an error.
        var fault = result.Root.Element("{" + nsSoap + "}Body").Element("{" + ns_soap + "}Fault");
        if (error != null)
        {
            var codeWithNs = error.Element("{" + nsSoap + "}Code").Element("{" + ns_soap + "}Value").Value;
            var codeSplit = codeWithNs.Split(':');
            var code = codeSplit.Length == 2 ? codeSplit[1] : codeSplit[0];
            if (code == "InvalidUsernameOrPassword")
            {
                throw new InvalidUsernameOrPasswordException();
            }
            else
            {
                throw new Exception("Something went wrong.");
            }
        }

There's a strong asymmetry here which I don't grok. WebService does the job of assembling some simple strings into the XML request document: why does it not also do the job of disassembling the XML response document? I understand that it can't fully do that, because the response might be a complex datastructure, but IMO WebService should be detecting <soap:Fault> and returning some kind of "option" type.

As it happens, there is an option type in .Net: Task<TResult>. So I think that WebService.Invoke should return a Task<XElement> which has Status Faulted and a placeholder exception, or Status RanToCompletion and the contents of <soap:Body>. Then GetUser would look like

public string GetUserId(string username, string password)
{
    var soapAction = "link-to-soap-action";
    var method = "GetUserId";
    var parameters = new Dictionary<string, string>
    {
        { "Username", username },
        { "Password", password}
    };

    var service = new WebService();
    var result = service.Invoke(soapAction, method, parameters);

    if (result.Status == TaskStatus.Faulted)
    {
        // Handle specific errors
        if (result.Exception.Message == "InvalidUsernameOrPassword")
        {
            throw new InvalidUsernameOrPasswordException();
        }
        else
        {
            throw new Exception("Something went wrong.", result.Exception);
        }
    }

    XNamespace ns = "link-to-namespace";
    return userId = result.Value.Descendants(ns + "UserId").First().Value;
}
\$\endgroup\$
  • \$\begingroup\$ Thank you. How can I make the Invoke method throw a Faulted response? I have this public Task<XDocument> Invoke(,,) { return new Task<XDocument>.Factory.StartNew(() => { // retrieve SOAP envelope and search for <Fault> element here \$\endgroup\$ – Jam Mar 2 '18 at 16:58
  • \$\begingroup\$ Throwing is one option, but it's not strictly necessary. See Task.FromException \$\endgroup\$ – Peter Taylor Mar 3 '18 at 7:26
  • \$\begingroup\$ Thanks. I made sure the WebService class now checks for faults. \$\endgroup\$ – Jam Mar 19 '18 at 11:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.