Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iTunes: Share common import logic between XML and macOS #11500

Merged
merged 21 commits into from
Apr 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
b3d6055
ITunesMacOSImporter: Factor out backend
fwcd Apr 19, 2023
f0bd704
ITunesImportBackend: Add constant for root playlist id
fwcd Apr 19, 2023
92c7b57
ITunesImportBackend: Add applyPathMapping
fwcd Apr 19, 2023
de47e20
ITunesXMLImporter: Use ITunesImportBackend
fwcd Apr 19, 2023
ac5aad8
ITunesXMLImporter: Import playlist relations
fwcd Apr 19, 2023
e652ff5
ITunesXMLImporter: Skip system playlists
fwcd Apr 19, 2023
cca170c
ITunesXMLImporter: Clean up variables in parsePlaylist
fwcd Apr 19, 2023
3056ea5
ITunesImportBackend: Filter out zeros before binding
fwcd Apr 19, 2023
a55d934
ITunesImportBackend: Handle empty year and track number
fwcd Apr 19, 2023
7197902
ITunesImportBackend: Avoid unnecessary copies
fwcd Apr 21, 2023
550d23b
ITunesImportBackend: Rename to ITunesDAO
fwcd Apr 21, 2023
47392bc
ITunesDAO: Inherit from DAO
fwcd Apr 21, 2023
fd429b6
ITunesImporter: Rename DAO member to m_dao
fwcd Apr 24, 2023
36f0775
ITunesFeature: Move ITunesDAO ownership into ITunesFeature
fwcd Apr 24, 2023
2c1b3b5
ITunesDAO: Make methods virtual
fwcd Apr 24, 2023
037008b
ITunesDAO: Pass TreeItem by not-null pointer
fwcd Apr 24, 2023
8140551
ITunesDAO: Translate '(empty)'
fwcd Apr 24, 2023
e626aa1
ITunesImporter: Use C++20-style designated initializers
fwcd Apr 24, 2023
7cf3503
ITunesFeature: Move DAO ownership to importer again
fwcd Apr 24, 2023
e4ef032
ITunesDAO: Add short note on the database reference
fwcd Apr 25, 2023
d71ab67
ITunesXMLImporter: Add missing designated init
fwcd Apr 25, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
src/library/export/trackexportworker.cpp
src/library/externaltrackcollection.cpp
src/library/hiddentablemodel.cpp
src/library/itunes/itunesdao.cpp
src/library/itunes/itunesfeature.cpp
src/library/itunes/itunesxmlimporter.cpp
src/library/library_prefs.cpp
Expand Down
155 changes: 155 additions & 0 deletions src/library/itunes/itunesdao.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
#include "library/itunes/itunesdao.h"

#include <QObject>
#include <QSqlQuery>
#include <gsl/pointers>

#include "library/itunes/ituneslocalhosttoken.h"
#include "library/itunes/itunespathmapping.h"
#include "library/queryutil.h"

void ITunesDAO::initialize(const QSqlDatabase& database) {
m_insertTrackQuery = QSqlQuery(database);
m_insertPlaylistQuery = QSqlQuery(database);
m_insertPlaylistTrackQuery = QSqlQuery(database);
m_applyPathMappingQuery = QSqlQuery(database);

m_insertTrackQuery.prepare(
"INSERT INTO itunes_library (id, artist, title, album, "
"album_artist, genre, grouping, year, duration, "
"location, rating, comment, tracknumber, bpm, bitrate) "
"VALUES (:id, :artist, :title, :album, :album_artist, "
":genre, :grouping, :year, :duration, :location, "
":rating, :comment, :tracknumber, :bpm, :bitrate)");

m_insertPlaylistQuery.prepare("INSERT INTO itunes_playlists (id, name) VALUES (:id, :name)");

m_insertPlaylistTrackQuery.prepare(
"INSERT INTO itunes_playlist_tracks (playlist_id, track_id, "
"position) VALUES (:playlist_id, :track_id, :position)");

m_applyPathMappingQuery.prepare(
"UPDATE itunes_library SET location = replace( location, "
":itunes_path, :mixxx_path )");
}

