Alternative to using Thread.current in API wrapper for Rails

Question

I've developed an application that allows our customers to create their own membership protected websites. My application then connects to an outside API service (customer specific api_key/api_url) to sync/update/add data to this other service. Well, I've had an API wrapper written for this other service that has worked up to this point. However, I'm now seeing very random drops where the connection is nil. Here is how I'm currently using the connection:

I have a xml/rpc connection class

class ApiConnection
  attr_accessor :api_url, :api_key, :retry_count

  def initialize(url, key)
    @api_url = url
    @api_key = key
    @retry_count = 1
  end

  def api_perform(class_type, method, *args)
    server = XMLRPC::Client.new3({'host' => @api_url, 'path' => "/api/xmlrpc", 'port' => 443, 'use_ssl' => true})
    result = server.call("#{class_type}.#{method}", @api_key, *args)
    return result
  end
end

I also have a module that I can include in my models to access and call the api methods

module ApiService

  # Set account specific ApiConnection obj
  def self.set_account_api_conn(url, key)
    if ac = Thread.current[:api_conn]
      ac.api_url, ac.api_key = url, key
    else
      Thread.current[:api_conn] = ApiConnection.new(url, key)
    end
  end

  ########################
  ###  Email Service   ###
  ########################

  def api_email_optin(email, reason)
    # Enables you to opt contacts in
    Thread.current[:api_conn].api_perform('APIEmailService', 'optIn', email, reason)
  end

  ### more methods here ###

end

Then in the application controller I create a new ApIConnection object on every request using a before filter which sets the Thread.current[:api_conn]. This is because I have hundreds of customers each with their own api_key and api_url, using the application at the same time.

# In before_filter of application controller
def set_api_connection
  Thread.current[:api_conn] = ApiService.set_account_api_conn(url, key)
end

Well my question is that I've read that using Thread.current is not the most ideal way of handling this, and I'm wondering if this is the cause for the ApiConnection to be nil on random requests. So I would like to know how I could better setup this wrapper.

Answer 1

Answer 1

I'd expect that the problem is the next request coming before the connection has finished, and then the before_filter overwrites the connection for the still ongoing connection. I'd try to stay away from threads. It's easier to fork_off, but there's certain caveats to that as well, especially regarding performance.

I try to move logic like this over to a background job of some sort. A common solution is delayed job https://github.com/collectiveidea/delayed_job that way you don't have to mess with threads and it's more robust and easy to debug. You can then start background jobs to asynchronously sync the service whenever somebody logs in.

@account.delay.optin_via_email(email,user)

This will serialize the account, save it to the job queue, where it will be picked up by delayed job unserialized and the method after delay will be called. You can have any number of background jobs, and even some job queues dedicated to certain types of actions (via using job priorities - let's say two bj for high prio jobs and one dedicated to low prio jobs)

Answer 2

Just make it as an object instead

def before_filter
  @api_connection =  ApiConnection.new(url, key)
end

then you can use that connection in your controller methods

def show
   #just use it straight off
   @api_connection.api_perform('APIEmailService', 'optIn', email, reason)
   # or send the connection as a parameter to some other class
   ApiService.do_stuff(@api_connection)
end

Answer 3

The easiest solution might just be to create the api connection whenever you need it

class User < ActiveRecord::Base
  def api_connection 
    # added caching of the the connection in object
    # doing this makes taking a block a little pointless but making methods take blocks
    # makes the scope of incoming variables more explicit and looks better imho
    # might be just as good to not keep @conn as an instance variable
    @conn = ApiConnection.new(url, key) unless @conn 
    if block_given?
      yield(@conn)
    else
      @conn
    end
  end
end

that way you can easily just forget about the creation of the connection and have a fresh one handy. There might be performance penalities with this but I suspect that they are insignificant unless there's an extra login request

@user.api_connection do { |conn| conn.optin_via_email(email,user) }