handle multiple paths for mapping delete operations

assignment-client/src/assets/AssetServer.cpp

@@ -200,7 +200,7 @@ void AssetServer::handleAssetMappingOperation(QSharedPointer<ReceivedMessage> me
             break;
         }
         case AssetMappingOperationType::Delete: {
-            handleDeleteMappingOperation(*message, senderNode, *replyPacket);
+            handleDeleteMappingsOperation(*message, senderNode, *replyPacket);
             break;
         }
     }
@@ -253,11 +253,18 @@ void AssetServer::handleSetMappingOperation(ReceivedMessage& message, SharedNode
     }
 }
 
-void AssetServer::handleDeleteMappingOperation(ReceivedMessage& message, SharedNodePointer senderNode, NLPacketList& replyPacket) {
+void AssetServer::handleDeleteMappingsOperation(ReceivedMessage& message, SharedNodePointer senderNode, NLPacketList& replyPacket) {
     if (senderNode->getCanRez()) {
-        QString assetPath = message.readString();
+        int numberOfDeletedMappings { 0 };
+        message.readPrimitive(&numberOfDeletedMappings);
 
-        if (deleteMapping(assetPath)) {
+        QStringList mappingsToDelete;
+
+        for (int i = 0; i < numberOfDeletedMappings; ++i) {
+            mappingsToDelete << message.readString();
+        }
+
+        if (deleteMappings(mappingsToDelete)) {
             replyPacket.writePrimitive(AssetServerError::NoError);
         } else {
             replyPacket.writePrimitive(AssetServerError::MappingOperationFailed);
@@ -484,20 +491,28 @@ bool AssetServer::setMapping(AssetPath path, AssetHash hash) {
     }
 }
 
-bool AssetServer::deleteMapping(AssetPath path) {
-    // keep the old mapping in case the delete fails
-    auto oldMapping = _fileMappings.take(path);
+bool AssetServer::deleteMappings(const AssetPathList& paths) {
+    // take a copy of the current mappings in case persistence of these deletes fails
+    auto oldMappings = _fileMappings;
 
-    if (!oldMapping.isNull()) {
-        // deleted the old mapping, attempt to persist to file
-        if (writeMappingsToFile()) {
-            // persistence succeeded we are good to go
-            return true;
+    // enumerate the paths to delete and remove them all
+    for (auto& path : paths) {
+        auto oldMapping = _fileMappings.take(path);
+        if (!oldMapping.isNull()) {
+            qDebug() << "Deleted a mapping:" << path << "=>" << oldMapping.toString();
         } else {
-            // we didn't delete the previous mapping, put it back in our in-memory representation
-            _fileMappings[path] = oldMapping.toString();
+            qDebug() << "Unable to delete a mapping that was not found:" << path;
         }
     }
 
-    return false;
+    // deleted the old mappings, attempt to persist to file
+    if (writeMappingsToFile()) {
+        // persistence succeeded we are good to go
+        return true;
+    } else {
+        // we didn't delete the previous mapping, put it back in our in-memory representation
+        _fileMappings = oldMappings;
+
+        return false;
+    }
 }

assignment-client/src/assets/AssetServer.h

@@ -44,7 +44,7 @@ private:
     void handleGetMappingOperation(ReceivedMessage& message, SharedNodePointer senderNode, NLPacketList& replyPacket);
     void handleGetAllMappingOperation(ReceivedMessage& message, SharedNodePointer senderNode, NLPacketList& replyPacket);
     void handleSetMappingOperation(ReceivedMessage& message, SharedNodePointer senderNode, NLPacketList& replyPacket);
-    void handleDeleteMappingOperation(ReceivedMessage& message, SharedNodePointer senderNode, NLPacketList& replyPacket);
+    void handleDeleteMappingsOperation(ReceivedMessage& message, SharedNodePointer senderNode, NLPacketList& replyPacket);
 
     // Mapping file operations must be called from main assignment thread only
     void loadMappingsFromFile();
@@ -53,8 +53,8 @@ private:
     /// Set the mapping for path to hash
     bool setMapping(AssetPath path, AssetHash hash);
 
-    /// Delete mapping `path`. Return `true` if mapping existed, else `false`.
-    bool deleteMapping(AssetPath path);
+    /// Delete mapping `path`. Returns `true` if deletion of mappings succeeds, else `false`.
+    bool deleteMappings(const AssetPathList& paths);
 
     static void writeError(NLPacketList* packetList, AssetServerError error);
 

libraries/networking/src/AssetClient.cpp

@@ -38,7 +38,7 @@ void MappingRequest::start() {
     doStart();
 };
 
-GetMappingRequest::GetMappingRequest(AssetPath path) : _path(path) {
+GetMappingRequest::GetMappingRequest(const AssetPath& path) : _path(path) {
 };
 
 void GetMappingRequest::doStart() {
@@ -113,7 +113,7 @@ void GetAllMappingsRequest::doStart() {
     });
 };
 
-SetMappingRequest::SetMappingRequest(AssetPath path, AssetHash hash) : _path(path), _hash(hash) {
+SetMappingRequest::SetMappingRequest(const AssetPath& path, const AssetHash& hash) : _path(path), _hash(hash) {
 };
 
 void SetMappingRequest::doStart() {
@@ -142,12 +142,12 @@ void SetMappingRequest::doStart() {
     });
 };
 
-DeleteMappingRequest::DeleteMappingRequest(AssetPath path) : _path(path) {
+DeleteMappingsRequest::DeleteMappingsRequest(const AssetPathList& paths) : _paths(paths) {
 };
 
-void DeleteMappingRequest::doStart() {
+void DeleteMappingsRequest::doStart() {
     auto assetClient = DependencyManager::get<AssetClient>();
-    assetClient->deleteAssetMapping(_path, [this, assetClient](bool responseReceived, AssetServerError error, QSharedPointer<ReceivedMessage> message) {
+    assetClient->deleteAssetMappings(_paths, [this, assetClient](bool responseReceived, AssetServerError error, QSharedPointer<ReceivedMessage> message) {
         if (!responseReceived) {
             _error = NetworkError;
         } else {
@@ -165,7 +165,10 @@ void DeleteMappingRequest::doStart() {
         }
 
         if (!error) {
-            assetClient->_mappingCache.remove(_path);
+            // enumerate the paths and remove them from the cache
+            for (auto& path : _paths) {
+                assetClient->_mappingCache.remove(path);
+            }
         }
         emit finished(this);
     });
@@ -289,8 +292,8 @@ GetAllMappingsRequest* AssetClient::createGetAllMappingsRequest() {
     return new GetAllMappingsRequest();
 }
 
-DeleteMappingRequest* AssetClient::createDeleteMappingRequest(const AssetPath& path) {
-    return new DeleteMappingRequest(path);
+DeleteMappingsRequest* AssetClient::createDeleteMappingsRequest(const AssetPathList& paths) {
+    return new DeleteMappingsRequest(paths);
 }
 
 SetMappingRequest* AssetClient::createSetMappingRequest(const AssetPath& path, const AssetHash& hash) {
@@ -530,7 +533,7 @@ bool AssetClient::getAllAssetMappings(MappingOperationCallback callback) {
     return false;
 }
 
-bool AssetClient::deleteAssetMapping(const AssetPath& path, MappingOperationCallback callback) {
+bool AssetClient::deleteAssetMappings(const AssetPathList& paths, MappingOperationCallback callback) {
     auto nodeList = DependencyManager::get<NodeList>();
     SharedNodePointer assetServer = nodeList->soloNodeOfType(NodeType::AssetServer);
     
@@ -542,7 +545,11 @@ bool AssetClient::deleteAssetMapping(const AssetPath& path, MappingOperationCall
 
         packetList->writePrimitive(AssetMappingOperationType::Delete);
 
-        packetList->writeString(path);
+        packetList->writePrimitive(int(paths.size()));
+
+        for (auto& path: paths) {
+            packetList->writeString(path);
+        }
 
         nodeList->sendPacketList(std::move(packetList), *assetServer);
 
@@ -787,11 +794,11 @@ void AssetScriptingInterface::getMapping(QString path, QScriptValue callback) {
     request->start();
 }
 
-void AssetScriptingInterface::deleteMapping(QString path, QScriptValue callback) {
+void AssetScriptingInterface::deleteMappings(QStringList paths, QScriptValue callback) {
     auto assetClient = DependencyManager::get<AssetClient>();
-    auto request = assetClient->createDeleteMappingRequest(path);
+    auto request = assetClient->createDeleteMappingsRequest(paths);
 
-    connect(request, &DeleteMappingRequest::finished, this, [this, callback](DeleteMappingRequest* request) mutable {
+    connect(request, &DeleteMappingsRequest::finished, this, [this, callback](DeleteMappingsRequest* request) mutable {
         QScriptValueList args { uint8_t(request->getError()) };
 
         callback.call(_engine->currentContext()->thisObject(), args);

libraries/networking/src/AssetClient.h

@@ -68,7 +68,7 @@ private:
 class GetMappingRequest : public MappingRequest {
     Q_OBJECT
 public:
-    GetMappingRequest(AssetPath path);
+    GetMappingRequest(const AssetPath& path);
 
     AssetHash getHash() const { return _hash;  }
 
@@ -85,7 +85,7 @@ private:
 class SetMappingRequest : public MappingRequest {
     Q_OBJECT
 public:
-    SetMappingRequest(AssetPath path, AssetHash hash);
+    SetMappingRequest(const AssetPath& path, const AssetHash& hash);
 
     AssetHash getHash() const { return _hash;  }
 
@@ -99,18 +99,18 @@ private:
     AssetHash _hash;
 };
 
-class DeleteMappingRequest : public MappingRequest {
+class DeleteMappingsRequest : public MappingRequest {
     Q_OBJECT
 public:
-    DeleteMappingRequest(AssetPath path);
+    DeleteMappingsRequest(const AssetPathList& path);
 
 signals:
-    void finished(DeleteMappingRequest* thisRequest);
+    void finished(DeleteMappingsRequest* thisRequest);
 
 private:
     virtual void doStart() override;
 
-    AssetPath _path;
+    AssetPathList _paths;
 };
 
 class GetAllMappingsRequest : public MappingRequest {
@@ -136,7 +136,7 @@ public:
 
     Q_INVOKABLE GetMappingRequest* createGetMappingRequest(const AssetPath& path);
     Q_INVOKABLE GetAllMappingsRequest* createGetAllMappingsRequest();
-    Q_INVOKABLE DeleteMappingRequest* createDeleteMappingRequest(const AssetPath& path);
+    Q_INVOKABLE DeleteMappingsRequest* createDeleteMappingsRequest(const AssetPathList& paths);
     Q_INVOKABLE SetMappingRequest* createSetMappingRequest(const AssetPath& path, const AssetHash& hash);
     Q_INVOKABLE AssetRequest* createRequest(const AssetHash& hash);
     Q_INVOKABLE AssetUpload* createUpload(const QString& filename);
@@ -160,7 +160,7 @@ private:
     bool getAssetMapping(const AssetHash& hash, MappingOperationCallback callback);
     bool getAllAssetMappings(MappingOperationCallback callback);
     bool setAssetMapping(const QString& path, const AssetHash& hash, MappingOperationCallback callback);
-    bool deleteAssetMapping(const AssetPath& path, MappingOperationCallback callback);
+    bool deleteAssetMappings(const AssetPathList& paths, MappingOperationCallback callback);
 
     bool getAssetInfo(const QString& hash, GetInfoCallback callback);
     bool getAsset(const QString& hash, DataOffset start, DataOffset end,
@@ -185,7 +185,7 @@ private:
     friend class GetMappingRequest;
     friend class GetAllMappingsRequest;
     friend class SetMappingRequest;
-    friend class DeleteMappingRequest;
+    friend class DeleteMappingsRequest;
 };
 
 
@@ -198,7 +198,7 @@ public:
     Q_INVOKABLE void downloadData(QString url, QScriptValue downloadComplete);
     Q_INVOKABLE void setMapping(QString path, QString hash, QScriptValue callback);
     Q_INVOKABLE void getMapping(QString path, QScriptValue callback);
-    Q_INVOKABLE void deleteMapping(QString path, QScriptValue callback);
+    Q_INVOKABLE void deleteMappings(QStringList paths, QScriptValue callback);
     Q_INVOKABLE void getAllMappings(QScriptValue callback);
 protected:
     QSet<AssetRequest*> _pendingRequests;

libraries/networking/src/AssetUtils.h

@@ -25,6 +25,7 @@ using DataOffset = int64_t;
 using AssetPath = QString;
 using AssetHash = QString;
 using AssetMapping = std::map<AssetPath, AssetHash>;
+using AssetPathList = QStringList;
 
 const size_t SHA256_HASH_LENGTH = 32;
 const size_t SHA256_HASH_HEX_LENGTH = 64;