Fix in resolving serializer id (akkadotnet#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

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) { }

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
+    {
+        /// <summary>
+        /// 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
+        /// </summary>
+        [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)
+        {
+        }
+
+        /// <summary>
+        /// This custom serializer overrides the Identifier implementation and returns a hard coded value
+        /// </summary>
+        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();
+        }
+    }
+}

src/core/Akka/Serialization/Serialization.cs

@@ -59,7 +59,8 @@ public static T SerializeWithTransport<T>(ActorSystem system, Address address, F
         private readonly Serializer _nullSerializer;
 
         private readonly ConcurrentDictionary<Type, Serializer> _serializerMap = new ConcurrentDictionary<Type, Serializer>();
-        private readonly Dictionary<int, Serializer> _serializers = new Dictionary<int, Serializer>();
+        private readonly Dictionary<int, Serializer> _serializersById = new Dictionary<int, Serializer>();
+        private readonly Dictionary<string, Serializer> _serializersByName = new Dictionary<string, Serializer>();
 
         /// <summary>
         /// 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<string, Serializer>();
+
             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;
         }
 
         /// <summary>
@@ -145,14 +135,26 @@ private Serializer GetSerializerByName(string name)
         public ActorSystem System { get; }
 
         /// <summary>
-        /// TBD
+        /// Adds the serializer to the internal state of the serialization subsystem
         /// </summary>
-        /// <param name="serializer">TBD</param>
-        /// <returns>TBD</returns>
+        /// <param name="serializer">Serializer instance</param>
+        [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);
+        }
+
+        /// <summary>
+        /// Adds the serializer to the internal state of the serialization subsystem
+        /// </summary>
+        /// <param name="name">Configuration name of the serializer</param>
+        /// <param name="serializer">Serializer instance</param>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public void AddSerializer(string name, Serializer serializer)
+        {
+            _serializersById.Add(serializer.Identifier, serializer);
+            _serializersByName.Add(name, serializer);
         }
 
         /// <summary>
@@ -179,7 +181,7 @@ public void AddSerializationMap(Type type, Serializer serializer)
         /// <returns>The resulting object</returns>
         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)
         /// <returns>The resulting object</returns>
         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];
         }
     }
 }