Review Request: Improve SqlMeta thread safety

[Ian Monroe]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100098/#review206
-----------------------------------------------------------



src/core-impl/collections/sqlcollection/SqlMeta.cpp
<http://git.reviewboard.kde.org/r/100098/#comment168>

    Could you document why you added these checks? I'm guessing its just a sanity check, but just say so here.



src/core-impl/collections/sqlcollection/SqlRegistry.cpp
<http://git.reviewboard.kde.org/r/100098/#comment169>

    This should maybe be a QLatin1String instead. I say maybe because obviously sql queries that include end-user strings should *not* be a qlatin1string. So I can't say that sqlregistry using just QString is a bad idea.


- Ian


On 2010-10-25 16:55:56, Ralf Engels wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100098/
> -----------------------------------------------------------
> 
> (Updated 2010-10-25 16:55:56)
> 
> 
> Review request for Amarok.
> 
> 
> Summary
> -------
> 
> This patch improves the SqlMeta thread safety.
> It has several parts:
> - Use QReadWriteLock for SqlTrack.
>  For this I also needed to pull the read accessor functions into the .cpp
> - SqlCollection is a const pointer.
>  All metas are directly dependent on the SqlCollection and it could and should never change.
>  Using weak pointers here is not needed.
>  Note that now the access to m_collection is also thread safe as it's const
> - SqlAlbum::setCompilation did modify the old album
>  This was changed an a new album will be created.
>  The positive side effect is that the UI will now update directly if setting a compilation
> - SqlAlbum::setImage will now copy embedded images directly when set
>  If the track-uid is set (maybe from another thread) the cover image will still be found.
>  Note that the image cache is and was not connected to the track uid.
> 
> 
> This addresses bug 254631.
>     https://bugs.kde.org/show_bug.cgi?id=254631
> 
> 
> Diffs
> -----
> 
>   src/core-impl/collections/sqlcollection/SqlCollection.cpp b88d257 
>   src/core-impl/collections/sqlcollection/SqlMeta.h b92e351 
>   src/core-impl/collections/sqlcollection/SqlMeta.cpp 6b29b6e 
>   src/core-impl/collections/sqlcollection/SqlRegistry.h 66ba632 
>   src/core-impl/collections/sqlcollection/SqlRegistry.cpp e1b3571 
> 
> Diff: http://git.reviewboard.kde.org/r/100098/diff
> 
> 
> Testing
> -------
> 
> setting and unsetting compilation
> changing album name
> scanning and fully scanning collection
> 
> 
> Thanks,
> 
> Ralf
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/amarok-devel/attachments/20101028/5e514956/attachment.htm