0

I'm developing a set of classes designed to communicate with external APIs, and I'm running into trouble with how to properly structure everything for proper loose coupling and unit testing.

Currently, each API we need to talk to has a distinct class, which implements an interface a bit like this:

public interface IApiIntegration
{
    Task<string> SearchApi (List<string> searchValues);
    Task<string> GetFromApi(string idToGet);
    Task<bool> PostToApi(PostObject api);
}

Each api class inherits from a base abstract with implements this interface. That class also contains a number of helper functions which are only relevant to handling data coming to and from Apis.

Beneath the public PostToApi method of each class there are also a bunch of helper functions to build the object to be posted. These are often quite complicated, and could really do with testing. However, they're specific to the class in question and are thus private.

Inside every public function on IApiIntegration there is also, of course, a call to an external Api. For example it might look something like:

public override async Task<string> GetFromApi(string id)
{
    string result = "";
    string path = $"{integration.RootUrl}items/{id}?username={integration.Username}&key={integration.Password}";

    // client is a static instance of HttpClient
    HttpResponseMessage response = await client.GetAsync(path);
    if (response.IsSuccessStatusCode)
    {
        result = await response.Content.ReadAsStringAsync();
    }

    return result;
}

This leaves me with two problems:

1) It feels right that the helper methods in the base class and the individual classes should be open to unit testing, but also that they should be protected/private. Something, therefore, is clearly wrong with the structure.

2) It's obviously wrong to be testing external APIs so I need somehow to bypass or mock out those dependencies. But that's not possible in this structure.

How can I refactor and restructure this to ensure everything is open for unit tests?

  • Have you considered decoupling Task from HttpClient? Set a new abstraction between these two. Something like Datasource. But do it only if Task has any logic (business) that worth to test. That might not be the case of the example. – Laiv Dec 3 '18 at 15:53
1

... there are also a bunch of helper functions to build the object to be posted. These are often quite complicated, and could really do with testing.

