Store connection in Thread local or instance instead of class

[wgrrrr]

I am writing a multi-tenant front-end application which queries a back-end service. The API is authenticated via HTTP basic, which I have implemented in json_api_client using a Faraday middleware:

self.connection do |connection|
  connection.use Faraday::Request::BasicAuthentication, api_key, api_secret
end

The problem is that this connection is stored as a class variable and is not thread safe. I considered just calling self.connection(true) to rebuild it on every request, but I believe race conditions will emerge. Ultimately, the design is the problem. I'd love to be able to store a connection as a Thread local variable, or worst case as an instance variable.

I'd like to open a conversation with any interested parties to discuss whether there might be a better approach, or if we should consider re-designing the storage of the connection.

[IvanGrgurevic]

I'm having the same issue on my project. Right now I'm resetting the connection before each set of new tenant requests, and race conditions have appeared. I would really like that the connection becomes thread safe.

[janvarljen]

@wgrrrr did you manage to build your multi-tenant app around this? I have the exact same problem :(

[wgrrrr]

I forked this repo and used the Request Store gem to store the connection in a Thread local rather than a class variable.

[janvarljen]

@wgrrrr thank you for the response. I ended up with the same approach but I store the site and connection_options in a Thread also because header token and site url is also changing because of the multi-tenant approach.

Furthermore, I reset the connection on every config change because it's cached internally.

Besides this, the gems looks solid. Too bad it's not maintained with more devotion...

[alachaum]

In case it might be helpful to anyone, here is how we've tackled the problem:

# The Base class is enabling multi-tenancy on the client
# itself (allowing use of different api keys based on context)
class Base < JsonApiClient::Resource
  # Yield a dup class with the proper site and credentials
  # configured
  def self.with_connection(site, api_key, api_secret)
    @klass_cache ||= {}
    digest = Digest::SHA256.hexdigest("#{site}_#{api_key}_#{api_secret}")
    
    @klass_cache[digest] ||= begin
      # Clone the class
      klass = dup
      
      # Set its resource name. This method is based on 'name'
      # which is unset due to the duplication
      klass_name = name
      res_name = resource_name
      klass.define_singleton_method(:resource_name) do
        res_name
      end
      
      # Define the class name. This is used for introspection.
      klass.define_singleton_method(:name) do
        klass_name
      end
      
      # Configure site and authentication
      # The 'true' flag ensures the connection is rebuilt in case
      # the original class was configured with default connection settings
      klass.site = site
      klass.connection(true) do |connection|
        connection.use Faraday::Request::BasicAuthentication, api_key, api_secret
      end
      
      klass
    end
    
    # Yield the duped class
    yield(@klass_cache[digest])
  end
end

All our jsonapi resources inherit from this class. We can then do:

MyResource.with_connection(site, api_key, api_secret) { |k| k.find(some_id) }

MyResource.with_connection(other_site, other_api_key, other_api_secret) { |k| k.find(some_id) }

It looks hacky for sure but so far it has worked well on our side. And no need to rely on Thread.local.

[mltsy]

This solution seems like it could get complex pretty fast, particularly if you happen to be working with more than one resource in a single method. It would be slightly better if this could be applied to the Base class somehow for use in a Rails around_action filter, for instance.

But best of all, I think, would be to implement it as ActiveResource did, using thread-local attributes for the connection and connection details: rails/activeresource@538588d

[mltsy]

I'm not sure #280 resolves this... entirely. This issue was created as an abstraction of the issue in #215 which is talking about setting the .site for a connection, not headers (#280 makes headers inheritable and overridable).

Does #280 somehow actually allow you to set .site dynamically in each thread?

[jsmestad]

@mltsy can you take a look at that PR on JSONAPI::Consumer?

[f-mer]

This issues really deals with two different topics. Swaping out the whole connection is a different thing than using different headers on each request.

#280 solves the initial Issue of passing an api key and secret via headers. Swaping out the site on a per request base should be discussed in #215.

[mltsy]

@f-mer I agree on your first point :) But this issue (#255) is also about connections, not headers. I believe PR #280 is unrelated to both this issue and issue #215 (which are both about connections being thread-safe) ;)

[mltsy]

(Sorry, #215 is not specifically about thread-safety, but both issues concern connections rather than headers in any case)