Fix issues with provider API

app/jobs/sync_market_data_job.rb

@@ -10,7 +10,11 @@
 class SyncMarketDataJob < ApplicationJob
   queue_as :scheduled
 
-  def perform(mode: :full, clear_cache: false)
+  def perform(opts)
-    MarketDataSyncer.new.sync_all(mode: mode, clear_cache: clear_cache)
+    opts = opts.symbolize_keys
+    mode = opts.fetch(:mode, :full)
+    clear_cache = opts.fetch(:clear_cache, false)
+
+    MarketDataSyncer.new(mode: mode, clear_cache: clear_cache).sync
   end
 end

app/models/market_data_syncer.rb

@@ -1,45 +1,51 @@
 class MarketDataSyncer
-  DEFAULT_HISTORY_DAYS = 30
+  # By default, our graphs show 1M as the view, so by fetching 31 days,
-  RATE_PROVIDER_NAME = :synth
+  # we ensure we can always show an accurate default graph
-  PRICE_PROVIDER_NAME = :synth
+  SNAPSHOT_DAYS = 31
 
-  # Syncer can optionally be scoped.  Otherwise, it syncs all user data
+  InvalidModeError = Class.new(StandardError)
-  def initialize(family: nil, account: nil)
+
-    @family = family
+  def initialize(mode: :full, clear_cache: false)
-    @account = account
+    @mode = set_mode!(mode)
+    @clear_cache = clear_cache
   end
 
-  def sync_all(full_history: false, clear_cache: false)
+  def sync
-    sync_exchange_rates(full_history: full_history, clear_cache: clear_cache)
+    sync_prices
-    sync_prices(full_history: full_history, clear_cache: clear_cache)
+    sync_exchange_rates
   end
 
-  def sync_exchange_rates(full_history: false, clear_cache: false)
+  # Syncs historical security prices (and details)
-    unless rate_provider
+  def sync_prices
-      Rails.logger.warn("No rate provider configured for MarketDataSyncer.sync_exchange_rates, skipping sync")
+    unless Security.provider
+      Rails.logger.warn("No provider configured for MarketDataSyncer.sync_prices, skipping sync")
       return
     end
 
-    # Finds distinct currency pairs
+    Security.where.not(exchange_operating_mic: nil).find_each do |security|
-    entry_pairs = entries_scope.joins(:account)
+      security.sync_provider_prices(
-                                  .where.not("entries.currency = accounts.currency")
+        start_date: get_first_required_price_date(security),
-                                  .select("entries.currency as source, accounts.currency as target")
+        end_date: end_date,
-                                  .distinct
+        clear_cache: clear_cache
+      )
 
-    # All accounts in currency not equal to the family currency require exchange rates to show a normalized historical graph
+      security.sync_provider_details(clear_cache: clear_cache)
-    account_pairs = accounts_scope.joins(:family)
+    end
-                                  .where.not("families.currency = accounts.currency")
+  end
-                                  .select("accounts.currency as source, families.currency as target")
-                                  .distinct
 
-    pairs = (entry_pairs + account_pairs).uniq
+  def sync_exchange_rates
+    unless ExchangeRate.provider
+      Rails.logger.warn("No provider configured for MarketDataSyncer.sync_exchange_rates, skipping sync")
+      return
+    end
 
