From 860c01a7f3f7468f40441856badf7e02e3b3236b Mon Sep 17 00:00:00 2001 From: JellyBrick Date: Mon, 19 Feb 2024 14:30:17 +0900 Subject: [PATCH 1/2] feat: use `BrowserWindow` instead of `shell.openExternal` --- src/i18n/resources/en.json | 8 ++ src/plugins/scrobbler/main.ts | 56 ++++++--- src/plugins/scrobbler/menu.ts | 12 +- src/plugins/scrobbler/services/base.ts | 5 +- src/plugins/scrobbler/services/lastfm.ts | 119 ++++++++++++++---- .../scrobbler/services/listenbrainz.ts | 12 +- 6 files changed, 155 insertions(+), 57 deletions(-) diff --git a/src/i18n/resources/en.json b/src/i18n/resources/en.json index 8db7a7fcca..6e33171778 100644 --- a/src/i18n/resources/en.json +++ b/src/i18n/resources/en.json @@ -579,6 +579,14 @@ }, "scrobbler": { "description": "Add scrobbling support (etc. last.fm, Listenbrainz)", + "dialog": { + "lastfm": { + "auth-failed": { + "title": "Authentication Failed", + "message": "Failed to authenticate with Last.fm" + } + } + }, "menu": { "scrobble-other-media": "Scrobble other media", "lastfm": { diff --git a/src/plugins/scrobbler/main.ts b/src/plugins/scrobbler/main.ts index e55e44a0ad..502c6dc0cf 100644 --- a/src/plugins/scrobbler/main.ts +++ b/src/plugins/scrobbler/main.ts @@ -1,10 +1,13 @@ +import { BrowserWindow } from 'electron'; + import registerCallback, { MediaType, type SongInfo } from '@/providers/song-info'; import { createBackend } from '@/utils'; -import { ScrobblerPluginConfig } from './index'; import { LastFmScrobbler } from './services/lastfm'; import { ListenbrainzScrobbler } from './services/listenbrainz'; -import { ScrobblerBase } from './services/base'; + +import type { ScrobblerPluginConfig } from './index'; +import type { ScrobblerBase } from './services/base'; export type SetConfType = ( conf: Partial>, @@ -12,14 +15,17 @@ export type SetConfType = ( export const backend = createBackend<{ config?: ScrobblerPluginConfig; + window?: BrowserWindow; enabledScrobblers: Map; - toggleScrobblers(config: ScrobblerPluginConfig): void; + toggleScrobblers(config: ScrobblerPluginConfig, window: BrowserWindow): void; + createSessions(config: ScrobblerPluginConfig, setConfig: SetConfType): Promise; + setConfig?: SetConfType; }, ScrobblerPluginConfig>({ enabledScrobblers: new Map(), - toggleScrobblers(config: ScrobblerPluginConfig) { + toggleScrobblers(config: ScrobblerPluginConfig, window: BrowserWindow) { if (config.scrobblers.lastfm && config.scrobblers.lastfm.enabled) { - this.enabledScrobblers.set('lastfm', new LastFmScrobbler()); + this.enabledScrobblers.set('lastfm', new LastFmScrobbler(window)); } else { this.enabledScrobblers.delete('lastfm'); } @@ -31,20 +37,27 @@ export const backend = createBackend<{ } }, + async createSessions(config: ScrobblerPluginConfig, setConfig: SetConfType) { + for (const [, scrobbler] of this.enabledScrobblers) { + if (!scrobbler.isSessionCreated(config)) { + await scrobbler.createSession(config, setConfig); + } + } + }, + async start({ getConfig, setConfig, + window, }) { const config = this.config = await getConfig(); // This will store the timeout that will trigger addScrobble let scrobbleTimer: NodeJS.Timeout | undefined; - this.toggleScrobblers(config); - for (const [, scrobbler] of this.enabledScrobblers) { - if (!scrobbler.isSessionCreated(config)) { - await scrobbler.createSession(config, setConfig); - } - } + this.window = window; + this.toggleScrobblers(config, window); + await this.createSessions(config, setConfig); + this.setConfig = setConfig; registerCallback((songInfo: SongInfo) => { // Set remove the old scrobble timer @@ -52,7 +65,7 @@ export const backend = createBackend<{ if (!songInfo.isPaused) { const configNonnull = this.config!; // Scrobblers normally have no trouble working with official music videos - if (!configNonnull.scrobble_other_media && (songInfo.mediaType !== MediaType.Audio && songInfo.mediaType !== MediaType.OriginalMusicVideo)) { + if (!configNonnull.scrobbleOtherMedia && (songInfo.mediaType !== MediaType.Audio && songInfo.mediaType !== MediaType.OriginalMusicVideo)) { return; } @@ -71,12 +84,25 @@ export const backend = createBackend<{ }); }, - onConfigChange(newConfig: ScrobblerPluginConfig) { + async onConfigChange(newConfig: ScrobblerPluginConfig) { this.enabledScrobblers.clear(); - this.config = newConfig; + this.toggleScrobblers(newConfig, this.window!); + for (const [scrobblerName, scrobblerConfig] of Object.entries(newConfig.scrobblers)) { + if (scrobblerConfig.enabled) { + const scrobbler = this.enabledScrobblers.get(scrobblerName); + if ( + this.config?.scrobblers?.[scrobblerName as keyof typeof newConfig.scrobblers]?.enabled !== scrobblerConfig.enabled && + scrobbler && + !scrobbler.isSessionCreated(newConfig) && + this.setConfig + ) { + await scrobbler.createSession(newConfig, this.setConfig); + } + } + } - this.toggleScrobblers(this.config); + this.config = newConfig; } }); diff --git a/src/plugins/scrobbler/menu.ts b/src/plugins/scrobbler/menu.ts index 1c663225b6..a24d2d00de 100644 --- a/src/plugins/scrobbler/menu.ts +++ b/src/plugins/scrobbler/menu.ts @@ -20,7 +20,7 @@ async function promptLastFmOptions(options: ScrobblerPluginConfig, setConfig: Se multiInputOptions: [ { label: t('plugins.scrobbler.prompt.lastfm.api-key'), - value: options.scrobblers.lastfm?.api_key, + value: options.scrobblers.lastfm?.apiKey, inputAttrs: { type: 'text' } @@ -42,7 +42,7 @@ async function promptLastFmOptions(options: ScrobblerPluginConfig, setConfig: Se if (output) { if (output[0]) { - options.scrobblers.lastfm.api_key = output[0]; + options.scrobblers.lastfm.apiKey = output[0]; } if (output[1]) { @@ -82,9 +82,9 @@ export const onMenu = async ({ { label: t('plugins.scrobbler.menu.scrobble-other-media'), type: 'checkbox', - checked: Boolean(config.scrobble_other_media), + checked: Boolean(config.scrobbleOtherMedia), click(item) { - config.scrobble_other_media = item.checked; + config.scrobbleOtherMedia = item.checked; setConfig(config); }, }, @@ -96,7 +96,7 @@ export const onMenu = async ({ type: 'checkbox', checked: Boolean(config.scrobblers.lastfm?.enabled), click(item) { - backend.toggleScrobblers(config); + backend.toggleScrobblers(config, window); config.scrobblers.lastfm.enabled = item.checked; setConfig(config); }, @@ -117,7 +117,7 @@ export const onMenu = async ({ type: 'checkbox', checked: Boolean(config.scrobblers.listenbrainz?.enabled), click(item) { - backend.toggleScrobblers(config); + backend.toggleScrobblers(config, window); config.scrobblers.listenbrainz.enabled = item.checked; setConfig(config); }, diff --git a/src/plugins/scrobbler/services/base.ts b/src/plugins/scrobbler/services/base.ts index ba988b3a00..dd3afc021e 100644 --- a/src/plugins/scrobbler/services/base.ts +++ b/src/plugins/scrobbler/services/base.ts @@ -1,6 +1,5 @@ -import { ScrobblerPluginConfig } from '../index'; -import { SetConfType } from '../main'; - +import type { ScrobblerPluginConfig } from '../index'; +import type { SetConfType } from '../main'; import type { SongInfo } from '@/providers/song-info'; export abstract class ScrobblerBase { diff --git a/src/plugins/scrobbler/services/lastfm.ts b/src/plugins/scrobbler/services/lastfm.ts index bd23a58c64..4c6eb9bbba 100644 --- a/src/plugins/scrobbler/services/lastfm.ts +++ b/src/plugins/scrobbler/services/lastfm.ts @@ -1,12 +1,13 @@ import crypto from 'node:crypto'; -import { net, shell } from 'electron'; +import { BrowserWindow, dialog, net } from 'electron'; import { ScrobblerBase } from './base'; -import { ScrobblerPluginConfig } from '../index'; -import { SetConfType } from '../main'; +import { t } from '@/i18n'; +import type { ScrobblerPluginConfig } from '../index'; +import type { SetConfType } from '../main'; import type { SongInfo } from '@/providers/song-info'; interface LastFmData { @@ -28,11 +29,22 @@ interface LastFmSongData { } export class LastFmScrobbler extends ScrobblerBase { - isSessionCreated(config: ScrobblerPluginConfig): boolean { + mainWindow: BrowserWindow; + + constructor(mainWindow: BrowserWindow) { + super(); + + this.mainWindow = mainWindow; + } + + override isSessionCreated(config: ScrobblerPluginConfig): boolean { return !!config.scrobblers.lastfm.sessionKey; } - async createSession(config: ScrobblerPluginConfig, setConfig: SetConfType): Promise { + override async createSession( + config: ScrobblerPluginConfig, + setConfig: SetConfType, + ): Promise { // Get and store the session key const data = { api_key: config.scrobblers.lastfm.apiKey, @@ -52,8 +64,15 @@ export class LastFmScrobbler extends ScrobblerBase { }; if (json.error) { config.scrobblers.lastfm.token = await createToken(config); - await authenticate(config); - setConfig(config); + // If is successful, we need retry the request + authenticate(config, setConfig, this.mainWindow).then((it) => { + if (it) { + this.createSession(config, setConfig); + } else { + // failed + setConfig(config); + } + }); } if (json.session) { config.scrobblers.lastfm.sessionKey = json.session.key; @@ -62,7 +81,7 @@ export class LastFmScrobbler extends ScrobblerBase { return config; } - setNowPlaying(songInfo: SongInfo, config: ScrobblerPluginConfig, setConfig: SetConfType): void { + override setNowPlaying(songInfo: SongInfo, config: ScrobblerPluginConfig, setConfig: SetConfType): void { if (!config.scrobblers.lastfm.sessionKey) { return; } @@ -74,7 +93,7 @@ export class LastFmScrobbler extends ScrobblerBase { this.postSongDataToAPI(songInfo, config, data, setConfig); } - addScrobble(songInfo: SongInfo, config: ScrobblerPluginConfig, setConfig: SetConfType): void { + override addScrobble(songInfo: SongInfo, config: ScrobblerPluginConfig, setConfig: SetConfType): void { if (!config.scrobblers.lastfm.sessionKey) { return; } @@ -87,7 +106,7 @@ export class LastFmScrobbler extends ScrobblerBase { this.postSongDataToAPI(songInfo, config, data, setConfig); } - async postSongDataToAPI( + private async postSongDataToAPI( songInfo: SongInfo, config: ScrobblerPluginConfig, data: LastFmData, @@ -128,7 +147,7 @@ export class LastFmScrobbler extends ScrobblerBase { // Session key is invalid, so remove it from the config and reauthenticate config.scrobblers.lastfm.sessionKey = undefined; config.scrobblers.lastfm.token = await createToken(config); - await authenticate(config); + await authenticate(config, setConfig, this.mainWindow); setConfig(config); } else { console.error(error); @@ -168,17 +187,17 @@ const createQueryString = ( const createApiSig = (parameters: LastFmSongData, secret: string) => { // This function creates the api signature, see: https://www.last.fm/api/authspec - const keys = Object.keys(parameters); - - keys.sort(); let sig = ''; - for (const key of keys) { - if (key === 'format') { - continue; - } - sig += `${key}${parameters[key as keyof LastFmSongData]}`; - } + Object + .entries(parameters) + .sort(([a], [b]) => a.localeCompare(b)) + .forEach(([key, value]) => { + if (key === 'format') { + return; + } + sig += key + value; + }); sig += secret; sig = crypto.createHash('md5').update(sig, 'utf-8').digest('hex'); @@ -195,7 +214,11 @@ const createToken = async ({ } }: ScrobblerPluginConfig) => { // Creates and stores the auth token - const data = { + const data: { + method: string; + api_key: string; + format: string; + } = { method: 'auth.gettoken', api_key: apiKey, format: 'json', @@ -208,9 +231,53 @@ const createToken = async ({ return json?.token; }; -const authenticate = async (config: ScrobblerPluginConfig) => { - // Asks the user for authentication - await shell.openExternal( - `https://www.last.fm/api/auth/?api_key=${config.scrobblers.lastfm.apiKey}&token=${config.scrobblers.lastfm.token}`, - ); +const authenticate = async (config: ScrobblerPluginConfig, setConfig: SetConfType, mainWindow: BrowserWindow) => { + return new Promise((resolve) => { + const url = `https://www.last.fm/api/auth/?api_key=${config.scrobblers.lastfm.apiKey}&token=${config.scrobblers.lastfm.token}`; + const browserWindow = new BrowserWindow({ + width: 500, + height: 600, + show: false, + webPreferences: { + nodeIntegration: false, + }, + autoHideMenuBar: true, + parent: mainWindow, + minimizable: false, + maximizable: false, + paintWhenInitiallyHidden: true, + modal: true, + center: true, + }); + browserWindow.loadURL(url).then(() => { + browserWindow.show(); + browserWindow.webContents.on('did-navigate', async (_, newUrl) => { + const url = new URL(newUrl); + if (url.hostname.endsWith('last.fm')) { + if (url.pathname === '/api/auth') { + const isApproveScreen = await browserWindow.webContents.executeJavaScript( + '!!document.getElementsByName(\'confirm\').length' + ) as boolean; + // successful authentication + if (!isApproveScreen) { + resolve(true); + browserWindow.close(); + } + } else if (url.pathname === '/api/None') { + dialog.showMessageBox({ + title: t('plugins.scrobbler.dialog.lastfm.auth-failed.title'), + message: t('plugins.scrobbler.dialog.lastfm.auth-failed.message'), + type: 'error' + }); + browserWindow.close(); + + config.scrobblers.lastfm.enabled = false; + setConfig(config); + mainWindow.webContents.send('refresh-in-app-menu'); + resolve(false); + } + } + }); + }); + }); }; diff --git a/src/plugins/scrobbler/services/listenbrainz.ts b/src/plugins/scrobbler/services/listenbrainz.ts index 7b19e78cd9..095b747740 100644 --- a/src/plugins/scrobbler/services/listenbrainz.ts +++ b/src/plugins/scrobbler/services/listenbrainz.ts @@ -2,10 +2,8 @@ import { net } from 'electron'; import { ScrobblerBase } from './base'; -import { SetConfType } from '../main'; - +import type { SetConfType } from '../main'; import type { SongInfo } from '@/providers/song-info'; - import type { ScrobblerPluginConfig } from '../index'; interface ListenbrainzRequestBody { @@ -27,15 +25,15 @@ interface ListenbrainzRequestBody { } export class ListenbrainzScrobbler extends ScrobblerBase { - isSessionCreated(): boolean { + override isSessionCreated(): boolean { return true; } - createSession(config: ScrobblerPluginConfig, _setConfig: SetConfType): Promise { + override createSession(config: ScrobblerPluginConfig, _setConfig: SetConfType): Promise { return Promise.resolve(config); } - setNowPlaying(songInfo: SongInfo, config: ScrobblerPluginConfig, _setConfig: SetConfType): void { + override setNowPlaying(songInfo: SongInfo, config: ScrobblerPluginConfig, _setConfig: SetConfType): void { if (!config.scrobblers.listenbrainz.apiRoot || !config.scrobblers.listenbrainz.token) { return; } @@ -44,7 +42,7 @@ export class ListenbrainzScrobbler extends ScrobblerBase { submitListen(body, config); } - addScrobble(songInfo: SongInfo, config: ScrobblerPluginConfig, _setConfig: SetConfType): void { + override addScrobble(songInfo: SongInfo, config: ScrobblerPluginConfig, _setConfig: SetConfType): void { if (!config.scrobblers.listenbrainz.apiRoot || !config.scrobblers.listenbrainz.token) { return; } From a182abc9153c87e84439d34a7c12048d489ffc47 Mon Sep 17 00:00:00 2001 From: JellyBrick Date: Mon, 19 Feb 2024 15:35:46 +0900 Subject: [PATCH 2/2] fix: duplicated popup --- src/i18n/resources/en.json | 2 +- src/plugins/scrobbler/services/lastfm.ts | 103 ++++++++++++++--------- 2 files changed, 63 insertions(+), 42 deletions(-) diff --git a/src/i18n/resources/en.json b/src/i18n/resources/en.json index 6e33171778..da1b50203a 100644 --- a/src/i18n/resources/en.json +++ b/src/i18n/resources/en.json @@ -583,7 +583,7 @@ "lastfm": { "auth-failed": { "title": "Authentication Failed", - "message": "Failed to authenticate with Last.fm" + "message": "Failed to authenticate with Last.fm\nHide the popup until the next restart." } } }, diff --git a/src/plugins/scrobbler/services/lastfm.ts b/src/plugins/scrobbler/services/lastfm.ts index 4c6eb9bbba..b577950bbb 100644 --- a/src/plugins/scrobbler/services/lastfm.ts +++ b/src/plugins/scrobbler/services/lastfm.ts @@ -65,7 +65,7 @@ export class LastFmScrobbler extends ScrobblerBase { if (json.error) { config.scrobblers.lastfm.token = await createToken(config); // If is successful, we need retry the request - authenticate(config, setConfig, this.mainWindow).then((it) => { + authenticate(config, this.mainWindow).then((it) => { if (it) { this.createSession(config, setConfig); } else { @@ -147,8 +147,14 @@ export class LastFmScrobbler extends ScrobblerBase { // Session key is invalid, so remove it from the config and reauthenticate config.scrobblers.lastfm.sessionKey = undefined; config.scrobblers.lastfm.token = await createToken(config); - await authenticate(config, setConfig, this.mainWindow); - setConfig(config); + authenticate(config, this.mainWindow).then((it) => { + if (it) { + this.createSession(config, setConfig); + } else { + // failed + setConfig(config); + } + }); } else { console.error(error); } @@ -231,53 +237,68 @@ const createToken = async ({ return json?.token; }; -const authenticate = async (config: ScrobblerPluginConfig, setConfig: SetConfType, mainWindow: BrowserWindow) => { +let authWindowOpened = false; +let latestAuthResult = false; + +const authenticate = async (config: ScrobblerPluginConfig, mainWindow: BrowserWindow) => { return new Promise((resolve) => { - const url = `https://www.last.fm/api/auth/?api_key=${config.scrobblers.lastfm.apiKey}&token=${config.scrobblers.lastfm.token}`; - const browserWindow = new BrowserWindow({ - width: 500, - height: 600, - show: false, - webPreferences: { - nodeIntegration: false, - }, - autoHideMenuBar: true, - parent: mainWindow, - minimizable: false, - maximizable: false, - paintWhenInitiallyHidden: true, - modal: true, - center: true, - }); - browserWindow.loadURL(url).then(() => { - browserWindow.show(); - browserWindow.webContents.on('did-navigate', async (_, newUrl) => { - const url = new URL(newUrl); - if (url.hostname.endsWith('last.fm')) { - if (url.pathname === '/api/auth') { - const isApproveScreen = await browserWindow.webContents.executeJavaScript( - '!!document.getElementsByName(\'confirm\').length' - ) as boolean; - // successful authentication - if (!isApproveScreen) { - resolve(true); + if (!authWindowOpened) { + authWindowOpened = true; + const url = `https://www.last.fm/api/auth/?api_key=${config.scrobblers.lastfm.apiKey}&token=${config.scrobblers.lastfm.token}`; + const browserWindow = new BrowserWindow({ + width: 500, + height: 600, + show: false, + webPreferences: { + nodeIntegration: false, + }, + autoHideMenuBar: true, + parent: mainWindow, + minimizable: false, + maximizable: false, + paintWhenInitiallyHidden: true, + modal: true, + center: true, + }); + browserWindow.loadURL(url).then(() => { + browserWindow.show(); + browserWindow.webContents.on('did-navigate', async (_, newUrl) => { + const url = new URL(newUrl); + if (url.hostname.endsWith('last.fm')) { + if (url.pathname === '/api/auth') { + const isApproveScreen = await browserWindow.webContents.executeJavaScript( + '!!document.getElementsByName(\'confirm\').length' + ) as boolean; + // successful authentication + if (!isApproveScreen) { + resolve(true); + latestAuthResult = true; + browserWindow.close(); + } + } else if (url.pathname === '/api/None') { + resolve(false); + latestAuthResult = false; browserWindow.close(); } - } else if (url.pathname === '/api/None') { + } + }); + browserWindow.on('closed', () => { + if (!latestAuthResult) { dialog.showMessageBox({ title: t('plugins.scrobbler.dialog.lastfm.auth-failed.title'), message: t('plugins.scrobbler.dialog.lastfm.auth-failed.message'), type: 'error' }); - browserWindow.close(); - - config.scrobblers.lastfm.enabled = false; - setConfig(config); - mainWindow.webContents.send('refresh-in-app-menu'); - resolve(false); } - } + authWindowOpened = false; + }); }); - }); + } else { + // wait for the previous window to close + while (authWindowOpened) { + // wait + } + resolve(latestAuthResult); + } }); };