From 89a1d6b78b7fd2bbf66894318f407b897b72c38e Mon Sep 17 00:00:00 2001 From: Arjen Smits Date: Tue, 10 Oct 2017 17:10:05 +0200 Subject: [PATCH] Fix in resolving serializer id (#3135) * Fix in resolving serializer id Serializer id is now only determined once, upon initialisation. And its the serializer implementation which dictates where that id comes from. We no longer assume it always comes from the hocon config. * Forgot the nullserializer case. * Api Approval * Updated according to remarks * Added spec * Fix tests --- .../CoreAPISpec.ApproveCore.approved.txt | 2 + .../Serialization/CustomSerializerSpec.cs | 64 +++++++++++++++++++ src/core/Akka/Serialization/Serialization.cs | 58 +++++++++-------- 3 files changed, 96 insertions(+), 28 deletions(-) create mode 100644 src/core/Akka.Tests/Serialization/CustomSerializerSpec.cs diff --git a/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt b/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt index 311606b9c51..5bba822a6f3 100644 --- a/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt +++ b/src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt @@ -4489,7 +4489,9 @@ namespace Akka.Serialization public Serialization(Akka.Actor.ExtendedActorSystem system) { } public Akka.Actor.ActorSystem System { get; } public void AddSerializationMap(System.Type type, Akka.Serialization.Serializer serializer) { } + [System.ObsoleteAttribute("No longer supported. Use the AddSerializer(name, serializer) overload instead.", true)] public void AddSerializer(Akka.Serialization.Serializer serializer) { } + public void AddSerializer(string name, Akka.Serialization.Serializer serializer) { } public object Deserialize(byte[] bytes, int serializerId, System.Type type) { } public object Deserialize(byte[] bytes, int serializerId, string manifest) { } public Akka.Serialization.Serializer FindSerializerFor(object obj, string defaultSerializerName = null) { } diff --git a/src/core/Akka.Tests/Serialization/CustomSerializerSpec.cs b/src/core/Akka.Tests/Serialization/CustomSerializerSpec.cs new file mode 100644 index 00000000000..731980490d9 --- /dev/null +++ b/src/core/Akka.Tests/Serialization/CustomSerializerSpec.cs @@ -0,0 +1,64 @@ +using System; +using System.Linq; +using Akka.Actor; +using Akka.Configuration; +using Akka.Serialization; +using Newtonsoft.Json; +using Newtonsoft.Json.Converters; +using Xunit; + +namespace Akka.Tests.Serialization +{ + public class CustomSerializerSpec + { + /// + /// Here we basically verify that a serializer decides where its Serializer Identifier is coming + /// from. When using the default Serializer base class, it read from hocon config. But this should not be + /// a neccesity + /// + [Fact] + public void Custom_serializer_must_be_owner_of_its_serializerId() + { + var config = ConfigurationFactory.ParseString(@" + akka.actor { + serializers { + custom = ""Akka.Tests.Serialization.CustomSerializer, Akka.Tests"" + } + serialization-bindings { + ""System.Object"" = custom + } + } + "); + //The above config explictly does not configures the serialization-identifiers section + using (var system = ActorSystem.Create(nameof(CustomSerializerSpec), config)) + { + var serializer = (CustomSerializer)system.Serialization.FindSerializerForType(typeof(object)); + Assert.Equal(666, serializer.Identifier); + } + } + } + + public class CustomSerializer : Serializer + { + public CustomSerializer(ExtendedActorSystem system) : base(system) + { + } + + /// + /// This custom serializer overrides the Identifier implementation and returns a hard coded value + /// + public override int Identifier => 666; + + public override bool IncludeManifest => false; + + public override byte[] ToBinary(object obj) + { + throw new NotImplementedException(); + } + + public override object FromBinary(byte[] bytes, Type type) + { + throw new NotImplementedException(); + } + } +} \ No newline at end of file diff --git a/src/core/Akka/Serialization/Serialization.cs b/src/core/Akka/Serialization/Serialization.cs index 27b931c6f4b..896d3e50cf2 100644 --- a/src/core/Akka/Serialization/Serialization.cs +++ b/src/core/Akka/Serialization/Serialization.cs @@ -59,7 +59,8 @@ public static T SerializeWithTransport(ActorSystem system, Address address, F private readonly Serializer _nullSerializer; private readonly ConcurrentDictionary _serializerMap = new ConcurrentDictionary(); - private readonly Dictionary _serializers = new Dictionary(); + private readonly Dictionary _serializersById = new Dictionary(); + private readonly Dictionary _serializersByName = new Dictionary(); /// /// TBD @@ -70,12 +71,12 @@ public Serialization(ExtendedActorSystem system) System = system; _nullSerializer = new NullSerializer(system); - AddSerializer(_nullSerializer); + AddSerializer("null",_nullSerializer); var serializersConfig = system.Settings.Config.GetConfig("akka.actor.serializers").AsEnumerable().ToList(); var serializerBindingConfig = system.Settings.Config.GetConfig("akka.actor.serialization-bindings").AsEnumerable().ToList(); var serializerSettingsConfig = system.Settings.Config.GetConfig("akka.actor.serialization-settings"); - var namedSerializers = new Dictionary(); + foreach (var kvp in serializersConfig) { var serializerTypeName = kvp.Value.GetString(); @@ -91,8 +92,8 @@ public Serialization(ExtendedActorSystem system) var serializer = serializerConfig != null ? (Serializer)Activator.CreateInstance(serializerType, system, serializerConfig) : (Serializer)Activator.CreateInstance(serializerType, system); - AddSerializer(serializer); - namedSerializers.Add(kvp.Key, serializer); + + AddSerializer(kvp.Key, serializer); } foreach (var kvp in serializerBindingConfig) @@ -109,7 +110,7 @@ public Serialization(ExtendedActorSystem system) } - if (!namedSerializers.TryGetValue(serializerName, out var serializer)) + if (!_serializersByName.TryGetValue(serializerName, out var serializer)) { system.Log.Warning("Serialization binding to non existing serializer: '{0}'", serializerName); continue; @@ -123,20 +124,9 @@ private Serializer GetSerializerByName(string name) { if (name == null) return null; - - var serializersConfig = System.Settings.Config.GetConfig("akka.actor.serializers").AsEnumerable().ToList(); - foreach (var kvp in serializersConfig) - { - if (kvp.Key.Equals(name)) - { - var serializerTypeName = kvp.Value.GetString(); - var serializerType = Type.GetType(serializerTypeName); - - var serializerId = SerializerIdentifierHelper.GetSerializerIdentifierFromConfig(serializerType, (ExtendedActorSystem)System); - return GetSerializerById(serializerId); - } - } - return null; + + _serializersByName.TryGetValue(name, out Serializer serializer); + return serializer; } /// @@ -145,14 +135,26 @@ private Serializer GetSerializerByName(string name) public ActorSystem System { get; } /// - /// TBD + /// Adds the serializer to the internal state of the serialization subsystem /// - /// TBD - /// TBD + /// Serializer instance + [Obsolete("No longer supported. Use the AddSerializer(name, serializer) overload instead.", true)] [MethodImpl(MethodImplOptions.AggressiveInlining)] public void AddSerializer(Serializer serializer) { - _serializers.Add(serializer.Identifier, serializer); + _serializersById.Add(serializer.Identifier, serializer); + } + + /// + /// Adds the serializer to the internal state of the serialization subsystem + /// + /// Configuration name of the serializer + /// Serializer instance + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void AddSerializer(string name, Serializer serializer) + { + _serializersById.Add(serializer.Identifier, serializer); + _serializersByName.Add(name, serializer); } /// @@ -179,7 +181,7 @@ public void AddSerializationMap(Type type, Serializer serializer) /// The resulting object public object Deserialize(byte[] bytes, int serializerId, Type type) { - if (!_serializers.TryGetValue(serializerId, out var serializer)) + if (!_serializersById.TryGetValue(serializerId, out var serializer)) throw new SerializationException( $"Cannot find serializer with id [{serializerId}]. The most probable reason" + " is that the configuration entry 'akka.actor.serializers' is not in sync between the two systems."); @@ -200,7 +202,7 @@ public object Deserialize(byte[] bytes, int serializerId, Type type) /// The resulting object public object Deserialize(byte[] bytes, int serializerId, string manifest) { - if (!_serializers.TryGetValue(serializerId, out var serializer)) + if (!_serializersById.TryGetValue(serializerId, out var serializer)) throw new SerializationException( $"Cannot find serializer with id [{serializerId}]. The most probable reason" + " is that the configuration entry 'akka.actor.serializers' is not in sync between the two systems."); @@ -267,7 +269,7 @@ public Serializer FindSerializerForType(Type objectType, string defaultSerialize } } - if (serializer == null) + if (serializer == null) serializer = GetSerializerByName(defaultSerializerName); // do a final check for the "object" serializer @@ -332,7 +334,7 @@ public static string SerializedActorPath(IActorRef actorRef) internal Serializer GetSerializerById(int serializerId) { - return _serializers[serializerId]; + return _serializersById[serializerId]; } } }