Ref T308, using an optimized (faster) function for YYYYmmdd string parsing

Ref T309, found that QDateTime::fromString returns a QDateTime object with local timestamp. Using a optimized version returns UTC objects now.

- The bug was found writing a unit test for Ref T308
- Slack: https://swift-project.slack.com/archives/G7GD2UP9C/p1534848725000100

samples/cliclient/client.cpp

@@ -276,7 +276,7 @@ namespace BlackSample
         const CFlightPlan::FlightRules flightRules = CFlightPlan::stringToFlightRules(flightRulesString);
         const CCallsign callsign("DAMBZ");
         CFlightPlan fp(callsign, equipmentIcao, originAirportIcao, destinationAirportIcao, alternateAirportIcao,
-                       QDateTime::fromString(takeoffTimePlanned, "hhmm"), QDateTime::fromString(takeoffTimeActual, "hhmm"),
+                       fromStringUtc(takeoffTimePlanned, "hhmm"), fromStringUtc(takeoffTimeActual, "hhmm"),
                        CTime(enrouteTime, CTimeUnit::hrmin()),
                        CTime(fuelTime, CTimeUnit::hrmin()),
                        CAltitude(cruiseAltitude, CAltitude::MeanSeaLevel, CLengthUnit::ft()),

src/blackcore/vatsim/networkvatlib.cpp

@@ -1338,8 +1338,8 @@ namespace BlackCore
                 self->fromFSD(fp->departAirport),
                 self->fromFSD(fp->destAirport),
                 self->fromFSD(fp->alternateAirport),
-                QDateTime::fromString(depTimePlanned, "hhmm"),
+                fromStringUtc(depTimePlanned, "hhmm"),
-                QDateTime::fromString(depTimeActual, "hhmm"),
+                fromStringUtc(depTimeActual, "hhmm"),
                 CTime(fp->enrouteHrs * 60 + fp->enrouteMins, BlackMisc::PhysicalQuantities::CTimeUnit::min()),
                 CTime(fp->fuelHrs * 60 + fp->fuelMins, BlackMisc::PhysicalQuantities::CTimeUnit::min()),
                 cruiseAlt,

src/blackcore/vatsim/vatsimbookingreader.cpp

@@ -112,8 +112,7 @@ namespace BlackCore
                     {
                         // normally the timestamp is always updated from backend
                         // if this changes in the future we're prepared
-                        updateTimestamp = QDateTime::fromString(ts, timestampFormat);
+                        updateTimestamp = fromStringUtc(ts, timestampFormat);
-                        updateTimestamp.setTimeSpec(Qt::UTC);
                         if (this->getUpdateTimestamp() == updateTimestamp) return; // nothing to do
 
                         // save parsing and all follow up actions if nothing changed
@@ -166,14 +165,12 @@ namespace BlackCore
                             }
                             else if (name == "time_end")
                             {
-                                QDateTime t = QDateTime::fromString(value, timestampFormat);
+                                QDateTime t = fromStringUtc(value, timestampFormat);
-                                t.setTimeSpec(Qt::UTC);
                                 bookedStation.setBookedUntilUtc(t);
                             }
                             else if (name == "time_start")
                             {
-                                QDateTime t = QDateTime::fromString(value, timestampFormat);
+                                QDateTime t = fromStringUtc(value, timestampFormat);
-                                t.setTimeSpec(Qt::UTC);
                                 bookedStation.setBookedFromUtc(t);
                             }
                         }

src/blackcore/vatsim/vatsimdatafilereader.cpp

@@ -344,8 +344,7 @@ namespace BlackCore
                                 const QStringList updateParts = currentLine.replace(" ", "").split('=');
                                 if (updateParts.length() < 2) break;
                                 const QString dts = updateParts.at(1).trimmed();
-                                updateTimestampFromFile = QDateTime::fromString(dts, "yyyyMMddHHmmss");
+                                updateTimestampFromFile = fromStringUtc(dts, "yyyyMMddHHmmss");
-                                updateTimestampFromFile.setOffsetFromUtc(0);
                                 const bool alreadyRead = (updateTimestampFromFile == this->getUpdateTimestamp());
                                 if (alreadyRead)
                                 {

src/blackmisc/db/artifact.cpp

@@ -229,8 +229,8 @@ namespace BlackMisc
                 return v;
             }
 
