Using IoC and Dependency Injection, how do I wrap an existing implementation with a new layer of implementation without changing existing code so as not to violate the Open-Closed principle?

Question

I'm trying to figure out how this would be done in practice, so as not to violate the Open Closed principle.

Say I have a class called HttpFileDownloader that has one function that takes a url and downloads a file returning the html as a string. This class implements an IFileDownloader interface which just has the one function. So all over my code I have references to the IFileDownloader interface and I have my IoC container returning an instance of HttpFileDownloader whenever an IFileDownloader is Resolved.

Then after some use, it becomes clear that occasionally the server is too busy at the time and an exception is thrown. I decide that to get around this, I'm going to auto-retry 3 times if I get an exception, and wait 5 seconds in between each retry.

So I create HttpFileDownloaderRetrier which has one function that uses HttpFileDownloader in a for loop with max 3 loops, and a 5 second wait between each loop. So that I can test the "retry" and "wait" abilities of the HttpFileDownloadRetrier I have the HttpFileDownloader dependency injected by having the HttpFileDownloaderRetrier constructor take an IFileDownloader.

So now I want all Resolving of IFileDownloader to return the HttpFileDownloaderRetrier. But if I do that, then HttpFileDownloadRetrier's IFileDownloader dependency will get an instance of itself and not of HttpFileDownloader.

So I can see that I could create a new interface for HttpFileDownloader called IFileDownloaderNoRetry, and change HttpFileDownloader to implement that. But that means I'm changing HttpFileDownloader, which violates Open Closed.

Or I could implement a new interface for HttpFileDownloaderRetrier called IFileDownloaderRetrier, and then change all my other code to refer to that instead of IFileDownloader. But again, I'm now violating Open Closed in all my other code.

So what am I missing here? How do I wrap an existing implementation (downloading) with a new layer of implementation (retrying and waiting) without changing existing code?

Here's some code if it helps:

public interface IFileDownloader
{
  string Download(string url);
}

public class HttpFileDownloader : IFileDownloader
{
  public string Download(string url)
  {
    //Cut for brevity - downloads file here returns as string
    return html;
  }
}

public class HttpFileDownloaderRetrier : IFileDownloader
{
  IFileDownloader fileDownloader;

  public HttpFileDownloaderRetrier(IFileDownloader fileDownloader)
  {
    this.fileDownloader = fileDownloader;
  }

  public string Download(string url)
  {
    Exception lastException = null;
    //try 3 shots of pulling a bad URL.  And wait 5 seconds after each failed attempt.
    for (int i = 0; i < 3; i++)
    {
      try { fileDownloader.Download(url); }
      catch (Exception ex) { lastException = ex; }
      Utilities.WaitForXSeconds(5);
    }
    throw lastException;
  }
}

Answer 1

How about deriving directly from HttpFileDownloader:

public class HttpFileDownloader : IFileDownloader
{
    public virtual string Download(string url)
    {
        //Cut for brevity - downloads file here returns as string
        return html;
    }
}

public class HttpFileDownloaderWithRetries : HttpFileDownloader
{
    private readonly int _retries;
    private readonly int _secondsBetweenRetries;

    public HttpFileDownloaderWithRetries(int retries, int secondsBetweenRetries)
    {
        _retries = retries;
        _secondsBetweenRetries = secondsBetweenRetries;
    }

    public override string Download(string url)
    {
        Exception lastException = null;
        for (int i = 0; i < _retries; i++)
        {
            try 
            { 
                return base.Download(url); 
            }
            catch (Exception ex) 
            { 
                lastException = ex; 
            }
            Utilities.WaitForXSeconds(_secondsBetweenRetries);
        }
        throw lastException;
    }
}

Answer 2

You are more or less implementing the Circuit Breaker design pattern. As always when it comes to implementing cross-cutting concerns using DI, the key is to apply the Decorator pattern.

Write a CircuitBreakingFileDownloader like this:

public class CircuitBreakingFileDownloader : IFileDownloader
{ 
    private readonly IFileDownloader fileDownloader;

    public CircuitBreakingFileDownloader(IFileDownloader fileDownloader)
    {
        if (fileDownloader == null)
        {
            throw new ArgumentNullException("fileDownloader");
        }

        this.fileDownloader = fileDownloader;
    }

    public string Download(string url)
    {
        // Apply Circuit Breaker implementation around a call to
        this.fileDownloader.Download(url)
        // here...
    }
} 

This approach follows the Open/Closed Principle and favors composition over inheritance. It also satisfies the Single Responsibility Principle because the Circuit Breaker deals only with that aspect, while the decorated IFileDownloader concentrates on its own responsibility.

Most proper DI Containers understand the Decorator pattern, so you can now configure your container to resolve a request for IFileDownloader by returning a CircuitBreakingFileDownloader that contains the real HttpFileDownloader.

In fact, this approach can be generalized so much that you may look into a general-purpose Circuit Breaker interceptor. Here's an example that uses Castle Windsor.