Bug 171213

Yes, I can confirm this.

What's the deal with this, is it broken, or simply not implemented?

Hi, I don't know if this is the right bug, but I think I have the same Problem.
Some of my music is on my internal HDD, an other part is on an external usb HDD. When I open amarok, add the two directories containing the music, everything is in the database, I close amarok, open it again, and still everything is in the database. Then I close amarok, umount the external drive, restart amarok, the music of the external drive is not there. OK, this makes sense. But when I mount this drive again, nothing happens, unless I rescan my entire collection.

My wish: when a part of the collection is on an removable drive, amarok should automatically detect whether this drive is accessible and according to this show the tracks in the collection or not.

In my case it is different: my whole collection is on an external usb drive.
No matter if the disk is mounted or not, Amarok will show the whole collection as being present.

(In reply to comment #2)
> What's the deal with this, is it broken, or simply not implemented?
> 

Not ported.  I think the vast majority of the groundwork is there, and parts have been ported, but it hasn't been completed.

Amarok 2.0.0 still loses the collection when started with a hdd unmounted.
In addition I get a crash (currently with no backtrace) when mounting the hdd that contains the collection while Amarok 2 is running.

*** Bug 172321 has been marked as a duplicate of this bug. ***

*** Bug 179306 has been marked as a duplicate of this bug. ***

*** Bug 180385 has been marked as a duplicate of this bug. ***

This is mostly fixed in trunk.  NFS/smb mounts are still not dynamic enabled, and unavailable music is shown in the collection browser, but it should be polished by the time 2.1 is released.

*** Bug 186274 has been marked as a duplicate of this bug. ***

*** Bug 175736 has been marked as a duplicate of this bug. ***

Any news on this? Not fixed in trunk so far, 2.2-SVN r997463

*** Bug 200750 has been marked as a duplicate of this bug. ***

*** Bug 194918 has been marked as a duplicate of this bug. ***

Is this in with UMS?
http://awainzin-foss.blogspot.com/2009/08/amarok-2-universal-mass-storage-device.html

Regarding comment #10:
SqlQueryMaker does not restrict queries to devices that are currently mounted.

Amarok 2.1.80 here.

I'm facing this problem with NFS shares too and it is really annoying, as a complete collection rescan takes here ~40 minutes.

So starting Amarok once without having mounted the NFS share before, makes me need to wait for 40 minutes until I finally can use Amarok again.

The scenario looks like this:
- NFS-share, containing ~8000 tracks, hosted on a fileserver
- NFS-share is mounted on 2 clients using autofs over a WLAN connection

The problem:
One of the clients is a laptop, so autofs is started manually because it just causes hanging connections/applications when autofs is trying to reach the NFS mountpoins when being on-the-road.
When I return home/reboot my laptop and start Amarok it shows 0 items in the collection.
Now I quit Amarok, mount the NFS share, and start Amarok again.
It does an automagic collection rescan now.
This scan takes ~40 minutes.
After the scan, all files + ratings etc. are available again.

This is IMHO a real showstopper as I don't like to have to wait for 40 minutes to use Amarok, just because I forgot mounting the NFS share.

Amarok should handle this in a more intelligent way.

Not going to happen for 2.2 I guess.

I loved Amarok 1, but I really can't use a music player that rescans my collection on a mounted logical partition every time I fire it up.  That makes it impossible for me to play music.  I'm on Ubuntu 9.04 and Sidux (shared home directory)

I'll guess I'll hop over to Rythymbox or something for a little while.  

That sucks because Amarok really could be the best.

jmjohn,

As far as I know the support is mostly there; it's just not fully finished. But the framework for using it is already in-place, and the components of A2 are all using it...it's just the backend code that's broken.

Essentially what I'm saying is that if you want to see it back, the best thing you could do would be to help out showing it some love. Us devs are all just too overloaded at the moment with other things.

I do plan on looking at it for 2.3 if no one has before then; but we have a bunch of 2.2 releases first. I'll look at it earlier if I can but no promises. So if someone wants to pick up the mantle and work on it...

So, as Jeff wrote, support for this feature is almost there. Anybody here motivated to finish this feature?

Thanks to Git, it's now possible to develop features safely in another clone of Amarok, and then send us a Merge Request when everything is finished.

Many users have already done this. See here:

http://gitorious.org/amarok/amarok/merge_requests

*** Bug 218000 has been marked as a duplicate of this bug. ***

*** Bug 220609 has been marked as a duplicate of this bug. ***

*** Bug 220525 has been marked as a duplicate of this bug. ***

*** Bug 221858 has been marked as a duplicate of this bug. ***

commit 51a6a9a25fd288814ddac36db554636f52f3eb73
Author: Maximilian Kossick <maximilian.kossick@googlemail.com>
Date:   Sat Jan 9 20:33:52 2010 +0100

    only query tracks on mounted devices in SqlQueryMaker.
    This is the missing bit of Dynamic Collection support
    BUG: 171213

diff --git a/ChangeLog b/ChangeLog
index 79a597b..e7d84c0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -5,6 +5,7 @@ Amarok ChangeLog
 
 VERSION 2.2.3
   FEATURES:
+    * Dynamic Collection is back. (BR 171213)
     * Searching covers from Last.fm using custom query in interactive mode.
  
   CHANGES:
diff --git a/src/collection/sqlcollection/SqlQueryMaker.cpp b/src/collection/sqlcollection/SqlQueryMaker.cpp
index dd56af7..97a2c96 100644
--- a/src/collection/sqlcollection/SqlQueryMaker.cpp
+++ b/src/collection/sqlcollection/SqlQueryMaker.cpp
@@ -735,6 +735,8 @@ SqlQueryMaker::linkTables()
 void
 SqlQueryMaker::buildQuery()
 {
+    //URLS is always required for dynamic collection
+    d->linkedTables |= Private::URLS_TAB;
     linkTables();
     QString query = "SELECT ";
     if ( d->withoutDuplicates )
@@ -743,6 +745,22 @@ SqlQueryMaker::buildQuery()
     query += " FROM ";
     query += d->queryFrom;
     query += " WHERE 1 ";
+
+    //dynamic collection
+    Q_ASSERT( m_collection->mountPointManager() );
+    IdList list = m_collection->mountPointManager()->getMountedDeviceIds();
+    QString commaSeparatedIds;
+    foreach( int id, list )
+    {
+        if( !commaSeparatedIds.isEmpty() )
+            commaSeparatedIds += ',';
+        commaSeparatedIds += QString::number( id );
+    }
+    if( !commaSeparatedIds.isEmpty() )
+    {
+        query += QString( " AND urls.deviceid in (%1)" ).arg( commaSeparatedIds );
+    }
+
     switch( d->albumMode )
     {
         case OnlyNormalAlbums:
diff --git a/tests/collection/sqlcollection/TestSqlQueryMaker.cpp b/tests/collection/sqlcollection/TestSqlQueryMaker.cpp
index c9273f3..4dda826 100644
--- a/tests/collection/sqlcollection/TestSqlQueryMaker.cpp
+++ b/tests/collection/sqlcollection/TestSqlQueryMaker.cpp
@@ -87,8 +87,15 @@ TestSqlQueryMaker::initTestCase()
     m_storage = new MySqlEmbeddedStorage( m_tmpDir->name() );
     m_collection = new SqlCollection( "testId", "testcollection" );
     m_collection->setSqlStorage( m_storage );
-    SqlMountPointManagerMock *mpm = new SqlMountPointManagerMock();
-    m_collection->setMountPointManager( mpm );
+
+    QMap<int,QString> mountPoints;
+    mountPoints.insert( 1, "/foo" );
+    mountPoints.insert( 2, "/bar" );
+
+    m_mpm = new SqlMountPointManagerMock();
+    m_mpm->mountPoints = mountPoints;
+
+    m_collection->setMountPointManager( m_mpm );
     SqlRegistry *registry = new SqlRegistry( m_collection );
     registry->setStorage( m_storage );
     m_collection->setRegistry( registry );
@@ -123,9 +130,9 @@ TestSqlQueryMaker::initTestCase()
     m_storage->query( "INSERT INTO urls(id,deviceid,rpath,uniqueid) VALUES (1, -1, './IDoNotExist.mp3','1');" );
     m_storage->query( "INSERT INTO urls(id,deviceid,rpath,uniqueid) VALUES (2, -1, './IDoNotExistAsWell.mp3','2');" );
     m_storage->query( "INSERT INTO urls(id,deviceid,rpath,uniqueid) VALUES (3, -1, './MeNeither.mp3','3');" );
-    m_storage->query( "INSERT INTO urls(id,deviceid,rpath,uniqueid) VALUES (4, -1, './NothingHere.mp3','4');" );
-    m_storage->query( "INSERT INTO urls(id,deviceid,rpath,uniqueid) VALUES (5, -1, './GuessWhat.mp3','5');" );
-    m_storage->query( "INSERT INTO urls(id,deviceid,rpath,uniqueid) VALUES (6, -1, './LookItsA.flac','6');" );
+    m_storage->query( "INSERT INTO urls(id,deviceid,rpath,uniqueid) VALUES (4, 2, './NothingHere.mp3','4');" );
+    m_storage->query( "INSERT INTO urls(id,deviceid,rpath,uniqueid) VALUES (5, 1, './GuessWhat.mp3','5');" );
+    m_storage->query( "INSERT INTO urls(id,deviceid,rpath,uniqueid) VALUES (6, 2, './LookItsA.flac','6');" );
 
     m_storage->query( "INSERT INTO tracks(id,url,title,comment,artist,album,genre,year,composer) "
                       "VALUES(1,1,'track1','comment1',1,1,1,1,1);" );
@@ -153,6 +160,12 @@ TestSqlQueryMaker::cleanupTestCase()
 }
 
 void
+TestSqlQueryMaker::cleanup()
+{
+    m_collection->setMountPointManager( m_mpm );
+}
+
+void
 TestSqlQueryMaker::testQueryAlbums()
 {
     SqlQueryMaker qm( m_collection );
@@ -747,6 +760,63 @@ TestSqlQueryMaker::testMatch()
     QCOMPARE( qm.data( "testId" ).count(), count );
 }
 
+void
+TestSqlQueryMaker::testDynamicCollection()
+{
+    //this will not crash as we reset the correct mock in cleanup()
+    SqlMountPointManagerMock mpm;
+
+    QMap<int, QString> mountPoints;
+
+    mpm.mountPoints = mountPoints;
+
+    m_collection->setMountPointManager( &mpm );
+
+    SqlQueryMaker trackQm( m_collection );
+    trackQm.setQueryType( QueryMaker::Track );
+    trackQm.setBlocking( true );
+    trackQm.run();
+    QCOMPARE( trackQm.tracks( "testId" ).count(), 3 );
+
+    mpm.mountPoints.insert( 1, "/foo" );
+
+    trackQm.reset();
+    trackQm.setQueryType( QueryMaker::Track );
+    trackQm.setBlocking( true );
+    trackQm.run();
+    QCOMPARE( trackQm.tracks( "testId" ).count(), 4 );
+
+    SqlQueryMaker artistQm( m_collection );
+    artistQm.setQueryType( QueryMaker::Artist );
+    artistQm.setBlocking( true );
+    artistQm.run();
+    QCOMPARE( artistQm.artists( "testId" ).count(), 2 );
+
+    SqlQueryMaker albumQm( m_collection );
+    albumQm.setQueryType( QueryMaker::Album );
+    albumQm.setBlocking( true );
+    albumQm.run();
+    QCOMPARE( albumQm.albums( "testId" ).count(), 4 );
+
+    SqlQueryMaker genreQm( m_collection );
+    genreQm.setQueryType( QueryMaker::Genre );
+    genreQm.setBlocking( true );
+    genreQm.run();
+    QCOMPARE( genreQm.genres( "testId" ).count(), 2 );
+
+    SqlQueryMaker composerQm( m_collection );
+    composerQm.setQueryType( QueryMaker::Composer );
+    composerQm.setBlocking( true );
+    composerQm.run();
+    QCOMPARE( composerQm.composers( "testId" ).count(), 2 );
+
+    SqlQueryMaker yearQm( m_collection );
+    yearQm.setQueryType( QueryMaker::Year );
+    yearQm.setBlocking( true );
+    yearQm.run();
+    QCOMPARE( yearQm.years( "testId" ).count(), 2 );
+
+}
 
 
 #include "TestSqlQueryMaker.moc"
diff --git a/tests/collection/sqlcollection/TestSqlQueryMaker.h b/tests/collection/sqlcollection/TestSqlQueryMaker.h
index 04e2699..264031e 100644
--- a/tests/collection/sqlcollection/TestSqlQueryMaker.h
+++ b/tests/collection/sqlcollection/TestSqlQueryMaker.h
@@ -23,6 +23,7 @@
 
 class SqlStorage;
 class SqlCollection;
+class SqlMountPointManagerMock;
 
 class TestSqlQueryMaker : public QObject
 {
@@ -34,6 +35,8 @@ private slots:
     void initTestCase();
     void cleanupTestCase();
 
+    void cleanup();
+
     void testQueryTracks();
     void testQueryAlbums();
     void testQueryGenres();
@@ -59,8 +62,11 @@ private slots:
     void testMatch();
     void testMatch_data();
 
+    void testDynamicCollection();
+
 private:
     SqlCollection *m_collection;
+    SqlMountPointManagerMock *m_mpm;
     SqlStorage *m_storage;
     KTempDir *m_tmpDir;
 };

commit 51a6a9a25fd288814ddac36db554636f52f3eb73
Author: Maximilian Kossick <maximilian.kossick@googlemail.com>
Date:   Sat Jan 9 20:33:52 2010 +0100

    only query tracks on mounted devices in SqlQueryMaker.
    This is the missing bit of Dynamic Collection support
    BUG: 171213

diff --git a/ChangeLog b/ChangeLog
index 79a597b..e7d84c0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -5,6 +5,7 @@ Amarok ChangeLog
 
 VERSION 2.2.3
   FEATURES:
+    * Dynamic Collection is back. (BR 171213)
     * Searching covers from Last.fm using custom query in interactive mode.
  
   CHANGES:
diff --git a/src/collection/sqlcollection/SqlQueryMaker.cpp b/src/collection/sqlcollection/SqlQueryMaker.cpp
index dd56af7..97a2c96 100644
--- a/src/collection/sqlcollection/SqlQueryMaker.cpp
+++ b/src/collection/sqlcollection/SqlQueryMaker.cpp
@@ -735,6 +735,8 @@ SqlQueryMaker::linkTables()
 void
 SqlQueryMaker::buildQuery()
 {
+    //URLS is always required for dynamic collection
+    d->linkedTables |= Private::URLS_TAB;
     linkTables();
     QString query = "SELECT ";
     if ( d->withoutDuplicates )
@@ -743,6 +745,22 @@ SqlQueryMaker::buildQuery()
     query += " FROM ";
     query += d->queryFrom;
     query += " WHERE 1 ";
+
+    //dynamic collection
+    Q_ASSERT( m_collection->mountPointManager() );
+    IdList list = m_collection->mountPointManager()->getMountedDeviceIds();
+    QString commaSeparatedIds;
+    foreach( int id, list )
+    {
+        if( !commaSeparatedIds.isEmpty() )
+            commaSeparatedIds += ',';
+        commaSeparatedIds += QString::number( id );
+    }
+    if( !commaSeparatedIds.isEmpty() )
+    {
+        query += QString( " AND urls.deviceid in (%1)" ).arg( commaSeparatedIds );
+    }
+
     switch( d->albumMode )
     {
         case OnlyNormalAlbums:
diff --git a/tests/collection/sqlcollection/TestSqlQueryMaker.cpp b/tests/collection/sqlcollection/TestSqlQueryMaker.cpp
index c9273f3..4dda826 100644
--- a/tests/collection/sqlcollection/TestSqlQueryMaker.cpp
+++ b/tests/collection/sqlcollection/TestSqlQueryMaker.cpp
@@ -87,8 +87,15 @@ TestSqlQueryMaker::initTestCase()
     m_storage = new MySqlEmbeddedStorage( m_tmpDir->name() );
     m_collection = new SqlCollection( "testId", "testcollection" );
     m_collection->setSqlStorage( m_storage );
-    SqlMountPointManagerMock *mpm = new SqlMountPointManagerMock();
-    m_collection->setMountPointManager( mpm );
+
+    QMap<int,QString> mountPoints;
+    mountPoints.insert( 1, "/foo" );
+    mountPoints.insert( 2, "/bar" );
+
+    m_mpm = new SqlMountPointManagerMock();
+    m_mpm->mountPoints = mountPoints;
+
+    m_collection->setMountPointManager( m_mpm );
     SqlRegistry *registry = new SqlRegistry( m_collection );
     registry->setStorage( m_storage );
     m_collection->setRegistry( registry );
@@ -123,9 +130,9 @@ TestSqlQueryMaker::initTestCase()
     m_storage->query( "INSERT INTO urls(id,deviceid,rpath,uniqueid) VALUES (1, -1, './IDoNotExist.mp3','1');" );
     m_storage->query( "INSERT INTO urls(id,deviceid,rpath,uniqueid) VALUES (2, -1, './IDoNotExistAsWell.mp3','2');" );
     m_storage->query( "INSERT INTO urls(id,deviceid,rpath,uniqueid) VALUES (3, -1, './MeNeither.mp3','3');" );
-    m_storage->query( "INSERT INTO urls(id,deviceid,rpath,uniqueid) VALUES (4, -1, './NothingHere.mp3','4');" );
-    m_storage->query( "INSERT INTO urls(id,deviceid,rpath,uniqueid) VALUES (5, -1, './GuessWhat.mp3','5');" );
-    m_storage->query( "INSERT INTO urls(id,deviceid,rpath,uniqueid) VALUES (6, -1, './LookItsA.flac','6');" );
+    m_storage->query( "INSERT INTO urls(id,deviceid,rpath,uniqueid) VALUES (4, 2, './NothingHere.mp3','4');" );
+    m_storage->query( "INSERT INTO urls(id,deviceid,rpath,uniqueid) VALUES (5, 1, './GuessWhat.mp3','5');" );
+    m_storage->query( "INSERT INTO urls(id,deviceid,rpath,uniqueid) VALUES (6, 2, './LookItsA.flac','6');" );
 
     m_storage->query( "INSERT INTO tracks(id,url,title,comment,artist,album,genre,year,composer) "
                       "VALUES(1,1,'track1','comment1',1,1,1,1,1);" );
@@ -153,6 +160,12 @@ TestSqlQueryMaker::cleanupTestCase()
 }
 
 void
+TestSqlQueryMaker::cleanup()
+{
+    m_collection->setMountPointManager( m_mpm );
+}
+
+void
 TestSqlQueryMaker::testQueryAlbums()
 {
     SqlQueryMaker qm( m_collection );
@@ -747,6 +760,63 @@ TestSqlQueryMaker::testMatch()
     QCOMPARE( qm.data( "testId" ).count(), count );
 }
 
+void
+TestSqlQueryMaker::testDynamicCollection()
+{
+    //this will not crash as we reset the correct mock in cleanup()
+    SqlMountPointManagerMock mpm;
+
+    QMap<int, QString> mountPoints;
+
+    mpm.mountPoints = mountPoints;
+
+    m_collection->setMountPointManager( &mpm );
+
+    SqlQueryMaker trackQm( m_collection );
+    trackQm.setQueryType( QueryMaker::Track );
+    trackQm.setBlocking( true );
+    trackQm.run();
+    QCOMPARE( trackQm.tracks( "testId" ).count(), 3 );
+
+    mpm.mountPoints.insert( 1, "/foo" );
+
+    trackQm.reset();
+    trackQm.setQueryType( QueryMaker::Track );
+    trackQm.setBlocking( true );
+    trackQm.run();
+    QCOMPARE( trackQm.tracks( "testId" ).count(), 4 );
+
+    SqlQueryMaker artistQm( m_collection );
+    artistQm.setQueryType( QueryMaker::Artist );
+    artistQm.setBlocking( true );
+    artistQm.run();
+    QCOMPARE( artistQm.artists( "testId" ).count(), 2 );
+
+    SqlQueryMaker albumQm( m_collection );
+    albumQm.setQueryType( QueryMaker::Album );
+    albumQm.setBlocking( true );
+    albumQm.run();
+    QCOMPARE( albumQm.albums( "testId" ).count(), 4 );
+
+    SqlQueryMaker genreQm( m_collection );
+    genreQm.setQueryType( QueryMaker::Genre );
+    genreQm.setBlocking( true );
+    genreQm.run();
+    QCOMPARE( genreQm.genres( "testId" ).count(), 2 );
+
+    SqlQueryMaker composerQm( m_collection );
+    composerQm.setQueryType( QueryMaker::Composer );
+    composerQm.setBlocking( true );
+    composerQm.run();
+    QCOMPARE( composerQm.composers( "testId" ).count(), 2 );
+
+    SqlQueryMaker yearQm( m_collection );
+    yearQm.setQueryType( QueryMaker::Year );
+    yearQm.setBlocking( true );
+    yearQm.run();
+    QCOMPARE( yearQm.years( "testId" ).count(), 2 );
+
+}
 
 
 #include "TestSqlQueryMaker.moc"
diff --git a/tests/collection/sqlcollection/TestSqlQueryMaker.h b/tests/collection/sqlcollection/TestSqlQueryMaker.h
index 04e2699..264031e 100644
--- a/tests/collection/sqlcollection/TestSqlQueryMaker.h
+++ b/tests/collection/sqlcollection/TestSqlQueryMaker.h
@@ -23,6 +23,7 @@
 
 class SqlStorage;
 class SqlCollection;
+class SqlMountPointManagerMock;
 
 class TestSqlQueryMaker : public QObject
 {
@@ -34,6 +35,8 @@ private slots:
     void initTestCase();
     void cleanupTestCase();
 
+    void cleanup();
+
     void testQueryTracks();
     void testQueryAlbums();
     void testQueryGenres();
@@ -59,8 +62,11 @@ private slots:
     void testMatch();
     void testMatch_data();
 
+    void testDynamicCollection();
+
 private:
     SqlCollection *m_collection;
+    SqlMountPointManagerMock *m_mpm;
     SqlStorage *m_storage;
     KTempDir *m_tmpDir;
 };

I am sorry, but this is not fixed. 
I just compiled after this fix and now my external HD is mounted on Amarok start, but my collection shows 0 tracks and all collection locations are gone from the settings.

This is a major issue for people using external HDs as you are required to Rescan your external media completely. Definitely not fixed in Git as of today.

It used to work at some point just before we released 2.2.2, now this is broken again on current git, so call this a Regression :(

*** Bug 227018 has been marked as a duplicate of this bug. ***

Maximilian, you might not have seen that this bug was reopened...

amarok doesn't react well to surprise removal of external collections. in fact when i reconnect i have to rescan said collection - not ideal especially in situations where the external collection is huge. Amarok needs to determine if the collection is on a removable device and was subject to surprise removal OR if the user removed the files from the collection. PRIOR to wiping it's info from the DB. 

in fact if amarok cant figure out what just happened, it should ask the user about it if more than $percentage-of-collection disappears in $defined-period-of-time BEFORE it does anything....

https://bugs.kde.org/show_bug.cgi?id=220525 is about that issue. IMHO amarok just takes the wrong approach i.e. tries to be smart instead of relying on the user. Digikam has a very well working collection management, including local and remote collections but instead of taking a working solution amarok tries to be smarter and thus stays broken.

IMO 220525 is not a duplicate of this bug because it does not ask to fix the current collection management but burry it and introduce digikam's. Don't try to fix what's broken by design.

*** Bug 232194 has been marked as a duplicate of this bug. ***

Looks like the port of MountPointManager is buggy, adding Dan.

Dynamic Collection code in collection and querymaker works fine.

*** Bug 227739 has been marked as a duplicate of this bug. ***

*** Bug 233431 has been marked as a duplicate of this bug. ***

commit 57e87dc0139577120a19c0f1c39f5b7fb02b002e
Author: Jeff Mitchell <mitchell@kde.org>
Date:   Sun Aug 8 18:50:39 2010 -0400

    Fix Dynamic Collections.
    
    There were three problems I could find:
    
    1) The more minor was that devices that were already mounted when Amarok
    was first started (or the database freshly wiped) wouldn't be found
    because the database would be created after the mount point detection
    code, so the device entries couldn't be inserted.
    
    2) When a device was added, the Solid predicate that was being searched
    could not always match. Dunno why...bug in Solid, or us, but regardless,
    instead find all storage volumes and search the returned devices
    manually.
    
    3) The MountPointManager code wasn't properly watching for mount changes.
    MediaDeviceCache was, though, so now MPM gets its signals from MDC
    instead of from Solid directly.
    
    This fixes all of the problems with Dynamic Collection that I could
    find. I'd appreciate if people could test this out and see if they can
    still reproduce problems. N.B.: you *must* rescan your files after you
    compile this code, because that's the only way to get the files associated
    with the proper device IDs (unless you know how to tweak your DB
    manually). So if you don't rescan your files you'll probably still have
    problems until you do.
    
    CCMAIL: amarok-devel@kde.org
    CCBUG: 171213

diff --git a/ChangeLog b/ChangeLog
index 7a856ec..714a7c4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -42,6 +42,9 @@ VERSION 2.3.2-Beta 1
       Patch by Richard Longland <rlongland@hotmail.com>.
 
   BUGFIXES:
+    * Fix regression in Dynamic Collections. For files that were scanned when
+      Dynamic Collections wasn't working, you will need to rescan them to get
+      them associated with the proper device.
     * Fix crash on exit with newer KDE versions. Patch by Martin Blumenstingl and Felix
       Geyer. (BR 245513)
     * Fixed playlist bottom toolbar getting to tall when using "Text only"
diff --git a/src/MediaDeviceCache.h b/src/MediaDeviceCache.h
index c34ea7c..11f0c01 100644
--- a/src/MediaDeviceCache.h
+++ b/src/MediaDeviceCache.h
@@ -55,7 +55,7 @@ class AMAROK_EXPORT MediaDeviceCache : public QObject
     signals:
         void deviceAdded( const QString &udi );
         void deviceRemoved( const QString &udi );
-	void accessibilityChanged( bool accessible, const QString &udi );
+        void accessibilityChanged( bool accessible, const QString &udi );
 
     public slots:
         void slotAddSolidDevice( const QString &udi );
diff --git a/src/core-impl/collections/sqlcollection/MountPointManager.cpp b/src/core-impl/collections/sqlcollection/MountPointManager.cpp
index 013376b..a28e976 100644
--- a/src/core-impl/collections/sqlcollection/MountPointManager.cpp
+++ b/src/core-impl/collections/sqlcollection/MountPointManager.cpp
@@ -18,6 +18,8 @@
 
 #include "MountPointManager.h"
 
+#include "MediaDeviceCache.h"
+
 #include "core/support/Amarok.h"
 #include "core/support/Debug.h"
 #include "statusbar/StatusBar.h"
@@ -52,8 +54,8 @@ MountPointManager::MountPointManager( QObject *parent, SqlStorage *storage )
         return;
     }
 
