From 65fcb62e1141f7f61281e06a859a89c2b1cbe5ca Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Thu, 27 Dec 2018 00:30:09 +0100 Subject: [PATCH 1/3] Get the (canonical) URL/URI of a track object --- src/library/basesqltablemodel.cpp | 3 +- src/library/browse/browsetablemodel.cpp | 3 +- src/sources/soundsourceproxy.cpp | 25 ++------------ src/track/track.cpp | 32 ++++++++++++++++++ src/track/track.h | 7 ++++ src/track/trackref.cpp | 44 +++++++++++++++++++++++++ src/track/trackref.h | 7 ++++ 7 files changed, 94 insertions(+), 27 deletions(-) diff --git a/src/library/basesqltablemodel.cpp b/src/library/basesqltablemodel.cpp index 415d2fa618e..80ed7b683c2 100644 --- a/src/library/basesqltablemodel.cpp +++ b/src/library/basesqltablemodel.cpp @@ -20,7 +20,6 @@ #include "track/trackmetadata.h" #include "util/db/dbconnection.h" #include "util/duration.h" -#include "util/dnd.h" #include "util/assert.h" #include "util/performancetimer.h" @@ -1094,7 +1093,7 @@ QMimeData* BaseSqlTableModel::mimeData(const QModelIndexList &indexes) const { continue; } rows.insert(index.row()); - QUrl url = DragAndDropHelper::urlFromLocation(getTrackLocation(index)); + QUrl url = TrackRef::locationUrl(getTrackLocation(index)); if (!url.isValid()) { qDebug() << this << "ERROR: invalid url" << url; continue; diff --git a/src/library/browse/browsetablemodel.cpp b/src/library/browse/browsetablemodel.cpp index fe9679b43e7..0b2d04eb757 100644 --- a/src/library/browse/browsetablemodel.cpp +++ b/src/library/browse/browsetablemodel.cpp @@ -13,7 +13,6 @@ #include "mixer/playermanager.h" #include "control/controlobject.h" #include "library/dao/trackdao.h" -#include "util/dnd.h" BrowseTableModel::BrowseTableModel(QObject* parent, TrackCollection* pTrackCollection, @@ -243,7 +242,7 @@ QMimeData* BrowseTableModel::mimeData(const QModelIndexList &indexes) const { if (index.isValid()) { if (!rows.contains(index.row())) { rows.push_back(index.row()); - QUrl url = DragAndDropHelper::urlFromLocation(getTrackLocation(index)); + QUrl url = TrackRef::locationUrl(getTrackLocation(index)); if (!url.isValid()) { qDebug() << "ERROR invalid url" << url; continue; diff --git a/src/sources/soundsourceproxy.cpp b/src/sources/soundsourceproxy.cpp index fbd28ad3006..f6889c3c585 100644 --- a/src/sources/soundsourceproxy.cpp +++ b/src/sources/soundsourceproxy.cpp @@ -51,27 +51,6 @@ namespace { const mixxx::Logger kLogger("SoundSourceProxy"); -QUrl getCanonicalUrlForTrack(const Track* pTrack) { - if (pTrack == nullptr) { - // Missing track - return QUrl(); - } - const QString canonicalLocation(pTrack->getCanonicalLocation()); - if (canonicalLocation.isEmpty()) { - // Corresponding file is missing or inaccessible - // - // NOTE(uklotzde): Special case handling is required for Qt 4.8! - // Creating an URL from an empty local file in Qt 4.8 will result - // in an URL with the string "file:" instead of an empty URL. - // - // TODO(XXX): This is no longer required for Qt 5.x - // http://doc.qt.io/qt-5/qurl.html#fromLocalFile - // "An empty localFile leads to an empty URL (since Qt 5.4)." - return QUrl(); - } - return QUrl::fromLocalFile(canonicalLocation); -} - } // anonymous namespace // static @@ -229,7 +208,7 @@ Track::ExportMetadataResult SoundSourceProxy::exportTrackMetadataBeforeSaving(Track* pTrack) { DEBUG_ASSERT(pTrack); mixxx::MetadataSourcePointer pMetadataSource = - SoundSourceProxy(getCanonicalUrlForTrack(pTrack)).m_pSoundSource; + SoundSourceProxy(pTrack->getCanonicalLocationUrl()).m_pSoundSource; if (pMetadataSource) { return pTrack->exportMetadata(pMetadataSource); } else { @@ -243,7 +222,7 @@ SoundSourceProxy::exportTrackMetadataBeforeSaving(Track* pTrack) { SoundSourceProxy::SoundSourceProxy( TrackPointer pTrack) : m_pTrack(std::move(pTrack)), - m_url(getCanonicalUrlForTrack(m_pTrack.get())), + m_url(m_pTrack ? m_pTrack->getCanonicalLocationUrl() : QUrl()), m_soundSourceProviderRegistrations(findSoundSourceProviderRegistrations(m_url)), m_soundSourceProviderRegistrationIndex(0) { initSoundSource(); diff --git a/src/track/track.cpp b/src/track/track.cpp index 5e8559e0333..c0812043c13 100644 --- a/src/track/track.cpp +++ b/src/track/track.cpp @@ -222,6 +222,38 @@ QString Track::getCanonicalLocation() const { return loc; } +QUrl Track::getLocationUrl() const { + // Copying QFileInfo is thread-safe due to "implicit sharing" + // (copy-on write). But operating on a single instance of QFileInfo + // might not be thread-safe due to internal caching! + QMutexLocker lock(&m_qMutex); + return TrackRef::locationUrl(m_fileInfo); +} + +QUrl Track::getCanonicalLocationUrl() const { + // Copying QFileInfo is thread-safe due to "implicit sharing" + // (copy-on write). But operating on a single instance of QFileInfo + // might not be thread-safe due to internal caching! + QMutexLocker lock(&m_qMutex); + return TrackRef::canonicalLocationUrl(m_fileInfo); +} + +QString Track::getLocationUri() const { + // Copying QFileInfo is thread-safe due to "implicit sharing" + // (copy-on write). But operating on a single instance of QFileInfo + // might not be thread-safe due to internal caching! + QMutexLocker lock(&m_qMutex); + return TrackRef::locationUri(m_fileInfo); +} + +QString Track::getCanonicalLocationUri() const { + // Copying QFileInfo is thread-safe due to "implicit sharing" + // (copy-on write). But operating on a single instance of QFileInfo + // might not be thread-safe due to internal caching! + QMutexLocker lock(&m_qMutex); + return TrackRef::canonicalLocationUri(m_fileInfo); +} + QString Track::getDirectory() const { // Copying QFileInfo is thread-safe due to "implicit sharing" // (copy-on write). But operating on a single instance of QFileInfo diff --git a/src/track/track.h b/src/track/track.h index cfdc2d3a4e6..dff3d9a78c7 100644 --- a/src/track/track.h +++ b/src/track/track.h @@ -4,6 +4,7 @@ #include #include #include +#include #include "track/beats.h" #include "track/cue.h" @@ -21,6 +22,8 @@ class Track; typedef std::shared_ptr TrackPointer; typedef std::weak_ptr TrackWeakPointer; +Q_DECLARE_METATYPE(TrackPointer); + class Track : public QObject { Q_OBJECT @@ -79,7 +82,11 @@ class Track : public QObject { // Accessors for various stats of the file on disk. // Returns absolute path to the file, including the filename. QString getLocation() const; + QUrl getLocationUrl() const; + QString getLocationUri() const; QString getCanonicalLocation() const; + QUrl getCanonicalLocationUrl() const; + QString getCanonicalLocationUri() const; // Returns the absolute path to the directory containing the file QString getDirectory() const; // Returns the name of the file. diff --git a/src/track/trackref.cpp b/src/track/trackref.cpp index 663a2f9790a..77e8f93f990 100644 --- a/src/track/trackref.cpp +++ b/src/track/trackref.cpp @@ -1,6 +1,50 @@ #include "track/trackref.h" +namespace { + +inline +QUrl urlFromLocalFilePath(QString localFilePath) { +#if QT_VERSION >= QT_VERSION_CHECK(5, 4, 0) + return QUrl::fromLocalFile(std::move(localFilePath)); +#else + if (localFilePath.isEmpty()) { + return QUrl(); + } else { + return QUrl::fromLocalFile(std::move(localFilePath)); + } +#endif +} + +inline +QString uriFromUrl(const QUrl& url) { + return url.toEncoded( + QUrl::StripTrailingSlash | + QUrl::NormalizePathSegments); +} + +} // anonymous namespace + +//static +QUrl TrackRef::locationUrl(const QFileInfo& fileInfo) { + return urlFromLocalFilePath(location(fileInfo)); +} + +//static +QUrl TrackRef::canonicalLocationUrl(const QFileInfo& fileInfo) { + return urlFromLocalFilePath(canonicalLocation(fileInfo)); +} + +//static +QString TrackRef::locationUri(const QFileInfo& fileInfo) { + return uriFromUrl(locationUrl(fileInfo)); +} + +//static +QString TrackRef::canonicalLocationUri(const QFileInfo& fileInfo) { + return uriFromUrl(canonicalLocationUrl(fileInfo)); +} + bool TrackRef::verifyConsistency() const { // Class invariant: The location can only be set together with // at least one of the other members! diff --git a/src/track/trackref.h b/src/track/trackref.h index c39974f44b2..b2cde967a5f 100644 --- a/src/track/trackref.h +++ b/src/track/trackref.h @@ -3,6 +3,7 @@ #include +#include #include "track/trackid.h" @@ -29,6 +30,12 @@ class TrackRef { return fileInfo.canonicalFilePath(); } + static QUrl locationUrl(const QFileInfo& fileInfo); + static QUrl canonicalLocationUrl(const QFileInfo& fileInfo); + + static QString locationUri(const QFileInfo& fileInfo); + static QString canonicalLocationUri(const QFileInfo& fileInfo); + // Converts a QFileInfo and an optional TrackId into a TrackRef. This // involves obtaining the file-related track properties from QFileInfo // (see above) and should used consciously! From bcf22a0d14356d2403dd456fdfd3fb020ccf889b Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Thu, 13 Jun 2019 17:44:29 +0200 Subject: [PATCH 2/3] Avoid unnecessary std::move --- src/track/trackref.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/track/trackref.cpp b/src/track/trackref.cpp index 77e8f93f990..f981263b86f 100644 --- a/src/track/trackref.cpp +++ b/src/track/trackref.cpp @@ -4,14 +4,14 @@ namespace { inline -QUrl urlFromLocalFilePath(QString localFilePath) { +QUrl urlFromLocalFilePath(const QString& localFilePath) { #if QT_VERSION >= QT_VERSION_CHECK(5, 4, 0) - return QUrl::fromLocalFile(std::move(localFilePath)); + return QUrl::fromLocalFile(localFilePath); #else if (localFilePath.isEmpty()) { return QUrl(); } else { - return QUrl::fromLocalFile(std::move(localFilePath)); + return QUrl::fromLocalFile(localFilePath); } #endif } From 5421769b421b85515d49d67eb2f30f3f3aae75ed Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Thu, 13 Jun 2019 17:44:44 +0200 Subject: [PATCH 3/3] Rename function --- src/track/trackref.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/track/trackref.cpp b/src/track/trackref.cpp index f981263b86f..bcd87bc3bad 100644 --- a/src/track/trackref.cpp +++ b/src/track/trackref.cpp @@ -17,7 +17,7 @@ QUrl urlFromLocalFilePath(const QString& localFilePath) { } inline -QString uriFromUrl(const QUrl& url) { +QString urlToString(const QUrl& url) { return url.toEncoded( QUrl::StripTrailingSlash | QUrl::NormalizePathSegments); @@ -37,12 +37,12 @@ QUrl TrackRef::canonicalLocationUrl(const QFileInfo& fileInfo) { //static QString TrackRef::locationUri(const QFileInfo& fileInfo) { - return uriFromUrl(locationUrl(fileInfo)); + return urlToString(locationUrl(fileInfo)); } //static QString TrackRef::canonicalLocationUri(const QFileInfo& fileInfo) { - return uriFromUrl(canonicalLocationUrl(fileInfo)); + return urlToString(canonicalLocationUrl(fileInfo)); } bool TrackRef::verifyConsistency() const {