Skip to content

Fix AOT warnings for LRO rehydration #44208

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
May 31, 2024
+26 −25
Merged

Fix AOT warnings for LRO rehydration #44208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1ac4ae8
Fix AOT warnings with JsonDocument.Parse
live1206 May 23, 2024
b470d31
dispose JsonDocument.Parse
live1206 May 23, 2024
ce30c20
Use existing framework method instead
live1206 May 24, 2024
0ff05ac
Merge branch 'main' into aot-json-element
live1206 May 24, 2024
a74611a
update comment
live1206 May 26, 2024
2e15346
ignore failing tests
live1206 May 27, 2024
1733b9d
fix more AOT warnings
live1206 May 29, 2024
e22c103
Merge branch 'main' into aot-json-element
live1206 May 29, 2024
e6737dd
Merge branch 'main' into aot-json-element
live1206 May 29, 2024
869d872
update expected AOT warnings
live1206 May 30, 2024
81afda8
Add BinaryData AOT warnings back
live1206 May 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions sdk/core/Azure.Core/src/Operation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.ClientModel.Primitives;
using System.ComponentModel;
using System.Diagnostics.CodeAnalysis;
using System.Threading;
using System.Threading.Tasks;
using Azure.Core;
Expand All @@ -25,7 +26,7 @@ public abstract class Operation
/// <param name="rehydrationToken">The rehydration token.</param>
/// <param name="options">The client options.</param>
/// <returns>The long-running operation.</returns>
public static Operation<T> Rehydrate<T>(HttpPipeline pipeline, RehydrationToken rehydrationToken, ClientOptions? options = null) where T : IPersistableModel<T>
public static Operation<T> Rehydrate<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] T>(HttpPipeline pipeline, RehydrationToken rehydrationToken, ClientOptions? options = null) where T : IPersistableModel<T>
{
Argument.AssertNotNull(pipeline, nameof(pipeline));
Argument.AssertNotNull(rehydrationToken, nameof(rehydrationToken));
Expand Down Expand Up @@ -61,7 +62,7 @@ public static Operation Rehydrate(HttpPipeline pipeline, RehydrationToken rehydr
/// <param name="rehydrationToken">The rehydration token.</param>
/// <param name="options">The client options.</param>
/// <returns>The long-running operation.</returns>
public static async Task<Operation<T>> RehydrateAsync<T>(HttpPipeline pipeline, RehydrationToken rehydrationToken, ClientOptions? options = null) where T : IPersistableModel<T>
public static async Task<Operation<T>> RehydrateAsync<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] T>(HttpPipeline pipeline, RehydrationToken rehydrationToken, ClientOptions? options = null) where T : IPersistableModel<T>
{
Argument.AssertNotNull(pipeline, nameof(pipeline));
Argument.AssertNotNull(rehydrationToken, nameof(rehydrationToken));
Expand Down
44 changes: 23 additions & 21 deletions sdk/core/Azure.Core/src/Shared/NextLinkOperationImplementation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,21 +97,26 @@ public static IOperation Create(
AssertNotNull(rehydrationToken, nameof(rehydrationToken));
AssertNotNull(pipeline, nameof(pipeline));

// TODO: Once we remove NextLinkOperationImplementation from internal shared and make it internal to Azure.Core only, we can access the internal members from RehydrationToken directly
var lroDetails = ModelReaderWriter.Write(rehydrationToken!, ModelReaderWriterOptions.Json).ToObjectFromJson<Dictionary<string, string>>();

var initialUri = GetContentFromRehydrationToken(lroDetails, "initialUri");
// TODO: Once we remove NextLinkOperationImplementation from internal shared and make it internal to Azure.Core only in https://github.com/Azure/azure-sdk-for-net/issues/43260
// We can access the internal members from RehydrationToken directly
var data = ModelReaderWriter.Write(rehydrationToken!, ModelReaderWriterOptions.Json);
using var document = JsonDocument.Parse(data);
var lroDetails = document.RootElement;

// We are sure that the following properties exists in the serialized rehydrationToken
var initialUri = lroDetails.GetProperty("initialUri").GetString();
if (!Uri.TryCreate(initialUri, UriKind.Absolute, out var startRequestUri))
{
throw new ArgumentException($"\"initialUri\" property on \"rehydrationToken\" is an invalid Uri", nameof(rehydrationToken));
}

string nextRequestUri = GetContentFromRehydrationToken(lroDetails, "nextRequestUri");
string requestMethodStr = GetContentFromRehydrationToken(lroDetails, "requestMethod");
RequestMethod requestMethod = new RequestMethod(requestMethodStr);
string lastKnownLocation = GetContentFromRehydrationToken(lroDetails, "lastKnownLocation");
// We are sure that the following properties(apart from nullable lastKnownLocation) are not null as they are required in the rehydrationToken
string nextRequestUri = lroDetails.GetProperty("nextRequestUri").GetString()!;
string requestMethodStr = lroDetails.GetProperty("requestMethod").GetString()!;
RequestMethod requestMethod = new RequestMethod(requestMethodStr)!;
string? lastKnownLocation = lroDetails.GetProperty("lastKnownLocation").GetString();

string finalStateViaStr = GetContentFromRehydrationToken(lroDetails, "finalStateVia");
string finalStateViaStr = lroDetails.GetProperty("finalStateVia").GetString()!;
OperationFinalStateVia finalStateVia;
if (Enum.IsDefined(typeof(OperationFinalStateVia), finalStateViaStr))
{
Expand All @@ -122,7 +127,7 @@ public static IOperation Create(
finalStateVia = OperationFinalStateVia.Location;
}

string headerSourceStr = GetContentFromRehydrationToken(lroDetails, "headerSource");
string headerSourceStr = lroDetails.GetProperty("headerSource").GetString()!;
HeaderSource headerSource;
if (Enum.IsDefined(typeof(HeaderSource), headerSourceStr))
{
Expand All @@ -136,16 +141,6 @@ public static IOperation Create(
return new NextLinkOperationImplementation(pipeline, requestMethod, startRequestUri, nextRequestUri, headerSource, lastKnownLocation, finalStateVia, null, rehydrationToken.Id);
}

private static string GetContentFromRehydrationToken(Dictionary<string, string> lroDetails, string key)
{
if (!lroDetails.TryGetValue(key, out var nextRequestUri))
{
throw new ArgumentException($"\"{key}\" is missing from rehydrationToken");
}

return nextRequestUri;
}

private NextLinkOperationImplementation(
HttpPipeline pipeline,
RequestMethod requestMethod,
Expand Down Expand Up @@ -222,10 +217,17 @@ public static RehydrationToken GetRehydrationToken(
string finalStateVia,
string? operationId = null)
{
var data = new BinaryData(new { version = RehydrationTokenVersion, id = operationId, requestMethod = requestMethod.ToString(), initialUri = startRequestUri.AbsoluteUri, nextRequestUri, headerSource, lastKnownLocation, finalStateVia });
// TODO: Once we remove NextLinkOperationImplementation from internal shared and make it internal to Azure.Core only in https://github.com/Azure/azure-sdk-for-net/issues/43260
// We can access the internal members from RehydrationToken directly
var json = $$"""
{"version":"{{RehydrationTokenVersion}}","id":{{ConstructStringValue(operationId)}},"requestMethod":"{{requestMethod}}","initialUri":"{{startRequestUri.AbsoluteUri}}","nextRequestUri":"{{nextRequestUri}}","headerSource":"{{headerSource}}","finalStateVia":"{{finalStateVia}}","lastKnownLocation":{{ConstructStringValue(lastKnownLocation)}}}
""";
var data = new BinaryData(json);
return ModelReaderWriter.Read<RehydrationToken>(data);
}

private static string? ConstructStringValue(string? value) => value is null ? "null" : $"\"{value}\"";

public async ValueTask<OperationState> UpdateStateAsync(bool async, CancellationToken cancellationToken)
{
Response response = async
Expand Down
2 changes: 0 additions & 2 deletions sdk/core/Azure.Core/tests/compatibility/ExpectedAotWarnings.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,10 @@ ILC : Trim analysis warning IL2026: Azure\.Core\.Json\.MutableJsonDocument: Usin
ILC : AOT analysis warning IL3050: Azure\.Core\.Json\.MutableJsonDocument: Using member 'Azure\.Core\.Json\.MutableJsonDocument\.MutableJsonDocumentConverter\.MutableJsonDocumentConverter\(\)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling\. Using MutableJsonDocument or MutableJsonDocumentConverter is not compatible with trimming due to reflection-based serialization
.*Azure\.Core.src.JsonPatchDocument\.cs\(\d*\): Trim analysis warning IL2026: Azure\.JsonPatchDocument\.JsonPatchDocument\(ReadOnlyMemory`1<Byte>\): Using member 'Azure\.Core\.Serialization\.JsonObjectSerializer\.JsonObjectSerializer\(\)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code\. This class uses reflection-based JSON serialization and deserialization that is not compatible with trimming
.*Azure\.Core.src.JsonPatchDocument\.cs\(\d*\): AOT analysis warning IL3050: Azure\.JsonPatchDocument\.JsonPatchDocument\(ReadOnlyMemory`1<Byte>\): Using member 'Azure\.Core\.Serialization\.JsonObjectSerializer\.JsonObjectSerializer\(\)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling\. This class uses reflection-based JSON serialization and deserialization that is not compatible with trimming
.*Azure\.Core.src.Operation\.cs\(\d*\): Trim analysis warning IL2091: Azure\.Operation\.Rehydrate<T>\(HttpPipeline,RehydrationToken,ClientOptions\): 'T' generic argument does not satisfy 'DynamicallyAccessedMemberTypes\.PublicConstructors', 'DynamicallyAccessedMemberTypes\.NonPublicConstructors' in 'Azure\.Core\.GenericOperationSource`1'\. The generic parameter 'T' of 'Azure\.Operation\.Rehydrate<T>\(HttpPipeline,RehydrationToken,ClientOptions\)' does not have matching annotations\. The source value must declare at least the same requirements as those declared on the target location it is assigned to
.*Azure\.Core.src.Serialization.AzureCoreExtensions\.cs\(\d*\): Trim analysis warning IL2026: Azure\.AzureCoreExtensions\.ToDynamicFromJson\(BinaryData,DynamicDataOptions\): Using member 'Azure\.Core\.Json\.MutableJsonDocument\.Parse\(BinaryData,JsonSerializerOptions\)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code\. This class utilizes reflection-based JSON serialization and deserialization which is not compatible with trimming
.*Azure\.Core.src.Serialization.AzureCoreExtensions\.cs\(\d*\): AOT analysis warning IL3050: Azure\.AzureCoreExtensions\.ToDynamicFromJson\(BinaryData,DynamicDataOptions\): Using member 'Azure\.Core\.Json\.MutableJsonDocument\.Parse\(BinaryData,JsonSerializerOptions\)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling\. This class utilizes reflection-based JSON serialization and deserialization which is not compatible with trimming
.*Azure\.Core.src.Serialization.AzureCoreExtensions\.cs\(\d*\): Trim analysis warning IL2026: Azure\.AzureCoreExtensions\.ToDynamicFromJson\(BinaryData,DynamicDataOptions\): Using member 'Azure\.Core\.Serialization\.DynamicData\.DynamicData\(MutableJsonElement,DynamicDataOptions\)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code\. This class utilizes reflection-based JSON serialization and deserialization which is not compatible with trimming
.*Azure\.Core.src.Serialization.AzureCoreExtensions\.cs\(\d*\): AOT analysis warning IL3050: Azure\.AzureCoreExtensions\.ToDynamicFromJson\(BinaryData,DynamicDataOptions\): Using member 'Azure\.Core\.Serialization\.DynamicData\.DynamicData\(MutableJsonElement,DynamicDataOptions\)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling\. This class utilizes reflection-based JSON serialization and deserialization which is not compatible with trimming
.*Azure\.Core.src.Operation\.cs\(\d*\): Trim analysis warning IL2091: Azure\.Operation\.<RehydrateAsync>d__2`1\.MoveNext\(\): 'T' generic argument does not satisfy 'DynamicallyAccessedMemberTypes\.PublicConstructors', 'DynamicallyAccessedMemberTypes\.NonPublicConstructors' in 'Azure\.Core\.GenericOperationSource`1'\. The generic parameter 'T' of 'Azure\.Operation\.<RehydrateAsync>d__2`1' does not have matching annotations\. The source value must declare at least the same requirements as those declared on the target location it is assigned
.*Azure\.Core.src.DynamicData.DynamicData\.AllowList\.cs\(\d*\): Trim analysis warning IL2070: Azure\.Core\.Serialization\.DynamicData\.AllowList\.IsAllowedAnonymousValue<T>\(Type,!!0\): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes\.PublicProperties' in call to 'System\.Type\.GetProperties\(\)'\. The parameter 'type' of method 'Azure\.Core\.Serialization\.DynamicData\.AllowList\.IsAllowedAnonymousValue<T>\(Type,!!0\)' does not have matching annotations\. The source value must declare at least the same requirements as those declared on the target location it is assigned to
.*Azure\.Core.src.DynamicData.DynamicData\.ArrayEnumerator\.cs\(\d*\): Trim analysis warning IL2026: Azure\.Core\.Serialization\.DynamicData\.ArrayEnumerator\.Current\.get: Using member 'Azure\.Core\.Serialization\.DynamicData\.DynamicData\(MutableJsonElement,DynamicDataOptions\)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code\. This class utilizes reflection-based JSON serialization and deserialization which is not compatible with trimming
.*Azure\.Core.src.DynamicData.DynamicData\.ArrayEnumerator\.cs\(\d*\): AOT analysis warning IL3050: Azure\.Core\.Serialization\.DynamicData\.ArrayEnumerator\.Current\.get: Using member 'Azure\.Core\.Serialization\.DynamicData\.DynamicData\(MutableJsonElement,DynamicDataOptions\)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling\. This class utilizes reflection-based JSON serialization and deserialization which is not compatible with trimming
Expand Down