Rearrange visibility of Backend methods, add DoPoll/DoExpire, add return comments

src/storage/Backend.h

@@ -112,7 +112,7 @@ public:
      * Optional method to allow a backend to poll for data. This can be used to
      * mimic sync mode even if the backend only supports async.
      */
-    virtual void Poll() {}
+    void Poll() { DoPoll(); }
 
     const RecordValPtr& Options() const { return backend_options; }
 
@@ -156,37 +156,11 @@ protected:
      */
     OperationResult Close(OperationResultCallback* cb = nullptr);
 
-    /**
-     * The workhorse method for Open().
-     */
-    virtual OperationResult DoOpen(RecordValPtr options, OpenResultCallback* cb = nullptr) = 0;
-
-    /**
-     * The workhorse method for Close().
-     */
-    virtual OperationResult DoClose(OperationResultCallback* cb = nullptr) = 0;
-
-    /**
-     * The workhorse method for Put().
-     */
-    virtual OperationResult DoPut(ValPtr key, ValPtr value, bool overwrite = true, double expiration_time = 0,
-                                  OperationResultCallback* cb = nullptr) = 0;
-
-    /**
-     * The workhorse method for Get().
-     */
-    virtual OperationResult DoGet(ValPtr key, OperationResultCallback* cb = nullptr) = 0;
-
-    /**
-     * The workhorse method for Erase().
-     */
-    virtual OperationResult DoErase(ValPtr key, OperationResultCallback* cb = nullptr) = 0;
-
     /**
      * Removes any entries in the backend that have expired. Can be overridden by
      * derived classes.
      */
-    virtual void Expire() {}
+    void Expire() { DoExpire(); }
 
     /**
      * Enqueues the Storage::backend_opened event. This is called automatically
@@ -210,6 +184,15 @@ protected:
     std::string tag;
 
 private:
+    virtual OperationResult DoOpen(RecordValPtr options, OpenResultCallback* cb = nullptr) = 0;
+    virtual OperationResult DoClose(OperationResultCallback* cb = nullptr) = 0;
+    virtual OperationResult DoPut(ValPtr key, ValPtr value, bool overwrite = true, double expiration_time = 0,
+                                  OperationResultCallback* cb = nullptr) = 0;
+    virtual OperationResult DoGet(ValPtr key, OperationResultCallback* cb = nullptr) = 0;
+    virtual OperationResult DoErase(ValPtr key, OperationResultCallback* cb = nullptr) = 0;
+    virtual void DoPoll() {}
+    virtual void DoExpire() {}
+
     uint8_t modes;
 };
 

src/storage/backend/redis/Redis.cc

@@ -353,7 +353,7 @@ OperationResult Redis::DoErase(ValPtr key, OperationResultCallback* cb) {
     return {ReturnCode::SUCCESS};
 }
 
-void Redis::Expire() {
+void Redis::DoExpire() {
     // Expiration is handled natively by Redis if not reading traces.
     if ( ! connected || ! zeek::run_state::reading_traces )
         return;
@@ -531,7 +531,7 @@ OperationResult Redis::ParseGetReply(redisReply* reply) const {
     return res;
 }
 
-void Redis::Poll() {
+void Redis::DoPoll() {
     while ( active_ops > 0 )
         int status = redisPollTick(async_ctx, 0.5);
 }

src/storage/backend/redis/Redis.h

@@ -28,43 +28,6 @@ public:
      */
     const char* Tag() override { return tag.c_str(); }
 
-    /**
-     * Called by the manager system to open the backend.
-     */
-    OperationResult DoOpen(RecordValPtr options, OpenResultCallback* cb = nullptr) override;
-
-    /**
-     * Finalizes the backend when it's being closed.
-     */
-    OperationResult DoClose(OperationResultCallback* cb = nullptr) override;
-
-    /**
-     * Returns whether the backend is opened.
-     */
-    bool IsOpen() override { return connected; }
-
-    /**
-     * The workhorse method for Retrieve().
-     */
-    OperationResult DoPut(ValPtr key, ValPtr value, bool overwrite = true, double expiration_time = 0,
-                          OperationResultCallback* cb = nullptr) override;
-
-    /**
-     * The workhorse method for Get().
-     */
-    OperationResult DoGet(ValPtr key, OperationResultCallback* cb = nullptr) override;
-
-    /**
-     * The workhorse method for Erase().
-     */
-    OperationResult DoErase(ValPtr key, OperationResultCallback* cb = nullptr) override;
-
-    /**
-     * Removes any entries in the backend that have expired. Can be overridden by
-     * derived classes.
-     */
-    void Expire() override;
-
     // IOSource interface
     double GetNextTimeout() override { return -1; }
     void Process() override {}
