From b02021ac8fcbfe6c33fb620cf2b7e8eb6623a043 Mon Sep 17 00:00:00 2001 From: emeric Date: Mon, 20 Nov 2023 09:51:11 +0100 Subject: [PATCH] Fallback on same album name only if in the same directory. fixes #370 --- src/libs/services/database/impl/Release.cpp | 10 +- .../include/services/database/Release.hpp | 7 +- src/libs/services/database/test/Release.cpp | 34 ++ .../scanner/impl/ScanStepScanFiles.cpp | 415 +++++++++--------- 4 files changed, 247 insertions(+), 219 deletions(-) diff --git a/src/libs/services/database/impl/Release.cpp b/src/libs/services/database/impl/Release.cpp index 37c87efb..226cdb9f 100644 --- a/src/libs/services/database/impl/Release.cpp +++ b/src/libs/services/database/impl/Release.cpp @@ -196,13 +196,15 @@ namespace Database return session.getDboSession().add(std::unique_ptr {new Release{ name, MBID }}); } - std::vector Release::find(Session& session, const std::string& name) + std::vector Release::find(Session& session, const std::string& name, const std::filesystem::path& releaseDirectory) { - session.checkWriteTransaction(); + session.checkReadTransaction(); auto res{ session.getDboSession() - .find() - .where("name = ?").bind(std::string(name, 0, _maxNameLength)) + .query>("SELECT DISTINCT r from release r") + .join("track t ON t.release_id = r.id") + .where("r.name = ?").bind(std::string(name, 0, _maxNameLength)) + .where("t.file_path LIKE ?").bind(Utils::escapeLikeKeyword(releaseDirectory.string()) + "%") .resultList() }; return std::vector(res.begin(), res.end()); diff --git a/src/libs/services/database/include/services/database/Release.hpp b/src/libs/services/database/include/services/database/Release.hpp index 2000e5e0..ccf191bb 100644 --- a/src/libs/services/database/include/services/database/Release.hpp +++ b/src/libs/services/database/include/services/database/Release.hpp @@ -19,6 +19,7 @@ #pragma once +#include #include #include @@ -61,9 +62,9 @@ namespace Database ArtistId artist; // only releases that involved this user EnumSet trackArtistLinkTypes; // and for these link types EnumSet excludedTrackArtistLinkTypes; // but not for these link types - std::optional primaryType; // if, set, matching this primary type + std::optional primaryType; // if set, matching this primary type EnumSet secondaryTypes; // Matching all this (if any) - + FindParameters& setClusters(const std::vector& _clusters) { clusters = _clusters; return *this; } FindParameters& setKeywords(const std::vector& _keywords) { keywords = _keywords; return *this; } FindParameters& setSortMethod(ReleaseSortMethod _sortMethod) { sortMethod = _sortMethod; return *this; } @@ -86,7 +87,7 @@ namespace Database static std::size_t getCount(Session& session); static bool exists(Session& session, ReleaseId id); static pointer find(Session& session, const UUID& MBID); - static std::vector find(Session& session, const std::string& name); + static std::vector find(Session& session, const std::string& name, const std::filesystem::path& releaseDirectory); static pointer find(Session& session, ReleaseId id); static RangeResults find(Session& session, const FindParameters& parameters); static void find(Session& session, const FindParameters& parameters, std::function func); diff --git a/src/libs/services/database/test/Release.cpp b/src/libs/services/database/test/Release.cpp index f7824883..9a8ede14 100644 --- a/src/libs/services/database/test/Release.cpp +++ b/src/libs/services/database/test/Release.cpp @@ -132,6 +132,40 @@ TEST_F(DatabaseFixture, Release_singleTrack) } } +TEST_F(DatabaseFixture, Release_findByNameAndPath) +{ + ScopedRelease release1{ session, "MyRelease" }; + ScopedRelease release2{ session, "MyRelease" }; + ScopedTrack track1{ session, "MyTrack" }; + ScopedTrack track2{ session, "MyTrack" }; + + { + auto transaction{ session.createWriteTransaction() }; + + track1.get().modify()->setRelease(release1.get()); + track1.get().modify()->setPath("/tmp/foo/foo.mp3"); + + track2.get().modify()->setRelease(release2.get()); + track2.get().modify()->setPath("/tmp/bar/bar.mp3"); + } + + { + auto transaction{ session.createReadTransaction() }; + std::cout << "OK HERE" << std::endl; + { + const auto releases{ Release::find(session, "MyRelease", "/tmp/foo") }; + ASSERT_EQ(releases.size(), 1); + EXPECT_EQ(releases.front()->getId(), release1.getId()); + } + + { + const auto releases{ Release::find(session, "MyRelease", "/tmp/bar") }; + ASSERT_EQ(releases.size(), 1); + EXPECT_EQ(releases.front()->getId(), release2.getId()); + } + } +} + TEST_F(DatabaseFixture, MulitpleReleaseSearchByName) { ScopedRelease release1{ session, "MyRelease" }; diff --git a/src/libs/services/scanner/impl/ScanStepScanFiles.cpp b/src/libs/services/scanner/impl/ScanStepScanFiles.cpp index 615900fa..f1e902aa 100644 --- a/src/libs/services/scanner/impl/ScanStepScanFiles.cpp +++ b/src/libs/services/scanner/impl/ScanStepScanFiles.cpp @@ -35,257 +35,249 @@ using namespace Database; -namespace +namespace Scanner { - Artist::pointer - createArtist(Session& session, const MetaData::Artist& artistInfo) + namespace { - Artist::pointer artist{ session.create(artistInfo.name) }; - - if (artistInfo.mbid) - artist.modify()->setMBID(*artistInfo.mbid); - if (artistInfo.sortName) - artist.modify()->setSortName(*artistInfo.sortName); - - return artist; - } - - void - updateArtistIfNeeded(Artist::pointer artist, const MetaData::Artist& artistInfo) - { - // Name may have been updated - if (artist->getName() != artistInfo.name) + Artist::pointer createArtist(Session& session, const MetaData::Artist& artistInfo) { - artist.modify()->setName(artistInfo.name); - } + Artist::pointer artist{ session.create(artistInfo.name) }; - // Sortname may have been updated - if (artistInfo.sortName && *artistInfo.sortName != artist->getSortName()) - { - artist.modify()->setSortName(*artistInfo.sortName); - } - } - - std::vector - getOrCreateArtists(Session& session, const std::vector& artistsInfo, bool allowFallbackOnMBIDEntries) - { - std::vector artists; - - for (const MetaData::Artist& artistInfo : artistsInfo) - { - Artist::pointer artist; - - // First try to get by MBID if (artistInfo.mbid) - { - artist = Artist::find(session, *artistInfo.mbid); - if (!artist) - artist = createArtist(session, artistInfo); - else - updateArtistIfNeeded(artist, artistInfo); + artist.modify()->setMBID(*artistInfo.mbid); + if (artistInfo.sortName) + artist.modify()->setSortName(*artistInfo.sortName); - artists.emplace_back(std::move(artist)); - continue; + return artist; + } + + void updateArtistIfNeeded(Artist::pointer artist, const MetaData::Artist& artistInfo) + { + // Name may have been updated + if (artist->getName() != artistInfo.name) + { + artist.modify()->setName(artistInfo.name); } - // Fall back on artist name (collisions may occur) - if (!artistInfo.name.empty()) + // Sortname may have been updated + if (artistInfo.sortName && *artistInfo.sortName != artist->getSortName()) { - for (const Artist::pointer& sameNamedArtist : Artist::find(session, artistInfo.name)) - { - // Do not fallback on artist that is correctly tagged - if (!allowFallbackOnMBIDEntries && sameNamedArtist->getMBID()) - continue; + artist.modify()->setSortName(*artistInfo.sortName); + } + } - artist = sameNamedArtist; - break; + std::vector getOrCreateArtists(Session& session, const std::vector& artistsInfo, bool allowFallbackOnMBIDEntries) + { + std::vector artists; + + for (const MetaData::Artist& artistInfo : artistsInfo) + { + Artist::pointer artist; + + // First try to get by MBID + if (artistInfo.mbid) + { + artist = Artist::find(session, *artistInfo.mbid); + if (!artist) + artist = createArtist(session, artistInfo); + else + updateArtistIfNeeded(artist, artistInfo); + + artists.emplace_back(std::move(artist)); + continue; } - // No Artist found with the same name and without MBID -> creating - if (!artist) - artist = createArtist(session, artistInfo); - else - updateArtistIfNeeded(artist, artistInfo); + // Fall back on artist name (collisions may occur) + if (!artistInfo.name.empty()) + { + for (const Artist::pointer& sameNamedArtist : Artist::find(session, artistInfo.name)) + { + // Do not fallback on artist that is correctly tagged + if (!allowFallbackOnMBIDEntries && sameNamedArtist->getMBID()) + continue; - artists.emplace_back(std::move(artist)); - continue; + artist = sameNamedArtist; + break; + } + + // No Artist found with the same name and without MBID -> creating + if (!artist) + artist = createArtist(session, artistInfo); + else + updateArtistIfNeeded(artist, artistInfo); + + artists.emplace_back(std::move(artist)); + continue; + } } + + return artists; } - return artists; - } - - ReleaseTypePrimary convertReleaseTypePrimary(MetaData::Release::PrimaryType type) - { - switch (type) - { - case MetaData::Release::PrimaryType::Album: return ReleaseTypePrimary::Album; - case MetaData::Release::PrimaryType::Single: return ReleaseTypePrimary::Single; - case MetaData::Release::PrimaryType::EP: return ReleaseTypePrimary::EP; - case MetaData::Release::PrimaryType::Broadcast: return ReleaseTypePrimary::Broadcast; - case MetaData::Release::PrimaryType::Other: return ReleaseTypePrimary::Other; - } - - return ReleaseTypePrimary::Other; - } - - EnumSet convertReleaseTypesSecondary(EnumSet types) - { - EnumSet res; - - for (MetaData::Release::SecondaryType type : types) + ReleaseTypePrimary convertReleaseTypePrimary(MetaData::Release::PrimaryType type) { switch (type) { - case MetaData::Release::SecondaryType::Compilation: - res.insert(ReleaseTypeSecondary::Compilation); - break; - case MetaData::Release::SecondaryType::Soundtrack: - res.insert(ReleaseTypeSecondary::Soundtrack); - break; - case MetaData::Release::SecondaryType::Spokenword: - res.insert(ReleaseTypeSecondary::Spokenword); - break; - case MetaData::Release::SecondaryType::Interview: - res.insert(ReleaseTypeSecondary::Interview); - break; - case MetaData::Release::SecondaryType::Audiobook: - res.insert(ReleaseTypeSecondary::Audiobook); - break; - case MetaData::Release::SecondaryType::AudioDrama: - res.insert(ReleaseTypeSecondary::AudioDrama); - break; - case MetaData::Release::SecondaryType::Live: - res.insert(ReleaseTypeSecondary::Live); - break; - case MetaData::Release::SecondaryType::Remix: - res.insert(ReleaseTypeSecondary::Remix); - break; - case MetaData::Release::SecondaryType::DJMix: - res.insert(ReleaseTypeSecondary::DJMix); - break; - case MetaData::Release::SecondaryType::Mixtape_Street: - res.insert(ReleaseTypeSecondary::Mixtape_Street); - break; - case MetaData::Release::SecondaryType::Demo: - res.insert(ReleaseTypeSecondary::Demo); - break; + case MetaData::Release::PrimaryType::Album: return ReleaseTypePrimary::Album; + case MetaData::Release::PrimaryType::Single: return ReleaseTypePrimary::Single; + case MetaData::Release::PrimaryType::EP: return ReleaseTypePrimary::EP; + case MetaData::Release::PrimaryType::Broadcast: return ReleaseTypePrimary::Broadcast; + case MetaData::Release::PrimaryType::Other: return ReleaseTypePrimary::Other; } + + return ReleaseTypePrimary::Other; } - return res; - } - - void - updateReleaseIfNeeded(Release::pointer release, const MetaData::Release& releaseInfo) - { - if (release->getName() != releaseInfo.name) - release.modify()->setName(releaseInfo.name); - if (release->getTotalDisc() != releaseInfo.mediumCount) - release.modify()->setTotalDisc(releaseInfo.mediumCount); - if (releaseInfo.primaryType) + EnumSet convertReleaseTypesSecondary(EnumSet types) { - const ReleaseTypePrimary primaryType{ convertReleaseTypePrimary(*releaseInfo.primaryType) }; - if (release->getPrimaryType() != primaryType) - release.modify()->setPrimaryType(primaryType); - } - const EnumSet secondaryTypes{ convertReleaseTypesSecondary(releaseInfo.secondaryTypes) }; - if (release->getSecondaryTypes() != secondaryTypes) - release.modify()->setSecondaryTypes(secondaryTypes); - if (release->getArtistDisplayName() != releaseInfo.artistDisplayName) - release.modify()->setArtistDisplayName(releaseInfo.artistDisplayName); - } + EnumSet res; - Release::pointer - getOrCreateRelease(Session& session, const MetaData::Release& releaseInfo) - { - Release::pointer release; - - // First try to get by MBID - if (releaseInfo.mbid) - { - release = Release::find(session, *releaseInfo.mbid); - if (!release) - release = session.create(releaseInfo.name, releaseInfo.mbid); - - updateReleaseIfNeeded(release, releaseInfo); - return release; - } - - // Fall back on release name (collisions may occur) - if (!releaseInfo.name.empty()) - { - for (const Release::pointer& sameNamedRelease : Release::find(session, releaseInfo.name)) + for (MetaData::Release::SecondaryType type : types) { - // do not fallback on properly tagged releases - if (sameNamedRelease->getMBID()) + switch (type) + { + case MetaData::Release::SecondaryType::Compilation: + res.insert(ReleaseTypeSecondary::Compilation); + break; + case MetaData::Release::SecondaryType::Soundtrack: + res.insert(ReleaseTypeSecondary::Soundtrack); + break; + case MetaData::Release::SecondaryType::Spokenword: + res.insert(ReleaseTypeSecondary::Spokenword); + break; + case MetaData::Release::SecondaryType::Interview: + res.insert(ReleaseTypeSecondary::Interview); + break; + case MetaData::Release::SecondaryType::Audiobook: + res.insert(ReleaseTypeSecondary::Audiobook); + break; + case MetaData::Release::SecondaryType::AudioDrama: + res.insert(ReleaseTypeSecondary::AudioDrama); + break; + case MetaData::Release::SecondaryType::Live: + res.insert(ReleaseTypeSecondary::Live); + break; + case MetaData::Release::SecondaryType::Remix: + res.insert(ReleaseTypeSecondary::Remix); + break; + case MetaData::Release::SecondaryType::DJMix: + res.insert(ReleaseTypeSecondary::DJMix); + break; + case MetaData::Release::SecondaryType::Mixtape_Street: + res.insert(ReleaseTypeSecondary::Mixtape_Street); + break; + case MetaData::Release::SecondaryType::Demo: + res.insert(ReleaseTypeSecondary::Demo); + break; + } + } + + return res; + } + + void updateReleaseIfNeeded(Release::pointer release, const MetaData::Release& releaseInfo) + { + if (release->getName() != releaseInfo.name) + release.modify()->setName(releaseInfo.name); + if (release->getTotalDisc() != releaseInfo.mediumCount) + release.modify()->setTotalDisc(releaseInfo.mediumCount); + if (releaseInfo.primaryType) + { + const ReleaseTypePrimary primaryType{ convertReleaseTypePrimary(*releaseInfo.primaryType) }; + if (release->getPrimaryType() != primaryType) + release.modify()->setPrimaryType(primaryType); + } + const EnumSet secondaryTypes{ convertReleaseTypesSecondary(releaseInfo.secondaryTypes) }; + if (release->getSecondaryTypes() != secondaryTypes) + release.modify()->setSecondaryTypes(secondaryTypes); + if (release->getArtistDisplayName() != releaseInfo.artistDisplayName) + release.modify()->setArtistDisplayName(releaseInfo.artistDisplayName); + } + + Release::pointer getOrCreateRelease(Session& session, const MetaData::Release& releaseInfo, const std::filesystem::path& expectedReleaseDirectory) + { + Release::pointer release; + + // First try to get by MBID + if (releaseInfo.mbid) + { + release = Release::find(session, *releaseInfo.mbid); + if (!release) + release = session.create(releaseInfo.name, releaseInfo.mbid); + + updateReleaseIfNeeded(release, releaseInfo); + return release; + } + + // Fall back on release name (collisions may occur), if and only if it is in the current directory + if (!releaseInfo.name.empty()) + { + for (const Release::pointer& sameNamedRelease : Release::find(session, releaseInfo.name, expectedReleaseDirectory)) + { + // do not fallback on properly tagged releases + if (sameNamedRelease->getMBID()) + continue; + + release = sameNamedRelease; + break; + } + + // No release found with the same name and without MBID -> creating + if (!release) + release = session.create(releaseInfo.name); + + updateReleaseIfNeeded(release, releaseInfo); + return release; + } + + return Release::pointer{}; + } + + std::vector getOrCreateClusters(Session& session, const MetaData::Tags& tags) + { + std::vector clusters; + + for (const auto& [tag, values] : tags) + { + auto clusterType = ClusterType::find(session, tag); + if (!clusterType) continue; - release = sameNamedRelease; - break; + for (auto clusterName : values) + { + auto cluster = clusterType->getCluster(clusterName); + if (!cluster) + cluster = session.create(clusterType, clusterName); + + clusters.push_back(cluster); + } } - // No release found with the same name and without MBID -> creating - if (!release) - release = session.create(releaseInfo.name); - - updateReleaseIfNeeded(release, releaseInfo); - return release; + return clusters; } - return Release::pointer{}; - } - - std::vector - getOrCreateClusters(Session& session, const MetaData::Tags& tags) - { - std::vector clusters; - - for (const auto& [tag, values] : tags) + MetaData::ParserReadStyle getParserReadStyle() { - auto clusterType = ClusterType::find(session, tag); - if (!clusterType) - continue; + std::string_view readStyle{ Service::get()->getString("scanner-parser-read-style", "accurate") }; - for (auto clusterName : values) - { - auto cluster = clusterType->getCluster(clusterName); - if (!cluster) - cluster = session.create(clusterType, clusterName); + if (readStyle == "fast") + return MetaData::ParserReadStyle::Fast; + else if (readStyle == "average") + return MetaData::ParserReadStyle::Average; + else if (readStyle == "accurate") + return MetaData::ParserReadStyle::Accurate; - clusters.push_back(cluster); - } + throw LmsException{ "Invalid value for 'scanner-parser-read-style'" }; } + } // namespace - return clusters; - } - - MetaData::ParserReadStyle - getParserReadStyle() - { - std::string_view readStyle{ Service::get()->getString("scanner-parser-read-style", "accurate") }; - - if (readStyle == "fast") - return MetaData::ParserReadStyle::Fast; - else if (readStyle == "average") - return MetaData::ParserReadStyle::Average; - else if (readStyle == "accurate") - return MetaData::ParserReadStyle::Accurate; - - throw LmsException{ "Invalid value for 'scanner-parser-read-style'" }; - } -} // namespace - -namespace Scanner -{ ScanStepScanFiles::ScanStepScanFiles(InitParams& initParams) : ScanStepBase{ initParams } , _metadataParser{ MetaData::createParser(MetaData::ParserType::TagLib, getParserReadStyle()) } // For now, always use TagLib { } - void - ScanStepScanFiles::process(ScanContext& context) + void ScanStepScanFiles::process(ScanContext& context) { _metadataParser->setClusterTypeNames(_settings.clusterTypeNames); @@ -317,8 +309,7 @@ namespace Scanner }, &excludeDirFileName); } - void - ScanStepScanFiles::scanAudioFile(const std::filesystem::path& file, ScanContext& context) + void ScanStepScanFiles::scanAudioFile(const std::filesystem::path& file, ScanContext& context) { ScanStats& stats{ context.stats }; Wt::WDateTime lastWriteTime; @@ -488,7 +479,7 @@ namespace Scanner track.modify()->setScanVersion(_settings.scanVersion); if (trackInfo->medium && trackInfo->medium->release) - track.modify()->setRelease(getOrCreateRelease(dbSession, *trackInfo->medium->release)); + track.modify()->setRelease(getOrCreateRelease(dbSession, *trackInfo->medium->release, file.parent_path())); else track.modify()->setRelease({}); track.modify()->setTotalTrack(trackInfo->medium ? trackInfo->medium->trackCount : std::nullopt);