Actor example method invocation doesn't respect method signature

[m3nax]

Expected Behavior

The SaveData method called by cli without remoting (as written in readme) correctly invokes the actor

Actual Behavior

As written in the readme of Actor example to call SaveData method from bash we need to use the following command:

curl -X POST http://127.0.0.1:3500/v1.0/actors/DemoActor/abc/method/SaveData -d '{ "PropertyA": "ValueA", "PropertyB": "ValueB" }'

but SaveData method have 2 parameters data and ttl

Task SaveData(MyData data, TimeSpan ttl);

Steps to Reproduce the Problem

First try, following the readme, failing for missing ttl parameter

# Run example app
dapr run --dapr-http-port 3500 --app-id demo_actor --app-port 5010 dotnet run

# Execute
curl -X POST http://127.0.0.1:3500/v1.0/actors/DemoActor/abc/method/SaveData -d '{ "PropertyA": "ValueA", "PropertyB": "ValueB" }'

Second try, with fixed payload, failing with exception:
System.ArgumentException: Method IDemoActor.SaveData has more than one parameter and can't be invoked through http

# Run example app
dapr run --dapr-http-port 3500 --app-id demo_actor --app-port 5010 dotnet run

# Execute
curl -X POST http://127.0.0.1:3500/v1.0/actors/DemoActor/abc/method/SaveData -d '{ "data": {"PropertyA": "ValueA", "PropertyB": "ValueB" }, "ttl": "00:10:00" }' 

Release Note

RELEASE NOTE:

[m3nax]

/assign

[philliphoff]

@m3nax The original intent was that non-remoted (plain HTTP) actor methods accept only a single argument (ignoring CancellationToken); it was the remoting infrastructure that allowed for multiple arguments. In this case, it would seem to be an oversight to have updated the sample and README.md to match that limitation when the TTL feature was added. (Probably a new type, MyDataWithTTL, should have been created with those two values as properties and passed to SetData() instead.)

Allowing for multiple arguments in non-remoted method invocations is interesting, but I wonder how generally useful it would be (given we already have a non-standard mechanism to do that--remoting--and this new mechanism would be inconsistent with other platform SDKs as well). Even if it's something worth pursuing, I think it would require some more upfront design. For example, JSON properties and .NET arguments cannot always be mapped 1:1, which likely means adding a mechanism to customize the mappings or, at least, clearly documenting its limitations.

My personal opinion is that we should move away from the remoting-based actor method invocation--hence the addition of the new generated actor proxies. One-request-object and one-response-object is simpler and more consistent with other platform SDKs as well as ASP.NET's request body handlers, and multiple arguments do not provide a significantly better experience.

[m3nax]

I think with the source generator we can work around the problem of having to create classes like MyDataWithTTL by hand just having them generated based on the signature of the various actor methods.

[philliphoff]

I think with the source generator we can work around the problem of having to create classes like MyDataWithTTL by hand just having them generated based on the signature of the various actor methods.

@m3nax We could do that, but I think the question is whether we should do that, and I'm personally not convinced that it provides a significant benefit over the remoting-based protocol which already has that capability. I'd rather not create a new form of remoting used only by .NET, or have to deal with the potential complexities of mapping JSON content to method arguments.

[m3nax]

I think we should remain as similar as possible to other sdk implementations to simplify possible communication between actors implemented with different programming languages.

One-request-object and one-response-object

The only problem that gives me thinking is the amount of objects that you need to create in order to comply with the one-request-object -> one-response-object paradigm, for example, in the actor example, for a single method I had to add an object (MyDataWithTTL).

[philliphoff]

I think we should remain as similar as possible to other sdk implementations to simplify possible communication between actors implemented with different programming languages.

@m3nax AFIAK, the other SDKs operate in a similar manner, that is, accepting just a single argument and returning a single result. (Let me know if my understanding is inaccurate.)

The only problem that gives me thinking is the amount of objects that you need to create in order to comply with the one-request-object -> one-response-object paradigm, for example, in the actor example, for a single method I had to add an object (MyDataWithTTL).

@m3nax Fair, though I don't think the overhead is onerous, particularly in .NET, and overall is on par with other RPC mechanisms such as gRPC. If one really wanted that functionality, a source generator could be used to pack/unpack the arguments, but I would consider that better kept outside the responsibility of the SDK itself. (And, again, that's basically what the remoting flavor of .NET actors already does.)