bool ITunesDAO::importTrack(const ITunesTrack& track) {
QSqlQuery& query = m_insertTrackQuery;

query.bindValue(":id", track.id);
query.bindValue(":artist", track.artist);
query.bindValue(":title", track.title);
query.bindValue(":album", track.album);
query.bindValue(":album_artist", track.albumArtist);
query.bindValue(":genre", track.genre);
query.bindValue(":grouping", track.grouping);
query.bindValue(":year", track.year > 0 ? QVariant(track.year) : QVariant());
query.bindValue(":duration", track.duration);
query.bindValue(":location", track.location);
query.bindValue(":rating", track.rating);
query.bindValue(":comment", track.comment);
query.bindValue(":tracknumber",
track.trackNumber > 0 ? QVariant(track.trackNumber) : QVariant());
query.bindValue(":bpm", track.bpm);
query.bindValue(":bitrate", track.bitrate);

if (!query.exec()) {
LOG_FAILED_QUERY(query);
return false;
}

return true;
}

bool ITunesDAO::importPlaylist(const ITunesPlaylist& playlist) {
QString uniqueName = uniquifyPlaylistName(playlist.name);
QSqlQuery& query = m_insertPlaylistQuery;

query.bindValue(":id", playlist.id);
query.bindValue(":name", uniqueName);

if (!query.exec()) {
LOG_FAILED_QUERY(query);
return false;
}

m_playlistNameById[playlist.id] = uniqueName;

return true;
}

bool ITunesDAO::importPlaylistRelation(int parentId, int childId) {
m_playlistIdsByParentId.insert({parentId, childId});
return true;
}

bool ITunesDAO::importPlaylistTrack(int playlistId, int trackId, int position) {
QSqlQuery& query = m_insertPlaylistTrackQuery;

query.bindValue(":playlist_id", playlistId);
query.bindValue(":track_id", trackId);
query.bindValue(":position", position);

if (!query.exec()) {
LOG_FAILED_QUERY(query);
return false;
}

return true;
}

bool ITunesDAO::applyPathMapping(const ITunesPathMapping& pathMapping) {
QSqlQuery& query = m_insertPlaylistTrackQuery;

query.bindValue(":itunes_path",
QString(pathMapping.dbITunesRoot).replace(kiTunesLocalhostToken, ""));
query.bindValue(":mixxx_path", pathMapping.mixxxITunesRoot);

if (!query.exec()) {
LOG_FAILED_QUERY(query);
return false;
}

return true;
}

void ITunesDAO::appendPlaylistTree(gsl::not_null<TreeItem*> item, int playlistId) {
auto childsRange = m_playlistIdsByParentId.equal_range(playlistId);
std::for_each(childsRange.first,
childsRange.second,
[this, &item](auto childEntry) {
int childId = childEntry.second;
QString childName = m_playlistNameById[childId];
TreeItem* child = item->appendChild(childName);
appendPlaylistTree(child, childId);
});
}

QString ITunesDAO::uniquifyPlaylistName(QString name) {
// iTunes permits users to have multiple playlists with the same name,
// our data model (both the database schema and the tree items) however
// require unique names since they identify the playlist via this name.
// We therefore keep track of duplicates and append a suffix
// accordingly. E.g. if the user has three playlists named 'House' in
// their library, the playlists would get named (in this order):
//
// House
// House #2
// House #3
//

// Avoid empty playlist names
if (name.isEmpty()) {
name = QObject::tr("(empty)");
}

auto existing = m_playlistDuplicatesByName.find(name);
if (existing != m_playlistDuplicatesByName.end()) {
m_playlistDuplicatesByName[name] += 1;
return QString("%1 #%2").arg(name).arg(
m_playlistDuplicatesByName[name] + 1);
} else {
m_playlistDuplicatesByName[name] = 0;
return name;
}
}
70 changes: 70 additions & 0 deletions src/library/itunes/itunesdao.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
#pragma once

#include <QHash>
#include <QSqlDatabase>
#include <QSqlQuery>
#include <QString>
#include <gsl/pointers>
#include <map>

#include "library/dao/dao.h"
#include "library/itunes/itunespathmapping.h"
#include "library/treeitem.h"

const int kRootITunesPlaylistId = -1;

struct ITunesTrack {
int id;
QString artist;
QString title;
QString album;
QString albumArtist;
QString genre;
QString grouping;
int year;
int duration;
QString location;
int rating;
QString comment;
int trackNumber;
int bpm;
int bitrate;
};

struct ITunesPlaylist {
int id;
QString name;
};