-    connect( Solid::DeviceNotifier::instance(), SIGNAL( deviceAdded( QString ) ), SLOT( deviceAdded( QString ) ) );
-    connect( Solid::DeviceNotifier::instance(), SIGNAL( deviceRemoved( QString ) ), SLOT( deviceRemoved( QString ) ) );
+    connect( MediaDeviceCache::instance(), SIGNAL( deviceAdded( QString ) ), SLOT( deviceAdded( QString ) ) );
+    connect( MediaDeviceCache::instance(), SIGNAL( deviceRemoved( QString ) ), SLOT( deviceRemoved( QString ) ) );
 
     init();
 
@@ -445,14 +447,22 @@ void
 MountPointManager::deviceAdded( const QString &udi )
 {
     DEBUG_BLOCK
-    Solid::Predicate predicate = Solid::Predicate( Solid::DeviceInterface::StorageVolume, "udi", udi );
+    Solid::Predicate predicate = Solid::Predicate( Solid::DeviceInterface::StorageVolume );
     QList<Solid::Device> devices = Solid::Device::listFromQuery( predicate );
-    //there'll be maximum one device because we are using the udi in the predicate
-    if( !devices.isEmpty() )
+    //Looking for a specific udi in predicate seems flaky/buggy; the foreach loop barely
+    //takes any time, so just be safe
+    bool found = false;
+    debug() << "looking for udi " << udi;
+    foreach( Solid::Device device, devices )
     {
-        Solid::Device device = devices[0];
-        createHandlerFromDevice( device, udi );
+        if( device.udi() == udi )
+        {
+            createHandlerFromDevice( device, udi );
+            found = true;
+        }
     }
+    if( !found )
+        debug() << "Did not find device from Solid for udi " << udi;
 }
 
 void
