diff --git a/CHANGELOG.md b/CHANGELOG.md index da99d3741e..769806e71c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Deprecated the `api/v1/admin/user`, `api/v1/user/:id` and `api/v1/user/:id/access-token` endpoints in favor of the `api/v1/users` endpoints - Upgraded `chart.js` from version `4.5.1` to `4.6.1` +### Fixed + +- Fixed the overwrite of `countries`, `holdings` and `sectors` with empty values in the data gathering of asset profiles +- Fixed the validation of the responses in the data enhancer for asset profile data via _Trackinsight_ + ## 3.95.0 - 2026-10-31 ### Changed diff --git a/apps/api/src/services/data-provider/data-enhancer/trackinsight/trackinsight.service.spec.ts b/apps/api/src/services/data-provider/data-enhancer/trackinsight/trackinsight.service.spec.ts new file mode 100644 index 0000000000..8f6720fddf --- /dev/null +++ b/apps/api/src/services/data-provider/data-enhancer/trackinsight/trackinsight.service.spec.ts @@ -0,0 +1,358 @@ +import { Logger } from '@nestjs/common'; + +import { TrackinsightDataEnhancerService } from './trackinsight.service'; + +jest.mock('@ghostfolio/api/services/fetch/fetch.service', () => { + return { + FetchService: jest.fn() + }; +}); + +const FAILURE_MESSAGE = { + _failure_reason: 'http_error', + error: 'HTTP 405: Method Not Allowed', + http_status: 405, + status: 'failed', + url: 'https://www.trackinsight.com/data-api/holdings/QQQ.json' +}; + +const FUND = { + cusip: '46090E103', + isins: ['US46090E1038', 'US46090E3018'], + ticker: 'QQQ' +}; + +const HOLDINGS = { + countries: { + Other: { count: 1, weight: 0.01 }, + USA: { count: 99, weight: 0.99 } + }, + sectors: { + Financials: { count: 10, weight: 0.2 }, + 'Information Technology': { count: 90, weight: 0.8 } + }, + topHoldings: [ + { label: 'NVIDIA Corp.', weight: 0.08 }, + { label: 'Other', weight: 0.92 } + ], + weight: 0.9999 +}; + +const SEARCH_RESULT = { + results: { count: 221, docs: [{ ticker: 'QQQ' }], words: ['qqq'] } +}; + +describe('TrackinsightDataEnhancerService', () => { + let fetchService: { fetch: jest.Mock }; + let trackinsightDataEnhancerService: TrackinsightDataEnhancerService; + + const enhance = (response = {}, symbol = 'QQQ') => { + return trackinsightDataEnhancerService.enhance({ + symbol, + requestTimeout: 1000, + response: { assetClass: 'EQUITY', assetSubClass: 'ETF', ...response } + }); + }; + + const mockResponses = ({ + fund = Response.json(FUND), + holdings = Response.json(HOLDINGS), + search = Response.json(SEARCH_RESULT) + }: { + fund?: Response | Error; + holdings?: Response | Error; + search?: Response | Error; + }) => { + fetchService.fetch.mockImplementation((url: string) => { + let result = fund; + + if (url.includes('/search-api/')) { + result = search; + } else if (url.includes('/data-api/holdings/')) { + result = holdings; + } + + return result instanceof Error + ? Promise.reject(result) + : Promise.resolve(result.clone()); + }); + }; + + beforeEach(() => { + fetchService = { fetch: jest.fn() }; + + trackinsightDataEnhancerService = new TrackinsightDataEnhancerService( + null, + fetchService as any + ); + }); + + it('should skip an asset which is not a fund', async () => { + const response = await enhance({ assetSubClass: 'STOCK' }); + + expect(fetchService.fetch).not.toHaveBeenCalled(); + expect(response.countries).toBeUndefined(); + }); + + it('should enhance the asset profile with a valid response', async () => { + mockResponses({}); + + await expect(enhance()).resolves.toEqual({ + assetClass: 'EQUITY', + assetSubClass: 'ETF', + countries: [{ code: 'US', weight: 0.99 }], + cusip: '46090E103', + holdings: [{ name: 'NVIDIA Corp.', weight: 0.08 }], + isin: 'US46090E1038', + sectors: [ + { name: 'Financial Services', weight: 0.2 }, + { name: 'Technology', weight: 0.8 } + ] + }); + }); + + it('should keep the lists of the data provider', async () => { + mockResponses({}); + + const countries = [{ code: 'DE', weight: 1 }]; + const holdings = [{ name: 'SAP SE', weight: 1 }]; + const sectors = [{ name: 'Technology', weight: 1 }]; + + const response = await enhance({ countries, holdings, sectors }); + + expect(response.countries).toEqual(countries); + expect(response.holdings).toEqual(holdings); + expect(response.sectors).toEqual(sectors); + }); + + it('should encode the symbol in the URL of the search', async () => { + mockResponses({ + search: Response.json({ results: { count: 0, docs: [] } }) + }); + + await enhance({}, 'BRK/B'); + + expect(fetchService.fetch.mock.calls[0][0]).toContain( + '/search_v2/BRK%2FB/' + ); + }); + + it.each([ + ['a failed request', new Error('The operation timed out')], + ['an error status', new Response(null, { status: 405 })], + ['a body which is not JSON', new Response('')], + ['a failure message', Response.json(FAILURE_MESSAGE)], + ['a false tool call', Response.json({ url: 'https://www.example.com' })] + ])( + 'should not change the asset profile on %s in the search', + async (_, search) => { + mockResponses({ search }); + + await expect(enhance()).resolves.toEqual({ + assetClass: 'EQUITY', + assetSubClass: 'ETF' + }); + } + ); + + it('should not use a search result without a ticker', async () => { + mockResponses({ + search: Response.json({ results: { count: 1, docs: [{}] } }) + }); + + await enhance(); + + expect(fetchService.fetch).toHaveBeenCalledTimes(1); + }); + + it.each([ + ['a single result', 1, 'QQQ.US'], + ['an exact match', 2, 'QQQ'], + ['a match with an exchange', 2, 'XETRA:QQQ'] + ])('should use the ticker of %s in the search', async (_, count, ticker) => { + mockResponses({ + search: Response.json({ results: { count, docs: [{ ticker }] } }) + }); + + await enhance(); + + expect(fetchService.fetch.mock.calls[1][0]).toContain( + `/data-api/funds/${ticker}.json` + ); + }); + + it('should not use a search result with another ticker', async () => { + mockResponses({ + search: Response.json({ + results: { count: 2, docs: [{ ticker: 'QQQ.US' }] } + }) + }); + + await enhance(); + + expect(fetchService.fetch).toHaveBeenCalledTimes(1); + }); + + it('should search again without the suffix of the symbol', async () => { + mockResponses({ + search: Response.json({ + results: { count: 2, docs: [{ ticker: 'VWCE' }] } + }) + }); + + await enhance({}, 'VWCE.DE'); + + const urls = fetchService.fetch.mock.calls.map(([url]) => { + return url; + }); + + expect(urls[0]).toContain('/search_v2/VWCE.DE/'); + expect(urls[1]).toContain('/search_v2/VWCE/'); + expect(urls[2]).toContain('/data-api/funds/VWCE.json'); + }); + + it.each([ + ['a failed request', new Error('The operation timed out')], + ['an error status', new Response(null, { status: 502 })], + ['a body which is not JSON', new Response('`AccessDenied`Access Denied')], + ['a failure message', Response.json(FAILURE_MESSAGE)], + ['a missing weight', Response.json({ ...HOLDINGS, weight: undefined })], + [ + 'a weight which is not a number', + Response.json({ ...HOLDINGS, weight: '1' }) + ], + [ + 'a weight below the threshold', + Response.json({ ...HOLDINGS, weight: 0.5 }) + ], + ['a weight above the maximum', Response.json({ ...HOLDINGS, weight: 1.5 })], + [ + 'an invalid weight of a country', + Response.json({ ...HOLDINGS, countries: { USA: { weight: 99 } } }) + ], + [ + 'an invalid weight of a sector', + Response.json({ ...HOLDINGS, sectors: { Materials: { weight: -0.1 } } }) + ], + [ + 'an invalid item of the top holdings', + Response.json({ ...HOLDINGS, topHoldings: [{ weight: 0.1 }] }) + ], + [ + 'top holdings which are not a list', + Response.json({ ...HOLDINGS, topHoldings: 'NVIDIA Corp.' }) + ], + [ + 'countries which are not an object', + Response.json({ ...HOLDINGS, countries: [{ weight: 0.5 }] }) + ], + [ + 'sectors which are not an object', + Response.json({ ...HOLDINGS, sectors: [{ weight: 0.5 }] }) + ] + ])('should not add lists on %s in the holdings', async (_, holdings) => { + mockResponses({ holdings }); + + const response = await enhance(); + + expect(response).not.toHaveProperty('countries'); + expect(response).not.toHaveProperty('holdings'); + expect(response).not.toHaveProperty('sectors'); + }); + + it('should keep the cusip and the isin if the holdings fail', async () => { + mockResponses({ holdings: Response.json(FAILURE_MESSAGE) }); + + const response = await enhance(); + + expect(response.cusip).toBe('46090E103'); + expect(response.isin).toBe('US46090E1038'); + }); + + it('should not report a failed request as an invalid response', async () => { + const warn = jest.spyOn(Logger.prototype, 'warn').mockImplementation(); + + mockResponses({ holdings: new Response(null, { status: 502 }) }); + + await enhance(); + + expect(warn).toHaveBeenCalledTimes(1); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('HTTP status 502') + ); + + warn.mockRestore(); + }); + + it('should set empty lists on a valid response without items', async () => { + mockResponses({ + holdings: Response.json({ + countries: {}, + sectors: {}, + topHoldings: [], + weight: 1 + }) + }); + + const response = await enhance(); + + expect(response.countries).toEqual([]); + expect(response.holdings).toEqual([]); + expect(response.sectors).toEqual([]); + }); + + it('should use the other lists if the top holdings are null', async () => { + mockResponses({ + holdings: Response.json({ ...HOLDINGS, topHoldings: null }) + }); + + const response = await enhance(); + + expect(response.countries).toEqual([{ code: 'US', weight: 0.99 }]); + expect(response.holdings).toEqual([]); + expect(response.sectors).toEqual([ + { name: 'Financial Services', weight: 0.2 }, + { name: 'Technology', weight: 0.8 } + ]); + }); + + it('should use the other lists if the countries are null', async () => { + mockResponses({ + holdings: Response.json({ ...HOLDINGS, countries: null }) + }); + + const response = await enhance(); + + expect(response.countries).toEqual([]); + expect(response.holdings).toEqual([{ name: 'NVIDIA Corp.', weight: 0.08 }]); + }); + + it('should accept floating point noise in a weight', async () => { + mockResponses({ + holdings: Response.json({ + ...HOLDINGS, + countries: { USA: { weight: 1.0000000000000002 } }, + weight: 1.0000000000000002 + }) + }); + + const response = await enhance(); + + expect(response.countries).toEqual([ + { code: 'US', weight: 1.0000000000000002 } + ]); + }); + + it.each([ + ['a failure message', Response.json(FAILURE_MESSAGE)], + ['an invalid cusip and isins', Response.json({ cusip: 1, isins: 'US' })], + ['a non-string isin', Response.json({ isins: [1] })] + ])('should not set the cusip or the isin on %s', async (_, fund) => { + mockResponses({ fund }); + + const response = await enhance(); + + expect(response).not.toHaveProperty('cusip'); + expect(response).not.toHaveProperty('isin'); + }); +}); diff --git a/apps/api/src/services/data-provider/data-enhancer/trackinsight/trackinsight.service.ts b/apps/api/src/services/data-provider/data-enhancer/trackinsight/trackinsight.service.ts index aa61c048c7..6956e8cf75 100644 --- a/apps/api/src/services/data-provider/data-enhancer/trackinsight/trackinsight.service.ts +++ b/apps/api/src/services/data-provider/data-enhancer/trackinsight/trackinsight.service.ts @@ -10,6 +10,7 @@ import { SectorName } from '@ghostfolio/common/types'; import { Injectable, Logger } from '@nestjs/common'; import { SymbolProfile } from '@prisma/client'; +import { isPlainObject } from 'lodash-es'; @Injectable() export class TrackinsightDataEnhancerService implements DataEnhancerInterface { @@ -36,6 +37,9 @@ export class TrackinsightDataEnhancerService implements DataEnhancerInterface { Materials: 'Basic Materials' }; + // Allows for floating point noise in the sums of Trackinsight + private static weightTolerance = 0.001; + private readonly logger = new Logger(TrackinsightDataEnhancerService.name); public constructor( @@ -75,44 +79,42 @@ export class TrackinsightDataEnhancerService implements DataEnhancerInterface { return response; } - const profile = await this.fetchService - .fetch( - `${TrackinsightDataEnhancerService.baseUrl}/data-api/funds/${trackinsightSymbol}.json`, - { - signal: AbortSignal.timeout(requestTimeout) - } - ) - .then((res) => res.json()) - .catch(() => { - return {}; - }); + const profile = await this.fetchJson({ + requestTimeout, + url: `${TrackinsightDataEnhancerService.baseUrl}/data-api/funds/${trackinsightSymbol}.json` + }); const cusip = profile?.cusip; - if (cusip) { + if (cusip && typeof cusip === 'string') { response.cusip = cusip; } - const isin = profile?.isins?.[0]; + const isin = Array.isArray(profile?.isins) ? profile.isins[0] : undefined; - if (isin) { + if (isin && typeof isin === 'string') { response.isin = isin; } - const holdings = await this.fetchService - .fetch( - `${TrackinsightDataEnhancerService.baseUrl}/data-api/holdings/${trackinsightSymbol}.json`, - { - signal: AbortSignal.timeout(requestTimeout) - } - ) - .then((res) => res.json()) - .catch(() => { - return {}; - }); + const holdings = await this.fetchJson({ + requestTimeout, + url: `${TrackinsightDataEnhancerService.baseUrl}/data-api/holdings/${trackinsightSymbol}.json` + }); + + if (!holdings) { + return response; + } + + if (!this.isValidHoldings(holdings)) { + this.logger.warn( + `Could not use the Trackinsight holdings of "${trackinsightSymbol}": Invalid response` + ); + + return response; + } if ( - holdings?.weight < TrackinsightDataEnhancerService.holdingsWeightTreshold + holdings.weight < TrackinsightDataEnhancerService.holdingsWeightTreshold ) { // Skip if data is inaccurate return response; @@ -125,7 +127,7 @@ export class TrackinsightDataEnhancerService implements DataEnhancerInterface { response.countries = []; for (const [name, value] of Object.entries( - holdings?.countries ?? {} + holdings.countries ?? {} )) { const code = getCountryCodeByName({ name, @@ -145,8 +147,8 @@ export class TrackinsightDataEnhancerService implements DataEnhancerInterface { ) { response.holdings = []; - for (const { label, weight } of holdings?.topHoldings ?? []) { - if (label?.toLowerCase() === 'other') { + for (const { label, weight } of holdings.topHoldings ?? []) { + if (label.toLowerCase() === 'other') { continue; } @@ -163,9 +165,7 @@ export class TrackinsightDataEnhancerService implements DataEnhancerInterface { ) { response.sectors = []; - for (const [name, value] of Object.entries( - holdings?.sectors ?? {} - )) { + for (const [name, value] of Object.entries(holdings.sectors ?? {})) { response.sectors.push({ name: getSectorName({ name, @@ -187,46 +187,113 @@ export class TrackinsightDataEnhancerService implements DataEnhancerInterface { return 'QQQ'; } - private async searchTrackinsightSymbol({ + private async fetchJson({ requestTimeout, - symbol + url }: { requestTimeout: number; - symbol: string; + url: string; }) { return this.fetchService - .fetch( - `${TrackinsightDataEnhancerService.baseUrl}/search-api/search_v2/${symbol}/_/ticker/default/0/3`, - { - signal: AbortSignal.timeout(requestTimeout) - } - ) - .then((res) => res.json()) - .then((jsonRes) => { - if ( - jsonRes['results']?.['count'] === 1 || - // Allow exact match - jsonRes['results']?.['docs']?.[0]?.['ticker'] === symbol || - // Allow EXCHANGE:SYMBOL - jsonRes['results']?.['docs']?.[0]?.['ticker']?.endsWith(`:${symbol}`) - ) { - return jsonRes['results']['docs'][0]['ticker']; + .fetch(url, { + signal: AbortSignal.timeout(requestTimeout) + }) + .then((res) => { + if (!res.ok) { + throw new Error(`HTTP status ${res.status}`); } - this.logger.debug( - `Could not match a Trackinsight symbol for "${symbol}": ${JSON.stringify( - jsonRes - )}` - ); - - return undefined; + return res.json(); }) .catch(({ message }) => { - this.logger.warn( - `Could not search Trackinsight symbol for "${symbol}": ${message}` - ); + this.logger.warn(`Could not fetch ${url}: ${message}`); return undefined; }); } + + private isValidHoldings(holdings: any) { + if (!this.isValidWeight(holdings?.weight)) { + return false; + } + + const { countries, sectors, topHoldings } = holdings; + + if ( + !isPlainObject(countries ?? {}) || + !isPlainObject(sectors ?? {}) || + !Array.isArray(topHoldings ?? []) + ) { + return false; + } + + return ( + Object.values(countries ?? {}).every((country) => { + return this.isValidWeight(country?.weight); + }) && + Object.values(sectors ?? {}).every((sector) => { + return this.isValidWeight(sector?.weight); + }) && + (topHoldings ?? []).every((topHolding) => { + return ( + typeof topHolding?.label === 'string' && + this.isValidWeight(topHolding?.weight) + ); + }) + ); + } + + private isValidWeight(weight: unknown) { + return ( + typeof weight === 'number' && + weight >= 0 && + weight <= 1 + TrackinsightDataEnhancerService.weightTolerance + ); + } + + private async searchTrackinsightSymbol({ + requestTimeout, + symbol + }: { + requestTimeout: number; + symbol: string; + }) { + const jsonRes = await this.fetchJson({ + requestTimeout, + url: `${TrackinsightDataEnhancerService.baseUrl}/search-api/search_v2/${encodeURIComponent(symbol)}/_/ticker/default/0/3` + }); + + if (!jsonRes) { + return undefined; + } + + if (!Array.isArray(jsonRes.results?.docs)) { + this.logger.warn( + `Could not search Trackinsight symbol for "${symbol}": Invalid response` + ); + + return undefined; + } + + const ticker = jsonRes.results.docs[0]?.ticker; + + if ( + typeof ticker === 'string' && + (jsonRes.results.count === 1 || + // Allow exact match + ticker === symbol || + // Allow EXCHANGE:SYMBOL + ticker.endsWith(`:${symbol}`)) + ) { + return ticker; + } + + this.logger.debug( + `Could not match a Trackinsight symbol for "${symbol}": ${JSON.stringify( + jsonRes + )}` + ); + + return undefined; + } } diff --git a/apps/api/src/services/queues/data-gathering/data-gathering.service.spec.ts b/apps/api/src/services/queues/data-gathering/data-gathering.service.spec.ts index 9b63037ce2..93aa233fba 100644 --- a/apps/api/src/services/queues/data-gathering/data-gathering.service.spec.ts +++ b/apps/api/src/services/queues/data-gathering/data-gathering.service.spec.ts @@ -9,31 +9,47 @@ import { DataGatheringService } from './data-gathering.service'; describe('DataGatheringService', () => { let dataGatheringQueue: { addBulk: jest.Mock; clean: jest.Mock }; let dataGatheringService: DataGatheringService; - let dataProviderService: { getHistoricalRaw: jest.Mock }; - let prismaService: { marketData: { groupBy: jest.Mock; upsert: jest.Mock } }; + let dataProviderService: { + getAssetProfiles: jest.Mock; + getHistoricalRaw: jest.Mock; + }; + let prismaService: { + marketData: { groupBy: jest.Mock; upsert: jest.Mock }; + symbolProfile: { upsert: jest.Mock }; + }; + let symbolProfileService: { getSymbolProfiles: jest.Mock }; beforeEach(() => { dataGatheringQueue = { addBulk: jest.fn().mockResolvedValue([]), clean: jest.fn().mockResolvedValue([]) }; - dataProviderService = { getHistoricalRaw: jest.fn() }; + dataProviderService = { + getAssetProfiles: jest.fn(), + getHistoricalRaw: jest.fn() + }; prismaService = { marketData: { groupBy: jest.fn().mockResolvedValue([]), upsert: jest.fn().mockResolvedValue({}) + }, + symbolProfile: { + upsert: jest.fn().mockResolvedValue({}) } }; + symbolProfileService = { + getSymbolProfiles: jest.fn().mockResolvedValue([]) + }; dataGatheringService = new DataGatheringService( - null, + [], dataGatheringQueue as any, dataProviderService as any, null, null, prismaService as any, null, - null + symbolProfileService as any ); }); @@ -41,6 +57,58 @@ describe('DataGatheringService', () => { jest.useRealTimers(); }); + describe('gatherAssetProfiles', () => { + const assetProfileIdentifier = { dataSource: 'YAHOO', symbol: 'VWCE.DE' }; + + it('keeps the stored lists if the lists of the asset profile are empty', async () => { + dataProviderService.getAssetProfiles.mockResolvedValue({ + 'VWCE.DE': { + ...assetProfileIdentifier, + countries: [], + holdings: [], + sectors: [] + } + }); + + await dataGatheringService.gatherAssetProfiles([ + assetProfileIdentifier as any + ]); + + const { create, update } = + prismaService.symbolProfile.upsert.mock.calls[0][0]; + + expect(create.holdings).toEqual([]); + expect(update.countries).toBeUndefined(); + expect(update.holdings).toBeUndefined(); + expect(update.sectors).toBeUndefined(); + }); + + it('updates the stored lists if the lists of the asset profile have items', async () => { + const countries = [{ code: 'US', weight: 1 }]; + const holdings = [{ name: 'NVIDIA Corp.', weight: 0.05 }]; + const sectors = [{ name: 'Technology', weight: 1 }]; + + dataProviderService.getAssetProfiles.mockResolvedValue({ + 'VWCE.DE': { + ...assetProfileIdentifier, + countries, + holdings, + sectors + } + }); + + await dataGatheringService.gatherAssetProfiles([ + assetProfileIdentifier as any + ]); + + const { update } = prismaService.symbolProfile.upsert.mock.calls[0][0]; + + expect(update).toEqual( + expect.objectContaining({ countries, holdings, sectors }) + ); + }); + }); + describe('getAssetProfileIdentifiersWithRecentMarketData', () => { it('queries real market prices since the start of yesterday (UTC)', async () => { jest.useFakeTimers().setSystemTime(new Date('2026-08-24T14:00:00.000Z')); diff --git a/apps/api/src/services/queues/data-gathering/data-gathering.service.ts b/apps/api/src/services/queues/data-gathering/data-gathering.service.ts index 83d722888f..fa96b1201c 100644 --- a/apps/api/src/services/queues/data-gathering/data-gathering.service.ts +++ b/apps/api/src/services/queues/data-gathering/data-gathering.service.ts @@ -41,6 +41,7 @@ import { subMilliseconds, subYears } from 'date-fns'; +import { isEmpty } from 'lodash-es'; import ms, { StringValue } from 'ms'; @Injectable() @@ -177,17 +178,18 @@ export class DataGatheringService { update: { assetClass, assetSubClass, - countries, currency, cusip, figi, figiComposite, figiShareClass, - holdings, isin, name, - sectors, - url + url, + // Keep the stored data if no list is available + countries: isEmpty(countries) ? undefined : countries, + holdings: isEmpty(holdings) ? undefined : holdings, + sectors: isEmpty(sectors) ? undefined : sectors }, where: { dataSource_symbol: {