From cc1c071c97eca9e5f2ec28fbe9adef3e56747862 Mon Sep 17 00:00:00 2001 From: JinmingHu Date: Thu, 29 Apr 2021 10:33:06 +0800 Subject: [PATCH] Change lease mutates LeaseClient (#2163) * Change lease mutates LeaseClient --- sdk/storage/azure-storage-blobs/CHANGELOG.md | 1 + .../azure/storage/blobs/blob_lease_client.hpp | 19 +++- .../src/blob_lease_client.cpp | 36 +++--- .../test/ut/blob_container_client_test.cpp | 106 +++++++++--------- .../test/ut/page_blob_client_test.cpp | 102 +++++++++-------- .../azure-storage-files-datalake/CHANGELOG.md | 1 + .../files/datalake/datalake_lease_client.hpp | 12 +- .../azure-storage-files-shares/CHANGELOG.md | 1 + .../files/shares/share_lease_client.hpp | 18 ++- .../src/share_lease_client.cpp | 34 ++++-- .../test/share_file_client_test.cpp | 76 +++++++------ 11 files changed, 230 insertions(+), 176 deletions(-) diff --git a/sdk/storage/azure-storage-blobs/CHANGELOG.md b/sdk/storage/azure-storage-blobs/CHANGELOG.md index 906f54a32..0edc19431 100644 --- a/sdk/storage/azure-storage-blobs/CHANGELOG.md +++ b/sdk/storage/azure-storage-blobs/CHANGELOG.md @@ -6,6 +6,7 @@ - Renamed `HasMorePages()` in paged response to `HasPage()`. - Default chunk size for concurrent upload was changed to nullable. +- `BlobLeaseClient::Change()` updates internal lease id. ## 12.0.0-beta.10 (2021-04-16) diff --git a/sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/blob_lease_client.hpp b/sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/blob_lease_client.hpp index be53b5cf3..392817091 100644 --- a/sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/blob_lease_client.hpp +++ b/sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/blob_lease_client.hpp @@ -4,6 +4,8 @@ #pragma once #include +#include +#include #include "azure/storage/blobs/blob_client.hpp" #include "azure/storage/blobs/blob_container_client.hpp" @@ -55,7 +57,11 @@ namespace Azure { namespace Storage { namespace Blobs { * * @return Lease id of this lease client. */ - const std::string& GetLeaseId() const { return m_leaseId; } + std::string GetLeaseId() + { + std::lock_guard guard(m_mutex); + return m_leaseId; + } /** * @brief Acquires a lease on the blob or blob container. @@ -71,7 +77,7 @@ namespace Azure { namespace Storage { namespace Blobs { Azure::Response Acquire( std::chrono::seconds duration, const AcquireLeaseOptions& options = AcquireLeaseOptions(), - const Azure::Core::Context& context = Azure::Core::Context()) const; + const Azure::Core::Context& context = Azure::Core::Context()); /** * @brief Renews the blob or blob container's previously-acquired lease. @@ -82,7 +88,7 @@ namespace Azure { namespace Storage { namespace Blobs { */ Azure::Response Renew( const RenewLeaseOptions& options = RenewLeaseOptions(), - const Azure::Core::Context& context = Azure::Core::Context()) const; + const Azure::Core::Context& context = Azure::Core::Context()); /** * @brief Releases the blob or blob container's previously-acquired lease. @@ -93,7 +99,7 @@ namespace Azure { namespace Storage { namespace Blobs { */ Azure::Response Release( const ReleaseLeaseOptions& options = ReleaseLeaseOptions(), - const Azure::Core::Context& context = Azure::Core::Context()) const; + const Azure::Core::Context& context = Azure::Core::Context()); /** * @brief Changes the lease of an active lease. @@ -107,7 +113,7 @@ namespace Azure { namespace Storage { namespace Blobs { Azure::Response Change( const std::string& proposedLeaseId, const ChangeLeaseOptions& options = ChangeLeaseOptions(), - const Azure::Core::Context& context = Azure::Core::Context()) const; + const Azure::Core::Context& context = Azure::Core::Context()); /** * @brief Breaks the previously-acquired lease. @@ -118,11 +124,12 @@ namespace Azure { namespace Storage { namespace Blobs { */ Azure::Response Break( const BreakLeaseOptions& options = BreakLeaseOptions(), - const Azure::Core::Context& context = Azure::Core::Context()) const; + const Azure::Core::Context& context = Azure::Core::Context()); private: Azure::Nullable m_blobClient; Azure::Nullable m_blobContainerClient; + std::mutex m_mutex; std::string m_leaseId; }; diff --git a/sdk/storage/azure-storage-blobs/src/blob_lease_client.cpp b/sdk/storage/azure-storage-blobs/src/blob_lease_client.cpp index 0a3d2811d..b9fa63ada 100644 --- a/sdk/storage/azure-storage-blobs/src/blob_lease_client.cpp +++ b/sdk/storage/azure-storage-blobs/src/blob_lease_client.cpp @@ -17,12 +17,12 @@ namespace Azure { namespace Storage { namespace Blobs { Azure::Response BlobLeaseClient::Acquire( std::chrono::seconds duration, const AcquireLeaseOptions& options, - const Azure::Core::Context& context) const + const Azure::Core::Context& context) { if (m_blobClient.HasValue()) { _detail::BlobRestClient::Blob::AcquireBlobLeaseOptions protocolLayerOptions; - protocolLayerOptions.ProposedLeaseId = m_leaseId; + protocolLayerOptions.ProposedLeaseId = GetLeaseId(); protocolLayerOptions.LeaseDuration = duration; protocolLayerOptions.IfModifiedSince = options.AccessConditions.IfModifiedSince; protocolLayerOptions.IfUnmodifiedSince = options.AccessConditions.IfUnmodifiedSince; @@ -47,7 +47,7 @@ namespace Azure { namespace Storage { namespace Blobs { else if (m_blobContainerClient.HasValue()) { _detail::BlobRestClient::BlobContainer::AcquireBlobContainerLeaseOptions protocolLayerOptions; - protocolLayerOptions.ProposedLeaseId = m_leaseId; + protocolLayerOptions.ProposedLeaseId = GetLeaseId(); protocolLayerOptions.LeaseDuration = duration; protocolLayerOptions.IfModifiedSince = options.AccessConditions.IfModifiedSince; protocolLayerOptions.IfUnmodifiedSince = options.AccessConditions.IfUnmodifiedSince; @@ -80,12 +80,12 @@ namespace Azure { namespace Storage { namespace Blobs { Azure::Response BlobLeaseClient::Renew( const RenewLeaseOptions& options, - const Azure::Core::Context& context) const + const Azure::Core::Context& context) { if (m_blobClient.HasValue()) { _detail::BlobRestClient::Blob::RenewBlobLeaseOptions protocolLayerOptions; - protocolLayerOptions.LeaseId = m_leaseId; + protocolLayerOptions.LeaseId = GetLeaseId(); protocolLayerOptions.IfModifiedSince = options.AccessConditions.IfModifiedSince; protocolLayerOptions.IfUnmodifiedSince = options.AccessConditions.IfUnmodifiedSince; protocolLayerOptions.IfMatch = options.AccessConditions.IfMatch; @@ -109,7 +109,7 @@ namespace Azure { namespace Storage { namespace Blobs { else if (m_blobContainerClient.HasValue()) { _detail::BlobRestClient::BlobContainer::RenewBlobContainerLeaseOptions protocolLayerOptions; - protocolLayerOptions.LeaseId = m_leaseId; + protocolLayerOptions.LeaseId = GetLeaseId(); protocolLayerOptions.IfModifiedSince = options.AccessConditions.IfModifiedSince; protocolLayerOptions.IfUnmodifiedSince = options.AccessConditions.IfUnmodifiedSince; @@ -142,12 +142,12 @@ namespace Azure { namespace Storage { namespace Blobs { Azure::Response BlobLeaseClient::Release( const ReleaseLeaseOptions& options, - const Azure::Core::Context& context) const + const Azure::Core::Context& context) { if (m_blobClient.HasValue()) { _detail::BlobRestClient::Blob::ReleaseBlobLeaseOptions protocolLayerOptions; - protocolLayerOptions.LeaseId = m_leaseId; + protocolLayerOptions.LeaseId = GetLeaseId(); protocolLayerOptions.IfModifiedSince = options.AccessConditions.IfModifiedSince; protocolLayerOptions.IfUnmodifiedSince = options.AccessConditions.IfUnmodifiedSince; protocolLayerOptions.IfMatch = options.AccessConditions.IfMatch; @@ -170,7 +170,7 @@ namespace Azure { namespace Storage { namespace Blobs { else if (m_blobContainerClient.HasValue()) { _detail::BlobRestClient::BlobContainer::ReleaseBlobContainerLeaseOptions protocolLayerOptions; - protocolLayerOptions.LeaseId = m_leaseId; + protocolLayerOptions.LeaseId = GetLeaseId(); protocolLayerOptions.IfModifiedSince = options.AccessConditions.IfModifiedSince; protocolLayerOptions.IfUnmodifiedSince = options.AccessConditions.IfUnmodifiedSince; @@ -203,12 +203,12 @@ namespace Azure { namespace Storage { namespace Blobs { Azure::Response BlobLeaseClient::Change( const std::string& proposedLeaseId, const ChangeLeaseOptions& options, - const Azure::Core::Context& context) const + const Azure::Core::Context& context) { if (m_blobClient.HasValue()) { _detail::BlobRestClient::Blob::ChangeBlobLeaseOptions protocolLayerOptions; - protocolLayerOptions.LeaseId = m_leaseId; + protocolLayerOptions.LeaseId = GetLeaseId(); protocolLayerOptions.ProposedLeaseId = proposedLeaseId; protocolLayerOptions.IfModifiedSince = options.AccessConditions.IfModifiedSince; protocolLayerOptions.IfUnmodifiedSince = options.AccessConditions.IfUnmodifiedSince; @@ -222,6 +222,11 @@ namespace Azure { namespace Storage { namespace Blobs { protocolLayerOptions, context); + { + std::lock_guard guard(m_mutex); + m_leaseId = response.Value.LeaseId; + } + Models::ChangeLeaseResult ret; ret.ETag = std::move(response.Value.ETag); ret.LastModified = std::move(response.Value.LastModified); @@ -233,7 +238,7 @@ namespace Azure { namespace Storage { namespace Blobs { else if (m_blobContainerClient.HasValue()) { _detail::BlobRestClient::BlobContainer::ChangeBlobContainerLeaseOptions protocolLayerOptions; - protocolLayerOptions.LeaseId = m_leaseId; + protocolLayerOptions.LeaseId = GetLeaseId(); protocolLayerOptions.ProposedLeaseId = proposedLeaseId; protocolLayerOptions.IfModifiedSince = options.AccessConditions.IfModifiedSince; protocolLayerOptions.IfUnmodifiedSince = options.AccessConditions.IfUnmodifiedSince; @@ -251,6 +256,11 @@ namespace Azure { namespace Storage { namespace Blobs { protocolLayerOptions, context); + { + std::lock_guard guard(m_mutex); + m_leaseId = response.Value.LeaseId; + } + Models::ChangeLeaseResult ret; ret.ETag = std::move(response.Value.ETag); ret.LastModified = std::move(response.Value.LastModified); @@ -267,7 +277,7 @@ namespace Azure { namespace Storage { namespace Blobs { Azure::Response BlobLeaseClient::Break( const BreakLeaseOptions& options, - const Azure::Core::Context& context) const + const Azure::Core::Context& context) { if (m_blobClient.HasValue()) { diff --git a/sdk/storage/azure-storage-blobs/test/ut/blob_container_client_test.cpp b/sdk/storage/azure-storage-blobs/test/ut/blob_container_client_test.cpp index 74126b8ef..401ee062d 100644 --- a/sdk/storage/azure-storage-blobs/test/ut/blob_container_client_test.cpp +++ b/sdk/storage/azure-storage-blobs/test/ut/blob_container_client_test.cpp @@ -388,61 +388,67 @@ namespace Azure { namespace Storage { namespace Test { StandardStorageConnectionString(), LowercaseRandomString()); containerClient.Create(); - std::string leaseId1 = Blobs::BlobLeaseClient::CreateUniqueLeaseId(); - auto leaseDuration = std::chrono::seconds(20); - Blobs::BlobLeaseClient leaseClient(containerClient, leaseId1); - auto aLease = leaseClient.Acquire(leaseDuration).Value; - EXPECT_TRUE(aLease.ETag.HasValue()); - EXPECT_TRUE(IsValidTime(aLease.LastModified)); - EXPECT_EQ(aLease.LeaseId, leaseId1); - EXPECT_EQ(leaseClient.GetLeaseId(), leaseId1); - aLease = leaseClient.Acquire(leaseDuration).Value; - EXPECT_TRUE(aLease.ETag.HasValue()); - EXPECT_TRUE(IsValidTime(aLease.LastModified)); - EXPECT_EQ(aLease.LeaseId, leaseId1); + { + std::string leaseId1 = Blobs::BlobLeaseClient::CreateUniqueLeaseId(); + auto leaseDuration = std::chrono::seconds(20); + Blobs::BlobLeaseClient leaseClient(containerClient, leaseId1); + auto aLease = leaseClient.Acquire(leaseDuration).Value; + EXPECT_TRUE(aLease.ETag.HasValue()); + EXPECT_TRUE(IsValidTime(aLease.LastModified)); + EXPECT_EQ(aLease.LeaseId, leaseId1); + EXPECT_EQ(leaseClient.GetLeaseId(), leaseId1); + aLease = leaseClient.Acquire(leaseDuration).Value; + EXPECT_TRUE(aLease.ETag.HasValue()); + EXPECT_TRUE(IsValidTime(aLease.LastModified)); + EXPECT_EQ(aLease.LeaseId, leaseId1); - auto properties = containerClient.GetProperties().Value; - EXPECT_EQ(properties.LeaseState, Blobs::Models::LeaseState::Leased); - EXPECT_EQ(properties.LeaseStatus, Blobs::Models::LeaseStatus::Locked); - EXPECT_EQ(properties.LeaseDuration.Value(), Blobs::Models::LeaseDurationType::Fixed); + auto properties = containerClient.GetProperties().Value; + EXPECT_EQ(properties.LeaseState, Blobs::Models::LeaseState::Leased); + EXPECT_EQ(properties.LeaseStatus, Blobs::Models::LeaseStatus::Locked); + EXPECT_EQ(properties.LeaseDuration.Value(), Blobs::Models::LeaseDurationType::Fixed); - auto rLease = leaseClient.Renew().Value; - EXPECT_TRUE(rLease.ETag.HasValue()); - EXPECT_TRUE(IsValidTime(rLease.LastModified)); - EXPECT_EQ(rLease.LeaseId, leaseId1); + auto rLease = leaseClient.Renew().Value; + EXPECT_TRUE(rLease.ETag.HasValue()); + EXPECT_TRUE(IsValidTime(rLease.LastModified)); + EXPECT_EQ(rLease.LeaseId, leaseId1); - std::string leaseId2 = Blobs::BlobLeaseClient::CreateUniqueLeaseId(); - EXPECT_NE(leaseId1, leaseId2); - auto cLease = leaseClient.Change(leaseId2).Value; - EXPECT_TRUE(cLease.ETag.HasValue()); - EXPECT_TRUE(IsValidTime(cLease.LastModified)); - EXPECT_EQ(cLease.LeaseId, leaseId2); - leaseClient = Blobs::BlobLeaseClient(containerClient, cLease.LeaseId); - EXPECT_EQ(leaseClient.GetLeaseId(), leaseId2); + std::string leaseId2 = Blobs::BlobLeaseClient::CreateUniqueLeaseId(); + EXPECT_NE(leaseId1, leaseId2); + auto cLease = leaseClient.Change(leaseId2).Value; + EXPECT_TRUE(cLease.ETag.HasValue()); + EXPECT_TRUE(IsValidTime(cLease.LastModified)); + EXPECT_EQ(cLease.LeaseId, leaseId2); + EXPECT_EQ(leaseClient.GetLeaseId(), leaseId2); - auto containerInfo = leaseClient.Release().Value; - EXPECT_TRUE(containerInfo.ETag.HasValue()); - EXPECT_TRUE(IsValidTime(containerInfo.LastModified)); + auto containerInfo = leaseClient.Release().Value; + EXPECT_TRUE(containerInfo.ETag.HasValue()); + EXPECT_TRUE(IsValidTime(containerInfo.LastModified)); + } - leaseClient - = Blobs::BlobLeaseClient(containerClient, Blobs::BlobLeaseClient::CreateUniqueLeaseId()); - aLease = leaseClient.Acquire(Blobs::BlobLeaseClient::InfiniteLeaseDuration).Value; - properties = containerClient.GetProperties().Value; - EXPECT_EQ(properties.LeaseDuration.Value(), Blobs::Models::LeaseDurationType::Infinite); - auto brokenLease = leaseClient.Break().Value; - EXPECT_TRUE(brokenLease.ETag.HasValue()); - EXPECT_TRUE(IsValidTime(brokenLease.LastModified)); + { + Blobs::BlobLeaseClient leaseClient( + containerClient, Blobs::BlobLeaseClient::CreateUniqueLeaseId()); + auto aLease = leaseClient.Acquire(Blobs::BlobLeaseClient::InfiniteLeaseDuration).Value; + auto properties = containerClient.GetProperties().Value; + EXPECT_EQ(properties.LeaseDuration.Value(), Blobs::Models::LeaseDurationType::Infinite); + auto brokenLease = leaseClient.Break().Value; + EXPECT_TRUE(brokenLease.ETag.HasValue()); + EXPECT_TRUE(IsValidTime(brokenLease.LastModified)); + } - leaseClient - = Blobs::BlobLeaseClient(containerClient, Blobs::BlobLeaseClient::CreateUniqueLeaseId()); - aLease = leaseClient.Acquire(leaseDuration).Value; - brokenLease = leaseClient.Break().Value; - EXPECT_TRUE(brokenLease.ETag.HasValue()); - EXPECT_TRUE(IsValidTime(brokenLease.LastModified)); + { + Blobs::BlobLeaseClient leaseClient( + containerClient, Blobs::BlobLeaseClient::CreateUniqueLeaseId()); + auto leaseDuration = std::chrono::seconds(20); + auto aLease = leaseClient.Acquire(leaseDuration).Value; + auto brokenLease = leaseClient.Break().Value; + EXPECT_TRUE(brokenLease.ETag.HasValue()); + EXPECT_TRUE(IsValidTime(brokenLease.LastModified)); - Blobs::BreakLeaseOptions options; - options.BreakPeriod = std::chrono::seconds(0); - leaseClient.Break(options); + Blobs::BreakLeaseOptions options; + options.BreakPeriod = std::chrono::seconds(0); + leaseClient.Break(options); + } containerClient.Delete(); } @@ -676,7 +682,7 @@ namespace Azure { namespace Storage { namespace Test { containerClient.Create(); std::string leaseId = Blobs::BlobLeaseClient::CreateUniqueLeaseId(); - auto leaseClient = Blobs::BlobLeaseClient(containerClient, leaseId); + Blobs::BlobLeaseClient leaseClient(containerClient, leaseId); leaseClient.Acquire(std::chrono::seconds(30)); EXPECT_THROW(containerClient.Delete(), StorageException); Blobs::DeleteBlobContainerOptions options; @@ -868,7 +874,7 @@ namespace Azure { namespace Storage { namespace Test { std::string leaseId = Blobs::BlobLeaseClient::CreateUniqueLeaseId(); Blobs::AcquireLeaseOptions options; options.AccessConditions.TagConditions = failWhereExpression; - auto leaseClient = Blobs::BlobLeaseClient(appendBlobClient, leaseId); + Blobs::BlobLeaseClient leaseClient(appendBlobClient, leaseId); EXPECT_THROW(leaseClient.Acquire(std::chrono::seconds(60), options), StorageException); options.AccessConditions.TagConditions = successWhereExpression; EXPECT_NO_THROW(leaseClient.Acquire(std::chrono::seconds(60), options)); diff --git a/sdk/storage/azure-storage-blobs/test/ut/page_blob_client_test.cpp b/sdk/storage/azure-storage-blobs/test/ut/page_blob_client_test.cpp index cda8d6b97..36fd56d07 100644 --- a/sdk/storage/azure-storage-blobs/test/ut/page_blob_client_test.cpp +++ b/sdk/storage/azure-storage-blobs/test/ut/page_blob_client_test.cpp @@ -188,61 +188,67 @@ namespace Azure { namespace Storage { namespace Test { TEST_F(PageBlobClientTest, Lease) { - std::string leaseId1 = Blobs::BlobLeaseClient::CreateUniqueLeaseId(); - auto leaseDuration = std::chrono::seconds(20); - auto leaseClient = Blobs::BlobLeaseClient(*m_pageBlobClient, leaseId1); - auto aLease = leaseClient.Acquire(leaseDuration).Value; - EXPECT_TRUE(aLease.ETag.HasValue()); - EXPECT_TRUE(IsValidTime(aLease.LastModified)); - EXPECT_EQ(aLease.LeaseId, leaseId1); - EXPECT_EQ(leaseClient.GetLeaseId(), leaseId1); - aLease = leaseClient.Acquire(leaseDuration).Value; - EXPECT_TRUE(aLease.ETag.HasValue()); - EXPECT_TRUE(IsValidTime(aLease.LastModified)); - EXPECT_EQ(aLease.LeaseId, leaseId1); + { + std::string leaseId1 = Blobs::BlobLeaseClient::CreateUniqueLeaseId(); + auto leaseDuration = std::chrono::seconds(20); + Blobs::BlobLeaseClient leaseClient(*m_pageBlobClient, leaseId1); + auto aLease = leaseClient.Acquire(leaseDuration).Value; + EXPECT_TRUE(aLease.ETag.HasValue()); + EXPECT_TRUE(IsValidTime(aLease.LastModified)); + EXPECT_EQ(aLease.LeaseId, leaseId1); + EXPECT_EQ(leaseClient.GetLeaseId(), leaseId1); + aLease = leaseClient.Acquire(leaseDuration).Value; + EXPECT_TRUE(aLease.ETag.HasValue()); + EXPECT_TRUE(IsValidTime(aLease.LastModified)); + EXPECT_EQ(aLease.LeaseId, leaseId1); - auto properties = m_pageBlobClient->GetProperties().Value; - EXPECT_EQ(properties.LeaseState.Value(), Blobs::Models::LeaseState::Leased); - EXPECT_EQ(properties.LeaseStatus.Value(), Blobs::Models::LeaseStatus::Locked); - EXPECT_EQ(properties.LeaseDuration.Value(), Blobs::Models::LeaseDurationType::Fixed); + auto properties = m_pageBlobClient->GetProperties().Value; + EXPECT_EQ(properties.LeaseState.Value(), Blobs::Models::LeaseState::Leased); + EXPECT_EQ(properties.LeaseStatus.Value(), Blobs::Models::LeaseStatus::Locked); + EXPECT_EQ(properties.LeaseDuration.Value(), Blobs::Models::LeaseDurationType::Fixed); - auto rLease = leaseClient.Renew().Value; - EXPECT_TRUE(rLease.ETag.HasValue()); - EXPECT_TRUE(IsValidTime(rLease.LastModified)); - EXPECT_EQ(rLease.LeaseId, leaseId1); + auto rLease = leaseClient.Renew().Value; + EXPECT_TRUE(rLease.ETag.HasValue()); + EXPECT_TRUE(IsValidTime(rLease.LastModified)); + EXPECT_EQ(rLease.LeaseId, leaseId1); - std::string leaseId2 = Blobs::BlobLeaseClient::CreateUniqueLeaseId(); - EXPECT_NE(leaseId1, leaseId2); - auto cLease = leaseClient.Change(leaseId2).Value; - EXPECT_TRUE(cLease.ETag.HasValue()); - EXPECT_TRUE(IsValidTime(cLease.LastModified)); - EXPECT_EQ(cLease.LeaseId, leaseId2); - leaseClient = Blobs::BlobLeaseClient(*m_pageBlobClient, cLease.LeaseId); - EXPECT_EQ(leaseClient.GetLeaseId(), leaseId2); + std::string leaseId2 = Blobs::BlobLeaseClient::CreateUniqueLeaseId(); + EXPECT_NE(leaseId1, leaseId2); + auto cLease = leaseClient.Change(leaseId2).Value; + EXPECT_TRUE(cLease.ETag.HasValue()); + EXPECT_TRUE(IsValidTime(cLease.LastModified)); + EXPECT_EQ(cLease.LeaseId, leaseId2); + EXPECT_EQ(leaseClient.GetLeaseId(), leaseId2); - auto blobInfo = leaseClient.Release().Value; - EXPECT_TRUE(blobInfo.ETag.HasValue()); - EXPECT_TRUE(IsValidTime(blobInfo.LastModified)); + auto blobInfo = leaseClient.Release().Value; + EXPECT_TRUE(blobInfo.ETag.HasValue()); + EXPECT_TRUE(IsValidTime(blobInfo.LastModified)); + } - leaseClient - = Blobs::BlobLeaseClient(*m_pageBlobClient, Blobs::BlobLeaseClient::CreateUniqueLeaseId()); - aLease = leaseClient.Acquire(Blobs::BlobLeaseClient::InfiniteLeaseDuration).Value; - properties = m_pageBlobClient->GetProperties().Value; - EXPECT_EQ(properties.LeaseDuration.Value(), Blobs::Models::LeaseDurationType::Infinite); - auto brokenLease = leaseClient.Break().Value; - EXPECT_TRUE(brokenLease.ETag.HasValue()); - EXPECT_TRUE(IsValidTime(brokenLease.LastModified)); + { + Blobs::BlobLeaseClient leaseClient( + *m_pageBlobClient, Blobs::BlobLeaseClient::CreateUniqueLeaseId()); + auto aLease = leaseClient.Acquire(Blobs::BlobLeaseClient::InfiniteLeaseDuration).Value; + auto properties = m_pageBlobClient->GetProperties().Value; + EXPECT_EQ(properties.LeaseDuration.Value(), Blobs::Models::LeaseDurationType::Infinite); + auto brokenLease = leaseClient.Break().Value; + EXPECT_TRUE(brokenLease.ETag.HasValue()); + EXPECT_TRUE(IsValidTime(brokenLease.LastModified)); + } - leaseClient - = Blobs::BlobLeaseClient(*m_pageBlobClient, Blobs::BlobLeaseClient::CreateUniqueLeaseId()); - aLease = leaseClient.Acquire(leaseDuration).Value; - brokenLease = leaseClient.Break().Value; - EXPECT_TRUE(brokenLease.ETag.HasValue()); - EXPECT_TRUE(IsValidTime(brokenLease.LastModified)); + { + Blobs::BlobLeaseClient leaseClient( + *m_pageBlobClient, Blobs::BlobLeaseClient::CreateUniqueLeaseId()); + auto leaseDuration = std::chrono::seconds(20); + auto aLease = leaseClient.Acquire(leaseDuration).Value; + auto brokenLease = leaseClient.Break().Value; + EXPECT_TRUE(brokenLease.ETag.HasValue()); + EXPECT_TRUE(IsValidTime(brokenLease.LastModified)); - Blobs::BreakLeaseOptions options; - options.BreakPeriod = std::chrono::seconds(0); - leaseClient.Break(options); + Blobs::BreakLeaseOptions options; + options.BreakPeriod = std::chrono::seconds(0); + leaseClient.Break(options); + } } TEST_F(PageBlobClientTest, ContentMd5) diff --git a/sdk/storage/azure-storage-files-datalake/CHANGELOG.md b/sdk/storage/azure-storage-files-datalake/CHANGELOG.md index 30c21cce4..09ee9ff68 100644 --- a/sdk/storage/azure-storage-files-datalake/CHANGELOG.md +++ b/sdk/storage/azure-storage-files-datalake/CHANGELOG.md @@ -6,6 +6,7 @@ - Renamed `HasMorePages()` in paged response to `HasPage()`. - Default chunk size for concurrent upload was changed to nullable. +- `DataLakeLeaseClient::Change()` updates internal lease id. ## 12.0.0-beta.10 (2021-04-16) diff --git a/sdk/storage/azure-storage-files-datalake/inc/azure/storage/files/datalake/datalake_lease_client.hpp b/sdk/storage/azure-storage-files-datalake/inc/azure/storage/files/datalake/datalake_lease_client.hpp index fa0cb37c6..3ad787aa6 100644 --- a/sdk/storage/azure-storage-files-datalake/inc/azure/storage/files/datalake/datalake_lease_client.hpp +++ b/sdk/storage/azure-storage-files-datalake/inc/azure/storage/files/datalake/datalake_lease_client.hpp @@ -60,7 +60,7 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { * * @return Lease id of this lease client. */ - const std::string& GetLeaseId() const { return m_blobLeaseClient.GetLeaseId(); } + std::string GetLeaseId() { return m_blobLeaseClient.GetLeaseId(); } /** * @brief Acquires a lease on the datalake path or datalake path container. @@ -76,7 +76,7 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { Azure::Response Acquire( std::chrono::seconds duration, const AcquireLeaseOptions& options = AcquireLeaseOptions(), - const Azure::Core::Context& context = Azure::Core::Context()) const + const Azure::Core::Context& context = Azure::Core::Context()) { return m_blobLeaseClient.Acquire(duration, options, context); } @@ -90,7 +90,7 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { */ Azure::Response Renew( const RenewLeaseOptions& options = RenewLeaseOptions(), - const Azure::Core::Context& context = Azure::Core::Context()) const + const Azure::Core::Context& context = Azure::Core::Context()) { return m_blobLeaseClient.Renew(options, context); } @@ -104,7 +104,7 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { */ Azure::Response Release( const ReleaseLeaseOptions& options = ReleaseLeaseOptions(), - const Azure::Core::Context& context = Azure::Core::Context()) const + const Azure::Core::Context& context = Azure::Core::Context()) { return m_blobLeaseClient.Release(options, context); } @@ -121,7 +121,7 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { Azure::Response Change( const std::string& proposedLeaseId, const ChangeLeaseOptions& options = ChangeLeaseOptions(), - const Azure::Core::Context& context = Azure::Core::Context()) const + const Azure::Core::Context& context = Azure::Core::Context()) { return m_blobLeaseClient.Change(proposedLeaseId, options, context); } @@ -135,7 +135,7 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { */ Azure::Response Break( const BreakLeaseOptions& options = BreakLeaseOptions(), - const Azure::Core::Context& context = Azure::Core::Context()) const + const Azure::Core::Context& context = Azure::Core::Context()) { return m_blobLeaseClient.Break(options, context); } diff --git a/sdk/storage/azure-storage-files-shares/CHANGELOG.md b/sdk/storage/azure-storage-files-shares/CHANGELOG.md index 1128c31d6..2d32467ba 100644 --- a/sdk/storage/azure-storage-files-shares/CHANGELOG.md +++ b/sdk/storage/azure-storage-files-shares/CHANGELOG.md @@ -5,6 +5,7 @@ ### Breaking Changes - Renamed `HasMorePages()` in paged response to `HasPage()`. +- `ShareLeaseClient::Change()` updates internal lease id. ## 12.0.0-beta.10 (2021-04-16) diff --git a/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/share_lease_client.hpp b/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/share_lease_client.hpp index 613cfa3ac..e4d1b151f 100644 --- a/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/share_lease_client.hpp +++ b/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/share_lease_client.hpp @@ -4,6 +4,7 @@ #pragma once #include +#include #include #include "azure/storage/files/shares/share_client.hpp" @@ -44,7 +45,11 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { * * @return Lease id of this lease client. */ - const std::string& GetLeaseId() const { return m_leaseId; } + const std::string& GetLeaseId() + { + std::lock_guard guard(m_mutex); + return m_leaseId; + } /** * @brief Acquires a lease on the file or share. @@ -59,7 +64,7 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { Azure::Response Acquire( std::chrono::seconds duration, const AcquireLeaseOptions& options = AcquireLeaseOptions(), - const Azure::Core::Context& context = Azure::Core::Context()) const; + const Azure::Core::Context& context = Azure::Core::Context()); /** * @brief Releases the file or share's previously-acquired lease. @@ -70,7 +75,7 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { */ Azure::Response Release( const ReleaseLeaseOptions& options = ReleaseLeaseOptions(), - const Azure::Core::Context& context = Azure::Core::Context()) const; + const Azure::Core::Context& context = Azure::Core::Context()); /** * @brief Changes the lease of an active lease. @@ -84,7 +89,7 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { Azure::Response Change( const std::string& proposedLeaseId, const ChangeLeaseOptions& options = ChangeLeaseOptions(), - const Azure::Core::Context& context = Azure::Core::Context()) const; + const Azure::Core::Context& context = Azure::Core::Context()); /** * @brief Breaks the previously-acquired lease. @@ -95,7 +100,7 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { */ Azure::Response Break( const BreakLeaseOptions& options = BreakLeaseOptions(), - const Azure::Core::Context& context = Azure::Core::Context()) const; + const Azure::Core::Context& context = Azure::Core::Context()); private: /** @@ -118,10 +123,11 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { */ Azure::Response Renew( const RenewLeaseOptions& options = RenewLeaseOptions(), - const Azure::Core::Context& context = Azure::Core::Context()) const; + const Azure::Core::Context& context = Azure::Core::Context()); Azure::Nullable m_fileClient; Azure::Nullable m_shareClient; + std::mutex m_mutex; std::string m_leaseId; }; diff --git a/sdk/storage/azure-storage-files-shares/src/share_lease_client.cpp b/sdk/storage/azure-storage-files-shares/src/share_lease_client.cpp index 402535e30..2ad1c3ef7 100644 --- a/sdk/storage/azure-storage-files-shares/src/share_lease_client.cpp +++ b/sdk/storage/azure-storage-files-shares/src/share_lease_client.cpp @@ -17,13 +17,13 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { Azure::Response ShareLeaseClient::Acquire( std::chrono::seconds duration, const AcquireLeaseOptions& options, - const Azure::Core::Context& context) const + const Azure::Core::Context& context) { (void)options; if (m_fileClient.HasValue()) { _detail::ShareRestClient::File::AcquireLeaseOptions protocolLayerOptions; - protocolLayerOptions.ProposedLeaseIdOptional = m_leaseId; + protocolLayerOptions.ProposedLeaseIdOptional = GetLeaseId(); protocolLayerOptions.LeaseDuration = static_cast(duration.count()); auto response = _detail::ShareRestClient::File::AcquireLease( @@ -43,7 +43,7 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { else if (m_shareClient.HasValue()) { _detail::ShareRestClient::Share::AcquireLeaseOptions protocolLayerOptions; - protocolLayerOptions.ProposedLeaseIdOptional = m_leaseId; + protocolLayerOptions.ProposedLeaseIdOptional = GetLeaseId(); protocolLayerOptions.LeaseDuration = static_cast(duration.count()); auto response = _detail::ShareRestClient::Share::AcquireLease( @@ -68,7 +68,7 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { Azure::Response ShareLeaseClient::Renew( const RenewLeaseOptions& options, - const Azure::Core::Context& context) const + const Azure::Core::Context& context) { (void)options; if (m_fileClient.HasValue()) @@ -79,7 +79,7 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { else if (m_shareClient.HasValue()) { _detail::ShareRestClient::Share::RenewLeaseOptions protocolLayerOptions; - protocolLayerOptions.LeaseIdRequired = m_leaseId; + protocolLayerOptions.LeaseIdRequired = GetLeaseId(); auto response = _detail::ShareRestClient::Share::RenewLease( m_shareClient.Value().m_shareUrl, @@ -103,13 +103,13 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { Azure::Response ShareLeaseClient::Release( const ReleaseLeaseOptions& options, - const Azure::Core::Context& context) const + const Azure::Core::Context& context) { (void)options; if (m_fileClient.HasValue()) { _detail::ShareRestClient::File::ReleaseLeaseOptions protocolLayerOptions; - protocolLayerOptions.LeaseIdRequired = m_leaseId; + protocolLayerOptions.LeaseIdRequired = GetLeaseId(); auto response = _detail::ShareRestClient::File::ReleaseLease( m_fileClient.Value().m_shareFileUrl, @@ -127,7 +127,7 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { else if (m_shareClient.HasValue()) { _detail::ShareRestClient::Share::ReleaseLeaseOptions protocolLayerOptions; - protocolLayerOptions.LeaseIdRequired = m_leaseId; + protocolLayerOptions.LeaseIdRequired = GetLeaseId(); auto response = _detail::ShareRestClient::Share::ReleaseLease( m_shareClient.Value().m_shareUrl, @@ -151,13 +151,13 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { Azure::Response ShareLeaseClient::Change( const std::string& proposedLeaseId, const ChangeLeaseOptions& options, - const Azure::Core::Context& context) const + const Azure::Core::Context& context) { (void)options; if (m_fileClient.HasValue()) { _detail::ShareRestClient::File::ChangeLeaseOptions protocolLayerOptions; - protocolLayerOptions.LeaseIdRequired = m_leaseId; + protocolLayerOptions.LeaseIdRequired = GetLeaseId(); protocolLayerOptions.ProposedLeaseIdOptional = proposedLeaseId; auto response = _detail::ShareRestClient::File::ChangeLease( @@ -166,6 +166,11 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { context, protocolLayerOptions); + { + std::lock_guard guard(m_mutex); + m_leaseId = response.Value.LeaseId; + } + Models::ChangeLeaseResult ret; ret.ETag = std::move(response.Value.ETag); ret.LastModified = std::move(response.Value.LastModified); @@ -177,7 +182,7 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { else if (m_shareClient.HasValue()) { _detail::ShareRestClient::Share::ChangeLeaseOptions protocolLayerOptions; - protocolLayerOptions.LeaseIdRequired = m_leaseId; + protocolLayerOptions.LeaseIdRequired = GetLeaseId(); protocolLayerOptions.ProposedLeaseIdOptional = proposedLeaseId; auto response = _detail::ShareRestClient::Share::ChangeLease( @@ -186,6 +191,11 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { context, protocolLayerOptions); + { + std::lock_guard guard(m_mutex); + m_leaseId = response.Value.LeaseId; + } + Models::ChangeLeaseResult ret; ret.ETag = std::move(response.Value.ETag); ret.LastModified = std::move(response.Value.LastModified); @@ -202,7 +212,7 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { Azure::Response ShareLeaseClient::Break( const BreakLeaseOptions& options, - const Azure::Core::Context& context) const + const Azure::Core::Context& context) { (void)options; if (m_fileClient.HasValue()) diff --git a/sdk/storage/azure-storage-files-shares/test/share_file_client_test.cpp b/sdk/storage/azure-storage-files-shares/test/share_file_client_test.cpp index e67ae3e37..b24e3d45d 100644 --- a/sdk/storage/azure-storage-files-shares/test/share_file_client_test.cpp +++ b/sdk/storage/azure-storage-files-shares/test/share_file_client_test.cpp @@ -272,45 +272,51 @@ namespace Azure { namespace Storage { namespace Test { TEST_F(FileShareFileClientTest, LeaseRelated) { - std::string leaseId1 = Files::Shares::ShareLeaseClient::CreateUniqueLeaseId(); - auto lastModified = m_fileClient->GetProperties().Value.LastModified; - auto leaseClient = Files::Shares::ShareLeaseClient(*m_fileClient, leaseId1); - auto aLease = leaseClient.Acquire(Files::Shares::ShareLeaseClient::InfiniteLeaseDuration).Value; - EXPECT_TRUE(aLease.ETag.HasValue()); - EXPECT_TRUE(aLease.LastModified >= lastModified); - EXPECT_EQ(aLease.LeaseId, leaseId1); - lastModified = m_fileClient->GetProperties().Value.LastModified; - aLease = leaseClient.Acquire(Files::Shares::ShareLeaseClient::InfiniteLeaseDuration).Value; - EXPECT_TRUE(aLease.ETag.HasValue()); - EXPECT_TRUE(aLease.LastModified >= lastModified); - EXPECT_EQ(aLease.LeaseId, leaseId1); + { + std::string leaseId1 = Files::Shares::ShareLeaseClient::CreateUniqueLeaseId(); + auto lastModified = m_fileClient->GetProperties().Value.LastModified; + Files::Shares::ShareLeaseClient leaseClient(*m_fileClient, leaseId1); + auto aLease + = leaseClient.Acquire(Files::Shares::ShareLeaseClient::InfiniteLeaseDuration).Value; + EXPECT_TRUE(aLease.ETag.HasValue()); + EXPECT_TRUE(aLease.LastModified >= lastModified); + EXPECT_EQ(aLease.LeaseId, leaseId1); + lastModified = m_fileClient->GetProperties().Value.LastModified; + aLease = leaseClient.Acquire(Files::Shares::ShareLeaseClient::InfiniteLeaseDuration).Value; + EXPECT_TRUE(aLease.ETag.HasValue()); + EXPECT_TRUE(aLease.LastModified >= lastModified); + EXPECT_EQ(aLease.LeaseId, leaseId1); - auto properties = m_fileClient->GetProperties().Value; - EXPECT_EQ(properties.LeaseState.Value(), Files::Shares::Models::LeaseState::Leased); - EXPECT_EQ(properties.LeaseStatus.Value(), Files::Shares::Models::LeaseStatus::Locked); + auto properties = m_fileClient->GetProperties().Value; + EXPECT_EQ(properties.LeaseState.Value(), Files::Shares::Models::LeaseState::Leased); + EXPECT_EQ(properties.LeaseStatus.Value(), Files::Shares::Models::LeaseStatus::Locked); - std::string leaseId2 = Files::Shares::ShareLeaseClient::CreateUniqueLeaseId(); - EXPECT_NE(leaseId1, leaseId2); - lastModified = m_fileClient->GetProperties().Value.LastModified; - auto cLease = leaseClient.Change(leaseId2).Value; - EXPECT_TRUE(cLease.ETag.HasValue()); - EXPECT_TRUE(cLease.LastModified >= lastModified); - EXPECT_EQ(cLease.LeaseId, leaseId2); - leaseClient = Files::Shares::ShareLeaseClient(*m_fileClient, cLease.LeaseId); - EXPECT_EQ(leaseClient.GetLeaseId(), leaseId2); + std::string leaseId2 = Files::Shares::ShareLeaseClient::CreateUniqueLeaseId(); + EXPECT_NE(leaseId1, leaseId2); + lastModified = m_fileClient->GetProperties().Value.LastModified; + auto cLease = leaseClient.Change(leaseId2).Value; + EXPECT_TRUE(cLease.ETag.HasValue()); + EXPECT_TRUE(cLease.LastModified >= lastModified); + EXPECT_EQ(cLease.LeaseId, leaseId2); + EXPECT_EQ(leaseClient.GetLeaseId(), leaseId2); - lastModified = m_fileClient->GetProperties().Value.LastModified; - auto fileInfo = leaseClient.Release().Value; - EXPECT_TRUE(fileInfo.ETag.HasValue()); - EXPECT_TRUE(fileInfo.LastModified >= lastModified); + lastModified = m_fileClient->GetProperties().Value.LastModified; + auto fileInfo = leaseClient.Release().Value; + EXPECT_TRUE(fileInfo.ETag.HasValue()); + EXPECT_TRUE(fileInfo.LastModified >= lastModified); + } - leaseClient = Files::Shares::ShareLeaseClient( - *m_fileClient, Files::Shares::ShareLeaseClient::CreateUniqueLeaseId()); - aLease = leaseClient.Acquire(Files::Shares::ShareLeaseClient::InfiniteLeaseDuration).Value; - lastModified = m_fileClient->GetProperties().Value.LastModified; - auto brokenLease = leaseClient.Break().Value; - EXPECT_TRUE(brokenLease.ETag.HasValue()); - EXPECT_TRUE(brokenLease.LastModified >= lastModified); + { + + Files::Shares::ShareLeaseClient leaseClient( + *m_fileClient, Files::Shares::ShareLeaseClient::CreateUniqueLeaseId()); + auto aLease + = leaseClient.Acquire(Files::Shares::ShareLeaseClient::InfiniteLeaseDuration).Value; + auto lastModified = m_fileClient->GetProperties().Value.LastModified; + auto brokenLease = leaseClient.Break().Value; + EXPECT_TRUE(brokenLease.ETag.HasValue()); + EXPECT_TRUE(brokenLease.LastModified >= lastModified); + } } TEST_F(FileShareFileClientTest, ConcurrentUpload)