dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.21k stars 9.95k forks source link

More flexibility for RazorViewEngine #24328

Closed rorymurphy closed 3 years ago

rorymurphy commented 4 years ago

Is your feature request related to a problem? Please describe.

RazorViewEngine being registered as a singleton limits the ability to support use cases such as applications with plugins (where a plugin may impact view resolution) and multi-tenancy (where tenants may require different view resolution). Generally speaking, singleton seems like it should be avoided where possible, as they function similar to statics, and limit applications' ability to customize behavior on a per-request basis. The registration with Singleton lifespan appears to be driven by the caching of state for the resolution of views. Currently, flushing the cache is not possible and the alternative method of clearing the cache (de-registering the RazorViewEngine and registering a new instance) quickly becomes messy as the full graph of dependents must also be re-instantiated.

Describe the solution you'd like

While the ability to support the aforementioned use cases need not be part of the default implementation of RazorViewEngine, at present it is also not possible to implement cleanly in a derived classes, as the ViewLookupCache cannot be set by derived classes. Consequently, in order to support such behavior, it becomes necessary to either implement IRazorViewEngine from scratch (where 95%+ of the code is copy/pasted from RazorViewEngine) or to use reflection to modify the private field behind ViewLookupCache. This feels like a miss in terms of the balance of open vs. closed. Adding a constructor, as show in this PR would allow derivations of RazorViewEngine to provide better control over caching, and to potentially inject the cache and move RazorViewEngine to Scoped lifespan (have tested).

davidfowl commented 4 years ago

We don't want more allocations by default to allow some applications to replace things per tenant. There are lots of ways to implement multitenancy so I'd suggest specifying what functionality you need from the system so we can best come up with a design to satisfy that.

pranavkm commented 4 years ago

/cc @sebastienros

sebastienros commented 4 years ago

Changing Razor would not solve the problem because there are tens of services that behave the same way, and anything that uses options. The only way you can support multi-tenancy and be able to configure each tenant individually is to create DI containers for each tenant. This way each tenant has its own set of services, including singletons. And you can still share common "host singletons" across if it's done right. Some .NET multi-tenancy libraries don't support this, some do. If you need this feature then you should use a library that supports it, for instance Orchard Core

rorymurphy commented 4 years ago

To clarify a bit - the default implementation need not change, as this is clearly not the most common use case. I'd considered the separate DI containers, which would actually present another benefit to providing some control over the cache. If potentially many DI containers are being kept in memory, rather than having each manage its own cache internally with no visibility into or control over the size of each, it would be preferable to allocate a single cache that they all share (segmented by each instance having its own key prefix, or similar), allowing the application to limit the total size across all instances.

This need not be supported by the default RazorViewEngine but it would be beneficial if derived classes could override cache behavior. At the moment, the only thing preventing this the fact that there is no supported way of injecting a cache implementation. Doing so requires either invoking reflection or implementing a new IRazorViewEngine where 99% of the code is copy/pasted from the default RazorViewEngine. I have a workable, albeit potentially fragile, solution for my needs but thought I'd submit an issue as it seems that there should be a clean way of doing this.

Thanks for the pointer to Orchard Core - I am familiar but, as I'm writing a CMS myself, am not looking to take on a dependency to it.

sebastienros commented 4 years ago

You can write your own CMS on the Orchard Core framework. It's a modular and multi-tenant framework. The Orchard Core CMS has independent projects from the framework. Take a look at the sample repo I linked.

rorymurphy commented 4 years ago

Am familiar with it, but it is not the direction I'm looking to take.

ghost commented 3 years ago

Thank you for contacting us. Due to a lack of activity on this discussion issue we're closing it in an effort to keep our backlog clean. If you believe there is a concern related to the ASP.NET Core framework, which hasn't been addressed yet, please file a new issue.

This issue will be locked after 30 more days of inactivity. If you still wish to discuss this subject after then, please create a new issue!