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

Breaking changes in SoapSerializer.XmlSerializer for previously generated ASMX client code #903

Closed
Aleksanderis opened this issue Sep 19, 2022 · 10 comments

Comments

@Aleksanderis
Copy link
Contributor

Aleksanderis commented Sep 19, 2022

I have legacy ASMX webservices, which are being used by some third-party stakeholders, who we cannot control.
So our ultimate goal is to migrate .NET 4.7.2 solution to .NET 6 without (major) breaking changes for our clients.
SoapCore looked like a savior here, and it was really promising when we were prototyping it with some basic web services.
However testing it in our main solution - there are some corner cases, which we cannot to find a way how to workaround.

1. Guid[] type in requests object in a legacy solution was specified as "ArrayOfGuid" complex type. It was looking like this (here and below - WSDL definition is truncated for brevity, but I can provide a full one if it would help):

<s:complexType name="GetPricesRequest">
    <s:sequence>
        <s:element minOccurs="0" maxOccurs="1" ref="s1:Product"/>
        <s:element minOccurs="0" maxOccurs="1" name="ProductIds" type="tns:ArrayOfGuid"/>
    </s:sequence>
</s:complexType>
<s:complexType name="ArrayOfGuid">
    <s:sequence>
        <s:element minOccurs="0" maxOccurs="unbounded" name="guid" type="s2:guid"/>
    </s:sequence>
</s:complexType>

<s:schema elementFormDefault="qualified" targetNamespace="http://microsoft.com/wsdl/types/">
    <s:simpleType name="guid">
        <s:restriction base="s:string">
            <s:pattern value="[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}"/>
        </s:restriction>
    </s:simpleType>
</s:schema>

With SoapCore, using XmlSerializer - it's just "ArrayOfString":

<xsd:complexType name="GetPricesRequest">
    <xsd:sequence>
        <xsd:element minOccurs="0" maxOccurs="1" name="Product" type="tns:ProductDto"/>
        <xsd:element minOccurs="0" maxOccurs="1" name="ProductIds" type="tns:ArrayOfString"/>
    </xsd:sequence>
</xsd:complexType>
<xsd:complexType name="ArrayOfString">
    <xsd:sequence>
        <xsd:element minOccurs="0" maxOccurs="unbounded" name="string" type="xsd:string"/>
    </xsd:sequence>
</xsd:complexType>

As the result, using that client - it fails to map, and webservices receives an empty array.
Is it by design, is it a configuration issue, or some potential missing feature?
Any ideas where we could look at in case we would like to contribute with some pull-request?

2. Another issue with same contract is with XmlElement attributes usage.
We have the following request object with a complex type inside:

[XmlType(Namespace = "urn:Bbb.Services.Management")]
public class GetPricesRequest
{
    [XmlElement("Product", Namespace = "urn:Bbb.Services.Common")]
    public ProductDto Product { get; set; }

    public Guid[] ProductIds { get; set; }
}

In a legacy solution it was looking like this:

<s:element name="Product" type="s1:ProductDto"/>
<s:complexType name="ProductDto">
    <s:sequence>
        <s:element minOccurs="1" maxOccurs="1" name="Id" type="s2:guid"/>
        <s:element minOccurs="0" maxOccurs="1" name="Name" type="s:string"/>
        <s:element minOccurs="0" maxOccurs="1" name="Type" type="s:string"/>
    </s:sequence>
</s:complexType>

In new one there is something completely broken, it returns invalid WSDL, with duplicated elements, which cannot be consumed (note how name="Product" is duplicated):

<xsd:element minOccurs="0" maxOccurs="1" name="Product" type="tns:ProductDto"/>
<xsd:complexType name="ProductDto">
<xsd:sequence>
    <xsd:element minOccurs="0" maxOccurs="1" name="Product" type="xsd:string"/>
    <xsd:element minOccurs="0" maxOccurs="1" name="Product" type="xsd:string"/>
    <xsd:element minOccurs="1" maxOccurs="1" name="Product" type="xsd:string"/>
</xsd:sequence>
</xsd:complexType>

If I drop XmlElement attribute - it produces a valid WSDL, but it still cannot be consumed by legacy clients (due to the first issue, and due to breaking change).
I won't advocate, that it was correctly configured in our legacy solution, but it was working like that for years. (If it was a bug, it was used as a feature.:) )
Any ideas how we could workaround it, or at least minimize possible breaking changes?

