I am cleaning up my code by way of removing duplicates, and found two classes that were almost identical, out of 55 lines, only a single predicate in an if
statement differed between them.
Both classes also had a suite of tests, which were almost identical copies as well.
The two original classes were FooApiAuthenticationProvider
and BarApiAuthenticationProvider
.
Below is my current refactored code using a template-pattern refactoring.
Template
public abstract class ApiAuthenticationProviderTemplate : IApiAuthenticationProvider
{
private readonly IClientRepository _clientRepository;
protected ApiAuthenticationProviderTemplate(IClientRepository clientRepository)
{
if (clientRepository == null)
{
throw new ArgumentNullException("clientRepository");
}
_clientRepository = clientRepository;
}
public GenericPrincipal GetPrincipal(string username, string password)
{
if (string.IsNullOrWhiteSpace(username))
{
throw new ArgumentNullException("username");
}
if (string.IsNullOrWhiteSpace(password))
{
throw new ArgumentNullException("password");
}
var client = clientRepository.GetByUsername(username);
if (client == null && !DoesPasswordMatch(password, client))
{
return null;
}
return PrincipalBuilder.BuildPrinciple(client.LoginIdentifier, client.ID);
}
protected abstract bool DoesPasswordMatch(string password, Client client);
}
FooApiAuthenticationProvider
public sealed class FooApiAuthenticationProvider : ApiAuthenticationProviderTemplate
{
public FooApiAuthenticationProvider(IClientRepository clientRepository)
: base(clientRepostitory)
{
}
protected override bool DoesPasswordMatch(string password, Client client)
{
return client.FooApiPassword == password;
}
}
BarApiAuthenticationProvider
public sealed class BarApiAuthenticationProvider : ApiAuthenticationProviderTemplate
{
public BarApiAuthenticationProvider(IClientRepository clientRepository)
: base(clientRepostitory)
{
}
protected override bool DoesPasswordMatch(string password, Client client)
{
return client.DoesBarServicePasswordMatch(password);
}
}
Both unit test fixtures for each specific class pass, and normally that would be the end of the thought, however, I have to extend the functionality that's common to both authentication providers (so I would obviously implement in the template base class).
My questions are:
Should I refactor the test code to remove test duplication? To test the abstract class somehow, and write specific tests for the
DoesPasswordMatch
methodShould I add new tests to both fixtures to ensure the actual types both work properly? Meaning the tests should have no idea the underlying class is a template, that way if both providers become significantly different where the template no longer makes sense, it could be refactored away without losing code coverage.
Should I leave both test fixtures the same, and just write a new fixture with tests against the template for the new functionality?
Is there something else I am not considering?