Skip to content

Support .net 6 preview 4 new Minimal APIs #695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
doddgu opened this issue Jun 8, 2021 · 16 comments
Closed

Support .net 6 preview 4 new Minimal APIs #695

doddgu opened this issue Jun 8, 2021 · 16 comments
Assignees
Labels
kind/bug Something isn't working
Milestone

Comments

@doddgu
Copy link
Contributor

doddgu commented Jun 8, 2021

Hi, is there anyone working on Minimal APIs support?

@doddgu
Copy link
Contributor Author

doddgu commented Jun 8, 2021

Maybe adding extensions for IEndpointRouteBuilder will work?

@doddgu
Copy link
Contributor Author

doddgu commented Jun 8, 2021

in DaprEndpointRouteBuilderExtensions.cs
I added an extension method. It's looks like worked, but I only test invocation.

        /// <summary>
        /// Adds Dapr integration for Minimal APIs to the provided <see cref="IEndpointRouteBuilder" />.
        /// </summary>
        /// <param name="builder">The <see cref="IEndpointRouteBuilder" />.</param>
        /// <param name="services">The <see cref="IServiceCollection" />.</param>
        /// <param name="configureClient">The (optional) <see cref="DaprClientBuilder" /> to use for configuring the DaprClient.</param>
        /// <returns>The <see cref="IEndpointRouteBuilder" /> builder.</returns>
        public static IEndpointRouteBuilder AddDapr(this IEndpointRouteBuilder builder, IServiceCollection services, Action<DaprClientBuilder> configureClient = null)
        {
            if (builder is null)
            {
                throw new ArgumentNullException(nameof(builder));
            }

            // This pattern prevents registering services multiple times in the case AddDapr is called
            // by non-user-code.
            if (services.Any(s => s.ImplementationType == typeof(DaprMinimalApisMarkerService)))
            {
                return builder;
            }

            services.AddDaprClient(configureClient);

            services.AddSingleton<DaprMinimalApisMarkerService>();
            services.AddSingleton<IApplicationModelProvider, StateEntryApplicationModelProvider>();
            services.Configure<MvcOptions>(options =>
            {
                options.ModelBinderProviders.Insert(0, new StateEntryModelBinderProvider());
            });

            return builder;
        }

@doddgu
Copy link
Contributor Author

doddgu commented Jun 30, 2021

I found another exception, result in the actor function does not work.
see #dotnet/aspnetcore#33964

And MapActorsHandlers function throw exception too.
Because endpoints.ServiceProvider.GetService() return null. I change it to builder.Services.BuildServiceProvider().GetService() , it's working, but health check broken down.

ActorsEndpointRouteBuilderExtensions.cs

// ------------------------------------------------------------
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
// ------------------------------------------------------------

namespace Microsoft.AspNetCore.Builder
{
    using System;
    using Dapr.Actors;
    using Dapr.Actors.Runtime;
    using Microsoft.AspNetCore.Http;
    using Microsoft.AspNetCore.Routing;
    using Microsoft.Extensions.DependencyInjection;

    /// <summary>
    /// Contains extension methods for using Dapr Actors with endpoint routing.
    /// </summary>
    public static class ActorsEndpointRouteBuilderExtensions
    {
        /// <summary>
        /// Maps endpoints for Dapr Actors into the <see cref="IEndpointRouteBuilder" />.
        /// </summary>
        /// <param name="endpoints">The <see cref="IEndpointRouteBuilder" />.</param>
        /// <returns>
        /// An <see cref="IEndpointConventionBuilder" /> that can be used to configure the endpoints.
        /// </returns>
        public static IEndpointConventionBuilder MapActorsHandlers(this IEndpointRouteBuilder endpoints)
        {
            if (endpoints.ServiceProvider.GetService<ActorRuntime>() is null)
            {
                throw new InvalidOperationException(
                    "The ActorRuntime service is not registered with the dependency injection container. " +
                    "Call AddActors() inside ConfigureServices() to register the actor runtime and actor types.");
            }

            var builders = new[]
            {
                MapDaprConfigEndpoint(endpoints,endpoints.ServiceProvider),
                MapActorDeactivationEndpoint(endpoints,endpoints.ServiceProvider),
                MapActorMethodEndpoint(endpoints,endpoints.ServiceProvider),
                MapReminderEndpoint(endpoints,endpoints.ServiceProvider),
                MapTimerEndpoint(endpoints,endpoints.ServiceProvider),
                MapActorHealthChecks(endpoints),
            };

            return new CompositeEndpointConventionBuilder(builders);
        }


