From 9292b057329f419ceb985d31b305a1245134e823 Mon Sep 17 00:00:00 2001 From: emeric Date: Fri, 12 Jul 2024 18:52:23 +0200 Subject: [PATCH 01/11] Remove files from db if they are no longer be able to be scanned (due to file corruption or third party lib support) --- .../scanner/impl/ScanStepScanFiles.cpp | 140 +++++++++--------- .../scanner/impl/ScanStepScanFiles.hpp | 4 +- 2 files changed, 76 insertions(+), 68 deletions(-) diff --git a/src/libs/services/scanner/impl/ScanStepScanFiles.cpp b/src/libs/services/scanner/impl/ScanStepScanFiles.cpp index df807ad5..ffaa9c51 100644 --- a/src/libs/services/scanner/impl/ScanStepScanFiles.cpp +++ b/src/libs/services/scanner/impl/ScanStepScanFiles.cpp @@ -497,32 +497,18 @@ namespace lms::scanner if (const AudioFileScanData * scanData{ std::get_if(&scanResult.scanData) }) { - if (metadata::Track * track{ scanData->get() }) - { - context.stats.scans++; - processAudioFileScanData(context, scanResult.path, *track, libraryInfo); - } - else - { - context.stats.errors.emplace_back(scanResult.path, ScanErrorType::CannotReadAudioFile); - } + context.stats.scans++; + processAudioFileScanData(context, scanResult.path, scanData->get(), libraryInfo); } else if (const ImageFileScanData * scanData{ std::get_if(&scanResult.scanData) }) { - if (scanData->has_value()) - { - context.stats.scans++; - processImageFileScanData(context, scanResult.path, scanData->value(), libraryInfo); - } - else - { - context.stats.errors.emplace_back(scanResult.path, ScanErrorType::CannotReadImageFile); - } + context.stats.scans++; + processImageFileScanData(context, scanResult.path, scanData->has_value() ? &scanData->value() : nullptr, libraryInfo); } } } - void ScanStepScanFiles::processAudioFileScanData(ScanContext& context, const std::filesystem::path& file, const metadata::Track& trackMetadata, const ScannerSettings::MediaLibraryInfo& libraryInfo) + void ScanStepScanFiles::processAudioFileScanData(ScanContext& context, const std::filesystem::path& file, const metadata::Track* trackMetadata, const ScannerSettings::MediaLibraryInfo& libraryInfo) { LMS_SCOPED_TRACE_DETAILED("Scanner", "ProcessAudioScanData"); @@ -538,9 +524,20 @@ namespace lms::scanner db::Session& dbSession{ _db.getTLSSession() }; Track::pointer track{ Track::findByPath(dbSession, file) }; - if (trackMetadata.mbid && (!track || _settings.skipDuplicateMBID)) + if (!trackMetadata) { - std::vector duplicateTracks{ Track::findByMBID(dbSession, *trackMetadata.mbid) }; + if (track) + { + track.remove(); + stats.deletions++; + } + context.stats.errors.emplace_back(file, ScanErrorType::CannotReadAudioFile); + return; + } + + if (trackMetadata->mbid && (!track || _settings.skipDuplicateMBID)) + { + std::vector duplicateTracks{ Track::findByMBID(dbSession, *trackMetadata->mbid) }; // find for an existing track MBID as the file may have just been moved if (!track && duplicateTracks.size() == 1) @@ -586,7 +583,7 @@ namespace lms::scanner } // We estimate this is an audio file if the duration is not null - if (trackMetadata.audioProperties.duration == std::chrono::milliseconds::zero()) + if (trackMetadata->audioProperties.duration == std::chrono::milliseconds::zero()) { LMS_LOG(DBUPDATER, DEBUG, "Skipped '" << file.string() << "' (duration is 0)"); @@ -596,14 +593,14 @@ namespace lms::scanner track.remove(); stats.deletions++; } - stats.errors.emplace_back(ScanError{ file, ScanErrorType::BadDuration }); + stats.errors.emplace_back(file, ScanErrorType::BadDuration); return; } // ***** Title std::string title; - if (!trackMetadata.title.empty()) - title = trackMetadata.title; + if (!trackMetadata->title.empty()) + title = trackMetadata->title; else { // TODO parse file name guess track etc. @@ -625,11 +622,11 @@ namespace lms::scanner assert(track); // Audio properties - track.modify()->setBitrate(trackMetadata.audioProperties.bitrate); - track.modify()->setBitsPerSample(trackMetadata.audioProperties.bitsPerSample); - track.modify()->setChannelCount(trackMetadata.audioProperties.channelCount); - track.modify()->setDuration(trackMetadata.audioProperties.duration); - track.modify()->setSampleRate(trackMetadata.audioProperties.sampleRate); + track.modify()->setBitrate(trackMetadata->audioProperties.bitrate); + track.modify()->setBitsPerSample(trackMetadata->audioProperties.bitsPerSample); + track.modify()->setChannelCount(trackMetadata->audioProperties.channelCount); + track.modify()->setDuration(trackMetadata->audioProperties.duration); + track.modify()->setSampleRate(trackMetadata->audioProperties.sampleRate); track.modify()->setRelativeFilePath(fileInfo->relativePath); track.modify()->setFileSize(fileInfo->fileSize); @@ -640,76 +637,76 @@ namespace lms::scanner track.modify()->clearArtistLinks(); // Do not fallback on artists with the same name but having a MBID for artist and releaseArtists, as it may be corrected by properly tagging files - for (const Artist::pointer& artist : getOrCreateArtists(dbSession, trackMetadata.artists, false)) + for (const Artist::pointer& artist : getOrCreateArtists(dbSession, trackMetadata->artists, false)) track.modify()->addArtistLink(TrackArtistLink::create(dbSession, track, artist, TrackArtistLinkType::Artist)); - if (trackMetadata.medium && trackMetadata.medium->release) + if (trackMetadata->medium && trackMetadata->medium->release) { - for (const Artist::pointer& releaseArtist : getOrCreateArtists(dbSession, trackMetadata.medium->release->artists, false)) + for (const Artist::pointer& releaseArtist : getOrCreateArtists(dbSession, trackMetadata->medium->release->artists, false)) track.modify()->addArtistLink(TrackArtistLink::create(dbSession, track, releaseArtist, TrackArtistLinkType::ReleaseArtist)); } // Allow fallbacks on artists with the same name even if they have MBID, since there is no tag to indicate the MBID of these artists // We could ask MusicBrainz to get all the information, but that would heavily slow down the import process - for (const Artist::pointer& conductor : getOrCreateArtists(dbSession, trackMetadata.conductorArtists, true)) + for (const Artist::pointer& conductor : getOrCreateArtists(dbSession, trackMetadata->conductorArtists, true)) track.modify()->addArtistLink(TrackArtistLink::create(dbSession, track, conductor, TrackArtistLinkType::Conductor)); - for (const Artist::pointer& composer : getOrCreateArtists(dbSession, trackMetadata.composerArtists, true)) + for (const Artist::pointer& composer : getOrCreateArtists(dbSession, trackMetadata->composerArtists, true)) track.modify()->addArtistLink(TrackArtistLink::create(dbSession, track, composer, TrackArtistLinkType::Composer)); - for (const Artist::pointer& lyricist : getOrCreateArtists(dbSession, trackMetadata.lyricistArtists, true)) + for (const Artist::pointer& lyricist : getOrCreateArtists(dbSession, trackMetadata->lyricistArtists, true)) track.modify()->addArtistLink(TrackArtistLink::create(dbSession, track, lyricist, TrackArtistLinkType::Lyricist)); - for (const Artist::pointer& mixer : getOrCreateArtists(dbSession, trackMetadata.mixerArtists, true)) + for (const Artist::pointer& mixer : getOrCreateArtists(dbSession, trackMetadata->mixerArtists, true)) track.modify()->addArtistLink(TrackArtistLink::create(dbSession, track, mixer, TrackArtistLinkType::Mixer)); - for (const auto& [role, performers] : trackMetadata.performerArtists) + for (const auto& [role, performers] : trackMetadata->performerArtists) { for (const Artist::pointer& performer : getOrCreateArtists(dbSession, performers, true)) track.modify()->addArtistLink(TrackArtistLink::create(dbSession, track, performer, TrackArtistLinkType::Performer, role)); } - for (const Artist::pointer& producer : getOrCreateArtists(dbSession, trackMetadata.producerArtists, true)) + for (const Artist::pointer& producer : getOrCreateArtists(dbSession, trackMetadata->producerArtists, true)) track.modify()->addArtistLink(TrackArtistLink::create(dbSession, track, producer, TrackArtistLinkType::Producer)); - for (const Artist::pointer& remixer : getOrCreateArtists(dbSession, trackMetadata.remixerArtists, true)) + for (const Artist::pointer& remixer : getOrCreateArtists(dbSession, trackMetadata->remixerArtists, true)) track.modify()->addArtistLink(TrackArtistLink::create(dbSession, track, remixer, TrackArtistLinkType::Remixer)); track.modify()->setScanVersion(_settings.scanVersion); - if (trackMetadata.medium && trackMetadata.medium->release) - track.modify()->setRelease(getOrCreateRelease(dbSession, *trackMetadata.medium->release, file.parent_path())); + if (trackMetadata->medium && trackMetadata->medium->release) + track.modify()->setRelease(getOrCreateRelease(dbSession, *trackMetadata->medium->release, file.parent_path())); else track.modify()->setRelease({}); - track.modify()->setTotalTrack(trackMetadata.medium ? trackMetadata.medium->trackCount : std::nullopt); - track.modify()->setReleaseReplayGain(trackMetadata.medium ? trackMetadata.medium->replayGain : std::nullopt); - track.modify()->setDiscSubtitle(trackMetadata.medium ? trackMetadata.medium->name : ""); - track.modify()->setClusters(getOrCreateClusters(dbSession, trackMetadata)); + track.modify()->setTotalTrack(trackMetadata->medium ? trackMetadata->medium->trackCount : std::nullopt); + track.modify()->setReleaseReplayGain(trackMetadata->medium ? trackMetadata->medium->replayGain : std::nullopt); + track.modify()->setDiscSubtitle(trackMetadata->medium ? trackMetadata->medium->name : ""); + track.modify()->setClusters(getOrCreateClusters(dbSession, *trackMetadata)); track.modify()->setName(title); track.modify()->setAddedTime(Wt::WDateTime::currentDateTime()); - track.modify()->setTrackNumber(trackMetadata.position); - track.modify()->setDiscNumber(trackMetadata.medium ? trackMetadata.medium->position : std::nullopt); - track.modify()->setDate(trackMetadata.date); - track.modify()->setYear(trackMetadata.year); - track.modify()->setOriginalDate(trackMetadata.originalDate); - track.modify()->setOriginalYear(trackMetadata.originalYear); + track.modify()->setTrackNumber(trackMetadata->position); + track.modify()->setDiscNumber(trackMetadata->medium ? trackMetadata->medium->position : std::nullopt); + track.modify()->setDate(trackMetadata->date); + track.modify()->setYear(trackMetadata->year); + track.modify()->setOriginalDate(trackMetadata->originalDate); + track.modify()->setOriginalYear(trackMetadata->originalYear); // If a file has an OriginalDate but no date, set it to ease filtering - if (!trackMetadata.date.isValid() && trackMetadata.originalDate.isValid()) - track.modify()->setDate(trackMetadata.originalDate); + if (!trackMetadata->date.isValid() && trackMetadata->originalDate.isValid()) + track.modify()->setDate(trackMetadata->originalDate); // If a file has an OriginalYear but no Year, set it to ease filtering - if (!trackMetadata.year && trackMetadata.originalYear) - track.modify()->setYear(trackMetadata.originalYear); + if (!trackMetadata->year && trackMetadata->originalYear) + track.modify()->setYear(trackMetadata->originalYear); - track.modify()->setRecordingMBID(trackMetadata.recordingMBID); - track.modify()->setTrackMBID(trackMetadata.mbid); + track.modify()->setRecordingMBID(trackMetadata->recordingMBID); + track.modify()->setTrackMBID(trackMetadata->mbid); if (auto trackFeatures{ TrackFeatures::find(dbSession, track->getId()) }) trackFeatures.remove(); // TODO: only if MBID changed? - track.modify()->setHasCover(trackMetadata.hasCover); - track.modify()->setCopyright(trackMetadata.copyright); - track.modify()->setCopyrightURL(trackMetadata.copyrightURL); - track.modify()->setTrackReplayGain(trackMetadata.replayGain); - track.modify()->setArtistDisplayName(trackMetadata.artistDisplayName); + track.modify()->setHasCover(trackMetadata->hasCover); + track.modify()->setCopyright(trackMetadata->copyright); + track.modify()->setCopyrightURL(trackMetadata->copyrightURL); + track.modify()->setTrackReplayGain(trackMetadata->replayGain); + track.modify()->setArtistDisplayName(trackMetadata->artistDisplayName); if (added) { @@ -723,7 +720,7 @@ namespace lms::scanner } } - void ScanStepScanFiles::processImageFileScanData(ScanContext& context, const std::filesystem::path& file, const ImageInfo& imageInfo, const ScannerSettings::MediaLibraryInfo& libraryInfo) + void ScanStepScanFiles::processImageFileScanData(ScanContext& context, const std::filesystem::path& file, const ImageInfo* imageInfo, const ScannerSettings::MediaLibraryInfo& libraryInfo) { LMS_SCOPED_TRACE_DETAILED("Scanner", "ProcessImageScanData"); @@ -739,6 +736,17 @@ namespace lms::scanner db::Session& dbSession{ _db.getTLSSession() }; db::Image::pointer image{ db::Image::find(dbSession, file) }; + if (!imageInfo) + { + if (image) + { + image.remove(); + stats.deletions++; + } + context.stats.errors.emplace_back(file, ScanErrorType::CannotReadImageFile); + return; + } + bool added; if (!image) { @@ -752,8 +760,8 @@ namespace lms::scanner image.modify()->setLastWriteTime(fileInfo->lastWriteTime); image.modify()->setFileSize(fileInfo->fileSize); - image.modify()->setHeight(imageInfo.height); - image.modify()->setWidth(imageInfo.width); + image.modify()->setHeight(imageInfo->height); + image.modify()->setWidth(imageInfo->width); image.modify()->setDirectory(getOrCreateDirectory(dbSession, file.parent_path(), libraryInfo.rootDirectory)); if (added) diff --git a/src/libs/services/scanner/impl/ScanStepScanFiles.hpp b/src/libs/services/scanner/impl/ScanStepScanFiles.hpp index 54d5ac66..fd9687d9 100644 --- a/src/libs/services/scanner/impl/ScanStepScanFiles.hpp +++ b/src/libs/services/scanner/impl/ScanStepScanFiles.hpp @@ -46,8 +46,8 @@ namespace lms::scanner bool checkImageFileNeedScan(ScanContext& context, const std::filesystem::path& file); void processFileScanResults(ScanContext& context, std::span scanResults, const ScannerSettings::MediaLibraryInfo& libraryInfo); - void processAudioFileScanData(ScanContext& context, const std::filesystem::path& path, const metadata::Track& trackMetadata, const ScannerSettings::MediaLibraryInfo& libraryInfo); - void processImageFileScanData(ScanContext& context, const std::filesystem::path& path, const ImageInfo& imageInfo, const ScannerSettings::MediaLibraryInfo& libraryInfo); + void processAudioFileScanData(ScanContext& context, const std::filesystem::path& path, const metadata::Track* trackMetadata, const ScannerSettings::MediaLibraryInfo& libraryInfo); + void processImageFileScanData(ScanContext& context, const std::filesystem::path& path, const ImageInfo* imageInfo, const ScannerSettings::MediaLibraryInfo& libraryInfo); std::unique_ptr _metadataParser; const std::vector _extraTagsToParse; From 980709dbcf6770092cf4042d146eefa226d5b26c Mon Sep 17 00:00:00 2001 From: emeric Date: Sat, 13 Jul 2024 14:22:19 +0200 Subject: [PATCH 02/11] Can now browse by directorie in the Subsonic API, ref #474 --- src/libs/database/impl/Directory.cpp | 38 +- src/libs/database/impl/Release.cpp | 6 +- src/libs/database/impl/Track.cpp | 3 + .../database/include/database/Directory.hpp | 34 +- .../database/include/database/Release.hpp | 7 + src/libs/database/include/database/Track.hpp | 7 + src/libs/database/test/Directory.cpp | 108 ++++- src/libs/subsonic/impl/SubsonicId.cpp | 21 + src/libs/subsonic/impl/SubsonicId.hpp | 5 + src/libs/subsonic/impl/SubsonicResource.cpp | 2 +- .../impl/entrypoints/AlbumSongLists.cpp | 8 +- .../subsonic/impl/entrypoints/Browsing.cpp | 406 +++++++++++------- .../impl/entrypoints/MediaAnnotation.cpp | 35 +- .../subsonic/impl/entrypoints/Searching.cpp | 57 ++- src/libs/subsonic/impl/responses/Album.cpp | 55 +-- src/libs/subsonic/impl/responses/Album.hpp | 3 +- src/libs/subsonic/impl/responses/Artist.cpp | 14 +- src/libs/subsonic/impl/responses/Artist.hpp | 3 +- src/libs/subsonic/impl/responses/Song.cpp | 23 +- src/libs/subsonic/impl/responses/Song.hpp | 2 +- 20 files changed, 613 insertions(+), 224 deletions(-) diff --git a/src/libs/database/impl/Directory.cpp b/src/libs/database/impl/Directory.cpp index 8b9ba07a..0b20237f 100644 --- a/src/libs/database/impl/Directory.cpp +++ b/src/libs/database/impl/Directory.cpp @@ -33,10 +33,25 @@ namespace lms::db { auto query{ session.getDboSession()->query>("SELECT d FROM directory d") }; + for (std::string_view keyword : params.keywords) + query.where("d.name LIKE ? ESCAPE '" ESCAPE_CHAR_STR "'").bind("%" + utils::escapeLikeKeyword(keyword) + "%"); + + if (params.artist.isValid() + || params.release.isValid()) + { + query.join("track t ON t.directory_id = d.id"); + query.groupBy("d.id"); + } + + if (params.parentDirectory.isValid()) + query.where("d.parent_directory_id = ?").bind(params.parentDirectory); + + if (params.release.isValid()) + query.where("t.release_id = ?").bind(params.release); + if (params.artist.isValid()) { - query.join("track t ON t.directory_id = d.id") - .join("artist a ON a.id = t_a_l.artist_id") + query.join("artist a ON a.id = t_a_l.artist_id") .join("track_artist_link t_a_l ON t_a_l.track_id = t.id") .where("a.id = ?") .bind(params.artist); @@ -57,10 +72,11 @@ namespace lms::db } query.where(oss.str()); } - - query.groupBy("d.id"); } + if (params.withNoTrack) + query.where("NOT EXISTS (SELECT 1 FROM track t WHERE t.directory_id = d.id)"); + return query; } } // namespace @@ -108,10 +124,16 @@ namespace lms::db }); } + RangeResults Directory::find(Session& session, const FindParameters& params) + { + auto query{ createQuery(session, params) }; + return utils::execRangeQuery(query, params.range); + } + void Directory::find(Session& session, const FindParameters& params, const std::function& func) { auto query{ createQuery(session, params) }; - utils::forEachQueryResult(query, [&func](const Directory::pointer& dir) { + utils::forEachQueryRangeResult(query, params.range, [&func](const Directory::pointer& dir) { func(dir); }); } @@ -131,6 +153,12 @@ namespace lms::db return utils::execRangeQuery(query, range); } + RangeResults Directory::findRootDirectories(Session& session, std::optional range) + { + auto query{ session.getDboSession()->query>("SELECT d from directory d").where("d.parent_directory_id IS NULL") }; + return utils::execRangeQuery(query, range); + } + void Directory::setAbsolutePath(const std::filesystem::path& p) { assert(p.is_absolute()); diff --git a/src/libs/database/impl/Release.cpp b/src/libs/database/impl/Release.cpp index b7322ff4..a597de86 100644 --- a/src/libs/database/impl/Release.cpp +++ b/src/libs/database/impl/Release.cpp @@ -53,7 +53,8 @@ namespace lms::db || params.dateRange || params.artist.isValid() || params.clusters.size() == 1 - || params.mediaLibrary.isValid()) + || params.mediaLibrary.isValid() + || params.directory.isValid()) { query.join("track t ON t.release_id = r.id"); } @@ -61,6 +62,9 @@ namespace lms::db if (params.mediaLibrary.isValid()) query.where("t.media_library_id = ?").bind(params.mediaLibrary); + if (params.directory.isValid()) + query.where("t.directory_id = ?").bind(params.directory); + if (!params.releaseType.empty()) { query.join("release_release_type r_r_t ON r_r_t.release_id = r.id"); diff --git a/src/libs/database/impl/Track.cpp b/src/libs/database/impl/Track.cpp index 80c10c18..a8652d3e 100644 --- a/src/libs/database/impl/Track.cpp +++ b/src/libs/database/impl/Track.cpp @@ -154,6 +154,9 @@ namespace lms::db if (params.mediaLibrary.isValid()) query.where("t.media_library_id = ?").bind(params.mediaLibrary); + if (params.directory.isValid()) + query.where("t.directory_id = ?").bind(params.directory); + switch (params.sortMethod) { case TrackSortMethod::None: diff --git a/src/libs/database/include/database/Directory.hpp b/src/libs/database/include/database/Directory.hpp index 51564cf6..7cecbc69 100644 --- a/src/libs/database/include/database/Directory.hpp +++ b/src/libs/database/include/database/Directory.hpp @@ -21,12 +21,16 @@ #include #include +#include +#include +#include #include #include "core/EnumSet.hpp" #include "database/ArtistId.hpp" #include "database/DirectoryId.hpp" +#include "database/ReleaseId.hpp" #include "database/Object.hpp" #include "database/Types.hpp" @@ -42,20 +46,44 @@ namespace lms::db struct FindParameters { std::optional range; - ArtistId artist; // only tracks that involve this artist + std::vector keywords; // if non empty, name must match all of these keywords + ArtistId artist; // only directory that involve this artist + ReleaseId release; // only releases that involve this artist core::EnumSet trackArtistLinkTypes; // and for these link types + DirectoryId parentDirectory; // If set, directories that have this parent + bool withNoTrack{}; // If set, directories that do not contain any track FindParameters& setRange(std::optional _range) { range = _range; return *this; } + FindParameters& setKeywords(const std::vector& _keywords) + { + keywords = _keywords; + return *this; + } FindParameters& setArtist(ArtistId _artist, core::EnumSet _trackArtistLinkTypes = {}) { artist = _artist; trackArtistLinkTypes = _trackArtistLinkTypes; return *this; } + FindParameters& setRelease(ReleaseId _release) + { + release = _release; + return *this; + } + FindParameters& setParentDirectory(DirectoryId _parentDirectory) + { + parentDirectory = _parentDirectory; + return *this; + } + FindParameters& setWithNoTrack(bool _withNoTrack) + { + withNoTrack = _withNoTrack; + return *this; + } }; // find @@ -63,13 +91,15 @@ namespace lms::db static pointer find(Session& session, DirectoryId id); static pointer find(Session& session, const std::filesystem::path& path); static void find(Session& session, DirectoryId& lastRetrievedDirectory, std::size_t count, const std::function& func); + static RangeResults find(Session& session, const FindParameters& params); static void find(Session& session, const FindParameters& parameters, const std::function& func); static RangeResults findOrphanIds(Session& session, std::optional range = std::nullopt); + static RangeResults findRootDirectories(Session& session, std::optional range = std::nullopt); // getters const std::filesystem::path& getAbsolutePath() const { return _absolutePath; } std::string_view getName() const { return _name; } - ObjectPtr getParent() const { return _parent; } + ObjectPtr getParentDirectory() const { return _parent; } // setters void setAbsolutePath(const std::filesystem::path& p); diff --git a/src/libs/database/include/database/Release.hpp b/src/libs/database/include/database/Release.hpp index 7af4e2bc..371df127 100644 --- a/src/libs/database/include/database/Release.hpp +++ b/src/libs/database/include/database/Release.hpp @@ -33,6 +33,7 @@ #include "core/UUID.hpp" #include "database/ArtistId.hpp" #include "database/ClusterId.hpp" +#include "database/DirectoryId.hpp" #include "database/MediaLibraryId.hpp" #include "database/Object.hpp" #include "database/ReleaseId.hpp" @@ -96,6 +97,7 @@ namespace lms::db core::EnumSet excludedTrackArtistLinkTypes; // but not for these link types std::string releaseType; // If set, albums that has this release type MediaLibraryId mediaLibrary; // If set, releases that has at least a track in this library + DirectoryId directory; // if set, tracks in this directory FindParameters& setClusters(std::span _clusters) { @@ -150,6 +152,11 @@ namespace lms::db mediaLibrary = _mediaLibrary; return *this; } + FindParameters& setDirectory(DirectoryId _directory) + { + directory = _directory; + return *this; + } }; Release() = default; diff --git a/src/libs/database/include/database/Track.hpp b/src/libs/database/include/database/Track.hpp index 19829e66..f06d9659 100644 --- a/src/libs/database/include/database/Track.hpp +++ b/src/libs/database/include/database/Track.hpp @@ -37,6 +37,7 @@ #include "core/UUID.hpp" #include "database/ArtistId.hpp" #include "database/ClusterId.hpp" +#include "database/DirectoryId.hpp" #include "database/MediaLibraryId.hpp" #include "database/Object.hpp" #include "database/ReleaseId.hpp" @@ -81,6 +82,7 @@ namespace lms::db std::optional trackNumber; // matching this track number std::optional discNumber; // matching this disc number MediaLibraryId mediaLibrary; // If set, tracks in this library + DirectoryId directory; // if set, tracks in this directory FindParameters& setClusters(std::span _clusters) { @@ -165,6 +167,11 @@ namespace lms::db mediaLibrary = _mediaLibrary; return *this; } + FindParameters& setDirectory(DirectoryId _directory) + { + directory = _directory; + return *this; + } }; struct PathResult diff --git a/src/libs/database/test/Directory.cpp b/src/libs/database/test/Directory.cpp index da7152ed..9e3be53f 100644 --- a/src/libs/database/test/Directory.cpp +++ b/src/libs/database/test/Directory.cpp @@ -107,23 +107,35 @@ namespace lms::db::tests { auto transaction{ session.createReadTransaction() }; - auto dir{ child->getParent() }; + auto dir{ child->getParentDirectory() }; EXPECT_EQ(dir, Directory::pointer{}); } { auto transaction{ session.createWriteTransaction() }; - child.get().modify()->setParent(parent.lockAndGet()); + child.get().modify()->setParent(parent.get()); } { auto transaction{ session.createReadTransaction() }; - auto dir{ child->getParent() }; + auto dir{ child->getParentDirectory() }; ASSERT_NE(dir, Directory::pointer{}); EXPECT_EQ(dir->getId(), parent.getId()); } + + { + auto transaction{ session.createReadTransaction() }; + + Directory::pointer foundDir; + Directory::find(session, Directory::FindParameters{}.setParentDirectory(parent->getId()), [&](const Directory::pointer& dir) { + ASSERT_EQ(foundDir, Directory::pointer{}); + foundDir = dir; + }); + ASSERT_NE(foundDir, Directory::pointer{}); + EXPECT_EQ(foundDir->getId(), child.getId()); + } } TEST_F(DatabaseFixture, Directory_orphaned) @@ -141,7 +153,7 @@ namespace lms::db::tests { auto transaction{ session.createWriteTransaction() }; - child.get().modify()->setParent(parent.lockAndGet()); + child.get().modify()->setParent(parent.get()); } { @@ -152,4 +164,92 @@ namespace lms::db::tests EXPECT_EQ(directories.front(), child.getId()); } } + + TEST_F(DatabaseFixture, Directory_findRootDirectories) + { + ScopedDirectory parent1{ session, "/root1" }; + ScopedDirectory child{ session, "/root1/child" }; + ScopedDirectory parent2{ session, "/root2" }; + + { + auto transaction{ session.createWriteTransaction() }; + + child.get().modify()->setParent(parent1.get()); + } + + { + auto transaction{ session.createReadTransaction() }; + + const auto directories{ Directory::findRootDirectories(session).results }; + ASSERT_EQ(directories.size(), 2); + EXPECT_EQ(directories[0]->getId(), parent1.getId()); + EXPECT_EQ(directories[1]->getId(), parent2.getId()); + } + } + + TEST_F(DatabaseFixture, Directory_findNonTrackDirectories) + { + ScopedDirectory parent{ session, "/root" }; + ScopedDirectory child1{ session, "/root/child1" }; + ScopedDirectory child2{ session, "/root/child2" }; + ScopedTrack track{ session }; + + { + auto transaction{ session.createWriteTransaction() }; + + child1.get().modify()->setParent(parent.get()); + child2.get().modify()->setParent(parent.get()); + } + + { + auto transaction{ session.createReadTransaction() }; + + Directory::FindParameters params; + params.setWithNoTrack(true); + + auto res{ Directory::find(session, params).results }; + + ASSERT_EQ(res.size(), 3); + EXPECT_EQ(res[0]->getId(), parent.getId()); + EXPECT_EQ(res[1]->getId(), child1.getId()); + EXPECT_EQ(res[2]->getId(), child2.getId()); + } + + { + auto transaction{ session.createWriteTransaction() }; + track.get().modify()->setDirectory(child2.get()); + } + + { + auto transaction{ session.createReadTransaction() }; + + Directory::FindParameters params; + params.setWithNoTrack(true); + + auto res{ Directory::find(session, params).results }; + ASSERT_EQ(res.size(), 2); + EXPECT_EQ(res[0]->getId(), parent.getId()); + EXPECT_EQ(res[1]->getId(), child1.getId()); + } + } + + TEST_F(DatabaseFixture, Directory_findWithKeywords) + { + ScopedDirectory parent{ session, "/root" }; + ScopedDirectory child1{ session, "/root/foo" }; + ScopedDirectory child2{ session, "/root/bar/foo" }; + + { + auto transaction{ session.createReadTransaction() }; + + Directory::FindParameters params; + params.setKeywords({ "foo" }); + + auto res{ Directory::find(session, params).results }; + + ASSERT_EQ(res.size(), 2); + EXPECT_EQ(res[0]->getId(), child1.getId()); + EXPECT_EQ(res[1]->getId(), child2.getId()); + } + } } // namespace lms::db::tests \ No newline at end of file diff --git a/src/libs/subsonic/impl/SubsonicId.cpp b/src/libs/subsonic/impl/SubsonicId.cpp index 9ec69508..b1fda5af 100644 --- a/src/libs/subsonic/impl/SubsonicId.cpp +++ b/src/libs/subsonic/impl/SubsonicId.cpp @@ -31,6 +31,11 @@ namespace lms::api::subsonic return "ar-" + id.toString(); } + std::string idToString(db::DirectoryId id) + { + return "dir-" + id.toString(); + } + std::string idToString(db::MediaLibraryId id) { // No need to prefix as this is only used at well known places @@ -76,6 +81,22 @@ namespace lms::core::stringUtils return std::nullopt; } + template<> + std::optional readAs(std::string_view str) + { + std::vector values{ core::stringUtils::splitString(str, '-') }; + if (values.size() != 2) + return std::nullopt; + + if (values[0] != "dir") + return std::nullopt; + + if (const auto value{ core::stringUtils::readAs(values[1]) }) + return db::DirectoryId{ *value }; + + return std::nullopt; + } + template<> std::optional readAs(std::string_view str) { diff --git a/src/libs/subsonic/impl/SubsonicId.hpp b/src/libs/subsonic/impl/SubsonicId.hpp index d7a88efa..331708a5 100644 --- a/src/libs/subsonic/impl/SubsonicId.hpp +++ b/src/libs/subsonic/impl/SubsonicId.hpp @@ -21,6 +21,7 @@ #include "core/String.hpp" #include "database/ArtistId.hpp" +#include "database/DirectoryId.hpp" #include "database/MediaLibraryId.hpp" #include "database/ReleaseId.hpp" #include "database/TrackId.hpp" @@ -33,6 +34,7 @@ namespace lms::api::subsonic }; std::string idToString(db::ArtistId id); + std::string idToString(db::DirectoryId id); std::string idToString(db::MediaLibraryId id); std::string idToString(db::ReleaseId id); std::string idToString(db::TrackId id); @@ -49,6 +51,9 @@ namespace lms::core::stringUtils template<> std::optional readAs(std::string_view str); + template<> + std::optional readAs(std::string_view str); + template<> std::optional readAs(std::string_view str); diff --git a/src/libs/subsonic/impl/SubsonicResource.cpp b/src/libs/subsonic/impl/SubsonicResource.cpp index 311c420f..6bf949f2 100644 --- a/src/libs/subsonic/impl/SubsonicResource.cpp +++ b/src/libs/subsonic/impl/SubsonicResource.cpp @@ -168,7 +168,7 @@ namespace lms::api::subsonic { "/getAlbum", { handleGetAlbumRequest } }, { "/getSong", { handleGetSongRequest } }, { "/getVideos", { handleNotImplemented } }, - { "/getArtistInfo", { handleGetArtistInfoRequest } }, + { "/getArtistInfo", { handleNotImplemented } }, { "/getArtistInfo2", { handleGetArtistInfo2Request } }, { "/getAlbumInfo", { handleNotImplemented } }, { "/getAlbumInfo2", { handleNotImplemented } }, diff --git a/src/libs/subsonic/impl/entrypoints/AlbumSongLists.cpp b/src/libs/subsonic/impl/entrypoints/AlbumSongLists.cpp index 59d6966c..916086be 100644 --- a/src/libs/subsonic/impl/entrypoints/AlbumSongLists.cpp +++ b/src/libs/subsonic/impl/entrypoints/AlbumSongLists.cpp @@ -169,7 +169,7 @@ namespace lms::api::subsonic for (const ReleaseId releaseId : releases.results) { const Release::pointer release{ Release::find(context.dbSession, releaseId) }; - albumListNode.addArrayChild("album", createAlbumNode(context, release, context.user, id3)); + albumListNode.addArrayChild("album", createAlbumNode(context, release, id3)); } return response; @@ -187,6 +187,8 @@ namespace lms::api::subsonic feedback::IFeedbackService& feedbackService{ *core::Service::get() }; + // We don't support starring directories + if (id3) { feedback::IFeedbackService::ArtistFindParameters artistFindParams; artistFindParams.setUser(context.user->getId()); @@ -194,7 +196,7 @@ namespace lms::api::subsonic for (const ArtistId artistId : feedbackService.findStarredArtists(artistFindParams).results) { if (auto artist{ Artist::find(context.dbSession, artistId) }) - starredNode.addArrayChild("artist", createArtistNode(context, artist, context.user, id3)); + starredNode.addArrayChild("artist", createArtistNode(context, artist)); } } @@ -205,7 +207,7 @@ namespace lms::api::subsonic for (const ReleaseId releaseId : feedbackService.findStarredReleases(findParameters).results) { if (auto release{ Release::find(context.dbSession, releaseId) }) - starredNode.addArrayChild("album", createAlbumNode(context, release, context.user, id3)); + starredNode.addArrayChild("album", createAlbumNode(context, release, id3)); } for (const TrackId trackId : feedbackService.findStarredTracks(findParameters).results) diff --git a/src/libs/subsonic/impl/entrypoints/Browsing.cpp b/src/libs/subsonic/impl/entrypoints/Browsing.cpp index 660a8a80..f7221b46 100644 --- a/src/libs/subsonic/impl/entrypoints/Browsing.cpp +++ b/src/libs/subsonic/impl/entrypoints/Browsing.cpp @@ -24,11 +24,13 @@ #include "core/Service.hpp" #include "database/Artist.hpp" #include "database/Cluster.hpp" +#include "database/Directory.hpp" #include "database/MediaLibrary.hpp" #include "database/Release.hpp" #include "database/Session.hpp" #include "database/Track.hpp" #include "database/User.hpp" +#include "services/feedback/IFeedbackService.hpp" #include "services/recommendation/IRecommendationService.hpp" #include "services/scrobbling/IScrobblingService.hpp" @@ -48,123 +50,58 @@ namespace lms::api::subsonic namespace { - Response handleGetArtistInfoRequestCommon(RequestContext& context, bool id3) + std::vector getRootDirectories(Session& session, MediaLibraryId libraryId) { - // Mandatory params - ArtistId id{ getMandatoryParameterAs(context.parameters, "id") }; - - // Optional params - std::size_t count{ getParameterAs(context.parameters, "count").value_or(20) }; - - Response response{ Response::createOkResponse(context.serverProtocolVersion) }; - Response::Node& artistInfoNode{ response.createNode(id3 ? Response::Node::Key{ "artistInfo2" } : Response::Node::Key{ "artistInfo" }) }; + std::vector res; + if (libraryId.isValid()) { - auto transaction{ context.dbSession.createReadTransaction() }; + const MediaLibrary::pointer library{ MediaLibrary::find(session, libraryId) }; + if (!library) + throw BadParameterGenericError{ "id" }; - const Artist::pointer artist{ Artist::find(context.dbSession, id) }; - if (!artist) - throw RequestedDataNotFoundError{}; - - std::optional artistMBID{ artist->getMBID() }; - if (artistMBID) - artistInfoNode.createChild("musicBrainzId").setValue(artistMBID->getAsString()); + if (Directory::pointer rootDirectory{ Directory::find(session, library->getPath()) }) + res.push_back(rootDirectory); + } + else + { + res = Directory::findRootDirectories(session).results; } - auto similarArtistsId{ core::Service::get()->getSimilarArtists(id, { TrackArtistLinkType::Artist, TrackArtistLinkType::ReleaseArtist }, count) }; - - { - auto transaction{ context.dbSession.createReadTransaction() }; - - for (const ArtistId similarArtistId : similarArtistsId) - { - const Artist::pointer similarArtist{ Artist::find(context.dbSession, similarArtistId) }; - if (similarArtist) - artistInfoNode.addArrayChild("similarArtist", createArtistNode(context, similarArtist, context.user, id3)); - } - } - - return response; + return res; } - Response handleGetArtistsRequestCommon(RequestContext& context, bool id3) + struct IndexComparator { - // Optional params - const MediaLibraryId mediaLibrary{ getParameterAs(context.parameters, "musicFolderId").value_or(MediaLibraryId{}) }; - - Response response{ Response::createOkResponse(context.serverProtocolVersion) }; - - Response::Node& artistsNode{ response.createNode(id3 ? "artists" : "indexes") }; - artistsNode.setAttribute("ignoredArticles", ""); - artistsNode.setAttribute("lastModified", reportedDummyDateULong); // TODO report last file write? - - Artist::FindParameters parameters; + constexpr bool operator()(char lhs, char rhs) const { - auto transaction{ context.dbSession.createReadTransaction() }; + if (lhs == '#' && std::isalpha(rhs)) + return false; + if (rhs == '#' && std::isalpha(lhs)) + return true; - parameters.setSortMethod(ArtistSortMethod::SortName); - switch (context.user->getSubsonicArtistListMode()) - { - case SubsonicArtistListMode::AllArtists: - break; - case SubsonicArtistListMode::ReleaseArtists: - parameters.setLinkType(TrackArtistLinkType::ReleaseArtist); - break; - case SubsonicArtistListMode::TrackArtists: - parameters.setLinkType(TrackArtistLinkType::Artist); - break; - } + return lhs < rhs; } - parameters.setMediaLibrary(mediaLibrary); + }; - // This endpoint does not scale: make sort lived transactions in order not to block the whole application + using IndexMap = std::map, IndexComparator>; + void getIndexedChildDirectories(RequestContext& context, const Directory::pointer& parentDirectory, IndexMap& res) + { + Directory::FindParameters params; + params.setParentDirectory(parentDirectory->getId()); - // first pass: dispatch the artists by first letter - LMS_LOG(API_SUBSONIC, DEBUG, "GetArtists: fetching all artists..."); - std::map> artistsSortedByFirstChar; - std::size_t currentArtistOffset{ 0 }; - constexpr std::size_t batchSize{ 100 }; - bool hasMoreArtists{ true }; - while (hasMoreArtists) - { - auto transaction{ context.dbSession.createReadTransaction() }; + Directory::find(context.dbSession, params, [&](const Directory::pointer& directory) { + const std::string_view name{ directory->getName() }; + assert(!name.empty()); - parameters.setRange(Range{ currentArtistOffset, batchSize }); - const auto artists{ Artist::find(context.dbSession, parameters) }; - for (const Artist::pointer& artist : artists.results) - { - std::string_view sortName{ artist->getSortName() }; + char sortChar; + if (name.empty() || !std::isalpha(name[0])) + sortChar = '#'; + else + sortChar = std::toupper(name[0]); - char sortChar; - if (sortName.empty() || !std::isalpha(sortName[0])) - sortChar = '?'; - else - sortChar = std::toupper(sortName[0]); - - artistsSortedByFirstChar[sortChar].push_back(artist->getId()); - } - - hasMoreArtists = artists.moreResults; - currentArtistOffset += artists.results.size(); - } - - // second pass: add each artist - LMS_LOG(API_SUBSONIC, DEBUG, "GetArtists: constructing response..."); - for (const auto& [sortChar, artistIds] : artistsSortedByFirstChar) - { - Response::Node& indexNode{ artistsNode.createArrayChild("index") }; - indexNode.setAttribute("name", std::string{ sortChar }); - - for (const ArtistId artistId : artistIds) - { - auto transaction{ context.dbSession.createReadTransaction() }; - - if (const Artist::pointer artist{ Artist::find(context.dbSession, artistId) }) - indexNode.addArrayChild("artist", createArtistNode(context, artist, context.user, id3)); - } - } - - return response; + res[sortChar].push_back(directory); + }); } std::vector findSimilarSongs(RequestContext& context, ArtistId artistId, std::size_t count) @@ -266,6 +203,21 @@ namespace lms::api::subsonic return response; } + Release::pointer getReleaseFromDirectory(Session& session, DirectoryId directoryId) + { + auto transaction{ session.createReadTransaction() }; + + Release::FindParameters params; + params.setDirectory(directoryId); + params.setRange(Range{ 0, 1 }); // only support 1 directory <-> 1 release + + Release::pointer res; + Release::find(session, params, [&](const Release::pointer& release) { + res = release; + }); + + return res; + } } // namespace Response handleGetMusicFoldersRequest(RequestContext& context) @@ -286,64 +238,112 @@ namespace lms::api::subsonic Response handleGetIndexesRequest(RequestContext& context) { - return handleGetArtistsRequestCommon(context, false /* no id3 */); + // Optional params + const MediaLibraryId mediaLibrary{ getParameterAs(context.parameters, "musicFolderId").value_or(MediaLibraryId{}) }; + + Response response{ Response::createOkResponse(context.serverProtocolVersion) }; + Response::Node& indexesNode{ response.createNode("indexes") }; + indexesNode.setAttribute("ignoredArticles", ""); + indexesNode.setAttribute("lastModified", reportedDummyDateULong); // TODO report last file write? + + auto transaction{ context.dbSession.createReadTransaction() }; + + const std::vector rootDirectories{ getRootDirectories(context.dbSession, mediaLibrary) }; + + IndexMap indexedDirectories; + for (const Directory::pointer& rootdirectory : rootDirectories) + { + Track::FindParameters params; + params.setDirectory(rootdirectory->getId()); + + Track::find(context.dbSession, params, [&](const Track::pointer& track) { + indexesNode.addArrayChild("child", createSongNode(context, track, context.user)); + }); + + getIndexedChildDirectories(context, rootdirectory, indexedDirectories); + } + + for (const auto& [index, directories] : indexedDirectories) + { + Response::Node& indexNode{ indexesNode.createArrayChild("index") }; + indexNode.setAttribute("name", std::string{ index }); + + for (const Directory::pointer& directory : directories) + { + // Legacy behavior: all sub directories are considered as artists (even if this is just containing an album, or just an intermediary directory) + + Response::Node childNode; + childNode.setAttribute("id", idToString(directory->getId())); + childNode.setAttribute("name", directory->getName()); + + indexNode.addArrayChild("artist", std::move(childNode)); + } + } + + return response; } Response handleGetMusicDirectoryRequest(RequestContext& context) { // Mandatory params - const auto artistId{ getParameterAs(context.parameters, "id") }; - const auto releaseId{ getParameterAs(context.parameters, "id") }; - const auto root{ getParameterAs(context.parameters, "id") }; - - if (!root && !artistId && !releaseId) - throw BadParameterGenericError{ "id" }; + const auto directoryId{ getMandatoryParameterAs(context.parameters, "id") }; Response response{ Response::createOkResponse(context.serverProtocolVersion) }; Response::Node& directoryNode{ response.createNode("directory") }; auto transaction{ context.dbSession.createReadTransaction() }; - if (root) - { - directoryNode.setAttribute("id", idToString(RootId{})); - directoryNode.setAttribute("name", "Music"); + const Directory::pointer directory{ Directory::find(context.dbSession, directoryId) }; + if (!directory) + throw RequestedDataNotFoundError{}; - // TODO: this does not scale when a lot of artists are present - Artist::find(context.dbSession, Artist::FindParameters{}.setSortMethod(ArtistSortMethod::SortName), [&](const Artist::pointer& artist) { - directoryNode.addArrayChild("child", createArtistNode(context, artist, context.user, false /* no id3 */)); + if (const Release::pointer release{ getReleaseFromDirectory(context.dbSession, directoryId) }) + { + directoryNode.setAttribute("playCount", core::Service::get()->getCount(context.user->getId(), release->getId())); + if (const Wt::WDateTime dateTime{ core::Service::get()->getStarredDateTime(context.user->getId(), release->getId()) }; dateTime.isValid()) + directoryNode.setAttribute("starred", core::stringUtils::toISO8601String(dateTime)); + } + + directoryNode.setAttribute("id", idToString(directory->getId())); + directoryNode.setAttribute("name", directory->getName()); + // Original Subsonic does not report parent if this the parent directory is the root directory + if (const Directory::pointer parentDirectory{ directory->getParentDirectory() }) + directoryNode.setAttribute("parent", idToString(parentDirectory->getId())); + + // list all sub directories + { + Directory::FindParameters params; + params.setParentDirectory(directory->getId()); + + Directory::find(context.dbSession, params, [&](const Directory::pointer& subDirectory) { + const Release::pointer release{ getReleaseFromDirectory(context.dbSession, subDirectory->getId()) }; + + if (release) + { + directoryNode.addArrayChild("child", createAlbumNode(context, release, false, subDirectory)); + } + else + { + Response::Node childNode; + childNode.setAttribute("id", idToString(subDirectory->getId())); + childNode.setAttribute("title", subDirectory->getName()); + childNode.setAttribute("isDir", true); + childNode.setAttribute("parent", idToString(directory->getId())); + + directoryNode.addArrayChild("child", std::move(childNode)); + } }); } - else if (artistId) + + // list all tracks { - directoryNode.setAttribute("id", idToString(*artistId)); + Track::FindParameters params; + params.setDirectory(directory->getId()); - auto artist{ Artist::find(context.dbSession, *artistId) }; - if (!artist) - throw RequestedDataNotFoundError{}; - - directoryNode.setAttribute("name", utils::makeNameFilesystemCompatible(artist->getName())); - - Release::find(context.dbSession, Release::FindParameters{}.setArtist(*artistId), [&](const Release::pointer& release) { - directoryNode.addArrayChild("child", createAlbumNode(context, release, context.user, false /* no id3 */)); - }); - } - else if (releaseId) - { - directoryNode.setAttribute("id", idToString(*releaseId)); - - auto release{ Release::find(context.dbSession, *releaseId) }; - if (!release) - throw RequestedDataNotFoundError{}; - - directoryNode.setAttribute("name", utils::makeNameFilesystemCompatible(release->getName())); - - Track::find(context.dbSession, Track::FindParameters{}.setRelease(*releaseId).setSortMethod(TrackSortMethod::Release), [&](const Track::pointer& track) { + Track::find(context.dbSession, params, [&](const Track::pointer& track) { directoryNode.addArrayChild("child", createSongNode(context, track, context.user)); }); } - else - throw BadParameterGenericError{ "id" }; return response; } @@ -370,7 +370,82 @@ namespace lms::api::subsonic Response handleGetArtistsRequest(RequestContext& context) { - return handleGetArtistsRequestCommon(context, true /* id3 */); + // Optional params + const MediaLibraryId mediaLibrary{ getParameterAs(context.parameters, "musicFolderId").value_or(MediaLibraryId{}) }; + + Response response{ Response::createOkResponse(context.serverProtocolVersion) }; + + Response::Node& artistsNode{ response.createNode("artists") }; + artistsNode.setAttribute("ignoredArticles", ""); + artistsNode.setAttribute("lastModified", reportedDummyDateULong); // TODO report last file write? + + Artist::FindParameters parameters; + { + auto transaction{ context.dbSession.createReadTransaction() }; + + parameters.setSortMethod(ArtistSortMethod::SortName); + switch (context.user->getSubsonicArtistListMode()) + { + case SubsonicArtistListMode::AllArtists: + break; + case SubsonicArtistListMode::ReleaseArtists: + parameters.setLinkType(TrackArtistLinkType::ReleaseArtist); + break; + case SubsonicArtistListMode::TrackArtists: + parameters.setLinkType(TrackArtistLinkType::Artist); + break; + } + } + parameters.setMediaLibrary(mediaLibrary); + + // This endpoint does not scale: make sort lived transactions in order not to block the whole application + + // first pass: dispatch the artists by first letter + LMS_LOG(API_SUBSONIC, DEBUG, "GetArtists: fetching all artists..."); + std::map> artistsSortedByFirstChar; + std::size_t currentArtistOffset{ 0 }; + constexpr std::size_t batchSize{ 100 }; + bool hasMoreArtists{ true }; + while (hasMoreArtists) + { + auto transaction{ context.dbSession.createReadTransaction() }; + + parameters.setRange(Range{ currentArtistOffset, batchSize }); + const auto artists{ Artist::find(context.dbSession, parameters) }; + for (const Artist::pointer& artist : artists.results) + { + std::string_view sortName{ artist->getSortName() }; + + char sortChar; + if (sortName.empty() || !std::isalpha(sortName[0])) + sortChar = '#'; + else + sortChar = std::toupper(sortName[0]); + + artistsSortedByFirstChar[sortChar].push_back(artist->getId()); + } + + hasMoreArtists = artists.moreResults; + currentArtistOffset += artists.results.size(); + } + + // second pass: add each artist + LMS_LOG(API_SUBSONIC, DEBUG, "GetArtists: constructing response..."); + for (const auto& [sortChar, artistIds] : artistsSortedByFirstChar) + { + Response::Node& indexNode{ artistsNode.createArrayChild("index") }; + indexNode.setAttribute("name", std::string{ sortChar }); + + for (const ArtistId artistId : artistIds) + { + auto transaction{ context.dbSession.createReadTransaction() }; + + if (const Artist::pointer artist{ Artist::find(context.dbSession, artistId) }) + indexNode.addArrayChild("artist", createArtistNode(context, artist)); + } + } + + return response; } Response handleGetArtistRequest(RequestContext& context) @@ -385,11 +460,11 @@ namespace lms::api::subsonic throw RequestedDataNotFoundError{}; Response response{ Response::createOkResponse(context.serverProtocolVersion) }; - Response::Node artistNode{ createArtistNode(context, artist, context.user, true /* id3 */) }; + Response::Node artistNode{ createArtistNode(context, artist) }; const auto releases{ Release::find(context.dbSession, Release::FindParameters{}.setArtist(artist->getId())) }; for (const Release::pointer& release : releases.results) - artistNode.addArrayChild("album", createAlbumNode(context, release, context.user, true /* id3 */)); + artistNode.addArrayChild("album", createAlbumNode(context, release, true /* id3 */)); response.addNode("artist", std::move(artistNode)); @@ -408,11 +483,11 @@ namespace lms::api::subsonic throw RequestedDataNotFoundError{}; Response response{ Response::createOkResponse(context.serverProtocolVersion) }; - Response::Node albumNode{ createAlbumNode(context, release, context.user, true /* id3 */) }; + Response::Node albumNode{ createAlbumNode(context, release, true /* id3 */) }; const auto tracks{ Track::find(context.dbSession, Track::FindParameters{}.setRelease(id).setSortMethod(TrackSortMethod::Release)) }; for (const Track::pointer& track : tracks.results) - albumNode.addArrayChild("song", createSongNode(context, track, context.user)); + albumNode.addArrayChild("song", createSongNode(context, track, true /* id3 */)); response.addNode("album", std::move(albumNode)); @@ -436,14 +511,43 @@ namespace lms::api::subsonic return response; } - Response handleGetArtistInfoRequest(RequestContext& context) - { - return handleGetArtistInfoRequestCommon(context, false /* no id3 */); - } - Response handleGetArtistInfo2Request(RequestContext& context) { - return handleGetArtistInfoRequestCommon(context, true /* id3 */); + // Mandatory params + ArtistId id{ getMandatoryParameterAs(context.parameters, "id") }; + + // Optional params + std::size_t count{ getParameterAs(context.parameters, "count").value_or(20) }; + + Response response{ Response::createOkResponse(context.serverProtocolVersion) }; + Response::Node& artistInfoNode{ response.createNode(Response::Node::Key{ "artistInfo2" }) }; + + { + auto transaction{ context.dbSession.createReadTransaction() }; + + const Artist::pointer artist{ Artist::find(context.dbSession, id) }; + if (!artist) + throw RequestedDataNotFoundError{}; + + std::optional artistMBID{ artist->getMBID() }; + if (artistMBID) + artistInfoNode.createChild("musicBrainzId").setValue(artistMBID->getAsString()); + } + + auto similarArtistsId{ core::Service::get()->getSimilarArtists(id, { TrackArtistLinkType::Artist, TrackArtistLinkType::ReleaseArtist }, count) }; + + { + auto transaction{ context.dbSession.createReadTransaction() }; + + for (const ArtistId similarArtistId : similarArtistsId) + { + const Artist::pointer similarArtist{ Artist::find(context.dbSession, similarArtistId) }; + if (similarArtist) + artistInfoNode.addArrayChild("similarArtist", createArtistNode(context, similarArtist)); + } + } + + return response; } Response handleGetSimilarSongsRequest(RequestContext& context) diff --git a/src/libs/subsonic/impl/entrypoints/MediaAnnotation.cpp b/src/libs/subsonic/impl/entrypoints/MediaAnnotation.cpp index 2abdd212..b8a156c1 100644 --- a/src/libs/subsonic/impl/entrypoints/MediaAnnotation.cpp +++ b/src/libs/subsonic/impl/entrypoints/MediaAnnotation.cpp @@ -23,7 +23,9 @@ #include "core/Service.hpp" #include "database/ArtistId.hpp" +#include "database/Release.hpp" #include "database/ReleaseId.hpp" +#include "database/Session.hpp" #include "database/TrackId.hpp" #include "database/User.hpp" #include "services/feedback/IFeedbackService.hpp" @@ -43,25 +45,50 @@ namespace lms::api::subsonic std::vector artistIds; std::vector releaseIds; std::vector trackIds; + std::vector directoryIds; }; StarParameters getStarParameters(const Wt::Http::ParameterMap& parameters) { StarParameters res; - // TODO handle parameters for legacy file browsing + // id could be either a trackId or a directory id + res.directoryIds = getMultiParametersAs(parameters, "id"); res.trackIds = getMultiParametersAs(parameters, "id"); res.artistIds = getMultiParametersAs(parameters, "artistId"); res.releaseIds = getMultiParametersAs(parameters, "albumId"); return res; } + + ReleaseId getReleaseFromDirectory(Session& session, DirectoryId directory) + { + auto transaction{ session.createReadTransaction() }; + + Release::FindParameters params; + params.setDirectory(directory); + params.setRange(Range{ 0, 1 }); // consider one directory <-> one release + + Release::pointer res; + Release::find(session, params, [&](const Release::pointer& release) { + res = release; + }); + + return res ? res->getId() : ReleaseId{}; + } + } // namespace Response handleStarRequest(RequestContext& context) { StarParameters params{ getStarParameters(context.parameters) }; + for (const DirectoryId id : params.directoryIds) + { + if (const ReleaseId releaseId{ getReleaseFromDirectory(context.dbSession, id) }; releaseId.isValid()) + core::Service::get()->star(context.user->getId(), releaseId); + } + for (const ArtistId id : params.artistIds) core::Service::get()->star(context.user->getId(), id); @@ -78,6 +105,12 @@ namespace lms::api::subsonic { StarParameters params{ getStarParameters(context.parameters) }; + for (const DirectoryId id : params.directoryIds) + { + if (const ReleaseId releaseId{ getReleaseFromDirectory(context.dbSession, id) }; releaseId.isValid()) + core::Service::get()->unstar(context.user->getId(), releaseId); + } + for (const ArtistId id : params.artistIds) core::Service::get()->unstar(context.user->getId(), id); diff --git a/src/libs/subsonic/impl/entrypoints/Searching.cpp b/src/libs/subsonic/impl/entrypoints/Searching.cpp index 008f3bdf..a5087945 100644 --- a/src/libs/subsonic/impl/entrypoints/Searching.cpp +++ b/src/libs/subsonic/impl/entrypoints/Searching.cpp @@ -25,6 +25,7 @@ #include "core/Random.hpp" #include "database/Artist.hpp" +#include "database/Directory.hpp" #include "database/Release.hpp" #include "database/Session.hpp" #include "database/Track.hpp" @@ -113,7 +114,36 @@ namespace lms::api::subsonic _ongoingScans[scanInfo] = { now, lastRetrievedId }; } - void findRequestedArtists(RequestContext& context, bool id3, const std::vector& keywords, MediaLibraryId mediaLibrary, const User::pointer& user, Response::Node& searchResultNode) + void findRequestedArtistDirectories(RequestContext& context, const std::vector& keywords, MediaLibraryId mediaLibrary, Response::Node& searchResultNode) + { + // For now, no need to "accelerate" all this + // Find all the directories that match the name and that do not contain any track (considered by the legacy API as artists) + const std::size_t artistCount{ getParameterAs(context.parameters, "artistCount").value_or(20) }; + if (artistCount == 0) + return; + + if (artistCount > defaultMaxCountSize) + throw ParameterValueTooHighGenericError{ "artistCount", defaultMaxCountSize }; + + const std::size_t artistOffset{ getParameterAs(context.parameters, "artistOffset").value_or(0) }; + + Directory::FindParameters params; + params.setKeywords(keywords); + params.setRange(Range{artistOffset, artistCount}); + params.setWithNoTrack(true); + // TODO media + + Directory::find(context.dbSession, params, [&](const Directory::pointer& directory) { + Response::Node childNode; + childNode.setAttribute("id", idToString(directory->getId())); + childNode.setAttribute("name", directory->getName()); + childNode.setAttribute("isDir", true); + + searchResultNode.addArrayChild("artist", std::move(childNode)); + }); + } + + void findRequestedArtists(RequestContext& context, const std::vector& keywords, MediaLibraryId mediaLibrary, Response::Node& searchResultNode) { static ScanTracker currentScansInProgress; @@ -135,7 +165,7 @@ namespace lms::api::subsonic params.setSortMethod(ArtistSortMethod::Id); // must be consistent with both methods Artist::find(context.dbSession, params, [&](const Artist::pointer& artist) { - searchResultNode.addArrayChild("artist", createArtistNode(context, artist, user, id3)); + searchResultNode.addArrayChild("artist", createArtistNode(context, artist)); lastRetrievedId = artist->getId(); }); } }; @@ -158,7 +188,7 @@ namespace lms::api::subsonic { Artist::find( context.dbSession, cachedLastRetrievedId, artistCount, [&](const Artist::pointer& artist) { - searchResultNode.addArrayChild("artist", createArtistNode(context, artist, user, id3)); + searchResultNode.addArrayChild("artist", createArtistNode(context, artist)); }, mediaLibrary); lastRetrievedId = cachedLastRetrievedId; @@ -176,7 +206,7 @@ namespace lms::api::subsonic } } - void findRequestedAlbums(RequestContext& context, bool id3, const std::vector& keywords, MediaLibraryId mediaLibrary, const User::pointer& user, Response::Node& searchResultNode) + void findRequestedAlbums(RequestContext& context, bool id3, const std::vector& keywords, MediaLibraryId mediaLibrary, Response::Node& searchResultNode) { static ScanTracker currentScansInProgress; @@ -199,7 +229,7 @@ namespace lms::api::subsonic params.setSortMethod(ReleaseSortMethod::Id); // must be consistent with both methods Release::find(context.dbSession, params, [&](const Release::pointer& release) { - searchResultNode.addArrayChild("album", createAlbumNode(context, release, user, id3)); + searchResultNode.addArrayChild("album", createAlbumNode(context, release, id3)); lastRetrievedId = release->getId(); }); } }; @@ -222,7 +252,7 @@ namespace lms::api::subsonic { Release::find( context.dbSession, cachedLastRetrievedId, albumCount, [&](const Release::pointer& release) { - searchResultNode.addArrayChild("album", createAlbumNode(context, release, user, id3)); + searchResultNode.addArrayChild("album", createAlbumNode(context, release, id3)); }, mediaLibrary); lastRetrievedId = cachedLastRetrievedId; @@ -240,7 +270,7 @@ namespace lms::api::subsonic } } - void findRequestedTracks(RequestContext& context, const std::vector& keywords, MediaLibraryId mediaLibrary, const User::pointer& user, Response::Node& searchResultNode) + void findRequestedTracks(RequestContext& context, bool id3, const std::vector& keywords, MediaLibraryId mediaLibrary, Response::Node& searchResultNode) { static ScanTracker currentScansInProgress; @@ -263,7 +293,7 @@ namespace lms::api::subsonic params.setSortMethod(TrackSortMethod::Id); // must be consistent with both methods Track::find(context.dbSession, params, [&](const Track::pointer& track) { - searchResultNode.addArrayChild("song", createSongNode(context, track, user)); + searchResultNode.addArrayChild("song", createSongNode(context, track, id3)); lastRetrievedId = track->getId(); }); } }; @@ -286,7 +316,7 @@ namespace lms::api::subsonic { Track::find( context.dbSession, cachedLastRetrievedId, songCount, [&](const Track::pointer& track) { - searchResultNode.addArrayChild("song", createSongNode(context, track, user)); + searchResultNode.addArrayChild("song", createSongNode(context, track, id3)); }, mediaLibrary); lastRetrievedId = cachedLastRetrievedId; @@ -329,9 +359,12 @@ namespace lms::api::subsonic auto transaction{ context.dbSession.createReadTransaction() }; - findRequestedArtists(context, id3, keywords, mediaLibrary, context.user, searchResultNode); - findRequestedAlbums(context, id3, keywords, mediaLibrary, context.user, searchResultNode); - findRequestedTracks(context, keywords, mediaLibrary, context.user, searchResultNode); + if (id3) + findRequestedArtists(context, keywords, mediaLibrary, searchResultNode); + else + findRequestedArtistDirectories(context, keywords, mediaLibrary, searchResultNode); + findRequestedAlbums(context, id3, keywords, mediaLibrary, searchResultNode); + findRequestedTracks(context, id3, keywords, mediaLibrary, searchResultNode); return response; } diff --git a/src/libs/subsonic/impl/responses/Album.cpp b/src/libs/subsonic/impl/responses/Album.cpp index 546a19e7..88d00e43 100644 --- a/src/libs/subsonic/impl/responses/Album.cpp +++ b/src/libs/subsonic/impl/responses/Album.cpp @@ -24,6 +24,7 @@ #include "core/String.hpp" #include "database/Artist.hpp" #include "database/Cluster.hpp" +#include "database/Directory.hpp" #include "database/Release.hpp" #include "database/User.hpp" #include "services/feedback/IFeedbackService.hpp" @@ -39,7 +40,7 @@ namespace lms::api::subsonic { using namespace db; - Response::Node createAlbumNode(RequestContext& context, const Release::pointer& release, const User::pointer& user, bool id3) + Response::Node createAlbumNode(RequestContext& context, const Release::pointer& release, bool id3, const Directory::pointer& directory) { LMS_SCOPED_TRACE_DETAILED("Subsonic", "CreateAlbum"); @@ -47,21 +48,38 @@ namespace lms::api::subsonic if (id3) { + albumNode.setAttribute("id", idToString(release->getId())); albumNode.setAttribute("name", release->getName()); albumNode.setAttribute("songCount", release->getTrackCount()); - albumNode.setAttribute( - "duration", std::chrono::duration_cast( - release->getDuration()) - .count()); + albumNode.setAttribute("duration", std::chrono::duration_cast(release->getDuration()).count()); } else { - albumNode.setAttribute("title", release->getName()); + Directory::pointer directoryToReport{ directory }; + + if (!directoryToReport) + { + Directory::FindParameters params; + params.setRelease(release->getId()); + params.setRange(Range{ 0, 1 }); // only support 1 directory <-> 1 release + Directory::find(context.dbSession, params, [&](const Directory::pointer& foundDirectory) { + directoryToReport = foundDirectory; + }); + } + + if (directoryToReport) + { + albumNode.setAttribute("title", directoryToReport->getName()); + albumNode.setAttribute("id", idToString(directoryToReport->getId())); + if (const Directory::pointer & parentDirectory{ directoryToReport->getParentDirectory() }) + albumNode.setAttribute("parent", idToString(parentDirectory->getId())); + } + + albumNode.setAttribute("album", release->getName()); albumNode.setAttribute("isDir", true); } albumNode.setAttribute("created", core::stringUtils::toISO8601String(release->getLastWritten())); - albumNode.setAttribute("id", idToString(release->getId())); albumNode.setAttribute("coverArt", idToString(release->getId())); if (const auto year{ release->getYear() }) albumNode.setAttribute("year", *year); @@ -70,11 +88,7 @@ namespace lms::api::subsonic if (artists.empty()) artists = release->getArtists(); - if (artists.empty() && !id3) - { - albumNode.setAttribute("parent", idToString(RootId{})); - } - else if (!artists.empty()) + if (!artists.empty()) { if (!release->getArtistDisplayName().empty()) albumNode.setAttribute("artist", release->getArtistDisplayName()); @@ -83,16 +97,11 @@ namespace lms::api::subsonic if (artists.size() == 1) { - albumNode.setAttribute(id3 ? Response::Node::Key{ "artistId" } : Response::Node::Key{ "parent" }, idToString(artists.front()->getId())); - } - else - { - if (!id3) - albumNode.setAttribute("parent", idToString(RootId{})); + albumNode.setAttribute("artistId", idToString(artists.front()->getId())); } } - albumNode.setAttribute("playCount", core::Service::get()->getCount(user->getId(), release->getId())); + albumNode.setAttribute("playCount", core::Service::get()->getCount(context.user->getId(), release->getId())); // Report the first GENRE for this track const ClusterType::pointer genreClusterType{ ClusterType::find(context.dbSession, "GENRE") }; @@ -103,7 +112,7 @@ namespace lms::api::subsonic albumNode.setAttribute("genre", clusters.front().front()->getName()); } - if (const Wt::WDateTime dateTime{ core::Service::get()->getStarredDateTime(user->getId(), release->getId()) }; dateTime.isValid()) + if (const Wt::WDateTime dateTime{ core::Service::get()->getStarredDateTime(context.user->getId(), release->getId()) }; dateTime.isValid()) albumNode.setAttribute("starred", core::stringUtils::toISO8601String(dateTime)); if (!context.enableOpenSubsonic) @@ -111,12 +120,10 @@ namespace lms::api::subsonic // OpenSubsonic specific fields (must always be set) albumNode.setAttribute("sortName", release->getSortName()); - - if (!id3) - albumNode.setAttribute("mediaType", "album"); + albumNode.setAttribute("mediaType", "album"); { - const Wt::WDateTime dateTime{ core::Service::get()->getLastListenDateTime(user->getId(), release->getId()) }; + const Wt::WDateTime dateTime{ core::Service::get()->getLastListenDateTime(context.user->getId(), release->getId()) }; albumNode.setAttribute("played", dateTime.isValid() ? core::stringUtils::toISO8601String(dateTime) : std::string{ "" }); } diff --git a/src/libs/subsonic/impl/responses/Album.hpp b/src/libs/subsonic/impl/responses/Album.hpp index a9508a05..d324fe24 100644 --- a/src/libs/subsonic/impl/responses/Album.hpp +++ b/src/libs/subsonic/impl/responses/Album.hpp @@ -25,6 +25,7 @@ namespace lms::db { + class Directory; class Release; class User; class Session; @@ -32,5 +33,5 @@ namespace lms::db namespace lms::api::subsonic { - Response::Node createAlbumNode(RequestContext& context, const db::ObjectPtr& release, const db::ObjectPtr& user, bool id3); + Response::Node createAlbumNode(RequestContext& context, const db::ObjectPtr& release, bool id3, const db::ObjectPtr& directory = {}); } \ No newline at end of file diff --git a/src/libs/subsonic/impl/responses/Artist.cpp b/src/libs/subsonic/impl/responses/Artist.cpp index 734b02cd..77481fba 100644 --- a/src/libs/subsonic/impl/responses/Artist.cpp +++ b/src/libs/subsonic/impl/responses/Artist.cpp @@ -85,7 +85,7 @@ namespace lms::api::subsonic } } // namespace utils - Response::Node createArtistNode(RequestContext& context, const Artist::pointer& artist, const User::pointer& user, bool id3) + Response::Node createArtistNode(RequestContext& context, const Artist::pointer& artist) { LMS_SCOPED_TRACE_DETAILED("Subsonic", "CreateArtist"); @@ -96,20 +96,16 @@ namespace lms::api::subsonic if (const db::Image::pointer artistImage{ artist->getImage() }) artistNode.setAttribute("coverArt", idToString(artist->getId())); - if (id3) - { - const std::size_t count{ Release::getCount(context.dbSession, Release::FindParameters{}.setArtist(artist->getId())) }; - artistNode.setAttribute("albumCount", count); - } + const std::size_t count{ Release::getCount(context.dbSession, Release::FindParameters{}.setArtist(artist->getId())) }; + artistNode.setAttribute("albumCount", count); - if (const Wt::WDateTime dateTime{ core::Service::get()->getStarredDateTime(user->getId(), artist->getId()) }; dateTime.isValid()) + if (const Wt::WDateTime dateTime{ core::Service::get()->getStarredDateTime(context.user->getId(), artist->getId()) }; dateTime.isValid()) artistNode.setAttribute("starred", core::stringUtils::toISO8601String(dateTime)); // OpenSubsonic specific fields (must always be set) if (context.enableOpenSubsonic) { - if (!id3) - artistNode.setAttribute("mediaType", "artist"); + artistNode.setAttribute("mediaType", "artist"); { std::optional mbid{ artist->getMBID() }; diff --git a/src/libs/subsonic/impl/responses/Artist.hpp b/src/libs/subsonic/impl/responses/Artist.hpp index b7ebc516..3a8a5687 100644 --- a/src/libs/subsonic/impl/responses/Artist.hpp +++ b/src/libs/subsonic/impl/responses/Artist.hpp @@ -41,6 +41,7 @@ namespace lms::api::subsonic std::string joinArtistNames(const std::vector>& artists); std::string_view toString(db::TrackArtistLinkType type); } // namespace utils - Response::Node createArtistNode(RequestContext& context, const db::ObjectPtr& artist, const db::ObjectPtr& user, bool id3); + + Response::Node createArtistNode(RequestContext& context, const db::ObjectPtr& artist); Response::Node createArtistNode(const db::ObjectPtr& artist); // only minimal info } // namespace lms::api::subsonic \ No newline at end of file diff --git a/src/libs/subsonic/impl/responses/Song.cpp b/src/libs/subsonic/impl/responses/Song.cpp index cc4e0365..9ff70831 100644 --- a/src/libs/subsonic/impl/responses/Song.cpp +++ b/src/libs/subsonic/impl/responses/Song.cpp @@ -27,6 +27,7 @@ #include "core/String.hpp" #include "database/Artist.hpp" #include "database/Cluster.hpp" +#include "database/Directory.hpp" #include "database/Release.hpp" #include "database/Track.hpp" #include "database/TrackArtistLink.hpp" @@ -67,14 +68,20 @@ namespace lms::api::subsonic } } // namespace - Response::Node createSongNode(RequestContext& context, const Track::pointer& track, const User::pointer& user) + Response::Node createSongNode(RequestContext& context, const Track::pointer& track, bool id3) { LMS_SCOPED_TRACE_DETAILED("Subsonic", "CreateSong"); Response::Node trackResponse; + if (!id3) + { + if (const auto directory{ track->getDirectory() }) + trackResponse.setAttribute("parent", idToString(directory->getId())); + trackResponse.setAttribute("isDir", false); + } + trackResponse.setAttribute("id", idToString(track->getId())); - trackResponse.setAttribute("isDir", false); trackResponse.setAttribute("title", track->getName()); if (track->getTrackNumber()) trackResponse.setAttribute("track", *track->getTrackNumber()); @@ -82,7 +89,7 @@ namespace lms::api::subsonic trackResponse.setAttribute("discNumber", *track->getDiscNumber()); if (track->getYear()) trackResponse.setAttribute("year", *track->getYear()); - trackResponse.setAttribute("playCount", core::Service::get()->getCount(user->getId(), track->getId())); + trackResponse.setAttribute("playCount", core::Service::get()->getCount(context.user->getId(), track->getId())); trackResponse.setAttribute("path", track->getRelativeFilePath().string()); trackResponse.setAttribute("size", track->getFileSize()); @@ -93,12 +100,13 @@ namespace lms::api::subsonic } { - const std::string fileSuffix{ formatToSuffix(user->getSubsonicDefaultTranscodingOutputFormat()) }; + const std::string fileSuffix{ formatToSuffix(context.user->getSubsonicDefaultTranscodingOutputFormat()) }; trackResponse.setAttribute("transcodedSuffix", fileSuffix); trackResponse.setAttribute("transcodedContentType", av::getMimeType(std::filesystem::path{ "." + fileSuffix })); } - trackResponse.setAttribute("coverArt", idToString(track->getId())); + if (track->hasCover()) + trackResponse.setAttribute("coverArt", idToString(track->getId())); const std::vector& artists{ track->getArtists({ TrackArtistLinkType::Artist }) }; if (!artists.empty()) @@ -117,7 +125,6 @@ namespace lms::api::subsonic { trackResponse.setAttribute("album", release->getName()); trackResponse.setAttribute("albumId", idToString(release->getId())); - trackResponse.setAttribute("parent", idToString(release->getId())); } trackResponse.setAttribute("duration", std::chrono::duration_cast(track->getDuration()).count()); @@ -126,7 +133,7 @@ namespace lms::api::subsonic trackResponse.setAttribute("created", core::stringUtils::toISO8601String(track->getLastWritten())); trackResponse.setAttribute("contentType", av::getMimeType(track->getAbsoluteFilePath().extension())); - if (const Wt::WDateTime dateTime{ core::Service::get()->getStarredDateTime(user->getId(), track->getId()) }; dateTime.isValid()) + if (const Wt::WDateTime dateTime{ core::Service::get()->getStarredDateTime(context.user->getId(), track->getId()) }; dateTime.isValid()) trackResponse.setAttribute("starred", core::stringUtils::toISO8601String(dateTime)); // Report the first GENRE for this track @@ -152,7 +159,7 @@ namespace lms::api::subsonic trackResponse.setAttribute("mediaType", "song"); { - const Wt::WDateTime dateTime{ core::Service::get()->getLastListenDateTime(user->getId(), track->getId()) }; + const Wt::WDateTime dateTime{ core::Service::get()->getLastListenDateTime(context.user->getId(), track->getId()) }; trackResponse.setAttribute("played", dateTime.isValid() ? core::stringUtils::toISO8601String(dateTime) : ""); } diff --git a/src/libs/subsonic/impl/responses/Song.hpp b/src/libs/subsonic/impl/responses/Song.hpp index 1a91549e..5b35227b 100644 --- a/src/libs/subsonic/impl/responses/Song.hpp +++ b/src/libs/subsonic/impl/responses/Song.hpp @@ -32,5 +32,5 @@ namespace lms::db namespace lms::api::subsonic { - Response::Node createSongNode(RequestContext& context, const db::ObjectPtr& track, const db::ObjectPtr& user); + Response::Node createSongNode(RequestContext& context, const db::ObjectPtr& track, bool id3); } \ No newline at end of file From 9ff387061a79fb9523b75760635da4bea5a8ab8c Mon Sep 17 00:00:00 2001 From: emeric Date: Sat, 13 Jul 2024 18:55:46 +0200 Subject: [PATCH 03/11] Subsonic API: added a way to filter files per media folder id when using directory commands --- src/libs/database/impl/Directory.cpp | 6 +- src/libs/database/impl/Migration.cpp | 36 +++++++- src/libs/database/impl/Session.cpp | 1 + .../database/include/database/Directory.hpp | 16 +++- .../impl/ScanStepAssociateArtistImages.cpp | 6 ++ .../scanner/impl/ScanStepScanFiles.cpp | 85 ++++++++++++++----- .../scanner/impl/ScanStepScanFiles.hpp | 2 + .../subsonic/impl/entrypoints/Searching.cpp | 11 +-- 8 files changed, 133 insertions(+), 30 deletions(-) diff --git a/src/libs/database/impl/Directory.cpp b/src/libs/database/impl/Directory.cpp index 0b20237f..f994cc0c 100644 --- a/src/libs/database/impl/Directory.cpp +++ b/src/libs/database/impl/Directory.cpp @@ -19,6 +19,7 @@ #include "database/Directory.hpp" +#include "database/MediaLibrary.hpp" #include "database/Session.hpp" #include "IdTypeTraits.hpp" @@ -43,12 +44,15 @@ namespace lms::db query.groupBy("d.id"); } + if (params.mediaLibrary.isValid()) + query.where("d.media_library_id = ?").bind(params.mediaLibrary); + if (params.parentDirectory.isValid()) query.where("d.parent_directory_id = ?").bind(params.parentDirectory); if (params.release.isValid()) query.where("t.release_id = ?").bind(params.release); - + if (params.artist.isValid()) { query.join("artist a ON a.id = t_a_l.artist_id") diff --git a/src/libs/database/impl/Migration.cpp b/src/libs/database/impl/Migration.cpp index e28d1233..d1d5de1e 100644 --- a/src/libs/database/impl/Migration.cpp +++ b/src/libs/database/impl/Migration.cpp @@ -35,7 +35,7 @@ namespace lms::db { namespace { - static constexpr Version LMS_DATABASE_VERSION{ 61 }; + static constexpr Version LMS_DATABASE_VERSION{ 62 }; } VersionInfo::VersionInfo() @@ -631,6 +631,39 @@ SELECT } // namespace + void migrateFromV61(Session& session) + { + // Added a media_library_id in Directory + session.getDboSession()->execute(R"( +CREATE TABLE IF NOT EXISTS "directory_backup" ( + "id" integer primary key autoincrement, + "version" integer not null, + "absolute_path" text not null, + "name" text not null, + "parent_directory_id" bigint, + "media_library_id" bigint, + constraint "fk_directory_parent_directory" foreign key ("parent_directory_id") references "directory" ("id") on delete cascade deferrable initially deferred, + constraint "fk_directory_media_library" foreign key ("media_library_id") references "media_library" ("id") on delete set null deferrable initially deferred + ))"); + + // Migrate data, with the new directory_id field set to null + session.getDboSession()->execute(R"(INSERT INTO directory_backup +SELECT + id, + version, + absolute_path, + name, + parent_directory_id, + NULL + FROM directory)"); + + session.getDboSession()->execute("DROP TABLE directory"); + session.getDboSession()->execute("ALTER TABLE directory_backup RENAME TO directory"); + + // Just increment the scan version of the settings to make the next scheduled scan rescan everything + session.getDboSession()->execute("UPDATE scan_settings SET scan_version = scan_version + 1"); + } + bool doDbMigration(Session& session) { static const std::string outdatedMsg{ "Outdated database, please rebuild it (delete the .db file and restart)" }; @@ -668,6 +701,7 @@ SELECT { 58, migrateFromV58 }, { 59, migrateFromV59 }, { 60, migrateFromV60 }, + { 61, migrateFromV61 }, }; bool migrationPerformed{}; diff --git a/src/libs/database/impl/Session.cpp b/src/libs/database/impl/Session.cpp index 03c9a2f7..73819aac 100644 --- a/src/libs/database/impl/Session.cpp +++ b/src/libs/database/impl/Session.cpp @@ -183,6 +183,7 @@ namespace lms::db _session.execute("CREATE INDEX IF NOT EXISTS directory_id_idx ON directory(id)"); _session.execute("CREATE INDEX IF NOT EXISTS directory_path_idx ON directory(absolute_path)"); + _session.execute("CREATE INDEX IF NOT EXISTS directory_media_library_idx ON directory(media_library_id)"); _session.execute("CREATE INDEX IF NOT EXISTS image_artist_idx ON image(artist_id)"); _session.execute("CREATE INDEX IF NOT EXISTS image_directory_idx ON image(directory_id)"); diff --git a/src/libs/database/include/database/Directory.hpp b/src/libs/database/include/database/Directory.hpp index 7cecbc69..ca74e156 100644 --- a/src/libs/database/include/database/Directory.hpp +++ b/src/libs/database/include/database/Directory.hpp @@ -21,22 +21,24 @@ #include #include -#include #include #include +#include #include #include "core/EnumSet.hpp" #include "database/ArtistId.hpp" #include "database/DirectoryId.hpp" -#include "database/ReleaseId.hpp" +#include "database/MediaLibraryId.hpp" #include "database/Object.hpp" +#include "database/ReleaseId.hpp" #include "database/Types.hpp" namespace lms::db { class Session; + class MediaLibrary; class Directory final : public Object { @@ -52,6 +54,7 @@ namespace lms::db core::EnumSet trackArtistLinkTypes; // and for these link types DirectoryId parentDirectory; // If set, directories that have this parent bool withNoTrack{}; // If set, directories that do not contain any track + MediaLibraryId mediaLibrary; // If set, directories in this library FindParameters& setRange(std::optional _range) { @@ -84,6 +87,11 @@ namespace lms::db withNoTrack = _withNoTrack; return *this; } + FindParameters& setMediaLibrary(MediaLibraryId _mediaLibrary) + { + mediaLibrary = _mediaLibrary; + return *this; + } }; // find @@ -100,10 +108,12 @@ namespace lms::db const std::filesystem::path& getAbsolutePath() const { return _absolutePath; } std::string_view getName() const { return _name; } ObjectPtr getParentDirectory() const { return _parent; } + ObjectPtr getMediaLibrary() const { return _mediaLibrary; } // setters void setAbsolutePath(const std::filesystem::path& p); void setParent(ObjectPtr parent); + void setMediaLibrary(ObjectPtr mediaLibrary) { _mediaLibrary = getDboPtr(mediaLibrary); } template void persist(Action& a) @@ -112,6 +122,7 @@ namespace lms::db Wt::Dbo::field(a, _name, "name"); Wt::Dbo::belongsTo(a, _parent, "parent_directory", Wt::Dbo::OnDeleteCascade); + Wt::Dbo::belongsTo(a, _mediaLibrary, "media_library", Wt::Dbo::OnDeleteSetNull); // don't delete directories on media library removal, we want to wait for the next scan to have a chance to migrate files } private: @@ -123,5 +134,6 @@ namespace lms::db std::string _name; Wt::Dbo::ptr _parent; + Wt::Dbo::ptr _mediaLibrary; }; } // namespace lms::db diff --git a/src/libs/services/scanner/impl/ScanStepAssociateArtistImages.cpp b/src/libs/services/scanner/impl/ScanStepAssociateArtistImages.cpp index 66d8ba0e..26c1bc0b 100644 --- a/src/libs/services/scanner/impl/ScanStepAssociateArtistImages.cpp +++ b/src/libs/services/scanner/impl/ScanStepAssociateArtistImages.cpp @@ -205,6 +205,9 @@ namespace lms::scanner void ScanStepAssociateArtistImages::process(ScanContext& context) { + if (_abortScan) + return; + if (context.stats.nbChanges() == 0) return; @@ -224,6 +227,9 @@ namespace lms::scanner ArtistImageAssociationContainer artistImageAssociations; while (fetchNextArtistImagesToUpdate(searchContext, artistImageAssociations)) { + if (_abortScan) + return; + updateArtistImages(session, artistImageAssociations); context.currentStepStats.processedElems += readBatchSize; _progressCallback(context.currentStepStats); diff --git a/src/libs/services/scanner/impl/ScanStepScanFiles.cpp b/src/libs/services/scanner/impl/ScanStepScanFiles.cpp index ffaa9c51..a59342e9 100644 --- a/src/libs/services/scanner/impl/ScanStepScanFiles.cpp +++ b/src/libs/services/scanner/impl/ScanStepScanFiles.cpp @@ -104,18 +104,20 @@ namespace lms::scanner return res; } - Directory::pointer getOrCreateDirectory(Session& session, const std::filesystem::path& path, const std::filesystem::path& rootPath) + Directory::pointer getOrCreateDirectory(Session& session, const std::filesystem::path& path, const MediaLibrary::pointer& mediaLibrary) { Directory::pointer directory{ Directory::find(session, path) }; if (!directory) { Directory::pointer parentDirectory; - if (path != rootPath) - parentDirectory = getOrCreateDirectory(session, path.parent_path(), rootPath); + if (path != mediaLibrary->getPath()) + parentDirectory = getOrCreateDirectory(session, path.parent_path(), mediaLibrary); directory = session.create(path); directory.modify()->setParent(parentDirectory); + directory.modify()->setMediaLibrary(mediaLibrary); } + // Don't update library if it does not match, will be updated elsewhere return directory; } @@ -350,12 +352,13 @@ namespace lms::scanner std::vector scanResults; + std::filesystem::path currentDirectory; core::pathUtils::exploreFilesRecursive( mediaLibrary.rootDirectory, [&](std::error_code ec, const std::filesystem::path& path) { LMS_SCOPED_TRACE_DETAILED("Scanner", "OnExploreFile"); if (_abortScan) - return false; + return false; // stop iterating if (ec) { @@ -364,22 +367,33 @@ namespace lms::scanner } else { - bool fileToProcess{}; + bool fileMatched{}; if (core::pathUtils::hasFileAnyExtension(path, _settings.supportedAudioFileExtensions)) { - fileToProcess = true; + fileMatched = true; if (checkAudioFileNeedScan(context, path, mediaLibrary)) _fileScanQueue.pushScanRequest(path, FileScanQueue::ScanRequestType::AudioFile); } else if (core::pathUtils::hasFileAnyExtension(path, _settings.supportedImageFileExtensions)) { - fileToProcess = true; + fileMatched = true; if (checkImageFileNeedScan(context, path)) _fileScanQueue.pushScanRequest(path, FileScanQueue::ScanRequestType::ImageFile); } - if (fileToProcess) + if (fileMatched) { + // Not very efficient way to update media_library for directories + if (path.has_parent_path()) + { + const std::filesystem::path directory{ path.parent_path() }; + if (directory != currentDirectory) + { + updateDirectoryIfNeeded(currentDirectory, mediaLibrary); + currentDirectory = directory; + } + } + context.currentStepStats.processedElems++; _progressCallback(context.currentStepStats); } @@ -415,17 +429,19 @@ namespace lms::scanner return false; } + if (context.scanOptions.fullScan) + return true; + bool needUpdateLibrary{}; - if (!context.scanOptions.fullScan) + db::Session& dbSession{ _db.getTLSSession() }; + { + auto transaction{ dbSession.createReadTransaction() }; + // Skip file if last write is the same - db::Session& dbSession{ _db.getTLSSession() }; - auto transaction{ _db.getTLSSession().createReadTransaction() }; - const Track::pointer track{ Track::findByPath(dbSession, file) }; - if (track - && track->getLastWriteTime().toTime_t() == lastWriteTime.toTime_t() + && track->getLastWriteTime() == lastWriteTime && track->getScanVersion() == _settings.scanVersion) { // this file may have been moved from one library to another, then we just need to update the media library id instead of a full rescan @@ -442,8 +458,7 @@ namespace lms::scanner if (needUpdateLibrary) { - db::Session& dbSession{ _db.getTLSSession() }; - auto transaction{ _db.getTLSSession().createWriteTransaction() }; + auto transaction{ dbSession.createWriteTransaction() }; Track::pointer track{ Track::findByPath(dbSession, file) }; assert(track); @@ -632,8 +647,9 @@ namespace lms::scanner track.modify()->setFileSize(fileInfo->fileSize); track.modify()->setLastWriteTime(fileInfo->lastWriteTime); - track.modify()->setMediaLibrary(MediaLibrary::find(dbSession, libraryInfo.id)); // may be null if settings are updated in // => next scan will correct this - track.modify()->setDirectory(getOrCreateDirectory(dbSession, file.parent_path(), libraryInfo.rootDirectory)); + MediaLibrary::pointer mediaLibrary{ MediaLibrary::find(dbSession, libraryInfo.id) }; // may be null if settings are updated in // => next scan will correct this + track.modify()->setMediaLibrary(mediaLibrary); + track.modify()->setDirectory(getOrCreateDirectory(dbSession, file.parent_path(), mediaLibrary)); track.modify()->clearArtistLinks(); // Do not fallback on artists with the same name but having a MBID for artist and releaseArtists, as it may be corrected by properly tagging files @@ -762,7 +778,8 @@ namespace lms::scanner image.modify()->setFileSize(fileInfo->fileSize); image.modify()->setHeight(imageInfo->height); image.modify()->setWidth(imageInfo->width); - image.modify()->setDirectory(getOrCreateDirectory(dbSession, file.parent_path(), libraryInfo.rootDirectory)); + MediaLibrary::pointer mediaLibrary{ MediaLibrary::find(dbSession, libraryInfo.id) }; // may be null if settings are updated in // => next scan will correct this + image.modify()->setDirectory(getOrCreateDirectory(dbSession, file.parent_path(), mediaLibrary)); if (added) { @@ -775,4 +792,34 @@ namespace lms::scanner stats.updates++; } } + + void ScanStepScanFiles::updateDirectoryIfNeeded(const std::filesystem::path& dirPath, const ScannerSettings::MediaLibraryInfo& libraryInfo) + { + db::Session& dbSession{ _db.getTLSSession() }; + + const bool needUpdateLibrary{ [&] { + auto transaction{ dbSession.createReadTransaction() }; + + Directory::pointer directory{ Directory::find(dbSession, dirPath) }; + if (!directory) + return false; // we create directories only of we find images or tracks inside + + MediaLibrary::pointer currentLibrary{ directory->getMediaLibrary() }; + return !currentLibrary || currentLibrary->getId() != libraryInfo.id; + }() }; + + if (needUpdateLibrary) + { + auto transaction{ dbSession.createWriteTransaction() }; + + Directory::pointer directory{ Directory::find(dbSession, dirPath) }; + assert(directory); + directory.modify()->setMediaLibrary(MediaLibrary::find(dbSession, libraryInfo.id)); // may be null if settings are updated in // => next scan will correct this + } + + if (dirPath != libraryInfo.rootDirectory && dirPath.has_parent_path()) + { + updateDirectoryIfNeeded(dirPath.parent_path(), libraryInfo); + } + } } // namespace lms::scanner diff --git a/src/libs/services/scanner/impl/ScanStepScanFiles.hpp b/src/libs/services/scanner/impl/ScanStepScanFiles.hpp index fd9687d9..6c94d21a 100644 --- a/src/libs/services/scanner/impl/ScanStepScanFiles.hpp +++ b/src/libs/services/scanner/impl/ScanStepScanFiles.hpp @@ -49,6 +49,8 @@ namespace lms::scanner void processAudioFileScanData(ScanContext& context, const std::filesystem::path& path, const metadata::Track* trackMetadata, const ScannerSettings::MediaLibraryInfo& libraryInfo); void processImageFileScanData(ScanContext& context, const std::filesystem::path& path, const ImageInfo* imageInfo, const ScannerSettings::MediaLibraryInfo& libraryInfo); + void updateDirectoryIfNeeded(const std::filesystem::path& directory, const ScannerSettings::MediaLibraryInfo& libraryInfo); + std::unique_ptr _metadataParser; const std::vector _extraTagsToParse; diff --git a/src/libs/subsonic/impl/entrypoints/Searching.cpp b/src/libs/subsonic/impl/entrypoints/Searching.cpp index a5087945..e0337491 100644 --- a/src/libs/subsonic/impl/entrypoints/Searching.cpp +++ b/src/libs/subsonic/impl/entrypoints/Searching.cpp @@ -116,7 +116,7 @@ namespace lms::api::subsonic void findRequestedArtistDirectories(RequestContext& context, const std::vector& keywords, MediaLibraryId mediaLibrary, Response::Node& searchResultNode) { - // For now, no need to "accelerate" all this + // For now, no need to optimize all this // Find all the directories that match the name and that do not contain any track (considered by the legacy API as artists) const std::size_t artistCount{ getParameterAs(context.parameters, "artistCount").value_or(20) }; if (artistCount == 0) @@ -126,12 +126,12 @@ namespace lms::api::subsonic throw ParameterValueTooHighGenericError{ "artistCount", defaultMaxCountSize }; const std::size_t artistOffset{ getParameterAs(context.parameters, "artistOffset").value_or(0) }; - + Directory::FindParameters params; params.setKeywords(keywords); - params.setRange(Range{artistOffset, artistCount}); + params.setRange(Range{ artistOffset, artistCount }); params.setWithNoTrack(true); - // TODO media + params.setMediaLibrary(mediaLibrary); Directory::find(context.dbSession, params, [&](const Directory::pointer& directory) { Response::Node childNode; @@ -333,10 +333,7 @@ namespace lms::api::subsonic } } } - } // namespace - namespace - { Response handleSearchRequestCommon(RequestContext& context, bool id3) { // Mandatory params From cbc387b8b76eb6629d2a318bc3802f424e3e714f Mon Sep 17 00:00:00 2001 From: emeric Date: Sat, 13 Jul 2024 19:02:58 +0200 Subject: [PATCH 04/11] Fixed format --- src/libs/database/impl/Directory.cpp | 2 +- src/libs/subsonic/impl/entrypoints/Searching.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/database/impl/Directory.cpp b/src/libs/database/impl/Directory.cpp index f994cc0c..e618c573 100644 --- a/src/libs/database/impl/Directory.cpp +++ b/src/libs/database/impl/Directory.cpp @@ -52,7 +52,7 @@ namespace lms::db if (params.release.isValid()) query.where("t.release_id = ?").bind(params.release); - + if (params.artist.isValid()) { query.join("artist a ON a.id = t_a_l.artist_id") diff --git a/src/libs/subsonic/impl/entrypoints/Searching.cpp b/src/libs/subsonic/impl/entrypoints/Searching.cpp index e0337491..fb9334cf 100644 --- a/src/libs/subsonic/impl/entrypoints/Searching.cpp +++ b/src/libs/subsonic/impl/entrypoints/Searching.cpp @@ -131,7 +131,7 @@ namespace lms::api::subsonic params.setKeywords(keywords); params.setRange(Range{ artistOffset, artistCount }); params.setWithNoTrack(true); - params.setMediaLibrary(mediaLibrary); + params.setMediaLibrary(mediaLibrary); Directory::find(context.dbSession, params, [&](const Directory::pointer& directory) { Response::Node childNode; From a2a2041f49d1d5612ff22afddf5db97dac2b02a4 Mon Sep 17 00:00:00 2001 From: emeric Date: Sat, 27 Jul 2024 14:27:37 +0200 Subject: [PATCH 05/11] Optimized library assignation to directories --- SUBSONIC.md | 7 +- approot/messages.xml | 1 + approot/messages_fr.xml | 1 + approot/messages_it.xml | 1 + approot/messages_pl.xml | 1 + approot/messages_zh.xml | 1 + src/libs/database/impl/Directory.cpp | 30 +++++++ .../database/include/database/Directory.hpp | 1 + src/libs/database/test/Directory.cpp | 33 +++++++ src/libs/services/scanner/CMakeLists.txt | 1 + .../scanner/impl/ScanStepScanFiles.cpp | 41 --------- .../scanner/impl/ScanStepScanFiles.hpp | 2 - .../impl/ScanStepUpdateLibraryFields.cpp | 87 +++++++++++++++++++ .../impl/ScanStepUpdateLibraryFields.hpp | 41 +++++++++ .../services/scanner/impl/ScannerService.cpp | 6 +- .../include/services/scanner/ScannerStats.hpp | 3 +- src/lms/ui/admin/ScannerController.cpp | 7 +- 17 files changed, 215 insertions(+), 49 deletions(-) create mode 100644 src/libs/services/scanner/impl/ScanStepUpdateLibraryFields.cpp create mode 100644 src/libs/services/scanner/impl/ScanStepUpdateLibraryFields.hpp diff --git a/SUBSONIC.md b/SUBSONIC.md index 1361440c..25f246cd 100644 --- a/SUBSONIC.md +++ b/SUBSONIC.md @@ -1,9 +1,12 @@ # Subsonic API The API version implemented is 1.16.0 and has been tested on _Android_ using _Subsonic Player_, _Ultrasonic_, _Symfonium_, and _DSub_. -Since _LMS_ uses metadata tags to organize music, a compatibility mode is used to browse the collection when using the directory browsing commands. + +Folder navigation commands are supported. However, since _LMS_ does not store information for each folder, it is not possible to star/unstar folders considered as artists. +Given the API limitations of folder navigation commands, it is recommended to place all tracks of an album in the same folder and not to mix multiple albums in the same folder. + The Subsonic API is enabled by default. -__Note__: since _LMS_ may store hashed and salted passwords or may forward authentication requests to external services, it cannot handle the __token authentication__ method. You may need to check your client to make sure to use the __password__ authentication method. +__Note__: since _LMS_ may store hashed and salted passwords or may forward authentication requests to external services, it cannot handle the __token authentication__ method. You may need to check your client to make sure to use the __password__ authentication method. Since logins/passwords are passed in plain text through URLs, it is highly recommended to use a unique password when using the Subsonic API. Note that this may affect the use of authentication via PAM. In any case, ensure that read access to the web server logs (and to the proxy, if applicable) is well protected. # OpenSubsonic API OpenSubsonic is an initiative to patch and extend the legacy Subsonic API. You'll find more details in the [official documentation](https://opensubsonic.netlify.app/) diff --git a/approot/messages.xml b/approot/messages.xml index d1adae65..2cfb7606 100644 --- a/approot/messages.xml +++ b/approot/messages.xml @@ -121,6 +121,7 @@ Removing orphaned entries: {1} entries... Scanning files: {1}/{2} ({3}%)... Step status +Updating library fields: {1} entries Export traces diff --git a/approot/messages_fr.xml b/approot/messages_fr.xml index d07e92ec..1f216c86 100644 --- a/approot/messages_fr.xml +++ b/approot/messages_fr.xml @@ -121,6 +121,7 @@ Retrait des entrées orphelines: {1} entrées... Scan des fichiers : {1}/{2} ({3}%)... Statut de l'étape +Mise à jour des champs des bibliothèques: {1} entrées Exporter les traces diff --git a/approot/messages_it.xml b/approot/messages_it.xml index 7f8b7fa6..a1daeb59 100644 --- a/approot/messages_it.xml +++ b/approot/messages_it.xml @@ -121,6 +121,7 @@ Rimozione voci orfane: {1} voci... Scansione dei file: {1}/{2} ({3}%)... Stato passo +Aggiornamento dei campi della libreria: {1} voci Esporta tracce diff --git a/approot/messages_pl.xml b/approot/messages_pl.xml index bf83a8af..a28bcb89 100644 --- a/approot/messages_pl.xml +++ b/approot/messages_pl.xml @@ -138,6 +138,7 @@ Usuwanie osieroconych wpisów: {1} wpisów... Skanowanie plików: {1}/{2} ({3}%)... Obecny krok +Aktualizowanie pól biblioteki: {1} wpisów Eksportuj ślady diff --git a/approot/messages_zh.xml b/approot/messages_zh.xml index c8608334..97f1b500 100644 --- a/approot/messages_zh.xml +++ b/approot/messages_zh.xml @@ -122,6 +122,7 @@ 扫描文件中: {1}/{2} 个文件 ({3}%)... 当前步骤状态 + diff --git a/src/libs/database/impl/Directory.cpp b/src/libs/database/impl/Directory.cpp index e618c573..726b15e6 100644 --- a/src/libs/database/impl/Directory.cpp +++ b/src/libs/database/impl/Directory.cpp @@ -83,6 +83,25 @@ namespace lms::db return query; } + + std::filesystem::path getPathWithTrailingSeparator(const std::filesystem::path& path) + { + if (path.empty()) + return path; + + // Convert the path to string + std::string pathStr{ path.string() }; + + // Check if the last character is a directory separator + if (pathStr.back() != std::filesystem::path::preferred_separator) + { + // If not, add the preferred separator + pathStr += std::filesystem::path::preferred_separator; + } + + // Return the new path + return std::filesystem::path{ pathStr }; + } } // namespace Directory::Directory(const std::filesystem::path& p) @@ -157,6 +176,17 @@ namespace lms::db return utils::execRangeQuery(query, range); } + RangeResults Directory::findMismatchedLibrary(Session& session, std::optional range, const std::filesystem::path& rootPath, MediaLibraryId expectedLibraryId) + { + session.checkReadTransaction(); + + auto query{ session.getDboSession()->query("SELECT d.id FROM directory d") }; + query.where("d.absolute_path = ? OR d.absolute_path LIKE ?").bind(rootPath).bind(getPathWithTrailingSeparator(rootPath).string() + "%"); + query.where("d.media_library_id <> ? OR d.media_library_id IS NULL").bind(expectedLibraryId); + + return utils::execRangeQuery(query, range); + } + RangeResults Directory::findRootDirectories(Session& session, std::optional range) { auto query{ session.getDboSession()->query>("SELECT d from directory d").where("d.parent_directory_id IS NULL") }; diff --git a/src/libs/database/include/database/Directory.hpp b/src/libs/database/include/database/Directory.hpp index ca74e156..62380e8d 100644 --- a/src/libs/database/include/database/Directory.hpp +++ b/src/libs/database/include/database/Directory.hpp @@ -102,6 +102,7 @@ namespace lms::db static RangeResults find(Session& session, const FindParameters& params); static void find(Session& session, const FindParameters& parameters, const std::function& func); static RangeResults findOrphanIds(Session& session, std::optional range = std::nullopt); + static RangeResults findMismatchedLibrary(Session& session, std::optional range, const std::filesystem::path& rootPath, MediaLibraryId expectedLibraryId); static RangeResults findRootDirectories(Session& session, std::optional range = std::nullopt); // getters diff --git a/src/libs/database/test/Directory.cpp b/src/libs/database/test/Directory.cpp index 9e3be53f..ddfc7455 100644 --- a/src/libs/database/test/Directory.cpp +++ b/src/libs/database/test/Directory.cpp @@ -252,4 +252,37 @@ namespace lms::db::tests EXPECT_EQ(res[1]->getId(), child2.getId()); } } + + TEST_F(DatabaseFixture, Directory_findMismatchedLibrary) + { + ScopedDirectory parent1{ session, "/root" }; + ScopedDirectory child1{ session, "/root/foo" }; + ScopedDirectory parent2{ session, "/root_1" }; + ScopedDirectory child2{ session, "/root_1/foo" }; + + ScopedMediaLibrary library{ session, "/root" }; + + { + auto transaction{ session.createReadTransaction() }; + + const auto res{ Directory::findMismatchedLibrary(session, std::nullopt, library->getPath(), library->getId()).results }; + ASSERT_EQ(res.size(), 2); + EXPECT_EQ(res[0], parent1.getId()); + EXPECT_EQ(res[1], child1.getId()); + } + + { + auto transaction{ session.createWriteTransaction() }; + + parent1.get().modify()->setMediaLibrary(library.get()); + child1.get().modify()->setMediaLibrary(library.get()); + } + + { + auto transaction{ session.createReadTransaction() }; + + const auto res{ Directory::findMismatchedLibrary(session, std::nullopt, library->getPath(), library->getId()).results }; + EXPECT_EQ(res.size(), 0); + } + } } // namespace lms::db::tests \ No newline at end of file diff --git a/src/libs/services/scanner/CMakeLists.txt b/src/libs/services/scanner/CMakeLists.txt index 41df9cc9..00af758d 100644 --- a/src/libs/services/scanner/CMakeLists.txt +++ b/src/libs/services/scanner/CMakeLists.txt @@ -12,6 +12,7 @@ add_library(lmsscanner SHARED impl/ScanStepOptimize.cpp impl/ScanStepRemoveOrphanedDbEntries.cpp impl/ScanStepScanFiles.cpp + impl/ScanStepUpdateLibraryFields.cpp ) target_include_directories(lmsscanner INTERFACE diff --git a/src/libs/services/scanner/impl/ScanStepScanFiles.cpp b/src/libs/services/scanner/impl/ScanStepScanFiles.cpp index a59342e9..a49d923b 100644 --- a/src/libs/services/scanner/impl/ScanStepScanFiles.cpp +++ b/src/libs/services/scanner/impl/ScanStepScanFiles.cpp @@ -383,17 +383,6 @@ namespace lms::scanner if (fileMatched) { - // Not very efficient way to update media_library for directories - if (path.has_parent_path()) - { - const std::filesystem::path directory{ path.parent_path() }; - if (directory != currentDirectory) - { - updateDirectoryIfNeeded(currentDirectory, mediaLibrary); - currentDirectory = directory; - } - } - context.currentStepStats.processedElems++; _progressCallback(context.currentStepStats); } @@ -792,34 +781,4 @@ namespace lms::scanner stats.updates++; } } - - void ScanStepScanFiles::updateDirectoryIfNeeded(const std::filesystem::path& dirPath, const ScannerSettings::MediaLibraryInfo& libraryInfo) - { - db::Session& dbSession{ _db.getTLSSession() }; - - const bool needUpdateLibrary{ [&] { - auto transaction{ dbSession.createReadTransaction() }; - - Directory::pointer directory{ Directory::find(dbSession, dirPath) }; - if (!directory) - return false; // we create directories only of we find images or tracks inside - - MediaLibrary::pointer currentLibrary{ directory->getMediaLibrary() }; - return !currentLibrary || currentLibrary->getId() != libraryInfo.id; - }() }; - - if (needUpdateLibrary) - { - auto transaction{ dbSession.createWriteTransaction() }; - - Directory::pointer directory{ Directory::find(dbSession, dirPath) }; - assert(directory); - directory.modify()->setMediaLibrary(MediaLibrary::find(dbSession, libraryInfo.id)); // may be null if settings are updated in // => next scan will correct this - } - - if (dirPath != libraryInfo.rootDirectory && dirPath.has_parent_path()) - { - updateDirectoryIfNeeded(dirPath.parent_path(), libraryInfo); - } - } } // namespace lms::scanner diff --git a/src/libs/services/scanner/impl/ScanStepScanFiles.hpp b/src/libs/services/scanner/impl/ScanStepScanFiles.hpp index 6c94d21a..fd9687d9 100644 --- a/src/libs/services/scanner/impl/ScanStepScanFiles.hpp +++ b/src/libs/services/scanner/impl/ScanStepScanFiles.hpp @@ -49,8 +49,6 @@ namespace lms::scanner void processAudioFileScanData(ScanContext& context, const std::filesystem::path& path, const metadata::Track* trackMetadata, const ScannerSettings::MediaLibraryInfo& libraryInfo); void processImageFileScanData(ScanContext& context, const std::filesystem::path& path, const ImageInfo* imageInfo, const ScannerSettings::MediaLibraryInfo& libraryInfo); - void updateDirectoryIfNeeded(const std::filesystem::path& directory, const ScannerSettings::MediaLibraryInfo& libraryInfo); - std::unique_ptr _metadataParser; const std::vector _extraTagsToParse; diff --git a/src/libs/services/scanner/impl/ScanStepUpdateLibraryFields.cpp b/src/libs/services/scanner/impl/ScanStepUpdateLibraryFields.cpp new file mode 100644 index 00000000..3520e7f4 --- /dev/null +++ b/src/libs/services/scanner/impl/ScanStepUpdateLibraryFields.cpp @@ -0,0 +1,87 @@ +/* + * Copyright (C) 2023 Emeric Poupon + * + * This file is part of LMS. + * + * LMS is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * LMS is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with LMS. If not, see . + */ + +#include "ScanStepUpdateLibraryFields.hpp" + +#include "core/ILogger.hpp" +#include "core/Path.hpp" +#include "database/Db.hpp" +#include "database/Directory.hpp" +#include "database/MediaLibrary.hpp" +#include "database/Session.hpp" + +namespace lms::scanner +{ + + void ScanStepUpdateLibraryFields::process(ScanContext& context) + { + processDirectories(context); + } + + void ScanStepUpdateLibraryFields::processDirectories(ScanContext& context) + { + for (const ScannerSettings::MediaLibraryInfo& mediaLibrary : _settings.mediaLibraries) + { + if (_abortScan) + break; + + processDirectory(context, mediaLibrary); + } + } + + void ScanStepUpdateLibraryFields::processDirectory(ScanContext& context, const ScannerSettings::MediaLibraryInfo& mediaLibrary) + { + db::Session& session{ _db.getTLSSession() }; + + constexpr std::size_t batchSize = 100; + + db::RangeResults entries; + while (!_abortScan) + { + { + auto transaction{ session.createReadTransaction() }; + + entries = db::Directory::findMismatchedLibrary(session, db::Range{ 0, batchSize }, mediaLibrary.rootDirectory, mediaLibrary.id); + }; + + if (entries.results.empty()) + break; + + { + auto transaction{ session.createWriteTransaction() }; + + db::MediaLibrary::pointer library{ db::MediaLibrary::find(session, mediaLibrary.id) }; + if (!library) // may be legit + break; + + for (const db::DirectoryId directoryId : entries.results) + { + if (_abortScan) + break; + + db::Directory::pointer directory{ db::Directory::find(session, directoryId) }; + directory.modify()->setMediaLibrary(library); + } + } + + context.currentStepStats.processedElems += entries.results.size(); + _progressCallback(context.currentStepStats); + } + } +} // namespace lms::scanner diff --git a/src/libs/services/scanner/impl/ScanStepUpdateLibraryFields.hpp b/src/libs/services/scanner/impl/ScanStepUpdateLibraryFields.hpp new file mode 100644 index 00000000..6bdfc9b2 --- /dev/null +++ b/src/libs/services/scanner/impl/ScanStepUpdateLibraryFields.hpp @@ -0,0 +1,41 @@ +/* + * Copyright (C) 2023 Emeric Poupon + * + * This file is part of LMS. + * + * LMS is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * LMS is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with LMS. If not, see . + */ + +#pragma once + +#include "database/DirectoryId.hpp" + +#include "ScanStepBase.hpp" + +namespace lms::scanner +{ + class ScanStepUpdateLibraryFields : public ScanStepBase + { + public: + using ScanStepBase::ScanStepBase; + + private: + core::LiteralString getStepName() const override { return "Update Library fields"; } + ScanStep getStep() const override { return ScanStep::UpdateLibraryFields; } + void process(ScanContext& context) override; + + void processDirectories(ScanContext& context); + void processDirectory(ScanContext& context, const ScannerSettings::MediaLibraryInfo& mediaLibrary); + }; +} // namespace lms::scanner diff --git a/src/libs/services/scanner/impl/ScannerService.cpp b/src/libs/services/scanner/impl/ScannerService.cpp index 321116f6..e9810f89 100644 --- a/src/libs/services/scanner/impl/ScannerService.cpp +++ b/src/libs/services/scanner/impl/ScannerService.cpp @@ -40,6 +40,7 @@ #include "ScanStepOptimize.hpp" #include "ScanStepRemoveOrphanedDbEntries.hpp" #include "ScanStepScanFiles.hpp" +#include "ScanStepUpdateLibraryFields.hpp" namespace lms::scanner { @@ -279,7 +280,7 @@ namespace lms::scanner LMS_SCOPED_TRACE_OVERVIEW("Scanner", scanStep->getStepName()); LMS_LOG(DBUPDATER, DEBUG, "Starting scan step '" << scanStep->getStepName() << "'"); - scanContext.currentStepStats = ScanStepStats{ .startTime = Wt::WDateTime::currentDateTime(), .stepIndex = stepIndex++, .currentStep = scanStep->getStep() }; + scanContext.currentStepStats = ScanStepStats{ .startTime = Wt::WDateTime::currentDateTime(), .stepCount = _scanSteps.size(), .stepIndex = stepIndex++, .currentStep = scanStep->getStep() }; notifyInProgress(scanContext.currentStepStats); scanStep->process(scanContext); @@ -339,11 +340,12 @@ namespace lms::scanner _db }; - // Order is important + // Order is important, steps are sequential _scanSteps.clear(); _scanSteps.push_back(std::make_unique(params)); _scanSteps.push_back(std::make_unique(params)); _scanSteps.push_back(std::make_unique(params)); + _scanSteps.push_back(std::make_unique(params)); _scanSteps.push_back(std::make_unique(params)); _scanSteps.push_back(std::make_unique(params)); _scanSteps.push_back(std::make_unique(params)); diff --git a/src/libs/services/scanner/include/services/scanner/ScannerStats.hpp b/src/libs/services/scanner/include/services/scanner/ScannerStats.hpp index 22ce1d1e..f92a6c19 100644 --- a/src/libs/services/scanner/include/services/scanner/ScannerStats.hpp +++ b/src/libs/services/scanner/include/services/scanner/ScannerStats.hpp @@ -72,14 +72,15 @@ namespace lms::scanner ReloadSimilarityEngine, RemoveOrphanedDbEntries, ScanFiles, + UpdateLibraryFields, }; - static inline constexpr unsigned ScanProgressStepCount{ 11 }; // reduced scan stats struct ScanStepStats { Wt::WDateTime startTime; + std::size_t stepCount{}; std::size_t stepIndex{}; ScanStep currentStep; diff --git a/src/lms/ui/admin/ScannerController.cpp b/src/lms/ui/admin/ScannerController.cpp index 4ccf7fda..42f5b0d8 100644 --- a/src/lms/ui/admin/ScannerController.cpp +++ b/src/lms/ui/admin/ScannerController.cpp @@ -256,7 +256,7 @@ namespace lms::ui _status->setText(Wt::WString::tr("Lms.Admin.ScannerController.status-in-progress") .arg(status.currentScanStepStats->stepIndex + 1) - .arg(scanner::ScanProgressStepCount)); + .arg(status.currentScanStepStats->stepCount)); refreshCurrentStep(*status.currentScanStepStats); break; @@ -326,6 +326,11 @@ namespace lms::ui .arg(stepStats.totalElems) .arg(stepStats.progress())); break; + + case ScanStep::UpdateLibraryFields: + _stepStatus->setText(Wt::WString::tr("Lms.Admin.ScannerController.step-updating-library-fields") + .arg(stepStats.processedElems)); + break; } } } // namespace lms::ui From 9bca05fb51cce54c340aa7a609413c63faa5aa4e Mon Sep 17 00:00:00 2001 From: emeric Date: Sat, 27 Jul 2024 18:03:58 +0200 Subject: [PATCH 06/11] Display disc subtitles if it is set, even for single disc releases, fixes #492 --- src/libs/database/impl/Release.cpp | 5 +++++ src/libs/database/include/database/Release.hpp | 1 + src/lms/ui/explore/ReleaseView.cpp | 7 +++++-- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/libs/database/impl/Release.cpp b/src/libs/database/impl/Release.cpp index a597de86..e758c1da 100644 --- a/src/libs/database/impl/Release.cpp +++ b/src/libs/database/impl/Release.cpp @@ -514,6 +514,11 @@ namespace lms::db return getArtists().size() > 1; } + bool Release::hasDiscSubtitle() const + { + return utils::fetchQuerySingleResult(session()->query("SELECT EXISTS (SELECT 1 FROM track WHERE disc_subtitle IS NOT NULL AND disc_subtitle <> '' AND release_id = ?)").bind(getId())); + } + std::size_t Release::getTrackCount() const { assert(session()); diff --git a/src/libs/database/include/database/Release.hpp b/src/libs/database/include/database/Release.hpp index 371df127..c402bf40 100644 --- a/src/libs/database/include/database/Release.hpp +++ b/src/libs/database/include/database/Release.hpp @@ -218,6 +218,7 @@ namespace lms::db std::vector> getReleaseArtists() const { return getArtists(TrackArtistLinkType::ReleaseArtist); } bool hasVariousArtists() const; std::vector getSimilarReleases(std::optional offset = {}, std::optional count = {}) const; + bool hasDiscSubtitle() const; template void persist(Action& a) diff --git a/src/lms/ui/explore/ReleaseView.cpp b/src/lms/ui/explore/ReleaseView.cpp index d58f2bfa..839c9bb2 100644 --- a/src/lms/ui/explore/ReleaseView.cpp +++ b/src/lms/ui/explore/ReleaseView.cpp @@ -321,7 +321,8 @@ namespace lms::ui const bool variousArtists{ release->hasVariousArtists() }; const auto totalDisc{ release->getTotalDisc() }; const std::size_t discCount{ release->getDiscCount() }; - const bool isReleaseMultiDisc{ (discCount > 1) || (totalDisc && *totalDisc > 1) }; + const bool hasDiscSubtitle{ release->hasDiscSubtitle() }; + const bool useSubtitleContainers{ (discCount > 1) || (totalDisc && *totalDisc > 1) || hasDiscSubtitle }; // Expect to be called in asc order std::map trackContainers; @@ -392,8 +393,10 @@ namespace lms::ui const auto discNumber{ track->getDiscNumber() }; Wt::WContainerWidget* container; - if (isReleaseMultiDisc && discNumber) + if (useSubtitleContainers && discNumber) container = getOrAddDiscContainer(*discNumber, track->getDiscSubtitle()); + else if (hasDiscSubtitle && !discNumber) + container = getOrAddDiscContainer(0, track->getDiscSubtitle()); else container = getOrAddNoDiscContainer(); From b3807f6a53bed5d093c78cd19c5da913a49a02c4 Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 28 Jul 2024 11:24:18 +0200 Subject: [PATCH 07/11] Made length checks more robusts + bumped sizes, fixes #496 --- src/libs/database/impl/Artist.cpp | 20 ++++-- src/libs/database/impl/Cluster.cpp | 14 +++- src/libs/database/impl/Release.cpp | 8 ++- src/libs/database/impl/Track.cpp | 21 ++++++ src/libs/database/include/database/Artist.hpp | 6 +- .../database/include/database/Cluster.hpp | 8 +-- .../database/include/database/Release.hpp | 4 +- src/libs/database/include/database/Track.hpp | 14 ++-- src/libs/database/include/database/Types.hpp | 8 +++ src/libs/database/include/database/User.hpp | 4 +- src/libs/database/test/Cluster.cpp | 70 ++++++++++++++++++- src/lms/ui/common/LoginNameValidator.cpp | 4 +- 12 files changed, 151 insertions(+), 30 deletions(-) diff --git a/src/libs/database/impl/Artist.cpp b/src/libs/database/impl/Artist.cpp index 94125bd3..64d1a03c 100644 --- a/src/libs/database/impl/Artist.cpp +++ b/src/libs/database/impl/Artist.cpp @@ -183,10 +183,10 @@ namespace lms::db } // namespace Artist::Artist(const std::string& name, const std::optional& MBID) - : _name{ std::string(name, 0, _maxNameLength) } - , _sortName{ _name } - , _MBID{ MBID ? MBID->getAsString() : "" } + : _MBID{ MBID ? MBID->getAsString() : "" } { + setName(name); + _sortName = _name; } Artist::pointer Artist::create(Session& session, const std::string& name, const std::optional& MBID) @@ -360,9 +360,19 @@ namespace lms::db return res; } - void Artist::setSortName(const std::string& sortName) + void Artist::setName(std::string_view name) { - _sortName = std::string(sortName, 0, _maxNameLength); + _name.assign(name, 0, _maxNameLength); + if (name.size() > _maxNameLength) + LMS_LOG(DB, WARNING, "Artist name too long, truncated to '" << _name << "'"); + } + + void Artist::setSortName(std::string_view sortName) + { + _sortName.assign(sortName, 0, _maxNameLength); + + if (sortName.size() > _maxNameLength) + LMS_LOG(DB, WARNING, "Artist sort name too long, truncated to '" << _sortName << "'"); } void Artist::setImage(ObjectPtr image) diff --git a/src/libs/database/impl/Cluster.cpp b/src/libs/database/impl/Cluster.cpp index af2cabb6..f59f9e59 100644 --- a/src/libs/database/impl/Cluster.cpp +++ b/src/libs/database/impl/Cluster.cpp @@ -96,9 +96,12 @@ namespace lms::db } // namespace Cluster::Cluster(ObjectPtr type, std::string_view name) - : _name{ std::string{ name, 0, _maxNameLength } } + : _name{ name } , _clusterType{ getDboPtr(type) } { + // As we use the name to uniquely identify clusters and cluster types, we must throw (and not truncate) + if (name.size() > maxNameLength) + throw Exception{ "Cluster name is too long: " + std::string{ name } + "'" }; } Cluster::pointer Cluster::create(Session& session, ObjectPtr type, std::string_view name) @@ -183,6 +186,9 @@ namespace lms::db ClusterType::ClusterType(std::string_view name) : _name{ name } { + // As we use the name to uniquely identify clusters and cluster types, we must throw + if (name.size() > maxNameLength) + throw Exception{ "ClusterType name is too long: " + std::string{ name } + "'" }; } ClusterType::pointer ClusterType::create(Session& session, std::string_view name) @@ -230,6 +236,9 @@ namespace lms::db { session.checkReadTransaction(); + if (name.size() > maxNameLength) + throw Exception{ "Requested ClusterType name is too long: " + std::string{ name } + "'" }; + return utils::fetchQuerySingleResult(session.getDboSession()->find().where("name = ?").bind(name)); } @@ -254,6 +263,9 @@ namespace lms::db assert(self()); assert(session()); + if (name.size() > Cluster::maxNameLength) + throw Exception{ "Requested Cluster name is too long: " + std::string{ name } + "'" }; + return utils::fetchQuerySingleResult(session()->find().where("name = ?").bind(name).where("cluster_type_id = ?").bind(getId())); } diff --git a/src/libs/database/impl/Release.cpp b/src/libs/database/impl/Release.cpp index e758c1da..baea5c02 100644 --- a/src/libs/database/impl/Release.cpp +++ b/src/libs/database/impl/Release.cpp @@ -217,8 +217,11 @@ namespace lms::db } // namespace ReleaseType::ReleaseType(std::string_view name) - : _name{ std::string(name, 0, _maxNameLength) } + : _name{ name } { + // As we use the name to uniquely identoify release type, we must throw (and not truncate) + if (name.size() > _maxNameLength) + throw Exception{ "ReleaseType name is too long: " + std::string{ name } + "'" }; } ReleaseType::pointer ReleaseType::create(Session& session, std::string_view name) @@ -237,6 +240,9 @@ namespace lms::db { session.checkReadTransaction(); + if (name.size() > _maxNameLength) + throw Exception{ "Requeted ReleaseType name is too long: " + std::string{ name } + "'" }; + return utils::fetchQuerySingleResult(session.getDboSession()->query>("SELECT r_t from release_type r_t").where("r_t.name = ?").bind(name)); } diff --git a/src/libs/database/impl/Track.cpp b/src/libs/database/impl/Track.cpp index a8652d3e..cefcb539 100644 --- a/src/libs/database/impl/Track.cpp +++ b/src/libs/database/impl/Track.cpp @@ -377,6 +377,27 @@ namespace lms::db _relativeFilePath = filePath; } + void Track::setName(std::string_view name) + { + _name = std::string{ name, 0, _maxNameLength }; + if (name.size() > _maxNameLength) + LMS_LOG(DB, WARNING, "Track name too long, truncated to '" << _name << "'"); + } + + void Track::setCopyright(std::string_view copyright) + { + _copyright = std::string{ copyright, 0, _maxCopyrightLength }; + if (copyright.size() > _maxCopyrightLength) + LMS_LOG(DB, WARNING, "Track copyright too long, truncated to '" << _copyright << "'"); + } + + void Track::setCopyrightURL(std::string_view copyrightURL) + { + _copyrightURL = std::string{ copyrightURL, 0, _maxCopyrightURLLength }; + if (copyrightURL.size() > _maxCopyrightURLLength) + LMS_LOG(DB, WARNING, "Track copyright URL too long, truncated to '" << _copyrightURL << "'"); + } + void Track::clearArtistLinks() { _trackArtistLinks.clear(); diff --git a/src/libs/database/include/database/Artist.hpp b/src/libs/database/include/database/Artist.hpp index 4f1dbbd4..7190cc77 100644 --- a/src/libs/database/include/database/Artist.hpp +++ b/src/libs/database/include/database/Artist.hpp @@ -150,9 +150,9 @@ namespace lms::db // size is the max number of cluster per cluster type std::vector>> getClusterGroups(std::vector clusterTypeIds, std::size_t size) const; - void setName(std::string_view name) { _name = name; } + void setName(std::string_view name); void setMBID(const std::optional& mbid) { _MBID = mbid ? mbid->getAsString() : ""; } - void setSortName(const std::string& sortName); + void setSortName(std::string_view sortName); void setImage(ObjectPtr image); template @@ -168,7 +168,7 @@ namespace lms::db } private: - static constexpr std::size_t _maxNameLength{ 256 }; + static constexpr std::size_t _maxNameLength{ 512 }; friend class Session; // Create diff --git a/src/libs/database/include/database/Cluster.hpp b/src/libs/database/include/database/Cluster.hpp index 8b6ce020..15b15b1b 100644 --- a/src/libs/database/include/database/Cluster.hpp +++ b/src/libs/database/include/database/Cluster.hpp @@ -43,6 +43,8 @@ namespace lms::db class Cluster final : public Object { public: + static constexpr std::size_t maxNameLength{ 512 }; + struct FindParameters { std::optional range; @@ -126,8 +128,6 @@ namespace lms::db Cluster(ObjectPtr type, std::string_view name); static pointer create(Session& session, ObjectPtr type, std::string_view name); - static const std::size_t _maxNameLength = 128; - std::string _name; int _trackCount{}; int _releaseCount{}; @@ -141,6 +141,8 @@ namespace lms::db public: ClusterType() = default; + static constexpr std::size_t maxNameLength{ 512 }; + // Getters static std::size_t getCount(Session& session); static RangeResults findIds(Session& session, std::optional range = std::nullopt); @@ -169,8 +171,6 @@ namespace lms::db ClusterType(std::string_view name); static pointer create(Session& session, std::string_view name); - static const std::size_t _maxNameLength = 128; - std::string _name; Wt::Dbo::collection> _clusters; }; diff --git a/src/libs/database/include/database/Release.hpp b/src/libs/database/include/database/Release.hpp index c402bf40..f8bae2b8 100644 --- a/src/libs/database/include/database/Release.hpp +++ b/src/libs/database/include/database/Release.hpp @@ -69,7 +69,7 @@ namespace lms::db } private: - static constexpr std::size_t _maxNameLength{ 128 }; + static constexpr std::size_t _maxNameLength{ 512 }; friend class Session; ReleaseType(std::string_view name); @@ -241,7 +241,7 @@ namespace lms::db Wt::WDate getDate(bool original) const; std::optional getYear(bool original) const; - static constexpr std::size_t _maxNameLength{ 256 }; + static constexpr std::size_t _maxNameLength{ 512 }; std::string _name; std::string _sortName; diff --git a/src/libs/database/include/database/Track.hpp b/src/libs/database/include/database/Track.hpp index f06d9659..cfe43e2b 100644 --- a/src/libs/database/include/database/Track.hpp +++ b/src/libs/database/include/database/Track.hpp @@ -204,8 +204,8 @@ namespace lms::db void setTrackNumber(std::optional num) { _trackNumber = num; } void setDiscNumber(std::optional num) { _discNumber = num; } void setTotalTrack(std::optional totalTrack) { _totalTrack = totalTrack; } - void setDiscSubtitle(const std::string& name) { _discSubtitle = name; } - void setName(const std::string& name) { _name = std::string(name, 0, _maxNameLength); } + void setDiscSubtitle(std::string_view name) { _discSubtitle = name; } + void setName(std::string_view name); void setAbsoluteFilePath(const std::filesystem::path& filePath); void setRelativeFilePath(const std::filesystem::path& filePath); void setFileSize(std::size_t fileSize) { _fileSize = fileSize; } @@ -223,8 +223,8 @@ namespace lms::db void setHasCover(bool hasCover) { _hasCover = hasCover; } void setTrackMBID(const std::optional& MBID) { _trackMBID = MBID ? MBID->getAsString() : ""; } void setRecordingMBID(const std::optional& MBID) { _recordingMBID = MBID ? MBID->getAsString() : ""; } - void setCopyright(const std::string& copyright) { _copyright = std::string(copyright, 0, _maxCopyrightLength); } - void setCopyrightURL(const std::string& copyrightURL) { _copyrightURL = std::string(copyrightURL, 0, _maxCopyrightURLLength); } + void setCopyright(std::string_view copyright); + void setCopyrightURL(std::string_view copyrightURL); void setTrackReplayGain(std::optional replayGain) { _trackReplayGain = replayGain; } void setReleaseReplayGain(std::optional replayGain) { _releaseReplayGain = replayGain; } // may be by disc! void setArtistDisplayName(std::string_view name) { _artistDisplayName = name; } @@ -318,9 +318,9 @@ namespace lms::db friend class Session; static pointer create(Session& session); - static constexpr std::size_t _maxNameLength{ 256 }; - static constexpr std::size_t _maxCopyrightLength{ 256 }; - static constexpr std::size_t _maxCopyrightURLLength{ 256 }; + static constexpr std::size_t _maxNameLength{ 512 }; + static constexpr std::size_t _maxCopyrightLength{ 512 }; + static constexpr std::size_t _maxCopyrightURLLength{ 512 }; int _scanVersion{}; std::optional _trackNumber{}; diff --git a/src/libs/database/include/database/Types.hpp b/src/libs/database/include/database/Types.hpp index 4c330313..aadea04e 100644 --- a/src/libs/database/include/database/Types.hpp +++ b/src/libs/database/include/database/Types.hpp @@ -25,8 +25,16 @@ #include +#include "core/Exception.hpp" + namespace lms::db { + class Exception : public core::LmsException + { + public: + using LmsException::LmsException; + }; + // Caution: do not change enum values if they are set! // Request: diff --git a/src/libs/database/include/database/User.hpp b/src/libs/database/include/database/User.hpp index d26a29b9..537ecd1e 100644 --- a/src/libs/database/include/database/User.hpp +++ b/src/libs/database/include/database/User.hpp @@ -68,8 +68,8 @@ namespace lms::db } }; - static inline constexpr std::size_t MinNameLength{ 3 }; - static inline constexpr std::size_t MaxNameLength{ 15 }; + static inline constexpr std::size_t minNameLength{ 3 }; + static inline constexpr std::size_t maxNameLength{ 32 }; static inline constexpr bool defaultSubsonicEnableTranscodingByDefault{ false }; static inline constexpr TranscodingOutputFormat defaultSubsonicTranscodingOutputFormat{ TranscodingOutputFormat::OGG_OPUS }; static inline constexpr Bitrate defaultSubsonicTranscodingOutputBitrate{ 128000 }; diff --git a/src/libs/database/test/Cluster.cpp b/src/libs/database/test/Cluster.cpp index d6c5ac0e..a0f88309 100644 --- a/src/libs/database/test/Cluster.cpp +++ b/src/libs/database/test/Cluster.cpp @@ -35,7 +35,7 @@ namespace lms::db::tests ScopedClusterType clusterType{ session, "MyType" }; { - auto transaction{ session.createWriteTransaction() }; + auto transaction{ session.createReadTransaction() }; EXPECT_EQ(ClusterType::getCount(session), 1); } @@ -43,7 +43,7 @@ namespace lms::db::tests ScopedCluster cluster{ session, clusterType.lockAndGet(), "MyCluster" }; { - auto transaction{ session.createWriteTransaction() }; + auto transaction{ session.createReadTransaction() }; EXPECT_EQ(Cluster::getCount(session), 1); EXPECT_EQ(cluster->getType()->getId(), clusterType.getId()); @@ -74,7 +74,7 @@ namespace lms::db::tests } { - auto transaction{ session.createWriteTransaction() }; + auto transaction{ session.createReadTransaction() }; auto clusterTypes{ ClusterType::findOrphanIds(session) }; ASSERT_EQ(clusterTypes.results.size(), 1); @@ -84,6 +84,70 @@ namespace lms::db::tests } } + TEST_F(DatabaseFixture, Cluster_find) + { + ScopedClusterType clusterType{ session, "MyType" }; + ScopedCluster cluster1{ session, clusterType.lockAndGet(), "MyCluster" }; + ScopedCluster cluster2{ session, clusterType.lockAndGet(), "Mycluster" }; + ScopedCluster cluster3{ session, clusterType.lockAndGet(), "MyOtherCluster" }; + + { + auto transaction{ session.createReadTransaction() }; + + EXPECT_EQ(clusterType->getCluster("MyCluster"), cluster1.get()); + EXPECT_EQ(clusterType->getCluster("Mycluster"), cluster2.get()); + + EXPECT_EQ(clusterType->getCluster(" Mycluster"), Cluster::pointer{}); + EXPECT_EQ(clusterType->getCluster("Mycluster "), Cluster::pointer{}); + EXPECT_EQ(clusterType->getCluster("mycluster"), Cluster::pointer{}); + EXPECT_EQ(clusterType->getCluster("My"), Cluster::pointer{}); + EXPECT_EQ(clusterType->getCluster("Cluster"), Cluster::pointer{}); + EXPECT_EQ(clusterType->getCluster("MyCluster1"), Cluster::pointer{}); + EXPECT_EQ(clusterType->getCluster("MyCluster2"), Cluster::pointer{}); + EXPECT_EQ(clusterType->getCluster(""), Cluster::pointer{}); + EXPECT_EQ(clusterType->getCluster(" "), Cluster::pointer{}); + EXPECT_EQ(clusterType->getCluster("*"), Cluster::pointer{}); + EXPECT_EQ(clusterType->getCluster(R"(%)"), Cluster::pointer{}); + EXPECT_EQ(clusterType->getCluster(R"(%%)"), Cluster::pointer{}); + EXPECT_EQ(clusterType->getCluster(R"(")"), Cluster::pointer{}); + EXPECT_EQ(clusterType->getCluster(R"("")"), Cluster::pointer{}); + } + } + + TEST_F(DatabaseFixture, Cluster_create) + { + ScopedClusterType clusterType{ session, "MyType" }; + + { + auto transaction{ session.createWriteTransaction() }; + + auto createdCluster{ session.create(clusterType.get(), "Foo") }; + auto foundCluster{ clusterType->getCluster("Foo") }; + EXPECT_EQ(createdCluster, foundCluster); + } + + { + auto transaction{ session.createWriteTransaction() }; + + auto createdCluster{ session.create(clusterType.get(), "") }; + auto foundCluster{ clusterType->getCluster("") }; + EXPECT_EQ(createdCluster, foundCluster); + } + } + + TEST_F(DatabaseFixture, Cluster_create_long) + { + ScopedClusterType clusterType{ session, "MyType" }; + + { + auto transaction{ session.createWriteTransaction() }; + + auto createdCluster{ session.create(clusterType.get(), "Alternative Rock; Art Pop; Art Rock; Britpop; Chamber Pop; Electronic Rock; Electronica; Experimental Rock; Neo-Progressive Rock; Foo") }; + auto foundCluster{ clusterType->getCluster("Alternative Rock; Art Pop; Art Rock; Britpop; Chamber Pop; Electronic Rock; Electronica; Experimental Rock; Neo-Progressive Rock; Foo") }; + EXPECT_EQ(createdCluster, foundCluster); + } + } + TEST_F(DatabaseFixture, Cluster_singleTrack) { ScopedTrack track{ session }; diff --git a/src/lms/ui/common/LoginNameValidator.cpp b/src/lms/ui/common/LoginNameValidator.cpp index aeb3e23d..b1abc51e 100644 --- a/src/lms/ui/common/LoginNameValidator.cpp +++ b/src/lms/ui/common/LoginNameValidator.cpp @@ -38,8 +38,8 @@ namespace lms::ui { auto v = std::make_unique(); v->setMandatory(true); - v->setMinimumLength(db::User::MinNameLength); - v->setMaximumLength(db::User::MaxNameLength); + v->setMinimumLength(db::User::minNameLength); + v->setMaximumLength(db::User::maxNameLength); return v; } } // namespace lms::ui From 2a02c6ede6c91944fab8068cc623b8ecef616000 Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 28 Jul 2024 13:43:07 +0200 Subject: [PATCH 08/11] Fixed xml attributes sometimes not written --- src/libs/subsonic/impl/SubsonicResponse.cpp | 26 +++++++++++---------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/libs/subsonic/impl/SubsonicResponse.cpp b/src/libs/subsonic/impl/SubsonicResponse.cpp index 993e2dfc..90ae1faf 100644 --- a/src/libs/subsonic/impl/SubsonicResponse.cpp +++ b/src/libs/subsonic/impl/SubsonicResponse.cpp @@ -193,18 +193,6 @@ namespace lms::api::subsonic std::function nodeToPropertyTree = [&](const Node& node) { boost::property_tree::ptree res; - for (const auto& [key, value] : node._attributes) - { - if (std::holds_alternative(value)) - res.put("." + std::string{ key.str() }, std::get(value)); - else if (std::holds_alternative(value)) - res.put("." + std::string{ key.str() }, std::get(value)); - else if (std::holds_alternative(value)) - res.put("." + std::string{ key.str() }, std::get(value)); - else if (std::holds_alternative(value)) - res.put("." + std::string{ key.str() }, std::get(value)); - } - auto valueToPropertyTree = [](const Node::ValueType& value) { boost::property_tree::ptree res; std::visit([&](const auto& rawValue) { @@ -239,6 +227,20 @@ namespace lms::api::subsonic } } + for (const auto& [key, value] : node._attributes) + { + if (std::holds_alternative(value)) + res.put("." + std::string{ key.str() }, std::get(value)); + else if (std::holds_alternative(value)) + res.put("." + std::string{ key.str() }, std::get(value)); + else if (std::holds_alternative(value)) + res.put("." + std::string{ key.str() }, std::get(value)); + else if (std::holds_alternative(value)) + res.put("." + std::string{ key.str() }, std::get(value)); + else + assert(false); + } + return res; }; From 9a91a2e080a4bec6bfbec618ac28a7053b77b7da Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 28 Jul 2024 14:42:16 +0200 Subject: [PATCH 09/11] Fixed bad json musicBrainzId field in getArtistInfo2, fixes #497 --- src/libs/subsonic/impl/RequestContext.hpp | 2 + src/libs/subsonic/impl/SubsonicResource.cpp | 12 ++- src/libs/subsonic/impl/SubsonicResponse.cpp | 92 +++++++++---------- src/libs/subsonic/impl/SubsonicResponse.hpp | 2 +- .../subsonic/impl/entrypoints/Browsing.cpp | 14 ++- .../impl/entrypoints/UserManagement.cpp | 4 +- src/libs/subsonic/impl/responses/Album.cpp | 1 + src/libs/subsonic/impl/responses/Album.hpp | 2 + src/libs/subsonic/impl/responses/Artist.cpp | 1 + src/libs/subsonic/impl/responses/Artist.hpp | 2 + src/libs/subsonic/impl/responses/Genre.cpp | 14 ++- src/libs/subsonic/impl/responses/Genre.hpp | 6 +- src/libs/subsonic/impl/responses/Song.cpp | 1 + src/libs/subsonic/impl/responses/Song.hpp | 4 +- src/libs/subsonic/impl/responses/User.cpp | 21 +++-- src/libs/subsonic/impl/responses/User.hpp | 4 +- 16 files changed, 110 insertions(+), 72 deletions(-) diff --git a/src/libs/subsonic/impl/RequestContext.hpp b/src/libs/subsonic/impl/RequestContext.hpp index f90b4940..e1f6f687 100644 --- a/src/libs/subsonic/impl/RequestContext.hpp +++ b/src/libs/subsonic/impl/RequestContext.hpp @@ -27,6 +27,7 @@ #include "ClientInfo.hpp" #include "ProtocolVersion.hpp" +#include "SubsonicResponse.hpp" namespace lms::db { @@ -43,6 +44,7 @@ namespace lms::api::subsonic const db::ObjectPtr user; ClientInfo clientInfo; ProtocolVersion serverProtocolVersion; + ResponseFormat responseFormat; bool enableOpenSubsonic{ true }; bool enableDefaultCover{}; }; diff --git a/src/libs/subsonic/impl/SubsonicResource.cpp b/src/libs/subsonic/impl/SubsonicResource.cpp index 6bf949f2..1a212a00 100644 --- a/src/libs/subsonic/impl/SubsonicResource.cpp +++ b/src/libs/subsonic/impl/SubsonicResource.cpp @@ -411,6 +411,7 @@ namespace lms::api::subsonic const db::UserId userId{ authenticateUser(request, clientInfo) }; bool enableOpenSubsonic{ _openSubsonicDisabledClients.find(clientInfo.name) == std::cend(_openSubsonicDisabledClients) }; bool enableDefaultCover{ _defaultCoverClients.find(clientInfo.name) != std::cend(_openSubsonicDisabledClients) }; + const ResponseFormat format{ getParameterAs(request.getParameterMap(), "f").value_or("xml") == "json" ? ResponseFormat::json : ResponseFormat::xml }; db::User::pointer user; { @@ -422,7 +423,16 @@ namespace lms::api::subsonic throw UserNotAuthorizedError{}; } - return { parameters, _db.getTLSSession(), user, clientInfo, getServerProtocolVersion(clientInfo.name), enableOpenSubsonic, enableDefaultCover }; + return RequestContext{ + .parameters = parameters, + .dbSession = _db.getTLSSession(), + .user = user, + .clientInfo = clientInfo, + .serverProtocolVersion = getServerProtocolVersion(clientInfo.name), + .responseFormat = format, + .enableOpenSubsonic = enableOpenSubsonic, + .enableDefaultCover = enableDefaultCover + }; } db::UserId SubsonicResource::authenticateUser(const Wt::Http::Request& request, const ClientInfo& clientInfo) diff --git a/src/libs/subsonic/impl/SubsonicResponse.cpp b/src/libs/subsonic/impl/SubsonicResponse.cpp index 90ae1faf..6788aead 100644 --- a/src/libs/subsonic/impl/SubsonicResponse.cpp +++ b/src/libs/subsonic/impl/SubsonicResponse.cpp @@ -266,74 +266,64 @@ namespace lms::api::subsonic first = false; } - if (node._value) + // Values are handled manually (using attributes) in json format + assert(!node._value); + + for (const auto& [key, childNode] : node._children) { if (!first) os << ','; - os << "\"value\":"; - serializeValue(os, *node._value); + serializeEscapedString(os, key.str()); + os << ':'; + serializeNode(os, childNode); first = false; } - else + + for (const auto& [key, childArrayNodes] : node._childrenArrays) { - for (const auto& [key, childNode] : node._children) - { - if (!first) - os << ','; + if (!first) + os << ','; + + serializeEscapedString(os, key.str()); + os << ":["; + + bool firstChild{ true }; + for (const Response::Node& childNode : childArrayNodes) + { + if (!firstChild) + os << ","; - serializeEscapedString(os, key.str()); - os << ':'; serializeNode(os, childNode); - - first = false; + firstChild = false; } + os << ']'; - for (const auto& [key, childArrayNodes] : node._childrenArrays) + first = false; + } + + for (const auto& [key, childValues] : node._childrenValues) + { + if (!first) + os << ','; + + serializeEscapedString(os, key.str()); + os << ":["; + + bool firstChild{ true }; + for (const Node::ValueType& childValue : childValues) { - if (!first) - os << ','; + if (!firstChild) + os << ","; - serializeEscapedString(os, key.str()); - os << ":["; + serializeValue(os, childValue); - bool firstChild{ true }; - for (const Response::Node& childNode : childArrayNodes) - { - if (!firstChild) - os << ","; - - serializeNode(os, childNode); - firstChild = false; - } - os << ']'; - - first = false; + firstChild = false; } + os << ']'; - for (const auto& [key, childValues] : node._childrenValues) - { - if (!first) - os << ','; - - serializeEscapedString(os, key.str()); - os << ":["; - - bool firstChild{ true }; - for (const Node::ValueType& childValue : childValues) - { - if (!firstChild) - os << ","; - - serializeValue(os, childValue); - - firstChild = false; - } - os << ']'; - - first = false; - } + first = false; } os << '}'; diff --git a/src/libs/subsonic/impl/SubsonicResponse.hpp b/src/libs/subsonic/impl/SubsonicResponse.hpp index 4c273ab5..b8cacdb6 100644 --- a/src/libs/subsonic/impl/SubsonicResponse.hpp +++ b/src/libs/subsonic/impl/SubsonicResponse.hpp @@ -27,7 +27,7 @@ #include "core/LiteralString.hpp" -#include "RequestContext.hpp" +#include "ProtocolVersion.hpp" #include "SubsonicResponseAllocator.hpp" namespace lms::api::subsonic diff --git a/src/libs/subsonic/impl/entrypoints/Browsing.cpp b/src/libs/subsonic/impl/entrypoints/Browsing.cpp index f7221b46..82c86a54 100644 --- a/src/libs/subsonic/impl/entrypoints/Browsing.cpp +++ b/src/libs/subsonic/impl/entrypoints/Browsing.cpp @@ -362,7 +362,7 @@ namespace lms::api::subsonic const auto clusters{ clusterType->getClusters() }; for (const Cluster::pointer& cluster : clusters) - genresNode.addArrayChild("genre", createGenreNode(cluster)); + genresNode.addArrayChild("genre", createGenreNode(context, cluster)); } return response; @@ -531,7 +531,17 @@ namespace lms::api::subsonic std::optional artistMBID{ artist->getMBID() }; if (artistMBID) - artistInfoNode.createChild("musicBrainzId").setValue(artistMBID->getAsString()); + { + switch (context.responseFormat) + { + case ResponseFormat::json: + artistInfoNode.setAttribute("musicBrainzId", artistMBID->getAsString()); + break; + case ResponseFormat::xml: + artistInfoNode.createChild("musicBrainzId").setValue(artistMBID->getAsString()); + break; + } + } } auto similarArtistsId{ core::Service::get()->getSimilarArtists(id, { TrackArtistLinkType::Artist, TrackArtistLinkType::ReleaseArtist }, count) }; diff --git a/src/libs/subsonic/impl/entrypoints/UserManagement.cpp b/src/libs/subsonic/impl/entrypoints/UserManagement.cpp index 3e335110..3858f7b1 100644 --- a/src/libs/subsonic/impl/entrypoints/UserManagement.cpp +++ b/src/libs/subsonic/impl/entrypoints/UserManagement.cpp @@ -35,7 +35,7 @@ namespace lms::api::subsonic throw RequestedDataNotFoundError{}; Response response{ Response::createOkResponse(context.serverProtocolVersion) }; - response.addNode("user", createUserNode(user)); + response.addNode("user", createUserNode(context, user)); return response; } @@ -47,7 +47,7 @@ namespace lms::api::subsonic auto transaction{ context.dbSession.createReadTransaction() }; User::find(context.dbSession, User::FindParameters{}, [&](const User::pointer& user) { - usersNode.addArrayChild("user", createUserNode(user)); + usersNode.addArrayChild("user", createUserNode(context, user)); }); return response; diff --git a/src/libs/subsonic/impl/responses/Album.cpp b/src/libs/subsonic/impl/responses/Album.cpp index 88d00e43..3d1fe45e 100644 --- a/src/libs/subsonic/impl/responses/Album.cpp +++ b/src/libs/subsonic/impl/responses/Album.cpp @@ -30,6 +30,7 @@ #include "services/feedback/IFeedbackService.hpp" #include "services/scrobbling/IScrobblingService.hpp" +#include "RequestContext.hpp" #include "SubsonicId.hpp" #include "responses/Artist.hpp" #include "responses/DiscTitle.hpp" diff --git a/src/libs/subsonic/impl/responses/Album.hpp b/src/libs/subsonic/impl/responses/Album.hpp index d324fe24..3b843772 100644 --- a/src/libs/subsonic/impl/responses/Album.hpp +++ b/src/libs/subsonic/impl/responses/Album.hpp @@ -33,5 +33,7 @@ namespace lms::db namespace lms::api::subsonic { + class RequestContext; + Response::Node createAlbumNode(RequestContext& context, const db::ObjectPtr& release, bool id3, const db::ObjectPtr& directory = {}); } \ No newline at end of file diff --git a/src/libs/subsonic/impl/responses/Artist.cpp b/src/libs/subsonic/impl/responses/Artist.cpp index 77481fba..2bf29e0d 100644 --- a/src/libs/subsonic/impl/responses/Artist.cpp +++ b/src/libs/subsonic/impl/responses/Artist.cpp @@ -29,6 +29,7 @@ #include "database/User.hpp" #include "services/feedback/IFeedbackService.hpp" +#include "RequestContext.hpp" #include "SubsonicId.hpp" namespace lms::api::subsonic diff --git a/src/libs/subsonic/impl/responses/Artist.hpp b/src/libs/subsonic/impl/responses/Artist.hpp index 3a8a5687..9830a102 100644 --- a/src/libs/subsonic/impl/responses/Artist.hpp +++ b/src/libs/subsonic/impl/responses/Artist.hpp @@ -36,6 +36,8 @@ namespace lms::db namespace lms::api::subsonic { + class RequestContext; + namespace utils { std::string joinArtistNames(const std::vector>& artists); diff --git a/src/libs/subsonic/impl/responses/Genre.cpp b/src/libs/subsonic/impl/responses/Genre.cpp index 308fe29f..2ae28e9f 100644 --- a/src/libs/subsonic/impl/responses/Genre.cpp +++ b/src/libs/subsonic/impl/responses/Genre.cpp @@ -21,13 +21,23 @@ #include "database/Cluster.hpp" +#include "RequestContext.hpp" + namespace lms::api::subsonic { - Response::Node createGenreNode(const db::Cluster::pointer& cluster) + Response::Node createGenreNode(RequestContext& context, const db::Cluster::pointer& cluster) { Response::Node clusterNode; - clusterNode.setValue(cluster->getName()); + switch (context.responseFormat) + { + case ResponseFormat::json: + clusterNode.setAttribute("value", cluster->getName()); + break; + case ResponseFormat::xml: + clusterNode.setValue(cluster->getName()); + break; + } clusterNode.setAttribute("songCount", cluster->getTrackCount()); clusterNode.setAttribute("albumCount", cluster->getReleasesCount()); diff --git a/src/libs/subsonic/impl/responses/Genre.hpp b/src/libs/subsonic/impl/responses/Genre.hpp index 45017625..cd34c9dd 100644 --- a/src/libs/subsonic/impl/responses/Genre.hpp +++ b/src/libs/subsonic/impl/responses/Genre.hpp @@ -30,5 +30,7 @@ namespace lms::db namespace lms::api::subsonic { - Response::Node createGenreNode(const db::ObjectPtr& cluster); -} + class RequestContext; + + Response::Node createGenreNode(RequestContext& context, const db::ObjectPtr& cluster); +} // namespace lms::api::subsonic diff --git a/src/libs/subsonic/impl/responses/Song.cpp b/src/libs/subsonic/impl/responses/Song.cpp index 9ff70831..d01205e5 100644 --- a/src/libs/subsonic/impl/responses/Song.cpp +++ b/src/libs/subsonic/impl/responses/Song.cpp @@ -35,6 +35,7 @@ #include "services/feedback/IFeedbackService.hpp" #include "services/scrobbling/IScrobblingService.hpp" +#include "RequestContext.hpp" #include "SubsonicId.hpp" #include "Utils.hpp" #include "responses/Artist.hpp" diff --git a/src/libs/subsonic/impl/responses/Song.hpp b/src/libs/subsonic/impl/responses/Song.hpp index 5b35227b..ac22ba1a 100644 --- a/src/libs/subsonic/impl/responses/Song.hpp +++ b/src/libs/subsonic/impl/responses/Song.hpp @@ -32,5 +32,7 @@ namespace lms::db namespace lms::api::subsonic { + class RequestContext; + Response::Node createSongNode(RequestContext& context, const db::ObjectPtr& track, bool id3); -} \ No newline at end of file +} // namespace lms::api::subsonic \ No newline at end of file diff --git a/src/libs/subsonic/impl/responses/User.cpp b/src/libs/subsonic/impl/responses/User.cpp index 69c8571a..557b6acb 100644 --- a/src/libs/subsonic/impl/responses/User.cpp +++ b/src/libs/subsonic/impl/responses/User.cpp @@ -19,13 +19,15 @@ #include "responses/User.hpp" +#include "database/MediaLibrary.hpp" #include "database/User.hpp" +#include "RequestContext.hpp" +#include "SubsonicId.hpp" + namespace lms::api::subsonic { - using namespace db; - - Response::Node createUserNode(const User::pointer& user) + Response::Node createUserNode(RequestContext& context, const db::User::pointer& user) { Response::Node userNode; @@ -38,14 +40,15 @@ namespace lms::api::subsonic userNode.setAttribute("playlistRole", true); userNode.setAttribute("coverArtRole", false); userNode.setAttribute("commentRole", false); - userNode.setAttribute("podcastRole", false); + userNode.setAttribute("podcastRole", false); // not supported userNode.setAttribute("streamRole", true); - userNode.setAttribute("jukeboxRole", false); - userNode.setAttribute("shareRole", false); + userNode.setAttribute("jukeboxRole", false); // not supported + userNode.setAttribute("shareRole", false); // not supported - Response::Node folder; - folder.setValue("0"); - userNode.addArrayChild("folder", std::move(folder)); + // users can access all libraries + db::MediaLibrary::find(context.dbSession, [&](const db::MediaLibrary::pointer& library) { + userNode.addArrayValue("folder", idToString(library->getId())); + }); return userNode; } diff --git a/src/libs/subsonic/impl/responses/User.hpp b/src/libs/subsonic/impl/responses/User.hpp index 9ced3725..37082684 100644 --- a/src/libs/subsonic/impl/responses/User.hpp +++ b/src/libs/subsonic/impl/responses/User.hpp @@ -30,5 +30,7 @@ namespace lms::db namespace lms::api::subsonic { - Response::Node createUserNode(const db::ObjectPtr& user); + class RequestContext; + + Response::Node createUserNode(RequestContext& context, const db::ObjectPtr& user); } \ No newline at end of file From 7a22a626b1cfbde23c8ca7dd1c455ded49edb853 Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 28 Jul 2024 14:42:44 +0200 Subject: [PATCH 10/11] Fixed format --- src/libs/subsonic/impl/responses/Album.hpp | 4 ++-- src/libs/subsonic/impl/responses/User.hpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/subsonic/impl/responses/Album.hpp b/src/libs/subsonic/impl/responses/Album.hpp index 3b843772..99688f4f 100644 --- a/src/libs/subsonic/impl/responses/Album.hpp +++ b/src/libs/subsonic/impl/responses/Album.hpp @@ -34,6 +34,6 @@ namespace lms::db namespace lms::api::subsonic { class RequestContext; - + Response::Node createAlbumNode(RequestContext& context, const db::ObjectPtr& release, bool id3, const db::ObjectPtr& directory = {}); -} \ No newline at end of file +} // namespace lms::api::subsonic \ No newline at end of file diff --git a/src/libs/subsonic/impl/responses/User.hpp b/src/libs/subsonic/impl/responses/User.hpp index 37082684..68bf29e6 100644 --- a/src/libs/subsonic/impl/responses/User.hpp +++ b/src/libs/subsonic/impl/responses/User.hpp @@ -33,4 +33,4 @@ namespace lms::api::subsonic class RequestContext; Response::Node createUserNode(RequestContext& context, const db::ObjectPtr& user); -} \ No newline at end of file +} // namespace lms::api::subsonic \ No newline at end of file From cb7ee34d417ba0ef8c5b0cb08f238578bfa30b1d Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 28 Jul 2024 14:52:37 +0200 Subject: [PATCH 11/11] Added a missing index to speed up direcotry browsing commands --- src/libs/database/impl/Session.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libs/database/impl/Session.cpp b/src/libs/database/impl/Session.cpp index 73819aac..bcd9c03a 100644 --- a/src/libs/database/impl/Session.cpp +++ b/src/libs/database/impl/Session.cpp @@ -206,6 +206,7 @@ namespace lms::db _session.execute("CREATE INDEX IF NOT EXISTS track_id_idx ON track(id)"); _session.execute("CREATE INDEX IF NOT EXISTS track_absolute_path_idx ON track(absolute_file_path)"); + _session.execute("CREATE INDEX IF NOT EXISTS track_directory_release_idx ON track(directory_id, release_id);"); _session.execute("CREATE INDEX IF NOT EXISTS track_name_idx ON track(name)"); _session.execute("CREATE INDEX IF NOT EXISTS track_name_nocase_idx ON track(name COLLATE NOCASE)"); _session.execute("CREATE INDEX IF NOT EXISTS track_mbid_idx ON track(mbid)");