From 5eb229d9e05e552ff41ae66b39c6b372f8d7d535 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Fri, 25 Sep 2026 10:20:26 -0400 Subject: [PATCH] refactor(rocksky): Transformer cleanup and undefined guards --- .../transforms/MusicbrainzTransformer.ts | 143 +++++++------- .../musicbrainz/MusicbrainzTransformerUtil.ts | 2 +- .../transforms/rocksky/RockskyTransformer.ts | 181 ++++++++++-------- .../rocksky/RockskyTransformerUtil.ts | 2 +- 4 files changed, 172 insertions(+), 156 deletions(-) diff --git a/src/backend/common/transforms/MusicbrainzTransformer.ts b/src/backend/common/transforms/MusicbrainzTransformer.ts index 6f5a3d4d..8bb39277 100644 --- a/src/backend/common/transforms/MusicbrainzTransformer.ts +++ b/src/backend/common/transforms/MusicbrainzTransformer.ts @@ -260,7 +260,7 @@ export default class MusicbrainzTransformer extends AtomicPartsTransformer { this.defaults = parseStageConfig(this.config.defaults, childLogger(this.logger, 'Defaults')); - this.api = new MusicbrainzApiClientPool(this.config.name, {apis: this.config.data!.apis}, { + this.api = new MusicbrainzApiClientPool(this.config.name, {apis: this.config.data?.apis ?? [{enable: true}]}, { logger: this.logger, cache: this.clientCache, logUrl: this.config.options?.logUrl @@ -340,25 +340,25 @@ export default class MusicbrainzTransformer extends AtomicPartsTransformer { + public async searchByBasicFields(play: PlayObject, stageConfig: MusicbrainzTransformerDataStage, opts: OptionalCacheUsage = {}): Promise { this.logger.debug({labels: ['Basic Search']}, 'Searching by artist/album/track'); return await this.api.searchByRecording(play, opts); } - public async searchByBasicFieldsOrMBIDs(play: PlayObject, stageConfig: MusicbrainzTransformerDataStage, opts: OptionalCacheUsage = {}): Promise { + public async searchByBasicFieldsOrMBIDs(play: PlayObject, stageConfig: MusicbrainzTransformerDataStage, opts: OptionalCacheUsage = {}): Promise { const using: UsingTypes[] = []; const { data: { @@ -412,7 +412,7 @@ export default class MusicbrainzTransformer extends AtomicPartsTransformer { + public async searchByIsrc(play: PlayObject, stageConfig: MusicbrainzTransformerDataStage, opts: OptionalCacheUsage = {}): Promise { if(play.data.isrc !== undefined) { this.logger.debug({labels: ['ISRC Search']},'Searching with ISRC'); return await this.api.searchByRecording(play, {using: ['isrc'], ...opts}); @@ -420,7 +420,7 @@ export default class MusicbrainzTransformer extends AtomicPartsTransformer { + public async searchByRecordingMbid(play: PlayObject, stageConfig: MusicbrainzTransformerDataStage, opts: OptionalCacheUsage = {}): Promise { if(play.data.meta?.brainz?.recording !== undefined) { this.logger.debug({labels: ['MBID Search']},'Searching with Recording MBID'); return await this.api.searchByRecording(play, {using: ['mbidrecording'], ...opts}); @@ -428,7 +428,7 @@ export default class MusicbrainzTransformer extends AtomicPartsTransformer { + public async searchByAlbum(play: PlayObject, stageConfig: MusicbrainzTransformerDataStage, opts: OptionalCacheUsage = {}): Promise { // possibly the artist is incorrect (may be combined as one string) // if we have an album we can likely still get a decent hit w/o using artist if(play.data.album !== undefined && play.data.artists !== undefined && play.data.artists.length > 0) { @@ -441,57 +441,56 @@ export default class MusicbrainzTransformer extends AtomicPartsTransformer { + public async searchByArtist(play: PlayObject, stageConfig: MusicbrainzTransformerDataStage, opts: OptionalCacheUsage = {}): Promise { const { searchArtistMethod = this.defaults.searchArtistMethod, } = stageConfig; - // if only one artist - // then its likely artist string is combined - if(play.data.artists !== undefined && play.data.artists.length === 1) { - - if(searchArtistMethod === 'naive') { - // try a naive split using any common delimiter found and use the first value as artist - // -- this will likely result in a less accurate match but at least it might find something - // -- usually the "primary artist" is listed first in a combined artist string so cross your fingers this works - const naiveSplit = splitByFirstRegexFound(play.data.artists[0].name, [play.data.artists[0].name]).map(x => x.trim()); - if(naiveSplit.length > 1) { - this.logger.debug({labels: ['Parsed Artist Search']},'Searching with track + first value from artist string split'); - return await this.api.searchByRecording({...play, data: { - ...play.data, - artists: [{name: naiveSplit[0]}] - }}, {using: ['title','artist']}); - } else { - throw new SearchPrerequisiteError('Naive parsing did not produce multiple artists'); - } - } else if(searchArtistMethod === 'native') { - // use MS native parsing to extract artists from artist string and title - // -- cleaning title is aggressive but at this point MB has not found anything which means its likely not "proper" - // IE "My Track (Cool Remix)" is a proper recording name but "My Track (feat. Someone)" is not bc MB would have removed the joiner from the name - // so we do the same to hopefully get a match - // - // ...additionally, since MB hasn't found anything with single artist string its likely the artist name does not have a common delimiter as part of their proper name - // IE "Crosby, Stills, Nash & Young" is a proper artist name with delimiters included but "My Artist & My Feat Artists" is not - // so we split out all artists by all found delimiters - const nativePlay = nativeParse(play, {titleClean: true, delimiters: DELIMITERS}); - this.logger.debug({labels: ['Parsed Artist Search']},'Searching with aggressive native parsing'); - return await this.api.searchByRecording(nativePlay, {using: ['title','artist'], ...opts}); - } - } - - if(play.data.artists === undefined) { - throw new SearchPrerequisiteError('Play does not have any artists'); - } - if(play.data.artists.length > 1) { - throw new SearchPrerequisiteError('Play has more than one artist already'); + if(play.data.artists === undefined) { + throw new SearchPrerequisiteError('Play does not have any artists'); + } + if(play.data.artists.length > 1) { + throw new SearchPrerequisiteError('Play has more than one artist already'); + } + if(play.data.artists.length === 0) { + throw new SearchPrerequisiteError('Play does not have any artists'); + } + // if only one artist + // then its likely artist string is combined + if(searchArtistMethod === 'naive') { + // try a naive split using any common delimiter found and use the first value as artist + // -- this will likely result in a less accurate match but at least it might find something + // -- usually the "primary artist" is listed first in a combined artist string so cross your fingers this works + const naiveSplit = splitByFirstRegexFound(play.data.artists[0].name, [play.data.artists[0].name]).map(x => x.trim()); + if(naiveSplit.length > 1) { + this.logger.debug({labels: ['Parsed Artist Search']},'Searching with track + first value from artist string split'); + return await this.api.searchByRecording({...play, data: { + ...play.data, + artists: [{name: naiveSplit[0]}] + }}, {using: ['title','artist']}); } + throw new SearchPrerequisiteError('Naive parsing did not produce multiple artists'); + } + if(searchArtistMethod === 'native') { + // use MS native parsing to extract artists from artist string and title + // -- cleaning title is aggressive but at this point MB has not found anything which means its likely not "proper" + // IE "My Track (Cool Remix)" is a proper recording name but "My Track (feat. Someone)" is not bc MB would have removed the joiner from the name + // so we do the same to hopefully get a match + // + // ...additionally, since MB hasn't found anything with single artist string its likely the artist name does not have a common delimiter as part of their proper name + // IE "Crosby, Stills, Nash & Young" is a proper artist name with delimiters included but "My Artist & My Feat Artists" is not + // so we split out all artists by all found delimiters + const nativePlay = nativeParse(play, {titleClean: true, delimiters: DELIMITERS}); + this.logger.debug({labels: ['Parsed Artist Search']},'Searching with aggressive native parsing'); + return await this.api.searchByRecording(nativePlay, {using: ['title','artist'], ...opts}); + } + throw new SearchPrerequisiteError(`Unknown searchArtistMethod '${searchArtistMethod}'`); } - public async searchByFreeText(play: PlayObject, stageConfig: MusicbrainzTransformerDataStage, opts: OptionalCacheUsage = {}): Promise { + public async searchByFreeText(play: PlayObject, stageConfig: MusicbrainzTransformerDataStage, opts: OptionalCacheUsage = {}): Promise { this.logger.debug({labels: ['Freetext Search']},'Trying freetext search'); const results = await this.api.searchByRecording(play, {freetext: true, ...opts}) as IRecordingMSList; results.freeText = true; @@ -667,7 +666,7 @@ export const filterByValidReleaseStatus = (list: T, }) })); return releaseFiltered.filter(x => ( - (list.find(y => y.id === x.id)!.releases ?? []).length === 0 + (list.find(y => y.id === x.id)?.releases ?? []).length === 0 && releaseAllowEmpty ) || x.releases.length > 0); @@ -692,7 +691,7 @@ export const filterByValidReleaseGroupPrimary = (li }) })); return releaseFiltered.filter(x => ( - (list.find(y => y.id === x.id)!.releases ?? []).length === 0 + (list.find(y => y.id === x.id)?.releases ?? []).length === 0 && releaseAllowEmpty ) || x.releases.length > 0); @@ -717,7 +716,7 @@ export const filterByValidReleaseGroupSecondary = (list: IRecordingMatch[], stag }) })); return releaseFiltered.filter(x => ( - (list.find(y => y.id === x.id)!.releases ?? []).length === 0 + (list.find(y => y.id === x.id)?.releases ?? []).length === 0 && releaseAllowEmpty ) || x.releases.length > 0); @@ -742,14 +741,15 @@ export const filterByValidReleaseCountry = (list: IRecordingMatch[], stageConfig }) })); return releaseFiltered.filter(x => ( - (list.find(y => y.id === x.id)!.releases ?? []).length === 0 + (list.find(y => y.id === x.id)?.releases ?? []).length === 0 && releaseAllowEmpty ) || x.releases.length > 0); } export const filterByExplicitTrackMbid = (list: IRecordingMatch[], play: PlayObject): [IRecordingMatch[], boolean] => { - if (play.data.meta?.brainz?.track === undefined) { + const trackMbid = play.data.meta?.brainz?.track; + if (trackMbid === undefined) { return [list, false]; } let recMatch: IRecordingMatch | undefined, @@ -758,13 +758,13 @@ export const filterByExplicitTrackMbid = (list: IRecordingMatch[], play: PlayObj if (recMatch !== undefined) { break; } - for (const rel of rec.releases!) { + for (const rel of rec.releases ?? []) { if (rel.media.some(x => { if(x.tracks !== undefined) { - return x.tracks.some(y => y.id === play.data.meta!.brainz!.track); + return x.tracks.some(y => y.id === trackMbid); } if(x.track !== undefined) { - return x.track.some(y => y.id === play.data.meta!.brainz!.track); + return x.track.some(y => y.id === trackMbid); } return false; })) { @@ -776,14 +776,15 @@ export const filterByExplicitTrackMbid = (list: IRecordingMatch[], play: PlayObj } if (recMatch !== undefined) { const r = structuredClone(recMatch); - r.releases = r.releases!.filter(x => x.id === releaseMatchId); + r.releases = (r.releases ?? []).filter(x => x.id === releaseMatchId); return [[r], true]; } return [list, false]; } export const filterByExplicitReleaseMbid = (list: IRecordingMatch[], play: PlayObject): [IRecordingMatch[], boolean] => { - if (play.data.meta?.brainz?.album === undefined) { + const albumMbid = play.data.meta?.brainz?.album; + if (albumMbid === undefined) { return [list, false]; } let recMatch: IRecordingMatch | undefined, @@ -792,8 +793,8 @@ export const filterByExplicitReleaseMbid = (list: IRecordingMatch[], play: PlayO if (recMatch !== undefined) { break; } - for (const rel of rec.releases!) { - if(rel.id === play.data.meta!.brainz!.album) { + for (const rel of rec.releases ?? []) { + if(rel.id === albumMbid) { releaseMatchId = rel.id; recMatch = rec; } @@ -801,7 +802,7 @@ export const filterByExplicitReleaseMbid = (list: IRecordingMatch[], play: PlayO } if (recMatch !== undefined) { const r = structuredClone(recMatch); - r.releases = r.releases!.filter(x => x.id === releaseMatchId); + r.releases = (r.releases ?? []).filter(x => x.id === releaseMatchId); return [[r], true]; } return [list, false]; @@ -836,7 +837,7 @@ export const rankReleasesByPriority = (list: IRecordingMatch[], stageConfig: Mus const grpPAScore = groupPrimaryPriority.findIndex(x => x === a["release-group"]?.["primary-type"]?.toLocaleLowerCase()) + 1; const grpSAScore = (a["release-group"]?.["secondary-types"] ?? []).reduce((acc: number, curr: string) => acc + groupSecPriority.findIndex(x => x === (curr as MBReleaseGroupSecondaryType).toLocaleLowerCase()) + 1,0); const countryAScore = countryPriority.findIndex(x => a.country === undefined ? false : x === a.country.toLocaleLowerCase()) + 1; - const compareScore = scoreNormalizedStringsWeighted(play.data.album!, a.title, albumWeight, albumWeight !== 0 ? 0.05 : 0); + const compareScore = scoreNormalizedStringsWeighted(play.data.album, a.title, albumWeight, albumWeight !== 0 ? 0.05 : 0); return { ...a, albumScore: statAScore + grpPAScore + grpSAScore + countryAScore + compareScore, @@ -848,7 +849,7 @@ export const rankReleasesByPriority = (list: IRecordingMatch[], stageConfig: Mus if(releases.length > 0) { albumScore = releases[0].albumCompareScore; } - const titleScore = titleWeight === 0 ? 0 : scoreTrackWeightedAndNormalized(play.data.track!, x.title, titleWeight, {exact: 0.05, naive: 0.03})[0]; + const titleScore = titleWeight === 0 ? 0 : scoreTrackWeightedAndNormalized(play.data.track ?? '', x.title, titleWeight, {exact: 0.05, naive: 0.03})[0]; return { ...x, diff --git a/src/backend/common/transforms/musicbrainz/MusicbrainzTransformerUtil.ts b/src/backend/common/transforms/musicbrainz/MusicbrainzTransformerUtil.ts index 93416dad..1a6562a2 100644 --- a/src/backend/common/transforms/musicbrainz/MusicbrainzTransformerUtil.ts +++ b/src/backend/common/transforms/musicbrainz/MusicbrainzTransformerUtil.ts @@ -182,7 +182,7 @@ export const configFromEnv = (logger: MaybeLogger = new MaybeLogger()) => { } if (soSet.size > 0) { - mbConfig.defaults!.searchOrder = Array.from(soSet); + mbConfig.defaults = {...mbConfig.defaults, searchOrder: Array.from(soSet)}; } logger.debug(`Using presets: ${presets.join(',')}`); } diff --git a/src/backend/common/transforms/rocksky/RockskyTransformer.ts b/src/backend/common/transforms/rocksky/RockskyTransformer.ts index 38cd457f..7f6e23a6 100644 --- a/src/backend/common/transforms/rocksky/RockskyTransformer.ts +++ b/src/backend/common/transforms/rocksky/RockskyTransformer.ts @@ -183,29 +183,36 @@ export default class RockskyTransformer extends AtomicPartsTransformer 0 && !results!.matches!.some(x => x.score! >= score)) { + if(matches.length > 0 && !matches.some(x => x.score !== undefined && x.score >= score)) { this.logger.debug(`'${searchType}' search type returned no matches with score >= ${score}`); continue; } @@ -235,11 +242,11 @@ export default class RockskyTransformer extends AtomicPartsTransformer { + public async searchByArtist(play: PlayObject, stageConfig: RockskyTransformerDataStage, opts: OptionalCacheUsage = {}): Promise { const { searchArtistMethod = this.defaults.searchArtistMethod, } = stageConfig; - // if only one artist - // then its likely artist string is combined - if(play.data.artists !== undefined && play.data.artists.length === 1) { - - if(searchArtistMethod === 'naive') { - // try a naive split using any common delimiter found and use the first value as artist - // -- this will likely result in a less accurate match but at least it might find something - // -- usually the "primary artist" is listed first in a combined artist string so cross your fingers this works - const naiveSplit = splitByFirstRegexFound(play.data.artists[0].name, [play.data.artists[0].name]).map(x => x.trim()); - if(naiveSplit.length > 1) { - this.logger.debug({labels: ['Parsed Artist Search']},'Searching with track + first value from artist string split'); - const requestQuery = JSON.stringify({ - title: play.data.track, - artitst: naiveSplit[0], - }); - try { - const res = await this.api.rsProxy.matchSong(play.data.track!, naiveSplit[0]); - return { - requestQuery, - ...res - }; - } catch (e: any) { - e.requestQuery = requestQuery; - throw e; - } - } else { - throw new SearchPrerequisiteError('Naive parsing did not produce multiple artists'); - } - } else if(searchArtistMethod === 'native') { - // use MS native parsing to extract artists from artist string and title - // -- cleaning title is aggressive but at this point MB has not found anything which means its likely not "proper" - // IE "My Track (Cool Remix)" is a proper recording name but "My Track (feat. Someone)" is not bc MB would have removed the joiner from the name - // so we do the same to hopefully get a match - // - // ...additionally, since MB hasn't found anything with single artist string its likely the artist name does not have a common delimiter as part of their proper name - // IE "Crosby, Stills, Nash & Young" is a proper artist name with delimiters included but "My Artist & My Feat Artists" is not - // so we split out all artists by all found delimiters - const nativePlay = nativeParse(play, {titleClean: true, delimiters: DELIMITERS}); - this.logger.debug({labels: ['Parsed Artist Search']},'Searching with aggressive native parsing'); - const requestQuery = JSON.stringify({ - title: nativePlay.data.track, - artitst: artistCreditsToNames(nativePlay.data.artists!).join(', '), - }); - try { - const res = await this.api.rsProxy.matchSong(nativePlay.data.track!, artistCreditsToNames(nativePlay.data.artists!).join(', ')); - return { - requestQuery, - ...res - }; - } catch (e: any) { - e.requestQuery = requestQuery; - throw e; - } + if(play.data.artists === undefined) { + throw new SearchPrerequisiteError('Play does not have any artists'); + } + if(play.data.artists.length > 1) { + throw new SearchPrerequisiteError('Play has more than one artist already'); + } + if(play.data.artists.length === 0) { + throw new SearchPrerequisiteError('Play does not have any artists'); + } + // if only one artist + // then its likely artist string is combined + if(searchArtistMethod === 'naive') { + // try a naive split using any common delimiter found and use the first value as artist + // -- this will likely result in a less accurate match but at least it might find something + // -- usually the "primary artist" is listed first in a combined artist string so cross your fingers this works + const naiveSplit = splitByFirstRegexFound(play.data.artists[0].name, [play.data.artists[0].name]).map(x => x.trim()); + if(naiveSplit.length > 1) { + this.logger.debug({labels: ['Parsed Artist Search']},'Searching with track + first value from artist string split'); + const requestQuery = JSON.stringify({ + title: play.data.track, + artitst: naiveSplit[0], + }); + try { + const res = await this.api.rsProxy.matchSong(requireTrack(play), naiveSplit[0]); + return { + requestQuery, + ...res + }; + } catch (e: any) { + e.requestQuery = requestQuery; + throw e; } } - - if(play.data.artists === undefined) { - throw new SearchPrerequisiteError('Play does not have any artists'); - } - if(play.data.artists.length > 1) { - throw new SearchPrerequisiteError('Play has more than one artist already'); + throw new SearchPrerequisiteError('Naive parsing did not produce multiple artists'); + } + if(searchArtistMethod === 'native') { + // use MS native parsing to extract artists from artist string and title + // -- cleaning title is aggressive but at this point MB has not found anything which means its likely not "proper" + // IE "My Track (Cool Remix)" is a proper recording name but "My Track (feat. Someone)" is not bc MB would have removed the joiner from the name + // so we do the same to hopefully get a match + // + // ...additionally, since MB hasn't found anything with single artist string its likely the artist name does not have a common delimiter as part of their proper name + // IE "Crosby, Stills, Nash & Young" is a proper artist name with delimiters included but "My Artist & My Feat Artists" is not + // so we split out all artists by all found delimiters + const nativePlay = nativeParse(play, {titleClean: true, delimiters: DELIMITERS}); + this.logger.debug({labels: ['Parsed Artist Search']},'Searching with aggressive native parsing'); + const requestQuery = JSON.stringify({ + title: nativePlay.data.track, + artitst: artistCreditsToNames(nativePlay.data.artists).join(', '), + }); + try { + const res = await this.api.rsProxy.matchSong(requireTrack(nativePlay), artistCreditsToNames(nativePlay.data.artists).join(', ')); + return { + requestQuery, + ...res + }; + } catch (e: any) { + e.requestQuery = requestQuery; + throw e; } + } + throw new SearchPrerequisiteError(`Unknown searchArtistMethod '${searchArtistMethod}'`); } public async handlePostFetch(play: PlayObject, transformData: SongViewDetailedMS, stageConfig: RockskyTransformerDataStage): Promise { @@ -412,19 +419,20 @@ export default class RockskyTransformer extends AtomicPartsTransformer x.score! >= score); + const filteredList: SongMatchView[] = matches.filter(x => x.score !== undefined && x.score >= score); if(filteredList.length === 0) { - throw new StagePrerequisiteError(`All ${transformData.matches!.length} candidate matches associated with this match had a score < ${score}, best match was ${transformData.matches![0].score}`, {shortStack: true}); + throw new StagePrerequisiteError(`All ${matches.length} candidate matches associated with this match had a score < ${score}, best match was ${matches[0].score}`, {shortStack: true}); } //const mergedConfig = Object.assign({}, removeUndefinedKeys({...this.defaults}), removeUndefinedKeys({...stageConfig})); //filteredList = rankSongMatchesByPriority(filteredList, mergedConfig, play); - this.logger.debug(`${filteredList.length} of ${transformData.matches!.length} were valid, filtered matches. Using match with best score of ${filteredList[0].score}`); + this.logger.debug(`${filteredList.length} of ${matches.length} were valid, filtered matches. Using match with best score of ${filteredList[0].score}`); mergedSongView = { ...transformData, title: filteredList[0].title ?? transformData.title, @@ -488,7 +496,7 @@ export default class RockskyTransformer extends AtomicPartsTransformer 1) { + if((play.data.artists ?? []).length === 1 && (transformData.data.artists ?? []).length > 1) { return transformData.data.artists; } @@ -592,7 +600,7 @@ export default class RockskyTransformer extends AtomicPartsTransformer { + if(play.data.track === undefined) { + throw new SearchPrerequisiteError('Play does not have a track title'); + } + return play.data.track; +} diff --git a/src/backend/common/transforms/rocksky/RockskyTransformerUtil.ts b/src/backend/common/transforms/rocksky/RockskyTransformerUtil.ts index 28a2295a..4c16df01 100644 --- a/src/backend/common/transforms/rocksky/RockskyTransformerUtil.ts +++ b/src/backend/common/transforms/rocksky/RockskyTransformerUtil.ts @@ -37,7 +37,7 @@ export const configFromEnv = (logger: MaybeLogger = new MaybeLogger()) => { } if (soSet.size > 0) { - rsConfig.defaults!.searchOrder = Array.from(soSet); + rsConfig.defaults = {...rsConfig.defaults, searchOrder: Array.from(soSet)}; } logger.debug(`Using presets: ${presets.join(',')}`); }