        /// <summary>
        /// Maps endpoints for Dapr Actors into the <see cref="IEndpointRouteBuilder" />.
        /// </summary>
        /// <param name="endpoints">The <see cref="IEndpointRouteBuilder" />.</param>
        /// <param name="serviceProvider">The <see cref="IServiceProvider" />.</param>
        /// <returns>
        /// An <see cref="IEndpointConventionBuilder" /> that can be used to configure the endpoints.
        /// </returns>
        public static IEndpointConventionBuilder MapActorsHandlers(this IEndpointRouteBuilder endpoints, IServiceProvider serviceProvider)
        {
            if (serviceProvider.GetService<ActorRuntime>() is null)
            {
                throw new InvalidOperationException(
                    "The ActorRuntime service is not registered with the dependency injection container. " +
                    "Call AddActors() inside ConfigureServices() to register the actor runtime and actor types.");
            }

            var builders = new[]
            {
                MapDaprConfigEndpoint(endpoints, serviceProvider),
                MapActorDeactivationEndpoint(endpoints, serviceProvider),
                MapActorMethodEndpoint(endpoints, serviceProvider),
                MapReminderEndpoint(endpoints, serviceProvider),
                MapTimerEndpoint(endpoints, serviceProvider),
                MapActorHealthChecks(endpoints),
            };

            return new CompositeEndpointConventionBuilder(builders);
        }

        private static IEndpointConventionBuilder MapDaprConfigEndpoint(this IEndpointRouteBuilder endpoints, IServiceProvider serviceProvider)
        {
            var runtime = serviceProvider.GetRequiredService<ActorRuntime>();
            return endpoints.MapGet("dapr/config", async context =>
            {
                context.Response.ContentType = "application/json";
                await runtime.SerializeSettingsAndRegisteredTypes(context.Response.BodyWriter);
            }).WithDisplayName(b => "Dapr Actors Config");
        }

        private static IEndpointConventionBuilder MapActorDeactivationEndpoint(this IEndpointRouteBuilder endpoints, IServiceProvider serviceProvider)
        {
            var runtime = serviceProvider.GetRequiredService<ActorRuntime>();
            return endpoints.MapDelete("actors/{actorTypeName}/{actorId}", async context =>
            {
                var routeValues = context.Request.RouteValues;
                var actorTypeName = (string)routeValues["actorTypeName"];
                var actorId = (string)routeValues["actorId"];
                await runtime.DeactivateAsync(actorTypeName, actorId);
            }).WithDisplayName(b => "Dapr Actors Deactivation");
        }

