From 9d860a8e8179c33d3439ad322b5d2660f92853a9 Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 16 Feb 2017 07:49:56 -0500 Subject: [PATCH 01/22] merge require/module changes into clean branch --- libraries/script-engine/src/ScriptEngine.cpp | 464 ++++++++++++++++++ libraries/script-engine/src/ScriptEngine.h | 19 + .../script-engine/src/ScriptEngineLogging.cpp | 1 + .../script-engine/src/ScriptEngineLogging.h | 1 + 4 files changed, 485 insertions(+) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 73a79f1bc6..8c458f71b7 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -70,6 +70,10 @@ #include "MIDIEvent.h" +const QString ScriptEngine::_SETTINGS_ENABLE_EXTENDED_MODULE_COMPAT { + "com.highfidelity.experimental.enableExtendedModuleCompatbility" +}; + static const QScriptEngine::QObjectWrapOptions DEFAULT_QOBJECT_WRAP_OPTIONS = QScriptEngine::ExcludeDeleteLater | QScriptEngine::ExcludeChildObjects; static const QScriptValue::PropertyFlags READONLY_PROP_FLAGS { QScriptValue::ReadOnly | QScriptValue::Undeletable }; @@ -532,6 +536,44 @@ static QScriptValue createScriptableResourcePrototype(QScriptEngine* engine) { return prototype; } +void ScriptEngine::resetModuleCache(bool deleteScriptCache) { + if (QThread::currentThread() != thread()) { + QMetaObject::invokeMethod(this, "resetModuleCache"); + return; + } + { + QMutexLocker locker(&_requireLock); + auto jsRequire = globalObject().property("Script").property("require"); + auto cache = jsRequire.property("cache"); + auto cacheMeta = jsRequire.data(); + + if (deleteScriptCache) { + QScriptValueIterator it(cache); + while (it.hasNext()) { + it.next(); + if (it.flags() & QScriptValue::SkipInEnumeration) + continue; + //scriptCache->deleteScript(it.name()); + qCDebug(scriptengine) << "resetModuleCache(true) -- staging " << it.name() << " for cache reset at next require"; + cacheMeta.setProperty(it.name(), true); + } + } + //_debugDump("cacheMeta", cacheMeta); + cache = newObject(); + if (!cacheMeta.isObject()) { + cacheMeta = newObject(); + cacheMeta.setProperty("id", "Script.require.cacheMeta"); + cacheMeta.setProperty("type", "cacheMeta"); + jsRequire.setData(cacheMeta); + } + cache.setProperty("__created__", (double)QDateTime::currentMSecsSinceEpoch(), QScriptValue::SkipInEnumeration); +#if DEBUG_JS_MODULES + cache.setProperty("__meta__", cacheMeta, HIDDEN_PROP_FLAGS); +#endif + jsRequire.setProperty("cache", cache, QScriptValue::ReadOnly | QScriptValue::Undeletable); + } +} + void ScriptEngine::init() { if (_isInitialized) { return; // only initialize once @@ -595,6 +637,15 @@ void ScriptEngine::init() { registerGlobalObject("Script", this); + { + // set up Script.require.resolve and Script.require.cache + auto Script = globalObject().property("Script"); + auto require = Script.property("require"); + auto resolve = Script.property("_requireResolve"); + require.setProperty("resolve", resolve, QScriptValue::ReadOnly | QScriptValue::Undeletable); + resetModuleCache(); + } + registerGlobalObject("Audio", &AudioScriptingInterface::getInstance()); registerGlobalObject("Entities", entityScriptingInterface.data()); registerGlobalObject("Quat", &_quatLibrary); @@ -1304,6 +1355,419 @@ void ScriptEngine::print(const QString& message) { emit printedMessage(message); } +static const auto MAX_MODULE_IDENTIFIER_LEN { 4096 }; +static const auto MAX_MODULE_IDENTIFIER_LOG_LEN { 60 }; + +// Script.require.resolve -- like resolvePath, but performs more validation and throws exceptions on invalid module identifiers (for consistency with Node.js) +QString ScriptEngine::_requireResolve(const QString& moduleId, const QString& relativeTo) { + QUrl defaultScriptsLoc = defaultScriptsLocation(); + QUrl url(moduleId); + + // helper to generate an exception and return a null string + auto resolverException = [=](const QString& detail) -> QString { + currentContext()->throwError( + QString("Cannot find module '%1' (%2)") + .arg(moduleId.left(MAX_MODULE_IDENTIFIER_LOG_LEN)) + .arg(detail)); + return QString(); + }; + + // de-fuzz the input a little by restricting to rational sizes + auto idLength = url.toString().length(); + if (idLength < 1 || idLength > MAX_MODULE_IDENTIFIER_LEN) { + return resolverException( + QString("rejecting invalid module id size (%1 chars [1,%2])") + .arg(idLength).arg(MAX_MODULE_IDENTIFIER_LEN) + ); + } + + // this regex matches: absolute, dotted or path-like URLs + // (ie: the kind of stuff ScriptEngine::resolvePath already handles) + QRegularExpression qualified ("^\\w+:|^/|^[.]{1,2}(/|$)"); + + // this is for module.require (which is a bound version of require that's always relative to the module path) + if (!relativeTo.isEmpty()) { + url = QUrl(relativeTo).resolved(moduleId); + url = resolvePath(url.toString()); + } else if (qualified.match(moduleId).hasMatch()) { + url = resolvePath(moduleId); + } else { + // check if the moduleId refers to a "system" module + QString defaultsPath = defaultScriptsLoc.path(); + QString systemModulePath = QString("%1/modules/%2.js").arg(defaultsPath).arg(moduleId); + url = defaultScriptsLoc; + url.setPath(systemModulePath); + if (!QFileInfo(url.toLocalFile()).isFile()) { + return resolverException("system module not found"); + } + } + + if (url.isRelative()) { + return resolverException("could not resolve module id"); + } + + // if it looks like a local file, verify that it's an allowed path and really a file + if (url.isLocalFile()) { + QFileInfo file(url.toLocalFile()); + QUrl canonical = url; + if (file.exists()) { + canonical.setPath(file.canonicalFilePath()); + } + + bool disallowOutsideFiles = !defaultScriptsLocation().isParentOf(canonical) && !currentSandboxURL.isLocalFile(); + if (disallowOutsideFiles && !PathUtils::isDescendantOf(canonical, currentSandboxURL)) { + return resolverException( + QString("path '%1' outside of origin script '%2' '%3'") + .arg(PathUtils::stripFilename(url)) + .arg(PathUtils::stripFilename(currentSandboxURL)) + .arg(canonical.toString()) + ); + } + if (!file.exists()) { + return resolverException("path does not exist: " + url.toLocalFile()); + } + if (!file.isFile()) { + return resolverException("path is not a file: " + url.toLocalFile()); + } + } + + return url.toString(); +} + +// retrieves the current parent module from the JS scope chain +QScriptValue ScriptEngine::currentModule() { + auto jsRequire = globalObject().property("Script").property("require"); + auto cache = jsRequire.property("cache"); + auto candidate = QScriptValue(); + for(auto ctx = currentContext(); ctx && !candidate.isObject(); ctx = ctx->parentContext()) { + QScriptContextInfo contextInfo { ctx }; + candidate = cache.property(contextInfo.fileName()); + } + if (!candidate.isObject()) { + return QScriptValue(); + } + return candidate; +} + +// replaces or adds "module" to "parent.children[]" array +// (for consistency with Node.js and userscript cache invalidation without "cache busters") +bool ScriptEngine::registerModuleWithParent(const QScriptValue& module, const QScriptValue& parent) { + auto children = parent.property("children"); + if (children.isArray()) { + auto key = module.property("id"); + auto length = children.property("length").toInt32(); + for(int i=0; i < length; i++) { + if (children.property(i).property("id").strictlyEquals(key)) { + qCDebug(scriptengine_module) << key.toString() << " updating parent.children[" << i << "] = module"; + children.setProperty(i, module); + return true; + } + } + qCDebug(scriptengine_module) << key.toString() << " appending parent.children[" << length << "] = module"; + children.setProperty(length, module); + return true; + } else if (parent.isValid()) { + qCDebug(scriptengine_module) << "registerModuleWithParent -- unrecognized parent" << parent.toVariant().toString(); + } + return false; +} + +// creates a new JS "module" Object with default metadata properties +QScriptValue ScriptEngine::newModule(const QString& modulePath, const QScriptValue& parent) { + auto closure = newObject(); + auto exports = newObject(); + auto module = newObject(); + qCDebug(scriptengine_module) << "newModule" << modulePath << parent.property("filename").toString(); + + closure.setProperty("module", module, READONLY_PROP_FLAGS); + + // note: this becomes the "exports" free variable, so should not be set read only + closure.setProperty("exports", exports); + + // make the closure available to module instantiation + module.setProperty("__closure__", closure, HIDDEN_PROP_FLAGS); + + // for consistency with Node.js Module + module.setProperty("id", modulePath, READONLY_PROP_FLAGS); + module.setProperty("filename", modulePath, READONLY_PROP_FLAGS); + module.setProperty("exports", exports); // not readonly + module.setProperty("loaded", false, READONLY_PROP_FLAGS); + module.setProperty("parent", parent, READONLY_PROP_FLAGS); + module.setProperty("children", newArray(), READONLY_PROP_FLAGS); + + // module.require is a bound version of require that always resolves relative to that module's path + auto boundRequire = QScriptEngine::evaluate("(function(id) { return Script.require(Script.require.resolve(id, this.filename)); })", "(boundRequire)"); + module.setProperty("require", boundRequire, READONLY_PROP_FLAGS); + + return module; +} + +// synchronously fetch a module's source code using BatchLoader +QScriptValue ScriptEngine::fetchModuleSource(const QString& modulePath, const bool forceDownload) { + using UrlMap = QMap; + auto scriptCache = DependencyManager::get(); + QScriptValue req = newObject(); + qCDebug(scriptengine_module) << "require.fetchModuleSource: " << QUrl(modulePath).fileName() << QThread::currentThread(); + + auto onload = [=, &req](const UrlMap& data, const UrlMap& _status) { + auto url = modulePath; + auto status = _status[url]; + auto contents = data[url]; + qCDebug(scriptengine_module) << "require.fetchModuleSource.onload: " << QUrl(url).fileName() << status << QThread::currentThread(); + if (isStopping()) { + req.setProperty("status", "Stopped"); + req.setProperty("success", false); + } else { + req.setProperty("url", url); + req.setProperty("status", status); + req.setProperty("success", ScriptCache::isSuccessStatus(status)); + req.setProperty("contents", contents, HIDDEN_PROP_FLAGS); + } + }; + + if (forceDownload) { + qCDebug(scriptengine_module) << "require.requestScript -- clearing cache for" << modulePath; + scriptCache->deleteScript(modulePath); + } + BatchLoader* loader = new BatchLoader({ modulePath }); + connect(loader, &BatchLoader::finished, this, onload); + connect(this, &QObject::destroyed, loader, &QObject::deleteLater); + // fail faster? (since require() blocks the engine thread while resolving dependencies) + const int MAX_RETRIES = 1; + + loader->start(MAX_RETRIES); + + if (!loader->isFinished()) { + QTimer monitor; + QEventLoop loop; + QObject::connect(loader, &BatchLoader::finished, this, [this, &monitor, &loop]{ + monitor.stop(); + loop.quit(); + }); + + // this helps detect the case where stop() is invoked during the download + // but not seen in time to abort processing in onload()... + connect(&monitor, &QTimer::timeout, this, [this, &loop, &loader]{ + if (isStopping()) { + loop.exit(-1); + } + }); + monitor.start(500); + loop.exec(); + } + loader->deleteLater(); + return req; +} + +// evaluate a pending module object using the fetched source code +QScriptValue ScriptEngine::instantiateModule(const QScriptValue& module, const QString& sourceCode) { + QScriptValue result; + auto modulePath = module.property("filename").toString(); + auto closure = module.property("__closure__"); + + qCDebug(scriptengine_module) << QString("require.instantiateModule: %1 / %2 bytes").arg(QUrl(modulePath).fileName()).arg(sourceCode.length()); + + { + ExceptionEmitter tryCatch(this, __FUNCTION__); + + if (module.property("content-type").toString() == "application/json") { + qCDebug(scriptengine_module) << "... parsing as JSON"; +#ifdef DEV_OTHER_MODULE_JSON_PARSE + auto JSON = globalObject().property("JSON"); + auto parse = JSON.property("parse"); + if (!parse.isFunction()) { + currentContext()->throwValue(makeError("global JSON.parse is not a function", "EvalError")); + return nullValue(); + } + result = parse.call(JSON, QScriptValueList({ sourceCode })); + closure.property("module").setProperty("exports", result); + return result; +#endif + closure.setProperty("__json", sourceCode); + result = evaluateInClosure(closure, { "module.exports = JSON.parse(__json)", modulePath }); + } else { + // scoped vars for consistency with Node.js + closure.setProperty("require", module.property("require")); + closure.setProperty("__filename", modulePath, HIDDEN_PROP_FLAGS); + closure.setProperty("__dirname", QString(modulePath).replace(QRegExp("/[^/]*$"), ""), HIDDEN_PROP_FLAGS); + result = evaluateInClosure(closure, { sourceCode, modulePath }); + } + } + return result; +} + +// CommonJS/Node.js like require/module support +QScriptValue ScriptEngine::require(const QString& moduleId) { + qCDebug(scriptengine_module) << "ScriptEngine::require(" << moduleId.left(MAX_MODULE_IDENTIFIER_LOG_LEN) << ")"; + if (QThread::currentThread() != thread()) { + qCDebug(scriptengine_module) << moduleId << " threads mismatch"; + return nullValue(); + } + + // serialize require calls so the ordering/caching works correctly across multiple Entities + // note: this is a Recursive mutex so nested require's should not affected + QMutexLocker locker(&_requireLock); + // _requireDepth++; + // QObject autoDecrement;connect(&autoDecrement, &QObject::destroyed, this, [this](){ _requireDepth--; }); + + auto jsRequire = globalObject().property("Script").property("require"); + auto cacheMeta = jsRequire.data(); + auto cache = jsRequire.property("cache"); + auto parent = currentModule(); + + auto throwModuleError = [this, &cache](const QString& modulePath, const QScriptValue& error) { + cache.setProperty(modulePath, nullValue()); + if (!error.isNull()) { +#ifdef DEBUG_JS_MODULES + qCWarning(scriptengine_module) << "throwing module error:" << error.toString() << modulePath << error.property("stack").toString(); +#endif + currentContext()->throwValue(makeError(error)); + } + return nullValue(); + }; + + // start by resolving the moduleId into a fully-qualified path/URL + QString modulePath = _requireResolve(moduleId); + if (hasUncaughtException()) { + // the resolver already threw an exception -- bail early + return nullValue(); + } + + // check the resolved path against the cache + auto module = cache.property(modulePath); + + // modules get cached in `Script.require.cache` and (similar to Node.js) users can access it + // to inspect particular entries and invalidate them by deleting the key: + // `delete Script.require.cache[Script.require.resolve(moduleId)];` + + // cacheMeta is just used right now to tell deleted keys apart from undefined ones + bool invalidateCache = module.isUndefined() && cacheMeta.property(moduleId).isValid(); + + // reset the cacheMeta record so invalidation won't apply next time, even if the module fails to load + cacheMeta.setProperty(modulePath, QScriptValue()); + + auto exports = module.property("exports"); + if (!invalidateCache && exports.isObject()) { + // we have found a cacheed module -- just need to possibly register it with current parent + qCDebug(scriptengine_module) << QString("require - using cached module '%1' for '%2' (loaded: %3)") + .arg(modulePath).arg(moduleId).arg(module.property("loaded").toString()); + registerModuleWithParent(module, parent); + return exports; + } + + // bootstrap / register new empty module + module = newModule(modulePath, parent); + registerModuleWithParent(module, parent); + + // add it to the cache (this is done early so any cyclic dependencies pick up) + cache.setProperty(modulePath, module); + + // download the module source + auto req = fetchModuleSource(modulePath, invalidateCache); + + if (!req.property("success").toBool()) { + auto error = QString("error retrieving script (%1)").arg(req.property("status").toString()); + return throwModuleError(modulePath, error); + } + +#if DEBUG_JS_MODULES + qCDebug(scriptengine_module) << "require.loaded: " << + QUrl(req.property("url").toString()).fileName() << req.property("status").toString(); +#endif + + auto sourceCode = req.property("contents").toString(); + + if (QUrl(modulePath).fileName().endsWith(".json", Qt::CaseInsensitive)) { + module.setProperty("content-type", "application/json"); + } else { + module.setProperty("content-type", "application/javascript"); + } + + { + // It seems that many JSON sources don't actually put .json in the URL... + // so for now as a workaround users wanting to indicate JSON parsing mode can + // do so by calling with a custom this context, eg: + // + // var ITEMS_URL = 'https://highfidelity.io/api/v1/marketplace/items'; + // var thisObject = { 'content-type': 'application/json' }; + // var items = Script.require.call(thisObject, ITEMS_URL + '?category=everything&sort=recent'); + + auto thisObject = currentContext()->thisObject(); + bool calledWithCustomThis = thisObject.isObject() && + !thisObject.strictlyEquals(globalObject()) && + !thisObject.toQObject(); + + if (calledWithCustomThis) { +#ifdef DEBUG_JS + _debugDump("this", thisObject); +#endif + _applyUserOptions(module, thisObject); + } + } + + // evaluate the module + auto result = instantiateModule(module, sourceCode); + + if (result.isError() && !result.strictlyEquals(module.property("exports"))) { + qCWarning(scriptengine_module) << "-- result.isError --" << result.toString(); + return throwModuleError(modulePath, result); + } + + // mark as fully-loaded + module.setProperty("loaded", true, READONLY_PROP_FLAGS); + + // set up a new reference point for detecting cache key deletion + cacheMeta.setProperty(modulePath, module); + + qCDebug(scriptengine_module) << "//ScriptEngine::require(" << moduleId << ")"; + + return module.property("exports"); +} + +// User-configurable override options +void ScriptEngine::_applyUserOptions(QScriptValue& module, QScriptValue& options) { + if (!options.isValid()) { + return; + } + // options['content-type'] === 'application/json' + // -- allows JSON modules to be used from URLs not ending in .json + if (options.property("content-type").isString()) { + module.setProperty("content-type", options.property("content-type")); + qCDebug(scriptengine_module) << "module['content-type'] =" << module.property("content-type").toString(); + } + + if (ScriptEngine::_enableExtendedModuleCompatbility.get()) { + auto closure = module.property("__closure__"); + + auto maybeSetToExports = [&](const QString& key) { + if (options.property(key).toString() == "exports") { + closure.setProperty(key, module.property("exports")); + qCDebug(scriptengine_module) << "module.closure[" << key << "] = exports"; + } + }; + + // options[{key}] = 'exports' + // several "agnostic" modules in the wild are just one step away from being compatible -- + // they just didn't know not to look specifically for this, self or global for attaching + // things onto. + maybeSetToExports("global"); + maybeSetToExports("self"); + maybeSetToExports("this"); + + // when options is an Object it will get used as the value of "this" during module evaluation + // (which is what one might expect when calling require.call(thisObject, ...)) + if (options.isObject()) { + closure.setProperty("this", options); + } + + // when options.global is an Object it'll get used as the global object (during evaluation only) + if (options.property("global").isObject()) { + closure.setProperty("global", options.property("global")); + qCDebug(scriptengine_module) << "module.closure['global'] = options.global"; + } + } +} + // If a callback is specified, the included files will be loaded asynchronously and the callback will be called // when all of the files have finished loading. // If no callback is specified, the included files will be loaded synchronously and will block execution until diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index b988ccfe90..f64dec44c7 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -41,6 +41,7 @@ #include "ScriptCache.h" #include "ScriptUUID.h" #include "Vec3.h" +#include "SettingHandle.h" class QScriptEngineDebugger; @@ -157,6 +158,16 @@ public: Q_INVOKABLE void include(const QStringList& includeFiles, QScriptValue callback = QScriptValue()); Q_INVOKABLE void include(const QString& includeFile, QScriptValue callback = QScriptValue()); + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // MODULE related methods + Q_INVOKABLE QScriptValue require(const QString& moduleId); + Q_INVOKABLE void resetModuleCache(bool deleteScriptCache = false); + QScriptValue currentModule(); + bool registerModuleWithParent(const QScriptValue& module, const QScriptValue& parent); + QScriptValue newModule(const QString& modulePath, const QScriptValue& parent = QScriptValue()); + QScriptValue fetchModuleSource(const QString& modulePath, const bool forceDownload = false); + QScriptValue instantiateModule(const QScriptValue& module, const QString& sourceCode); + Q_INVOKABLE QObject* setInterval(const QScriptValue& function, int intervalMS); Q_INVOKABLE QObject* setTimeout(const QScriptValue& function, int timeoutMS); Q_INVOKABLE void clearInterval(QObject* timer) { stopTimer(reinterpret_cast(timer)); } @@ -237,6 +248,9 @@ signals: protected: void init(); Q_INVOKABLE void executeOnScriptThread(std::function function, const Qt::ConnectionType& type = Qt::QueuedConnection ); + // note: this is not meant to be called directly, but just to have QMetaObject take care of wiring it up in general; + // then inside of init() we just have to do "Script.require.resolve = Script._requireResolve;" + Q_INVOKABLE QString _requireResolve(const QString& moduleId, const QString& relativeTo = QString()); QString logException(const QScriptValue& exception); void timerFired(); @@ -272,6 +286,7 @@ protected: QHash _entityScripts; QHash _occupiedScriptURLs; QList _deferredEntityLoads; + QMutex _requireLock { QMutex::Recursive }; bool _isThreaded { false }; QScriptEngineDebugger* _debugger { nullptr }; @@ -295,6 +310,10 @@ protected: std::recursive_mutex _lock; std::chrono::microseconds _totalTimerExecution { 0 }; + + static const QString _SETTINGS_ENABLE_EXTENDED_MODULE_COMPAT; + Setting::Handle _enableExtendedModuleCompatbility { _SETTINGS_ENABLE_EXTENDED_MODULE_COMPAT, false }; + void _applyUserOptions(QScriptValue& module, QScriptValue& options); }; #endif // hifi_ScriptEngine_h diff --git a/libraries/script-engine/src/ScriptEngineLogging.cpp b/libraries/script-engine/src/ScriptEngineLogging.cpp index 2e5d293728..392bc05129 100644 --- a/libraries/script-engine/src/ScriptEngineLogging.cpp +++ b/libraries/script-engine/src/ScriptEngineLogging.cpp @@ -12,3 +12,4 @@ #include "ScriptEngineLogging.h" Q_LOGGING_CATEGORY(scriptengine, "hifi.scriptengine") +Q_LOGGING_CATEGORY(scriptengine_module, "hifi.scriptengine.module") diff --git a/libraries/script-engine/src/ScriptEngineLogging.h b/libraries/script-engine/src/ScriptEngineLogging.h index 0e614dd5bf..62e46632a6 100644 --- a/libraries/script-engine/src/ScriptEngineLogging.h +++ b/libraries/script-engine/src/ScriptEngineLogging.h @@ -15,6 +15,7 @@ #include Q_DECLARE_LOGGING_CATEGORY(scriptengine) +Q_DECLARE_LOGGING_CATEGORY(scriptengine_module) #endif // hifi_ScriptEngineLogging_h From efc61c25ad6bc6a5383d1345c8a5c249f60d2327 Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 16 Feb 2017 18:33:34 -0500 Subject: [PATCH 02/22] * use explicit int's for moduleId constants * maxTestConstructorValueSize was already limiting debug output size -- adopt its strategy, use shared MAX_DEBUG_VALUE_LENGTH const --- libraries/script-engine/src/ScriptEngine.cpp | 35 +++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 8c458f71b7..8fb7a88c66 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -74,13 +74,14 @@ const QString ScriptEngine::_SETTINGS_ENABLE_EXTENDED_MODULE_COMPAT { "com.highfidelity.experimental.enableExtendedModuleCompatbility" }; +static const int MAX_MODULE_ID_LENTGH { 4096 }; +static const int MAX_DEBUG_VALUE_LENGTH { 80 }; + static const QScriptEngine::QObjectWrapOptions DEFAULT_QOBJECT_WRAP_OPTIONS = QScriptEngine::ExcludeDeleteLater | QScriptEngine::ExcludeChildObjects; static const QScriptValue::PropertyFlags READONLY_PROP_FLAGS { QScriptValue::ReadOnly | QScriptValue::Undeletable }; static const QScriptValue::PropertyFlags READONLY_HIDDEN_PROP_FLAGS { READONLY_PROP_FLAGS | QScriptValue::SkipInEnumeration }; - - static const bool HIFI_AUTOREFRESH_FILE_SCRIPTS { true }; Q_DECLARE_METATYPE(QScriptEngine::FunctionSignature) @@ -551,8 +552,9 @@ void ScriptEngine::resetModuleCache(bool deleteScriptCache) { QScriptValueIterator it(cache); while (it.hasNext()) { it.next(); - if (it.flags() & QScriptValue::SkipInEnumeration) + if (it.flags() & QScriptValue::SkipInEnumeration) { continue; + } //scriptCache->deleteScript(it.name()); qCDebug(scriptengine) << "resetModuleCache(true) -- staging " << it.name() << " for cache reset at next require"; cacheMeta.setProperty(it.name(), true); @@ -1355,29 +1357,31 @@ void ScriptEngine::print(const QString& message) { emit printedMessage(message); } -static const auto MAX_MODULE_IDENTIFIER_LEN { 4096 }; -static const auto MAX_MODULE_IDENTIFIER_LOG_LEN { 60 }; - // Script.require.resolve -- like resolvePath, but performs more validation and throws exceptions on invalid module identifiers (for consistency with Node.js) QString ScriptEngine::_requireResolve(const QString& moduleId, const QString& relativeTo) { QUrl defaultScriptsLoc = defaultScriptsLocation(); QUrl url(moduleId); // helper to generate an exception and return a null string - auto resolverException = [=](const QString& detail) -> QString { - currentContext()->throwError( + auto resolverException = [=](const QString& detail, const QString& type = "Error") -> QString { + auto displayId = moduleId; + if (displayId.length() > MAX_DEBUG_VALUE_LENGTH) { + displayId = displayId.mid(0, MAX_DEBUG_VALUE_LENGTH) + "..."; + } + currentContext()->throwValue(makeError( QString("Cannot find module '%1' (%2)") - .arg(moduleId.left(MAX_MODULE_IDENTIFIER_LOG_LEN)) - .arg(detail)); + .arg(displayId) + .arg(detail), type)); return QString(); }; // de-fuzz the input a little by restricting to rational sizes auto idLength = url.toString().length(); - if (idLength < 1 || idLength > MAX_MODULE_IDENTIFIER_LEN) { + if (idLength < 1 || idLength > MAX_MODULE_ID_LENTGH) { return resolverException( QString("rejecting invalid module id size (%1 chars [1,%2])") - .arg(idLength).arg(MAX_MODULE_IDENTIFIER_LEN) + .arg(idLength).arg(MAX_MODULE_ID_LENTGH), + "RangeError" ); } @@ -1598,7 +1602,7 @@ QScriptValue ScriptEngine::instantiateModule(const QScriptValue& module, const Q // CommonJS/Node.js like require/module support QScriptValue ScriptEngine::require(const QString& moduleId) { - qCDebug(scriptengine_module) << "ScriptEngine::require(" << moduleId.left(MAX_MODULE_IDENTIFIER_LOG_LEN) << ")"; + qCDebug(scriptengine_module) << "ScriptEngine::require(" << moduleId.left(MAX_DEBUG_VALUE_LENGTH) << ")"; if (QThread::currentThread() != thread()) { qCDebug(scriptengine_module) << moduleId << " threads mismatch"; return nullValue(); @@ -2245,9 +2249,8 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co testConstructorType = "empty"; } QString testConstructorValue = testConstructor.toString(); - const int maxTestConstructorValueSize = 80; - if (testConstructorValue.size() > maxTestConstructorValueSize) { - testConstructorValue = testConstructorValue.mid(0, maxTestConstructorValueSize) + "..."; + if (testConstructorValue.size() > MAX_DEBUG_VALUE_LENGTH) { + testConstructorValue = testConstructorValue.mid(0, MAX_DEBUG_VALUE_LENGTH) + "..."; } auto message = QString("failed to load entity script -- expected a function, got %1, %2") .arg(testConstructorType).arg(testConstructorValue); From 32e450e6c23c50e005edd798ca8f631e4a9ab2ef Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 16 Feb 2017 20:19:40 -0500 Subject: [PATCH 03/22] * rework _requireResolve error throws (to avoid using lambda w/default args -- which vc++ didn't care for) * add ExceptionEmitters to require/resolve so errors get logged even if invoked indpendently from a script --- libraries/script-engine/src/ScriptEngine.cpp | 48 ++++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 8fb7a88c66..882fec6c72 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -1361,28 +1361,22 @@ void ScriptEngine::print(const QString& message) { QString ScriptEngine::_requireResolve(const QString& moduleId, const QString& relativeTo) { QUrl defaultScriptsLoc = defaultScriptsLocation(); QUrl url(moduleId); + ExceptionEmitter tryCatcher(this, __FUNCTION__); - // helper to generate an exception and return a null string - auto resolverException = [=](const QString& detail, const QString& type = "Error") -> QString { - auto displayId = moduleId; - if (displayId.length() > MAX_DEBUG_VALUE_LENGTH) { - displayId = displayId.mid(0, MAX_DEBUG_VALUE_LENGTH) + "..."; - } - currentContext()->throwValue(makeError( - QString("Cannot find module '%1' (%2)") - .arg(displayId) - .arg(detail), type)); - return QString(); - }; + auto displayId = moduleId; + if (displayId.length() > MAX_DEBUG_VALUE_LENGTH) { + displayId = displayId.mid(0, MAX_DEBUG_VALUE_LENGTH) + "..."; + } + auto message = QString("Cannot find module '%1' (%2)").arg(displayId); + auto ctx = currentContext(); // de-fuzz the input a little by restricting to rational sizes auto idLength = url.toString().length(); if (idLength < 1 || idLength > MAX_MODULE_ID_LENTGH) { - return resolverException( - QString("rejecting invalid module id size (%1 chars [1,%2])") - .arg(idLength).arg(MAX_MODULE_ID_LENTGH), - "RangeError" - ); + auto details = QString("rejecting invalid module id size (%1 chars [1,%2])") + .arg(idLength).arg(MAX_MODULE_ID_LENTGH); + ctx->throwValue(makeError(message.arg(details), "RangeError")); + return nullptr; } // this regex matches: absolute, dotted or path-like URLs @@ -1402,12 +1396,14 @@ QString ScriptEngine::_requireResolve(const QString& moduleId, const QString& re url = defaultScriptsLoc; url.setPath(systemModulePath); if (!QFileInfo(url.toLocalFile()).isFile()) { - return resolverException("system module not found"); + ctx->throwValue(makeError(message.arg("system module not found"))); + return nullptr; } } if (url.isRelative()) { - return resolverException("could not resolve module id"); + ctx->throwValue(makeError(message.arg("could not resolve module id"))); + return nullptr; } // if it looks like a local file, verify that it's an allowed path and really a file @@ -1420,18 +1416,21 @@ QString ScriptEngine::_requireResolve(const QString& moduleId, const QString& re bool disallowOutsideFiles = !defaultScriptsLocation().isParentOf(canonical) && !currentSandboxURL.isLocalFile(); if (disallowOutsideFiles && !PathUtils::isDescendantOf(canonical, currentSandboxURL)) { - return resolverException( + ctx->throwValue(makeError(message.arg( QString("path '%1' outside of origin script '%2' '%3'") .arg(PathUtils::stripFilename(url)) .arg(PathUtils::stripFilename(currentSandboxURL)) .arg(canonical.toString()) - ); + ))); + return nullptr; } if (!file.exists()) { - return resolverException("path does not exist: " + url.toLocalFile()); + ctx->throwValue(makeError(message.arg("path does not exist: " + url.toLocalFile()))); + return nullptr; } if (!file.isFile()) { - return resolverException("path is not a file: " + url.toLocalFile()); + ctx->throwValue(makeError(message.arg("path is not a file: " + url.toLocalFile()))); + return nullptr; } } @@ -1611,6 +1610,7 @@ QScriptValue ScriptEngine::require(const QString& moduleId) { // serialize require calls so the ordering/caching works correctly across multiple Entities // note: this is a Recursive mutex so nested require's should not affected QMutexLocker locker(&_requireLock); + ExceptionEmitter tryCatcher(this, __FUNCTION__); // _requireDepth++; // QObject autoDecrement;connect(&autoDecrement, &QObject::destroyed, this, [this](){ _requireDepth--; }); @@ -1632,7 +1632,7 @@ QScriptValue ScriptEngine::require(const QString& moduleId) { // start by resolving the moduleId into a fully-qualified path/URL QString modulePath = _requireResolve(moduleId); - if (hasUncaughtException()) { + if (modulePath.isNull() || tryCatcher.hasPending()) { // the resolver already threw an exception -- bail early return nullValue(); } From e91de1775ec192c610bf3c9596bc9cd0c30d6d53 Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 16 Feb 2017 21:10:50 -0500 Subject: [PATCH 04/22] use a fully-qualified initializer when constructing BatchLoader --- libraries/script-engine/src/ScriptEngine.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 882fec6c72..09d267f3ba 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -1532,7 +1532,7 @@ QScriptValue ScriptEngine::fetchModuleSource(const QString& modulePath, const bo qCDebug(scriptengine_module) << "require.requestScript -- clearing cache for" << modulePath; scriptCache->deleteScript(modulePath); } - BatchLoader* loader = new BatchLoader({ modulePath }); + BatchLoader* loader = new BatchLoader(QList({ modulePath })); connect(loader, &BatchLoader::finished, this, onload); connect(this, &QObject::destroyed, loader, &QObject::deleteLater); // fail faster? (since require() blocks the engine thread while resolving dependencies) From 9b096513373662b3d88c61a20f6d9f42cb0dfd7c Mon Sep 17 00:00:00 2001 From: humbletim Date: Wed, 1 Mar 2017 09:11:40 -0500 Subject: [PATCH 05/22] Move BaseScriptEngine from script-engine to shared. --- libraries/{script-engine => shared}/src/BaseScriptEngine.cpp | 0 libraries/{script-engine => shared}/src/BaseScriptEngine.h | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename libraries/{script-engine => shared}/src/BaseScriptEngine.cpp (100%) rename libraries/{script-engine => shared}/src/BaseScriptEngine.h (100%) diff --git a/libraries/script-engine/src/BaseScriptEngine.cpp b/libraries/shared/src/BaseScriptEngine.cpp similarity index 100% rename from libraries/script-engine/src/BaseScriptEngine.cpp rename to libraries/shared/src/BaseScriptEngine.cpp diff --git a/libraries/script-engine/src/BaseScriptEngine.h b/libraries/shared/src/BaseScriptEngine.h similarity index 100% rename from libraries/script-engine/src/BaseScriptEngine.h rename to libraries/shared/src/BaseScriptEngine.h From 40ba8185a06a02f1106d54424032c9b8456bc336 Mon Sep 17 00:00:00 2001 From: humbletim Date: Wed, 1 Mar 2017 09:14:19 -0500 Subject: [PATCH 06/22] * Update per 21114-part2 changes. * Add explicit thread safety guards. * Add Entities.queryPropertyMetdata for unit testing Entity script module support. * Cleanup / commenting pass. --- .../src/EntityTreeRenderer.cpp | 1 + .../src/EntitiesScriptEngineProvider.h | 4 +- .../entities/src/EntityScriptingInterface.cpp | 109 +++++++ .../entities/src/EntityScriptingInterface.h | 20 ++ libraries/script-engine/src/ScriptEngine.cpp | 280 ++++++++++-------- libraries/script-engine/src/ScriptEngine.h | 13 +- libraries/shared/src/BaseScriptEngine.cpp | 129 ++++++-- libraries/shared/src/BaseScriptEngine.h | 43 ++- 8 files changed, 440 insertions(+), 159 deletions(-) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index bd25bcf905..2c07c368f9 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -146,6 +146,7 @@ void EntityTreeRenderer::clear() { void EntityTreeRenderer::reloadEntityScripts() { _entitiesScriptEngine->unloadAllEntityScripts(); + _entitiesScriptEngine->resetModuleCache(); foreach(auto entity, _entitiesInScene) { if (!entity->getScript().isEmpty()) { _entitiesScriptEngine->loadEntityScript(entity->getEntityItemID(), entity->getScript(), true); diff --git a/libraries/entities/src/EntitiesScriptEngineProvider.h b/libraries/entities/src/EntitiesScriptEngineProvider.h index 69bf73e688..d87dd105c2 100644 --- a/libraries/entities/src/EntitiesScriptEngineProvider.h +++ b/libraries/entities/src/EntitiesScriptEngineProvider.h @@ -15,11 +15,13 @@ #define hifi_EntitiesScriptEngineProvider_h #include +#include #include "EntityItemID.h" class EntitiesScriptEngineProvider { public: virtual void callEntityScriptMethod(const EntityItemID& entityID, const QString& methodName, const QStringList& params = QStringList()) = 0; + virtual QFuture getLocalEntityScriptDetails(const EntityItemID& entityID) = 0; }; -#endif // hifi_EntitiesScriptEngineProvider_h \ No newline at end of file +#endif // hifi_EntitiesScriptEngineProvider_h diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp index 540eba4511..df88194f9f 100644 --- a/libraries/entities/src/EntityScriptingInterface.cpp +++ b/libraries/entities/src/EntityScriptingInterface.cpp @@ -10,6 +10,9 @@ // #include "EntityScriptingInterface.h" +#include +#include + #include "EntityItemID.h" #include #include @@ -24,6 +27,7 @@ #include "ModelEntityItem.h" #include "QVariantGLM.h" #include "SimulationOwner.h" +#include "BaseScriptEngine.h" #include "ZoneEntityItem.h" #include "WebEntityItem.h" #include @@ -680,6 +684,111 @@ bool EntityScriptingInterface::reloadServerScripts(QUuid entityID) { return client->reloadServerScript(entityID); } +bool EntityScriptingInterface::queryPropertyMetadata(QUuid entityID, QScriptValue property, QScriptValue scopeOrCallback, QScriptValue methodOrName) { + auto name = property.toString(); + auto handler = makeScopedHandlerObject(scopeOrCallback, methodOrName); + QPointer engine = dynamic_cast(handler.engine()); + if (!engine) { + qCDebug(entities) << "queryPropertyMetadata without detectable engine" << entityID << name; + return false; + } + connect(engine, &QObject::destroyed, this, [=]() { + qDebug() << "queryPropertyMetadata -- engine destroyed!" << (!engine ? "nullptr" : "engine"); + }); + if (!handler.property("callback").isFunction()) { + qDebug() << "!handler.callback.isFunction" << engine; + engine->raiseException(engine->makeError("callback is not a function", "TypeError")); + return false; + } + if (name == "userData") { + EntityItemPointer entity = _entityTree->findEntityByEntityItemID(entityID); + QScriptValue err, result; + if (entity) { + auto JSON = engine->globalObject().property("JSON"); + auto parsed = JSON.property("parse").call(JSON, QScriptValueList({ entity->getUserData() })); + if (engine->hasUncaughtException()) { + err = engine->cloneUncaughtException(__FUNCTION__); + engine->clearExceptions(); + } else { + result = parsed; + } + } else { + err = engine->makeError("entity not found"); + } + QFutureWatcher *request = new QFutureWatcher; + connect(request, &QFutureWatcher::finished, engine, [=]() mutable { + if (!engine) { + qCDebug(entities) << "queryPropertyMetadata -- engine destroyed while inflight" << entityID << name; + return; + } + callScopedHandlerObject(handler, err, result); + request->deleteLater(); + }); + request->setFuture(QtConcurrent::run([]() -> QVariant { + QThread::sleep(1); + return 1; + })); + return true; + } else if (name == "script") { + using LocalScriptStatusRequest = QFutureWatcher; + LocalScriptStatusRequest *request = new LocalScriptStatusRequest; + connect(request, &LocalScriptStatusRequest::finished, engine, [=]() mutable { + if (!engine) { + qCDebug(entities) << "queryPropertyMetadata -- engine destroyed while inflight" << entityID << name; + return; + } + auto details = request->result().toMap(); + QScriptValue err, result; + if (details.contains("isError")) { + if (!details.contains("message")) { + details["message"] = details["errorInfo"]; + } + err = engine->makeError(engine->toScriptValue(details)); + } else { + details["success"] = true; + result = engine->toScriptValue(details); + } + callScopedHandlerObject(handler, err, result); + request->deleteLater(); + }); + request->setFuture(_entitiesScriptEngine->getLocalEntityScriptDetails(entityID)); + return true; + } else if (name == "serverScripts") { + auto client = DependencyManager::get(); + auto request = client->createScriptStatusRequest(entityID); + connect(request, &GetScriptStatusRequest::finished, engine, [=](GetScriptStatusRequest* request) mutable { + if (!engine) { + qCDebug(entities) << "queryPropertyMetadata -- engine destroyed while inflight" << entityID << name; + return; + } + QVariantMap details; + details["success"] = request->getResponseReceived(); + details["isRunning"] = request->getIsRunning(); + details["status"] = EntityScriptStatus_::valueToKey(request->getStatus()).toLower(); + details["errorInfo"] = request->getErrorInfo(); + + QScriptValue err, result; + if (!details["success"].toBool()) { + if (!details.contains("message") && details.contains("errorInfo")) { + details["message"] = details["errorInfo"]; + } + if (details["message"].toString().isEmpty()) { + details["message"] = "entity server script details not found"; + } + err = engine->makeError(engine->toScriptValue(details)); + } else { + result = engine->toScriptValue(details); + } + callScopedHandlerObject(handler, err, result); + request->deleteLater(); + }); + request->start(); + return true; + } + engine->raiseException(engine->makeError("property has no mapped metadata: " + name)); + return false; +} + bool EntityScriptingInterface::getServerScriptStatus(QUuid entityID, QScriptValue callback) { auto client = DependencyManager::get(); auto request = client->createScriptStatusRequest(entityID); diff --git a/libraries/entities/src/EntityScriptingInterface.h b/libraries/entities/src/EntityScriptingInterface.h index e9f0637830..2c3c654528 100644 --- a/libraries/entities/src/EntityScriptingInterface.h +++ b/libraries/entities/src/EntityScriptingInterface.h @@ -211,6 +211,26 @@ public slots: Q_INVOKABLE RayToEntityIntersectionResult findRayIntersectionBlocking(const PickRay& ray, bool precisionPicking = false, const QScriptValue& entityIdsToInclude = QScriptValue(), const QScriptValue& entityIdsToDiscard = QScriptValue()); Q_INVOKABLE bool reloadServerScripts(QUuid entityID); + + /**jsdoc + * Query for the available metadata behind one of an Entity's "magic" properties (eg: `script` and `serverScripts`). + * + * @function Entities.queryPropertyMetadata + * @param {EntityID} entityID The ID of the entity. + * @param {string} property The name of the property extended metadata is wanted for. + * @param {ResultCallback} callback Executes callback(err, result) with the query results. + */ + /**jsdoc + * Query for the available metadata behind one of an Entity's "magic" properties (eg: `script` and `serverScripts`). + * + * @function Entities.queryPropertyMetadata + * @param {EntityID} entityID The ID of the entity. + * @param {string} property The name of the property extended metadata is wanted for. + * @param {Object} thisObject The scoping "this" context that callback will be executed within. + * @param {ResultCallback} callbackOrMethodName Executes thisObject[callbackOrMethodName](err, result) with the query results. + */ + Q_INVOKABLE bool queryPropertyMetadata(QUuid entityID, QScriptValue property, QScriptValue scopeOrCallback, QScriptValue methodOrName = QScriptValue()); + Q_INVOKABLE bool getServerScriptStatus(QUuid entityID, QScriptValue callback); Q_INVOKABLE void setLightsArePickable(bool value); diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 09d267f3ba..c7364450a1 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -19,6 +19,9 @@ #include #include +#include +#include + #include #include @@ -74,6 +77,10 @@ const QString ScriptEngine::_SETTINGS_ENABLE_EXTENDED_MODULE_COMPAT { "com.highfidelity.experimental.enableExtendedModuleCompatbility" }; +const QString ScriptEngine::_SETTINGS_ENABLE_EXTENDED_EXCEPTIONS { + "com.highfidelity.experimental.enableExtendedJSExceptions" +}; + static const int MAX_MODULE_ID_LENTGH { 4096 }; static const int MAX_DEBUG_VALUE_LENGTH { 80 }; @@ -146,7 +153,7 @@ QString encodeEntityIdIntoEntityUrl(const QString& url, const QString& entityID) } QString ScriptEngine::logException(const QScriptValue& exception) { - auto message = formatException(exception); + auto message = formatException(exception, _enableExtendedJSExceptions.get()); scriptErrorMessage(qPrintable(message)); return message; } @@ -338,7 +345,11 @@ void ScriptEngine::runInThread() { // The thread interface cannot live on itself, and we want to move this into the thread, so // the thread cannot have this as a parent. QThread* workerThread = new QThread(); +#ifdef Q_OS_LINUX + workerThread->setObjectName(QString("js:") + getFilename()); +#else workerThread->setObjectName(QString("Script Thread:") + getFilename()); +#endif moveToThread(workerThread); // NOTE: If you connect any essential signals for proper shutdown or cleanup of @@ -539,41 +550,37 @@ static QScriptValue createScriptableResourcePrototype(QScriptEngine* engine) { void ScriptEngine::resetModuleCache(bool deleteScriptCache) { if (QThread::currentThread() != thread()) { - QMetaObject::invokeMethod(this, "resetModuleCache"); + executeOnScriptThread([=](){ resetModuleCache(deleteScriptCache); }); return; } - { - QMutexLocker locker(&_requireLock); - auto jsRequire = globalObject().property("Script").property("require"); - auto cache = jsRequire.property("cache"); - auto cacheMeta = jsRequire.data(); + auto jsRequire = globalObject().property("Script").property("require"); + auto cache = jsRequire.property("cache"); + auto cacheMeta = jsRequire.data(); - if (deleteScriptCache) { - QScriptValueIterator it(cache); - while (it.hasNext()) { - it.next(); - if (it.flags() & QScriptValue::SkipInEnumeration) { - continue; - } - //scriptCache->deleteScript(it.name()); - qCDebug(scriptengine) << "resetModuleCache(true) -- staging " << it.name() << " for cache reset at next require"; - cacheMeta.setProperty(it.name(), true); + if (deleteScriptCache) { + QScriptValueIterator it(cache); + while (it.hasNext()) { + it.next(); + if (it.flags() & QScriptValue::SkipInEnumeration) { + continue; } + //scriptCache->deleteScript(it.name()); + qCDebug(scriptengine) << "resetModuleCache(true) -- staging " << it.name() << " for cache reset at next require"; + cacheMeta.setProperty(it.name(), true); } - //_debugDump("cacheMeta", cacheMeta); - cache = newObject(); - if (!cacheMeta.isObject()) { - cacheMeta = newObject(); - cacheMeta.setProperty("id", "Script.require.cacheMeta"); - cacheMeta.setProperty("type", "cacheMeta"); - jsRequire.setData(cacheMeta); - } - cache.setProperty("__created__", (double)QDateTime::currentMSecsSinceEpoch(), QScriptValue::SkipInEnumeration); -#if DEBUG_JS_MODULES - cache.setProperty("__meta__", cacheMeta, HIDDEN_PROP_FLAGS); -#endif - jsRequire.setProperty("cache", cache, QScriptValue::ReadOnly | QScriptValue::Undeletable); } + cache = newObject(); + if (!cacheMeta.isObject()) { + cacheMeta = newObject(); + cacheMeta.setProperty("id", "Script.require.cacheMeta"); + cacheMeta.setProperty("type", "cacheMeta"); + jsRequire.setData(cacheMeta); + } + cache.setProperty("__created__", (double)QDateTime::currentMSecsSinceEpoch(), QScriptValue::SkipInEnumeration); +#if DEBUG_JS_MODULES + cache.setProperty("__meta__", cacheMeta, READONLY_HIDDEN_PROP_FLAGS); +#endif + jsRequire.setProperty("cache", cache, QScriptValue::ReadOnly | QScriptValue::Undeletable); } void ScriptEngine::init() { @@ -916,6 +923,11 @@ void ScriptEngine::addEventHandler(const EntityItemID& entityID, const QString& handlersForEvent << handlerData; // Note that the same handler can be added many times. See removeEntityEventHandler(). } +// this is not redundant -- the version in BaseScriptEngine is specifically not Q_INVOKABLE +QScriptValue ScriptEngine::evaluateInClosure(const QScriptValue& closure, const QScriptProgram& program) { + return BaseScriptEngine::evaluateInClosure(closure, program); +} + QScriptValue ScriptEngine::evaluate(const QString& sourceCode, const QString& fileName, int lineNumber) { if (DependencyManager::get()->isStopped()) { return QScriptValue(); // bail early @@ -938,29 +950,26 @@ QScriptValue ScriptEngine::evaluate(const QString& sourceCode, const QString& fi // Check syntax auto syntaxError = lintScript(sourceCode, fileName); if (syntaxError.isError()) { - if (isEvaluating()) { - currentContext()->throwValue(syntaxError); - } else { + if (!isEvaluating()) { syntaxError.setProperty("detail", "evaluate"); - emit unhandledException(syntaxError); } + raiseException(syntaxError); + maybeEmitUncaughtException(__FUNCTION__); return syntaxError; } QScriptProgram program { sourceCode, fileName, lineNumber }; if (program.isNull()) { // can this happen? auto err = makeError("could not create QScriptProgram for " + fileName); - emit unhandledException(err); + raiseException(err); + maybeEmitUncaughtException(__FUNCTION__); return err; } QScriptValue result; { result = BaseScriptEngine::evaluate(program); - if (!isEvaluating() && hasUncaughtException()) { - emit unhandledException(cloneUncaughtException(__FUNCTION__)); - clearExceptions(); - } + maybeEmitUncaughtException(__FUNCTION__); } return result; } @@ -983,10 +992,7 @@ void ScriptEngine::run() { { evaluate(_scriptContents, _fileNameString); - if (!isEvaluating() && hasUncaughtException()) { - emit unhandledException(cloneUncaughtException(__FUNCTION__)); - clearExceptions(); - } + maybeEmitUncaughtException(__FUNCTION__); } #ifdef _WIN32 // VS13 does not sleep_until unless it uses the system_clock, see: @@ -1359,24 +1365,30 @@ void ScriptEngine::print(const QString& message) { // Script.require.resolve -- like resolvePath, but performs more validation and throws exceptions on invalid module identifiers (for consistency with Node.js) QString ScriptEngine::_requireResolve(const QString& moduleId, const QString& relativeTo) { + if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) { + return QString(); + } QUrl defaultScriptsLoc = defaultScriptsLocation(); QUrl url(moduleId); - ExceptionEmitter tryCatcher(this, __FUNCTION__); auto displayId = moduleId; if (displayId.length() > MAX_DEBUG_VALUE_LENGTH) { displayId = displayId.mid(0, MAX_DEBUG_VALUE_LENGTH) + "..."; } auto message = QString("Cannot find module '%1' (%2)").arg(displayId); - auto ctx = currentContext(); + + auto throwResolveError = [&](const QScriptValue& error) -> QString { + raiseException(error); + maybeEmitUncaughtException("require.resolve"); + return nullptr; + }; // de-fuzz the input a little by restricting to rational sizes auto idLength = url.toString().length(); if (idLength < 1 || idLength > MAX_MODULE_ID_LENTGH) { auto details = QString("rejecting invalid module id size (%1 chars [1,%2])") .arg(idLength).arg(MAX_MODULE_ID_LENTGH); - ctx->throwValue(makeError(message.arg(details), "RangeError")); - return nullptr; + return throwResolveError(makeError(message.arg(details), "RangeError")); } // this regex matches: absolute, dotted or path-like URLs @@ -1396,14 +1408,12 @@ QString ScriptEngine::_requireResolve(const QString& moduleId, const QString& re url = defaultScriptsLoc; url.setPath(systemModulePath); if (!QFileInfo(url.toLocalFile()).isFile()) { - ctx->throwValue(makeError(message.arg("system module not found"))); - return nullptr; + return throwResolveError(makeError(message.arg("system module not found"))); } } if (url.isRelative()) { - ctx->throwValue(makeError(message.arg("could not resolve module id"))); - return nullptr; + return throwResolveError(makeError(message.arg("could not resolve module id"))); } // if it looks like a local file, verify that it's an allowed path and really a file @@ -1416,34 +1426,35 @@ QString ScriptEngine::_requireResolve(const QString& moduleId, const QString& re bool disallowOutsideFiles = !defaultScriptsLocation().isParentOf(canonical) && !currentSandboxURL.isLocalFile(); if (disallowOutsideFiles && !PathUtils::isDescendantOf(canonical, currentSandboxURL)) { - ctx->throwValue(makeError(message.arg( + return throwResolveError(makeError(message.arg( QString("path '%1' outside of origin script '%2' '%3'") .arg(PathUtils::stripFilename(url)) .arg(PathUtils::stripFilename(currentSandboxURL)) .arg(canonical.toString()) ))); - return nullptr; } if (!file.exists()) { - ctx->throwValue(makeError(message.arg("path does not exist: " + url.toLocalFile()))); - return nullptr; + return throwResolveError(makeError(message.arg("path does not exist: " + url.toLocalFile()))); } if (!file.isFile()) { - ctx->throwValue(makeError(message.arg("path is not a file: " + url.toLocalFile()))); - return nullptr; + return throwResolveError(makeError(message.arg("path is not a file: " + url.toLocalFile()))); } } + maybeEmitUncaughtException(__FUNCTION__); return url.toString(); } // retrieves the current parent module from the JS scope chain QScriptValue ScriptEngine::currentModule() { + if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) { + return unboundNullValue(); + } auto jsRequire = globalObject().property("Script").property("require"); auto cache = jsRequire.property("cache"); auto candidate = QScriptValue(); - for(auto ctx = currentContext(); ctx && !candidate.isObject(); ctx = ctx->parentContext()) { - QScriptContextInfo contextInfo { ctx }; + for (auto c = currentContext(); c && !candidate.isObject(); c = c->parentContext()) { + QScriptContextInfo contextInfo { c }; candidate = cache.property(contextInfo.fileName()); } if (!candidate.isObject()) { @@ -1459,7 +1470,7 @@ bool ScriptEngine::registerModuleWithParent(const QScriptValue& module, const QS if (children.isArray()) { auto key = module.property("id"); auto length = children.property("length").toInt32(); - for(int i=0; i < length; i++) { + for (int i=0; i < length; i++) { if (children.property(i).property("id").strictlyEquals(key)) { qCDebug(scriptengine_module) << key.toString() << " updating parent.children[" << i << "] = module"; children.setProperty(i, module); @@ -1488,7 +1499,7 @@ QScriptValue ScriptEngine::newModule(const QString& modulePath, const QScriptVal closure.setProperty("exports", exports); // make the closure available to module instantiation - module.setProperty("__closure__", closure, HIDDEN_PROP_FLAGS); + module.setProperty("__closure__", closure, READONLY_HIDDEN_PROP_FLAGS); // for consistency with Node.js Module module.setProperty("id", modulePath, READONLY_PROP_FLAGS); @@ -1524,7 +1535,7 @@ QScriptValue ScriptEngine::fetchModuleSource(const QString& modulePath, const bo req.setProperty("url", url); req.setProperty("status", status); req.setProperty("success", ScriptCache::isSuccessStatus(status)); - req.setProperty("contents", contents, HIDDEN_PROP_FLAGS); + req.setProperty("contents", contents, READONLY_HIDDEN_PROP_FLAGS); } }; @@ -1568,73 +1579,54 @@ QScriptValue ScriptEngine::instantiateModule(const QScriptValue& module, const Q auto modulePath = module.property("filename").toString(); auto closure = module.property("__closure__"); - qCDebug(scriptengine_module) << QString("require.instantiateModule: %1 / %2 bytes").arg(QUrl(modulePath).fileName()).arg(sourceCode.length()); + qCDebug(scriptengine_module) << QString("require.instantiateModule: %1 / %2 bytes") + .arg(QUrl(modulePath).fileName()).arg(sourceCode.length()); - { - ExceptionEmitter tryCatch(this, __FUNCTION__); - - if (module.property("content-type").toString() == "application/json") { - qCDebug(scriptengine_module) << "... parsing as JSON"; -#ifdef DEV_OTHER_MODULE_JSON_PARSE - auto JSON = globalObject().property("JSON"); - auto parse = JSON.property("parse"); - if (!parse.isFunction()) { - currentContext()->throwValue(makeError("global JSON.parse is not a function", "EvalError")); - return nullValue(); - } - result = parse.call(JSON, QScriptValueList({ sourceCode })); - closure.property("module").setProperty("exports", result); - return result; -#endif - closure.setProperty("__json", sourceCode); - result = evaluateInClosure(closure, { "module.exports = JSON.parse(__json)", modulePath }); - } else { - // scoped vars for consistency with Node.js - closure.setProperty("require", module.property("require")); - closure.setProperty("__filename", modulePath, HIDDEN_PROP_FLAGS); - closure.setProperty("__dirname", QString(modulePath).replace(QRegExp("/[^/]*$"), ""), HIDDEN_PROP_FLAGS); - result = evaluateInClosure(closure, { sourceCode, modulePath }); - } + if (module.property("content-type").toString() == "application/json") { + qCDebug(scriptengine_module) << "... parsing as JSON"; + closure.setProperty("__json", sourceCode); + result = evaluateInClosure(closure, { "module.exports = JSON.parse(__json)", modulePath }); + } else { + // scoped vars for consistency with Node.js + closure.setProperty("require", module.property("require")); + closure.setProperty("__filename", modulePath, READONLY_HIDDEN_PROP_FLAGS); + closure.setProperty("__dirname", QString(modulePath).replace(QRegExp("/[^/]*$"), ""), READONLY_HIDDEN_PROP_FLAGS); + result = evaluateInClosure(closure, { sourceCode, modulePath }); } + maybeEmitUncaughtException(__FUNCTION__); return result; } // CommonJS/Node.js like require/module support QScriptValue ScriptEngine::require(const QString& moduleId) { qCDebug(scriptengine_module) << "ScriptEngine::require(" << moduleId.left(MAX_DEBUG_VALUE_LENGTH) << ")"; - if (QThread::currentThread() != thread()) { - qCDebug(scriptengine_module) << moduleId << " threads mismatch"; - return nullValue(); + if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) { + return unboundNullValue(); } - // serialize require calls so the ordering/caching works correctly across multiple Entities - // note: this is a Recursive mutex so nested require's should not affected - QMutexLocker locker(&_requireLock); - ExceptionEmitter tryCatcher(this, __FUNCTION__); - // _requireDepth++; - // QObject autoDecrement;connect(&autoDecrement, &QObject::destroyed, this, [this](){ _requireDepth--; }); - auto jsRequire = globalObject().property("Script").property("require"); auto cacheMeta = jsRequire.data(); auto cache = jsRequire.property("cache"); auto parent = currentModule(); - auto throwModuleError = [this, &cache](const QString& modulePath, const QScriptValue& error) { + auto throwModuleError = [&](const QString& modulePath, const QScriptValue& error) { cache.setProperty(modulePath, nullValue()); if (!error.isNull()) { #ifdef DEBUG_JS_MODULES qCWarning(scriptengine_module) << "throwing module error:" << error.toString() << modulePath << error.property("stack").toString(); #endif - currentContext()->throwValue(makeError(error)); + raiseException(error); } - return nullValue(); + maybeEmitUncaughtException("module"); + return unboundNullValue(); }; // start by resolving the moduleId into a fully-qualified path/URL QString modulePath = _requireResolve(moduleId); - if (modulePath.isNull() || tryCatcher.hasPending()) { + if (modulePath.isNull() || hasUncaughtException()) { // the resolver already threw an exception -- bail early - return nullValue(); + maybeEmitUncaughtException(__FUNCTION__); + return unboundNullValue(); } // check the resolved path against the cache @@ -1656,6 +1648,7 @@ QScriptValue ScriptEngine::require(const QString& moduleId) { qCDebug(scriptengine_module) << QString("require - using cached module '%1' for '%2' (loaded: %3)") .arg(modulePath).arg(moduleId).arg(module.property("loaded").toString()); registerModuleWithParent(module, parent); + maybeEmitUncaughtException(__FUNCTION__); return exports; } @@ -1705,7 +1698,7 @@ QScriptValue ScriptEngine::require(const QString& moduleId) { #ifdef DEBUG_JS _debugDump("this", thisObject); #endif - _applyUserOptions(module, thisObject); + applyUserOptions(module, thisObject); } } @@ -1725,11 +1718,15 @@ QScriptValue ScriptEngine::require(const QString& moduleId) { qCDebug(scriptengine_module) << "//ScriptEngine::require(" << moduleId << ")"; + maybeEmitUncaughtException(__FUNCTION__); return module.property("exports"); } // User-configurable override options -void ScriptEngine::_applyUserOptions(QScriptValue& module, QScriptValue& options) { +void ScriptEngine::applyUserOptions(QScriptValue& module, QScriptValue& options) { + if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) { + return; + } if (!options.isValid()) { return; } @@ -1770,6 +1767,7 @@ void ScriptEngine::_applyUserOptions(QScriptValue& module, QScriptValue& options qCDebug(scriptengine_module) << "module.closure['global'] = options.global"; } } + maybeEmitUncaughtException(__FUNCTION__); } // If a callback is specified, the included files will be loaded asynchronously and the callback will be called @@ -1777,6 +1775,9 @@ void ScriptEngine::_applyUserOptions(QScriptValue& module, QScriptValue& options // If no callback is specified, the included files will be loaded synchronously and will block execution until // all of the files have finished loading. void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callback) { + if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) { + return; + } if (DependencyManager::get()->isStopped()) { scriptWarningMessage("Script.include() while shutting down is ignored... includeFiles:" + includeFiles.join(",") + "parent script:" + getFilename()); @@ -1886,6 +1887,9 @@ void ScriptEngine::include(const QString& includeFile, QScriptValue callback) { // as a stand-alone script. To accomplish this, the ScriptEngine class just emits a signal which // the Application or other context will connect to in order to know to actually load the script void ScriptEngine::load(const QString& loadFile) { + if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) { + return; + } if (DependencyManager::get()->isStopped()) { scriptWarningMessage("Script.load() while shutting down is ignored... loadFile:" + loadFile + "parent script:" + getFilename()); @@ -1955,6 +1959,52 @@ void ScriptEngine::updateEntityScriptStatus(const EntityItemID& entityID, const emit entityScriptDetailsUpdated(); } +QVariant ScriptEngine::cloneEntityScriptDetails(const EntityItemID& entityID) { + static const QVariant NULL_VARIANT { qVariantFromValue((QObject*)nullptr) }; + QVariantMap map; + if (entityID.isNull()) { + // TODO: find better way to report JS Error across thread/process boundaries + map["isError"] = true; + map["errorInfo"] = "Error: getEntityScriptDetails -- invalid entityID"; + } else { +#ifdef DEBUG_ENTITY_STATES + qDebug() << "cloneEntityScriptDetails" << entityID << QThread::currentThread(); +#endif + EntityScriptDetails scriptDetails; + if (getEntityScriptDetails(entityID, scriptDetails)) { +#ifdef DEBUG_ENTITY_STATES + qDebug() << "gotEntityScriptDetails" << scriptDetails.status << QThread::currentThread(); +#endif + map["isRunning"] = isEntityScriptRunning(entityID); + map["status"] = EntityScriptStatus_::valueToKey(scriptDetails.status).toLower(); + map["errorInfo"] = scriptDetails.errorInfo; + map["entityID"] = entityID.toString(); +#ifdef DEBUG_ENTITY_STATES + { + auto debug = QVariantMap(); + debug["script"] = scriptDetails.scriptText; + debug["scriptObject"] = scriptDetails.scriptObject.toVariant(); + debug["lastModified"] = (qlonglong)scriptDetails.lastModified; + debug["sandboxURL"] = scriptDetails.definingSandboxURL; + map["debug"] = debug; + } +#endif + } else { +#ifdef DEBUG_ENTITY_STATES + qDebug() << "!gotEntityScriptDetails" << QThread::currentThread(); +#endif + map["isError"] = true; + map["errorInfo"] = "Entity script details unavailable"; + map["entityID"] = entityID.toString(); + } + } + return map; +} + +QFuture ScriptEngine::getLocalEntityScriptDetails(const EntityItemID& entityID) { + return QtConcurrent::run(this, &ScriptEngine::cloneEntityScriptDetails, entityID); +} + bool ScriptEngine::getEntityScriptDetails(const EntityItemID& entityID, EntityScriptDetails &details) const { auto it = _entityScripts.constFind(entityID); if (it == _entityScripts.constEnd()) { @@ -2093,10 +2143,10 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& auto scriptCache = DependencyManager::get(); // note: see EntityTreeRenderer.cpp for shared pointer lifecycle management - QWeakPointer weakRef(sharedFromThis()); + QWeakPointer weakRef(sharedFromThis()); scriptCache->getScriptContents(entityScript, [this, weakRef, entityScript, entityID](const QString& url, const QString& contents, bool isURL, bool success, const QString& status) { - QSharedPointer strongRef(weakRef); + QSharedPointer strongRef(weakRef); if (!strongRef) { qCWarning(scriptengine) << "loadEntityScript.contentAvailable -- ScriptEngine was deleted during getScriptContents!!"; return; @@ -2215,13 +2265,12 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co timeout.setSingleShot(true); timeout.start(SANDBOX_TIMEOUT); connect(&timeout, &QTimer::timeout, [&sandbox, SANDBOX_TIMEOUT, scriptOrURL]{ - auto context = sandbox.currentContext(); - if (context) { qCDebug(scriptengine) << "ScriptEngine::entityScriptContentAvailable timeout(" << scriptOrURL << ")"; // Guard against infinite loops and non-performant code - context->throwError(QString("Timed out (entity constructors are limited to %1ms)").arg(SANDBOX_TIMEOUT)); - } + sandbox.raiseException( + sandbox.makeError(QString("Timed out (entity constructors are limited to %1ms)").arg(SANDBOX_TIMEOUT)) + ); }); testConstructor = sandbox.evaluate(program); @@ -2237,7 +2286,7 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co if (exception.isError()) { // create a local copy using makeError to decouple from the sandbox engine exception = makeError(exception); - setError(formatException(exception), EntityScriptStatus::ERROR_RUNNING_SCRIPT); + setError(formatException(exception, _enableExtendedJSExceptions.get()), EntityScriptStatus::ERROR_RUNNING_SCRIPT); emit unhandledException(exception); return; } @@ -2288,7 +2337,7 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co if (entityScriptObject.isError()) { auto exception = entityScriptObject; - setError(formatException(exception), EntityScriptStatus::ERROR_RUNNING_SCRIPT); + setError(formatException(exception, _enableExtendedJSExceptions.get()), EntityScriptStatus::ERROR_RUNNING_SCRIPT); emit unhandledException(exception); return; } @@ -2420,10 +2469,7 @@ void ScriptEngine::doWithEnvironment(const EntityItemID& entityID, const QUrl& s #else operation(); #endif - if (!isEvaluating() && hasUncaughtException()) { - emit unhandledException(cloneUncaughtException(!entityID.isNull() ? entityID.toString() : __FUNCTION__)); - clearExceptions(); - } + maybeEmitUncaughtException(!entityID.isNull() ? entityID.toString() : __FUNCTION__); currentEntityIdentifier = oldIdentifier; currentSandboxURL = oldSandboxURL; } diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index f64dec44c7..ef6f3b6896 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -79,7 +79,7 @@ public: QUrl definingSandboxURL { QUrl("about:EntityScript") }; }; -class ScriptEngine : public BaseScriptEngine, public EntitiesScriptEngineProvider, public QEnableSharedFromThis { +class ScriptEngine : public BaseScriptEngine, public EntitiesScriptEngineProvider { Q_OBJECT Q_PROPERTY(QString context READ getContext) public: @@ -138,6 +138,8 @@ public: /// evaluate some code in the context of the ScriptEngine and return the result Q_INVOKABLE QScriptValue evaluate(const QString& program, const QString& fileName, int lineNumber = 1); // this is also used by the script tool widget + Q_INVOKABLE QScriptValue evaluateInClosure(const QScriptValue& locals, const QScriptProgram& program); + /// if the script engine is not already running, this will download the URL and start the process of seting it up /// to run... NOTE - this is used by Application currently to load the url. We don't really want it to be exposed /// to scripts. we may not need this to be invokable @@ -181,6 +183,8 @@ public: Q_INVOKABLE bool isEntityScriptRunning(const EntityItemID& entityID) { return _entityScripts.contains(entityID) && _entityScripts[entityID].status == EntityScriptStatus::RUNNING; } + QVariant cloneEntityScriptDetails(const EntityItemID& entityID); + QFuture getLocalEntityScriptDetails(const EntityItemID& entityID) override; Q_INVOKABLE void loadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload); Q_INVOKABLE void unloadEntityScript(const EntityItemID& entityID); // will call unload method Q_INVOKABLE void unloadAllEntityScripts(); @@ -286,7 +290,6 @@ protected: QHash _entityScripts; QHash _occupiedScriptURLs; QList _deferredEntityLoads; - QMutex _requireLock { QMutex::Recursive }; bool _isThreaded { false }; QScriptEngineDebugger* _debugger { nullptr }; @@ -312,8 +315,12 @@ protected: std::chrono::microseconds _totalTimerExecution { 0 }; static const QString _SETTINGS_ENABLE_EXTENDED_MODULE_COMPAT; + static const QString _SETTINGS_ENABLE_EXTENDED_EXCEPTIONS; + Setting::Handle _enableExtendedModuleCompatbility { _SETTINGS_ENABLE_EXTENDED_MODULE_COMPAT, false }; - void _applyUserOptions(QScriptValue& module, QScriptValue& options); + Setting::Handle _enableExtendedJSExceptions { _SETTINGS_ENABLE_EXTENDED_EXCEPTIONS, true }; + + void applyUserOptions(QScriptValue& module, QScriptValue& options); }; #endif // hifi_ScriptEngine_h diff --git a/libraries/shared/src/BaseScriptEngine.cpp b/libraries/shared/src/BaseScriptEngine.cpp index 16308c0650..d803e85ed6 100644 --- a/libraries/shared/src/BaseScriptEngine.cpp +++ b/libraries/shared/src/BaseScriptEngine.cpp @@ -10,6 +10,7 @@ // #include "BaseScriptEngine.h" +#include "SharedLogging.h" #include #include @@ -18,18 +19,26 @@ #include #include -#include "ScriptEngineLogging.h" #include "Profile.h" -const QString BaseScriptEngine::_SETTINGS_ENABLE_EXTENDED_EXCEPTIONS { - "com.highfidelity.experimental.enableExtendedJSExceptions" -}; - const QString BaseScriptEngine::SCRIPT_EXCEPTION_FORMAT { "[%0] %1 in %2:%3" }; const QString BaseScriptEngine::SCRIPT_BACKTRACE_SEP { "\n " }; +bool BaseScriptEngine::IS_THREADSAFE_INVOCATION(const QThread *thread, const QString& method) { + if (QThread::currentThread() == thread) { + return true; + } + qCCritical(shared) << QString("Scripting::%1 @ %2 -- ignoring thread-unsafe call from %3") + .arg(method).arg(thread ? thread->objectName() : "(!thread)").arg(QThread::currentThread()->objectName()); + qCDebug(shared) << "(please resolve on the calling side by using invokeMethod, executeOnScriptThread, etc.)"; + return false; +} + // engine-aware JS Error copier and factory QScriptValue BaseScriptEngine::makeError(const QScriptValue& _other, const QString& type) { + if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) { + return unboundNullValue(); + } auto other = _other; if (other.isString()) { other = newObject(); @@ -41,7 +50,7 @@ QScriptValue BaseScriptEngine::makeError(const QScriptValue& _other, const QStri } if (!proto.isFunction()) { #ifdef DEBUG_JS_EXCEPTIONS - qCDebug(scriptengine) << "BaseScriptEngine::makeError -- couldn't find constructor for" << type << " -- using Error instead"; + qCDebug(shared) << "BaseScriptEngine::makeError -- couldn't find constructor for" << type << " -- using Error instead"; #endif proto = globalObject().property("Error"); } @@ -64,6 +73,9 @@ QScriptValue BaseScriptEngine::makeError(const QScriptValue& _other, const QStri // check syntax and when there are issues returns an actual "SyntaxError" with the details QScriptValue BaseScriptEngine::lintScript(const QString& sourceCode, const QString& fileName, const int lineNumber) { + if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) { + return unboundNullValue(); + } const auto syntaxCheck = checkSyntax(sourceCode); if (syntaxCheck.state() != QScriptSyntaxCheckResult::Valid) { auto err = globalObject().property("SyntaxError") @@ -82,13 +94,16 @@ QScriptValue BaseScriptEngine::lintScript(const QString& sourceCode, const QStri } return err; } - return undefinedValue(); + return QScriptValue(); } // this pulls from the best available information to create a detailed snapshot of the current exception QScriptValue BaseScriptEngine::cloneUncaughtException(const QString& extraDetail) { + if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) { + return unboundNullValue(); + } if (!hasUncaughtException()) { - return QScriptValue(); + return unboundNullValue(); } auto exception = uncaughtException(); // ensure the error object is engine-local @@ -144,7 +159,10 @@ QScriptValue BaseScriptEngine::cloneUncaughtException(const QString& extraDetail return err; } -QString BaseScriptEngine::formatException(const QScriptValue& exception) { +QString BaseScriptEngine::formatException(const QScriptValue& exception, bool includeExtendedDetails) { + if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) { + return QString(); + } QString note { "UncaughtException" }; QString result; @@ -156,8 +174,8 @@ QString BaseScriptEngine::formatException(const QScriptValue& exception) { const auto lineNumber = exception.property("lineNumber").toString(); const auto stacktrace = exception.property("stack").toString(); - if (_enableExtendedJSExceptions.get()) { - // This setting toggles display of the hints now being added during the loading process. + if (includeExtendedDetails) { + // Display additional exception / troubleshooting hints that can be added via the custom Error .detail property // Example difference: // [UncaughtExceptions] Error: Can't find variable: foobar in atp:/myentity.js\n... // [UncaughtException (construct {1eb5d3fa-23b1-411c-af83-163af7220e3f})] Error: Can't find variable: foobar in atp:/myentity.js\n... @@ -173,14 +191,39 @@ QString BaseScriptEngine::formatException(const QScriptValue& exception) { return result; } +bool BaseScriptEngine::raiseException(const QScriptValue& exception) { + if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) { + return false; + } + if (currentContext()) { + // we have an active context / JS stack frame so throw the exception per usual + currentContext()->throwValue(makeError(exception)); + return true; + } else { + // we are within a pure C++ stack frame (ie: being called directly by other C++ code) + // in this case no context information is available so just emit the exception for reporting + emit unhandledException(makeError(exception)); + } + return false; +} + +bool BaseScriptEngine::maybeEmitUncaughtException(const QString& debugHint) { + if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) { + return false; + } + if (!isEvaluating() && hasUncaughtException()) { + emit unhandledException(cloneUncaughtException(debugHint)); + clearExceptions(); + return true; + } + return false; +} + QScriptValue BaseScriptEngine::evaluateInClosure(const QScriptValue& closure, const QScriptProgram& program) { PROFILE_RANGE(script, "evaluateInClosure"); - if (QThread::currentThread() != thread()) { - qCCritical(scriptengine) << "*** CRITICAL *** ScriptEngine::evaluateInClosure() is meant to be called from engine thread only."; - // note: a recursive mutex might be needed around below code if this method ever becomes Q_INVOKABLE - return QScriptValue(); + if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) { + return unboundNullValue(); } - const auto fileName = program.fileName(); const auto shortName = QUrl(fileName).fileName(); @@ -189,7 +232,7 @@ QScriptValue BaseScriptEngine::evaluateInClosure(const QScriptValue& closure, co auto global = closure.property("global"); if (global.isObject()) { #ifdef DEBUG_JS - qCDebug(scriptengine) << " setting global = closure.global" << shortName; + qCDebug(shared) << " setting global = closure.global" << shortName; #endif oldGlobal = globalObject(); setGlobalObject(global); @@ -200,34 +243,34 @@ QScriptValue BaseScriptEngine::evaluateInClosure(const QScriptValue& closure, co auto thiz = closure.property("this"); if (thiz.isObject()) { #ifdef DEBUG_JS - qCDebug(scriptengine) << " setting this = closure.this" << shortName; + qCDebug(shared) << " setting this = closure.this" << shortName; #endif context->setThisObject(thiz); } context->pushScope(closure); #ifdef DEBUG_JS - qCDebug(scriptengine) << QString("[%1] evaluateInClosure %2").arg(isEvaluating()).arg(shortName); + qCDebug(shared) << QString("[%1] evaluateInClosure %2").arg(isEvaluating()).arg(shortName); #endif { result = BaseScriptEngine::evaluate(program); if (hasUncaughtException()) { auto err = cloneUncaughtException(__FUNCTION__); #ifdef DEBUG_JS_EXCEPTIONS - qCWarning(scriptengine) << __FUNCTION__ << "---------- hasCaught:" << err.toString() << result.toString(); + qCWarning(shared) << __FUNCTION__ << "---------- hasCaught:" << err.toString() << result.toString(); err.setProperty("_result", result); #endif result = err; } } #ifdef DEBUG_JS - qCDebug(scriptengine) << QString("[%1] //evaluateInClosure %2").arg(isEvaluating()).arg(shortName); + qCDebug(shared) << QString("[%1] //evaluateInClosure %2").arg(isEvaluating()).arg(shortName); #endif popContext(); if (oldGlobal.isValid()) { #ifdef DEBUG_JS - qCDebug(scriptengine) << " restoring global" << shortName; + qCDebug(shared) << " restoring global" << shortName; #endif setGlobalObject(oldGlobal); } @@ -236,7 +279,6 @@ QScriptValue BaseScriptEngine::evaluateInClosure(const QScriptValue& closure, co } // Lambda - QScriptValue BaseScriptEngine::newLambdaFunction(std::function operation, const QScriptValue& data, const QScriptEngine::ValueOwnership& ownership) { auto lambda = new Lambda(this, operation, data); auto object = newQObject(lambda, ownership); @@ -262,26 +304,57 @@ Lambda::Lambda(QScriptEngine *engine, std::functionthread(), __FUNCTION__)) { + return BaseScriptEngine::unboundNullValue(); + } return operation(engine->currentContext(), engine); } +QScriptValue makeScopedHandlerObject(QScriptValue scopeOrCallback, QScriptValue methodOrName) { + auto engine = scopeOrCallback.engine(); + if (!engine) { + return scopeOrCallback; + } + auto scope = QScriptValue(); + auto callback = scopeOrCallback; + if (scopeOrCallback.isObject()) { + if (methodOrName.isString()) { + scope = scopeOrCallback; + callback = scope.property(methodOrName.toString()); + } else if (methodOrName.isFunction()) { + scope = scopeOrCallback; + callback = methodOrName; + } + } + auto handler = engine->newObject(); + handler.setProperty("scope", scope); + handler.setProperty("callback", callback); + return handler; +} + +QScriptValue callScopedHandlerObject(QScriptValue handler, QScriptValue err, QScriptValue result) { + return handler.property("callback").call(handler.property("scope"), QScriptValueList({ err, result })); +} + #ifdef DEBUG_JS void BaseScriptEngine::_debugDump(const QString& header, const QScriptValue& object, const QString& footer) { + if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) { + return; + } if (!header.isEmpty()) { - qCDebug(scriptengine) << header; + qCDebug(shared) << header; } if (!object.isObject()) { - qCDebug(scriptengine) << "(!isObject)" << object.toVariant().toString() << object.toString(); + qCDebug(shared) << "(!isObject)" << object.toVariant().toString() << object.toString(); return; } QScriptValueIterator it(object); while (it.hasNext()) { it.next(); - qCDebug(scriptengine) << it.name() << ":" << it.value().toString(); + qCDebug(shared) << it.name() << ":" << it.value().toString(); } if (!footer.isEmpty()) { - qCDebug(scriptengine) << footer; + qCDebug(shared) << footer; } } #endif - diff --git a/libraries/shared/src/BaseScriptEngine.h b/libraries/shared/src/BaseScriptEngine.h index 27a6eff33d..138e46fafa 100644 --- a/libraries/shared/src/BaseScriptEngine.h +++ b/libraries/shared/src/BaseScriptEngine.h @@ -16,38 +16,61 @@ #include #include -#include "SettingHandle.h" - // common base class for extending QScriptEngine itself -class BaseScriptEngine : public QScriptEngine { +class BaseScriptEngine : public QScriptEngine, public QEnableSharedFromThis { Q_OBJECT public: static const QString SCRIPT_EXCEPTION_FORMAT; static const QString SCRIPT_BACKTRACE_SEP; - BaseScriptEngine() {} + // threadsafe "unbound" version of QScriptEngine::nullValue() + static const QScriptValue unboundNullValue() { return QScriptValue(0, QScriptValue::NullValue); } - Q_INVOKABLE QScriptValue evaluateInClosure(const QScriptValue& locals, const QScriptProgram& program); + BaseScriptEngine() {} Q_INVOKABLE QScriptValue lintScript(const QString& sourceCode, const QString& fileName, const int lineNumber = 1); Q_INVOKABLE QScriptValue makeError(const QScriptValue& other = QScriptValue(), const QString& type = "Error"); - Q_INVOKABLE QString formatException(const QScriptValue& exception); - QScriptValue cloneUncaughtException(const QString& detail = QString()); + Q_INVOKABLE QString formatException(const QScriptValue& exception, bool includeExtendedDetails); + QScriptValue cloneUncaughtException(const QString& detail = QString()); + QScriptValue evaluateInClosure(const QScriptValue& locals, const QScriptProgram& program); + + // if there is a pending exception and we are at the top level (non-recursive) stack frame, this emits and resets it + bool maybeEmitUncaughtException(const QString& debugHint = QString()); + + // if the currentContext() is valid then throw the passed exception; otherwise, immediately emit it. + // note: this is used in cases where C++ code might call into JS API methods directly + bool raiseException(const QScriptValue& exception); + + // helper to detect and log warnings when other code invokes QScriptEngine/BaseScriptEngine in thread-unsafe ways + static bool IS_THREADSAFE_INVOCATION(const QThread *thread, const QString& method); signals: void unhandledException(const QScriptValue& exception); protected: - void _emitUnhandledException(const QScriptValue& exception); + // like `newFunction`, but allows mapping inline C++ lambdas with captures as callable QScriptValues + // even though the context/engine parameters are redundant in most cases, the function signature matches `newFunction` + // anyway so that newLambdaFunction can be used to rapidly prototype / test utility APIs and then if becoming + // permanent more easily promoted into regular static newFunction scenarios. QScriptValue newLambdaFunction(std::function operation, const QScriptValue& data = QScriptValue(), const QScriptEngine::ValueOwnership& ownership = QScriptEngine::AutoOwnership); - static const QString _SETTINGS_ENABLE_EXTENDED_EXCEPTIONS; - Setting::Handle _enableExtendedJSExceptions { _SETTINGS_ENABLE_EXTENDED_EXCEPTIONS, true }; #ifdef DEBUG_JS static void _debugDump(const QString& header, const QScriptValue& object, const QString& footer = QString()); #endif }; +// Standardized CPS callback helpers (see: http://fredkschott.com/post/2014/03/understanding-error-first-callbacks-in-node-js/) +// These two helpers allow async JS APIs that use a callback parameter to be more friendly to scripters by accepting thisObject +// context and adopting a consistent and intuitable callback signature: +// function callback(err, result) { if (err) { ... } else { /* do stuff with result */ } } +// +// To use, first pass the user-specified callback args in the same order used with optionally-scoped Qt signal connections: +// auto handler = makeScopedHandlerObject(scopeOrCallback, optionalMethodOrName); +// And then invoke the scoped handler later per CPS conventions: +// auto result = callScopedHandlerObject(handler, err, result); +QScriptValue makeScopedHandlerObject(QScriptValue scopeOrCallback, QScriptValue methodOrName); +QScriptValue callScopedHandlerObject(QScriptValue handler, QScriptValue err, QScriptValue result); + // Lambda helps create callable QScriptValues out of std::functions: // (just meant for use from within the script engine itself) class Lambda : public QObject { From 143b67e47dafd7cd1a042ec5ed6b43dd30b665b4 Mon Sep 17 00:00:00 2001 From: humbletim Date: Wed, 1 Mar 2017 09:20:53 -0500 Subject: [PATCH 07/22] Add require/module unit tests --- .../developer/libraries/jasmine/hifi-boot.js | 11 +- .../tests/unit_tests/moduleTests/cycles/a.js | 9 + .../tests/unit_tests/moduleTests/cycles/b.js | 9 + .../unit_tests/moduleTests/cycles/main.js | 13 + .../entity/entityConstructorAPIException.js | 12 + .../entity/entityConstructorModule.js | 21 ++ .../entity/entityConstructorNested.js | 13 + .../entity/entityConstructorNested2.js | 24 ++ .../entityConstructorRequireException.js | 9 + .../entity/entityPreloadAPIError.js | 12 + .../entity/entityPreloadRequire.js | 10 + .../tests/unit_tests/moduleTests/example.json | 9 + .../moduleTests/exceptions/exception.js | 3 + .../exceptions/exceptionInFunction.js | 37 ++ .../tests/unit_tests/moduleUnitTests.js | 317 ++++++++++++++++++ .../developer/tests/unit_tests/package.json | 6 + 16 files changed, 513 insertions(+), 2 deletions(-) create mode 100644 scripts/developer/tests/unit_tests/moduleTests/cycles/a.js create mode 100644 scripts/developer/tests/unit_tests/moduleTests/cycles/b.js create mode 100644 scripts/developer/tests/unit_tests/moduleTests/cycles/main.js create mode 100644 scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorAPIException.js create mode 100644 scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorModule.js create mode 100644 scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorNested.js create mode 100644 scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorNested2.js create mode 100644 scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorRequireException.js create mode 100644 scripts/developer/tests/unit_tests/moduleTests/entity/entityPreloadAPIError.js create mode 100644 scripts/developer/tests/unit_tests/moduleTests/entity/entityPreloadRequire.js create mode 100644 scripts/developer/tests/unit_tests/moduleTests/example.json create mode 100644 scripts/developer/tests/unit_tests/moduleTests/exceptions/exception.js create mode 100644 scripts/developer/tests/unit_tests/moduleTests/exceptions/exceptionInFunction.js create mode 100644 scripts/developer/tests/unit_tests/moduleUnitTests.js create mode 100644 scripts/developer/tests/unit_tests/package.json diff --git a/scripts/developer/libraries/jasmine/hifi-boot.js b/scripts/developer/libraries/jasmine/hifi-boot.js index f490a3618f..8757550ae8 100644 --- a/scripts/developer/libraries/jasmine/hifi-boot.js +++ b/scripts/developer/libraries/jasmine/hifi-boot.js @@ -6,7 +6,7 @@ var lastSpecStartTime; function ConsoleReporter(options) { var startTime = new Date().getTime(); - var errorCount = 0; + var errorCount = 0, pending = []; this.jasmineStarted = function (obj) { print('Jasmine started with ' + obj.totalSpecsDefined + ' tests.'); }; @@ -15,11 +15,14 @@ var endTime = new Date().getTime(); print('
'); if (errorCount === 0) { - print ('All tests passed!'); + print ('All enabled tests passed!'); } else { print('Tests completed with ' + errorCount + ' ' + ERROR + '.'); } + if (pending.length) + print ('disabled:
   '+ + pending.join('
   ')+''); print('Tests completed in ' + (endTime - startTime) + 'ms.'); }; this.suiteStarted = function(obj) { @@ -32,6 +35,10 @@ lastSpecStartTime = new Date().getTime(); }; this.specDone = function(obj) { + if (obj.status === 'pending') { + pending.push(obj.fullName); + return print('...(pending ' + obj.fullName +')'); + } var specEndTime = new Date().getTime(); var symbol = obj.status === PASSED ? '' + CHECKMARK + '' : diff --git a/scripts/developer/tests/unit_tests/moduleTests/cycles/a.js b/scripts/developer/tests/unit_tests/moduleTests/cycles/a.js new file mode 100644 index 0000000000..7934180da7 --- /dev/null +++ b/scripts/developer/tests/unit_tests/moduleTests/cycles/a.js @@ -0,0 +1,9 @@ +var a = exports; +a.done = false; +var b = require('./b.js'); +a.done = true; +a.name = 'a'; +a['a.done?'] = a.done; +a['b.done?'] = b.done; + +print('from a.js a.done =', a.done, '/ b.done =', b.done); diff --git a/scripts/developer/tests/unit_tests/moduleTests/cycles/b.js b/scripts/developer/tests/unit_tests/moduleTests/cycles/b.js new file mode 100644 index 0000000000..285f176597 --- /dev/null +++ b/scripts/developer/tests/unit_tests/moduleTests/cycles/b.js @@ -0,0 +1,9 @@ +var b = exports; +b.done = false; +var a = require('./a.js'); +b.done = true; +b.name = 'b'; +b['a.done?'] = a.done; +b['b.done?'] = b.done; + +print('from b.js a.done =', a.done, '/ b.done =', b.done); diff --git a/scripts/developer/tests/unit_tests/moduleTests/cycles/main.js b/scripts/developer/tests/unit_tests/moduleTests/cycles/main.js new file mode 100644 index 0000000000..2e9a878c82 --- /dev/null +++ b/scripts/developer/tests/unit_tests/moduleTests/cycles/main.js @@ -0,0 +1,13 @@ +print('main.js'); +var a = require('./a.js'), + b = require('./b.js'); + +print('from main.js a.done =', a.done, 'and b.done =', b.done); + +module.exports = { + name: 'main', + a: a, + b: b, + 'a.done?': a.done, + 'b.done?': b.done, +}; diff --git a/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorAPIException.js b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorAPIException.js new file mode 100644 index 0000000000..b1bc0e33e4 --- /dev/null +++ b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorAPIException.js @@ -0,0 +1,12 @@ +// test module method exception being thrown within main constructor +(function() { + var apiMethod = Script.require('../exceptions/exceptionInFunction.js'); + print(Script.resolvePath(''), "apiMethod", apiMethod); + // this next line throws from within apiMethod + print(apiMethod()); + return { + preload: function(uuid) { + print("entityConstructorAPIException::preload -- never seen --", uuid, Script.resolvePath('')); + }, + }; +}) diff --git a/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorModule.js b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorModule.js new file mode 100644 index 0000000000..5f0e8a5938 --- /dev/null +++ b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorModule.js @@ -0,0 +1,21 @@ +// test dual-purpose module and standalone Entity script +function MyEntity(filename) { + return { + preload: function(uuid) { + print("entityConstructorModule.js::preload"); + if (typeof module === 'object') { + print("module.filename", module.filename); + print("module.parent.filename", module.parent && module.parent.filename); + } + }, + clickDownOnEntity: function(uuid, evt) { + print("entityConstructorModule.js::clickDownOnEntity"); + }, + }; +} + +try { + module.exports = MyEntity; +} catch(e) {} +print('entityConstructorModule::MyEntity', typeof MyEntity); +(MyEntity) diff --git a/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorNested.js b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorNested.js new file mode 100644 index 0000000000..5a2b8d5974 --- /dev/null +++ b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorNested.js @@ -0,0 +1,13 @@ +// test Entity constructor based on inherited constructor from a module +function constructor() { + print("entityConstructorNested::constructor"); + var MyEntity = Script.require('./entityConstructorModule.js'); + return new MyEntity("-- created from entityConstructorNested --"); +} + +try { + module.exports = constructor; +} catch(e) { + constructor; +} + diff --git a/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorNested2.js b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorNested2.js new file mode 100644 index 0000000000..85a6b977b0 --- /dev/null +++ b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorNested2.js @@ -0,0 +1,24 @@ +// test Entity constructor based on nested, inherited module constructors +function constructor() { + print("entityConstructorNested2::constructor"); + + // inherit from entityConstructorNested + var Entity = Script.require('./entityConstructorNested.js'); + function SubEntity() {} + SubEntity.prototype = new MyEntity('-- created from entityConstructorNested2 --'); + + // create new instance + var entity = new SubEntity(); + // "override" clickDownOnEntity for just this new instance + entity.clickDownOnEntity = function(uuid, evt) { + print("entityConstructorNested2::clickDownOnEntity"); + SubEntity.prototype.clickDownOnEntity.apply(this, arguments); + }; + return entity; +} + +try { + module.exports = constructor; +} catch(e) { + constructor; +} diff --git a/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorRequireException.js b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorRequireException.js new file mode 100644 index 0000000000..269ca8e7f0 --- /dev/null +++ b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorRequireException.js @@ -0,0 +1,9 @@ +// test module-related exception from within "require" evaluation itself +(function() { + var mod = Script.require('../exceptions/exception.js'); + return { + preload: function(uuid) { + print("entityConstructorRequireException::preload (never happens)", uuid, Script.resolvePath('')); + }, + }; +}) diff --git a/scripts/developer/tests/unit_tests/moduleTests/entity/entityPreloadAPIError.js b/scripts/developer/tests/unit_tests/moduleTests/entity/entityPreloadAPIError.js new file mode 100644 index 0000000000..3be0b50d43 --- /dev/null +++ b/scripts/developer/tests/unit_tests/moduleTests/entity/entityPreloadAPIError.js @@ -0,0 +1,12 @@ +// test module method exception being thrown within preload +(function() { + var apiMethod = Script.require('../exceptions/exceptionInFunction.js'); + print(Script.resolvePath(''), "apiMethod", apiMethod); + return { + preload: function(uuid) { + // this next line throws from within apiMethod + print(apiMethod()); + print("entityPreloadAPIException::preload -- never seen --", uuid, Script.resolvePath('')); + }, + }; +}) diff --git a/scripts/developer/tests/unit_tests/moduleTests/entity/entityPreloadRequire.js b/scripts/developer/tests/unit_tests/moduleTests/entity/entityPreloadRequire.js new file mode 100644 index 0000000000..fc70838c80 --- /dev/null +++ b/scripts/developer/tests/unit_tests/moduleTests/entity/entityPreloadRequire.js @@ -0,0 +1,10 @@ +// test requiring a module from within preload +(function constructor() { + return { + preload: function(uuid) { + print("entityPreloadRequire::preload"); + var example = Script.require('../example.json'); + print("entityPreloadRequire::example::name", example.name); + }, + }; +}) diff --git a/scripts/developer/tests/unit_tests/moduleTests/example.json b/scripts/developer/tests/unit_tests/moduleTests/example.json new file mode 100644 index 0000000000..42d7fe07da --- /dev/null +++ b/scripts/developer/tests/unit_tests/moduleTests/example.json @@ -0,0 +1,9 @@ +{ + "name": "Example JSON Module", + "last-modified": 1485789862, + "config": { + "title": "My Title", + "width": 800, + "height": 600 + } +} diff --git a/scripts/developer/tests/unit_tests/moduleTests/exceptions/exception.js b/scripts/developer/tests/unit_tests/moduleTests/exceptions/exception.js new file mode 100644 index 0000000000..636ee82f79 --- /dev/null +++ b/scripts/developer/tests/unit_tests/moduleTests/exceptions/exception.js @@ -0,0 +1,3 @@ +module.exports = "n/a"; +throw new Error('exception on line 2'); + diff --git a/scripts/developer/tests/unit_tests/moduleTests/exceptions/exceptionInFunction.js b/scripts/developer/tests/unit_tests/moduleTests/exceptions/exceptionInFunction.js new file mode 100644 index 0000000000..dc2ce3c438 --- /dev/null +++ b/scripts/developer/tests/unit_tests/moduleTests/exceptions/exceptionInFunction.js @@ -0,0 +1,37 @@ +// dummy lines to make sure exception line number is well below parent test script +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// + + +function myfunc() { + throw new Error('exception on line 32 in myfunc'); + return "myfunc"; +} +module.exports = myfunc; +if (Script[module.filename] === 'throw') + myfunc(); diff --git a/scripts/developer/tests/unit_tests/moduleUnitTests.js b/scripts/developer/tests/unit_tests/moduleUnitTests.js new file mode 100644 index 0000000000..c1c20d6980 --- /dev/null +++ b/scripts/developer/tests/unit_tests/moduleUnitTests.js @@ -0,0 +1,317 @@ +/* eslint-env jasmine */ + +var isNode = instrument_testrunner(); + +var NETWORK_describe = xdescribe, + INTERFACE_describe = !isNode ? describe : xdescribe, + NODE_describe = isNode ? describe : xdescribe; + +print("DESCRIBING"); +describe('require', function() { + describe('resolve', function() { + it('should resolve relative filenames', function() { + var expected = Script.resolvePath('./moduleTests/example.json'); + expect(require.resolve('./moduleTests/example.json')).toEqual(expected); + }); + }); + + describe('JSON', function() { + it('should import .json modules', function() { + var example = require('./moduleTests/example.json'); + expect(example.name).toEqual('Example JSON Module'); + }); + INTERFACE_describe('inteface', function() { + NETWORK_describe('network', function() { + //xit('should import #content-type=application/json modules', function() { + // var results = require('https://jsonip.com#content-type=application/json'); + // expect(results.ip).toMatch(/^[.0-9]+$/); + //}); + it('should import content-type: application/json modules', function() { + var scope = { 'content-type': 'application/json' }; + var results = require.call(scope, 'https://jsonip.com'); + expect(results.ip).toMatch(/^[.0-9]+$/); + }); + }); + }); + + }); + + INTERFACE_describe('system', function() { + it('require(id)', function() { + expect(require('vec3')).toEqual(jasmine.any(Function)); + }); + it('require(id).function', function() { + expect(require('vec3')().isValid).toEqual(jasmine.any(Function)); + }); + }); + + describe('exceptions', function() { + it('should reject blank "" module identifiers', function() { + expect(function() { + require.resolve(''); + }).toThrowError(/Cannot find/); + }); + it('should reject excessive identifier sizes', function() { + expect(function() { + require.resolve(new Array(8193).toString()); + }).toThrowError(/Cannot find/); + }); + it('should reject implicitly-relative filenames', function() { + expect(function() { + var mod = require.resolve('example.js'); + }).toThrowError(/Cannot find/); + }); + it('should reject non-existent filenames', function() { + expect(function() { + var mod = require.resolve('./404error.js'); + }).toThrowError(/Cannot find/); + }); + it('should reject identifiers resolving to a directory', function() { + expect(function() { + var mod = require.resolve('.'); + //console.warn('resolved(.)', mod); + }).toThrowError(/Cannot find/); + expect(function() { + var mod = require.resolve('..'); + //console.warn('resolved(..)', mod); + }).toThrowError(/Cannot find/); + expect(function() { + var mod = require.resolve('../'); + //console.warn('resolved(../)', mod); + }).toThrowError(/Cannot find/); + }); + if (typeof MODE !== 'undefined' && MODE !== 'node') { + it('should reject non-system, extensionless identifiers', function() { + expect(function() { + require.resolve('./example'); + }).toThrowError(/Cannot find/); + }); + } + }); + + describe('cache', function() { + it('should cache modules by resolved module id', function() { + var value = new Date; + var example = require('./moduleTests/example.json'); + example['.test'] = value; + var example2 = require('../../tests/unit_tests/moduleTests/example.json'); + expect(example2).toBe(example); + expect(example2['.test']).toBe(example['.test']); + }); + it('should reload cached modules set to null', function() { + var value = new Date; + var example = require('./moduleTests/example.json'); + example['.test'] = value; + require.cache[require.resolve('../../tests/unit_tests/moduleTests/example.json')] = null; + var example2 = require('../../tests/unit_tests/moduleTests/example.json'); + expect(example2).not.toBe(example); + expect(example2['.test']).not.toBe(example['.test']); + }); + it('should reload when module property is deleted', function() { + var value = new Date; + var example = require('./moduleTests/example.json'); + example['.test'] = value; + delete require.cache[require.resolve('../../tests/unit_tests/moduleTests/example.json')]; + var example2 = require('../../tests/unit_tests/moduleTests/example.json'); + expect(example2).not.toBe(example); + expect(example2['.test']).not.toBe(example['.test']); + }); + }); + + describe('cyclic dependencies', function() { + describe('should allow lazy-ref cyclic module resolution', function() { + const MODULE_PATH = './moduleTests/cycles/main.js'; + var main; + beforeEach(function() { + try { this._print = print; } catch(e) {} + // for this test print is no-op'd so it doesn't disrupt the reporter output + //console = typeof console === 'object' ? console : { log: function() {} }; + print = function() {}; + Script.resetModuleCache(); + }); + afterEach(function() { + print = this._print; + }); + it('main requirable', function() { + main = require(MODULE_PATH); + expect(main).toEqual(jasmine.any(Object)); + }); + it('main with both a and b', function() { + expect(main.a['b.done?']).toBe(true); + expect(main.b['a.done?']).toBe(false); + }); + it('a.done?', function() { + expect(main['a.done?']).toBe(true); + }); + it('b.done?', function() { + expect(main['b.done?']).toBe(true); + }); + }); + }); + + describe('JS', function() { + it('should throw catchable local file errors', function() { + expect(function() { + require('file:///dev/null/non-existent-file.js'); + }).toThrowError(/path not found|Cannot find.*non-existent-file/); + }); + it('should throw catchable invalid id errors', function() { + expect(function() { + require(new Array(4096 * 2).toString()); + }).toThrowError(/invalid.*size|Cannot find.*,{30}/); + }); + it('should throw catchable unresolved id errors', function() { + expect(function() { + require('foobar:/baz.js'); + }).toThrowError(/could not resolve|Cannot find.*foobar:/); + }); + + NETWORK_describe('network', function() { + // note: with retries these tests can take up to 60 seconds each to timeout + var timeout = 75 * 1000; + it('should throw catchable host errors', function() { + expect(function() { + var mod = require('http://non.existent.highfidelity.io/moduleUnitTest.js'); + print("mod", Object.keys(mod)); + }).toThrowError(/error retrieving script .ServerUnavailable.|Cannot find.*non.existent/); + }, timeout); + it('should throw catchable network timeouts', function() { + expect(function() { + require('http://ping.highfidelity.io:1024'); + }).toThrowError(/error retrieving script .Timeout.|Cannot find.*ping.highfidelity/); + }, timeout); + }); + }); + + INTERFACE_describe('entity', function() { + var sampleScripts = [ + 'entityConstructorAPIException.js', + 'entityConstructorModule.js', + 'entityConstructorNested2.js', + 'entityConstructorNested.js', + 'entityConstructorRequireException.js', + 'entityPreloadAPIError.js', + 'entityPreloadRequire.js', + ].filter(Boolean).map(function(id) { return Script.require.resolve('./moduleTests/entity/'+id); }); + + var uuids = []; + + for(var i=0; i < sampleScripts.length; i++) { + (function(i) { + var script = sampleScripts[ i % sampleScripts.length ]; + var shortname = '['+i+'] ' + script.split('/').pop(); + var position = MyAvatar.position; + position.y -= i/2; + it(shortname, function(done) { + var uuid = Entities.addEntity({ + text: shortname, + description: Script.resolvePath('').split('/').pop(), + type: 'Text', + position: position, + rotation: MyAvatar.orientation, + script: script, + scriptTimestamp: +new Date, + lifetime: 20, + lineHeight: 1/8, + dimensions: { x: 2, y: .5, z: .01 }, + backgroundColor: { red: 0, green: 0, blue: 0 }, + color: { red: 0xff, green: 0xff, blue: 0xff }, + }, !Entities.serversExist() || !Entities.canRezTmp()); + uuids.push(uuid); + var ii = Script.setInterval(function() { + Entities.queryPropertyMetadata(uuid, "script", function(err, result) { + if (err) { + throw new Error(err); + } + if (result.success) { + clearInterval(ii); + if (/Exception/.test(script)) + expect(result.status).toMatch(/^error_(loading|running)_script$/); + else + expect(result.status).toEqual("running"); + done(); + } else { + print('!result.success', JSON.stringify(result)); + } + }); + }, 100); + Script.setTimeout(function() { + Script.clearInterval(ii); + }, 4900); + }, 5000 /* timeout */); + })(i); + } + Script.scriptEnding.connect(function() { + uuids.forEach(function(uuid) { Entities.deleteEntity(uuid); }); + }); + }); +}); + +function run() {} +function instrument_testrunner() { + var isNode = typeof process === 'object' && process.title === 'node'; + if (isNode) { + // for consistency this still uses the same local jasmine.js library + var jasmineRequire = require('../../libraries/jasmine/jasmine.js'); + var jasmine = jasmineRequire.core(jasmineRequire); + var env = jasmine.getEnv(); + var jasmineInterface = jasmineRequire.interface(jasmine, env); + for (var p in jasmineInterface) + global[p] = jasmineInterface[p]; + env.addReporter(new (require('jasmine-console-reporter'))); + // testing mocks + Script = { + resetModuleCache: function() { + module.require.cache = {}; + }, + setTimeout: setTimeout, + clearTimeout: clearTimeout, + resolvePath: function(id) { + // this attempts to accurately emulate how Script.resolvePath works + var trace = {}; Error.captureStackTrace(trace); + var base = trace.stack.split('\n')[2].replace(/^.*[(]|[)].*$/g,'').replace(/:[0-9]+:[0-9]+.*$/,''); + if (!id) + return base; + var rel = base.replace(/[^\/]+$/, id); + console.info('rel', rel); + return require.resolve(rel); + }, + require: function(mod) { + return require(Script.require.resolve(mod)); + } + }; + Script.require.cache = require.cache; + Script.require.resolve = function(mod) { + if (mod === '.' || /^\.\.($|\/)/.test(mod)) + throw new Error("Cannot find module '"+mod+"' (is dir)"); + var path = require.resolve(mod); + //console.info('node-require-reoslved', mod, path); + try { + if (require('fs').lstatSync(path).isDirectory()) { + throw new Error("Cannot find module '"+path+"' (is directory)"); + } + //console.info('!path', path); + } catch(e) { console.info(e) } + return path; + }; + print = console.info.bind(console, '[print]'); + } else { + global = this; + // Interface Test mode + Script.require('../../../system/libraries/utils.js'); + this.jasmineRequire = Script.require('../../libraries/jasmine/jasmine.js'); + Script.require('../../libraries/jasmine/hifi-boot.js') + require = Script.require; + // polyfill console + console = { + log: print, + info: print.bind(this, '[info]'), + warn: print.bind(this, '[warn]'), + error: print.bind(this, '[error]'), + debug: print.bind(this, '[debug]'), + }; + } + run = function() { global.jasmine.getEnv().execute(); }; + return isNode; +} +run(); diff --git a/scripts/developer/tests/unit_tests/package.json b/scripts/developer/tests/unit_tests/package.json new file mode 100644 index 0000000000..91d719b687 --- /dev/null +++ b/scripts/developer/tests/unit_tests/package.json @@ -0,0 +1,6 @@ +{ + "name": "unit_tests", + "devDependencies": { + "jasmine-console-reporter": "^1.2.7" + } +} From fa0d3a18455b7bb8d7a58ec96a0f049c4c0ce26c Mon Sep 17 00:00:00 2001 From: humbletim Date: Wed, 1 Mar 2017 15:19:23 -0500 Subject: [PATCH 08/22] unit test cleanup pass --- .../tests/unit_tests/moduleTests/cycles/a.js | 1 + .../tests/unit_tests/moduleTests/cycles/b.js | 1 + .../unit_tests/moduleTests/cycles/main.js | 4 + .../entity/entityConstructorAPIException.js | 3 +- .../entity/entityConstructorModule.js | 6 +- .../entity/entityConstructorNested.js | 3 +- .../entity/entityConstructorNested2.js | 5 +- .../entityConstructorRequireException.js | 5 +- .../entity/entityPreloadAPIError.js | 3 +- .../entity/entityPreloadRequire.js | 3 +- .../moduleTests/exceptions/exception.js | 1 + .../exceptions/exceptionInFunction.js | 5 +- .../tests/unit_tests/moduleUnitTests.js | 309 ++++++++++-------- 13 files changed, 206 insertions(+), 143 deletions(-) diff --git a/scripts/developer/tests/unit_tests/moduleTests/cycles/a.js b/scripts/developer/tests/unit_tests/moduleTests/cycles/a.js index 7934180da7..265cfaa2df 100644 --- a/scripts/developer/tests/unit_tests/moduleTests/cycles/a.js +++ b/scripts/developer/tests/unit_tests/moduleTests/cycles/a.js @@ -1,3 +1,4 @@ +/* eslint-env node */ var a = exports; a.done = false; var b = require('./b.js'); diff --git a/scripts/developer/tests/unit_tests/moduleTests/cycles/b.js b/scripts/developer/tests/unit_tests/moduleTests/cycles/b.js index 285f176597..c46c872828 100644 --- a/scripts/developer/tests/unit_tests/moduleTests/cycles/b.js +++ b/scripts/developer/tests/unit_tests/moduleTests/cycles/b.js @@ -1,3 +1,4 @@ +/* eslint-env node */ var b = exports; b.done = false; var a = require('./a.js'); diff --git a/scripts/developer/tests/unit_tests/moduleTests/cycles/main.js b/scripts/developer/tests/unit_tests/moduleTests/cycles/main.js index 2e9a878c82..0ec39cd656 100644 --- a/scripts/developer/tests/unit_tests/moduleTests/cycles/main.js +++ b/scripts/developer/tests/unit_tests/moduleTests/cycles/main.js @@ -1,3 +1,7 @@ +/* eslint-env node */ +/* global print */ +/* eslint-disable comma-dangle */ + print('main.js'); var a = require('./a.js'), b = require('./b.js'); diff --git a/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorAPIException.js b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorAPIException.js index b1bc0e33e4..bbe694b578 100644 --- a/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorAPIException.js +++ b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorAPIException.js @@ -1,3 +1,4 @@ +/* eslint-disable comma-dangle */ // test module method exception being thrown within main constructor (function() { var apiMethod = Script.require('../exceptions/exceptionInFunction.js'); @@ -9,4 +10,4 @@ print("entityConstructorAPIException::preload -- never seen --", uuid, Script.resolvePath('')); }, }; -}) +}); diff --git a/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorModule.js b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorModule.js index 5f0e8a5938..a4e8c17ab6 100644 --- a/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorModule.js +++ b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorModule.js @@ -1,3 +1,5 @@ +/* global module */ +/* eslint-disable comma-dangle */ // test dual-purpose module and standalone Entity script function MyEntity(filename) { return { @@ -16,6 +18,6 @@ function MyEntity(filename) { try { module.exports = MyEntity; -} catch(e) {} +} catch (e) {} // eslint-disable-line no-empty print('entityConstructorModule::MyEntity', typeof MyEntity); -(MyEntity) +(MyEntity); diff --git a/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorNested.js b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorNested.js index 5a2b8d5974..a90d979877 100644 --- a/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorNested.js +++ b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorNested.js @@ -1,3 +1,4 @@ +/* global module */ // test Entity constructor based on inherited constructor from a module function constructor() { print("entityConstructorNested::constructor"); @@ -7,7 +8,7 @@ function constructor() { try { module.exports = constructor; -} catch(e) { +} catch (e) { constructor; } diff --git a/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorNested2.js b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorNested2.js index 85a6b977b0..29e0ed65b1 100644 --- a/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorNested2.js +++ b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorNested2.js @@ -1,9 +1,10 @@ +/* global module */ // test Entity constructor based on nested, inherited module constructors function constructor() { print("entityConstructorNested2::constructor"); // inherit from entityConstructorNested - var Entity = Script.require('./entityConstructorNested.js'); + var MyEntity = Script.require('./entityConstructorNested.js'); function SubEntity() {} SubEntity.prototype = new MyEntity('-- created from entityConstructorNested2 --'); @@ -19,6 +20,6 @@ function constructor() { try { module.exports = constructor; -} catch(e) { +} catch (e) { constructor; } diff --git a/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorRequireException.js b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorRequireException.js index 269ca8e7f0..5872bce529 100644 --- a/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorRequireException.js +++ b/scripts/developer/tests/unit_tests/moduleTests/entity/entityConstructorRequireException.js @@ -1,9 +1,10 @@ +/* eslint-disable comma-dangle */ // test module-related exception from within "require" evaluation itself (function() { var mod = Script.require('../exceptions/exception.js'); return { preload: function(uuid) { - print("entityConstructorRequireException::preload (never happens)", uuid, Script.resolvePath('')); + print("entityConstructorRequireException::preload (never happens)", uuid, Script.resolvePath(''), mod); }, }; -}) +}); diff --git a/scripts/developer/tests/unit_tests/moduleTests/entity/entityPreloadAPIError.js b/scripts/developer/tests/unit_tests/moduleTests/entity/entityPreloadAPIError.js index 3be0b50d43..eaee178b0a 100644 --- a/scripts/developer/tests/unit_tests/moduleTests/entity/entityPreloadAPIError.js +++ b/scripts/developer/tests/unit_tests/moduleTests/entity/entityPreloadAPIError.js @@ -1,3 +1,4 @@ +/* eslint-disable comma-dangle */ // test module method exception being thrown within preload (function() { var apiMethod = Script.require('../exceptions/exceptionInFunction.js'); @@ -9,4 +10,4 @@ print("entityPreloadAPIException::preload -- never seen --", uuid, Script.resolvePath('')); }, }; -}) +}); diff --git a/scripts/developer/tests/unit_tests/moduleTests/entity/entityPreloadRequire.js b/scripts/developer/tests/unit_tests/moduleTests/entity/entityPreloadRequire.js index fc70838c80..50dab9fa7c 100644 --- a/scripts/developer/tests/unit_tests/moduleTests/entity/entityPreloadRequire.js +++ b/scripts/developer/tests/unit_tests/moduleTests/entity/entityPreloadRequire.js @@ -1,3 +1,4 @@ +/* eslint-disable comma-dangle */ // test requiring a module from within preload (function constructor() { return { @@ -7,4 +8,4 @@ print("entityPreloadRequire::example::name", example.name); }, }; -}) +}); diff --git a/scripts/developer/tests/unit_tests/moduleTests/exceptions/exception.js b/scripts/developer/tests/unit_tests/moduleTests/exceptions/exception.js index 636ee82f79..8d25d6b7a4 100644 --- a/scripts/developer/tests/unit_tests/moduleTests/exceptions/exception.js +++ b/scripts/developer/tests/unit_tests/moduleTests/exceptions/exception.js @@ -1,3 +1,4 @@ +/* eslint-env node */ module.exports = "n/a"; throw new Error('exception on line 2'); diff --git a/scripts/developer/tests/unit_tests/moduleTests/exceptions/exceptionInFunction.js b/scripts/developer/tests/unit_tests/moduleTests/exceptions/exceptionInFunction.js index dc2ce3c438..69415a0741 100644 --- a/scripts/developer/tests/unit_tests/moduleTests/exceptions/exceptionInFunction.js +++ b/scripts/developer/tests/unit_tests/moduleTests/exceptions/exceptionInFunction.js @@ -1,3 +1,4 @@ +/* eslint-env node */ // dummy lines to make sure exception line number is well below parent test script // // @@ -30,8 +31,8 @@ function myfunc() { throw new Error('exception on line 32 in myfunc'); - return "myfunc"; } module.exports = myfunc; -if (Script[module.filename] === 'throw') +if (Script[module.filename] === 'throw') { myfunc(); +} diff --git a/scripts/developer/tests/unit_tests/moduleUnitTests.js b/scripts/developer/tests/unit_tests/moduleUnitTests.js index c1c20d6980..a9446d1d6e 100644 --- a/scripts/developer/tests/unit_tests/moduleUnitTests.js +++ b/scripts/developer/tests/unit_tests/moduleUnitTests.js @@ -1,18 +1,65 @@ -/* eslint-env jasmine */ +/* eslint-env jasmine, node */ +/* global print:true, Script:true, global:true, require:true */ +/* eslint-disable comma-dangle */ +var isNode = instrumentTestrunner(), + runInterfaceTests = !isNode, + runNetworkTests = true; -var isNode = instrument_testrunner(); +// describe wrappers (note: `xdescribe` indicates a disabled or "pending" jasmine test) +var INTERFACE = { describe: runInterfaceTests ? describe : xdescribe }, + NETWORK = { describe: runNetworkTests ? describe : xdescribe }; -var NETWORK_describe = xdescribe, - INTERFACE_describe = !isNode ? describe : xdescribe, - NODE_describe = isNode ? describe : xdescribe; - -print("DESCRIBING"); describe('require', function() { describe('resolve', function() { it('should resolve relative filenames', function() { var expected = Script.resolvePath('./moduleTests/example.json'); expect(require.resolve('./moduleTests/example.json')).toEqual(expected); }); + describe('exceptions', function() { + it('should reject blank "" module identifiers', function() { + expect(function() { + require.resolve(''); + }).toThrowError(/Cannot find/); + }); + it('should reject excessive identifier sizes', function() { + expect(function() { + require.resolve(new Array(8193).toString()); + }).toThrowError(/Cannot find/); + }); + it('should reject implicitly-relative filenames', function() { + expect(function() { + var mod = require.resolve('example.js'); + mod.exists; + }).toThrowError(/Cannot find/); + }); + it('should reject non-existent filenames', function() { + expect(function() { + require.resolve('./404error.js'); + }).toThrowError(/Cannot find/); + }); + it('should reject identifiers resolving to a directory', function() { + expect(function() { + var mod = require.resolve('.'); + mod.exists; + // console.warn('resolved(.)', mod); + }).toThrowError(/Cannot find/); + expect(function() { + var mod = require.resolve('..'); + mod.exists; + // console.warn('resolved(..)', mod); + }).toThrowError(/Cannot find/); + expect(function() { + var mod = require.resolve('../'); + mod.exists; + // console.warn('resolved(../)', mod); + }).toThrowError(/Cannot find/); + }); + (isNode ? xit : it)('should reject non-system, extensionless identifiers', function() { + expect(function() { + require.resolve('./example'); + }).toThrowError(/Cannot find/); + }); + }); }); describe('JSON', function() { @@ -20,12 +67,12 @@ describe('require', function() { var example = require('./moduleTests/example.json'); expect(example.name).toEqual('Example JSON Module'); }); - INTERFACE_describe('inteface', function() { - NETWORK_describe('network', function() { - //xit('should import #content-type=application/json modules', function() { - // var results = require('https://jsonip.com#content-type=application/json'); - // expect(results.ip).toMatch(/^[.0-9]+$/); - //}); + INTERFACE.describe('interface', function() { + NETWORK.describe('network', function() { + // xit('should import #content-type=application/json modules', function() { + // var results = require('https://jsonip.com#content-type=application/json'); + // expect(results.ip).toMatch(/^[.0-9]+$/); + // }); it('should import content-type: application/json modules', function() { var scope = { 'content-type': 'application/json' }; var results = require.call(scope, 'https://jsonip.com'); @@ -36,66 +83,32 @@ describe('require', function() { }); - INTERFACE_describe('system', function() { - it('require(id)', function() { + INTERFACE.describe('system', function() { + it('require("vec3")', function() { expect(require('vec3')).toEqual(jasmine.any(Function)); }); - it('require(id).function', function() { + it('require("vec3").method', function() { expect(require('vec3')().isValid).toEqual(jasmine.any(Function)); }); - }); - - describe('exceptions', function() { - it('should reject blank "" module identifiers', function() { - expect(function() { - require.resolve(''); - }).toThrowError(/Cannot find/); + it('require("vec3") as constructor', function() { + var vec3 = require('vec3'); + var v = vec3(1.1, 2.2, 3.3); + expect(v).toEqual(jasmine.any(Object)); + expect(v.isValid).toEqual(jasmine.any(Function)); + expect(v.isValid()).toBe(true); + expect(v.toString()).toEqual('[Vec3 (1.100,2.200,3.300)]'); }); - it('should reject excessive identifier sizes', function() { - expect(function() { - require.resolve(new Array(8193).toString()); - }).toThrowError(/Cannot find/); - }); - it('should reject implicitly-relative filenames', function() { - expect(function() { - var mod = require.resolve('example.js'); - }).toThrowError(/Cannot find/); - }); - it('should reject non-existent filenames', function() { - expect(function() { - var mod = require.resolve('./404error.js'); - }).toThrowError(/Cannot find/); - }); - it('should reject identifiers resolving to a directory', function() { - expect(function() { - var mod = require.resolve('.'); - //console.warn('resolved(.)', mod); - }).toThrowError(/Cannot find/); - expect(function() { - var mod = require.resolve('..'); - //console.warn('resolved(..)', mod); - }).toThrowError(/Cannot find/); - expect(function() { - var mod = require.resolve('../'); - //console.warn('resolved(../)', mod); - }).toThrowError(/Cannot find/); - }); - if (typeof MODE !== 'undefined' && MODE !== 'node') { - it('should reject non-system, extensionless identifiers', function() { - expect(function() { - require.resolve('./example'); - }).toThrowError(/Cannot find/); - }); - } }); describe('cache', function() { it('should cache modules by resolved module id', function() { var value = new Date; var example = require('./moduleTests/example.json'); + // earmark the module object with a unique value example['.test'] = value; var example2 = require('../../tests/unit_tests/moduleTests/example.json'); expect(example2).toBe(example); + // verify earmark is still the same after a second require() expect(example2['.test']).toBe(example['.test']); }); it('should reload cached modules set to null', function() { @@ -104,6 +117,7 @@ describe('require', function() { example['.test'] = value; require.cache[require.resolve('../../tests/unit_tests/moduleTests/example.json')] = null; var example2 = require('../../tests/unit_tests/moduleTests/example.json'); + // verify the earmark is *not* the same as before expect(example2).not.toBe(example); expect(example2['.test']).not.toBe(example['.test']); }); @@ -113,6 +127,7 @@ describe('require', function() { example['.test'] = value; delete require.cache[require.resolve('../../tests/unit_tests/moduleTests/example.json')]; var example2 = require('../../tests/unit_tests/moduleTests/example.json'); + // verify the earmark is *not* the same as before expect(example2).not.toBe(example); expect(example2['.test']).not.toBe(example['.test']); }); @@ -120,30 +135,29 @@ describe('require', function() { describe('cyclic dependencies', function() { describe('should allow lazy-ref cyclic module resolution', function() { - const MODULE_PATH = './moduleTests/cycles/main.js'; var main; beforeEach(function() { - try { this._print = print; } catch(e) {} - // for this test print is no-op'd so it doesn't disrupt the reporter output - //console = typeof console === 'object' ? console : { log: function() {} }; + // eslint-disable-next-line + try { this._print = print; } catch (e) {} + // during these tests print() is no-op'd so that it doesn't disrupt the reporter output print = function() {}; Script.resetModuleCache(); }); afterEach(function() { print = this._print; }); - it('main requirable', function() { - main = require(MODULE_PATH); + it('main is requirable', function() { + main = require('./moduleTests/cycles/main.js'); expect(main).toEqual(jasmine.any(Object)); }); - it('main with both a and b', function() { + it('transient a and b done values', function() { expect(main.a['b.done?']).toBe(true); expect(main.b['a.done?']).toBe(false); }); - it('a.done?', function() { + it('ultimate a.done?', function() { expect(main['a.done?']).toBe(true); }); - it('b.done?', function() { + it('ultimate b.done?', function() { expect(main['b.done?']).toBe(true); }); }); @@ -166,8 +180,8 @@ describe('require', function() { }).toThrowError(/could not resolve|Cannot find.*foobar:/); }); - NETWORK_describe('network', function() { - // note: with retries these tests can take up to 60 seconds each to timeout + NETWORK.describe('network', function() { + // note: depending on retries these tests can take up to 60 seconds each to timeout var timeout = 75 * 1000; it('should throw catchable host errors', function() { expect(function() { @@ -183,7 +197,7 @@ describe('require', function() { }); }); - INTERFACE_describe('entity', function() { + INTERFACE.describe('entity', function() { var sampleScripts = [ 'entityConstructorAPIException.js', 'entityConstructorModule.js', @@ -192,72 +206,98 @@ describe('require', function() { 'entityConstructorRequireException.js', 'entityPreloadAPIError.js', 'entityPreloadRequire.js', - ].filter(Boolean).map(function(id) { return Script.require.resolve('./moduleTests/entity/'+id); }); + ].filter(Boolean).map(function(id) { + return Script.require.resolve('./moduleTests/entity/'+id); + }); var uuids = []; - - for(var i=0; i < sampleScripts.length; i++) { - (function(i) { - var script = sampleScripts[ i % sampleScripts.length ]; - var shortname = '['+i+'] ' + script.split('/').pop(); - var position = MyAvatar.position; - position.y -= i/2; - it(shortname, function(done) { - var uuid = Entities.addEntity({ - text: shortname, - description: Script.resolvePath('').split('/').pop(), - type: 'Text', - position: position, - rotation: MyAvatar.orientation, - script: script, - scriptTimestamp: +new Date, - lifetime: 20, - lineHeight: 1/8, - dimensions: { x: 2, y: .5, z: .01 }, - backgroundColor: { red: 0, green: 0, blue: 0 }, - color: { red: 0xff, green: 0xff, blue: 0xff }, - }, !Entities.serversExist() || !Entities.canRezTmp()); - uuids.push(uuid); - var ii = Script.setInterval(function() { - Entities.queryPropertyMetadata(uuid, "script", function(err, result) { - if (err) { - throw new Error(err); - } - if (result.success) { - clearInterval(ii); - if (/Exception/.test(script)) - expect(result.status).toMatch(/^error_(loading|running)_script$/); - else - expect(result.status).toEqual("running"); - done(); - } else { - print('!result.success', JSON.stringify(result)); - } - }); - }, 100); - Script.setTimeout(function() { - Script.clearInterval(ii); - }, 4900); - }, 5000 /* timeout */); - })(i); + function cleanup() { + uuids.splice(0,uuids.length).forEach(function(uuid) { + Entities.deleteEntity(uuid); + }); + } + afterAll(cleanup); + // extra sanity check to avoid lingering entities + Script.scriptEnding.connect(cleanup); + + for (var i=0; i < sampleScripts.length; i++) { + maketest(i); + } + + function maketest(i) { + var script = sampleScripts[ i % sampleScripts.length ]; + var shortname = '['+i+'] ' + script.split('/').pop(); + var position = MyAvatar.position; + position.y -= i/2; + // define a unique jasmine test for the current entity script + it(shortname, function(done) { + var uuid = Entities.addEntity({ + text: shortname, + description: Script.resolvePath('').split('/').pop(), + type: 'Text', + position: position, + rotation: MyAvatar.orientation, + script: script, + scriptTimestamp: +new Date, + lifetime: 20, + lineHeight: 1/8, + dimensions: { x: 2, y: 0.5, z: 0.01 }, + backgroundColor: { red: 0, green: 0, blue: 0 }, + color: { red: 0xff, green: 0xff, blue: 0xff }, + }, !Entities.serversExist() || !Entities.canRezTmp()); + uuids.push(uuid); + function stopChecking() { + if (ii) { + Script.clearInterval(ii); + ii = 0; + } + } + var ii = Script.setInterval(function() { + Entities.queryPropertyMetadata(uuid, "script", function(err, result) { + if (err) { + stopChecking(); + throw new Error(err); + } + if (result.success) { + stopChecking(); + if (/Exception/.test(script)) { + expect(result.status).toMatch(/^error_(loading|running)_script$/); + } else { + expect(result.status).toEqual("running"); + } + Entities.deleteEntity(uuid); + done(); + } else { + print('!result.success', JSON.stringify(result)); + } + }); + }, 100); + Script.setTimeout(stopChecking, 4900); + }, 5000 /* jasmine async timeout */); } - Script.scriptEnding.connect(function() { - uuids.forEach(function(uuid) { Entities.deleteEntity(uuid); }); - }); }); }); +// support for isomorphic Node.js / Interface unit testing +// note: run `npm install` from unit_tests/ and then `node moduleUnitTests.js` function run() {} -function instrument_testrunner() { +function instrumentTestrunner() { var isNode = typeof process === 'object' && process.title === 'node'; + if (typeof describe === 'function') { + // already running within a test runner; assume jasmine is ready-to-go + return isNode; + } if (isNode) { - // for consistency this still uses the same local jasmine.js library + /* eslint-disable no-console */ + // Node.js test mode + // to keep things consistent Node.js uses the local jasmine.js library (instead of an npm version) var jasmineRequire = require('../../libraries/jasmine/jasmine.js'); var jasmine = jasmineRequire.core(jasmineRequire); var env = jasmine.getEnv(); var jasmineInterface = jasmineRequire.interface(jasmine, env); - for (var p in jasmineInterface) + for (var p in jasmineInterface) { global[p] = jasmineInterface[p]; + } env.addReporter(new (require('jasmine-console-reporter'))); // testing mocks Script = { @@ -270,39 +310,45 @@ function instrument_testrunner() { // this attempts to accurately emulate how Script.resolvePath works var trace = {}; Error.captureStackTrace(trace); var base = trace.stack.split('\n')[2].replace(/^.*[(]|[)].*$/g,'').replace(/:[0-9]+:[0-9]+.*$/,''); - if (!id) + if (!id) { return base; + } var rel = base.replace(/[^\/]+$/, id); console.info('rel', rel); return require.resolve(rel); }, require: function(mod) { return require(Script.require.resolve(mod)); - } + }, }; Script.require.cache = require.cache; Script.require.resolve = function(mod) { - if (mod === '.' || /^\.\.($|\/)/.test(mod)) + if (mod === '.' || /^\.\.($|\/)/.test(mod)) { throw new Error("Cannot find module '"+mod+"' (is dir)"); + } var path = require.resolve(mod); - //console.info('node-require-reoslved', mod, path); + // console.info('node-require-reoslved', mod, path); try { if (require('fs').lstatSync(path).isDirectory()) { throw new Error("Cannot find module '"+path+"' (is directory)"); } - //console.info('!path', path); - } catch(e) { console.info(e) } + // console.info('!path', path); + } catch (e) { + console.error(e); + } return path; }; print = console.info.bind(console, '[print]'); + /* eslint-enable no-console */ } else { + // Interface test mode global = this; - // Interface Test mode Script.require('../../../system/libraries/utils.js'); this.jasmineRequire = Script.require('../../libraries/jasmine/jasmine.js'); - Script.require('../../libraries/jasmine/hifi-boot.js') + Script.require('../../libraries/jasmine/hifi-boot.js'); require = Script.require; // polyfill console + /* global console:true */ console = { log: print, info: print.bind(this, '[info]'), @@ -311,6 +357,7 @@ function instrument_testrunner() { debug: print.bind(this, '[debug]'), }; } + // eslint-disable-next-line run = function() { global.jasmine.getEnv().execute(); }; return isNode; } From 5e5bba5aad5d5ac09732c386a35d1c17551aae8b Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 2 Mar 2017 16:58:37 -0500 Subject: [PATCH 09/22] Separate EntityPropertyMetadataRequest for easier documentation and testing --- .../entities/src/EntityScriptingInterface.cpp | 246 +++++++++++------- .../entities/src/EntityScriptingInterface.h | 27 +- 2 files changed, 180 insertions(+), 93 deletions(-) diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp index df88194f9f..aa241023c8 100644 --- a/libraries/entities/src/EntityScriptingInterface.cpp +++ b/libraries/entities/src/EntityScriptingInterface.cpp @@ -27,7 +27,6 @@ #include "ModelEntityItem.h" #include "QVariantGLM.h" #include "SimulationOwner.h" -#include "BaseScriptEngine.h" #include "ZoneEntityItem.h" #include "WebEntityItem.h" #include @@ -684,26 +683,24 @@ bool EntityScriptingInterface::reloadServerScripts(QUuid entityID) { return client->reloadServerScript(entityID); } -bool EntityScriptingInterface::queryPropertyMetadata(QUuid entityID, QScriptValue property, QScriptValue scopeOrCallback, QScriptValue methodOrName) { - auto name = property.toString(); - auto handler = makeScopedHandlerObject(scopeOrCallback, methodOrName); - QPointer engine = dynamic_cast(handler.engine()); +#ifdef DEBUG_ENTITY_METADATA +// baseline example -- return parsed userData as a standard CPS callback +bool EntityPropertyMetadataRequest::_userData(EntityItemID entityID, QScriptValue handler) { + QScriptValue err, result; + auto engine = _engine; if (!engine) { - qCDebug(entities) << "queryPropertyMetadata without detectable engine" << entityID << name; + qCDebug(entities) << __FUNCTION__ << " -- engine destroyed while inflight" << entityID; return false; } - connect(engine, &QObject::destroyed, this, [=]() { - qDebug() << "queryPropertyMetadata -- engine destroyed!" << (!engine ? "nullptr" : "engine"); - }); - if (!handler.property("callback").isFunction()) { - qDebug() << "!handler.callback.isFunction" << engine; - engine->raiseException(engine->makeError("callback is not a function", "TypeError")); - return false; - } - if (name == "userData") { - EntityItemPointer entity = _entityTree->findEntityByEntityItemID(entityID); - QScriptValue err, result; - if (entity) { + auto entityScriptingInterface = DependencyManager::get(); + auto entityTree = entityScriptingInterface ? entityScriptingInterface->getEntityTree() : nullptr; + if (!entityTree) { + err = engine->makeError("Entities Tree unavailable", "InternalError"); + } else { + EntityItemPointer entity = entityTree->findEntityByID(entityID); + if (!entity) { + err = engine->makeError("entity not found"); + } else { auto JSON = engine->globalObject().property("JSON"); auto parsed = JSON.property("parse").call(JSON, QScriptValueList({ entity->getUserData() })); if (engine->hasUncaughtException()) { @@ -712,81 +709,148 @@ bool EntityScriptingInterface::queryPropertyMetadata(QUuid entityID, QScriptValu } else { result = parsed; } - } else { - err = engine->makeError("entity not found"); } - QFutureWatcher *request = new QFutureWatcher; - connect(request, &QFutureWatcher::finished, engine, [=]() mutable { - if (!engine) { - qCDebug(entities) << "queryPropertyMetadata -- engine destroyed while inflight" << entityID << name; - return; - } - callScopedHandlerObject(handler, err, result); - request->deleteLater(); - }); - request->setFuture(QtConcurrent::run([]() -> QVariant { - QThread::sleep(1); - return 1; - })); - return true; - } else if (name == "script") { - using LocalScriptStatusRequest = QFutureWatcher; - LocalScriptStatusRequest *request = new LocalScriptStatusRequest; - connect(request, &LocalScriptStatusRequest::finished, engine, [=]() mutable { - if (!engine) { - qCDebug(entities) << "queryPropertyMetadata -- engine destroyed while inflight" << entityID << name; - return; - } - auto details = request->result().toMap(); - QScriptValue err, result; - if (details.contains("isError")) { - if (!details.contains("message")) { - details["message"] = details["errorInfo"]; - } - err = engine->makeError(engine->toScriptValue(details)); - } else { - details["success"] = true; - result = engine->toScriptValue(details); - } - callScopedHandlerObject(handler, err, result); - request->deleteLater(); - }); - request->setFuture(_entitiesScriptEngine->getLocalEntityScriptDetails(entityID)); - return true; - } else if (name == "serverScripts") { - auto client = DependencyManager::get(); - auto request = client->createScriptStatusRequest(entityID); - connect(request, &GetScriptStatusRequest::finished, engine, [=](GetScriptStatusRequest* request) mutable { - if (!engine) { - qCDebug(entities) << "queryPropertyMetadata -- engine destroyed while inflight" << entityID << name; - return; - } - QVariantMap details; - details["success"] = request->getResponseReceived(); - details["isRunning"] = request->getIsRunning(); - details["status"] = EntityScriptStatus_::valueToKey(request->getStatus()).toLower(); - details["errorInfo"] = request->getErrorInfo(); - - QScriptValue err, result; - if (!details["success"].toBool()) { - if (!details.contains("message") && details.contains("errorInfo")) { - details["message"] = details["errorInfo"]; - } - if (details["message"].toString().isEmpty()) { - details["message"] = "entity server script details not found"; - } - err = engine->makeError(engine->toScriptValue(details)); - } else { - result = engine->toScriptValue(details); - } - callScopedHandlerObject(handler, err, result); - request->deleteLater(); - }); - request->start(); - return true; } - engine->raiseException(engine->makeError("property has no mapped metadata: " + name)); - return false; + // this one second delay can be used with a Client script to query metadata and immediately Script.stop() + // (testing that the signal handler never gets called once the engine is destroyed) + // note: we still might want to check engine->isStopping() as an optimization in some places + QFutureWatcher *request = new QFutureWatcher; + QObject::connect(request, &QFutureWatcher::finished, engine, [=]() mutable { + if (!engine) { + qCDebug(entities) << "queryPropertyMetadata -- engine destroyed while inflight" << entityID; + return; + } + callScopedHandlerObject(handler, err, result); + request->deleteLater(); + }); + request->setFuture(QtConcurrent::run([]() -> QVariant { + QThread::sleep(1); + return QVariant(); + })); + return true; +} +#endif + +bool EntityPropertyMetadataRequest::script(EntityItemID entityID, QScriptValue handler) { + using LocalScriptStatusRequest = QFutureWatcher; + + LocalScriptStatusRequest *request = new LocalScriptStatusRequest; + QObject::connect(request, &LocalScriptStatusRequest::finished, _engine, [=]() mutable { + auto engine = _engine; + if (!engine) { + // this is just to address any lingering doubts -- when _engine is destroyed, this connect gets broken automatically + qCDebug(entities) << __FUNCTION__ << " -- engine destroyed while inflight" << entityID; + return; + } + auto details = request->result().toMap(); + QScriptValue err, result; + if (details.contains("isError")) { + if (!details.contains("message")) { + details["message"] = details["errorInfo"]; + } + err = engine->makeError(engine->toScriptValue(details)); + } else { + details["success"] = true; + result = engine->toScriptValue(details); + } + callScopedHandlerObject(handler, err, result); + request->deleteLater(); + }); + auto entityScriptingInterface = DependencyManager::get(); + entityScriptingInterface->withEntitiesScriptEngine([&](EntitiesScriptEngineProvider* entitiesScriptEngine) { + if (entitiesScriptEngine) { + request->setFuture(entitiesScriptEngine->getLocalEntityScriptDetails(entityID)); + } + }); + if (!request->isStarted()) { + request->deleteLater(); + callScopedHandlerObject(handler, _engine->makeError("Entities Scripting Provider unavailable", "InternalError"), QScriptValue()); + return false; + } + return true; +} + +bool EntityPropertyMetadataRequest::serverScripts(EntityItemID entityID, QScriptValue handler) { + auto client = DependencyManager::get(); + auto request = client->createScriptStatusRequest(entityID); + QPointer engine = _engine; + QObject::connect(request, &GetScriptStatusRequest::finished, _engine, [=](GetScriptStatusRequest* request) mutable { + auto engine = _engine; + if (!engine) { + qCDebug(entities) << __FUNCTION__ << " -- engine destroyed while inflight" << entityID; + return; + } + QVariantMap details; + details["success"] = request->getResponseReceived(); + details["isRunning"] = request->getIsRunning(); + details["status"] = EntityScriptStatus_::valueToKey(request->getStatus()).toLower(); + details["errorInfo"] = request->getErrorInfo(); + + QScriptValue err, result; + if (!details["success"].toBool()) { + if (!details.contains("message") && details.contains("errorInfo")) { + details["message"] = details["errorInfo"]; + } + if (details["message"].toString().isEmpty()) { + details["message"] = "entity server script details not found"; + } + err = engine->makeError(engine->toScriptValue(details)); + } else { + result = engine->toScriptValue(details); + } + callScopedHandlerObject(handler, err, result); + request->deleteLater(); + }); + request->start(); + return true; +} + +bool EntityScriptingInterface::queryPropertyMetadata(QUuid entityID, QScriptValue property, QScriptValue scopeOrCallback, QScriptValue methodOrName) { + auto name = property.toString(); + auto handler = makeScopedHandlerObject(scopeOrCallback, methodOrName); + QPointer engine = dynamic_cast(handler.engine()); + if (!engine) { + qCDebug(entities) << "queryPropertyMetadata without detectable engine" << entityID << name; + return false; + } +#ifdef DEBUG_ENGINE_STATE + connect(engine, &QObject::destroyed, this, [=]() { + qDebug() << "queryPropertyMetadata -- engine destroyed!" << (!engine ? "nullptr" : "engine"); + }); +#endif + if (!handler.property("callback").isFunction()) { + qDebug() << "!handler.callback.isFunction" << engine; + engine->raiseException(engine->makeError("callback is not a function", "TypeError")); + return false; + } + + // NOTE: this approach is a work-in-progress and for now just meant to work 100% correctly and provide + // some initial structure for organizing metadata adapters around. + + // The extra layer of indirection is *essential* because in real world conditions errors are often introduced + // by accident and sometimes without exact memory of "what just changed." + + // Here the scripter only needs to know an entityID and a property name -- which means all scripters can + // level this method when stuck in dead-end scenarios or to learn more about "magic" Entity properties + // like .script that work in terms of side-effects. + + // This is an async callback pattern -- so if needed C++ can easily throttle or restrict queries later. + + EntityPropertyMetadataRequest request(engine); + + if (name == "script") { + return request.script(entityID, handler); + } else if (name == "serverScripts") { + return request.serverScripts(entityID, handler); +#ifdef DEBUG_ENTITY_METADATA + } else if (name == "userData") { + return request.userData(entityID, handler); +#endif + } else { + engine->raiseException(engine->makeError("metadata for property " + name + " is not yet queryable")); + engine->maybeEmitUncaughtException(__FUNCTION__); + return false; + } } bool EntityScriptingInterface::getServerScriptStatus(QUuid entityID, QScriptValue callback) { diff --git a/libraries/entities/src/EntityScriptingInterface.h b/libraries/entities/src/EntityScriptingInterface.h index 2c3c654528..d6fe93b41e 100644 --- a/libraries/entities/src/EntityScriptingInterface.h +++ b/libraries/entities/src/EntityScriptingInterface.h @@ -34,8 +34,25 @@ #include "EntitiesScriptEngineProvider.h" #include "EntityItemProperties.h" +#include "BaseScriptEngine.h" + class EntityTree; +// helper factory to compose standardized, async metadata queries for "magic" Entity properties +// like .script and .serverScripts. This is used for automated testing of core scripting features +// as well as to provide early adopters a self-discoverable, consistent way to diagnose common +// problems with their own Entity scripts. +class EntityPropertyMetadataRequest { +public: + EntityPropertyMetadataRequest(BaseScriptEngine* engine) : _engine(engine) {}; + bool script(EntityItemID entityID, QScriptValue handler); + bool serverScripts(EntityItemID entityID, QScriptValue handler); + // this is used for internal testing and only available when DEBUG_ENTITY_METADATA is defined in the .cpp file + bool userData(EntityItemID entityID, QScriptValue handler); +private: + QPointer _engine; +}; + class RayToEntityIntersectionResult { public: RayToEntityIntersectionResult(); @@ -67,6 +84,7 @@ class EntityScriptingInterface : public OctreeScriptingInterface, public Depende Q_PROPERTY(float costMultiplier READ getCostMultiplier WRITE setCostMultiplier) Q_PROPERTY(QUuid keyboardFocusEntity READ getKeyboardFocusEntity WRITE setKeyboardFocusEntity) + friend EntityPropertyMetadataRequest; public: EntityScriptingInterface(bool bidOnSimulationOwnership); @@ -213,7 +231,7 @@ public slots: Q_INVOKABLE bool reloadServerScripts(QUuid entityID); /**jsdoc - * Query for the available metadata behind one of an Entity's "magic" properties (eg: `script` and `serverScripts`). + * Query additional metadata for "magic" Entity properties like `script` and `serverScripts`. * * @function Entities.queryPropertyMetadata * @param {EntityID} entityID The ID of the entity. @@ -221,7 +239,7 @@ public slots: * @param {ResultCallback} callback Executes callback(err, result) with the query results. */ /**jsdoc - * Query for the available metadata behind one of an Entity's "magic" properties (eg: `script` and `serverScripts`). + * Query additional metadata for "magic" Entity properties like `script` and `serverScripts`. * * @function Entities.queryPropertyMetadata * @param {EntityID} entityID The ID of the entity. @@ -343,6 +361,11 @@ signals: void webEventReceived(const EntityItemID& entityItemID, const QVariant& message); +protected: + void withEntitiesScriptEngine(std::function function) { + std::lock_guard lock(_entitiesScriptEngineLock); + function(_entitiesScriptEngine); + }; private: bool actionWorker(const QUuid& entityID, std::function actor); bool setVoxels(QUuid entityID, std::function actor); From d93047f9e2e74c6c9a7ffe452fc3ffdcc1e55548 Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 2 Mar 2017 17:23:24 -0500 Subject: [PATCH 10/22] Switch to READONLY_PROP_FLAGS for require.cache/require.resolve properties --- libraries/script-engine/src/ScriptEngine.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index c7364450a1..9b88e2bd24 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -580,7 +580,7 @@ void ScriptEngine::resetModuleCache(bool deleteScriptCache) { #if DEBUG_JS_MODULES cache.setProperty("__meta__", cacheMeta, READONLY_HIDDEN_PROP_FLAGS); #endif - jsRequire.setProperty("cache", cache, QScriptValue::ReadOnly | QScriptValue::Undeletable); + jsRequire.setProperty("cache", cache, READONLY_PROP_FLAGS); } void ScriptEngine::init() { @@ -651,7 +651,7 @@ void ScriptEngine::init() { auto Script = globalObject().property("Script"); auto require = Script.property("require"); auto resolve = Script.property("_requireResolve"); - require.setProperty("resolve", resolve, QScriptValue::ReadOnly | QScriptValue::Undeletable); + require.setProperty("resolve", resolve, READONLY_PROP_FLAGS); resetModuleCache(); } From 6b927de9f1aa0e98c906096bc094e3139dfc06a7 Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 2 Mar 2017 18:11:24 -0500 Subject: [PATCH 11/22] Add example vec3 system module --- scripts/modules/vec3.js | 69 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 scripts/modules/vec3.js diff --git a/scripts/modules/vec3.js b/scripts/modules/vec3.js new file mode 100644 index 0000000000..f164f01374 --- /dev/null +++ b/scripts/modules/vec3.js @@ -0,0 +1,69 @@ +// Example of using a "system module" to decouple Vec3's implementation details. +// +// Users would bring Vec3 support in as a module: +// var vec3 = Script.require('vec3'); +// + +// (this example is compatible with using as a Script.include and as a Script.require module) +try { + // Script.require + module.exports = vec3; +} catch(e) { + // Script.include + Script.registerValue("vec3", vec3); +} + +vec3.fromObject = function(v) { + //return new vec3(v.x, v.y, v.z); + //... this is even faster and achieves the same effect + v.__proto__ = vec3.prototype; + return v; +}; + +vec3.prototype = { + multiply: function(v2) { + // later on could support overrides like so: + // if (v2 instanceof quat) { [...] } + // which of the below is faster (C++ or JS)? + // (dunno -- but could systematically find out and go with that version) + + // pure JS option + // return new vec3(this.x * v2.x, this.y * v2.y, this.z * v2.z); + + // hybrid C++ option + return vec3.fromObject(Vec3.multiply(this, v2)); + }, + // detects any NaN and Infinity values + isValid: function() { + return isFinite(this.x) && isFinite(this.y) && isFinite(this.z); + }, + // format Vec3's, eg: + // var v = vec3(); + // print(v); // outputs [Vec3 (0.000, 0.000, 0.000)] + toString: function() { + if (this === vec3.prototype) { + return "{Vec3 prototype}"; + } + function fixed(n) { return n.toFixed(3); } + return "[Vec3 (" + [this.x, this.y, this.z].map(fixed) + ")]"; + }, +}; + +vec3.DEBUG = true; + +function vec3(x, y, z) { + if (!(this instanceof vec3)) { + // if vec3 is called as a function then re-invoke as a constructor + // (so that `value instanceof vec3` holds true for created values) + return new vec3(x, y, z); + } + + // unfold default arguments (vec3(), vec3(.5), vec3(0,1), etc.) + this.x = x !== undefined ? x : 0; + this.y = y !== undefined ? y : this.x; + this.z = z !== undefined ? z : this.y; + + if (vec3.DEBUG && !this.isValid()) + throw new Error('vec3() -- invalid initial values ['+[].slice.call(arguments)+']'); +}; + From 8582c7af7bfbf22f6dc9678c986ba85ef2a43e61 Mon Sep 17 00:00:00 2001 From: humbletim Date: Tue, 7 Mar 2017 14:02:20 -0500 Subject: [PATCH 12/22] Use specific debug name literal instead of __FUNCTION__ from within lambda --- libraries/script-engine/src/ScriptEngine.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 9b88e2bd24..f96b733c74 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -1840,7 +1840,7 @@ void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callbac doWithEnvironment(capturedEntityIdentifier, capturedSandboxURL, operation); if (hasUncaughtException()) { - emit unhandledException(cloneUncaughtException(__FUNCTION__)); + emit unhandledException(cloneUncaughtException("evaluateInclude")); clearExceptions(); } } else { From c559838dbb50c1e33dbc68c10b7b19985b320876 Mon Sep 17 00:00:00 2001 From: humbletim Date: Tue, 7 Mar 2017 14:05:29 -0500 Subject: [PATCH 13/22] Add a few more .resolvePath characterization tests --- .../tests/unit_tests/scriptUnitTests.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/scripts/developer/tests/unit_tests/scriptUnitTests.js b/scripts/developer/tests/unit_tests/scriptUnitTests.js index 63b451e97f..fa8cb44608 100644 --- a/scripts/developer/tests/unit_tests/scriptUnitTests.js +++ b/scripts/developer/tests/unit_tests/scriptUnitTests.js @@ -15,10 +15,20 @@ describe('Script', function () { // characterization tests // initially these are just to capture how the app works currently var testCases = { + // special relative resolves '': filename, '.': dirname, '..': parentdir, + + // local file "magic" tilde path expansion + '/~/defaultScripts.js': ScriptDiscoveryService.defaultScriptsPath + '/defaultScripts.js', + + // these schemes appear to always get resolved to empty URLs + 'qrc://test': '', 'about:Entities 1': '', + 'ftp://host:port/path': '', + 'data:text/html;text,foo': '', + 'Entities 1': dirname + 'Entities 1', './file.js': dirname + 'file.js', 'c:/temp/': 'file:///c:/temp/', @@ -31,6 +41,12 @@ describe('Script', function () { '/~/libraries/utils.js': 'file:///~/libraries/utils.js', '/temp/file.js': 'file:///temp/file.js', '/~/': 'file:///~/', + + // these schemes appear to always get resolved to the same URL again + 'http://highfidelity.com': 'http://highfidelity.com', + 'atp:/highfidelity': 'atp:/highfidelity', + 'atp:c2d7e3a48cadf9ba75e4f8d9f4d80e75276774880405a093fdee36543aa04f': + 'atp:c2d7e3a48cadf9ba75e4f8d9f4d80e75276774880405a093fdee36543aa04f', }; describe('resolvePath', function () { Object.keys(testCases).forEach(function(input) { @@ -42,7 +58,7 @@ describe('Script', function () { describe('include', function () { var old_cache_buster; - var cache_buster = '#' + +new Date; + var cache_buster = '#' + new Date().getTime().toString(36); beforeAll(function() { old_cache_buster = Settings.getValue('cache_buster'); Settings.setValue('cache_buster', cache_buster); From 075574b428d098cb6c989bb035ff81671e8f983c Mon Sep 17 00:00:00 2001 From: humbletim Date: Mon, 13 Mar 2017 16:33:42 -0400 Subject: [PATCH 14/22] log cleanup per CR; add more specific hints (instead of relying on __FUNCTION__) --- libraries/script-engine/src/ScriptEngine.cpp | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index f96b733c74..615c385a52 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -564,7 +564,6 @@ void ScriptEngine::resetModuleCache(bool deleteScriptCache) { if (it.flags() & QScriptValue::SkipInEnumeration) { continue; } - //scriptCache->deleteScript(it.name()); qCDebug(scriptengine) << "resetModuleCache(true) -- staging " << it.name() << " for cache reset at next require"; cacheMeta.setProperty(it.name(), true); } @@ -594,8 +593,8 @@ void ScriptEngine::init() { entityScriptingInterface->init(); connect(entityScriptingInterface.data(), &EntityScriptingInterface::deletingEntity, this, [this](const EntityItemID& entityID) { if (_entityScripts.contains(entityID)) { - if (isEntityScriptRunning(entityID)) { - qCWarning(scriptengine) << "deletingEntity while entity script is still running!" << entityID; + if (isEntityScriptRunning(entityID) && !isStopping()) { + qCWarning(scriptengine) << "deletingEntity while entity script is still running" << entityID; } _entityScripts.remove(entityID); emit entityScriptDetailsUpdated(); @@ -954,7 +953,7 @@ QScriptValue ScriptEngine::evaluate(const QString& sourceCode, const QString& fi syntaxError.setProperty("detail", "evaluate"); } raiseException(syntaxError); - maybeEmitUncaughtException(__FUNCTION__); + maybeEmitUncaughtException("lint"); return syntaxError; } QScriptProgram program { sourceCode, fileName, lineNumber }; @@ -962,14 +961,14 @@ QScriptValue ScriptEngine::evaluate(const QString& sourceCode, const QString& fi // can this happen? auto err = makeError("could not create QScriptProgram for " + fileName); raiseException(err); - maybeEmitUncaughtException(__FUNCTION__); + maybeEmitUncaughtException("compile"); return err; } QScriptValue result; { result = BaseScriptEngine::evaluate(program); - maybeEmitUncaughtException(__FUNCTION__); + maybeEmitUncaughtException("evaluate"); } return result; } @@ -1644,11 +1643,11 @@ QScriptValue ScriptEngine::require(const QString& moduleId) { auto exports = module.property("exports"); if (!invalidateCache && exports.isObject()) { - // we have found a cacheed module -- just need to possibly register it with current parent + // we have found a cached module -- just need to possibly register it with current parent qCDebug(scriptengine_module) << QString("require - using cached module '%1' for '%2' (loaded: %3)") .arg(modulePath).arg(moduleId).arg(module.property("loaded").toString()); registerModuleWithParent(module, parent); - maybeEmitUncaughtException(__FUNCTION__); + maybeEmitUncaughtException("cached module"); return exports; } @@ -2380,9 +2379,12 @@ void ScriptEngine::unloadEntityScript(const EntityItemID& entityID) { const EntityScriptDetails &oldDetails = _entityScripts[entityID]; if (isEntityScriptRunning(entityID)) { callEntityScriptMethod(entityID, "unload"); - } else { + } +#ifdef DEBUG_ENTITY_STATES + else { qCDebug(scriptengine) << "unload called while !running" << entityID << oldDetails.status; } +#endif if (oldDetails.status != EntityScriptStatus::UNLOADED) { EntityScriptDetails newDetails; newDetails.status = EntityScriptStatus::UNLOADED; From db73c80ba19f21416eac33e66db1cb929004d366 Mon Sep 17 00:00:00 2001 From: humbletim Date: Mon, 13 Mar 2017 19:16:00 -0400 Subject: [PATCH 15/22] remove unused meta property adapter --- .../entities/src/EntityScriptingInterface.cpp | 52 ------------------- .../entities/src/EntityScriptingInterface.h | 2 - 2 files changed, 54 deletions(-) diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp index aa241023c8..54efa3d89f 100644 --- a/libraries/entities/src/EntityScriptingInterface.cpp +++ b/libraries/entities/src/EntityScriptingInterface.cpp @@ -683,54 +683,6 @@ bool EntityScriptingInterface::reloadServerScripts(QUuid entityID) { return client->reloadServerScript(entityID); } -#ifdef DEBUG_ENTITY_METADATA -// baseline example -- return parsed userData as a standard CPS callback -bool EntityPropertyMetadataRequest::_userData(EntityItemID entityID, QScriptValue handler) { - QScriptValue err, result; - auto engine = _engine; - if (!engine) { - qCDebug(entities) << __FUNCTION__ << " -- engine destroyed while inflight" << entityID; - return false; - } - auto entityScriptingInterface = DependencyManager::get(); - auto entityTree = entityScriptingInterface ? entityScriptingInterface->getEntityTree() : nullptr; - if (!entityTree) { - err = engine->makeError("Entities Tree unavailable", "InternalError"); - } else { - EntityItemPointer entity = entityTree->findEntityByID(entityID); - if (!entity) { - err = engine->makeError("entity not found"); - } else { - auto JSON = engine->globalObject().property("JSON"); - auto parsed = JSON.property("parse").call(JSON, QScriptValueList({ entity->getUserData() })); - if (engine->hasUncaughtException()) { - err = engine->cloneUncaughtException(__FUNCTION__); - engine->clearExceptions(); - } else { - result = parsed; - } - } - } - // this one second delay can be used with a Client script to query metadata and immediately Script.stop() - // (testing that the signal handler never gets called once the engine is destroyed) - // note: we still might want to check engine->isStopping() as an optimization in some places - QFutureWatcher *request = new QFutureWatcher; - QObject::connect(request, &QFutureWatcher::finished, engine, [=]() mutable { - if (!engine) { - qCDebug(entities) << "queryPropertyMetadata -- engine destroyed while inflight" << entityID; - return; - } - callScopedHandlerObject(handler, err, result); - request->deleteLater(); - }); - request->setFuture(QtConcurrent::run([]() -> QVariant { - QThread::sleep(1); - return QVariant(); - })); - return true; -} -#endif - bool EntityPropertyMetadataRequest::script(EntityItemID entityID, QScriptValue handler) { using LocalScriptStatusRequest = QFutureWatcher; @@ -842,10 +794,6 @@ bool EntityScriptingInterface::queryPropertyMetadata(QUuid entityID, QScriptValu return request.script(entityID, handler); } else if (name == "serverScripts") { return request.serverScripts(entityID, handler); -#ifdef DEBUG_ENTITY_METADATA - } else if (name == "userData") { - return request.userData(entityID, handler); -#endif } else { engine->raiseException(engine->makeError("metadata for property " + name + " is not yet queryable")); engine->maybeEmitUncaughtException(__FUNCTION__); diff --git a/libraries/entities/src/EntityScriptingInterface.h b/libraries/entities/src/EntityScriptingInterface.h index d6fe93b41e..7631541b3e 100644 --- a/libraries/entities/src/EntityScriptingInterface.h +++ b/libraries/entities/src/EntityScriptingInterface.h @@ -47,8 +47,6 @@ public: EntityPropertyMetadataRequest(BaseScriptEngine* engine) : _engine(engine) {}; bool script(EntityItemID entityID, QScriptValue handler); bool serverScripts(EntityItemID entityID, QScriptValue handler); - // this is used for internal testing and only available when DEBUG_ENTITY_METADATA is defined in the .cpp file - bool userData(EntityItemID entityID, QScriptValue handler); private: QPointer _engine; }; From 52a571558cc974eedae95cc3f2f4908433c40c0e Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 16 Mar 2017 01:23:23 -0400 Subject: [PATCH 16/22] * changes per CR feedback * revert JSON content-type workaround * add specific error check / advice for unanchored module ids * update unit tests --- .../entities/src/EntityScriptingInterface.cpp | 12 +- libraries/script-engine/src/ScriptEngine.cpp | 132 +++++------------- libraries/script-engine/src/ScriptEngine.h | 7 +- .../developer/libraries/jasmine/hifi-boot.js | 2 +- .../tests/unit_tests/moduleUnitTests.js | 40 ++++-- 5 files changed, 65 insertions(+), 128 deletions(-) diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp index 54efa3d89f..08216b015a 100644 --- a/libraries/entities/src/EntityScriptingInterface.cpp +++ b/libraries/entities/src/EntityScriptingInterface.cpp @@ -686,24 +686,18 @@ bool EntityScriptingInterface::reloadServerScripts(QUuid entityID) { bool EntityPropertyMetadataRequest::script(EntityItemID entityID, QScriptValue handler) { using LocalScriptStatusRequest = QFutureWatcher; - LocalScriptStatusRequest *request = new LocalScriptStatusRequest; + LocalScriptStatusRequest* request = new LocalScriptStatusRequest; QObject::connect(request, &LocalScriptStatusRequest::finished, _engine, [=]() mutable { - auto engine = _engine; - if (!engine) { - // this is just to address any lingering doubts -- when _engine is destroyed, this connect gets broken automatically - qCDebug(entities) << __FUNCTION__ << " -- engine destroyed while inflight" << entityID; - return; - } auto details = request->result().toMap(); QScriptValue err, result; if (details.contains("isError")) { if (!details.contains("message")) { details["message"] = details["errorInfo"]; } - err = engine->makeError(engine->toScriptValue(details)); + err = _engine->makeError(_engine->toScriptValue(details)); } else { details["success"] = true; - result = engine->toScriptValue(details); + result = _engine->toScriptValue(details); } callScopedHandlerObject(handler, err, result); request->deleteLater(); diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 615c385a52..d956deed2f 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -73,15 +73,11 @@ #include "MIDIEvent.h" -const QString ScriptEngine::_SETTINGS_ENABLE_EXTENDED_MODULE_COMPAT { - "com.highfidelity.experimental.enableExtendedModuleCompatbility" -}; - const QString ScriptEngine::_SETTINGS_ENABLE_EXTENDED_EXCEPTIONS { "com.highfidelity.experimental.enableExtendedJSExceptions" }; -static const int MAX_MODULE_ID_LENTGH { 4096 }; +static const int MAX_MODULE_ID_LENGTH { 4096 }; static const int MAX_DEBUG_VALUE_LENGTH { 80 }; static const QScriptEngine::QObjectWrapOptions DEFAULT_QOBJECT_WRAP_OPTIONS = @@ -96,7 +92,7 @@ int functionSignatureMetaID = qRegisterMetaTypeargumentCount(); i++) { if (i > 0) { @@ -345,11 +341,7 @@ void ScriptEngine::runInThread() { // The thread interface cannot live on itself, and we want to move this into the thread, so // the thread cannot have this as a parent. QThread* workerThread = new QThread(); -#ifdef Q_OS_LINUX - workerThread->setObjectName(QString("js:") + getFilename()); -#else - workerThread->setObjectName(QString("Script Thread:") + getFilename()); -#endif + workerThread->setObjectName(QString("js:") + getFilename().replace("about:","")); moveToThread(workerThread); // NOTE: If you connect any essential signals for proper shutdown or cleanup of @@ -550,7 +542,7 @@ static QScriptValue createScriptableResourcePrototype(QScriptEngine* engine) { void ScriptEngine::resetModuleCache(bool deleteScriptCache) { if (QThread::currentThread() != thread()) { - executeOnScriptThread([=](){ resetModuleCache(deleteScriptCache); }); + executeOnScriptThread([=]() { resetModuleCache(deleteScriptCache); }); return; } auto jsRequire = globalObject().property("Script").property("require"); @@ -1379,14 +1371,14 @@ QString ScriptEngine::_requireResolve(const QString& moduleId, const QString& re auto throwResolveError = [&](const QScriptValue& error) -> QString { raiseException(error); maybeEmitUncaughtException("require.resolve"); - return nullptr; + return QString(); }; // de-fuzz the input a little by restricting to rational sizes auto idLength = url.toString().length(); - if (idLength < 1 || idLength > MAX_MODULE_ID_LENTGH) { + if (idLength < 1 || idLength > MAX_MODULE_ID_LENGTH) { auto details = QString("rejecting invalid module id size (%1 chars [1,%2])") - .arg(idLength).arg(MAX_MODULE_ID_LENTGH); + .arg(idLength).arg(MAX_MODULE_ID_LENGTH); return throwResolveError(makeError(message.arg(details), "RangeError")); } @@ -1402,11 +1394,21 @@ QString ScriptEngine::_requireResolve(const QString& moduleId, const QString& re url = resolvePath(moduleId); } else { // check if the moduleId refers to a "system" module - QString defaultsPath = defaultScriptsLoc.path(); - QString systemModulePath = QString("%1/modules/%2.js").arg(defaultsPath).arg(moduleId); + QString systemPath = defaultScriptsLoc.path(); + QString systemModulePath = QString("%1/modules/%2.js").arg(systemPath).arg(moduleId); url = defaultScriptsLoc; url.setPath(systemModulePath); if (!QFileInfo(url.toLocalFile()).isFile()) { + if (!moduleId.contains("./")) { + // the user might be trying to refer to a relative file without anchoring it + // let's do them a favor and test for that case -- offering specific advice if detected + auto unanchoredUrl = resolvePath("./" + moduleId); + if (QFileInfo(unanchoredUrl.toLocalFile()).isFile()) { + auto msg = QString("relative module ids must be anchored; use './%1' instead") + .arg(moduleId); + return throwResolveError(makeError(message.arg(msg))); + } + } return throwResolveError(makeError(message.arg("system module not found"))); } } @@ -1469,7 +1471,7 @@ bool ScriptEngine::registerModuleWithParent(const QScriptValue& module, const QS if (children.isArray()) { auto key = module.property("id"); auto length = children.property("length").toInt32(); - for (int i=0; i < length; i++) { + for (int i = 0; i < length; i++) { if (children.property(i).property("id").strictlyEquals(key)) { qCDebug(scriptengine_module) << key.toString() << " updating parent.children[" << i << "] = module"; children.setProperty(i, module); @@ -1516,10 +1518,10 @@ QScriptValue ScriptEngine::newModule(const QString& modulePath, const QScriptVal } // synchronously fetch a module's source code using BatchLoader -QScriptValue ScriptEngine::fetchModuleSource(const QString& modulePath, const bool forceDownload) { +QVariantMap ScriptEngine::fetchModuleSource(const QString& modulePath, const bool forceDownload) { using UrlMap = QMap; auto scriptCache = DependencyManager::get(); - QScriptValue req = newObject(); + QVariantMap req; qCDebug(scriptengine_module) << "require.fetchModuleSource: " << QUrl(modulePath).fileName() << QThread::currentThread(); auto onload = [=, &req](const UrlMap& data, const UrlMap& _status) { @@ -1528,13 +1530,13 @@ QScriptValue ScriptEngine::fetchModuleSource(const QString& modulePath, const bo auto contents = data[url]; qCDebug(scriptengine_module) << "require.fetchModuleSource.onload: " << QUrl(url).fileName() << status << QThread::currentThread(); if (isStopping()) { - req.setProperty("status", "Stopped"); - req.setProperty("success", false); + req["status"] = "Stopped"; + req["success"] = false; } else { - req.setProperty("url", url); - req.setProperty("status", status); - req.setProperty("success", ScriptCache::isSuccessStatus(status)); - req.setProperty("contents", contents, READONLY_HIDDEN_PROP_FLAGS); + req["url"] = url; + req["status"] = status; + req["success"] = ScriptCache::isSuccessStatus(status); + req["contents"] = contents; } }; @@ -1661,17 +1663,17 @@ QScriptValue ScriptEngine::require(const QString& moduleId) { // download the module source auto req = fetchModuleSource(modulePath, invalidateCache); - if (!req.property("success").toBool()) { - auto error = QString("error retrieving script (%1)").arg(req.property("status").toString()); + if (!req.contains("success") || !req["success"].toBool()) { + auto error = QString("error retrieving script (%1)").arg(req["status"].toString()); return throwModuleError(modulePath, error); } #if DEBUG_JS_MODULES qCDebug(scriptengine_module) << "require.loaded: " << - QUrl(req.property("url").toString()).fileName() << req.property("status").toString(); + QUrl(req["url"].toString()).fileName() << req["status"].toString(); #endif - auto sourceCode = req.property("contents").toString(); + auto sourceCode = req["contents"].toString(); if (QUrl(modulePath).fileName().endsWith(".json", Qt::CaseInsensitive)) { module.setProperty("content-type", "application/json"); @@ -1679,28 +1681,6 @@ QScriptValue ScriptEngine::require(const QString& moduleId) { module.setProperty("content-type", "application/javascript"); } - { - // It seems that many JSON sources don't actually put .json in the URL... - // so for now as a workaround users wanting to indicate JSON parsing mode can - // do so by calling with a custom this context, eg: - // - // var ITEMS_URL = 'https://highfidelity.io/api/v1/marketplace/items'; - // var thisObject = { 'content-type': 'application/json' }; - // var items = Script.require.call(thisObject, ITEMS_URL + '?category=everything&sort=recent'); - - auto thisObject = currentContext()->thisObject(); - bool calledWithCustomThis = thisObject.isObject() && - !thisObject.strictlyEquals(globalObject()) && - !thisObject.toQObject(); - - if (calledWithCustomThis) { -#ifdef DEBUG_JS - _debugDump("this", thisObject); -#endif - applyUserOptions(module, thisObject); - } - } - // evaluate the module auto result = instantiateModule(module, sourceCode); @@ -1721,54 +1701,6 @@ QScriptValue ScriptEngine::require(const QString& moduleId) { return module.property("exports"); } -// User-configurable override options -void ScriptEngine::applyUserOptions(QScriptValue& module, QScriptValue& options) { - if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) { - return; - } - if (!options.isValid()) { - return; - } - // options['content-type'] === 'application/json' - // -- allows JSON modules to be used from URLs not ending in .json - if (options.property("content-type").isString()) { - module.setProperty("content-type", options.property("content-type")); - qCDebug(scriptengine_module) << "module['content-type'] =" << module.property("content-type").toString(); - } - - if (ScriptEngine::_enableExtendedModuleCompatbility.get()) { - auto closure = module.property("__closure__"); - - auto maybeSetToExports = [&](const QString& key) { - if (options.property(key).toString() == "exports") { - closure.setProperty(key, module.property("exports")); - qCDebug(scriptengine_module) << "module.closure[" << key << "] = exports"; - } - }; - - // options[{key}] = 'exports' - // several "agnostic" modules in the wild are just one step away from being compatible -- - // they just didn't know not to look specifically for this, self or global for attaching - // things onto. - maybeSetToExports("global"); - maybeSetToExports("self"); - maybeSetToExports("this"); - - // when options is an Object it will get used as the value of "this" during module evaluation - // (which is what one might expect when calling require.call(thisObject, ...)) - if (options.isObject()) { - closure.setProperty("this", options); - } - - // when options.global is an Object it'll get used as the global object (during evaluation only) - if (options.property("global").isObject()) { - closure.setProperty("global", options.property("global")); - qCDebug(scriptengine_module) << "module.closure['global'] = options.global"; - } - } - maybeEmitUncaughtException(__FUNCTION__); -} - // If a callback is specified, the included files will be loaded asynchronously and the callback will be called // when all of the files have finished loading. // If no callback is specified, the included files will be loaded synchronously and will block execution until diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index ef6f3b6896..a3f709eff1 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -167,7 +167,7 @@ public: QScriptValue currentModule(); bool registerModuleWithParent(const QScriptValue& module, const QScriptValue& parent); QScriptValue newModule(const QString& modulePath, const QScriptValue& parent = QScriptValue()); - QScriptValue fetchModuleSource(const QString& modulePath, const bool forceDownload = false); + QVariantMap fetchModuleSource(const QString& modulePath, const bool forceDownload = false); QScriptValue instantiateModule(const QScriptValue& module, const QString& sourceCode); Q_INVOKABLE QObject* setInterval(const QScriptValue& function, int intervalMS); @@ -308,7 +308,7 @@ protected: AssetScriptingInterface _assetScriptingInterface{ this }; - std::function _emitScriptUpdates{ [](){ return true; } }; + std::function _emitScriptUpdates{ []() { return true; } }; std::recursive_mutex _lock; @@ -317,10 +317,7 @@ protected: static const QString _SETTINGS_ENABLE_EXTENDED_MODULE_COMPAT; static const QString _SETTINGS_ENABLE_EXTENDED_EXCEPTIONS; - Setting::Handle _enableExtendedModuleCompatbility { _SETTINGS_ENABLE_EXTENDED_MODULE_COMPAT, false }; Setting::Handle _enableExtendedJSExceptions { _SETTINGS_ENABLE_EXTENDED_EXCEPTIONS, true }; - - void applyUserOptions(QScriptValue& module, QScriptValue& options); }; #endif // hifi_ScriptEngine_h diff --git a/scripts/developer/libraries/jasmine/hifi-boot.js b/scripts/developer/libraries/jasmine/hifi-boot.js index 8757550ae8..772dd8c17e 100644 --- a/scripts/developer/libraries/jasmine/hifi-boot.js +++ b/scripts/developer/libraries/jasmine/hifi-boot.js @@ -62,7 +62,7 @@ clearTimeout = Script.clearTimeout; clearInterval = Script.clearInterval; - var jasmine = jasmineRequire.core(jasmineRequire); + var jasmine = this.jasmine = jasmineRequire.core(jasmineRequire); var env = jasmine.getEnv(); diff --git a/scripts/developer/tests/unit_tests/moduleUnitTests.js b/scripts/developer/tests/unit_tests/moduleUnitTests.js index a9446d1d6e..6810dd8b6d 100644 --- a/scripts/developer/tests/unit_tests/moduleUnitTests.js +++ b/scripts/developer/tests/unit_tests/moduleUnitTests.js @@ -32,6 +32,18 @@ describe('require', function() { mod.exists; }).toThrowError(/Cannot find/); }); + it('should reject unanchored, existing filenames with advice', function() { + expect(function() { + var mod = require.resolve('moduleTests/example.json'); + mod.exists; + }).toThrowError(/use '.\/moduleTests\/example\.json'/); + }); + it('should reject unanchored, non-existing filenames', function() { + expect(function() { + var mod = require.resolve('asdfssdf/example.json'); + mod.exists; + }).toThrowError(/Cannot find.*system module not found/); + }); it('should reject non-existent filenames', function() { expect(function() { require.resolve('./404error.js'); @@ -67,19 +79,21 @@ describe('require', function() { var example = require('./moduleTests/example.json'); expect(example.name).toEqual('Example JSON Module'); }); - INTERFACE.describe('interface', function() { - NETWORK.describe('network', function() { - // xit('should import #content-type=application/json modules', function() { - // var results = require('https://jsonip.com#content-type=application/json'); - // expect(results.ip).toMatch(/^[.0-9]+$/); - // }); - it('should import content-type: application/json modules', function() { - var scope = { 'content-type': 'application/json' }; - var results = require.call(scope, 'https://jsonip.com'); - expect(results.ip).toMatch(/^[.0-9]+$/); - }); - }); - }); + // noet: support for loading JSON via content type workarounds reverted + // (leaving these tests intact in case ever revisited later) + // INTERFACE.describe('interface', function() { + // NETWORK.describe('network', function() { + // xit('should import #content-type=application/json modules', function() { + // var results = require('https://jsonip.com#content-type=application/json'); + // expect(results.ip).toMatch(/^[.0-9]+$/); + // }); + // xit('should import content-type: application/json modules', function() { + // var scope = { 'content-type': 'application/json' }; + // var results = require.call(scope, 'https://jsonip.com'); + // expect(results.ip).toMatch(/^[.0-9]+$/); + // }); + // }); + // }); }); From 366c90ef6acfb46e5f85998cc433ba2fce1a7639 Mon Sep 17 00:00:00 2001 From: humbletim Date: Thu, 16 Mar 2017 01:28:50 -0400 Subject: [PATCH 17/22] add Q_ASSERT to IS_THREADSAFE_INVOCATION --- libraries/shared/src/BaseScriptEngine.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/shared/src/BaseScriptEngine.cpp b/libraries/shared/src/BaseScriptEngine.cpp index d803e85ed6..c92d629b75 100644 --- a/libraries/shared/src/BaseScriptEngine.cpp +++ b/libraries/shared/src/BaseScriptEngine.cpp @@ -31,6 +31,7 @@ bool BaseScriptEngine::IS_THREADSAFE_INVOCATION(const QThread *thread, const QSt qCCritical(shared) << QString("Scripting::%1 @ %2 -- ignoring thread-unsafe call from %3") .arg(method).arg(thread ? thread->objectName() : "(!thread)").arg(QThread::currentThread()->objectName()); qCDebug(shared) << "(please resolve on the calling side by using invokeMethod, executeOnScriptThread, etc.)"; + Q_ASSERT(false); return false; } From e21d7d9edf7c39693988bb4418cae3edfa85daf0 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Fri, 17 Mar 2017 13:34:11 +1300 Subject: [PATCH 18/22] Add Developer > Network > Clear Disk Cache menu action --- interface/src/Menu.cpp | 2 ++ interface/src/Menu.h | 1 + 2 files changed, 3 insertions(+) diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index beacbaccab..765d0f4a09 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -554,6 +554,8 @@ Menu::Menu() { "NetworkingPreferencesDialog"); }); addActionToQMenuAndActionHash(networkMenu, MenuOption::ReloadContent, 0, qApp, SLOT(reloadResourceCaches())); + addActionToQMenuAndActionHash(networkMenu, MenuOption::ClearDiskCache, 0, + DependencyManager::get().data(), SLOT(clearCache())); addCheckableActionToQMenuAndActionHash(networkMenu, MenuOption::DisableActivityLogger, 0, diff --git a/interface/src/Menu.h b/interface/src/Menu.h index c806ffa9ee..2755e11dd4 100644 --- a/interface/src/Menu.h +++ b/interface/src/Menu.h @@ -57,6 +57,7 @@ namespace MenuOption { const QString CameraEntityMode = "Entity Mode"; const QString CenterPlayerInView = "Center Player In View"; const QString Chat = "Chat..."; + const QString ClearDiskCache = "Clear Disk Cache"; const QString Collisions = "Collisions"; const QString Connexion = "Activate 3D Connexion Devices"; const QString Console = "Console..."; From 939b4f16129c9d81f5b2ae145933cbf145fbb8f9 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Fri, 17 Mar 2017 13:49:09 +1300 Subject: [PATCH 19/22] Remove Developer > Network > Disk Cache Editor menu item and dialog --- interface/src/Menu.cpp | 2 - interface/src/Menu.h | 1 - interface/src/ui/DialogsManager.cpp | 6 -- interface/src/ui/DialogsManager.h | 3 - interface/src/ui/DiskCacheEditor.cpp | 146 --------------------------- interface/src/ui/DiskCacheEditor.h | 49 --------- 6 files changed, 207 deletions(-) delete mode 100644 interface/src/ui/DiskCacheEditor.cpp delete mode 100644 interface/src/ui/DiskCacheEditor.h diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index 765d0f4a09..9afd2d6472 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -564,8 +564,6 @@ Menu::Menu() { SLOT(disable(bool))); addActionToQMenuAndActionHash(networkMenu, MenuOption::CachesSize, 0, dialogsManager.data(), SLOT(cachesSizeDialog())); - addActionToQMenuAndActionHash(networkMenu, MenuOption::DiskCacheEditor, 0, - dialogsManager.data(), SLOT(toggleDiskCacheEditor())); addActionToQMenuAndActionHash(networkMenu, MenuOption::ShowDSConnectTable, 0, dialogsManager.data(), SLOT(showDomainConnectionDialog())); diff --git a/interface/src/Menu.h b/interface/src/Menu.h index 2755e11dd4..367abe935a 100644 --- a/interface/src/Menu.h +++ b/interface/src/Menu.h @@ -84,7 +84,6 @@ namespace MenuOption { const QString DisableActivityLogger = "Disable Activity Logger"; const QString DisableEyelidAdjustment = "Disable Eyelid Adjustment"; const QString DisableLightEntities = "Disable Light Entities"; - const QString DiskCacheEditor = "Disk Cache Editor"; const QString DisplayCrashOptions = "Display Crash Options"; const QString DisplayHandTargets = "Show Hand Targets"; const QString DisplayModelBounds = "Display Model Bounds"; diff --git a/interface/src/ui/DialogsManager.cpp b/interface/src/ui/DialogsManager.cpp index 3252fef4f0..ac8b943aa8 100644 --- a/interface/src/ui/DialogsManager.cpp +++ b/interface/src/ui/DialogsManager.cpp @@ -21,7 +21,6 @@ #include "AddressBarDialog.h" #include "CachesSizeDialog.h" #include "ConnectionFailureDialog.h" -#include "DiskCacheEditor.h" #include "DomainConnectionDialog.h" #include "HMDToolsDialog.h" #include "LodToolsDialog.h" @@ -67,11 +66,6 @@ void DialogsManager::setDomainConnectionFailureVisibility(bool visible) { } } -void DialogsManager::toggleDiskCacheEditor() { - maybeCreateDialog(_diskCacheEditor); - _diskCacheEditor->toggle(); -} - void DialogsManager::toggleLoginDialog() { LoginDialog::toggleAction(); } diff --git a/interface/src/ui/DialogsManager.h b/interface/src/ui/DialogsManager.h index 54aef38984..bb144ba99b 100644 --- a/interface/src/ui/DialogsManager.h +++ b/interface/src/ui/DialogsManager.h @@ -22,7 +22,6 @@ class AnimationsDialog; class AttachmentsDialog; class CachesSizeDialog; -class DiskCacheEditor; class LodToolsDialog; class OctreeStatsDialog; class ScriptEditorWindow; @@ -46,7 +45,6 @@ public slots: void showAddressBar(); void showFeed(); void setDomainConnectionFailureVisibility(bool visible); - void toggleDiskCacheEditor(); void toggleLoginDialog(); void showLoginDialog(); void octreeStatsDetails(); @@ -77,7 +75,6 @@ private: QPointer _animationsDialog; QPointer _attachmentsDialog; QPointer _cachesSizeDialog; - QPointer _diskCacheEditor; QPointer _ircInfoBox; QPointer _hmdToolsDialog; QPointer _lodToolsDialog; diff --git a/interface/src/ui/DiskCacheEditor.cpp b/interface/src/ui/DiskCacheEditor.cpp deleted file mode 100644 index 1a7be8642b..0000000000 --- a/interface/src/ui/DiskCacheEditor.cpp +++ /dev/null @@ -1,146 +0,0 @@ -// -// DiskCacheEditor.cpp -// -// -// Created by Clement on 3/4/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 "DiskCacheEditor.h" - -#include -#include -#include -#include -#include -#include -#include - -#include - -#include "OffscreenUi.h" - -DiskCacheEditor::DiskCacheEditor(QWidget* parent) : QObject(parent) { -} - -QWindow* DiskCacheEditor::windowHandle() { - return (_dialog) ? _dialog->windowHandle() : nullptr; -} - -void DiskCacheEditor::toggle() { - if (!_dialog) { - makeDialog(); - } - - if (!_dialog->isActiveWindow()) { - _dialog->show(); - _dialog->raise(); - _dialog->activateWindow(); - } else { - _dialog->close(); - } -} - -void DiskCacheEditor::makeDialog() { - _dialog = new QDialog(static_cast(parent())); - Q_CHECK_PTR(_dialog); - _dialog->setAttribute(Qt::WA_DeleteOnClose); - _dialog->setWindowTitle("Disk Cache Editor"); - - QGridLayout* layout = new QGridLayout(_dialog); - Q_CHECK_PTR(layout); - _dialog->setLayout(layout); - - - QLabel* path = new QLabel("Path : ", _dialog); - Q_CHECK_PTR(path); - path->setAlignment(Qt::AlignRight); - layout->addWidget(path, 0, 0); - - QLabel* size = new QLabel("Current Size : ", _dialog); - Q_CHECK_PTR(size); - size->setAlignment(Qt::AlignRight); - layout->addWidget(size, 1, 0); - - QLabel* maxSize = new QLabel("Max Size : ", _dialog); - Q_CHECK_PTR(maxSize); - maxSize->setAlignment(Qt::AlignRight); - layout->addWidget(maxSize, 2, 0); - - - _path = new QLabel(_dialog); - Q_CHECK_PTR(_path); - _path->setAlignment(Qt::AlignLeft); - layout->addWidget(_path, 0, 1, 1, 3); - - _size = new QLabel(_dialog); - Q_CHECK_PTR(_size); - _size->setAlignment(Qt::AlignLeft); - layout->addWidget(_size, 1, 1, 1, 3); - - _maxSize = new QLabel(_dialog); - Q_CHECK_PTR(_maxSize); - _maxSize->setAlignment(Qt::AlignLeft); - layout->addWidget(_maxSize, 2, 1, 1, 3); - - refresh(); - - - static const int REFRESH_INTERVAL = 100; // msec - _refreshTimer = new QTimer(_dialog); - _refreshTimer->setInterval(REFRESH_INTERVAL); // Qt::CoarseTimer acceptable, no need for real time accuracy - _refreshTimer->setSingleShot(false); - QObject::connect(_refreshTimer.data(), &QTimer::timeout, this, &DiskCacheEditor::refresh); - _refreshTimer->start(); - - QPushButton* clearCacheButton = new QPushButton(_dialog); - Q_CHECK_PTR(clearCacheButton); - clearCacheButton->setText("Clear"); - clearCacheButton->setToolTip("Erases the entire content of the disk cache."); - connect(clearCacheButton, SIGNAL(clicked()), SLOT(clear())); - layout->addWidget(clearCacheButton, 3, 3); -} - -void DiskCacheEditor::refresh() { - DependencyManager::get()->cacheInfoRequest(this, "cacheInfoCallback"); -} - -void DiskCacheEditor::cacheInfoCallback(QString cacheDirectory, qint64 cacheSize, qint64 maximumCacheSize) { - static const auto stringify = [](qint64 number) { - static const QStringList UNITS = QStringList() << "B" << "KB" << "MB" << "GB"; - static const qint64 CHUNK = 1024; - QString unit; - int i = 0; - for (i = 0; i < 4; ++i) { - if (number / CHUNK > 0) { - number /= CHUNK; - } else { - break; - } - } - return QString("%0 %1").arg(number).arg(UNITS[i]); - }; - - if (_path) { - _path->setText(cacheDirectory); - } - if (_size) { - _size->setText(stringify(cacheSize)); - } - if (_maxSize) { - _maxSize->setText(stringify(maximumCacheSize)); - } -} - -void DiskCacheEditor::clear() { - auto buttonClicked = OffscreenUi::question(_dialog, "Clearing disk cache", - "You are about to erase all the content of the disk cache, " - "are you sure you want to do that?", - QMessageBox::Ok | QMessageBox::Cancel); - if (buttonClicked == QMessageBox::Ok) { - DependencyManager::get()->clearCache(); - } -} diff --git a/interface/src/ui/DiskCacheEditor.h b/interface/src/ui/DiskCacheEditor.h deleted file mode 100644 index 3f8fa1a883..0000000000 --- a/interface/src/ui/DiskCacheEditor.h +++ /dev/null @@ -1,49 +0,0 @@ -// -// DiskCacheEditor.h -// -// -// Created by Clement on 3/4/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_DiskCacheEditor_h -#define hifi_DiskCacheEditor_h - -#include -#include - -class QDialog; -class QLabel; -class QWindow; -class QTimer; - -class DiskCacheEditor : public QObject { - Q_OBJECT - -public: - DiskCacheEditor(QWidget* parent = nullptr); - - QWindow* windowHandle(); - -public slots: - void toggle(); - -private slots: - void refresh(); - void cacheInfoCallback(QString cacheDirectory, qint64 cacheSize, qint64 maximumCacheSize); - void clear(); - -private: - void makeDialog(); - - QPointer _dialog; - QPointer _path; - QPointer _size; - QPointer _maxSize; - QPointer _refreshTimer; -}; - -#endif // hifi_DiskCacheEditor_h \ No newline at end of file From 38bb48e72c41ad30bd825f2aa48f6ff89110aa74 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Sat, 18 Mar 2017 12:13:13 +1300 Subject: [PATCH 20/22] Disable Developer > Assets > ATP Asset Migration menu item --- interface/src/Menu.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index 8e124d27c7..02b8f5de78 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -417,6 +417,8 @@ Menu::Menu() { } // Developer > Assets >>> + // Menu item is not currently needed but code should be kept in case it proves useful again at some stage. + /* MenuWrapper* assetDeveloperMenu = developerMenu->addMenu("Assets"); auto& atpMigrator = ATPAssetMigrator::getInstance(); atpMigrator.setDialogParent(this); @@ -424,6 +426,7 @@ Menu::Menu() { addActionToQMenuAndActionHash(assetDeveloperMenu, MenuOption::AssetMigration, 0, &atpMigrator, SLOT(loadEntityServerFile())); + */ // Developer > Avatar >>> MenuWrapper* avatarDebugMenu = developerMenu->addMenu("Avatar"); From ab7d82edb6ba5f9a32770afd8cc941f639f6230c Mon Sep 17 00:00:00 2001 From: David Rowe Date: Sat, 18 Mar 2017 12:56:34 +1300 Subject: [PATCH 21/22] Use #ifdef instead of comment --- interface/src/Menu.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index 02b8f5de78..b1d97e6467 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -418,7 +418,8 @@ Menu::Menu() { // Developer > Assets >>> // Menu item is not currently needed but code should be kept in case it proves useful again at some stage. - /* +//#define WANT_ASSET_MIGRATION +#ifdef WANT_ASSET_MIGRATION MenuWrapper* assetDeveloperMenu = developerMenu->addMenu("Assets"); auto& atpMigrator = ATPAssetMigrator::getInstance(); atpMigrator.setDialogParent(this); @@ -426,7 +427,7 @@ Menu::Menu() { addActionToQMenuAndActionHash(assetDeveloperMenu, MenuOption::AssetMigration, 0, &atpMigrator, SLOT(loadEntityServerFile())); - */ +#endif // Developer > Avatar >>> MenuWrapper* avatarDebugMenu = developerMenu->addMenu("Avatar"); From 3a08611c26d96e01f5c0a32e2869dea595d3d364 Mon Sep 17 00:00:00 2001 From: Sam Cake Date: Sun, 19 Mar 2017 17:33:28 -0700 Subject: [PATCH 22/22] Fix the typo in the assert for isWireframe --- libraries/render-utils/src/RenderPipelines.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/render-utils/src/RenderPipelines.cpp b/libraries/render-utils/src/RenderPipelines.cpp index 4fbac4170e..414bcf0d63 100644 --- a/libraries/render-utils/src/RenderPipelines.cpp +++ b/libraries/render-utils/src/RenderPipelines.cpp @@ -307,7 +307,7 @@ void initForwardPipelines(render::ShapePlumber& plumber) { void addPlumberPipeline(ShapePlumber& plumber, const ShapeKey& key, const gpu::ShaderPointer& vertex, const gpu::ShaderPointer& pixel) { // These key-values' pipelines are added by this functor in addition to the key passed - assert(!key.isWireFrame()); + assert(!key.isWireframe()); assert(!key.isDepthBiased()); assert(key.isCullFace());