Issue 5183 - Provide async interface for FileSystem

include/AdblockPlus/FileSystem.h

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 #ifndef ADBLOCK_PLUS_FILE_SYSTEM_H
 #define ADBLOCK_PLUS_FILE_SYSTEM_H
 
 #include <istream>
 #include <stdint.h>
 #include <string>
 #include <memory>
 
+#include "IFileSystem.h"
+
 namespace AdblockPlus
 {
   /**
    * File system interface.
    */
   class FileSystem

[sergei, 2017/05/26 13:32:04]

I expected to have a completely new interface with ansynchronous methods. I
think it will allow to gradually migrate to that new interface in other
projects. Currently it will require to make changes on the next update.

   {
   public:
-    /**
-     * Result of a stat operation, i.e.\ information about a file.
-     */
-    struct StatResult
-    {
-      StatResult()
-      {
-        exists = false;
-        isDirectory = false;
-        isFile = false;
-        lastModified = 0;
-      }
-
-      /**
-       * File exists.
-       */
-      bool exists;
-
-      /**
-       * File is a directory.
-       */
-      bool isDirectory;
-
-      /**
-       * File is a regular file.
-       */
-      bool isFile;
-
-      /**
-       * POSIX time of the last modification.
-       */
-      int64_t lastModified;
-    };
-
     virtual ~FileSystem() {}
-
-    typedef std::function<void (void)> Callback;

[sergei, 2017/05/26 13:32:04]

Not sure whether we use a space between return type and arguments.

[sergei, 2017/05/26 13:32:04]

We need a documentation for public things.

[sergei, 2017/05/26 13:32:04]

Could you please not use (void), it's equivalent to ().

[hub, 2017/06/02 16:04:16]

I would put a space, but then elsewhere it doesn't have one, so I'll match that
style for consistency.

[hub, 2017/06/02 16:04:16]

Acknowledged.

-    typedef std::function<void (std::shared_ptr<std::istream>)> ReadCallback;

[sergei, 2017/05/26 13:32:04]

I would propose to change the signature to
std::function<void(const std::vector<uint8_t>&)>)>.
The idea of the asynchronous function is to not block the current execution by
IO operations, so when the callback is invoked the data should be available as
fast as possible, at least the content of the buffer used in the callback should
be already read into the memory, however the work with std::istream will block
the execution because it should read the data from the disk, for instance.

 
     /**
      * Reads from a file.
      * @param path File path.
-     * @return Input stream with the file's contents.
+     * @return Buffer with the file content.
      */
-    virtual std::shared_ptr<std::istream>
-      Read(const std::string& path) const = 0;
-    virtual void Read(const std::string& path,
-                      const ReadCallback& callback) const = 0;
+    virtual IFileSystem::IOBuffer Read(const std::string& path) const = 0;
 
     /**
      * Writes to a file.
      * @param path File path.
-     * @param data Input stream with the data to write.
+     * @param data Buffer with the data to write.
      */
     virtual void Write(const std::string& path,
-                       std::istream& data) = 0;
-    virtual void Write(const std::string& path,
-                       std::istream& data,
-                       const Callback& callback) = 0;
+                       const IFileSystem::IOBuffer& data) = 0;
 
     /**
      * Moves a file (i.e.\ renames it).
      * @param fromPath Current path to the file.
      * @param toPath New path to the file.
      */
     virtual void Move(const std::string& fromPath,
                       const std::string& toPath) = 0;
-    virtual void Move(const std::string& fromPath,
-                      const std::string& toPath,
-                      const Callback& callback) = 0;
 
     /**
      * Removes a file.
      * @param path File path.
      */
     virtual void Remove(const std::string& path) = 0;
-    virtual void Remove(const std::string& path, const Callback& callback) = 0;
 
-    typedef std::function<void (const StatResult&)> StatCallback;
     /**
      * Retrieves information about a file.
      * @param path File path.
      * @return File information.
      */
-    virtual StatResult Stat(const std::string& path) const = 0;
-    virtual void Stat(const std::string& path,
-                      const StatCallback& callback) const = 0;
+    virtual IFileSystem::StatResult Stat(const std::string& path) const = 0;
 
-    typedef std::function<void (const std::string&)> ResolveCallback;
     /**
      * Returns the absolute path to a file.
      * @param path File path (can be relative or absolute).
      * @return Absolute file path.
      */
     virtual std::string Resolve(const std::string& path) const = 0;
-    virtual void Resolve(const std::string& path,

[sergei, 2017/05/26 13:32:04]

I'm not sure whether resolve should be asynchronous. Even more, I suppose it
will be removed very soon, so I would propose to keep it as simple and small as
possible.

[hub, 2017/06/02 16:04:16]

I thought twice about that. I looked at NodeJS for example, and they have a
similar API that has both sync and async. 

I'll remove the async version.

-                         const ResolveCallback& callback) const = 0;
   };
 
   /**
    * Shared smart pointer to a `FileSystem` instance.
    */
-  typedef std::shared_ptr<FileSystem> FileSystemPtr;
+  typedef std::shared_ptr<FileSystem> FileSystemSyncPtr;
 }
 
 #endif