JSON stats reliable and ordered

assignment-client/src/octree/OctreeServer.cpp

@@ -1293,12 +1293,6 @@ QString OctreeServer::getStatusLink() {
 }
 
 void OctreeServer::sendStatsPacket() {
-    // TODO: we have too many stats to fit in a single MTU... so for now, we break it into multiple JSON objects and
-    // send them separately. What we really should do is change the NodeList::sendStatsToDomainServer() to handle the
-    // the following features:
-    //    1) remember last state sent
-    //    2) only send new data
-    
     // Stats Array 1
     QJsonArray threadsStats;
     quint64 oneSecondAgo = usecTimestampNow() - USECS_PER_SECOND;

domain-server/src/DomainGatekeeper.cpp

@@ -17,7 +17,6 @@
 
 #include <AccountManager.h>
 #include <Assignment.h>
-#include <JSONBreakableMarshal.h>
 
 #include "DomainServer.h"
 #include "DomainServerNodeData.h"
@@ -272,8 +271,8 @@ SharedNodePointer DomainGatekeeper::processAgentConnectRequest(const NodeConnect
     // if we have a username from the connect request, set it on the DomainServerNodeData
     nodeData->setUsername(username);
     
-    // also add an interpolation to JSONBreakableMarshal so that servers can get username in stats
+    // also add an interpolation to DomainServerNodeData so that servers can get username in stats
-    JSONBreakableMarshal::addInterpolationForKey(USERNAME_UUID_REPLACEMENT_STATS_KEY,
+    nodeData->addOverrideForKey(USERNAME_UUID_REPLACEMENT_STATS_KEY,
                                 uuidStringWithoutCurlyBraces(newNode->getUUID()), username);
     
     return newNode;

domain-server/src/DomainServer.cpp

@@ -25,7 +25,6 @@
 #include <ApplicationVersion.h>
 #include <HifiConfigVariantMap.h>
 #include <HTTPConnection.h>
-#include <JSONBreakableMarshal.h>
 #include <LogUtils.h>
 #include <NetworkingConstants.h>
 #include <udt/PacketHeaders.h>
@@ -1010,7 +1009,7 @@ void DomainServer::sendHeartbeatToIceServer() {
 void DomainServer::processNodeJSONStatsPacket(QSharedPointer<NLPacketList> packetList, SharedNodePointer sendingNode) {
     auto nodeData = dynamic_cast<DomainServerNodeData*>(sendingNode->getLinkedData());
     if (nodeData) {
-        nodeData->processJSONStatsPacket(packetList->getMessage());
+        nodeData->updateJSONStats(packetList->getMessage());
     }
 }
 
@@ -1676,8 +1675,8 @@ void DomainServer::nodeKilled(SharedNodePointer node) {
             }
         }
 
-        // If this node was an Agent ask JSONBreakableMarshal to potentially remove the interpolation we stored
+        // If this node was an Agent ask DomainServerNodeData to potentially remove the interpolation we stored
-        JSONBreakableMarshal::removeInterpolationForKey(USERNAME_UUID_REPLACEMENT_STATS_KEY,
+        nodeData->removeOverrideForKey(USERNAME_UUID_REPLACEMENT_STATS_KEY,
                                        uuidStringWithoutCurlyBraces(node->getUUID()));
 
         // cleanup the connection secrets that we set up for this node (on the other nodes)

domain-server/src/DomainServerNodeData.cpp

@@ -10,40 +10,72 @@
 //
 
 #include <QtCore/QDataStream>
+#include <QtCore/QJsonArray>
+#include <QtCore/QJsonDocument>
 #include <QtCore/QJsonObject>
 #include <QtCore/QVariant>
 
-#include <JSONBreakableMarshal.h>
 #include <udt/PacketHeaders.h>
 
 #include "DomainServerNodeData.h"
 
-DomainServerNodeData::DomainServerNodeData() :
+QHash<QPair<QString, QString>, QString> DomainServerNodeData::_overrideHash;
-    _sessionSecretHash(),
+
-    _assignmentUUID(),
+DomainServerNodeData::DomainServerNodeData() {
-    _walletUUID(),
-    _username(),
-    _paymentIntervalTimer(),
-    _statsJSONObject(),
-    _sendingSockAddr(),
-    _isAuthenticated(true)
-{
     _paymentIntervalTimer.start();
 }
 
-void DomainServerNodeData::processJSONStatsPacket(QByteArray statsByteArray) {
+void DomainServerNodeData::updateJSONStats(QByteArray statsByteArray) {
-    QJsonObject packetJsonObject = JSONBreakableMarshal::fromByteArray(statsByteArray);
+    auto document = QJsonDocument::fromBinaryData(statsByteArray);
-    _statsJSONObject = mergeJSONStatsFromNewObject(packetJsonObject, _statsJSONObject);
+    Q_ASSERT(document.isObject());
+    _statsJSONObject = overrideValuesIfNeeded(document.object());
 }
 
-QJsonObject DomainServerNodeData::mergeJSONStatsFromNewObject(const QJsonObject& newObject, QJsonObject destinationObject) {
+QJsonObject DomainServerNodeData::overrideValuesIfNeeded(const QJsonObject& newStats) {
-    foreach(const QString& key, newObject.keys()) {
+    QJsonObject result;
-        if (newObject[key].isObject() && destinationObject.contains(key)) {
+    for(auto it = newStats.constBegin(); it != newStats.constEnd(); ++it) {
-            destinationObject[key] = mergeJSONStatsFromNewObject(newObject[key].toObject(), destinationObject[key].toObject());
+        const auto& key = it.key();
+        const auto& value = it.value();
+        
+        auto overrideIt = value.isString() ? _overrideHash.find({key, value.toString()}) : _overrideHash.end();
+        if (overrideIt != _overrideHash.end()) {
+            // We have a match, override the value
+            result[key] = *overrideIt;
+        } else if (value.isObject()) {
+            result[key] = overrideValuesIfNeeded(value.toObject());
+        } else if (value.isArray()) {
+            result[key] = overrideValuesIfNeeded(value.toArray());
         } else {
-            destinationObject[key] = newObject[key];
+            result[key] = newStats[key];
         }
     }
+    return result;
+}
 
-    return destinationObject;
+QJsonArray DomainServerNodeData::overrideValuesIfNeeded(const QJsonArray& newStats) {
+    QJsonArray result;
+    for (int i = 0; i < newStats.size(); ++i) {
+        const auto& value = newStats[i];
+        
+        if (value.isObject()) {
+            
+            result.push_back(overrideValuesIfNeeded(value.toObject()));
+        } else if (value.isArray()) {
+            result.push_back(overrideValuesIfNeeded(value.toArray()));
+        } else {
+            result.push_back(newStats[i]);
+        }
+    }
+    return result;
+}
+
+void DomainServerNodeData::addOverrideForKey(const QString& key, const QString& value,
+                                                  const QString& overrideValue) {
+    // Insert override value
+    _overrideHash.insert({key, value}, overrideValue);
+}
+
+void DomainServerNodeData::removeOverrideForKey(const QString& key, const QString& value) {
+    // Remove override value
+    _overrideHash.remove({key, value});
 }

domain-server/src/DomainServerNodeData.h

@@ -27,7 +27,7 @@ public:
 
     const QJsonObject& getStatsJSONObject() const { return _statsJSONObject; }
 
-    void processJSONStatsPacket(QByteArray statsByteArray);
+    void updateJSONStats(QByteArray statsByteArray);
 
     void setAssignmentUUID(const QUuid& assignmentUUID) { _assignmentUUID = assignmentUUID; }
     const QUuid& getAssignmentUUID() const { return _assignmentUUID; }
@@ -54,17 +54,24 @@ public:
     void setNodeVersion(const QString& nodeVersion) { _nodeVersion = nodeVersion; }
     const QString& getNodeVersion() { return _nodeVersion; }
     
+    void addOverrideForKey(const QString& key, const QString& value, const QString& overrideValue);
+    void removeOverrideForKey(const QString& key, const QString& value);
+    
 private:
-    QJsonObject mergeJSONStatsFromNewObject(const QJsonObject& newObject, QJsonObject destinationObject);
+    QJsonObject overrideValuesIfNeeded(const QJsonObject& newStats);
+    QJsonArray overrideValuesIfNeeded(const QJsonArray& newStats);
     
     QHash<QUuid, QUuid> _sessionSecretHash;
     QUuid _assignmentUUID;
     QUuid _walletUUID;
     QString _username;
     QElapsedTimer _paymentIntervalTimer;
+    
     QJsonObject _statsJSONObject;
+    static QHash<QPair<QString, QString>, QString> _overrideHash;
+    
     HifiSockAddr _sendingSockAddr;
-    bool _isAuthenticated;
+    bool _isAuthenticated = true;
     NodeSet _nodeInterestSet;
     QString _nodeVersion;
 };

libraries/networking/src/JSONBreakableMarshal.cpp

@@ -1,88 +0,0 @@
-//
-//  JSONBreakableMarshal.cpp
-//  libraries/networking/src
-//
-//  Created by Stephen Birarda on 04/28/15.
-//  Copyright 2015 High Fidelity, Inc.
-//
-//  Distributed under the Apache License, Version 2.0.
-//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
-//
-
-#include "JSONBreakableMarshal.h"
-
-#include <QtCore/QDebug>
-#include <QtCore/QJsonArray>
-#include <QtCore/QJsonDocument>
-#include <QtCore/QJsonObject>
-
-QVariantMap JSONBreakableMarshal::_interpolationMap = QVariantMap();
-
-QByteArray JSONBreakableMarshal::toByteArray(const QJsonObject& jsonObject) {
-    return QJsonDocument(jsonObject).toBinaryData();
-}
-
-QJsonObject JSONBreakableMarshal::fromByteArray(const QByteArray& buffer) {
-    auto document = QJsonDocument::fromBinaryData(buffer);
-    Q_ASSERT(document.isObject());
-    auto object = document.object();
-    
-    QStringList currentKey;
-    for (auto key : object.keys()) {
-        interpolate(object[key], key);
-    }
-    
-    return object;
-}
-
-void JSONBreakableMarshal::addInterpolationForKey(const QString& rootKey, const QString& interpolationKey,
-                                                  const QString& interpolationValue) {
-    // if there is no map already beneath this key in our _interpolationMap create a QVariantMap there now
-    auto it = _interpolationMap.find(rootKey);
-    if (it == _interpolationMap.end()) {
-        it = _interpolationMap.insert(rootKey, QVariantMap());
-    }
-    Q_ASSERT(it->canConvert(QMetaType::QVariantMap));
-    static_cast<QVariantMap*>(it->data())->insert(interpolationKey, interpolationValue);
-}
-
-void JSONBreakableMarshal::removeInterpolationForKey(const QString& rootKey, const QString& interpolationKey) {
-    // make sure the interpolation map contains this root key and that the value is a map
-    auto it = _interpolationMap.find(rootKey);
-    if (it != _interpolationMap.end() && !it->isNull()) {
-        // remove the value at the interpolationKey
-        Q_ASSERT(it->canConvert(QMetaType::QVariantMap));
-        static_cast<QVariantMap*>(it->data())->remove(interpolationKey);
-    }
-}
-
-void JSONBreakableMarshal::interpolate(QJsonValueRef currentValue, QString lastKey) {
-    if (currentValue.isArray()) {
-        auto array = currentValue.toArray();
-        for (int i = 0; i < array.size(); ++i) {
-            // pass last key and recurse array
-            interpolate(array[i], QString::number(i));
-        }
-        currentValue = array;
-    } else if (currentValue.isObject()) {
-        auto object = currentValue.toObject();
-        for (auto key : object.keys()) {
-            // pass last key and recurse object
-            interpolate(object[key], key);
-        }
-        currentValue = object;
-    } else if (currentValue.isString()) {
-        // Maybe interpolate
-        auto mapIt = _interpolationMap.find(lastKey);
-        if (mapIt != _interpolationMap.end()) {
-            Q_ASSERT(mapIt->canConvert(QMetaType::QVariantMap));
-            auto interpolationMap = mapIt->toMap();
-            
-            auto result = interpolationMap.find(currentValue.toString());
-            if (result != interpolationMap.end()) {
-                // Replace value
-                currentValue = result->toString();
-            }
-        }
-    }
-}

libraries/networking/src/JSONBreakableMarshal.h

@@ -1,37 +0,0 @@
-//
-//  JSONBreakableMarshal.h 
-//  libraries/networking/src
-//
-//  Created by Stephen Birarda on 04/28/15.
-//  Copyright 2015 High Fidelity, Inc.
-//
-//  Distributed under the Apache License, Version 2.0.
-//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
-//
-
-#ifndef hifi_JSONBreakableMarshal_h
-#define hifi_JSONBreakableMarshal_h
-
-#pragma once
-
-#include <QtCore/QJsonValue>
-#include <QtCore/QString>
-#include <QtCore/QStringList>
-#include <QtCore/QVariantMap>
-
-class JSONBreakableMarshal {
-public:
-    static QByteArray toByteArray(const QJsonObject& jsonObject);
-    static QJsonObject fromByteArray(const QByteArray& buffer);
-
-    static void addInterpolationForKey(const QString& rootKey, const QString& interpolationKey,
-                                       const QString& interpolationValue);
-    static void removeInterpolationForKey(const QString& rootKey, const QString& interpolationKey); 
-    
-private:
-    static void interpolate(QJsonValueRef currentValue, QString lastKey);
-    
-    static QVariantMap _interpolationMap;
-};
-
-#endif // hifi_JSONBreakableMarshal_h

libraries/networking/src/NodeList.cpp

@@ -27,7 +27,6 @@
 #include "AddressManager.h"
 #include "Assignment.h"
 #include "HifiSockAddr.h"
-#include "JSONBreakableMarshal.h"
 
 #include "NetworkLogging.h"
 #include "udt/PacketHeaders.h"
@@ -109,7 +108,8 @@ NodeList::NodeList(char newOwnerType, unsigned short socketListenPort, unsigned
 qint64 NodeList::sendStats(const QJsonObject& statsObject, const HifiSockAddr& destination) {
     auto statsPacketList = NLPacketList::create(PacketType::NodeJsonStats, QByteArray(), true, true);
 
-    statsPacketList->write(JSONBreakableMarshal::toByteArray(statsObject));
+    QJsonDocument jsonDocument(statsObject);
+    statsPacketList->write(jsonDocument.toBinaryData());
 
     sendPacketList(std::move(statsPacketList), destination);
 

tools/udt-test/src/UDTTest.cpp

@@ -267,9 +267,7 @@ void UDTTest::sendPacket() {
         
         if (call++ % refillCount == 0) {
             // construct a reliable and ordered packet list
-            auto packetList = std::unique_ptr<udt::PacketList>({
+            auto packetList = udt::PacketList::create(PacketType::BulkAvatarData, QByteArray(), true, true);
-                new udt::PacketList(PacketType::BulkAvatarData, QByteArray(), true, true)
-            });
             
             // fill the packet list with random data according to the constant seed (so receiver can verify)
             for (int i = 0; i < messageSizePackets; ++i) {