From b5dbde66c46d944b12bddc28ce33c8153932f6dd Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Mon, 27 Jan 2025 21:15:00 +0000 Subject: [PATCH 1/3] WIP testing and improvements for artist astring parsing * Implement faker data generation for artist string * Add test suite for parsing different styles of artist strings --- .../tests/listenbrainz/listenbrainz.test.ts | 18 ++-- src/backend/tests/plays/playParsing.test.ts | 67 ++++++++++++++ src/backend/tests/utils/PlayTestUtils.ts | 92 ++++++++++++++++++- src/core/Atomic.ts | 13 ++- src/core/StringUtils.ts | 26 ++++++ 5 files changed, 204 insertions(+), 12 deletions(-) create mode 100644 src/backend/tests/plays/playParsing.test.ts diff --git a/src/backend/tests/listenbrainz/listenbrainz.test.ts b/src/backend/tests/listenbrainz/listenbrainz.test.ts index 12e01d30..8010a4a7 100644 --- a/src/backend/tests/listenbrainz/listenbrainz.test.ts +++ b/src/backend/tests/listenbrainz/listenbrainz.test.ts @@ -10,18 +10,18 @@ import { ListenbrainzApiClient } from "../../common/vendor/ListenbrainzApiClient import { ListenResponse } from '../../common/vendor/listenbrainz/interfaces.js'; import { ExpectedResults } from "../utils/interfaces.js"; import { withRequestInterception } from "../utils/networking.js"; -import artistWithProperJoiner from './correctlyMapped/artistProperHasJoinerInName.json'; +import artistWithProperJoiner from './correctlyMapped/artistProperHasJoinerInName.json' with { type: "json" }; // correct mappings -import multiArtistInArtistName from './correctlyMapped/multiArtistInArtistName.json'; -import multiArtistsInTrackName from './correctlyMapped/multiArtistInTrackName.json'; -import multiMappedArtistsWithSingleUserArtist from './correctlyMapped/multiArtistMappingWithSingleRecordedArtist.json'; -import noArtistMapping from './correctlyMapped/noArtistMapping.json'; -import normalizedValues from './correctlyMapped/normalizedName.json'; -import slightlyDifferentNames from './correctlyMapped/trackNameSlightlyDifferent.json'; +import multiArtistInArtistName from './correctlyMapped/multiArtistInArtistName.json' with { type: "json" }; +import multiArtistsInTrackName from './correctlyMapped/multiArtistInTrackName.json' with { type: "json" }; +import multiMappedArtistsWithSingleUserArtist from './correctlyMapped/multiArtistMappingWithSingleRecordedArtist.json' with { type: "json" }; +import noArtistMapping from './correctlyMapped/noArtistMapping.json' with { type: "json" }; +import normalizedValues from './correctlyMapped/normalizedName.json' with { type: "json" }; +import slightlyDifferentNames from './correctlyMapped/trackNameSlightlyDifferent.json' with { type: "json" }; // incorrect mappings -import incorrectMultiArtistsTrackName from './incorrectlyMapped/multiArtistsInTrackName.json'; -import veryWrong from './incorrectlyMapped/veryWrong.json'; +import incorrectMultiArtistsTrackName from './incorrectlyMapped/multiArtistsInTrackName.json' with { type: "json" }; +import veryWrong from './incorrectlyMapped/veryWrong.json' with { type: "json" }; interface LZTestFixture { data: ListenResponse diff --git a/src/backend/tests/plays/playParsing.test.ts b/src/backend/tests/plays/playParsing.test.ts new file mode 100644 index 00000000..8cd70106 --- /dev/null +++ b/src/backend/tests/plays/playParsing.test.ts @@ -0,0 +1,67 @@ +import { loggerTest, loggerDebug, childLogger } from "@foxxmd/logging"; +import chai, { assert, expect } from 'chai'; +import asPromised from 'chai-as-promised'; +import { after, before, describe, it } from 'mocha'; + +import { asPlays, generateArtistsStr, generatePlay, normalizePlays } from "../utils/PlayTestUtils.js"; +import { parseArtistCredits, parseCredits } from "../../utils/StringUtils.js"; + +describe('Parsing Artists from String', function() { + + it('Parses Artists from an Artist-like string', function () { + for(const i of Array(20)) { + const [str, primaries, secondaries] = generateArtistsStr(); + const credits = parseArtistCredits(str); + const allArtists = primaries.concat(secondaries); + const parsed = [credits.primary].concat(credits.secondary ?? []) + expect(primaries.concat(secondaries),` +'${str}' +Expected => ${allArtists.join(' || ')} +Found => ${parsed.join(' || ')}`) +.eql(parsed) + } + }); + + it('Parses singlar Artist with wrapped vs multiple', function () { + const [str, primaries, secondaries] = generateArtistsStr({primary: 1, secondary: {num: 2, ft: 'vs', joiner: '/', ftWrap: true}}); + const credits = parseArtistCredits(str); + const moreCredits = parseCredits(str); + expect(true).eq(true); + }); + + describe('When joiner is known', function () { + + it('Parses many primary artists', function () { + for(const i of Array(10)) { + const [str, primaries, secondaries] = generateArtistsStr({primary: {max: 3, joiner: '/'}, secondary: 0}); + const credits = parseArtistCredits(str, ['/']); + const allArtists = primaries.concat(secondaries); + const parsed = [credits.primary].concat(credits.secondary ?? []) + expect(primaries.concat(secondaries),` +'${str}' +Expected => ${allArtists.join(' || ')} +Found => ${parsed.join(' || ')}`) + .eql(parsed) + } + }); + + it('Parses many secondary artists', function () { + // fails on -- Peso Pluma / Lil Baby / R. Kelly (featuring TOMORROW X TOGETHER / AC/DC / DaVido) + for(const i of Array(10)) { + const [str, primaries, secondaries] = generateArtistsStr({primary: {max: 3, joiner: '/'}, secondary: {joiner: '/', finalJoiner: false}}); + const credits = parseArtistCredits(str, ['/']); + const allArtists = primaries.concat(secondaries); + const parsed = [credits.primary].concat(credits.secondary ?? []) + expect(primaries.concat(secondaries),` +'${str}' +Expected => ${allArtists.join(' || ')} +Found => ${parsed.join(' || ')}`) + .eql(parsed) + } + }); + + + }); + + +}); \ No newline at end of file diff --git a/src/backend/tests/utils/PlayTestUtils.ts b/src/backend/tests/utils/PlayTestUtils.ts index 5e19ecf1..e9ec580e 100644 --- a/src/backend/tests/utils/PlayTestUtils.ts +++ b/src/backend/tests/utils/PlayTestUtils.ts @@ -5,9 +5,10 @@ import isBetween from "dayjs/plugin/isBetween.js"; import relativeTime from "dayjs/plugin/relativeTime.js"; import timezone from "dayjs/plugin/timezone.js"; import utc from "dayjs/plugin/utc.js"; -import { JsonPlayObject, ObjectPlayData, PlayMeta, PlayObject } from "../../../core/Atomic.js"; +import { FEAT, JOINERS, JOINERS_FINAL, JsonPlayObject, ObjectPlayData, PlayMeta, PlayObject } from "../../../core/Atomic.js"; import { sortByNewestPlayDate } from "../../utils.js"; import { NO_DEVICE, NO_USER, PlayerStateDataMaybePlay, PlayPlatformId, ReportedPlayerStatus } from '../../common/infrastructure/Atomic.js'; +import { arrayListAnd } from '../../../core/StringUtils.js'; dayjs.extend(utc) dayjs.extend(isBetween); @@ -176,3 +177,92 @@ export const generatePlayPlatformId = (deviceId?: string, userId?: string): Play export const generatePlays = (numberOfPlays: number, data: ObjectPlayData = {}, meta: PlayMeta = {}): PlayObject[] => { return Array.from(Array(numberOfPlays), () => generatePlay(data, meta)); } + +export const generateArtist = () => faker.music.artist; + +export const generateArtists = (num?: number, max: number = 3) => { + // if(num !== undefined) { + // return Array(num).map(x => faker.music.artist); + // } + if(num === 0 || max === 0) { + return []; + } + return faker.helpers.multiple(faker.music.artist, {count: {min: num ?? 1, max: num ?? max}}); +} + +export interface ArtistGenerateOptions { + num?: number + max?: number + joiner?: string + finalJoiner?: false | string + spacedJoiners?: boolean +} + +export interface SecondaryArtistGenerateOptions extends ArtistGenerateOptions { + ft?: string + ftWrap?: boolean +} + +export interface CompoundArtistGenerateOptions { + primary?: number | ArtistGenerateOptions + secondary?: number | SecondaryArtistGenerateOptions +} + +export const generateArtistsStr = (options: CompoundArtistGenerateOptions = {}): [string, string[], string[]] => { + + const {primary = {}, secondary = {}} = options; + + const primaryOpts: ArtistGenerateOptions = typeof primary === 'number' ? {num: primary} : primary; + const secondaryOpts: SecondaryArtistGenerateOptions = typeof secondary === 'number' ? {num: secondary} : secondary; + + const primaryArt = generateArtists(primaryOpts.num, primaryOpts.max) + const secondaryArt = generateArtists(secondaryOpts.num, secondaryOpts.max); + + + const joinerPrimary: string = primaryOpts.joiner ?? faker.helpers.arrayElement(JOINERS); + let finalJoinerPrimary: string = joinerPrimary; + if(primaryOpts.finalJoiner !== false) { + if(primaryOpts.finalJoiner === undefined) { + if(joinerPrimary === ',') { + finalJoinerPrimary = faker.helpers.arrayElement(JOINERS_FINAL); + } + + } else { + finalJoinerPrimary = primaryOpts.finalJoiner; + } + } + + const primaryStr = arrayListAnd(primaryArt, joinerPrimary, finalJoinerPrimary, primaryOpts.spacedJoiners); + + if(secondaryArt.length === 0) { + return [primaryStr, primaryArt, []]; + } + + const joinerSecondary: string = secondaryOpts.joiner ?? faker.helpers.arrayElement(JOINERS); + let finalJoinerSecondary: string = joinerSecondary; + if(secondaryOpts.finalJoiner !== false) { + if(secondaryOpts.finalJoiner === undefined) { + if(joinerSecondary === ',') { + finalJoinerSecondary = faker.helpers.arrayElement(JOINERS_FINAL); + } + } else { + finalJoinerSecondary = secondaryOpts.finalJoiner; + } + } + + const secondaryStr = arrayListAnd(secondaryArt, joinerSecondary, finalJoinerSecondary, secondaryOpts.spacedJoiners); + const ft = secondaryOpts.ft ?? faker.helpers.arrayElement(FEAT); + let sec = `${ft} ${secondaryStr}`; + let wrap: boolean; + if(secondaryOpts.ftWrap !== undefined) { + wrap = secondaryOpts.ftWrap; + } else { + wrap = faker.datatype.boolean(); + } + if(wrap) { + sec = `(${sec})`; + } + const artistStr = `${primaryStr} ${sec}`; + + return [artistStr, primaryArt, secondaryArt]; +} \ No newline at end of file diff --git a/src/core/Atomic.ts b/src/core/Atomic.ts index 0cda60b3..18c332c2 100644 --- a/src/core/Atomic.ts +++ b/src/core/Atomic.ts @@ -254,7 +254,7 @@ export interface SourcePlayerObj { play: PlayObject, playFirstSeenAt?: string, playLastUpdatedAt?: string, - playerLastUpdatedAt: string + playerLastUpdatedAt: strin position?: Second listenedDuration: Second status: { @@ -342,4 +342,13 @@ export interface URLData { url: URL normal: string port: number -} \ No newline at end of file +} + +export type Joiner = ',' | '&' | '/' | '\\' | string; +export const JOINERS: Joiner[] = [',','&','/','\\']; + +export type FinalJoiners = '&'; +export const JOINERS_FINAL: FinalJoiners[] = ['&']; + +export type Feat = 'ft' | 'feat' | 'vs' | 'ft.' | 'feat.' | 'vs.' | 'featuring' +export const FEAT: Feat[] = ['ft','feat','vs','ft.','feat.','vs.','featuring']; \ No newline at end of file diff --git a/src/core/StringUtils.ts b/src/core/StringUtils.ts index c1d8fab2..3b60a501 100644 --- a/src/core/StringUtils.ts +++ b/src/core/StringUtils.ts @@ -201,3 +201,29 @@ export const combinePartsToString = (parts: any[], glue: string = '-'): string | } return undefined; } + +export const arrayListOxfordAnd = (list: string[], joiner: string, finalJoiner: string, spaced: boolean = true): string => { + if(list.length === 1) { + return list[0]; + } + const start = list.slice(0, list.length - 1); + const end = list.slice(list.length - 1); + + const joinerProper = joiner === ',' ? ', ' : (spaced ? ` ${joiner} ` : joiner); + const finalProper = spaced ? ` ${finalJoiner} ` : finalJoiner; + + return [start.join(joinerProper), end].join(joiner === ',' && spaced ? `,${finalProper}` : finalProper); +} + +export const arrayListAnd = (list: string[], joiner: string, finalJoiner: string, spaced: boolean = true): string => { + if(list.length === 1) { + return list[0]; + } + const start = list.slice(0, list.length - 1); + const end = list.slice(list.length - 1); + + const joinerProper = joiner === ',' ? ', ' : (spaced ? ` ${joiner} ` : joiner); + const finalProper = spaced ? ` ${finalJoiner} ` : finalJoiner; + + return [start.join(joinerProper), end].join(finalProper); +} \ No newline at end of file -- 2.51.2 From a3d8372f4895348f2cc99d25521764dec87a3ad4 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Wed, 19 Mar 2025 19:54:25 +0000 Subject: [PATCH 2/3] feat: More artist parsing improvements * Use more context-aware list parsing with ampersands * Ignore artists with slashes when wrapped by word-boundary --- src/backend/tests/plays/playParsing.test.ts | 49 ++++++++++++++++++- src/backend/tests/utils/PlayTestUtils.ts | 38 ++++++++++++--- src/backend/utils/StringUtils.ts | 52 +++++++++++++++++++-- src/core/Atomic.ts | 4 +- 4 files changed, 129 insertions(+), 14 deletions(-) diff --git a/src/backend/tests/plays/playParsing.test.ts b/src/backend/tests/plays/playParsing.test.ts index 8cd70106..39d5dd19 100644 --- a/src/backend/tests/plays/playParsing.test.ts +++ b/src/backend/tests/plays/playParsing.test.ts @@ -4,7 +4,7 @@ import asPromised from 'chai-as-promised'; import { after, before, describe, it } from 'mocha'; import { asPlays, generateArtistsStr, generatePlay, normalizePlays } from "../utils/PlayTestUtils.js"; -import { parseArtistCredits, parseCredits } from "../../utils/StringUtils.js"; +import { parseArtistCredits, parseContextAwareStringList, parseCredits } from "../../utils/StringUtils.js"; describe('Parsing Artists from String', function() { @@ -18,10 +18,57 @@ describe('Parsing Artists from String', function() { '${str}' Expected => ${allArtists.join(' || ')} Found => ${parsed.join(' || ')}`) + .eql(parsed) } }); + it('Parses & as "local" joiner when other delimiters present', function () { + + const data = [{ + str: `Melendi \\ Ryan Lewis \\ The Righteous Brothers (featuring Joan Jett & The Blackhearts \\ Robin Schulz)`, + expected: ['Melendi', 'Ryan Lewis', 'The Righteous Brothers', 'Joan Jett & The Blackhearts', 'Robin Schulz'] + }, { + str: `Gigi D'Agostino \\ YOASOBI (vs Sam Hunt, Lisa Loeb & Booba)`, + expected: [`Gigi D'Agostino`, 'YOASOBI', 'Sam Hunt', 'Lisa Loeb', 'Booba'] + }]; + + for(const d of data) { + const credits = parseArtistCredits(d.str); + const parsed = [credits.primary].concat(credits.secondary ?? []) + expect(d.expected).eql(parsed) + } + + }); + + it('Only parses & as "global" joiner when no other delimiters present', function () { + + const data = [{ + str: `Melendi & Ryan Lewis & The Righteous Brothers (featuring The Blackhearts \\ Robin Schulz)`, + expected: ['Melendi', 'Ryan Lewis', 'The Righteous Brothers', 'The Blackhearts', 'Robin Schulz'] + }]; + + for(const d of data) { + const credits = parseArtistCredits(d.str); + const parsed = [credits.primary].concat(credits.secondary ?? []) + expect(d.expected).eql(parsed) + } + }); + + it('Parses secondary free regex', function () { + + const data = [{ + str: `Diddy & Grand Funk Railroad feat. Daya & (G)I-DLE`, + expected: ['Diddy', 'Grand Funk Railroad', 'Daya', '(G)I-DLE'] + }]; + + for(const d of data) { + const credits = parseArtistCredits(d.str); + const parsed = [credits.primary].concat(credits.secondary ?? []) + expect(d.expected).eql(parsed) + } + }); + it('Parses singlar Artist with wrapped vs multiple', function () { const [str, primaries, secondaries] = generateArtistsStr({primary: 1, secondary: {num: 2, ft: 'vs', joiner: '/', ftWrap: true}}); const credits = parseArtistCredits(str); diff --git a/src/backend/tests/utils/PlayTestUtils.ts b/src/backend/tests/utils/PlayTestUtils.ts index e9ec580e..ae1a3b1b 100644 --- a/src/backend/tests/utils/PlayTestUtils.ts +++ b/src/backend/tests/utils/PlayTestUtils.ts @@ -9,6 +9,7 @@ import { FEAT, JOINERS, JOINERS_FINAL, JsonPlayObject, ObjectPlayData, PlayMeta, import { sortByNewestPlayDate } from "../../utils.js"; import { NO_DEVICE, NO_USER, PlayerStateDataMaybePlay, PlayPlatformId, ReportedPlayerStatus } from '../../common/infrastructure/Atomic.js'; import { arrayListAnd } from '../../../core/StringUtils.js'; +import { findDelimiters } from '../../utils/StringUtils.js'; dayjs.extend(utc) dayjs.extend(isBetween); @@ -180,14 +181,37 @@ export const generatePlays = (numberOfPlays: number, data: ObjectPlayData = {}, export const generateArtist = () => faker.music.artist; -export const generateArtists = (num?: number, max: number = 3) => { - // if(num !== undefined) { - // return Array(num).map(x => faker.music.artist); - // } +export const generateArtists = (num?: number, max: number = 3, opts: {ambiguousJoinedNames?: boolean, trailingAmpersand?: boolean} = {}) => { if(num === 0 || max === 0) { return []; } - return faker.helpers.multiple(faker.music.artist, {count: {min: num ?? 1, max: num ?? max}}); + let artists = faker.helpers.multiple(faker.music.artist, {count: {min: num ?? 1, max: num ?? max}}); + + const { + trailingAmpersand = false, + ambiguousJoinedNames = false + } = opts; + + if(!trailingAmpersand) { + // its really hard to parse an artist name that contains an '&' when it comes at the end of a list + // because its ambigious if the list is joining the list with & or if & is part of the artist name + // so by default don't generate these (we test for specific scenarios in playParsing.test.ts) + while(artists[artists.length - 1].includes('&')) { + artists = artists.slice(0, artists.length - 1).concat(faker.music.artist()); + } + } + if(!ambiguousJoinedNames) { + artists = artists.map(x => { + let a = x; + let foundDelims = findDelimiters(a); + while(foundDelims !== undefined && foundDelims.length > 0 && !(foundDelims.length === 1 && foundDelims[0] === '&')) { + a = faker.music.artist(); + foundDelims = findDelimiters(a); + } + return a; + }); + } + return artists; } export interface ArtistGenerateOptions { @@ -223,7 +247,7 @@ export const generateArtistsStr = (options: CompoundArtistGenerateOptions = {}): let finalJoinerPrimary: string = joinerPrimary; if(primaryOpts.finalJoiner !== false) { if(primaryOpts.finalJoiner === undefined) { - if(joinerPrimary === ',') { + if(joinerPrimary === ',' && !primaryArt.some(x => x.includes('&'))) { finalJoinerPrimary = faker.helpers.arrayElement(JOINERS_FINAL); } @@ -242,7 +266,7 @@ export const generateArtistsStr = (options: CompoundArtistGenerateOptions = {}): let finalJoinerSecondary: string = joinerSecondary; if(secondaryOpts.finalJoiner !== false) { if(secondaryOpts.finalJoiner === undefined) { - if(joinerSecondary === ',') { + if(joinerSecondary === ',' && !secondaryArt.some(x => x.includes('&'))) { finalJoinerSecondary = faker.helpers.arrayElement(JOINERS_FINAL); } } else { diff --git a/src/backend/utils/StringUtils.ts b/src/backend/utils/StringUtils.ts index 276bcbe7..07640756 100644 --- a/src/backend/utils/StringUtils.ts +++ b/src/backend/utils/StringUtils.ts @@ -1,7 +1,7 @@ import { strategies, stringSameness, StringSamenessResult } from "@foxxmd/string-sameness"; import { PlayObject } from "../../core/Atomic.js"; import { asPlayerStateData, DELIMITERS, PlayerStateDataMaybePlay } from "../common/infrastructure/Atomic.js"; -import { genGroupIdStr, getPlatformIdFromData, parseRegexSingleOrFail } from "../utils.js"; +import { genGroupIdStr, getPlatformIdFromData, intersect, parseRegexSingleOrFail } from "../utils.js"; import { buildTrackString } from "../../core/StringUtils.js"; const {levenStrategy, diceStrategy} = strategies; @@ -61,7 +61,7 @@ export const SECONDARY_CAPTURED_REGEX = new RegExp(/[([]\s*(?ft\.?\W|fea * !!!! ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ******* * * */ -export const SECONDARY_FREE_REGEX = new RegExp(/^\s*(?ft\.?\W|feat\.?\W|featuring|vs\.?\W)\s*(?(?:.+?(?= - |\s*[([]))|(?:.*))(?.*)/i); +export const SECONDARY_FREE_REGEX = new RegExp(/^\s*(?ft\.?\W|feat\.?\W|featuring|vs\.?\W)\s*(?(?:.+?(?= - |\s*[([].+[)\]]$))|(?:.*))(?.*)/i); const SECONDARY_REGEX_STRATS: RegExp[] = [SECONDARY_CAPTURED_REGEX, SECONDARY_FREE_REGEX]; @@ -116,7 +116,7 @@ export const parseCredits = (str: string, delimiters?: boolean | string[]): Play for(const strat of SECONDARY_REGEX_STRATS) { const secCredits = parseRegexSingleOrFail(strat, results.named.secondary); if(secCredits !== undefined) { - secondary = parseStringList(secCredits.named.credits as string, delims) + secondary = parseContextAwareStringList(secCredits.named.credits as string, delims) suffix = secCredits.named.creditsSuffix; break; } @@ -148,7 +148,7 @@ export const parseArtistCredits = (str: string, delimiters?: boolean | string[]) if (withJoiner !== undefined) { // all this does is make sure and "ft" or parenthesis/brackets are separated -- // it doesn't also separate primary artists so do that now - const primaries = parseStringList(withJoiner.primary, delims); + const primaries = parseContextAwareStringList(withJoiner.primary, delims); if (primaries.length > 1) { return { primary: primaries[0], @@ -182,6 +182,50 @@ export const parseStringList = (str: string, delimiters: string[] = [',', '&', ' return explodedStrings.flat(1); }, [str]).map(x => x.trim()); } +export const parseContextAwareStringList = (str: string, delimiters: string[] = [',', '/', '\\'], opts: {ignoreGlobalAmpersand?: boolean} = {}): string[] => { + if (delimiters.length === 0) { + return [str]; + } + // bypass tokens using slashes without spaces + const cleanStr = bypassJoiners(str); + const nonAmpersandDelims = delimiters.some(x => cleanStr.includes(x)); + const shouldIgnoreGlobalAmpersand = opts.ignoreGlobalAmpersand ?? nonAmpersandDelims; + + let awareList: string[] = []; + + const list = parseStringList(cleanStr, nonAmpersandDelims === false && shouldIgnoreGlobalAmpersand === false ? ['&'] : delimiters); + if(shouldIgnoreGlobalAmpersand && list.length > 1 && list[list.length - 1].includes('&') && nonAmpersandDelims) { //&& !list[list.length - 1].includes('& the') + awareList = list.slice(0, list.length - 1).concat(list[list.length - 1].split('&') ); + } else { + awareList = list; + } + return awareList.map(x =>rejoinBypassed(x.trim())); +} + +const bypassJoinerMap = [ + { + rejoin: str => str.replaceAll(/(.*?\S)(\^\^\^)(\S.*?)/g, '$1/$3'), + bypass: str => str.replaceAll(/(.*?\S)(\/)(\S.*?)/g, '$1^^^$3') + }, + { + rejoin: str => str.replaceAll(/(.*)(###)(.*)/g, '$1\\$3'), + bypass: str => str.replaceAll(/(.*\S)(\\)(.*\S)/g, '$1###$3') + } +]; +export const bypassJoiners = (str: string): string => { + let bypassed: string = str; + for(const b of bypassJoinerMap) { + bypassed = b.bypass(bypassed) + } + return bypassed; +} +export const rejoinBypassed = (str: string): string => { + let bypassed: string = str; + for(const b of bypassJoinerMap) { + bypassed = b.rejoin(bypassed) + } + return bypassed; +} export const containsDelimiters = (str: string) => null !== str.match(/[,&/\\]+/i) export const findDelimiters = (str: string) => { const found: string[] = []; diff --git a/src/core/Atomic.ts b/src/core/Atomic.ts index 18c332c2..17e14d48 100644 --- a/src/core/Atomic.ts +++ b/src/core/Atomic.ts @@ -254,7 +254,7 @@ export interface SourcePlayerObj { play: PlayObject, playFirstSeenAt?: string, playLastUpdatedAt?: string, - playerLastUpdatedAt: strin + playerLastUpdatedAt: string position?: Second listenedDuration: Second status: { @@ -345,7 +345,7 @@ export interface URLData { } export type Joiner = ',' | '&' | '/' | '\\' | string; -export const JOINERS: Joiner[] = [',','&','/','\\']; +export const JOINERS: Joiner[] = [',','/','\\']; export type FinalJoiners = '&'; export const JOINERS_FINAL: FinalJoiners[] = ['&']; -- 2.51.2 From 33fda80d0a9dd492b7f5b74bbd7876c0ef467e07 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Thu, 7 Aug 2025 19:32:51 +0000 Subject: [PATCH 3/3] feat: Improve artist string parsing * Don't split artist with joiner when only one joiner is present * Override naive parsing if metabrainz mapped artists fit with provided joiners --- .../common/vendor/ListenbrainzApiClient.ts | 14 +++++-- .../multiArtistInArtistName.json | 4 +- .../tests/listenbrainz/listenbrainz.test.ts | 4 +- src/backend/tests/plays/playParsing.test.ts | 40 +++++++++++++++---- src/backend/tests/utils/PlayTestUtils.ts | 10 ++++- src/backend/tests/utils/strings.test.ts | 2 +- src/backend/utils/StringUtils.ts | 12 +++--- 7 files changed, 63 insertions(+), 23 deletions(-) diff --git a/src/backend/common/vendor/ListenbrainzApiClient.ts b/src/backend/common/vendor/ListenbrainzApiClient.ts index 944b179b..08fcda9e 100644 --- a/src/backend/common/vendor/ListenbrainzApiClient.ts +++ b/src/backend/common/vendor/ListenbrainzApiClient.ts @@ -14,11 +14,11 @@ import { } from "../../utils/StringUtils.js"; import { getScrobbleTsSOCDate } from "../../utils/TimeUtils.js"; import { UpstreamError } from "../errors/UpstreamError.js"; -import { AbstractApiOptions, DEFAULT_RETRY_MULTIPLIER, FormatPlayObjectOptions } from "../infrastructure/Atomic.js"; +import { AbstractApiOptions, DEFAULT_RETRY_MULTIPLIER, DELIMITERS, FormatPlayObjectOptions } from "../infrastructure/Atomic.js"; import { ListenBrainzClientData } from "../infrastructure/config/client/listenbrainz.js"; import AbstractApiClient from "./AbstractApiClient.js"; import { getBaseFromUrl, isPortReachableConnect, joinedUrl, normalizeWebAddress } from '../../utils/NetworkUtils.js'; -import { removeUndefinedKeys } from '../../utils.js'; +import { removeUndefinedKeys, unique } from '../../utils.js'; import {ListensResponse as KoitoListensResponse} from '../infrastructure/config/client/koito.js' import { listenObjectResponseToPlay } from './koito/KoitoApiClient.js'; import { version } from '../../ioc.js'; @@ -393,7 +393,15 @@ export class ListenbrainzApiClient extends AbstractApiClient { } // now try to extract any remaining artists from filtered artist/name values - const parsedArtists = parseArtistCredits(filteredSubmittedArtistName); + const splitAmpersand = artistsWithJoiners.length === 0 && artistMappings.some(x => x.join_phrase.includes('&')); + let nonProperJoinedDelims = undefined; + if(artistsWithJoiners.length === 0) { + nonProperJoinedDelims = unique(artistMappings.filter(x => DELIMITERS.includes(x.join_phrase.trim())).map(x => x.join_phrase.trim())); + if(nonProperJoinedDelims.length === 0) { + nonProperJoinedDelims = undefined; + } + } + const parsedArtists = parseArtistCredits(filteredSubmittedArtistName, nonProperJoinedDelims); if (parsedArtists !== undefined) { if (parsedArtists.primary !== undefined) { artistsFromUserValues.push(parsedArtists.primary); diff --git a/src/backend/tests/listenbrainz/correctlyMapped/multiArtistInArtistName.json b/src/backend/tests/listenbrainz/correctlyMapped/multiArtistInArtistName.json index fb5a3564..4a49a802 100644 --- a/src/backend/tests/listenbrainz/correctlyMapped/multiArtistInArtistName.json +++ b/src/backend/tests/listenbrainz/correctlyMapped/multiArtistInArtistName.json @@ -74,7 +74,7 @@ { "artist_credit_name": "Metro Boomin", "artist_mbid": "59db3d82-86ea-451f-881f-dffc8ec387c9", - "join_phrase": " & " + "join_phrase": " , " }, { "artist_credit_name": "James Blake", @@ -116,7 +116,7 @@ { "artist_credit_name": "Childish Gambino", "artist_mbid": "7fb57fba-a6ef-44c2-abab-2fa3bdee607e", - "join_phrase": " & " + "join_phrase": " , " }, { "artist_credit_name": "Ariana Grande", diff --git a/src/backend/tests/listenbrainz/listenbrainz.test.ts b/src/backend/tests/listenbrainz/listenbrainz.test.ts index 8010a4a7..c2021fd6 100644 --- a/src/backend/tests/listenbrainz/listenbrainz.test.ts +++ b/src/backend/tests/listenbrainz/listenbrainz.test.ts @@ -27,7 +27,7 @@ interface LZTestFixture { data: ListenResponse expected: ExpectedResults } -describe('Listenbrainz Listen Parsing', function () { +describe('#PlayParse Listenbrainz Listen Parsing', function () { describe('When user-submitted artist/track do NOT match MB mappings', function() { it('Uses user submitted values when no artist mappings', async function () { @@ -56,7 +56,7 @@ describe('Listenbrainz Listen Parsing', function () { }) - describe('When user-submitted artist/track matches a MB mapped value', function() { + describe('#PlayParse When user-submitted artist/track matches a MB mapped value', function() { it('Detects slightly different track names as equal', async function () { for(const test of slightlyDifferentNames as unknown as LZTestFixture[]) { diff --git a/src/backend/tests/plays/playParsing.test.ts b/src/backend/tests/plays/playParsing.test.ts index 39d5dd19..ae12ce2e 100644 --- a/src/backend/tests/plays/playParsing.test.ts +++ b/src/backend/tests/plays/playParsing.test.ts @@ -6,14 +6,14 @@ import { after, before, describe, it } from 'mocha'; import { asPlays, generateArtistsStr, generatePlay, normalizePlays } from "../utils/PlayTestUtils.js"; import { parseArtistCredits, parseContextAwareStringList, parseCredits } from "../../utils/StringUtils.js"; -describe('Parsing Artists from String', function() { +describe('#PlayParse Parsing Artists from String', function() { it('Parses Artists from an Artist-like string', function () { - for(const i of Array(20)) { - const [str, primaries, secondaries] = generateArtistsStr(); + for(const i of Array(40)) { + const [str, primaries, secondaries] = generateArtistsStr({primary: {max: 3, ambiguousJoinedNames: true, trailingAmpersand: true, finalJoiner: false}}); const credits = parseArtistCredits(str); const allArtists = primaries.concat(secondaries); - const parsed = [credits.primary].concat(credits.secondary ?? []) + const parsed = [credits.primary].concat(credits.secondary ?? []); expect(primaries.concat(secondaries),` '${str}' Expected => ${allArtists.join(' || ')} @@ -25,13 +25,20 @@ Found => ${parsed.join(' || ')}`) it('Parses & as "local" joiner when other delimiters present', function () { - const data = [{ + const data = [ + { str: `Melendi \\ Ryan Lewis \\ The Righteous Brothers (featuring Joan Jett & The Blackhearts \\ Robin Schulz)`, expected: ['Melendi', 'Ryan Lewis', 'The Righteous Brothers', 'Joan Jett & The Blackhearts', 'Robin Schulz'] - }, { + }, + { str: `Gigi D'Agostino \\ YOASOBI (vs Sam Hunt, Lisa Loeb & Booba)`, expected: [`Gigi D'Agostino`, 'YOASOBI', 'Sam Hunt', 'Lisa Loeb', 'Booba'] - }]; + }, + { + str: `Wham!, Hillsong Worship & Bruce Channel feat. I Prevail`, + expected: ['Wham!', 'Hillsong Worship & Bruce Channel', 'I Prevail'] + } + ]; for(const d of data) { const credits = parseArtistCredits(d.str); @@ -55,6 +62,25 @@ Found => ${parsed.join(' || ')}`) } }); + it('Does not split artist name when only one joiner is present', function () { + + const data = [ + { + str: `Melendi & Ryan Lewis`, + expected: ['Melendi & Ryan Lewis'] + },{ + str: `Melendi and Ryan Lewis`, + expected: ['Melendi and Ryan Lewis'] + }, + ]; + + for(const d of data) { + const credits = parseArtistCredits(d.str); + const parsed = [credits.primary].concat(credits.secondary ?? []) + expect(d.expected).eql(parsed) + } + }); + it('Parses secondary free regex', function () { const data = [{ diff --git a/src/backend/tests/utils/PlayTestUtils.ts b/src/backend/tests/utils/PlayTestUtils.ts index ae1a3b1b..69ba4c9c 100644 --- a/src/backend/tests/utils/PlayTestUtils.ts +++ b/src/backend/tests/utils/PlayTestUtils.ts @@ -181,7 +181,13 @@ export const generatePlays = (numberOfPlays: number, data: ObjectPlayData = {}, export const generateArtist = () => faker.music.artist; -export const generateArtists = (num?: number, max: number = 3, opts: {ambiguousJoinedNames?: boolean, trailingAmpersand?: boolean} = {}) => { +export interface ArtistGenerationOptions +{ + ambiguousJoinedNames?: boolean, + trailingAmpersand?: boolean +} + +export const generateArtists = (num?: number, max: number = 3, opts: ArtistGenerationOptions = {}) => { if(num === 0 || max === 0) { return []; } @@ -214,7 +220,7 @@ export const generateArtists = (num?: number, max: number = 3, opts: {ambiguousJ return artists; } -export interface ArtistGenerateOptions { +export interface ArtistGenerateOptions extends ArtistGenerationOptions { num?: number max?: number joiner?: string diff --git a/src/backend/tests/utils/strings.test.ts b/src/backend/tests/utils/strings.test.ts index 2738a095..d1c2f570 100644 --- a/src/backend/tests/utils/strings.test.ts +++ b/src/backend/tests/utils/strings.test.ts @@ -9,7 +9,7 @@ import { uniqueNormalizedStrArr } from "../../utils/StringUtils.js"; import { ExpectedResults } from "./interfaces.js"; -import testData from './playTestData.json'; +import testData from './playTestData.json' with { type: "json" }; import { splitByFirstFound } from '../../../core/StringUtils.js'; interface PlayTestFixture { diff --git a/src/backend/utils/StringUtils.ts b/src/backend/utils/StringUtils.ts index 07640756..588af50d 100644 --- a/src/backend/utils/StringUtils.ts +++ b/src/backend/utils/StringUtils.ts @@ -134,7 +134,7 @@ export const parseCredits = (str: string, delimiters?: boolean | string[]): Play } return undefined; } -export const parseArtistCredits = (str: string, delimiters?: boolean | string[]): PlayCredits | undefined => { +export const parseArtistCredits = (str: string, delimiters?: boolean | string[], ignoreGlobalAmpersand?: boolean): PlayCredits | undefined => { if (str.trim() === '') { return undefined; } @@ -148,7 +148,7 @@ export const parseArtistCredits = (str: string, delimiters?: boolean | string[]) if (withJoiner !== undefined) { // all this does is make sure and "ft" or parenthesis/brackets are separated -- // it doesn't also separate primary artists so do that now - const primaries = parseContextAwareStringList(withJoiner.primary, delims); + const primaries = parseContextAwareStringList(withJoiner.primary, delims, {ignoreGlobalAmpersand: ignoreGlobalAmpersand ?? false}); if (primaries.length > 1) { return { primary: primaries[0], @@ -159,7 +159,7 @@ export const parseArtistCredits = (str: string, delimiters?: boolean | string[]) return withJoiner; } // likely this is a plain string with just delims - const artists = parseStringList(str, delims); + const artists = parseContextAwareStringList(str, delims, {ignoreGlobalAmpersand: ignoreGlobalAmpersand ?? true}); if (artists.length > 1) { return { primary: artists[0], @@ -173,7 +173,7 @@ export const parseArtistCredits = (str: string, delimiters?: boolean | string[]) } } export const parseTrackCredits = (str: string, delimiters?: boolean | string[]): PlayCredits | undefined => parseCredits(str, delimiters); -export const parseStringList = (str: string, delimiters: string[] = [',', '&', '/', '\\']): string[] => { +export const parseStringList = (str: string, delimiters: string[] = DELIMITERS): string[] => { if (delimiters.length === 0) { return [str]; } @@ -227,9 +227,9 @@ export const rejoinBypassed = (str: string): string => { return bypassed; } export const containsDelimiters = (str: string) => null !== str.match(/[,&/\\]+/i) -export const findDelimiters = (str: string) => { +export const findDelimiters = (str: string, delimiters = DELIMITERS) => { const found: string[] = []; - for (const d of DELIMITERS) { + for (const d of delimiters) { if (str.indexOf(d) !== -1) { found.push(d); } -- 2.51.2