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

Reply via email to