Update: my intermediate achievement - old clients are working, but new ones are failing to map request parameters correctly (webservice receives empty parameters). 🤷‍♂️
It was achieved by leaving XmlElement without "ElementName", just with "Namesapce".

@andersjonsson
Copy link
Collaborator

Your first issue could probably be partly solved by adding your own type. Something like
[XmlType(TypeName = "ArrayOfGuid")]
public class GuidList : List
{
public GuidList (int capacity)
{
Capacity = capacity;
}

public GuidList ()
{
}

}

As far as I know there is no support for adding a pattern to a string, but this way you'll get the correct type name in the wsdl.
I had to do something similar in my own legacy service.
Pattern support would be nice, so if you feel like giving it a shot you are more than welcome.

For your second problem I think that it's a straight up bug. Most likely it uses the attribute from the property for the members in the complextype as well. That is obviously wrong. Same deal here, if you feel like fixing it a PR would be more than welcome!

This is, btw, the same way that I started contributing to the project. I ran across bugs and missing features when I was migrating a large legacy service where breaking changes wasn't an option

@Aleksanderis
Copy link
Contributor Author

Aleksanderis commented Sep 20, 2022

Thanks for suggestions. I cloned the repository, added a direct reference to it from my project and was trying to debug all day long today to understand how it works and how it could be improved.

So far creating custom type to cover Guid[] didn't work correctly. Tried like that:

[XmlType(Namespace = "urn:Bbb.Services.Management")]
public class GetPricesRequest
{
    [XmlElement("Product", Namespace = "urn:Bbb.Services.Common")]
    public ProductDto Product { get; set; }

    public GuidList ProductIds { get; set; }
}

[XmlType(TypeName = "ArrayOfGuid")]
public class GuidList : List<Guid>
{
}

Tried different combinations, and it was either old client wasn't working or a newly generated one was not working.
Closest what I found is this change - https://github.com/DigDes/SoapCore/pull/133/files
Changing it to resolvedType = "xs:guid"; seems work better, but there are probably some other cases to consider, which I might know. On the other hand, even for legacy ASMX implementation Guid types eventually are stored/serialized as strings - just with some additional patterns/restrictions as you noted. These patterns/restrictions are not really critical, any other server side validation can give something similar, but currently it just fails to map and deserialize it correctly.
I also noticed that in old ASMX implementation Microsoft was using some additional custom types (not just for Guid[]) from the "http://microsoft.com/wsdl/types/" namespace. I don't have enough knowledge how it suppose to work (XML is not very trending these days :)), but I really wonder what should be needed to be done to support these custom types. Any ideas?
Maybe there would be a cheap way to reduce possible breaking changes if we try to reuse something from the legacy implementation..

Regarding the issue about duplicated element names for complex types marked with XmlElement("name") - seems like it's coming from this place: https://github.com/DigDes/SoapCore/blob/develop/src/SoapCore/Meta/MetaBodyWriter.cs#L878

AddSchemaType(writer, toBuild, parentTypeToBuild.ChildElementName ?? member.Name, isArray: createListWithoutProxyType, isListWithoutWrapper: createListWithoutProxyType, isUnqualified: isUnqualified);

parentTypeToBuild.ChildElementName ?? member.Name - parentTypeToBuild.ChildElementName is always not null here, therefore it takes parent element. I tried to change it to toBuild.ChildElementName (currently being process element) - and it kind of returns a better looking WSDL, but it still fails to map/deserialize everything correctly. So perhaps it's just a logic from one side, but it also should be changed something where deserialization happens.

So even if these changes would work for me after some more digging, I'm not really sure if it won't break some other cases.

@Aleksanderis
Copy link
Contributor Author

Aleksanderis commented Sep 22, 2022

Some more findings - after digging a bit more, seems like mapping/deserializing is failing due to additional namespace used, which is specified with XmlElement attribute:

[XmlElement("Product", Namespace = "urn:Bbb.Services.Common")]

In a legacy ASMX implementation these additional schemas are added into WSDL like this (note <s:schema> elements - one is default, second is additional one, and the third is something from Microsoft implementation)":

