From 882a0272c08dc8fd0cccdff3b824136f3279cd11 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 31 Mar 2016 09:54:30 -0700 Subject: [PATCH 1/7] add simple cleanup of unmapped assets on start --- assignment-client/src/assets/AssetServer.cpp | 30 ++++++++++++++++++++ assignment-client/src/assets/AssetServer.h | 3 ++ 2 files changed, 33 insertions(+) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index fad8ece7bf..33ae631b77 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -136,6 +136,8 @@ void AssetServer::completeSetup() { performMappingMigration(); + cleanupUnmappedFiles(); + nodeList->addNodeTypeToInterestSet(NodeType::Agent); } @@ -189,6 +191,34 @@ void AssetServer::performMappingMigration() { } } +void AssetServer::cleanupUnmappedFiles() { + QRegExp hashFileRegex { "^[a-f0-9]{" + QString::number(SHA256_HASH_HEX_LENGTH) + "}" }; + + auto files = _filesDirectory.entryInfoList(QDir::Files); + + // grab the currently mapped hashes + auto mappedHashes = _fileMappings.values(); + + qDebug() << "Performing unmapped asset cleanup."; + + for (const auto& fileInfo : files) { + if (hashFileRegex.exactMatch(fileInfo.fileName())) { + if (mappedHashes.contains(fileInfo.fileName())) { + qDebug() << "\tLeaving" << fileInfo.fileName() << "in asset files directory since it is mapped"; + } else { + // remove the unmapped file + QFile removeableFile { fileInfo.absoluteFilePath() }; + + if (removeableFile.remove()) { + qDebug() << "\tDeleted" << fileInfo.fileName() << "from asset files directory since it is unmapped."; + } else { + qDebug() << "\tAttempt to delete unmapped file" << fileInfo.fileName() << "failed"; + } + } + } + } +} + void AssetServer::handleAssetMappingOperation(QSharedPointer message, SharedNodePointer senderNode) { MessageID messageID; message->readPrimitive(&messageID); diff --git a/assignment-client/src/assets/AssetServer.h b/assignment-client/src/assets/AssetServer.h index 1a8ebed50b..7e7c3c8b69 100644 --- a/assignment-client/src/assets/AssetServer.h +++ b/assignment-client/src/assets/AssetServer.h @@ -62,6 +62,9 @@ private: void performMappingMigration(); + // deletes any unmapped files from the local asset directory + void cleanupUnmappedFiles(); + Mappings _fileMappings; QDir _resourcesDirectory; From 4d0976f7306756a9a82806634beae2ed5c444d34 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 11 Apr 2016 08:41:39 -0700 Subject: [PATCH 2/7] remove migration code for pre-mapping migrations --- assignment-client/src/assets/AssetServer.cpp | 52 -------------------- assignment-client/src/assets/AssetServer.h | 2 - 2 files changed, 54 deletions(-) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index 33ae631b77..d9af9f1019 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -134,63 +134,11 @@ void AssetServer::completeSetup() { qInfo() << "There are" << hashedFiles.size() << "asset files in the asset directory."; - performMappingMigration(); - cleanupUnmappedFiles(); nodeList->addNodeTypeToInterestSet(NodeType::Agent); } -void AssetServer::performMappingMigration() { - QRegExp hashFileRegex { "^[a-f0-9]{" + QString::number(SHA256_HASH_HEX_LENGTH) + "}(\\.[\\w]+)+$" }; - - auto files = _resourcesDirectory.entryInfoList(QDir::Files); - - for (const auto& fileInfo : files) { - if (hashFileRegex.exactMatch(fileInfo.fileName())) { - // we have a pre-mapping file that we should migrate to the new mapping system - qDebug() << "Migrating pre-mapping file" << fileInfo.fileName(); - - // rename the file to the same name with no extension - QFile oldFile { fileInfo.absoluteFilePath() }; - - auto oldAbsolutePath = fileInfo.absoluteFilePath(); - auto oldFilename = fileInfo.fileName(); - auto hash = oldFilename.left(SHA256_HASH_HEX_LENGTH); - auto fullExtension = oldFilename.mid(oldFilename.indexOf('.')); - - qDebug() << "\tMoving" << oldAbsolutePath << "to" << oldAbsolutePath.replace(fullExtension, ""); - - bool renamed = oldFile.copy(_filesDirectory.filePath(hash)); - if (!renamed) { - qWarning() << "\tCould not migrate pre-mapping file" << fileInfo.fileName(); - } else { - qDebug() << "\tRenamed pre-mapping file" << fileInfo.fileName(); - - // add a new mapping with the old extension and a truncated version of the hash - const int TRUNCATED_HASH_NUM_CHAR = 16; - auto fakeFileName = "/" + hash.left(TRUNCATED_HASH_NUM_CHAR) + fullExtension; - - qDebug() << "\tAdding a migration mapping from" << fakeFileName << "to" << hash; - - auto it = _fileMappings.find(fakeFileName); - if (it == _fileMappings.end()) { - _fileMappings[fakeFileName] = hash; - - if (writeMappingsToFile()) { - // mapping added and persisted, we can remove the migrated file - oldFile.remove(); - qDebug() << "\tMigration completed for" << oldFilename; - } - } else { - qDebug() << "\tCould not add migration mapping for" << hash << "since a mapping for" << fakeFileName - << "already exists."; - } - } - } - } -} - void AssetServer::cleanupUnmappedFiles() { QRegExp hashFileRegex { "^[a-f0-9]{" + QString::number(SHA256_HASH_HEX_LENGTH) + "}" }; diff --git a/assignment-client/src/assets/AssetServer.h b/assignment-client/src/assets/AssetServer.h index 7e7c3c8b69..a0a85f8fd5 100644 --- a/assignment-client/src/assets/AssetServer.h +++ b/assignment-client/src/assets/AssetServer.h @@ -60,8 +60,6 @@ private: /// Rename mapping from `oldPath` to `newPath`. Returns true if successful bool renameMapping(AssetPath oldPath, AssetPath newPath); - void performMappingMigration(); - // deletes any unmapped files from the local asset directory void cleanupUnmappedFiles(); From fae9b061a3e4036a5afb039bb1e0b3d899e15943 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 11 Apr 2016 08:42:52 -0700 Subject: [PATCH 3/7] promote a debug to info --- assignment-client/src/assets/AssetServer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index d9af9f1019..0542da45d7 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -147,7 +147,7 @@ void AssetServer::cleanupUnmappedFiles() { // grab the currently mapped hashes auto mappedHashes = _fileMappings.values(); - qDebug() << "Performing unmapped asset cleanup."; + qInfo() << "Performing unmapped asset cleanup."; for (const auto& fileInfo : files) { if (hashFileRegex.exactMatch(fileInfo.fileName())) { From 775898893bf54700118db301a915a600a3b81c43 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 11 Apr 2016 08:57:07 -0700 Subject: [PATCH 4/7] add deletion of unmapped files during delete op --- assignment-client/src/assets/AssetServer.cpp | 29 ++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index 0542da45d7..00fbdf7ab7 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -544,6 +544,8 @@ bool AssetServer::deleteMappings(AssetPathList& paths) { // take a copy of the current mappings in case persistence of these deletes fails auto oldMappings = _fileMappings; + QSet hashesToCheck; + // enumerate the paths to delete and remove them all for (auto& path : paths) { @@ -557,6 +559,9 @@ bool AssetServer::deleteMappings(AssetPathList& paths) { while (it != _fileMappings.end()) { if (it.key().startsWith(path)) { + // add this hash to the list we need to check for asset removal from the server + hashesToCheck << it.value().toString(); + it = _fileMappings.erase(it); } else { ++it; @@ -583,6 +588,30 @@ bool AssetServer::deleteMappings(AssetPathList& paths) { // deleted the old mappings, attempt to persist to file if (writeMappingsToFile()) { // persistence succeeded we are good to go + + // grab the current mapped hashes + auto mappedHashes = _fileMappings.values(); + + // enumerate the mapped hashes and clear the list of hashes to check for anything that's present + for (auto& hashVariant : mappedHashes) { + auto it = hashesToCheck.find(hashVariant.toString()); + if (it != hashesToCheck.end()) { + hashesToCheck.erase(it); + } + } + + // we now have a set of hashes that are unmapped - we will delete those asset files + for (auto& hash : hashesToCheck) { + // remove the unmapped file + QFile removeableFile { _filesDirectory.absoluteFilePath(hash) }; + + if (removeableFile.remove()) { + qDebug() << "\tDeleted" << hash << "from asset files directory since it is now unmapped."; + } else { + qDebug() << "\tAttempt to delete unmapped file" << hash << "failed"; + } + } + return true; } else { qWarning() << "Failed to persist deleted mappings, rolling back"; From 334dc3cb6c1a25cbf9394ef73fac98a2fa690991 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 11 Apr 2016 09:05:11 -0700 Subject: [PATCH 5/7] cleanup left file debug, add check for deleted mapping --- assignment-client/src/assets/AssetServer.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index 00fbdf7ab7..a06163c396 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -151,9 +151,7 @@ void AssetServer::cleanupUnmappedFiles() { for (const auto& fileInfo : files) { if (hashFileRegex.exactMatch(fileInfo.fileName())) { - if (mappedHashes.contains(fileInfo.fileName())) { - qDebug() << "\tLeaving" << fileInfo.fileName() << "in asset files directory since it is mapped"; - } else { + if (!mappedHashes.contains(fileInfo.fileName())) { // remove the unmapped file QFile removeableFile { fileInfo.absoluteFilePath() }; @@ -578,6 +576,9 @@ bool AssetServer::deleteMappings(AssetPathList& paths) { } else { auto oldMapping = _fileMappings.take(path); if (!oldMapping.isNull()) { + // add this hash to the list we need to check for asset removal from server + hashesToCheck << oldMapping.toString(); + qDebug() << "Deleted a mapping:" << path << "=>" << oldMapping.toString(); } else { qDebug() << "Unable to delete a mapping that was not found:" << path; From 026f58c8666a6b44d625ba68825652b254e68a4b Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 11 Apr 2016 17:14:31 -0700 Subject: [PATCH 6/7] don't perform asset cleanup if mapping file load fails --- assignment-client/src/assets/AssetServer.cpp | 34 ++++++++++++-------- assignment-client/src/assets/AssetServer.h | 2 +- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index a06163c396..b522890b61 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -122,21 +122,27 @@ void AssetServer::completeSetup() { } // load whatever mappings we currently have from the local file - loadMappingsFromFile(); + if (loadMappingsFromFile()) { + qInfo() << "Serving files from: " << _filesDirectory.path(); - qInfo() << "Serving files from: " << _filesDirectory.path(); + // Check the asset directory to output some information about what we have + auto files = _filesDirectory.entryList(QDir::Files); - // Check the asset directory to output some information about what we have - auto files = _filesDirectory.entryList(QDir::Files); + QRegExp hashFileRegex { ASSET_HASH_REGEX_STRING }; + auto hashedFiles = files.filter(hashFileRegex); - QRegExp hashFileRegex { ASSET_HASH_REGEX_STRING }; - auto hashedFiles = files.filter(hashFileRegex); + qInfo() << "There are" << hashedFiles.size() << "asset files in the asset directory."; - qInfo() << "There are" << hashedFiles.size() << "asset files in the asset directory."; + if (_fileMappings.count() > 0) { + cleanupUnmappedFiles(); + } - cleanupUnmappedFiles(); + nodeList->addNodeTypeToInterestSet(NodeType::Agent); + } else { + qCritical() << "Asset Server assignment will not continue because mapping file could not be loaded."; + setFinished(true); + } - nodeList->addNodeTypeToInterestSet(NodeType::Agent); } void AssetServer::cleanupUnmappedFiles() { @@ -427,7 +433,7 @@ void AssetServer::sendStatsPacket() { static const QString MAP_FILE_NAME = "map.json"; -void AssetServer::loadMappingsFromFile() { +bool AssetServer::loadMappingsFromFile() { auto mapFilePath = _resourcesDirectory.absoluteFilePath(MAP_FILE_NAME); @@ -464,15 +470,17 @@ void AssetServer::loadMappingsFromFile() { } qInfo() << "Loaded" << _fileMappings.count() << "mappings from map file at" << mapFilePath; - return; + return true; } } - qCritical() << "Failed to read mapping file at" << mapFilePath << "- assignment will not continue."; - setFinished(true); + qCritical() << "Failed to read mapping file at" << mapFilePath; + return false; } else { qInfo() << "No existing mappings loaded from file since no file was found at" << mapFilePath; } + + return true; } bool AssetServer::writeMappingsToFile() { diff --git a/assignment-client/src/assets/AssetServer.h b/assignment-client/src/assets/AssetServer.h index a0a85f8fd5..07ff0a92b3 100644 --- a/assignment-client/src/assets/AssetServer.h +++ b/assignment-client/src/assets/AssetServer.h @@ -48,7 +48,7 @@ private: void handleRenameMappingOperation(ReceivedMessage& message, SharedNodePointer senderNode, NLPacketList& replyPacket); // Mapping file operations must be called from main assignment thread only - void loadMappingsFromFile(); + bool loadMappingsFromFile(); bool writeMappingsToFile(); /// Set the mapping for path to hash From 190d11e0bf68a6facac1b4cfe1086893eddd1b9a Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 11 Apr 2016 17:16:52 -0700 Subject: [PATCH 7/7] rename the set of hashes to check for deletion --- assignment-client/src/assets/AssetServer.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index b522890b61..5a5f51eb58 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -550,7 +550,7 @@ bool AssetServer::deleteMappings(AssetPathList& paths) { // take a copy of the current mappings in case persistence of these deletes fails auto oldMappings = _fileMappings; - QSet hashesToCheck; + QSet hashesToCheckForDeletion; // enumerate the paths to delete and remove them all for (auto& path : paths) { @@ -566,7 +566,7 @@ bool AssetServer::deleteMappings(AssetPathList& paths) { while (it != _fileMappings.end()) { if (it.key().startsWith(path)) { // add this hash to the list we need to check for asset removal from the server - hashesToCheck << it.value().toString(); + hashesToCheckForDeletion << it.value().toString(); it = _fileMappings.erase(it); } else { @@ -585,7 +585,7 @@ bool AssetServer::deleteMappings(AssetPathList& paths) { auto oldMapping = _fileMappings.take(path); if (!oldMapping.isNull()) { // add this hash to the list we need to check for asset removal from server - hashesToCheck << oldMapping.toString(); + hashesToCheckForDeletion << oldMapping.toString(); qDebug() << "Deleted a mapping:" << path << "=>" << oldMapping.toString(); } else { @@ -603,14 +603,14 @@ bool AssetServer::deleteMappings(AssetPathList& paths) { // enumerate the mapped hashes and clear the list of hashes to check for anything that's present for (auto& hashVariant : mappedHashes) { - auto it = hashesToCheck.find(hashVariant.toString()); - if (it != hashesToCheck.end()) { - hashesToCheck.erase(it); + auto it = hashesToCheckForDeletion.find(hashVariant.toString()); + if (it != hashesToCheckForDeletion.end()) { + hashesToCheckForDeletion.erase(it); } } // we now have a set of hashes that are unmapped - we will delete those asset files - for (auto& hash : hashesToCheck) { + for (auto& hash : hashesToCheckForDeletion) { // remove the unmapped file QFile removeableFile { _filesDirectory.absoluteFilePath(hash) };