Improve null-object handling in ImageTagPair and TagProperties.

When attempt is made to create an object with in invalid id, now
a null object is created and methods detect this properly, refusing to edit
the db with invalid parameters.
(shows that the SharedNull paradigm is not always so much more elegant than d = 0)

CCBUG: 277169

libs/database/imagetagpair.cpp

@@ -41,11 +41,12 @@
 namespace Digikam
 {
 
+typedef QSharedDataPointer<ImageTagPairPriv> ImageTagPairPrivSharedPointer;
 class ImageTagPairPriv : public QSharedData
 {
 public:
 
-    static ImageTagPairPriv* createGuarded(qlonglong imageId, int tagId);
+    static ImageTagPairPrivSharedPointer createGuarded(qlonglong imageId, int tagId);
 
     ImageTagPairPriv()
     {
@@ -54,6 +55,7 @@ public:
         propertiesLoaded = false;
     }
 
+    bool isNull() const;
     void init(const ImageInfo& info, int tagId);
     void checkProperties();
 
@@ -65,26 +67,26 @@ public:
     QMultiMap<QString, QString> properties;
 };
 
-class ImageTagPairPrivSharedNull : public QSharedDataPointer<ImageTagPairPriv>
+class ImageTagPairPrivSharedNull : public ImageTagPairPrivSharedPointer
 {
 public:
     ImageTagPairPrivSharedNull() : QSharedDataPointer<ImageTagPairPriv>(new ImageTagPairPriv) {}
 };
 K_GLOBAL_STATIC(ImageTagPairPrivSharedNull, imageTagPairPrivSharedNull)
 
-ImageTagPairPriv* ImageTagPairPriv::createGuarded(qlonglong imageId, int tagId)
+ImageTagPairPrivSharedPointer ImageTagPairPriv::createGuarded(qlonglong imageId, int tagId)
 {
     if (imageId <= 0 || tagId <= 0)
     {
         kDebug() << "Attempt to create invalid tag pair image id" << imageId << "tag id" << tagId;
         return *imageTagPairPrivSharedNull;
     }
-    return new ImageTagPairPriv;
+    return ImageTagPairPrivSharedPointer(new ImageTagPairPriv);
 }
 
 void ImageTagPairPriv::init(const ImageInfo& i, int t)
 {
-    if (this == *imageTagPairPrivSharedNull)
+    if (isNull())
     {
         return;
     }
@@ -95,7 +97,7 @@ void ImageTagPairPriv::init(const ImageInfo& i, int t)
 
 void ImageTagPairPriv::checkProperties()
 {
-    if (!propertiesLoaded)
+    if (!isNull() && !propertiesLoaded)
     {
         QList<ImageTagProperty> props = DatabaseAccess().db()->getImageTagProperties(info.id(), tagId);
         foreach (const ImageTagProperty& p, props)
@@ -106,6 +108,11 @@ void ImageTagPairPriv::checkProperties()
     }
 }
 
+bool ImageTagPairPriv::isNull() const
+{
+    return this == imageTagPairPrivSharedNull->constData();
+}
+
 ImageTagPair::ImageTagPair()
     : d(*imageTagPairPrivSharedNull)
 {
@@ -182,7 +189,7 @@ bool ImageTagPair::isAssigned() const
 
 void ImageTagPair::assignTag()
 {
-    if (!d->isAssigned)
+    if (!d->isNull() && !d->isAssigned)
     {
         d->info.setTag(d->tagId);
         d->isAssigned = true;
@@ -191,7 +198,7 @@ void ImageTagPair::assignTag()
 
 void ImageTagPair::unAssignTag()
 {
-    if (d->isAssigned)
+    if (!d->isNull() && d->isAssigned)
     {
         d->info.removeTag(d->tagId);
         d->isAssigned = false;
@@ -261,7 +268,7 @@ QMap<QString, QString> ImageTagPair::properties() const
 
 void ImageTagPair::setProperty(const QString& key, const QString& value)
 {
-    if (d->info.isNull())
+    if (d->isNull() || d->info.isNull())
     {
         return;
     }
@@ -276,7 +283,7 @@ void ImageTagPair::setProperty(const QString& key, const QString& value)
 
 void ImageTagPair::addProperty(const QString& key, const QString& value)
 {
-    if (d->info.isNull())
+    if (d->isNull() || d->info.isNull())
     {
         return;
     }
@@ -292,7 +299,7 @@ void ImageTagPair::addProperty(const QString& key, const QString& value)
 
 void ImageTagPair::removeProperty(const QString& key, const QString& value)
 {
-    if (d->info.isNull())
+    if (d->isNull() || d->info.isNull())
     {
         return;
     }
@@ -308,7 +315,7 @@ void ImageTagPair::removeProperty(const QString& key, const QString& value)
 
 void ImageTagPair::removeProperties(const QString& key)
 {
-    if (d->info.isNull())
+    if (d->isNull() || d->info.isNull())
     {
         return;
     }
@@ -324,7 +331,7 @@ void ImageTagPair::removeProperties(const QString& key)
 
 void ImageTagPair::clearProperties()
 {
-    if (d->info.isNull())
+    if (d->isNull() || d->info.isNull())
     {
         return;
     }

libs/database/tagproperties.cpp

@@ -28,6 +28,10 @@
 #include <QSharedData>
 #include <QMultiMap>
 
+// KDE includes
+
+#include <kdebug.h>
+
 // Local includes
 
 #include "albumdb.h"
@@ -37,33 +41,53 @@
 namespace Digikam
 {
 
+typedef QExplicitlySharedDataPointer<TagProperties::TagPropertiesPriv> TagPropertiesPrivSharedPointer;
 class TagProperties::TagPropertiesPriv : public QSharedData
 {
 public:
 
+    static TagPropertiesPrivSharedPointer createGuarded(int tagId);
+
     TagPropertiesPriv()
     {
         tagId = -1;
     }
 
+    bool isNull() const;
+
     int                         tagId;
     QMultiMap<QString, QString> properties;
 };
 
 // ------------------------------------------------------------------------------------------------
 
-class TagPropertiesPrivSharedNull : public QSharedDataPointer<TagProperties::TagPropertiesPriv>
+class TagPropertiesPrivSharedNull : public TagPropertiesPrivSharedPointer
 {
 public:
 
     TagPropertiesPrivSharedNull() 
-        : QSharedDataPointer<TagProperties::TagPropertiesPriv>(new TagProperties::TagPropertiesPriv)
+        : TagPropertiesPrivSharedPointer(new TagProperties::TagPropertiesPriv)
     {
     }
 };
 
 K_GLOBAL_STATIC(TagPropertiesPrivSharedNull, tagPropertiesPrivSharedNull)
 
+TagPropertiesPrivSharedPointer TagProperties::TagPropertiesPriv::createGuarded(int tagId)
+{
+    if (tagId <= 0)
+    {
+        kDebug() << "Attempt to create tag properties for tag id" << tagId;
+        return *tagPropertiesPrivSharedNull;
+    }
+    return TagPropertiesPrivSharedPointer(new TagPropertiesPriv);
+}
+
+bool TagProperties::TagPropertiesPriv::isNull() const
+{
+    return this == tagPropertiesPrivSharedNull->constData();
+}
+
 // ------------------------------------------------------------------------------------------------
 
 TagProperties::TagProperties()
@@ -72,8 +96,13 @@ TagProperties::TagProperties()
 }
 
 TagProperties::TagProperties(int tagId)
-    : d(new TagPropertiesPriv)
+    : d(TagPropertiesPriv::createGuarded(tagId))
 {
+    if (d->isNull())
+    {
+        return;
+    }
+
     d->tagId                      = tagId;
     QList<TagProperty> properties = DatabaseAccess().db()->getTagProperties(tagId);
     foreach (const TagProperty& p, properties)
@@ -140,6 +169,10 @@ QMap<QString, QString> TagProperties::properties() const
 
 void TagProperties::setProperty(const QString& key, const QString& value)
 {
+    if (d->isNull())
+    {
+        return;
+    }
     if (d->properties.contains(key, value) && d->properties.count(key) == 1)
     {
         return;
@@ -153,7 +186,7 @@ void TagProperties::setProperty(const QString& key, const QString& value)
 
 void TagProperties::addProperty(const QString& key, const QString& value)
 {
-    if (d->properties.contains(key, value))
+    if (d->isNull() || d->properties.contains(key, value))
     {
         return;
     }
@@ -164,7 +197,7 @@ void TagProperties::addProperty(const QString& key, const QString& value)
 
 void TagProperties::removeProperty(const QString& key, const QString& value)
 {
-    if (d->properties.contains(key, value))
+    if (!d->isNull() && d->properties.contains(key, value))
     {
         DatabaseAccess().db()->removeTagProperties(d->tagId, key, value);
         d->properties.remove(key, value);
@@ -173,7 +206,7 @@ void TagProperties::removeProperty(const QString& key, const QString& value)
 
 void TagProperties::removeProperties(const QString& key)
 {
-    if (d->properties.contains(key))
+    if (!d->isNull() && d->properties.contains(key))
     {
         DatabaseAccess().db()->removeTagProperties(d->tagId, key);
         d->properties.remove(key);