3

For development, I have a number of AWS profiles, I use the AWS Profile section in appsettings.json to define the profile I want use:

"AWS": {
    "Profile": "CorpAccount",
    "Region": "us-east-1"
  }

Since this is not the default profile, I need the context to use a named profile when I debug and run unit tests (xunit). What I'd like to know is what is the best practice to config the profile.

Here is a class that shows three approaches (two work in local):

public class EmailQueueService : IEmailQueueService
{
    private IConfiguration _configuration;
    private readonly ILogger _logger;

    public EmailQueueService(IConfiguration configuration, ILogger<EmailQueueService> logger)
    {
        _configuration = configuration;
        _logger = logger;
    }

    public async Task<bool> Add1Async(ContactFormModel contactForm)
    {
        var sqsClient = new AmazonSQSClient();

        var sendRequest = // removed for clarity

        var response = await sqsClient.SendMessageAsync(sendRequest);

        return response.HttpStatusCode == System.Net.HttpStatusCode.OK;
    }

    public async Task<bool> Add2Async(ContactFormModel contactForm)
    {
        var sqsClient = _configuration.GetAWSOptions().CreateServiceClient<IAmazonSQS>();

        var sendRequest = // removed for clarity

        var response = await sqsClient.SendMessageAsync(sendRequest);

        return response.HttpStatusCode == System.Net.HttpStatusCode.OK;
    }

    public async Task<bool> Add3Async(ContactFormModel contactForm)
    {
        var sqsClient = new AmazonSQSClient(credentials: Common.Credentials(_configuration));

        var sendRequest = // removed for clarity

        var response = await sqsClient.SendMessageAsync(sendRequest);

        return response.HttpStatusCode == System.Net.HttpStatusCode.OK;
    }

    public AWSCredentials Credentials(IConfiguration config)
    {
        var chain = new CredentialProfileStoreChain();

        if (!chain.TryGetAWSCredentials(config.GetAWSOptions().Profile, out AWSCredentials awsCredentials))
        {
            throw new Exception("Profile not found.");
        }

        return awsCredentials;
    }
}

Results:

  • Add1Async fails locally because it uses the default profile not the "CorpAccount".
  • Add2Async works locally, but is seems like an odd way to create a new instance.
  • Add3Async works locally, but it will likely fail when deployed because config.GetAWSOptions().Profile doesn't exist outside of the local environment.

For completeness, here is a unit test I'm calling it from:

[Fact]
public async void AddAsyncTest()
{
    // Arrange 
    var configuration = TestConfigure.Getconfiguration();

    var service = new EmailQueueService(configuration, Mock.Of<ILogger<EmailQueueService>>());

    // Act
    var result = await service.AddAsync(ContactFormModelMock.GetNew());

    // Assert
    Assert.True(result);
}

public static IConfiguration Getconfiguration()
{
    var builder = new ConfigurationBuilder()
                    .AddJsonFile("appsettings.json", optional: true, reloadOnChange: true)
                    .AddEnvironmentVariables();

    return builder.Build();
}
Nkosi
  • 235,767
  • 35
  • 427
  • 472
Josh
  • 8,219
  • 13
  • 76
  • 123

1 Answers1

9

This is a design issue. You are tightly coupling your code to implementation concerns that make testing your code in isolation difficult.

You first need to refactor out the creation of the client (Implementation Concern) and explicitly inject its abstraction into the dependent class.

There really is no need to be injecting framework concerns like IConfiguration into your services. This can be seen as a code smell where your class is not following the Explicit Dependencies Principle and is misleading about what it actually depends on.

With that, the dependent class simplifies to

public class EmailQueueService : IEmailQueueService {
    private readonly IAmazonSQS sqsClient 
    private readonly ILogger logger;

    public EmailQueueService(IAmazonSQS sqsClient, ILogger<EmailQueueService> logger) {
        this.sqsClient = sqsClient;
        this.logger = logger;
    }

