From e3462da7718f35cfa1335c3955509fc476386658 Mon Sep 17 00:00:00 2001 From: Muhammad Hamza Sajjad Date: Tue, 20 Feb 2024 18:03:26 +0000 Subject: [PATCH] Refactor formatting + code style --- .../storage/azure/azure_client_wrapper.hpp | 17 ++++++++------ .../storage/azure/azure_mock_client.cpp | 2 +- .../storage/azure/azure_mock_client.hpp | 2 +- cpp/arcticdb/storage/azure/azure_storage.cpp | 23 +++++++++---------- .../storage/test/test_storage_exceptions.cpp | 10 ++++---- 5 files changed, 28 insertions(+), 26 deletions(-) diff --git a/cpp/arcticdb/storage/azure/azure_client_wrapper.hpp b/cpp/arcticdb/storage/azure/azure_client_wrapper.hpp index 44ea19c32d1..bfae88ae648 100644 --- a/cpp/arcticdb/storage/azure/azure_client_wrapper.hpp +++ b/cpp/arcticdb/storage/azure/azure_client_wrapper.hpp @@ -19,7 +19,7 @@ namespace arcticdb::storage::azure { static const size_t BATCH_SUBREQUEST_LIMIT = 256; //https://github.com/Azure/azure-sdk-for-python/blob/767facc39f2487504bcde4e627db16a79f96b297/sdk/storage/azure-storage-blob/azure/storage/blob/_container_client.py#L1608 // some common error codes as per https://learn.microsoft.com/en-us/rest/api/storageservices/blob-service-error-codes -enum AzureErrorCode { +enum class AzureErrorCode { BlobAlreadyExists, BlobNotFound, ContainerNotFound, @@ -30,12 +30,15 @@ enum AzureErrorCode { }; inline std::string AzureErrorCode_to_string(AzureErrorCode error) { - if(error == AzureErrorCode::BlobAlreadyExists) return "BlobAlreadyExists"; - if(error == AzureErrorCode::BlobNotFound) return "BlobNotFound"; - if(error == AzureErrorCode::ContainerNotFound) return "ContainerNotFound"; - if(error == AzureErrorCode::BlobOperationNotSupported) return "BlobOperationNotSupported"; - if(error == AzureErrorCode::UnauthorizedBlobOverwrite) return "UnauthorizedBlobOverwrite"; - if(error == AzureErrorCode::InvalidBlobOrBlock) return "InvalidBlobOrBlock"; + switch (error) { + case AzureErrorCode::BlobAlreadyExists: return "BlobAlreadyExists"; + case AzureErrorCode::BlobNotFound: return "BlobNotFound"; + case AzureErrorCode::ContainerNotFound: return "ContainerNotFound"; + case AzureErrorCode::BlobOperationNotSupported: return "BlobOperationNotSupported"; + case AzureErrorCode::UnauthorizedBlobOverwrite: return "UnauthorizedBlobOverwrite"; + case AzureErrorCode::InvalidBlobOrBlock: return "InvalidBlobOrBlock"; + case AzureErrorCode::OtherError: return "Other Unspecified error"; + } return "Other Unspecified error"; } diff --git a/cpp/arcticdb/storage/azure/azure_mock_client.cpp b/cpp/arcticdb/storage/azure/azure_mock_client.cpp index 44874ecc8e5..71422940529 100644 --- a/cpp/arcticdb/storage/azure/azure_mock_client.cpp +++ b/cpp/arcticdb/storage/azure/azure_mock_client.cpp @@ -30,7 +30,7 @@ std::string operation_to_string(AzureOperation operation){ std::string MockAzureClient::get_failure_trigger( const std::string& blob_name, AzureOperation operation_to_fail, - std::string error_code, + const std::string& error_code, Azure::Core::Http::HttpStatusCode error_to_fail_with) { return fmt::format("{}#Failure_{}_{}_{}", blob_name, operation_to_string(operation_to_fail), error_code, (int)error_to_fail_with); } diff --git a/cpp/arcticdb/storage/azure/azure_mock_client.hpp b/cpp/arcticdb/storage/azure/azure_mock_client.hpp index 5cc332ef42e..ed7bc3f9775 100644 --- a/cpp/arcticdb/storage/azure/azure_mock_client.hpp +++ b/cpp/arcticdb/storage/azure/azure_mock_client.hpp @@ -51,7 +51,7 @@ class MockAzureClient : public AzureClientWrapper { static std::string get_failure_trigger( const std::string& blob_name, AzureOperation operation_to_fail, - std::string error_code, + const std::string& error_code, Azure::Core::Http::HttpStatusCode error_to_fail_with); private: diff --git a/cpp/arcticdb/storage/azure/azure_storage.cpp b/cpp/arcticdb/storage/azure/azure_storage.cpp index 59de8d9c35b..8f218bb8f41 100644 --- a/cpp/arcticdb/storage/azure/azure_storage.cpp +++ b/cpp/arcticdb/storage/azure/azure_storage.cpp @@ -48,14 +48,14 @@ namespace fg = folly::gen; namespace detail { // TODO: fix this temporary workaround to read error code. azure-sdk-cpp client sometimes doesn't properly set the error code. -// This issue has been raised on the sdk repo. Once fixed, we should no longer need the following function and would just read e.ErrorCode. +// This issue has been raised on the sdk repo https://github.com/Azure/azure-sdk-for-cpp/issues/5369. Once fixed, we should no longer need the following function and would just read e.ErrorCode. std::string get_error_code(const Azure::Core::RequestFailedException& e) { auto error_code = e.ErrorCode; if(error_code.empty() && e.RawResponse ) { auto headers_map = e.RawResponse->GetHeaders(); - if(headers_map.find("x-ms-error-code") != headers_map.end()) { - error_code = headers_map["x-ms-error-code"]; + if(auto ec = headers_map.find("x-ms-error-code") ; ec != headers_map.end()) { + error_code = ec->second; } } return error_code; @@ -68,29 +68,28 @@ void raise_azure_exception(const Azure::Core::RequestFailedException& e) { if(status_code == Azure::Core::Http::HttpStatusCode::NotFound && error_code == AzureErrorCode_to_string(AzureErrorCode::BlobNotFound)) { throw KeyNotFoundException(fmt::format("Key Not Found Error: AzureError#{} {}: {}", - int(status_code), error_code, e.ReasonPhrase)); + static_cast(status_code), error_code, e.ReasonPhrase)); } if(status_code == Azure::Core::Http::HttpStatusCode::Unauthorized || status_code == Azure::Core::Http::HttpStatusCode::Forbidden) { raise(fmt::format("Permission error: AzureError#{} {}: {}", - int(status_code), error_code, e.ReasonPhrase)); + static_cast(status_code), error_code, e.ReasonPhrase)); } - if((int) status_code >= 500) { + if(static_cast(status_code) >= 500) { error_message = fmt::format("Unexpected Server Error: AzureError#{} {}: {} {}", - int(status_code), error_code, e.ReasonPhrase, e.what()); - } - else { + static_cast(status_code), error_code, e.ReasonPhrase, e.what()); + } else { error_message = fmt::format("Unexpected Error: AzureError#{} {}: {} {}", - int(status_code), error_code, e.ReasonPhrase, e.what()); + static_cast(status_code), error_code, e.ReasonPhrase, e.what()); } raise(error_message); } bool is_expected_error_type(const std::string& error_code, Azure::Core::Http::HttpStatusCode status_code) { - return (status_code == Azure::Core::Http::HttpStatusCode::NotFound && error_code == AzureErrorCode_to_string(AzureErrorCode::BlobNotFound)) || - (status_code == Azure::Core::Http::HttpStatusCode::NotFound && error_code == AzureErrorCode_to_string(AzureErrorCode::ContainerNotFound)); + return status_code == Azure::Core::Http::HttpStatusCode::NotFound && (error_code == AzureErrorCode_to_string(AzureErrorCode::BlobNotFound) || + error_code == AzureErrorCode_to_string(AzureErrorCode::ContainerNotFound)); } void raise_if_unexpected_error(const Azure::Core::RequestFailedException& e) { diff --git a/cpp/arcticdb/storage/test/test_storage_exceptions.cpp b/cpp/arcticdb/storage/test/test_storage_exceptions.cpp index 55ee517f134..5ef56159913 100644 --- a/cpp/arcticdb/storage/test/test_storage_exceptions.cpp +++ b/cpp/arcticdb/storage/test/test_storage_exceptions.cpp @@ -130,7 +130,7 @@ TEST_P(GenericStorageTest, WriteDuplicateKeyException) { } TEST_P(GenericStorageTest, ReadKeyNotFoundException) { - ASSERT_TRUE(!exists_in_store(*storage, "sym")); + ASSERT_FALSE(exists_in_store(*storage, "sym")); ASSERT_THROW({ read_in_store(*storage, "sym"); }, arcticdb::storage::KeyNotFoundException); @@ -138,7 +138,7 @@ TEST_P(GenericStorageTest, ReadKeyNotFoundException) { } TEST_P(GenericStorageTest, UpdateKeyNotFoundException) { - ASSERT_TRUE(!exists_in_store(*storage, "sym")); + ASSERT_FALSE(exists_in_store(*storage, "sym")); ASSERT_THROW({ update_in_store(*storage, "sym"); }, arcticdb::storage::KeyNotFoundException); @@ -146,7 +146,7 @@ TEST_P(GenericStorageTest, UpdateKeyNotFoundException) { } TEST_P(GenericStorageTest, RemoveKeyNotFoundException) { - ASSERT_TRUE(!exists_in_store(*storage, "sym")); + ASSERT_FALSE(exists_in_store(*storage, "sym")); ASSERT_THROW({ remove_in_store(*storage, {"sym"}); }, arcticdb::storage::KeyNotFoundException); @@ -204,7 +204,7 @@ TEST(S3MockStorageTest, TestReadKeyNotFoundException) { S3MockStorageFactory factory; auto storage = factory.create(); - ASSERT_TRUE(!exists_in_store(*storage, "sym")); + ASSERT_FALSE(exists_in_store(*storage, "sym")); ASSERT_THROW({ read_in_store(*storage, "sym"); }, arcticdb::storage::KeyNotFoundException); @@ -263,7 +263,7 @@ TEST(AzureMockStorageTest, TestReadKeyNotFoundException) { AzureMockStorageFactory factory; auto storage = factory.create(); - ASSERT_TRUE(!exists_in_store(*storage, "sym")); + ASSERT_FALSE(exists_in_store(*storage, "sym")); ASSERT_THROW({ read_in_store(*storage, "sym"); }, arcticdb::storage::KeyNotFoundException);