EnableImplicitTypingFromAll breaks EnableReferences

[gmkado]

I'm new to this library so still playing around with how ConfigurationContainer works. Apologies if this is the intended behavior.

Here's a simplified version of my data structures:

public class MainDTO
{          
    public SubDTO Sub1 { get; set; } = new SubDTO();
    public SubDTO Sub2 { get; set; } = new SubDTO();
    public SubDTO Sub3 { get; set; } = new SubDTO();
            
}

public class SubDTO
{
    public SubSubDTO SubSub1 { get; set; } = SubSubDTO.NullObject;

}

public class SubSubDTO
{
    public static SubSubDTO NullObject { get; } = new SubSubDTO();

    public string Id { get; set; } = Guid.NewGuid().ToString();
}

And here is my container

var serializer = new ConfigurationContainer()
                .UseAutoFormatting()
                .UseOptimizedNamespaces()
                .EnableImplicitTyping(new[] {
                    typeof(MainDTO), // <-- this causes assertion error
                    typeof(SubDTO),
                    typeof(SubSubDTO)
                })
                .Type<SubSubDTO>()
                    .EnableReferences(definition => definition.Id)                
                .Create();

I would expect all SubDTO objects in MainDTO to be different objects that hold a reference to the same SubSubDTO object.

Here is the output

<?xml version="1.0" encoding="utf-8"?>
<SerializerPlayground-MainDTO xmlns="clr-namespace:UnitTests.RecipeTests;assembly=UnitTests">
  <Sub1>
    <SubSub1 Id="78238952-0d09-48dd-8725-4a7326fa897b" />
  </Sub1>
  <Sub2>
    <SubSub1 xmlns:exs="https://extendedxmlserializer.github.io/v2" exs:entity="78238952-0d09-48dd-8725-4a7326fa897b" />
  </Sub2>
  <Sub3>
    <SubSub1 xmlns:exs="https://extendedxmlserializer.github.io/v2" exs:entity="78238952-0d09-48dd-8725-4a7326fa897b" />
  </Sub3>
</SerializerPlayground-MainDTO>

This looks right, at least they reference the correct ids. But when I deserialize it, Sub1 is correct but the other two have new Guids.

I tracked down the culprit to this line:

.EnableImplicitTyping(typeof(MainDTO))

If I take this out, all works as expected.

Another strange thing, if MainDTO references SubSubDTO directly (instead of the middle object) it works fine, even when enabling implicit typing for all types.

[issue-label-bot]

Issue-Label Bot is automatically applying the label bug to this issue, with a confidence of 0.73. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

[Mike-E-angelo]

Doh, EnableReferences is already on notice with #360. I will look into this one and see if it can be salvaged. 👍

[Mike-E-angelo]

A quick note here that I have been able to reproduce this with your provided code. Thank you for providing it. It appears that the object graph gets serialized properly, but in reading it in something gets grumpy. I will attempt to look further into this but this appears to be another ugly one involving EnableReferences. 😬

[Mike-E-angelo]

Alright tracked this one down. The issue stems from work done to fix #341 which had problems with references being applied to attribute values, which made no sense. So what I did was make a quick check for an identifier being used, which signifies an element node and not an attribute node.

Unless 😅 you are using an extensible formatting system that allows you to have implicit typing for particular elements.

So I basically fixed one issue while introducing another. If I can find some way of denoting an attribute node then we should be able to address this. I believe I went with the identifier route as probing the XmlReader for its current node value was more expensive. We may not have any other choice now.

We'll see how this goes.

[Mike-E-angelo]

Unfortunately the solution I had in mind won't work. This actually isn't an attribute/node problem but a context/scoping problem, which fundamentally is caused by our inner-content reader which has always posed some challenges. As I mentioned, we're doing (or I thought we were doing) some checking to see if the node is in an "attribute node" but really what it's doing is checking the current "identifier" or xmlns attribute within the current context and if it is assigned.

In a typical document this value is set. However, with implicit typing enabled, it's not and instead returning an empty string, and that is what is throwing the reader for a loop at present.

I will give this some more thought, but it is not looking too pretty ATM. One final try/thought/attempt is to perhaps force the XmlReader to have an xmlns if one isn't set in the case of implicit typing, but not sure how involved that would be and/or if that is even possible. The other implied trick here is to ensure that everything still works as-is and the tests for #341 continue to pass.

As per #383 I am giving issues a fair shake but if they involve a lot of work and/or disruption I will have to defer until a simpler solution emerges and/or someone can submit a PR that does my thinking for me. 😁

Marking this issue as help wanted in the meantime.

