Merge pull request #91 from birarda/atp-mappings

add short-circuiting for invalid paths or hashes
This commit is contained in:
Clément Brisset 2016-03-10 12:46:41 -08:00
commit fc0675d0b8
14 changed files with 265 additions and 227 deletions

View file

@ -482,7 +482,7 @@ void AssetServer::loadMappingsFromFile() {
}
}
qCritical() << "Failed to read mapping file at" << mapFilePath << "- assignment with not continue.";
qCritical() << "Failed to read mapping file at" << mapFilePath << "- assignment will not continue.";
setFinished(true);
} else {
qInfo() << "No existing mappings loaded from file since no file was found at" << mapFilePath;
@ -604,7 +604,7 @@ bool AssetServer::deleteMappings(const AssetPathList& paths) {
}
bool AssetServer::renameMapping(const AssetPath& oldPath, const AssetPath& newPath) {
if (oldPath[0] != '/' || newPath[0] != '/') {
if (!isValidPath(oldPath) || !isValidPath(newPath)) {
qWarning() << "Cannot perform rename with invalid paths - both should have leading forward slashes:"
<< oldPath << "=>" << newPath;

View file

@ -33,7 +33,8 @@ Window {
HifiConstants { id: hifi }
property var scripts: ScriptDiscoveryService;
property var assetMappingsModel: Assets.proxyModel;
property var assetProxyModel: Assets.proxyModel;
property var assetMappingsModel: Assets.mappingModel;
property var currentDirectory;
property alias currentFileUrl: fileUrlTextField.text;
@ -44,7 +45,10 @@ Window {
property alias directory: root.currentDirectory
}
Component.onCompleted: reload()
Component.onCompleted: {
assetMappingsModel.errorGettingMappings.connect(handleGetMappingsError)
reload()
}
function doDeleteFile(path) {
console.log("Deleting " + path);
@ -103,7 +107,7 @@ Window {
function canAddToWorld() {
var supportedExtensions = [/\.fbx\b/i, /\.obj\b/i];
var path = assetMappingsModel.data(treeView.currentIndex, 0x100);
var path = assetProxyModel.data(treeView.currentIndex, 0x100);
return supportedExtensions.reduce(function(total, current) {
return total | new RegExp(current).test(path);
@ -114,6 +118,12 @@ Window {
print("reload");
Assets.mappingModel.refresh();
}
function handleGetMappingsError() {
errorMessage("There was a problem retreiving the list of assets from your Asset Server.\n"
+ "Please make sure you are connected to the Asset Server and try again. ");
}
function addToWorld() {
var url = assetMappingsModel.data(treeView.currentIndex, 0x102);
if (!url) {
@ -124,7 +134,7 @@ Window {
}
function copyURLToClipboard() {
var path = assetMappingsModel.data(treeView.currentIndex, 0x103);
var path = assetProxyModel.data(treeView.currentIndex, 0x103);
if (!path) {
return;
}
@ -132,7 +142,7 @@ Window {
}
function renameFile() {
var path = assetMappingsModel.data(treeView.currentIndex, 0x100);
var path = assetProxyModel.data(treeView.currentIndex, 0x100);
if (!path) {
return;
}
@ -156,12 +166,12 @@ Window {
});
}
function deleteFile() {
var path = assetMappingsModel.data(treeView.currentIndex, 0x100);
var path = assetProxyModel.data(treeView.currentIndex, 0x100);
if (!path) {
return;
}
var isFolder = assetMappingsModel.data(treeView.currentIndex, 0x101);
var isFolder = assetProxyModel.data(treeView.currentIndex, 0x101);
var typeString = isFolder ? 'folder' : 'file';
var object = desktop.messageBox({
@ -195,7 +205,7 @@ Window {
var fileUrl = fileUrlTextField.text
var addToWorld = addToWorldCheckBox.checked
var path = assetMappingsModel.data(treeView.currentIndex, 0x100);
var path = assetProxyModel.data(treeView.currentIndex, 0x100);
var directory = path ? path.slice(0, path.lastIndexOf('/') + 1) : "";
var filename = fileUrl.slice(fileUrl.lastIndexOf('/') + 1);
@ -297,7 +307,7 @@ Window {
HifiControls.Tree {
id: treeView
height: 400
treeModel: assetMappingsModel
treeModel: assetProxyModel
colorScheme: root.colorScheme
anchors.left: parent.left
anchors.right: parent.right

View file

@ -144,7 +144,6 @@ void ATPAssetMigrator::migrateResource(ResourceRequest* request) {
auto upload = assetClient->createUpload(request->getData());
if (upload) {
// add this URL to our hash of AssetUpload to original URL
_originalURLs.insert(upload, request->getUrl());
@ -155,13 +154,6 @@ void ATPAssetMigrator::migrateResource(ResourceRequest* request) {
// 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(assetInfo.fileName());
QMessageBox::information(_dialogParent, "Failed to Upload", messageBoxText);
}
}
void ATPAssetMigrator::assetUploadFinished(AssetUpload *upload, const QString& hash) {

View file

@ -45,19 +45,11 @@ void AssetUploadDialogFactory::showDialog() {
auto assetClient = DependencyManager::get<AssetClient>();
auto upload = assetClient->createUpload(filename);
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).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

View file

@ -138,67 +138,66 @@ bool haveAssetServer() {
return true;
}
GetMappingRequest* AssetClient::createGetMappingRequest(const AssetPath& path) {
return new GetMappingRequest(path);
}
GetAllMappingsRequest* AssetClient::createGetAllMappingsRequest() {
return new GetAllMappingsRequest();
auto request = new GetAllMappingsRequest();
request->moveToThread(thread());
return request;
}
DeleteMappingsRequest* AssetClient::createDeleteMappingsRequest(const AssetPathList& paths) {
return new DeleteMappingsRequest(paths);
auto request = new DeleteMappingsRequest(paths);
request->moveToThread(thread());
return request;
}
SetMappingRequest* AssetClient::createSetMappingRequest(const AssetPath& path, const AssetHash& hash) {
return new SetMappingRequest(path, hash);
auto request = new SetMappingRequest(path, hash);
request->moveToThread(thread());
return request;
}
RenameMappingRequest* AssetClient::createRenameMappingRequest(const AssetPath& oldPath, const AssetPath& newPath) {
return new RenameMappingRequest(oldPath, newPath);
auto request = new RenameMappingRequest(oldPath, newPath);
request->moveToThread(thread());
return request;
}
AssetRequest* AssetClient::createRequest(const AssetHash& hash) {
if (hash.length() != SHA256_HASH_HEX_LENGTH) {
qCWarning(asset_client) << "Invalid hash size";
return nullptr;
}
if (haveAssetServer()) {
auto request = new AssetRequest(hash);
// Move to the AssetClient thread in case we are not currently on that thread (which will usually be the case)
request->moveToThread(thread());
return request;
} else {
return nullptr;
}
}
AssetUpload* AssetClient::createUpload(const QString& filename) {
if (haveAssetServer()) {
auto upload = new AssetUpload(filename);
upload->moveToThread(thread());
return upload;
} else {
return nullptr;
}
}
AssetUpload* AssetClient::createUpload(const QByteArray& data) {
if (haveAssetServer()) {
auto upload = new AssetUpload(data);
upload->moveToThread(thread());
return upload;
} else {
return nullptr;
}
}
bool AssetClient::getAsset(const QString& hash, DataOffset start, DataOffset end,
@ -232,9 +231,12 @@ bool AssetClient::getAsset(const QString& hash, DataOffset start, DataOffset end
_pendingRequests[assetServer][messageID] = { callback, progressCallback };
return true;
} else {
callback(false, AssetServerError::NoError, QByteArray());
return false;
}
return false;
}
bool AssetClient::getAssetInfo(const QString& hash, GetInfoCallback callback) {
@ -255,10 +257,11 @@ bool AssetClient::getAssetInfo(const QString& hash, GetInfoCallback callback) {
_pendingInfoRequests[assetServer][messageID] = callback;
return true;
}
} else {
callback(false, AssetServerError::NoError, { "", 0 });
return false;
}
}
void AssetClient::handleAssetGetInfoReply(QSharedPointer<ReceivedMessage> message, SharedNodePointer senderNode) {
MessageID messageID;
@ -364,10 +367,11 @@ bool AssetClient::getAssetMapping(const AssetPath& path, MappingOperationCallbac
_pendingMappingRequests[assetServer][messageID] = callback;
return true;
}
} else {
callback(false, AssetServerError::NoError, QSharedPointer<ReceivedMessage>());
return false;
}
}
bool AssetClient::getAllAssetMappings(MappingOperationCallback callback) {
auto nodeList = DependencyManager::get<NodeList>();
@ -386,10 +390,11 @@ bool AssetClient::getAllAssetMappings(MappingOperationCallback callback) {
_pendingMappingRequests[assetServer][messageID] = callback;
return true;
}
} else {
callback(false, AssetServerError::NoError, QSharedPointer<ReceivedMessage>());
return false;
}
}
bool AssetClient::deleteAssetMappings(const AssetPathList& paths, MappingOperationCallback callback) {
auto nodeList = DependencyManager::get<NodeList>();
@ -414,10 +419,11 @@ bool AssetClient::deleteAssetMappings(const AssetPathList& paths, MappingOperati
_pendingMappingRequests[assetServer][messageID] = callback;
return true;
}
} else {
callback(false, AssetServerError::NoError, QSharedPointer<ReceivedMessage>());
return false;
}
}
bool AssetClient::setAssetMapping(const QString& path, const AssetHash& hash, MappingOperationCallback callback) {
auto nodeList = DependencyManager::get<NodeList>();
@ -439,10 +445,11 @@ bool AssetClient::setAssetMapping(const QString& path, const AssetHash& hash, Ma
_pendingMappingRequests[assetServer][messageID] = callback;
return true;
}
} else {
callback(false, AssetServerError::NoError, QSharedPointer<ReceivedMessage>());
return false;
}
}
bool AssetClient::renameAssetMapping(const AssetPath& oldPath, const AssetPath& newPath, MappingOperationCallback callback) {
auto nodeList = DependencyManager::get<NodeList>();
@ -465,10 +472,11 @@ bool AssetClient::renameAssetMapping(const AssetPath& oldPath, const AssetPath&
return true;
}
} else {
callback(false, AssetServerError::NoError, QSharedPointer<ReceivedMessage>());
return false;
}
}
bool AssetClient::uploadAsset(const QByteArray& data, UploadResultCallback callback) {
auto nodeList = DependencyManager::get<NodeList>();
@ -489,9 +497,11 @@ bool AssetClient::uploadAsset(const QByteArray& data, UploadResultCallback callb
_pendingUploads[assetServer][messageID] = callback;
return true;
}
} else {
callback(false, AssetServerError::NoError, QString());
return false;
}
}
void AssetClient::handleAssetUploadReply(QSharedPointer<ReceivedMessage> message, SharedNodePointer senderNode) {
MessageID messageID;

View file

@ -36,6 +36,15 @@ void AssetRequest::start() {
return;
}
// in case we haven't parsed a valid hash, return an error now
if (!isValidHash(_hash)) {
_error = InvalidHash;
_state = Finished;
emit finished(this);
return;
}
// Try to load from cache
_data = loadFromCache(getUrl());
if (!_data.isNull()) {

View file

@ -34,6 +34,7 @@ public:
NoError,
NotFound,
InvalidByteRange,
InvalidHash,
HashVerificationFailed,
NetworkError,
UnknownError

View file

@ -58,15 +58,6 @@ void AssetResourceRequest::requestMappingForPath(const AssetPath& path) {
auto assetClient = DependencyManager::get<AssetClient>();
_assetMappingRequest = assetClient->createGetMappingRequest(path);
// if we get a nullptr for createGetMappingRequest assume that there is no currently available asset-server
if (!_assetMappingRequest) {
_result = ServerUnavailable;
_state = Finished;
emit finished();
return;
}
// make sure we'll hear about the result of the get mapping request
connect(_assetMappingRequest, &GetMappingRequest::finished, this, [this, path](GetMappingRequest* request){
Q_ASSERT(_state == InProgress);
@ -80,18 +71,20 @@ void AssetResourceRequest::requestMappingForPath(const AssetPath& path) {
requestHash(request->getHash());
break;
default: {
switch (request->getError()) {
case MappingRequest::NotFound:
// no result for the mapping request, set error to not found
_result = NotFound;
// since we've failed we know we are finished
_state = Finished;
emit finished();
break;
case MappingRequest::NetworkError:
// didn't hear back from the server, mark it unavailable
_result = ServerUnavailable;
break;
default:
// these are unexpected errors for a GetMappingRequest object
_result = Error;
break;
}
// since we've failed we know we are finished
_state = Finished;
@ -99,6 +92,7 @@ void AssetResourceRequest::requestMappingForPath(const AssetPath& path) {
break;
}
}
_assetMappingRequest->deleteLater();
_assetMappingRequest = nullptr;
@ -109,27 +103,10 @@ void AssetResourceRequest::requestMappingForPath(const AssetPath& path) {
void AssetResourceRequest::requestHash(const AssetHash& hash) {
// in case we haven't parsed a valid hash, return an error now
if (hash.length() != SHA256_HASH_HEX_LENGTH) {
_result = InvalidURL;
_state = Finished;
emit finished();
return;
}
// Make request to atp
auto assetClient = DependencyManager::get<AssetClient>();
_assetRequest = assetClient->createRequest(hash);
if (!_assetRequest) {
_result = ServerUnavailable;
_state = Finished;
emit finished();
return;
}
connect(_assetRequest, &AssetRequest::progress, this, &AssetResourceRequest::progress);
connect(_assetRequest, &AssetRequest::finished, this, [this](AssetRequest* req) {
Q_ASSERT(_state == InProgress);
@ -141,6 +118,9 @@ void AssetResourceRequest::requestHash(const AssetHash& hash) {
_data = req->getData();
_result = Success;
break;
case AssetRequest::InvalidHash:
_result = InvalidURL;
break;
case AssetRequest::Error::NotFound:
_result = NotFound;
break;

View file

@ -41,7 +41,7 @@ QString AssetUpload::getErrorString() const {
case AssetUpload::FileOpenError:
return "The file could not be opened. Please check your permissions and try again.";
case AssetUpload::NetworkError:
return "The file could not be opened. Please check your network connectivity.";
return "There was a problem reaching your Asset Server. Please check your network connectivity.";
default:
// not handled, do not show a message box
return QString();

View file

@ -30,6 +30,13 @@ GetMappingRequest::GetMappingRequest(const AssetPath& path) : _path(path) {
void GetMappingRequest::doStart() {
// short circuit the request if the path is invalid
if (!isValidPath(_path)) {
_error = MappingRequest::InvalidPath;
emit finished(this);
return;
}
auto assetClient = DependencyManager::get<AssetClient>();
assetClient->getAssetMapping(_path, [this, assetClient](bool responseReceived, AssetServerError error, QSharedPointer<ReceivedMessage> message) {
@ -89,11 +96,26 @@ void GetAllMappingsRequest::doStart() {
});
};
SetMappingRequest::SetMappingRequest(const AssetPath& path, const AssetHash& hash) : _path(path), _hash(hash) {
SetMappingRequest::SetMappingRequest(const AssetPath& path, const AssetHash& hash) :
_path(path),
_hash(hash)
{
};
void SetMappingRequest::doStart() {
// short circuit the request if the hash or path are invalid
auto validPath = isValidPath(_path);
auto validHash = isValidHash(_hash);
if (!validPath || !validHash) {
_error = validPath ? MappingRequest::InvalidPath : MappingRequest::InvalidHash;
emit finished(this);
return;
}
auto assetClient = DependencyManager::get<AssetClient>();
assetClient->setAssetMapping(_path, _hash, [this, assetClient](bool responseReceived, AssetServerError error, QSharedPointer<ReceivedMessage> message) {
if (!responseReceived) {
_error = NetworkError;
@ -119,7 +141,18 @@ DeleteMappingsRequest::DeleteMappingsRequest(const AssetPathList& paths) : _path
};
void DeleteMappingsRequest::doStart() {
// short circuit the request if any of the paths are invalid
for (auto& path : _paths) {
if (!isValidPath(path)) {
_error = MappingRequest::InvalidPath;
emit finished(this);
return;
}
}
auto assetClient = DependencyManager::get<AssetClient>();
assetClient->deleteAssetMappings(_paths, [this, assetClient](bool responseReceived, AssetServerError error, QSharedPointer<ReceivedMessage> message) {
if (!responseReceived) {
_error = NetworkError;
@ -149,7 +182,16 @@ _newPath(newPath)
}
void RenameMappingRequest::doStart() {
// short circuit the request if either of the paths are invalid
if (!isValidPath(_oldPath) || !isValidPath(_newPath)) {
_error = InvalidPath;
emit finished(this);
return;
}
auto assetClient = DependencyManager::get<AssetClient>();
assetClient->renameAssetMapping(_oldPath, _newPath, [this, assetClient](bool responseReceived,
AssetServerError error,
QSharedPointer<ReceivedMessage> message) {

View file

@ -26,6 +26,8 @@ public:
NotFound,
NetworkError,
PermissionDenied,
InvalidPath,
InvalidHash,
UnknownError
};

View file

@ -135,6 +135,7 @@ void AssetMappingModel::refresh() {
auto request = assetClient->createGetAllMappingsRequest();
connect(request, &GetAllMappingsRequest::finished, this, [this](GetAllMappingsRequest* request) mutable {
if (request->getError() == MappingRequest::NoError) {
auto mappings = request->getMappings();
auto existingPaths = _pathToItemMap.keys();
for (auto& mapping : mappings) {
@ -219,6 +220,9 @@ void AssetMappingModel::refresh() {
}
//removeitem->index();
}
} else {
emit errorGettingMappings(uint8_t(request->getError()));
}
});
request->start();

View file

@ -42,6 +42,9 @@
bool isKnownMapping(QString path) const { return _pathToItemMap.contains(path); };
signals:
void errorGettingMappings(uint8_t error);
private:
QHash<QString, QStandardItem*> _pathToItemMap;
};
@ -65,6 +68,7 @@ public:
Q_INVOKABLE void deleteMapping(QString path, QJSValue callback) { deleteMappings(QStringList(path), callback); }
Q_INVOKABLE void getAllMappings(QJSValue callback);
Q_INVOKABLE void renameMapping(QString oldPath, QString newPath, QJSValue callback);
protected:
QSet<AssetRequest*> _pendingRequests;
AssetMappingModel _assetMappingModel;

View file

@ -27,14 +27,10 @@ AssetScriptingInterface::AssetScriptingInterface(QScriptEngine* engine) :
void AssetScriptingInterface::uploadData(QString data, QScriptValue callback) {
QByteArray dataByteArray = data.toUtf8();
auto upload = DependencyManager::get<AssetClient>()->createUpload(dataByteArray);
if (!upload) {
qCWarning(asset_client) << "Error uploading file to asset server";
return;
}
QObject::connect(upload, &AssetUpload::finished, this, [this, callback](AssetUpload* upload, const QString& hash) mutable {
if (callback.isFunction()) {
QString url = "atp://" + hash;
QString url = "atp:" + hash;
QScriptValueList args { url };
callback.call(_engine->currentContext()->thisObject(), args);
}
@ -43,7 +39,7 @@ void AssetScriptingInterface::uploadData(QString data, QScriptValue callback) {
}
void AssetScriptingInterface::downloadData(QString urlString, QScriptValue callback) {
const QString ATP_SCHEME { "atp://" };
const QString ATP_SCHEME { "atp:" };
if (!urlString.startsWith(ATP_SCHEME)) {
return;
@ -61,10 +57,6 @@ void AssetScriptingInterface::downloadData(QString urlString, QScriptValue callb
auto assetClient = DependencyManager::get<AssetClient>();
auto assetRequest = assetClient->createRequest(hash);
if (!assetRequest) {
return;
}
_pendingRequests << assetRequest;
connect(assetRequest, &AssetRequest::finished, this, [this, callback](AssetRequest* request) mutable {