-            const QString versionTimestampString = BlackMisc::digitOnlyString(ts1Match.captured(0));
+            const QString versionTimestampString = digitOnlyString(ts1Match.captured(0));
-            const QDateTime versionTimestamp = QDateTime::fromString(versionTimestampString, "yyyyMMddHHmmss");
+            const QDateTime versionTimestamp = fromStringUtc(versionTimestampString, "yyyyMMddHHmmss");
             const QString lastSegment = QString::number(CBuildConfig::buildTimestampAsVersionSegment(versionTimestamp));
 
             v += lastSegment;
@@ -262,7 +262,7 @@ namespace BlackMisc
         {
             // yyyyMMddHHmmss (14): offset is 2010xxxxx
             if (seg.length() <= 13) { return seg; }
-            const int fs = CBuildConfig::buildTimestampAsVersionSegment(QDateTime::fromString(seg, "yyyyMMddHHmmss"));
+            const int fs = CBuildConfig::buildTimestampAsVersionSegment(fromStringUtc(seg, "yyyyMMddHHmmss"));
             return QString::number(fs);
         }
     } // ns

src/blackmisc/json.cpp

@@ -76,7 +76,7 @@ const QJsonValue &operator >>(const QJsonValue &json, bool &value)
 
 const QJsonValue &operator >>(const QJsonValue &json, QDateTime &value)
 {
-    value = QDateTime::fromString(json.toString());
+    value = fromStringUtc(json.toString());
     return json;
 }
 
@@ -150,7 +150,7 @@ QJsonValueRef operator >>(QJsonValueRef json, bool &value)
 
 QJsonValueRef operator >>(QJsonValueRef json, QDateTime &value)
 {
-    value = QDateTime::fromString(json.toString());
+    value = fromStringUtc(json.toString());
     return json;
 }
 

src/blackmisc/simulation/fscommon/vpilotrulesreader.cpp

@@ -247,7 +247,7 @@ namespace BlackMisc
 
                 // "2/1/2014 12:00:00 AM", "5/26/2014 2:00:00 PM"
                 const QString updated = attributes.namedItem("UpdatedOn").nodeValue();
-                QDateTime qt = QDateTime::fromString(updated, "M/d/yyyy h:mm:ss AP");
+                QDateTime qt = fromStringUtc(updated, "M/d/yyyy h:mm:ss AP");
                 qint64 updatedTimestamp = qt.toMSecsSinceEpoch();
 
                 int rulesSize = rules.size();

src/blackmisc/stringutils.cpp

@@ -17,6 +17,14 @@
 
 namespace BlackMisc
 {
+    QString removeDateTimeSeparators(const QString &s)
+    {
+        return removeChars(s, [](QChar c)
+        {
+            return c == ' ' || c == ':' || c == '_' || c == '-' || c == '.';
+        });
+    }
+
     QList<QStringRef> splitLinesRefs(const QString &s)
     {
         return splitStringRefs(s, [](QChar c) { return c == '\n' || c == '\r'; });
@@ -251,6 +259,20 @@ namespace BlackMisc
         return removeChars(name.toUpper(), [](QChar c) { return !c.isUpper(); });
     }
 
+    QDateTime fromStringUtc(const QString &dateTimeString, const QString &format)
+    {
+        QDateTime dt = QDateTime::fromString(dateTimeString, format);
+        dt.setUtcOffset(0);
+        return dt;
+    }
+
+    QDateTime fromStringUtc(const QString &dateTimeString, Qt::DateFormat format)
+    {
+        QDateTime dt = QDateTime::fromString(dateTimeString, format);
+        dt.setUtcOffset(0);
+        return dt;
+    }
+
     QDateTime parseMultipleDateTimeFormats(const QString &dateTimeString)
     {
         if (dateTimeString.isEmpty()) { return QDateTime(); }
@@ -259,47 +281,44 @@ namespace BlackMisc
             // 2017 0301 124421 321
             if (dateTimeString.length() == 17)
             {
-                return QDateTime::fromString(dateTimeString, "yyyyMMddHHmmsszzz");
+                return fromStringUtc(dateTimeString, "yyyyMMddHHmmsszzz");
             }
             if (dateTimeString.length() == 14)
             {
-                return QDateTime::fromString(dateTimeString, "yyyyMMddHHmmss");
+                return fromStringUtc(dateTimeString, "yyyyMMddHHmmss");
             }
             if (dateTimeString.length() == 12)
             {
-                return QDateTime::fromString(dateTimeString, "yyyyMMddHHmm");
+                return fromStringUtc(dateTimeString, "yyyyMMddHHmm");
             }
             if (dateTimeString.length() == 8)
             {
-                return QDateTime::fromString(dateTimeString, "yyyyMMdd");
+                return fromStringUtc(dateTimeString, "yyyyMMdd");
             }
             return QDateTime();
         }
 
         // remove simple separators and check if digits only again
-        const QString simpleSeparatorsRemoved = removeChars(dateTimeString, [](QChar c)
+        const QString simpleSeparatorsRemoved = removeDateTimeSeparators(dateTimeString);
-        {
-            return c == ' ' || c == ':' || c == '_' || c == '-';
-        });
         if (isDigitsOnlyString(simpleSeparatorsRemoved))
         {
             return parseMultipleDateTimeFormats(simpleSeparatorsRemoved);
         }
 
         // stupid trial and error
-        QDateTime ts = QDateTime::fromString(dateTimeString, Qt::ISODateWithMs);
+        QDateTime ts = fromStringUtc(dateTimeString, Qt::ISODateWithMs);
         if (ts.isValid()) return ts;
 
-        ts = QDateTime::fromString(dateTimeString, Qt::ISODate);
+        ts = fromStringUtc(dateTimeString, Qt::ISODate);
         if (ts.isValid()) return ts;
 
-        ts = QDateTime::fromString(dateTimeString, Qt::TextDate);
+        ts = fromStringUtc(dateTimeString, Qt::TextDate);
         if (ts.isValid()) return ts;
 
-        ts = QDateTime::fromString(dateTimeString, Qt::DefaultLocaleLongDate);
+        ts = fromStringUtc(dateTimeString, Qt::DefaultLocaleLongDate);
         if (ts.isValid()) return ts;
 
-        ts = QDateTime::fromString(dateTimeString, Qt::DefaultLocaleShortDate);
+        ts = fromStringUtc(dateTimeString, Qt::DefaultLocaleShortDate);
         if (ts.isValid()) return ts;
 
         // SystemLocaleShortDate,
@@ -307,6 +326,31 @@ namespace BlackMisc
         return QDateTime();
     }
 