-    pairs.each do |pair|
+    required_exchange_rate_pairs.each do |pair|
-      start_date = full_history ? find_oldest_required_rate(from_currency: pair.source) : default_start_date
+      # pair is a Hash with keys :source, :target, and :start_date
+      start_date = snapshot? ? default_start_date : pair[:start_date]
 
       ExchangeRate.sync_provider_rates(
-        from: pair.source,
+        from: pair[:source],
-        to: pair.target,
+        to: pair[:target],
         start_date: start_date,
         end_date: end_date,
         clear_cache: clear_cache
@@ -47,61 +53,80 @@ class MarketDataSyncer
     end
   end
 
-  def sync_prices(full_history: false, clear_cache: false)
-    unless price_provider
-      Rails.logger.warn("No price provider configured for MarketDataSyncer.sync_prices, skipping sync")
-      nil
-    end
-
-    securities_scope.each do |security|
-      start_date = full_history ? find_oldest_required_price(security: security) : default_start_date
-
-      security.sync_provider_prices(start_date: start_date, end_date: end_date, clear_cache: clear_cache)
-      security.sync_provider_details(clear_cache: clear_cache)
-    end
-  end
-
   private
-    attr_reader :family, :account
+    attr_reader :mode, :clear_cache
 
-    def accounts_scope
+    def snapshot?
-      return Account.where(id: account.id) if account
+      mode.to_sym == :snapshot
-      return family.accounts if family
-      Account.all
     end
 
-    def entries_scope
+    # Builds a unique list of currency pairs with the earliest date we need
-      account&.entries || family&.entries || Entry.all
+    # exchange rates for.
-    end
+    #
+    # Returns: Array of Hashes – [{ source:, target:, start_date: }, ...]
+    def required_exchange_rate_pairs
+      pair_dates = {} # { [source, target] => earliest_date }
 
-    def securities_scope
+      # 1. ENTRY-BASED PAIRS – we need rates from the first entry date
-      if account
+      Entry.joins(:account)
-        account.trades.joins(:security).where.not(securities: { exchange_operating_mic: nil })
+           .where.not("entries.currency = accounts.currency")
-      elsif family
+           .group("entries.currency", "accounts.currency")
-        family.trades.joins(:security).where.not(securities: { exchange_operating_mic: nil })
+           .minimum("entries.date")
-      else
+           .each do |(source, target), date|
-        Security.where.not(exchange_operating_mic: nil)
+        key = [ source, target ]
+        pair_dates[key] = [ pair_dates[key], date ].compact.min
+      end
+
+      # 2. ACCOUNT-BASED PAIRS – use the account's oldest entry date
+      account_first_entry_dates = Entry.group(:account_id).minimum(:date)
+
+      Account.joins(:family)
+             .where.not("families.currency = accounts.currency")
+             .select("accounts.id, accounts.currency AS source, families.currency AS target")
+             .find_each do |account|
+        earliest_entry_date = account_first_entry_dates[account.id]
+
+        chosen_date = [ earliest_entry_date, default_start_date ].compact.min
+
+        key = [ account.source, account.target ]
+        pair_dates[key] = [ pair_dates[key], chosen_date ].compact.min
+      end
+
+      # Convert to array of hashes for ease of use
+      pair_dates.map do |(source, target), date|
+        { source: source, target: target, start_date: date }
       end
     end
 
-    def rate_provider
+    def get_first_required_price_date(security)
-      Provider::Registry.for_concept(:exchange_rates).get_provider(RATE_PROVIDER_NAME)
+      return default_start_date if snapshot?
+
+      Trade.with_entry.where(security: security).minimum(:date)
     end
 
-    def price_provider
+    # An approximation that grabs more than we likely need, but simplifies the logic
-      Provider::Registry.for_concept(:securities).get_provider(PRICE_PROVIDER_NAME)
+    def get_first_required_exchange_rate_date(from_currency:)
-    end
+      return default_start_date if snapshot?
 
-    def find_oldest_required_rate(from_currency:)
+      Entry.where(currency: from_currency).minimum(:date)
-      entries_scope.where(currency: from_currency).minimum(:date) || default_start_date
     end
 
     def default_start_date
-      DEFAULT_HISTORY_DAYS.days.ago.to_date
+      SNAPSHOT_DAYS.days.ago.to_date
     end
 
     # Since we're querying market data from a US-based API, end date should always be today (EST)
     def end_date
       Date.current.in_time_zone("America/New_York").to_date
     end
+
+    def set_mode!(mode)
+      valid_modes = [ :full, :snapshot ]
+
+      unless valid_modes.include?(mode.to_sym)
+        raise InvalidModeError, "Invalid mode for MarketDataSyncer, can only be :full or :snapshot, but was #{mode}"
+      end
+
+      mode.to_sym
+    end
 end

app/models/provider/synth.rb

@@ -50,7 +50,7 @@ class Provider::Synth < Provider
 
       rates = JSON.parse(response.body).dig("data", "rates")
 
-      Rate.new(date:, from:, to:, rate: rates.dig(to))
+      Rate.new(date: date.to_date, from:, to:, rate: rates.dig(to))
     end
   end
 
@@ -77,7 +77,7 @@ class Provider::Synth < Provider
           next
         end
 
-        Rate.new(date:, from:, to:, rate:)
+        Rate.new(date: date.to_date, from:, to:, rate:)
       end.compact
     end
   end
@@ -170,7 +170,7 @@ class Provider::Synth < Provider
 
         Price.new(
           symbol: symbol,
-          date: date,
+          date: date.to_date,
           price: price,
           currency: currency,
           exchange_operating_mic: exchange_operating_mic

app/models/security/price/syncer.rb

@@ -66,7 +66,8 @@ class Security::Price::Syncer
         provider_fetch_start_date = effective_start_date - 5.days
 
         response = security_provider.fetch_security_prices(
-          security,
+          symbol: security.ticker,
+          exchange_operating_mic: security.exchange_operating_mic,
           start_date: provider_fetch_start_date,
           end_date:   end_date
         )

app/models/security/provided.rb

@@ -59,15 +59,21 @@ module Security::Provided
       return
     end
 
-    details = provider.fetch_security_info(
+    response = provider.fetch_security_info(
       symbol: ticker,
       exchange_operating_mic: exchange_operating_mic
     )
 
-    update(
+    if response.success?
-      name: details.name,
+      update(
-      logo_url: details.logo_url,
+        name: response.data.name,
-    )
+        logo_url: response.data.logo_url,
+      )
+    else
+      err = StandardError.new("Failed to fetch security info for #{ticker} from #{provider.class.name}: #{response.error.message}")
+      Rails.logger.warn(err.message)
+      Sentry.capture_exception(err, level: :warning)
+    end
   end
 
   def sync_provider_prices(start_date:, end_date:, clear_cache: false)

config/initializers/sidekiq.rb

@@ -1,11 +1,13 @@
 require "sidekiq/web"
 
-Sidekiq::Web.use(Rack::Auth::Basic) do |username, password|
+if Rails.env.production?
-  configured_username = ::Digest::SHA256.hexdigest(ENV.fetch("SIDEKIQ_WEB_USERNAME", "maybe"))
+  Sidekiq::Web.use(Rack::Auth::Basic) do |username, password|
-  configured_password = ::Digest::SHA256.hexdigest(ENV.fetch("SIDEKIQ_WEB_PASSWORD", "maybe"))
+    configured_username = ::Digest::SHA256.hexdigest(ENV.fetch("SIDEKIQ_WEB_USERNAME", "maybe"))
+    configured_password = ::Digest::SHA256.hexdigest(ENV.fetch("SIDEKIQ_WEB_PASSWORD", "maybe"))
 
-  ActiveSupport::SecurityUtils.secure_compare(::Digest::SHA256.hexdigest(username), configured_username) &&
+    ActiveSupport::SecurityUtils.secure_compare(::Digest::SHA256.hexdigest(username), configured_username) &&
-    ActiveSupport::SecurityUtils.secure_compare(::Digest::SHA256.hexdigest(password), configured_password)
+      ActiveSupport::SecurityUtils.secure_compare(::Digest::SHA256.hexdigest(password), configured_password)
+  end
 end
 
 Sidekiq::Cron.configure do |config|

config/schedule.yml

@@ -1,5 +1,9 @@
 sync_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)
+  cron: "* * * * *" # every minute
   class: "SyncMarketDataJob"
   queue: "scheduled"
   description: "Syncs market data daily at 5:00 PM EST (1 hour after market close)"
+  args:
+    mode: "full"
+    clear_cache: false

test/interfaces/exchange_rate_provider_interface_test.rb

@@ -15,6 +15,7 @@ module ExchangeRateProviderInterfaceTest
 
       assert_equal "USD", rate.from
       assert_equal "GBP", rate.to
+      assert rate.date.is_a?(Date)
       assert_in_delta 0.78, rate.rate, 0.01
     end
   end
@@ -26,6 +27,7 @@ module ExchangeRateProviderInterfaceTest
       )
 
       assert_equal 213, response.data.count # 213 days between 01.01.2024 and 31.07.2024
+      assert response.data.first.date.is_a?(Date)
     end
   end
 

test/interfaces/security_provider_interface_test.rb

@@ -26,6 +26,7 @@ module SecurityProviderInterfaceTest
       )
 
       assert response.success?
