QJsonObject: add QLatin1String overloads of non-const methods

Also optimized the existing QL1S overload of non-const operator[](), and
applied Extract Method refactoring to the other existing QL1S overloads.

[ChangeLog][QtCore][QJsonObject] Added insert(), remove(), and take()
overloads taking QLatin1String.

Change-Id: I5e737cf2d7d9ffb325d6981db1e4a6a9f093657b
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Anton Kudryavtsev <antkudr@mail.ru>

src/corelib/serialization/qjson_p.h

@@ -66,11 +66,13 @@
 
 #include <limits.h>
 #include <limits>
+#include <type_traits>
 
 QT_BEGIN_NAMESPACE
 
 // in qstring.cpp
 void qt_to_latin1_unchecked(uchar *dst, const ushort *uc, qsizetype len);
+void qt_from_latin1(ushort *dst, const char *str, size_t size) noexcept;
 
 /*
   This defines a binary data structure for Json data. The data structure is optimised for fast reading
@@ -153,16 +155,24 @@ typedef qle_uint offset;
 // round the size up to the next 4 byte boundary
 inline int alignedSize(int size) { return (size + 3) & ~3; }
 
+const int MaxLatin1Length = 0x7fff;
+
 static inline bool useCompressed(QStringView s)
 {
-    if (s.length() >= 0x8000)
+    if (s.length() > MaxLatin1Length)
         return false;
     return QtPrivate::isLatin1(s);
 }
 
-static inline int qStringSize(QStringView string, bool compress)
+static inline bool useCompressed(QLatin1String s)
 {
-    int l = 2 + string.length();
+    return s.size() <= MaxLatin1Length;
+}
+
+template <typename T>
+static inline int qStringSize(T string, bool compress)
+{
+    int l = 2 + string.size();
     if (!compress)
         l *= 2;
     return alignedSize(l);
@@ -218,11 +228,29 @@ public:
     {
         d->length = str.length();
         qToLittleEndian<quint16>(str.utf16(), str.length(), d->utf16);
-        if (str.length() & 1)
+        fillTrailingZeros();
-            d->utf16[str.length()] = 0;
         return *this;
     }
 
+    inline String &operator=(QLatin1String str)
+    {
+        d->length = str.size();
+#if Q_BYTE_ORDER == Q_BIG_ENDIAN
+        for (int i = 0; i < str.size(); ++i)
+            d->utf16[i] = str[i].unicode();
+#else
+        qt_from_latin1((ushort *)d->utf16, str.data(), str.size());
+#endif
+        fillTrailingZeros();
+        return *this;
+    }
+
+    void fillTrailingZeros()
+    {
+        if (d->length & 1)
+            d->utf16[d->length] = 0;
+    }
+
     inline bool operator ==(QStringView str) const {
         int slen = str.length();
         int l = d->length;
@@ -293,11 +321,27 @@ public:
         const ushort *uc = (const ushort *)str.utf16();
         qt_to_latin1_unchecked(l, uc, len);
 
-        for ( ; (quintptr)(l+len) & 0x3; ++len)
+        fillTrailingZeros();
-            l[len] = 0;
         return *this;
     }
 
+    inline Latin1String &operator=(QLatin1String str)
+    {
+        int len = d->length = str.size();
+        uchar *l = (uchar *)d->latin1;
+        memcpy(l, str.data(), len);
+
+        fillTrailingZeros();
+        return *this;
+    }
+
+    void fillTrailingZeros()
+    {
+        uchar *l = (uchar *)d->latin1;
+        for (int len = d->length; (quintptr)(l + len) & 0x3; ++len)
+            l[len] = 0;
+    }
+
     QLatin1String toQLatin1String() const noexcept {
         return QLatin1String(d->latin1, d->length);
     }
@@ -413,7 +457,8 @@ inline bool String::operator<(const Latin1String &str) const
 
 }
 
-static inline void copyString(char *dest, QStringView str, bool compress)
+template <typename T>
+static inline void copyString(char *dest, T str, bool compress)
 {
     if (compress) {
         Latin1String string(dest);

src/corelib/serialization/qjsonobject.cpp

@@ -397,14 +397,7 @@ QJsonValue QJsonObject::value(const QString &key) const
 */
 QJsonValue QJsonObject::value(QStringView key) const
 {
-    if (!d)
+    return valueImpl(key);
-        return QJsonValue(QJsonValue::Undefined);
-
-    bool keyExists;
-    int i = o->indexOf(key, &keyExists);
-    if (!keyExists)
-        return QJsonValue(QJsonValue::Undefined);
-    return QJsonValue(d, o, o->entryAt(i)->value);
 }
 
 /*!
@@ -412,6 +405,15 @@ QJsonValue QJsonObject::value(QStringView key) const
     \since 5.7
 */
 QJsonValue QJsonObject::value(QLatin1String key) const
