diff --git a/app/jobs/security_health_check_job.rb b/app/jobs/security_health_check_job.rb index f413f388..7b56cc31 100644 --- a/app/jobs/security_health_check_job.rb +++ b/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 diff --git a/app/models/market_data_importer.rb b/app/models/market_data_importer.rb index d0ef723f..86c3c235 100644 --- a/app/models/market_data_importer.rb +++ b/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, diff --git a/app/models/security.rb b/app/models/security.rb index ad28c61c..6a395a17 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? diff --git a/app/models/security/provided.rb b/app/models/security/provided.rb index 6a264081..df20fc9c 100644 --- a/app/models/security/provided.rb +++ b/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| diff --git a/app/models/security/resolver.rb b/app/models/security/resolver.rb index 295dccec..abd37419 100644 --- a/app/models/security/resolver.rb +++ b/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 diff --git a/app/models/trade_builder.rb b/app/models/trade_builder.rb index cf9800e5..3893a3e0 100644 --- a/app/models/trade_builder.rb +++ b/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 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/test/models/security/resolver_test.rb b/test/models/security/resolver_test.rb index 5599037b..b0c35621 100644 --- a/test/models/security/resolver_test.rb +++ b/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 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