/// A wrapper around the iTunes database tables. Keeps track of the
/// playlist tree, deals with duplicate disambiguation and can export
/// the tree afterwards.
class ITunesDAO : public DAO {
public:
~ITunesDAO() override = default;

void initialize(const QSqlDatabase& database) override;

virtual bool importTrack(const ITunesTrack& track);
virtual bool importPlaylist(const ITunesPlaylist& playlist);
Comment on lines +48 to +49
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the fact that these have to be virtual just in order to be tested. Is there no feature in gtest that can avoid that using some linker hacks?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've thought about this and the other option would be to use templates and parameterize the importer over the DAO. Unfortunately, that would also mean that we'd have to move the implementation into headers etc.

Since virtual methods are generally not thaat expensive (I've tested it on a library with hundreds of playlists and >10k tracks with pretty much no noticeable performance difference), I think this should be fine. The database calls are likely more an order of magnitude more expensive than the virtual function calls.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, in the scope of everything else, those virtual calls are probably going to be cheap, I'm just used to them being expensive in the context of cache latency and branch prediction.

virtual bool importPlaylistRelation(int parentId, int childId);
virtual bool importPlaylistTrack(int playlistId, int trackId, int position);
virtual bool applyPathMapping(const ITunesPathMapping& pathMapping);

virtual void appendPlaylistTree(gsl::not_null<TreeItem*> item,
int playlistId = kRootITunesPlaylistId);

private:
QHash<QString, int> m_playlistDuplicatesByName;
QHash<int, QString> m_playlistNameById;
std::multimap<int, int> m_playlistIdsByParentId;

// Note that these queries reference the database, which is expected
// to outlive the DAO.
QSqlQuery m_insertTrackQuery;
QSqlQuery m_insertPlaylistQuery;
QSqlQuery m_insertPlaylistTrackQuery;
QSqlQuery m_applyPathMappingQuery;

QString uniquifyPlaylistName(QString name);
};
7 changes: 5 additions & 2 deletions src/library/itunes/itunesfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "library/baseexternaltrackmodel.h"
#include "library/basetrackcache.h"
#include "library/dao/settingsdao.h"
#include "library/itunes/itunesdao.h"
#include "library/itunes/itunesimporter.h"
#include "library/itunes/ituneslocalhosttoken.h"
#include "library/itunes/itunesxmlimporter.h"
Expand Down Expand Up @@ -307,14 +308,16 @@ QString ITunesFeature::getiTunesMusicPath() {
}

std::unique_ptr<ITunesImporter> ITunesFeature::makeImporter() {
std::unique_ptr<ITunesDAO> dao = std::make_unique<ITunesDAO>();
dao->initialize(m_database);
#ifdef __MACOS_ITUNES_LIBRARY__
if (isMacOSImporterUsed()) {
qDebug() << "Using ITunesMacOSImporter to read default iTunes library";
return std::make_unique<ITunesMacOSImporter>(this, m_database, m_cancelImport);
return std::make_unique<ITunesMacOSImporter>(this, m_cancelImport, std::move(dao));
}
#endif
qDebug() << "Using ITunesXMLImporter to read iTunes library from " << m_dbfile;
return std::make_unique<ITunesXMLImporter>(this, m_dbfile, m_database, m_cancelImport);
return std::make_unique<ITunesXMLImporter>(this, m_dbfile, m_cancelImport, std::move(dao));
}

// This method is executed in a separate thread
Expand Down
8 changes: 5 additions & 3 deletions src/library/itunes/itunesmacosimporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
#include <QSqlDatabase>
#include <QString>
#include <atomic>
#include <memory>

#include "library/itunes/itunesdao.h"
#include "library/itunes/itunesimporter.h"
#include "library/libraryfeature.h"

Expand All @@ -12,8 +14,8 @@
class ITunesMacOSImporter : public ITunesImporter {
public:
ITunesMacOSImporter(LibraryFeature* parentFeature,
const QSqlDatabase& database,
const std::atomic<bool>& cancelImport);
const std::atomic<bool>& cancelImport,
std::unique_ptr<ITunesDAO> dao);

ITunesImport importLibrary() override;

Expand All @@ -22,6 +24,6 @@ class ITunesMacOSImporter : public ITunesImporter {
// The values behind these references are owned by the parent `ITunesFeature`,
// thus there is an implicit contract here that this `ITunesMacOSImporter` cannot
// outlive the feature (which should not happen anyway, since importers are short-lived).
const QSqlDatabase& m_database;
const std::atomic<bool>& m_cancelImport;
std::unique_ptr<ITunesDAO> m_dao;
};
Loading