Obsolete Finally

[mariusz96]

The PR for #22.

[ipvalverde]

Thanks for the contributions!

I've added a few comments, didn't have time to review all of it though

[ipvalverde]

Suggested change

	## Migrate from PipelineNet 0.10 to 0.20
	In PipelineNet 0.20, `Finally` overloads that use `Func` have been made obsolete.
	## Migrate from PipelineNet 0.10 to 0.11
	In PipelineNet 0.11, `Finally` overloads that use `Func` have been made obsolete. This will be removed in the next major version.

[mariusz96]

Most nuget packages use semantic versioning - meaning breaking changes are only allowed with major version bumps: X._._. Since this is pre-1.0.0 I just took the next most significant number so it sticks out.

Let me know if you want this changed despite this though.

[ipvalverde]

It looks like we can extract a pattern for checking the disposability of the resolved middlewares. Maybe something like this:

namespace PipelineNet.MiddlewareResolver
{
    internal static class MiddlewareResolverExtensions
    {
        public static void ValidateDisposableValues(this MiddlewareResolverResult resolverResult)
        {
            if (resolverResult.IsDisposable && !(resolverResult.Middleware is IDisposable))
            {
#if NETSTANDARD2_1_OR_GREATER
                if (resolverResult.Middleware is IAsyncDisposable)
                {
                    throw new InvalidOperationException($"'{resolverResult.Middleware.GetType()}' type only implements IAsyncDisposable." +
                        " Use the AsyncResponsibilityChain or AsyncPipeline.");
                }
#endif

                throw new InvalidOperationException($"'{resolverResult.Middleware.GetType()}' type does not implement IDisposable.");
            }
        }
    }
}

Then this code could be simplified with something like this:

        // new method just to resolve the finally function and the disposable middleware:
        private Func<TParameter, TReturn> ResolveFinallyFunction(ref IDisposable finallyHandler)
        {
            Func<TParameter, TReturn> func;
            if (_finallyType != null)
            {
                var finallyResolverResult = MiddlewareResolver.Resolve(_finallyType);

                if (finallyResolverResult == null || finallyResolverResult.Middleware == null)
                {
                    throw new InvalidOperationException($"'{MiddlewareResolver.GetType()}' failed to resolve finally of type '{_finallyType}'.");
                }

                finallyResolverResult.ValidateDisposableValues();

                var @finally = (IFinally<TParameter, TReturn>) finallyResolverResult.Middleware;
                finallyHandler = @finally as IDisposable;

                func = (p) => @finally.Finally(p);
            }
            else if (_finallyFunc != null)
            {
                func = _finallyFunc;
            }
            else
            {
                func = (p) => default(TReturn);
            }

            return func;
        }

Then the method would be much simpler:

Func<TParameter, TReturn> func = null;
func = (param) =>
{
    MiddlewareResolverResult resolverResult = null;
    IDisposable disposableFinallyMiddleware = null;
    try
    {
        var type = MiddlewareTypes[index];
        resolverResult = MiddlewareResolver.Resolve(type);
        var middleware = (IMiddleware<TParameter, TReturn>)resolverResult.Middleware;

        index++;
        // If the current instance of middleware is the last one in the list,
        // the "next" function is assigned to the finally function or a 
        // default empty function.
        if (index == MiddlewareTypes.Count)
        {
            func = ResolveFinallyFunction(ref disposableFinallyMiddleware);
        }

        resolverResult.ValidateDisposableValues();

        return middleware.Run(param, func);
    }
    finally
    {
        if (resolverResult != null && resolverResult.IsDisposable)
        {
            var middleware = resolverResult.Middleware;
            if (middleware != null)
            {
                if (middleware is IDisposable disposable)
                {
                    disposable.Dispose();
                }
            }
        }

        disposableFinallyMiddleware?.Dispose();
    }
};

[mariusz96]

Oh, definitely, also for the disposal itself.

Could you please ignore the code duplication here for now though? I have a refactoring PR which I can open after all current ones are merged.

[ipvalverde]

As always, thanks for the contribution!