[prev in list] [next in list] [prev in thread] [next in thread]
List: amarok-devel
Subject: Re: [Amarok] 57e87dc Fix Dynamic Collections.
From: Maximilian Kossick <maximilian.kossick () googlemail ! com>
Date: 2010-08-09 14:40:56
Message-ID: AANLkTimN8jaL4yeQaamG2ztAi8a=wp4Qo=Za1qyVXraZ () mail ! gmail ! com
[Download RAW message or body]
Please check if UrlUpdateJob in MountPointManager.cpp has to be
reenabled. It might not be necessary, as statistics and labels now use
a FK on the urls tables, but please make sure that the deviceid/rpath
pair in the urls table gets updated.
On Mon, Aug 9, 2010 at 12:56 AM, Jeff Mitchell <mitchell@kde.org> wrote:
> 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() ) );
> _______________________________________________
> Amarok-devel mailing list
> Amarok-devel@kde.org
> https://mail.kde.org/mailman/listinfo/amarok-devel
>
_______________________________________________
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic