From afb2e76b2d5104a0b4deff533e1e27399145140d Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Tue, 9 Jan 2024 11:50:42 -0500 Subject: [PATCH] feat: Improve track credits parsing to recognize post-feat suffixes * Refactor credit parsing into two stages: joiner separation and credits parsing * Break credit parsing into wrapped vs. non-wrapped for simpler regexes * Implement suffix matching after credits * Add tests for wrapped vs. non-wrapped credits and with suffixes --- package.json | 2 +- .../tests/listenbrainz/listenbrainz.test.ts | 5 +- src/backend/tests/utils/interfaces.ts | 5 + src/backend/tests/utils/playTestData.json | 188 ++++++++++++++++++ src/backend/tests/{ => utils}/strings.test.ts | 34 +++- src/backend/utils/StringUtils.ts | 86 +++++++- 6 files changed, 302 insertions(+), 18 deletions(-) create mode 100644 src/backend/tests/utils/interfaces.ts create mode 100644 src/backend/tests/utils/playTestData.json rename src/backend/tests/{ => utils}/strings.test.ts (75%) diff --git a/package.json b/package.json index e86d3b21..5e0a1d6a 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,7 @@ "typedoc": "typedoc", "circular": "madge --circular --extensions ts src/index.ts", "test": "react-scripts test", - "test:backend": "mocha --extension ts --reporter spec --recursive src/backend/tests/scrobbler/**/*.test.ts", + "test:backend": "mocha --extension ts --reporter spec --recursive src/backend/tests/**/*.test.ts", "eject": "react-scripts eject", "dev": "concurrently -p name -c \"yellow,magenta,blue\" -n \"webpack-server,nodemon-server,CRA\" \"npm run dev:server:webpack\" \"npm run dev:server:nodemon\" \"npm run dev:client\"", "dev:client": "BROWSER=none REACT_APP_VERSION=$npm_package_version react-scripts start", diff --git a/src/backend/tests/listenbrainz/listenbrainz.test.ts b/src/backend/tests/listenbrainz/listenbrainz.test.ts index 15897816..b47cd1eb 100644 --- a/src/backend/tests/listenbrainz/listenbrainz.test.ts +++ b/src/backend/tests/listenbrainz/listenbrainz.test.ts @@ -19,11 +19,8 @@ import dayjs from "dayjs"; import {withRequestInterception} from "../utils/networking"; import {http, HttpResponse} from "msw"; import {UpstreamError} from "../../common/errors/UpstreamError"; +import {ExpectedResults} from "../utils/interfaces"; -interface ExpectedResults { - artists: string[] - track: string -} interface LZTestFixture { data: ListenResponse expected: ExpectedResults diff --git a/src/backend/tests/utils/interfaces.ts b/src/backend/tests/utils/interfaces.ts new file mode 100644 index 00000000..90791a1c --- /dev/null +++ b/src/backend/tests/utils/interfaces.ts @@ -0,0 +1,5 @@ +export interface ExpectedResults { + artists: string[] + track: string + album?: String +} diff --git a/src/backend/tests/utils/playTestData.json b/src/backend/tests/utils/playTestData.json new file mode 100644 index 00000000..8ca16818 --- /dev/null +++ b/src/backend/tests/utils/playTestData.json @@ -0,0 +1,188 @@ +[ + { + "caseHints": [ + "track", + "joiner" + ], + "data": { + "artists": [ + "Eugene Naumenko" + ], + "track": "Take-Off feat. Diag", + "album": "1000 Years" + }, + "expected": { + "artists": [ + "Eugene Naumenko", + "Diag" + ], + "track": "Take-Off", + "album": "1000 Years" + } + }, + { + "caseHints": [ + "track", + "remix" + ], + "data": { + "artists": [ + "Djfredse" + ], + "track": "Anny Sky - Together With You (Djfredse Remix)" + }, + "expected": { + "artists": [ + "Djfredse", + "Anny Sky" + ], + "track": "Together With You (Djfredse Remix)" + } + }, + { + "caseHints": [ + "track", + "remix", + "joiner" + ], + "data": { + "artists": [ + "Chriss" + ], + "track": "Criminal mind Ft Akon (Remix Braquer vos têtes)", + "album": "Music Is Life" + }, + "expected": { + "artists": [ + "Chriss", + "Akon" + ], + "track": "Criminal mind (Remix Braquer vos têtes)", + "album": "Music Is Life" + } + }, + { + "caseHints": [ + "track", + "joiner" + ], + "data": { + "artists": [ + "Luis Zunel" + ], + "track": "Reset my Blues (Featuring Peergynt Lobogris)" + }, + "expected": { + "artists": [ + "Luis Zunel", + "Peergynt Lobogris" + ], + "track": "Reset my Blues" + } + }, + { + "caseHints": [ + "track", + "joiner" + ], + "data": { + "artists": [ + "Woochia" + ], + "track": "La Flaque (feat.Ecstatik)" + }, + "expected": { + "artists": [ + "Woochia", + "Ecstatik" + ], + "track": "La Flaque" + } + }, + { + "caseHints": [ + "track", + "joiner", + "remix" + ], + "data": { + "artists": [ + "Alicia Keys", + "Kaash Paige", + "Diamond Platnumz" + ], + "track": "Wasted Energy (feat. Kaash Paige & Diamond Platnumz) - Remix" + }, + "expected": { + "artists": [ + "Alicia Keys", + "Kaash Paige", + "Diamond Platnumz" + ], + "track": "Wasted Energy - Remix" + } + }, + { + "caseHints": [ + "track", + "joiner" + ], + "data": { + "artists": [ + "Tyler, the Creator" + ], + "track": "WUSYANAME (feat. Youngboy Never Broke Again & Ty Dolla $ign)" + }, + "expected": { + "artists": [ + "Tyler, the Creator", + "Youngboy Never Broke Again", + "Ty Dolla $ign" + ], + "track": "WUSYANAME" + } + }, + { + "caseHints": [ + "track", + "joiner" + ], + "data": { + "artists": [ + "Tyler, the Creator" + ], + "track": "Trashwang (feat. Na' kel, Jasper Dolphin, Lucas & L-Boy)" + }, + "expected": { + "artists": [ + "Tyler, the Creator", + "Na' kel", + "Jasper Dolphin", + "L-Boy", + "Lucas" + ], + "track": "Trashwang" + } + }, + { + "caseHints": [ + "track", + "joiner" + ], + "data": { + "artists": [ + "Childish Gambino" + ], + "track": "12.38 (feat. 21 Savage, Ink & Kadhja Bonet)" + }, + "expected": { + "artists": [ + "Childish Gambino", + "21 Savage", + "Ink", + "Kadhja Bonet" + ], + "track": "12.38" + } + } +] diff --git a/src/backend/tests/strings.test.ts b/src/backend/tests/utils/strings.test.ts similarity index 75% rename from src/backend/tests/strings.test.ts rename to src/backend/tests/utils/strings.test.ts index 626844d6..912dbb38 100644 --- a/src/backend/tests/strings.test.ts +++ b/src/backend/tests/utils/strings.test.ts @@ -1,7 +1,19 @@ import {describe, it} from 'mocha'; import {assert} from 'chai'; -import {compareNormalizedStrings} from "../utils/StringUtils"; - +import {compareNormalizedStrings, parseTrackCredits, uniqueNormalizedStrArr} from "../../utils/StringUtils"; +import testData from './playTestData.json'; +import {ExpectedResults} from "./interfaces"; +import {intersect} from "../../utils"; + +interface PlayTestFixture { + caseHints: string[] + data: { + track: string + artists: string[] + album?: string + } + expected: ExpectedResults +} describe('String Comparisons', function () { @@ -95,3 +107,21 @@ describe('String Comparisons', function () { assert.equal( result1.highScore, result2.highScore, `Comparing: '${longerString}' | '${shorterString}'`); }); }); + +describe('Play Strings',function () { + + const testFixtures = testData as unknown as PlayTestFixture[]; + const joinerData = testFixtures.filter(x => intersect(['joiner','track'], x.caseHints).length === 2); + + it('should parse joiners from track title', function() { + for(const test of joinerData) { + const res = parseTrackCredits(test.data.track); + let artists: string[] = [...test.data.artists]; + if(res.secondary !== undefined) { + artists = uniqueNormalizedStrArr([...artists, ...res.secondary]); + } + assert.equal(res.primaryComposite, test.expected.track); + assert.sameDeepMembers(artists, test.expected.artists); + } + }); +}); diff --git a/src/backend/utils/StringUtils.ts b/src/backend/utils/StringUtils.ts index 8451635d..922dad4c 100644 --- a/src/backend/utils/StringUtils.ts +++ b/src/backend/utils/StringUtils.ts @@ -31,14 +31,59 @@ export const normalizeStr = (str: string, options?: {keepSingleWhitespace?: bool export interface PlayCredits { primary: string + primaryComposite: string secondary?: string[] + suffix?: string } +/** + * Matches if the secondary string is wrapped in parenthesis-like symbols. Returns joiner, credits, and + * suffix = if anything appears after end of wrapped string + * + * EX (feat. Kaash Paige & Diamond Platnumz) - Remix + * !!!! ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ******* + * + * + * */ +export const SECONDARY_CAPTURED_REGEX = new RegExp(/[(\[]\s*(?ft\.?\W|feat\.?\W|featuring|vs\.?\W)\s*(?.*)[)\]](?.*)/i); + + +/** + * Matches if the secondary string is NOT wrapped in parenthesis-like symbols. Returns joiner, credits, and + * suffix = if anything appears wrapped or starting with " - " proceeding string appearing after joiner + * + * EX feat. Diag + * !!!! ^^^^ + * + * Ft Akon, Paige & Djfredse (Remix Braquer vos têtes) + * !! ^^^^^^^^^^^^^^^^^^^^^^ ************************* + * + * feat. Kaash Paige & Diamond Platnumz - Remix + * !!!! ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ******* + * + * */ +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]; + +/** + * Matches JUST primary and secondary sections separated by a REQUIRED joiner that can optionally be wrapped in parenthesis-like symbols + * + * EX + * Criminal mind Ft Akon (Remix Braquer vos têtes) + * ^^^^^^^^^^^^^********************************** + * + * Wasted Energy (feat. Kaash Paige & Diamond Platnumz) - Remix + * ^^^^^^^^^^^^^^********************************************** + * + * */ +export const PRIMARY_SECONDARY_SECTIONS_REGEX = new RegExp(/^(?.+?)(?(?:[(\[]?(?:\Wft\.?|\Wfeat\.?|featuring|\Wvs\.)).*)/i); + /** * For matching the most common track/artist pattern that has a joiner * * Primary ft. 2nd Artist, 3rd Artist - * Primary (2nd Artist) + * Primary (2nd Artist) - Suffix * Primary [featuring 2nd Artist] * * ____ @@ -49,28 +94,44 @@ export interface PlayCredits { * => MUST begin with joiner ft. feat. featuring with vs. * => May have closing character ) ] * */ -export const SECONDARY_ARTISTS_SECTION_REGEX = new RegExp(/^(?[^(\[]*)?(?[(\[]?(?\Wft\.?|\Wfeat\.?|featuring|\Wvs\.?) (?[^)\]]*)(?:[)\]]|\s*)$)/i); // export const SECONDARY_ARTISTS_REGEX = new RegExp(//ig); export const parseCredits = (str: string, delimiters?: boolean | string[]): PlayCredits => { if (str.trim() === '') { return undefined; } + let primary: string | undefined; let secondary: string[] = []; - const results = parseRegexSingleOrFail(SECONDARY_ARTISTS_SECTION_REGEX, str); - if (results !== undefined) { - primary = results.named.primary !== undefined ? results.named.primary.trim() : undefined; + let suffix: string | undefined; + const results = parseRegexSingleOrFail(PRIMARY_SECONDARY_SECTIONS_REGEX, str); + if(results !== undefined) { + let delims: string[] | undefined; if (Array.isArray(delimiters)) { delims = delimiters; } else if (delimiters === false) { delims = []; } - secondary = parseStringList(results.named.secondaryArtists as string, delims) + + primary = results.named.primary.trim(); + 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) + suffix = secCredits.named.creditsSuffix; + break; + } + } + if(secondary === undefined) { + // uh oh, this shouldn't have happened! Return nothing since we don't know how to parse this + return undefined; + } return { primary, - secondary - }; + primaryComposite: `${primary}${suffix ?? ''}`, + secondary, + suffix + } } return undefined; } @@ -92,6 +153,7 @@ export const parseArtistCredits = (str: string, delimiters?: boolean | string[]) if (primaries.length > 1) { return { primary: primaries[0], + primaryComposite: primaries[0], secondary: primaries.slice(1).concat(withJoiner.secondary) } } @@ -102,11 +164,13 @@ export const parseArtistCredits = (str: string, delimiters?: boolean | string[]) if (artists.length > 1) { return { primary: artists[0], + primaryComposite: artists[0], secondary: artists.slice(1) } } return { - primary: artists[0] + primary: artists[0], + primaryComposite: artists[0], } } export const parseTrackCredits = (str: string, delimiters?: boolean | string[]): PlayCredits | undefined => parseCredits(str, delimiters); @@ -150,10 +214,10 @@ export const compareScrobbleTracks = (existing: PlayObject, candidate: PlayObjec // try to remove any joiners based on existing artists const existingCredits = parseTrackCredits(existingTrack); - const existingPrimary = existingCredits !== undefined ? existingCredits.primary : existingTrack; + const existingPrimary = existingCredits !== undefined ? existingCredits.primaryComposite : existingTrack; const candidateCredits = parseTrackCredits(candidateTrack); - const candidatePrimary = candidateCredits !== undefined ? candidateCredits.primary : candidateTrack; + const candidatePrimary = candidateCredits !== undefined ? candidateCredits.primaryComposite : candidateTrack; // take whichever score is higher const creditsCleanedTrackSameness = compareNormalizedStrings(existingPrimary, candidatePrimary); -- 2.51.2