diff --git a/app/jobs/security_health_check_job.rb b/app/jobs/security_health_check_job.rb new file mode 100644 index 00000000..387dcd9b --- /dev/null +++ b/app/jobs/security_health_check_job.rb @@ -0,0 +1,7 @@ +class SecurityHealthCheckJob < ApplicationJob + queue_as :scheduled + + def perform + Security::HealthChecker.check_all + end +end diff --git a/app/models/market_data_importer.rb b/app/models/market_data_importer.rb index 9eaf5964..86c3c235 100644 --- a/app/models/market_data_importer.rb +++ b/app/models/market_data_importer.rb @@ -22,7 +22,8 @@ class MarketDataImporter return end - Security.where.not(exchange_operating_mic: nil).find_each do |security| + # Import all securities that aren't marked as "offline" (i.e. they're available from the provider) + Security.online.find_each do |security| security.import_provider_prices( start_date: get_first_required_price_date(security), end_date: end_date, diff --git a/app/models/provider/synth.rb b/app/models/provider/synth.rb index eba22967..ed2514ac 100644 --- a/app/models/provider/synth.rb +++ b/app/models/provider/synth.rb @@ -94,7 +94,8 @@ class Provider::Synth < Provider req.params["name"] = symbol req.params["dataset"] = "limited" req.params["country_code"] = country_code if country_code.present? - req.params["exchange_operating_mic"] = exchange_operating_mic if exchange_operating_mic.present? + # Synth uses mic_code, which encompasses both exchange_mic AND exchange_operating_mic (union) + req.params["mic_code"] = exchange_operating_mic if exchange_operating_mic.present? req.params["limit"] = 25 end @@ -132,7 +133,7 @@ class Provider::Synth < Provider end end - def fetch_security_price(symbol:, exchange_operating_mic:, date:) + def fetch_security_price(symbol:, exchange_operating_mic: nil, date:) with_provider_response do historical_data = fetch_security_prices(symbol:, exchange_operating_mic:, start_date: date, end_date: date) @@ -142,13 +143,13 @@ class Provider::Synth < Provider end end - def fetch_security_prices(symbol:, exchange_operating_mic:, start_date:, end_date:) + def fetch_security_prices(symbol:, exchange_operating_mic: nil, start_date:, end_date:) with_provider_response do params = { start_date: start_date, end_date: end_date, operating_mic_code: exchange_operating_mic - } + }.compact data = paginate( "#{base_url}/tickers/#{symbol}/open-close", diff --git a/app/models/security.rb b/app/models/security.rb index dd638ece..b6e85295 100644 --- a/app/models/security.rb +++ b/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? @@ -25,10 +27,6 @@ class Security < ApplicationRecord ) end - def has_prices? - exchange_operating_mic.present? - end - private def upcase_symbols self.ticker = ticker.upcase diff --git a/app/models/security/health_checker.rb b/app/models/security/health_checker.rb new file mode 100644 index 00000000..241e3845 --- /dev/null +++ b/app/models/security/health_checker.rb @@ -0,0 +1,120 @@ +# There are hundreds of thousands of market securities that Maybe must handle. +# Due to the always-changing nature of the market, the health checker is responsible +# for periodically checking active securities to ensure we can still fetch prices for them. +# +# Each security goes through some basic health checks. If failed, this class is responsible for: +# - Marking failed attempts and incrementing the failed attempts counter +# - Marking the security offline if enough consecutive failed checks occur +# - When we move a security "offline", delete all prices for that security as we assume they are bad data +# +# The health checker is run daily through SecurityHealthCheckJob (see config/schedule.yml), but not all +# securities will be checked every day (we run in batches) +class Security::HealthChecker + MAX_CONSECUTIVE_FAILURES = 5 + HEALTH_CHECK_INTERVAL = 7.days + DAILY_BATCH_SIZE = 1000 + + class << self + def check_all + # No daily limit for unchecked securities (they are prioritized) + never_checked_scope.find_each do |security| + new(security).run_check + end + + # Daily limit for checked securities + due_for_check_scope.limit(DAILY_BATCH_SIZE).each do |security| + new(security).run_check + end + end + + private + # If a security has never had a health check, we prioritize it, regardless of batch size + def never_checked_scope + Security.where(last_health_check_at: nil) + end + + # Any securities not checked for 30 days are due + # We only process the batch size, which means some "due" securities will not be checked today + # This is by design, to prevent all securities from coming due at the same time + def due_for_check_scope + Security.where(last_health_check_at: ..HEALTH_CHECK_INTERVAL.ago) + .order(last_health_check_at: :asc) + end + end + + def initialize(security) + @security = security + end + + def run_check + Rails.logger.info("Running health check for #{security.ticker}") + + if latest_provider_price + handle_success + else + handle_failure + end + rescue => e + Sentry.capture_exception(e) do |scope| + scope.set_tags(security_id: @security.id) + end + ensure + security.update!(last_health_check_at: Time.current) + end + + private + attr_reader :security + + def provider + Security.provider + end + + def latest_provider_price + return nil unless provider.present? + + response = provider.fetch_security_price( + symbol: security.ticker, + exchange_operating_mic: security.exchange_operating_mic, + date: Date.current + ) + + return nil unless response.success? + + response.data.price + end + + # On success, reset any failure counters and ensure it is "online" + def handle_success + security.update!( + offline: false, + failed_fetch_count: 0, + failed_fetch_at: nil + ) + end + + def handle_failure + new_failure_count = security.failed_fetch_count.to_i + 1 + new_failure_at = Time.current + + if new_failure_count > MAX_CONSECUTIVE_FAILURES + convert_to_offline_security! + else + security.update!( + failed_fetch_count: new_failure_count, + failed_fetch_at: new_failure_at + ) + end + end + + # The "offline" state tells our MarketDataImporter (daily cron) to skip this security when fetching prices + def convert_to_offline_security! + Security.transaction do + security.update!( + offline: true, + failed_fetch_count: MAX_CONSECUTIVE_FAILURES + 1, + failed_fetch_at: Time.current + ) + security.prices.delete_all + end + end +end diff --git a/app/models/security/provided.rb b/app/models/security/provided.rb index f72de993..ca886420 100644 --- a/app/models/security/provided.rb +++ b/app/models/security/provided.rb @@ -10,9 +10,14 @@ module Security::Provided end def search_provider(symbol, country_code: nil, exchange_operating_mic: nil) - return [] if symbol.blank? || symbol.length < 2 + 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| @@ -38,7 +43,11 @@ module Security::Provided # Make sure we have a data provider before fetching return nil unless provider.present? - response = provider.fetch_security_price(self, date: date) + response = provider.fetch_security_price( + symbol: ticker, + exchange_operating_mic: exchange_operating_mic, + date: date + ) return nil unless response.success? # Provider error diff --git a/app/models/security/resolver.rb b/app/models/security/resolver.rb new file mode 100644 index 00000000..2b0e13bc --- /dev/null +++ b/app/models/security/resolver.rb @@ -0,0 +1,156 @@ +class Security::Resolver + def initialize(symbol, exchange_operating_mic: nil, country_code: nil) + @symbol = validate_symbol!(symbol) + @exchange_operating_mic = exchange_operating_mic + @country_code = country_code + end + + # Attempts several paths to resolve a security: + # 1. Exact match in DB + # 2. Search provider for an exact match + # 3. Search provider for close match, ranked by relevance + # 4. Create offline security if no match is found in either DB or provider + def resolve + return nil if symbol.blank? + + exact_match_from_db || + exact_match_from_provider || + close_match_from_provider || + offline_security + end + + private + attr_reader :symbol, :exchange_operating_mic, :country_code + + def validate_symbol!(symbol) + raise ArgumentError, "Symbol is required and cannot be blank" if symbol.blank? + symbol.strip.upcase + end + + def offline_security + security = Security.find_or_initialize_by( + ticker: symbol, + exchange_operating_mic: exchange_operating_mic, + ) + + security.assign_attributes( + country_code: country_code, + offline: true # This tells us that we shouldn't try to fetch prices later + ) + + security.save! + + security + end + + def exact_match_from_db + Security.find_by( + { + ticker: symbol, + exchange_operating_mic: exchange_operating_mic, + country_code: country_code.presence + }.compact + ) + end + + # If provided a ticker + exchange (and optionally, a country code), we can find exact matches + def exact_match_from_provider + # Without an exchange, we can never know if we have an exact match + return nil unless exchange_operating_mic.present? + + match = provider_search_result.find do |s| + ticker_matches = s.ticker.upcase.to_s == symbol.upcase.to_s + exchange_matches = s.exchange_operating_mic.upcase.to_s == exchange_operating_mic.upcase.to_s + + if country_code && exchange_operating_mic + ticker_matches && exchange_matches && s.country_code.upcase.to_s == country_code.upcase.to_s + else + ticker_matches && exchange_matches + end + end + + return nil unless match + + find_or_create_provider_match!(match) + end + + def close_match_from_provider + filtered_candidates = provider_search_result + + # If a country code is specified, we MUST find a match with the same code + if country_code.present? + filtered_candidates = filtered_candidates.select { |s| s.country_code.upcase.to_s == country_code.upcase.to_s } + end + + # 1. Prefer exact exchange_operating_mic matches (if one was provided) + # 2. Rank by country relevance (lower index in the list is more relevant) + # 3. Rank by exchange_operating_mic relevance (lower index in the list is more relevant) + sorted_candidates = filtered_candidates.sort_by do |s| + [ + exchange_operating_mic.present? && s.exchange_operating_mic.upcase.to_s == exchange_operating_mic.upcase.to_s ? 0 : 1, + sorted_country_codes_by_relevance.index(s.country_code&.upcase.to_s) || sorted_country_codes_by_relevance.length, + sorted_exchange_operating_mics_by_relevance.index(s.exchange_operating_mic&.upcase.to_s) || sorted_exchange_operating_mics_by_relevance.length + ] + end + + match = sorted_candidates.first + + return nil unless match + + find_or_create_provider_match!(match) + end + + def find_or_create_provider_match!(match) + security = Security.find_or_initialize_by( + ticker: match.ticker, + exchange_operating_mic: match.exchange_operating_mic, + ) + + security.country_code = match.country_code + security.save! + + security + end + + def provider_search_result + 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 + # These are generally sorted by market cap. + def sorted_country_codes_by_relevance + %w[US CN JP IN GB CA FR DE CH SA TW AU NL SE KR IE ES AE IT HK BR DK SG MX RU IL ID BE TH NO] + end + + # Non-exhaustive list of common exchange operating MICs for help in choosing "close" matches + # This is very US-centric since our prices provider and user base is a majority US-based + def sorted_exchange_operating_mics_by_relevance + [ + "XNYS", # New York Stock Exchange + "XNAS", # NASDAQ Stock Market + "XOTC", # OTC Markets Group (OTC Link) + "OTCM", # OTC Markets Group + "OTCN", # OTC Bulletin Board + "OTCI", # OTC International + "OPRA", # Options Price Reporting Authority + "MEMX", # Members Exchange + "IEXA", # IEX All-Market + "IEXG", # IEX Growth Market + "EDXM", # Cboe EDGX Exchange (Equities) + "XCME", # CME Group (Derivatives) + "XCBT", # Chicago Board of Trade + "XPUS", # Nasdaq PSX (U.S.) + "XPSE", # Nasdaq PHLX (U.S.) + "XTRD", # Nasdaq TRF (Trade Reporting Facility) + "XTXD", # FINRA TRACE (Trade Reporting) + "XARC", # NYSE Arca + "XBOX", # BOX Options Exchange + "XBXO" # BZX Options (Cboe) + ] + end +end diff --git a/app/models/trade_builder.rb b/app/models/trade_builder.rb index cf9800e5..72d9aa01 100644 --- a/app/models/trade_builder.rb +++ b/app/models/trade_builder.rb @@ -129,9 +129,9 @@ class TradeBuilder def security ticker_symbol, exchange_operating_mic = ticker.present? ? ticker.split("|") : [ manual_ticker, nil ] - Security.find_or_create_by!( - ticker: ticker_symbol, + Security::Resolver.new( + ticker_symbol, exchange_operating_mic: exchange_operating_mic - ) + ).resolve end end diff --git a/app/models/trade_import.rb b/app/models/trade_import.rb index e7a57f64..67fac05d 100644 --- a/app/models/trade_import.rb +++ b/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 diff --git a/app/views/pages/dashboard/_cashflow_sankey.html.erb b/app/views/pages/dashboard/_cashflow_sankey.html.erb index 7a9da831..96a40636 100644 --- a/app/views/pages/dashboard/_cashflow_sankey.html.erb +++ b/app/views/pages/dashboard/_cashflow_sankey.html.erb @@ -41,4 +41,4 @@ <% end %> - \ No newline at end of file + diff --git a/app/views/rules/_rule.html.erb b/app/views/rules/_rule.html.erb index 9f35c465..3157a904 100644 --- a/app/views/rules/_rule.html.erb +++ b/app/views/rules/_rule.html.erb @@ -1,5 +1,5 @@ <%# locals: (rule:) %> -
+
">
<% if rule.name.present? %> diff --git a/app/views/shared/_ruler.html.erb b/app/views/shared/_ruler.html.erb index fa963914..faf96d8e 100644 --- a/app/views/shared/_ruler.html.erb +++ b/app/views/shared/_ruler.html.erb @@ -1,2 +1,2 @@ <%# locals: (classes: nil) %> -
+
"> diff --git a/config/schedule.yml b/config/schedule.yml index 75b709a6..731cc49b 100644 --- a/config/schedule.yml +++ b/config/schedule.yml @@ -1,5 +1,5 @@ import_market_data: - cron: "0 22 * * 1-5" # 5:00 PM EST / 6:00 PM EDT (NY time) + cron: "0 22 * * 1-5" # 5:00 PM EST / 6:00 PM EDT (NY time) Monday through Friday class: "ImportMarketDataJob" queue: "scheduled" description: "Imports market data daily at 5:00 PM EST (1 hour after market close)" @@ -12,3 +12,9 @@ clean_syncs: class: "SyncCleanerJob" queue: "scheduled" description: "Cleans up stale syncs" + +run_security_health_checks: + cron: "0 2 * * 1-5" # 2:00 AM EST / 3:00 AM EDT (NY time) Monday through Friday + class: "SecurityHealthCheckJob" + queue: "scheduled" + description: "Runs security health checks to detect issues with security data" diff --git a/db/migrate/20250521112347_add_security_resolver_fields.rb b/db/migrate/20250521112347_add_security_resolver_fields.rb new file mode 100644 index 00000000..29391512 --- /dev/null +++ b/db/migrate/20250521112347_add_security_resolver_fields.rb @@ -0,0 +1,8 @@ +class AddSecurityResolverFields < ActiveRecord::Migration[7.2] + def change + add_column :securities, :offline, :boolean, default: false, null: false + add_column :securities, :failed_fetch_at, :datetime + add_column :securities, :failed_fetch_count, :integer, default: 0, null: false + add_column :securities, :last_health_check_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index fe602824..9f079956 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2025_05_18_181619) do +ActiveRecord::Schema[7.2].define(version: 2025_05_21_112347) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -30,7 +30,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_05_18_181619) do t.decimal "balance", precision: 19, scale: 4 t.string "currency" t.boolean "is_active", default: true, null: false - t.virtual "classification", type: :string, as: "\nCASE\n WHEN ((accountable_type)::text = ANY (ARRAY[('Loan'::character varying)::text, ('CreditCard'::character varying)::text, ('OtherLiability'::character varying)::text])) THEN 'liability'::text\n ELSE 'asset'::text\nEND", stored: true + t.virtual "classification", type: :string, as: "\nCASE\n WHEN ((accountable_type)::text = ANY ((ARRAY['Loan'::character varying, 'CreditCard'::character varying, 'OtherLiability'::character varying])::text[])) THEN 'liability'::text\n ELSE 'asset'::text\nEND", stored: true t.uuid "import_id" t.uuid "plaid_account_id" t.boolean "scheduled_for_deletion", default: false @@ -513,6 +513,10 @@ ActiveRecord::Schema[7.2].define(version: 2025_05_18_181619) do t.string "exchange_acronym" t.string "logo_url" t.string "exchange_operating_mic" + t.boolean "offline", default: false, null: false + t.datetime "failed_fetch_at" + t.integer "failed_fetch_count", default: 0, null: false + t.datetime "last_health_check_at" t.index ["country_code"], name: "index_securities_on_country_code" t.index ["exchange_operating_mic"], name: "index_securities_on_exchange_operating_mic" t.index ["ticker", "exchange_operating_mic"], name: "index_securities_on_ticker_and_exchange_operating_mic", unique: true diff --git a/test/models/security/health_checker_test.rb b/test/models/security/health_checker_test.rb new file mode 100644 index 00000000..a8efe8f7 --- /dev/null +++ b/test/models/security/health_checker_test.rb @@ -0,0 +1,158 @@ +require "test_helper" + +class Security::HealthCheckerTest < ActiveSupport::TestCase + include ProviderTestHelper + + setup do + # Clean slate + Holding.destroy_all + Trade.destroy_all + Security::Price.delete_all + Security.delete_all + + @provider = mock + Security.stubs(:provider).returns(@provider) + + # Brand new, no health check has been run yet + @new_security = Security.create!( + ticker: "NEW", + offline: false, + last_health_check_at: nil + ) + + # New security, offline + # This will be checked, but unless it gets a price, we keep it offline + @new_offline_security = Security.create!( + ticker: "NEW_OFFLINE", + offline: true, + last_health_check_at: nil + ) + + # Online, recently checked, healthy + @healthy_security = Security.create!( + ticker: "HEALTHY", + offline: false, + last_health_check_at: 2.hours.ago + ) + + # Online, due for a health check + @due_for_check_security = Security.create!( + ticker: "DUE", + offline: false, + last_health_check_at: Security::HealthChecker::HEALTH_CHECK_INTERVAL.ago - 1.day + ) + + # Offline, recently checked (keep offline, don't check) + @offline_security = Security.create!( + ticker: "OFFLINE", + offline: true, + last_health_check_at: 20.days.ago + ) + + # Currently offline, but has had no health check and actually has prices (needs to convert to "online") + @offline_never_checked_with_prices = Security.create!( + ticker: "OFFLINE_NEVER_CHECKED", + offline: true, + last_health_check_at: nil + ) + end + + test "any security without a health check runs" do + to_check = Security.where(last_health_check_at: nil).or(Security.where(last_health_check_at: ..Security::HealthChecker::HEALTH_CHECK_INTERVAL.ago)) + Security::HealthChecker.any_instance.expects(:run_check).times(to_check.count) + Security::HealthChecker.check_all + end + + test "offline security with no health check that fails stays offline" do + hc = Security::HealthChecker.new(@new_offline_security) + + @provider.expects(:fetch_security_price) + .with( + symbol: @new_offline_security.ticker, + exchange_operating_mic: @new_offline_security.exchange_operating_mic, + date: Date.current + ) + .returns( + provider_error_response(StandardError.new("No prices found")) + ) + .once + + hc.run_check + + assert_equal 1, @new_offline_security.failed_fetch_count + assert @new_offline_security.offline? + end + + test "after enough consecutive health check failures, security goes offline and prices are deleted" do + # Create one test price + Security::Price.create!( + security: @due_for_check_security, + date: Date.current, + price: 100, + currency: "USD" + ) + + hc = Security::HealthChecker.new(@due_for_check_security) + + @provider.expects(:fetch_security_price) + .with( + symbol: @due_for_check_security.ticker, + exchange_operating_mic: @due_for_check_security.exchange_operating_mic, + date: Date.current + ) + .returns(provider_error_response(StandardError.new("No prices found"))) + .times(Security::HealthChecker::MAX_CONSECUTIVE_FAILURES + 1) + + Security::HealthChecker::MAX_CONSECUTIVE_FAILURES.times do + hc.run_check + end + + refute @due_for_check_security.offline? + assert_equal 1, @due_for_check_security.prices.count + + # We've now exceeded the max consecutive failures, so the security should be marked offline + hc.run_check + assert @due_for_check_security.offline? + assert_equal 0, @due_for_check_security.prices.count + end + + test "failure incrementor increases for each health check failure" do + hc = Security::HealthChecker.new(@due_for_check_security) + + @provider.expects(:fetch_security_price) + .with( + symbol: @due_for_check_security.ticker, + exchange_operating_mic: @due_for_check_security.exchange_operating_mic, + date: Date.current + ) + .returns(provider_error_response(StandardError.new("No prices found"))) + .twice + + hc.run_check + assert_equal 1, @due_for_check_security.failed_fetch_count + + hc.run_check + assert_equal 2, @due_for_check_security.failed_fetch_count + end + + test "failure incrementor resets to 0 when health check succeeds" do + hc = Security::HealthChecker.new(@offline_never_checked_with_prices) + + @provider.expects(:fetch_security_price) + .with( + symbol: @offline_never_checked_with_prices.ticker, + exchange_operating_mic: @offline_never_checked_with_prices.exchange_operating_mic, + date: Date.current + ) + .returns(provider_success_response(OpenStruct.new(price: 100, date: Date.current, currency: "USD"))) + .once + + assert @offline_never_checked_with_prices.offline? + + hc.run_check + + refute @offline_never_checked_with_prices.offline? + assert_equal 0, @offline_never_checked_with_prices.failed_fetch_count + assert_nil @offline_never_checked_with_prices.failed_fetch_at + end +end diff --git a/test/models/security/price_test.rb b/test/models/security/price_test.rb index 14c022fe..6e8e49da 100644 --- a/test/models/security/price_test.rb +++ b/test/models/security/price_test.rb @@ -43,7 +43,7 @@ class Security::PriceTest < ActiveSupport::TestCase with_provider_response = provider_error_response(StandardError.new("Test error")) @provider.expects(:fetch_security_price) - .with(security, date: Date.current) + .with(symbol: security.ticker, exchange_operating_mic: security.exchange_operating_mic, date: Date.current) .returns(with_provider_response) assert_not @security.find_or_fetch_price(date: Date.current) @@ -52,7 +52,7 @@ class Security::PriceTest < ActiveSupport::TestCase private def expect_provider_price(security:, price:, date:) @provider.expects(:fetch_security_price) - .with(security, date: date) + .with(symbol: security.ticker, exchange_operating_mic: security.exchange_operating_mic, date: date) .returns(provider_success_response(price)) end diff --git a/test/models/security/resolver_test.rb b/test/models/security/resolver_test.rb new file mode 100644 index 00000000..1a557f22 --- /dev/null +++ b/test/models/security/resolver_test.rb @@ -0,0 +1,78 @@ +require "test_helper" + +class Security::ResolverTest < ActiveSupport::TestCase + setup do + @provider = mock + Security.stubs(:provider).returns(@provider) + end + + test "resolves DB security" do + # Given an existing security in the DB that exactly matches the lookup params + db_security = Security.create!(ticker: "TSLA", exchange_operating_mic: "XNAS", country_code: "US") + + # The resolver should return the DB record and never hit the provider + Security.expects(:search_provider).never + + resolved = Security::Resolver.new("TSLA", exchange_operating_mic: "XNAS", country_code: "US").resolve + + assert_equal db_security, resolved + end + + test "resolves exact provider match" do + # Provider returns multiple results, one of which exactly matches symbol + exchange (and country) + exact_match = Security.new(ticker: "NVDA", exchange_operating_mic: "XNAS", country_code: "US") + near_miss = Security.new(ticker: "NVDA", exchange_operating_mic: "XNYS", country_code: "US") + + Security.expects(:search_provider) + .with("NVDA", exchange_operating_mic: "XNAS", country_code: "US") + .returns([ near_miss, exact_match ]) + + assert_difference "Security.count", 1 do + resolved = Security::Resolver.new("NVDA", exchange_operating_mic: "XNAS", country_code: "US").resolve + + assert resolved.persisted? + assert_equal "NVDA", resolved.ticker + assert_equal "XNAS", resolved.exchange_operating_mic + assert_equal "US", resolved.country_code + refute resolved.offline, "Exact provider matches should not be marked offline" + end + end + + test "resolves close provider match" do + # No exact match – resolver should choose the most relevant close match based on exchange + country ranking + preferred = Security.new(ticker: "TEST1", exchange_operating_mic: "XNAS", country_code: "US") + other = Security.new(ticker: "TEST2", exchange_operating_mic: "XNYS", country_code: "GB") + + # Return in reverse-priority order to prove the sorter works + Security.expects(:search_provider) + .with("TEST", exchange_operating_mic: "XNAS") + .returns([ other, preferred ]) + + assert_difference "Security.count", 1 do + resolved = Security::Resolver.new("TEST", exchange_operating_mic: "XNAS").resolve + + assert resolved.persisted? + assert_equal "TEST1", resolved.ticker + assert_equal "XNAS", resolved.exchange_operating_mic + assert_equal "US", resolved.country_code + refute resolved.offline, "Provider matches should not be marked offline" + end + end + + test "resolves offline security" do + Security.expects(:search_provider).returns([]) + + assert_difference "Security.count", 1 do + resolved = Security::Resolver.new("FOO").resolve + + assert resolved.persisted?, "Offline security should be saved" + assert_equal "FOO", resolved.ticker + assert resolved.offline, "Offline securities should be flagged offline" + end + end + + test "returns nil when symbol blank" do + assert_raises(ArgumentError) { Security::Resolver.new(nil).resolve } + assert_raises(ArgumentError) { Security::Resolver.new("").resolve } + end +end diff --git a/test/models/trade_import_test.rb b/test/models/trade_import_test.rb index f5fab129..54b30797 100644 --- a/test/models/trade_import_test.rb +++ b/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