[Amarok] 57e87dc Fix Dynamic Collections.
Jeff Mitchell
mitchell at kde.org
Mon Aug 9 00:56:31 CEST 2010
commit 57e87dc0139577120a19c0f1c39f5b7fb02b002e
Author: Jeff Mitchell <mitchell at 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 at 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 at 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() ) );
More information about the Amarok-devel
mailing list