github-actions[bot] commented on code in PR #33269: URL: https://github.com/apache/doris/pull/33269#discussion_r1553303757
########## cloud/test/meta_service_test.cpp: ########## @@ -5109,6 +5109,99 @@ EXPECT_EQ(fs_name, "hdfs://127.0.0.1:8020"); } +TEST(MetaServiceTest, AddObjInfoTest) { + auto meta_service = get_meta_service(); + + auto sp = cloud::SyncPoint::get_instance(); + sp->enable_processing(); + sp->set_call_back("encrypt_ak_sk:get_encryption_key_ret", + [](void* p) { *reinterpret_cast<int*>(p) = 0; }); + sp->set_call_back("encrypt_ak_sk:get_encryption_key", [](void* p) { + *reinterpret_cast<std::string*>(p) = "selectdbselectdbselectdbselectdb"; + }); + sp->set_call_back("encrypt_ak_sk:get_encryption_key_id", + [](void* p) { *reinterpret_cast<int*>(p) = 1; }); + + std::unique_ptr<Transaction> txn; + ASSERT_EQ(meta_service->txn_kv()->create_txn(&txn), TxnErrorCode::TXN_OK); + std::string key; + std::string val; + InstanceKeyInfo key_info {"test_instance"}; + instance_key(key_info, &key); + + InstanceInfoPB instance; + val = instance.SerializeAsString(); + txn->put(key, val); + ASSERT_EQ(txn->commit(), TxnErrorCode::TXN_OK); + + auto get_test_instance = [&](InstanceInfoPB& i) { + std::string key; + std::string val; + std::unique_ptr<Transaction> txn; + ASSERT_EQ(meta_service->txn_kv()->create_txn(&txn), TxnErrorCode::TXN_OK); + InstanceKeyInfo key_info {"test_instance"}; + instance_key(key_info, &key); + ASSERT_EQ(txn->get(key, &val), TxnErrorCode::TXN_OK); + i.ParseFromString(val); + }; + + // update failed + { + AlterObjStoreInfoRequest req; + req.set_cloud_unique_id("test_cloud_unique_id"); + req.set_op(AlterObjStoreInfoRequest::ADD_OBJ_INFO); + + brpc::Controller cntl; + AlterObjStoreInfoResponse res; + meta_service->alter_obj_store_info( + reinterpret_cast<::google::protobuf::RpcController*>(&cntl), &req, &res, nullptr); + ASSERT_EQ(res.status().code(), MetaServiceCode::INVALID_ARGUMENT) << res.status().msg(); + } + + // update successful + { + AlterObjStoreInfoRequest req; + req.set_cloud_unique_id("test_cloud_unique_id"); + req.set_op(AlterObjStoreInfoRequest::ADD_OBJ_INFO); + auto sp = SyncPoint::get_instance(); Review Comment: warning: 'auto sp' can be declared as 'auto *sp' [readability-qualified-auto] ```suggestion auto *sp = SyncPoint::get_instance(); ``` ########## cloud/test/meta_service_test.cpp: ########## @@ -5770,6 +5902,143 @@ SyncPoint::get_instance()->clear_all_call_backs(); } +TEST(MetaServiceTest, CreateTabletsVaultsTest) { Review Comment: warning: function 'TEST' exceeds recommended size/complexity thresholds [readability-function-size] ```cpp TEST(MetaServiceTest, CreateTabletsVaultsTest) { ^ ``` <details> <summary>Additional context</summary> **cloud/test/meta_service_test.cpp:5904:** 135 lines including whitespace and comments (threshold 80) ```cpp TEST(MetaServiceTest, CreateTabletsVaultsTest) { ^ ``` </details> ########## cloud/test/meta_service_test.cpp: ########## @@ -5770,6 +5902,143 @@ SyncPoint::get_instance()->clear_all_call_backs(); } +TEST(MetaServiceTest, CreateTabletsVaultsTest) { + auto meta_service = get_meta_service(); + + auto sp = cloud::SyncPoint::get_instance(); + sp->enable_processing(); + sp->set_call_back("encrypt_ak_sk:get_encryption_key_ret", + [](void* p) { *reinterpret_cast<int*>(p) = 0; }); + sp->set_call_back("encrypt_ak_sk:get_encryption_key", [](void* p) { + *reinterpret_cast<std::string*>(p) = "selectdbselectdbselectdbselectdb"; + }); + sp->set_call_back("encrypt_ak_sk:get_encryption_key_id", + [](void* p) { *reinterpret_cast<int*>(p) = 1; }); + + std::unique_ptr<Transaction> txn; + ASSERT_EQ(meta_service->txn_kv()->create_txn(&txn), TxnErrorCode::TXN_OK); + std::string key; + std::string val; + InstanceKeyInfo key_info {"test_instance"}; + instance_key(key_info, &key); + + InstanceInfoPB instance; + val = instance.SerializeAsString(); + txn->put(key, val); + ASSERT_EQ(txn->commit(), TxnErrorCode::TXN_OK); + + auto get_test_instance = [&](InstanceInfoPB& i) { + std::string key; + std::string val; + std::unique_ptr<Transaction> txn; + ASSERT_EQ(meta_service->txn_kv()->create_txn(&txn), TxnErrorCode::TXN_OK); + InstanceKeyInfo key_info {"test_instance"}; + instance_key(key_info, &key); + ASSERT_EQ(txn->get(key, &val), TxnErrorCode::TXN_OK); + i.ParseFromString(val); + }; + + // tablet_metas_size is 0 + { + CreateTabletsRequest request; + request.set_cloud_unique_id("test_cloud_unique_id"); + + brpc::Controller cntl; + CreateTabletsResponse response; + meta_service->create_tablets(reinterpret_cast<::google::protobuf::RpcController*>(&cntl), + &request, &response, nullptr); + ASSERT_EQ(response.status().code(), MetaServiceCode::INVALID_ARGUMENT) + << response.status().msg(); + } + + // try to use default + { + CreateTabletsRequest req; + req.set_cloud_unique_id("test_cloud_unique_id"); + req.set_storage_vault_name(""); + req.add_tablet_metas(); + + brpc::Controller cntl; + CreateTabletsResponse res; + meta_service->create_tablets(reinterpret_cast<::google::protobuf::RpcController*>(&cntl), + &req, &res, nullptr); + // failed because no default + ASSERT_EQ(res.status().code(), MetaServiceCode::INVALID_ARGUMENT) << res.status().msg(); + } + + // Create One Hdfs info as built_in vault + { + AlterObjStoreInfoRequest req; + req.set_cloud_unique_id("test_cloud_unique_id"); + req.set_op(AlterObjStoreInfoRequest::ADD_HDFS_INFO); + StorageVaultPB hdfs; + hdfs.set_name("test_alter_add_hdfs_info"); + HdfsVaultInfo params; + params.mutable_build_conf()->set_fs_name("hdfs://ip:port"); + + hdfs.mutable_hdfs_info()->CopyFrom(params); + req.mutable_hdfs()->CopyFrom(hdfs); + + brpc::Controller cntl; + AlterObjStoreInfoResponse res; + meta_service->alter_obj_store_info( + reinterpret_cast<::google::protobuf::RpcController*>(&cntl), &req, &res, nullptr); + ASSERT_EQ(res.status().code(), MetaServiceCode::OK) << res.status().msg(); + + InstanceInfoPB i; + get_test_instance(i); + ASSERT_EQ(i.default_storage_vault_id(), "1"); + ASSERT_EQ(i.default_storage_vault_name(), "built_in_storage_vault"); + } + + // try to use default vault + { + CreateTabletsRequest req; + req.set_cloud_unique_id("test_cloud_unique_id"); + req.set_storage_vault_name(""); + req.add_tablet_metas(); + + auto sp = SyncPoint::get_instance(); Review Comment: warning: 'auto sp' can be declared as 'auto *sp' [readability-qualified-auto] ```suggestion auto *sp = SyncPoint::get_instance(); ``` ########## cloud/test/meta_service_test.cpp: ########## @@ -5109,6 +5109,99 @@ TEST(MetaServiceTest, NormalizeHdfsConfTest) { EXPECT_EQ(fs_name, "hdfs://127.0.0.1:8020"); } +TEST(MetaServiceTest, AddObjInfoTest) { Review Comment: warning: function 'TEST' exceeds recommended size/complexity thresholds [readability-function-size] ```cpp TEST(MetaServiceTest, AddObjInfoTest) { ^ ``` <details> <summary>Additional context</summary> **cloud/test/meta_service_test.cpp:5111:** 91 lines including whitespace and comments (threshold 80) ```cpp TEST(MetaServiceTest, AddObjInfoTest) { ^ ``` </details> ########## cloud/test/meta_service_test.cpp: ########## @@ -5770,6 +5902,143 @@ SyncPoint::get_instance()->clear_all_call_backs(); } +TEST(MetaServiceTest, CreateTabletsVaultsTest) { + auto meta_service = get_meta_service(); + + auto sp = cloud::SyncPoint::get_instance(); + sp->enable_processing(); + sp->set_call_back("encrypt_ak_sk:get_encryption_key_ret", + [](void* p) { *reinterpret_cast<int*>(p) = 0; }); + sp->set_call_back("encrypt_ak_sk:get_encryption_key", [](void* p) { + *reinterpret_cast<std::string*>(p) = "selectdbselectdbselectdbselectdb"; + }); + sp->set_call_back("encrypt_ak_sk:get_encryption_key_id", + [](void* p) { *reinterpret_cast<int*>(p) = 1; }); + + std::unique_ptr<Transaction> txn; + ASSERT_EQ(meta_service->txn_kv()->create_txn(&txn), TxnErrorCode::TXN_OK); + std::string key; + std::string val; + InstanceKeyInfo key_info {"test_instance"}; + instance_key(key_info, &key); + + InstanceInfoPB instance; + val = instance.SerializeAsString(); + txn->put(key, val); + ASSERT_EQ(txn->commit(), TxnErrorCode::TXN_OK); + + auto get_test_instance = [&](InstanceInfoPB& i) { + std::string key; + std::string val; + std::unique_ptr<Transaction> txn; + ASSERT_EQ(meta_service->txn_kv()->create_txn(&txn), TxnErrorCode::TXN_OK); + InstanceKeyInfo key_info {"test_instance"}; + instance_key(key_info, &key); + ASSERT_EQ(txn->get(key, &val), TxnErrorCode::TXN_OK); + i.ParseFromString(val); + }; + + // tablet_metas_size is 0 + { + CreateTabletsRequest request; + request.set_cloud_unique_id("test_cloud_unique_id"); + + brpc::Controller cntl; + CreateTabletsResponse response; + meta_service->create_tablets(reinterpret_cast<::google::protobuf::RpcController*>(&cntl), + &request, &response, nullptr); + ASSERT_EQ(response.status().code(), MetaServiceCode::INVALID_ARGUMENT) + << response.status().msg(); + } + + // try to use default + { + CreateTabletsRequest req; + req.set_cloud_unique_id("test_cloud_unique_id"); + req.set_storage_vault_name(""); + req.add_tablet_metas(); + + brpc::Controller cntl; + CreateTabletsResponse res; + meta_service->create_tablets(reinterpret_cast<::google::protobuf::RpcController*>(&cntl), + &req, &res, nullptr); + // failed because no default + ASSERT_EQ(res.status().code(), MetaServiceCode::INVALID_ARGUMENT) << res.status().msg(); + } + + // Create One Hdfs info as built_in vault + { + AlterObjStoreInfoRequest req; + req.set_cloud_unique_id("test_cloud_unique_id"); + req.set_op(AlterObjStoreInfoRequest::ADD_HDFS_INFO); + StorageVaultPB hdfs; + hdfs.set_name("test_alter_add_hdfs_info"); + HdfsVaultInfo params; + params.mutable_build_conf()->set_fs_name("hdfs://ip:port"); + + hdfs.mutable_hdfs_info()->CopyFrom(params); + req.mutable_hdfs()->CopyFrom(hdfs); + + brpc::Controller cntl; + AlterObjStoreInfoResponse res; + meta_service->alter_obj_store_info( + reinterpret_cast<::google::protobuf::RpcController*>(&cntl), &req, &res, nullptr); + ASSERT_EQ(res.status().code(), MetaServiceCode::OK) << res.status().msg(); + + InstanceInfoPB i; + get_test_instance(i); + ASSERT_EQ(i.default_storage_vault_id(), "1"); + ASSERT_EQ(i.default_storage_vault_name(), "built_in_storage_vault"); + } + + // try to use default vault + { + CreateTabletsRequest req; + req.set_cloud_unique_id("test_cloud_unique_id"); + req.set_storage_vault_name(""); + req.add_tablet_metas(); + + auto sp = SyncPoint::get_instance(); + sp->set_call_back("create_tablets::pred", + [](void* pred) { *reinterpret_cast<bool*>(pred) = true; }); + sp->enable_processing(); + + brpc::Controller cntl; + CreateTabletsResponse res; + meta_service->create_tablets(reinterpret_cast<::google::protobuf::RpcController*>(&cntl), + &req, &res, nullptr); + ASSERT_EQ(res.status().code(), MetaServiceCode::OK) << res.status().msg(); + ASSERT_EQ(res.storage_vault_id(), "1"); + ASSERT_EQ(res.storage_vault_name(), "built_in_storage_vault"); + + sp->clear_call_back("create_tablets::pred"); + } + + // try to use one non-existent vault + { + CreateTabletsRequest req; + req.set_cloud_unique_id("test_cloud_unique_id"); + req.set_storage_vault_name("non-existent"); + req.add_tablet_metas(); + + auto sp = SyncPoint::get_instance(); Review Comment: warning: 'auto sp' can be declared as 'auto *sp' [readability-qualified-auto] ```suggestion auto *sp = SyncPoint::get_instance(); ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org