From f2b8257b8f410d19ed06fd3c92060b45be61128c Mon Sep 17 00:00:00 2001 From: gearama <50641385+gearama@users.noreply.github.com> Date: Tue, 21 Jan 2025 16:34:19 -0800 Subject: [PATCH] update update approach v2 (#6339) * update update v2 * clang * Update sdk/tables/azure-data-tables/inc/azure/data/tables/table_client.hpp Co-authored-by: Ahson Khan * PR comments * remove etag * tired of trying to be smart * clang * const * fsd * fsdf * clang * clarify the headers * clangs * fsdfsd * gdfggdf * more pr * ooops --------- Co-authored-by: Ahson Khan --- sdk/tables/assets.json | 2 +- .../inc/azure/data/tables/models.hpp | 51 --------- .../inc/azure/data/tables/table_client.hpp | 57 ++++++---- .../data/tables/table_service_client.hpp | 12 +- .../azure-data-tables/src/table_clients.cpp | 105 +++++++++--------- .../test/ut/table_client_test.cpp | 37 +++++- 6 files changed, 129 insertions(+), 135 deletions(-) diff --git a/sdk/tables/assets.json b/sdk/tables/assets.json index ddb764b03..9d45ade1c 100644 --- a/sdk/tables/assets.json +++ b/sdk/tables/assets.json @@ -2,5 +2,5 @@ "AssetsRepo": "Azure/azure-sdk-assets", "AssetsRepoPrefixPath": "cpp", "TagPrefix": "cpp/tables", - "Tag": "cpp/tables_e3324fac2a" + "Tag": "cpp/tables_ab62ccac48" } diff --git a/sdk/tables/azure-data-tables/inc/azure/data/tables/models.hpp b/sdk/tables/azure-data-tables/inc/azure/data/tables/models.hpp index 056a97d08..f6d9bc6c4 100644 --- a/sdk/tables/azure-data-tables/inc/azure/data/tables/models.hpp +++ b/sdk/tables/azure-data-tables/inc/azure/data/tables/models.hpp @@ -546,16 +546,6 @@ namespace Azure { namespace Data { namespace Tables { } }; - /** - * @brief Upsert Kind - * - */ - enum class UpsertKind - { - Update, - Merge, - }; - /** * @brief Add Entity result. * @@ -600,47 +590,6 @@ namespace Azure { namespace Data { namespace Tables { { }; - /** - * @brief Upsert Entity result. - * - */ - struct UpsertEntityResult final : public MergeEntityResult, UpdateEntityResult, AddEntityResult - { - /** - * ETag - */ - std::string ETag; - /** - * @brief Upsert Entity result default constructor. - * - */ - UpsertEntityResult() = default; - /** - * @brief Upsert Entity result constructor. - * - * @param other Merge Entity result. - */ - UpsertEntityResult(MergeEntityResult const& other) - : MergeEntityResult(other), ETag(other.ETag) - { - } - /** - * @brief Upsert Entity result constructor. - * - * @param other Update Entity result. - */ - UpsertEntityResult(UpdateEntityResult const& other) - : UpdateEntityResult(other), ETag(other.ETag) - { - } - /** - * @brief Upsert Entity result constructor. - * - * @param other Add Entity result. - */ - UpsertEntityResult(AddEntityResult const& other) : AddEntityResult(other), ETag(other.ETag) {} - }; - /** * @brief Query Entities options. * diff --git a/sdk/tables/azure-data-tables/inc/azure/data/tables/table_client.hpp b/sdk/tables/azure-data-tables/inc/azure/data/tables/table_client.hpp index 3756b2148..6c32d4169 100644 --- a/sdk/tables/azure-data-tables/inc/azure/data/tables/table_client.hpp +++ b/sdk/tables/azure-data-tables/inc/azure/data/tables/table_client.hpp @@ -95,7 +95,7 @@ namespace Azure { namespace Data { namespace Tables { */ Response AddEntity( Models::TableEntity const& tableEntity, - Core::Context const& context = {}); + Core::Context const& context = {}) const; /** * @brief Update entity in a table. @@ -106,7 +106,7 @@ namespace Azure { namespace Data { namespace Tables { */ Response UpdateEntity( Models::TableEntity const& tableEntity, - Core::Context const& context = {}); + Core::Context const& context = {}) const; /** * @brief Merge entity in a table. @@ -117,7 +117,7 @@ namespace Azure { namespace Data { namespace Tables { */ Response MergeEntity( Models::TableEntity const& tableEntity, - Core::Context const& context = {}); + Core::Context const& context = {}) const; /** * @brief Deletes the specified entity in a table. @@ -128,21 +128,29 @@ namespace Azure { namespace Data { namespace Tables { */ Response DeleteEntity( Models::TableEntity const& tableEntity, - Core::Context const& context = {}); + Core::Context const& context = {}) const; /** - * @brief Upsert specified entity in a table. + * @brief Update or insert specified entity in a table. * - * @param tableEntity The TableEntity to upsert. - * @param upsertKind Upsert kind(update/merge), default update. + * @param tableEntity The TableEntity to update or insert. * @param context for canceling long running operations. - * @return Upsert entity result. + * @return Update entity result. */ - Response UpsertEntity( + Response UpdateOrInsertEntity( Models::TableEntity const& tableEntity, - Models::UpsertKind upsertKind = Models::UpsertKind::Update, - Core::Context const& context = {}); + Core::Context const& context = {}) const; + /** + * @brief Merge or insert the specified entity in a table. + * + * @param tableEntity The TableEntity to merge or insert. + * @param context for canceling long running operations. + * @return Merge entity result. + */ + Response MergeOrInsertEntity( + Models::TableEntity const& tableEntity, + Core::Context const& context = {}) const; /** * @brief Queries entities in a table. * @@ -152,7 +160,7 @@ namespace Azure { namespace Data { namespace Tables { */ Models::QueryEntitiesPagedResponse QueryEntities( Models::QueryEntitiesOptions const& options = {}, - Core::Context const& context = {}); + Core::Context const& context = {}) const; /** * @brief Queries a single entity in a table. @@ -165,7 +173,7 @@ namespace Azure { namespace Data { namespace Tables { Response GetEntity( std::string const& partitionKey, std::string const& rowKey, - Core::Context const& context = {}); + Core::Context const& context = {}) const; /** * @brief Submits a transaction. @@ -176,7 +184,7 @@ namespace Azure { namespace Data { namespace Tables { */ Response SubmitTransaction( std::vector const& steps, - Core::Context const& context = {}); + Core::Context const& context = {}) const; private: #ifdef _azure_TABLES_TESTING_BUILD @@ -190,15 +198,24 @@ namespace Azure { namespace Data { namespace Tables { friend class Azure::Data::Test::TransactionsBodyTest_TransactionBodyAddOp_Test; #endif + Response UpdateEntityImpl( + Models::TableEntity const& tableEntity, + bool isUpsert, + Core::Context const& context = {}) const; + + Response MergeEntityImpl( + Models::TableEntity const& tableEntity, + bool isUpsert, + Core::Context const& context = {}) const; std::string PreparePayload( std::string const& batchId, std::string const& changesetId, - std::vector const& steps); - std::string PrepAddEntity(std::string const& changesetId, Models::TableEntity entity); - std::string PrepDeleteEntity(std::string const& changesetId, Models::TableEntity entity); - std::string PrepMergeEntity(std::string const& changesetId, Models::TableEntity entity); - std::string PrepUpdateEntity(std::string const& changesetId, Models::TableEntity entity); - std::string PrepInsertEntity(std::string const& changesetId, Models::TableEntity entity); + std::vector const& steps) const; + std::string PrepAddEntity(std::string const& changesetId, Models::TableEntity entity) const; + std::string PrepDeleteEntity(std::string const& changesetId, Models::TableEntity entity) const; + std::string PrepMergeEntity(std::string const& changesetId, Models::TableEntity entity) const; + std::string PrepUpdateEntity(std::string const& changesetId, Models::TableEntity entity) const; + std::string PrepInsertEntity(std::string const& changesetId, Models::TableEntity entity) const; std::shared_ptr m_pipeline; Core::Url m_url; std::string m_tableName; diff --git a/sdk/tables/azure-data-tables/inc/azure/data/tables/table_service_client.hpp b/sdk/tables/azure-data-tables/inc/azure/data/tables/table_service_client.hpp index 94ebcc857..8f47e22be 100644 --- a/sdk/tables/azure-data-tables/inc/azure/data/tables/table_service_client.hpp +++ b/sdk/tables/azure-data-tables/inc/azure/data/tables/table_service_client.hpp @@ -59,7 +59,7 @@ namespace Azure { namespace Data { namespace Tables { */ Response CreateTable( std::string const& tableName, - Core::Context const& context = {}); + Core::Context const& context = {}) const; /** * @brief Operation permanently deletes the specified table. @@ -70,7 +70,7 @@ namespace Azure { namespace Data { namespace Tables { */ Response DeleteTable( std::string const& tableName, - Core::Context const& context = {}); + Core::Context const& context = {}) const; /** * @brief Queries tables under the given account. @@ -91,7 +91,7 @@ namespace Azure { namespace Data { namespace Tables { */ Response SetServiceProperties( Models::SetServicePropertiesOptions const& options = {}, - Core::Context const& context = {}); + Core::Context const& context = {}) const; /** * @brief Get service properties @@ -100,7 +100,7 @@ namespace Azure { namespace Data { namespace Tables { * @return Get service properties result. */ Response GetServiceProperties( - Core::Context const& context = {}); + Core::Context const& context = {}) const; /** * @brief Get service statistics @@ -108,7 +108,7 @@ namespace Azure { namespace Data { namespace Tables { * @param context for canceling long running operations. * @return Get service statistics result. */ - Response GetStatistics(Core::Context const& context = {}); + Response GetStatistics(Core::Context const& context = {}) const; /** * @brief Pre flight check @@ -119,7 +119,7 @@ namespace Azure { namespace Data { namespace Tables { */ Response PreflightCheck( Models::PreflightCheckOptions const& options, - Core::Context const& context = {}); + Core::Context const& context = {}) const; /** * @brief Get table client. * diff --git a/sdk/tables/azure-data-tables/src/table_clients.cpp b/sdk/tables/azure-data-tables/src/table_clients.cpp index 35c0f6d09..1f530b03c 100644 --- a/sdk/tables/azure-data-tables/src/table_clients.cpp +++ b/sdk/tables/azure-data-tables/src/table_clients.cpp @@ -82,7 +82,7 @@ TableClient TableServiceClient::GetTableClient( Azure::Response TableServiceClient::PreflightCheck( Models::PreflightCheckOptions const& options, - Core::Context const& context) + Core::Context const& context) const { auto url = m_url; url.AppendPath(Azure::Core::Url::Encode(options.TableName)); @@ -104,7 +104,7 @@ Azure::Response TableServiceClient::PreflightCheck Azure::Response TableServiceClient::SetServiceProperties( Models::SetServicePropertiesOptions const& options, - Core::Context const& context) + Core::Context const& context) const { std::string xmlBody = Serializers::SetServiceProperties(options); auto url = m_url; @@ -133,7 +133,7 @@ Azure::Response TableServiceClient::SetServi } Azure::Response TableServiceClient::GetServiceProperties( - Core::Context const& context) + Core::Context const& context) const { auto url = m_url; url.AppendQueryParameter(ResourceTypeHeader, ResourceTypeService); @@ -156,7 +156,7 @@ Azure::Response TableServiceClient::GetServicePr } Azure::Response TableServiceClient::GetStatistics( - const Core::Context& context) + const Core::Context& context) const { auto url = m_url; std::string host = url.GetHost(); @@ -285,7 +285,7 @@ TableClient::TableClient( Azure::Response TableServiceClient::CreateTable( std::string const& tableName, - Core::Context const& context) + Core::Context const& context) const { auto url = m_url; url.AppendPath("Tables"); @@ -395,7 +395,7 @@ Models::QueryTablesPagedResponse TableServiceClient::QueryTables( Azure::Response TableServiceClient::DeleteTable( std::string const& tableName, - Core::Context const& context) + Core::Context const& context) const { auto url = m_url; url.AppendPath("Tables('" + Azure::Core::Url::Encode(tableName) + ClosingFragment); @@ -419,7 +419,7 @@ Azure::Response TableServiceClient::DeleteTable( Azure::Response TableClient::AddEntity( Models::TableEntity const& tableEntity, - Core::Context const& context) + Core::Context const& context) const { auto url = m_url; url.AppendPath(Azure::Core::Url::Encode(m_tableName)); @@ -447,9 +447,10 @@ Azure::Response TableClient::AddEntity( return Response(std::move(response), std::move(rawResponse)); } -Azure::Response TableClient::UpdateEntity( +Azure::Response TableClient::UpdateEntityImpl( Models::TableEntity const& tableEntity, - Core::Context const& context) + bool isUpsert, + Core::Context const& context) const { auto url = m_url; url.AppendPath(Azure::Core::Url::Encode( @@ -467,14 +468,11 @@ Azure::Response TableClient::UpdateEntity( request.SetHeader(ContentLengthHeader, std::to_string(requestBody.Length())); request.SetHeader(AcceptHeader, AcceptFullMeta); request.SetHeader(PreferHeader, PreferNoContent); - if (!tableEntity.GetETag().Value.empty()) + + if (!isUpsert && !tableEntity.GetETag().Value.empty()) { request.SetHeader(IfMatch, tableEntity.GetETag().Value); } - else - { - request.SetHeader(IfMatch, "*"); - } auto rawResponse = m_pipeline->Send(request, context); auto const httpStatusCode = rawResponse->GetStatusCode(); @@ -489,9 +487,10 @@ Azure::Response TableClient::UpdateEntity( return Response(std::move(response), std::move(rawResponse)); } -Azure::Response TableClient::MergeEntity( +Azure::Response TableClient::MergeEntityImpl( Models::TableEntity const& tableEntity, - Core::Context const& context) + bool isUpsert, + Core::Context const& context) const { auto url = m_url; url.AppendPath(Azure::Core::Url::Encode( @@ -509,14 +508,11 @@ Azure::Response TableClient::MergeEntity( request.SetHeader(ContentLengthHeader, std::to_string(requestBody.Length())); request.SetHeader(AcceptHeader, AcceptFullMeta); request.SetHeader(PreferHeader, PreferNoContent); - if (!tableEntity.GetETag().Value.empty()) + + if (!isUpsert && !tableEntity.GetETag().Value.empty()) { request.SetHeader(IfMatch, tableEntity.GetETag().Value); } - else - { - request.SetHeader(IfMatch, "*"); - } auto rawResponse = m_pipeline->Send(request, context); auto const httpStatusCode = rawResponse->GetStatusCode(); @@ -533,7 +529,7 @@ Azure::Response TableClient::MergeEntity( Azure::Response TableClient::DeleteEntity( Models::TableEntity const& tableEntity, - Core::Context const& context) + Core::Context const& context) const { auto url = m_url; url.AppendPath(Azure::Core::Url::Encode( @@ -562,33 +558,32 @@ Azure::Response TableClient::DeleteEntity( return Response(std::move(response), std::move(rawResponse)); } -Azure::Response TableClient::UpsertEntity( +Azure::Response TableClient::UpdateOrInsertEntity( Models::TableEntity const& tableEntity, - Models::UpsertKind upsertKind, - Core::Context const& context) + Core::Context const& context) const { - try - { - switch (upsertKind) - { - case Models::UpsertKind::Merge: { - auto response = MergeEntity(tableEntity, context); - return Azure::Response( - Models::UpsertEntityResult(response.Value), std::move(response.RawResponse)); - } - default: { - auto response = UpdateEntity(tableEntity, context); - return Azure::Response( - Models::UpsertEntityResult(response.Value), std::move(response.RawResponse)); - } - } - } - catch (const Azure::Core::RequestFailedException&) - { - auto response = AddEntity(tableEntity, context); - return Azure::Response( - Models::UpsertEntityResult(response.Value), std::move(response.RawResponse)); - } + return UpdateEntityImpl(tableEntity, true, context); +} + +Azure::Response TableClient::MergeOrInsertEntity( + Models::TableEntity const& tableEntity, + Core::Context const& context) const +{ + return MergeEntityImpl(tableEntity, true, context); +} + +Azure::Response TableClient::UpdateEntity( + Models::TableEntity const& tableEntity, + Core::Context const& context) const +{ + return UpdateEntityImpl(tableEntity, false, context); +} + +Azure::Response TableClient::MergeEntity( + Models::TableEntity const& tableEntity, + Core::Context const& context) const +{ + return MergeEntityImpl(tableEntity, false, context); } void Models::QueryEntitiesPagedResponse::OnNextPage(const Azure::Core::Context& context) @@ -601,7 +596,7 @@ void Models::QueryEntitiesPagedResponse::OnNextPage(const Azure::Core::Context& Azure::Response TableClient::GetEntity( const std::string& partitionKey, const std::string& rowKey, - Core::Context const& context) + Core::Context const& context) const { auto url = m_url; url.AppendPath(Azure::Core::Url::Encode( @@ -632,7 +627,7 @@ Azure::Response TableClient::GetEntity( Models::QueryEntitiesPagedResponse TableClient::QueryEntities( Models::QueryEntitiesOptions const& options, - Core::Context const& context) + Core::Context const& context) const { auto url = m_url; if (!options.NextPartitionKey.empty() && !options.NextRowKey.empty()) @@ -718,7 +713,7 @@ Models::QueryEntitiesPagedResponse TableClient::QueryEntities( Azure::Response TableClient::SubmitTransaction( std::vector const& steps, - Core::Context const& context) + Core::Context const& context) const { auto url = m_url; url.AppendPath("$batch"); @@ -787,7 +782,7 @@ Azure::Response TableClient::SubmitTransaction( std::string TableClient::PreparePayload( std::string const& batchId, std::string const& changesetId, - std::vector const& steps) + std::vector const& steps) const { std::string accumulator = "--" + batchId + "\nContent-Type: multipart/mixed; boundary=" + changesetId + "\n\n"; @@ -820,6 +815,7 @@ std::string TableClient::PreparePayload( return accumulator; } std::string TableClient::PrepAddEntity(std::string const& changesetId, Models::TableEntity entity) + const { std::string returnValue = "--" + changesetId + "\n"; returnValue += "Content-Type: application/http\n"; @@ -836,7 +832,7 @@ std::string TableClient::PrepAddEntity(std::string const& changesetId, Models::T } std::string TableClient::PrepDeleteEntity( std::string const& changesetId, - Models::TableEntity entity) + Models::TableEntity entity) const { std::string returnValue = "--" + changesetId + "\n"; returnValue += "Content-Type: application/http\n"; @@ -862,6 +858,7 @@ std::string TableClient::PrepDeleteEntity( } std::string TableClient::PrepMergeEntity(std::string const& changesetId, Models::TableEntity entity) + const { std::string returnValue = "--" + changesetId + "\n"; returnValue += "Content-Type: application/http\n"; @@ -881,7 +878,7 @@ std::string TableClient::PrepMergeEntity(std::string const& changesetId, Models: std::string TableClient::PrepUpdateEntity( std::string const& changesetId, - Models::TableEntity entity) + Models::TableEntity entity) const { std::string returnValue = "--" + changesetId + "\n"; returnValue += "Content-Type: application/http\n"; @@ -910,7 +907,7 @@ std::string TableClient::PrepUpdateEntity( std::string TableClient::PrepInsertEntity( std::string const& changesetId, - Models::TableEntity entity) + Models::TableEntity entity) const { std::string payload = Serializers::UpdateEntity(entity); std::string returnValue = "--" + changesetId + "\n"; diff --git a/sdk/tables/azure-data-tables/test/ut/table_client_test.cpp b/sdk/tables/azure-data-tables/test/ut/table_client_test.cpp index 14c051928..94e48a41a 100644 --- a/sdk/tables/azure-data-tables/test/ut/table_client_test.cpp +++ b/sdk/tables/azure-data-tables/test/ut/table_client_test.cpp @@ -234,6 +234,7 @@ namespace Azure { namespace Data { namespace Test { entity.Properties["Product"] = TableEntityProperty("Tables"); auto createResponse = m_tableServiceClient->CreateTable(m_tableName); auto response = m_tableClient->AddEntity(entity); + entity.SetETag(response.Value.ETag); EXPECT_EQ(response.RawResponse->GetStatusCode(), Azure::Core::Http::HttpStatusCode::NoContent); EXPECT_FALSE(response.Value.ETag.empty()); @@ -263,6 +264,7 @@ namespace Azure { namespace Data { namespace Test { entity.Properties["Product"] = TableEntityProperty("Tables"); auto createResponse = m_tableServiceClient->CreateTable(m_tableName); auto response = m_tableClient->AddEntity(entity); + entity.SetETag(response.Value.ETag); EXPECT_EQ(response.RawResponse->GetStatusCode(), Azure::Core::Http::HttpStatusCode::NoContent); EXPECT_FALSE(response.Value.ETag.empty()); @@ -331,7 +333,7 @@ namespace Azure { namespace Data { namespace Test { } } - TEST_P(TablesClientTest, EntityUpsert) + TEST_P(TablesClientTest, EntityUpdateInsert) { Azure::Data::Tables::Models::TableEntity entity; @@ -340,10 +342,10 @@ namespace Azure { namespace Data { namespace Test { entity.Properties["Name"] = TableEntityProperty("Azure"); entity.Properties["Product"] = TableEntityProperty("Tables"); auto createResponse = m_tableServiceClient->CreateTable(m_tableName); - auto response = m_tableClient->UpsertEntity(entity); + auto response = m_tableClient->UpdateOrInsertEntity(entity); EXPECT_EQ(response.RawResponse->GetStatusCode(), Azure::Core::Http::HttpStatusCode::NoContent); EXPECT_FALSE(response.Value.ETag.empty()); - + entity.SetETag(response.Value.ETag); entity.Properties["Product"] = TableEntityProperty("Tables2"); auto updateResponse = m_tableClient->MergeEntity(entity); @@ -351,6 +353,35 @@ namespace Azure { namespace Data { namespace Test { updateResponse.RawResponse->GetStatusCode(), Azure::Core::Http::HttpStatusCode::NoContent); EXPECT_FALSE(updateResponse.Value.ETag.empty()); + entity.Properties["Product3"] = TableEntityProperty("Tables3"); + entity.SetETag(updateResponse.Value.ETag); + auto updateResponse2 = m_tableClient->UpdateEntity(entity); + + EXPECT_EQ( + updateResponse2.RawResponse->GetStatusCode(), Azure::Core::Http::HttpStatusCode::NoContent); + EXPECT_FALSE(updateResponse2.Value.ETag.empty()); + } + + TEST_P(TablesClientTest, EntityMergeInsert) + { + Azure::Data::Tables::Models::TableEntity entity; + + entity.SetPartitionKey("P1"); + entity.SetRowKey("R1"); + entity.Properties["Name"] = TableEntityProperty("Azure"); + entity.Properties["Product"] = TableEntityProperty("Tables"); + auto createResponse = m_tableServiceClient->CreateTable(m_tableName); + auto response = m_tableClient->MergeOrInsertEntity(entity); + EXPECT_EQ(response.RawResponse->GetStatusCode(), Azure::Core::Http::HttpStatusCode::NoContent); + EXPECT_FALSE(response.Value.ETag.empty()); + entity.SetETag(response.Value.ETag); + entity.Properties["Product"] = TableEntityProperty("Tables2"); + auto updateResponse = m_tableClient->UpdateEntity(entity); + + EXPECT_EQ( + updateResponse.RawResponse->GetStatusCode(), Azure::Core::Http::HttpStatusCode::NoContent); + EXPECT_FALSE(updateResponse.Value.ETag.empty()); + entity.Properties["Product3"] = TableEntityProperty("Tables3"); entity.SetETag(updateResponse.Value.ETag); auto updateResponse2 = m_tableClient->MergeEntity(entity);