+    QDateTime parseDateTimeStringOptimized(const QString &dateTimeString)
+    {
+        // yyyyMMddHHmmsszzz
+        // 01234567890123456
+        int year(dateTimeString.leftRef(4).toInt());
+        int month(dateTimeString.midRef(4, 2).toInt());
+        int day(dateTimeString.midRef(6, 2).toInt());
+        QDate date;
+        date.setDate(year, month, day);
+        QDateTime dt;
+        dt.setOffsetFromUtc(0);
+        dt.setDate(date);
+        if (dateTimeString.length() < 12) { return dt; }
+
+        QTime t;
+        const int hour(dateTimeString.midRef(8, 2).toInt());
+        const int minute(dateTimeString.midRef(10, 2).toInt());
+        const int second(dateTimeString.length() < 14 ? 0 : dateTimeString.midRef(12, 2).toInt());
+        const int ms(dateTimeString.length() < 17 ? 0 : dateTimeString.rightRef(3).toInt());
+
+        t.setHMS(hour, minute, second, ms);
+        dt.setTime(t);
+        return dt;
+    }
+
     QString dotToLocaleDecimalPoint(QString &input)
     {
         return input.replace('.', QLocale::system().decimalPoint());
@@ -343,7 +387,7 @@ namespace BlackMisc
     QString withQUestionMark(const QString &question)
     {
         if (question.endsWith("?")) { return question; }
-        return question + "?";
+        return question % QStringLiteral("?");
     }
 } // ns
 

src/blackmisc/stringutils.h

@@ -42,6 +42,9 @@ namespace BlackMisc
         return result;
     }
 
