Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(maloja): Improve handling for Maloja warnings-as-errors #181

Merged
merged 2 commits into from
Aug 27, 2024
Merged

fix(maloja): Improve handling for Maloja warnings-as-errors #181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b53b74
fix(maloja): Improve handling for Maloja warnings-as-errors
FoxxMD Aug 26, 2024
08056a6
fix: Actually store recent scrobbles
FoxxMD Aug 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/backend/common/vendor/maloja/interfaces.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export interface MalojaScrobbleV3RequestData extends MalojaScrobbleRequestData {
nofix?: boolean
}

interface MalojaScrobbleWarning {
export interface MalojaScrobbleWarning {
type: string
value: string[] | string
desc: string
Expand Down
1 change: 1 addition & 0 deletions src/backend/scrobblers/AbstractScrobbleClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ export default abstract class AbstractScrobbleClient extends AbstractComponent i
this.logger.debug('Refreshing recent scrobbles');
const recent = await this.getScrobblesForRefresh(limit);
this.logger.debug(`Found ${recent.length} recent scrobbles`);
this.recentScrobbles = recent;
if (this.recentScrobbles.length > 0) {
const [{data: {playDate: newestScrobbleTime = dayjs()} = {}} = {}] = this.recentScrobbles.slice(-1);
const [{data: {playDate: oldestScrobbleTime = dayjs()} = {}} = {}] = this.recentScrobbles.slice(0, 1);
Expand Down
30 changes: 25 additions & 5 deletions src/backend/scrobblers/MalojaScrobbler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
MalojaScrobbleRequestData,
MalojaScrobbleV2RequestData,
MalojaScrobbleV3RequestData,
MalojaScrobbleV3ResponseData,
MalojaScrobbleV3ResponseData, MalojaScrobbleWarning,
MalojaV2ScrobbleData,
MalojaV3ScrobbleData,
} from "../common/vendor/maloja/interfaces.js";
Expand Down Expand Up @@ -59,7 +59,7 @@ export default class MalojaScrobbler extends AbstractScrobbleClient {
// when the track was scrobbled
time: mTime,
track: {
artists: mArtists,
artists: mArtists = [],
title: mTitle,
album: mAlbum,
// length of the track
Expand All @@ -77,14 +77,14 @@ export default class MalojaScrobbler extends AbstractScrobbleClient {
const {
albumtitle,
name: mAlbumName,
artists: albumArtists
artists: albumArtists = []
} = mAlbum || {};
album = albumtitle ?? mAlbumName;
}
} else {
// scrobble data structure for v2 and below
const {
artists: mArtists,
artists: mArtists = [],
title: mTitle,
album: mAlbum,
duration: mDuration,
Expand Down Expand Up @@ -438,7 +438,11 @@ export default class MalojaScrobbler extends AbstractScrobbleClient {
}
if(warnings.length > 0) {
for(const w of warnings) {
this.logger.warn(`Maloja Warning: ${w.desc} => ${JSON.stringify(w.value)}`)
const warnStr = buildWarningString(w);
if(warnStr.includes('The submitted scrobble was not added')) {
throw new UpstreamError(`Maloja returned a warning but MS treating as error: ${warnStr}`, {showStopper: false});
}
this.logger.warn(`Maloja Warning: ${warnStr}`);
}
}
} else {
Expand Down Expand Up @@ -517,3 +521,19 @@ const buildErrorString = (body: MalojaResponseV3CommonData) => {
}
return `Maloja API returned ${status} of type ${type} "${desc}"${valString !== undefined ? `: ${valString}` : ''}`;
}

const buildWarningString = (w: MalojaScrobbleWarning): string => {
const parts: string[] = [`${typeof w.type === 'string' ? `(${w.type}) ` : ''}${w.desc ?? ''}`];
let vals: string[] = [];
if(w.value !== null && w.value !== undefined) {
if(Array.isArray(w.value)) {
vals = w.value;
} else {
vals.push(w.value);
}
}
if(vals.length > 0) {
parts.push(vals.join(' | '));
}
return parts.join(' => ');
}
9 changes: 6 additions & 3 deletions src/backend/tests/scrobbler/TestScrobbler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,19 @@ import { Notifiers } from "../../notifier/Notifiers.js";
import AbstractScrobbleClient from "../../scrobblers/AbstractScrobbleClient.js";

export class TestScrobbler extends AbstractScrobbleClient {
protected async getScrobblesForRefresh(limit: number): Promise<PlayObject[]> {
return [];
}

testRecentScrobbles: PlayObject[] = [];

constructor() {
const logger = loggerTest;
const notifier = new Notifiers(new EventEmitter(), new EventEmitter(), new EventEmitter(), logger);
super('test', 'Test', {name: 'test'}, notifier, new EventEmitter(), logger);
}

protected async getScrobblesForRefresh(limit: number): Promise<PlayObject[]> {
return this.testRecentScrobbles;
}

doScrobble(playObj: PlayObject): Promise<PlayObject> {
return Promise.resolve(playObj);
}
Expand Down
96 changes: 55 additions & 41 deletions src/backend/tests/scrobbler/scrobblers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,66 +458,80 @@ describe('Detects duplicate and unique scrobbles using actively tracked scrobble
});
});

describe('Detects when upstream scrobbles should be refreshed', function() {
describe('Upstream Scrobbles', function() {

it('Stores upstream scrobbles on refresh', async function () {
const scrobbler = generateTestScrobbler();
scrobbler.testRecentScrobbles = normalizedWithMixedDur;
assert.isEmpty(scrobbler.recentScrobbles);
await scrobbler.refreshScrobbles();
assert.isNotEmpty(scrobbler.recentScrobbles);
});

const normalizedClose = normalizePlays(withDurPlays, {initialDate: dayjs().subtract(100, 'seconds')});
describe('Detects when upstream scrobbles should be refreshed', function() {

beforeEach(function () {
testScrobbler = generateTestScrobbler();
testScrobbler.recentScrobbles = normalizedWithMixedDur;
testScrobbler.newestScrobbleTime = normalizedWithMixedDur[0].data.playDate;
testScrobbler.lastScrobbleCheck = dayjs().subtract(60, 'seconds');
testScrobbler.queuedScrobbles = [];
testScrobbler.config.options = {};
});
const normalizedClose = normalizePlays(withDurPlays, {initialDate: dayjs().subtract(100, 'seconds')});

it('Detects queued scrobble date is newer than last scrobble refresh', async function() {
const newScrobble = generatePlay({
playDate: dayjs()
beforeEach(function () {
testScrobbler = generateTestScrobbler();
testScrobbler.recentScrobbles = normalizedWithMixedDur;
testScrobbler.newestScrobbleTime = normalizedWithMixedDur[0].data.playDate;
testScrobbler.lastScrobbleCheck = dayjs().subtract(60, 'seconds');
testScrobbler.queuedScrobbles = [];
testScrobbler.config.options = {};
});

testScrobbler.queueScrobble(newScrobble, 'test');
assert.isTrue(testScrobbler.shouldRefreshScrobble());
});

it('Detects queued scrobble date is older than newest scrobble', async function() {
testScrobbler.recentScrobbles = normalizedClose;
testScrobbler.newestScrobbleTime = normalizedClose[0].data.playDate;
it('Detects queued scrobble date is newer than last scrobble refresh', async function() {
const newScrobble = generatePlay({
playDate: dayjs()
});

const newScrobble = generatePlay({
playDate: dayjs().subtract(120, 'seconds')
testScrobbler.queueScrobble(newScrobble, 'test');
assert.isTrue(testScrobbler.shouldRefreshScrobble());
});

testScrobbler.queueScrobble(newScrobble, 'test');
assert.isTrue(testScrobbler.shouldRefreshScrobble());
});
it('Detects queued scrobble date is older than newest scrobble', async function() {
testScrobbler.recentScrobbles = normalizedClose;
testScrobbler.newestScrobbleTime = normalizedClose[0].data.playDate;

it('Forces refresh if refreshStaleAfter is set', async function() {
testScrobbler.recentScrobbles = normalizedClose;
testScrobbler.newestScrobbleTime = normalizedClose[0].data.playDate;
testScrobbler.config.options = { refreshStaleAfter: 10 };
const newScrobble = generatePlay({
playDate: dayjs().subtract(120, 'seconds')
});

const newScrobble = generatePlay({
playDate: dayjs().subtract(80, 'seconds')
testScrobbler.queueScrobble(newScrobble, 'test');
assert.isTrue(testScrobbler.shouldRefreshScrobble());
});

testScrobbler.queueScrobble(newScrobble, 'test');
assert.isTrue(testScrobbler.shouldRefreshScrobble());
});
it('Forces refresh if refreshStaleAfter is set', async function() {
testScrobbler.recentScrobbles = normalizedClose;
testScrobbler.newestScrobbleTime = normalizedClose[0].data.playDate;
testScrobbler.config.options = { refreshStaleAfter: 10 };

it('Does not refresh if scrobble is older than last check but newer than newest upstream scrobble', async function() {
testScrobbler.recentScrobbles = normalizedClose;
testScrobbler.newestScrobbleTime = normalizedClose[0].data.playDate;
const newScrobble = generatePlay({
playDate: dayjs().subtract(80, 'seconds')
});

const newScrobble = generatePlay({
playDate: dayjs().subtract(80, 'seconds')
testScrobbler.queueScrobble(newScrobble, 'test');
assert.isTrue(testScrobbler.shouldRefreshScrobble());
});

testScrobbler.queueScrobble(newScrobble, 'test');
assert.isFalse(testScrobbler.shouldRefreshScrobble());
it('Does not refresh if scrobble is older than last check but newer than newest upstream scrobble', async function() {
testScrobbler.recentScrobbles = normalizedClose;
testScrobbler.newestScrobbleTime = normalizedClose[0].data.playDate;

const newScrobble = generatePlay({
playDate: dayjs().subtract(80, 'seconds')
});

testScrobbler.queueScrobble(newScrobble, 'test');
assert.isFalse(testScrobbler.shouldRefreshScrobble());
});
});

});



describe('Scrobble client uses transform plays correctly', function() {

beforeEach(async function() {
Expand Down