@@ -79,10 +42,21 @@ public:
     void HandleEraseResult(redisReply* reply, OperationResultCallback* callback);
     void HandleGeneric(redisReply* reply);
 
-protected:
+    /**
-    void Poll() override;
+     * Returns whether the backend is opened.
+     */
+    bool IsOpen() override { return connected; }
 
 private:
+    OperationResult DoOpen(RecordValPtr options, OpenResultCallback* cb = nullptr) override;
+    OperationResult DoClose(OperationResultCallback* cb = nullptr) override;
+    OperationResult DoPut(ValPtr key, ValPtr value, bool overwrite = true, double expiration_time = 0,
+                          OperationResultCallback* cb = nullptr) override;
+    OperationResult DoGet(ValPtr key, OperationResultCallback* cb = nullptr) override;
+    OperationResult DoErase(ValPtr key, OperationResultCallback* cb = nullptr) override;
+    void DoExpire() override;
+    void DoPoll() override;
+
     OperationResult ParseGetReply(redisReply* reply) const;
 
     redisAsyncContext* async_ctx = nullptr;

src/storage/backend/sqlite/SQLite.cc

@@ -227,7 +227,7 @@ OperationResult SQLite::DoErase(ValPtr key, OperationResultCallback* cb) {
  * Removes any entries in the backend that have expired. Can be overridden by
  * derived classes.
  */
-void SQLite::Expire() {
+void SQLite::DoExpire() {
     auto stmt = prepared_stmts["expire"];
 
     if ( auto res = CheckError(sqlite3_bind_double(stmt, 1, run_state::network_time));

src/storage/backend/sqlite/SQLite.h

@@ -17,45 +17,32 @@ public:
 
     static BackendPtr Instantiate(std::string_view tag);
 
-    /**
-     * Called by the manager system to open the backend.
-     */
-    OperationResult DoOpen(RecordValPtr options, OpenResultCallback* cb = nullptr) override;
-
-    /**
-     * Finalizes the backend when it's being closed.
-     */
-    OperationResult DoClose(OperationResultCallback* cb = nullptr) override;
-
     /**
      * Returns whether the backend is opened.
      */
     bool IsOpen() override { return db != nullptr; }
 
-    /**
+private:
-     * The workhorse method for Put().
+    OperationResult DoOpen(RecordValPtr options, OpenResultCallback* cb = nullptr) override;
-     */
+    OperationResult DoClose(OperationResultCallback* cb = nullptr) override;
     OperationResult DoPut(ValPtr key, ValPtr value, bool overwrite = true, double expiration_time = 0,
                           OperationResultCallback* cb = nullptr) override;
-
-    /**
-     * The workhorse method for Get().
-     */
     OperationResult DoGet(ValPtr key, OperationResultCallback* cb = nullptr) override;
-
-    /**
-     * The workhorse method for Erase().
-     */
     OperationResult DoErase(ValPtr key, OperationResultCallback* cb = nullptr) override;
+    void DoExpire() override;
 
     /**
-     * Removes any entries in the backend that have expired. Can be overridden by
+     * Checks whether a status code returned by an sqlite call is a success.
-     * derived classes.
+     *
+     * @return A result structure containing a result code and an optional error
+     * string based on the status code.
      */
-    void Expire() override;
-
-private:
     OperationResult CheckError(int code);
+
+    /**
+     * Abstracts calls to sqlite3_step to properly create an OperationResult
+     * structure based on the result.
+     */
     OperationResult Step(sqlite3_stmt* stmt, bool parse_value = false);
 
     sqlite3* db = nullptr;