Skip to content

Commit 2e0fa80

Browse files
paule96yaron2WhitWaldo
authored
Bug/476 multiple methods per interface with JSON serialization doesn´t work (#1343)
* update devcontainer Signed-off-by: paule96 <[email protected]> * update test setup Signed-off-by: paule96 <[email protected]> * Now the json serialization should work with multiple methods in an interface Signed-off-by: paule96 <[email protected]> * 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 <[email protected]> * fix bugs with the current implementation Signed-off-by: paule96 <[email protected]> * add a test that checks excatly the behavior Signed-off-by: paule96 <[email protected]> * fix devcontainer post creatd command Signed-off-by: paule96 <[email protected]> * change the default to dotnet 8.0 Signed-off-by: paule96 <[email protected]> * I don't know what is different but we commit. Maybe it resolves the need of chmod for it 🤷‍♀️ Signed-off-by: paule96 <[email protected]> * make it easier to see why the application of an E2E test couldn't start Signed-off-by: paule96 <[email protected]> * make the exception in E2E more percise Signed-off-by: paule96 <[email protected]> * fix exception message Signed-off-by: paule96 <[email protected]> --------- Signed-off-by: paule96 <[email protected]> Co-authored-by: Yaron Schneider <[email protected]> Co-authored-by: Whit Waldo <[email protected]>
1 parent 80f0c74 commit 2e0fa80

File tree

10 files changed

+170
-32
lines changed

10 files changed

+170
-32
lines changed

‎.devcontainer/devcontainer.json‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,15 @@
1010
"ghcr.io/devcontainers/features/azure-cli:1": {
1111
"version": "2.38"
1212
},
13-
"ghcr.io/devcontainers/features/docker-from-docker:1": {
14-
"version": "20.10"
13+
"ghcr.io/devcontainers/features/docker-in-docker": {
14+
"version": "latest"
1515
},
16-
"ghcr.io/devcontainers/features/dotnet:1": {
17-
"version": "6.0"
16+
"ghcr.io/devcontainers/features/dotnet": {
17+
"version": "8.0",
18+
"additionalVersions": [
19+
"6.0",
20+
"7.0"
21+
]
1822
},
1923
"ghcr.io/devcontainers/features/github-cli:1": {
2024
"version": "2"
@@ -32,7 +36,8 @@
3236
"ms-dotnettools.csharp",
3337
"ms-dotnettools.vscode-dotnet-runtime",
3438
"ms-azuretools.vscode-dapr",
35-
"GitHub.copilot"
39+
"GitHub.copilot",
40+
"ms-dotnettools.csdevkit"
3641
],
3742
"forwardPorts": [
3843
3000,
@@ -42,10 +47,9 @@
4247
5000,
4348
5007
4449
],
45-
"postCreateCommand": ".devcontainer/localinit.sh",
50+
"postCreateCommand": "chmod +x .devcontainer/localinit.sh && .devcontainer/localinit.sh",
4651
"remoteUser": "vscode",
4752
"hostRequirements": {
4853
"memory": "8gb"
4954
}
50-
}
51-
55+
}

‎.devcontainer/localinit.sh‎

100644100755
File mode changed.

‎src/Dapr.Actors/Communication/ActorMessageBodyJsonSerializationProvider.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,10 @@ public MemoryStreamMessageBodySerializer(
103103
{
104104
var _methodRequestParameterTypes = new List<Type>(methodRequestParameterTypes);
105105
var _wrappedRequestMessageTypes = new List<Type>(wrappedRequestMessageTypes);
106-
106+
if(_wrappedRequestMessageTypes.Count > 1){
107+
throw new NotSupportedException("JSON serialisation should always provide the actor method (or nothing), that was called" +
108+
" to support (de)serialisation. This is a Dapr SDK error, open an issue on GitHub.");
109+
}
107110
this.serializerOptions = new(serializerOptions)
108111
{
109112
// Workaround since WrappedMessageBody creates an object

‎src/Dapr.Actors/Communication/ActorMessageSerializersManager.cs‎

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,14 @@ namespace Dapr.Actors.Communication
1515
{
1616
using System;
1717
using System.Collections.Concurrent;
18+
using System.Collections.Generic;
19+
using System.Diagnostics.CodeAnalysis;
20+
using System.Linq;
1821
using Dapr.Actors.Builder;
1922

2023
internal class ActorMessageSerializersManager
2124
{
22-
private readonly ConcurrentDictionary<int, CacheEntry> cachedBodySerializers;
25+
private readonly ConcurrentDictionary<(int, string), CacheEntry> cachedBodySerializers;
2326
private readonly IActorMessageHeaderSerializer headerSerializer;
2427
private readonly IActorMessageBodySerializationProvider serializationProvider;
2528

@@ -38,7 +41,7 @@ public ActorMessageSerializersManager(
3841
}
3942

4043
this.serializationProvider = serializationProvider;
41-
this.cachedBodySerializers = new ConcurrentDictionary<int, CacheEntry>();
44+
this.cachedBodySerializers = new ConcurrentDictionary<(int, string), CacheEntry>();
4245
this.headerSerializer = headerSerializer;
4346
}
4447

@@ -52,19 +55,19 @@ public IActorMessageHeaderSerializer GetHeaderSerializer()
5255
return this.headerSerializer;
5356
}
5457

55-
public IActorRequestMessageBodySerializer GetRequestMessageBodySerializer(int interfaceId)
58+
public IActorRequestMessageBodySerializer GetRequestMessageBodySerializer(int interfaceId, [AllowNull] string methodName = null)
5659
{
57-
return this.cachedBodySerializers.GetOrAdd(interfaceId, this.CreateSerializers).RequestMessageBodySerializer;
60+
return this.cachedBodySerializers.GetOrAdd((interfaceId, methodName), this.CreateSerializers).RequestMessageBodySerializer;
5861
}
5962

60-
public IActorResponseMessageBodySerializer GetResponseMessageBodySerializer(int interfaceId)
63+
public IActorResponseMessageBodySerializer GetResponseMessageBodySerializer(int interfaceId, [AllowNull] string methodName = null)
6164
{
62-
return this.cachedBodySerializers.GetOrAdd(interfaceId, this.CreateSerializers).ResponseMessageBodySerializer;
65+
return this.cachedBodySerializers.GetOrAdd((interfaceId, methodName), this.CreateSerializers).ResponseMessageBodySerializer;
6366
}
6467

65-
internal CacheEntry CreateSerializers(int interfaceId)
68+
internal CacheEntry CreateSerializers((int interfaceId, string methodName) data)
6669
{
67-
var interfaceDetails = this.GetInterfaceDetails(interfaceId);
70+
var interfaceDetails = this.GetInterfaceDetails(data.interfaceId);
6871

6972
// get the service interface type from the code gen layer
7073
var serviceInterfaceType = interfaceDetails.ServiceInterfaceType;
@@ -74,10 +77,29 @@ internal CacheEntry CreateSerializers(int interfaceId)
7477

7578
// get the known types from the codegen layer
7679
var responseBodyTypes = interfaceDetails.ResponseKnownTypes;
80+
if (data.methodName is null)
81+
{
82+
// Path is mainly used for XML serialization
83+
return new CacheEntry(
84+
this.serializationProvider.CreateRequestMessageBodySerializer(serviceInterfaceType, requestBodyTypes, interfaceDetails.RequestWrappedKnownTypes),
85+
this.serializationProvider.CreateResponseMessageBodySerializer(serviceInterfaceType, responseBodyTypes, interfaceDetails.ResponseWrappedKnownTypes));
86+
}
87+
else
88+
{
89+
// This path should be used for JSON serialization
90+
var requestWrapperTypeAsList = interfaceDetails.RequestWrappedKnownTypes.Where(r => r.Name == $"{data.methodName}ReqBody").ToList();
91+
if(requestWrapperTypeAsList.Count > 1){
92+
throw new NotSupportedException($"More then one wrappertype was found for {data.methodName}");
93+
}
94+
var responseWrapperTypeAsList = interfaceDetails.ResponseWrappedKnownTypes.Where(r => r.Name == $"{data.methodName}RespBody").ToList();
95+
if(responseWrapperTypeAsList.Count > 1){
96+
throw new NotSupportedException($"More then one wrappertype was found for {data.methodName}");
97+
}
98+
return new CacheEntry(
99+
this.serializationProvider.CreateRequestMessageBodySerializer(serviceInterfaceType, requestBodyTypes, requestWrapperTypeAsList),
100+
this.serializationProvider.CreateResponseMessageBodySerializer(serviceInterfaceType, responseBodyTypes, responseWrapperTypeAsList));
101+
}
77102

78-
return new CacheEntry(
79-
this.serializationProvider.CreateRequestMessageBodySerializer(serviceInterfaceType, requestBodyTypes, interfaceDetails.RequestWrappedKnownTypes),
80-
this.serializationProvider.CreateResponseMessageBodySerializer(serviceInterfaceType, responseBodyTypes, interfaceDetails.ResponseWrappedKnownTypes));
81103
}
82104

83105
internal InterfaceDetails GetInterfaceDetails(int interfaceId)

‎src/Dapr.Actors/DaprHttpInteractor.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ public async Task<IActorResponseMessage> InvokeActorMethodWithRemotingAsync(Acto
116116
var serializedHeader = serializersManager.GetHeaderSerializer()
117117
.SerializeRequestHeader(remotingRequestRequestMessage.GetHeader());
118118

119-
var msgBodySeriaizer = serializersManager.GetRequestMessageBodySerializer(interfaceId);
119+
var msgBodySeriaizer = serializersManager.GetRequestMessageBodySerializer(interfaceId, methodName);
120120
var serializedMsgBody = msgBodySeriaizer.Serialize(remotingRequestRequestMessage.GetBody());
121121

122122
// Send Request
@@ -170,7 +170,7 @@ HttpRequestMessage RequestFunc()
170170

171171
// Deserialize Actor Response Message Body
172172
// Deserialize to ActorInvokeException when there is response header otherwise normal path
173-
var responseBodySerializer = serializersManager.GetResponseMessageBodySerializer(interfaceId);
173+
var responseBodySerializer = serializersManager.GetResponseMessageBodySerializer(interfaceId, methodName);
174174

175175
// actorResponseMessageHeader is not null, it means there is remote exception
176176
if (actorResponseMessageHeader != null)

‎src/Dapr.Actors/Runtime/ActorManager.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,8 @@ internal async Task<Tuple<string, byte[]>> DispatchWithRemotingAsync(ActorId act
106106
var interfaceId = actorMessageHeader.InterfaceId;
107107

108108
// Get the deserialized Body.
109-
var msgBodySerializer = this.serializersManager.GetRequestMessageBodySerializer(actorMessageHeader.InterfaceId);
110-
109+
var msgBodySerializer = this.serializersManager.GetRequestMessageBodySerializer(actorMessageHeader.InterfaceId, actorMethodContext.MethodName);
110+
111111
IActorRequestMessageBody actorMessageBody;
112112
using (var stream = new MemoryStream())
113113
{
@@ -130,7 +130,7 @@ async Task<Tuple<string, byte[]>> RequestFunc(Actor actor, CancellationToken ct)
130130
this.messageBodyFactory,
131131
ct);
132132

133-
return this.CreateResponseMessage(responseMsgBody, interfaceId);
133+
return this.CreateResponseMessage(responseMsgBody, interfaceId, actorMethodContext.MethodName);
134134
}
135135

136136
return await this.DispatchInternalAsync(actorId, actorMethodContext, RequestFunc, cancellationToken);
@@ -386,12 +386,12 @@ private async Task<T> DispatchInternalAsync<T>(ActorId actorId, ActorMethodConte
386386
return retval;
387387
}
388388

389-
private Tuple<string, byte[]> CreateResponseMessage(IActorResponseMessageBody msgBody, int interfaceId)
389+
private Tuple<string, byte[]> CreateResponseMessage(IActorResponseMessageBody msgBody, int interfaceId, string methodName)
390390
{
391391
var responseMsgBodyBytes = Array.Empty<byte>();
392392
if (msgBody != null)
393393
{
394-
var responseSerializer = this.serializersManager.GetResponseMessageBodySerializer(interfaceId);
394+
var responseSerializer = this.serializersManager.GetResponseMessageBodySerializer(interfaceId, methodName);
395395
responseMsgBodyBytes = responseSerializer.Serialize(msgBody);
396396
}
397397

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using System.Collections.Generic;
23
using System.Text.Json;
34
using System.Text.Json.Serialization;
@@ -10,6 +11,7 @@ namespace Dapr.E2E.Test.Actors
1011
public interface ISerializationActor : IActor, IPingActor
1112
{
1213
Task<SerializationPayload> SendAsync(string name, SerializationPayload payload, CancellationToken cancellationToken = default);
14+
Task<DateTime> AnotherMethod(DateTime payload);
1315
}
1416

1517
public record SerializationPayload(string Message)

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11

2+
using System;
23
using System.Threading;
34
using System.Threading.Tasks;
45
using Dapr.Actors.Runtime;
@@ -22,5 +23,9 @@ public Task<SerializationPayload> SendAsync(string name,
2223
{
2324
return Task.FromResult(payload);
2425
}
26+
27+
public Task<DateTime> AnotherMethod(DateTime payload){
28+
return Task.FromResult(payload);
29+
}
2530
}
2631
}

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,5 +84,32 @@ public async Task ActorCanSupportCustomSerializer()
8484
Assert.Equal(JsonSerializer.Serialize(kvp.Value), JsonSerializer.Serialize(value));
8585
}
8686
}
87+
88+
/// <summary>
89+
/// This was actually a problem that is why the test exists.
90+
/// It just checks, if the interface of the actor has more than one method defined,
91+
/// that if can call it and serialize the payload correctly.
92+
/// </summary>
93+
/// <remarks>
94+
/// More than one methods means here, that in the exact interface must be two methods defined.
95+
/// That excludes hirachies.
96+
/// So <see cref="IPingActor.Ping"/> wouldn't count here, because it's not directly defined in
97+
/// <see cref="ISerializationActor"/>. (it's defined in the base of it.)
98+
/// That why <see cref="ISerializationActor.AnotherMethod(DateTime)"/> was created,
99+
/// so there are now more then one method.
100+
/// </remark>
101+
[Fact]
102+
public async Task ActorCanSupportCustomSerializerAndCallMoreThenOneDefinedMethod()
103+
{
104+
using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(60));
105+
var proxy = this.ProxyFactory.CreateActorProxy<ISerializationActor>(ActorId.CreateRandom(), "SerializationActor");
106+
107+
await ActorRuntimeChecker.WaitForActorRuntimeAsync(this.AppId, this.Output, proxy, cts.Token);
108+
109+
var payload = DateTime.MinValue;
110+
var result = await proxy.AnotherMethod(payload);
111+
112+
Assert.Equal(payload, result);
113+
}
87114
}
88115
}

