weikio / PluginFramework

Everything is a Plugin in .NET
MIT License
549 stars 105 forks source link

Add support for appsettings.json #1

Closed mikoskinen closed 3 years ago

mikoskinen commented 4 years ago

It should be possible to configure Plugin Framework using a configuration file (appsettings.json or some other json-file).

Add this functionality into a new library. The AspNet-project should reference this new library as the functionality is most likely used with ASP.NET Core.

panoukos41 commented 4 years ago

Hey is anyone adding support for this? If not I am thinking of trying to add implement it myself 😄, if I can try to implement it (my first contribution to oss) could I ask some questions regarding the implementation?

Thanks for your time and awesome project by the way, just what I wanted, saw it on blazor community standup.

panoukos41 commented 4 years ago

@mikoskinen hey I started this in my free time in a worker service. I added really basic implementation that is as follows:

Settings classes that represent the settings inside the configuration:

public class PluginFrameworkSettings
{
    public List<CatalogueConfiguration>? Catalogs { get; set; }
}

public class CatalogueConfiguration
{
    public string? Type { get; set; }

    public string? Path { get; set; }

    public TypeFinderCriteria? SearchCriteria { get; set; }
}

I believe Creating another class to replace TypeFinderCriteria would be better and then an object type for the provided options depending on the type of the catalog.


A problem I came across that might need a separate issue.

As I was implementing the 2 catalogs (Assembly and Folder) using the aspnetcore package I noticed that the services are added via a hosted service (backgroundservice), this actually creates a racing problem.

Aspnet will block the thread until the first await is hit, then it just keeps executing the app and the service is left in the background. I noticed this because I was adding another hosted service that was using an Interface from the plugin but got the error that no interface had been registered since the first service did not finish in time.

To fix this I actually created an extension method on IHost that executes the same logic (after .Build() before Run()) so the services are all there but the app has not executed yet.

I did it with GetAwaiter().GetResult() since the tasks just return Completed and see if it works but I think having a synchronous and asynchronous like the Run method would be the best for all. Another way would be to do the searching etc just when the AddPluginFramework(IConfiguration configuration) method is executed but I don't know if it's appropriate.

I believe the aspnetcore problem should go to another issue in which I could contribute to the new methods. I could also implement a static class (PluginCatalog) that will do this for any app like a console application.

Sorry for the long post 😄🥔

mikoskinen commented 4 years ago

Thank you for your work and feedback, really appreciate it!

You're right that a background service is used to initialize the catalogs. The thinking behind this was that as ASP.NET Core 3.1 runs the hosted services before the application starts accepting requests, that provides us a good place to initialize the catalogs. The app startup should wait until the catalogs are initialized and then continue.

We would like to avoid the situation where developer has to touch Program.cs. If possible, all the configuration should be done through ConfigureServices.

I wonder if this would be doable by using something like the following construct. Though I'm not sure if I'm overcomplicating this :)

Additional benefit to this is that the developers could then add Plugin catalogs using the IOptions-system, for example:

services.Configure<CatalogueConfiguration>(config => ...);

What do you think? Thanks again for the work you have done with this!

mikoskinen commented 4 years ago

Converting CatalogConfiguration to actual Catalog is also one thing. There can quite many different types of catalogs so I don't think we can have one class taking care of every conversion.

Maybe something like IConfigurationToCatalogConverter with bool CanConvert and IPluginCatalog Convert is needed? Then we could have a bunch of classes implementing the interface.

panoukos41 commented 4 years ago

Converting CatalogConfiguration to actual Catalog is also one thing. There can quite many different types of catalogs so I don't think we can have one class taking care of every conversion.

Maybe something like IConfigurationToCatalogConverter with bool CanConvert and IPluginCatalog Convert is needed? Then we could have a bunch of classes implementing the interface.

Yea when I started I thought that most catalogs might not be configured by a single class, I just wanted the settings to convert to a base class on which I can then build up. I believe it's better to have an IConfigurationToCatalogConverter where we pass the IConfiguration key the catalog has and leave it to a converter to return an IPluginCatalog. This way we could implement one for every catalog in time and just think of their structure. The static method AddPluginFramework that adds the IPluginCatalogs could then have another overload that accepts a func to handle custom PluginProvider types.

I also like the idea of IPluginCatalogConfigurationProvider it would further abstract the logic from the static method I created and could allow for a second method that can accept a custom IPluginCatalogConfigurationProvider etc. So a user could have many options in the end from this or IConfigurationToCatalogConverter to implement custom logic.


So I will create the interface IConfigurationToCatalogConverter and keep the class CatalogConfiguration with the basic type parameter to grab the type of the catalog, then I will just pass the catalog key on which each class can use to grab extra configuration parameters. I will then implement AssemblyCatalogConfigurationConverter and FolderCatalogConfigurationConverter. I will use the Type method of CatalogConfiguration to determine which converter to use.

I will also create IPluginCatalogConfigurationProvider and a class to implement it. Then I will create the static methods that calls it aka AddFrameworkPlugin and an overload that accepts an implementation of the interface.

I named the project Weikio.PluginFramework.Configuration, is this appropriate?

When I have it on my copy of the repo I will open a pull request for further refinements.

For the OptionsPluginCatalogConfigurationProvider and on I got a bit confused but the idea I believe is to use composition for better extensibility right? I think it could be implemented later on a bigger update since I don't see it overlapping with IPluginCatalogConfigurationProvider.


You're right that a background service is used to initialize the catalogs. The thinking behind this was that as ASP.NET Core 3.1 runs the hosted services before the application starts accepting requests, that provides us a good place to initialize the catalogs. The app startup should wait until the catalogs are initialized and then continue.

We would like to avoid the situation where developer has to touch Program.cs. If possible, all the configuration should be done through ConfigureServices.

I get the idea, when I saw it I was like this is great but when I added another hosted service I came across the racing issues. I will open another issue to elaborate further later tonight when I will have some time and we can discuss maybe a workaround that will be documented and have the appropriate XML comments.

mikoskinen commented 3 years ago

This feature is available in the newly released 1.1.0.