Move back to brandfetch (#1427)

* Move back to brandfetch

* Update security.rb

* Update security.rb
This commit is contained in:
soky srm
2026-04-10 17:42:16 +02:00
committed by GitHub
parent 0aca297e9c
commit dcebda05de
6 changed files with 129 additions and 189 deletions

View File

@@ -31,16 +31,6 @@ class Provider::BinancePublic < Provider
"TRY" => "TRY"
}.freeze
# Per-asset logo PNGs served via jsDelivr from a GitHub repo that tracks the
# full Binance-listed asset set. We originally used bin.bnbstatic.com directly
# — Binance's own CDN — but that host enforces Referer-based hotlink
# protection at CloudFront: any request with a non-Binance Referer returns
# 403. A server-side HEAD from Faraday (no Referer) succeeds, which masked
# the breakage until the URL hit an actual <img> tag in the browser. jsDelivr
# is CORS-open and hotlink-friendly, so the URL we persist is the URL the
# browser can actually load. File names are uppercase PNGs (BTC.png, ETH.png).
LOGO_CDN_BASE = "https://cdn.jsdelivr.net/gh/lindomar-oliveira/binance-data-plus/assets/img".freeze
KLINE_MAX_LIMIT = 1000
MS_PER_DAY = 24 * 60 * 60 * 1000
SEARCH_LIMIT = 25
@@ -116,7 +106,12 @@ class Provider::BinancePublic < Provider
Security.new(
symbol: "#{base}#{display_currency}",
name: base,
logo_url: "#{LOGO_CDN_BASE}/#{base}.png",
# Brandfetch /crypto/{base} URL — unknown coins (rare) will 400 and
# render as a broken img in the dropdown; same tradeoff as stocks
# with obscure tickers. `::Security` reaches the AR model —
# unqualified `Security` here resolves to the Data value-object
# from SecurityConcept.
logo_url: ::Security.brandfetch_crypto_url(base),
exchange_operating_mic: BINANCE_MIC,
country_code: nil,
currency: display_currency
@@ -130,11 +125,14 @@ class Provider::BinancePublic < Provider
parsed = parse_ticker(symbol)
raise Error, "Unsupported Binance ticker: #{symbol}" if parsed.nil?
# logo_url is intentionally nil — crypto logos are set at save time by
# Security#generate_logo_url_from_brandfetch via the /crypto/{base}
# route, not returned from this provider.
SecurityInfo.new(
symbol: symbol,
name: parsed[:base],
links: "https://www.binance.com/en/trade/#{parsed[:binance_pair]}",
logo_url: verified_logo_url(parsed[:base]),
logo_url: nil,
description: nil,
kind: "crypto",
exchange_operating_mic: exchange_operating_mic
@@ -281,34 +279,6 @@ class Provider::BinancePublic < Provider
Time.utc(date.year, date.month, date.day).to_i * 1000
end
# Returns the asset-specific jsDelivr logo URL if the HEAD succeeds, else
# nil. Returning nil (rather than a hard-coded fallback URL) lets
# Security#display_logo_url swap in a Brandfetch binance.com URL at render
# time — a config-dependent path that can't be baked into a constant here.
# Cached per base asset for 30 days so we HEAD at most once per coin and
# only when Security#import_provider_details runs (never during search,
# which must stay fast).
def verified_logo_url(base_asset)
Rails.cache.fetch("binance_public:logo:#{base_asset}", expires_in: 30.days) do
candidate = "#{LOGO_CDN_BASE}/#{base_asset}.png"
logo_client.head(candidate)
candidate
rescue Faraday::Error
nil
end
end
# Dedicated Faraday client for the logo CDN host (jsdelivr.net is a
# different origin from data-api.binance.vision). HEAD-only with a tight
# timeout so CDN hiccups can't stall Security info imports.
def logo_client
@logo_client ||= Faraday.new(url: LOGO_CDN_BASE, ssl: self.class.faraday_ssl_options) do |faraday|
faraday.options.timeout = 3
faraday.options.open_timeout = 2
faraday.response :raise_error
end
end
# Preserve BinancePublic::Error subclasses (e.g. InvalidSecurityPriceError)
# through with_provider_response. The inherited RateLimitable transformer
# only preserves RateLimitError and would otherwise downcast our custom

View File

@@ -18,6 +18,15 @@ class Security < ApplicationRecord
Provider::Registry.for_concept(:securities).provider_keys.map(&:to_s)
end
# Builds the Brandfetch crypto URL for a base asset (e.g. "BTC"). Returns
# nil when Brandfetch isn't configured.
def self.brandfetch_crypto_url(base_asset)
return nil if base_asset.blank?
return nil unless Setting.brand_fetch_client_id.present?
size = Setting.brand_fetch_logo_size
"https://cdn.brandfetch.io/crypto/#{base_asset}/icon/fallback/lettermark/w/#{size}/h/#{size}?c=#{Setting.brand_fetch_client_id}"
end
before_validation :upcase_symbols
before_save :generate_logo_url_from_brandfetch, if: :should_generate_logo?
before_save :reset_first_provider_price_on_if_provider_changed
@@ -60,19 +69,26 @@ class Security < ApplicationRecord
exchange_operating_mic == Provider::BinancePublic::BINANCE_MIC
end
# Single source of truth for which logo URL the UI should render. The order
# differs by asset class:
#
# - Crypto: prefer the verified per-asset jsDelivr logo (set during
# import by Provider::BinancePublic#verified_logo_url). On a miss, fall
# back to Brandfetch with a forced `binance.com` identifier so the
# generic Binance brand mark shows instead of a ticker lettermark.
#
# - Everything else: Brandfetch first (domain-derived or ticker lettermark),
# then any provider-set logo_url.
# Strips the display-currency suffix from a crypto ticker (BTCUSD -> BTC,
# ETHEUR -> ETH). Returns nil for non-crypto securities or when the ticker
# doesn't end in a supported quote.
def crypto_base_asset
return nil unless crypto?
Provider::BinancePublic::QUOTE_TO_CURRENCY.each_value do |suffix|
next unless ticker.end_with?(suffix)
base = ticker.delete_suffix(suffix)
return base unless base.empty?
end
nil
end
# Single source of truth for which logo URL the UI should render. Crypto
# and stocks share the same shape: prefer a freshly computed Brandfetch
# URL (honors current client_id + size) and fall back to any stored
# logo_url for the provider-returns-its-own-URL case (e.g. Tiingo S3).
def display_logo_url
if crypto?
logo_url.presence || brandfetch_icon_url(identifier: "binance.com")
self.class.brandfetch_crypto_url(crypto_base_asset).presence || logo_url.presence
else
brandfetch_icon_url.presence || logo_url.presence
end
@@ -106,13 +122,13 @@ class Security < ApplicationRecord
)
end
def brandfetch_icon_url(width: nil, height: nil, identifier: nil)
def brandfetch_icon_url(width: nil, height: nil)
return nil unless Setting.brand_fetch_client_id.present?
w = width || Setting.brand_fetch_logo_size
h = height || Setting.brand_fetch_logo_size
identifier ||= extract_domain(website_url) if website_url.present?
identifier = extract_domain(website_url) if website_url.present?
identifier ||= ticker
return nil unless identifier.present?
@@ -137,8 +153,7 @@ class Security < ApplicationRecord
def should_generate_logo?
return false if cash?
url = brandfetch_icon_url
return false unless url.present?
return false unless Setting.brand_fetch_client_id.present?
return true if logo_url.blank?
return false unless logo_url.include?("cdn.brandfetch.io")
@@ -147,7 +162,11 @@ class Security < ApplicationRecord
end
def generate_logo_url_from_brandfetch
self.logo_url = brandfetch_icon_url
self.logo_url = if crypto?
self.class.brandfetch_crypto_url(crypto_base_asset)
else
brandfetch_icon_url
end
end
# When a user remaps a security to a different provider (via the holdings

View File

@@ -18,7 +18,11 @@ class Setting < RailsSettings::Base
BRAND_FETCH_LOGO_SIZE_STANDARD = 40
BRAND_FETCH_LOGO_SIZE_HIGH_RES = 120
BRAND_FETCH_URL_PATTERN = %r{(https://cdn\.brandfetch\.io/[^/]+/icon/fallback/lettermark/)w/\d+/h/\d+(\?c=.+)}
# Matches both legacy single-segment URLs (`/apple.com/icon/...`) and
# explicit type-routed URLs introduced 2026 (`/crypto/BTC/icon/...`,
# `/domain/apple.com/icon/...`). `[^?]+` reaches across the extra slash
# so transform_brand_fetch_url can rewrite the size params on both shapes.
BRAND_FETCH_URL_PATTERN = %r{(https://cdn\.brandfetch\.io/[^?]+/icon/fallback/lettermark/)w/\d+/h/\d+(\?c=.+)}
def self.brand_fetch_logo_size
brand_fetch_high_res_logos ? BRAND_FETCH_LOGO_SIZE_HIGH_RES : BRAND_FETCH_LOGO_SIZE_STANDARD

View File

@@ -4,9 +4,6 @@ class Provider::BinancePublicTest < ActiveSupport::TestCase
setup do
@provider = Provider::BinancePublic.new
@provider.stubs(:throttle_request)
# Logo cache is keyed per base asset and persists across tests; clear it
# so verified_logo_url tests don't see each other's results.
Rails.cache.delete_matched("binance_public:logo:*")
end
# ================================
@@ -399,41 +396,16 @@ class Provider::BinancePublicTest < ActiveSupport::TestCase
# Info
# ================================
test "fetch_security_info returns crypto kind" do
stub_logo_head_success
test "fetch_security_info returns crypto kind and nil logo_url" do
response = @provider.fetch_security_info(symbol: "BTCUSD", exchange_operating_mic: "BNCX")
assert response.success?
assert_equal "BTC", response.data.name
assert_equal "crypto", response.data.kind
assert_match(/binance\.com/, response.data.links)
end
test "fetch_security_info returns the CDN logo URL when the HEAD succeeds" do
stub_logo_head_success
response = @provider.fetch_security_info(symbol: "ETHEUR", exchange_operating_mic: "BNCX")
assert_equal "https://cdn.jsdelivr.net/gh/lindomar-oliveira/binance-data-plus/assets/img/ETH.png", response.data.logo_url
end
test "fetch_security_info returns nil logo_url on a HEAD 403" do
stub_logo_head_raising(Faraday::ForbiddenError.new("403"))
response = @provider.fetch_security_info(symbol: "NOPECOINUSD", exchange_operating_mic: "BNCX")
# Nil (rather than a baked-in fallback URL) lets Security#display_logo_url
# substitute a Brandfetch binance.com URL at render time — a path that
# depends on runtime config and can't live on this provider.
assert_nil response.data.logo_url
end
test "fetch_security_info returns nil logo_url when the CDN HEAD times out" do
stub_logo_head_raising(Faraday::TimeoutError.new("timeout"))
response = @provider.fetch_security_info(symbol: "BTCUSD", exchange_operating_mic: "BNCX")
# logo_url is always nil — crypto logos are resolved at render time via
# Security#display_logo_url using the Brandfetch probe verdict, so the
# provider has nothing sensible to persist here.
assert_nil response.data.logo_url
end
@@ -492,40 +464,24 @@ class Provider::BinancePublicTest < ActiveSupport::TestCase
# Logo URL plumbing
# ================================
test "search_securities sets the optimistic CDN logo URL on every result" do
test "search_securities populates each result with the Brandfetch crypto URL" do
@provider.stubs(:exchange_info_symbols).returns(sample_exchange_info)
Setting.stubs(:brand_fetch_client_id).returns("test-client-id")
Setting.stubs(:brand_fetch_logo_size).returns(120)
response = @provider.search_securities("BTC")
assert response.data.all? { |s| s.logo_url == "https://cdn.jsdelivr.net/gh/lindomar-oliveira/binance-data-plus/assets/img/BTC.png" }
expected = "https://cdn.brandfetch.io/crypto/BTC/icon/fallback/lettermark/w/120/h/120?c=test-client-id"
assert response.data.all? { |s| s.logo_url == expected }
end
test "verified_logo_url caches the happy-path result per base asset" do
with_memory_cache do
mock_logo_client = mock
mock_logo_client.expects(:head).once.returns(mock)
@provider.stubs(:logo_client).returns(mock_logo_client)
test "search_securities leaves logo_url nil when Brandfetch is not configured" do
@provider.stubs(:exchange_info_symbols).returns(sample_exchange_info)
Setting.stubs(:brand_fetch_client_id).returns(nil)
url1 = @provider.send(:verified_logo_url, "BTC")
url2 = @provider.send(:verified_logo_url, "BTC")
response = @provider.search_securities("BTC")
assert_equal "https://cdn.jsdelivr.net/gh/lindomar-oliveira/binance-data-plus/assets/img/BTC.png", url1
assert_equal url1, url2
end
end
test "verified_logo_url caches the nil fallback per base asset" do
with_memory_cache do
mock_logo_client = mock
mock_logo_client.expects(:head).once.raises(Faraday::ForbiddenError.new("403"))
@provider.stubs(:logo_client).returns(mock_logo_client)
url1 = @provider.send(:verified_logo_url, "NEVERCOIN")
url2 = @provider.send(:verified_logo_url, "NEVERCOIN")
assert_nil url1
assert_nil url2
end
assert response.data.all? { |s| s.logo_url.nil? }
end
# ================================
@@ -583,18 +539,6 @@ class Provider::BinancePublicTest < ActiveSupport::TestCase
@provider.stubs(:client).returns(mock_client)
end
def stub_logo_head_success
mock_logo_client = mock
mock_logo_client.stubs(:head).returns(mock)
@provider.stubs(:logo_client).returns(mock_logo_client)
end
def stub_logo_head_raising(error)
mock_logo_client = mock
mock_logo_client.stubs(:head).raises(error)
@provider.stubs(:logo_client).returns(mock_logo_client)
end
# Rails.cache in the test env is a NullStore by default, so Rails.cache.fetch
# re-runs the block every time. Swap in a real MemoryStore so cache-hit
# assertions are meaningful, then restore the original.

View File

@@ -161,17 +161,16 @@ class Security::ResolverTest < ActiveSupport::TestCase
# Documents that find_or_create_provider_match! intentionally copies only
# ticker, MIC, country_code, and price_provider from the match — not name
# or logo_url. This means Security#import_provider_details always has
# blank metadata on first resolution and does NOT short-circuit at
# `return if self.name.present? && ...`, so fetch_security_info runs as
# expected on the first sync. Regression guard: if someone adds name/logo
# copying to the resolver, the Binance logo-fallback path would become
# dead code on first sync.
# blank metadata on first resolution and runs fetch_security_info +
# probe_brandfetch_crypto_coverage! as expected. Regression guard: if
# someone adds name/logo copying to the resolver, the Brandfetch
# coverage probe would be bypassed on first sync.
match = Security.new(
ticker: "BTCUSD",
exchange_operating_mic: "BNCX",
country_code: nil,
name: "BTC",
logo_url: "https://cdn.jsdelivr.net/gh/lindomar-oliveira/binance-data-plus/assets/img/BTC.png",
logo_url: "https://cdn.brandfetch.io/crypto/BTC/icon/fallback/lettermark/w/120/h/120?c=test",
price_provider: "binance_public"
)

View File

@@ -39,50 +39,6 @@ class SecurityTest < ActiveSupport::TestCase
assert_equal [ "has already been taken" ], duplicate.errors[:ticker]
end
test "first_provider_price_on resets when price_provider changes" do
sec = Security.create!(
ticker: "TEST",
exchange_operating_mic: "XNAS",
price_provider: "twelve_data",
first_provider_price_on: Date.parse("2020-01-03")
)
sec.update!(price_provider: "yahoo_finance")
assert_nil sec.reload.first_provider_price_on
end
test "first_provider_price_on is preserved when unrelated fields change" do
sec = Security.create!(
ticker: "TEST",
exchange_operating_mic: "XNAS",
price_provider: "twelve_data",
first_provider_price_on: Date.parse("2020-01-03"),
offline: false
)
sec.update!(offline: true, failed_fetch_count: 3)
assert_equal Date.parse("2020-01-03"), sec.reload.first_provider_price_on
end
test "first_provider_price_on respects explicit assignment alongside provider change" do
sec = Security.create!(
ticker: "TEST",
exchange_operating_mic: "XNAS",
price_provider: "twelve_data",
first_provider_price_on: Date.parse("2020-01-03")
)
# Caller changes both in the same save — honor the explicit value.
sec.update!(
price_provider: "yahoo_finance",
first_provider_price_on: Date.parse("2024-03-21")
)
assert_equal Date.parse("2024-03-21"), sec.reload.first_provider_price_on
end
test "cash_for lazily creates a per-account synthetic cash security" do
account = accounts(:investment)
@@ -123,25 +79,58 @@ class SecurityTest < ActiveSupport::TestCase
assert_not offline.crypto?
end
test "display_logo_url for crypto prefers logo_url and falls back to brandfetch with binance.com" do
test "crypto_base_asset strips the display-currency suffix" do
%w[USD EUR JPY BRL TRY].each do |quote|
sec = Security.new(ticker: "BTC#{quote}", exchange_operating_mic: Provider::BinancePublic::BINANCE_MIC)
assert_equal "BTC", sec.crypto_base_asset, "expected BTC#{quote} -> BTC"
end
end
test "crypto_base_asset returns nil for non-crypto securities" do
sec = Security.new(ticker: "AAPL", exchange_operating_mic: "XNAS")
assert_nil sec.crypto_base_asset
end
test "brandfetch_crypto_url uses the /crypto/ route and current size setting" do
Setting.stubs(:brand_fetch_client_id).returns("test-client-id")
Setting.stubs(:brand_fetch_logo_size).returns(120)
with_logo = Security.new(
assert_equal(
"https://cdn.brandfetch.io/crypto/BTC/icon/fallback/lettermark/w/120/h/120?c=test-client-id",
Security.brandfetch_crypto_url("BTC")
)
end
test "brandfetch_crypto_url returns nil when Brandfetch is not configured" do
Setting.stubs(:brand_fetch_client_id).returns(nil)
assert_nil Security.brandfetch_crypto_url("BTC")
end
test "display_logo_url for crypto returns the /crypto/{base} Brandfetch URL" do
Setting.stubs(:brand_fetch_client_id).returns("test-client-id")
Setting.stubs(:brand_fetch_logo_size).returns(120)
sec = Security.new(
ticker: "BTCUSD",
exchange_operating_mic: Provider::BinancePublic::BINANCE_MIC
)
assert_equal(
"https://cdn.brandfetch.io/crypto/BTC/icon/fallback/lettermark/w/120/h/120?c=test-client-id",
sec.display_logo_url
)
end
test "display_logo_url for crypto falls back to stored logo_url when Brandfetch is disabled" do
Setting.stubs(:brand_fetch_client_id).returns(nil)
sec = Security.new(
ticker: "BTCUSD",
exchange_operating_mic: Provider::BinancePublic::BINANCE_MIC,
logo_url: "https://cdn.jsdelivr.net/gh/lindomar-oliveira/binance-data-plus/assets/img/BTC.png"
logo_url: "https://example.com/btc.png"
)
assert_equal "https://cdn.jsdelivr.net/gh/lindomar-oliveira/binance-data-plus/assets/img/BTC.png",
with_logo.display_logo_url
without_logo = Security.new(
ticker: "NOPECOIN",
exchange_operating_mic: Provider::BinancePublic::BINANCE_MIC,
logo_url: nil
)
assert_equal "https://cdn.brandfetch.io/binance.com/icon/fallback/lettermark/w/120/h/120?c=test-client-id",
without_logo.display_logo_url
assert_equal "https://example.com/btc.png", sec.display_logo_url
end
test "display_logo_url for non-crypto prefers brandfetch over stored logo_url" do
@@ -170,4 +159,19 @@ class SecurityTest < ActiveSupport::TestCase
assert_equal "https://example.com/aapl.png", sec.display_logo_url
end
test "before_save writes the /crypto/{base} URL to logo_url for new crypto securities" do
Setting.stubs(:brand_fetch_client_id).returns("test-client-id")
Setting.stubs(:brand_fetch_logo_size).returns(120)
sec = Security.create!(
ticker: "BTCUSD",
exchange_operating_mic: Provider::BinancePublic::BINANCE_MIC
)
assert_equal(
"https://cdn.brandfetch.io/crypto/BTC/icon/fallback/lettermark/w/120/h/120?c=test-client-id",
sec.logo_url
)
end
end