PipelineNet icon indicating copy to clipboard operation
PipelineNet copied to clipboard

Add ServiceCollectionExtensions and ServiceProviderMiddlewareResolver

Open mariusz96 opened this issue 1 year ago • 1 comments

I could not stand that this is newing things up in the DI-enabled app so I went ahead and added:

  • the factories to hide the newing (this is the best pattern I could find)
  • the auto-registration methods for the IServiceCollection
  • the IServiceProvider implementation of the IMiddlewareResolver
  • the <GenerateDocumentationFile> property to the csprojs to include the documentation in the nuget and I fixed missing xml comment warnings

Let me know how you feel about the changes, I think they're a significant improvement over the previous version of PipelineNet.ServiceProvider.

mariusz96 avatar Aug 29 '24 06:08 mariusz96

+ namespace PipelineNet.ChainOfResponsibilityFactories

+ public interface IAsyncResponsibilityChainFactory<TParameter, TReturn>
+ {
+     IAsyncResponsibilityChain<TParameter, TReturn> Create();
+ }

+ public interface IResponsibilityChainFactory<TParameter, TReturn>
+ {
+     IResponsibilityChain<TParameter, TReturn> Create();
+ }
+ namespace PipelineNet.PipelineFactories

+ public interface IPipelineFactory<TParameter>
+ {
+     IPipeline<TParameter> Create();
+ }

+ public interface IAsyncPipelineFactory<TParameter>
+ {
+     IAsyncPipeline<TParameter> Create();
+ }
+ namespace PipelineNet.ServiceProvider.MiddlewareResolver

+ public class ServiceProviderMiddlewareResolver : IMiddlewareResolver
+ {
+     public MiddlewareResolverResult Resolve(Type type);
+ }
+ namespace PipelineNet.ServiceProvider

+ public static class ServiceCollectionExtensions
+ {
+     public static IServiceCollection AddPipelineNet(
+         this IServiceCollection services,
+         IEnumerable<Assembly> assemblies,
+         ServiceLifetime lifetime = ServiceLifetime.Scoped);

+     public static IServiceCollection AddPipelineNet(
+         this IServiceCollection services,
+         Assembly assembly,
+         ServiceLifetime lifetime = ServiceLifetime.Scoped);

+     public static IServiceCollection AddMiddlewareFromAssemblies(
+         this IServiceCollection services,
+         IEnumerable<Assembly> assemblies,
+         ServiceLifetime lifetime = ServiceLifetime.Scoped);

+     public static IServiceCollection AddMiddlewareFromAssembly(
+         this IServiceCollection services,
+         Assembly assembly,
+         ServiceLifetime lifetime = ServiceLifetime.Scoped);

+     public static IServiceCollection AddPipelineNetCore(
+         this IServiceCollection services,
+         ServiceLifetime lifetime = ServiceLifetime.Scoped);
+ }

mariusz96 avatar Sep 02 '24 06:09 mariusz96

Thanks for the contribution!

I think we could simplify it a bit by avoiding the factory pattern. The idea was that the pipeline would be configured in the start-up time so developers don't have to actually new up classes during the runtime.

An example of how the startup configuration would look like:

builder.Services.AddScoped<ProcessImageService>();
builder.Services.AddScoped<IPipeline<Image>>(s =>
{
    return new Pipeline<Image>(new ActivatorUtilitiesMiddlewareResolver(s))
        .Add<CloneImage>()
        .Add<ResizeImage>()
        .Add<RemoveColour>()
        .Add<FlipImageHorizontally>()
        .Add<SaveImage>();
});

Then, inside the service, we can simply get an instance for the pipeline with the generic type, no need for factories:

    internal class ProcessImageService
    {
        private readonly IPipeline<Image> pipeline;

        public ProcessImageService(IPipeline<Image> pipeline)
        {
            this.pipeline = pipeline;
        }

        public void Execute(string imagePath)
        {
            using var image = Image.Load(imagePath);

            pipeline.Execute(image);

            image.Save("processed-image.png");
        }
    }

ipvalverde avatar Sep 22 '24 17:09 ipvalverde

Thanks for the contribution!

I think we could simplify it a bit by avoiding the factory pattern. The idea was that the pipeline would be configured in the start-up time so developers don't have to actually new up classes during the runtime.

An example of how the startup configuration would look like:

builder.Services.AddScoped<ProcessImageService>();
builder.Services.AddScoped<IPipeline<Image>>(s =>
{
    return new Pipeline<Image>(new ActivatorUtilitiesMiddlewareResolver(s))
        .Add<CloneImage>()
        .Add<ResizeImage>()
        .Add<RemoveColour>()
        .Add<FlipImageHorizontally>()
        .Add<SaveImage>();
});

Then, inside the service, we can simply get an instance for the pipeline with the generic type, no need for factories:

    internal class ProcessImageService
    {
        private readonly IPipeline<Image> pipeline;

        public ProcessImageService(IPipeline<Image> pipeline)
        {
            this.pipeline = pipeline;
        }

        public void Execute(string imagePath)
        {
            using var image = Image.Load(imagePath);

            pipeline.Execute(image);

            image.Save("processed-image.png");
        }
    }

Yeah, that was my initial thought as well.

However when actually opening up a new project I found using factory pattern was the better experience:

  • all interfaces in this package are very similar and after figuring out correct middleware ones and configuring correct pipeline/asyncPipeline/chain/asyncChain one it's very easy to errounously inject incorrect middleware flow one and get a runtime exception. And I also don't particulary like repating generic type parameters twice (once at startup and again in consuming service constructor)
  • if the DI container does not implement IKeyedServiceProvider it is not possibile to create two middleware flows with same generic type parameters
  • middleware flows are mutable so someone might still call .Add<TMiddleware>() in the consuming service even though they "should" configure it at startup

However factory pattern is not a perfect solution either as it adds a new concept - maybe I can just remove it for now leaving developers to register middleware flows themselves?

That would make it possible to add a better registration pattern in the future.

mariusz96 avatar Sep 23 '24 05:09 mariusz96

Ok, I've just removed all factories - let's be done with this already😆

mariusz96 avatar Sep 23 '24 20:09 mariusz96