Skip to content

Commit

Permalink
Improved reference tracking for multiple instances (#594)
Browse files Browse the repository at this point in the history
  • Loading branch information
Mike-E-angelo authored Jan 9, 2023
1 parent 534a00e commit 3d5c9fd
Show file tree
Hide file tree
Showing 13 changed files with 125 additions and 83 deletions.
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using ExtendedXmlSerializer.ContentModel.Members;
using ExtendedXmlSerializer.ContentModel.Members;
using ExtendedXmlSerializer.Core;
using ExtendedXmlSerializer.Core.Sources;
using ExtendedXmlSerializer.Core.Specifications;
using ExtendedXmlSerializer.ReflectionModel;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;

namespace ExtendedXmlSerializer.ContentModel.Reflection
{
class VariableTypeWalker : TypeMemberWalkerBase<TypeInfo>, ISource<IEnumerable<TypeInfo>>
{
readonly static ReflectionModel.VariableTypeSpecification Specification =
ReflectionModel.VariableTypeSpecification.Default;
readonly static ISpecification<TypeInfo> Specification =
ReflectionModel.VariableTypeSpecification.Default.Or(IsArraySpecification.Default);

readonly ISpecification<TypeInfo> _specification;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

namespace ExtendedXmlSerializer.ExtensionModel.References;

sealed class ProcessReference : ICommand<ReferenceSet>
sealed class ProcessReference : ICommand<ProcessReferenceInput>
{
readonly ISpecification<TypeInfo> _allowed;
readonly ITypeMembers _members;
Expand All @@ -29,24 +29,37 @@ public ProcessReference(ISpecification<TypeInfo> allowed, ITypeMembers members,
_store = store;
}

public void Execute(ReferenceSet parameter)
public void Execute(ProcessReferenceInput parameter)
{
using var boundary = parameter.Get();
var next = boundary.Subject;
var type = next.GetType();
var (results, current) = parameter;
results.IsSatisfiedBy(current);
Process(results, current);
}

void Process(ReferenceSet results, object current)
{
using var boundary = results.Get(current);
var type = current.GetType();
if (_allowed.IsSatisfiedBy(type))
{
var members = _members.Get(type);
for (var i = 0; i < members.Length; i++)
{
var value = _accessors.Get(members[i]).Get(next);
parameter.Execute(value);
var value = _accessors.Get(members[i]).Get(current);
if (results.IsSatisfiedBy(value))
{
Process(results, value);
}
}

var iterator = _store.For(next);
var iterator = _store.For(current);
while (iterator?.MoveNext() ?? false)
{
parameter.Execute(iterator.Current);
var o = iterator.Current;
if (results.IsSatisfiedBy(o))
{
Process(results, o);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace ExtendedXmlSerializer.ExtensionModel.References;

readonly record struct ProcessReferenceInput(ReferenceSet Results, object Current);
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public void Write(IFormatWriter writer, object instance)
$"{line}{line}Here is a list of found references:{line}{string.Join(line, references.Encountered.Select(x => $"- {x}"))}";

throw new MultipleReferencesDetectedException(
$"The provided instance of type '{type}' contains the same reference multiple times in its graph. While this is technically allowed, it is recommended to instead enable referential support by calling EnableReferences on the ConfigurationContainer. Doing so will ensure that multiple references found in the graph are emitted only once in the serialized document.{message}",
$"The provided instance of type '{type}' contains the same reference multiple times in its graph. While this is technically allowed, it is recommended to instead enable referential support by calling EnableReferences on the ConfigurationContainer. Doing so will ensure that multiple references found in the graph are emitted only once in the serialized document. Additionally, if you do want to allow multiple instances emitted as-is, make use of the `AllowMultipleReferences` method on the ConfigurationContainer.{message}",
_container);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,10 @@ namespace ExtendedXmlSerializer.ExtensionModel.References;
{
readonly Stack<object> _context;

public ReferenceBoundary(Stack<object> context, object subject)
{
Subject = subject;
_context = context;
}

public object Subject { get; }
public ReferenceBoundary(Stack<object> context) => _context = context;

public void Dispose()
{
_context.Push(ReferenceCompleted.Default);
_context.Pop();
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ namespace ExtendedXmlSerializer.ExtensionModel.References;

record ReferenceResult(HashSet<object> Encountered, HashSet<object> Cyclical)
{
public ReferenceResult() : this(new HashSet<object>(), new HashSet<object>()) {}
protected ReferenceResult() : this(new HashSet<object>(), new HashSet<object>()) {}
}
47 changes: 17 additions & 30 deletions src/ExtendedXmlSerializer/ExtensionModel/References/ReferenceSet.cs
Original file line number Diff line number Diff line change
@@ -1,35 +1,31 @@
using ExtendedXmlSerializer.Core;
using ExtendedXmlSerializer.Core.Sources;
using ExtendedXmlSerializer.Core.Specifications;
using ExtendedXmlSerializer.ReflectionModel;
using System.Collections.Generic;

namespace ExtendedXmlSerializer.ExtensionModel.References;

sealed record ReferenceSet : ReferenceResult, ICommand<object>, ISource<ReferenceBoundary>
sealed record ReferenceSet : ReferenceResult, ISpecification<object>, IParameterizedSource<object, ReferenceBoundary>
{
readonly IReferencesPolicy _policy;
readonly Stack<object> _remaining, _depth = new();
readonly Stack<object> _depth;
readonly HashSet<object> _tracked;

public ReferenceSet(IReferencesPolicy policy)
: this(policy, new Stack<object>(), new HashSet<object>()) {}
public ReferenceSet(IReferencesPolicy policy) : this(policy, new Stack<object>(), new HashSet<object>()) {}

public ReferenceSet(IReferencesPolicy policy, Stack<object> remaining, HashSet<object> tracked)
public ReferenceSet(IReferencesPolicy policy, Stack<object> depth, HashSet<object> tracked)
{
_policy = policy;
_remaining = remaining;
_tracked = tracked;
_policy = policy;
_depth = depth;
_tracked = tracked;
}

public void Execute(object parameter)
public bool IsSatisfiedBy(object parameter)
{
if (parameter is not null)
{
if (_tracked.Add(parameter))
{
_remaining.Push(parameter);
}
else
var result = _tracked.Add(parameter);
if (!result)
{
var info = parameter.GetType();
if (!info.IsValueType && _policy.IsSatisfiedBy(info))
Expand All @@ -41,24 +37,15 @@ public void Execute(object parameter)
}
}
}
return result;
}

return false;
}

public ReferenceBoundary Get()
public ReferenceBoundary Get(object parameter)
{
var subject = _remaining.Pop();
while (subject is ReferenceCompleted)
{
_depth.Pop();
subject = Any() ? _remaining.Pop() : null;
}

if (subject is not null)
{
_depth.Push(subject);
}
return new(_depth, subject);
_depth.Push(parameter);
return new(_depth);
}

public bool Any() => _remaining.Count > 0;
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,5 @@ public ReferenceView(IContainsCustomSerialization custom, IReferencesPolicy poli
// ReSharper disable once TooManyDependencies
ReferenceView(ReferenceWalker walker) => _walker = walker;

public ReferenceResult Get(object parameter)
{
var result = _walker.Get(parameter);
return result;
}
public ReferenceResult Get(object parameter) => _walker.Get(parameter);
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,7 @@ public ReferenceWalker(IReferencesPolicy policy, ProcessReference process)
public ReferenceResult Get(object parameter)
{
var result = new ReferenceSet(_policy);
result.Execute(parameter);
while (result.Any())
{
_process.Execute(result);
}

_process.Execute(new (result, parameter));
return result;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public void Verify()
Action action = () => new ConfigurationContainer().Create().Cycle(subject).Should().BeEquivalentTo(subject);
action.Should()
.Throw<Exception>()
.WithMessage("The provided instance of type 'ExtendedXmlSerializer.Tests.ReportedIssues.Issue583Tests+RootObject' contains the same reference multiple times in its graph. While this is technically allowed, it is recommended to instead enable referential support by calling EnableReferences on the ConfigurationContainer. Doing so will ensure that multiple references found in the graph are emitted only once in the serialized document.\r\n\r\nHere is a list of found references:\r\n- ExtendedXmlSerializer.Tests.ReportedIssues.Issue583Tests+InnerObject");
.WithMessage("The provided instance of type 'ExtendedXmlSerializer.Tests.ReportedIssues.Issue583Tests+RootObject' contains the same reference multiple times in its graph. While this is technically allowed, it is recommended to instead enable referential support by calling EnableReferences on the ConfigurationContainer. Doing so will ensure that multiple references found in the graph are emitted only once in the serialized document. Additionally, if you do want to allow multiple instances emitted as-is, make use of the `AllowMultipleReferences` method on the ConfigurationContainer.\r\n\r\nHere is a list of found references:\r\n- ExtendedXmlSerializer.Tests.ReportedIssues.Issue583Tests+InnerObject");
}

[Fact]
Expand All @@ -39,7 +39,7 @@ public void VerifyOnePropertyAndCollectionThrows()
Action action = () => new ConfigurationContainer().Create().Cycle(subject).Should().BeEquivalentTo(subject);
action.Should()
.Throw<Exception>()
.WithMessage("The provided instance of type 'ExtendedXmlSerializer.Tests.ReportedIssues.Issue583Tests+RootObjectOneProperty' contains the same reference multiple times in its graph. While this is technically allowed, it is recommended to instead enable referential support by calling EnableReferences on the ConfigurationContainer. Doing so will ensure that multiple references found in the graph are emitted only once in the serialized document.\r\n\r\nHere is a list of found references:\r\n- ExtendedXmlSerializer.Tests.ReportedIssues.Issue583Tests+InnerObject");
.WithMessage("The provided instance of type 'ExtendedXmlSerializer.Tests.ReportedIssues.Issue583Tests+RootObjectOneProperty' contains the same reference multiple times in its graph. While this is technically allowed, it is recommended to instead enable referential support by calling EnableReferences on the ConfigurationContainer. Doing so will ensure that multiple references found in the graph are emitted only once in the serialized document. Additionally, if you do want to allow multiple instances emitted as-is, make use of the `AllowMultipleReferences` method on the ConfigurationContainer.\r\n\r\nHere is a list of found references:\r\n- ExtendedXmlSerializer.Tests.ReportedIssues.Issue583Tests+InnerObject");
}

[Fact]
Expand Down
65 changes: 65 additions & 0 deletions test/ExtendedXmlSerializer.Tests.ReportedIssues/Issue584Tests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
using ExtendedXmlSerializer.Configuration;
using ExtendedXmlSerializer.Tests.ReportedIssues.Support;
using FluentAssertions;
using System;
using Xunit;

namespace ExtendedXmlSerializer.Tests.ReportedIssues
{
public sealed class Issue584Tests
{
[Fact]
public void Verify()
{
var items = new[] { "Some" };
var same = new InnerObject { Items = items };
var instance = new RootObject
{
Item1 = same,
Item2 = new InnerObject2 { Items = items },
};

var action = () =>
{
var serializer = new ConfigurationContainer()
.Create()
.ForTesting();
serializer.Cycle(instance).Should().BeEquivalentTo(instance);
};
action.Should().Throw<Exception>().Where(x => x.GetType().Name == "MultipleReferencesDetectedException");
}

[Fact]
public void VerifyConfiguration()
{
var items = new[] { "Some" };
var same = new InnerObject { Items = items };
var instance = new RootObject
{
Item1 = same,
Item2 = new InnerObject2 { Items = items },
};

var serializer = new ConfigurationContainer().AllowMultipleReferences()
.Create()
.ForTesting();
serializer.Cycle(instance).Should().BeEquivalentTo(instance);
}

public class RootObject
{
public InnerObject Item1 { get; set; }
public InnerObject2 Item2 { get; set; }
}

public class InnerObject
{
public string[] Items { get; set; }
}

public class InnerObject2
{
public string[] Items { get; set; }
}
}
}
10 changes: 3 additions & 7 deletions test/ExtendedXmlSerializer.Tests.ReportedIssues/Issue98Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,13 @@ void Verify()
var instance = new Foo {Bar = new Bar()};
instance.Bar.Foos.Add(instance);

var serializer = new ConfigurationContainer().EnableReferences()
.Create();
var serializer = new ConfigurationContainer().EnableReferences().Create().ForTesting();

var cycled = serializer.Cycle(instance);

cycled.Should()
.BeSameAs(cycled.Bar.Foos.Only());
cycled.Should().BeSameAs(cycled.Bar.Foos.Only());

cycled.Bar.Should()
.BeSameAs(cycled.Bar.Foos.Only()
.Bar);
cycled.Bar.Should().BeSameAs(cycled.Bar.Foos.Only().Bar);
}

[Fact]
Expand Down

0 comments on commit 3d5c9fd

Please sign in to comment.