Merge branch 'master' into feature/slash-commands

.github/workflows/build.yml

@@ -1,6 +1,6 @@
 name: Lint & build
 
-on: push
+on: [push, pull_request]
 
 jobs:
   build:

.github/workflows/pr.yml

@@ -1,10 +1,6 @@
 name: PR Workflow
 
-on:
-  pull_request:
-    types: [opened, synchronize, ready_for_review, edited, closed]
-    branches:
-      - master
+on: pull_request_target
 
 jobs:
   release:
@@ -21,9 +17,9 @@ jobs:
         uses: actions/cache@v2
         with:
           path: /tmp/.buildx-cache
-          key: ${{ runner.os }}-buildx-${{ github.sha }}
+          key: ${{ runner.os }}-buildx-prs-${{ github.sha }}
           restore-keys: |
-            ${{ runner.os }}-buildx-
+            ${{ runner.os }}-buildx-prs-
 
       - name: Login to DockerHub
         uses: docker/login-action@v1

.github/workflows/publish.yml

@@ -27,8 +27,8 @@ jobs:
       - name: Create/update release
         uses: ncipollo/release-action@v1
         with:
-          tag: ${GITHUB_REF#refs/tags}
-          name: Release ${{ steps.changelog_reader.outputs.version }}
+          tag: v${{ steps.changelog_reader.outputs.version }}
+          name: Release v${{ steps.changelog_reader.outputs.version }}
           body: ${{ steps.changelog_reader.outputs.changes }}
           prerelease: ${{ steps.changelog_reader.outputs.status == 'prereleased' }}
           draft: ${{ steps.changelog_reader.outputs.status == 'unreleased' }}
@@ -83,7 +83,7 @@ jobs:
           cache-to: type=local,dest=/tmp/.buildx-cache,mode=max
 
       - name: Update Docker Hub description
-        uses: peter-evans/dockerhub-description@v2.1.0
+        uses: peter-evans/dockerhub-description@v2.4.3
         env:
           DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }}
           DOCKERHUB_PASSWORD: ${{ secrets.DOCKERHUB_PASSWORD }}

.release-it.json

@@ -0,0 +1,12 @@
+{
+  "plugins": {
+    "@release-it/keep-a-changelog": {
+      "filename": "CHANGELOG.md",
+      "addUnreleased": true,
+      "addVersionUrl": true
+    }
+  },
+  "npm": {
+    "publish": false
+  }
+}

CHANGELOG.md