+      assert response.data.first.date.is_a?(Date)
       assert_equal 147, response.data.count # Synth won't return prices on weekends / holidays, so less than total day count of 213
     end
   end

test/models/market_data_syncer_test.rb

@@ -2,90 +2,84 @@ require "test_helper"
 require "ostruct"
 
 class MarketDataSyncerTest < ActiveSupport::TestCase
-  include EntriesTestHelper, ProviderTestHelper
+  include ProviderTestHelper
 
-  test "syncs exchange rates with upsert" do
+  SNAPSHOT_START_DATE = MarketDataSyncer::SNAPSHOT_DAYS.days.ago.to_date
-    empty_db
+  PROVIDER_BUFFER     = 5.days
 
-    family1 = Family.create!(name: "Family 1", currency: "USD")
+  setup do
-    account1 = family1.accounts.create!(name: "Account 1", currency: "USD", balance: 100, accountable: Depository.new)
+    Security::Price.delete_all
-    account2 = family1.accounts.create!(name: "Account 2", currency: "CAD", balance: 100, accountable: Depository.new)
+    ExchangeRate.delete_all
+    Trade.delete_all
+    Holding.delete_all
+    Security.delete_all
 
-    family2 = Family.create!(name: "Family 2", currency: "EUR")
+    @provider = mock("provider")
-    account3 = family2.accounts.create!(name: "Account 3", currency: "EUR", balance: 100, accountable: Depository.new)
+    Provider::Registry.any_instance
-    account4 = family2.accounts.create!(name: "Account 4", currency: "USD", balance: 100, accountable: Depository.new)
+                      .stubs(:get_provider)
-
+                      .with(:synth)
-    mock_provider = mock
+                      .returns(@provider)
-    Provider::Registry.any_instance.expects(:get_provider).with(:synth).returns(mock_provider).at_least_once
-
-    start_date = 1.month.ago.to_date
-    end_date = Date.current.in_time_zone("America/New_York").to_date
-
-    # Put an existing rate in DB to test upsert
-    ExchangeRate.create!(from_currency: "CAD", to_currency: "USD", date: start_date, rate: 2.0)
-
-    # The individual syncers fetch with a 5-day buffer to ensure we have a «starting» price/rate
-    provider_start_date = get_provider_fetch_start_date(start_date)
-    provider_start_date_for_cad_usd = get_provider_fetch_start_date(start_date + 1.day) # first missing date is +1 day
-
-    mock_provider.expects(:fetch_exchange_rates)
-                 .with(from: "CAD", to: "USD", start_date: provider_start_date_for_cad_usd, end_date: end_date)
-                 .returns(provider_success_response([ OpenStruct.new(from: "CAD", to: "USD", date: start_date, rate: 1.0) ]))
-
-    mock_provider.expects(:fetch_exchange_rates)
-                 .with(from: "USD", to: "EUR", start_date: provider_start_date, end_date: end_date)
-                 .returns(provider_success_response([ OpenStruct.new(from: "USD", to: "EUR", date: start_date, rate: 1.0) ]))
-
-    before_count = ExchangeRate.count
-    MarketDataSyncer.new.sync_exchange_rates
-    after_count = ExchangeRate.count
-
-    assert_operator after_count, :>, before_count, "Expected at least one new exchange-rate row to be inserted"
-
-    # The original CAD→USD rate on start_date should remain (no clear_cache), so value stays 2.0
-    assert_equal 2.0, ExchangeRate.where(from_currency: "CAD", to_currency: "USD", date: start_date).first.rate
   end
 
