From 49bd1c88366cd511429f0e5b6e98266e93e6058a Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Wed, 14 Apr 2021 16:22:05 -0400 Subject: [PATCH] Refactor initialization and authentication stages for client/source building * Break up into different functions and add properties to concrete classes signalling its capabilities * More detailed logging based on which fails/succeeds * Simplify client/source init/auth step (no more need for case switch specifics other than creating object) --- apis/LastfmApiClient.js | 10 ++-- clients/AbstractScrobbleClient.js | 14 ++++++ clients/LastfmScrobbler.js | 13 +++++ clients/MalojaScrobbler.js | 64 ++++++++++++++++++------- clients/ScrobbleClients.js | 56 ++++++++++++++-------- index.js | 80 +++++++++++++++++-------------- sources/AbstractSource.js | 19 ++++++++ sources/JellyfinSource.js | 1 + sources/LastfmSource.js | 15 +++++- sources/PlexSource.js | 1 + sources/ScrobbleSources.js | 59 ++++++++++++++++------- sources/SpotifySource.js | 22 +++++++++ sources/SubsonicSource.js | 26 +++++++++- views/status.ejs | 4 +- 14 files changed, 287 insertions(+), 97 deletions(-) diff --git a/apis/LastfmApiClient.js b/apis/LastfmApiClient.js index c00c1adb..55eb3213 100644 --- a/apis/LastfmApiClient.js +++ b/apis/LastfmApiClient.js @@ -132,13 +132,17 @@ export default class LastfmApiClient extends AbstractApiClient { if (this.client.sessionKey === undefined && sessionKey !== undefined) { this.client.sessionKey = sessionKey; } + return true; } catch (e) { this.logger.warn('Current lastfm credentials file exists but could not be parsed', {path: this.workingCredsPath}); + return false; } + } + testAuth = async () => { if (this.client.sessionKey === undefined) { this.logger.info('No session key found. User interaction for authentication required.'); - return; + return false; } try { const infoResp = await this.callApi(client => client.userGetInfo()); @@ -152,8 +156,8 @@ export default class LastfmApiClient extends AbstractApiClient { this.logger.info(`Client authorized for user ${name}`) return true; } catch (e) { - this.logger.error('Testing connection failed'); - return false; + this.logger.error('Testing auth failed'); + throw e; } } diff --git a/clients/AbstractScrobbleClient.js b/clients/AbstractScrobbleClient.js index 402e12a4..754fecc5 100644 --- a/clients/AbstractScrobbleClient.js +++ b/clients/AbstractScrobbleClient.js @@ -6,6 +6,9 @@ export default class AbstractScrobbleClient { name; type; initialized = false; + requiresAuth = false; + requiresAuthInteraction = false; + authed = false; recentScrobbles = []; scrobbledPlayObjs = []; @@ -60,6 +63,17 @@ export default class AbstractScrobbleClient { }; } + // default init function, should be overridden if init stage is required + initialize = async () => { + this.initialized = true; + return this.initialized; + } + + // default init function, should be overridden if auth stage is required + testAuth = async () => { + return this.authed; + } + scrobblesLastCheckedAt = () => { return this.lastScrobbleCheck; } diff --git a/clients/LastfmScrobbler.js b/clients/LastfmScrobbler.js index 1b31c2e8..169f565d 100644 --- a/clients/LastfmScrobbler.js +++ b/clients/LastfmScrobbler.js @@ -14,6 +14,8 @@ export default class LastfmScrobbler extends AbstractScrobbleClient { api; initialized = false; + requiresAuth = true; + requiresAuthInteraction = true; constructor(name, config = {}, options = {}) { super('lastfm', name, config, options); @@ -27,6 +29,17 @@ export default class LastfmScrobbler extends AbstractScrobbleClient { return this.initialized; } + testAuth = async () => { + try { + this.authed = await this.api.testAuth(); + } catch (e) { + this.logger.error('Could not successfully communicate with Last.fm API'); + this.logger.error(e); + this.authed = false; + } + return this.authed; + } + refreshScrobbles = async () => { if (this.refreshEnabled) { this.logger.debug('Refreshing recent scrobbles'); diff --git a/clients/MalojaScrobbler.js b/clients/MalojaScrobbler.js index 3f84f916..d4f84f6c 100644 --- a/clients/MalojaScrobbler.js +++ b/clients/MalojaScrobbler.js @@ -15,6 +15,8 @@ const feat = ["ft.", "ft", "feat.", "feat", "featuring", "Ft.", "Ft", "Feat.", " export default class MalojaScrobbler extends AbstractScrobbleClient { + requiresAuth = true; + constructor(name, config = {}, options = {}) { super('maloja', name, config, options); const {url, apiKey} = config; @@ -93,24 +95,50 @@ export default class MalojaScrobbler extends AbstractScrobbleClient { testConnection = async () => { - const {url, apiKey} = this.config; + const {url} = this.config; try { const serverInfoResp = await this.callApi(request.get(`${url}/apis/mlj_1/serverinfo`)); const { + statusCode, body: { version = [], versionstring = '', } = {}, } = serverInfoResp; - if (version.length === 0) { - this.logger.error('Server did not respond with a version. Either the base URL is incorrect or this Maloja server is too old :('); + + if (statusCode >= 300) { + this.logger.info('Test connection failed'); return false; } - this.logger.info(`Maloja Server Version: ${versionstring}`); - if (version[0] < 2 || version[1] < 7) { - this.logger.warn('Maloja Server Version is less than 2.7, please upgrade to ensure compatibility'); + + this.logger.info('Test connection succeeded!'); + + if (version.length === 0) { + this.logger.warn('Server did not respond with a version. Either the base URL is incorrect or this Maloja server is too old :('); + } else { + this.logger.info(`Maloja Server Version: ${versionstring}`); + if (version[0] < 2 || version[1] < 7) { + this.logger.warn('Maloja Server Version is less than 2.7, please upgrade to ensure compatibility'); + } } + return true; + } catch (e) { + this.logger.error('Testing connection failed'); + this.logger.error(e); + return false; + } + } + initialize = async () => { + // just checking that we can get a connection + this.initialized = await this.testConnection(); + return this.initialized; + } + + testAuth = async (withKey = true) => { + + const {url, apiKey} = this.config; + try { const resp = await this.callApi(request .get(`${url}/apis/mlj_1/test`) .query({key: apiKey})); @@ -124,20 +152,22 @@ export default class MalojaScrobbler extends AbstractScrobbleClient { text = '', } = resp; if (bodyStatus.toLocaleLowerCase() === 'ok') { - this.logger.info('Test connection succeeded!'); - this.initialized = true; - return true; + this.logger.info('Auth test passed!'); + this.authed = true; + } else { + this.authed = false; + this.logger.error('Testing connection failed => Server Response body was malformed -- should have returned "status: ok"...is the URL correct?', { + status, + body, + text: text.slice(0, 50) + }); } - this.logger.error('Testing connection failed => Server Response body was malformed -- should have returned "status: ok"...is the URL correct?', { - status, - body, - text: text.slice(0, 50) - }); - return false; } catch (e) { - this.logger.error('Testing connection failed'); - return false; + this.logger.error('Auth test failed'); + this.logger.error(e); + this.authed = false; } + return this.authed; } refreshScrobbles = async () => { diff --git a/clients/ScrobbleClients.js b/clients/ScrobbleClients.js index bd3f7f24..49c46d9e 100644 --- a/clients/ScrobbleClients.js +++ b/clients/ScrobbleClients.js @@ -208,32 +208,46 @@ ${sources.join('\n')}`); const {type, name, data: d = {}} = clientConfig; // add defaults const data = {...defaults, ...d}; + let newClient; + this.logger.debug(`(${name}) Constructing ${type} client...`); switch (type) { case 'maloja': - this.logger.debug(`(${name}) Attempting Maloja initialization...`); - const mj = new MalojaScrobbler(name, data); - const testSuccess = await mj.testConnection(); - if (testSuccess === false) { - this.logger.error(`(${name}) Maloja client not initialized due to failure during connection testing. Client needs to be successfully initialized before scrobbling.`); - } else { - this.logger.info(`(${name}) Maloja client initialized`); - } - this.clients.push(mj); + newClient = new MalojaScrobbler(name, data); break; case 'lastfm': - this.logger.debug(`(${name}) Attempting Lastfm initialization...`); - const lfm = new LastfmScrobbler(name, {...data, configDir: this.configDir}); - try { - await lfm.initialize() - this.logger.info(`(${name}) Lastfm client initialized`); - } catch(e) { - this.logger.info(`(${name}) Could not initialize Lastfm client. Client needs to be successfully initialized before scrobbling.`) - } - this.clients.push(lfm); + newClient = new LastfmScrobbler(name, {...data, configDir: this.configDir}); break; default: break; } + + if(newClient === undefined) { + // really shouldn't get here! + throw new Error(`Client of type ${type} was not recognized??`); + } + if(newClient.initialized === false) { + this.logger.debug(`(${name}) Attempting ${type} initialization...`); + if (await newClient.initialize() === false) { + this.logger.error(`(${name}) ${type} client failed to initialize. Client needs to be successfully initialized before scrobbling.`); + } else { + this.logger.info(`(${name}) ${type} client initialized`); + } + } + if(newClient.requiresAuth && !newClient.authed) { + this.logger.debug(`(${name}) Checking ${type} client auth...`); + let success; + try { + success = await newClient.testAuth(); + } catch (e) { + success = false; + } + if(!success) { + this.logger.warn(`(${name}) ${type} client auth failed.`); + } else { + this.logger.warn(`(${name}) ${type} client auth OK`); + } + } + this.clients.push(newClient); } /** @@ -262,7 +276,11 @@ ${sources.join('\n')}`); continue; } if(client.initialized === false) { - this.logger.debug(`Client '${client.name}' is not yet initialized (check authorization?)`); + this.logger.warn(`Cannot scrobble to Client '${client.name}' because it is not yet initialized`); + continue; + } + if(client.requiresAuthInteraction === true && !client.authed) { + this.logger.warn(`Cannot scrobble to Client '${client.name}' because user interaction is required for authentication`); continue; } diff --git a/index.js b/index.js index 50208323..96398f70 100644 --- a/index.js +++ b/index.js @@ -167,56 +167,64 @@ app.use(bodyParser.json()); if (logConfig.sort === 'ascending') { slicedLog.reverse(); } + // TODO links for re-trying auth and variables for signalling it (and API recently played) const sourceData = scrobbleSources.sources.map((x) => { - const {type, tracksDiscovered = 0, name, canPoll = false, polling = false} = x; - const base = {type, display: capitalize(type), tracksDiscovered, name, canPoll, hasAuth: false}; - if (canPoll) { + const { + type, + tracksDiscovered = 0, + name, + canPoll = false, + polling = false, + initialized = false, + requiresAuth = false, + requiresAuthInteraction = false, + authed = false, + } = x; + const base = { + type, + display: capitalize(type), + tracksDiscovered, + name, + canPoll, + hasAuth: requiresAuth, + hasAuthInteraction: requiresAuthInteraction, + }; + if(!initialized) { + base.status = 'Not Initialized'; + } else if(requiresAuth && !authed) { + base.status = requiresAuthInteraction ? 'Auth Interaction Required' : 'Authentication Failed Or Not Attempted' + } else if(canPoll) { base.status = polling ? 'Running' : 'Idle'; } else { base.status = tracksDiscovered > 0 ? 'Received Data' : 'Awaiting Data' } - switch (x.type) { - case 'spotify': - const authed = x.spotifyApi === undefined || x.spotifyApi.getAccessToken() !== undefined; - return { - ...base, - hasAuth: true, - authed, - status: authed ? base.status : 'Auth Interaction Required', - } - case 'lastfm': - return { - ...base, - hasAuth: true, - authed: x.initialized, - status: x.initialized ? base.status : 'Auth Interaction Required', - } - default: - return base; - } + return base; }); const clientData = scrobbleClients.clients.map((x) => { - const {type, tracksScrobbled = 0, name} = x; + const { + type, + tracksScrobbled = 0, + name, + initialized = false, + requiresAuth = false, + requiresAuthInteraction = false, + authed = false, + } = x; const base = { type, display: capitalize(type), tracksDiscovered: tracksScrobbled, name, - hasAuth: false, - status: tracksScrobbled > 0 ? 'Received Data' : 'Awaiting Data' + hasAuth: requiresAuth, }; - switch (x.type) { - case 'lastfm': - const authed = x.initialized; - return { - ...base, - hasAuth: true, - authed, - status: authed ? base.status : 'Auth Interaction Required', - } - default: - return base; + if(!initialized) { + base.status = 'Not Initialized'; + } else if(requiresAuth && !authed) { + base.status = requiresAuthInteraction ? 'Auth Interaction Required' : 'Authentication Failed Or Not Attempted' + } else { + base.status = tracksScrobbled > 0 ? 'Received Data' : 'Awaiting Data'; } + return base; }) res.render('status', { sources: sourceData, diff --git a/sources/AbstractSource.js b/sources/AbstractSource.js index 187625b5..a70f0b31 100644 --- a/sources/AbstractSource.js +++ b/sources/AbstractSource.js @@ -11,6 +11,10 @@ export default class AbstractSource { clients; logger; instantiatedAt; + initialized = false; + requiresAuth = false; + requiresAuthInteraction = false; + authed = false; canPoll = false; polling = false; @@ -27,6 +31,17 @@ export default class AbstractSource { this.instantiatedAt = dayjs(); } + // default init function, should be overridden if init stage is required + initialize = async () => { + this.initialized = true; + return this.initialized; + } + + // default init function, should be overridden if auth stage is required + testAuth = async () => { + return this.authed; + } + getRecentlyPlayed = async (options = {}) => { return []; } @@ -43,6 +58,10 @@ export default class AbstractSource { } startPolling = async (allClients) => { + if(this.requiresAuthInteraction && !this.authed) { + this.logger.error('Cannot start polling because user interaction is required for authentication'); + return; + } // reset poll attempts if already previously run this.pollRetries = 0; diff --git a/sources/JellyfinSource.js b/sources/JellyfinSource.js index a2511365..a5c70478 100644 --- a/sources/JellyfinSource.js +++ b/sources/JellyfinSource.js @@ -38,6 +38,7 @@ export default class JellyfinSource extends MemorySource { } else { this.logger.info(`Initializing with the following filters => Users: ${this.users === undefined ? 'N/A' : this.users.join(', ')} | Servers: ${this.servers === undefined ? 'N/A' : this.servers.join(', ')}`); } + this.initialized = true; } static formatPlayObj(obj, newFromSource = false) { diff --git a/sources/LastfmSource.js b/sources/LastfmSource.js index aad4d87e..8c50f279 100644 --- a/sources/LastfmSource.js +++ b/sources/LastfmSource.js @@ -5,7 +5,8 @@ import {sortByPlayDate} from "../utils.js"; export default class LastfmSource extends AbstractSource { api; - initialized = false; + requiresAuth = true; + requiresAuthInteraction = true; constructor(name, config = {}, clients = []) { super('lastfm', name, config, clients); @@ -22,6 +23,18 @@ export default class LastfmSource extends AbstractSource { return this.initialized; } + testAuth = async () => { + try { + this.authed = await this.api.testAuth(); + } catch (e) { + this.logger.error('Could not successfully communicate with Last.fm API'); + this.logger.error(e); + this.authed = false; + } + return this.authed; + } + + getRecentlyPlayed = async(options = {}) => { const {limit = 20, formatted = false} = options; const resp = await this.api.callApi(client => client.userGetRecentTracks({user: this.api.user, limit, extended: true})); diff --git a/sources/PlexSource.js b/sources/PlexSource.js index 2e3f0f9f..4f3b423d 100644 --- a/sources/PlexSource.js +++ b/sources/PlexSource.js @@ -50,6 +50,7 @@ export default class PlexSource extends AbstractSource { } else { this.logger.info(`Initializing with the following filters => Users: ${this.users === undefined ? 'N/A' : this.users.join(', ')} | Libraries: ${this.libraries === undefined ? 'N/A' : this.libraries.join(', ')} | Servers: ${this.servers === undefined ? 'N/A' : this.servers.join(', ')}`); } + this.initialized = true; } static formatPlayObj(obj, newFromSource = false) { diff --git a/sources/ScrobbleSources.js b/sources/ScrobbleSources.js index b6ebc84e..5a34533c 100644 --- a/sources/ScrobbleSources.js +++ b/sources/ScrobbleSources.js @@ -209,6 +209,7 @@ export default class ScrobbleSources { const isValid = isValidConfigStructure(c, {type: true, data: true}); if (isValid !== true) { this.logger.error(`Source config from ${c.source} with name [${c.name || 'unnamed'}] of type [${c.type || 'unknown'}] will not be used because it has structural errors: ${isValid.join(' | ')}`); + return acc; } return acc.concat(c); }, []); @@ -261,42 +262,66 @@ export default class ScrobbleSources { const {type, name, clients = [], data: d = {}} = clientConfig; // add defaults const data = {...defaults, ...d}; - this.logger.debug(`(${name}) Initializing ${type} source`); + this.logger.debug(`(${name}) Constructing ${type} source`); + let newSource; switch (type) { case 'spotify': - const spotifySource = new SpotifySource(name, { + newSource = new SpotifySource(name, { ...data, localUrl: this.localUrl, configDir: this.configDir }, clients); - await spotifySource.buildSpotifyApi(); - this.sources.push(spotifySource); break; case 'plex': - const plexSource = await new PlexSource(name, data, clients); - this.sources.push(plexSource); + newSource = await new PlexSource(name, data, clients); break; case 'tautulli': - const tautulliSource = await new TautulliSource(name, data, clients); - this.sources.push(tautulliSource); + newSource = await new TautulliSource(name, data, clients); break; case 'subsonic': - const ssSource = new SubsonicSource(name, data, clients); - await ssSource.testConnection(); - this.sources.push(ssSource); + newSource = new SubsonicSource(name, data, clients); break; case 'jellyfin': - const jellyfinSource = await new JellyfinSource(name, data, clients); - this.sources.push(jellyfinSource); + newSource = await new JellyfinSource(name, data, clients); break; case 'lastfm': - const lastfmSource = await new LastfmSource(name, {...data, configDir: this.configDir}, clients); - await lastfmSource.initialize(); - this.sources.push(lastfmSource); + newSource = await new LastfmSource(name, {...data, configDir: this.configDir}, clients); break; default: break; } - this.logger.info(`(${name}) ${type} source initialized`); + + if(newSource === undefined) { + // really shouldn't get here! + throw new Error(`Source of type ${type} was not recognized??`); + } + if(newSource.initialized === false) { + this.logger.debug(`(${name}) Attempting ${type} initialization...`); + if (await newSource.initialize() === false) { + this.logger.error(`(${name}) ${type} source failed to initialize. Source needs to be successfully initialized before activity capture can begin.`); + return; + } else { + this.logger.info(`(${name}) ${type} source initialized`); + } + } else { + this.logger.info(`(${name}) ${type} source initialized`); + } + + if(newSource.requiresAuth && !newSource.authed) { + this.logger.debug(`(${name}) Checking ${type} source auth...`); + let success; + try { + success = await newSource.testAuth(); + } catch (e) { + success = false; + } + if(!success) { + this.logger.warn(`(${name}) ${type} source auth failed.`); + } else { + this.logger.info(`(${name}) ${type} source auth OK`); + } + } + + this.sources.push(newSource); } } diff --git a/sources/SpotifySource.js b/sources/SpotifySource.js index 4bdef907..1b97ddc5 100644 --- a/sources/SpotifySource.js +++ b/sources/SpotifySource.js @@ -17,6 +17,9 @@ export default class SpotifySource extends AbstractSource { workingCredsPath; configDir; + requiresAuth = true; + requiresAuthInteraction = true; + constructor(name, config = {}, clients = []) { super('spotify', name, config, clients); const { @@ -140,6 +143,25 @@ export default class SpotifySource extends AbstractSource { this.spotifyApi = new SpotifyWebApi(apiConfig); } + initialize = async () => { + if(this.spotifyApi === undefined) { + await this.buildSpotifyApi(); + } + this.initialized = true; + return this.initialized; + } + + testAuth = async () => { + try { + await this.callApi((api => api.getMe())); + this.authed = true; + } catch (e) { + this.logger.error('Could not successfully communicate with Spotify API'); + this.authed = false; + } + return this.authed; + } + createAuthUrl = () => { return this.spotifyApi.createAuthorizeURL(scopes, this.name); } diff --git a/sources/SubsonicSource.js b/sources/SubsonicSource.js index a617f1c8..fc6ee8d1 100644 --- a/sources/SubsonicSource.js +++ b/sources/SubsonicSource.js @@ -10,6 +10,8 @@ dayjs.extend(isSameOrAfter); export class SubsonicSource extends MemorySource { + requiresAuth = true; + constructor(name, config = {}, clients = []) { // default to quick interval so we can get a decently accurate nowPlaying const subsonicConfig = {interval: 10, maxSleep: 30, ...config}; @@ -124,14 +126,34 @@ export class SubsonicSource extends MemorySource { } } - testConnection = async () => { + initialize = async () => { + const {url} = this.config; + try { + await request.get(`${url}/`); + this.logger.info('Subsonic Connection: ok'); + this.initialized = true; + } catch (e) { + if(e.status !== undefined && e.status !== 404) { + this.logger.info('Subsonic Connection: ok'); + // we at least got a response! + this.initialized = true; + } + } + + return this.initialized; + } + + testAuth= async () => { const {url} = this.config; try { await this.callApi(request.get(`${url}/rest/ping`)); + this.authed = true; this.logger.info('Subsonic API Status: ok'); } catch (e) { - this.logger.error(e); + this.authed = false; } + + return this.authed; } getRecentlyPlayed = async (options = {}) => { diff --git a/views/status.ejs b/views/status.ejs index 988a324c..b63ae507 100644 --- a/views/status.ejs +++ b/views/status.ejs @@ -33,7 +33,7 @@ <% sources.forEach(function (source){ %>
-

<%= source.display %> - <%= source.name %>

+

(Source) <%= source.display %> - <%= source.name %>

Status: <%= source.status %>
@@ -53,7 +53,7 @@ <% clients.forEach(function (client){ %>
-

<%= client.display %> - <%= client.name %>

+

(Client) <%= client.display %> - <%= client.name %>

    -- 2.51.2