[BUG] ShareClient.GetStatistics(); can't handle more bytes than can be stored in an Int.

[kemmis]

Describe the bug
ShareClient.GetStatistics(); throws exception when ShareUsageBytes value coming back from the API is too large.

Exception or Stack Trace
System.OverflowException
HResult=0x80131516
Message=Value was either too large or too small for an Int32.
Source=System.Private.CoreLib
StackTrace:
at System.Number.ThrowOverflowOrFormatException(ParsingStatus status, TypeCode type) in /_/src/System.Private.CoreLib/shared/System/Number.Parsing.cs:line 1933
at System.Number.ThrowOverflowOrFormatException(ParsingStatus status, TypeCode type)   at System.Int32.Parse(String s, IFormatProvider provider)   at Azure.Storage.Files.Shares.Models.ShareStatistics.FromXml(XElement element)   at Azure.Storage.Files.Shares.FileRestClient.Share.GetStatisticsAsync_CreateResponse(Response response)   at Azure.Storage.Files.Shares.FileRestClient.Share.GetStatisticsAsync(ClientDiagnostics clientDiagnostics, HttpPipeline pipeline, Uri resourceUri, Nullable 1 timeout, Boolean async, String operationName, CancellationToken cancellationToken)   at Azure.Storage.Files.Shares.ShareClient.GetStatisticsInternal(Boolean async, CancellationToken cancellationToken)   at Azure.Storage.TaskExtensions.EnsureCompleted[T](Task 1 task)   at Azure.Storage.Files.Shares.ShareClient.GetStatistics(CancellationToken cancellationToken)   at UserQuery.Main() in C:\Users\xxxx\AppData\Local\Temp\LINQPad6\xxxxx\LINQPadQuery:line 6   at LINQPad.ExecutionModel.ClrQueryRunner.Run()   at LINQPad.ExecutionModel.Server.RunQuery(QueryRunner runner)

To Reproduce

• Utilize more space (bytes) in your storage account file share than can be stored in an Int variable type.
• Call the ShareClient.GetStatistics(); Sdk.
• Notice that the number returning from the Azure Storage API is larger than an Int can represent.
• See exception "Value was either too large or too small for an Int32"

Code Snippet

using System;
using Azure.Storage;
using Azure.Storage.Files.Shares;

namespace ConsoleApp5
{
    class Program
    {
        static void Main(string[] args)
        {
            var cs = @"your-connection-string-here";
            var shareName = @"your-file-share-name-here";

            ShareClient share = new ShareClient(cs, shareName);
            var stats = share.GetStatistics();
        }
    }
}

Expected behavior
You should get back a statistic data model that represents the number of bytes if you have more than the number that can be represented by an in. Consider using an UInt64 instead of Int in this class property: ShareStatistics.ShareUsageBytes

Screenshots