[gmkado]

Hmm can't say I really grasp the underlying issue here, but is there a way to EnableImplicitTypingFromAll for all types except my MainDTO object?

I'd like to keep the namespaces to a minimum to declutter the document, and this would allow me to not have to specify all types in my call to EnableImplicitTyping

[Mike-E-angelo]

Yeah, the underlying issue is that we have an extensible system that has trouble with its different parts playing nicely with each other. 😁

You could enable implicit typing for everything but MainDTO, however with your object graph as provided that doesn't do too much. What this will do is emit a xmlns attribute at your root and provide the document a default identifier whenever it probes an element.

This will still emit a namespace at the root node which is the primary utility of using implicit typing, however.

The other thing that I thought of, is that we could expose the conditional that is causing the grief here. This isn't much but what it would do is allow you to do is use references + implicit typing, and not references + attributes. You would have to abandon the use of UseAutoFormatting or anything attribute-related to make this work, however.

Kind of hacky, but at least it would give you the option of choosing which extensions and functionality you would prefer to use, which is better than what you have now.

[Mike-E-angelo]

Good news, @gmkado! Turns out I was actually super close yesterday but had overlooked a condition that was previously applied. In reviewing the files I saw my oversight and was able to more specifically check for attributes in the case of references being applied. So now everything should work better for you. Or at least, get us to the next problem. 😆

You can try it out here:

#400 (comment)

I have a passing test demonstrating this here:

home/test/ExtendedXmlSerializer.Tests.ReportedIssues/Issue397Tests.cs

Lines 11 to 46 in 6872b5f

	[Fact]
	public void Verify()
	{
	var serializer = new ConfigurationContainer().UseAutoFormatting()
	.UseOptimizedNamespaces()
	.EnableImplicitTyping(typeof(MainDTO),
	typeof(SubDTO), typeof(SubSubDTO))
	.Type<SubSubDTO>()
	.EnableReferences(definition => definition.Id)
	.Create()
	.ForTesting();
	var instance = new MainDTO();
	var cycled = serializer.Cycle(instance);
	cycled.Sub1.SubSub1.Should().BeSameAs(cycled.Sub2.SubSub1);
	cycled.Sub1.SubSub1.Should().BeSameAs(cycled.Sub3.SubSub1);
	}
	public class MainDTO
	{
	public SubDTO Sub1 { get; set; } = new SubDTO();
	public SubDTO Sub2 { get; set; } = new SubDTO();
	public SubDTO Sub3 { get; set; } = new SubDTO();
	}
	public class SubDTO
	{
	public SubSubDTO SubSub1 { get; set; } = SubSubDTO.NullObject;
	}
	public class SubSubDTO
	{
	public static SubSubDTO NullObject { get; } = new SubSubDTO();
	public string Id { get; set; } = Guid.NewGuid().ToString();
	}

[gmkado]

Thanks @Mike-E-wins , that fix worked for me as well. You are awesome!

Two questions for you:

1. My xml now looks like this:

<?xml version="1.0" encoding="utf-8"?>
<SerializerPlayground-MainDTO>
  <Sub1>
    <SubSub1 Id="a6567700-31af-492b-b3d0-fc6c40b453c6" />
  </Sub1>
  <Sub2>
    <SubSub1 xmlns:exs="https://extendedxmlserializer.github.io/v2" exs:entity="a6567700-31af-492b-b3d0-fc6c40b453c6" />
  </Sub2>
  <Sub3>
    <SubSub1 xmlns:exs="https://extendedxmlserializer.github.io/v2" exs:entity="a6567700-31af-492b-b3d0-fc6c40b453c6" />
  </Sub3>
</SerializerPlayground-MainDTO>

Is there any way to get rid of the xmlns:exs namespaces, or at least pull that definition up to the MainDTO element?

2. I notice sometimes the references use exs:entity=id and other times they use exs:member="" exs:reference="#". Curious what the difference is.

Thanks again!

[Mike-E-angelo]

Great! Glad to hear we got something working after all. As for your questions:

1. Unfortunately exs is a required attribute and cannot be removed. However, you are correct in that it ideally should be pulled up to the root node if OptimizeNamespaces is utilized. I will look into this with EnableReferences + UseOptimizedNamespaces #401.
2. The difference between these two approaches is if the key location is utilized. If you call EnableReferences(x => x.Key) then that should result in one (member/reference, I believe), whereas EnableReferences() should result in the other (entity I believe). I should probably put this in the documentation somewhere but based on how much trouble EnableReferences has been causing me I am also wanting to keep it as much of a secret as possible. 😅

