From 9084906e27160187cdd91128273ec427ff766507 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Tue, 5 Mar 2024 09:21:11 -0800 Subject: [PATCH] ClientModel: Updates to ClientModel public APIs from Azure.Core 2.0 integration (#42328) * Updates to ClientModel public APIs from Azure.Core 2.0 integration * Enable setting message property to null --- sdk/core/System.ClientModel/CHANGELOG.md | 5 +++ .../api/System.ClientModel.net6.0.cs | 7 ++-- .../api/System.ClientModel.netstandard2.0.cs | 7 ++-- .../src/Convenience/ClientResultException.cs | 21 +----------- .../src/Internal/ArrayBackedPropertyBag.cs | 34 +++++++++---------- .../src/Message/ArrayBackedRequestHeaders.cs | 15 ++++---- .../src/Message/PipelineMessage.cs | 14 ++++---- .../src/Options/RequestOptions.cs | 9 +++-- .../HttpClientPipelineTransport.Request.cs | 2 +- 9 files changed, 53 insertions(+), 61 deletions(-) diff --git a/sdk/core/System.ClientModel/CHANGELOG.md b/sdk/core/System.ClientModel/CHANGELOG.md index 3111f298b5a87..7ba35ad102388 100644 --- a/sdk/core/System.ClientModel/CHANGELOG.md +++ b/sdk/core/System.ClientModel/CHANGELOG.md @@ -4,8 +4,13 @@ ### Features Added +- Added protected `Apply(PipelineMessage)` method to `RequestOptions` so that derived types can extend its functionality. + ### Breaking Changes +- Removed `[Serializable]` attribute and serialization constructor from `ClientResultException`. +- Made `value` parameter nullable in `PipelineMessage.SetProperty` method. + ### Bugs Fixed ### Other Changes diff --git a/sdk/core/System.ClientModel/api/System.ClientModel.net6.0.cs b/sdk/core/System.ClientModel/api/System.ClientModel.net6.0.cs index f37941fb2731d..7551ae4726833 100644 --- a/sdk/core/System.ClientModel/api/System.ClientModel.net6.0.cs +++ b/sdk/core/System.ClientModel/api/System.ClientModel.net6.0.cs @@ -25,14 +25,12 @@ protected ClientResult(System.ClientModel.Primitives.PipelineResponse response) public static System.ClientModel.ClientResult FromValue(T value, System.ClientModel.Primitives.PipelineResponse response) { throw null; } public System.ClientModel.Primitives.PipelineResponse GetRawResponse() { throw null; } } - public partial class ClientResultException : System.Exception, System.Runtime.Serialization.ISerializable + public partial class ClientResultException : System.Exception { public ClientResultException(System.ClientModel.Primitives.PipelineResponse response, System.Exception? innerException = null) { } - protected ClientResultException(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { } public ClientResultException(string message, System.ClientModel.Primitives.PipelineResponse? response = null, System.Exception? innerException = null) { } public int Status { get { throw null; } protected set { } } public static System.Threading.Tasks.Task CreateAsync(System.ClientModel.Primitives.PipelineResponse response, System.Exception? innerException = null) { throw null; } - public override void GetObjectData(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { } public System.ClientModel.Primitives.PipelineResponse? GetRawResponse() { throw null; } } public partial class ClientResult : System.ClientModel.ClientResult @@ -153,7 +151,7 @@ public void Apply(System.ClientModel.Primitives.RequestOptions options) { } public void Dispose() { } protected virtual void Dispose(bool disposing) { } public System.ClientModel.Primitives.PipelineResponse? ExtractResponse() { throw null; } - public void SetProperty(System.Type key, object value) { } + public void SetProperty(System.Type key, object? value) { } public bool TryGetProperty(System.Type key, out object? value) { throw null; } } public abstract partial class PipelineMessageClassifier @@ -244,6 +242,7 @@ public RequestOptions() { } public System.ClientModel.Primitives.ClientErrorBehaviors ErrorOptions { get { throw null; } set { } } public void AddHeader(string name, string value) { } public void AddPolicy(System.ClientModel.Primitives.PipelinePolicy policy, System.ClientModel.Primitives.PipelinePosition position) { } + protected internal virtual void Apply(System.ClientModel.Primitives.PipelineMessage message) { } protected void AssertNotFrozen() { } public virtual void Freeze() { } public void SetHeader(string name, string value) { } diff --git a/sdk/core/System.ClientModel/api/System.ClientModel.netstandard2.0.cs b/sdk/core/System.ClientModel/api/System.ClientModel.netstandard2.0.cs index e23c8f4bfff7a..c1401ba27e916 100644 --- a/sdk/core/System.ClientModel/api/System.ClientModel.netstandard2.0.cs +++ b/sdk/core/System.ClientModel/api/System.ClientModel.netstandard2.0.cs @@ -25,14 +25,12 @@ protected ClientResult(System.ClientModel.Primitives.PipelineResponse response) public static System.ClientModel.ClientResult FromValue(T value, System.ClientModel.Primitives.PipelineResponse response) { throw null; } public System.ClientModel.Primitives.PipelineResponse GetRawResponse() { throw null; } } - public partial class ClientResultException : System.Exception, System.Runtime.Serialization.ISerializable + public partial class ClientResultException : System.Exception { public ClientResultException(System.ClientModel.Primitives.PipelineResponse response, System.Exception? innerException = null) { } - protected ClientResultException(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { } public ClientResultException(string message, System.ClientModel.Primitives.PipelineResponse? response = null, System.Exception? innerException = null) { } public int Status { get { throw null; } protected set { } } public static System.Threading.Tasks.Task CreateAsync(System.ClientModel.Primitives.PipelineResponse response, System.Exception? innerException = null) { throw null; } - public override void GetObjectData(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { } public System.ClientModel.Primitives.PipelineResponse? GetRawResponse() { throw null; } } public partial class ClientResult : System.ClientModel.ClientResult @@ -152,7 +150,7 @@ public void Apply(System.ClientModel.Primitives.RequestOptions options) { } public void Dispose() { } protected virtual void Dispose(bool disposing) { } public System.ClientModel.Primitives.PipelineResponse? ExtractResponse() { throw null; } - public void SetProperty(System.Type key, object value) { } + public void SetProperty(System.Type key, object? value) { } public bool TryGetProperty(System.Type key, out object? value) { throw null; } } public abstract partial class PipelineMessageClassifier @@ -243,6 +241,7 @@ public RequestOptions() { } public System.ClientModel.Primitives.ClientErrorBehaviors ErrorOptions { get { throw null; } set { } } public void AddHeader(string name, string value) { } public void AddPolicy(System.ClientModel.Primitives.PipelinePolicy policy, System.ClientModel.Primitives.PipelinePosition position) { } + protected internal virtual void Apply(System.ClientModel.Primitives.PipelineMessage message) { } protected void AssertNotFrozen() { } public virtual void Freeze() { } public void SetHeader(string name, string value) { } diff --git a/sdk/core/System.ClientModel/src/Convenience/ClientResultException.cs b/sdk/core/System.ClientModel/src/Convenience/ClientResultException.cs index f64c10d1b5b70..15356c161c7de 100644 --- a/sdk/core/System.ClientModel/src/Convenience/ClientResultException.cs +++ b/sdk/core/System.ClientModel/src/Convenience/ClientResultException.cs @@ -4,7 +4,6 @@ using System.ClientModel.Internal; using System.ClientModel.Primitives; using System.Globalization; -using System.Runtime.Serialization; using System.Text; using System.Threading.Tasks; @@ -13,8 +12,7 @@ namespace System.ClientModel; /// /// The exception that is thrown when the processing of a client request failed. /// -[Serializable] -public class ClientResultException : Exception, ISerializable +public class ClientResultException : Exception { private const string DefaultMessage = "Service request failed."; @@ -84,23 +82,6 @@ public ClientResultException(string message, PipelineResponse? response = defaul _status = response?.Status ?? 0; } - /// - protected ClientResultException(SerializationInfo info, StreamingContext context) - : base(info, context) - { - _status = info.GetInt32(nameof(Status)); - } - - /// - public override void GetObjectData(SerializationInfo info, StreamingContext context) - { - Argument.AssertNotNull(info, nameof(info)); - - info.AddValue(nameof(Status), Status); - - base.GetObjectData(info, context); - } - /// /// Gets the , if any, that led to the exception. /// diff --git a/sdk/core/System.ClientModel/src/Internal/ArrayBackedPropertyBag.cs b/sdk/core/System.ClientModel/src/Internal/ArrayBackedPropertyBag.cs index 78696bd63b796..fcab5b9a5412b 100644 --- a/sdk/core/System.ClientModel/src/Internal/ArrayBackedPropertyBag.cs +++ b/sdk/core/System.ClientModel/src/Internal/ArrayBackedPropertyBag.cs @@ -15,9 +15,9 @@ namespace System.ClientModel.Internal; /// internal struct ArrayBackedPropertyBag where TKey : struct, IEquatable { - private (TKey Key, TValue Value) _first; - private (TKey Key, TValue Value) _second; - private (TKey Key, TValue Value)[]? _rest; + private (TKey Key, TValue? Value) _first; + private (TKey Key, TValue? Value) _second; + private (TKey Key, TValue? Value)[]? _rest; private int _count; private readonly object _lock = new(); #if DEBUG @@ -46,7 +46,7 @@ public bool IsEmpty } } - public void GetAt(int index, out TKey key, out TValue value) + public void GetAt(int index, out TKey key, out TValue? value) { CheckDisposed(); (key, value) = index switch @@ -57,7 +57,7 @@ public void GetAt(int index, out TKey key, out TValue value) }; } - public bool TryGetValue(TKey key, [MaybeNullWhen(false)] out TValue value) + public bool TryGetValue(TKey key, [MaybeNullWhen(false)] out TValue? value) { CheckDisposed(); var index = GetIndex(key); @@ -86,7 +86,7 @@ public bool TryAdd(TKey key, TValue value, out TValue? existingValue) return true; } - public void Set(TKey key, TValue value) + public void Set(TKey key, TValue? value) { CheckDisposed(); var index = GetIndex(key); @@ -131,7 +131,7 @@ public bool TryRemove(TKey key) return false; default: - (TKey Key, TValue Value)[] rest = GetRest(); + (TKey Key, TValue? Value)[] rest = GetRest(); if (IsFirst(key)) { _first = _second; @@ -173,7 +173,7 @@ public bool TryRemove(TKey key) private bool IsSecond(TKey key) => _second.Key.Equals(key); [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void AddInternal(TKey key, TValue value) + private void AddInternal(TKey key, TValue? value) { switch (_count) { @@ -196,18 +196,18 @@ private void AddInternal(TKey key, TValue value) default: if (_rest == null) { - _rest = ArrayPool<(TKey Key, TValue Value)>.Shared.Rent(8); + _rest = ArrayPool<(TKey Key, TValue? Value)>.Shared.Rent(8); _rest[_count++ - 2] = (key, value); return; } if (_rest.Length <= _count) { - var larger = ArrayPool<(TKey Key, TValue Value)>.Shared.Rent(_rest.Length << 1); + var larger = ArrayPool<(TKey Key, TValue? Value)>.Shared.Rent(_rest.Length << 1); _rest.CopyTo(larger, 0); var old = _rest; _rest = larger; - ArrayPool<(TKey Key, TValue Value)>.Shared.Return(old, true); + ArrayPool<(TKey Key, TValue? Value)>.Shared.Return(old, true); } _rest[_count++ - 2] = (key, value); return; @@ -215,7 +215,7 @@ private void AddInternal(TKey key, TValue value) } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void SetAt(int index, (TKey Key, TValue Value) value) + private void SetAt(int index, (TKey Key, TValue? Value) value) { if (index == 0) _first = value; @@ -226,7 +226,7 @@ private void SetAt(int index, (TKey Key, TValue Value) value) } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private TValue GetAt(int index) => index switch + private TValue? GetAt(int index) => index switch { 0 => _first.Value, 1 => _second.Value, @@ -246,7 +246,7 @@ private int GetIndex(TKey key) if (_count <= 2) return -1; - (TKey Key, TValue Value)[] rest = GetRest(); + (TKey Key, TValue? Value)[] rest = GetRest(); int max = _count - 2; for (var i = 0; i < max; i++) { @@ -276,13 +276,13 @@ public void Dispose() return; } - var rest = _rest; + (TKey Key, TValue? Value)[] rest = _rest; _rest = default; - ArrayPool<(TKey Key, TValue Value)>.Shared.Return(rest, true); + ArrayPool<(TKey Key, TValue? Value)>.Shared.Return(rest, true); } } - private (TKey Key, TValue Value)[] GetRest() => _rest ?? + private (TKey Key, TValue? Value)[] GetRest() => _rest ?? throw new InvalidOperationException($"{nameof(_rest)} field is null while {nameof(_count)} == {_count}"); [Conditional("DEBUG")] diff --git a/sdk/core/System.ClientModel/src/Message/ArrayBackedRequestHeaders.cs b/sdk/core/System.ClientModel/src/Message/ArrayBackedRequestHeaders.cs index 860ddacd491cd..ff2a540d3361f 100644 --- a/sdk/core/System.ClientModel/src/Message/ArrayBackedRequestHeaders.cs +++ b/sdk/core/System.ClientModel/src/Message/ArrayBackedRequestHeaders.cs @@ -39,7 +39,7 @@ public override bool TryGetValue(string name, out string? value) { if (_headers.TryGetValue(new IgnoreCaseString(name), out object? headerValue)) { - value = GetHeaderValueString(name, headerValue); + value = GetHeaderValueString(name, headerValue!); return true; } @@ -51,7 +51,7 @@ public override bool TryGetValues(string name, out IEnumerable? values) { if (_headers.TryGetValue(new IgnoreCaseString(name), out object? value)) { - values = GetHeaderValueEnumerable(name, value); + values = GetHeaderValueEnumerable(name, value!); return true; } @@ -63,16 +63,17 @@ public override IEnumerator> GetEnumerator() => GetHeadersStringValues().GetEnumerator(); // Internal API provided to take advantage of performance-optimized implementation. - internal bool GetNextValue(int index, out string name, out object value) + internal bool GetNextValue(int index, out string name, out object? value) { if (index >= _headers.Count) { name = default!; - value = default!; + value = default; return false; } - _headers.GetAt(index, out IgnoreCaseString headerName, out object headerValue); + _headers.GetAt(index, out IgnoreCaseString headerName, out object? headerValue); + name = headerName; value = headerValue; return true; @@ -83,8 +84,8 @@ private IEnumerable> GetHeadersStringValues() { for (int i = 0; i < _headers.Count; i++) { - _headers.GetAt(i, out IgnoreCaseString name, out object value); - string values = GetHeaderValueString(name, value); + _headers.GetAt(i, out IgnoreCaseString name, out object? value); + string values = GetHeaderValueString(name, value!); yield return new KeyValuePair(name, values); } } diff --git a/sdk/core/System.ClientModel/src/Message/PipelineMessage.cs b/sdk/core/System.ClientModel/src/Message/PipelineMessage.cs index 1345c1ccc8b48..7c62f7fc7f66e 100644 --- a/sdk/core/System.ClientModel/src/Message/PipelineMessage.cs +++ b/sdk/core/System.ClientModel/src/Message/PipelineMessage.cs @@ -104,17 +104,19 @@ public CancellationToken CancellationToken /// /// Apply the options from the provided to - /// this instance. This method is intended to - /// be called after the creation of the and - /// its is complete, and prior to the call to - /// . + /// this instance. /// /// The to apply to this /// instance. + /// This method is intended to be called after the creation of + /// the and its is + /// complete, and prior to the call to + /// . + /// public void Apply(RequestOptions options) { // This design moves the client-author API (options.Apply) off the - // client-user type RequestOptions. Its only purpose is to call through to + // client-user type RequestOptions. Its only purpose is to call through to // the internal options.Apply method. options.Apply(this); } @@ -144,7 +146,7 @@ public bool TryGetProperty(Type key, out object? value) => /// The key for the property in the message's property /// bag. /// The value of the property. - public void SetProperty(Type key, object value) => + public void SetProperty(Type key, object? value) => _propertyBag.Set((ulong)key.TypeHandle.Value, value); #endregion diff --git a/sdk/core/System.ClientModel/src/Options/RequestOptions.cs b/sdk/core/System.ClientModel/src/Options/RequestOptions.cs index 367f8d8baede3..1d1665eba6e62 100644 --- a/sdk/core/System.ClientModel/src/Options/RequestOptions.cs +++ b/sdk/core/System.ClientModel/src/Options/RequestOptions.cs @@ -128,8 +128,13 @@ public void AddPolicy(PipelinePolicy policy, PipelinePosition position) } } - // Set options on the message before sending it through the pipeline. - internal void Apply(PipelineMessage message) + /// + /// Apply the options provided in this + /// instance to the . + /// + /// The to apply the + /// options to. + protected internal virtual void Apply(PipelineMessage message) { Freeze(); diff --git a/sdk/core/System.ClientModel/src/Pipeline/HttpClientPipelineTransport.Request.cs b/sdk/core/System.ClientModel/src/Pipeline/HttpClientPipelineTransport.Request.cs index e0b5a8ea8997b..7239c987e30e0 100644 --- a/sdk/core/System.ClientModel/src/Pipeline/HttpClientPipelineTransport.Request.cs +++ b/sdk/core/System.ClientModel/src/Pipeline/HttpClientPipelineTransport.Request.cs @@ -103,7 +103,7 @@ internal static HttpRequestMessage BuildHttpRequestMessage(PipelineRequest reque } int i = 0; - while (headers.GetNextValue(i++, out string headerName, out object headerValue)) + while (headers.GetNextValue(i++, out string headerName, out object? headerValue)) { switch (headerValue) {