Merge pull request #69 from OP-Engineering/refactor

Code clean
OP-Engineering · Mar 11, 2024 · fbdf13e · fbdf13e
2 parents d290e4a + ae21f19
commit fbdf13e
Show file tree

Hide file tree

Showing 8 changed files with 56 additions and 132 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -65,18 +65,6 @@ jobs:
             echo "turbo_cache_hit=1" >> $GITHUB_ENV
           fi
 
-      # - name: Install JDK
-      #   if: env.turbo_cache_hit != 1
-      #   uses: actions/setup-java@v3
-      #   with:
-      #     distribution: 'zulu'
-      #     java-version: '17'
-
-      # - name: Finalize Android SDK
-      #   if: env.turbo_cache_hit != 1
-      #   run: |
-      #     /bin/bash -c "yes | $ANDROID_HOME/cmdline-tools/latest/bin/sdkmanager --licenses > /dev/null"
-
       - name: Cache Gradle
         if: env.turbo_cache_hit != 1
         uses: actions/cache@v3
@@ -100,7 +88,7 @@ jobs:
       TURBO_CACHE_DIR: .turbo/ios
     steps:
       - name: Checkout
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
 
       - name: Setup
         uses: ./.github/actions/setup
@@ -126,16 +114,16 @@ jobs:
             echo "turbo_cache_hit=1" >> $GITHUB_ENV
           fi
 
-      # - name: Cache cocoapods
-      #   if: env.turbo_cache_hit != 1
-      #   id: cocoapods-cache
-      #   uses: actions/cache@v3
-      #   with:
-      #     path: |
-      #       **/ios/Pods
-      #     key: ${{ runner.os }}-cocoapods-${{ hashFiles('example/ios/Podfile.lock') }}
-      #     restore-keys: |
-      #       ${{ runner.os }}-cocoapods-
+      - name: Cache cocoapods
+        if: env.turbo_cache_hit != 1
+        id: cocoapods-cache
+        uses: actions/cache@v3
+        with:
+          path: |
+            **/ios/Pods
+          key: ${{ runner.os }}-cocoapods-${{ hashFiles('example/ios/Podfile.lock') }}
+          restore-keys: |
+            ${{ runner.os }}-cocoapods-
 
       - name: Install cocoapods
         # if: env.turbo_cache_hit != 1 && steps.cocoapods-cache.outputs.cache-hit != 'true'

diff --git a/cpp/PreparedStatementHostObject.cpp b/cpp/PreparedStatementHostObject.cpp
@@ -1,10 +1,3 @@
-//
-//  PreparedStatementHostObject.cpp
-//  op-sqlite
-//
-//  Created by Oscar Franco on 5/12/23.
-//
-
 #include "PreparedStatementHostObject.h"
 #include "bridge.h"
 #include "macros.h"