[Mike-E-angelo]

Nope, I had it backwards, @gmkado ... looks like EnableReferences(x => x.Key) results in exs:entity. We do have this in documentation, but it needs more detail around the two modes:

https://github.com/ExtendedXmlSerializer/home/wiki/Features#object-reference-and-circular-reference

GitHub

ExtendedXmlSerializer/home

A highly configurable and eXtensible Xml serializer for .NET. - ExtendedXmlSerializer/home

[Mike-E-angelo]

Actually @gmkado don't listen to me. 😆 I am looking into #401 now and I am seeing identity where I would expect entity. I will get a grasp around this again (it's been a few years) and get a write-up made for documentation and let you know. 👍

[Mike-E-angelo]

Alright @gmkado check out this build:
#402 (comment)

That should have fixes for each of the items that you have identified in this issue, the latest being the exs reference-based attributes when UseOptimizedNamespaces is applied.

I will wait a week in case anything else comes up and then shoot for next Tuesday for a release to NuGet.

As for the documentation, it turns out I was right in my understanding, and the unexpected discrepancies were due to a copy/paste error. 😆 However, this should definitely be captured and I have this assigned as #403.

[Mike-E-angelo]

OK got some further documentation for you @gmkado:
https://github.com/ExtendedXmlSerializer/home/wiki/Example-Scenarios#references

I believe that covers everything but exs:member which is not related to references. exs:member is a marker attribute used to differentiate members from classes with the same name. It's a bit of a hack and something I am not happy with, but alas it's what we have so that is that. 😅

GitHub

ExtendedXmlSerializer/home

A highly configurable and eXtensible Xml serializer for .NET. - ExtendedXmlSerializer/home

[gmkado]

Hi @Mike-E-wins , works on my end. Thanks for updating the documentation!

[Mike-E-angelo]

Great! We'll circle for next Tuesday for a release to NuGet. In the meantime, you can use the preview feed for the latest version which will be used as the deployed version:

Install-Package ExtendedXmlSerializer -Version 3.1.4.6 -Source https://ci.appveyor.com/nuget/extendedxmlserializer-preview

(The above should ™ work, please let me know if you run into any problems and I will look into it for you. 👍)

[Mike-E-angelo]

The fixes here have been deployed to v3.2.0 on NuGet:

https://www.nuget.org/packages/ExtendedXmlSerializer/

Here's hoping we didn't break anything else. 😆 Thank you for helping to make ExtendedXmlSerializer a better product!

ExtendedXmlSerializer 3.2.0

An extensible Xml Serializer for .NET that builds on the functionality of the classic XmlSerializer with a powerful and robust extension model.

[gmkado]

Hi @Mike-E-wins I think v3.2.1 broke the references in v3.2.0 somehow. My reference Ids aren't being read in properly anymore, instead they are being set to a new GUID every time.

I don't have time to dig into it right now, but I can lightswitch the issue by switching back and forth between the two versions.

[Mike-E-angelo]

Pressing this one here right now:
http://www.nooooooooooooooo.com/

😝

Instant No Button! Star Wars funnies FTW!

star wars, star wars lightsabers,star wars lightsaber,darth vader,star wars light saber,star wars clone wars,star war,darth vader pictures,darth vader,r2d2,yoda,vader,anakin,darth vader lightsaber,stormtrooper,darth vader lightsaber,jedi,padme,sith,darth maul,darth vader clone,unleashed darth vader,wars darth vader,rebelscum star wars,droid star wars,grievous star wars,clone trooper,star wars darthvader,star wars luke skywalker,chewbacca star wars,star wars emperor,star wars anakin darth vader,boba fett,palpatine star wars,star wars darth vader light saber,attack of the clones,empire strikes back,darth vader jedi,star wars figures

[Mike-E-angelo]

Sort of a bummer/surprise because we have passing tests around this. I'm becoming nervous around this issue as it can cause breaking changes with deployed code, as already demonstrated by #407, which kind of freaked me out. 😆

When I first approached this issue, I tried to fix it by addressing it with extensions. We might end up doing this and reverting the changes here if it continues to cause problems.

Anyways, if you can find a simple reproduction/code/document that exposes the problem I will take a look into it for you. 👍

[gmkado]

Hey @Mike-E-wins I'm doing my own reference resolving after deserialization, so I likely won't come back to this. Just FYI in case you want to close the issue.

[Mike-E-angelo]

OK cool @gmkado I was wondering about that! If you do run into any issues again for sure let me know and I will look into it for you. Having context/test/code would be ideal as well. 👍 Thanks for the update!