toddams / RazorLight

Template engine based on Microsoft's Razor parsing engine for .NET Core
Apache License 2.0
1.52k stars 259 forks source link

Does not work with generic host #317

Closed twenzel closed 3 years ago

twenzel commented 4 years ago

Describe the bug InvalidOperationException: Unable to resolve service for type 'RazorLight.RazorLightOptions' while attempting to activate 'RazorLight.EngineHandler'.

To Reproduce Steps to reproduce the behavior: Create a new ASP.NET Core 3.1 MVC application with the new generic host template

 public class Program
    {
        public static void Main(string[] args)
        {
            CreateHostBuilder(args).Build().Run();
        }

        public static IHostBuilder CreateHostBuilder(string[] args) =>
            Host.CreateDefaultBuilder(args)
                .ConfigureWebHostDefaults(webBuilder =>
                {
                    webBuilder.UseStartup<Startup>();
                });
    }

Add RazorLight to the services:

  public void ConfigureServices(IServiceCollection services)
        {
            services.AddControllersWithViews();
            services.AddRazorLight(() =>
  new RazorLightEngineBuilder()
                   .UseEmbeddedResourcesProject(typeof(Startup)) // exception without this (or another project type)
                   .UseMemoryCachingProvider()
                   .Build()
            );
        }

Exception: InvalidOperationException: Unable to resolve service for type 'RazorLight.RazorLightOptions' while attempting to activate 'RazorLight.EngineHandler'.

Expected behavior Runs smoothly.

The problem is that the RazorLightOptions are not registered which are required by the EngineHandler.

Workaround Register missing dependencies separately

public void ConfigureServices(IServiceCollection services)
        {
 var engine = new RazorLightEngineBuilder()
                   .UseEmbeddedResourcesProject(typeof(Startup)) // exception without this (or another project type)
                   .UseMemoryCachingProvider()
                   .Build();

            services.AddRazorLight(() => engine);
            services.AddSingleton(engine.Options);
            services.AddSingleton(engine.Handler.Compiler);
            services.AddSingleton(engine.Handler.FactoryProvider);
            services.AddSingleton(engine.Handler.Cache);
}

Information (please complete the following information):

jzabroski commented 4 years ago

@twenzel There is not enough information here to reproduce the issue you're explaining. There is a ServiceCollectionExtensionsTest which covers using RazorLight with Microsoft DI. I'm not clear what code you wrote to get an InvalidOperationException - you gave me all the code except for the call site that triggers this exception. I'm also not sure why you're registering all these things as singletons after you call Build. The whole point of calling UseMemoryCachingProvider is so that it sets up the cache for when you call Build.

georgiosd commented 4 years ago

@jzabroski this is happening for me also on aspnet core 3.1, you shouldn't really need much to reproduce it:

services.AddRazorLight(() => new RazorLightEngineBuilder()
                .UseFileSystemProject($"{environment.ContentRootPath}/Templates")
                .UseMemoryCachingProvider()
                .Build());

Trace:

