From a0a3c6d5c5e0abc0a2a99b44d996dbbfe5424469 Mon Sep 17 00:00:00 2001 From: Kan Tang Date: Thu, 21 Jan 2021 21:37:03 -0800 Subject: [PATCH] Changed ContentRange from std::string to Azure::Core::Http::Range. (#1415) * Changed ContentRange from std::string to Azure::Core::Http::Range. * Resolved further comments. * Resolved test issues --- .../azure-storage-files-datalake/CHANGELOG.md | 4 ++ .../files/datalake/datalake_responses.hpp | 7 +- .../protocol/datalake_rest_client.hpp | 14 ---- .../src/datalake_file_client.cpp | 8 +-- .../src/datalake_path_client.cpp | 4 +- .../test/datalake_file_client_test.cpp | 13 +++- .../azure-storage-files-shares/CHANGELOG.md | 2 + .../shares/protocol/share_rest_client.hpp | 70 +++++++++++++++---- .../src/share_file_client.cpp | 6 +- .../test/share_file_client_test.cpp | 13 ++-- 10 files changed, 94 insertions(+), 47 deletions(-) diff --git a/sdk/storage/azure-storage-files-datalake/CHANGELOG.md b/sdk/storage/azure-storage-files-datalake/CHANGELOG.md index 4aeabd4f5..2122877d4 100644 --- a/sdk/storage/azure-storage-files-datalake/CHANGELOG.md +++ b/sdk/storage/azure-storage-files-datalake/CHANGELOG.md @@ -5,6 +5,7 @@ ### New Features - Added `Owner`, `Permissions`, and `Group` to `GetDataLakePathAccessControlResult`. +- `ReadDataLakeFileResult` now has a new field `FileSize`. - Added support for `GetAccessPolicy` and `SetAccessPolicy` in `DataLakeFileSystemClient`. ### Breaking Changes @@ -12,6 +13,9 @@ - Removed `GetDfsUri` in all clients since they are currently implementation details. - Removed `Data` suffix for `FlushData` and `AppendData` and modified all related structs to align the change. - `DataLakePathClient` can no longer set permissions with `SetAccessControl`, instead, a new API `SetPermissions` is created for such functionality. Renamed the original API to `SetAccessControlList` to be more precise. +- `ContentRange` in `ReadDataLakeFileResult` is now `Azure::Core::Http::Range`. +- Removed `ContentRange` in `PathGetPropertiesResult`. +- Renamed `ContentLength` in `GetDataLakePathPropertiesResult` and `CreateDataLakePathResult` to `FileSize` to be more accurate. - Renamed `GetUri` to `GetUrl`. ## 12.0.0-beta.6 (2020-01-14) diff --git a/sdk/storage/azure-storage-files-datalake/inc/azure/storage/files/datalake/datalake_responses.hpp b/sdk/storage/azure-storage-files-datalake/inc/azure/storage/files/datalake/datalake_responses.hpp index 2b44be0a9..dc8b6674e 100644 --- a/sdk/storage/azure-storage-files-datalake/inc/azure/storage/files/datalake/datalake_responses.hpp +++ b/sdk/storage/azure-storage-files-datalake/inc/azure/storage/files/datalake/datalake_responses.hpp @@ -110,7 +110,7 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { nam std::string ETag; Core::DateTime LastModified; Core::DateTime CreatedOn; - int64_t ContentLength; + int64_t FileSize; Storage::Metadata Metadata; Azure::Core::Nullable LeaseDuration; Azure::Core::Nullable LeaseState; @@ -156,7 +156,7 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { nam bool Created = true; std::string ETag; Core::DateTime LastModified; - Azure::Core::Nullable ContentLength; + Azure::Core::Nullable FileSize; }; using SetDataLakePathAccessControlListResult = PathSetAccessControlResult; @@ -173,7 +173,8 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { nam { std::unique_ptr Body; PathHttpHeaders HttpHeaders; - Azure::Core::Nullable ContentRange; + int64_t FileSize = int64_t(); + Azure::Core::Http::Range ContentRange; Azure::Core::Nullable TransactionalContentHash; std::string ETag; Core::DateTime LastModified; diff --git a/sdk/storage/azure-storage-files-datalake/inc/azure/storage/files/datalake/protocol/datalake_rest_client.hpp b/sdk/storage/azure-storage-files-datalake/inc/azure/storage/files/datalake/protocol/datalake_rest_client.hpp index 6bf39f40a..4fdb9a8aa 100644 --- a/sdk/storage/azure-storage-files-datalake/inc/azure/storage/files/datalake/protocol/datalake_rest_client.hpp +++ b/sdk/storage/azure-storage-files-datalake/inc/azure/storage/files/datalake/protocol/datalake_rest_client.hpp @@ -92,7 +92,6 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { constexpr static const char* HeaderXMsLeaseBreakPeriod = "x-ms-lease-break-period"; constexpr static const char* HeaderLeaseTime = "x-ms-lease-time"; constexpr static const char* HeaderAcceptRanges = "accept-ranges"; - constexpr static const char* HeaderContentRange = "content-range"; constexpr static const char* HeaderResourceType = "x-ms-resource-type"; constexpr static const char* HeaderLeaseState = "x-ms-lease-state"; constexpr static const char* HeaderLeaseStatus = "x-ms-lease-status"; @@ -421,8 +420,6 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { { Azure::Core::Nullable AcceptRanges; PathHttpHeaders HttpHeaders; - int64_t ContentLength = int64_t(); - Azure::Core::Nullable ContentRange; std::string ETag; Core::DateTime LastModified; Azure::Core::Nullable ResourceType; @@ -1931,17 +1928,6 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { { result.HttpHeaders.ContentLanguage = response.GetHeaders().at("content-language"); } - if (response.GetHeaders().find(Details::HeaderContentLength) - != response.GetHeaders().end()) - { - result.ContentLength - = std::stoll(response.GetHeaders().at(Details::HeaderContentLength)); - } - if (response.GetHeaders().find(Details::HeaderContentRange) - != response.GetHeaders().end()) - { - result.ContentRange = response.GetHeaders().at(Details::HeaderContentRange); - } if (response.GetHeaders().find("content-type") != response.GetHeaders().end()) { result.HttpHeaders.ContentType = response.GetHeaders().at("content-type"); diff --git a/sdk/storage/azure-storage-files-datalake/src/datalake_file_client.cpp b/sdk/storage/azure-storage-files-datalake/src/datalake_file_client.cpp index d5963f882..57bf43093 100644 --- a/sdk/storage/azure-storage-files-datalake/src/datalake_file_client.cpp +++ b/sdk/storage/azure-storage-files-datalake/src/datalake_file_client.cpp @@ -326,12 +326,8 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { Models::ReadDataLakeFileResult ret; ret.Body = std::move(result->BodyStream); ret.HttpHeaders = FromBlobHttpHeaders(std::move(result->HttpHeaders)); - // FIXME - if (result.GetRawResponse().GetHeaders().find("content-range") - != result.GetRawResponse().GetHeaders().end()) - { - ret.ContentRange = result.GetRawResponse().GetHeaders().at("content-range"); - } + ret.ContentRange = std::move(result->ContentRange); + ret.FileSize = result->BlobSize; ret.TransactionalContentHash = std::move(result->TransactionalContentHash); ret.ETag = std::move(result->ETag); ret.LastModified = std::move(result->LastModified); diff --git a/sdk/storage/azure-storage-files-datalake/src/datalake_path_client.cpp b/sdk/storage/azure-storage-files-datalake/src/datalake_path_client.cpp index cdc77fcb3..8b65098db 100644 --- a/sdk/storage/azure-storage-files-datalake/src/datalake_path_client.cpp +++ b/sdk/storage/azure-storage-files-datalake/src/datalake_path_client.cpp @@ -274,7 +274,7 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { Models::CreateDataLakePathResult ret; ret.ETag = std::move(result->ETag.GetValue()); ret.LastModified = std::move(result->LastModified.GetValue()); - ret.ContentLength = std::move(result->ContentLength); + ret.FileSize = std::move(result->ContentLength); return Azure::Core::Response( std::move(ret), result.ExtractRawResponse()); } @@ -382,7 +382,7 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { ret.CopyCompletedOn = std::move(result->CopyCompletedOn); ret.ExpiresOn = std::move(result->ExpiriesOn); ret.LastAccessedOn = std::move(result->LastAccessedOn); - ret.ContentLength = result->ContentLength; + ret.FileSize = result->ContentLength; return Azure::Core::Response( std::move(ret), result.ExtractRawResponse()); } diff --git a/sdk/storage/azure-storage-files-datalake/test/datalake_file_client_test.cpp b/sdk/storage/azure-storage-files-datalake/test/datalake_file_client_test.cpp index b3d5fb45b..c49b1eec0 100644 --- a/sdk/storage/azure-storage-files-datalake/test/datalake_file_client_test.cpp +++ b/sdk/storage/azure-storage-files-datalake/test/datalake_file_client_test.cpp @@ -378,6 +378,9 @@ namespace Azure { namespace Storage { namespace Test { auto result = newFileClient->Read(); auto downloaded = ReadBodyStream(result->Body); EXPECT_EQ(buffer, downloaded); + EXPECT_EQ(bufferSize, result->FileSize); + EXPECT_EQ(bufferSize, result->ContentRange.Length.GetValue()); + EXPECT_EQ(0, result->ContentRange.Offset); // Read Range { @@ -390,6 +393,9 @@ namespace Azure { namespace Storage { namespace Test { downloaded = ReadBodyStream(result->Body); EXPECT_EQ(firstHalf.size(), downloaded.size()); EXPECT_EQ(firstHalf, downloaded); + EXPECT_EQ(bufferSize, result->FileSize); + EXPECT_EQ(bufferSize / 2, result->ContentRange.Length.GetValue()); + EXPECT_EQ(0, result->ContentRange.Offset); } { auto secondHalf = std::vector(buffer.begin() + bufferSize / 2, buffer.end()); @@ -400,6 +406,9 @@ namespace Azure { namespace Storage { namespace Test { result = newFileClient->Read(options); downloaded = ReadBodyStream(result->Body); EXPECT_EQ(secondHalf, downloaded); + EXPECT_EQ(bufferSize, result->FileSize); + EXPECT_EQ(bufferSize / 2, result->ContentRange.Length.GetValue()); + EXPECT_EQ(bufferSize / 2, result->ContentRange.Offset); } { // Read with last modified access condition. @@ -491,7 +500,7 @@ namespace Azure { namespace Storage { namespace Test { EXPECT_TRUE(IsValidTime(res->LastModified)); EXPECT_EQ(res->LastModified, lastModified); auto properties = *fileClient.GetProperties(); - EXPECT_EQ(properties.ContentLength, fileSize); + EXPECT_EQ(properties.FileSize, fileSize); EXPECT_EQ(properties.HttpHeaders, options.HttpHeaders); EXPECT_EQ(properties.Metadata, options.Metadata); EXPECT_EQ(properties.ETag, res->ETag); @@ -525,7 +534,7 @@ namespace Azure { namespace Storage { namespace Test { EXPECT_TRUE(IsValidTime(res->LastModified)); EXPECT_EQ(res->LastModified, lastModified); auto properties = *fileClient.GetProperties(); - EXPECT_EQ(properties.ContentLength, fileSize); + EXPECT_EQ(properties.FileSize, fileSize); EXPECT_EQ(properties.HttpHeaders, options.HttpHeaders); EXPECT_EQ(properties.Metadata, options.Metadata); EXPECT_EQ(properties.ETag, res->ETag); diff --git a/sdk/storage/azure-storage-files-shares/CHANGELOG.md b/sdk/storage/azure-storage-files-shares/CHANGELOG.md index 1d99450ba..993ebe6ab 100644 --- a/sdk/storage/azure-storage-files-shares/CHANGELOG.md +++ b/sdk/storage/azure-storage-files-shares/CHANGELOG.md @@ -5,6 +5,8 @@ ### Breaking Changes - Removed `GetDirectoryClient` and `GetFileClient` from `ShareClient`. `ShareDirectoryClient` and `ShareFileClient` now initializes with the name of the resource, not path, to indicate that no path parsing is done for the API +- `ContentRange` in `FileDownloadResult` is now `Azure::Core::Http::Range`. +- `ContentLength` in `FileDownloadResult` is renamed to `FileSize`. - Renamed `GetUri` to `GetUrl`. ## 12.0.0-beta.6 (2020-01-14) diff --git a/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/protocol/share_rest_client.hpp b/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/protocol/share_rest_client.hpp index 27526c7d7..b97c510b5 100644 --- a/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/protocol/share_rest_client.hpp +++ b/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/protocol/share_rest_client.hpp @@ -772,9 +772,9 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { std::unique_ptr BodyStream; Core::DateTime LastModified; Storage::Metadata Metadata; - int64_t ContentLength = int64_t(); ShareFileHttpHeaders HttpHeaders; - Azure::Core::Nullable ContentRange; + Azure::Core::Http::Range ContentRange; + int64_t FileSize; std::string ETag; Azure::Core::Nullable TransactionalContentHash; std::string AcceptRanges; @@ -6099,13 +6099,36 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { { result.Metadata.emplace(i->first.substr(10), i->second); } - result.ContentLength - = std::stoll(response.GetHeaders().at(Details::HeaderContentLength)); result.HttpHeaders.ContentType = response.GetHeaders().at(Details::HeaderContentType); - if (response.GetHeaders().find(Details::HeaderContentRange) - != response.GetHeaders().end()) + + auto content_range_iterator = response.GetHeaders().find(Details::HeaderContentRange); + if (content_range_iterator != response.GetHeaders().end()) { - result.ContentRange = response.GetHeaders().at(Details::HeaderContentRange); + const std::string& content_range = content_range_iterator->second; + auto bytes_pos = content_range.find("bytes "); + auto dash_pos = content_range.find("-", bytes_pos + 6); + auto slash_pos = content_range.find("/", dash_pos + 1); + int64_t range_start_offset = std::stoll(std::string( + content_range.begin() + bytes_pos + 6, content_range.begin() + dash_pos)); + int64_t range_end_offset = std::stoll(std::string( + content_range.begin() + dash_pos + 1, content_range.begin() + slash_pos)); + result.ContentRange = Azure::Core::Http::Range{ + range_start_offset, range_end_offset - range_start_offset + 1}; + } + else + { + result.ContentRange = Azure::Core::Http::Range{ + 0, std::stoll(response.GetHeaders().at(Details::HeaderContentLength))}; + } + if (content_range_iterator != response.GetHeaders().end()) + { + const std::string& content_range = content_range_iterator->second; + auto slash_pos = content_range.find("/"); + result.FileSize = std::stoll(content_range.substr(slash_pos + 1)); + } + else + { + result.FileSize = std::stoll(response.GetHeaders().at(Details::HeaderContentLength)); } result.ETag = response.GetHeaders().at(Details::HeaderETag); if (response.GetHeaders().find(Details::HeaderTransactionalContentHashMd5) @@ -6235,13 +6258,36 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { { result.Metadata.emplace(i->first.substr(10), i->second); } - result.ContentLength - = std::stoll(response.GetHeaders().at(Details::HeaderContentLength)); result.HttpHeaders.ContentType = response.GetHeaders().at(Details::HeaderContentType); - if (response.GetHeaders().find(Details::HeaderContentRange) - != response.GetHeaders().end()) + + auto content_range_iterator = response.GetHeaders().find(Details::HeaderContentRange); + if (content_range_iterator != response.GetHeaders().end()) { - result.ContentRange = response.GetHeaders().at(Details::HeaderContentRange); + const std::string& content_range = content_range_iterator->second; + auto bytes_pos = content_range.find("bytes "); + auto dash_pos = content_range.find("-", bytes_pos + 6); + auto slash_pos = content_range.find("/", dash_pos + 1); + int64_t range_start_offset = std::stoll(std::string( + content_range.begin() + bytes_pos + 6, content_range.begin() + dash_pos)); + int64_t range_end_offset = std::stoll(std::string( + content_range.begin() + dash_pos + 1, content_range.begin() + slash_pos)); + result.ContentRange = Azure::Core::Http::Range{ + range_start_offset, range_end_offset - range_start_offset + 1}; + } + else + { + result.ContentRange = Azure::Core::Http::Range{ + 0, std::stoll(response.GetHeaders().at(Details::HeaderContentLength))}; + } + if (content_range_iterator != response.GetHeaders().end()) + { + const std::string& content_range = content_range_iterator->second; + auto slash_pos = content_range.find("/"); + result.FileSize = std::stoll(content_range.substr(slash_pos + 1)); + } + else + { + result.FileSize = std::stoll(response.GetHeaders().at(Details::HeaderContentLength)); } result.ETag = response.GetHeaders().at(Details::HeaderETag); if (response.GetHeaders().find(Details::HeaderTransactionalContentHashMd5) diff --git a/sdk/storage/azure-storage-files-shares/src/share_file_client.cpp b/sdk/storage/azure-storage-files-shares/src/share_file_client.cpp index 36819656d..1e022bb80 100644 --- a/sdk/storage/azure-storage-files-shares/src/share_file_client.cpp +++ b/sdk/storage/azure-storage-files-shares/src/share_file_client.cpp @@ -641,8 +641,7 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { int64_t fileRangeSize; if (firstChunkOptions.Range.HasValue()) { - fileSize = std::stoll(firstChunk->ContentRange.GetValue().substr( - firstChunk->ContentRange.GetValue().find('/') + 1)); + fileSize = firstChunk->FileSize; fileRangeSize = fileSize - firstChunkOffset; if (options.Range.GetValue().Length.HasValue()) { @@ -763,8 +762,7 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { int64_t fileRangeSize; if (firstChunkOptions.Range.HasValue()) { - fileSize = std::stoll(firstChunk->ContentRange.GetValue().substr( - firstChunk->ContentRange.GetValue().find('/') + 1)); + fileSize = firstChunk->FileSize; fileRangeSize = fileSize - firstChunkOffset; if (options.Range.GetValue().Length.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 8abd296df..94187d447 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 @@ -592,15 +592,20 @@ namespace Azure { namespace Storage { namespace Test { for (int32_t i = 0; i < numOfChunks; ++i) { - std::vector resultBuffer; Files::Shares::DownloadShareFileOptions downloadOptions; downloadOptions.Range = Core::Http::Range(); downloadOptions.Range.GetValue().Offset = static_cast(rangeSize) * i; downloadOptions.Range.GetValue().Length = rangeSize; - EXPECT_NO_THROW( - resultBuffer = Core::Http::BodyStream::ReadToEnd( - Core::Context(), *fileClient.Download(downloadOptions)->BodyStream)); + Files::Shares::Models::DownloadShareFileResult result; + EXPECT_NO_THROW(result = fileClient.Download(downloadOptions).ExtractValue()); + auto resultBuffer + = Core::Http::BodyStream::ReadToEnd(Core::Context(), *(result.BodyStream)); EXPECT_EQ(rangeContent, resultBuffer); + EXPECT_EQ( + downloadOptions.Range.GetValue().Length.GetValue(), + result.ContentRange.Length.GetValue()); + EXPECT_EQ(downloadOptions.Range.GetValue().Offset, result.ContentRange.Offset); + EXPECT_EQ(static_cast(numOfChunks) * rangeSize, result.FileSize); } }