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