Unhandled exception. System.AggregateException: Some services are not able to be constructed (Error while validating the service descriptor 'ServiceType: RazorLight.IEngineHandler Lifetime: Singleton ImplementationType: RazorLight.EngineHandler': Unable to resolve service for type 'RazorLight.RazorLightOptions' while attempting to activate 'RazorLight.EngineHandler'.)
 ---> System.InvalidOperationException: Error while validating the service descriptor 'ServiceType: RazorLight.IEngineHandler Lifetime: Singleton ImplementationType: RazorLight.EngineHandler': Unable to resolve service for type 'RazorLight.RazorLightOptions' while attempting to activate 'RazorLight.EngineHandler'.
 ---> System.InvalidOperationException: Unable to resolve service for type 'RazorLight.RazorLightOptions' while attempting to activate 'RazorLight.EngineHandler'.
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteFactory.CreateArgumentCallSites(Type serviceType, Type implementationType, CallSiteChain callSiteChain, ParameterInfo[] parameters, Boolean throwIfCallSiteNotFound)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteFactory.CreateConstructorCallSite(ResultCache lifetime, Type serviceType, Type implementationType, CallSiteChain callSiteChain)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteFactory.TryCreateExact(ServiceDescriptor descriptor, Type serviceType, CallSiteChain callSiteChain, Int32 slot)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteFactory.GetCallSite(ServiceDescriptor serviceDescriptor, CallSiteChain callSiteChain)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngine.ValidateService(ServiceDescriptor descriptor)
   --- End of inner exception stack trace ---
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngine.ValidateService(ServiceDescriptor descriptor)
   at Microsoft.Extensions.DependencyInjection.ServiceProvider..ctor(IEnumerable`1 serviceDescriptors, ServiceProviderOptions options)
   --- End of inner exception stack trace ---
   at Microsoft.Extensions.DependencyInjection.ServiceProvider..ctor(IEnumerable`1 serviceDescriptors, ServiceProviderOptions options)
   at Microsoft.Extensions.DependencyInjection.ServiceCollectionContainerBuilderExtensions.BuildServiceProvider(IServiceCollection services, ServiceProviderOptions options)
   at Microsoft.Extensions.DependencyInjection.DefaultServiceProviderFactory.CreateServiceProvider(IServiceCollection containerBuilder)
   at Microsoft.Extensions.Hosting.Internal.ServiceFactoryAdapter`1.CreateServiceProvider(Object containerBuilder)
   at Microsoft.Extensions.Hosting.HostBuilder.CreateServiceProvider()
georgiosd commented 4 years ago

@twenzel I digged into an older project that started in core 2.1 before migrating to 3.1, where Razorlight works!

The difference is, which I hadn't noticed, that the working project uses WebHost (default in 2.1) instead of Host (default in 3.1)

https://stackoverflow.com/questions/59745401/what-is-the-difference-between-host-and-webhost-class-in-asp-net-core

jzabroski commented 4 years ago

I started cleaning this up.

jzabroski commented 4 years ago

@georgiosd So, I would appreciate a repro.

I spent an hour today figuring out if I could repro @twenzel or your problem, and I cannot. Below is an example of what I tried.

using Microsoft.AspNetCore.Hosting;
using Microsoft.Extensions.DependencyInjection;
using Moq;
using Xunit;
using RazorLight.Extensions;
using System;
using Microsoft.Extensions.Hosting;
using Microsoft.AspNetCore.Builder;

namespace RazorLight.Tests.Extensions
{
    public class ServiceCollectionExtensionsTest
    {
        public class EmbeddedEngineStartup
        {
            public void Configure(IApplicationBuilder app)
            {

            }
            public void ConfigureServices(IServiceCollection services)
            {
                var embeddedEngine = new RazorLightEngineBuilder()
                                  .UseEmbeddedResourcesProject(typeof(EmbeddedEngineStartup)) // exception without this (or another project type)
                                  .UseMemoryCachingProvider()
                                  .Build();

                //services.AddSingleton(embeddedEngine.Options);
                //services.AddSingleton(embeddedEngine.Handler.Compiler);
                //services.AddSingleton(embeddedEngine.Handler.FactoryProvider);
                //services.AddSingleton(embeddedEngine.Handler.Cache);
                services.AddRazorLight(() => embeddedEngine);

            }
        }

#if !(NETCOREAPP2_0)
        [Fact]
        public void Ensure_Works_With_Generic_Host()
        {
            static IHostBuilder CreateHostBuilder(string[] args)
            {
                return Host.CreateDefaultBuilder(args)
                    .ConfigureWebHostDefaults(webBuilder =>
                    {
                    webBuilder.UseStartup<EmbeddedEngineStartup>();
                    });
            }

            var hostBuilder = CreateHostBuilder(null);

            Assert.NotNull(hostBuilder);
            var host = hostBuilder.Build();
            Assert.NotNull(host);
            host.Services.GetService<IRazorLightEngine>();
        }
#endif
     }
}
sibeliuz commented 4 years ago

Workaround Register missing dependencies separately

public void ConfigureServices(IServiceCollection services)
        {
 var engine = new RazorLightEngineBuilder()
                   .UseEmbeddedResourcesProject(typeof(Startup)) // exception without this (or another project type)
                   .UseMemoryCachingProvider()
                   .Build();

            services.AddRazorLight(() => engine);
            services.AddSingleton(engine.Options);
            services.AddSingleton(engine.Handler.Compiler);
            services.AddSingleton(engine.Handler.FactoryProvider);
            services.AddSingleton(engine.Handler.Cache);
}

This workaround fixes it for me too.

knuxbbs commented 3 years ago

The workaround proposed by @twenzel only works for 2.0.0-beta4. The 2.0.0-rc.2 version throws:

System.AggregateException: 'Some services are not able to be constructed (Error while validating the service descriptor 'ServiceType: RazorLight.IEngineHandler Lifetime: Singleton ImplementationType: RazorLight.EngineHandler': Unable to activate type 'RazorLight.EngineHandler'. The following constructors are ambiguous: Void .ctor(RazorLight.RazorLightOptions, RazorLight.Compilation.IRazorTemplateCompiler, RazorLight.Compilation.ITemplateFactoryProvider, RazorLight.Caching.ICachingProvider) Void .ctor(Microsoft.Extensions.Options.IOptions`1[RazorLight.RazorLightOptions], RazorLight.Compilation.IRazorTemplateCompiler, RazorLight.Compilation.ITemplateFactoryProvider, RazorLight.Caching.ICachingProvider))'

