refactor(ytm): Clean up tests and history parsing functions

src/backend/sources/YTMusicSource.ts

@@ -10,6 +10,8 @@ import { parseBool, sleep } from "../utils.js";
 import {
     getPlaysDiff,
     humanReadableDiff,
+    PlayOrderChangeType,
+    PlayOrderConsistencyResults,
     playsAreAddedOnly,
     playsAreBumpedOnly,
     playsAreSortConsistent
@@ -20,6 +22,14 @@ import { joinedUrl } from "../utils/NetworkUtils.js";
 import { FixedSizeList } from "fixed-size-list";
 import { todayAwareFormat } from "../utils/TimeUtils.js";
 
+export interface HistoryIngressResult {
+    plays: PlayObject[], 
+    consistent: boolean, 
+    diffResults?: PlayOrderConsistencyResults<PlayOrderChangeType>, 
+    diffType?: 'bump' | 'added', 
+    reason?: string
+}
+
 export const ytiHistoryResponseToListItems = (res: ApiResponse): YTNodes.MusicResponsiveListItem[] => {
     const page = Parser.parseResponse<IBrowseResponse>(res.data);
     const items = page.contents_memo.getType(YTNodes.MusicResponsiveListItem);
@@ -423,86 +433,119 @@ Redirect URI  : ${this.redirectUri}`);
 
         const listPlays = ytiHistoryResponseFromShelfToPlays(playlistDetail);
         
-        return this.parseRecentAgainstResponse(listPlays);
+        return this.parseRecentAgainstResponse(listPlays).plays;
         
     }
 
-    parseRecentAgainstResponse = (responsePlays: PlayObject[]): PlayObject[] => {
-
+    getIncomingHistoryConsistencyResult = (plays: PlayObject[]): HistoryIngressResult => {
         let newPlays: PlayObject[] = [];
+        
+        if(playsAreSortConsistent(this.recentlyPlayed, plays)) {
+            return {plays: newPlays, consistent: true};
+        }
 
-        const plays = responsePlays.slice(0, 20);
-        if(this.polling === false) {
-            this.recentlyPlayed = plays;
-            newPlays = plays;
-        } else {
-            if(playsAreSortConsistent(this.recentlyPlayed, plays)) {
-                return newPlays;
+        let warnMsg: string;
+        let diffResults: PlayOrderConsistencyResults<PlayOrderChangeType>;
+        let diffType: 'bump' | 'added';
+        diffResults = playsAreBumpedOnly(this.recentlyPlayed, plays);
+        if(diffResults[0] === true) {
+            diffType = 'bump';
+            if(diffResults[2] !== 'prepend') {
+                warnMsg = `(Bump Plays Detected) Previously seen YTM history was bumped in an unexpected way (${diffResults[2]}), resetting history to new list`;
+            } else {
+                newPlays = [...diffResults[1]].reverse();
+                if(newPlays.length > 1) {
+                    warnMsg = `(Bump Plays Detected) Expected to see only 1 new track in YTM History but found ${newPlays.length}. This may be OK if monitoring was stopped or tracks truly are short in length.`;
+                }
             }
-
-            let warnMsg: string;
-            const [bumpOk, bumpDiff, bumpType] = playsAreBumpedOnly(this.recentlyPlayed, plays);
-            if(bumpOk === true) {
-                if(bumpType !== 'prepend') {
-                    warnMsg = `(Bump Plays Detected) Previously seen YTM history was bumped in an unexpected way (${bumpType}), resetting history to new list`;
+        } else {
+            diffResults = playsAreAddedOnly(this.recentlyPlayed, plays);
+            if(diffResults[0] === true) {
+                diffType = 'added';
+                if(diffResults[2] !== 'prepend') {
+                    warnMsg = `(Add Plays Detected) New tracks were added to YTM history in an unexpected way (${diffResults[2]}), resetting watched history to new list`;
                 } else {
-                    newPlays = [...bumpDiff].reverse();
-                    if(newPlays.length > 1) {
-                        warnMsg = `(Bump Plays Detected) Expected to see only 1 new track in YTM History but found ${newPlays.length}. This may be OK if monitoring was stopped or tracks truly are short in length.`;
+                    const revertedToRecent = this.recentChangedHistoryResponses.findIndex(x => playsAreSortConsistent(x.plays, plays));
+                    if(revertedToRecent !== -1) {
+                        warnMsg = `(Add Plays Detected) YTM History has exact order as another recent response *where history was changed* (${revertedToRecent + 1} ago @ ${todayAwareFormat(this.recentChangedHistoryResponses[revertedToRecent].ts)}) which means last history (n - 1) was probably out of date. Resetting history to current list and NOT ADDING new tracks since we probably already discovered them earlier.`
+                    } else {
+                        newPlays = [...diffResults[1]].reverse();
+                        if(newPlays.length > 1) {
+                            warnMsg = `(Add Plays Detected) Expected to see only 1 new track in YTM History but found ${newPlays.length}. This may be OK if monitoring was stopped or tracks truly are short in length.`;
+                        }
                     }
                 }
             } else {
-                const [addOk, addDiff, addType] = playsAreAddedOnly(this.recentlyPlayed, plays);
-                if(addOk === true) {
-                    if(addType !== 'prepend') {
-                        warnMsg = `(Add Plays Detected) New tracks were added to YTM history in an unexpected way (${addType}), resetting watched history to new list`;
-                    } else {
-                        const revertedToRecent = this.recentChangedHistoryResponses.findIndex(x => playsAreSortConsistent(x.plays, plays));
-                        if(revertedToRecent !== -1) {
-                            warnMsg = `(Add Plays Detected) YTM History has exact order as another recent response *where history was changed* (${revertedToRecent + 1} ago @ ${todayAwareFormat(this.recentChangedHistoryResponses[revertedToRecent].ts)}) which means last history (n - 1) was probably out of date. Resetting history to current list and NOT ADDING new tracks since we probably already discovered them earlier.`
-                        } else {
-                            newPlays = [...addDiff].reverse();
-                            if(newPlays.length > 1) {
-                                warnMsg = `(Add Plays Detected) Expected to see only 1 new track in YTM History but found ${newPlays.length}. This may be OK if monitoring was stopped or tracks truly are short in length.`;
-                            }
-                        }
-                    }
-                } else {
-                    warnMsg = 'YTM History returned temporally inconsistent order, resetting history to new list.';
-                }
+                warnMsg = 'YTM History returned temporally inconsistent order, resetting history to new list.';
             }
+        }
 
-            if(warnMsg !== undefined || (newPlays.length > 0 && this.config.options?.logDiff === true)) {
+        return {
+            plays: newPlays,
+            consistent: warnMsg === undefined,
+            reason: warnMsg,
+            diffResults,
+            diffType
+        }
+    }
+
+    parseRecentAgainstResponse = (responsePlays: PlayObject[]): HistoryIngressResult => {
+
+        //let newPlays: PlayObject[] = [];
+        let results: HistoryIngressResult = {
+            plays: [],
+            consistent: true
+        }
+
+        const plays = responsePlays.slice(0, 20);
+        if(this.polling === false) {
+            results.plays = plays;
+        } else {
+
+            const cResults = this.getIncomingHistoryConsistencyResult(plays);
+
+            const {
+                reason,
+                plays: newPlays,
+                consistent,
+                diffResults,
+                diffType
+            } = cResults;
+
+            results = cResults;
+
+            if(!consistent || (newPlays.length > 0 && this.config.options?.logDiff === true)) {
                 const playsDiff = getPlaysDiff(this.recentlyPlayed, plays)
                 const humanDiff = humanReadableDiff(this.recentlyPlayed, plays, playsDiff);
                 const diffMsg = `Changes from last seen list:
 ${humanDiff}`;
-                if(warnMsg !== undefined) {
-                    this.logger.warn(warnMsg);
+                if(reason !== undefined) {
+                    this.logger.warn(reason);
                     this.logger.warn(diffMsg);
                 } else {
                     this.logger.verbose(diffMsg);
                 }
             }
-
-            this.recentlyPlayed = plays;
-            if(newPlays.length > 0) {
-                this.recentChangedHistoryResponses = [{plays, ts: dayjs()}, ...this.recentChangedHistoryResponses.slice(0, 3)]
-            }
-
-                newPlays = newPlays.map((x, index) => ({
-                    data: {
-                        ...x.data,
-                        playDate: dayjs().startOf('minute').add(index + 1, 's')
-                    },
-                    meta: {
-                        ...x.meta,
-                        newFromSource: true
-                    }
-                }));
         }
 
-        return newPlays;
+        this.recentlyPlayed = plays;
+
+        if(results.plays.length > 0) {
+            this.recentChangedHistoryResponses = [{plays, ts: dayjs()}, ...this.recentChangedHistoryResponses.slice(0, 3)]
+        }
+
+        results.plays = results.plays.map((x, index) => ({
+            data: {
+                ...x.data,
+                playDate: dayjs().startOf('minute').add(index + 1, 's')
+            },
+            meta: {
+                ...x.meta,
+                newFromSource: true
+            }
+        }));
+
+        return results;
     }
 
     onPollPostAuthCheck = async () => {

src/backend/tests/ytm/ytm.test.ts

@@ -48,17 +48,22 @@ describe('Handles temporal inconsistency in history', function () {
 
         const plays = [...generatePlays(10, {}, { comment: 'Today' }), ...generatePlays(10, {}, { comment: 'Yesterday' })];
 
-        expect(source.parseRecentAgainstResponse(plays)).length(20);
+        // emulating init, get history to use as base truth without discovering tracks
+        expect(source.parseRecentAgainstResponse(plays).plays).length(20);
 
         source.polling = true;
 
-        expect(source.parseRecentAgainstResponse(plays)).length(0);
+        // first true poll emulating no new tracks played (should not add new tracks from base truth)
+        expect(source.parseRecentAgainstResponse(plays).plays).length(0);
 
+        // add new track played
         const prependedPlays = [generatePlay({}, { comment: 'Today' }), ...plays];
+        const prependResult = source.parseRecentAgainstResponse(prependedPlays);
+        expect(prependResult.plays).length(1);
+        expect(prependResult).to.deep.include({consistent: true, diffType: 'added'});
+        expect(prependResult.diffResults[2]).eq('prepend');
 
-        expect(source.parseRecentAgainstResponse(prependedPlays)).length(1);
-
-        expect(source.parseRecentAgainstResponse(prependedPlays)).length(0);
+        expect(source.parseRecentAgainstResponse(prependedPlays).plays).length(0);
     });
 
     it(`Adds bumped, prepended track`, async function () {
@@ -67,18 +72,24 @@ describe('Handles temporal inconsistency in history', function () {
 
         const plays = [...generatePlays(10, {}, { comment: 'Today' }), ...generatePlays(10, {}, { comment: 'Yesterday' })];
 
-        expect(source.parseRecentAgainstResponse(plays)).length(20);
+        // emulating init, get history to use as base truth without discovering tracks
+        expect(source.parseRecentAgainstResponse(plays).plays).length(20);
 
         source.polling = true;
 
-        expect(source.parseRecentAgainstResponse(plays)).length(0);
+        // first true poll emulating no new tracks played (should not add new tracks from base truth)
+        expect(source.parseRecentAgainstResponse(plays).plays).length(0);
 
+        // add new track played that was seen in base truth (YT *bumps* track from earlier position to top of list)
         const bumpedList = [...plays.map(x => clone(x))];
         const bumped = bumpedList[6];
         bumpedList.splice(6, 1);
         bumpedList.unshift(bumped);
 
-        expect(source.parseRecentAgainstResponse(bumpedList)).length(1);
+        const bumpedResults = source.parseRecentAgainstResponse(bumpedList);
+        expect(bumpedResults.plays).length(1);
+        expect(bumpedResults).to.deep.include({consistent: true, diffType: 'bump'});
+        expect(bumpedResults.diffResults[2]).eq('prepend');
     });
 
     it(`Does not add appended track`, async function () {
@@ -87,15 +98,20 @@ describe('Handles temporal inconsistency in history', function () {
 
         const plays = [...generatePlays(10, {}, { comment: 'Today' }), ...generatePlays(10, {}, { comment: 'Yesterday' })];
 
-        expect(source.parseRecentAgainstResponse(plays)).length(20);
+        // emulating init, get history to use as base truth without discovering tracks
+        expect(source.parseRecentAgainstResponse(plays).plays).length(20);
 
         source.polling = true;
 
-        expect(source.parseRecentAgainstResponse(plays)).length(0);
+        // first true poll emulating no new tracks played (should not add new tracks from base truth)
+        expect(source.parseRecentAgainstResponse(plays).plays).length(0);
 
+        // track is erroneously added to end of history ("new" track played in the past, not temporally consistent)
         const appendPlays = [...plays.slice(1), generatePlay({}, { comment: 'Yesterday' })];
-
-        expect(source.parseRecentAgainstResponse(appendPlays)).length(0);
+        const appenedResult =source.parseRecentAgainstResponse(appendPlays);
+        expect(appenedResult.plays).length(0);
+        expect(appenedResult).to.deep.include({consistent: false, diffType: 'added'});
+        expect(appenedResult.diffResults[2]).eq('append');
     });
 
     it(`Detects outdated recent history when order was previously seen`, async function () {
@@ -106,28 +122,38 @@ describe('Handles temporal inconsistency in history', function () {
 
         const plays = [...generatePlays(10, {}, { comment: 'Today' }), ...generatePlays(10, {}, { comment: 'Yesterday' })];
 
-        expect(source.parseRecentAgainstResponse(plays)).length(20);
+        // emulating init, get history to use as base truth without discovering tracks
+        expect(source.parseRecentAgainstResponse(plays).plays).length(20);
 
         source.polling = true;
 
-        expect(source.parseRecentAgainstResponse(plays)).length(0);
+        // first true poll emulating no new tracks played (should not add new tracks from base truth)
+        expect(source.parseRecentAgainstResponse(plays).plays).length(0);
 
+        // add new track played
         const newPlay = generatePlay({}, { comment: 'Today' });
-
         const prependedPlays = [newPlay, ...plays];
-
-        expect(source.parseRecentAgainstResponse(prependedPlays)).length(1);
+        expect(source.parseRecentAgainstResponse(prependedPlays).plays).length(1);
 
         await sleep(1000);
 
-        expect(source.parseRecentAgainstResponse(plays)).length(0);
+        // YT returns outdated history
+        // should be detected as append since "removed" track in last position from previous history is seen again
+        const badAppend = source.parseRecentAgainstResponse(plays);
+        expect(badAppend).to.deep.include({consistent: false, diffType: 'added', plays: []});
+        expect(badAppend.diffResults[2]).eq('append');
 
         await sleep(500);
 
-        expect(source.parseRecentAgainstResponse(plays)).length(0);
+        // contiuned outdated history
+        expect(source.parseRecentAgainstResponse(plays)).to.deep.include({consistent: true, plays: []});
 
         await sleep(500);
 
-        expect(source.parseRecentAgainstResponse(prependedPlays)).length(0);
+        // correct, current history is finally returned correctly
+        const recentHistoryResult = source.parseRecentAgainstResponse(prependedPlays);
+        expect(recentHistoryResult).to.deep.include({consistent: false, plays: []});
+        // should detect that we have seen this history before and not duplicate add already discovered tracks
+        expect(recentHistoryResult.reason).includes('(Add Plays Detected) YTM History has exact order as another recent response *where history was changed*')
     });
 });

src/backend/utils/PlayComparisonUtils.ts

@@ -66,7 +66,14 @@ export const getDiffIndexState = (results: any, index: number) => {
     return undefined;
 }
 
-export const playsAreAddedOnly = (aPlays: PlayObject[], bPlays: PlayObject[], transformers: ListTransformers = defaultListTransformers): [boolean, PlayObject[]?, ('append' | 'prepend' | 'insert')?] => {
+export type PlayOrderBumpedType = 'append' | 'prepend';
+export type PlayOrderAddedType = PlayOrderBumpedType | 'insert';
+export type PlayOrderChangeType = PlayOrderAddedType | PlayOrderBumpedType;
+
+
+export type PlayOrderConsistencyResults<T extends PlayOrderChangeType> = [boolean, PlayObject[]?, T?]
+
+export const playsAreAddedOnly = (aPlays: PlayObject[], bPlays: PlayObject[], transformers: ListTransformers = defaultListTransformers): PlayOrderConsistencyResults<PlayOrderAddedType> => {
     const results = getPlaysDiff(aPlays, bPlays, transformers);
      if(results.status === 'equal' || results.status === 'deleted') {
         return [false];
@@ -117,7 +124,7 @@ export const playsAreAddedOnly = (aPlays: PlayObject[], bPlays: PlayObject[], tr
     return [addType !== 'insert' && addType !== undefined, added.map(x => bPlays[x.newIndex]), addType];
 }
 
-export const playsAreBumpedOnly = (aPlays: PlayObject[], bPlays: PlayObject[], transformers: ListTransformers = defaultListTransformers): [boolean, PlayObject[]?, ('append' | 'prepend')?] => {
+export const playsAreBumpedOnly = (aPlays: PlayObject[], bPlays: PlayObject[], transformers: ListTransformers = defaultListTransformers): PlayOrderConsistencyResults<PlayOrderBumpedType> => {
     const results = getPlaysDiff(aPlays, bPlays, transformers);
     if(results.status === 'equal' || results.status === 'deleted') {
        return [false];