From 93414d802d740cfaa99d974f548924b5199ff381 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Mon, 30 Jan 2017 11:06:36 -0800 Subject: [PATCH] fetch entity filter script asynchronously (but reject edits while waiting) --- assignment-client/src/entities/EntityServer.cpp | 17 ++++------------- assignment-client/src/entities/EntityServer.h | 1 - libraries/entities/src/EntityTree.cpp | 12 ++++++++++-- libraries/entities/src/EntityTree.h | 1 + 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/assignment-client/src/entities/EntityServer.cpp b/assignment-client/src/entities/EntityServer.cpp index 83a1b1ef84..dc1a693590 100644 --- a/assignment-client/src/entities/EntityServer.cpp +++ b/assignment-client/src/entities/EntityServer.cpp @@ -294,7 +294,9 @@ void EntityServer::readAdditionalConfiguration(const QJsonObject& settingsSectio } if (readOptionString("entityEditFilter", settingsSectionObject, _entityEditFilter) && !_entityEditFilter.isEmpty()) { - // Fetch script from file synchronously. We don't want the server processing edits while a restarting entity server is fetching from a DOS'd source. + // Tell the tree that we have a filter, so that it doesn't accept edits until we have a filter function set up. + std::static_pointer_cast(_tree)->setHasEntityFilter(true); + // Now fetch script from file asynchronously. QUrl scriptURL(_entityEditFilter); // The following should be abstracted out for use in Agent.cpp (and maybe later AvatarMixer.cpp) @@ -315,8 +317,6 @@ void EntityServer::readAdditionalConfiguration(const QJsonObject& settingsSectio qInfo() << "Requesting script at URL" << qPrintable(scriptRequest->getUrl().toString()); scriptRequest->send(); qDebug() << "script request sent"; - _scriptRequestLoop.exec(); // Block here, but allow the request to be processed and its signals to be handled. - qDebug() << "script request event loop complete"; } } @@ -367,11 +367,7 @@ void EntityServer::scriptRequestFinished() { return hadUncaughtExceptions(_entityEditFilterEngine, _entityEditFilter); }); scriptRequest->deleteLater(); - qDebug() << "script request ending event loop. running:" << _scriptRequestLoop.isRunning(); - if (_scriptRequestLoop.isRunning()) { - _scriptRequestLoop.quit(); - } - qDebug() << "script request event loop quit"; + qDebug() << "script request filter processed"; return; } } @@ -386,11 +382,6 @@ void EntityServer::scriptRequestFinished() { // Alas, only indications will be the above logging with assignment client restarting repeatedly, and clients will not see any entities. qDebug() << "script request failure causing stop"; stop(); - qDebug() << "script request ending event loop. running:" << _scriptRequestLoop.isRunning(); - if (_scriptRequestLoop.isRunning()) { - _scriptRequestLoop.quit(); - } - qDebug() << "script request event loop quit"; } void EntityServer::nodeAdded(SharedNodePointer node) { diff --git a/assignment-client/src/entities/EntityServer.h b/assignment-client/src/entities/EntityServer.h index 25270c9dd5..f142145d5f 100644 --- a/assignment-client/src/entities/EntityServer.h +++ b/assignment-client/src/entities/EntityServer.h @@ -80,7 +80,6 @@ private: QString _entityEditFilter{}; QScriptEngine _entityEditFilterEngine{}; - QEventLoop _scriptRequestLoop{}; }; #endif // hifi_EntityServer_h diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 7c3eb7bec3..e75c5442b2 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -927,13 +927,21 @@ void EntityTree::initEntityEditFilterEngine(QScriptEngine* engine, std::function _entityEditFilterHadUncaughtExceptions = entityEditFilterHadUncaughtExceptions; auto global = _entityEditFilterEngine->globalObject(); _entityEditFilterFunction = global.property("filter"); - _hasEntityEditFilter = _entityEditFilterFunction.isFunction(); + if (!_entityEditFilterFunction.isFunction()) { + qCDebug(entities) << "Filter function specified but not found. Will reject all edits."; + _entityEditFilterEngine = nullptr; // So that we don't try to call it. See filterProperties. + } + _hasEntityEditFilter = true; } bool EntityTree::filterProperties(EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged) { - if (!_hasEntityEditFilter || !_entityEditFilterEngine) { + if (!_entityEditFilterEngine) { propertiesOut = propertiesIn; wasChanged = false; // not changed + if (_hasEntityEditFilter) { + qCDebug(entities) << "Rejecting properties because filter has not been set."; + return false; + } return true; // allowed } auto oldProperties = propertiesIn.getDesiredProperties(); diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index cc179e7de0..23bc2cf8af 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -267,6 +267,7 @@ public: void notifyNewCollisionSoundURL(const QString& newCollisionSoundURL, const EntityItemID& entityID); void initEntityEditFilterEngine(QScriptEngine* engine, std::function entityEditFilterHadUncaughtExceptions); + void setHasEntityFilter(bool hasFilter) { _hasEntityEditFilter = hasFilter; } static const float DEFAULT_MAX_TMP_ENTITY_LIFETIME;