rgw/sfs: Few changes around sqlite modern cpp adoption

Started using move semantics for conversions (more to come).
DBConn::get uses sqlite_orm's `get_storage` so we are opening the
database in the same way.
More blob generalisation.

Signed-off-by: Xavi Garcia <[email protected]>

src/rgw/driver/sfs/sqlite/bindings/blob.h

@@ -23,34 +23,23 @@
 #include "rgw/driver/sfs/sqlite/sqlite_orm.h"
 #include "rgw_common.h"
 
-namespace blob_utils {
-
-template <typename T, typename Tuple>
-struct has_type;
-
-template <typename T>
-struct has_type<T, std::tuple<>> : std::false_type {};
-
-template <typename T, typename U, typename... Ts>
-struct has_type<T, std::tuple<U, Ts...>> : has_type<T, std::tuple<Ts...>> {};
-
-template <typename T, typename... Ts>
-struct has_type<T, std::tuple<T, Ts...>> : std::true_type {};
+namespace sqlite_orm {
 
-// list of types that are stored as blobs and have the encode/decode functions
+// Add to this tuple all the types that you need to store in sqlite as a blob.
+// Those types need to have encode/decode methods based on ceph's bufferlist.
+// Also if your type has the decode/encode methods out of the ceph namespace, go
+// to conversion-utils.h and add your type to the
+// TypesDecodeIsNOTInCephNamespace tuple.
 using BlobTypes = std::tuple<
     rgw::sal::Attrs, ACLOwner, rgw_placement_rule,
     std::map<std::string, RGWAccessKey>, std::map<std::string, RGWSubUser>,
     RGWUserCaps, std::list<std::string>, std::map<int, std::string>,
     RGWQuotaInfo, std::set<std::string>, RGWBucketWebsiteConf,
     std::map<std::string, uint32_t>, RGWObjectLock, rgw_sync_policy_info>;
-}  // namespace blob_utils
-
-namespace sqlite_orm {
 
 template <typename T>
 inline constexpr bool is_sqlite_blob =
-    blob_utils::has_type<T, blob_utils::BlobTypes>::value;
+    blob_utils::has_type<T, BlobTypes>::value;
 
 template <class T>
 struct type_printer<T, typename std::enable_if<is_sqlite_blob<T>, void>::type>
@@ -97,7 +86,7 @@ struct row_extractor<
 namespace rgw::sal::sfs::dbapi::sqlite {
 template <typename T>
 struct has_sqlite_type<T, SQLITE_BLOB, void>
-    : blob_utils::has_type<T, blob_utils::BlobTypes> {};
+    : blob_utils::has_type<T, sqlite_orm::BlobTypes> {};
 
 template <class T>
 inline std::enable_if<sqlite_orm::is_sqlite_blob<T>, int>::type bind_col_in_db(

src/rgw/driver/sfs/sqlite/conversion_utils.h

@@ -17,28 +17,37 @@
 #include "rgw_acl.h"
 #include "rgw_common.h"
 
-namespace rgw::sal::sfs::sqlite {
+namespace blob_utils {
+
+template <typename T, typename Tuple>
+struct has_type;
 
-/// by default type's decode function is under the ceph namespace
 template <typename T>
-struct __ceph_ns_decode : std::true_type {};
+struct has_type<T, std::tuple<>> : std::false_type {};
+
+template <typename T, typename U, typename... Ts>
+struct has_type<T, std::tuple<U, Ts...>> : has_type<T, std::tuple<Ts...>> {};
+
+template <typename T, typename... Ts>
+struct has_type<T, std::tuple<T, Ts...>> : std::true_type {};
+}  // namespace blob_utils
+
+namespace rgw::sal::sfs::sqlite {
 
+// Normally the encode/decode methods for rgw types are found in the ceph
+// namespace. But there are a few types where that's not true.
+// This tuple lists all the types where the encode/decode methods are NOT in the
+// ceph namespace.
+// This is required to specify which call will need your type when encoding or
+// decoding it from/to a bufferlist
+using TypesDecodeIsNOTInCephNamespace = std::tuple<
+    RGWAccessControlPolicy, RGWQuotaInfo, RGWObjectLock, RGWUserCaps, ACLOwner,
+    rgw_placement_rule>;
+
+/// Returns if a type has its encode/decode methods in the ceph namespace.
 template <typename T>
-inline constexpr bool ceph_ns_decode = __ceph_ns_decode<T>::value;
-
-// specialize the ones that are not under the ceph namespace
-template <>
-struct __ceph_ns_decode<RGWAccessControlPolicy> : std::false_type {};
-template <>
-struct __ceph_ns_decode<RGWQuotaInfo> : std::false_type {};
-template <>
-struct __ceph_ns_decode<RGWObjectLock> : std::false_type {};
-template <>
-struct __ceph_ns_decode<RGWUserCaps> : std::false_type {};
-template <>
-struct __ceph_ns_decode<ACLOwner> : std::false_type {};
-template <>
-struct __ceph_ns_decode<rgw_placement_rule> : std::false_type {};
+inline constexpr bool ceph_ns_decode =
+    !blob_utils::has_type<T, TypesDecodeIsNOTInCephNamespace>::value;
 
 template <typename BLOB_HOLDER, typename DEST>
 void decode_blob(const BLOB_HOLDER& blob_holder, DEST& dest) {

src/rgw/driver/sfs/sqlite/dbconn.cc

@@ -205,12 +205,19 @@ dbapi::sqlite::database DBConn::get() {
     auto connection = storage_pool_new.at(this_thread);
     return dbapi::sqlite::database(connection);
   } catch (const std::out_of_range& ex) {
-    // using the same mutex as meanwhile code is being ported connections might
-    // be created for sqlite_orm code or sqlite_modern_cpp
-    std::unique_lock lock(storage_pool_mutex);
-    dbapi::sqlite::database db(getDBPath(cct));
-    storage_pool_new.emplace(this_thread, db.connection());
-    return db;
+    // call get_storage to open the connection the same way it was opened in
+    // the main thread.
+    get_storage();
+    std::shared_lock lock(storage_pool_mutex);
+    if (storage_pool_new.find(this_thread) == storage_pool_new.end()) {
+      // something went really really wrong.
+      throw std::system_error(
+          ENOENT, std::system_category(),
+          "Could not find a valid SQLITE connection"
+      );
+    }
+    auto connection = storage_pool_new.at(this_thread);
+    return dbapi::sqlite::database(connection);
   }
 }
 

src/rgw/driver/sfs/sqlite/objects/object_definitions.h

@@ -31,10 +31,10 @@ struct DBObject {
 
   DBObject() = default;
 
-  explicit DBObject(DBObjectQueryResult values)
-      : uuid(std::get<0>(values)),
-        bucket_id(std::get<1>(values)),
-        name(std::get<2>(values)) {}
+  explicit DBObject(DBObjectQueryResult&& values)
+      : uuid(std::move(std::get<0>(values))),
+        bucket_id(std::move(std::get<1>(values))),
+        name(std::move(std::get<2>(values))) {}
 };
 
 }  // namespace rgw::sal::sfs::sqlite

src/rgw/driver/sfs/sqlite/sqlite_query_utils.h

@@ -33,7 +33,7 @@ inline std::vector<Target> GetSQLiteObjects(
   auto rows = db << fmt::format("SELECT * FROM {};", table_name);
   std::vector<Target> ret;
   for (auto&& row : rows) {
-    ret.emplace_back(Target(row));
+    ret.emplace_back(Target(std::move(row)));
   }
   return ret;
 }
@@ -52,7 +52,7 @@ inline std::vector<Target> GetSQLiteObjectsWhere(
          << column_value;
   std::vector<Target> ret;
   for (auto&& row : rows) {
-    ret.emplace_back(Target(row));
+    ret.emplace_back(Target(std::move(row)));
   }
   return ret;
 }
@@ -64,13 +64,14 @@ inline std::optional<Target> GetSQLiteSingleObject(
     const std::string& key_name, const KeyType& key_value
 ) {
   auto rows =
-      db << fmt::format("SELECT * FROM {} WHERE {} = ?;", table_name, key_name)
+      db << fmt::format(
+                "SELECT * FROM {} WHERE {} = ? LIMIT 1;", table_name, key_name
+            )
          << key_value;
   std::optional<Target> ret;
-  for (auto&& row : rows) {
-    ret = Target(row);
-    break;  // looking for a single object, it should return 0 or 1 entries.
-            // TODO Return an error in there are more than 1 entry?
+  for (auto& row : rows) {
+    ret = Target(std::move(row));
+    break;
   }
   return ret;
 }

src/rgw/driver/sfs/sqlite/sqlite_users.cc

@@ -52,7 +52,7 @@ std::vector<std::string> SQLiteUsers::get_user_ids() const {
   auto rows = db << R"sql(SELECT user_id FROM users;)sql";
   std::vector<std::string> ret;
   for (std::tuple<std::string> row : rows) {
-    ret.emplace_back(std::get<0>(row));
+    ret.emplace_back(std::move(std::get<0>(row)));
   }
   return ret;
 }
@@ -120,7 +120,7 @@ std::optional<std::string> SQLiteUsers::_get_user_id_by_access_key(
     WHERE access_key = ?;)sql"
                  << key;
   for (std::tuple<std::string> row : rows) {
-    ret = std::get<0>(row);
+    ret = std::move(std::get<0>(row));
     // in case we have 2 keys that are equal in different users we return
     // the first one.
     // TODO Consider this an error?

src/rgw/driver/sfs/sqlite/users/users_definitions.h

@@ -96,8 +96,8 @@ struct DBOPUserInfo {
   // sqlite_modern_cpp returns rows as a tuple.
   // This is a helper constructor for the case in which we want to get all the
   // columns and return a full object.
-  explicit DBOPUserInfo(DBUserQueryResult values) {
-    uinfo.user_id.id = std::get<0>(values);
+  explicit DBOPUserInfo(DBUserQueryResult&& values) {
+    uinfo.user_id.id = std::move(std::get<0>(values));
     assign_optional_value(std::get<1>(values), uinfo.user_id.tenant);
     assign_optional_value(std::get<2>(values), uinfo.user_id.ns);
     assign_optional_value(std::get<3>(values), uinfo.display_name);