Subscribe to the Hired Download: our newsletter for top talent like you!

screen-shot-2018-02-26-at-10-55-12-am

Always Wrap Third-Party API Clients

Have you seen Ruby code like this?

class AwardsController < ApplicationController
  def create
   User.find(params[:user_id]).give_award

   alert_user_on_slack('Congratulations!')
 end

 private

 def alert_user_on_slack(text)
   slack_client = Slack::Web::Client.new
   im_id = slack_client.im_open(user: @user.slack_id).fetch(:channel).fetch(:id)
   slack_client.chat_postMessage(channel: im_id, text: text)
   slack_client.chat_postMessage(channel: '#awards', text: text)
 end
end

And here are the tests:

require 'rails_helper'

describe AwardsController do
 context '#create' do
   it 'awards the correct user' do
     stub_slack_client

     post :create, user_id: 3

     expect(User.find(3).awards.count).to eq(1)
   end
   it 'DMs the user' do
     client = stub_slack_client
     post :create, user_id: 3
     expect(client).to have_received(:chat_postMessage).with(
       hash_including(text: 'Success!')
     )
   end

   def stub_slack_client
     client = instance_double(Slack::Web::Client, chat_postMessage: nil)
     allow(Slack::Web::Client).to receive(:new).and_return(client)
     allow(client).to receive(:im_open).and_return(channel: { id: 'M123' })
     client
   end
 end
end

(This is adapted from a real Slack bot that I wrote to help teammates publicly award each other at Hired.)

It doesn’t look so bad at first glance, but there are a few problems that show up on closer examination:

  1. The Slack code operates at a lower level of abstraction than the Rails controller code.
  2. If we need to do the same (or even a slightly different) thing elsewhere, it’s hard to reuse. Rails controllers are isolated from the rest of the app and don’t lend themselves well to object-oriented design principles.
  3. The testing is complex: we need to do some deep, chained stubbing in order to verify that Slack is running the right code.

The Wrong Abstraction

It can be hard to find the right abstraction for your code. In this case, there are a few levels of abstraction (as well as separate concerns):

* The controller code is high-level

* The Slack code is lower-level and requires the controller (and tests) to know a lot about the Slack API

Slack’s API client matches the Ruby methods (like `chat_postMessage`) to their URLs (like

[`chat.postMessage`](https://api.slack.com/methods/chat.postMessage)). But your application code doesn’t need to know about the Slack API’s URLs. It should operate at a higher level of abstraction, like “alert the current user”. To wrap that logic, create a new class with high-level methods that understands how to translate them into one or more Slack API calls.

That new class might look something like this:

# app/models/slack_client.rb

class SlackClient
 def initialize(text)
   @client = Slack::Web::Client.new
 end

 def alert_user_to_award(recipient_id:, text:)
   message(recipient_id: '#awards', text: text)
   direct_message(recipient_id: recipient_id, text: text)
 end

 def message(recipient_id:, text:)
   @client.chat_postMessage(channel: recipient_id, text: text)
 end

 def direct_message(recipient_id:, text:)
   im_id = @client.im_open(user: recipient_id).fetch(:channel).fetch(:id)
   message(recipient_id: im_id, text: text)
 end
end

And the controller now looks like this:

diff
class AwardsController < ApplicationController
  def create
    User.find(params[:user_id]).give_award

-    alert_user_on_slack('Success!')
+    slack_client = SlackClient.new
+    slack_client.alert_user_to_award(
+      recipient_id: @user.slack_id
+      text: 'Success!'
+    )
  end
-
-  private
-
-  def alert_user_on_slack(text)
-    slack_client = Slack::Web::Client.new
-    im_id = slack_client.im_open(user: @user.slack_id).fetch(:channel).fetch(:id)
-    slack_client.chat_postMessage(channel: im_id, text: text)
-  end
end

Let’s look at the new tests:

require 'rails_helper'

describe AwardsController do
 context '#create' do
   it 'awards the correct user' do
     stub_slack_client

     post :create, user_id: 3

     expect(User.find(3).awards.count).to eq(1)
   end

   it 'DMs the user' do
     client = stub_slack_client

     post :create, user_id: 3

     expect(client).to have_received(:alert_user_to_award).with(
       hash_including(text: 'Success!')
     )
   end

   def stub_slack_client
     client = instance_double(SlackClient, alert_user_to_award: nil)
     allow(SlackClient).to receive(:new).and_return(client)
     client
   end
 end
end

This test is now separated from the details of interacting with Slack’s API. If we decide to alert the user in a different way, or if the Slack API changes, this test does not need to change at all. `SlackClient`’s internals will change, and the tests for `SlackClient` will change, but the controller tests are correctly shielded from that churn.

This fixes each of the problems listed above:

  1. The new code operates at a high level of abstraction now. The code talks about “alerting the user”, not specific API calls.
  2. The Slack code is in its own class and can be reused anywhere in the app without the controller’s baggage.
  3. The tests are easier to write and maintain. When testing the controller, the `SlackClient` code can be stubbed out without any references to `chat_postMessage`. When testing the Slack client, there’s no controller dependency at all so we can easily unit test it.

There’s an additional benefit as well: refactoring will be easier since the Slack code is centralized in `SlackClient`. If we add caching or other optimizations, it will benefit everywhere that interacts with Slack. If we scattered direct `Slack::Web::Client` calls all over, it would be much harder to find all of them and refactor in a few months.

Don’t forget: if you’re using a 3rd-party API client, don’t wait: wrap it in your own class on first use!

candidate-banner-2