Bug/476 multiple methods per interface with JSON serialization doesn´…

…t work (dapr#1343)

* update devcontainer

Signed-off-by: paule96 <paul-jeschke@outlook.com>

* update test setup

Signed-off-by: paule96 <paul-jeschke@outlook.com>

* Now the json serialization should work with multiple methods in an interface

Signed-off-by: paule96 <paul-jeschke@outlook.com>

* fixed devcontainer to run actors

Now the devcontainer uses docker in docker, so you can reach the dapr setup after you did run dapr init. This will then only affect the dev container, without compromising the host of the devcontainer

Signed-off-by: paule96 <paul-jeschke@outlook.com>

* fix bugs with the current implementation

Signed-off-by: paule96 <paul-jeschke@outlook.com>

* add a test that checks excatly the behavior

Signed-off-by: paule96 <paul-jeschke@outlook.com>

* fix devcontainer post creatd command

Signed-off-by: paule96 <paul-jeschke@outlook.com>

* change the default to dotnet 8.0

Signed-off-by: paule96 <paul-jeschke@outlook.com>

* I don't know what is different but we commit.

Maybe it resolves the need of chmod for it 🤷‍♀️

Signed-off-by: paule96 <paul-jeschke@outlook.com>

* make it easier to see why the application of an E2E test couldn't start

Signed-off-by: paule96 <paul-jeschke@outlook.com>

* make the exception in E2E more percise

Signed-off-by: paule96 <paul-jeschke@outlook.com>

* fix exception message

Signed-off-by: paule96 <paul-jeschke@outlook.com>

---------

Signed-off-by: paule96 <paul-jeschke@outlook.com>
Co-authored-by: Yaron Schneider <schneider.yaron@live.com>
Co-authored-by: Whit Waldo <whit.waldo@innovian.net>
Signed-off-by: Siri Varma Vegiraju <svegiraju@microsoft.com>

.devcontainer/devcontainer.json

@@ -10,11 +10,15 @@
 		"ghcr.io/devcontainers/features/azure-cli:1": {
 			"version": "2.38"
 		},
-		"ghcr.io/devcontainers/features/docker-from-docker:1": {
-			"version": "20.10"
+		"ghcr.io/devcontainers/features/docker-in-docker": {
+			"version": "latest"
 		},
-		"ghcr.io/devcontainers/features/dotnet:1": {
-			"version": "6.0"
+		"ghcr.io/devcontainers/features/dotnet": {
+			"version": "8.0",
+			"additionalVersions": [
+				"6.0",
+				"7.0"
+			]
 		},
 		"ghcr.io/devcontainers/features/github-cli:1": {
 			"version": "2"
@@ -32,7 +36,8 @@
 		"ms-dotnettools.csharp",
 		"ms-dotnettools.vscode-dotnet-runtime",
 		"ms-azuretools.vscode-dapr",
-		"GitHub.copilot"
+		"GitHub.copilot",
+		"ms-dotnettools.csdevkit"
 	],
 	"forwardPorts": [
 		3000,
@@ -42,10 +47,9 @@
 		5000,
 		5007
 	],
-	"postCreateCommand": ".devcontainer/localinit.sh",
+	"postCreateCommand": "chmod +x .devcontainer/localinit.sh && .devcontainer/localinit.sh",
 	"remoteUser": "vscode",
 	"hostRequirements": {
 		"memory": "8gb"
 	}
-  }
-
+}

src/Dapr.Actors/Communication/ActorMessageBodyJsonSerializationProvider.cs

@@ -103,7 +103,10 @@ public MemoryStreamMessageBodySerializer(
             {
                 var _methodRequestParameterTypes = new List<Type>(methodRequestParameterTypes);
                 var _wrappedRequestMessageTypes = new List<Type>(wrappedRequestMessageTypes);
-
+                if(_wrappedRequestMessageTypes.Count > 1){
+                    throw new NotSupportedException("JSON serialisation should always provide the actor method (or nothing), that was called" +
+                        " to support (de)serialisation. This is a Dapr SDK error, open an issue on GitHub.");
+                }
                 this.serializerOptions = new(serializerOptions)
                 {
                     // Workaround since WrappedMessageBody creates an object

src/Dapr.Actors/Communication/ActorMessageSerializersManager.cs

@@ -15,11 +15,14 @@ namespace Dapr.Actors.Communication
 {
     using System;
     using System.Collections.Concurrent;
+    using System.Collections.Generic;
+    using System.Diagnostics.CodeAnalysis;
+    using System.Linq;
     using Dapr.Actors.Builder;
 
     internal class ActorMessageSerializersManager
     {
-        private readonly ConcurrentDictionary<int, CacheEntry> cachedBodySerializers;
+        private readonly ConcurrentDictionary<(int, string), CacheEntry> cachedBodySerializers;
         private readonly IActorMessageHeaderSerializer headerSerializer;
         private readonly IActorMessageBodySerializationProvider serializationProvider;
 
@@ -38,7 +41,7 @@ public ActorMessageSerializersManager(
             }
 
             this.serializationProvider = serializationProvider;
-            this.cachedBodySerializers = new ConcurrentDictionary<int, CacheEntry>();
+            this.cachedBodySerializers = new ConcurrentDictionary<(int, string), CacheEntry>();
             this.headerSerializer = headerSerializer;
         }
 
@@ -52,19 +55,19 @@ public IActorMessageHeaderSerializer GetHeaderSerializer()
             return this.headerSerializer;
         }
 
-        public IActorRequestMessageBodySerializer GetRequestMessageBodySerializer(int interfaceId)
+        public IActorRequestMessageBodySerializer GetRequestMessageBodySerializer(int interfaceId, [AllowNull] string methodName = null)
         {
-            return this.cachedBodySerializers.GetOrAdd(interfaceId, this.CreateSerializers).RequestMessageBodySerializer;
+            return this.cachedBodySerializers.GetOrAdd((interfaceId, methodName), this.CreateSerializers).RequestMessageBodySerializer;
         }
 
-        public IActorResponseMessageBodySerializer GetResponseMessageBodySerializer(int interfaceId)
+        public IActorResponseMessageBodySerializer GetResponseMessageBodySerializer(int interfaceId, [AllowNull] string methodName = null)
         {
-            return this.cachedBodySerializers.GetOrAdd(interfaceId, this.CreateSerializers).ResponseMessageBodySerializer;
+            return this.cachedBodySerializers.GetOrAdd((interfaceId, methodName), this.CreateSerializers).ResponseMessageBodySerializer;
         }
 
-        internal CacheEntry CreateSerializers(int interfaceId)
+        internal CacheEntry CreateSerializers((int interfaceId, string methodName) data)
         {
-            var interfaceDetails = this.GetInterfaceDetails(interfaceId);
+            var interfaceDetails = this.GetInterfaceDetails(data.interfaceId);
 
             // get the service interface type from the code gen layer
             var serviceInterfaceType = interfaceDetails.ServiceInterfaceType;
@@ -74,10 +77,29 @@ internal CacheEntry CreateSerializers(int interfaceId)
 
             // get the known types from the codegen layer
             var responseBodyTypes = interfaceDetails.ResponseKnownTypes;
+            if (data.methodName is null)
+            {
+                // Path is mainly used for XML serialization
+                return new CacheEntry(
+                    this.serializationProvider.CreateRequestMessageBodySerializer(serviceInterfaceType, requestBodyTypes, interfaceDetails.RequestWrappedKnownTypes),
+                    this.serializationProvider.CreateResponseMessageBodySerializer(serviceInterfaceType, responseBodyTypes, interfaceDetails.ResponseWrappedKnownTypes));
+            }
+            else
+            {
+                // This path should be used for JSON serialization
+                var requestWrapperTypeAsList = interfaceDetails.RequestWrappedKnownTypes.Where(r => r.Name == $"{data.methodName}ReqBody").ToList();
+                if(requestWrapperTypeAsList.Count > 1){
+                    throw new NotSupportedException($"More then one wrappertype was found for {data.methodName}");
+                }
+                var responseWrapperTypeAsList = interfaceDetails.ResponseWrappedKnownTypes.Where(r => r.Name == $"{data.methodName}RespBody").ToList();
+                    if(responseWrapperTypeAsList.Count > 1){
+                    throw new NotSupportedException($"More then one wrappertype was found for {data.methodName}");
+                }
+                return new CacheEntry(
+                    this.serializationProvider.CreateRequestMessageBodySerializer(serviceInterfaceType, requestBodyTypes, requestWrapperTypeAsList),
+                    this.serializationProvider.CreateResponseMessageBodySerializer(serviceInterfaceType, responseBodyTypes, responseWrapperTypeAsList));
+            }
 
-            return new CacheEntry(
-               this.serializationProvider.CreateRequestMessageBodySerializer(serviceInterfaceType, requestBodyTypes, interfaceDetails.RequestWrappedKnownTypes),
-               this.serializationProvider.CreateResponseMessageBodySerializer(serviceInterfaceType, responseBodyTypes, interfaceDetails.ResponseWrappedKnownTypes));
         }
 
         internal InterfaceDetails GetInterfaceDetails(int interfaceId)

src/Dapr.Actors/DaprHttpInteractor.cs

@@ -116,7 +116,7 @@ public async Task<IActorResponseMessage> InvokeActorMethodWithRemotingAsync(Acto
             var serializedHeader = serializersManager.GetHeaderSerializer()
                 .SerializeRequestHeader(remotingRequestRequestMessage.GetHeader());
 
-            var msgBodySeriaizer = serializersManager.GetRequestMessageBodySerializer(interfaceId);
+            var msgBodySeriaizer = serializersManager.GetRequestMessageBodySerializer(interfaceId, methodName);
             var serializedMsgBody = msgBodySeriaizer.Serialize(remotingRequestRequestMessage.GetBody());
 
             // Send Request
@@ -170,7 +170,7 @@ HttpRequestMessage RequestFunc()
 
                 // Deserialize Actor Response Message Body
                 // Deserialize to ActorInvokeException when there is response header otherwise normal path
-                var responseBodySerializer = serializersManager.GetResponseMessageBodySerializer(interfaceId);
+                var responseBodySerializer = serializersManager.GetResponseMessageBodySerializer(interfaceId, methodName);
 
                 // actorResponseMessageHeader is not null, it means there is remote exception
                 if (actorResponseMessageHeader != null)

src/Dapr.Actors/Runtime/ActorManager.cs

@@ -106,8 +106,8 @@ internal async Task<Tuple<string, byte[]>> DispatchWithRemotingAsync(ActorId act
             var interfaceId = actorMessageHeader.InterfaceId;
 
             // Get the deserialized Body.
-            var msgBodySerializer = this.serializersManager.GetRequestMessageBodySerializer(actorMessageHeader.InterfaceId);
-
+            var msgBodySerializer = this.serializersManager.GetRequestMessageBodySerializer(actorMessageHeader.InterfaceId, actorMethodContext.MethodName);
+            
             IActorRequestMessageBody actorMessageBody;
             using (var stream = new MemoryStream())
             {
@@ -130,7 +130,7 @@ async Task<Tuple<string, byte[]>> RequestFunc(Actor actor, CancellationToken ct)
                     this.messageBodyFactory,
                     ct);
 
-                return this.CreateResponseMessage(responseMsgBody, interfaceId);
+                return this.CreateResponseMessage(responseMsgBody, interfaceId, actorMethodContext.MethodName);
             }
 
             return await this.DispatchInternalAsync(actorId, actorMethodContext, RequestFunc, cancellationToken);
@@ -386,12 +386,12 @@ private async Task<T> DispatchInternalAsync<T>(ActorId actorId, ActorMethodConte
             return retval;
         }
 
-        private Tuple<string, byte[]> CreateResponseMessage(IActorResponseMessageBody msgBody, int interfaceId)
+        private Tuple<string, byte[]> CreateResponseMessage(IActorResponseMessageBody msgBody, int interfaceId, string methodName)
         {
             var responseMsgBodyBytes = Array.Empty<byte>();
             if (msgBody != null)
             {
-                var responseSerializer = this.serializersManager.GetResponseMessageBodySerializer(interfaceId);
+                var responseSerializer = this.serializersManager.GetResponseMessageBodySerializer(interfaceId, methodName);
                 responseMsgBodyBytes = responseSerializer.Serialize(msgBody);
             }
 

test/Dapr.E2E.Test.Actors/ISerializationActor.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 using System.Text.Json;
 using System.Text.Json.Serialization;
@@ -10,6 +11,7 @@ namespace Dapr.E2E.Test.Actors
     public interface ISerializationActor : IActor, IPingActor
     {
         Task<SerializationPayload> SendAsync(string name, SerializationPayload payload, CancellationToken cancellationToken = default);
+        Task<DateTime> AnotherMethod(DateTime payload);
     }
 
     public record SerializationPayload(string Message)

test/Dapr.E2E.Test.App/Actors/SerializationActor.cs

@@ -1,4 +1,5 @@
 
+using System;
 using System.Threading;
 using System.Threading.Tasks;
 using Dapr.Actors.Runtime;
@@ -22,5 +23,9 @@ public Task<SerializationPayload> SendAsync(string name,
         {
             return Task.FromResult(payload);
         }
+
+        public Task<DateTime> AnotherMethod(DateTime payload){
+            return Task.FromResult(payload);
+        }
     }
 }

test/Dapr.E2E.Test/Actors/E2ETests.CustomSerializerTests.cs

@@ -84,5 +84,32 @@ public async Task ActorCanSupportCustomSerializer()
                 Assert.Equal(JsonSerializer.Serialize(kvp.Value), JsonSerializer.Serialize(value));
             }
         }
+
+        /// <summary>
+        /// This was actually a problem that is why the test exists.
+        /// It just checks, if the interface of the actor has more than one method defined,
+        /// that if can call it and serialize the payload correctly.
+        /// </summary>
+        /// <remarks>
+        /// More than one methods means here, that in the exact interface must be two methods defined.
+        /// That excludes hirachies. 
+        /// So <see cref="IPingActor.Ping"/> wouldn't count here, because it's not directly defined in 
+        /// <see cref="ISerializationActor"/>. (it's defined in the base of it.)
+        /// That why <see cref="ISerializationActor.AnotherMethod(DateTime)"/> was created,
+        /// so there are now more then one method.
+        /// </remark>
+        [Fact]
+        public async Task ActorCanSupportCustomSerializerAndCallMoreThenOneDefinedMethod()
+        {
+            using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(60));
+            var proxy = this.ProxyFactory.CreateActorProxy<ISerializationActor>(ActorId.CreateRandom(), "SerializationActor");
+
+            await ActorRuntimeChecker.WaitForActorRuntimeAsync(this.AppId, this.Output, proxy, cts.Token);
+
+            var payload = DateTime.MinValue;
+            var result = await proxy.AnotherMethod(payload);
+
+            Assert.Equal(payload, result);
+        }
     }
 }

