From 2a30782573ccc830a2997b3de4877dec01328f6d Mon Sep 17 00:00:00 2001 From: Xavi Garcia Date: Tue, 14 Nov 2023 08:57:24 +0100 Subject: [PATCH] rgw/sfs: sqlite_modern_cpp blobs, users and objects changes Adds another step on moving the sqlite code to sqlite_modern_cpp. Adds the code to store/retrieve ceph types as blobs. Adds a new mechanism to declare when a type that has encode and decode functions needs to be stored as a blob. In order to add a new type, simple add the type to the following tuple. ```c++ using BlobTypes = std::tuple< rgw::sal::Attrs, ACLOwner, rgw_placement_rule, std::map, std::map, RGWUserCaps, std::list, std::map, RGWQuotaInfo, std::set, RGWBucketWebsiteConf, std::map, RGWObjectLock, rgw_sync_policy_info> ``` Adds a new header (`dbabpi_type_wrapper.h`) file that should be included when declaring type bindings for `sqlite_modern_cpp`. We need this to avoid circular dependencies with the `bind_col_in_db` function from `sqlite_moden_cpp`. This (from the main `sqlite_modern_cpp.h` file): ```c++ template database_binder &operator<<(database_binder& db, index_binding_helper val) { db._next_index(); --db._inx; int result = bind_col_in_db(db._stmt.get(), val.index, std::forward(val.value)); if(result != SQLITE_OK) exceptions::throw_sqlite_error(result, db.sql(), sqlite3_errmsg(db._db.get())); return db; } ``` calls `bind_col_in_db`, but the function specialisation needs to be declared first. So, when declaring type bindings we should only include `sqlite_modern_cpp/type_wrapper.h` Changes all BLOB types in Users to the binding types. Changes all functions in `sqlite_users` to use `sqlite_modern_cpp`. Deletes the conversions files for Users. (conversion is still needed because we need to create the `RGWUserInfo` object needed for SAL layer, the conversion required is only db->SAL ) Changes all funcions in `sqlite_objects` to use `sqlite_modern_cpp` Signed-off-by: Xavi Garcia --- src/rgw/driver/sfs/CMakeLists.txt | 1 - src/rgw/driver/sfs/object_state.h | 2 +- src/rgw/driver/sfs/sqlite/bindings/blob.h | 93 ++++++++++-- .../driver/sfs/sqlite/bindings/real_time.h | 2 +- src/rgw/driver/sfs/sqlite/bindings/uuid_d.h | 40 ++++++ .../driver/sfs/sqlite/dbapi_type_wrapper.h | 9 ++ .../sfs/sqlite/objects/object_definitions.h | 11 ++ src/rgw/driver/sfs/sqlite/sqlite_objects.cc | 54 +++---- .../driver/sfs/sqlite/sqlite_query_utils.h | 77 ++++++++++ src/rgw/driver/sfs/sqlite/sqlite_users.cc | 135 +++++++++--------- src/rgw/driver/sfs/sqlite/sqlite_users.h | 10 +- .../sfs/sqlite/users/users_conversions.cc | 89 ------------ .../sfs/sqlite/users/users_conversions.h | 25 ---- .../sfs/sqlite/users/users_definitions.h | 90 ++++++++++-- src/test/rgw/sfs/test_rgw_sfs_sqlite_users.cc | 40 ------ 15 files changed, 397 insertions(+), 281 deletions(-) create mode 100644 src/rgw/driver/sfs/sqlite/dbapi_type_wrapper.h create mode 100644 src/rgw/driver/sfs/sqlite/sqlite_query_utils.h delete mode 100644 src/rgw/driver/sfs/sqlite/users/users_conversions.cc delete mode 100644 src/rgw/driver/sfs/sqlite/users/users_conversions.h diff --git a/src/rgw/driver/sfs/CMakeLists.txt b/src/rgw/driver/sfs/CMakeLists.txt index 1233c11fead410..365846bc2e49c1 100644 --- a/src/rgw/driver/sfs/CMakeLists.txt +++ b/src/rgw/driver/sfs/CMakeLists.txt @@ -20,7 +20,6 @@ set(sfs_srcs sqlite/sqlite_versioned_objects.cc sqlite/sqlite_lifecycle.cc sqlite/sqlite_multipart.cc - sqlite/users/users_conversions.cc sqlite/buckets/bucket_conversions.cc sqlite/dbconn.cc sqlite/errors.cc diff --git a/src/rgw/driver/sfs/object_state.h b/src/rgw/driver/sfs/object_state.h index 292f8384cb9dd8..f7aa32ee49a76c 100644 --- a/src/rgw/driver/sfs/object_state.h +++ b/src/rgw/driver/sfs/object_state.h @@ -20,7 +20,7 @@ #if FMT_VERSION >= 90000 #include #endif -#include "sqlite/dbapi.h" +#include "sqlite/dbapi_type_wrapper.h" namespace rgw::sal::sfs { diff --git a/src/rgw/driver/sfs/sqlite/bindings/blob.h b/src/rgw/driver/sfs/sqlite/bindings/blob.h index 1e694620f95b48..7de28bb23abc7d 100644 --- a/src/rgw/driver/sfs/sqlite/bindings/blob.h +++ b/src/rgw/driver/sfs/sqlite/bindings/blob.h @@ -13,28 +13,44 @@ */ #pragma once +#include #include #include "rgw/driver/sfs/sqlite/conversion_utils.h" +// we need to include dbapi_type_wrapper.h only because including dbapi.h +// creates circular dependencies +#include "rgw/driver/sfs/sqlite/dbapi_type_wrapper.h" #include "rgw/driver/sfs/sqlite/sqlite_orm.h" #include "rgw_common.h" -namespace sqlite_orm { +namespace blob_utils { -template -struct __is_sqlite_blob : std::false_type {}; +template +struct has_type; template -inline constexpr bool is_sqlite_blob = __is_sqlite_blob::value; +struct has_type> : std::false_type {}; -template <> -struct __is_sqlite_blob : std::true_type {}; +template +struct has_type> : has_type> {}; -template <> -struct __is_sqlite_blob : std::true_type {}; +template +struct has_type> : std::true_type {}; -template <> -struct __is_sqlite_blob : std::true_type {}; +// list of types that are stored as blobs and have the encode/decode functions +using BlobTypes = std::tuple< + rgw::sal::Attrs, ACLOwner, rgw_placement_rule, + std::map, std::map, + RGWUserCaps, std::list, std::map, + RGWQuotaInfo, std::set, RGWBucketWebsiteConf, + std::map, RGWObjectLock, rgw_sync_policy_info>; +} // namespace blob_utils + +namespace sqlite_orm { + +template +inline constexpr bool is_sqlite_blob = + blob_utils::has_type::value; template struct type_printer, void>::type> @@ -77,3 +93,60 @@ struct row_extractor< } }; } // namespace sqlite_orm + +namespace rgw::sal::sfs::dbapi::sqlite { +template +struct has_sqlite_type + : blob_utils::has_type {}; + +template +inline std::enable_if, int>::type bind_col_in_db( + sqlite3_stmt* stmt, int inx, const T& val +) { + std::vector blobValue; + rgw::sal::sfs::sqlite::encode_blob(val, blobValue); + return dbapi::sqlite::bind_col_in_db(stmt, inx, blobValue); +} +template +inline std::enable_if, void>::type +store_result_in_db(sqlite3_context* db, const T& val) { + std::vector blobValue; + rgw::sal::sfs::sqlite::encode_blob(val, blobValue); + dbapi::sqlite::store_result_in_db(db, blobValue); +} +template +inline std::enable_if, T>::type +get_col_from_db(sqlite3_stmt* stmt, int inx, result_type) { + if (sqlite3_column_type(stmt, inx) == SQLITE_NULL) { + ceph_abort_msg("cannot make blob value from NULL"); + } + auto blob_data = sqlite3_column_blob(stmt, inx); + auto blob_size = sqlite3_column_bytes(stmt, inx); + if (blob_data == nullptr || blob_size < 0) { + ceph_abort_msg("Invalid blob at column : (" + std::to_string(inx) + ")"); + } + T ret; + rgw::sal::sfs::sqlite::decode_blob( + reinterpret_cast(blob_data), static_cast(blob_size), + ret + ); + return ret; +} + +template +inline std::enable_if, T>::type +get_val_from_db(sqlite3_value* value, result_type) { + if (sqlite3_value_type(value) == SQLITE_NULL) { + ceph_abort_msg("cannot make blob value from NULL"); + } + std::vector vector_value; + vector_value = get_val_from_db(value, result_type>()); + T ret; + rgw::sal::sfs::sqlite::decode_blob( + reinterpret_cast(vector_value), + static_cast(vector_value.size()), ret + ); + return ret; +} + +} // namespace rgw::sal::sfs::dbapi::sqlite diff --git a/src/rgw/driver/sfs/sqlite/bindings/real_time.h b/src/rgw/driver/sfs/sqlite/bindings/real_time.h index a4454817ab4475..7757ac0086309c 100644 --- a/src/rgw/driver/sfs/sqlite/bindings/real_time.h +++ b/src/rgw/driver/sfs/sqlite/bindings/real_time.h @@ -15,7 +15,7 @@ #include "common/ceph_time.h" #include "include/ceph_assert.h" -#include "rgw/driver/sfs/sqlite/dbapi.h" +#include "rgw/driver/sfs/sqlite/dbapi_type_wrapper.h" #include "rgw/driver/sfs/sqlite/sqlite_orm.h" /// ceph::real_time is represented as a uint64 (unsigned). diff --git a/src/rgw/driver/sfs/sqlite/bindings/uuid_d.h b/src/rgw/driver/sfs/sqlite/bindings/uuid_d.h index 70a295a76b229d..c01f5f1202350c 100644 --- a/src/rgw/driver/sfs/sqlite/bindings/uuid_d.h +++ b/src/rgw/driver/sfs/sqlite/bindings/uuid_d.h @@ -17,6 +17,9 @@ #include "rgw/driver/sfs/sqlite/sqlite_orm.h" #include "rgw_common.h" +// we need to include dbapi_type_wrapper.h only because including dbapi.h +// creates circular dependencies +#include "rgw/driver/sfs/sqlite/dbapi_type_wrapper.h" namespace sqlite_orm { template <> @@ -69,3 +72,40 @@ struct row_extractor { } }; } // namespace sqlite_orm + +namespace rgw::sal::sfs::dbapi::sqlite { + +template <> +struct has_sqlite_type : ::std::true_type {}; + +inline int bind_col_in_db(sqlite3_stmt* stmt, int inx, const uuid_d& val) { + return bind_col_in_db(stmt, inx, val.to_string()); +} +inline void store_result_in_db(sqlite3_context* db, const uuid_d& val) { + store_result_in_db(db, val.to_string()); +} +inline uuid_d +get_col_from_db(sqlite3_stmt* stmt, int inx, result_type) { + std::string db_value = get_col_from_db(stmt, inx, result_type()); + uuid_d ret_value; + if (!ret_value.parse(db_value.c_str())) { + throw std::system_error( + ERANGE, std::system_category(), + "incorrect uuid string (" + db_value + ")" + ); + } + return ret_value; +} + +inline uuid_d get_val_from_db(sqlite3_value* value, result_type) { + std::string db_value = get_val_from_db(value, result_type()); + uuid_d ret_value; + if (!ret_value.parse(db_value.c_str())) { + throw std::system_error( + ERANGE, std::system_category(), + "incorrect uuid string (" + db_value + ")" + ); + } + return ret_value; +} +} // namespace rgw::sal::sfs::dbapi::sqlite diff --git a/src/rgw/driver/sfs/sqlite/dbapi_type_wrapper.h b/src/rgw/driver/sfs/sqlite/dbapi_type_wrapper.h new file mode 100644 index 00000000000000..b35e6d1dba4b37 --- /dev/null +++ b/src/rgw/driver/sfs/sqlite/dbapi_type_wrapper.h @@ -0,0 +1,9 @@ +#pragma once + +#include + +namespace rgw::sal::sfs::dbapi { + +#include "sqlite_modern_cpp/hdr/sqlite_modern_cpp/type_wrapper.h" + +} // namespace rgw::sal::sfs::dbapi diff --git a/src/rgw/driver/sfs/sqlite/objects/object_definitions.h b/src/rgw/driver/sfs/sqlite/objects/object_definitions.h index 0bcec65717869e..67839777df95fe 100644 --- a/src/rgw/driver/sfs/sqlite/objects/object_definitions.h +++ b/src/rgw/driver/sfs/sqlite/objects/object_definitions.h @@ -24,6 +24,17 @@ struct DBObject { uuid_d uuid; std::string bucket_id; std::string name; + + using DBObjectQueryResult = std::tuple< + decltype(DBObject::uuid), decltype(DBObject::bucket_id), + decltype(DBObject::name)>; + + DBObject() = default; + + explicit DBObject(DBObjectQueryResult values) + : uuid(std::get<0>(values)), + bucket_id(std::get<1>(values)), + name(std::get<2>(values)) {} }; } // namespace rgw::sal::sfs::sqlite diff --git a/src/rgw/driver/sfs/sqlite/sqlite_objects.cc b/src/rgw/driver/sfs/sqlite/sqlite_objects.cc index 00eae474c74f76..b1903e8fa8a64e 100644 --- a/src/rgw/driver/sfs/sqlite/sqlite_objects.cc +++ b/src/rgw/driver/sfs/sqlite/sqlite_objects.cc @@ -13,57 +13,57 @@ */ #include "sqlite_objects.h" +#include "dbapi.h" +#include "sqlite_query_utils.h" #include "sqlite_versioned_objects.h" -using namespace sqlite_orm; - namespace rgw::sal::sfs::sqlite { SQLiteObjects::SQLiteObjects(DBConnRef _conn) : conn(_conn) {} std::vector SQLiteObjects::get_objects(const std::string& bucket_id ) const { - auto storage = conn->get_storage(); - return storage->get_all( - where(is_equal(&DBObject::bucket_id, bucket_id)) + return GetSQLiteObjectsWhere( + conn->get(), "objects", "bucket_id", bucket_id ); } std::optional SQLiteObjects::get_object(const uuid_d& uuid) const { - auto storage = conn->get_storage(); - auto object = storage->get_pointer(uuid.to_string()); - std::optional ret_value; - if (object) { - ret_value = *object; - } - return ret_value; + return GetSQLiteSingleObject( + conn->get(), "objects", "uuid", uuid.to_string() + ); } std::optional SQLiteObjects::get_object( const std::string& bucket_id, const std::string& object_name ) const { - auto storage = conn->get_storage(); - auto objects = storage->get_all(where( - is_equal(&DBObject::bucket_id, bucket_id) and - is_equal(&DBObject::name, object_name) - )); - - std::optional ret_value; - // value must be unique - if (objects.size() == 1) { - ret_value = objects[0]; + auto rows = + conn->get() + << R"sql(SELECT * FROM objects WHERE bucket_id = ? AND name = ?;)sql" + << bucket_id << object_name; + std::optional ret_object; + for (auto&& row : rows) { + ret_object = DBObject(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? } - return ret_value; + return ret_object; } void SQLiteObjects::store_object(const DBObject& object) const { - auto storage = conn->get_storage(); - storage->replace(object); + dbapi::sqlite::database db = conn->get(); + db << R"sql( + REPLACE INTO objects ( uuid, bucket_id, name ) + VALUES (?, ?, ?);)sql" + << object.uuid << object.bucket_id << object.name; } void SQLiteObjects::remove_object(const uuid_d& uuid) const { - auto storage = conn->get_storage(); - storage->remove(uuid); + dbapi::sqlite::database db = conn->get(); + db << R"sql( + DELETE FROM objects + WHERE uuid = ?;)sql" + << uuid; } } // namespace rgw::sal::sfs::sqlite diff --git a/src/rgw/driver/sfs/sqlite/sqlite_query_utils.h b/src/rgw/driver/sfs/sqlite/sqlite_query_utils.h new file mode 100644 index 00000000000000..949b29fac18e05 --- /dev/null +++ b/src/rgw/driver/sfs/sqlite/sqlite_query_utils.h @@ -0,0 +1,77 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t +// vim: ts=8 sw=2 smarttab ft=cpp +/* + * Ceph - scalable distributed file system + * SFS SAL implementation + * + * Copyright (C) 2023 SUSE LLC + * + * This is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License version 2.1, as published by the Free Software + * Foundation. See file COPYING. + */ +#pragma once + +#include +#include + +#include "common/Formatter.h" +#include "rgw/driver/sfs/sqlite/dbapi.h" + +namespace rgw::sal::sfs::sqlite { + +// Helper to return a vector with all the objects in a given table +// This needs that Target has a constructor with a tuple listing all the +// columns. +// TODO See if we can do this in a more elegant way without investing too much +// time. +template +inline std::vector GetSQLiteObjects( + dbapi::sqlite::database db, const std::string& table_name +) { + auto rows = db << fmt::format("SELECT * FROM {};", table_name); + std::vector ret; + for (auto&& row : rows) { + ret.emplace_back(Target(row)); + } + return ret; +} + +// Helper to get a vector with all the objects in a given table with a single +// condition. +template +inline std::vector GetSQLiteObjectsWhere( + dbapi::sqlite::database db, const std::string& table_name, + const std::string& column_name, const ColumWhereType& column_value +) { + auto rows = + db << fmt::format( + "SELECT * FROM {} WHERE {} = ?;", table_name, column_name + ) + << column_value; + std::vector ret; + for (auto&& row : rows) { + ret.emplace_back(Target(row)); + } + return ret; +} + +// Helper to get a single object from a table +template +inline std::optional GetSQLiteSingleObject( + dbapi::sqlite::database db, const std::string& table_name, + const std::string& key_name, const KeyType& key_value +) { + auto rows = + db << fmt::format("SELECT * FROM {} WHERE {} = ?;", table_name, key_name) + << key_value; + std::optional 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? + } + return ret; +} +} // namespace rgw::sal::sfs::sqlite diff --git a/src/rgw/driver/sfs/sqlite/sqlite_users.cc b/src/rgw/driver/sfs/sqlite/sqlite_users.cc index 284d8cac276d38..ee6740d3f5c27c 100644 --- a/src/rgw/driver/sfs/sqlite/sqlite_users.cc +++ b/src/rgw/driver/sfs/sqlite/sqlite_users.cc @@ -16,9 +16,8 @@ #include #include -#include "users/users_conversions.h" - -using namespace sqlite_orm; +#include "dbapi.h" +#include "sqlite_query_utils.h" namespace rgw::sal::sfs::sqlite { @@ -26,101 +25,107 @@ SQLiteUsers::SQLiteUsers(DBConnRef _conn) : conn(_conn) {} std::optional SQLiteUsers::get_user(const std::string& userid ) const { - auto storage = conn->get_storage(); - auto user = storage->get_pointer(userid); - std::optional ret_value; - if (user) { - ret_value = get_rgw_user(*user); - } - return ret_value; + return GetSQLiteSingleObject( + conn->get(), "users", "user_id", userid + ); } std::optional SQLiteUsers::get_user_by_email( const std::string& email ) const { - auto users = get_users_by(where(c(&DBUser::user_email) = email)); - std::optional ret_value; - if (users.size()) { - ret_value = users[0]; - } - return ret_value; + return GetSQLiteSingleObject( + conn->get(), "users", "user_email", email + ); } std::optional SQLiteUsers::get_user_by_access_key( const std::string& key ) const { - auto storage = conn->get_storage(); - auto user_id = _get_user_id_by_access_key(storage, key); - std::optional ret_value; - if (user_id.has_value()) { - auto user = storage->get_pointer(user_id); - if (user) { - ret_value = get_rgw_user(*user); - } - } - return ret_value; + auto user_id = _get_user_id_by_access_key(key); + return GetSQLiteSingleObject( + conn->get(), "users", "user_id", user_id + ); } std::vector SQLiteUsers::get_user_ids() const { - auto storage = conn->get_storage(); - return storage->select(&DBUser::user_id); + dbapi::sqlite::database db = conn->get(); + auto rows = db << R"sql(SELECT user_id FROM users;)sql"; + std::vector ret; + for (std::tuple row : rows) { + ret.emplace_back(std::get<0>(row)); + } + return ret; } void SQLiteUsers::store_user(const DBOPUserInfo& user) const { - auto storage = conn->get_storage(); - auto db_user = get_db_user(user); - storage->replace(db_user); - _store_access_keys(storage, user); + dbapi::sqlite::database db = conn->get(); + db << R"sql( + REPLACE INTO users ( user_id, tenant, ns, display_name, user_email, + access_keys, swift_keys, sub_users, suspended, + max_buckets, op_mask, user_caps, admin, system, + placement_name, placement_storage_class, + placement_tags, bucket_quota, temp_url_keys, + user_quota, type, mfa_ids, assumed_role_arn, + user_attrs, user_version, user_version_tag ) + VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, + ?, ?, ?, ?, ?, ?, ?, ? );)sql" + << user.uinfo.user_id.id << user.uinfo.user_id.tenant + << user.uinfo.user_id.ns << user.uinfo.display_name + << user.uinfo.user_email << user.uinfo.access_keys << user.uinfo.swift_keys + << user.uinfo.subusers << user.uinfo.suspended << user.uinfo.max_buckets + << user.uinfo.op_mask << user.uinfo.caps << user.uinfo.admin + << user.uinfo.system << user.uinfo.default_placement.name + << user.uinfo.default_placement.storage_class << user.uinfo.placement_tags + << user.uinfo.quota.bucket_quota << user.uinfo.temp_url_keys + << user.uinfo.quota.user_quota << user.uinfo.type << user.uinfo.mfa_ids + << nullptr << user.user_attrs << user.user_version.ver + << user.user_version.tag; + _store_access_keys(user); } void SQLiteUsers::remove_user(const std::string& userid) const { - auto storage = conn->get_storage(); - _remove_access_keys(storage, userid); - storage->remove(userid); + dbapi::sqlite::database db = conn->get(); + db << R"sql( + DELETE FROM users + WHERE user_id = ?;)sql" + << userid; } -template -std::vector SQLiteUsers::get_users_by(Args... args) const { - std::vector users_return; - auto storage = conn->get_storage(); - auto users = storage->get_all(args...); - for (auto& user : users) { - users_return.push_back(get_rgw_user(user)); - } - return users_return; -} - -void SQLiteUsers::_store_access_keys( - StorageRef storage, const DBOPUserInfo& user -) const { +void SQLiteUsers::_store_access_keys(const DBOPUserInfo& user) const { // remove existing keys for the user (in case any of them had changed) - _remove_access_keys(storage, user.uinfo.user_id.id); + _remove_access_keys(user.uinfo.user_id.id); + dbapi::sqlite::database db = conn->get(); for (auto const& key : user.uinfo.access_keys) { - DBAccessKey db_key; - db_key.access_key = key.first; - db_key.user_id = user.uinfo.user_id.id; - storage->insert(db_key); + db << R"sql(INSERT INTO access_keys (access_key, user_id) VALUES (?,?);)sql" + << key.first << user.uinfo.user_id.id; } } -void SQLiteUsers::_remove_access_keys( - StorageRef storage, const std::string& userid -) const { - storage->remove_all(where(c(&DBAccessKey::user_id) = userid)); +void SQLiteUsers::_remove_access_keys(const std::string& userid) const { + dbapi::sqlite::database db = conn->get(); + db << R"sql( + DELETE FROM access_keys + WHERE user_id = ?;)sql" + << userid; } std::optional SQLiteUsers::_get_user_id_by_access_key( - StorageRef storage, const std::string& key + const std::string& key ) const { - auto keys = - storage->get_all(where(c(&DBAccessKey::access_key) = key)); - std::optional ret_value; - if (keys.size() > 0) { + std::optional ret; + dbapi::sqlite::database db = conn->get(); + auto rows = db << R"sql( + SELECT user_id FROM access_keys + WHERE access_key = ?;)sql" + << key; + for (std::tuple row : rows) { + ret = std::get<0>(row); // in case we have 2 keys that are equal in different users we return // the first one. - ret_value = keys[0].user_id; + // TODO Consider this an error? + break; } - return ret_value; + return ret; } } // namespace rgw::sal::sfs::sqlite diff --git a/src/rgw/driver/sfs/sqlite/sqlite_users.h b/src/rgw/driver/sfs/sqlite/sqlite_users.h index 2cc81228f159c6..4350852d0ab54e 100644 --- a/src/rgw/driver/sfs/sqlite/sqlite_users.h +++ b/src/rgw/driver/sfs/sqlite/sqlite_users.h @@ -37,13 +37,9 @@ class SQLiteUsers { void remove_user(const std::string& userid) const; private: - template - std::vector get_users_by(Args... args) const; - - void _store_access_keys(StorageRef storage, const DBOPUserInfo& user) const; - void _remove_access_keys(StorageRef storage, const std::string& userid) const; - std::optional _get_user_id_by_access_key( - StorageRef storage, const std::string& key + void _store_access_keys(const DBOPUserInfo& user) const; + void _remove_access_keys(const std::string& userid) const; + std::optional _get_user_id_by_access_key(const std::string& key ) const; }; diff --git a/src/rgw/driver/sfs/sqlite/users/users_conversions.cc b/src/rgw/driver/sfs/sqlite/users/users_conversions.cc deleted file mode 100644 index 3c36f6a713a74c..00000000000000 --- a/src/rgw/driver/sfs/sqlite/users/users_conversions.cc +++ /dev/null @@ -1,89 +0,0 @@ -// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -// vim: ts=8 sw=2 smarttab ft=cpp -/* - * Ceph - scalable distributed file system - * SFS SAL implementation - * - * Copyright (C) 2022 SUSE LLC - * - * This is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License version 2.1, as published by the Free Software - * Foundation. See file COPYING. - */ -#include "users_conversions.h" - -#include "../conversion_utils.h" - -namespace rgw::sal::sfs::sqlite { - -DBOPUserInfo get_rgw_user(const DBUser& user) { - DBOPUserInfo rgw_user; - rgw_user.uinfo.user_id.id = user.user_id; - assign_optional_value(user.tenant, rgw_user.uinfo.user_id.tenant); - assign_optional_value(user.ns, rgw_user.uinfo.user_id.ns); - assign_optional_value(user.display_name, rgw_user.uinfo.display_name); - assign_optional_value(user.user_email, rgw_user.uinfo.user_email); - assign_optional_value(user.access_keys, rgw_user.uinfo.access_keys); - assign_optional_value(user.swift_keys, rgw_user.uinfo.swift_keys); - assign_optional_value(user.sub_users, rgw_user.uinfo.subusers); - assign_optional_value(user.suspended, rgw_user.uinfo.suspended); - assign_optional_value(user.max_buckets, rgw_user.uinfo.max_buckets); - assign_optional_value(user.op_mask, rgw_user.uinfo.op_mask); - assign_optional_value(user.user_caps, rgw_user.uinfo.caps); - assign_optional_value(user.admin, rgw_user.uinfo.admin); - assign_optional_value(user.system, rgw_user.uinfo.system); - assign_optional_value( - user.placement_name, rgw_user.uinfo.default_placement.name - ); - assign_optional_value( - user.placement_storage_class, - rgw_user.uinfo.default_placement.storage_class - ); - assign_optional_value(user.placement_tags, rgw_user.uinfo.placement_tags); - assign_optional_value(user.bucket_quota, rgw_user.uinfo.quota.bucket_quota); - assign_optional_value(user.temp_url_keys, rgw_user.uinfo.temp_url_keys); - assign_optional_value(user.user_quota, rgw_user.uinfo.quota.user_quota); - assign_optional_value(user.type, rgw_user.uinfo.type); - assign_optional_value(user.mfa_ids, rgw_user.uinfo.mfa_ids); - assign_optional_value(user.user_attrs, rgw_user.user_attrs); - assign_optional_value(user.user_version, rgw_user.user_version.ver); - assign_optional_value(user.user_version_tag, rgw_user.user_version.tag); - - return rgw_user; -} - -DBUser get_db_user(const DBOPUserInfo& user) { - DBUser db_user; - db_user.user_id = user.uinfo.user_id.id; - assign_db_value(user.uinfo.user_id.tenant, db_user.tenant); - assign_db_value(user.uinfo.user_id.ns, db_user.ns); - assign_db_value(user.uinfo.display_name, db_user.display_name); - assign_db_value(user.uinfo.user_email, db_user.user_email); - assign_db_value(user.uinfo.access_keys, db_user.access_keys); - assign_db_value(user.uinfo.swift_keys, db_user.swift_keys); - assign_db_value(user.uinfo.subusers, db_user.sub_users); - assign_db_value(user.uinfo.suspended, db_user.suspended); - assign_db_value(user.uinfo.max_buckets, db_user.max_buckets); - assign_db_value(user.uinfo.op_mask, db_user.op_mask); - assign_db_value(user.uinfo.caps, db_user.user_caps); - assign_db_value(user.uinfo.system, db_user.system); - assign_db_value(user.uinfo.admin, db_user.admin); - assign_db_value(user.uinfo.default_placement.name, db_user.placement_name); - assign_db_value( - user.uinfo.default_placement.storage_class, - db_user.placement_storage_class - ); - assign_db_value(user.uinfo.placement_tags, db_user.placement_tags); - assign_db_value(user.uinfo.quota.bucket_quota, db_user.bucket_quota); - assign_db_value(user.uinfo.temp_url_keys, db_user.temp_url_keys); - assign_db_value(user.uinfo.quota.user_quota, db_user.user_quota); - assign_db_value(user.uinfo.type, db_user.type); - assign_db_value(user.uinfo.mfa_ids, db_user.mfa_ids); - assign_db_value(user.user_attrs, db_user.user_attrs); - assign_db_value(user.user_version.ver, db_user.user_version); - assign_db_value(user.user_version.tag, db_user.user_version_tag); - - return db_user; -} -} // namespace rgw::sal::sfs::sqlite diff --git a/src/rgw/driver/sfs/sqlite/users/users_conversions.h b/src/rgw/driver/sfs/sqlite/users/users_conversions.h deleted file mode 100644 index 3a00afac661f19..00000000000000 --- a/src/rgw/driver/sfs/sqlite/users/users_conversions.h +++ /dev/null @@ -1,25 +0,0 @@ -// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -// vim: ts=8 sw=2 smarttab ft=cpp -/* - * Ceph - scalable distributed file system - * SFS SAL implementation - * - * Copyright (C) 2022 SUSE LLC - * - * This is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License version 2.1, as published by the Free Software - * Foundation. See file COPYING. - */ -#pragma once - -#include "../sqlite_orm.h" -#include "users_definitions.h" - -namespace rgw::sal::sfs::sqlite { - -// Functions that convert DB type to RGW type (and vice-versa) -DBOPUserInfo get_rgw_user(const DBUser& user); -DBUser get_db_user(const DBOPUserInfo& user); - -} // namespace rgw::sal::sfs::sqlite diff --git a/src/rgw/driver/sfs/sqlite/users/users_definitions.h b/src/rgw/driver/sfs/sqlite/users/users_definitions.h index f198d0ba9995d5..aa5b99390a7978 100644 --- a/src/rgw/driver/sfs/sqlite/users/users_definitions.h +++ b/src/rgw/driver/sfs/sqlite/users/users_definitions.h @@ -17,45 +17,63 @@ #include #include +// #include "../conversion_utils.h" +#include "rgw/driver/sfs/sqlite/bindings/blob.h" +#include "rgw/driver/sfs/sqlite/dbapi.h" #include "rgw_common.h" namespace rgw::sal::sfs::sqlite { -using BLOB = std::vector; - // user to be mapped in DB // Optional values mean they might have (or not) a value defined. -// Blobs are stored as std::vector but we could specialise the encoder and decoder templates -// from sqlite_orm to store blobs in any user defined type. +// Blobs are stored as their type and converted using encode/decode functions struct DBUser { std::string user_id; std::optional tenant; std::optional ns; std::optional display_name; std::optional user_email; - std::optional access_keys; - std::optional swift_keys; - std::optional sub_users; + std::optional> access_keys; + std::optional> swift_keys; + std::optional> sub_users; std::optional suspended; std::optional max_buckets; std::optional op_mask; - std::optional user_caps; + std::optional user_caps; std::optional admin; std::optional system; std::optional placement_name; std::optional placement_storage_class; - std::optional placement_tags; - std::optional bucket_quota; - std::optional temp_url_keys; - std::optional user_quota; + std::optional> placement_tags; + std::optional bucket_quota; + std::optional> temp_url_keys; + std::optional user_quota; std::optional type; - std::optional mfa_ids; + std::optional> mfa_ids; std::optional assumed_role_arn; - std::optional user_attrs; + std::optional user_attrs; std::optional user_version; std::optional user_version_tag; }; +// This is a helper for queries in which we want to retrieve all the columns in +// the table. +// Could be used with the GetSQLiteSingleObject or GetSQLiteObjects helpers +using DBUserQueryResult = std::tuple< + decltype(DBUser::user_id), decltype(DBUser::tenant), decltype(DBUser::ns), + decltype(DBUser::display_name), decltype(DBUser::user_email), + decltype(DBUser::access_keys), decltype(DBUser::swift_keys), + decltype(DBUser::sub_users), decltype(DBUser::suspended), + decltype(DBUser::max_buckets), decltype(DBUser::op_mask), + decltype(DBUser::user_caps), decltype(DBUser::admin), + decltype(DBUser::system), decltype(DBUser::placement_name), + decltype(DBUser::placement_storage_class), decltype(DBUser::placement_tags), + decltype(DBUser::bucket_quota), decltype(DBUser::temp_url_keys), + decltype(DBUser::user_quota), decltype(DBUser::type), + decltype(DBUser::mfa_ids), decltype(DBUser::assumed_role_arn), + decltype(DBUser::user_attrs), decltype(DBUser::user_version), + decltype(DBUser::user_version_tag)>; + // access keys are stored in a different table because a user could have more // than one key and we need to be able to query by all of them. // Keys are stored as a blob in the user, so this table is only used for the @@ -67,12 +85,54 @@ struct DBAccessKey { }; // Struct with information needed by SAL layer -// Because sqlite_orm doesn't like nested members like, for instance, uinfo.user_id.id +// Because sqlite doesn't like nested members like, for instance, uinfo.user_id.id // we need to create this structure that will be returned to the user using the SQLiteUsers API. // The structure stored and retrieved from the database will be DBUser and the one // received and returned by the SQLiteUsers API will be DBOPUserInfo. // SQLiteUsers does the needed conversions. struct DBOPUserInfo { + DBOPUserInfo() = default; + + // 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); + 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); + assign_optional_value(std::get<4>(values), uinfo.user_email); + assign_optional_value(std::get<5>(values), uinfo.access_keys); + assign_optional_value(std::get<6>(values), uinfo.swift_keys); + assign_optional_value(std::get<7>(values), uinfo.subusers); + assign_optional_value(std::get<8>(values), uinfo.suspended); + assign_optional_value(std::get<9>(values), uinfo.max_buckets); + assign_optional_value(std::get<10>(values), uinfo.op_mask); + assign_optional_value(std::get<11>(values), uinfo.caps); + assign_optional_value(std::get<12>(values), uinfo.admin); + assign_optional_value(std::get<13>(values), uinfo.system); + assign_optional_value(std::get<14>(values), uinfo.default_placement.name); + assign_optional_value( + std::get<15>(values), uinfo.default_placement.storage_class + ); + assign_optional_value(std::get<16>(values), uinfo.placement_tags); + assign_optional_value(std::get<17>(values), uinfo.quota.bucket_quota); + assign_optional_value(std::get<18>(values), uinfo.temp_url_keys); + assign_optional_value(std::get<19>(values), uinfo.quota.user_quota); + assign_optional_value(std::get<20>(values), uinfo.type); + assign_optional_value(std::get<21>(values), uinfo.mfa_ids); + // assumed_role_arn no longer exists + assign_optional_value(std::get<23>(values), user_attrs); + assign_optional_value(std::get<24>(values), user_version.ver); + assign_optional_value(std::get<25>(values), user_version.tag); + } + + DBOPUserInfo( + const RGWUserInfo& _uinfo, const obj_version& _user_version, + const rgw::sal::Attrs& _user_attrs + ) + : uinfo(_uinfo), user_version(_user_version), user_attrs(_user_attrs) {} + RGWUserInfo uinfo; obj_version user_version; rgw::sal::Attrs user_attrs; diff --git a/src/test/rgw/sfs/test_rgw_sfs_sqlite_users.cc b/src/test/rgw/sfs/test_rgw_sfs_sqlite_users.cc index e19cc54a65ef55..6990738a12d66e 100644 --- a/src/test/rgw/sfs/test_rgw_sfs_sqlite_users.cc +++ b/src/test/rgw/sfs/test_rgw_sfs_sqlite_users.cc @@ -9,7 +9,6 @@ #include "common/ceph_context.h" #include "rgw/driver/sfs/sqlite/dbconn.h" #include "rgw/driver/sfs/sqlite/sqlite_users.h" -#include "rgw/driver/sfs/sqlite/users/users_conversions.h" #include "rgw/rgw_sal_sfs.h" #include "rgw_sfs_utils.h" @@ -322,45 +321,6 @@ TEST_F(TestSFSSQLiteUsers, AddMoreThanOneUserWithSameAccessKey) { compareUsers(user1, *ret_user); } -TEST_F(TestSFSSQLiteUsers, UseStorage) { - auto ceph_context = std::make_shared(CEPH_ENTITY_TYPE_CLIENT); - ceph_context->_conf.set_val("rgw_sfs_data_path", getTestDir()); - ceph_context->_log->start(); - - DBConnRef conn = std::make_shared(ceph_context.get()); - SQLiteUsers db_users(conn); - auto storage = conn->get_storage(); - - DBUser db_user; - db_user.user_id = "test_storage"; - - // we have to use replace because the primary key of rgw_user is a string - storage->replace(db_user); - - auto user = storage->get_pointer("test_storage"); - - ASSERT_NE(user, nullptr); - ASSERT_EQ(user->user_id, "test_storage"); - - // convert the DBUser to RGWUser (blobs are decoded here) - auto rgw_user = get_rgw_user(*user); - ASSERT_EQ(rgw_user.uinfo.user_id.id, user->user_id); - - // creates a RGWUser for testing (id = test1, email = test1@test.com, etc..) - auto rgw_user_2 = createTestUser("1"); - - // convert to DBUser (blobs are encoded here) - auto db_user_2 = get_db_user(rgw_user_2); - - // we have to use replace because the primary key of rgw_user is a string - storage->replace(db_user_2); - - // now use the SqliteUsers method, so user is already converted - auto ret_user = db_users.get_user("test1"); - ASSERT_TRUE(ret_user.has_value()); - compareUsers(rgw_user_2, *ret_user); -} - // User management tests ------------------------------------------------------ TEST_F(TestSFSSQLiteUsers, StoreListUsers) {