This is an automated email from the ASF dual-hosted git repository.
apitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 145c4d35b2 GH-40731: [C++][Parquet] Minor enhancement code of
encryption (#40732)
145c4d35b2 is described below
commit 145c4d35b273fd1a6cfa7c7adad23e6c02ae9a61
Author: mwish <[email protected]>
AuthorDate: Tue Mar 26 17:28:04 2024 +0800
GH-40731: [C++][Parquet] Minor enhancement code of encryption (#40732)
### Rationale for this change
* Applying more `std::move` in encryption
* Trying to use `NULLPTR` rather than `NULL`.
### What changes are included in this PR?
Applying more `std::move` in encryption
### Are these changes tested?
No need
### Are there any user-facing changes?
No
* GitHub Issue: #40731
Authored-by: mwish <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/parquet/encryption/crypto_factory.cc | 2 +-
cpp/src/parquet/encryption/file_key_unwrapper.cc | 14 +++++++++-----
cpp/src/parquet/encryption/file_key_unwrapper.h | 4 +++-
cpp/src/parquet/encryption/file_key_wrapper.cc | 7 ++++---
cpp/src/parquet/encryption/key_management_test.cc | 2 +-
cpp/src/parquet/encryption/key_toolkit.cc | 2 +-
cpp/src/parquet/encryption/key_toolkit.h | 4 ++--
cpp/src/parquet/encryption/kms_client.cc | 2 +-
cpp/src/parquet/encryption/kms_client.h | 2 +-
9 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/cpp/src/parquet/encryption/crypto_factory.cc
b/cpp/src/parquet/encryption/crypto_factory.cc
index 2cf15b5680..72506bdc01 100644
--- a/cpp/src/parquet/encryption/crypto_factory.cc
+++ b/cpp/src/parquet/encryption/crypto_factory.cc
@@ -30,7 +30,7 @@ namespace parquet::encryption {
void CryptoFactory::RegisterKmsClientFactory(
std::shared_ptr<KmsClientFactory> kms_client_factory) {
- key_toolkit_->RegisterKmsClientFactory(kms_client_factory);
+ key_toolkit_->RegisterKmsClientFactory(std::move(kms_client_factory));
}
std::shared_ptr<FileEncryptionProperties>
CryptoFactory::GetFileEncryptionProperties(
diff --git a/cpp/src/parquet/encryption/file_key_unwrapper.cc
b/cpp/src/parquet/encryption/file_key_unwrapper.cc
index 9e430de853..02bea127fd 100644
--- a/cpp/src/parquet/encryption/file_key_unwrapper.cc
+++ b/cpp/src/parquet/encryption/file_key_unwrapper.cc
@@ -30,33 +30,37 @@ FileKeyUnwrapper::FileKeyUnwrapper(
const std::string& file_path,
const std::shared_ptr<::arrow::fs::FileSystem>& file_system)
: FileKeyUnwrapper(std::move(key_toolkit), /*key_toolkit=*/nullptr,
- kms_connection_config, cache_lifetime_seconds,
file_path,
- file_system) {}
+ kms_connection_config, cache_lifetime_seconds,
+ /*key_material_store=*/nullptr, file_path, file_system)
{}
FileKeyUnwrapper::FileKeyUnwrapper(
KeyToolkit* key_toolkit, const KmsConnectionConfig& kms_connection_config,
double cache_lifetime_seconds, const std::string& file_path,
const std::shared_ptr<::arrow::fs::FileSystem>& file_system)
: FileKeyUnwrapper(/*key_toolkit_owner=*/nullptr, key_toolkit,
kms_connection_config,
- cache_lifetime_seconds, file_path, file_system) {}
+ cache_lifetime_seconds, /*key_material_store=*/nullptr,
file_path,
+ file_system) {}
FileKeyUnwrapper::FileKeyUnwrapper(
KeyToolkit* key_toolkit, const KmsConnectionConfig& kms_connection_config,
double cache_lifetime_seconds,
std::shared_ptr<FileKeyMaterialStore> key_material_store)
: FileKeyUnwrapper(/*key_toolkit_owner=*/nullptr, key_toolkit,
kms_connection_config,
- cache_lifetime_seconds, /*file_path=*/"",
+ cache_lifetime_seconds, std::move(key_material_store),
+ /*file_path=*/"",
/*file_system=*/nullptr) {}
FileKeyUnwrapper::FileKeyUnwrapper(
std::shared_ptr<KeyToolkit> key_toolkit_owner, KeyToolkit* key_toolkit,
const KmsConnectionConfig& kms_connection_config, double
cache_lifetime_seconds,
+ std::shared_ptr<FileKeyMaterialStore> key_material_store,
const std::string& file_path,
const std::shared_ptr<::arrow::fs::FileSystem>& file_system)
: key_toolkit_owner_(std::move(key_toolkit_owner)),
key_toolkit_(key_toolkit ? key_toolkit : key_toolkit_owner_.get()),
kms_connection_config_(kms_connection_config),
cache_entry_lifetime_seconds_(cache_lifetime_seconds),
+ key_material_store_(std::move(key_material_store)),
file_path_(file_path),
file_system_(file_system) {
kek_per_kek_id_ =
key_toolkit_->kek_read_cache_per_token().GetOrCreateInternalCache(
@@ -120,7 +124,7 @@ KeyWithMasterId
FileKeyUnwrapper::GetDataEncryptionKey(const KeyMaterial& key_ma
data_key = internal::DecryptKeyLocally(encoded_wrapped_dek, kek_bytes,
aad);
}
- return KeyWithMasterId(data_key, master_key_id);
+ return KeyWithMasterId(std::move(data_key), std::move(master_key_id));
}
std::shared_ptr<KmsClient>
FileKeyUnwrapper::GetKmsClientFromConfigOrKeyMaterial(
diff --git a/cpp/src/parquet/encryption/file_key_unwrapper.h
b/cpp/src/parquet/encryption/file_key_unwrapper.h
index d05c3efd17..c60c0c71ba 100644
--- a/cpp/src/parquet/encryption/file_key_unwrapper.h
+++ b/cpp/src/parquet/encryption/file_key_unwrapper.h
@@ -73,7 +73,9 @@ class PARQUET_EXPORT FileKeyUnwrapper : public
DecryptionKeyRetriever {
private:
FileKeyUnwrapper(std::shared_ptr<KeyToolkit> key_toolkit_owner, KeyToolkit*
key_toolkit,
const KmsConnectionConfig& kms_connection_config,
- double cache_lifetime_seconds, const std::string& file_path,
+ double cache_lifetime_seconds,
+ std::shared_ptr<FileKeyMaterialStore> key_material_store,
+ const std::string& file_path,
const std::shared_ptr<::arrow::fs::FileSystem>&
file_system);
std::shared_ptr<KmsClient> GetKmsClientFromConfigOrKeyMaterial(
diff --git a/cpp/src/parquet/encryption/file_key_wrapper.cc
b/cpp/src/parquet/encryption/file_key_wrapper.cc
index 704651ebaa..032ae45821 100644
--- a/cpp/src/parquet/encryption/file_key_wrapper.cc
+++ b/cpp/src/parquet/encryption/file_key_wrapper.cc
@@ -53,7 +53,7 @@ std::string FileKeyWrapper::GetEncryptionKeyMetadata(const
std::string& data_key
const std::string&
master_key_id,
bool is_footer_key,
std::string
key_id_in_file) {
- if (kms_client_ == NULL) {
+ if (kms_client_ == NULLPTR) {
throw ParquetException("No KMS client available. See previous errors.");
}
@@ -103,7 +103,7 @@ std::string FileKeyWrapper::GetEncryptionKeyMetadata(const
std::string& data_key
key_counter_++;
}
}
- key_material_store_->AddKeyMaterial(key_id_in_file, serialized_key_material);
+ key_material_store_->AddKeyMaterial(key_id_in_file,
std::move(serialized_key_material));
std::string serialized_key_metadata =
KeyMetadata::CreateSerializedForExternalMaterial(key_id_in_file);
return serialized_key_metadata;
@@ -120,7 +120,8 @@ KeyEncryptionKey FileKeyWrapper::CreateKeyEncryptionKey(
// Encrypt KEK with Master key
std::string encoded_wrapped_kek = kms_client_->WrapKey(kek_bytes,
master_key_id);
- return KeyEncryptionKey(kek_bytes, kek_id, encoded_wrapped_kek);
+ return KeyEncryptionKey(std::move(kek_bytes), std::move(kek_id),
+ std::move(encoded_wrapped_kek));
}
} // namespace parquet::encryption
diff --git a/cpp/src/parquet/encryption/key_management_test.cc
b/cpp/src/parquet/encryption/key_management_test.cc
index 2ae8ed96b6..1506a00a14 100644
--- a/cpp/src/parquet/encryption/key_management_test.cc
+++ b/cpp/src/parquet/encryption/key_management_test.cc
@@ -69,7 +69,7 @@ class TestEncryptionKeyManagement : public ::testing::Test {
wrap_locally_ = wrap_locally;
std::shared_ptr<KmsClientFactory> kms_client_factory =
std::make_shared<TestOnlyInMemoryKmsClientFactory>(wrap_locally,
key_list_);
- crypto_factory_.RegisterKmsClientFactory(kms_client_factory);
+ crypto_factory_.RegisterKmsClientFactory(std::move(kms_client_factory));
}
std::string GetFileName(bool double_wrapping, bool wrap_locally,
diff --git a/cpp/src/parquet/encryption/key_toolkit.cc
b/cpp/src/parquet/encryption/key_toolkit.cc
index cb488d3fa2..81e102126d 100644
--- a/cpp/src/parquet/encryption/key_toolkit.cc
+++ b/cpp/src/parquet/encryption/key_toolkit.cc
@@ -31,7 +31,7 @@ namespace parquet::encryption {
std::shared_ptr<KmsClient> KeyToolkit::GetKmsClient(
const KmsConnectionConfig& kms_connection_config, double
cache_entry_lifetime_ms) {
- if (kms_client_factory_ == NULL) {
+ if (kms_client_factory_ == nullptr) {
throw ParquetException("No KmsClientFactory is registered.");
}
auto kms_client_per_kms_instance_cache =
diff --git a/cpp/src/parquet/encryption/key_toolkit.h
b/cpp/src/parquet/encryption/key_toolkit.h
index f63ade4c8c..339692a99a 100644
--- a/cpp/src/parquet/encryption/key_toolkit.h
+++ b/cpp/src/parquet/encryption/key_toolkit.h
@@ -62,10 +62,10 @@ class PARQUET_EXPORT KeyToolkit {
void RemoveCacheEntriesForAllTokens();
void RegisterKmsClientFactory(std::shared_ptr<KmsClientFactory>
kms_client_factory) {
- if (kms_client_factory_ != NULL) {
+ if (kms_client_factory_ != NULLPTR) {
throw ParquetException("KMS client factory has already been
registered.");
}
- kms_client_factory_ = kms_client_factory;
+ kms_client_factory_ = std::move(kms_client_factory);
}
/// Key rotation. In the single wrapping mode, decrypts data keys with old
master keys,
diff --git a/cpp/src/parquet/encryption/kms_client.cc
b/cpp/src/parquet/encryption/kms_client.cc
index fee03dd3db..0ce41da177 100644
--- a/cpp/src/parquet/encryption/kms_client.cc
+++ b/cpp/src/parquet/encryption/kms_client.cc
@@ -34,7 +34,7 @@ void KmsConnectionConfig::SetDefaultIfEmpty() {
if (kms_instance_url.empty()) {
kms_instance_url = KmsClient::kKmsInstanceUrlDefault;
}
- if (refreshable_key_access_token == NULL) {
+ if (refreshable_key_access_token == nullptr) {
refreshable_key_access_token = std::make_shared<KeyAccessToken>();
}
}
diff --git a/cpp/src/parquet/encryption/kms_client.h
b/cpp/src/parquet/encryption/kms_client.h
index a55fd552ee..ef363d9c2c 100644
--- a/cpp/src/parquet/encryption/kms_client.h
+++ b/cpp/src/parquet/encryption/kms_client.h
@@ -63,7 +63,7 @@ struct PARQUET_EXPORT KmsConnectionConfig {
KmsConnectionConfig();
const std::string& key_access_token() const {
- if (refreshable_key_access_token == NULL ||
+ if (refreshable_key_access_token == NULLPTR ||
refreshable_key_access_token->value().empty()) {
throw ParquetException("key access token is not set!");
}