‎test/Dapr.E2E.Test/DaprCommand.cs‎

Lines changed: 80 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@ namespace Dapr.E2E.Test
1616
using System;
1717
using System.Collections.Generic;
1818
using System.Diagnostics;
19+
using System.Drawing;
1920
using System.Linq;
2021
using System.Threading;
2122
using Xunit.Abstractions;
2223

2324
public class DaprCommand
2425
{
2526
private readonly ITestOutputHelper output;
27+
private readonly CircularBuffer<string> logBuffer = new CircularBuffer<string>(1000);
2628

2729
public DaprCommand(ITestOutputHelper output)
2830
{
@@ -66,7 +68,12 @@ public void Run()
6668
var done = outputReceived.WaitOne(this.Timeout);
6769
if (!done)
6870
{
69-
throw new Exception($"Command: \"{this.Command}\" timed out while waiting for output: \"{this.OutputToMatch}\"");
71+
var ex = new Exception($"Command: \"{this.Command}\" timed out while waiting for output: \"{this.OutputToMatch}\"{System.Environment.NewLine}" +
72+
"This could also mean the E2E app had a startup error. For more details see the Data property of this exception.");
73+
// we add here the log buffer of the last 1000 lines, of the application log
74+
// to make it easier to debug failing tests
75+
ex.Data.Add("log", this.logBuffer.ToArray());
76+
throw ex;
7077
}
7178
}
7279

@@ -79,8 +86,7 @@ private void CheckOutput(object sendingProcess, DataReceivedEventArgs e)
7986

8087
try
8188
{
82-
// see: https://github.com/xunit/xunit/issues/2146
83-
this.output.WriteLine(e.Data.TrimEnd(Environment.NewLine.ToCharArray()));
89+
WriteLine(e.Data);
8490
}
8591
catch (InvalidOperationException)
8692
{
@@ -101,12 +107,81 @@ private void OnErrorOutput(object sender, DataReceivedEventArgs e)
101107

102108
try
103109
{
104-
// see: https://github.com/xunit/xunit/issues/2146
105-
this.output.WriteLine(e.Data.TrimEnd(Environment.NewLine.ToCharArray()));
110+
WriteLine(e.Data);
106111
}
107112
catch (InvalidOperationException)
108113
{
109114
}
110115
}
116+
117+
private void WriteLine(string message)
118+
{
119+
// see: https://github.com/xunit/xunit/issues/2146
120+
var formattedMessage = message.TrimEnd(Environment.NewLine.ToCharArray());
121+
this.output.WriteLine(formattedMessage);
122+
this.logBuffer.Add(formattedMessage);
123+
}
124+
}
125+
126+
/// <summary>
127+
/// A circular buffer that can be used to store a fixed number of items.
128+
/// When the buffer is full, the oldest item is overwritten.
129+
/// The buffer can be read in the same order as the items were added.
130+
/// More information can be found <see href="https://en.wikipedia.org/wiki/Circular_buffer">here</see>.
131+
/// </summary>
132+
/// <remarks>
133+
/// The buffer gets initialized by the call to the constructor and will allocate,
134+
/// the memory for the buffer. The buffer is not resizable.
135+
/// That means be carefull with <see cref="size"/>, because it can cause an <see cref="OutOfMemoryException"/>.
136+
/// </remarks>
137+
/// <typeparam name="T">The type of what the cicular buffer is off.</typeparam>
138+
internal class CircularBuffer<T>{
139+
private readonly int size;
140+
private readonly T[] buffer;
141+
private int readPosition = 0;
142+
private int writePosition = 0;
143+
/// <summary>
144+
/// Initialize the buffer with the buffer size of <paramref name="size"/>.
145+
/// </summary>
146+
/// <param name="size">
147+
/// The size the buffer will have
148+
/// </param>
149+
public CircularBuffer(int size)
150+
{
151+
this.size = size;
152+
buffer = new T[size];
153+
}
154+
/// <summary>
155+
/// Adds an item and move the write position to the next value
156+
/// </summary>
157+
/// <param name="item">The item that should be written.</param>
158+
public void Add(T item)
159+
{
160+
buffer[writePosition] = item;
161+
writePosition = (writePosition + 1) % size;
162+
}
163+
/// <summary>
164+
/// Reads on value and move the position to the next value
165+
/// </summary>
166+
/// <returns></returns>
167+
public T Read(){
168+
var value = buffer[readPosition];
169+
readPosition = (readPosition + 1) % size;
170+
return value;
171+
}
172+
/// <summary>
173+
/// Read the full buffer.
174+
/// While the buffer is read, the read position is moved to the next value
175+
/// </summary>
176+
/// <returns></returns>
177+
public T[] ToArray()
178+
{
179+
var result = new T[size];
180+
for (int i = 0; i < size; i++)
181+
{
182+
result[i] = Read();
183+
}
184+
return result;
185+
}
111186
}
112187
}

0 commit comments

Comments
 (0)