@@ -9,9 +9,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Migrated to [Slash Commands](https://support.discord.com/hc/en-us/articles/1500000368501-Slash-Commands-FAQ)
 - The queue embed now automatically updates every 5 seconds (and has buttons for quick interactions)
 
+## [0.2.1] - 2021-12-18
+### Added
+- [release-it](https://www.npmjs.com/package/release-it): makes it easier to generate new tags and releases
+
+## [0.2.0]
+### Added
+- A custom track limit can now be set when queueing playlists from Spotify (default stays at 50). See #370.
+
+## [0.1.1]
+### Fixed
+- Fixes a race condition in the file cache service (see #420)
+
 ## [0.1.0]
 ### Added
 - Initial release
 
-[Unreleased]: https://github.com/codetheweb/muse/compare/v0.1.0...HEAD
+[Unreleased]: https://github.com/codetheweb/muse/compare/v0.2.1...HEAD
+[0.2.1]: https://github.com/codetheweb/muse/compare/v0.2.0...v0.2.1
+[0.2.0]: https://github.com/codetheweb/muse/releases/tag/v0.2.0
+[0.1.1]: https://github.com/codetheweb/muse/releases/tag/v0.1.1
 [0.1.0]: https://github.com/codetheweb/muse/releases/tag/v0.1.0

package.json

@@ -1,6 +1,6 @@
 {
   "name": "muse",
-  "version": "0.1.0",
+  "version": "0.2.1",
   "description": "🎧 a self-hosted Discord music bot that doesn't suck ",
   "exports": "./dist/src/index.js",
   "repository": "git@github.com:codetheweb/muse.git",
@@ -26,18 +26,19 @@
     "prepack": "npm run clean && npm run build",
     "start": "node --experimental-json-modules dist/src/index.js",
     "dev": "concurrently nodemon 'tsc --watch'",
-    "docker-publish": "docker buildx build --platform linux/amd64,linux/arm64,linux/arm/v7 -t codetheweb/muse:latest --push ."
+    "release": "release-it"
   },
   "devDependencies": {
+    "@release-it/keep-a-changelog": "^2.3.0",
     "@types/bluebird": "^3.5.35",
     "@types/debug": "^4.1.5",
     "@types/fluent-ffmpeg": "^2.1.17",
     "@types/fs-capacitor": "^2.0.0",
-    "@types/node": "^16.11.6",
+    "@types/node": "^17.0.0",
     "@types/node-emoji": "^1.8.1",
     "@types/spotify-web-api-node": "^5.0.2",
     "@types/validator": "^13.1.4",
-    "@types/ws": "^8.2.0",
+    "@types/ws": "^8.2.2",
     "@typescript-eslint/eslint-plugin": "^4.31.1",
     "@typescript-eslint/parser": "^4.31.1",
     "concurrently": "^6.4.0",
@@ -46,10 +47,11 @@
     "eslint-config-xo-typescript": "^0.44.0",
     "husky": "^4.3.8",
     "nodemon": "^2.0.7",
+    "release-it": "^14.11.8",
     "ts-node": "^10.4.0",
-    "type-fest": "^2.5.4",
+    "type-fest": "^2.8.0",
     "typed-emitter": "^1.4.0",
-    "typescript": "^4.5.3"
+    "typescript": "^4.5.4"
   },
   "eslintConfig": {
     "extends": [
@@ -64,7 +66,8 @@
       "new-cap": "off",
       "@typescript-eslint/no-unused-vars": "off",
       "@typescript-eslint/no-unused-vars-experimental": "error",
-      "@typescript-eslint/prefer-readonly-parameter-types": "off"
+      "@typescript-eslint/prefer-readonly-parameter-types": "off",
+      "@typescript-eslint/no-implicit-any-catch": "off"
     }
   },
   "husky": {
@@ -79,7 +82,7 @@
     "@discordjs/voice": "^0.7.5",
     "@types/libsodium-wrappers": "^0.7.9",
     "array-shuffle": "^3.0.0",
-    "debug": "^4.3.1",
+    "debug": "^4.3.3",
     "delay": "^5.0.0",
     "discord-api-types": "^0.25.2",
     "discord.js": "^13.3.0",
@@ -100,7 +103,7 @@
     "p-limit": "^4.0.0",
     "p-queue": "^7.1.0",
     "reflect-metadata": "^0.1.13",
-    "sequelize": "^6.11.0",
+    "sequelize": "6.11.0",
     "sequelize-typescript": "^2.1.1",
     "spotify-uri": "^2.2.0",
     "spotify-web-api-node": "^5.0.2",

src/commands/play.ts

@@ -10,6 +10,7 @@ import PlayerManager from '../managers/player.js';
 import {getMostPopularVoiceChannel, getMemberVoiceChannel} from '../utils/channels.js';
 import errorMsg from '../utils/error-msg.js';
 import GetSongs from '../services/get-songs.js';
+import Settings from '../models/settings.js';
 
 @injectable()
 export default class implements Command {
@@ -38,6 +39,9 @@ export default class implements Command {
   public async executeFromInteraction(interaction: CommandInteraction): Promise<void> {
     const [targetVoiceChannel] = getMemberVoiceChannel(interaction.member as GuildMember) ?? getMostPopularVoiceChannel(interaction.guild!);
 
+    const settings = await Settings.findByPk(interaction.guild!.id);
+    const {playlistLimit} = settings!;
+
     const player = this.playerManager.get(interaction.guild!.id);
     const wasPlayingSong = player.getCurrent() !== null;
 
@@ -98,13 +102,13 @@ export default class implements Command {
           }
         }
       } else if (url.protocol === 'spotify:' || url.host === 'open.spotify.com') {
-        const [convertedSongs, nSongsNotFound, totalSongs] = await this.getSongs.spotifySource(query);
+        const [convertedSongs, nSongsNotFound, totalSongs] = await this.getSongs.spotifySource(query, playlistLimit);
 
-        if (totalSongs > 50) {
-          extraMsg = 'a random sample of 50 songs was taken';
+        if (totalSongs > playlistLimit) {
+          extraMsg = `a random sample of ${playlistLimit} songs was taken`;
         }
 
-        if (totalSongs > 50 && nSongsNotFound !== 0) {
+        if (totalSongs > playlistLimit && nSongsNotFound !== 0) {
           extraMsg += ' and ';
         }
 

src/models/settings.ts

@@ -15,4 +15,8 @@ export default class Settings extends Model {
   @Default(false)
   @Column
   finishedSetup!: boolean;
+
+  @Default(50)
+  @Column
+  playlistLimit!: number;
 }

src/services/file-cache.ts

@@ -5,9 +5,12 @@ import sequelize from 'sequelize';
 import {FileCache} from '../models/index.js';
 import {TYPES} from '../types.js';
 import Config from './config.js';
+import PQueue from 'p-queue';
+import debug from '../utils/debug.js';
 
 @injectable()
 export default class FileCacheProvider {
+  private static readonly evictionQueue = new PQueue({concurrency: 1});
   private readonly config: Config;
 
   constructor(@inject(TYPES.Config) config: Config) {
@@ -58,10 +61,14 @@ export default class FileCacheProvider {
       const stats = await fs.stat(tmpPath);
 
       if (stats.size !== 0) {
-        await fs.rename(tmpPath, finalPath);
-      }
+        try {
+          await fs.rename(tmpPath, finalPath);
 
-      await FileCache.create({hash, bytes: stats.size, accessedAt: new Date()});
+          await FileCache.create({hash, bytes: stats.size, accessedAt: new Date()});
+        } catch (error) {
+          debug('Errored when moving a finished cache file:', error);
+        }
+      }
 
       await this.evictOldestIfNecessary();
     });
@@ -80,13 +87,19 @@ export default class FileCacheProvider {
   }
 
   private async evictOldestIfNecessary() {
-    const [{dataValues: {totalSizeBytes}}] = await FileCache.findAll({
-      attributes: [
-        [sequelize.fn('sum', sequelize.col('bytes')), 'totalSizeBytes'],
-      ],
-    }) as unknown as [{dataValues: {totalSizeBytes: number}}];
+    void FileCacheProvider.evictionQueue.add(this.evictOldest.bind(this));
 
-    if (totalSizeBytes > this.config.CACHE_LIMIT_IN_BYTES) {
+    return FileCacheProvider.evictionQueue.onEmpty();
+  }
+
+  private async evictOldest() {
+    debug('Evicting oldest files...');
+
+    let totalSizeBytes = await this.getDiskUsageInBytes();
+    let numOfEvictedFiles = 0;
+    // Continue to evict until we're under the limit
+    /* eslint-disable no-await-in-loop */
+    while (totalSizeBytes > this.config.CACHE_LIMIT_IN_BYTES) {
       const oldest = await FileCache.findOne({
         order: [
           ['accessedAt', 'ASC'],
@@ -96,22 +109,111 @@ export default class FileCacheProvider {
       if (oldest) {
         await oldest.destroy();
         await fs.unlink(path.join(this.config.CACHE_DIR, oldest.hash));
+        debug(`${oldest.hash} has been evicted`);
+        numOfEvictedFiles++;
       }
 
-      // Continue to evict until we're under the limit
-      await this.evictOldestIfNecessary();
+      totalSizeBytes = await this.getDiskUsageInBytes();
+    }
+    /* eslint-enable no-await-in-loop */
+
+    if (numOfEvictedFiles > 0) {
+      debug(`${numOfEvictedFiles} files have been evicted`);
+    } else {
+      debug(`No files needed to be evicted. Total size of the cache is currently ${totalSizeBytes} bytes, and the cache limit is ${this.config.CACHE_LIMIT_IN_BYTES} bytes.`);
     }
   }
 
   private async removeOrphans() {
+    // Check filesystem direction (do files exist on the disk but not in the database?)
     for await (const dirent of await fs.opendir(this.config.CACHE_DIR)) {
       if (dirent.isFile()) {
         const model = await FileCache.findByPk(dirent.name);
 
         if (!model) {
+          debug(`${dirent.name} was present on disk but was not in the database. Removing from disk.`);
           await fs.unlink(path.join(this.config.CACHE_DIR, dirent.name));
         }
       }
     }
+
+    // Check database direction (do entries exist in the database but not on the disk?)
+    for await (const model of this.getFindAllIterable()) {
+      const filePath = path.join(this.config.CACHE_DIR, model.hash);
+
+      try {
+        await fs.access(filePath);
+      } catch {
+        debug(`${model.hash} was present in database but was not on disk. Removing from database.`);
+        await model.destroy();
+      }
+    }
+  }
+
+  /**
+   * Pulls from the database rather than the filesystem,
+   * so may be slightly inaccurate.
+   * @returns the total size of the cache in bytes
+   */
+  private async getDiskUsageInBytes() {
+    const [{dataValues: {totalSizeBytes}}] = await FileCache.findAll({
+      attributes: [
+        [sequelize.fn('sum', sequelize.col('bytes')), 'totalSizeBytes'],
+      ],
+    }) as unknown as [{dataValues: {totalSizeBytes: number}}];
+
+    return totalSizeBytes;
+  }
+
+  /**
+   * An efficient way to iterate over all rows.
+   * @returns an iterable for the result of FileCache.findAll()
+   */
+  private getFindAllIterable() {
+    const limit = 50;
+    let previousCreatedAt: Date | null = null;
+
+    let models: FileCache[] = [];
+
+    const fetchNextBatch = async () => {
+      let where = {};
+
+      if (previousCreatedAt) {
+        where = {
+          createdAt: {
+            [sequelize.Op.gt]: previousCreatedAt,
+          },
+        };
+      }
+
+      models = await FileCache.findAll({
+        where,
+        limit,
+        order: ['createdAt'],
+      });
+
+      if (models.length > 0) {
+        previousCreatedAt = models[models.length - 1].createdAt as Date;
+      }
+    };
+
+    return {
+      [Symbol.asyncIterator]() {
+        return {
+          async next() {
+            if (models.length === 0) {
+              await fetchNextBatch();
+            }
+
+            if (models.length === 0) {
+              // Must return value here for types to be inferred correctly
+              return {done: true, value: null as unknown as FileCache};
+            }
+
+            return {value: models.shift()!, done: false};
+          },
+        };
+      },
+    };
   }
 }

src/services/get-songs.ts

@@ -188,7 +188,7 @@ export default class {
     return songsToReturn;
   }
 
-  async spotifySource(url: string): Promise<[QueuedSongWithoutChannel[], number, number]> {
+  async spotifySource(url: string, playlistLimit: number): Promise<[QueuedSongWithoutChannel[], number, number]> {
     const parsed = spotifyURI.parse(url);
 
     let tracks: SpotifyApi.TrackObjectSimplified[] = [];
@@ -252,13 +252,13 @@ export default class {
       }
     }
 
-    // Get 50 random songs if many
+    // Get random songs if the playlist is larger than limit
     const originalNSongs = tracks.length;
 
-    if (tracks.length > 50) {
+    if (tracks.length > playlistLimit) {
       const shuffled = shuffle(tracks);
 
-      tracks = shuffled.slice(0, 50);
+      tracks = shuffled.slice(0, playlistLimit);
     }
 
     let songs = await Promise.all(tracks.map(async track => this.spotifyToYouTube(track, playlist)));