test/Dapr.E2E.Test/DaprCommand.cs

@@ -16,13 +16,15 @@ namespace Dapr.E2E.Test
     using System;
     using System.Collections.Generic;
     using System.Diagnostics;
+    using System.Drawing;
     using System.Linq;
     using System.Threading;
     using Xunit.Abstractions;
 
     public class DaprCommand
     {
         private readonly ITestOutputHelper output;
+        private readonly CircularBuffer<string> logBuffer = new CircularBuffer<string>(1000);
 
         public DaprCommand(ITestOutputHelper output)
         {
@@ -66,7 +68,12 @@ public void Run()
             var done = outputReceived.WaitOne(this.Timeout);
             if (!done)
             {
-                throw new Exception($"Command: \"{this.Command}\" timed out while waiting for output: \"{this.OutputToMatch}\"");
+                var ex = new Exception($"Command: \"{this.Command}\" timed out while waiting for output: \"{this.OutputToMatch}\"{System.Environment.NewLine}" + 
+                "This could also mean the E2E app had a startup error. For more details see the Data property of this exception.");
+                // we add here the log buffer of the last 1000 lines, of the application log
+                // to make it easier to debug failing tests
+                ex.Data.Add("log", this.logBuffer.ToArray());
+                throw ex;
             }
         }
 
@@ -79,8 +86,7 @@ private void CheckOutput(object sendingProcess, DataReceivedEventArgs e)
 
             try
             {
-                // see: https://github.com/xunit/xunit/issues/2146
-                this.output.WriteLine(e.Data.TrimEnd(Environment.NewLine.ToCharArray()));
+                WriteLine(e.Data);
             }
             catch (InvalidOperationException)
             {
@@ -101,12 +107,81 @@ private void OnErrorOutput(object sender, DataReceivedEventArgs e)
 
             try
             {
-                // see: https://github.com/xunit/xunit/issues/2146
-                this.output.WriteLine(e.Data.TrimEnd(Environment.NewLine.ToCharArray()));
+                WriteLine(e.Data);
             }
             catch (InvalidOperationException)
             {
             }
         }
+
+        private void WriteLine(string message)
+        {
+            // see: https://github.com/xunit/xunit/issues/2146
+            var formattedMessage = message.TrimEnd(Environment.NewLine.ToCharArray());
+            this.output.WriteLine(formattedMessage);
+            this.logBuffer.Add(formattedMessage);
+        }
+    }
+
+    /// <summary>
+    /// A circular buffer that can be used to store a fixed number of items.
+    /// When the buffer is full, the oldest item is overwritten.
+    /// The buffer can be read in the same order as the items were added.
+    /// More information can be found <see href="https://en.wikipedia.org/wiki/Circular_buffer">here</see>.
+    /// </summary>
+    /// <remarks>
+    /// The buffer gets initialized by the call to the constructor and will allocate,
+    /// the memory for the buffer. The buffer is not resizable.
+    /// That means be carefull with <see cref="size"/>, because it can cause an <see cref="OutOfMemoryException"/>.
+    /// </remarks>
+    /// <typeparam name="T">The type of what the cicular buffer is off.</typeparam>
+    internal class CircularBuffer<T>{
+        private readonly int size;
+        private readonly T[] buffer;
+        private int readPosition = 0;
+        private int writePosition = 0;
+        /// <summary>
+        /// Initialize the buffer with the buffer size of <paramref name="size"/>.
+        /// </summary>
+        /// <param name="size">
+        /// The size the buffer will have
+        /// </param>
+        public CircularBuffer(int size)
+        {
+            this.size = size;
+            buffer = new T[size];
+        }
+        /// <summary>
+        /// Adds an item and move the write position to the next value
+        /// </summary>
+        /// <param name="item">The item that should be written.</param>
+        public void Add(T item)
+        {
+            buffer[writePosition] = item;
+            writePosition = (writePosition + 1) % size;
+        }
+        /// <summary>
+        /// Reads on value and move the position to the next value
+        /// </summary>
+        /// <returns></returns>
+        public T Read(){
+            var value = buffer[readPosition];
+            readPosition = (readPosition + 1) % size;
+            return value;
+        }
+        /// <summary>
+        /// Read the full buffer. 
+        /// While the buffer is read, the read position is moved to the next value
+        /// </summary>
+        /// <returns></returns>
+        public T[] ToArray()
+        {
+            var result = new T[size];
+            for (int i = 0; i < size; i++)
+            {
+                result[i] = Read();
+            }
+            return result;
+        }
     }
 }