From a470957606868ef6753b6259f6c0c3d596100ed5 Mon Sep 17 00:00:00 2001 From: Kan Tang Date: Wed, 6 Jan 2021 13:10:00 +0800 Subject: [PATCH] Http::Range integration for file/datalake (#1258) * Added support for Core::Http::Range for file/datalake service * Resolved test issue and added changelog. --- .../azure-storage-files-datalake/CHANGELOG.md | 2 + .../files/datalake/datalake_options.hpp | 48 +---- .../files/datalake/datalake_responses.hpp | 3 +- .../src/datalake_file_client.cpp | 41 +--- .../test/datalake_file_client_test.cpp | 10 +- .../azure-storage-files-shares/CHANGELOG.md | 2 + .../shares/protocol/share_rest_client.hpp | 187 ++++++------------ .../storage/files/shares/share_options.hpp | 39 +--- .../src/share_file_client.cpp | 85 ++++---- .../test/share_file_client_test.cpp | 77 +++++--- 10 files changed, 177 insertions(+), 317 deletions(-) diff --git a/sdk/storage/azure-storage-files-datalake/CHANGELOG.md b/sdk/storage/azure-storage-files-datalake/CHANGELOG.md index 3af7d18eb..097cbbb55 100644 --- a/sdk/storage/azure-storage-files-datalake/CHANGELOG.md +++ b/sdk/storage/azure-storage-files-datalake/CHANGELOG.md @@ -15,6 +15,8 @@ - `ExpiryTime` is renamed to `ExpiresOn`. - `LastAccessTime` is renamed to `LastAccessedOn`. - Move version strings into `Details` namespace. +- `ReadFileResult` now have `ContentRange` as string. +- `ReadFileOptions` now have `Azure::Core::Http::Range Range` instead of `Content-Length` and `Offset`. ## 12.0.0-beta.5 (2020-11-13) diff --git a/sdk/storage/azure-storage-files-datalake/inc/azure/storage/files/datalake/datalake_options.hpp b/sdk/storage/azure-storage-files-datalake/inc/azure/storage/files/datalake/datalake_options.hpp index 707de2cab..550a8d273 100644 --- a/sdk/storage/azure-storage-files-datalake/inc/azure/storage/files/datalake/datalake_options.hpp +++ b/sdk/storage/azure-storage-files-datalake/inc/azure/storage/files/datalake/datalake_options.hpp @@ -488,14 +488,9 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { Azure::Core::Context Context; /** - * @brief Specify the offset of the starting range to be retrieved. + * @brief Specify the range of the resource to be retrieved. */ - Azure::Core::Nullable Offset; - - /** - * @brief Specify the length to be retreived if an offset has been specified. - */ - Azure::Core::Nullable Length; + Azure::Core::Nullable Range; /** * @brief When this header is set to "true" and specified together with the Range header, @@ -731,45 +726,6 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { Azure::Core::Nullable ExpiresOn; }; - /** - * @brief Optional parameters for FileClient::DownloadToBuffer and FileClient::DownloadToFile. - */ - struct DownloadFileToBufferOptions - { - /** - * @brief Context for cancelling long running operations. - */ - Azure::Core::Context Context; - - /** - * @brief Downloads only the bytes of the blob from this offset. - */ - Azure::Core::Nullable Offset; - - /** - * @brief Returns at most this number of bytes of the blob from the offset. Null means - * download until the end. - */ - Azure::Core::Nullable Length; - - /** - * @brief The size of the first range request in bytes. Blobs smaller than this limit will be - * downloaded in a single request. Blobs larger than this limit will continue being downloaded - * in chunks of size ChunkSize. - */ - Azure::Core::Nullable InitialChunkSize; - - /** - * @brief The maximum number of bytes in a single request. - */ - Azure::Core::Nullable ChunkSize; - - /** - * @brief The maximum number of threads that may be used in a parallel transfer. - */ - int Concurrency = 5; - }; - using AcquirePathLeaseOptions = Blobs::AcquireBlobLeaseOptions; using BreakPathLeaseOptions = Blobs::BreakBlobLeaseOptions; using RenewPathLeaseOptions = Blobs::RenewBlobLeaseOptions; 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 53f7bd168..3cd55f36f 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 @@ -142,8 +142,7 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { nam { std::unique_ptr Body; PathHttpHeaders HttpHeaders; - Azure::Core::Nullable RangeOffset; - Azure::Core::Nullable RangeLength; + Azure::Core::Nullable ContentRange; Azure::Core::Nullable TransactionalContentHash; std::string ETag; Core::DateTime LastModified; 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 b9ea4bb99..d715e9d71 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 @@ -21,29 +21,6 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { namespace { - std::pair GetOffsetLength(const std::string& rangeString) - { - int64_t offset = std::numeric_limits::max(); - int64_t length = std::numeric_limits::max(); - const std::string c_bytesPrefix = "bytes "; - if (rangeString.length() > c_bytesPrefix.length()) - { - auto subRangeString = rangeString.substr(c_bytesPrefix.length()); - std::string::const_iterator cur = subRangeString.begin(); - offset = std::stoll(Details::GetSubstringTillDelimiter('-', subRangeString, cur)); - if (cur != subRangeString.end()) - { - length = std::stoll(Details::GetSubstringTillDelimiter('/', subRangeString, cur)) - offset - + 1; - } - else - { - throw std::runtime_error("The format of the range string is not correct: " + rangeString); - } - } - return std::make_pair(offset, length); - } - Models::PathHttpHeaders FromBlobHttpHeaders(Blobs::Models::BlobHttpHeaders headers) { Models::PathHttpHeaders ret; @@ -325,12 +302,7 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { { Blobs::DownloadBlobOptions blobOptions; blobOptions.Context = options.Context; - if (options.Offset.HasValue()) - { - blobOptions.Range = Core::Http::Range(); - blobOptions.Range.GetValue().Offset = options.Offset.GetValue(); - blobOptions.Range.GetValue().Length = options.Length; - } + blobOptions.Range = options.Range; blobOptions.AccessConditions.IfMatch = options.AccessConditions.IfMatch; blobOptions.AccessConditions.IfNoneMatch = options.AccessConditions.IfNoneMatch; blobOptions.AccessConditions.IfModifiedSince = options.AccessConditions.IfModifiedSince; @@ -340,16 +312,7 @@ namespace Azure { namespace Storage { namespace Files { namespace DataLake { Models::ReadFileResult ret; ret.Body = std::move(result->BodyStream); ret.HttpHeaders = FromBlobHttpHeaders(std::move(result->HttpHeaders)); - Azure::Core::Nullable RangeOffset; - Azure::Core::Nullable RangeLength; - if (result->ContentRange.HasValue()) - { - auto range = GetOffsetLength(result->ContentRange.GetValue()); - RangeOffset = range.first; - RangeLength = range.second; - } - ret.RangeOffset = RangeOffset; - ret.RangeLength = RangeLength; + ret.ContentRange = std::move(result->ContentRange); 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/test/datalake_file_client_test.cpp b/sdk/storage/azure-storage-files-datalake/test/datalake_file_client_test.cpp index 501b18e7b..ebfaf4b7d 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 @@ -359,8 +359,9 @@ namespace Azure { namespace Storage { namespace Test { { auto firstHalf = std::vector(buffer.begin(), buffer.begin() + (bufferSize / 2)); Files::DataLake::ReadFileOptions options; - options.Offset = 0; - options.Length = bufferSize / 2; + options.Range = Azure::Core::Http::Range(); + options.Range.GetValue().Offset = 0; + options.Range.GetValue().Length = bufferSize / 2; result = newFileClient->Read(options); downloaded = ReadBodyStream(result->Body); EXPECT_EQ(firstHalf.size(), downloaded.size()); @@ -369,8 +370,9 @@ namespace Azure { namespace Storage { namespace Test { { auto secondHalf = std::vector(buffer.begin() + bufferSize / 2, buffer.end()); Files::DataLake::ReadFileOptions options; - options.Offset = bufferSize / 2; - options.Length = bufferSize / 2; + options.Range = Azure::Core::Http::Range(); + options.Range.GetValue().Offset = bufferSize / 2; + options.Range.GetValue().Length = bufferSize / 2; result = newFileClient->Read(options); downloaded = ReadBodyStream(result->Body); EXPECT_EQ(secondHalf, downloaded); diff --git a/sdk/storage/azure-storage-files-shares/CHANGELOG.md b/sdk/storage/azure-storage-files-shares/CHANGELOG.md index bc0f036d2..6d65d6c38 100644 --- a/sdk/storage/azure-storage-files-shares/CHANGELOG.md +++ b/sdk/storage/azure-storage-files-shares/CHANGELOG.md @@ -16,6 +16,8 @@ - `FileShareHttpHeaders` is renamed to `ShareFileHttpHeaders`, and member `std::string ContentMd5` is changed to `Storage::ContentHash ContentHash`. - All date time related strings are now changed to `Azure::Core::DateTime` type. - Move version strings into `Details` namespace. +- Removed `FileRange` and `ClearRange`, they are now represented with `Azure::Core::Http::Range`. +- Removed `Offset` and `Length` pair in options. They are now represented with `Azure::Core::Http::Range`. ## 12.0.0-beta.5 (2020-11-13) 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 761a4c80d..7dc3c928d 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 @@ -376,8 +376,8 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { // The list of file ranges struct ShareFileRangeList { - std::vector Ranges; - std::vector ClearRanges; + std::vector Ranges; + std::vector ClearRanges; }; // Stats for the share. @@ -808,8 +808,8 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { struct FileGetRangeListResult { - std::vector Ranges; - std::vector ClearRanges; + std::vector Ranges; + std::vector ClearRanges; Core::DateTime LastModified; std::string ETag; int64_t FileContentLength = int64_t(); @@ -1185,6 +1185,61 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { } class ShareRestClient { + private: + static Azure::Core::Http::Range HttpRangeFromXml(Storage::Details::XmlReader& reader) + { + int depth = 0; + bool is_start = false; + bool is_end = false; + int64_t start = 0; + int64_t end = 0; + while (true) + { + auto node = reader.Read(); + if (node.Type == Storage::Details::XmlNodeType::End) + { + break; + } + else if ( + node.Type == Storage::Details::XmlNodeType::StartTag + && strcmp(node.Name, "Start") == 0) + { + ++depth; + is_start = true; + } + else if ( + node.Type == Storage::Details::XmlNodeType::StartTag && strcmp(node.Name, "End") == 0) + { + ++depth; + is_end = true; + } + else if (node.Type == Storage::Details::XmlNodeType::EndTag) + { + is_start = false; + is_end = false; + if (depth-- == 0) + { + break; + } + } + if (depth == 1 && node.Type == Storage::Details::XmlNodeType::Text) + { + if (is_start) + { + start = std::stoll(node.Value); + } + else if (is_end) + { + end = std::stoll(node.Value); + } + } + } + Azure::Core::Http::Range ret; + ret.Offset = start; + ret.Length = end - start + 1; + return ret; + } + public: class Service { public: @@ -6916,126 +6971,6 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { } } - static Models::FileRange FileRangeFromXml(Storage::Details::XmlReader& reader) - { - auto result = Models::FileRange(); - enum class XmlTagName - { - End, - Start, - Unknown, - }; - std::vector path; - - while (true) - { - auto node = reader.Read(); - if (node.Type == Storage::Details::XmlNodeType::End) - { - break; - } - else if (node.Type == Storage::Details::XmlNodeType::EndTag) - { - if (path.size() > 0) - { - path.pop_back(); - } - else - { - break; - } - } - else if (node.Type == Storage::Details::XmlNodeType::StartTag) - { - - if (std::strcmp(node.Name, "End") == 0) - { - path.emplace_back(XmlTagName::End); - } - else if (std::strcmp(node.Name, "Start") == 0) - { - path.emplace_back(XmlTagName::Start); - } - else - { - path.emplace_back(XmlTagName::Unknown); - } - } - else if (node.Type == Storage::Details::XmlNodeType::Text) - { - if (path.size() == 1 && path[0] == XmlTagName::End) - { - result.End = std::stoll(node.Value); - } - else if (path.size() == 1 && path[0] == XmlTagName::Start) - { - result.Start = std::stoll(node.Value); - } - } - } - return result; - } - - static Models::ClearRange ClearRangeFromXml(Storage::Details::XmlReader& reader) - { - auto result = Models::ClearRange(); - enum class XmlTagName - { - End, - Start, - Unknown, - }; - std::vector path; - - while (true) - { - auto node = reader.Read(); - if (node.Type == Storage::Details::XmlNodeType::End) - { - break; - } - else if (node.Type == Storage::Details::XmlNodeType::EndTag) - { - if (path.size() > 0) - { - path.pop_back(); - } - else - { - break; - } - } - else if (node.Type == Storage::Details::XmlNodeType::StartTag) - { - - if (std::strcmp(node.Name, "End") == 0) - { - path.emplace_back(XmlTagName::End); - } - else if (std::strcmp(node.Name, "Start") == 0) - { - path.emplace_back(XmlTagName::Start); - } - else - { - path.emplace_back(XmlTagName::Unknown); - } - } - else if (node.Type == Storage::Details::XmlNodeType::Text) - { - if (path.size() == 1 && path[0] == XmlTagName::End) - { - result.End = std::stoll(node.Value); - } - else if (path.size() == 1 && path[0] == XmlTagName::Start) - { - result.Start = std::stoll(node.Value); - } - } - } - return result; - } - static Models::ShareFileRangeList ShareFileRangeListFromXml( Storage::Details::XmlReader& reader) { @@ -7088,14 +7023,14 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { } if (path.size() == 2 && path[0] == XmlTagName::Ranges && path[1] == XmlTagName::Range) { - result.Ranges.emplace_back(FileRangeFromXml(reader)); + result.Ranges.emplace_back(HttpRangeFromXml(reader)); path.pop_back(); } else if ( path.size() == 2 && path[0] == XmlTagName::Ranges && path[1] == XmlTagName::ClearRange) { - result.ClearRanges.emplace_back(ClearRangeFromXml(reader)); + result.ClearRanges.emplace_back(HttpRangeFromXml(reader)); path.pop_back(); } } diff --git a/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/share_options.hpp b/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/share_options.hpp index 7909e20ec..3df73f1df 100644 --- a/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/share_options.hpp +++ b/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/share_options.hpp @@ -474,15 +474,9 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { Azure::Core::Context Context; /** - * @brief Downloads only the bytes of the file from this offset. + * @brief Downloads only the bytes of the file from this range. */ - Azure::Core::Nullable Offset; - - /** - * @brief Returns at most this number of bytes of the file from the offset. Null means - * download until the end. - */ - Azure::Core::Nullable Length; + Azure::Core::Nullable Range; /** * @brief When this parameter is set to true and specified together with the Range parameter, @@ -649,14 +643,9 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { Azure::Core::Context Context; /** - * @brief The offset of the source file. + * @brief The range of the source file. */ - Azure::Core::Nullable SourceOffset; - - /** - * @brief The length of the source file. - */ - Azure::Core::Nullable SourceLength; + Azure::Core::Nullable SourceRange; /** * @brief Specify the crc64 calculated for the range of bytes that must be read from the copy @@ -683,15 +672,9 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { Azure::Core::Context Context; /** - * @brief The offset of the ranges to be get from service. + * @brief The range to be get from service. */ - Azure::Core::Nullable Offset; - - /** - * @brief The length starting from the offset to be get from the service. When present, 'Offset' - * must not be null, otherwise it is ignored. - */ - Azure::Core::Nullable Length; + Azure::Core::Nullable Range; /** * @brief The previous snapshot parameter is an opaque DateTime value that, when present, @@ -763,15 +746,9 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { Azure::Core::Context Context; /** - * @brief Downloads only the bytes of the file from this offset. + * @brief Downloads only the bytes of the file from this range. */ - Azure::Core::Nullable Offset; - - /** - * @brief Returns at most this number of bytes of the file from the offset. Null means - * download until the end. - */ - Azure::Core::Nullable Length; + Azure::Core::Nullable Range; /** * @brief The size of the first range request in bytes. Files smaller than this limit will be 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 1864be4d3..9754ce995 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 @@ -202,18 +202,18 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { const DownloadFileOptions& options) const { auto protocolLayerOptions = Details::ShareRestClient::File::DownloadOptions(); - if (options.Offset.HasValue()) + if (options.Range.HasValue()) { - if (options.Length.HasValue()) + if (options.Range.GetValue().Length.HasValue()) { protocolLayerOptions.Range = std::string("bytes=") - + std::to_string(options.Offset.GetValue()) + std::string("-") - + std::to_string(options.Offset.GetValue() + options.Length.GetValue() - 1); + + std::to_string(options.Range.GetValue().Offset) + std::string("-") + + std::to_string(options.Range.GetValue().Offset + options.Range.GetValue().Length.GetValue() - 1); } else { - protocolLayerOptions.Range - = std::string("bytes=") + std::to_string(options.Offset.GetValue()) + std::string("-"); + protocolLayerOptions.Range = std::string("bytes=") + + std::to_string(options.Range.GetValue().Offset) + std::string("-"); } } protocolLayerOptions.GetRangeContentMd5 = options.GetRangeContentMd5; @@ -233,12 +233,13 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { unused(context); DownloadFileOptions newOptions = options; - newOptions.Offset - = (options.Offset.HasValue() ? options.Offset.GetValue() : 0) + retryInfo.Offset; - newOptions.Length = options.Length; - if (newOptions.Length.HasValue()) + newOptions.Range = Core::Http::Range(); + newOptions.Range.GetValue().Offset + = (options.Range.HasValue() ? options.Range.GetValue().Offset : 0) + retryInfo.Offset; + if (options.Range.HasValue() && options.Range.GetValue().Length.HasValue()) { - newOptions.Length = options.Length.GetValue() - retryInfo.Offset; + newOptions.Range.GetValue().Length + = options.Range.GetValue().Length.GetValue() - retryInfo.Offset; } auto newResponse = Download(newOptions); @@ -465,18 +466,18 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { const GetFileRangeListOptions& options) const { auto protocolLayerOptions = Details::ShareRestClient::File::GetRangeListOptions(); - if (options.Offset.HasValue()) + if (options.Range.HasValue()) { - if (options.Length.HasValue()) + if (options.Range.GetValue().Length.HasValue()) { protocolLayerOptions.XMsRange = std::string("bytes=") - + std::to_string(options.Offset.GetValue()) + std::string("-") - + std::to_string(options.Offset.GetValue() + options.Length.GetValue() - 1); + + std::to_string(options.Range.GetValue().Offset) + std::string("-") + + std::to_string(options.Range.GetValue().Offset + options.Range.GetValue().Length.GetValue() - 1); } else { protocolLayerOptions.XMsRange - = std::string("bytes=") + std::to_string(options.Offset.GetValue()) + std::string("-"); + = std::string("bytes=") + std::to_string(options.Range.GetValue().Offset) + std::string("-"); } } @@ -573,37 +574,37 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { // Just start downloading using an initial chunk. If it's a small file, we'll get the whole // thing in one shot. If it's a large file, we'll get its full size in Content-Range and can // keep downloading it in chunks. - int64_t firstChunkOffset = options.Offset.HasValue() ? options.Offset.GetValue() : 0; + int64_t firstChunkOffset = options.Range.HasValue() ? options.Range.GetValue().Offset : 0; int64_t firstChunkLength = Details::c_FileDownloadDefaultChunkSize; if (options.InitialChunkSize.HasValue()) { firstChunkLength = options.InitialChunkSize.GetValue(); } - if (options.Length.HasValue()) + if (options.Range.HasValue() && options.Range.GetValue().Length.HasValue()) { - firstChunkLength = std::min(firstChunkLength, options.Length.GetValue()); + firstChunkLength = std::min(firstChunkLength, options.Range.GetValue().Length.GetValue()); } DownloadFileOptions firstChunkOptions; firstChunkOptions.Context = options.Context; - firstChunkOptions.Offset = options.Offset; - if (firstChunkOptions.Offset.HasValue()) + firstChunkOptions.Range = options.Range; + if (firstChunkOptions.Range.HasValue()) { - firstChunkOptions.Length = firstChunkLength; + firstChunkOptions.Range.GetValue().Length = firstChunkLength; } auto firstChunk = Download(firstChunkOptions); int64_t fileSize; int64_t fileRangeSize; - if (firstChunkOptions.Offset.HasValue()) + if (firstChunkOptions.Range.HasValue()) { fileSize = std::stoll(firstChunk->ContentRange.GetValue().substr( firstChunk->ContentRange.GetValue().find('/') + 1)); fileRangeSize = fileSize - firstChunkOffset; - if (options.Length.HasValue()) + if (options.Range.GetValue().Length.HasValue()) { - fileRangeSize = std::min(fileRangeSize, options.Length.GetValue()); + fileRangeSize = std::min(fileRangeSize, options.Range.GetValue().Length.GetValue()); } } else @@ -645,15 +646,16 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { = [&](int64_t offset, int64_t length, int64_t chunkId, int64_t numChunks) { DownloadFileOptions chunkOptions; chunkOptions.Context = options.Context; - chunkOptions.Offset = offset; - chunkOptions.Length = length; + chunkOptions.Range = Core::Http::Range(); + chunkOptions.Range.GetValue().Offset = offset; + chunkOptions.Range.GetValue().Length = length; auto chunk = Download(chunkOptions); int64_t bytesRead = Azure::Core::Http::BodyStream::ReadToCount( chunkOptions.Context, *(chunk->BodyStream), buffer + (offset - firstChunkOffset), - chunkOptions.Length.GetValue()); - if (bytesRead != chunkOptions.Length.GetValue()) + chunkOptions.Range.GetValue().Length.GetValue()); + if (bytesRead != chunkOptions.Range.GetValue().Length.GetValue()) { throw std::runtime_error("error when reading body stream"); } @@ -692,23 +694,23 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { // Just start downloading using an initial chunk. If it's a small file, we'll get the whole // thing in one shot. If it's a large file, we'll get its full size in Content-Range and can // keep downloading it in chunks. - int64_t firstChunkOffset = options.Offset.HasValue() ? options.Offset.GetValue() : 0; + int64_t firstChunkOffset = options.Range.HasValue() ? options.Range.GetValue().Offset : 0; int64_t firstChunkLength = Details::c_FileDownloadDefaultChunkSize; if (options.InitialChunkSize.HasValue()) { firstChunkLength = options.InitialChunkSize.GetValue(); } - if (options.Length.HasValue()) + if (options.Range.HasValue() && options.Range.GetValue().Length.HasValue()) { - firstChunkLength = std::min(firstChunkLength, options.Length.GetValue()); + firstChunkLength = std::min(firstChunkLength, options.Range.GetValue().Length.GetValue()); } DownloadFileOptions firstChunkOptions; firstChunkOptions.Context = options.Context; - firstChunkOptions.Offset = options.Offset; - if (firstChunkOptions.Offset.HasValue()) + firstChunkOptions.Range = options.Range; + if (firstChunkOptions.Range.HasValue()) { - firstChunkOptions.Length = firstChunkLength; + firstChunkOptions.Range.GetValue().Length = firstChunkLength; } Storage::Details::FileWriter fileWriter(fileName); @@ -717,14 +719,14 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { int64_t fileSize; int64_t fileRangeSize; - if (firstChunkOptions.Offset.HasValue()) + if (firstChunkOptions.Range.HasValue()) { fileSize = std::stoll(firstChunk->ContentRange.GetValue().substr( firstChunk->ContentRange.GetValue().find('/') + 1)); fileRangeSize = fileSize - firstChunkOffset; - if (options.Length.HasValue()) + if (options.Range.GetValue().Length.HasValue()) { - fileRangeSize = std::min(fileRangeSize, options.Length.GetValue()); + fileRangeSize = std::min(fileRangeSize, options.Range.GetValue().Length.GetValue()); } } else @@ -778,14 +780,15 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { = [&](int64_t offset, int64_t length, int64_t chunkId, int64_t numChunks) { DownloadFileOptions chunkOptions; chunkOptions.Context = options.Context; - chunkOptions.Offset = offset; - chunkOptions.Length = length; + chunkOptions.Range = Core::Http::Range(); + chunkOptions.Range.GetValue().Offset = offset; + chunkOptions.Range.GetValue().Length = length; auto chunk = Download(chunkOptions); bodyStreamToFile( *(chunk->BodyStream), fileWriter, offset - firstChunkOffset, - chunkOptions.Length.GetValue(), + chunkOptions.Range.GetValue().Length.GetValue(), chunkOptions.Context); if (chunkId == numChunks - 1) 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 d680a206b..4fd4bc986 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 @@ -380,8 +380,13 @@ namespace Azure { namespace Storage { namespace Test { downloadBuffer.resize(static_cast(downloadSize), '\x00'); Files::Shares::DownloadFileToOptions options; options.Concurrency = concurrency; - options.Offset = offset; - options.Length = length; + if (offset.HasValue()) + { + options.Range = Core::Http::Range(); + options.Range.GetValue().Offset = offset.GetValue(); + options.Range.GetValue().Length = length; + } + options.InitialChunkSize = initialChunkSize; options.ChunkSize = chunkSize; if (actualDownloadSize > 0) @@ -439,8 +444,12 @@ namespace Azure { namespace Storage { namespace Test { } Files::Shares::DownloadFileToOptions options; options.Concurrency = concurrency; - options.Offset = offset; - options.Length = length; + if (offset.HasValue()) + { + options.Range = Core::Http::Range(); + options.Range.GetValue().Offset = offset.GetValue(); + options.Range.GetValue().Length = length; + } options.InitialChunkSize = initialChunkSize; options.ChunkSize = chunkSize; if (actualDownloadSize > 0) @@ -514,12 +523,13 @@ namespace Azure { namespace Storage { namespace Test { // buffer not big enough Files::Shares::DownloadFileToOptions options; options.Concurrency = c; - options.Offset = 1; + options.Range = Core::Http::Range(); + options.Range.GetValue().Offset = 1; for (int64_t length : {1ULL, 2ULL, 4_KB, 5_KB, 8_KB, 11_KB, 20_KB}) { std::vector downloadBuffer; downloadBuffer.resize(static_cast(length - 1)); - options.Length = length; + options.Range.GetValue().Length = length; EXPECT_THROW( m_fileClient->DownloadTo( downloadBuffer.data(), static_cast(length - 1), options), @@ -553,8 +563,9 @@ namespace Azure { namespace Storage { namespace Test { { std::vector resultBuffer; Files::Shares::DownloadFileOptions downloadOptions; - downloadOptions.Offset = static_cast(rangeSize) * i; - downloadOptions.Length = rangeSize; + 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)); @@ -632,10 +643,12 @@ namespace Azure { namespace Storage { namespace Test { Files::Shares::Models::GetFileRangeListResult result; EXPECT_NO_THROW(result = fileClient.GetRangeList().ExtractValue()); EXPECT_EQ(2U, result.Ranges.size()); - EXPECT_EQ(0, result.Ranges[0].Start); - EXPECT_EQ(511, result.Ranges[0].End); - EXPECT_EQ(1024, result.Ranges[1].Start); - EXPECT_EQ(static_cast(fileSize / 2) - 1, result.Ranges[1].End); + EXPECT_EQ(0, result.Ranges[0].Offset); + EXPECT_TRUE(result.Ranges[0].Length.HasValue()); + EXPECT_EQ(512, result.Ranges[0].Length.GetValue()); + EXPECT_EQ(1024, result.Ranges[1].Offset); + EXPECT_TRUE(result.Ranges[1].Length.HasValue()); + EXPECT_EQ(static_cast(fileSize / 2) - 1024, result.Ranges[1].Length.GetValue()); } TEST_F(FileShareFileClientTest, PreviousRangeWithSnapshot) @@ -663,29 +676,37 @@ namespace Azure { namespace Storage { namespace Test { options.PrevShareSnapshot = snapshot1; EXPECT_NO_THROW(result = fileClient.GetRangeList(options).ExtractValue()); EXPECT_EQ(2U, result.Ranges.size()); - EXPECT_EQ(0, result.Ranges[0].Start); - EXPECT_EQ(511, result.Ranges[0].End); - EXPECT_EQ(2048, result.Ranges[1].Start); - EXPECT_EQ(2559, result.Ranges[1].End); + EXPECT_EQ(0, result.Ranges[0].Offset); + EXPECT_TRUE(result.Ranges[0].Length.HasValue()); + EXPECT_EQ(512, result.Ranges[0].Length.GetValue()); + EXPECT_EQ(2048, result.Ranges[1].Offset); + EXPECT_TRUE(result.Ranges[1].Length.HasValue()); + EXPECT_EQ(512, result.Ranges[1].Length.GetValue()); EXPECT_NO_THROW(fileClient.ClearRange(3096, 2048)); auto snapshot3 = m_shareClient->CreateSnapshot()->Snapshot; options.PrevShareSnapshot = snapshot1; EXPECT_NO_THROW(result = fileClient.GetRangeList(options).ExtractValue()); EXPECT_EQ(4U, result.Ranges.size()); - EXPECT_EQ(0, result.Ranges[0].Start); - EXPECT_EQ(511, result.Ranges[0].End); - EXPECT_EQ(2048, result.Ranges[1].Start); - EXPECT_EQ(2559, result.Ranges[1].End); - EXPECT_EQ(3072, result.Ranges[2].Start); - EXPECT_EQ(3583, result.Ranges[2].End); - EXPECT_EQ(5120, result.Ranges[3].Start); - EXPECT_EQ(5631, result.Ranges[3].End); + EXPECT_EQ(0, result.Ranges[0].Offset); + EXPECT_TRUE(result.Ranges[0].Length.HasValue()); + EXPECT_EQ(512, result.Ranges[0].Length.GetValue()); + EXPECT_EQ(2048, result.Ranges[1].Offset); + EXPECT_TRUE(result.Ranges[1].Length.HasValue()); + EXPECT_EQ(512, result.Ranges[1].Length.GetValue()); + EXPECT_EQ(3072, result.Ranges[2].Offset); + EXPECT_TRUE(result.Ranges[2].Length.HasValue()); + EXPECT_EQ(512, result.Ranges[2].Length.GetValue()); + EXPECT_EQ(5120, result.Ranges[3].Offset); + EXPECT_TRUE(result.Ranges[3].Length.HasValue()); + EXPECT_EQ(512, result.Ranges[3].Length.GetValue()); EXPECT_EQ(2U, result.ClearRanges.size()); - EXPECT_EQ(512, result.ClearRanges[0].Start); - EXPECT_EQ(2047, result.ClearRanges[0].End); - EXPECT_EQ(3584, result.ClearRanges[1].Start); - EXPECT_EQ(5119, result.ClearRanges[1].End); + EXPECT_EQ(512, result.ClearRanges[0].Offset); + EXPECT_TRUE(result.ClearRanges[0].Length.HasValue()); + EXPECT_EQ(1536, result.ClearRanges[0].Length.GetValue()); + EXPECT_EQ(3584, result.ClearRanges[1].Offset); + EXPECT_TRUE(result.ClearRanges[1].Length.HasValue()); + EXPECT_EQ(1536, result.ClearRanges[1].Length.GetValue()); } }}} // namespace Azure::Storage::Test