        private static IEndpointConventionBuilder MapActorMethodEndpoint(this IEndpointRouteBuilder endpoints, IServiceProvider serviceProvider)
        {
            var runtime = serviceProvider.GetRequiredService<ActorRuntime>();
            return endpoints.MapPut("actors/{actorTypeName}/{actorId}/method/{methodName}", async context =>
            {
                var routeValues = context.Request.RouteValues;
                var actorTypeName = (string)routeValues["actorTypeName"];
                var actorId = (string)routeValues["actorId"];
                var methodName = (string)routeValues["methodName"];

                // If Header is present, call is made using Remoting, use Remoting dispatcher.
                if (context.Request.Headers.ContainsKey(Constants.RequestHeaderName))
                {
                    var daprActorheader = context.Request.Headers[Constants.RequestHeaderName];
                    var (header, body) = await runtime.DispatchWithRemotingAsync(actorTypeName, actorId, methodName, daprActorheader, context.Request.Body);

                    // Item 1 is header , Item 2 is body
                    if (header != string.Empty)
                    {
                        // exception case
                        context.Response.Headers.Add(Constants.ErrorResponseHeaderName, header); // add error header
                    }

                    await context.Response.Body.WriteAsync(body, 0, body.Length); // add response message body
                }
                else
                {
                    await runtime.DispatchWithoutRemotingAsync(actorTypeName, actorId, methodName, context.Request.Body, context.Response.Body);
                }
            }).WithDisplayName(b => "Dapr Actors Invoke");
        }

        private static IEndpointConventionBuilder MapReminderEndpoint(this IEndpointRouteBuilder endpoints, IServiceProvider serviceProvider)
        {
            var runtime = serviceProvider.GetRequiredService<ActorRuntime>();
            return endpoints.MapPut("actors/{actorTypeName}/{actorId}/method/remind/{reminderName}", async context =>
            {
                var routeValues = context.Request.RouteValues;
                var actorTypeName = (string)routeValues["actorTypeName"];
                var actorId = (string)routeValues["actorId"];
                var reminderName = (string)routeValues["reminderName"];

                // read dueTime, period and data from Request Body.
                await runtime.FireReminderAsync(actorTypeName, actorId, reminderName, context.Request.Body);
            }).WithDisplayName(b => "Dapr Actors Reminder");
        }

        private static IEndpointConventionBuilder MapTimerEndpoint(this IEndpointRouteBuilder endpoints, IServiceProvider serviceProvider)
        {
            var runtime = serviceProvider.GetRequiredService<ActorRuntime>();
            return endpoints.MapPut("actors/{actorTypeName}/{actorId}/method/timer/{timerName}", async context =>
            {
                var routeValues = context.Request.RouteValues;
                var actorTypeName = (string)routeValues["actorTypeName"];
                var actorId = (string)routeValues["actorId"];
                var timerName = (string)routeValues["timerName"];

                // read dueTime, period and data from Request Body.
                await runtime.FireTimerAsync(actorTypeName, actorId, timerName, context.Request.Body);
            }).WithDisplayName(b => "Dapr Actors Timer");
        }


        private static IEndpointConventionBuilder MapActorHealthChecks(this IEndpointRouteBuilder endpoints)
        {
            var builder = endpoints.MapHealthChecks("/healthz");
            builder.Add(b =>
            {
                // Sets the route order so that this is matched with lower priority than an endpoint
                // configured by default.
                //
                // This is necessary because it allows a user defined `/healthz` endpoint to win in the
                // most common cases, but still fulfills Dapr's contract when the user doesn't have
                // a health check at `/healthz`.
                ((RouteEndpointBuilder)b).Order = 100;
            });
            return builder.WithDisplayName(b => "Dapr Actors Health Check");
        }

        private class CompositeEndpointConventionBuilder : IEndpointConventionBuilder
        {
            private readonly IEndpointConventionBuilder[] inner;

            public CompositeEndpointConventionBuilder(IEndpointConventionBuilder[] inner)
            {
                this.inner = inner;
            }

            public void Add(Action<EndpointBuilder> convention)
            {
                for (var i = 0; i < inner.Length; i++)
                {
                    inner[i].Add(convention);
                }
            }
        }
    }
}

@davidfowl
Copy link

Explicit support shouldn't need to be added for minimal APIs. There are a few things decoupled from MVC won't be supported:

  • There's no support for custom model binders (so the FromState attribute won't work). StateEntry<T> won't work either.
  • AddDapr currently hangs off IMvcBuilder, but it does some other non-MVC specific work (like adding the DaprClient). This means to use it with minimal APIs you might have to call AddDaprClient explicitly. This doesn't seem like a big deal.

