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

ClientModel: Updates to ClientModel public APIs from Azure.Core 2.0 integration #42328

Merged
merged 3 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 5 additions & 0 deletions sdk/core/System.ClientModel/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,12 @@ protected ClientResult(System.ClientModel.Primitives.PipelineResponse response)
public static System.ClientModel.ClientResult<T> FromValue<T>(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
annelo-msft marked this conversation as resolved.
Show resolved Hide resolved
{
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<System.ClientModel.ClientResultException> 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<T> : System.ClientModel.ClientResult
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,12 @@ protected ClientResult(System.ClientModel.Primitives.PipelineResponse response)
public static System.ClientModel.ClientResult<T> FromValue<T>(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<System.ClientModel.ClientResultException> 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<T> : System.ClientModel.ClientResult
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -13,8 +12,7 @@ namespace System.ClientModel;
/// <summary>
/// The exception that is thrown when the processing of a client request failed.
/// </summary>
[Serializable]
public class ClientResultException : Exception, ISerializable
public class ClientResultException : Exception
{
private const string DefaultMessage = "Service request failed.";

Expand Down Expand Up @@ -84,23 +82,6 @@ public ClientResultException(string message, PipelineResponse? response = defaul
_status = response?.Status ?? 0;
}

/// <inheritdoc />
protected ClientResultException(SerializationInfo info, StreamingContext context)
: base(info, context)
{
_status = info.GetInt32(nameof(Status));
}

/// <inheritdoc />
public override void GetObjectData(SerializationInfo info, StreamingContext context)
{
Argument.AssertNotNull(info, nameof(info));

info.AddValue(nameof(Status), Status);

base.GetObjectData(info, context);
}

/// <summary>
/// Gets the <see cref="PipelineResponse"/>, if any, that led to the exception.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ namespace System.ClientModel.Internal;
/// </summary>
internal struct ArrayBackedPropertyBag<TKey, TValue> where TKey : struct, IEquatable<TKey>
{
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
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand All @@ -196,26 +196,26 @@ 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;
}
}

[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;
Expand All @@ -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,
Expand All @@ -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++)
{
Expand Down Expand Up @@ -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")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -51,7 +51,7 @@ public override bool TryGetValues(string name, out IEnumerable<string>? values)
{
if (_headers.TryGetValue(new IgnoreCaseString(name), out object? value))
{
values = GetHeaderValueEnumerable(name, value);
values = GetHeaderValueEnumerable(name, value!);
return true;
}

Expand All @@ -63,16 +63,17 @@ public override IEnumerator<KeyValuePair<string, string>> 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;
Expand All @@ -83,8 +84,8 @@ private IEnumerable<KeyValuePair<string, string>> 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<string, string>(name, values);
}
}
Expand Down
14 changes: 8 additions & 6 deletions sdk/core/System.ClientModel/src/Message/PipelineMessage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -104,17 +104,19 @@ public CancellationToken CancellationToken

/// <summary>
/// Apply the options from the provided <see cref="RequestOptions"/> to
/// this <see cref="PipelineMessage"/> instance. This method is intended to
/// be called after the creation of the <see cref="PipelineMessage"/> and
/// its <see cref="Request"/> is complete, and prior to the call to
/// <see cref="ClientPipeline.Send(PipelineMessage)"/>.
/// this <see cref="PipelineMessage"/> instance.
/// </summary>
/// <param name="options">The <see cref="RequestOptions"/> to apply to this
/// <see cref="PipelineMessage"/> instance.</param>
/// <remarks> This method is intended to be called after the creation of
/// the <see cref="PipelineMessage"/> and its <see cref="Request"/> is
/// complete, and prior to the call to
/// <see cref="ClientPipeline.Send(PipelineMessage)"/>.
/// </remarks>
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);
}
Expand Down Expand Up @@ -144,7 +146,7 @@ public bool TryGetProperty(Type key, out object? value) =>
/// <param name="key">The key for the property in the message's property
/// bag.</param>
/// <param name="value">The value of the property.</param>
public void SetProperty(Type key, object value) =>
public void SetProperty(Type key, object? value) =>
_propertyBag.Set((ulong)key.TypeHandle.Value, value);

#endregion
Expand Down
9 changes: 7 additions & 2 deletions sdk/core/System.ClientModel/src/Options/RequestOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
/// <summary>
/// Apply the options provided in this <see cref="RequestOptions"/>
/// instance to the <paramref name="message"/>.
/// </summary>
/// <param name="message">The <see cref="PipelineMessage"/> to apply the
/// options to.</param>
protected internal virtual void Apply(PipelineMessage message)
{
Freeze();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down