commit 57e87dc0139577120a19c0f1c39f5b7fb02b002e Author: Jeff Mitchell 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 . 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 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(); + 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(); const Solid::StorageAccess *volumeAccess = device.as(); 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() ) ); _______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel