Add configuration support

[panoukos41]

Adds support to load PluginCatalogs from configuration as discussed in #1

In-depth:

1. Added a new project Weikio.PluginFramework.Configuration and referenced it from the Weikio.PluginFramework.AspNetcore project. I also implemented the new AddPluginFramework() extension methods.

2. Added IPluginCatalogConfigurationProvider and PluginCatalogConfigurationProvider inside Providers namespace.

3. Added IConfigurationToCatalogConverter, simple Assembly, and Folder converter implementations inside Converters namespace.

4. Added the base class for the Configuration CatalogConfiguration.

5. Added static class CatalogTypes to keep all the types that can be converted with their configuration type keys in one place.

[panoukos41]

After some thought on the names maybe I should rename IPluginCatalogConfigurationProvider to IPluginCatalogConfigurationLoader since it is not providing the configuration it just loads it.

[mikoskinen]

Looking really good, thanks! Good idea to separate the configuration into its own project. I would assume this is going to be mostly used with ASP.NET Core applications but there's many cases where appsettings.json are used in console apps too. Releasing a separate package for configuration allows the usage of this code in a console app.

I'll go through this with more detail and add comments related to naming etc.

Thanks again 👍

[panoukos41]

No problem, if you want anything let me know and I will get to it.

The only thing that's missing is a class to do the loading like an IAppPluginCatalog initializer or something.

Right now it's done only in aspnetcore project but this can come as people start using it on all kinds of applications.

[mikoskinen]

Currently in ServiceCollectionExtensions.AddPluginFramework, the catalogs are added inside a for-loop as concrete objects. What do you think, would it be possible to change the loop so that it doesn't call services.AddPluginCatalog(catalog) in the end but instead we register a factory for the catalog?

Meaning, can we go from this:

for (var i = 0; i < catalogs.Count; i++)
{
    ...
    services.AddPluginCatalog(catalog);
}

To this:

for (var i = 0; i < catalogs.Count; i++)
{
    ...
    services.TryAddEnumerable(ServiceDescriptor.Singleton(typeof(IPluginCatalog), serviceProvider =>
    {
        ...
        return catalog;
    }));
}

Why I'm asking is that using the factory-model we could simplify the code related to converters. We could register all the converters into the IServiceCollection and in the factory do something like this:

services.TryAddEnumerable(ServiceDescriptor.Singleton(typeof(IPluginCatalog), serviceProvider =>
{
    var converters = serviceProvider.GetServices<IConfigurationToCatalogConverter>();

    foreach (var catalogConverter in converters)
    {
        if (catalogConverter.CanConvert(type))
        {
            catalog = catalogConverter.Convert(configuration.GetSection(key));
        }
    }

    if (catalog == null)
    {
        throw new Exception("Couldn't locate converter...");
    }

    return catalog;
}));

The added bonus of this would be that the developer could easily register their own converters.

[panoukos41]

Oh this is an awesome idea, sorry for the late reply I was away from home.

I will implement it in the next few days and will commit back when it's ready 😁

[mikoskinen]

Oh this is an awesome idea, sorry for the late reply I was away from home.

I will implement it in the next few days and will commit back when it's ready 😁

Great, thanks :)

[panoukos41]

@mikoskinen hey I got it done I only have some questions,

1. Should I remove the overloads that pass a converter? Since now converters load from the service provider.
2. Should I rename IPluginCatalogConfigurationProvider to IPluginCatalogConfigurationLoader since it does not provide the configuration like a real provider aka the json provider, it just loads the parameters it needs, another name could be IPluginCatalogConfigurationReader but I believe loader describes the operation better.
3. There is a static class called CatalogTypes, should I create another one CatalogConverters that will provide the default converters as static parameters? I was thinking of something that uses lazyloading in case a default converter is used multiple times.

This is the code that will replace the whole "for" statement, I used a "switch expression" instead of "if-else" but I can replace it if you want.

The only thing that I don't really find a big problem is that only one catalog is added which is a CompositePluginCatalog that contains all the others, so the console only displays that one catalog was added. This was easier to implement and everything is done only once.

services.TryAddSingleton<IPluginCatalog>(serviceProvider =>
{
    var converters = serviceProvider.GetServices<IConfigurationToCatalogConverter>();
    var catalogs = new List<IPluginCatalog>();

    for (int i = 0; i < catalogConfigurations.Count; i++)
    {
        var item = catalogConfigurations[i];
        var key = $"{provider.SectionKey}:{provider.CatalogsKey}:{i}";

        // Check if a type is provided.
        var type = string.IsNullOrWhiteSpace(item.Type)
               ? throw new ArgumentException($"A type must be provided for catalog at position {i + 1}")
               : item.Type;

        // Try to find any registered converter that can convert the specified type.
        var foundConverter = converters.FirstOrDefault(converter => converter.CanConvert(type));

        // Add a catalog to the list of catalogs.
        catalogs.Add(foundConverter != null
            // If a converter was found we call it's convert method.
            ? foundConverter.Convert(configuration.GetSection(key))
            // If no converter was found (it's null) we proceed with the built-in type converters.
            : type switch
            {
                // Assembly type.
                CatalogTypes.Assembly => new AssemblyCatalogConfigurationCoverter().Convert(configuration.GetSection(key)),

                // Folder type.
                CatalogTypes.Folder => new FolderCatalogConfigurationConverter().Convert(configuration.GetSection(key)),

                // Unkown type.
                _ => throw new ArgumentException($"The type provided for catalog at position {i + 1} is unknown.")
            });
    }

    return new CompositePluginCatalog(catalogs.ToArray());
});

[mikoskinen]

Sorry for the delay! I think that looks good. Regarding your question:

1. Yep, these could be removed.
2. IPluginCatalogConfigurationLoader is I think the most clear.
3. I'm not sure about this one, I think I have to play with the code little to be sure how this affects things.

Returning a single CompositePluginCatalog is OK. It might be that this could be changed at some point so that services.TryAddEnumerable(ServiceDescriptor.Singleton(typeof(IPluginCatalog) is used instead as there can be some options we want to configure on catalog level.

I'm really happy of how this looks and I could merge it into master after a "go live" comment from you :) We have added some tagging related code into master so I'm currently thinking that we could release the 1.1 version next week, with this feature + tagging changes.

[panoukos41]

@mikoskinen Great, I will be commenting back tomorrow when I commit the changes 😃

[panoukos41]

For the IPluginCatalogConfigurationProvider i renamed it to IPluginCatalogConfigurationLoader and made it so that it can be registered in the service provider like the converters with some changes, so now someone could add his own implementation that could bring the configuration in any way he wants.

As a result, I removed the overload that could take an IPluginCatalogConfigurationLoader parameter.

The feature is now complete, I tested it in a project of mine but I will create a sample in the near future.

I also want to apologize for taking longer, I wanted to implement the loader in the way I did but I also had a power outage where I live for a whole day and due to this it took longer than expected.

If you find everything to your liking you can merge and close this pull request 😄

[mikoskinen]

No worries about the timeline :) Sorry from my behalf, things have been too hectic lately at work so my comments have lagged too much.

I'm going to merge this now, it's great :)

Thank you!