Added support for serialization for classes containing IList properties

[scottschluer]

Given the following class structure:

public class Person
{
    [SerializeAs(Name = "contact-data")]
    public ContactData ContactData { get;set; }
}

public class ContactData
{
    [SerializeAs(Name = "email-addresses")]
    public List<EmailAddress> EmailAddresses { get; set; }
}

[SerializeAs(Name = "email-address")]
public class EmailAddress
{
    [SerializeAs(Name = "address")]
    public string Address { get;set; }
}

RestSharp's XmlSerializer returns the following XML:

<person>
    <contact-data>
        <email-addresses>
            <EmailAddress>
                <address>my@email.com</address> 
             </EmailAddress>
        </email-addresses>
    </contact-data>
</person>

Note that the serialization of the "EmailAddress" element ignored the SerializeAs attribute.

Code attached to fix this issue and correctly serialize class names when included as an IList<> property.

[pseudomuto]

@haacked this look good to me. Can you verify the indentation?

[haacked]

One question. Why is this for IList and not for ICollection or even IEnumerable? Are those handled elsewhere?

[pseudomuto]

Hmm...good point. Maybe IEnumerable would be a better bet

On 2013-09-05, at 10:19 PM, Phil Haack notifications@github.com wrote:

One question. Why is this for IList and not for ICollection or even IEnumerable? Are those handled elsewhere?

—
Reply to this email directly or view it on GitHub.

[pseudomuto]

@scottschluer think you could change it to support IEnumerable real quick?

[scottschluer]

Sure. I submitted this PR a while back so my memory is a little fuzzy on it but I seem to recall there was a problem specific to IList. I'll test it tomorrow morning and if it's a problem that extends to IEnumerable, I'll update the PR.

[pseudomuto]

@scottschluer nice!

[scottschluer]

I believe this is why it's specific to IList. There was an existing logic branch that my patch modified that operated specifically on IList collections. Changing this to IEnumerable would widen the scope of my patch. Following the logic branching in this section of code, IEnumerable or ICollection would fall down to the "else" statement beneath the one I modified.

[scottschluer]

I added a unit test locally for IEnumerable and it works as intended. The problem was specific to IList collections as there was a logic branch in the existing code that handled IList differently than other types.

[haacked]

So maybe we should just remove that branch that handles IList separately. Does it look like there's a good reason for it?

[scottschluer]

I'd have to look further into it, but removing that branch causes three unit tests to fail:

• Can_serialize_a_list_which_is_the_root_element()
• Can_serialize_simple_POCO()
• Can_serialize_simple_POCO_With_Attribute_Options_Defined_And_Property_Containing_IList_Elements()

The last one was the test I added as part of this PR.

[haacked]

Ok, let's just go with your change for now since you have test coverage and it doesn't break existing tests. :)

[scottschluer]

Sounds good. I still feel like there's room for improvement on this logic branch now that I'm looking at it again, but that'd be more of a refactor than a bug fix so I'll let it go for now. :) Thanks for the code review.

[haacked]

Thanks!