<wsdl:definitions xmlns:s="http://www.w3.org/2001/XMLSchema"
xmlns:soap12="http://schemas.xmlsoap.org/wsdl/soap12/"
xmlns:http="http://schemas.xmlsoap.org/wsdl/http/"
xmlns:mime="http://schemas.xmlsoap.org/wsdl/mime/"
xmlns:tns="urn:Bbb.Services.Management"
xmlns:s2="http://microsoft.com/wsdl/types/"
xmlns:s1="urn:Bbb.Common" xmlns:soap="http://schemas.xmlsoap.org/wsdl/soap/" xmlns:tm="http://microsoft.com/wsdl/mime/textMatching/"
xmlns:soapenc="http://schemas.xmlsoap.org/soap/encoding/" xmlns:wsdl="http://schemas.xmlsoap.org/wsdl/" targetNamespace="urn:Bbb.Services.Management">
<wsdl:types>
<s:schema elementFormDefault="qualified" targetNamespace="urn:Bbb.Services.Management">
...
</s:schema>
<s:schema elementFormDefault="qualified" targetNamespace="urn:Bbb.Common">
...
</s:schema>
<s:schema elementFormDefault="qualified" targetNamespace="http://microsoft.com/wsdl/types/">
...
</s:schema>
</wsdl:types>
<wsdl:message name="...">
...
</wsdl:message>
<wsdl:message name="...">
...
</wsdl:message>
</wsdl:definitions>

After request is created, it's created like this - note additional xmlns on the Product:

<request xmlns="urn:Bbb.Services.Management">
    <Product xmlns="urn:Bbb.Common">
        <Id>aaa</Id>
        <Name>ddd</Name>
        <Type>ccc</Type>
    </Product>
    <ProductIds>
        <guid>44643eb1-3f76-4c1c-a672-402ae8085934</guid>
        <guid>c84ae939-adf8-4cce-a468-13c3014c73ca</guid>
    </ProductIds>
</request>

With SoapCore these additional schemas are not included in the WSDL and then xmlns are also not included in the request.
I tried to manually craft the request (with the Postman), including additional xmlns in the request - then it works with SoapCore as well..

Any ideas if it's possible to add additional namespaces/schemas to WSDL? Or maybe it's not really necessary and something else could be enough? We have a full control over server side code - we can alter XmlElement attributes, etc as much as needed, but we cannot alter anything on the client side.

I found these "secret" flags in the code - https://github.com/DigDes/SoapCore/blob/develop/src/SoapCore/Meta/BodyWriterExtensions.cs#L17
Setting UseXmlReflectionImporter = true - seems like it's trying to build additional schemas, but then ASMX service doesn't work at all. So perhaps it's not finished feature (although it's a quite old part of code).

@andersjonsson
Copy link
Collaborator

In general I've found that the wsdl is not super important when it comes to supporting old clients.
The wsdl is normally only used when connecting to a new service, in order to generate client code.

I don't know of any way to add additional namespaces to the WSDL, but I think you will be fine if you can manage to add it to the response payload.

I think the namespace Property in XmlTypeAttribute works. Perhaps try to add that to "Product"?

[XmlType(Namespace = "urn:Bbb.Common")]
public class Product
{
...
}

Any PR's you would like to contribute towards making your use case work would be welcome. That way SoapCore gets better for all users 😄

@Aleksanderis
Copy link
Contributor Author

Aleksanderis commented Sep 22, 2022

Yeah, I also realized this fact, that WSDL is not super important for old clients. However I'm still not sure how it works in general - like maybe same logic is used for WSDL and then for deserialization. I.e. if WSDL is wrongly generated, then it might be there is same reason why requests are failing as well.

Regarding XmlType attribute, I already have it, and tried with and without it. Also tried "IncludeInSchema=true", but there is no difference:

[XmlType(TypeName = "Product", Namespace = "urn:Bbb.Common", IncludeInSchema = true)]

I'm not against any PR's, and not waiting that someone fix these issues for me - but at this moment I'm a bit too far away of understanding how it should work, and what was the reasoning for some already partially included but disabled features. :)

@andersjonsson
Copy link
Collaborator

andersjonsson commented Sep 22, 2022

I'm new to this project, but I think most contributions have been made to fix a particular issue. Probably there was an ambition to add support, but the person never got around to finishing it.

In WriteParameterElement I can see that the namespace property of XmlElement is passed into the AddSchemaType method