+{
+    return valueImpl(key);
+}
+
+/*!
+    \internal
+*/
+template <typename T>
+QJsonValue QJsonObject::valueImpl(T key) const
 {
     if (!d)
         return QJsonValue(QJsonValue::Undefined);
@@ -477,13 +479,7 @@ QJsonValueRef QJsonObject::operator [](const QString &key)
 */
 QJsonValueRef QJsonObject::operator [](QStringView key)
 {
-    bool keyExists = false;
+    return atImpl(key);
-    int index = o ? o->indexOf(key, &keyExists) : 0;
-    if (!keyExists) {
-        iterator i = insertAt(index, key, QJsonValue(), false);
-        index = i.i;
-    }
-    return QJsonValueRef(this, index);
 }
 
 /*!
@@ -492,8 +488,22 @@ QJsonValueRef QJsonObject::operator [](QStringView key)
 */
 QJsonValueRef QJsonObject::operator [](QLatin1String key)
 {
-    // ### optimize me
+    return atImpl(key);
-    return operator[](QString(key));
+}
+
+/*!
+    \internal
+*/
+template <typename T>
+QJsonValueRef QJsonObject::atImpl(T key)
+{
+    bool keyExists = false;
+    int index = o ? o->indexOf(key, &keyExists) : 0;
+    if (!keyExists) {
+        iterator i = insertAt(index, key, QJsonValue(), false);
+        index = i.i;
+    }
+    return QJsonValueRef(this, index);
 }
 
 #if QT_STRINGVIEW_LEVEL < 2
@@ -521,6 +531,24 @@ QJsonObject::iterator QJsonObject::insert(const QString &key, const QJsonValue &
     \since 5.14
 */
 QJsonObject::iterator QJsonObject::insert(QStringView key, const QJsonValue &value)
+{
+    return insertImpl(key, value);
+}
+
+/*!
+    \overload
+    \since 5.14
+*/
+QJsonObject::iterator QJsonObject::insert(QLatin1String key, const QJsonValue &value)
+{
+    return insertImpl(key, value);
+}
+
+/*!
+    \internal
+*/
+template <typename T>
+QJsonObject::iterator QJsonObject::insertImpl(T key, const QJsonValue &value)
 {
     if (value.t == QJsonValue::Undefined) {
         remove(key);
@@ -534,7 +562,8 @@ QJsonObject::iterator QJsonObject::insert(QStringView key, const QJsonValue &val
 /*!
     \internal
  */
-QJsonObject::iterator QJsonObject::insertAt(int pos, QStringView key, const QJsonValue &value, bool keyExists)
+template <typename T>
+QJsonObject::iterator QJsonObject::insertAt(int pos, T key, const QJsonValue &value, bool keyExists)
 {
     QJsonValue val = value;
 
@@ -589,6 +618,24 @@ void QJsonObject::remove(const QString &key)
     \since 5.14
 */
 void QJsonObject::remove(QStringView key)
+{
+    removeImpl(key);
+}
+
+/*!
+    \overload
+    \since 5.14
+*/
+void QJsonObject::remove(QLatin1String key)
+{
+    removeImpl(key);
+}
+
+/*!
+    \internal
+*/
+template <typename T>
+void QJsonObject::removeImpl(T key)
 {
     if (!d)
         return;
@@ -622,6 +669,24 @@ QJsonValue QJsonObject::take(const QString &key)
     \since 5.14
 */
 QJsonValue QJsonObject::take(QStringView key)
+{
+    return takeImpl(key);
+}
+
+/*!
+    \overload
+    \since 5.14
+*/
+QJsonValue QJsonObject::take(QLatin1String key)
+{
+    return takeImpl(key);
+}
+
+/*!
+    \internal
+*/
+template <typename T>
+QJsonValue QJsonObject::takeImpl(T key)
 {
     if (!o)
         return QJsonValue(QJsonValue::Undefined);
@@ -655,12 +720,7 @@ bool QJsonObject::contains(const QString &key) const
 */
 bool QJsonObject::contains(QStringView key) const
 {
-    if (!o)
+    return containsImpl(key);
-        return false;
-
-    bool keyExists;
-    o->indexOf(key, &keyExists);
-    return keyExists;
 }
 
 /*!
@@ -668,6 +728,15 @@ bool QJsonObject::contains(QStringView key) const
     \since 5.7
 */
 bool QJsonObject::contains(QLatin1String key) const
+{
+    return containsImpl(key);
+}
+
+/*!
+    \internal
+*/
+template <typename T>
+bool QJsonObject::containsImpl(T key) const
 {
     if (!o)
         return false;
@@ -751,12 +820,7 @@ QJsonObject::iterator QJsonObject::find(const QString &key)
 */
 QJsonObject::iterator QJsonObject::find(QStringView key)
 {
-    bool keyExists = false;
+    return findImpl(key);
-    int index = o ? o->indexOf(key, &keyExists) : 0;
-    if (!keyExists)
-        return end();
-    detach2();
-    return iterator(this, index);
 }
 
 /*!
@@ -764,6 +828,15 @@ QJsonObject::iterator QJsonObject::find(QStringView key)
     \since 5.7
 */
 QJsonObject::iterator QJsonObject::find(QLatin1String key)
+{
+    return findImpl(key);
+}
+
+/*!
+    \internal
+*/
+template <typename T>
+QJsonObject::iterator QJsonObject::findImpl(T key)
 {
     bool keyExists = false;
     int index = o ? o->indexOf(key, &keyExists) : 0;
@@ -812,11 +885,7 @@ QJsonObject::const_iterator QJsonObject::constFind(const QString &key) const
 */
 QJsonObject::const_iterator QJsonObject::constFind(QStringView key) const
 {
-    bool keyExists = false;
+    return constFindImpl(key);
-    int index = o ? o->indexOf(key, &keyExists) : 0;
-    if (!keyExists)
-        return end();
-    return const_iterator(this, index);
 }
 
 /*!
@@ -824,6 +893,15 @@ QJsonObject::const_iterator QJsonObject::constFind(QStringView key) const
     \since 5.7
 */
 QJsonObject::const_iterator QJsonObject::constFind(QLatin1String key) const
+{
+    return constFindImpl(key);
+}
+
+/*!
+    \internal
+*/
+template <typename T>
+QJsonObject::const_iterator QJsonObject::constFindImpl(T key) const
 {
     bool keyExists = false;
     int index = o ? o->indexOf(key, &keyExists) : 0;

src/corelib/serialization/qjsonobject.h

@@ -118,7 +118,9 @@ public:
     bool contains(const QString &key) const;
 #endif
     void remove(QStringView key);
+    void remove(QLatin1String key);
     QJsonValue take(QStringView key);
+    QJsonValue take(QLatin1String key);
     bool contains(QStringView key) const;
     bool contains(QLatin1String key) const;
 
@@ -241,6 +243,7 @@ public:
     const_iterator constFind(QStringView key) const;
     const_iterator constFind(QLatin1String key) const;
     iterator insert(QStringView key, const QJsonValue &value);
+    iterator insert(QLatin1String key, const QJsonValue &value);
 
     // STL compatibility
     typedef QJsonValue mapped_type;
@@ -265,11 +268,20 @@ private:
     void compact();
     void compactIfNeeded();
 
+    template <typename T> QJsonValue valueImpl(T key) const;
+    template <typename T> QJsonValueRef atImpl(T key);
+    template <typename T> void removeImpl(T key);
+    template <typename T> QJsonValue takeImpl(T key);
+    template <typename T> bool containsImpl(T key) const;
+    template <typename T> iterator findImpl(T key);
+    template <typename T> const_iterator constFindImpl(T key) const;
+    template <typename T> iterator insertImpl(T key, const QJsonValue &value);
+
     QString keyAt(int i) const;
     QJsonValue valueAt(int i) const;
     void setValueAt(int i, const QJsonValue &val);
     void removeAt(int i);
-    iterator insertAt(int i, QStringView key, const QJsonValue &val, bool exists);
+    template <typename T> iterator insertAt(int i, T key, const QJsonValue &val, bool exists);
 
     QJsonPrivate::Data *d;
     QJsonPrivate::Object *o;

tests/auto/corelib/serialization/json/tst_qtjson.cpp

@@ -437,6 +437,7 @@ void tst_QtJson::testObjectSimple()
     QCOMPARE(object.value("boolean").toBool(), true);
     QCOMPARE(object.value(QLatin1String("boolean")).toBool(), true);
     QJsonObject object2 = object;
+    QJsonObject object3 = object;
 
     QStringList keys = object.keys();
     QVERIFY2(keys.contains("number"), "key number not found");
@@ -479,6 +480,23 @@ void tst_QtJson::testObjectSimple()
     object2.insert(QStringView(u"string"), QString::fromLatin1("foo"));
     QVERIFY2(object2.value(QStringView(u"string")).toString() != before, "value should have been updated");
 
+    // same tests again but with QLatin1String keys
+    object3.insert(QLatin1String("value"), value);
+    QCOMPARE(object3.value("value"), value);
+
+    size = object3.size();
+    object3.remove(QLatin1String("boolean"));
+    QCOMPARE(object3.size(), size - 1);
+    QVERIFY2(!object3.contains("boolean"), "key boolean should have been removed");
+
+    taken = object3.take(QLatin1String("value"));
+    QCOMPARE(taken, value);
+    QVERIFY2(!object3.contains("value"), "key value should have been removed");
+
+    before = object3.value("string").toString();
+    object3.insert(QLatin1String("string"), QString::fromLatin1("foo"));
+    QVERIFY2(object3.value(QLatin1String("string")).toString() != before, "value should have been updated");
+
     size = object.size();
     QJsonObject subobject;
     subobject.insert("number", 42);
@@ -2716,6 +2734,8 @@ void tst_QtJson::longStrings()
     // test around 15 and 16 bit boundaries, as these are limits
     // in the data structures (for Latin1String in qjson_p.h)
     QString s(0x7ff0, 'a');
+    QByteArray ba(0x7ff0, 'a');
+    ba.append(0x8010 - 0x7ff0, 'c');
     for (int i = 0x7ff0; i < 0x8010; i++) {
         s.append(QLatin1Char('c'));
 
@@ -2732,9 +2752,21 @@ void tst_QtJson::longStrings()
         /* ... and a QByteArray from the QJsonDocument */
         QByteArray a2 = d2.toJson();
         QCOMPARE(a1, a2);
+
+        // Test long keys
+        QJsonObject o1, o2;
+        o1[s] = 42;
+        o2[QLatin1String(ba.data(), i + 1)] = 42;
+        d1.setObject(o1);
+        d2.setObject(o2);
+        a1 = d1.toJson();
+        a2 = d2.toJson();
+        QCOMPARE(a1, a2);
     }
 
     s = QString(0xfff0, 'a');
+    ba = QByteArray(0xfff0, 'a');
+    ba.append(0x10010 - 0xfff0, 'c');
     for (int i = 0xfff0; i < 0x10010; i++) {
         s.append(QLatin1Char('c'));
 
@@ -2751,6 +2783,16 @@ void tst_QtJson::longStrings()
         /* ... and a QByteArray from the QJsonDocument */
         QByteArray a2 = d2.toJson();
         QCOMPARE(a1, a2);
+
+        // Test long keys
+        QJsonObject o1, o2;
+        o1[s] = 42;
+        o2[QLatin1String(ba.data(), i + 1)] = 42;
+        d1.setObject(o1);
+        d2.setObject(o2);
+        a1 = d1.toJson();
+        a2 = d2.toJson();
+        QCOMPARE(a1, a2);
     }
 }