adding a mutex for certificate fetches

and DRYing up the class chooser based
on the configuration

Author: Jay OConnor

README.md

@@ -112,7 +112,7 @@ FirebaseIdToken::Certificates.find('ec8f292sd30224afac5c55540df66d1f999d')
 
 #### Downloading in Rails
 If you pass in the `cache_store` configuration option (see [configuration](#configuration)), the certificates will be
-requested at runtime when needed using `ActiveSupport::Cache.fetch()` and you can ignore this section.
+requested at runtime when needed and you can ignore this section.
 
 If you are using Rails, it's clever to download certificates in a cron task, you can use [whenever](https://github.com/javan/whenever).
 

lib/firebase_id_token/certificates.rb

@@ -47,7 +47,7 @@ class Certificates
     # @return [nil, Hash]
     # @see Certificates.request!
     def self.request
-      new.request
+      new_child.request
     end
 
     # Triggers a HTTPS request to Google's x509 certificates API. If it
@@ -61,7 +61,7 @@ def self.request
     # {Exceptions::CertificatesTtlError}. You are mostly like to never face it.
     # @return [Hash]
     def self.request!
-      new.request!
+      new_child.request!
     end
 
     # @deprecated Use only `request!` in favor of Ruby conventions.
@@ -71,7 +71,7 @@ def self.request_anyway
       warn 'WARNING: FirebaseIdToken::Certificates.request_anyway is '\
         'deprecated. Use FirebaseIdToken::Certificates.request! instead.'
 
-      new.request!
+      new_child.request!
     end
 
     # Returns `true` if there's certificates data in the cache, `false` otherwise.
@@ -80,7 +80,7 @@ def self.request_anyway
     #   FirebaseIdToken::Certificates.request
     #   FirebaseIdToken::Certificates.present? #=> true
     def self.present?
-      ! new.local_certs.empty?
+      ! new_child.local_certs.empty?
     end
 
     # Returns an array of hashes, each hash is a single `{key => value}` pair
@@ -94,7 +94,7 @@ def self.present?
     #   certs = FirebaseIdToken::Certificates.all
     #   certs.first #=> {"1d6d01c7[...]" => #<OpenSSL::X509::Certificate[...]}
     def self.all
-      new.local_certs.map { |kid, cert|
+      new_child.local_certs.map { |kid, cert|
         { kid => OpenSSL::X509::Certificate.new(cert) } }
     end
 
@@ -110,7 +110,7 @@ def self.all
     #   cert = FirebaseIdToken::Certificates.find "1d6d01f4w7d54c7[...]"
     #   #=> <OpenSSL::X509::Certificate: subject=#<OpenSSL [...]
     def self.find(kid, raise_error: false)
-      certs = new.local_certs
+      certs = new_child.local_certs
       raise Exceptions::NoCertificatesError if certs.empty?
 
       return OpenSSL::X509::Certificate.new certs[kid] if certs[kid]
@@ -145,8 +145,13 @@ def self.find!(kid)
     # @return [Fixnum]
     def self.ttl
       # call a child class based on the configuration
-      return FirebaseIdToken::Certificates::Redis.ttl if FirebaseIdToken.configuration.redis
-      FirebaseIdToken::Certificates::ActiveSupport.ttl
+      klass = FirebaseIdToken.configuration.klass
+      klass.ttl
+    end
+
+    def self.new_child
+      klass = FirebaseIdToken.configuration.klass
+      klass.new
     end
 
     # Sets two instance attributes: `:cach_store` and `:local_certs`. Those are

lib/firebase_id_token/certificates/active_support.rb

@@ -22,23 +22,54 @@ def self.ttl
     private
 
     def read_certificates
-      entry = cache_store.fetch('certificates', race_condition_ttl: RACE_CONDITION_TIME) do
-        request
+      entry = cache_store.read('certificates')
+      if entry.nil?
+        lock do
+          request!
+        end
+        entry = cache_store.read('certificates')
       end
       certs = {}
       certs = JSON.parse(JSON.parse(entry)["data"]) if entry
       certs
-    rescue StandardError
+    rescue StandardError => e
       return {}
     end
 
+    # we can't use ActiveSupport's fetch, because we need to set the expiration time of
+    # the key based on a value we read from the request. This is a rudimentary mutex instead
+    def lock
+      acquire_lock
+      yield
+    ensure
+      release_lock
+    end
+
+    def acquire_lock
+      maybe_sleep
+      cache_store.write('certificate_lock', expires_in: 5.seconds)
+    end
+
+    def maybe_sleep
+      iteration = 0
+      while cache_store.exist?('certificate_lock')
+        iteration += 1
+        sleep 1
+        break if iteration > 5
+      end
+    end
+
+    def release_lock
+      cache_store.delete('certificate_lock')
+    end
+
     def save_certificates
       expires_at = Time.now.to_i + ttl
       # set the expiration of the key to the certification expiration - RACE_CONDITION_TIME, so that the entry
       # will be expired before the certificate is
       cache_store.write 'certificates', { data: @request.body, expires_at: expires_at }.to_json,
         expires_in: (ttl - RACE_CONDITION_TIME)
-      @local_certs = read_certificates
+      @local_certs = @request.body
     end
 
     def ttl

lib/firebase_id_token/configuration.rb

@@ -4,11 +4,22 @@ module FirebaseIdToken
   LIB_PATH = File.expand_path('../../', __FILE__)
 
   class Configuration
-    attr_accessor :redis, :project_ids, :certificates, :cache_store
+    attr_accessor :redis, :project_ids, :cache_store
 
     def initialize
       @project_ids = []
-      @certificates = FirebaseIdToken::Certificates
+    end
+
+    def certificates
+      klass
+    end
+
+    def certificates=(value)
+      @certificates = klass
+    end
+
+    def klass
+      redis ? FirebaseIdToken::Certificates::Redis : FirebaseIdToken::Certificates::ActiveSupport
     end
   end
 end