Prompt to rescan "dirty" files when Music app loaded

- If the user's library contains any files potentially needing rescanning,
  this is now prompted to the user upon app loading, similarly as any new
  files to scan

- The rescanning of modified files, both with the new prompt and using
  `occ music:scan --rescan-modified`, can now also remove files from the
  library if the modified file turns out to be within an excluded folder

- When moving folder within the music library, and that folder contains more
  than 15 audio files, those files are now marked as "dirty" to trigger the
  mechanism described above. Previously, up to 30 files were rescanned
  immediately and over 30 files were removed from the library, which was
  sub-optimal.

- This completes the fix for #1173
  which was started with the commit 9c43c9c. This also provides a partial
  fix for the old issue #706.

CHANGELOG.md

@@ -24,6 +24,8 @@
 - Ampache API:
   * Action `playlist_songs` returning internal error 500 if the playlist contains any broken track references
 - Song progress shown incorrectly in the media session integration of Chrome when playing (exotic file types) with the fallback Aurora.js player
+- Track disappearing from playlists when moved to another folder within the library folder
+  [#1173](https://github.com/owncloud/music/issues/1173)
 
 ## 2.0.1 - 2024-09-08
 

appinfo/database.xml

@@ -312,6 +312,13 @@
 				<type>timestamp</type>
 				<notnull>false</notnull>
 			</field>
+			<field>
+				<name>dirty</name>
+				<type>integer</type>
+				<length>1</integer>
+				<notnull>true</notnull>
+				<default>0</default>
+			</field>
 
 			<index>
 				<name>music_tracks_artist_id_idx</name>

js/app/controllers/maincontroller.js

@@ -176,7 +176,7 @@ function ($rootScope, $scope, $timeout, $window, ArtistFactory,
 			// check the availability of unscanned files after the collection has been loaded,
 			// unless we are already in the middle of scanning (and intermediate results were just loaded)
 			if (!$scope.scanning) {
-				$scope.updateFilesToScan();
+				updateFilesToScan();
 			}
 		},
 		function(response) { // error handling
@@ -228,34 +228,30 @@ function ($rootScope, $scope, $timeout, $window, ArtistFactory,
 	$scope.updateRadio();
 	$scope.updatePodcasts();
 
-	let FILES_TO_SCAN_PER_STEP = 10;
+	const FILES_TO_SCAN_PER_STEP = 10;
 	let filesToScan = null;
-	let filesToScanIterator = 0;
-	let previouslyScannedCount = 0;
+	$scope.unscannedFiles = null;
+	$scope.dirtyFiles = null;
 
-	$scope.updateFilesToScan = function() {
+	function updateFilesToScan() {
 		$scope.checkingUnscanned = true;
 		Restangular.one('scanstate').get().then(function(state) {
 			$scope.checkingUnscanned = false;
-			previouslyScannedCount = state.scannedCount;
-			filesToScan = state.unscannedFiles;
-			filesToScanIterator = 0;
-			$scope.toScan = (filesToScan.length > 0);
-			$scope.scanningScanned = previouslyScannedCount;
-			$scope.scanningTotal = previouslyScannedCount + filesToScan.length;
-			$scope.noMusicAvailable = ($scope.scanningTotal === 0);
+			$scope.unscannedFiles = state.unscannedFiles;
+			$scope.dirtyFiles = state.dirtyFiles;
+			$scope.noMusicAvailable = (state.scannedCount + state.unscannedFiles.length === 0);
 		},
 		function(error) {
 			$scope.checkingUnscanned = false;
 			OC.Notification.showTemporary(
 					gettextCatalog.getString('Failed to check for new audio files (error {{ code }}); check the server logs for details', {code: error.status})
 			);
 		});
-	};
+	}
 
 	function processNextScanStep() {
-		let sliceEnd = filesToScanIterator + FILES_TO_SCAN_PER_STEP;
-		let filesForStep = filesToScan.slice(filesToScanIterator, sliceEnd);
+		let sliceEnd = $scope.scanningScanned + FILES_TO_SCAN_PER_STEP;
+		let filesForStep = filesToScan.slice($scope.scanningScanned, sliceEnd);
 		let params = {
 				files: filesForStep.join(','),
 				finalize: sliceEnd >= filesToScan.length
@@ -264,15 +260,13 @@ function ($rootScope, $scope, $timeout, $window, ArtistFactory,
 			// Ignore the results if scanning has been cancelled while we
 			// were waiting for the result.
 			if ($scope.scanning) {
-				filesToScanIterator = sliceEnd;
+				$scope.scanningScanned = sliceEnd;
 
 				if (result.filesScanned || result.albumCoversUpdated) {
 					$scope.updateAvailable = true;
 				}
 
-				$scope.scanningScanned = previouslyScannedCount + filesToScanIterator;
-
-				if (filesToScanIterator < filesToScan.length) {
+				if ($scope.scanningScanned < filesToScan.length) {
 					processNextScanStep();
 				} else {
 					$scope.scanning = false;
@@ -289,15 +283,16 @@ function ($rootScope, $scope, $timeout, $window, ArtistFactory,
 		});
 	}
 
-	$scope.startScanning = function(fileIds = null) {
-		if (fileIds) {
-			filesToScan = fileIds;
-			previouslyScannedCount = 0;
-			$scope.scanningScanned = 0;
-			$scope.scanningTotal = fileIds.length;
-		}
+	$scope.startScanning = function(fileIds) {
+		filesToScan = fileIds;
+		$scope.scanningScanned = 0;
+		$scope.scanningTotal = filesToScan.length;
 
-		$scope.toScan = false;
+		if (fileIds == $scope.unscannedFiles) {
+			$scope.unscannedFiles = null;
+		} else if (fileIds == $scope.dirtyFiles) {
+			$scope.dirtyFiles = null;
+		}
 		$scope.scanning = true;
 		processNextScanStep();
 	};
@@ -307,10 +302,9 @@ function ($rootScope, $scope, $timeout, $window, ArtistFactory,
 	};
 
 	$scope.resetScanned = function() {
-		$scope.toScan = false;
+		$scope.unscannedFiles = null;
+		$scope.dirtyFiles = null;
 		filesToScan = null;
-		filesToScanIterator = 0;
-		previouslyScannedCount = 0;
 		// Genre and artist IDs have got invalidated while resetting the library, drop any related filters
 		if ($scope.smartListParams !== null) {
 			$scope.smartListParams.genres = [];

lib/BusinessLayer/TrackBusinessLayer.php

@@ -118,15 +118,17 @@ public function findAllByNameArtistOrAlbum(?string $name, ?string $artistName, ?
 	}
 
 	/**
-	 * Returns all tracks where the 'modified' time in the file system (actually in the cloud's file cache)
-	 * is later than the 'updated' field of the entity in the database.
+	 * Returns all tracks of the user which should be rescanned to ensure that the library details are up-to-date.
+	 * The track may be considered "dirty" for on of two reasons:
+	 * - its 'modified' time in the file system (actually in the cloud's file cache) is later than the 'updated' field of the entity in the database
+	 * - it has been specifically marked as dirty, maybe in response to being moved to another directory
 	 * @return Track[]
 	 */
 	public function findAllDirty(string $userId) : array {
 		$tracks = $this->findAll($userId);
 		return \array_filter($tracks, function (Track $track) {
 			$dbModTime = new \DateTime($track->getUpdated());
-			return ($dbModTime->getTimestamp() < $track->getFileModTime());
+			return ($track->getDirty() || $dbModTime->getTimestamp() < $track->getFileModTime());
 		});
 	}
 
@@ -363,11 +365,12 @@ public function addOrUpdateTrack(
 		$track->setUserId($userId);
 		$track->setLength($length);
 		$track->setBitrate($bitrate);
+		$track->setDirty(0);
 		return $this->mapper->insertOrUpdate($track);
 	}
 
 	/**
-	 * Deletes a track
+	 * Deletes tracks
 	 * @param int[] $fileIds file IDs of the tracks to delete
 	 * @param string[]|null $userIds the target users; if omitted, the tracks matching the
 	 *                      $fileIds are deleted from all users
@@ -436,4 +439,19 @@ public function deleteTracks(array $fileIds, ?array $userIds=null) {
 
 		return $result;
 	}
+
+	/**
+	 * Marks tracks as dirty, ultimately requesting the user to rescan them
+	 * @param int[] $fileIds file IDs of the tracks to mark as dirty
+	 * @param string[]|null $userIds the target users; if omitted, the tracks matching the
+	 *                      $fileIds are marked for all users
+	 */
+	public function markTracksDirty(array $fileIds, ?array $userIds=null) : void {
+		// be prepared for huge number of file IDs
+		$chunkMaxSize = self::MAX_SQL_ARGS - \count($userIds ?? []);
+		$idChunks = \array_chunk($fileIds, $chunkMaxSize);
+		foreach ($idChunks as $idChunk) {
+			$this->mapper->markTracksDirty($idChunk, $userIds);
+		}
+	}
 }

lib/Command/Scan.php

@@ -76,6 +76,7 @@ protected function doConfigure() : void {
 	protected function doExecute(InputInterface $input, OutputInterface $output, array $users) : void {
 		if (!$input->getOption('debug')) {
 			$this->scanner->listen(Scanner::class, 'update', fn($path) => $output->writeln("Scanning <info>$path</info>"));
+			$this->scanner->listen(Scanner::class, 'exclude', fn($path) => $output->writeln("!! Removing <info>$path</info>"));
 		}
 
 		if ($input->getOption('rescan') && $input->getOption('rescan-modified')) {

lib/Controller/ApiController.php

@@ -196,6 +196,7 @@ public function trackByFileId(int $fileId) {
 	public function getScanState() {
 		return new JSONResponse([
 			'unscannedFiles' => $this->scanner->getUnscannedMusicFileIds($this->userId),
+			'dirtyFiles' => $this->scanner->getDirtyMusicFileIds($this->userId),
 			'scannedCount' => $this->trackBusinessLayer->count($this->userId)
 		]);
 	}

lib/Db/Track.php

@@ -51,6 +51,8 @@
  * @method void setPlayCount(int $count)
  * @method ?string getLastPlayed()
  * @method void setLastPlayed(?string $timestamp)
+ * @method int getDirty()
+ * @method void setDirty(int $dirty)
  *
  * @method string getFilename()
  * @method int getSize()
@@ -77,6 +79,7 @@ class Track extends Entity {
 	public $genreId;
 	public $playCount;
 	public $lastPlayed;
+	public $dirty;
 
 	// not from the music_tracks table but still part of the standard content of this entity:
 	public $filename;

lib/Db/TrackMapper.php

@@ -120,16 +120,12 @@ public function findAllByGenre(int $genreId, string $userId, ?int $limit=null, ?
 	}
 
 	/**
-	 * @param string $userId
 	 * @return int[]
 	 */
 	public function findAllFileIds(string $userId) : array {
 		$sql = 'SELECT `file_id` FROM `*PREFIX*music_tracks` WHERE `user_id` = ?';
 		$result = $this->execute($sql, [$userId]);
-
-		return \array_map(function ($i) {
-			return (int)$i['file_id'];
-		}, $result->fetchAll());
+		return $result->fetchAll(\PDO::FETCH_COLUMN);
 	}
 
 	/**
@@ -471,6 +467,28 @@ public function recordTrackPlayed(int $trackId, string $userId, \DateTime $timeO
 		return ($result->rowCount() > 0);
 	}
 
+	/**
+	 * Marks tracks as dirty, ultimately requesting the user to rescan them
+	 * @param int[] $fileIds file IDs of the tracks to mark as dirty
+	 * @param string[]|null $userIds the target users; if omitted, the tracks matching the
+	 *                      $fileIds are marked for all users
+	 * @return int number of rows affected
+	 */
+	public function markTracksDirty(array $fileIds, ?array $userIds=null) : int {
+		$sql = 'UPDATE `*PREFIX*music_tracks`
+				SET `dirty` = 1
+				WHERE `file_id` IN ' . $this->questionMarks(\count($fileIds));
+		$params = $fileIds;
+
+		if (!empty($userIds)) {
+			$sql .= ' AND `user_id` IN ' . $this->questionMarks(\count($userIds));
+			$params = \array_merge($params, $userIds);
+		}
+
+		$result = $this->execute($sql, $params);
+		return $result->rowCount();
+	}
+
 	/**
 	 * Overridden from the base implementation to provide support for table-specific rules
 	 *

lib/Migration/Version020100Date20241114220300.php

@@ -0,0 +1,59 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OCA\Music\Migration;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\Migration\SimpleMigrationStep;
+use OCP\Migration\IOutput;
+
+/**
+ * Migrate the DB schema to Music v2.1.0 level from the v1.9.1 level
+ */
+class Version020100Date20241114220300 extends SimpleMigrationStep {
+
+	/**
+	 * @param IOutput $output
+	 * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
+	 * @param array $options
+	 */
+	public function preSchemaChange(IOutput $output, Closure $schemaClosure, array $options) {
+	}
+
+	/**
+	 * @param IOutput $output
+	 * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
+	 * @param array $options
+	 * @return null|ISchemaWrapper
+	 */
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+		$this->addDirtyFieldToTrack($schema);
+		return $schema;
+	}
+
+	/**
+	 * @param IOutput $output
+	 * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
+	 * @param array $options
+	 */
+	public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) {
+	}
+
+	/**
+	 * Add the new field 'dirty' to the table 'music_tracks'
+	 */
+	private function addDirtyFieldToTrack(ISchemaWrapper $schema) {
+		$table = $schema->getTable('music_tracks');
+		$this->setColumn($table, 'dirty', 'smallint', ['notnull' => true, 'default' => 0]);
+	}
+
+	private function setColumn($table, string $name, string $type, array $args) : void {
+		if (!$table->hasColumn($name)) {
+			$table->addColumn($name, $type, $args);
+		}
+	}
+}

lib/Utility/Scanner.php

@@ -131,17 +131,16 @@ public function folderMoved(Folder $folder, string $userId) : void {
 			if ($this->librarySettings->pathBelongsToMusicLibrary($folder->getPath(), $userId)) {
 				// The new path of the folder belongs to the library but this doesn't necessarily mean
 				// that all the file paths below belong to the library, because of the path exclusions.
-				// Each file needs to be checked and updated separately but this may take too much time
-				// if there is extensive number of files.
-				if (\count($audioFiles) <= 30) {
+				// Each file needs to be checked and updated separately.
+				if (\count($audioFiles) <= 15) {
 					foreach ($audioFiles as $file) {
 						\assert($file instanceof File); // a clue for PHPStan
 						$this->fileMoved($file, $userId);
 					}
 				} else {
-					// Remove the scanned files to get them rescanned when the Music app is opened.
-					// TODO: Better handling e.g. by marking the files as dirty.
-					$this->deleteAudio(Util::extractIds($audioFiles), [$userId]);	
+					// There are too many files to handle them now as we don't want to delay the move operation
+					// too much. The user will be prompted to rescan the files upon opening the Music app.
+					$this->trackBusinessLayer->markTracksDirty(Util::extractIds($audioFiles), [$userId]);
 				}
 			}
 			else {
@@ -562,6 +561,10 @@ public function scanFiles(string $userId, array $fileIds, OutputInterface $debug
 		$count = 0;
 		foreach ($fileIds as $fileId) {
 			$file = $libraryRoot->getById($fileId)[0] ?? null;
+			if ($file != null && !$this->librarySettings->pathBelongsToMusicLibrary($file->getPath(), $userId)) {
+				$this->emit(self::class, 'exclude', [$file->getPath()]);
+				$file = null;
+			}
 			if ($file instanceof File) {
 				$memBefore = $debugOutput ? \memory_get_usage(true) : 0;
 				$this->updateAudio($file, $userId, $libraryRoot, $file->getPath(), $file->getMimetype(), /*partOfScan=*/true);
@@ -575,7 +578,8 @@ public function scanFiles(string $userId, array $fileIds, OutputInterface $debug
 				}
 				$count++;
 			} else {
-				$this->logger->log("File with id $fileId not found for user $userId", 'warn');
+				$this->logger->log("File with id $fileId not found for user $userId, removing it from the library if present", 'info');
+				$this->deleteAudio([$fileId], [$userId]);
 			}
 		}
 

templates/main.php

@@ -48,14 +48,22 @@
 				</div>
 			</div>
 
-			<div id="toScan" class="emptycontent clickable" ng-show="toScan && viewingLibrary()" ng-click="startScanning()">
+			<div id="toScan" class="emptycontent clickable" ng-show="!scanning && unscannedFiles.length && viewingLibrary()" ng-click="startScanning(unscannedFiles)">
 				<div class="icon-audio svg"></div>
 				<div>
 					<h2 translate>New music available</h2>
 					<p translate>Click here to start the scan</p>
 				</div>
 			</div>
 
+			<div id="toRescan" class="emptycontent clickable" ng-show="!scanning && !unscannedFiles.length && dirtyFiles.length && viewingLibrary()" ng-click="startScanning(dirtyFiles)">
+				<div class="icon-audio svg"></div>
+				<div>
+					<h2 translate>Some of the previously scanned files may have changed</h2>
+					<p translate>Click here to rescan these files</p>
+				</div>
+			</div>
+
 			<div id="scanning" class="emptycontent" ng-show="scanning && viewingLibrary()">
 				<div class="icon-loading svg"></div>
 				<div>