From a08e6640c916d38489bf29f17ea7bb774e23d10a Mon Sep 17 00:00:00 2001 From: Bulat Gayazov Date: Tue, 3 Sep 2024 19:13:28 +0000 Subject: [PATCH] Refactored operation id --- .../library/operation_id/operation_id.h | 37 ++- src/library/operation_id/CMakeLists.txt | 3 + src/library/operation_id/operation_id.cpp | 271 ++++++++++++------ .../operation_id/protos/CMakeLists.txt | 13 + .../operation_id/protos/operation_id.proto | 27 ++ .../library/operation_id/operation_id_ut.cpp | 47 ++- 6 files changed, 283 insertions(+), 115 deletions(-) create mode 100644 src/library/operation_id/protos/CMakeLists.txt create mode 100644 src/library/operation_id/protos/operation_id.proto diff --git a/include/ydb-cpp-sdk/library/operation_id/operation_id.h b/include/ydb-cpp-sdk/library/operation_id/operation_id.h index 8418288129..ed509e1684 100644 --- a/include/ydb-cpp-sdk/library/operation_id/operation_id.h +++ b/include/ydb-cpp-sdk/library/operation_id/operation_id.h @@ -3,14 +3,15 @@ #include #include #include -#include + +namespace Ydb { + class TOperationId; +} namespace NKikimr { namespace NOperationId { class TOperationId { - static constexpr int kEKindMinValue = 0; - static constexpr int kEKindMaxValue = 10; public: enum EKind : int { UNUSED = 0, @@ -31,40 +32,38 @@ class TOperationId { std::string Value; }; - using TDataList = std::vector>; - TOperationId(); explicit TOperationId(const std::string& string, bool allowEmpty = false); - + TOperationId(const TOperationId& other); - TOperationId(TOperationId&& other) = default; + TOperationId(TOperationId&& other); TOperationId& operator=(const TOperationId& other); - TOperationId& operator=(TOperationId&& other) = default; + TOperationId& operator=(TOperationId&& other); - ~TOperationId() = default; + ~TOperationId(); EKind GetKind() const; void SetKind(const EKind& kind); - const TDataList& GetData() const; - TDataList& GetMutableData(); + std::vector GetData() const; + void AddOptionalValue(const std::string& key, const std::string& value); const std::vector& GetValue(const std::string& key) const; + std::string GetSubKind() const; std::string ToString() const; + const Ydb::TOperationId& GetProto() const; private: - bool IsValidKind(int kind); - void CopyData(const TOperationId::TDataList& otherData); - - EKind Kind; - TDataList Data; - std::unordered_map> Index; + class TImpl; + std::unique_ptr Impl; }; -void AddOptionalValue(TOperationId& operarionId, const std::string& key, const std::string& value); -void AddOptionalValue(TOperationId& operarionId, const std::string& key, const char* value, size_t size); +std::string ProtoToString(const Ydb::TOperationId& proto); + +void AddOptionalValue(Ydb::TOperationId& operarionId, const std::string& key, const std::string& value); + TOperationId::EKind ParseKind(const std::string_view value); std::string FormatPreparedQueryIdCompat(const std::string& str); diff --git a/src/library/operation_id/CMakeLists.txt b/src/library/operation_id/CMakeLists.txt index ee46c5f1da..3d525a37c1 100644 --- a/src/library/operation_id/CMakeLists.txt +++ b/src/library/operation_id/CMakeLists.txt @@ -1,3 +1,5 @@ +add_subdirectory(protos) + _ydb_sdk_add_library(library-operation_id) target_link_libraries(library-operation_id @@ -5,6 +7,7 @@ target_link_libraries(library-operation_id yutil cgiparam uri + lib-operation_id-protos ) target_sources(library-operation_id PRIVATE diff --git a/src/library/operation_id/operation_id.cpp b/src/library/operation_id/operation_id.cpp index 53638dc742..650699fe78 100644 --- a/src/library/operation_id/operation_id.cpp +++ b/src/library/operation_id/operation_id.cpp @@ -1,4 +1,7 @@ #include +#include + +#include #include #include @@ -28,157 +31,254 @@ bool DecodePreparedQueryIdCompat(const std::string& in, std::string& out) { return false; } -std::string TOperationId::ToString() const { +std::string ProtoToString(const Ydb::TOperationId& proto) { + using namespace ::google::protobuf; + const Reflection& reflection = *proto.GetReflection(); + std::vector fields; + reflection.ListFields(proto, &fields); TStringStream res; - switch (Kind) { - case TOperationId::OPERATION_DDL: - case TOperationId::OPERATION_DML: + switch (proto.kind()) { + case Ydb::TOperationId::OPERATION_DDL: + case Ydb::TOperationId::OPERATION_DML: res << "ydb://operation"; break; - case TOperationId::SESSION_YQL: + case Ydb::TOperationId::SESSION_YQL: res << "ydb://session"; break; - case TOperationId::PREPARED_QUERY_ID: + case Ydb::TOperationId::PREPARED_QUERY_ID: res << "ydb://preparedqueryid"; break; - case TOperationId::CMS_REQUEST: + case Ydb::TOperationId::CMS_REQUEST: res << "ydb://cmsrequest"; break; - case TOperationId::EXPORT: + case Ydb::TOperationId::EXPORT: res << "ydb://export"; break; - case TOperationId::IMPORT: + case Ydb::TOperationId::IMPORT: res << "ydb://import"; break; - case TOperationId::BUILD_INDEX: + case Ydb::TOperationId::BUILD_INDEX: res << "ydb://buildindex"; break; - case TOperationId::SCRIPT_EXECUTION: + case Ydb::TOperationId::SCRIPT_EXECUTION: res << "ydb://scriptexec"; break; default: Y_ABORT_UNLESS(false, "unexpected kind"); } + // According to protobuf documentation: + // Fields (both normal fields and extension fields) will be listed ordered by field number, + // so we can rely on it to build url string + for (const FieldDescriptor* field : fields) { + Y_ASSERT(field != nullptr); + if (field) { + if (field->is_repeated()) { + int size = reflection.FieldSize(proto, field); + if (size) { + res << "?"; + } + for (int i = 0; i < size; i++) { + const auto& message = reflection.GetRepeatedMessage(proto, field, i); + const auto& data = dynamic_cast(message); + TUri::ReEncode(res, data.key()); + res << "="; + TUri::ReEncode(res, data.value()); + if (i < size - 1) { + res << "&"; + } + } + } else { + res << "/"; + const FieldDescriptor::CppType type = field->cpp_type(); + switch (type) { + case FieldDescriptor::CPPTYPE_ENUM: + res << reflection.GetEnumValue(proto, field); + break; + default: + Y_ABORT_UNLESS(false, "unexpected protobuf field type"); + break; + } + } + } + } + return res.Str(); +} - res << "/" << static_cast(Kind); - if (!Data.empty()) { - res << "?"; +class TOperationId::TImpl { +public: + TImpl() { + Proto.set_kind(Ydb::TOperationId::UNUSED); } - for (size_t i = 0; i < Data.size(); ++i) { - TUri::ReEncode(res, Data[i]->Key); - res << "="; - TUri::ReEncode(res, Data[i]->Value); - if (i < Data.size() - 1) { - res << "&"; + TImpl(const std::string &string, bool allowEmpty) { + if (allowEmpty && string.empty()) { + Proto.set_kind(Ydb::TOperationId::UNUSED); + return; } - } - return res.Str(); -} + TUri uri; + TState::EParsed er = uri.Parse(string, TFeature::FeaturesDefault | TFeature::FeatureSchemeFlexible); + if (er != TState::ParsedOK) { + ythrow yexception() << "Unable to parse input string"; + } + std::string path = uri.PrintS(TField::FlagPath).substr(1); // start from 1 to remove first '/' + if (path.length() < 1) { + ythrow yexception() << "Invalid path length"; + } + int kind; + if (!TryFromString(path, kind)) { + ythrow yexception() << "Unable to cast \"kind\" field: " << path; + } -TOperationId::TOperationId() { - Kind = TOperationId::UNUSED; -} + if (!Proto.EKind_IsValid(kind)) { + ythrow yexception() << "Invalid operation kind: " << kind; + } -TOperationId::TOperationId(const std::string &string, bool allowEmpty) { - if (allowEmpty && string.empty()) { - Kind = TOperationId::UNUSED; - return; + Proto.set_kind(static_cast(kind)); + + std::string query = uri.PrintS(TField::FlagQuery); + + if (!query.empty()) { + TCgiParameters params(query.substr(1)); // start from 1 to remove first '?' + for (auto it : params) { + auto data = Proto.add_data(); + data->set_key(it.first); + data->set_value(it.second); +#ifdef YDB_SDK_USE_STD_STRING + Index[it.first].push_back(&data->value()); +#else + Index[it.first].push_back(&data->value().ConstRef()); +#endif + } + } } - TUri uri; - TState::EParsed er = uri.Parse(string, TFeature::FeaturesDefault | TFeature::FeatureSchemeFlexible); - if (er != TState::ParsedOK) { - ythrow yexception() << "Unable to parse input string"; + TImpl(const TImpl& other) { + Proto.CopyFrom(other.Proto); + BuildIndex(); } - std::string path = uri.PrintS(TField::FlagPath).substr(1); // start from 1 to remove first '/' - if (path.length() < 1) { - ythrow yexception() << "Invalid path length"; + TImpl(TImpl&&) = default; + + TImpl& operator=(const TImpl&) = delete; + TImpl& operator=(TImpl&&) = delete; + + ~TImpl() = default; + + Ydb::TOperationId& GetProto() { + return Proto; } - int kind; - if (!TryFromString(path, kind)) { - ythrow yexception() << "Unable to cast \"kind\" field: " << path; + const Ydb::TOperationId& GetProto() const { + return Proto; } - if (!IsValidKind(kind)) { - ythrow yexception() << "Invalid operation kind: " << kind; + const std::vector& GetValue(const std::string& key) const { + auto it = Index.find(key); + if (it != Index.end()) { + return it->second; + } + ythrow yexception() << "Unable to find key: " << key; } - Kind = static_cast(kind); + std::string GetSubKind() const { + auto it = Index.find("kind"); + if (it == Index.end()) { + return std::string(); + } + + if (it->second.size() != 1) { + ythrow yexception() << "Unable to retreive sub-kind"; + } - std::string query = uri.PrintS(TField::FlagQuery); + return *it->second.at(0); + } - if (!query.empty()) { - TCgiParameters params(query.substr(1)); // start from 1 to remove first '?' - for (auto it : params) { - Data.push_back(std::make_unique(it.first, it.second)); - Index[it.first].push_back(&Data.back()->Value); +private: + void BuildIndex() { + for (const auto& data : Proto.data()) { +#ifdef YDB_SDK_USE_STD_STRING + Index[data.key()].push_back(&data.value()); +#else + Index[data.key()].push_back(&data.value().ConstRef()); +#endif } } + + Ydb::TOperationId Proto; + std::unordered_map> Index; +}; + +TOperationId::TOperationId() { + Impl = std::make_unique(); +} + +TOperationId::TOperationId(const std::string &string, bool allowEmpty) { + Impl = std::make_unique(string, allowEmpty); } TOperationId::TOperationId(const TOperationId& other) { - Kind = other.Kind; - CopyData(other.Data); + Impl = std::make_unique(*other.Impl); +} + +TOperationId::TOperationId(TOperationId&& other) { + Impl = std::make_unique(std::move(*other.Impl)); } TOperationId& TOperationId::operator=(const TOperationId& other) { - Kind = other.Kind; - Data.clear(); - Index.clear(); - CopyData(other.Data); + Impl = std::make_unique(*other.Impl); return *this; } -void TOperationId::CopyData(const TOperationId::TDataList& otherData) { - for (const auto& data : otherData) { - Data.push_back(std::make_unique(data->Key, data->Value)); - Index[data->Key].push_back(&Data.back()->Value); - } +TOperationId& TOperationId::operator=(TOperationId&& other) { + Impl = std::make_unique(std::move(*other.Impl)); + return *this; +} + +TOperationId::~TOperationId() { + Impl.reset(); } TOperationId::EKind TOperationId::GetKind() const { - return Kind; + return static_cast(Impl->GetProto().kind()); } void TOperationId::SetKind(const EKind& kind) { - Kind = kind; + Impl->GetProto().set_kind(static_cast(kind)); } -const TOperationId::TDataList& TOperationId::GetData() const { - return Data; +std::vector TOperationId::GetData() const { + std::vector result; + for (auto data : Impl->GetProto().data()) { + result.push_back({data.key(), data.value()}); + } + return result; } -TOperationId::TDataList& TOperationId::GetMutableData() { - return Data; +void TOperationId::AddOptionalValue(const std::string& key, const std::string& value) { + NKikimr::NOperationId::AddOptionalValue(Impl->GetProto(), key, value); } -const std::vector& TOperationId::GetValue(const std::string &key) const { - auto it = Index.find(key); - if (it != Index.end()) { - return it->second; - } - ythrow yexception() << "Unable to find key: " << key; +const std::vector& TOperationId::GetValue(const std::string& key) const { + return Impl->GetValue(key); } std::string TOperationId::GetSubKind() const { - auto it = Index.find("kind"); - if (it == Index.end()) { - return std::string(); - } + return Impl->GetSubKind(); +} - if (it->second.size() != 1) { - ythrow yexception() << "Unable to retreive sub-kind"; - } +std::string TOperationId::ToString() const { + return ProtoToString(Impl->GetProto()); +} - return *it->second.at(0); +const Ydb::TOperationId& TOperationId::GetProto() const { + return Impl->GetProto(); } -void AddOptionalValue(TOperationId& operationId, const std::string& key, const std::string& value) { - operationId.GetMutableData().push_back(std::make_unique(key, value)); +void AddOptionalValue(Ydb::TOperationId& proto, const std::string& key, const std::string& value) { + auto data = proto.add_data(); + data->set_key(NYdb::TStringType{key}); + data->set_value(NYdb::TStringType{value}); } TOperationId::EKind ParseKind(const std::string_view value) { @@ -205,12 +305,5 @@ TOperationId::EKind ParseKind(const std::string_view value) { return TOperationId::UNUSED; } -bool TOperationId::IsValidKind(int kind) { - if (kEKindMinValue <= kind && kind <= kEKindMaxValue) { - return true; - } - return false; -} - } // namespace NOperationId } // namespace NKikimr diff --git a/src/library/operation_id/protos/CMakeLists.txt b/src/library/operation_id/protos/CMakeLists.txt new file mode 100644 index 0000000000..95e4bcf0bd --- /dev/null +++ b/src/library/operation_id/protos/CMakeLists.txt @@ -0,0 +1,13 @@ +file(GLOB_RECURSE LIB_OPERATION_ID_PROTOS + *.proto +) + +_ydb_sdk_add_proto_library(lib-operation_id-protos + SOURCES ${LIB_OPERATION_ID_PROTOS} +) + +target_include_directories(lib-operation_id-protos PRIVATE + ${YDB_SDK_BINARY_DIR}/ydb-cpp-sdk +) + +_ydb_sdk_install_targets(TARGETS lib-operation_id-protos) diff --git a/src/library/operation_id/protos/operation_id.proto b/src/library/operation_id/protos/operation_id.proto new file mode 100644 index 0000000000..0be1c0ad36 --- /dev/null +++ b/src/library/operation_id/protos/operation_id.proto @@ -0,0 +1,27 @@ +syntax = "proto3"; + +package Ydb; + +message TOperationId { + enum EKind { + UNUSED = 0; + OPERATION_DDL = 1; + OPERATION_DML = 2; + SESSION_YQL = 3; + PREPARED_QUERY_ID = 4; + CMS_REQUEST = 5; + EXPORT = 6; + BUILD_INDEX = 7; + IMPORT = 8; + SCRIPT_EXECUTION = 9; + SS_BG_TASKS = 10; + } + + message TData { + string Key = 1; + string Value = 2; + } + + EKind Kind = 1; + repeated TData Data = 3; +} diff --git a/tests/unit/library/operation_id/operation_id_ut.cpp b/tests/unit/library/operation_id/operation_id_ut.cpp index a7d5c51f99..58bda37900 100644 --- a/tests/unit/library/operation_id/operation_id_ut.cpp +++ b/tests/unit/library/operation_id/operation_id_ut.cpp @@ -1,4 +1,5 @@ #include +#include #include #include @@ -10,17 +11,20 @@ Y_UNIT_TEST_SUITE(OperationIdTest) { const std::string PreparedQueryId = "9d629c27-2c3036b3-4b180476-64435bca"; Y_UNIT_TEST(ConvertKindOnly) { - TOperationId id("ydb://operation/1"); - UNIT_ASSERT_EQUAL(id.ToString(), "ydb://operation/1"); - UNIT_ASSERT_EQUAL(id.GetKind(), TOperationId::OPERATION_DDL); - UNIT_ASSERT_EQUAL(id.GetData().size(), 0); + Ydb::TOperationId proto; + proto.set_kind(Ydb::TOperationId::OPERATION_DDL); + auto str = ProtoToString(proto); + UNIT_ASSERT_EQUAL(str, "ydb://operation/1"); + auto newProto = TOperationId(str); + UNIT_ASSERT_EQUAL(newProto.GetProto().kind(), proto.kind()); + UNIT_ASSERT_EQUAL(newProto.GetProto().data_size(), 0); } Y_UNIT_TEST(PreparedQueryIdCompatibleFormatter) { - TOperationId opId; - opId.SetKind(TOperationId::PREPARED_QUERY_ID); + Ydb::TOperationId opId; + opId.set_kind(Ydb::TOperationId::PREPARED_QUERY_ID); AddOptionalValue(opId, "id", PreparedQueryId); - auto result = opId.ToString(); + auto result = ProtoToString(opId); UNIT_ASSERT_VALUES_EQUAL(FormatPreparedQueryIdCompat(PreparedQueryId), result); } @@ -85,6 +89,35 @@ Y_UNIT_TEST_SUITE(OperationIdTest) { std::cerr << x << std::endl; } #endif + Y_UNIT_TEST(ConvertKindAndValues) { + Ydb::TOperationId proto; + proto.set_kind(Ydb::TOperationId::OPERATION_DDL); + { + auto data = proto.add_data(); + data->set_key("key1"); + data->set_value("value1"); + } + { + auto data = proto.add_data(); + data->set_key("txId"); + data->set_value("42"); + } + auto str = ProtoToString(proto); + UNIT_ASSERT_EQUAL(str, "ydb://operation/1?key1=value1&txId=42"); + auto newProto = TOperationId(str); + UNIT_ASSERT_EQUAL(newProto.GetProto().kind(), proto.kind()); + UNIT_ASSERT_EQUAL(newProto.GetProto().data_size(), 2); + { + auto data = newProto.GetProto().data(0); + UNIT_ASSERT_EQUAL(data.key(), "key1"); + UNIT_ASSERT_EQUAL(data.value(), "value1"); + } + { + auto data = newProto.GetProto().data(1); + UNIT_ASSERT_EQUAL(data.key(), "txId"); + UNIT_ASSERT_EQUAL(data.value(), "42"); + } + } } } // namespace NOperationId