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

Fix preserved reference surrogate in collection deserialization bug #264

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Hyperion.API.Tests/CoreApiSpec.ApproveApi.approved.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ namespace Hyperion
public object GetDeserializedObject(int id) { }
public System.Type GetTypeFromTypeId(int typeId) { }
public Hyperion.TypeVersionInfo GetVersionInfo([Hyperion.Internal.NotNull] System.Type type) { }
public void ReplaceOrAddTrackedDeserializedObject([Hyperion.Internal.NotNull] object origin, [Hyperion.Internal.NotNull] object replacement) { }
public void TrackDeserializedObject([Hyperion.Internal.NotNull] object obj) { }
public void TrackDeserializedType([Hyperion.Internal.NotNull] System.Type type) { }
public void TrackDeserializedTypeWithVersion([Hyperion.Internal.NotNull] System.Type type, [Hyperion.Internal.NotNull] Hyperion.TypeVersionInfo versionInfo) { }
Expand Down
27 changes: 27 additions & 0 deletions src/Hyperion.Akka.Integration.Tests/IntegrationSpec.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using Akka.Actor;
using Akka.Configuration;
using Akka.Serialization;
Expand Down Expand Up @@ -60,6 +61,32 @@ public void Akka_HyperionSerializer_should_serialize_properly()
deserialized.Count.Should().Be(24);
}

[Fact]
public void Bugfix263_Akka_HyperionSerializer_should_serialize_ActorPath_list()
{
var actor = Sys.ActorOf(Props.Create<MyActor>());
var container = new ContainerClass(new List<ActorPath>{ actor.Path, actor.Path });
var serialized = _serializer.ToBinary(container);
var deserialized = _serializer.FromBinary<ContainerClass>(serialized);
deserialized.Destinations.Count.Should().Be(2);
deserialized.Destinations[0].Should().Be(deserialized.Destinations[1]);
}

private class MyActor: ReceiveActor
{

}

private class ContainerClass
{
public ContainerClass(List<ActorPath> destinations)
{
Destinations = destinations;
}

public List<ActorPath> Destinations { get; }
}

private class MyPocoClass
{
public string Name { get; set; }
Expand Down
80 changes: 80 additions & 0 deletions src/Hyperion.Tests/CollectionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using System.Dynamic;
using System.IO;
using System.Linq;
using FluentAssertions;
using Xunit;
using Hyperion.SerializerFactories;
using Hyperion.ValueSerializers;
Expand Down Expand Up @@ -415,6 +416,35 @@ public void Issue18()
Assert.True(msg.SequenceEqual(deserialized));
}

[Fact]
public void Issue263_CanSerializeArrayOfSurrogate_WhenPreservingObjectReference()
{
var invoked = new List<SurrogatedClass.ClassSurrogate>();
var serializer = new Serializer(new SerializerOptions(
preserveObjectReferences: true,
surrogates: new []
{
Surrogate.Create<SurrogatedClass, SurrogatedClass.ClassSurrogate>(
to => to.ToSurrogate(),
from => {
invoked.Add(from);
return from.FromSurrogate();
}),
}));

var objectRef = new SurrogatedClass(5);
var expected = new List<SurrogatedClass> { objectRef, objectRef };
using (var stream = new MemoryStream())
{
serializer.Serialize(expected, stream);
stream.Position = 0;
var deserialized = serializer.Deserialize<List<SurrogatedClass>>(stream);
deserialized.Count.Should().Be(2);
invoked.Count.Should().Be(1);
ReferenceEquals(deserialized[0], deserialized[1]).Should().BeTrue();
}
}

#region test classes

public class CustomAdd : IEnumerable<int>
Expand Down Expand Up @@ -451,6 +481,56 @@ public CustomAddRange(IImmutableList<int> inner)
IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}

public class SurrogatedClass
{
public class ClassSurrogate
{
public ClassSurrogate(string value)
{
Value = value;
}

public string Value { get; }

public SurrogatedClass FromSurrogate()
=> new SurrogatedClass(int.Parse(Value));

public override bool Equals(object obj)
{
if (ReferenceEquals(null, obj)) return false;
if (ReferenceEquals(this, obj)) return true;
return obj is ClassSurrogate s && s.Value == Value;
}

public override int GetHashCode()
{
return (Value != null ? Value.GetHashCode() : 0);
}
}

public SurrogatedClass(int value)
{
Value = value;
}

public int Value { get; }

public ClassSurrogate ToSurrogate()
=> new ClassSurrogate(Value.ToString());

public override bool Equals(object obj)
{
if (ReferenceEquals(null, obj)) return false;
if (ReferenceEquals(this, obj)) return true;
return obj is SurrogatedClass s && s.Value == Value;
}

public override int GetHashCode()
{
return Value;
}
}

#endregion

[Fact]
Expand Down
9 changes: 9 additions & 0 deletions src/Hyperion/DeserializeSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,15 @@ public object GetDeserializedObject(int id)
return _objectById[id];
}

public void ReplaceOrAddTrackedDeserializedObject([NotNull] object origin, [NotNull] object replacement)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method will search the current reference array for surrogated object instance and replace it with the actual object reference value

{
var index = _objectById.IndexOf(origin);
if (index == -1)
_objectById.Add(origin);
else
_objectById[index] = replacement;
}

public void TrackDeserializedType([NotNull]Type type)
{
if (_identifierToType == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public override ValueSerializer BuildSerializer(Serializer serializer, Type type
var surrogate = serializer.Options.Surrogates.FirstOrDefault(s => s.To.GetTypeInfo().IsAssignableFrom(type.GetTypeInfo()));
var objectSerializer = new ObjectSerializer(type);
// ReSharper disable once PossibleNullReferenceException
var fromSurrogateSerializer = new FromSurrogateSerializer(surrogate.FromSurrogate, objectSerializer);
var fromSurrogateSerializer = new FromSurrogateSerializer(surrogate.FromSurrogate, objectSerializer, serializer.Options.PreserveObjectReferences);
typeMapping.TryAdd(type, fromSurrogateSerializer);


Expand Down
9 changes: 8 additions & 1 deletion src/Hyperion/ValueSerializers/FromSurrogateSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,16 @@ internal sealed class FromSurrogateSerializer : ValueSerializer
{
private readonly ValueSerializer _surrogateSerializer;
private readonly Func<object, object> _translator;
private readonly bool _preserveObjectReferences;

public FromSurrogateSerializer(Func<object, object> translator, ValueSerializer surrogateSerializer)
public FromSurrogateSerializer(
Func<object, object> translator,
ValueSerializer surrogateSerializer,
bool preserveObjectReferences)
{
_translator = translator;
_surrogateSerializer = surrogateSerializer;
_preserveObjectReferences = preserveObjectReferences;
}

public override void WriteManifest(Stream stream, SerializerSession session)
Expand All @@ -37,6 +42,8 @@ public override object ReadValue(Stream stream, DeserializerSession session)
{
var surrogateValue = _surrogateSerializer.ReadValue(stream, session);
var value = _translator(surrogateValue);
if(_preserveObjectReferences)
session.ReplaceOrAddTrackedDeserializedObject(surrogateValue, value);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace the wrongly injected surrogate object with the actual deserialized de-surrogated value.
Value might be wrongly stored by _surrogateSerializer.ReadValue() in line 43.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boy this must have been a fun one to track down.

return value;
}

Expand Down