Fix Stale Connections (#10)

- Primarily ensures the new access token is used when it's automatically refreshed.
- Improves response handling.
- Minor refactor and cleanup.

.rubocop.yml

@@ -29,5 +29,8 @@ Naming/FileName:
     - 'lib/help_scout-sdk.rb'
     - 'spec/**/*_spec.rb'
 
+Naming/MemoizedInstanceVariableName:
+    EnforcedStyleForLeadingUnderscores: required
+
 Style/Documentation:
   Enabled: False

help_scout-sdk.gemspec

@@ -11,7 +11,7 @@ Gem::Specification.new do |spec|
   spec.email         = ['[email protected]']
 
   spec.summary       = 'Ruby Help Scout SDK'
-  spec.description   = 'Ruby Help Scout SDK.'
+  spec.description   = 'Ruby Help Scout (aka HelpScout) SDK.'
   spec.homepage      = 'https://github.com/taxjar/help_scout-sdk'
   spec.license       = 'MIT'
 

lib/help_scout-sdk.rb

@@ -35,7 +35,7 @@ def access_token
     end
 
     def api
-      @api ||= HelpScout::API.new
+      @_api ||= HelpScout::API.new
     end
 
     def app_id
@@ -47,7 +47,7 @@ def app_secret
     end
 
     def configuration
-      @configuration ||= Configuration.new
+      @_configuration ||= Configuration.new
     end
 
     def configure
@@ -59,8 +59,8 @@ def default_mailbox
       configuration.default_mailbox
     end
 
-    def refresh!
-      @api = nil
+    def reset!
+      @_api = HelpScout::API.new
     end
   end
 end

lib/help_scout/api.rb

@@ -30,29 +30,30 @@ def put(path, params)
 
     private
 
-    def handle_response(result) # rubocop:disable AbcSize
-      case result.status
-      when 400 then raise BadRequest, result.body&.dig('validationErrors')
-      when 401 then raise NotAuthorized, result.body&.dig('error_description')
-      when 404 then raise NotFound, 'Resource Not Found'
-      when 429 then raise ThrottleLimitReached, result.body&.dig('error')
-      when 500, 501, 503 then raise InternalError, result.body&.dig('error')
+    def handle_response(result) # rubocop:disable AbcSize, Metrics/MethodLength
+      if (200...300).include? result.status
+        HelpScout::Response.new(result)
+      else
+        case result.status
+        when 400 then raise BadRequest, result.body&.dig('validationErrors')
+        when 401 then raise NotAuthorized, result.body&.dig('error_description')
+        when 404 then raise NotFound, 'Resource Not Found'
+        when 429 then raise ThrottleLimitReached, result.body&.dig('error')
+        else raise InternalError, result.body
+        end
       end
-
-      HelpScout::Response.new(result)
     end
 
     def new_connection
-      HelpScout::API::Client.new.authorized_connection
+      HelpScout::API::Client.new.connection
     end
 
     def send_request(action, path, params)
-      connection = new_connection
-      response = connection.send(action, path, params.compact)
+      response = new_connection.send(action, path, params.compact)
 
       if response.status == 401
-        HelpScout::API::AccessToken.refresh!
-        response = connection.send(action, path, params.compact)
+        access_token.invalidate!
+        response = new_connection.send(action, path, params.compact)
       end
 
       handle_response(response)

lib/help_scout/api/access_token.rb

@@ -5,15 +5,14 @@ class API
     class AccessToken
       class << self
         def create
-          connection = HelpScout::API::Client.new.unauthorized_connection
+          connection = HelpScout::API::Client.new(authorize: false).connection
           response = connection.post('oauth2/token', token_request_params)
 
           case response.status
-          when 429 then raise ThrottleLimitReached, response.body&.dig('error')
-          when 500, 501, 503 then raise InternalError, response.body&.dig('error')
+          when 200 then new HelpScout::Response.new(response).body
+          when 429 then raise HelpScout::API::ThrottleLimitReached, response.body&.dig('error')
+          else raise HelpScout::API::InternalError, "unexpected response (status #{response.status})"
           end
-
-          new HelpScout::Response.new(response).body
         end
 
         def refresh!
@@ -23,29 +22,41 @@ def refresh!
         private
 
         def token_request_params
-          @token_request_params ||= {
+          @_token_request_params ||= {
             grant_type: 'client_credentials',
             client_id: HelpScout.app_id,
             client_secret: HelpScout.app_secret
           }
         end
       end
 
-      attr_reader :expires_at, :expires_in, :value
+      attr_accessor :invalid
+      attr_reader :expires_in, :value
 
       def initialize(params)
         @value = params[:access_token]
-        expires_in = params[:expires_in]
-
-        return unless expires_in
+        @expires_in = params[:expires_in]
+      end
 
-        @expires_in = expires_in
-        @expires_at = Time.now.utc + expires_in
+      def expires_at
+        @_expires_at ||= Time.now.utc + expires_in
       end
 
       def expired?
         Time.now.utc > expires_at
       end
+
+      def invalid?
+        invalid
+      end
+
+      def invalidate!
+        self.invalid = true
+      end
+
+      def stale?
+        invalid? || expired?
+      end
     end
   end
 end

lib/help_scout/api/client.rb

@@ -3,29 +3,37 @@
 module HelpScout
   class API
     class Client
-      def authorized_connection
-        @authorized_connection ||= begin
-          HelpScout::API::AccessToken.refresh! if HelpScout.access_token.nil?
-          build_connection
-        end
+      attr_reader :authorize
+
+      def initialize(authorize: true)
+        @authorize = authorize
       end
 
-      def unauthorized_connection
-        @unauthorized_connection ||= begin
-          build_connection(authorize: false)
+      def connection
+        @_connection ||= begin
+          HelpScout::API::AccessToken.refresh! if authorize? && token_needs_refresh?
+          build_connection
         end
       end
 
       private
 
-      def build_connection(authorize: true)
+      def authorize?
+        authorize
+      end
+
+      def build_connection
         Faraday.new(url: BASE_URL) do |conn|
           conn.request :json
-          conn.authorization(:Bearer, HelpScout.access_token.value) if authorize && HelpScout.access_token&.value
+          conn.authorization(:Bearer, HelpScout.access_token.value) if authorize? && HelpScout.access_token&.value
           conn.response(:json, content_type: /\bjson$/)
           conn.adapter(Faraday.default_adapter)
         end
       end
+
+      def token_needs_refresh?
+        HelpScout.access_token.nil? || HelpScout.access_token.stale?
+      end
     end
   end
 end

lib/help_scout/configuration.rb

@@ -6,6 +6,8 @@ class Configuration
     attr_reader :access_token
 
     def access_token=(token_value)
+      return unless token_value
+
       @access_token = HelpScout::API::AccessToken.new(access_token: token_value)
     end
   end

lib/help_scout/conversation.rb

@@ -61,7 +61,7 @@ def initialize(params)
     end
 
     def populated_threads
-      @populated_threads ||= HelpScout::Thread.list(id)
+      @_populated_threads ||= HelpScout::Thread.list(id)
     end
 
     def update(operation, path, value = nil)

lib/help_scout/mailbox.rb

@@ -27,11 +27,11 @@ def initialize(params)
     end
 
     def fields
-      @fields ||= HelpScout.api.get(fields_path).embedded[:fields]
+      @_fields ||= HelpScout.api.get(fields_path).embedded[:fields]
     end
 
     def folders
-      @folders ||= HelpScout::Folder.list(id: id)
+      @_folders ||= HelpScout::Folder.list(id: id)
     end
 
     private

lib/help_scout/response.rb

@@ -10,7 +10,7 @@ def initialize(response)
     end
 
     def body
-      @body ||= response.body.deep_transform_keys { |key| key.to_s.underscore.to_sym }
+      @_body ||= response.body.deep_transform_keys { |key| key.to_s.underscore.to_sym }
     end
 
     def embedded

lib/help_scout/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module HelpScout
-  VERSION = '1.0.0'
+  VERSION = '1.0.1'
 end

spec/spec_helper.rb

@@ -31,7 +31,7 @@ def model_name
 end
 
 def logger
-  @logger ||= Logger.new($stdout, level: ENV.fetch('LOG_LEVEL', 'INFO'))
+  @_logger ||= Logger.new($stdout, level: ENV.fetch('LOG_LEVEL', 'INFO'))
 end
 
 def valid_access_token
@@ -111,7 +111,7 @@ def valid_access_token
   end
 
   config.before(:each) do
-    HelpScout.refresh!
+    HelpScout.reset!
   end
 
   config.before(:each, :unit) do