Use resolver throughout codebase

app/jobs/security_health_check_job.rb

@@ -1,5 +1,5 @@
 class SecurityHealthCheckJob < ApplicationJob
-  queue_as :default
+  queue_as :scheduled
 
   def perform
     Security::HealthChecker.new.perform

app/models/market_data_importer.rb

@@ -23,7 +23,7 @@ class MarketDataImporter
     end
 
     # Import all securities that aren't marked as "offline" (i.e. they're available from the provider)
-    Security.where.not(offline: true).find_each do |security|
+    Security.online.find_each do |security|
       security.import_provider_prices(
         start_date: get_first_required_price_date(security),
         end_date: end_date,

app/models/security.rb

@@ -9,6 +9,8 @@ class Security < ApplicationRecord
   validates :ticker, presence: true
   validates :ticker, uniqueness: { scope: :exchange_operating_mic, case_sensitive: false }
 
+  scope :online, -> { where(offline: false) }
+
   def current_price
     @current_price ||= find_or_fetch_price
     return nil if @current_price.nil?

app/models/security/provided.rb

@@ -12,7 +12,12 @@ module Security::Provided
     def search_provider(symbol, country_code: nil, exchange_operating_mic: nil)
       return [] if provider.nil? || symbol.blank?
 
-      response = provider.search_securities(symbol, country_code: country_code, exchange_operating_mic: exchange_operating_mic)
+      params = {
+        country_code: country_code,
+        exchange_operating_mic: exchange_operating_mic
+      }.compact_blank
+
+      response = provider.search_securities(symbol, **params)
 
       if response.success?
         response.data.map do |provider_security|

app/models/security/resolver.rb

@@ -108,11 +108,12 @@ class Security::Resolver
     end
 
     def provider_search_result
-      @provider_search_result ||= Security.search_provider(
-        symbol,
+      params = {
         exchange_operating_mic: exchange_operating_mic,
         country_code: country_code
-      )
+      }.compact_blank
+
+      @provider_search_result ||= Security.search_provider(symbol, **params)
     end
 
     # Non-exhaustive list of common country codes for help in choosing "close" matches

app/models/trade_builder.rb

@@ -1,6 +1,8 @@
 class TradeBuilder
   include ActiveModel::Model
 
+  Error = Class.new(StandardError)
+
   attr_accessor :account, :date, :amount, :currency, :qty,
                 :price, :ticker, :manual_ticker, :type, :transfer_account_id
 
@@ -129,6 +131,10 @@ class TradeBuilder
     def security
       ticker_symbol, exchange_operating_mic = ticker.present? ? ticker.split("|") : [ manual_ticker, nil ]
 
+      unless ticker_symbol.present?
+        raise Error, "Ticker symbol is required to create a trade"
+      end
+
       Security.find_or_create_by!(
         ticker: ticker_symbol,
         exchange_operating_mic: exchange_operating_mic

app/models/trade_import.rb

@@ -76,42 +76,25 @@ class TradeImport < Import
   end
 
   private
-    def find_or_create_security(ticker:, exchange_operating_mic:)
-      # Normalize empty string to nil for consistency
-      exchange_operating_mic = nil if exchange_operating_mic.blank?
+    def find_or_create_security(ticker: nil, exchange_operating_mic: nil)
+      return nil unless ticker.present?
 
-      # First try to find an exact match in our DB, or if no exchange_operating_mic is provided, find by ticker only
-      internal_security = if exchange_operating_mic.present?
-        Security.find_by(ticker:, exchange_operating_mic:)
-      else
-        Security.find_by(ticker:)
-      end
+      # Avoids resolving the same security over and over again (resolver potentially makes network calls)
+      @security_cache ||= {}
 
-      return internal_security if internal_security.present?
+      cache_key = [ ticker, exchange_operating_mic ].compact.join(":")
 
-      # If security prices provider isn't properly configured or available, create with nil exchange_operating_mic
-      return Security.find_or_create_by!(ticker: ticker&.upcase, exchange_operating_mic: nil) unless Security.provider.present?
+      security = @security_cache[cache_key]
 
-      # Cache provider responses so that when we're looping through rows and importing,
-      # we only hit our provider for the unique combinations of ticker / exchange_operating_mic
-      cache_key = [ ticker, exchange_operating_mic ]
-      @provider_securities_cache ||= {}
+      return security if security.present?
 
-      provider_security = @provider_securities_cache[cache_key] ||= begin
-        Security.search_provider(
-          ticker,
-          exchange_operating_mic: exchange_operating_mic
-        ).first
-      end
+      security = Security::Resolver.new(
+        ticker,
+        exchange_operating_mic: exchange_operating_mic.presence
+      ).resolve
 
-      return Security.find_or_create_by!(ticker: ticker&.upcase, exchange_operating_mic: nil) if provider_security.nil?
+      @security_cache[cache_key] = security
 
-      Security.find_or_create_by!(ticker: provider_security[:ticker]&.upcase, exchange_operating_mic: provider_security[:exchange_operating_mic]&.upcase) do |security|
-        security.name = provider_security[:name]
-        security.country_code = provider_security[:country_code]
-        security.logo_url = provider_security[:logo_url]
-        security.exchange_acronym = provider_security[:exchange_acronym]
-        security.exchange_mic = provider_security[:exchange_mic]
-      end
+      security
     end
 end

test/models/security/resolver_test.rb

@@ -45,7 +45,7 @@ class Security::ResolverTest < ActiveSupport::TestCase
 
     # Return in reverse-priority order to prove the sorter works
     Security.expects(:search_provider)
-            .with("TEST", exchange_operating_mic: "XNAS", country_code: nil)
+            .with("TEST", exchange_operating_mic: "XNAS")
             .returns([ other, preferred ])
 
     assert_difference "Security.count", 1 do

test/models/trade_import_test.rb

@@ -11,23 +11,24 @@ class TradeImportTest < ActiveSupport::TestCase
   end
 
   test "imports trades and accounts" do
-    # Create an existing AAPL security with no exchange_operating_mic
-    aapl = Security.create!(ticker: "AAPL", exchange_operating_mic: nil)
+    aapl_resolver = mock
+    googl_resolver = mock
 
-    # We should only hit the provider for GOOGL since AAPL already exists
-    Security.expects(:search_provider).with(
-      "GOOGL",
-      exchange_operating_mic: "XNAS"
-    ).returns([
-      Security.new(
-        ticker: "GOOGL",
-        name: "Google Inc.",
-        country_code: "US",
-        exchange_mic: "XNGS",
-        exchange_operating_mic: "XNAS",
-        exchange_acronym: "NGS"
-      )
-    ]).once
+    Security::Resolver.expects(:new)
+                      .with("AAPL", exchange_operating_mic: nil)
+                      .returns(aapl_resolver)
+                      .once
+
+    Security::Resolver.expects(:new)
+                      .with("GOOGL", exchange_operating_mic: "XNAS")
+                      .returns(googl_resolver)
+                      .once
+
+    aapl = securities(:aapl)
+    googl = Security.create!(ticker: "GOOGL", exchange_operating_mic: "XNAS")
+
+    aapl_resolver.stubs(:resolve).returns(aapl)
+    googl_resolver.stubs(:resolve).returns(googl)
 
     import = <<~CSV
       date,ticker,qty,price,currency,account,name,exchange_operating_mic
@@ -55,19 +56,10 @@ class TradeImportTest < ActiveSupport::TestCase
 
     assert_difference -> { Entry.count } => 2,
                       -> { Trade.count } => 2,
-                      -> { Security.count } => 1,
                       -> { Account.count } => 1 do
       @import.publish
     end
 
     assert_equal "complete", @import.status
-
-    # Verify the securities were created/updated correctly
-    aapl.reload
-    assert_nil aapl.exchange_operating_mic
-
-    googl = Security.find_by(ticker: "GOOGL")
-    assert_equal "XNAS", googl.exchange_operating_mic
-    assert_equal "XNGS", googl.exchange_mic
   end
 end