(=6068040432 Bytes - can't be represented as Int type)

Setup (please complete the following information):

• OS: any
• IDE : any (used both linqpad 6 and vs 2019 enterprise)
• Version of the Library used: Azure.Storage.Files.Shares 12.0.0
• If you need to capture the API call result, you can turn on Fiddler's reverse-proxy, and it will start showing the .net core API calls. See here: https://www.telerik.com/forums/traffic-not-capture-unless-net-core-sdk-installed

edit: updated suggested property type to UInt64 instead of Int64.

[kemmis]

I looked into this a bit more, and it seems like the model we need to modify is generated by swagger. So somewhere in the project needs a config tweaked so that it generates that property as int64.

[seanmcc-msft]

We already have generator support for longs.

https://github.com/Azure/azure-rest-api-specs/blob/storage-dataplane-preview/specification/storage/data-plane/Microsoft.FileStorage/preview/2019-02-02/file.json#L3925

We just need to add "format": "int64":

        "ShareStats": {
            "description": "Stats for the share.",
            "type": "object",
            "required": [
                "ShareUsageBytes"
            ],
            "properties": {
                "ShareUsageBytes": {
                    "description": "The approximate size of the data stored in bytes. Note that this value may not include all recently created or recently resized files.",
                    "type": "integer",
                    "format": "int64"
                }
            }
        },

@tg-msft, could we make an exception to our backwards compatibility rules to make this change?

[kemmis]

You might consider an unsigned type since the value will never be negative.

…

On Wed, Dec 18, 2019 at 5:45 PM Sean McCullough ***@***.***> wrote: We already have generator support for longs. https://github.com/Azure/azure-rest-api-specs/blob/storage-dataplane-preview/specification/storage/data-plane/Microsoft.FileStorage/preview/2019-02-02/file.json#L3925 We just need to add "format": "int64": "ShareStats": { "description": "Stats for the share.", "type": "object", "required": [ "ShareUsageBytes" ], "properties": { "ShareUsageBytes": { "description": "The approximate size of the data stored in bytes. Note that this value may not include all recently created or recently resized files.", "type": "integer", "format": "int64" } } }, @tg-msft <https://github.com/tg-msft>, could we make an exception to our backwards compatibility rules to make this change? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#9186?email_source=notifications&email_token=AAE6C6NKDOZ5VONEKYVH5DDQZKRX7A5CNFSM4J4PJI72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHHX3WA#issuecomment-567246296>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAE6C6NQJ6N6YRNPBKKV46LQZKRX7ANCNFSM4J4PJI7Q> .

[kemmis]

Uint64 https://docs.microsoft.com/en-us/dotnet/api/system.uint64?view=netframework-4.8

…

On Wed, Dec 18, 2019 at 6:39 PM Rafe Kemmis ***@***.***> wrote: You might consider an unsigned type since the value will never be negative. On Wed, Dec 18, 2019 at 5:45 PM Sean McCullough ***@***.***> wrote: > We already have generator support for longs. > > > https://github.com/Azure/azure-rest-api-specs/blob/storage-dataplane-preview/specification/storage/data-plane/Microsoft.FileStorage/preview/2019-02-02/file.json#L3925 > > We just need to add "format": "int64": > > "ShareStats": { > "description": "Stats for the share.", > "type": "object", > "required": [ > "ShareUsageBytes" > ], > "properties": { > "ShareUsageBytes": { > "description": "The approximate size of the data stored in bytes. Note that this value may not include all recently created or recently resized files.", > "type": "integer", > "format": "int64" > } > } > }, > > @tg-msft <https://github.com/tg-msft>, could we make an exception to our > backwards compatibility rules to make this change? > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#9186?email_source=notifications&email_token=AAE6C6NKDOZ5VONEKYVH5DDQZKRX7A5CNFSM4J4PJI72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHHX3WA#issuecomment-567246296>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AAE6C6NQJ6N6YRNPBKKV46LQZKRX7ANCNFSM4J4PJI7Q> > . >

[tg-msft]

We can't break compat, but we could change the property type, rename it to ShareUsageInBytes or similar, and add a hidden redirect from the old property in a partial class like [EditorBrowsable(Never)] public int ShareUsageBytes => (int)ShareUsageInBytes; that will still throw an overflow exception. This will prevent anything from breaking if they're automatically upgraded to a newer version.

[kemmis]

Excellent solution. I’m happy to help put together a patch and pull request. Let me know if you would like my contribution. Thanks.

…

On Fri, Dec 20, 2019 at 1:00 PM tg-msft ***@***.***> wrote: We can't break compat, but we could change the property type, rename it to ShareUsageInBytes or similar, and add a hidden redirect from the old property in a partial class like [EditorBrowsable(Never)] public int ShareUsageBytes => (int)ShareUsageInBytes; that will still throw an overflow exception. This will prevent anything from breaking if they're automatically upgraded to a newer version. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#9186?email_source=notifications&email_token=AAE6C6OR5QTYMIZORPSHLGDQZUB33A5CNFSM4J4PJI72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHNVECA#issuecomment-568021512>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAE6C6NRTVBR5F73IA3PKIDQZUB33ANCNFSM4J4PJI7Q> .

[tg-msft]

If you want to try it, that'd be really awesome. Here's a rough guide to get you started:

• Take a look at the code gen section of Contributing.md
• Add a transform in the Files.Shares README by searching for $.ShareStatistics.properties.ShareUsageBytes:
  • I think it'll just be $.ShareStatistics.properties.ShareUsageBytes.format = 'int64';
  • And then renaming the property with with $.ShareStatistics.properties.ShareUsageBytes['x-ms-client-name'] = 'ShareUsageInBytes';
  • Re-run the code generator and make sure you're happy with the git diff
• Open Azure.Storage.sln
• Add a partial class ShareStatistics in the Models folder and forward the old property to the new property you just added
• Do a Find/Replace All to change from ShareUsageBytes to ShareUsageInBytes
• Run the (recorded) unit tests in the VS Test Explorer or via the command line
• Update our API listings by running eng/Export-API.ps1
• ???
• Profit Submit a PR

Ping here with any questions. Thanks!

[kemmis]

That all makes sense. Thanks for the guide!

…

On Fri, Dec 20, 2019 at 8:13 PM tg-msft ***@***.***> wrote: If you want to try it, that'd be really awesome. Here's a rough guide to get you started: - Take a look at the code gen section <https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/storage/CONTRIBUTING.md#code-generation> of Contributing.md - Add a transform in the Files.Shares README <https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/storage/Azure.Storage.Files.Shares/swagger/readme.md> by searching for $.ShareStatistics.properties.ShareUsageBytes: - I think it'll just be $.ShareStatistics.properties.ShareUsageBytes.format = 'int64'; - And then renaming the property with with $.ShareStatistics.properties.ShareUsageBytes['x-ms-client-name'] = 'ShareUsageInBytes'; - Re-run the code generator and make sure you're happy with the git diff - Open Azure.Storage.sln <https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/storage/Azure.Storage.sln> - Add a partial class ShareStatistics in the Models folder <https://github.com/Azure/azure-sdk-for-net/tree/master/sdk/storage/Azure.Storage.Files.Shares/src/Models> and forward the old property to the new property you just added - Do a Find/Replace All to change from ShareUsageBytes to ShareUsageInBytes - Run the (recorded) unit tests in the VS Test Explorer or via the command line - ??? - Profit Submit a PR <https://github.com/Azure/azure-sdk-for-net/blob/master/CONTRIBUTING.md#code-review-process> Ping here with any questions. Thanks! — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#9186?email_source=notifications&email_token=AAE6C6M4SSCV7FQT2GTZ5ETQZVUR7A5CNFSM4J4PJI72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHORV2Q#issuecomment-568138474>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAE6C6LZZX6XWFNHF2IYR3DQZVUR7ANCNFSM4J4PJI7Q> .

[seanmcc-msft]

@kemmis, I'm going to jump in with the fix so we can get it out in the next release.