From 69e866fa53f5a68dbf45bffa555c404383190d64 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Tue, 13 Oct 2026 13:32:19 +0000 Subject: [PATCH] refactor: Decouple transform manager init * refactor all transform getters so that its solely based on config rather than what transformers are arleady initialized * use promise to init all valid transformers for metadata providers, separate from tranform stages --- .../common/transforms/TransformerManager.ts | 330 +++++++----------- .../tests/component/transformers.test.ts | 77 +++- 2 files changed, 198 insertions(+), 209 deletions(-) diff --git a/src/backend/common/transforms/TransformerManager.ts b/src/backend/common/transforms/TransformerManager.ts index bcac038e..dea125e5 100644 --- a/src/backend/common/transforms/TransformerManager.ts +++ b/src/backend/common/transforms/TransformerManager.ts @@ -1,7 +1,7 @@ import { childLogger, type Logger } from "@foxxmd/logging"; import type AbstractTransformer from "./AbstractTransformer.ts"; import type {MusicServicesBase, OptionalCacheUsage, TransformerCommon, TransformerCommonConfig} from "../../../core/Atomic.ts"; -import {DEFAULT_TRANSFORMER_ENV_NAME, DEFAULT_TRANSFORMER_NAME, type MetadataProviderStageType, type StageConfig} from "../../../core/Transform.ts"; +import {DEFAULT_TRANSFORMER_ENV_NAME, DEFAULT_TRANSFORMER_NAME, META_PROVIDER_STAGE_TYPES, type MetadataProviderStageType, type StageConfig} from "../../../core/Transform.ts"; import type {PlayObject} from "../../../core/Atomic.ts"; import { isStageTyped } from "../../utils/PlayTransformUtils.ts"; import type { MSCache } from "../Cache.ts"; @@ -33,6 +33,10 @@ export default class TransformerManager { protected transformMetadataProviders: Partial> = {}; protected transformMetadataPreferredProviders: Partial> | undefined; + /** One register + initialize attempt per transformer, keyed by type and name */ + protected ready: Map> = new Map(); + protected metadataInit?: Promise; + public constructor(logger: Logger, cache: MSCache) { this.logger = childLogger(logger, 'Transformer Manager'); this.parentLogger = logger; @@ -66,21 +70,7 @@ export default class TransformerManager { return this.transformerConfigs.some(x => x.type === type.toLocaleLowerCase().trim()); } - public async registerByIdentifiers(type: string, name: string = DEFAULT_TRANSFORMER_NAME) { - const transformers = this.transformers.get(type.toLocaleLowerCase().trim()); - if(transformers !== undefined && transformers.has(name.toLocaleLowerCase().trim())) { - this.logger.debug(`Transformer type ${type} with name ${name} already registered`); - return; - } - const config = this.transformerConfigs.find(x => x.name.toLocaleLowerCase().trim() === name.toLocaleLowerCase().trim() && x.type === type.toLocaleLowerCase().trim()); - if(config === undefined) { - throw new Error(`No existing configuration for transformer of type ${type} with name ${name} exists`); - } - this.logger.debug(`Registering transform type ${type} with name ${name}`); - await this.register(config); - } - - public async register(config: TransformerCommon): Promise { + public async register(config: TransformerCommon): Promise { // TODO replace with getOrInsert in node 26+ let namedTransformerMap = this.transformers.get(config.type.toLocaleLowerCase().trim()); if(namedTransformerMap === undefined) { @@ -92,6 +82,10 @@ export default class TransformerManager { throw new Error(`Cannot register ${config.type} transformer with name '${config.name}' because an existing transformer already has that name`); } + if(!this.hasTransformerConfigByIdentifiers(config.type, config.name)) { + this.transformerConfigs.push({...config, type: config.type.toLocaleLowerCase().trim()}); + } + this.logger.verbose(`Registering ${config.type} transformer with name '${config.name}'`); const tLogger = childLogger(this.parentLogger, ['Transformer', () => this.asyncStore.getStore() ?? undefined]); @@ -128,6 +122,43 @@ export default class TransformerManager { this.transformers.get(config.type.toLocaleLowerCase().trim())!.set(config.name.toLocaleLowerCase().trim(), t); this.logger.verbose(`${config.type} transformer with name '${config.name}' registered`); + return t; + } + + protected transformerKey(config: TransformerCommon): string { + return `${config.type}:${config.name}`.toLocaleLowerCase().trim(); + } + + /** + * Register (if needed) and initialize a single transformer, once. + * + * Callers only wait on the transformer they asked for. A failed initialization is logged and the (not ready) transformer is still returned. + * */ + protected ensure(config: TransformerCommon): Promise { + const key = this.transformerKey(config); + let p = this.ready.get(key); + if(p === undefined) { + const existing = this.transformers.get(config.type.toLocaleLowerCase().trim())?.get(config.name.toLocaleLowerCase().trim()); + p = (existing !== undefined ? Promise.resolve(existing) : this.register(config)).then(t => this.initTransformer(t)); + // only a failed registration rejects, let it be tried again + p.catch(() => this.ready.delete(key)); + this.ready.set(key, p); + } + return p; + } + + protected async initTransformer(transformer: AbstractTransformer): Promise { + if (!transformer.isReady() && !transformer.initializing) { + if (!transformer.canAuthUnattended()) { + transformer.logger.warn({ label: 'Heartbeat' }, 'Transformer is not ready but will not try to initialize because auth state is not good and cannot be correct unattended.'); + } + try { + await transformer.initialize({ force: false, notify: true, notifyTitle: 'Could not initialize automatically' }); + } catch (e) { + transformer.logger.error(new Error('Could not initialize source automatically', { cause: e })); + } + } + return transformer; } public async registerFromEnv() { @@ -185,74 +216,50 @@ export default class TransformerManager { } } - public async initTransformers() { - this.logger.verbose('Initializing transformers...'); - for (const list of this.transformers.values()) { - for (const transformer of list.values()) { - if (!transformer.isReady() && !transformer.initializing) { - if (!transformer.canAuthUnattended()) { - transformer.logger.warn({ label: 'Heartbeat' }, 'Transformer is not ready but will not try to initialize because auth state is not good and cannot be correct unattended.'); - } - try { - await transformer.initialize({ force: false, notify: true, notifyTitle: 'Could not initialize automatically' }); - - if(asMetadataProvider(transformer)) { - const t = transformer.transformType as MetadataProviderStageType; - if(this.transformMetadataPreferredProviders !== undefined) { - // only use transformers user prefers and don't add any they don't explicitly set - if(this.transformMetadataPreferredProviders[t] !== undefined) { - - if(this.transformMetadataPreferredProviders[t] === transformer.name) { - // use transformer if defined by name - this.transformMetadataProviders[t] = transformer.name; - this.logger.debug(`Using preferred named ${t} transformer '${transformer.name}' as metadata provider.`); - } else if(this.transformMetadataPreferredProviders[t] === true) { - // or user has set `true` to indicate any - if(this.transformMetadataProviders[t] === undefined) { - // use this one if none is already set - this.transformMetadataProviders[t] = transformer.name; - this.logger.debug(`Using preferred ${t} transformer with priorities. Not set yet so using '${transformer.name}' as metadata provider.`); - } else { - // otherwise set based on priority - const p = getTransformByPriority([this.transformers.get(t)?.get(this.transformMetadataProviders[t]) as AbstractTransformer, transformer]); - if(p.nonDefaultMany !== true) { - const old = this.transformMetadataProviders[t]; - this.transformMetadataProviders[t] = p.nonDefault?.name ?? p.env?.name ?? p.default?.name; - this.logger.debug(`Using preferred ${t} transformer with priorities, a new transformer had higher priority. Replaced '${old}' with ${this.transformMetadataProviders[t]}`); - } else { - this.logger.debug(`Using preferred ${t} transformer with priorities and a non-default was already set, skipping '${transformer.name}' as metadata provider.`); - } - // if both are non default then use the one that was already set - } - } - // otherwise do not add - } else { - this.logger.debug(`Preferred named metadata providers are defined but type ${t} is not set, skipping adding ${transformer.name}`); - } - // not explicitly set, don't add any - } else { - // otherwise set all that can be used and set based on priority - // when more than one is available - if(this.transformMetadataProviders[t] === undefined) { - this.logger.debug(`No preferred metadata providers and none already set for ${t}, using '${transformer.name}' as metadata provider.`); - this.transformMetadataProviders[t] = transformer.name; - } else { - const p = getTransformByPriority([this.transformers.get(t)?.get(this.transformMetadataProviders[t]) as AbstractTransformer, transformer]); - if(p.nonDefaultMany !== true) { - const old = this.transformMetadataProviders[t]; - this.transformMetadataProviders[t] = p.nonDefault?.name ?? p.env?.name ?? p.default?.name; - this.logger.debug(`No preferred metadata providers, a new transformer for ${t} had higher priority. Replaced ${old} with ${this.transformMetadataProviders[t]}`); - } - } - } - } - } catch (e) { - transformer.logger.error(new Error('Could not initialize source automatically', { cause: e })); - } + protected configByPriority(type: string): TransformPriorityObj { + return getTransformByPriority(this.transformerConfigs.filter(x => x.type === type)); + } + + /** + * Register and initialize the transformers used as metadata providers, once, on first use + * */ + protected initMetadataProviders(): Promise { + return this.metadataInit ??= (async () => { + const preferred = this.transformMetadataPreferredProviders; + const configs: TransformerCommon[] = []; + for(const t of META_PROVIDER_STAGE_TYPES.options) { + const pref = preferred?.[t]; + if(preferred !== undefined && pref === undefined) { + // only use transformers user prefers and don't add any they don't explicitly set + this.logger.debug(`Preferred metadata providers are defined but type ${t} is not set, skipping`); + continue; + } + let config: TransformerCommon | undefined; + if(typeof pref === 'string') { + config = this.transformerConfigs.find(x => x.type === t && x.name.toLocaleLowerCase() === pref.toLocaleLowerCase().trim()); + } else { + const p = this.configByPriority(t); + config = p.nonDefault ?? p.env ?? p.default; } + if(config === undefined) { + this.logger.debug(`No usable ${t} transformer config for metadata provider`); + continue; + } + this.logger.debug(`Using ${t} transformer '${config.name}' as metadata provider.`); + this.transformMetadataProviders[t] = config.name; + configs.push(config); } - } - this.logger.verbose('Done initializing transformers'); + const results = await Promise.allSettled(configs.map(x => this.ensure(x))); + results.forEach((r, i) => { + const config = configs[i]; + if(r.status === 'rejected') { + this.logger.error(new Error(`Could not register ${config.type} transformer '${config.name}' as metadata provider`, {cause: r.reason})); + delete this.transformMetadataProviders[config.type as MetadataProviderStageType]; + } else if(!asMetadataProvider(r.value)) { + delete this.transformMetadataProviders[config.type as MetadataProviderStageType]; + } + }); + })(); } public hasTransformerType(type: string): boolean { @@ -265,86 +272,36 @@ export default class TransformerManager { public async getTransformerByStage(data: StageConfig): Promise { const transformType: string = data.type.toLocaleLowerCase().trim(); - let configName: string | undefined = data.name; - - let namedMap = this.transformers.get(transformType); - if (namedMap === undefined || namedMap.size === 0) { - if(!this.hasTransformerConfigByType(transformType)) { - throw new Error(`No transformer configurations of type '${transformType}' exist.`); - } - if(configName !== undefined) { - // if name for transform was specific then try to init and use that specific one - if(this.hasTransformerConfigByIdentifiers(transformType, configName)) { - await this.registerByIdentifiers(transformType, configName); - - await this.initTransformers(); - namedMap = this.transformers.get(transformType) - } else { - throw new Error(`No transformer configuration of type '${transformType}' with name '${configName}' exists.`); - } - } else { - // otherwise we try to get *any* transform of this type, starting with non-default, then env, then default - let anyExistingConfig: TransformerCommonConfig | undefined = undefined; - const configs = getTransformByPriority(this.transformerConfigs.filter(x => x.type === transformType)); - if(configs.nonDefault !== undefined) { - if(configs.nonDefaultMany) { - this.logger.warn(`No config name specified and more than one non-default exists. Using the first found non-default (${configs.nonDefault.name})`); - } else { - this.logger.verbose(`No config name specified, using the first found non-default config (${configs.nonDefault.name})`); - } - anyExistingConfig = configs.nonDefault; - } else if(configs.env !== undefined) { - this.logger.verbose(`No config name specified, using found ENV config since no non-default configs exist (${configs.env.name})`); - anyExistingConfig = configs.env; - } else if(configs.default !== undefined) { - this.logger.verbose(`No config name specified and no non-default or ENV configs exist, using default (${configs.default.name})`); - anyExistingConfig = configs.default; - } - if(anyExistingConfig === undefined) { - throw new Error(`No transformer configurations of type '${transformType}' exist.`); - } - await this.registerByIdentifiers(anyExistingConfig.type, anyExistingConfig.name); - await this.initTransformers(); - configName = anyExistingConfig.name; - namedMap = this.transformers.get(transformType) - } + if(!this.hasTransformerConfigByType(transformType)) { + throw new Error(`No transformer configurations of type '${transformType}' exist.`); } - if (namedMap === undefined || namedMap.size === 0) { - throw new Error(`No transformers of type '${transformType}' could be registered.`); - } - - if(configName === undefined) { - const transformers = getTransformByPriority(namedMap.values().toArray()); - if(transformers.nonDefault !== undefined) { - return transformers.nonDefault; - } - if(transformers.env !== undefined) { - return transformers.env; + let config: TransformerCommon | undefined; + if(data.name !== undefined) { + const configName = data.name.toLocaleLowerCase().trim(); + config = this.transformerConfigs.find(x => x.type === transformType && x.name.toLocaleLowerCase() === configName); + if(config === undefined) { + throw new Error(`No transformer configuration of type '${transformType}' with name '${data.name}' exists.`); } - if(transformers.default !== undefined) { - return transformers.default; + } else { + // otherwise we try to get *any* transform of this type, starting with non-default, then env, then default + const configs = this.configByPriority(transformType); + config = configs.nonDefault ?? configs.env ?? configs.default; + if(config === undefined) { + // shouldn't happen at this point but covering strict ts + throw new Error(`No transformer configurations of type '${transformType}' exist.`); } - // shouldn't happen at this point but covering strict ts - throw new Error(`No transformers of type '${transformType}' could be found`); - } - - let namedTransformer = namedMap.get(configName.toLocaleLowerCase().trim()); - if(namedTransformer === undefined) { - if(this.hasTransformerConfigByIdentifiers(data.type, data.name)) { - await this.registerByIdentifiers(data.type, data.name); - await this.initTransformers(); - namedTransformer = this.transformers.get(data.type)?.get(configName.toLocaleLowerCase().trim()); - if(namedTransformer === undefined) { - // this shouldn't really happen but just covering bases - throw new SimpleError(`Component wanted transformer type ${data.type} with name ${data.name}. Transforms of this type are registered but none have this name.`); + if(!this.ready.has(this.transformerKey(config))) { + if(configs.nonDefaultMany) { + this.logger.warn(`No config name specified and more than one non-default exists. Using the first found non-default (${config.name})`); + } else { + this.logger.verbose(`No config name specified, using ${config.name}`); } - return namedTransformer; } - throw new SimpleError(`Component wanted transformer type ${data.type} with name ${data.name}. Transforms of this type are registered but none have this name.`); } - return namedTransformer; + + return await this.ensure(config); } public async parseTransformerConfig(data: any) { @@ -369,16 +326,17 @@ export default class TransformerManager { } } - public async getTrackResults(query:TrackDataCreditBase | MusicServicesBase): Promise> { - const readyMps: Record>>> = {}; + protected async aggregateResults(method: keyof MetadataProvider, query: TrackDataCreditBase | MusicServicesBase): Promise> { + await this.initMetadataProviders(); + const readyMps: Record> = {}; for(const [type, name] of Object.entries(this.transformMetadataProviders)) { const t = this.transformers.get(type)?.get(name.toLocaleLowerCase()) as unknown as MetadataProvider & AbstractTransformer; if(t !== undefined && t.isReady()) { - readyMps[type] = t.getTrackResults(query); + readyMps[type] = t[method](query) as Promise; } } const all = await pPropsAllSettled(readyMps); - const res: AggregateMetadataResponse = { + const res: AggregateMetadataResponse = { data: [], errors: [] } @@ -396,59 +354,15 @@ export default class TransformerManager { res.data.sort((a, b) => (b.score ?? 0) - (a.score ?? 0)); return res; } + + public async getTrackResults(query:TrackDataCreditBase | MusicServicesBase): Promise> { + return this.aggregateResults('getTrackResults', query); + } public async getArtistResults(query: TrackDataCreditBase | MusicServicesBase): Promise> { - const readyMps: Record>>> = {}; - for(const [type, name] of Object.entries(this.transformMetadataProviders)) { - const t = this.transformers.get(type)?.get(name.toLocaleLowerCase()) as unknown as MetadataProvider & AbstractTransformer; - if(t !== undefined && t.isReady()) { - readyMps[type] = t.getArtistResults(query); - } - } - const all = await pPropsAllSettled(readyMps); - const res: AggregateMetadataResponse = { - data: [], - errors: [] - } - for(const [name, r] of Object.entries(all)) { - if(r.status === 'fulfilled') { - if(r.value === false) { - res.errors.push({service: name, error: {message: 'Not Ready'}}); - } else { - res.data = res.data.concat(r.value) - } - } else { - res.errors.push({service: name, error: r.reason}); - } - } - res.data.sort((a, b) => (b.score ?? 0) - (a.score ?? 0)); - return res; + return this.aggregateResults('getArtistResults', query); } public async getAlbumResults(query: TrackDataCreditBase | MusicServicesBase): Promise> { - const readyMps: Record>>> = {}; - for(const [type, name] of Object.entries(this.transformMetadataProviders)) { - const t = this.transformers.get(type)?.get(name.toLocaleLowerCase()) as unknown as MetadataProvider & AbstractTransformer; - if(t !== undefined && t.isReady()) { - readyMps[type] = t.getAlbumResults(query); - } - } - const all = await pPropsAllSettled(readyMps); - const res: AggregateMetadataResponse = { - data: [], - errors: [] - } - for(const [name, r] of Object.entries(all)) { - if(r.status === 'fulfilled') { - if(r.value === false) { - res.errors.push({service: name, error: {message: 'Not Ready'}}); - } else { - res.data = res.data.concat(r.value) - } - } else { - res.errors.push({service: name, error: r.reason}); - } - } - res.data.sort((a, b) => (b.score ?? 0) - (a.score ?? 0)); - return res; + return this.aggregateResults('getAlbumResults', query); } } diff --git a/src/backend/tests/component/transformers.test.ts b/src/backend/tests/component/transformers.test.ts index ede192b6..5c028abd 100644 --- a/src/backend/tests/component/transformers.test.ts +++ b/src/backend/tests/component/transformers.test.ts @@ -4,7 +4,7 @@ import asPromised from 'chai-as-promised'; import { describe, it } from 'mocha'; import AbstractComponent, { type AbstractComponentConfig } from "../../common/AbstractComponent.ts"; -import { type ConditionalSearchAndReplaceRegExp, type StageTypeMetadata, TRANSFORM_HOOK } from "../../../core/Transform.ts"; +import { type ConditionalSearchAndReplaceRegExp, DEFAULT_TRANSFORMER_NAME, type StageTypeMetadata, TRANSFORM_HOOK } from "../../../core/Transform.ts"; import { isConditionalSearchAndReplace } from "../../utils/PlayTransformUtils.ts"; import { generateArtistsStr, generatePlay } from "../../../core/tests/utils/PlayTestUtils.ts"; @@ -15,6 +15,7 @@ import { initMemoryCache } from "../../common/Cache.ts"; import { Cacheable } from "cacheable"; import type {PlayMatchResult, PlayObject, QueueContext, TransformerCommon, TransformerCommonConfig} from "../../../core/Atomic.ts"; import TransformerManager from "../../common/transforms/TransformerManager.ts"; +import type AbstractTransformer from "../../common/transforms/AbstractTransformer.ts"; import { transientCache } from "../utils/TransientTestUtils.ts"; import dayjs from "dayjs"; import clone from "clone"; @@ -27,6 +28,25 @@ import type { PlayProcessingResult } from "../../common/infrastructure/PlayProce chai.use(asPromised); +class TestTransformerManager extends TransformerManager { + public hanging: string[] = []; + public setPreferredProviders(preferred: TransformerManager['transformMetadataPreferredProviders']) { + this.transformMetadataPreferredProviders = preferred; + } + public getProviders() { + return this.transformMetadataProviders; + } + public initProviders() { + return this.initMetadataProviders(); + } + protected async initTransformer(transformer: AbstractTransformer): Promise { + if(this.hanging.includes(transformer.name)) { + return new Promise(() => {}); + } + return super.initTransformer(transformer); + } +} + const componentId = () => `test-${Date.now()}`; class TestComponent extends AbstractComponent { existingPlay(playObjPre: PlayObject, existingScrobbles: PlayObject[], log?: boolean): Promise { @@ -945,6 +965,61 @@ describe('Play Transforms', function () { expect(multiTransformComponent.transformRules.preCompare![0].name).eq('MYFOOCONFIG') expect(multiTransformComponent.transformRules.preCompare![1].name).eq('mybarconfig') }); + + it('Registers metadata providers without any stage usage', async function() { + const tmanager = new TestTransformerManager(loggerTest, transientCache()); + expect(tmanager.hasTransformerType('musicbrainz')).is.false; + + await tmanager.initProviders(); + + expect(tmanager.getProviders()).to.have.all.keys('musicbrainz', 'rocksky'); + expect(tmanager.getTransformerType('musicbrainz')?.get('msdefault')?.isReady()).is.true; + expect(tmanager.getTransformerType('rocksky')?.get('msdefault')?.isReady()).is.true; + }); + + it('Registers only preferred metadata providers', async function() { + const tmanager = new TestTransformerManager(loggerTest, transientCache()); + tmanager.setPreferredProviders({musicbrainz: true}); + + await tmanager.initProviders(); + + expect(tmanager.getProviders()).to.have.all.keys('musicbrainz'); + expect(tmanager.hasTransformerType('rocksky')).is.false; + }); + + it('Registers a transformer once when requested concurrently', async function() { + const tmanager = new TransformerManager(loggerTest, transientCache()); + const [a, b] = await Promise.all([ + tmanager.getTransformerByStage({type: 'user'}), + tmanager.getTransformerByStage({type: 'user'}) + ]); + expect(a).eq(b); + expect(a.isReady()).is.true; + }); + + it('Does not wait on other transformers initializing', async function() { + const tmanager = new TestTransformerManager(loggerTest, transientCache()); + tmanager.addTransformerConfig({type: 'user', name: 'slow'}); + tmanager.addTransformerConfig({type: 'user', name: 'fast'}); + tmanager.hanging.push('slow'); + + void tmanager.getTransformerByStage({type: 'user', name: 'slow'}); + const fast = await tmanager.getTransformerByStage({type: 'user', name: 'fast'}); + + expect(fast.name).eq('fast'); + expect(fast.isReady()).is.true; + }); + + it('Uses highest priority config for unnamed stage regardless of what is registered', async function() { + const tmanager = new TransformerManager(loggerTest, transientCache()); + tmanager.addTransformerConfig({type: 'user', name: 'mine'}); + + const def = await tmanager.getTransformerByStage({type: 'user', name: DEFAULT_TRANSFORMER_NAME}); + expect(def.name).eq(DEFAULT_TRANSFORMER_NAME); + + const unnamed = await tmanager.getTransformerByStage({type: 'user'}); + expect(unnamed.name).eq('mine'); + }); }); }) -- 2.54.0