Labels and uniqueid Was: some crash report by Rich

[Jeff Mitchell]

On Wednesday 08 November 2006 18:47, Alexandre Oliveira wrote:
> The crash is in:
>  for(QMap<QString, QStringList>::ConstIterator it = newLabels.begin();
> it != endLabels; ++it ) {
>         CollectionDB::instance()->setLabels( it.key(), it.data(),
> m_playlistItem->uniqueId(), CollectionDB::typeUser );
>     }
>
> m_playlistItem->uniqueId() is wrong there, as when tagdialog isn't
> called from playlist, m_playlistItem will be null. Also, it'll return
> current item, so if the user uses next or previous and change labels
> for different tracks, all of them would use the same uniqueid on that
> call.

Whoops.

> I could fix it by making newLabels use a QPair with the url and the id
> as key, or by storing the ids in other map, or even by refactoring
> CollectionDB::setLabels() so that it finds the id by itself, but all
> this brought me question:

Sounds good?  I haven't looked at the labels code much, but if you know what 
you're doing with it and are willing to do the fixing, it'd be great.

> Isn't having urls and uniqueids on labels table redundant? We already
> have both on statistics

No.  The reason is because they are both pieces of identifying information 
that are used to help track files.  Remember that in some cases a file is the 
same but the UID has changed, and in other cases the UID is the same but the 
location has changed.  So you need to be able to query for one or the other, 
in order to update the other one appropriately.  I'm not sure what having 
that info in the statistics table has to do with anything -- it's used for 
the same purpose there.  You can't just tie it to the uniqueid table either, 
as both of those tables have persistent entries that should remain even if an 
entry is no longer in the uniqueid table.

Having that information in the various tables is not necessarily a bad 
thing...you could probably come up with some slick SQL queries that work on 
either a UID or a url and pull out all information from all tables for a 
particular matching file, all in a single query.

> so when amarok finds out about url changes, 
> it could update the url on migrateFile(), like it happens for lyrics
> table.

migrateFile won't suffice; it takes an old and new url and changes things 
around.  You need to be able to have separate actions depending on whether a 
url or uniqueid has changed on a file.

If you'll notice, since lyrics are not AFT-enabled, despite having 
migrateFile, they can easily be lost as you shuffle songs around on your 
filesystem.  A better answer would be to add the uniqueid column to the 
lyrics table, then add the lyrics table to the list of tables in 
aftCheckPermanentTables and the two aftMigrate* functions (I should probably 
pull the list of tables out to a statically-defined variable to prevent 
syncing issues).  That way both the URL and UID updating would happen 
automatically.

--Jeff