Move ITunesImporter::canceled() to a common base class

CMakeLists.txt

@@ -798,6 +798,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
   src/library/hiddentablemodel.cpp
   src/library/itunes/itunesdao.cpp
   src/library/itunes/itunesfeature.cpp
+  src/library/itunes/itunesimporter.cpp
   src/library/itunes/itunesplaylistmodel.cpp
   src/library/itunes/itunesxmlimporter.cpp
   src/library/library_prefs.cpp

src/library/itunes/itunesimporter.cpp

@@ -0,0 +1,14 @@
+#include "library/itunes/itunesfeature.h"
+#include "library/itunes/itunesxmlimporter.h"
+
+ITunesImporter::ITunesImporter(ITunesFeature* pParentFeature)
+        : m_pParentFeature(pParentFeature) {
+}
+
+bool ITunesImporter::canceled() {
+    // The parent feature may be null during testing
+    if (m_pParentFeature) {
+        return m_pParentFeature->isImportCanceled();
+    }
+    return false;
+}

src/library/itunes/itunesimporter.h

@@ -2,22 +2,23 @@
 
 #include <memory>
 
-#include "library/treeitem.h"
+class TreeItem;
+class ITunesFeature;
 
 struct ITunesImport {
     std::unique_ptr<TreeItem> playlistRoot;
 };
 
 class ITunesImporter {
   public:
+    ITunesImporter(ITunesFeature* pParentFeature);
+
     virtual ~ITunesImporter() = default;
 
     virtual ITunesImport importLibrary() = 0;
 
-    // TODO: Add thread-safe `cancelImport` method and move `m_cancelImport`
-    // from ITunesFeature to the individual importers (replacing the current
-    // mechanism of passing (the private member) `m_cancelImport` by reference
-    // to each importer). We should then call `cancelImport` from the
-    // `ITunesFeature` destructor on the importer (which we'd need to store e.g.
-    // in a member of `ITunesFeature`).
+    bool canceled();
+
+  protected:
+    ITunesFeature* m_pParentFeature;
 };

src/library/itunes/itunesmacosimporter.h

@@ -5,22 +5,22 @@
 #include <atomic>
 #include <memory>
 
-#include "library/itunes/itunesdao.h"
 #include "library/itunes/itunesimporter.h"
-#include "library/libraryfeature.h"
+
+class ITunesDAO;
+class ITunesFeature;
 
 /// An importer that reads the user's default iTunes/Music.app library
 /// using the native `iTunesLibrary` framework on macOS.
 class ITunesMacOSImporter : public ITunesImporter {
   public:
-    ITunesMacOSImporter(LibraryFeature* parentFeature,
-            const std::atomic<bool>& cancelImport,
+    ITunesMacOSImporter(
+            ITunesFeature* pParentFeature,
             std::unique_ptr<ITunesDAO> dao);
 
     ITunesImport importLibrary() override;
 
   private:
-    LibraryFeature* m_parentFeature;
     // 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).

src/library/itunes/itunesmacosimporter.mm

@@ -2,7 +2,6 @@
 
 #import <iTunesLibrary/iTunesLibrary.h>
 #include <gsl/pointers>
-#include "library/itunes/itunesdao.h"
 
 #include <QHash>
 #include <QSqlDatabase>
@@ -16,8 +15,8 @@
 #include <optional>
 #include <utility>
 
-#include "library/itunes/itunesimporter.h"
-#include "library/libraryfeature.h"
+#include "library/itunes/itunesdao.h"
+#include "library/itunes/itunesfeature.h"
 #include "library/queryutil.h"
 #include "library/treeitem.h"
 #include "library/treeitemmodel.h"
@@ -30,8 +29,8 @@ QString qStringFrom(NSString* nsString) {
 
 class ImporterImpl {
   public:
-    ImporterImpl(const std::atomic<bool>& cancelImport, ITunesDAO& dao)
-            : m_cancelImport(cancelImport), m_dao(dao) {
+    ImporterImpl(ITunesMacOSImporter* pImporter, ITunesDAO& dao)
+            : m_pImporter(pImporter), m_dao(dao) {
     }
 
     void importPlaylists(NSArray<ITLibPlaylist*>* playlists) {
@@ -43,7 +42,7 @@ void importPlaylists(NSArray<ITLibPlaylist*>* playlists) {
         // interact well with Objective-C collections.
 
         for (ITLibPlaylist* playlist in playlists) {
-            if (m_cancelImport.load()) {
+            if (m_pImporter->canceled()) {
                 break;
             }
 
@@ -55,7 +54,7 @@ void importMediaItems(NSArray<ITLibMediaItem*>* items) {
         qDebug() << "Importing media items via native iTunesLibrary framework";
 
         for (ITLibMediaItem* item in items) {
-            if (m_cancelImport.load()) {
+            if (m_pImporter->canceled()) {
                 break;
             }
 
@@ -68,7 +67,7 @@ void appendPlaylistTree(gsl::not_null<TreeItem*> item) {
     }
 
   private:
-    const std::atomic<bool>& m_cancelImport;
+    ITunesMacOSImporter* m_pImporter;
 
     QHash<unsigned long long, int> m_dbIdByPersistentId;
     ITunesDAO& m_dao;
@@ -152,7 +151,7 @@ void importPlaylist(ITLibPlaylist* itPlaylist) {
 
         int i = 0;
         for (ITLibMediaItem* item in itPlaylist.items) {
-            if (m_cancelImport.load()) {
+            if (m_pImporter->canceled()) {
                 return;
             }
 
@@ -198,12 +197,9 @@ void importMediaItem(ITLibMediaItem* item) {
 
 } // anonymous namespace
 
-ITunesMacOSImporter::ITunesMacOSImporter(LibraryFeature* parentFeature,
-        const std::atomic<bool>& cancelImport,
-        std::unique_ptr<ITunesDAO> dao)
-        : m_parentFeature(parentFeature),
-          m_cancelImport(cancelImport),
-          m_dao(std::move(dao)) {
+ITunesMacOSImporter::ITunesMacOSImporter(
+        ITunesFeature* pParentFeature, std::unique_ptr<ITunesDAO> dao)
+        : ITunesImporter(pParentFeature), m_dao(std::move(dao)) {
 }
 
 ITunesImport ITunesMacOSImporter::importLibrary() {
@@ -215,7 +211,7 @@ void importMediaItem(ITLibMediaItem* item) {
 
     if (library) {
         std::unique_ptr<TreeItem> rootItem = TreeItem::newRoot(m_parentFeature);
-        ImporterImpl impl(m_cancelImport, *m_dao);
+        ImporterImpl impl(this, *m_dao);
 
         impl.importPlaylists(library.allPlaylists);
         impl.importMediaItems(library.allMediaItems);

src/library/itunes/itunesxmlimporter.cpp

@@ -48,7 +48,7 @@ ITunesXMLImporter::ITunesXMLImporter(
         ITunesFeature* pParentFeature,
         const QString& xmlFilePath,
         std::unique_ptr<ITunesDAO> dao)
-        : m_pParentFeature(pParentFeature),
+        : ITunesImporter(pParentFeature),
           m_xmlFilePath(xmlFilePath),
           m_xmlFile(xmlFilePath),
           m_xml(&m_xmlFile),
@@ -541,11 +541,3 @@ void ITunesXMLImporter::parsePlaylist() {
         m_dao->importPlaylistRelation(parentId, playlist.id);
     }
 }
-
-bool ITunesXMLImporter::canceled() {
-    // The parent feature may be null during testing
-    if (m_pParentFeature) {
-        return m_pParentFeature->isImportCanceled();
-    }
-    return false;
-}

src/library/itunes/itunesxmlimporter.h

@@ -23,9 +23,6 @@ class ITunesXMLImporter : public ITunesImporter {
     ITunesImport importLibrary() override;
 
   private:
-    bool canceled();
-
-    ITunesFeature* m_pParentFeature;
     const QString m_xmlFilePath;
     QFile m_xmlFile;
     QXmlStreamReader m_xml;