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

BeXmlSerializable does not respect XmlIgnoreAttribute #2626

Open
sa-he opened this issue Apr 15, 2024 · 5 comments
Open

BeXmlSerializable does not respect XmlIgnoreAttribute #2626

sa-he opened this issue Apr 15, 2024 · 5 comments

Comments

@sa-he
Copy link

sa-he commented Apr 15, 2024

Description

BeXmlSerializable creates a clone using XmlSerializer. XmlSerializer ignores members marked by XmlIgnoreAttribute.

The resulting clone is compared with the original. But this comparison does NOT ignore members marked by XmlIgnoreAttibute.

Reproduction Steps

public class DemoData
{
    public string Id { get; set; }

    [XmlIgnore]
    public Guid IdWrapper
    {
        get => Guid.Parse(this.Id);
        set => this.Id = value.ToString();
    }

    [XmlIgnore]
    public string IgnoreMe { get; set; }

    public string Name { get; set; }
}

// Arrange
var sut = new DemoData { Id = "m2", Name = "test 2", IgnoreMe = "ignore me" };

// Act
sut.Should().BeXmlSerializable();

Expected behavior

Test should succeed.

Actual behavior

Test fails.

Regression?

No response

Known Workarounds

No response

Configuration

FluentAssertions Version="6.12.0"
MSTest.TestAdapter/Framework Version="2.2.8"
net472 and net6.0-windows

Other information

No response

Are you willing to help with a pull-request?

No

@tacosontitan
Copy link

tacosontitan commented Apr 16, 2024

I'm not sure this is an issue with the process not ignoring members. Based on the code for the BeXmlSerializable method, it looks like this is working as expected and equivalency is failing:

public static AndConstraint<ObjectAssertions> BeXmlSerializable(this ObjectAssertions assertions, string because = "",
params object[] becauseArgs)
{
try
{
object deserializedObject = CreateCloneUsingXmlSerializer(assertions.Subject);
deserializedObject.Should().BeEquivalentTo(assertions.Subject,
options => options.RespectingRuntimeTypes().IncludingFields().IncludingProperties());
}
catch (Exception exc)
{
Execute.Assertion
.BecauseOf(because, becauseArgs)
.FailWith("Expected {0} to be serializable{reason}, but serialization failed with:"
+ Environment.NewLine + Environment.NewLine + "{1}.",
assertions.Subject,
exc.Message);
}
return new AndConstraint<ObjectAssertions>(assertions);
}

According to line 107 there, the deserialized object should be equivalent to the subject of the assertion. This, combined with my (albeit severely lacking) understanding of how the equivalency test works, should (currently) fail because the ignored members won't be set in the deserialized object.

@dennisdoomen, can this be assigned to me?

@tacosontitan
Copy link

tacosontitan commented Apr 16, 2024

After further investigation, this is certainly not an issue with XML serialization, but rather how the assertion process handles equivalency:

image

As my screenshot shows, ignored members are not set during the deserialization process, and as a result the equivalency will fail. I am currently researching how involved of a change it would be to add support for this. I think the most surgical route would be to update EquivalencyOptions<TExpectation> to exclude ignored members and the BeXmlSerializable method to specify this as part of its options:

deserializedObject.Should().BeEquivalentTo(assertions.Subject, 
    static options => options
        .RespectingRuntimeTypes()
        .IncludingFields()
        .IncludingProperties()
        .ExcludeIgnoredMembers());

My biggest remaining question is if this new method should be specific to XML or not as ExcludeXmlIgnoredMembers for clarity that other serialization methods are not supported.

@tacosontitan
Copy link

I've defined a unit test to cover this scenario and opened a draft PR to begin working this in full. I simply need guidance on the following to prove as effective as possible on the issue:

  • Should this be added to the EquivalencyOptions model?
    • If so:
      • What should the name of the method be?
        • I recommend ExcludeXmlIgnoredMembers.
      • Are there any niche references of this that I need to be careful about?
    • If not:
      • How should this be accounted for?
  • Are there any ecosystem-wide scenarios or goals I need to keep in mind?
  • Are there any known gotchas with the BeXmlSerializable method?
  • Is there anything else I should be aware of prior to proceeding?

@dennisdoomen
Copy link
Member

I think a simpler approach could be to add a custom IMemberSelectionRule that is based on AllPropertiesSelectionRule and AllFieldsSelectionRule and filters out properties and fields marked with the XmlIgnore attribute.

@sa-he
Copy link
Author

sa-he commented Apr 17, 2024

Is it possible to choose an implementation that also enables BeEquivalentTo to optionally ignore properties with the XmlIgnoreAttribute?

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

No branches or pull requests

3 participants