You'll want to get @rynowak to guide here as he's well versed in this part of the stack. My current take is that not much needs to be done.

@doddgu
Copy link
Contributor Author

doddgu commented Jul 3, 2021

Explicit support shouldn't need to be added for minimal APIs. There are a few things decoupled from MVC won't be supported:

  • There's no support for custom model binders (so the FromState attribute won't work). StateEntry won't work either.
  • AddDapr currently hangs off IMvcBuilder, but it does some other non-MVC specific work (like adding the DaprClient). This means to use it with minimal APIs you might have to call AddDaprClient explicitly. This doesn't seem like a big deal.

You'll want to get @rynowak to guide here as he's well versed in this part of the stack. My current take is that not much needs to be done.

I think this can be a feature. Because support ModelBinderProvider was moved to feature at aspnetcore.
dotnet/aspnetcore#33955

If you decide not to support it, can you make a guide for Minimal APIs working. I don't have to use FromState, but the actor is not working( #705 ).

@davidfowl
Copy link

I don’t know what doesn’t work beyond the StateEntry and the FromState attributes

@rynowak
Copy link
Contributor

rynowak commented Aug 17, 2021

Hi @doddgu - sorry I'm showing up late here...

Can you explain what the things are that are not working or missing? I don't expect Minimal APIs support to require work from the Dapr SDK.

For example it is expected that [FromState] does not work. [FromState] is an MVC feature and MVC is not part of minimal APIs. What else?

@rynowak rynowak added this to the Investigations milestone Aug 17, 2021
@doddgu
Copy link
Contributor Author

doddgu commented Aug 18, 2021

#713 and #711 can fix actor not working on Minimal APIs, but that is preview 4 and 5. I will try it again on .net 6 preview 7 without #713 .

@doddgu
Copy link
Contributor Author

doddgu commented Aug 18, 2021

Hi @rynowak
I can't try it, because #dotnet/aspnetcore#35285

@doddgu
Copy link
Contributor Author

doddgu commented Aug 18, 2021

I found /dapr/config response is empty.

@doddgu
Copy link
Contributor Author

doddgu commented Aug 18, 2021

In the method Dapr.Actors.ActorRuntime.SerializeSettingsAndRegisteredTypes, it needs to build json into response, but Flush is not work.

@doddgu
Copy link
Contributor Author

doddgu commented Aug 18, 2021

I use MemoryStream, I get the json.

@doddgu
Copy link
Contributor Author

doddgu commented Aug 18, 2021

Hi @rynowak
I solved the problem, because context.Response.BodyWriter need to call FlushAsync, I don't know why, but add this ,it works.

private static IEndpointConventionBuilder MapDaprConfigEndpoint(this IEndpointRouteBuilder endpoints, IServiceProvider serviceProvider)
        {
            var runtime = serviceProvider.GetRequiredService<ActorRuntime>();
            return endpoints.MapGet("dapr/config", async context =>
            {
                context.Response.ContentType = "application/json";
                await runtime.SerializeSettingsAndRegisteredTypes(context.Response.BodyWriter);
                await context.Response.BodyWriter.FlushAsync(); // this is my add
            }).WithDisplayName(b => "Dapr Actors Config");
        }

@doddgu doddgu mentioned this issue Aug 18, 2021
3 tasks
@doddgu
Copy link
Contributor Author

doddgu commented Aug 18, 2021

I started a pr, please review.

@davidfowl
Copy link

Yes this looks like legitimate bug.

@rynowak rynowak modified the milestones: Investigations, v1.5 Sep 16, 2021
@rynowak rynowak added the kind/bug Something isn't working label Sep 16, 2021
@rynowak
Copy link
Contributor

rynowak commented Sep 16, 2021

Thanks @doddgu

@rynowak rynowak closed this as completed Sep 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
3 participants