update FileCache to rm manifest

libraries/networking/src/FileCache.cpp

@@ -60,45 +60,17 @@ void FileCache::initialize() {
     QDir dir(_dirpath.c_str());
 
     if (dir.exists()) {
-        std::unordered_map<std::string, std::pair<Key, std::string>> persistedEntries;
+        auto nameFilters = QStringList(("*." + _ext).c_str());
-        if (dir.exists(MANIFEST_NAME.c_str())) {
+        auto filters = QDir::Filters(QDir::NoDotAndDotDot | QDir::Files);
-            std::ifstream manifest;
+        auto sort = QDir::SortFlags(QDir::Time);
-            manifest.open(dir.absoluteFilePath(MANIFEST_NAME.c_str()).toStdString());
+        auto files = dir.entryList(nameFilters, filters, sort);
-            while (manifest.good()) {
-                std::string key, metadata;
-                std::getline(manifest, key, '\t');
-                std::getline(manifest, metadata, '\n');
-                if (!key.empty()) {
-                    qCDebug(file_cache, "[%s] Manifest contains %s (%s)", _dirname.c_str(), key.c_str(), metadata.c_str());
-                    auto filename = key + '.' + _ext;
-                    persistedEntries[filename] = { key, metadata };
-                }
-            }
-        } else {
-            qCWarning(file_cache, "[%s] Missing manifest", _dirname.c_str());
-        }
 
-        std::unordered_map<Key, std::string> entries;
+        // load persisted files
-
+        foreach(QString filename, files) {
-        foreach(QString filename, dir.entryList(QDir::Filters(QDir::NoDotAndDotDot | QDir::Files))) {
+            const Key key = filename.section('.', 0, 1).toStdString();
-            const auto& it = persistedEntries.find(filename.toStdString());
+            const std::string filepath = dir.filePath(filename).toStdString();
-            if (it == persistedEntries.cend()) {
+            const size_t length = std::ifstream(filepath, std::ios::binary | std::ios::ate).tellg();
-                // unlink extra files
+            addFile(Metadata(key, length), filepath);
-                dir.remove(filename);
-                qCDebug(file_cache, "[%s] Cleaned %s", _dirname.c_str(), filename.toStdString().c_str());
-            } else {
-                // load existing files
-                const Key& key = it->second.first;
-                const std::string& metadata = it->second.second;
-                const std::string filepath = dir.filePath(filename).toStdString();
-                const size_t length = std::ifstream(filepath, std::ios::binary | std::ios::ate).tellg();
-
-                FilePointer file(loadFile(key, filepath, length, metadata).release(), &fileDeleter);
-                file->_cache = this;
-                _files[key] = file;
-                _numTotalFiles += 1;
-                _totalFilesSize += length;
-            }
         }
 
         qCDebug(file_cache, "[%s] Initialized %s", _dirname.c_str(), _dirpath.c_str());
@@ -110,32 +82,40 @@ void FileCache::initialize() {
     _initialized = true;
 }
 
-FilePointer FileCache::writeFile(const Key& key, const char* data, size_t length, void* extra) {
+FilePointer FileCache::addFile(Metadata&& metadata, const std::string& filepath) {
+    FilePointer file(createFile(std::move(metadata), filepath).release(), &fileDeleter);
+    if (file) {
+        _numTotalFiles += 1;
+        _totalFilesSize += file->getLength();
+        file->_cache = this;
+        emit dirty();
+
+        Lock lock(_filesMutex);
+        _files[file->getKey()] = file;
+    }
+    return file;
+}
+
+FilePointer FileCache::writeFile(const char* data, File::Metadata&& metadata) {
     assert(_initialized);
 
-    std::string filepath = getFilepath(key);
+    std::string filepath = getFilepath(metadata.key);
 
     Lock lock(_filesMutex);
 
     // if file already exists, return it
-    FilePointer file = getFile(key);
+    FilePointer file = getFile(metadata.key);
     if (file) {
-        qCWarning(file_cache, "[%s] Attempted to overwrite %s", _dirname.c_str(), key.c_str());
+        qCWarning(file_cache, "[%s] Attempted to overwrite %s", _dirname.c_str(), metadata.key.c_str());
         return file;
     }
 
     // write the new file
     FILE* saveFile = fopen(filepath.c_str(), "wb");
-    if (saveFile != nullptr && fwrite(data, length, 1, saveFile) && fclose(saveFile) == 0) {
+    if (saveFile != nullptr && fwrite(data, metadata.length, 1, saveFile) && fclose(saveFile) == 0) {
-        file.reset(createFile(key, filepath, length, extra).release(), &fileDeleter);
+        file = addFile(std::move(metadata), filepath);
-        file->_cache = this;
-        _files[key] = file;
-        _numTotalFiles += 1;
-        _totalFilesSize += length;
-
-        emit dirty();
     } else {
-        qCWarning(file_cache, "[%s] Failed to write %s (%s)", _dirname.c_str(), key.c_str(), strerror(errno));
+        qCWarning(file_cache, "[%s] Failed to write %s (%s)", _dirname.c_str(), metadata.key.c_str(), strerror(errno));
         errno = 0;
     }
 
@@ -149,7 +129,7 @@ FilePointer FileCache::getFile(const Key& key) {
 
     Lock lock(_filesMutex);
 
-    // check if file already exists
+    // check if file exists
     const auto it = _files.find(key);
     if (it != _files.cend()) {
         file = it->second.lock();
@@ -221,58 +201,22 @@ void FileCache::reserve(size_t length) {
         _numUnusedFiles -= 1;
         _totalFilesSize -= length;
         _unusedFilesSize -= length;
-
-        unusedLock.unlock();
-        evictedFile(file);
-        unusedLock.lock();
     }
 }
 
 void FileCache::clear() {
-    auto forAllFiles = [&](std::function<void(const FilePointer& file)> functor) {
+    Lock unusedFilesLock(_unusedFilesMutex);
-        Lock unusedFilesLock(_unusedFilesMutex);
+    for (const auto& pair : _unusedFiles) {
-        for (const auto& pair : _unusedFiles) {
+        auto& file = pair.second;
-            functor(pair.second);
+        file->_cache = nullptr;
+
+        if (_totalFilesSize > _offlineFilesMaxSize) {
+            _totalFilesSize -= file->getLength();
+        } else {
+            file->_shouldPersist = true;
+            qCDebug(file_cache, "[%s] Persisting %s", _dirname.c_str(), file->getKey().c_str());
         }
-        // clear files so they are not reiterated from _files
-        _unusedFiles.clear();
-        unusedFilesLock.unlock();
-
-        Lock filesLock(_filesMutex);
-        for (const auto& pair : _files) {
-            FilePointer file;
-            if ((file = pair.second.lock())) {
-                functor(file);
-            }
-        }
-    };
-
-    try {
-        std::string manifestPath= _dirpath + '/' + MANIFEST_NAME;
-        std::ofstream manifest(manifestPath);
-
-        forAllFiles([&](const FilePointer& file) {
-            file->_cache = nullptr;
-
-            if (_totalFilesSize > _offlineFilesMaxSize) {
-                _totalFilesSize -= file->getLength();
-            } else {
-                manifest << file->getKey() << '\t' << file->getMetadata() << '\n';
-                file->_shouldPersist = true;
-                qCDebug(file_cache, "[%s] Persisting %s (%s)",
-                        _dirname.c_str(), file->getKey().c_str(), file->getMetadata().c_str());
-            }
-        });
-    } catch (std::exception& e) {
-        qCWarning(file_cache, "[%s] Failed to write manifest (%s)", _dirname.c_str(), e.what());
-
-        forAllFiles([](const FilePointer& file) {
-            file->_cache = nullptr;
-            file->_shouldPersist = false;
-        });
     }
-
-    Lock lock(_unusedFilesMutex);
     _unusedFiles.clear();
 }
 
@@ -285,6 +229,11 @@ void File::deleter() {
     }
 }
 
+File::File(Metadata&& metadata, const std::string& filepath) :
+    _key(std::move(metadata.key)),
+    _length(metadata.length),
+    _filepath(filepath) {}
+
 File::~File() {
     QFile file(getFilepath().c_str());
     if (file.exists() && !_shouldPersist) {

libraries/networking/src/FileCache.h

@@ -52,21 +52,24 @@ public:
 
     // initialize FileCache with a directory name (not a path, ex.: "temp_jpgs") and an ext (ex.: "jpg")
     FileCache(const std::string& dirname, const std::string& ext, QObject* parent = nullptr);
-    // precondition: there should be no references to Files when FileCache is destroyed
     virtual ~FileCache();
 
-    // derived classes are left to implement hashing of the files on their own
     using Key = std::string;
+    struct Metadata {
+        Metadata(const Key& key, size_t length) :
+            key(key), length(length) {}
+        Key key;
+        size_t length;
+    };
 
     // derived classes should implement a setter/getter, for example, for a FileCache backing a network cache:
     //
-    // DerivedFilePointer writeFile(const DerivedData& data) {
+    // DerivedFilePointer writeFile(const char* data, DerivedMetadata&& metadata) {
-    //  return writeFile(data->key, data->data, data->length, &data);
+    //  return writeFile(data, std::forward(metadata));
     // }
     //
     // DerivedFilePointer getFile(const QUrl& url) {
-    //  // assuming storage/removal of url->hash in createFile/evictedFile overrides
+    //  auto key = lookup_hash_for(url); // assuming hashing url in create/evictedFile overrides
-    //  auto key = lookup_hash_for(url);
     //  return getFile(key);
     // }
 
@@ -77,15 +80,11 @@ protected:
     /// must be called after construction to create the cache on the fs and restore persisted files
     void initialize();
 
-    FilePointer writeFile(const Key& key, const char* data, size_t length, void* extra);
+    FilePointer writeFile(const char* data, Metadata&& metadata);
     FilePointer getFile(const Key& key);
 
-    /// create a file (ex.: create a class derived from File and store it in a secondary map with extra->url)
+    /// create a file
-    virtual std::unique_ptr<File> createFile(const Key& key, const std::string& filepath, size_t length, void* extra) = 0;
+    virtual std::unique_ptr<File> createFile(Metadata&& metadata, const std::string& filepath) = 0;
-    /// load a file
-    virtual std::unique_ptr<File> loadFile(const Key& key, const std::string& filepath, size_t length, const std::string& metadata) = 0;
-    /// take action when a file is evicted from the cache (ex.: evict it from a secondary map)
-    virtual void evictedFile(const FilePointer& file) = 0;
 
 private:
     using Mutex = std::recursive_mutex;
@@ -95,6 +94,7 @@ private:
 
     std::string getFilepath(const Key& key);
 
+    FilePointer addFile(Metadata&& metadata, const std::string& filepath);
     void addUnusedFile(const FilePointer file);
     void removeUnusedFile(const FilePointer file);
     void reserve(size_t length);
@@ -126,31 +126,26 @@ class File : public QObject {
 
 public:
     using Key = FileCache::Key;
+    using Metadata = FileCache::Metadata;
 
-    std::string getFilepath() const { return _filepath; }
     Key getKey() const { return _key; }
     size_t getLength() const { return _length; }
+    std::string getFilepath() const { return _filepath; }
 
-    // the destructor should handle unlinking of the actual filepath
     virtual ~File();
-    // overrides should call File::deleter to maintain caching behavior
+    /// overrides should call File::deleter to maintain caching behavior
     virtual void deleter();
 
 protected:
-    // when constructed, the file has already been created/written
+    /// when constructed, the file has already been created/written
-    File(const Key& key, const std::string& filepath, size_t length) :
+    File(Metadata&& metadata, const std::string& filepath);
-        _filepath(filepath), _key(key), _length(length) {}
-
-    /// get metadata to store with a file between instances (ex.: return the url of a hash)
-    virtual std::string getMetadata() const = 0;
-
-    const std::string _filepath;
 
 private:
     friend class FileCache;
 
     const Key _key;
     const size_t _length;
+    const std::string _filepath;
 
     FileCache* _cache;
     int _LRUKey { 0 };