-  test "syncs security prices with upsert" do
+  test "syncs required exchange rates" do
-    empty_db
+    family = Family.create!(name: "Smith", currency: "USD")
+    family.accounts.create!(name: "Chequing",
+                            currency: "CAD",
+                            balance: 100,
+                            accountable: Depository.new)
 
-    aapl = Security.create!(ticker: "AAPL", exchange_operating_mic: "XNAS")
+    # Seed stale rate so only the next missing day is fetched
+    ExchangeRate.create!(from_currency: "CAD",
+                         to_currency: "USD",
+                         date: SNAPSHOT_START_DATE,
+                         rate: 2.0)
 
-    family = Family.create!(name: "Family 1", currency: "USD")
+    expected_start_date = (SNAPSHOT_START_DATE + 1.day) - PROVIDER_BUFFER
-    account = family.accounts.create!(name: "Account 1", currency: "USD", balance: 100, accountable: Investment.new)
+    end_date            = Date.current.in_time_zone("America/New_York").to_date
 
-    mock_provider = mock
+    @provider.expects(:fetch_exchange_rates)
-    Provider::Registry.any_instance.expects(:get_provider).with(:synth).returns(mock_provider).at_least_once
+             .with(from: "CAD",
+                   to: "USD",
+                   start_date: expected_start_date,
+                   end_date: end_date)
+             .returns(provider_success_response([
+               OpenStruct.new(from: "CAD", to: "USD", date: SNAPSHOT_START_DATE, rate: 1.5)
+             ]))
 
-    start_date = 1.month.ago.to_date
+    before = ExchangeRate.count
-    end_date = Date.current.in_time_zone("America/New_York").to_date
+    MarketDataSyncer.new(mode: :snapshot).sync_exchange_rates
+    after  = ExchangeRate.count
 