private void WriteParameterElement(XmlDictionaryWriter writer, SoapMethodParameterInfo parameterInfo)
{
var elementAttribute = parameterInfo.Parameter.GetCustomAttribute<XmlElementAttribute>();
bool isUnqualified = elementAttribute?.Form == XmlSchemaForm.Unqualified;
var elementName = string.IsNullOrWhiteSpace(elementAttribute?.ElementName) ? null : elementAttribute.ElementName;
var xmlRootAttr = parameterInfo.Parameter.ParameterType.GetCustomAttributes<XmlRootAttribute>().FirstOrDefault();
var typeRootName = string.IsNullOrWhiteSpace(xmlRootAttr?.ElementName) ? null : xmlRootAttr.ElementName;
var parameterName = elementName
?? parameterInfo.Parameter.GetCustomAttribute<MessageParameterAttribute>()?.Name
?? typeRootName
?? parameterInfo.Parameter.Name;
AddSchemaType(writer, parameterInfo.Parameter.ParameterType, parameterName, @namespace: elementAttribute?.Namespace, isUnqualified: isUnqualified);
}

However, in AddSchemaTypePropertyOrField the namespace property is not used. Maybe that is the reason why your namespaces doesn't appear in the WSDL?

I can't find any traces of it when it comes to serialization though. For deserialization there is the DeserializeArrayXmlSerializer method that does some "manual" things. Might be worth checking out if you're having issues there.

@Aleksanderis
Copy link
Contributor Author

Aleksanderis commented Sep 22, 2022

Thanks for the clues, but seems it's not the case.
WriteParameterElement() method is called only for the ASMX method request parameter itself (GetPricesRequest in my examples). However XmlElement attribute is set on internal properties of this request object. So elementAttribute?.Namespace is null in WriteParameterElement().

I found some other possible improvement points like these in MetaBodyWriter.cs:
image

After it I got following WSDL - there is "targetNamespace" added, and name="Product" is not duplicated for the elements inside <xsd:sequence> like in my first post.

<xsd:element minOccurs="0" maxOccurs="1" name="Product" type="tns:ProductDto"/>
<xsd:complexType name="ProductDto" targetNamespace="urn:Bbb.Common">
    <xsd:sequence>
        <xsd:element minOccurs="0" maxOccurs="1" name="Id"  type="xsd:string"/>
        <xsd:element minOccurs="0" maxOccurs="1" name="Type"    type="xsd:string"/>
        <xsd:element minOccurs="1" maxOccurs="1" name="Name" type="xsd:string"/>
    </xsd:sequence>
</xsd:complexType>

..but there is no gain further. WSDL looks better, but new client code still doesn't work, while old generated client works without all of these changes. So I'm really not sure if I fixed anything here.

In fact it could be good enough for us, since we are not planning to add any new features for ASMX web services, but there is no confidence if all legacy client code will work without issues. During these years there were many different ways to generate client code, but we always could say - "if it something doesn't work, please regenerate your client". However if the new generated client code won't work - we will loose that argument.

@andersjonsson
Copy link
Collaborator

If you're only having issue with newly generated clients then I believe a focus on WSDL differences is the correct approach.

There is an option to use a file as your WSDL
https://github.com/DigDes/SoapCore#using-with-external-wsdl--xsd-schemas

Try using the legacy WSDL this way, and see if newly generated clients work.
If you then feel like making it work with the WSDL generated by SoapCore, remove differences from the file until you find the culprit (for example, remove the "schema" bit and see if it stops working). Then support could be added for the critical bits.

@Aleksanderis
Copy link
Contributor Author

Aleksanderis commented Sep 22, 2022

Hmm, that sounds interesting. I will try that external WSDL schemas option. Thanks for pointing out!

@Aleksanderis
Copy link
Contributor Author

Aleksanderis commented Sep 26, 2022

So, seems like external WSDL schemas support are fully enough for us, since we are not planning to add any new features to our legacy ASMX web services. Thanks for pointing it out once again! 👍
I said fully, but actually there were some missing options. :) It kind of was working, but some additional workarounds were needed (like duplicated endpoints with different paths for same webservice). As promised about contributing to this project, I created my first pull-request with potential improvements - #904. Will see how it goes, but I hope someone manages to review it and move further. :)

(The mentioned pull-request is not to fix this issue, but perhaps others would be able to solve similar issues in a similar way.)

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

No branches or pull requests

2 participants