From e70c189576a80a162a3183172aac74f288e66ccd Mon Sep 17 00:00:00 2001 From: Florrie Date: Mon, 7 Aug 2017 17:08:37 -0300 Subject: Delete temporary files when done with them (safeUnlink isn't really tested) --- src/downloaders.js | 7 +++++++ src/loop-play.js | 48 +++++++++++++++++++++++++++++++++++++++--------- src/play.js | 4 ++-- src/playlist-utils.js | 30 +++++++++++++++++++++++++++++- todo.txt | 1 + 5 files changed, 78 insertions(+), 12 deletions(-) diff --git a/src/downloaders.js b/src/downloaders.js index b9cc33d..2df6655 100644 --- a/src/downloaders.js +++ b/src/downloaders.js @@ -14,6 +14,13 @@ const { promisify } = require('util') const writeFile = promisify(fs.writeFile) const copyFile = fse.copy +// Pseudo-tempy!! +/* +const tempy = { + directory: () => './tempy-fake' +} +*/ + function makeHTTPDownloader() { return function(arg) { const dir = tempy.directory() diff --git a/src/loop-play.js b/src/loop-play.js index 9d87423..dd477a1 100644 --- a/src/loop-play.js +++ b/src/loop-play.js @@ -11,12 +11,14 @@ const { spawn } = require('child_process') const FIFO = require('fifo-js') const EventEmitter = require('events') +const promisifyProcess = require('./promisify-process') +const { getItemPathString } = require('./playlist-utils') + +const { safeUnlink } = require('./playlist-utils') + const { - getDownloaderFor, makeConverterDownloader, - byName: downloadersByName + getDownloaderFor, byName: downloadersByName } = require('./downloaders') -const { getItemPathString } = require('./playlist-utils') -const promisifyProcess = require('./promisify-process') class DownloadController extends EventEmitter { waitForDownload() { @@ -47,6 +49,7 @@ class DownloadController extends EventEmitter { // (The reasoning is that it's possible for a download to // be canceled and replaced with a new download (see cancel) // which would void the result of the old download.) + // The resulting file is a WAV. this.cleanupListeners() @@ -68,8 +71,13 @@ class DownloadController extends EventEmitter { return } - if (!canceled) { + // If this current download has been canceled, we should get rid of the + // download file (and shouldn't emit a download success). + if (canceled) { + this.emit('deleteFile', file) + } else { this.emit('downloaded', file) + this.cleanupListeners() } } @@ -113,10 +121,21 @@ class PlayController extends EventEmitter { while (this.nextTrack) { this.currentTrack = this.nextTrack + const next = this.nextFile + this.nextFile = undefined + this.startNextDownload() - if (this.nextFile) { - await this.playFile(this.nextFile) + if (next) { + await this.playFile(next) + + // Now that we're done playing the file, we should delete it.. unless + // it's the file that's coming up! (This would only happen in the case + // that all temporary files are stored in the same folder, together; + // indeed an unusual case, but technically possible.) + if (next !== this.nextFile) { + this.emit('deleteFile', next) + } } await this.waitForDownload() @@ -159,7 +178,6 @@ class PlayController extends EventEmitter { downloader = getDownloaderFor(picked.downloaderArg) } - downloader = makeConverterDownloader(downloader, 'wav') this.downloadController.download(downloader, picked.downloaderArg) this.downloadController.waitForDownload() @@ -266,6 +284,15 @@ class PlayController extends EventEmitter { this.kill() } + async skipUpNext() { + if (this.nextFile) { + this.emit('deleteFile', this.nextFile) + } + + this.downloadController.cancel() + this.startNextDownload() + } + seekAhead(secs) { this.sendCommand(`seek +${parseFloat(secs)}`) } @@ -329,7 +356,7 @@ class PlayController extends EventEmitter { } module.exports = function loopPlay( - picker, playerCommand = 'mpv', playOpts = [] + playlist, picker, playerCommand = 'mpv', playOpts = [] ) { // Looping play function. Takes one argument, the "picker" function, // which returns a track to play. Stops when the result of the picker @@ -340,6 +367,9 @@ module.exports = function loopPlay( const playController = new PlayController(picker, downloadController) + downloadController.on('deleteFile', f => safeUnlink(f, playlist)) + playController.on('deleteFile', f => safeUnlink(f, playlist)) + Object.assign(playController, {playerCommand, playOpts}) const promise = playController.loopPlay() diff --git a/src/play.js b/src/play.js index 7807c93..43576ff 100755 --- a/src/play.js +++ b/src/play.js @@ -343,7 +343,7 @@ async function main(args) { promise: playPromise, playController: play, downloadController - } = loopPlay(picker, playerCommand, playOpts) + } = loopPlay(activePlaylist, picker, playerCommand, playOpts) // We're looking to gather standard input one keystroke at a time. // But that isn't *always* possible, e.g. when piping into the http-music @@ -413,7 +413,7 @@ async function main(args) { // TODO: It would be nice to have this as a method of // PlayController. // Double TODO: This doesn't actually work!! - play.startNextDownload() + play.skipUpNext() } if ( diff --git a/src/playlist-utils.js b/src/playlist-utils.js index aed4964..765a317 100644 --- a/src/playlist-utils.js +++ b/src/playlist-utils.js @@ -1,5 +1,11 @@ 'use strict' +const path = require('path') +const fs = require('fs') + +const { promisify } = require('util') +const unlink = promisify(fs.unlink) + const parentSymbol = Symbol('parent') function updatePlaylistFormat(playlist) { @@ -287,6 +293,27 @@ function isTrack(obj) { // return typeof array[1] === 'string' } +function safeUnlink(file, playlist) { + if (!playlist) { + throw new Error('No playlist given to safe-unlink.') + } + + // TODO: Is it good to call this every time? - But flattening a list probably + // isn't THAT big of a task. + const flat = flattenGrouplike(playlist) + + if ( + flat.items.some(t => path.resolve(t.downloaderArg) === path.resolve(file)) + ) { + throw new Error( + 'Attempted to delete a file path found in the playlist.json file - ' + + 'this is almost definitely a bug!' + ) + } + + return unlink(file) +} + module.exports = { parentSymbol, updatePlaylistFormat, updateTrackFormat, @@ -296,5 +323,6 @@ module.exports = { getPlaylistTreeString, getItemPath, getItemPathString, parsePathString, - isGroup, isTrack + isGroup, isTrack, + safeUnlink } diff --git a/todo.txt b/todo.txt index a6e4f48..417e506 100644 --- a/todo.txt +++ b/todo.txt @@ -267,3 +267,4 @@ TODO: Handle avconv failing (probably handle downloader rejections from within TODO: Delete temporary files when done with them - seriously! http-music alone filled up a good 9GB of disk space, solely on temporary music files. + (Done!) -- cgit 1.3.0-6-gf8a5