[KPhotoAlbum] Keywords with id=0?

[Tobias Leupold]

Hey, nice work :-) Thanks for tracking this down! Your approach seems
reasonable to me. I'll try it out asap.

Am Sonntag, 15. Dezember 2019, 17:09:02 CET schrieben Sie:
> On Sun, 15 Dec 2019 10:17:34 +0100, Tobias Leupold wrote:
> > Okay. After being able to reproduce this and reading the code for quite a
> > long time, I must admit that I have no idea how or why this happens.
> > Maybe somebody with more clue about KPA's internals has? Johannes?!
>
> OK, found it.  The problem is in XMLDB::XMLCategory::renameItem().
>
> Background: IDs are normally only assigned in
> XMLDB::XMLCategory::initIdMap(), which is called from void
> XMLDB::XMLCategoryCollection::initIdMap(), which is only called from
> XMLDB::FileWriter::save().  So IDs are only assigned when the index
> file is saved.  So if you're trying to hunt down where an ID is
> assigned when you add an item, the search will be fruitless.  However,
> the IDs do persist for the life of the session, so that any IDs that
> are assigned will remain.
>
> Next, idForMap simply returns
>
> m_idMap[name]
>
> What happens if name is not present in a qMap?  It returns a default
> constructed value, which in this case is a default int, which is 0.
> But rename() always sets the id mapping for the new name, even if one
> previously didn't exist.  So when you rename something, the new name
> always gets the value 0.
>
> There are a couple of ways of solving this.  One is what I'm doing
> here, for rename to not set the ID if it's zero, and more importantly
> refuse to set an ID of zero so that if something else tries to set it
> to 0 it will not succeed.  So strictly speaking the change to
> renameItem() isn't needed, but this will prevent an unneeded warning
> from being emitted.
>
> Perhaps we should improve checking in setIdMapping() and add
> removeIdMapping() too.
>
> In any case, if one of you could review this, I'll push it later today.
>
> diff --git a/XMLDB/XMLCategory.cpp b/XMLDB/XMLCategory.cpp
> index 712cc689..30e55cdb 100644
> --- a/XMLDB/XMLCategory.cpp
> +++ b/XMLDB/XMLCategory.cpp
> @@ -20,6 +20,7 @@
>  #include <DB/ImageDB.h>
>  #include <DB/MemberMap.h>
>  #include <Utilities/List.h>
> +#include "Logging.h"
>
>  XMLDB::XMLCategory::XMLCategory(const QString &name, const QString &icon,
> ViewType type, int thumbnailSize, bool show, bool positionable)
>      : m_name(name)
>
> @@ -130,7 +131,8 @@ void XMLDB::XMLCategory::renameItem(const QString
> &oldValue, const QString &newV m_idMap.remove(oldValue);
>
>      addItem(newValue);
> -    setIdMapping(newValue, id);
> +    if ( id != 0)
> +        setIdMapping(newValue, id);
>      emit itemRenamed(oldValue, newValue);
>  }
>
> @@ -179,8 +181,12 @@ void XMLDB::XMLCategory::initIdMap()
>
>  void XMLDB::XMLCategory::setIdMapping(const QString &name, int id)
>  {
> -    m_nameMap.insert(id, name);
> -    m_idMap.insert(name, id);
> +    if (id == 0) {
> +        qCWarning(XMLDBLog, "XMLDB::XMLCategory::setIdMapping attempting to
> set id for %s to invalid value %d", qPrintable(name), id); +    } else {
> +        m_nameMap.insert(id, name);
> +        m_idMap.insert(name, id);
> +    }
>  }
>
>  QString XMLDB::XMLCategory::nameForId(int id) const