    public async Task<bool> AddAsync(ContactFormModel contactForm) {

        var sendRequest = //...removed for clarity

        var response = await sqsClient.SendMessageAsync(sendRequest);

        return response.HttpStatusCode == System.Net.HttpStatusCode.OK;
    }
}

Now move the creation of the client and its dependency on the options to the composition root, which would be in your start up.

public Startup(IHostingEnvironment env) {
    var builder = new ConfigurationBuilder()
        .SetBasePath(env.ContentRootPath)
        .AddJsonFile("appsettings.json", optional: true, reloadOnChange: true)
        .AddJsonFile($"appsettings.{env.EnvironmentName}.json", optional: true)
        .AddEnvironmentVariables();
    Configuration = builder.Build();
}

IConfiguration Configuration;

public void ConfigureServices(IServiceCollection services) {
    // Add framework services.
    services.AddMvc();

    // Add AWS services
    var options = Configuration.GetAWSOptions();
    services.AddDefaultAWSOptions(options);
    services.AddAWSService<IAmazonSQS>();
    services.AddAWSService<IAmazonDynamoDB>();

    services.AddSingleton<IEmailQueueService, EmailQueueService>();

    //...omitted for brevity
}

Reference Configuring the AWS SDK for .NET with .NET Core

That takes care of cleaning the code so that it can run locally, deployed or when testing.

When testing, you can create the client externally to the subject under test and configure it as needed specifically for the test

public class EmailQueueServiceTests {
    [Fact]
    public async Task Should_AddAsync() {
        // Arrange 
        var configuration = GetConfiguration();
        IAmazonSQS client = configuration.GetAWSOptions().CreateServiceClient<IAmazonSQS>();

        var subject = new EmailQueueService(client, Mock.Of<ILogger<EmailQueueService>>());

        // Act
        var result = await subject.AddAsync(ContactFormModelMock.GetNew());

        // Assert
        Assert.True(result);
    }

    static IConfiguration GetConfiguration() {
        var builder = new ConfigurationBuilder()
            .AddJsonFile("appsettings.json", optional: true, reloadOnChange: true)
            .AddEnvironmentVariables();

        return builder.Build();
    }
}

The IConfiguration could also be completely mocked if so desired or the AWSOptions manually created with the values needed for testing.

Your choices available are now increased and more flexible.

Nkosi
  • 235,767
  • 35
  • 427
  • 472
  • Solid logic and great detail. I had added the IConfiguration into the constructor while trying to figure out how to set the get the non-default profile to load properly. Quick question, I see in service configuration example you setup IEmailQueueService as an AddSingleton. I had setup as AddTransient because it is lightweight and stateless. Did you see something in the implementation that it should be treated as a Singleton instead? The same question for IAmazonSQS, would it be better to setup as Transient: services.AddAWSService(lifetime: ServiceLifetime.Transient); – Josh May 07 '19 at 14:28
  • I'll try out your approach today. – Josh May 07 '19 at 14:33
  • @Josh when I checked the source code of the `AddAWSService` I saw that under the hood they were adding them (the services) as singletons by default, so I just followed suit. It is more a matter of personal preference in this case. I had originally scoped but noticed the singleton and remembered that they don't mix well. – Nkosi May 07 '19 at 14:34
  • @Josh while the email service may be light weight, it's dependencies may not be. If you have to spin up a heavy dependency each time you service is resolved it can be a performance issue. If there are no knock on effects of having a single instance then that is what I would suggest. That is however just my opinion based on my experiences. – Nkosi May 07 '19 at 14:44
  • You solution worked, glad you took a higher level look. I have to wait 3 hours to award bounty. – Josh May 07 '19 at 15:35
  • For the scope, I think transient is a better option, I would be concerned if I used the same service for different queues because they would share the same instance. Good article here about pros and cons: https://dotnetcoretutorials.com/2017/03/25/net-core-dependency-injection-lifetimes-explained/ – Josh May 07 '19 at 15:36