@@ -479,6 +489,7 @@ MountPointManager::deviceRemoved( const QString &udi )
 
 void MountPointManager::createHandlerFromDevice( const Solid::Device& device, const QString &udi )
 {
+    DEBUG_BLOCK
     if ( device.isValid() )
     {
         debug() << "Device added and mounted, checking handlers";
@@ -507,8 +518,12 @@ void MountPointManager::createHandlerFromDevice( const Solid::Device& device, co
                 emit deviceAdded( key );
                 break;  //we found the added medium and don't have to check the other device handlers
             }
+            else
+                debug() << "Factory can't handle device " << udi;
         }
     }
+    else
+        debug() << "Device not valid!";
 }
 
 
diff --git a/src/core-impl/collections/sqlcollection/SqlCollection.cpp b/src/core-impl/collections/sqlcollection/SqlCollection.cpp
index 0176fd3..f7a9ad8 100644
--- a/src/core-impl/collections/sqlcollection/SqlCollection.cpp
+++ b/src/core-impl/collections/sqlcollection/SqlCollection.cpp
@@ -73,17 +73,26 @@ SqlCollection::~SqlCollection()
 }
 
 void
-SqlCollection::init()
+SqlCollection::setUpdater( DatabaseUpdater *updater )
 {
-    QTimer::singleShot( 0, this, SLOT( initXesam() ) );
-    if( !m_updater )
+    DEBUG_BLOCK
+    if( !updater )
     {
         debug() << "Could not load updater!";
         return;
     }
-
+    m_updater = updater;
     if( m_updater->needsUpdate() )
+    {
+        debug() << "Needs update!";
         m_updater->update();
+    }
+}
+
+void
+SqlCollection::init()
+{
+    QTimer::singleShot( 0, this, SLOT( initXesam() ) );
 
     QStringList result = m_sqlStorage->query( "SELECT count(*) FROM tracks" );
     // If database version is updated, the collection needs to be rescanned.
diff --git a/src/core-impl/collections/sqlcollection/SqlCollection.h b/src/core-impl/collections/sqlcollection/SqlCollection.h
index da7e340..89fcdc0 100644
--- a/src/core-impl/collections/sqlcollection/SqlCollection.h
+++ b/src/core-impl/collections/sqlcollection/SqlCollection.h
@@ -105,7 +105,7 @@ class AMAROK_SQLCOLLECTION_EXPORT SqlCollection : public Collections::Collection
 
         void setSqlStorage( SqlStorage *storage ) { m_sqlStorage = storage; }
         void setRegistry( SqlRegistry *registry ) { m_registry = registry; }
-        void setUpdater( DatabaseUpdater *updater ) { m_updater = updater; }
+        void setUpdater( DatabaseUpdater *updater );
         void setCapabilityDelegate( Capabilities::CollectionCapabilityDelegate *delegate ) { m_capabilityDelegate = delegate; }
         void setCollectionLocationFactory( SqlCollectionLocationFactory *factory ) { m_collectionLocationFactory = factory; }
         void setQueryMakerFactory( SqlQueryMakerFactory *factory ) { m_queryMakerFactory = factory; }
diff --git a/src/core-impl/collections/sqlcollection/SqlCollectionFactory.cpp b/src/core-impl/collections/sqlcollection/SqlCollectionFactory.cpp
index c852111..a1966ac 100644
--- a/src/core-impl/collections/sqlcollection/SqlCollectionFactory.cpp
+++ b/src/core-impl/collections/sqlcollection/SqlCollectionFactory.cpp
@@ -160,13 +160,17 @@ SqlCollectionFactory::createSqlCollection( const QString &id, const QString &pre
     SqlCollection *coll = new SqlCollection( id, prettyName );
     coll->setCapabilityDelegate( new Capabilities::CollectionCapabilityDelegateImpl() );
 
-    MountPointManager *mpm = new MountPointManager( coll, storage );
-
-    coll->setMountPointManager( new SqlMountPointManagerImpl( mpm ) );
     DatabaseUpdater *updater = new DatabaseUpdater();
     updater->setStorage( storage );
     updater->setCollection( coll );
+
+    //setUpdater runs the update function; this must be run *before* MountPointManager is initialized or its handlers may try to insert
+    //into the database before it's created/updated!
     coll->setUpdater( updater );
+
+    MountPointManager *mpm = new MountPointManager( coll, storage );
+
+    coll->setMountPointManager( new SqlMountPointManagerImpl( mpm ) );
     ScanManager *scanMgr = new ScanManager( coll );
     scanMgr->setCollection( coll );
     scanMgr->setStorage( storage );
diff --git a/src/core-impl/collections/sqlcollection/device/massstorage/MassStorageDeviceHandler.cpp b/src/core-impl/collections/sqlcollection/device/massstorage/MassStorageDeviceHandler.cpp
index f0cc20a..eb4286f 100644
--- a/src/core-impl/collections/sqlcollection/device/massstorage/MassStorageDeviceHandler.cpp
+++ b/src/core-impl/collections/sqlcollection/device/massstorage/MassStorageDeviceHandler.cpp
@@ -39,6 +39,7 @@ MassStorageDeviceHandler::MassStorageDeviceHandler( int deviceId, const QString
     , m_mountPoint( mountPoint )
     , m_udi( udi )
 {
+    DEBUG_BLOCK
 }
 
 MassStorageDeviceHandler::~MassStorageDeviceHandler()
@@ -104,7 +105,19 @@ bool MassStorageDeviceHandlerFactory::canCreateFromConfig( ) const
 
 bool MassStorageDeviceHandlerFactory::canHandle( const Solid::Device &device ) const
 {
+    DEBUG_BLOCK
     const Solid::StorageVolume *volume = device.as<Solid::StorageVolume>();
+    if( !volume )
+    {
+        debug() << "found no volume";
+        return false;
+    }
+    if( volume->uuid().isEmpty() )
+        debug() << "has empty uuid";
+    if( volume->isIgnored() )
+        debug() << "volume is ignored";
+    if( excludedFilesystem( volume->fsType() ) )
+        debug() << "excluded filesystem of type " << volume->fsType();
     return volume && !volume->uuid().isEmpty()
            && !volume->isIgnored() && !excludedFilesystem( volume->fsType() );
 }
@@ -126,7 +139,10 @@ DeviceHandler * MassStorageDeviceHandlerFactory::createHandler( const Solid::Dev
 {
     DEBUG_BLOCK
     if( !s )
+    {
+        debug() << "!s, returning 0";
         return 0;
+    }
     const Solid::StorageVolume *volume = device.as<Solid::StorageVolume>();
     const Solid::StorageAccess *volumeAccess = device.as<Solid::StorageAccess>();
     if( !volume || !volumeAccess )
@@ -135,7 +151,10 @@ DeviceHandler * MassStorageDeviceHandlerFactory::createHandler( const Solid::Dev
         return 0;
     }
     if( volumeAccess->filePath().isEmpty() )
+    {
+        debug() << "not mounted, can't do anything";
         return 0; // It's not mounted, we can't do anything.
+    }
     QStringList ids = s->query( QString( "SELECT id, label, lastmountpoint "
                                          "FROM devices WHERE type = 'uuid' "
                                          "AND uuid = '%1';" ).arg( volume->uuid() ) );

I've just done some limited testing, and "knock on wood" it seems to work! I tested with a SD card with the build in sd reader on my laptop (couldn't find a usb cable to test with my external HD). After adding the sd card mount path to amarok, removing and inserting the card would make the songs disappear and reappear.

Not likely related, but I did notice once I re-inserted the card amarok didn't re-scan the contents in the "removable" collection it auto populates. It only seemed to scan it the first time I inserted the card, now it just shows up with 0 tracks.

Yeah, I noticed problems with that too. I don't know much about the mediadevice code, but that's an entirely separate issue -- Dynamic Collection pertains to the main Local Tracks collection.

I'm going to close this since I believe all known problems are fixed in the upcoming 2.3.2. Please reopen (or preferably start a new bug) at some future point if there are issues in 2.3.2.

*** Bug 251423 has been marked as a duplicate of this bug. ***

Git commit d57eedc5563b3950446257f09d439aaa3b8c21ee by Peter C. Ndikuwera.
Committed on 03/03/2011 at 16:01.
Pushed by pndiku into branch 'master'.

Fix NFS & SMB/CIFS Remote collection

BUG: 249760
CCBUG: 232976
CCBUG: 171213
CCBUG: 187692

Using Max's Mass Storage Device code as a guide, this commit returns
network-based collections, shared over NFS & SMB/CIFS to the dynamic
collection architecture.

This should re-validate http://amarok.kde.org/wiki/Dynamic_Collection,
which some people have complained to longer applies.

Test it & break it!

M  +3    -3    src/core-impl/collections/db/sql/MountPointManager.cpp     
M  +2    -2    src/core-impl/collections/db/sql/device/CMakeLists.txt     
M  +14   -6    src/core-impl/collections/db/sql/device/nfs/CMakeLists.txt     
M  +77   -30   src/core-impl/collections/db/sql/device/nfs/NfsDeviceHandler.cpp     
M  +12   -8    src/core-impl/collections/db/sql/device/nfs/NfsDeviceHandler.h     
M  +14   -6    src/core-impl/collections/db/sql/device/smb/CMakeLists.txt     
M  +76   -32   src/core-impl/collections/db/sql/device/smb/SmbDeviceHandler.cpp     
M  +11   -7    src/core-impl/collections/db/sql/device/smb/SmbDeviceHandler.h     

http://commits.kde.org/amarok/d57eedc5563b3950446257f09d439aaa3b8c21ee