Added .NET client for Dapr Jobs API

[WhitWaldo]

Description

Added a Dapr Jobs client for the new Jobs API.

This PR is intended to replace the previous one and was performed as a squash merge of it - the third commit (early on) was missing a period in my email address which apparently breaks the DCO process (even though I have an email alias and I can still receive email there). Because I cannot figure out how to fix multi-line comments, I'm providing this updated PR instead.

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #1321

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

• Code compiles correctly
• Created/updated tests
• Extended the documentation
• Added sample project

[WhitWaldo]

@philliphoff Any other changes you think should be made here?

[WhitWaldo]

I'm seeing the E2E tests fail on my system following the recent merge with master. In hopes those are due to something wrong with those tests, I'll hold off on any other changes to this PR to address them until it's confirmed it's a problem with this PR and not them.

I did add two more unit tests to validate the DaprClientBuilder a bit more where it was lacking existing tests and those are passing.

[yaron2]

cc @halspang @philliphoff

[philliphoff]

I feel like this tuple will become unwieldy if/when the number of dependencies/options increase. I wonder if something like this would be more flexible:

public TClientBuilder Build()
{
  DaprClientDependencies dependencies = // Generate dependencies;

  return this.Build(dependencies);
}

protected virtual TClientBuilder Build(DaprClientDependencies dependencies);

protected sealed record DaprClientDependencies(GrpcChannel Channel, HttpClient HttpClient, Uri HttpEndpoint);

[WhitWaldo]

I pulled this out primarily to remove duplicate code when spinning up clients in other packages ensuring that they're all using the same configuration data for endpoints, API keys and whatnot.

However, as was evidenced in the subscriptions implementation, this approach doesn't really lend itself to any use of injected functionality down the line (e.g. logging, compression handlers, serialization handlers, etc) provided by the developer at registration time. Do you have any thoughts on abandoning this approach altogether and instead opting for more of a DI-first, factory-style approach?

I'd be happy to put together a POC in a separate PR if you're more inclined to visit that elsewhere.

[WhitWaldo]

Given the similarity of the downloads between Dapr.Client and Dapr.AspNetCore, I'm curious if you know if there have been any surveys done on just how many people use the DaprClient without relying on dependency injection?

[philliphoff]

I don't know of any, though my personal expectation is that more people are using the AspNetCore provided DI method than building one manually.

[codecov]

Codecov Report

Attention: Patch coverage is 77.77778% with 84 lines in your changes missing coverage. Please review.

Project coverage is 68.01%. Comparing base (1b7c9f4) to head (d6e88d2).
Report is 19 commits behind head on master.

Files with missing lines	Patch %	Lines
src/Dapr.Jobs/DaprJobsGrpcClient.cs	33.33%	45 Missing and 7 partials ⚠️
...apr.Jobs/Extensions/DaprSerializationExtensions.cs	0.00%	11 Missing ⚠️
src/Dapr.Common/DaprDefaults.cs	66.66%	4 Missing and 6 partials ⚠️
src/Dapr.Jobs/DaprJobsClient.cs	37.50%	5 Missing ⚠️
....Jobs/Extensions/EndpointRouteBuilderExtensions.cs	0.00%	4 Missing ⚠️
.../Extensions/DaprJobsServiceCollectionExtensions.cs	92.85%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1331      +/-   ##
==========================================
+ Coverage   67.28%   68.01%   +0.72%     
==========================================
  Files         174      187      +13     
  Lines        6025     6352     +327     
  Branches      671      722      +51     
==========================================
+ Hits         4054     4320     +266     
- Misses       1802     1853      +51     
- Partials      169      179      +10     

Flag	Coverage Δ
net6	67.98% <77.77%> (+0.72%)	⬆️
net7	67.98% <77.77%> (+0.72%)	⬆️
net8	67.99% <77.77%> (+0.72%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[WhitWaldo]

@philliphoff I changed how the inbound job triggers are handled in the latest commit. Where it handled before more like how topics are subscribed to in the pub/sub context, after responding to an inquiry in Discord, it occurred to me that should someone schedule some jobs during runtime, this approach would require redeploying the app.

Instead, I've changed it to operate more like the dynamic subscription in that it maps to all POST requests to "/job/{jobName}". It attempts to deserialize the inbound request body to a DaprJobDetails and then passes both the job name and job details (or their null values) through to either of the async delegates (one provides an IServiceProvider in case someone needs to access some injected services as part of processing the data).

I've also updated the unit tests and example project to validate the new approach.

While updating the unit tests, I realized that I didn't have a really solid approach to dealing with the serialization and deserialization of the DaprJobDetails and DaprJobSchedule, so I've added some JsonConverters and an internal type so that the developer is still presented with a straightforward way of interacting with the SDK while ensuring that those complex types. This makes the payload itself smaller and simpler when it's serialized, so win-win.

[philliphoff]

A thought: there may be other things we'll want to pass to such handlers (e.g. a CancellationToken). It might be best to consolidate the individual arguments into a DaprJobHandlerContext object and just pass that one thing. In that way, additional things can be added in a way that won't break existing code and won't require additional overloads of the MapDaprJob() method.

It would be cool to be able to reuse some of ASP.NET's minimal API parameter binding logic so that users can just write their handlers in the same way as they normally would (while just injecting the additional job details), but I don't think that's exposed from the infrastructure.

[WhitWaldo]

@philliphoff

I've been trying to think of some way to make this more flexible and while reading through a little bit of how ASP.NET Core does it in their minimal APIs, I've come up with a similar approach and while it's not as fancy as theirs, it also doesn't need to do route binding. Ideally, we can add an analyzer for this or something at some point, but the developer can pass a delegate of their own into the extension and the first two parameters must be a string? for the job name and a DaprJobDetails? for the job details. Anything after that is dynamically injected for them like the minimal APIs do and the cancellationToken is passed in as the last parameter.

This approach gets rid of the two delegate handlers I defined and means the developer doesn't have to deal with an IServiceProvider, but gets their CancellationToken and we don't have to deal with maintaining a context object.

I want to write several unit tests to validate this out and make sure it works, but I updated the example to show off the new approach.