-    # The individual syncers fetch with a 5-day buffer to ensure we have a «starting» price/rate
+    assert_operator after, :>, before, "Should insert at least one new exchange-rate row"
-    provider_start_date = get_provider_fetch_start_date(start_date)
-
-    mock_provider.expects(:fetch_security_prices)
-                 .with(aapl, start_date: provider_start_date, end_date: end_date)
-                 .returns(provider_success_response([ OpenStruct.new(security: aapl, date: start_date, price: 100, currency: "USD") ]))
-
-    # The syncer also enriches security details, so stub that out as well
-    mock_provider.stubs(:fetch_security_info)
-                 .with(symbol: "AAPL", exchange_operating_mic: "XNAS")
-                 .returns(OpenStruct.new(name: "Apple", logo_url: "logo"))
-
-    MarketDataSyncer.new.sync_prices
-
-    assert_equal 1, Security::Price.where(security: aapl, date: start_date).count
   end
 
-  private
+  test "syncs security prices" do
-    def empty_db
+    security = Security.create!(ticker: "AAPL", exchange_operating_mic: "XNAS")
-      Invitation.destroy_all
-      Family.destroy_all
-      Security.destroy_all
-    end
 
-    # Match the internal syncer logic of adding a 5-day buffer before provider calls
+    expected_start_date = SNAPSHOT_START_DATE - PROVIDER_BUFFER
-    def get_provider_fetch_start_date(start_date)
+    end_date            = Date.current.in_time_zone("America/New_York").to_date
-      start_date - 5.days
+
-    end
+    @provider.expects(:fetch_security_prices)
+             .with(symbol: security.ticker,
+                   exchange_operating_mic: security.exchange_operating_mic,
+                   start_date: expected_start_date,
+                   end_date: end_date)
+             .returns(provider_success_response([
+               OpenStruct.new(security: security,
+                              date: SNAPSHOT_START_DATE,
+                              price: 100,
+                              currency: "USD")
+             ]))
+
+    @provider.stubs(:fetch_security_info)
+             .with(symbol: "AAPL", exchange_operating_mic: "XNAS")
+             .returns(provider_success_response(OpenStruct.new(name: "Apple", logo_url: "logo")))
+
+    # Ignore exchange rate calls for this test
+    @provider.stubs(:fetch_exchange_rates).returns(provider_success_response([]))
+
+    MarketDataSyncer.new(mode: :snapshot).sync_prices
+
+    assert_equal 1, Security::Price.where(security: security, date: SNAPSHOT_START_DATE).count
+  end
 end

test/models/security/price/syncer_test.rb

@@ -19,7 +19,8 @@ class Security::Price::SyncerTest < ActiveSupport::TestCase
     ])
 
     @provider.expects(:fetch_security_prices)
-             .with(@security, start_date: get_provider_fetch_start_date(2.days.ago.to_date), end_date: Date.current)
+             .with(symbol: @security.ticker, exchange_operating_mic: @security.exchange_operating_mic,
+                   start_date: get_provider_fetch_start_date(2.days.ago.to_date), end_date: Date.current)
              .returns(provider_response)
 
     Security::Price::Syncer.new(
@@ -47,7 +48,8 @@ class Security::Price::SyncerTest < ActiveSupport::TestCase
     ])
 
     @provider.expects(:fetch_security_prices)
-             .with(@security, start_date: get_provider_fetch_start_date(1.day.ago.to_date), end_date: Date.current)
+             .with(symbol: @security.ticker, exchange_operating_mic: @security.exchange_operating_mic,
+                   start_date: get_provider_fetch_start_date(1.day.ago.to_date), end_date: Date.current)
              .returns(provider_response)
 
     Security::Price::Syncer.new(
@@ -94,7 +96,8 @@ class Security::Price::SyncerTest < ActiveSupport::TestCase
     ])
 
     @provider.expects(:fetch_security_prices)
-             .with(@security, start_date: get_provider_fetch_start_date(2.days.ago.to_date), end_date: Date.current)
+             .with(symbol: @security.ticker, exchange_operating_mic: @security.exchange_operating_mic,
+                   start_date: get_provider_fetch_start_date(2.days.ago.to_date), end_date: Date.current)
              .returns(provider_response)
 
     Security::Price::Syncer.new(
@@ -119,7 +122,8 @@ class Security::Price::SyncerTest < ActiveSupport::TestCase
     ])
 
     @provider.expects(:fetch_security_prices)
-             .with(@security, start_date: get_provider_fetch_start_date(Date.current), end_date: Date.current)
+             .with(symbol: @security.ticker, exchange_operating_mic: @security.exchange_operating_mic,
+                   start_date: get_provider_fetch_start_date(Date.current), end_date: Date.current)
              .returns(provider_response)
 
     Security::Price::Syncer.new(