From dc1f726283c1ad239baf4a64a3390108e7a890c2 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 26 Aug 2015 12:36:48 -0700 Subject: [PATCH 01/20] don't assign the asset-server in domain by default --- domain-server/resources/describe-settings.json | 15 +++++++++++++++ domain-server/src/DomainServer.cpp | 14 ++++++++++++++ libraries/networking/src/Assignment.cpp | 2 ++ 3 files changed, 31 insertions(+) diff --git a/domain-server/resources/describe-settings.json b/domain-server/resources/describe-settings.json index b04ea95a23..e3ab2c51b8 100644 --- a/domain-server/resources/describe-settings.json +++ b/domain-server/resources/describe-settings.json @@ -166,6 +166,21 @@ } ] }, + { + "name": "asset_server", + "label": "Asset Server", + "assignment-types": [3], + "settings": [ + { + "name": "enabled", + "type": "checkbox", + "label": "Enabled", + "help": "Assigns an asset-server in your domain to serve files to clients via the ATP protocol (over UDP)", + "default": false, + "advanced": true + } + ] + }, { "name": "audio_env", "label": "Audio Environment", diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 52b20d721f..5081ff55dc 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -574,6 +574,20 @@ void DomainServer::populateDefaultStaticAssignmentsExcludingTypes(const QSet Date: Wed, 26 Aug 2015 12:37:47 -0700 Subject: [PATCH 02/20] remove an empty line --- domain-server/src/DomainServer.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 5081ff55dc..da1d493e69 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -587,7 +587,6 @@ void DomainServer::populateDefaultStaticAssignmentsExcludingTypes(const QSet Date: Wed, 26 Aug 2015 13:29:40 -0700 Subject: [PATCH 03/20] add a string for asset-server node type --- libraries/networking/src/Node.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/networking/src/Node.cpp b/libraries/networking/src/Node.cpp index 9f580c6d20..2b332d820d 100644 --- a/libraries/networking/src/Node.cpp +++ b/libraries/networking/src/Node.cpp @@ -31,6 +31,7 @@ void NodeType::init() { TypeNameHash.insert(NodeType::Agent, "Agent"); TypeNameHash.insert(NodeType::AudioMixer, "Audio Mixer"); TypeNameHash.insert(NodeType::AvatarMixer, "Avatar Mixer"); + TypeNameHash.insert(NodeType::AssetServer, "Asset Server"); TypeNameHash.insert(NodeType::Unassigned, "Unassigned"); } From 4039c2e3e0fc5995b10a431c18f01a02eee307d5 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 26 Aug 2015 14:27:12 -0700 Subject: [PATCH 04/20] fix for asset-server naming, deadlock in timeout/wait --- domain-server/src/DomainGatekeeper.cpp | 13 +++++++++++- libraries/networking/src/LimitedNodeList.cpp | 3 +++ libraries/networking/src/udt/SendQueue.cpp | 21 ++++++++++++++++++-- libraries/networking/src/udt/SendQueue.h | 2 +- libraries/networking/src/udt/Socket.cpp | 11 ++++++++++ libraries/networking/src/udt/Socket.h | 1 + 6 files changed, 47 insertions(+), 4 deletions(-) diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index 72b353f8a0..07afa007c5 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -48,7 +48,8 @@ QUuid DomainGatekeeper::assignmentUUIDForPendingAssignment(const QUuid& tempUUID } const NodeSet STATICALLY_ASSIGNED_NODES = NodeSet() << NodeType::AudioMixer - << NodeType::AvatarMixer << NodeType::EntityServer; + << NodeType::AvatarMixer << NodeType::EntityServer + << NodeType::AssetServer; void DomainGatekeeper::processConnectRequestPacket(QSharedPointer packet) { if (packet->getPayloadSize() == 0) { @@ -65,6 +66,16 @@ void DomainGatekeeper::processConnectRequestPacket(QSharedPointer pack return; } + static const NodeSet VALID_NODE_TYPES { + NodeType::AudioMixer, NodeType::AvatarMixer, NodeType::AssetServer, NodeType::EntityServer, NodeType::Agent + }; + + if (!VALID_NODE_TYPES.contains(nodeConnection.nodeType)) { + qDebug() << "Received an invalid node type with connect request. Will not allow connection from" + << nodeConnection.senderSockAddr; + return; + } + // check if this connect request matches an assignment in the queue auto pendingAssignment = _pendingAssignedNodes.find(nodeConnection.connectUUID); diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index c342f660a3..e5923b059f 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -409,6 +409,9 @@ void LimitedNodeList::eraseAllNodes() { void LimitedNodeList::reset() { eraseAllNodes(); + + // we need to make sure any socket connections are gone so wait on that here + _nodeSocket.clearConnections(); } void LimitedNodeList::killNodeWithUUID(const QUuid& nodeUUID) { diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp index da702c7166..c0638a60f7 100644 --- a/libraries/networking/src/udt/SendQueue.cpp +++ b/libraries/networking/src/udt/SendQueue.cpp @@ -134,6 +134,12 @@ void SendQueue::queuePacketList(std::unique_ptr packetList) { void SendQueue::stop() { _isRunning = false; + + // in case we're waiting to send another handshake, release the condition_variable now so we cleanup sooner + _handshakeACKCondition.notify_one(); + + // in case the empty condition is waiting for packets/loss release it now so that the queue is cleaned up + _emptyCondition.notify_one(); } void SendQueue::sendPacket(const Packet& packet) { @@ -286,14 +292,22 @@ void SendQueue::run() { } _flowWindowWasFull = flowWindowFull; + // since we're a while loop, give the thread a chance to process events + QCoreApplication::processEvents(); + + // we just processed events so check now if we were just told to stop + if (!_isRunning) { + break; + } if (_hasReceivedHandshakeACK && !sentAPacket) { static const std::chrono::seconds CONSIDER_INACTIVE_AFTER { 5 }; if (flowWindowFull && (high_resolution_clock::now() - _flowWindowFullSince) > CONSIDER_INACTIVE_AFTER) { // If the flow window has been full for over CONSIDER_INACTIVE_AFTER, - // then signal the queue is inactive + // then signal the queue is inactive and return so it can be cleaned up emit queueInactive(); + return; } else { // During our processing above we didn't send any packets and the flow window is not full. @@ -303,15 +317,18 @@ void SendQueue::run() { // The packets queue and loss list mutexes are now both locked - check if they're still both empty if (doubleLock.try_lock() && _packets.empty() && _naks.getLength() == 0) { + // both are empty - let's use a condition_variable_any to wait auto cvStatus = _emptyCondition.wait_for(doubleLock, CONSIDER_INACTIVE_AFTER); // we have the double lock again - Make sure to unlock it doubleLock.unlock(); - // Check if we've been inactive for too long if (cvStatus == std::cv_status::timeout) { + // the wait_for released because we've been inactive for too long + // so emit our inactive signal and return so the send queue can be cleaned up emit queueInactive(); + return; } // skip to the next iteration diff --git a/libraries/networking/src/udt/SendQueue.h b/libraries/networking/src/udt/SendQueue.h index 89dc2fae8f..70493f2054 100644 --- a/libraries/networking/src/udt/SendQueue.h +++ b/libraries/networking/src/udt/SendQueue.h @@ -83,7 +83,7 @@ private: void sendNewPacketAndAddToSentList(std::unique_ptr newPacket, SequenceNumber sequenceNumber); bool maybeSendNewPacket(); // Figures out what packet to send next - bool maybeResendPacket(); // Determines whether to resend a packet and wich one + bool maybeResendPacket(); // Determines whether to resend a packet and which one // Increments current sequence number and return it SequenceNumber getNextSequenceNumber(); diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index d5e57df60b..db0377de05 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -154,6 +154,17 @@ Connection& Socket::findOrCreateConnection(const HifiSockAddr& sockAddr) { return *it->second; } +void Socket::clearConnections() { + if (thread() != QThread::currentThread()) { + QMetaObject::invokeMethod(this, "clearConnections", Qt::BlockingQueuedConnection); + return; + } + + // clear all of the current connections in the socket + qDebug() << "Clearing all remaining connections in Socket."; + _connectionsHash.clear(); +} + void Socket::cleanupConnection(HifiSockAddr sockAddr) { qCDebug(networking) << "Socket::cleanupConnection called for connection to" << sockAddr; _connectionsHash.erase(sockAddr); diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h index 0014a97e2b..f421b98288 100644 --- a/libraries/networking/src/udt/Socket.h +++ b/libraries/networking/src/udt/Socket.h @@ -75,6 +75,7 @@ public: public slots: void cleanupConnection(HifiSockAddr sockAddr); + void clearConnections(); private slots: void readPendingDatagrams(); From d3b19f36fd85bab43458bf76096a69505d8182a3 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 26 Aug 2015 14:43:09 -0700 Subject: [PATCH 05/20] don't use remove_if since it can't be used on associative --- libraries/networking/src/PacketReceiver.cpp | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index c55461e8cf..556216bbd1 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -200,7 +200,7 @@ void PacketReceiver::registerVerifiedListener(PacketType type, QObject* object, qCWarning(networking) << "Registering a packet listener for packet type" << type << "that will remove a previously registered listener"; } - + // add the mapping _packetListenerMap[type] = ObjectMethodPair(QPointer(object), slot); } @@ -210,10 +210,17 @@ void PacketReceiver::unregisterListener(QObject* listener) { { QMutexLocker packetListenerLocker(&_packetListenerLock); - std::remove_if(std::begin(_packetListenerMap), std::end(_packetListenerMap), - [&listener](const ObjectMethodPair& pair) { - return pair.first == listener; - }); + + // clear any registrations for this listener + auto it = _packetListenerMap.begin(); + + while (it != _packetListenerMap.end()) { + if (it->first == listener) { + it = _packetListenerMap.erase(it); + } + + ++it; + } } QMutexLocker directConnectSetLocker(&_directConnectSetMutex); From c340c34b3889ca729120711218729da3cf7b24ab Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 26 Aug 2015 14:53:05 -0700 Subject: [PATCH 06/20] add comments for PacketReceiver TODOs --- libraries/networking/src/PacketReceiver.cpp | 15 ++++++++++++++- libraries/networking/src/PacketReceiver.h | 1 + 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index 556216bbd1..035432cf4b 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -211,7 +211,9 @@ void PacketReceiver::unregisterListener(QObject* listener) { { QMutexLocker packetListenerLocker(&_packetListenerLock); - // clear any registrations for this listener + // TODO: replace the two while loops below with a replace_if on the vector (once we move to Message everywhere) + + // clear any registrations for this listener in _packetListenerMap auto it = _packetListenerMap.begin(); while (it != _packetListenerMap.end()) { @@ -221,6 +223,17 @@ void PacketReceiver::unregisterListener(QObject* listener) { ++it; } + + // clear any registrations for this listener in _packetListListener + auto listIt = _packetListenerMap.end(); + + while (listIt != _packetListListenerMap.end()) { + if (listIt->first == listener) { + listIt = _packetListListenerMap.erase(listIt); + } + + ++listIt; + } } QMutexLocker directConnectSetLocker(&_directConnectSetMutex); diff --git a/libraries/networking/src/PacketReceiver.h b/libraries/networking/src/PacketReceiver.h index 9965eccdc2..1c6f9e73d2 100644 --- a/libraries/networking/src/PacketReceiver.h +++ b/libraries/networking/src/PacketReceiver.h @@ -69,6 +69,7 @@ private: using ObjectMethodPair = std::pair, QMetaMethod>; QMutex _packetListenerLock; + // TODO: replace the two following hashes with an std::vector once we switch Packet/PacketList to Message QHash _packetListenerMap; QHash _packetListListenerMap; int _inPacketCount = 0; From 9e286666b5dfa62d3cf0a6409a56a8ea171e74ed Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 26 Aug 2015 14:55:59 -0700 Subject: [PATCH 07/20] add UDT to comment for Connection cleanup --- libraries/networking/src/udt/Socket.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index db0377de05..1a404eb6c1 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -166,7 +166,7 @@ void Socket::clearConnections() { } void Socket::cleanupConnection(HifiSockAddr sockAddr) { - qCDebug(networking) << "Socket::cleanupConnection called for connection to" << sockAddr; + qCDebug(networking) << "Socket::cleanupConnection called for UDT connection to" << sockAddr; _connectionsHash.erase(sockAddr); } From 5b5941fea2e7101cbf9fac487dd74ed92d57d329 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 26 Aug 2015 16:30:09 -0700 Subject: [PATCH 08/20] move asset upload to AssetUploadDialogFactory --- interface/src/Menu.cpp | 53 ++++++------------ interface/src/Menu.h | 1 + interface/src/ui/AddressBarDialog.h | 3 +- interface/src/ui/AssetUploadDialogFactory.cpp | 55 +++++++++++++++++++ interface/src/ui/AssetUploadDialogFactory.h | 35 ++++++++++++ interface/src/ui/DialogsManager.cpp | 2 +- interface/src/ui/DialogsManager.h | 2 +- interface/src/ui/LoginDialog.h | 3 +- 8 files changed, 112 insertions(+), 42 deletions(-) create mode 100644 interface/src/ui/AssetUploadDialogFactory.cpp create mode 100644 interface/src/ui/AssetUploadDialogFactory.h diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index 3305da5ba7..6c41a3d7da 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -22,8 +22,6 @@ #include #include -#include - #include "Application.h" #include "AccountManager.h" #include "audio/AudioScope.h" @@ -34,13 +32,15 @@ #include "devices/3DConnexionClient.h" #include "MainWindow.h" #include "scripting/MenuScriptingInterface.h" -#if defined(Q_OS_MAC) || defined(Q_OS_WIN) -#include "SpeechRecognizer.h" -#endif +#include "ui/AssetUploadDialogFactory.h" #include "ui/DialogsManager.h" #include "ui/StandAloneJSConsole.h" #include "InterfaceLogging.h" +#if defined(Q_OS_MAC) || defined(Q_OS_WIN) +#include "SpeechRecognizer.h" +#endif + #include "Menu.h" Menu* Menu::_instance = NULL; @@ -90,36 +90,6 @@ Menu::Menu() { addActionToQMenuAndActionHash(fileMenu, MenuOption::RunningScripts, Qt::CTRL | Qt::Key_J, qApp, SLOT(toggleRunningScriptsWidget())); - // Asset uploading - { - auto action = new QAction("Upload File", fileMenu); - fileMenu->addAction(action); - action->setMenuRole(QAction::NoRole); - _actionHash.insert("Upload File", action); - - connect(action, &QAction::triggered, [this](bool checked) { - qDebug() << "Clicked upload file"; - auto filename = QFileDialog::getOpenFileUrl(nullptr, "Select a file to upload"); - if (!filename.isEmpty()) { - qDebug() << "Selected: " << filename; - QFile file { filename.path() }; - if (file.open(QIODevice::ReadOnly)) { - QFileInfo fileInfo { filename.path() }; - auto extension = fileInfo.suffix(); - auto data = file.readAll(); - auto assetClient = DependencyManager::get(); - assetClient->uploadAsset(data, extension, [this, extension](bool result, QString hash) mutable { - if (result) { - QMessageBox::information(this, "Upload Successful", "URL: apt:/" + hash + "." + extension); - } else { - QMessageBox::warning(this, "Upload Failed", "There was an error uploading the file."); - } - }); - } - } - }); - } - auto addressManager = DependencyManager::get(); addDisabledActionAndSeparator(fileMenu, "History"); @@ -400,7 +370,18 @@ Menu::Menu() { addActionToQMenuAndActionHash(renderOptionsMenu, MenuOption::LodTools, 0, // QML Qt::SHIFT | Qt::Key_L, dialogsManager.data(), SLOT(lodTools())); - + + MenuWrapper* assetDeveloperMenu = developerMenu->addMenu("Assets"); + + auto& assetDialogFactory = AssetUploadDialogFactory::getInstance(); + assetDialogFactory.setParent(this); + + addActionToQMenuAndActionHash(assetDeveloperMenu, + MenuOption::UploadAsset, + 0, + &AssetUploadDialogFactory::getInstance(), + SLOT(showDialog())); + MenuWrapper* avatarDebugMenu = developerMenu->addMenu("Avatar"); MenuWrapper* faceTrackingMenu = avatarDebugMenu->addMenu("Face Tracking"); diff --git a/interface/src/Menu.h b/interface/src/Menu.h index 4bd1e7f664..948e159b04 100644 --- a/interface/src/Menu.h +++ b/interface/src/Menu.h @@ -288,6 +288,7 @@ namespace MenuOption { const QString ToolWindow = "Tool Window"; const QString TransmitterDrive = "Transmitter Drive"; const QString TurnWithHead = "Turn using Head"; + const QString UploadAsset = "Upload File to Local Asset Server"; const QString UseAudioForMouth = "Use Audio for Mouth"; const QString UseCamera = "Use Camera"; const QString VelocityFilter = "Velocity Filter"; diff --git a/interface/src/ui/AddressBarDialog.h b/interface/src/ui/AddressBarDialog.h index e0be8aa3fb..eec3acdbfc 100644 --- a/interface/src/ui/AddressBarDialog.h +++ b/interface/src/ui/AddressBarDialog.h @@ -14,8 +14,7 @@ #include -class AddressBarDialog : public OffscreenQmlDialog -{ +class AddressBarDialog : public OffscreenQmlDialog { Q_OBJECT HIFI_QML_DECL Q_PROPERTY(bool backEnabled READ backEnabled NOTIFY backEnabledChanged) diff --git a/interface/src/ui/AssetUploadDialogFactory.cpp b/interface/src/ui/AssetUploadDialogFactory.cpp new file mode 100644 index 0000000000..79e3cfa2db --- /dev/null +++ b/interface/src/ui/AssetUploadDialogFactory.cpp @@ -0,0 +1,55 @@ +// +// AssetUploadDialogFactory.cpp +// interface/src/ui +// +// Created by Stephen Birarda on 2015-08-26. +// 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 "AssetUploadDialogFactory.h" + +#include + +#include +#include +#include + +AssetUploadDialogFactory& AssetUploadDialogFactory::getInstance() { + static AssetUploadDialogFactory staticInstance; + return staticInstance; +} + +AssetUploadDialogFactory::AssetUploadDialogFactory() { + +} + +void AssetUploadDialogFactory::showDialog() { + auto filename = QFileDialog::getOpenFileUrl(nullptr, "Select a file to upload"); + + if (!filename.isEmpty()) { + qDebug() << "Selected filename for upload to asset-server: " << filename; + + QFile file { filename.path() }; + + if (file.open(QIODevice::ReadOnly)) { + + QFileInfo fileInfo { filename.path() }; + auto extension = fileInfo.suffix(); + + auto data = file.readAll(); + + auto assetClient = DependencyManager::get(); + + assetClient->uploadAsset(data, extension, [this, extension](bool result, QString hash) mutable { + if (result) { + QMessageBox::information(_dialogParent, "Upload Successful", "URL: apt:/" + hash + "." + extension); + } else { + QMessageBox::warning(_dialogParent, "Upload Failed", "There was an error uploading the file."); + } + }); + } + } +} diff --git a/interface/src/ui/AssetUploadDialogFactory.h b/interface/src/ui/AssetUploadDialogFactory.h new file mode 100644 index 0000000000..3d830aa4bb --- /dev/null +++ b/interface/src/ui/AssetUploadDialogFactory.h @@ -0,0 +1,35 @@ +// +// AssetUploadDialogFactory.h +// interface/src/ui +// +// Created by Stephen Birarda on 2015-08-26. +// 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 +// + +#pragma once + +#ifndef hifi_AssetUploadDialogFactory_h +#define hifi_AssetUploadDialogFactory_h + +#include + +class AssetUploadDialogFactory : public QObject { + Q_OBJECT +public: + AssetUploadDialogFactory(); + AssetUploadDialogFactory(const AssetUploadDialogFactory& other) = delete; + AssetUploadDialogFactory& operator=(const AssetUploadDialogFactory& rhs) = delete; + + static AssetUploadDialogFactory& getInstance(); + + void setDialogParent(QWidget* dialogParent) { _dialogParent = dialogParent; } +public slots: + void showDialog(); +private: + QWidget* _dialogParent { nullptr }; +}; + +#endif // hifi_AssetUploadDialogFactory_h diff --git a/interface/src/ui/DialogsManager.cpp b/interface/src/ui/DialogsManager.cpp index 58d2550752..308cfc9e8c 100644 --- a/interface/src/ui/DialogsManager.cpp +++ b/interface/src/ui/DialogsManager.cpp @@ -1,6 +1,6 @@ // // DialogsManager.cpp -// +// interface/src/ui // // Created by Clement on 1/18/15. // Copyright 2015 High Fidelity, Inc. diff --git a/interface/src/ui/DialogsManager.h b/interface/src/ui/DialogsManager.h index 2db700e72a..68d371021a 100644 --- a/interface/src/ui/DialogsManager.h +++ b/interface/src/ui/DialogsManager.h @@ -1,6 +1,6 @@ // // DialogsManager.h -// +// interface/src/ui // // Created by Clement on 1/18/15. // Copyright 2015 High Fidelity, Inc. diff --git a/interface/src/ui/LoginDialog.h b/interface/src/ui/LoginDialog.h index 50c820aa07..25ecf45898 100644 --- a/interface/src/ui/LoginDialog.h +++ b/interface/src/ui/LoginDialog.h @@ -16,8 +16,7 @@ #include -class LoginDialog : public OffscreenQmlDialog -{ +class LoginDialog : public OffscreenQmlDialog { Q_OBJECT HIFI_QML_DECL From 513cae0d401abb68e7eca391a987a3dc1138b039 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 27 Aug 2015 10:23:02 -0700 Subject: [PATCH 09/20] thread the AssetUpload so it doesn't take over main --- assignment-client/src/assets/AssetServer.cpp | 5 -- .../src/octree/OctreePacketProcessor.cpp | 5 +- interface/src/ui/AssetUploadDialogFactory.cpp | 75 ++++++++++++++----- interface/src/ui/AssetUploadDialogFactory.h | 4 + libraries/networking/src/AssetClient.cpp | 29 +++++-- libraries/networking/src/AssetClient.h | 16 ++-- .../networking/src/AssetResourceRequest.cpp | 2 +- libraries/networking/src/AssetUpload.cpp | 53 +++++++++++++ libraries/networking/src/AssetUpload.h | 47 ++++++++++++ libraries/networking/src/AssetUtils.h | 2 + 10 files changed, 201 insertions(+), 37 deletions(-) create mode 100644 libraries/networking/src/AssetUpload.cpp create mode 100644 libraries/networking/src/AssetUpload.h diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index 2c294e652c..281257e904 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -80,11 +80,6 @@ void AssetServer::run() { file.rename(_resourcesDirectory.absoluteFilePath(hash)); } } - - while (!_isFinished) { - // since we're a while loop we need to help Qt's event processing - QCoreApplication::processEvents(); - } } void AssetServer::handleAssetGetInfo(QSharedPointer packet, SharedNodePointer senderNode) { diff --git a/interface/src/octree/OctreePacketProcessor.cpp b/interface/src/octree/OctreePacketProcessor.cpp index 5b8ff78fad..4a92e7e8ac 100644 --- a/interface/src/octree/OctreePacketProcessor.cpp +++ b/interface/src/octree/OctreePacketProcessor.cpp @@ -18,9 +18,8 @@ OctreePacketProcessor::OctreePacketProcessor() { auto& packetReceiver = DependencyManager::get()->getPacketReceiver(); - - packetReceiver.registerDirectListenerForTypes({ PacketType::OctreeStats, PacketType::EntityData, - PacketType::EntityErase, PacketType::OctreeStats }, + + packetReceiver.registerDirectListenerForTypes({ PacketType::OctreeStats, PacketType::EntityData, PacketType::EntityErase }, this, "handleOctreePacket"); } diff --git a/interface/src/ui/AssetUploadDialogFactory.cpp b/interface/src/ui/AssetUploadDialogFactory.cpp index 79e3cfa2db..a948ba54ba 100644 --- a/interface/src/ui/AssetUploadDialogFactory.cpp +++ b/interface/src/ui/AssetUploadDialogFactory.cpp @@ -12,10 +12,15 @@ #include "AssetUploadDialogFactory.h" #include +#include +#include #include -#include +#include #include +#include +#include +#include AssetUploadDialogFactory& AssetUploadDialogFactory::getInstance() { static AssetUploadDialogFactory staticInstance; @@ -27,29 +32,63 @@ AssetUploadDialogFactory::AssetUploadDialogFactory() { } void AssetUploadDialogFactory::showDialog() { - auto filename = QFileDialog::getOpenFileUrl(nullptr, "Select a file to upload"); + auto filename = QFileDialog::getOpenFileUrl(_dialogParent, "Select a file to upload"); if (!filename.isEmpty()) { qDebug() << "Selected filename for upload to asset-server: " << filename; - QFile file { filename.path() }; + auto assetClient = DependencyManager::get(); + auto upload = assetClient->createUpload(filename.path()); - if (file.open(QIODevice::ReadOnly)) { + if (upload) { + // connect to the finished signal so we know when the AssetUpload is done + QObject::connect(upload, &AssetUpload::finished, this, &AssetUploadDialogFactory::handleUploadFinished); - QFileInfo fileInfo { filename.path() }; - auto extension = fileInfo.suffix(); - - auto data = file.readAll(); - - auto assetClient = DependencyManager::get(); - - assetClient->uploadAsset(data, extension, [this, extension](bool result, QString hash) mutable { - if (result) { - QMessageBox::information(_dialogParent, "Upload Successful", "URL: apt:/" + hash + "." + extension); - } else { - QMessageBox::warning(_dialogParent, "Upload Failed", "There was an error uploading the file."); - } - }); + // start the upload now + upload->start(); } } } + +void AssetUploadDialogFactory::handleUploadFinished(AssetUpload* upload, const QString& hash) { + if (true) { + // show message box for successful upload, with copiable text for ATP hash + QDialog* hashCopyDialog = new QDialog(_dialogParent); + + // delete the dialog on close + hashCopyDialog->setAttribute(Qt::WA_DeleteOnClose); + + // set the window title + hashCopyDialog->setWindowTitle(tr("Successful Asset Upload")); + + // setup a layout for the contents of the dialog + QVBoxLayout* boxLayout = new QVBoxLayout; + + // set the label text (this shows above the text box) + QLabel* lineEditLabel = new QLabel; + lineEditLabel->setText(QString("ATP URL for %1").arg(upload->getFilename())); + + // setup the line edit to hold the copiable text + QLineEdit* lineEdit = new QLineEdit; + + // set the ATP URL as the text value so it's copiable + lineEdit->insert(QString("%1://%2").arg(ATP_SCHEME).arg(hash)); + + // add the label and line edit to the dialog + boxLayout->addWidget(lineEditLabel); + boxLayout->addWidget(lineEdit); + + // setup an OK button to close the dialog + QDialogButtonBox* buttonBox = new QDialogButtonBox(QDialogButtonBox::Ok); + connect(buttonBox, &QDialogButtonBox::accepted, hashCopyDialog, &QDialog::close); + boxLayout->addWidget(buttonBox); + + // set the new layout on the dialog + hashCopyDialog->setLayout(boxLayout); + + // show the new dialog + hashCopyDialog->show(); + } else { + // + } +} diff --git a/interface/src/ui/AssetUploadDialogFactory.h b/interface/src/ui/AssetUploadDialogFactory.h index 3d830aa4bb..0d3372cb03 100644 --- a/interface/src/ui/AssetUploadDialogFactory.h +++ b/interface/src/ui/AssetUploadDialogFactory.h @@ -16,6 +16,8 @@ #include +class AssetUpload; + class AssetUploadDialogFactory : public QObject { Q_OBJECT public: @@ -28,6 +30,8 @@ public: void setDialogParent(QWidget* dialogParent) { _dialogParent = dialogParent; } public slots: void showDialog(); +private slots: + void handleUploadFinished(AssetUpload* upload, const QString& hash); private: QWidget* _dialogParent { nullptr }; }; diff --git a/libraries/networking/src/AssetClient.cpp b/libraries/networking/src/AssetClient.cpp index 3877f05fe9..78ddd5e6e0 100644 --- a/libraries/networking/src/AssetClient.cpp +++ b/libraries/networking/src/AssetClient.cpp @@ -14,6 +14,7 @@ #include #include "AssetRequest.h" +#include "AssetUpload.h" #include "NodeList.h" #include "PacketReceiver.h" @@ -27,7 +28,7 @@ AssetClient::AssetClient() { packetReceiver.registerListener(PacketType::AssetUploadReply, this, "handleAssetUploadReply"); } -AssetRequest* AssetClient::create(QString hash) { +AssetRequest* AssetClient::createRequest(QString hash) { if (QThread::currentThread() != thread()) { AssetRequest* req; QMetaObject::invokeMethod(this, "create", @@ -46,15 +47,32 @@ AssetRequest* AssetClient::create(QString hash) { SharedNodePointer assetServer = nodeList->soloNodeOfType(NodeType::AssetServer); if (assetServer) { - auto assetClient = DependencyManager::get(); - auto request = new AssetRequest(assetClient.data(), hash); - - return request; + return new AssetRequest(this, hash); } return nullptr; } +AssetUpload* AssetClient::createUpload(QString filename) { + if (QThread::currentThread() != thread()) { + AssetUpload* upload; + QMetaObject::invokeMethod(this, "createUpload", + Qt::BlockingQueuedConnection, + Q_RETURN_ARG(AssetUpload*, upload), + Q_ARG(QString, filename)); + return upload; + } + + auto nodeList = DependencyManager::get(); + SharedNodePointer assetServer = nodeList->soloNodeOfType(NodeType::AssetServer); + + if (assetServer) { + return new AssetUpload(this, filename); + } + + return nullptr; +} + bool AssetClient::getAsset(QString hash, DataOffset start, DataOffset end, ReceivedAssetCallback callback) { if (hash.length() != HASH_HEX_LENGTH) { qDebug() << "Invalid hash size"; @@ -156,6 +174,7 @@ void AssetClient::handleAssetGetReply(QSharedPointer packetList, S bool AssetClient::uploadAsset(QByteArray data, QString extension, UploadResultCallback callback) { auto nodeList = DependencyManager::get(); SharedNodePointer assetServer = nodeList->soloNodeOfType(NodeType::AssetServer); + if (assetServer) { auto packetList = std::unique_ptr(new NLPacketList(PacketType::AssetUpload, QByteArray(), true, true)); diff --git a/libraries/networking/src/AssetClient.h b/libraries/networking/src/AssetClient.h index 5e26ff18b6..9bd3b98be6 100644 --- a/libraries/networking/src/AssetClient.h +++ b/libraries/networking/src/AssetClient.h @@ -21,6 +21,7 @@ #include "NLPacket.h" class AssetRequest; +class AssetUpload; struct AssetInfo { QString hash; @@ -36,11 +37,8 @@ class AssetClient : public QObject, public Dependency { public: AssetClient(); - Q_INVOKABLE AssetRequest* create(QString hash); - bool getAssetInfo(QString hash, GetInfoCallback callback); - bool getAsset(QString hash, DataOffset start, DataOffset end, ReceivedAssetCallback callback); - bool uploadAsset(QByteArray data, QString extension, UploadResultCallback callback); - bool abortDataRequest(MessageID messageID); + Q_INVOKABLE AssetRequest* createRequest(QString hash); + Q_INVOKABLE AssetUpload* createUpload(QString filename); private slots: void handleAssetGetInfoReply(QSharedPointer packet, SharedNodePointer senderNode); @@ -48,10 +46,18 @@ private slots: void handleAssetUploadReply(QSharedPointer packet, SharedNodePointer senderNode); private: + bool getAssetInfo(QString hash, GetInfoCallback callback); + bool getAsset(QString hash, DataOffset start, DataOffset end, ReceivedAssetCallback callback); + bool uploadAsset(QByteArray data, QString extension, UploadResultCallback callback); + bool abortDataRequest(MessageID messageID); + static MessageID _currentID; QHash _pendingRequests; QHash _pendingInfoRequests; QHash _pendingUploads; + + friend class AssetRequest; + friend class AssetUpload; }; #endif diff --git a/libraries/networking/src/AssetResourceRequest.cpp b/libraries/networking/src/AssetResourceRequest.cpp index c29a54a787..7c58a5cb9b 100644 --- a/libraries/networking/src/AssetResourceRequest.cpp +++ b/libraries/networking/src/AssetResourceRequest.cpp @@ -18,7 +18,7 @@ void ATPResourceRequest::doSend() { auto assetClient = DependencyManager::get(); auto hash = _url.path(); - auto request = assetClient->create(hash); + auto request = assetClient->createRequest(hash); if (!request) { return; diff --git a/libraries/networking/src/AssetUpload.cpp b/libraries/networking/src/AssetUpload.cpp new file mode 100644 index 0000000000..806709a6d6 --- /dev/null +++ b/libraries/networking/src/AssetUpload.cpp @@ -0,0 +1,53 @@ +// +// AssetUpload.cpp +// libraries/networking/src +// +// Created by Stephen Birarda on 2015-08-26. +// 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 "AssetUpload.h" + +#include +#include + +#include "AssetClient.h" + +AssetUpload::AssetUpload(QObject* object, const QString& filename) : + _filename(filename) +{ + +} + +void AssetUpload::start() { + if (QThread::currentThread() != thread()) { + QMetaObject::invokeMethod(this, "start", Qt::AutoConnection); + return; + } + + // try to open the file at the given filename + QFile file { _filename }; + + if (file.open(QIODevice::ReadOnly)) { + + // file opened, read the data and grab the extension + auto extension = QFileInfo(_filename).suffix(); + + auto data = file.readAll(); + + // ask the AssetClient to upload the asset and emit the proper signals from the passed callback + auto assetClient = DependencyManager::get(); + + assetClient->uploadAsset(data, extension, [this](bool result, QString hash){ + if (result) { + // successful upload - emit finished with a point to ourselves and the resulting hash + emit finished(this, hash); + } else { + + } + }); + } +} diff --git a/libraries/networking/src/AssetUpload.h b/libraries/networking/src/AssetUpload.h new file mode 100644 index 0000000000..c8921fcf84 --- /dev/null +++ b/libraries/networking/src/AssetUpload.h @@ -0,0 +1,47 @@ +// +// AssetUpload.h +// libraries/networking/src +// +// Created by Stephen Birarda on 2015-08-26. +// 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 +// + +#pragma once + +#ifndef hifi_AssetUpload_h +#define hifi_AssetUpload_h + +#include + +// You should be able to upload an asset from any thread, and handle the responses in a safe way +// on your own thread. Everything should happen on AssetClient's thread, the caller should +// receive events by connecting to signals on an object that lives on AssetClient's threads. + +class AssetUpload : public QObject { + Q_OBJECT +public: + + enum Result { + Success = 0, + Timeout, + TooLarge, + }; + + AssetUpload(QObject* parent, const QString& filename); + + Q_INVOKABLE void start(); + + const QString& getFilename() const { return _filename; } + +signals: + void finished(AssetUpload* upload, const QString& hash); + void progress(uint64_t totalReceived, uint64_t total); + +private: + QString _filename; +}; + +#endif // hifi_AssetUpload_h diff --git a/libraries/networking/src/AssetUtils.h b/libraries/networking/src/AssetUtils.h index c23b54de5d..55091d7a7e 100644 --- a/libraries/networking/src/AssetUtils.h +++ b/libraries/networking/src/AssetUtils.h @@ -24,4 +24,6 @@ enum AssetServerError : uint8_t { INVALID_BYTE_RANGE, }; +const QString ATP_SCHEME = "atp"; + #endif From f92ee597e5a4038929c2a6c462c107d35da7b9a0 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 27 Aug 2015 11:32:03 -0700 Subject: [PATCH 10/20] fix width of asset upload dialog to fit ATP url --- interface/src/ui/AssetUploadDialogFactory.cpp | 20 +++++++++++++++++-- libraries/networking/src/AssetClient.cpp | 5 ++++- libraries/networking/src/AssetRequest.cpp | 1 + 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/interface/src/ui/AssetUploadDialogFactory.cpp b/interface/src/ui/AssetUploadDialogFactory.cpp index a948ba54ba..8a431ac55f 100644 --- a/interface/src/ui/AssetUploadDialogFactory.cpp +++ b/interface/src/ui/AssetUploadDialogFactory.cpp @@ -46,6 +46,8 @@ void AssetUploadDialogFactory::showDialog() { // start the upload now upload->start(); + } else { + // TODO: show a QMessageBox to say that there is no local asset server } } } @@ -66,13 +68,27 @@ void AssetUploadDialogFactory::handleUploadFinished(AssetUpload* upload, const Q // set the label text (this shows above the text box) QLabel* lineEditLabel = new QLabel; - lineEditLabel->setText(QString("ATP URL for %1").arg(upload->getFilename())); + lineEditLabel->setText(QString("ATP URL for %1").arg(QFileInfo(upload->getFilename()).fileName())); // setup the line edit to hold the copiable text QLineEdit* lineEdit = new QLineEdit; + + QString atpURL = QString("%1://%2").arg(ATP_SCHEME).arg(hash); // set the ATP URL as the text value so it's copiable - lineEdit->insert(QString("%1://%2").arg(ATP_SCHEME).arg(hash)); + lineEdit->insert(atpURL); + + // figure out what size this line edit should be using font metrics + QFontMetrics textMetrics { lineEdit->font() }; + + // set the fixed width on the line edit + // pad it by 10 to cover the border and some extra space on the right side (for clicking) + static const int LINE_EDIT_RIGHT_PADDING { 10 }; + + lineEdit->setFixedWidth(textMetrics.width(atpURL) + LINE_EDIT_RIGHT_PADDING ); + + // left align the ATP URL line edit + lineEdit->home(true); // add the label and line edit to the dialog boxLayout->addWidget(lineEditLabel); diff --git a/libraries/networking/src/AssetClient.cpp b/libraries/networking/src/AssetClient.cpp index 8e4ae48078..b06593da0a 100644 --- a/libraries/networking/src/AssetClient.cpp +++ b/libraries/networking/src/AssetClient.cpp @@ -32,7 +32,7 @@ AssetClient::AssetClient() { AssetRequest* AssetClient::createRequest(QString hash) { if (QThread::currentThread() != thread()) { AssetRequest* req; - QMetaObject::invokeMethod(this, "create", + QMetaObject::invokeMethod(this, "createRequest", Qt::BlockingQueuedConnection, Q_RETURN_ARG(AssetRequest*, req), Q_ARG(QString, hash)); @@ -87,6 +87,9 @@ bool AssetClient::getAsset(QString hash, DataOffset start, DataOffset end, Recei auto packet = NLPacket::create(PacketType::AssetGet); auto messageID = ++_currentID; + + qDebug() << "Requesting data from" << start << "to" << end << "of" << hash << "from asset-server."; + packet->writePrimitive(messageID); packet->write(hash.toLatin1().constData(), HASH_HEX_LENGTH); packet->writePrimitive(start); diff --git a/libraries/networking/src/AssetRequest.cpp b/libraries/networking/src/AssetRequest.cpp index 9e77899961..1d9f21c329 100644 --- a/libraries/networking/src/AssetRequest.cpp +++ b/libraries/networking/src/AssetRequest.cpp @@ -48,6 +48,7 @@ void AssetRequest::start() { ++_numPendingRequests; auto start = i * CHUNK_SIZE; auto end = std::min((i + 1) * CHUNK_SIZE, info.size); + assetClient->getAsset(_hash, start, end, [this, start, end](bool success, QByteArray data) { Q_ASSERT(data.size() == (end - start)); From b3bd94c8db7aaf47ae8cba4e319fec9298c75e2a Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 27 Aug 2015 11:37:27 -0700 Subject: [PATCH 11/20] show a QMessageBox if not connected to asset-server --- interface/src/ui/AssetUploadDialogFactory.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/interface/src/ui/AssetUploadDialogFactory.cpp b/interface/src/ui/AssetUploadDialogFactory.cpp index 8a431ac55f..9e8d909a6e 100644 --- a/interface/src/ui/AssetUploadDialogFactory.cpp +++ b/interface/src/ui/AssetUploadDialogFactory.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -47,7 +48,11 @@ void AssetUploadDialogFactory::showDialog() { // start the upload now upload->start(); } else { - // TODO: show a QMessageBox to say that there is no local asset server + // show a QMessageBox to say that there is no local asset server + QString messageBoxText = QString("Could not upload \n\n%1\n\nbecause you are currently not connected" \ + " to a local asset-server.").arg(QFileInfo(filename.toString()).fileName()); + + QMessageBox::information(_dialogParent, "Failed to Upload", messageBoxText); } } } From c212ef5734f3894eb949d67fe67d863b6e73fbac Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 27 Aug 2015 12:01:24 -0700 Subject: [PATCH 12/20] fix PacketReceiver cleanup, return permission error for upload --- assignment-client/src/assets/AssetServer.cpp | 22 +++++++++++++++++++ interface/src/ui/AssetUploadDialogFactory.cpp | 20 +++++++++++++++-- libraries/networking/src/AssetUpload.cpp | 12 ++++++++-- libraries/networking/src/AssetUpload.h | 3 +++ libraries/networking/src/AssetUtils.h | 1 + libraries/networking/src/PacketReceiver.cpp | 14 ++++++------ 6 files changed, 61 insertions(+), 11 deletions(-) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index d1bea55f36..9085828d88 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -139,6 +139,28 @@ void AssetServer::handleAssetGet(QSharedPointer packet, SharedNodePoin } void AssetServer::handleAssetUpload(QSharedPointer packetList, SharedNodePointer senderNode) { + + if (!senderNode->getCanRez()) { + // this is a node the domain told us is not allowed to rez entities + // for now this also means it isn't allowed to add assets + // so return a packet with error that indicates that + + auto permissionErrorPacket = NLPacket::create(PacketType::AssetUploadReply, sizeof(MessageID) + sizeof(AssetServerError)); + + MessageID defaultMessageID; + + // write the default message ID and permission denied error + permissionErrorPacket->writePrimitive(defaultMessageID); + permissionErrorPacket->writePrimitive(AssetServerError::PERMISSION_DENIED); + + // send off the packet + auto nodeList = DependencyManager::get(); + nodeList->sendPacket(std::move(permissionErrorPacket), *senderNode); + + // return so we're not attempting to handle upload + return; + } + auto data = packetList->getMessage(); QBuffer buffer { &data }; buffer.open(QIODevice::ReadOnly); diff --git a/interface/src/ui/AssetUploadDialogFactory.cpp b/interface/src/ui/AssetUploadDialogFactory.cpp index 9e8d909a6e..6edf71e758 100644 --- a/interface/src/ui/AssetUploadDialogFactory.cpp +++ b/interface/src/ui/AssetUploadDialogFactory.cpp @@ -58,7 +58,7 @@ void AssetUploadDialogFactory::showDialog() { } void AssetUploadDialogFactory::handleUploadFinished(AssetUpload* upload, const QString& hash) { - if (true) { + if (upload->getResult() == AssetUpload::Success) { // show message box for successful upload, with copiable text for ATP hash QDialog* hashCopyDialog = new QDialog(_dialogParent); @@ -110,6 +110,22 @@ void AssetUploadDialogFactory::handleUploadFinished(AssetUpload* upload, const Q // show the new dialog hashCopyDialog->show(); } else { - // + // figure out the right error message for the message box + QString errorMessage = QString("Failed to upload %1.\n\n").arg(QFileInfo(upload->getFilename()).fileName()); + + switch (upload->getResult()) { + case AssetUpload::PermissionDenied: + errorMessage += "You do not have permission to upload content to this asset-server."; + break; + case AssetUpload::TooLarge: + errorMessage += "The uploaded content was too large and could not be stored in the asset-server."; + break; + default: + // not handled, do not show a message box + return; + } + + // display a message box with the error + QMessageBox::warning(_dialogParent, "Failed Upload", errorMessage); } } diff --git a/libraries/networking/src/AssetUpload.cpp b/libraries/networking/src/AssetUpload.cpp index 806709a6d6..b7c1fa1c34 100644 --- a/libraries/networking/src/AssetUpload.cpp +++ b/libraries/networking/src/AssetUpload.cpp @@ -41,12 +41,20 @@ void AssetUpload::start() { // ask the AssetClient to upload the asset and emit the proper signals from the passed callback auto assetClient = DependencyManager::get(); - assetClient->uploadAsset(data, extension, [this](bool result, QString hash){ - if (result) { + assetClient->uploadAsset(data, extension, [this](bool success, QString hash){ + if (success) { // successful upload - emit finished with a point to ourselves and the resulting hash + _result = Success; + emit finished(this, hash); } else { + // error during upload - emit finished with an empty hash + // callers can get the error from this object + // TODO: get the actual error from the callback + _result = PermissionDenied; + + emit finished(this, hash); } }); } diff --git a/libraries/networking/src/AssetUpload.h b/libraries/networking/src/AssetUpload.h index c8921fcf84..c969e4373e 100644 --- a/libraries/networking/src/AssetUpload.h +++ b/libraries/networking/src/AssetUpload.h @@ -28,6 +28,7 @@ public: Success = 0, Timeout, TooLarge, + PermissionDenied }; AssetUpload(QObject* parent, const QString& filename); @@ -35,6 +36,7 @@ public: Q_INVOKABLE void start(); const QString& getFilename() const { return _filename; } + const Result& getResult() const { return _result; } signals: void finished(AssetUpload* upload, const QString& hash); @@ -42,6 +44,7 @@ signals: private: QString _filename; + Result _result; }; #endif // hifi_AssetUpload_h diff --git a/libraries/networking/src/AssetUtils.h b/libraries/networking/src/AssetUtils.h index f484bcc49f..0cc71dab70 100644 --- a/libraries/networking/src/AssetUtils.h +++ b/libraries/networking/src/AssetUtils.h @@ -24,6 +24,7 @@ enum AssetServerError : uint8_t { ASSET_NOT_FOUND, INVALID_BYTE_RANGE, ASSET_TOO_LARGE, + PERMISSION_DENIED }; const QString ATP_SCHEME = "atp"; diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index 035432cf4b..03827d19af 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -217,22 +217,22 @@ void PacketReceiver::unregisterListener(QObject* listener) { auto it = _packetListenerMap.begin(); while (it != _packetListenerMap.end()) { - if (it->first == listener) { + if (it.value().first == listener) { it = _packetListenerMap.erase(it); + } else { + ++it; } - - ++it; } // clear any registrations for this listener in _packetListListener - auto listIt = _packetListenerMap.end(); + auto listIt = _packetListListenerMap.begin(); while (listIt != _packetListListenerMap.end()) { - if (listIt->first == listener) { + if (listIt.value().first == listener) { listIt = _packetListListenerMap.erase(listIt); + } else { + ++listIt; } - - ++listIt; } } From 16eee10cabfeb82e898a023bc3c2052c09e9369a Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 27 Aug 2015 12:13:20 -0700 Subject: [PATCH 13/20] repairs to permission error for asset-server upload --- assignment-client/src/assets/AssetServer.cpp | 20 +++++++++----------- interface/src/Menu.h | 2 +- libraries/networking/src/udt/Connection.cpp | 4 ++++ 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index 9085828d88..347e79fd5c 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -140,6 +140,13 @@ void AssetServer::handleAssetGet(QSharedPointer packet, SharedNodePoin void AssetServer::handleAssetUpload(QSharedPointer packetList, SharedNodePointer senderNode) { + auto data = packetList->getMessage(); + QBuffer buffer { &data }; + buffer.open(QIODevice::ReadOnly); + + MessageID messageID; + buffer.read(reinterpret_cast(&messageID), sizeof(messageID)); + if (!senderNode->getCanRez()) { // this is a node the domain told us is not allowed to rez entities // for now this also means it isn't allowed to add assets @@ -147,10 +154,8 @@ void AssetServer::handleAssetUpload(QSharedPointer packetList, Sha auto permissionErrorPacket = NLPacket::create(PacketType::AssetUploadReply, sizeof(MessageID) + sizeof(AssetServerError)); - MessageID defaultMessageID; - - // write the default message ID and permission denied error - permissionErrorPacket->writePrimitive(defaultMessageID); + // write the message ID and a permission denied error + permissionErrorPacket->writePrimitive(messageID); permissionErrorPacket->writePrimitive(AssetServerError::PERMISSION_DENIED); // send off the packet @@ -160,13 +165,6 @@ void AssetServer::handleAssetUpload(QSharedPointer packetList, Sha // return so we're not attempting to handle upload return; } - - auto data = packetList->getMessage(); - QBuffer buffer { &data }; - buffer.open(QIODevice::ReadOnly); - - MessageID messageID; - buffer.read(reinterpret_cast(&messageID), sizeof(messageID)); uint8_t extensionLength; buffer.read(reinterpret_cast(&extensionLength), sizeof(extensionLength)); diff --git a/interface/src/Menu.h b/interface/src/Menu.h index 948e159b04..7dfd1bc8d8 100644 --- a/interface/src/Menu.h +++ b/interface/src/Menu.h @@ -288,7 +288,7 @@ namespace MenuOption { const QString ToolWindow = "Tool Window"; const QString TransmitterDrive = "Transmitter Drive"; const QString TurnWithHead = "Turn using Head"; - const QString UploadAsset = "Upload File to Local Asset Server"; + const QString UploadAsset = "Upload File to Asset Server"; const QString UseAudioForMouth = "Use Audio for Mouth"; const QString UseCamera = "Use Camera"; const QString VelocityFilter = "Velocity Filter"; diff --git a/libraries/networking/src/udt/Connection.cpp b/libraries/networking/src/udt/Connection.cpp index 1fa4111f85..61e2ac2cdf 100644 --- a/libraries/networking/src/udt/Connection.cpp +++ b/libraries/networking/src/udt/Connection.cpp @@ -655,6 +655,7 @@ void Connection::processTimeoutNAK(std::unique_ptr controlPacket) } void Connection::resetReceiveState() { + // reset all SequenceNumber member variables back to default SequenceNumber defaultSequenceNumber; @@ -665,6 +666,9 @@ void Connection::resetReceiveState() { _lastSentACK = defaultSequenceNumber; + // clear the sent ACKs + _sentACKs.clear(); + // clear the loss list and _lastNAKTime _lossList.clear(); _lastNAKTime = high_resolution_clock::time_point(); From 229015a5dac56e531fb611c616e94bc6e2bcc658 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 27 Aug 2015 12:21:38 -0700 Subject: [PATCH 14/20] don't allow asset-server upload if no server present --- interface/src/Application.cpp | 7 +++++++ interface/src/Menu.cpp | 12 +++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index d94cff19f1..28cd315c4c 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -3782,6 +3782,9 @@ void Application::nodeAdded(SharedNodePointer node) { if (node->getType() == NodeType::AvatarMixer) { // new avatar mixer, send off our identity packet right away _myAvatar->sendIdentityPacket(); + } else if (node->getType() == NodeType::AssetServer) { + // the addition of an asset-server always re-enables the upload to asset server menu option + Menu::getInstance()->getActionForOption(MenuOption::UploadAsset)->setEnabled(true); } } @@ -3829,6 +3832,10 @@ void Application::nodeKilled(SharedNodePointer node) { } else if (node->getType() == NodeType::AvatarMixer) { // our avatar mixer has gone away - clear the hash of avatars DependencyManager::get()->clearOtherAvatars(); + } else if (node->getType() == NodeType::AssetServer + && !DependencyManager::get()->soloNodeOfType(NodeType::AssetServer)) { + // this was our last asset server - disable the menu option to upload an asset + Menu::getInstance()->getActionForOption(MenuOption::UploadAsset)->setEnabled(false); } } diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index 6c41a3d7da..6d5eff8fa0 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -376,11 +376,13 @@ Menu::Menu() { auto& assetDialogFactory = AssetUploadDialogFactory::getInstance(); assetDialogFactory.setParent(this); - addActionToQMenuAndActionHash(assetDeveloperMenu, - MenuOption::UploadAsset, - 0, - &AssetUploadDialogFactory::getInstance(), - SLOT(showDialog())); + QAction* assetUpload = addActionToQMenuAndActionHash(assetDeveloperMenu, + MenuOption::UploadAsset, + 0, + &AssetUploadDialogFactory::getInstance(), + SLOT(showDialog())); + // disable the asset upload action by default - it gets enabled only if asset server becomes present + assetUpload->setEnabled(false); MenuWrapper* avatarDebugMenu = developerMenu->addMenu("Avatar"); From f238c1b167cd03fafa87bc68fcca60e1b3744a5c Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 27 Aug 2015 14:29:42 -0700 Subject: [PATCH 15/20] include the extension in upload confirmation --- interface/src/ui/AssetUploadDialogFactory.cpp | 2 +- libraries/networking/src/AssetUpload.cpp | 4 ++-- libraries/networking/src/AssetUpload.h | 2 ++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/interface/src/ui/AssetUploadDialogFactory.cpp b/interface/src/ui/AssetUploadDialogFactory.cpp index 6edf71e758..3073171563 100644 --- a/interface/src/ui/AssetUploadDialogFactory.cpp +++ b/interface/src/ui/AssetUploadDialogFactory.cpp @@ -78,7 +78,7 @@ void AssetUploadDialogFactory::handleUploadFinished(AssetUpload* upload, const Q // setup the line edit to hold the copiable text QLineEdit* lineEdit = new QLineEdit; - QString atpURL = QString("%1://%2").arg(ATP_SCHEME).arg(hash); + QString atpURL = QString("%1://%2.%3").arg(ATP_SCHEME).arg(hash).arg(upload->getExtension); // set the ATP URL as the text value so it's copiable lineEdit->insert(atpURL); diff --git a/libraries/networking/src/AssetUpload.cpp b/libraries/networking/src/AssetUpload.cpp index b7c1fa1c34..c9f5b804d4 100644 --- a/libraries/networking/src/AssetUpload.cpp +++ b/libraries/networking/src/AssetUpload.cpp @@ -34,14 +34,14 @@ void AssetUpload::start() { if (file.open(QIODevice::ReadOnly)) { // file opened, read the data and grab the extension - auto extension = QFileInfo(_filename).suffix(); + _extension = QFileInfo(_filename).suffix(); auto data = file.readAll(); // ask the AssetClient to upload the asset and emit the proper signals from the passed callback auto assetClient = DependencyManager::get(); - assetClient->uploadAsset(data, extension, [this](bool success, QString hash){ + assetClient->uploadAsset(data, _extension, [this](bool success, QString hash){ if (success) { // successful upload - emit finished with a point to ourselves and the resulting hash _result = Success; diff --git a/libraries/networking/src/AssetUpload.h b/libraries/networking/src/AssetUpload.h index c969e4373e..f55f4f9b71 100644 --- a/libraries/networking/src/AssetUpload.h +++ b/libraries/networking/src/AssetUpload.h @@ -36,6 +36,7 @@ public: Q_INVOKABLE void start(); const QString& getFilename() const { return _filename; } + const QString& getExtension() const { return _extension; } const Result& getResult() const { return _result; } signals: @@ -44,6 +45,7 @@ signals: private: QString _filename; + QString _extension; Result _result; }; From 26757eae7a4c28ce9fa713bec1faeaf1fa4dcd3b Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 27 Aug 2015 14:37:44 -0700 Subject: [PATCH 16/20] immediately show permission denied error if applicable --- interface/src/ui/AssetUploadDialogFactory.cpp | 63 ++++++++++++------- interface/src/ui/AssetUploadDialogFactory.h | 2 + 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/interface/src/ui/AssetUploadDialogFactory.cpp b/interface/src/ui/AssetUploadDialogFactory.cpp index 3073171563..89a6046d71 100644 --- a/interface/src/ui/AssetUploadDialogFactory.cpp +++ b/interface/src/ui/AssetUploadDialogFactory.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -32,29 +33,39 @@ AssetUploadDialogFactory::AssetUploadDialogFactory() { } +static const QString PERMISSION_DENIED_ERROR = "You do not have permission to upload content to this asset-server."; + void AssetUploadDialogFactory::showDialog() { - auto filename = QFileDialog::getOpenFileUrl(_dialogParent, "Select a file to upload"); + auto nodeList = DependencyManager::get(); - if (!filename.isEmpty()) { - qDebug() << "Selected filename for upload to asset-server: " << filename; + if (nodeList->getThisNodeCanRez()) { + auto filename = QFileDialog::getOpenFileUrl(_dialogParent, "Select a file to upload"); - auto assetClient = DependencyManager::get(); - auto upload = assetClient->createUpload(filename.path()); - - if (upload) { - // connect to the finished signal so we know when the AssetUpload is done - QObject::connect(upload, &AssetUpload::finished, this, &AssetUploadDialogFactory::handleUploadFinished); + if (!filename.isEmpty()) { + qDebug() << "Selected filename for upload to asset-server: " << filename; - // start the upload now - upload->start(); - } else { - // show a QMessageBox to say that there is no local asset server - QString messageBoxText = QString("Could not upload \n\n%1\n\nbecause you are currently not connected" \ - " to a local asset-server.").arg(QFileInfo(filename.toString()).fileName()); + auto assetClient = DependencyManager::get(); + auto upload = assetClient->createUpload(filename.path()); - QMessageBox::information(_dialogParent, "Failed to Upload", messageBoxText); + if (upload) { + // connect to the finished signal so we know when the AssetUpload is done + QObject::connect(upload, &AssetUpload::finished, this, &AssetUploadDialogFactory::handleUploadFinished); + + // start the upload now + upload->start(); + } else { + // show a QMessageBox to say that there is no local asset server + QString messageBoxText = QString("Could not upload \n\n%1\n\nbecause you are currently not connected" \ + " to a local asset-server.").arg(QFileInfo(filename.toString()).fileName()); + + QMessageBox::information(_dialogParent, "Failed to Upload", messageBoxText); + } } + } else { + // we don't have permission to upload to asset server in this domain - show the permission denied error + showErrorDialog(QString(), PERMISSION_DENIED_ERROR); } + } void AssetUploadDialogFactory::handleUploadFinished(AssetUpload* upload, const QString& hash) { @@ -78,7 +89,7 @@ void AssetUploadDialogFactory::handleUploadFinished(AssetUpload* upload, const Q // setup the line edit to hold the copiable text QLineEdit* lineEdit = new QLineEdit; - QString atpURL = QString("%1://%2.%3").arg(ATP_SCHEME).arg(hash).arg(upload->getExtension); + QString atpURL = QString("%1://%2.%3").arg(ATP_SCHEME).arg(hash).arg(upload->getExtension()); // set the ATP URL as the text value so it's copiable lineEdit->insert(atpURL); @@ -111,14 +122,14 @@ void AssetUploadDialogFactory::handleUploadFinished(AssetUpload* upload, const Q hashCopyDialog->show(); } else { // figure out the right error message for the message box - QString errorMessage = QString("Failed to upload %1.\n\n").arg(QFileInfo(upload->getFilename()).fileName()); + QString additionalError; switch (upload->getResult()) { case AssetUpload::PermissionDenied: - errorMessage += "You do not have permission to upload content to this asset-server."; + additionalError = PERMISSION_DENIED_ERROR; break; case AssetUpload::TooLarge: - errorMessage += "The uploaded content was too large and could not be stored in the asset-server."; + additionalError = "The uploaded content was too large and could not be stored in the asset-server."; break; default: // not handled, do not show a message box @@ -126,6 +137,16 @@ void AssetUploadDialogFactory::handleUploadFinished(AssetUpload* upload, const Q } // display a message box with the error - QMessageBox::warning(_dialogParent, "Failed Upload", errorMessage); + showErrorDialog(QFileInfo(upload->getFilename()).fileName(), additionalError); } } + +void AssetUploadDialogFactory::showErrorDialog(const QString& filename, const QString& additionalError) { + QString errorMessage; + + if (!filename.isEmpty()) { + errorMessage += QString("Failed to upload %1.\n\n").arg(filename); + } + + QMessageBox::warning(_dialogParent, "Failed Upload", errorMessage); +} diff --git a/interface/src/ui/AssetUploadDialogFactory.h b/interface/src/ui/AssetUploadDialogFactory.h index 0d3372cb03..ae7f103437 100644 --- a/interface/src/ui/AssetUploadDialogFactory.h +++ b/interface/src/ui/AssetUploadDialogFactory.h @@ -33,6 +33,8 @@ public slots: private slots: void handleUploadFinished(AssetUpload* upload, const QString& hash); private: + void showErrorDialog(const QString& filename, const QString& additionalError); + QWidget* _dialogParent { nullptr }; }; From 59775e5756f24752f0379e80f101d3f935b1ad91 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 27 Aug 2015 14:40:12 -0700 Subject: [PATCH 17/20] actually add the additional error to the dialog --- interface/src/ui/AssetUploadDialogFactory.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/interface/src/ui/AssetUploadDialogFactory.cpp b/interface/src/ui/AssetUploadDialogFactory.cpp index 89a6046d71..16d966fe66 100644 --- a/interface/src/ui/AssetUploadDialogFactory.cpp +++ b/interface/src/ui/AssetUploadDialogFactory.cpp @@ -148,5 +148,7 @@ void AssetUploadDialogFactory::showErrorDialog(const QString& filename, const QS errorMessage += QString("Failed to upload %1.\n\n").arg(filename); } + errorMessage += additionalError; + QMessageBox::warning(_dialogParent, "Failed Upload", errorMessage); } From 777e6c3c2d29165c771a948afd1f6d70c0a478fe Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 27 Aug 2015 15:03:13 -0700 Subject: [PATCH 18/20] use existing AssetUploadDialogFactory ref --- interface/src/Menu.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index 230cfacf7a..abe08f58b2 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -362,8 +362,9 @@ Menu::Menu() { QAction* assetUpload = addActionToQMenuAndActionHash(assetDeveloperMenu, MenuOption::UploadAsset, 0, - &AssetUploadDialogFactory::getInstance(), + &assetDialogFactory, SLOT(showDialog())); + // disable the asset upload action by default - it gets enabled only if asset server becomes present assetUpload->setEnabled(false); From 46990184d66e77691972fa7e31c4b0f9d0e6c4c4 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 27 Aug 2015 15:07:15 -0700 Subject: [PATCH 19/20] AssetUploadDialogFactory constructor should be private --- interface/src/ui/AssetUploadDialogFactory.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/interface/src/ui/AssetUploadDialogFactory.h b/interface/src/ui/AssetUploadDialogFactory.h index ae7f103437..50980862e6 100644 --- a/interface/src/ui/AssetUploadDialogFactory.h +++ b/interface/src/ui/AssetUploadDialogFactory.h @@ -21,7 +21,6 @@ class AssetUpload; class AssetUploadDialogFactory : public QObject { Q_OBJECT public: - AssetUploadDialogFactory(); AssetUploadDialogFactory(const AssetUploadDialogFactory& other) = delete; AssetUploadDialogFactory& operator=(const AssetUploadDialogFactory& rhs) = delete; @@ -33,6 +32,8 @@ public slots: private slots: void handleUploadFinished(AssetUpload* upload, const QString& hash); private: + AssetUploadDialogFactory(); + void showErrorDialog(const QString& filename, const QString& additionalError); QWidget* _dialogParent { nullptr }; From 549c514400a203270f25bdcb729c2dc1dfdf4b64 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 27 Aug 2015 15:12:08 -0700 Subject: [PATCH 20/20] fix upload leak, return error for failed load --- interface/src/ui/AssetUploadDialogFactory.cpp | 5 +++++ libraries/networking/src/AssetUpload.cpp | 6 ++++++ libraries/networking/src/AssetUpload.h | 3 ++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/interface/src/ui/AssetUploadDialogFactory.cpp b/interface/src/ui/AssetUploadDialogFactory.cpp index 16d966fe66..e163947c7c 100644 --- a/interface/src/ui/AssetUploadDialogFactory.cpp +++ b/interface/src/ui/AssetUploadDialogFactory.cpp @@ -131,6 +131,9 @@ void AssetUploadDialogFactory::handleUploadFinished(AssetUpload* upload, const Q case AssetUpload::TooLarge: additionalError = "The uploaded content was too large and could not be stored in the asset-server."; break; + case AssetUpload::ErrorLoadingFile: + additionalError = "The file could not be opened. Please check your permissions and try again."; + break; default: // not handled, do not show a message box return; @@ -139,6 +142,8 @@ void AssetUploadDialogFactory::handleUploadFinished(AssetUpload* upload, const Q // display a message box with the error showErrorDialog(QFileInfo(upload->getFilename()).fileName(), additionalError); } + + upload->deleteLater(); } void AssetUploadDialogFactory::showErrorDialog(const QString& filename, const QString& additionalError) { diff --git a/libraries/networking/src/AssetUpload.cpp b/libraries/networking/src/AssetUpload.cpp index c9f5b804d4..780d062084 100644 --- a/libraries/networking/src/AssetUpload.cpp +++ b/libraries/networking/src/AssetUpload.cpp @@ -57,5 +57,11 @@ void AssetUpload::start() { emit finished(this, hash); } }); + } else { + // we couldn't open the file - set the error result + _result = ErrorLoadingFile; + + // emit that we are done + emit finished(this, QString()); } } diff --git a/libraries/networking/src/AssetUpload.h b/libraries/networking/src/AssetUpload.h index f55f4f9b71..a678c0dc7e 100644 --- a/libraries/networking/src/AssetUpload.h +++ b/libraries/networking/src/AssetUpload.h @@ -28,7 +28,8 @@ public: Success = 0, Timeout, TooLarge, - PermissionDenied + PermissionDenied, + ErrorLoadingFile }; AssetUpload(QObject* parent, const QString& filename);