Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

format-by-name-rules does not set the deserialization format string for datetime #3090

Closed
fengzhou-msft opened this issue Feb 9, 2023 · 1 comment
Labels
Mgmt This issue is related to a management-plane library.

Comments

@fengzhou-msft
Copy link
Member

In OperationalInsights, using below config:

format-by-name-rules:
   'lastSkuUpdatedOn': 'datetime'

Actual behavior:
The generated deserialization code will use:
lastSkuUpdate = property.Value.GetDateTimeOffset();

This calls the JsonElement.GetDateTimeOffset() method that only supports ISO 8601 format.

This caused issue: Azure/azure-sdk-for-net#33536

Expected behavior:
The generated code should have a datetime format string:
lastSkuUpdate = property.Value.GetDateTimeOffset("O");

This calls an extension method:

public static DateTimeOffset GetDateTimeOffset(in this JsonElement element, string format) => format switch
{
"U" when element.ValueKind == JsonValueKind.Number => DateTimeOffset.FromUnixTimeSeconds(element.GetInt64()),
// relying on the param check of the inner call to throw ArgumentNullException if GetString() returns null
_ => TypeFormatters.ParseDateTimeOffset(element.GetString()!, format)
};

It treats the JsonElement as Int64 (Unix Timestamp) or string and the string case can support multiple formats despite the "O" format (ISO8601) being passed in.
public static DateTimeOffset ParseDateTimeOffset(string value, string format)
{
return format switch
{
"U" => DateTimeOffset.FromUnixTimeSeconds(long.Parse(value, CultureInfo.InvariantCulture)),
_ => DateTimeOffset.Parse(value, CultureInfo.InvariantCulture, DateTimeStyles.AssumeUniversal)
};
}

@ArthurMa1978 ArthurMa1978 added the Mgmt This issue is related to a management-plane library. label Feb 10, 2023
@ArthurMa1978
Copy link
Member

Have another issue #2524 opened to track all unsupported format.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mgmt This issue is related to a management-plane library.
Projects
None yet
Development

No branches or pull requests

2 participants