There is a sample project: https://github.com/knuxbbs/RazorLightSample

edvinklaebo commented 3 years ago

@jzabroski You need to setup the service provider to validate on build like this:

            static IHostBuilder CreateHostBuilder(string[] args)
            {
                return Host.CreateDefaultBuilder(args).UseDefaultServiceProvider((context, options) =>
                {
                    options.ValidateOnBuild = true;
                }).ConfigureWebHostDefaults(webBuilder => { webBuilder.UseStartup<EmbeddedEngineStartup>(); });
            }
knuxbbs commented 3 years ago

For 2.0.0-rc.2, what worked for me was registering IEngineHandler before call AddRazorLight.

public void ConfigureServices(IServiceCollection services)
{
            var engine = new RazorLightEngineBuilder()
                   .UseEmbeddedResourcesProject(typeof(Startup))
                   .UseMemoryCachingProvider()
                   .Build();

            services.AddSingleton(engine.Handler);
            services.AddRazorLight(() => engine);
}
jzabroski commented 3 years ago

@jzabroski You need to setup the service provider to validate on build like this:

            static IHostBuilder CreateHostBuilder(string[] args)
            {
                return Host.CreateDefaultBuilder(args).UseDefaultServiceProvider((context, options) =>
                {
                    options.ValidateOnBuild = true;
                }).ConfigureWebHostDefaults(webBuilder => { webBuilder.UseStartup<EmbeddedEngineStartup>(); });
            }

@edvinklaebo - so, you're saying the following test is incorrect?

https://github.com/toddams/RazorLight/blob/4fb414a87412f4bbbff96db8849980993a3060ba/tests/RazorLight.Tests/Extensions/ServiceCollectionExtensionsTest.cs#L80-L100

I am not sure why I would need to call options.ValidateOnBuild = true; in the test, as I am verifying host.Services.GetService<IRazorLightEngine>();?

Or, did you mean to reply to @knuxbbs ?

jzabroski commented 3 years ago

@edvinklaebo Hmm... I just updated the test and it looks like you're right. Why would host.Services.GetService<IRazorLightEngine>(); succeed?

jzabroski commented 3 years ago

@edvinklaebo I played around with your sample code and ValidateOnBuild = true does cause the test to fail. You can see the above commit.

~One thing I do not understand is why changing IEngineHandler to be resolved using a transient makes the ValidateOnBuild error go away. It actually makes whatever problem theoretically worse. So there is something ValidateOnBuild does that doesn't make sense to me.~ (Ignore this comment. I ran the wrong test.)

Below is a brain dump

ValidateOnBuild was introduced in .netcoreapp3.0 TFM, so it makes sense this would only have caused issues starting with .NET Core 3.x.

jzabroski commented 3 years ago

I think I figured this out.

The issue is that since we resolve the IEngineHandler from the RazorLightEngineBuilder call, the ValidateOnBuild cannot statically verify the contents of the lambda. The only way it can "know" that IEngineHandler cannot be resolved is because we told it about the dependency in the line before. TryAdd does what it's supposed to do, which is only register the engine handler if it hasn't already been provided. But, the static verification doesn't know that we've provided a concrete instance in the next registration.

tl;dr: By eliminating this line, all tests pass:

https://github.com/toddams/RazorLight/blame/4fb414a87412f4bbbff96db8849980993a3060ba/src/RazorLight/Extensions/ServiceCollectionExtensions.cs#L30

It appears I broke this, although I can't remember why I did this (it was almost certainly to handle a scenario where people do silly things like register RazorLight using the RazorLightEngineBuilder, but then call the EngineHandler interface directly), and I didn't submit a issue number along with the check-in.

Is there any alternative to removing this line?

jzabroski commented 3 years ago

I think the workaround is to fail hard if people try to call the engine handler directly after calling AddRazorLight:

            services.TryAddSingleton<IEngineHandler>(p =>
                throw new InvalidOperationException($"This exception can only occur if you inject {nameof(IEngineHandler)} directly using {nameof(ServiceCollectionExtensions)}.{nameof(AddRazorLight)}"));

This might break some people, but at least the reason is fairly clear.

Feedback? Thoughts? Questions? Concerns?

jzabroski commented 3 years ago

One last thought. I guess there could be an AddRazorLightEngineHandler() extension method for those that don't want RazorLightEngine. The exception message could then be updated to suggest a call to action to "Consider using AddRazorLightEngineHandler() instead." But that still has to deal with things RazorLightEngine bundles, like support for @inject, dynamic templates, and any other callbacks callers wish to make to enhance RazorLight.