Building an object, especially a complicated one, is logic which should not be hidden from the testing harness. A public function should exist that can build and return the object. It is effectively a constructor and likely should be a pure function (dependent only on input parameters and doesn't change any state.)

Similarly, processing API data is logic that should not be hidden. Again, a publicly available function that takes the kind of data you expect the API to emit and returns objects after processing the data is the best way to go.

Then your getFromAPI function will merely call the object builder to make the proper input and then call the data processor to create the correct objects. This function will end up being so simple that you shouldn't feel the need to test it (IMHO.)

The key point is that you should not feel the need to test the API functions. If you are feeling especially cautious, you might want to test that they are called by using a test double/mock, but that's about it. All you need to test is that you are creating valid object to pass into the API and that when the API returns valid data, you are processing that data correctly.

Some good videos on the subject:

  • Yes - this question is a bit old now & I had to move on and this was the approach I took. Remembering SRP it made sense to split things into smaller classes with open public functions, available to test. – Matt Thrower Dec 23 '18 at 10:31
1

When you have an external API you cant control, ie it doesnt have a test instance with static data, your best bet is to record and replay the network traffic from real calls.

Once you have captured this data you can setup a mock server which listens for incomming requests, matches them against its known requests and returns the matching response.

Now you can test your client against this server without having to expose internal methods

1

I have a small application in Ruby that addresses a similar issue using Alistair Cockburn's Ports and Adapters (hexagonal) design pattern. The application sends alerts to individual recipients on their communication platform of choice (e.g. SMS, email, Twitter, etc.). Here's what it looks like (in Ruby/Rails):

class Recipient < ActiveModel
  # Recipient model has two attributes:
  #  - channel: one of sms, email, twitter, messenger, whatsapp
  #  - address: one of phone number, email address, twitter handle, etc.

  NOTIFIER_CLASSES = {
    sms: SMSNotifier, 
    email: EmailNotifier,
    twitter: TwitterNotifier,
    messenger: FacebookMessengerNotifier,
    whatsapp: WhatsNotifier
  }.freeze

  def notify(message)
    notifier.notify(address, message)
  end

  private

  def notifier
    NOTIFIER_CLASSES[channel].new(address)
  end
end

Here, I test that I'm instantiating the correct notifier and that I'm passing it the correct arguments. Like so (in RSpec):

describe Recipient do
  subject(:recipient) { described_class.new(recipient_attributes) }

  let(:recipient_attributes) { { channel: channel, address: address } }
  let(:channel) { 'some channel' }
  let(:address) { 'some address' }
  let(:message) { 'some message' }

  describe "#notify" do
    subject(:notify) { recipient.notify(message) }

    context 'when channel is sms' do
      let(:channel) { :sms }

      before do
        allow(SMSNotifier).to receive(:notify)
        notify
      end

      it 'uses the correct notifier with the correct attributes' do
        expect(SMSNotifier).to have_received(:notify).with(address, message)
      end
    end

    context 'when channel is email' do
      let(:channel) { :email }

      before do
        allow(EmailNotifier).to receive(:notify)
        notify
      end

      it 'uses the correct notifier with the correct attributes' do
        expect(EmailNotifier).to have_received(:notify).with(address, message)
      end
    end
  end
end

And, so on... (Note: in reality, these tests would be dried up with shared examples.)

The notifiers all define the same notify method (duck typing in Ruby, though you would want to use an interface in a statically typed language). They look like this:

class SMSNotifier
  FROM = '+18005551212'.freeze

  def notify(address, message)
    messages.create(from: FROM, to: address, body: message)
  end

  private

  def messages
    client.api.account.messages
  end      

  def client
    Twilio::REST::Client.new(ENV['ACCOUNT_SID'], ENV['AUTH_TOKEN'])
  end
end

class EmailNotifier
  SENDGRID_ENDPOINT = 'https://api.sendgrid.com/v3/mail/send'.freeze
  FROM = 'noreply@example.com'.freeze

  def notify(address, message)
    RestClient.post(
      SENDGRID_ENDPOINT,
      {
        to: [{ email: address }],
        subject: 'Alert',
        content: [{ value: message, type: 'text/plain' }]
      },
      'Authorization' => "Bearer #{ENV['SENDGRID_API_KEY']}"
    )
  end
end

As you can see, the internals are quite different for each Notifier, depending on the underlying API. In these cases, I use mocks to ensure that I am making the correct API calls with the correct parameters. I trust that those APIs are fully tested. There's no need for me to make a round trip in a unit test.

describe SMSNotifier do
  subject(:notifier) { described_class.new }

  describe "#notify" do
    subject(:notify) { notifier.notify(address, message) }

    let(:address) { '+12345678900' }
    let(:message) { 'some message' }

    let(:client) { instance_double(Twilio::REST::Client, api: api) }
    let(:api) { instance_double('api', account: account) }
    let(:account) { instance_double('account', messages: messages) }
    let(:messages) { instance_double('messages', create: true) }

    before do
      allow(Twilio::REST::Client).to receive(:new).and_return(client)
      notify
    end

    it 'uses the correct notifier with the correct attributes' do
      expect(Twilio::REST::Client).to have_received(:new).with(account_sid, auth_token)
    end

    it 'makes the correct api call with the correct attributes' do
      expect(messages).to have_received(:create).with(address, message)
    end
  end
end

describe EmailNotifier do
  subject(:notifier) { described_class.new }

  describe "#notify" do
    subject(:notify) { notifier.notify(address, message) }

    let(:address) { 'john.doe@example.com' }
    let(:message) { 'some message' }

    let(:email_attributes) do
      {
        to: [{ email: address }],
        subject: 'Alert',
        content: [{ type: 'text/plain', value: message }]
      }
    end
    let(:headers) { { 'Authorization' => "Bearer sendgrid_api_key" } }

    before do
      allow(RestClient).to receive(:post)
      notify
    end

    it 'uses the correct notifier with the correct attributes' do
      expect(RestClient).to have_received(:post).with(
        'https://api.sendgrid.com/v3/mail/send',
        email_attributes,
        headers
      )
    end
  end
end

So, the pattern boils down to ports and adapters, where the Notifiers are the ports, and the Twilio and SendGrid APIs are adapters for the specific services.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.