+    //! Remove the typical separators such as "-", " "
+    BLACKMISC_EXPORT QString removeDateTimeSeparators(const QString &s);
+
     //! True if any character in the string matches the given predicate.
     template <class F> bool containsChar(const QString &s, F predicate)
     {
@@ -192,10 +195,24 @@ namespace BlackMisc
     //! Add a question mark at the end if not existing
     BLACKMISC_EXPORT QString withQUestionMark(const QString &question);
 
+    //! Same as QDateTime::fromString but QDateTime will be set to UTC
+    //! \remark potentially slow, so only to be used when format is unknown
+    BLACKMISC_EXPORT QDateTime fromStringUtc(const QString &dateTimeString, const QString &format);
+
+    //! Same as QDateTime::fromString but QDateTime will be set to UTC
+    //! \remark potentially slow, so only to be used when format is unknown
+    BLACKMISC_EXPORT QDateTime fromStringUtc(const QString &dateTimeString, Qt::DateFormat format = Qt::TextDate);
+
     //! Parse multiple date time formats
     //! \remark potentially slow, so only to be used when format is unknown
+    //! \remark TZ is UTC
     BLACKMISC_EXPORT QDateTime parseMultipleDateTimeFormats(const QString &dateTimeString);
 
+    //! Parse yyyyMMddHHmmsszzz strings optimized
+    //! \remark string needs to be cleaned up and containing only numbers
+    //! \remark TZ is UTC
+    BLACKMISC_EXPORT QDateTime parseDateTimeStringOptimized(const QString &dateTimeString);
+
     namespace Mixin
     {
         /*!

src/blackmisc/timestampbased.cpp

@@ -9,6 +9,7 @@
 
 #include "blackmisc/comparefunctions.h"
 #include "blackmisc/timestampbased.h"
+#include "blackmisc/stringutils.h"
 #include "blackmisc/variant.h"
 #include "blackmisc/verify.h"
 
@@ -45,30 +46,8 @@ namespace BlackMisc
         // 0123 45 67 89 01 23 456
         // 1234 56 78 90 12 34 567
 
-        QString s(yyyyMMddhhmmsszzz);
+        const QString s(removeDateTimeSeparators(yyyyMMddhhmmsszzz));
-        s.remove(':').remove(' ').remove('-').remove('.'); // plain vanilla string
+        const QDateTime dt = parseDateTimeStringOptimized(s);
-        int year(s.leftRef(4).toInt());
-        int month(s.midRef(4, 2).toInt());
-        int day(s.midRef(6, 2).toInt());
-        QDate date;
-        date.setDate(year, month, day);
-        QDateTime dt;
-        dt.setOffsetFromUtc(0);
-        dt.setDate(date);
-        if (s.length() < 12)
-        {
-            this->setUtcTimestamp(dt);
-            return;
-        }
-
-        QTime t;
-        int hour(s.midRef(8, 2).toInt());
-        int minute(s.midRef(10, 2).toInt());
-        int second(s.length() < 14 ? 0 : s.midRef(12, 2).toInt());
-        int ms(s.length() < 17 ? 0 : s.rightRef(3).toInt());
-
-        t.setHMS(hour, minute, second, ms);
-        dt.setTime(t);
         this->setUtcTimestamp(dt);
     }
 
@@ -242,7 +221,7 @@ namespace BlackMisc
             case IndexUtcTimestampFormattedHms:
             case IndexUtcTimestampFormattedDhms:
                 {
-                    const QDateTime dt = QDateTime::fromString(variant.toQString());
+                    const QDateTime dt = fromStringUtc(variant.toQString());
                     m_timestampMSecsSinceEpoch = dt.toMSecsSinceEpoch();
                 }
                 return;

src/blackmisc/variant.cpp

@@ -180,7 +180,7 @@ namespace BlackMisc
         {
         case QVariant::Invalid:     throw CJsonException("Type not recognized by QMetaType");
         case QVariant::Int:         m_v.setValue(value.toInt()); break;
-        case QVariant::UInt:        m_v.setValue<uint>(value.toInt()); break;
+        case QVariant::UInt:        m_v.setValue<uint>(static_cast<uint>(value.toInt())); break;
         case QVariant::Bool:        m_v.setValue(value.toBool()); break;
         case QVariant::Double:      m_v.setValue(value.toDouble()); break;
         case QVariant::LongLong:    m_v.setValue(static_cast<qlonglong>(value.toDouble())); break;
@@ -188,7 +188,7 @@ namespace BlackMisc
         case QVariant::String:      m_v.setValue(value.toString()); break;
         case QVariant::Char:        m_v.setValue(value.toString().size() > 0 ? value.toString().at(0) : '\0'); break;
         case QVariant::ByteArray:   m_v.setValue(value.toString().toLatin1()); break;
-        case QVariant::DateTime:    m_v.setValue(QDateTime::fromString(value.toString(), Qt::ISODate)); break;
+        case QVariant::DateTime:    m_v.setValue(fromStringUtc(value.toString(), Qt::ISODate)); break;
         case QVariant::Date:        m_v.setValue(QDate::fromString(value.toString(), Qt::ISODate)); break;
         case QVariant::Time:        m_v.setValue(QTime::fromString(value.toString(), Qt::ISODate)); break;
         case QVariant::StringList:  m_v.setValue(QVariant(value.toArray().toVariantList()).toStringList()); break;

tests/blackmisc/teststringutils.cpp

@@ -18,6 +18,7 @@
 #include "blackmisc/stringutils.h"
 
 #include <QTest>
+#include <QTime>
 
 using namespace BlackMisc;
 
@@ -48,7 +49,7 @@ namespace BlackMiscTest
 
     void CTestStringUtils::testSplit()
     {
-        QString s = "line one\nline two\r\nline three\n";
+        const QString s = "line one\nline two\r\nline three\n";
         QStringList lines = splitLines(s);
         QVERIFY2(lines.size() == 3, "Test split string into lines: correct number of lines");
         QVERIFY2(lines[0] == "line one", "Test split string into lines: correct first line");
@@ -56,6 +57,76 @@ namespace BlackMiscTest
         QVERIFY2(lines[2] == "line three", "Test split string into lines: correct third line");
     }
 
+    void CTestStringUtils::testTimestampParsing()
+    {
+        const QStringList dts(
+        {
+            "2018-01-01 11:11:11",
+            "2012-05-09 03:04:05.777",
+            "2012-05-09 00:00:00.000",
+            "2015-12-31 03:04:05",
+            "1999-12-31 23:59:59.999",
+            "1975-01-01 14:13:17",
+            "1982-05-09 03:01:05.123",
+            "2000-05-02 00:04:00.000",
+            "2002-12-31 03:34:33",
+            "1992-11-01 21:59:29.999"
+        });
+
+        const int size = QString("yyyyMMddHHmmss").size();
+        for (const QString &dt : dts)
+        {
+            const QString c = removeDateTimeSeparators(dt);
+            const QDateTime dt1 = parseDateTimeStringOptimized(c);
+            const QDateTime dt2 = (c.length() == size) ?
+                                  fromStringUtc(c, "yyyyMMddHHmmss") :
+                                  fromStringUtc(c, "yyyyMMddHHmmsszzz");
+            QDateTime dt3 = (c.length() == size) ?
+                            QDateTime::fromString(c, "yyyyMMddHHmmss") :
+                            QDateTime::fromString(c, "yyyyMMddHHmmsszzz");
+            dt3.setUtcOffset(0);
+
+            const qint64 ms1 = dt1.toMSecsSinceEpoch();
+            const qint64 ms2 = dt2.toMSecsSinceEpoch();
+            const qint64 delta = ms1 - ms2;
+
+            QVERIFY2(dt1 == dt2, "Expect same results of QDateTime");
+            QVERIFY2(dt1 == dt3, "Expect same results of QDateTime");
+            QVERIFY2(delta == 0, "Expect same results timestamp");
+        }
+
+        // performance
+        int constexpr Loops = 10000;
+        QTime time;
+        time.start();
+        for (int i = 0; i < Loops; i++)
+        {
+            for (const QString &dt : dts)
+            {
+                const QString c = removeDateTimeSeparators(dt);
+                const QDateTime dateTime = parseDateTimeStringOptimized(c);
+                parseDateTimeStringOptimized(c);
+                Q_UNUSED(dateTime); // avoid optimizing out of call
+            }
+        }
+        const int elapsedOptimized = time.restart();
+
+        for (int i = 0; i < Loops; i++)
+        {
+            for (const QString &dt : dts)
+            {
+                const QString c = removeDateTimeSeparators(dt);
+                const QDateTime dateTime = (c.length() == size) ?
+                                           fromStringUtc(c, "yyyyMMddHHmmss") :
+                                           fromStringUtc(c, "yyyyMMddHHmmsszzz");
+                Q_UNUSED(dateTime); // avoid optimizing out of call
+            }
+        }
+        const int elapsedQt = time.restart();
+
+        qDebug() << "Parsing date/time, optimized" << elapsedOptimized << "vs. QDateTime: " << elapsedQt;
+        QVERIFY2(elapsedOptimized < elapsedQt, "Expect optimized being faster as QDateTim::fromString");
+    }
 }
 
 //! \endcond

tests/blackmisc/teststringutils.h

@@ -21,7 +21,6 @@
 
 namespace BlackMiscTest
 {
-
     //! Testing string utilities
     class CTestStringUtils : public QObject
     {
@@ -36,8 +35,8 @@ namespace BlackMiscTest
         void testContains();
         void testIndexOf();
         void testSplit();
+        void testTimestampParsing();
     };
-
 }
 
 //! \endcond