@@ -31,7 +24,7 @@ jsi::Value PreparedStatementHostObject::get(jsi::Runtime &rt,
 
   if (name == "bind") {
     return HOSTFN("bind", 1) {
-      if (_statement == NULL) {
+      if (_statement == nullptr) {
         throw std::runtime_error("statement has been freed");
       }
 
@@ -46,7 +39,7 @@ jsi::Value PreparedStatementHostObject::get(jsi::Runtime &rt,
 
   if (name == "execute") {
     return HOSTFN("execute", 1) {
-      if (_statement == NULL) {
+      if (_statement == nullptr) {
         throw std::runtime_error("statement has been freed");
       }
       std::vector<DumbHostObject> results;
@@ -69,9 +62,9 @@ jsi::Value PreparedStatementHostObject::get(jsi::Runtime &rt,
 }
 
 PreparedStatementHostObject::~PreparedStatementHostObject() {
-  if (_statement != NULL) {
+  if (_statement != nullptr) {
     sqlite3_finalize(_statement);
-    _statement = NULL;
+    _statement = nullptr;
   }
 }
 

diff --git a/cpp/bridge.cpp b/cpp/bridge.cpp
@@ -80,14 +80,11 @@ BridgeResult opsqlite_attach(std::string const &mainDBName,
                              std::string const &docPath,
                              std::string const &databaseToAttach,
                              std::string const &alias) {
-  /**
-   * There is no need to check if mainDBName is opened because
-   * sqliteExecuteLiteral will do that.
-   * */
   std::string dbPath = get_db_path(databaseToAttach, docPath);
   std::string statement = "ATTACH DATABASE '" + dbPath + "' AS " + alias;
 
-  BridgeResult result = opsqlite_execute_literal(mainDBName, statement);
+  BridgeResult result =
+      opsqlite_execute(mainDBName, statement, nullptr, nullptr, nullptr);
 
   if (result.type == SQLiteError) {
     return {
@@ -103,12 +100,9 @@ BridgeResult opsqlite_attach(std::string const &mainDBName,
 
 BridgeResult opsqlite_detach(std::string const &mainDBName,
                              std::string const &alias) {
-  /**
-   * There is no need to check if mainDBName is opened because
-   * sqliteExecuteLiteral will do that.
-   * */
   std::string statement = "DETACH DATABASE " + alias;
-  BridgeResult result = opsqlite_execute_literal(mainDBName, statement);
+  BridgeResult result =
+      opsqlite_execute(mainDBName, statement, nullptr, nullptr, nullptr);
   if (result.type == SQLiteError) {
     return BridgeResult{
         .type = SQLiteError,
@@ -526,7 +520,7 @@ opsqlite_execute_raw(std::string const &dbName, std::string const &query,
   bool isConsuming = true;
   bool isFailed = false;
 
-  int result = SQLITE_OK;
+  int step = SQLITE_OK;
 
   do {
     const char *queryStr =
@@ -562,13 +556,14 @@ opsqlite_execute_raw(std::string const &dbName, std::string const &query,
     std::string column_name, column_declared_type;
 
     while (isConsuming) {
-      result = sqlite3_step(statement);
+      step = sqlite3_step(statement);
 
-      switch (result) {
+      switch (step) {
       case SQLITE_ROW: {
-        if (results == NULL) {
+        if (results == nullptr) {
           break;
         }
+
         std::vector<JSVariant> row;
 
         i = 0;
@@ -616,7 +611,8 @@ opsqlite_execute_raw(std::string const &dbName, std::string const &query,
           }
 
           case SQLITE_NULL:
-            // Intentionally left blank
+            row.push_back(JSVariant(nullptr));
+            break;
 
           default:
             row.push_back(JSVariant(nullptr));
@@ -649,7 +645,7 @@ opsqlite_execute_raw(std::string const &dbName, std::string const &query,
 
     return {.type = SQLiteError,
             .message =
-                "[op-sqlite] SQLite error code: " + std::to_string(result) +
+                "[op-sqlite] SQLite error code: " + std::to_string(step) +
                 ", description: " + std::string(errorMessage) +
                 ".\nSee SQLite error codes reference: "
                 "https://www.sqlite.org/rescode.html"};
@@ -663,60 +659,6 @@ opsqlite_execute_raw(std::string const &dbName, std::string const &query,
           .insertId = static_cast<double>(latestInsertRowId)};
 }
 
-/// Executes without returning any results, Useful for performance critical
-/// operations
-BridgeResult opsqlite_execute_literal(std::string const &dbName,
-                                      std::string const &query) {
-  check_db_open(dbName);
-
-  sqlite3 *db = dbMap[dbName];
-  sqlite3_stmt *statement;
-
-  int statementStatus =
-      sqlite3_prepare_v2(db, query.c_str(), -1, &statement, NULL);
-
-  if (statementStatus != SQLITE_OK) {
-    const char *message = sqlite3_errmsg(db);
-    return {SQLiteError,
-            "[op-sqlite] SQL statement error: " + std::string(message), 0};
-  }
-
-  bool isConsuming = true;
-  bool isFailed = false;
-
-  int result;
-  std::string column_name;
-
-  while (isConsuming) {
-    result = sqlite3_step(statement);
-
-    switch (result) {
-    case SQLITE_ROW:
-      isConsuming = true;
-      break;
-
-    case SQLITE_DONE:
-      isConsuming = false;
-      break;
-
-    default:
-      isFailed = true;
-      isConsuming = false;
-    }
-  }
-
-  sqlite3_finalize(statement);
-
-  if (isFailed) {
-    const char *message = sqlite3_errmsg(db);
-    return {SQLiteError,
-            "[op-sqlite] SQL execution error: " + std::string(message), 0};
-  }
-
-  int changedRowCount = sqlite3_changes(db);
-  return {SQLiteOk, "", changedRowCount};
-}
-
 void opsqlite_close_all() {
   for (auto const &x : dbMap) {
     // Interrupt will make all pending operations to fail with SQLITE_INTERRUPT

diff --git a/cpp/bridge.h b/cpp/bridge.h
@@ -46,9 +46,6 @@ BridgeResult opsqlite_execute_raw(std::string const &dbName,
                                   const std::vector<JSVariant> *params,
                                   std::vector<std::vector<JSVariant>> *results);
 
-BridgeResult opsqlite_execute_literal(std::string const &dbName,
-                                      std::string const &query);
-
 void opsqlite_close_all();
 
 BridgeResult opsqlite_register_update_hook(std::string const &dbName,

diff --git a/cpp/sqlbatchexecutor.cpp b/cpp/sqlbatchexecutor.cpp
@@ -57,15 +57,16 @@ BatchResult sqliteExecuteBatch(std::string dbName,
 
   try {
     int affectedRows = 0;
-    opsqlite_execute_literal(dbName, "BEGIN EXCLUSIVE TRANSACTION");
+    opsqlite_execute(dbName, "BEGIN EXCLUSIVE TRANSACTION", nullptr, nullptr,
+                     nullptr);
     for (int i = 0; i < commandCount; i++) {
       auto command = commands->at(i);
       // We do not provide a datastructure to receive query data because we
       // don't need/want to handle this results in a batch execution
       auto result = opsqlite_execute(dbName, command.sql, command.params.get(),
                                      nullptr, nullptr);
       if (result.type == SQLiteError) {
-        opsqlite_execute_literal(dbName, "ROLLBACK");
+        opsqlite_execute(dbName, "ROLLBACK", nullptr, nullptr, nullptr);
         return BatchResult{
             .type = SQLiteError,
             .message = result.message,
@@ -74,14 +75,14 @@ BatchResult sqliteExecuteBatch(std::string dbName,
         affectedRows += result.affectedRows;
       }
     }
-    opsqlite_execute_literal(dbName, "COMMIT");
+    opsqlite_execute(dbName, "COMMIT", nullptr, nullptr, nullptr);
     return BatchResult{
         .type = SQLiteOk,
         .affectedRows = affectedRows,
         .commands = static_cast<int>(commandCount),
     };
   } catch (std::exception &exc) {
-    opsqlite_execute_literal(dbName, "ROLLBACK");
+    opsqlite_execute(dbName, "ROLLBACK", nullptr, nullptr, nullptr);
     return BatchResult{
         .type = SQLiteError,
         .message = exc.what(),

diff --git a/cpp/types.h b/cpp/types.h
@@ -2,8 +2,25 @@
 #define types_h
 
 #include <memory>
+#include <string>
 #include <variant>
 
+enum ResultType { SQLiteOk, SQLiteError };
+
+struct BridgeResult {
+  ResultType type;
+  std::string message;
+  int affectedRows;
+  double insertId;
+};
+
+struct BatchResult {
+  ResultType type;
+  std::string message;
+  int affectedRows;
+  int commands;
+};
+
 struct ArrayBuffer {
   std::shared_ptr<uint8_t> data;
   size_t size;

diff --git a/cpp/utils.cpp b/cpp/utils.cpp
@@ -205,12 +205,14 @@ BatchResult importSQLFile(std::string dbName, std::string fileLocation) {
     try {
       int affectedRows = 0;
       int commands = 0;
-      opsqlite_execute_literal(dbName, "BEGIN EXCLUSIVE TRANSACTION");
+      opsqlite_execute(dbName, "BEGIN EXCLUSIVE TRANSACTION", nullptr, nullptr,
+                       nullptr);
       while (std::getline(sqFile, line, '\n')) {
         if (!line.empty()) {
-          BridgeResult result = opsqlite_execute_literal(dbName, line);
+          BridgeResult result =
+              opsqlite_execute(dbName, line, nullptr, nullptr, nullptr);
           if (result.type == SQLiteError) {
-            opsqlite_execute_literal(dbName, "ROLLBACK");
+            opsqlite_execute(dbName, "ROLLBACK", nullptr, nullptr, nullptr);
             sqFile.close();
             return {SQLiteError, result.message, 0, commands};
           } else {
@@ -220,11 +222,11 @@ BatchResult importSQLFile(std::string dbName, std::string fileLocation) {
         }
       }
       sqFile.close();
-      opsqlite_execute_literal(dbName, "COMMIT");
+      opsqlite_execute(dbName, "COMMIT", nullptr, nullptr, nullptr);
       return {SQLiteOk, "", affectedRows, commands};
     } catch (...) {
       sqFile.close();
-      opsqlite_execute_literal(dbName, "ROLLBACK");
+      opsqlite_execute(dbName, "ROLLBACK", nullptr, nullptr, nullptr);
       return {SQLiteError,
               "[op-sqlite][loadSQLFile] Unexpected error, transaction was "
               "rolledback",

diff --git a/cpp/utils.h b/cpp/utils.h
@@ -15,22 +15,6 @@ namespace opsqlite {
 
 namespace jsi = facebook::jsi;
 
-enum ResultType { SQLiteOk, SQLiteError };
-
-struct BridgeResult {
-  ResultType type;
-  std::string message;
-  int affectedRows;
-  double insertId;
-};
-
-struct BatchResult {
-  ResultType type;
-  std::string message;
-  int affectedRows;
-  int commands;
-};
-
 jsi::Value toJSI(jsi::Runtime &rt, JSVariant value);
 
 JSVariant toVariant(jsi::Runtime &rt, jsi::Value const &value);