filesystem: move file object with stats, threading + flushing to AccessLogFile; add generic File object

include/envoy/api/os_sys_calls.h

@@ -34,6 +34,7 @@ typedef SysCallResult<int> SysCallIntResult;
 typedef SysCallResult<ssize_t> SysCallSizeResult;
 typedef SysCallResult<void*> SysCallPtrResult;
 typedef SysCallResult<std::string> SysCallStringResult;
+typedef SysCallResult<bool> SysCallBoolResult;
 
 class OsSysCalls {
 public:
@@ -49,18 +50,6 @@ class OsSysCalls {
    */
   virtual SysCallIntResult ioctl(int sockfd, unsigned long int request, void* argp) PURE;
 
-  /**
-   * Open file by full_path with given flags and mode.
-   * @return file descriptor.
-   */
-  virtual SysCallIntResult open(const std::string& full_path, int flags, int mode) PURE;
-
-  /**
-   * Write num_bytes to fd from buffer.
-   * @return number of bytes written if non negative, otherwise error code.
-   */
-  virtual SysCallSizeResult write(int fd, const void* buffer, size_t num_bytes) PURE;
-
   /**
    * @see writev (man 2 writev)
    */

include/envoy/filesystem/filesystem.h

@@ -14,58 +14,61 @@ namespace Envoy {
 namespace Filesystem {
 
 /**
- * Abstraction for a file on disk.
+ * Abstraction for a basic file on disk.
  */
-class File {
+class RawFile {
 public:
-  virtual ~File() {}
+  virtual ~RawFile() {}
 
   /**
-   * Write data to the file.
+   * Open the file with O_RDWR | O_APPEND | O_CREAT
+   * The file will be closed when this object is destructed
+   *
+   * @return bool whether the open succeeded
    */
-  virtual void write(absl::string_view) PURE;
+  virtual Api::SysCallBoolResult open() PURE;
 
   /**
-   * Reopen the file.
+   * Write the buffer to the file. The file must be explicitly opened before writing.
+   *
+   * @return ssize_t number of bytes written, or -1 for failure
    */
-  virtual void reopen() PURE;
+  virtual Api::SysCallSizeResult write(absl::string_view buffer) PURE;
 
   /**
-   * Synchronously flush all pending data to disk.
+   * Close the file.
+   *
+   * @return bool whether the close succeeded
    */
-  virtual void flush() PURE;
-};
+  virtual Api::SysCallBoolResult close() PURE;
 
-typedef std::shared_ptr<File> FileSharedPtr;
+  /**
+   * @return bool is the file open
+   */
+  virtual bool isOpen() PURE;
 
-/**
- * Captures state, properties, and stats of a file-system.
- */
-class Instance {
-public:
-  virtual ~Instance() {}
+  /**
+   * @return string the file path
+   */
+  virtual std::string path() PURE;
 
   /**
-   * Creates a file, overriding the flush-interval set in the class.
-   *
-   * @param path The path of the file to open.
-   * @param dispatcher The dispatcher used for set up timers to run flush().
-   * @param lock The lock.
-   * @param file_flush_interval_msec Number of milliseconds to delay before flushing.
+   * @return string a human-readable string describing the error code
    */
-  virtual FileSharedPtr createFile(const std::string& path, Event::Dispatcher& dispatcher,
-                                   Thread::BasicLockable& lock,
-                                   std::chrono::milliseconds file_flush_interval_msec) PURE;
+  virtual std::string errorToString(int error) PURE;
+};
+
+typedef std::unique_ptr<RawFile> RawFilePtr;
+
+class RawInstance {
+public:
+  virtual ~RawInstance() {}
 
   /**
-   * Creates a file, using the default flush-interval for the class.
-   *
-   * @param path The path of the file to open.
-   * @param dispatcher The dispatcher used for set up timers to run flush().
-   * @param lock The lock.
+   *  @param path The path of the RawFile
+   *  @return a RawFilePtr. The file is not opened.
    */
-  virtual FileSharedPtr createFile(const std::string& path, Event::Dispatcher& dispatcher,
-                                   Thread::BasicLockable& lock) PURE;
+  virtual RawFilePtr createRawFile(const std::string& path) PURE;
 
   /**
    * @return bool whether a file exists on disk and can be opened for read.
@@ -109,6 +112,61 @@ class Instance {
   virtual bool illegalPath(const std::string& path) PURE;
 };
 
+/**
+ * Abstraction for a file on disk. It is specifically designed for writing access logs.
+ */
+class File {
+public:
+  virtual ~File() {}
+
+  /**
+   * Write data to the file.
+   */
+  virtual void write(absl::string_view) PURE;
+
+  /**
+   * Reopen the file.
+   */
+  virtual void reopen() PURE;
+
+  /**
+   * Synchronously flush all pending data to disk.
+   */
+  virtual void flush() PURE;
+};
+
+typedef std::shared_ptr<File> FileSharedPtr;
+
+/**
+ * Captures state, properties, and stats of a file-system.
+ */
+class Instance : public RawInstance {
+public:
+  virtual ~Instance() {}
+
+  /**
+   * Creates a file, overriding the flush-interval set in the class.
+   *
+   * @param path The path of the file to open.
+   * @param dispatcher The dispatcher used for set up timers to run flush().
+   * @param lock The lock.
+   * @param file_flush_interval_msec Number of milliseconds to delay before flushing.
+   */
+  virtual FileSharedPtr createFile(const std::string& path, Event::Dispatcher& dispatcher,
+                                   Thread::BasicLockable& lock,
+                                   std::chrono::milliseconds file_flush_interval_msec) PURE;
+
+  /**
+   * Creates a file, using the default flush-interval for the class.
+   *
+   * @param path The path of the file to open.
+   * @param dispatcher The dispatcher used for set up timers to run flush().
+   * @param lock The lock.
+   */
+  virtual FileSharedPtr createFile(const std::string& path, Event::Dispatcher& dispatcher,
+                                   Thread::BasicLockable& lock) PURE;
+};
+
 typedef std::unique_ptr<Watcher> WatcherPtr;
 
 enum class FileType { Regular, Directory, Other };

source/common/api/BUILD

@@ -18,6 +18,7 @@ envoy_cc_library(
         "//source/common/common:thread_lib",
         "//source/common/event:dispatcher_lib",
         "//source/common/filesystem:filesystem_lib",
+        "//source/common/filesystem:raw_instance_lib",
     ],
 )
 

source/common/api/api_impl.cc

@@ -13,7 +13,7 @@ Impl::Impl(std::chrono::milliseconds file_flush_interval_msec,
            Thread::ThreadFactory& thread_factory, Stats::Store& stats_store,
            Event::TimeSystem& time_system)
     : thread_factory_(thread_factory),
-      file_system_(file_flush_interval_msec, thread_factory, stats_store),
+      file_system_(file_flush_interval_msec, thread_factory, stats_store, raw_instance_),
       time_system_(time_system) {}
 
 Event::DispatcherPtr Impl::allocateDispatcher() {

source/common/api/api_impl.h

@@ -9,6 +9,7 @@
 #include "envoy/thread/thread.h"
 
 #include "common/filesystem/filesystem_impl.h"
+#include "common/filesystem/raw_instance_impl.h"
 
 namespace Envoy {
 namespace Api {
@@ -29,6 +30,9 @@ class Impl : public Api {
 
 private:
   Thread::ThreadFactory& thread_factory_;
+  // TODO(sesmith177): Inject a RawInstance& when we have separate versions
+  // for POSIX / Windows
+  Filesystem::RawInstanceImpl raw_instance_;
   Filesystem::InstanceImpl file_system_;
   Event::TimeSystem& time_system_;
 };

source/common/api/os_sys_calls_impl.cc

@@ -18,21 +18,11 @@ SysCallIntResult OsSysCallsImpl::ioctl(int sockfd, unsigned long int request, vo
   return {rc, errno};
 }
 
-SysCallIntResult OsSysCallsImpl::open(const std::string& full_path, int flags, int mode) {
-  const int rc = ::open(full_path.c_str(), flags, mode);
-  return {rc, errno};
-}
-
 SysCallIntResult OsSysCallsImpl::close(int fd) {
   const int rc = ::close(fd);
   return {rc, errno};
 }
 
-SysCallSizeResult OsSysCallsImpl::write(int fd, const void* buffer, size_t num_bytes) {
-  const ssize_t rc = ::write(fd, buffer, num_bytes);
-  return {rc, errno};
-}
-
 SysCallSizeResult OsSysCallsImpl::writev(int fd, const iovec* iovec, int num_iovec) {
   const ssize_t rc = ::writev(fd, iovec, num_iovec);
   return {rc, errno};

source/common/api/os_sys_calls_impl.h

@@ -12,8 +12,6 @@ class OsSysCallsImpl : public OsSysCalls {
   // Api::OsSysCalls
   SysCallIntResult bind(int sockfd, const sockaddr* addr, socklen_t addrlen) override;
   SysCallIntResult ioctl(int sockfd, unsigned long int request, void* argp) override;
-  SysCallIntResult open(const std::string& full_path, int flags, int mode) override;
-  SysCallSizeResult write(int fd, const void* buffer, size_t num_bytes) override;
   SysCallSizeResult writev(int fd, const iovec* iovec, int num_iovec) override;
   SysCallSizeResult readv(int fd, const iovec* iovec, int num_iovec) override;
   SysCallSizeResult recv(int socket, void* buffer, size_t length, int flags) override;

source/common/filesystem/BUILD

@@ -43,16 +43,22 @@ envoy_cc_library(
     srcs = ["filesystem_impl.cc"],
     hdrs = ["filesystem_impl.h"],
     deps = [
-        "//include/envoy/api:api_interface",
-        "//include/envoy/api:os_sys_calls_interface",
         "//include/envoy/event:dispatcher_interface",
         "//include/envoy/filesystem:filesystem_interface",
-        "//source/common/api:os_sys_calls_lib",
         "//source/common/buffer:buffer_lib",
         "//source/common/common:thread_lib",
     ],
 )
 
+envoy_cc_library(
+    name = "raw_instance_lib",
+    srcs = ["raw_instance_impl.cc"],
+    hdrs = ["raw_instance_impl.h"],
+    deps = [
+        "//include/envoy/filesystem:filesystem_interface",
+    ],
+)
+
 envoy_cc_library(
     name = "watcher_lib",
     srcs = select({