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); }