diff --git a/css/customstyles.css b/css/customstyles.css index d4453cfa457..c8e8a0f3657 100644 --- a/css/customstyles.css +++ b/css/customstyles.css @@ -670,6 +670,11 @@ h4 { font-size: 120%; font-style: italic; } +h5 { + font-weight: bold; + font-size: 115%; +} + .alert, .callout { diff --git a/docs/dotnet/implementation.md b/docs/dotnet/implementation.md index e22da82d96a..a1c16fdc887 100644 --- a/docs/dotnet/implementation.md +++ b/docs/dotnet/implementation.md @@ -6,7 +6,83 @@ folder: dotnet sidebar: general_sidebar --- -## Parameter validation {#dotnet-parameters} +## API Implementation + +### The Service Client + +TODO: add a brief mention of the approach to implementing service clients. + +#### Service Methods + +TODO: Briefly introduce that service methods are implemented via an `HttpPipeline` instance. Mention that much of this is done for you using code generation. + +##### Using HttpPipeline {#dotnet-usage-httppipeline} + +The following example shows a typical way of using `HttpPipeline` to implement a service call method. The `HttpPipeline` will handle common HTTP requirements such as the user agent, logging, distributed tracing, retries, and proxy configuration. + +```csharp +public virtual async Task> AddAsync(ConfigurationSetting setting, CancellationToken cancellationToken = default) +{ + if (setting == null) throw new ArgumentNullException(nameof(setting)); + ... // validate other preconditions + + // Use HttpPipeline _pipeline filed of the client type to create new HTTP request + using (Request request = _pipeline.CreateRequest()) { + + // specify HTTP request line + request.Method = RequestMethod.Put; + request.Uri.Reset(_endpoint); + request.Uri.AppendPath(KvRoute, escape: false); + requast.Uri.AppendPath(key); + + // add headers + request.Headers.Add(IfNoneMatchWildcard); + request.Headers.Add(MediaTypeKeyValueApplicationHeader); + request.Headers.Add(HttpHeader.Common.JsonContentType); + request.Headers.Add(HttpHeader.Common.CreateContentLength(content.Length)); + + // add content + ReadOnlyMemory content = Serialize(setting); + request.Content = HttpPipelineRequestContent.Create(content); + + // send the request + var response = await Pipeline.SendRequestAsync(request).ConfigureAwait(false); + + if (response.Status == 200) { + // deserialize content + Response result = await CreateResponse(response, cancellationToken); + } + else + { + throw await response.CreateRequestFailedExceptionAsync(message); + } + } +} +``` + +TODO: do we still want this code sample now that we're encouraging moving to Code Gen? + +For a more complete example, see the [configuration client](https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/appconfiguration/Azure.Data.AppConfiguration/src/ConfigurationClient.cs) implementation. + +##### Using HttpPipelinePolicy + +The HTTP pipeline includes a number of policies that all requests pass through. Examples of policies include setting required headers, authentication, generating a request ID, and implementing proxy authentication. `HttpPipelinePolicy` is the base type of all policies (plugins) of the `HttpPipeline`. This section describes guidelines for designing custom policies. + +{% include requirement/MUST id="dotnet-http-pipeline-policy-inherit" %} inherit from `HttpPipelinePolicy` if the policy implementation calls asynchronous APIs. + +See an example [here](https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/core/Azure.Core/src/Pipeline/BearerTokenAuthenticationPolicy.cs). + +{% include requirement/MUST id="dotnet-sync-http-pipeline-policy-inherit" %} inherit from `HttpPipelineSynchronousPolicy` if the policy implementation calls only synchronous APIs. + +See an example [here](https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/core/Azure.Core/src/Pipeline/Internal/ClientRequestIdPolicy.cs). + +{% include requirement/MUST id="dotnet-http-pipeline-thread-safety" %} ensure `ProcessAsync` and `Process` methods are thread safe. + +`HttpMessage`, `Request`, and `Response` don't have to be thread-safe. + +#### Service Method Parameters + +##### Parameter validation {#dotnet-parameters} In addition to [general parameter validation guidelines](introduction.md#dotnet-parameters): @@ -25,97 +101,62 @@ Just add the following to your project to include it: See remarks on the `Argument` class for more detail. -## EventSource - -{% include requirement/MUST id="dotnet-tracing-eventsource" %} use `EventSource` to produce diagnostic events. - -{% include requirement/MUST id="dotnet-tracing-eventsource-logging-guidelines" %} follow the logging guidelines when implementing an `EventSource`. - -{% include requirement/MUST id="dotnet-tracing-eventsource-single" %} have a single `EventSource` type per library. - -{% include requirement/MUST id="dotnet-tracing-eventsource-internal" %} define `EventSource` class as `internal sealed`. - -{% include requirement/MUST id="dotnet-tracing-eventsource-singleton" %} define and use a singleton instance of `EventSource`: - -{% include requirement/MUST id="dotnet-tracing-eventsource-traits" %} define `AzureEventSource` trait with value `true` to make the `EventSource` discoverable by listeners (you can use `AzureEventSourceListener.TraitName` `AzureEventSourceListener.TraitValue` constants): +### Supporting Types -{% include requirement/MUST id="dotnet-tracing-eventsource-name" %} set `EventSource` name to package name replacing `.` with `-` (i.e. . `Azure-Core` for `Azure.Core` package) +#### Serialization {#dotnet-usage-json} -{% include requirement/MUST id="dotnet-tracing-eventsource-event-message" %} have `Message` property of EventAttribute set for all events. +##### JSON Serialization -{% include requirement/MUST id="dotnet-tracing-eventsource-public-api" %} treat `EventSource` name, guid, event id and parameters as public API and follow the appropriate versioning rules. +{% include requirement/MUST id="dotnet-json-use-system-text-json" %} use [`System.Text.Json`](https://www.nuget.org/packages/System.Text.Json) package to write and read JSON content. -{% include requirement/SHOULD id="dotnet-tracing-eventsource-is-enabled" %} check IsEnabled property before doing expensive work (formatting parameters, calling ToString, allocations etc.) +{% include requirement/SHOULD id="dotnet-json-use-utf8jsonwriter" %} use `Utf8JsonWriter` to write JSON payloads: -{% include requirement/MUSTNOT id="dotnet-tracing-eventsource-event-param-exception" %} define events with `Exception` parameters as they are not supported by `EventSource`. +```csharp +var json = new Utf8JsonWriter(writer); +json.WriteStartObject(); +json.WriteString("value", setting.Value); +json.WriteString("content_type", setting.ContentType); +json.WriteEndObject(); +json.Flush(); +written = (int)json.BytesWritten; +``` -{% include requirement/MUST id="dotnet-tracing-eventsource-test" %} have a test that asserts `EventSource` name, guid and generates the manifest to verify that event source is correctly defined. +{% include requirement/SHOULD id="dotnet-json-use-jsondocument" %} use `JsonDocument` to read JSON payloads: -{% include requirement/MUST id="dotnet-tracing-eventsource-test-events" %} test that expected events are produced when appropriate. `TestEventListener` class can be used to collect events. Make sure you mark the test as `[NonParallelizable]`. +```csharp +using (JsonDocument json = await JsonDocument.ParseAsync(content, default, cancellationToken).ConfigureAwait(false)) +{ + JsonElement root = json.RootElement; -{% include requirement/SHOULD id="dotnet-tracing-eventsource-thrown-exceptions" %} avoid logging exceptions that are going to get thrown to user code anyway. + var setting = new ConfigurationSetting(); -{% include requirement/SHOULD id="dotnet-tracing-eventsource-event-size-limit" %} be aware of event size limit of 64K. + // required property + setting.Key = root.GetProperty("key").GetString(); -``` -[Test] -public void MatchesNameAndGuid() -{ - // Arrange & Act - var eventSourceType = typeof(AzureCoreEventSource); + // optional property + if (root.TryGetProperty("last_modified", out var lastModified)) { + if(lastModified.Type == JsonValueType.Null) { + setting.LastModified = null; + } + else { + setting.LastModified = DateTimeOffset.Parse(lastModified.GetString()); + } + } + ... - // Assert - Assert.NotNull(eventSourceType); - Assert.AreEqual("Azure-Core", EventSource.GetName(eventSourceType)); - Assert.AreEqual(Guid.Parse("1015ab6c-4cd8-53d6-aec3-9b937011fa95"), EventSource.GetGuid(eventSourceType)); - Assert.IsNotEmpty(EventSource.GenerateManifest(eventSourceType, "assemblyPathToIncludeInManifest")); + return setting; } ``` -Sample `EventSource` declaration: +{% include requirement/SHOULD id="dotnet-json-use-utf8jsonreader" %} consider using `Utf8JsonReader` to read JSON payloads. -``` C# +`Utf8JsonReader` is faster than `JsonDocument` but much less convenient to use. -[EventSource(Name = EventSourceName)] -internal sealed class AzureCoreEventSource : EventSource -{ - private const string EventSourceName = "Azure-Core"; - - // Having event ids defined as const makes it easy to keep track of them - private const int MessageSentEventId = 0; - private const int ClientClosingEventId = 1; - - public static AzureCoreEventSource Shared { get; } = new AzureCoreEventSource(); - - private AzureCoreEventSource() : base(EventSourceName, EventSourceSettings.Default, AzureEventSourceListener.TraitName, AzureEventSourceListener.TraitValue) { } - - [NonEvent] - public void MessageSent(Guid clientId, string messageBody) - { - // We are calling Guid.ToString make sure anyone is listening before spending resources - if (IsEnabled(EventLevel.Informational, EventKeywords.None)) - { - MessageSent(clientId.ToString("N"), messageBody); - } - } - - // In this example we don't do any expensive parameter formatting so we can avoid extra method and IsEnabled check - - [Event(ClientClosingEventId, Level = EventLevel.Informational, Message = "Client {0} is closing the connection.")] - public void ClientClosing(string clientId) - { - WriteEvent(ClientClosingEventId, clientId); - } - - [Event(MessageSentEventId, Level = EventLevel.Informational, Message = "Client {0} sent message with body '{1}'")] - private void MessageSent(string clientId, string messageBody) - { - WriteEvent(MessageSentEventId, clientId, messageBody); - } -} -``` +{% include requirement/MUST id="dotnet-json-serialization-resilience" %} make your serialization and deserialization code version resilient. -## Enumeration-like structures {#dotnet-enums} +Optional JSON properties should be deserialized into nullable model properties. + +#### Enumeration-like structures {#dotnet-enums} As described in [general enumeration guidelines](introduction.md#dotnet-enums), you should use `enum` types whenever passing or deserializing a well-known set of values to or from the service. There may be times, however, where a `readonly struct` is necessary to capture an extensible value from the service even if well-known values are defined, @@ -221,7 +262,7 @@ public partial readonly struct EncryptionAlgorithm : IEquatable {#dotnet-implement-operation} + +Subtypes of `Operation` are returned from service client methods invoking long running operations. + +{% include requirement/MUST id="dotnet-lro-return" %} check the value of `HasCompleted` in subclass implementations of `UpdateStatus` and `UpdateStatusAsync` and immediately return the result of `GetRawResponse` if it is true. + +```csharp +public override Response UpdateStatus( + CancellationToken cancellationToken = default) => + UpdateStatusAsync(false, cancellationToken).EnsureCompleted(); + +public override async ValueTask UpdateStatusAsync( + CancellationToken cancellationToken = default) => + await UpdateStatusAsync(true, cancellationToken).ConfigureAwait(false); + +private async Task UpdateStatusAsync(bool async, CancellationToken cancellationToken) +{ + // Short-circuit when already completed (which improves mocking scenarios that won't have a client). + if (HasCompleted) + { + return GetRawResponse(); + } + + // some service operation call here, which the above check avoids if the operation has already completed. + ... +``` + +{% include requirement/MUST id="dotnet-lro-return" %} throw from methods on `Operation` subclasses in the following scenarios. + +- If an underlying service operation call from `UpdateStatus`, `WaitForCompletion`, or `WaitForCompletionAsync` throws, re-throw `RequestFailedException` or its subtype. +- If the operation completes with a non-success result, throw `RequestFailedException` or its subtype from `UpdateStatus`, `WaitForCompletion`, or `WaitForCompletionAsync`. + - Include any relevant error state information in the exception details. + +```csharp +private async ValueTask UpdateStatusAsync(bool async, CancellationToken cancellationToken) +{ + ... + + try + { + Response update = async + ? await _serviceClient.GetExampleResultAsync(new Guid(Id), cancellationToken).ConfigureAwait(false) + : _serviceClient.GetExampleResult(new Guid(Id), cancellationToken); + + _response = update.GetRawResponse(); + + if (update.Value.Status == OperationStatus.Succeeded) + { + // we need to first assign a value and then mark the operation as completed to avoid race conditions + _value = ConvertValue(update.Value.ExampleResult.PageResults, update.Value.ExampleResult.ReadResults); + _hasCompleted = true; + } + else if (update.Value.Status == OperationStatus.Failed) + { + _requestFailedException = await ClientCommon + .CreateExceptionForFailedOperationAsync(async, _diagnostics, _response, update.Value.AnalyzeResult.Errors) + .ConfigureAwait(false); + _hasCompleted = true; + + // the operation didn't throw, but it completed with a non-success result. + throw _requestFailedException; + } + } + catch (Exception e) + { + scope.Failed(e); + throw; + } + } + + return GetRawResponse(); +} +``` + +- If the ```Value``` property is evaluated after the operation failed (```HasValue``` is false and ```HasCompleted``` is true) throw the same exception as the one thrown when the operation failed. + +```csharp +public override FooResult Value +{ + get + { + if (HasCompleted && !HasValue) +#pragma warning disable CA1065 // Do not raise exceptions in unexpected locations + + // throw the exception captured when the operation failed. + throw _requestFailedException; + +#pragma warning restore CA1065 // Do not raise exceptions in unexpected locations + else + return OperationHelpers.GetValue(ref _value); + } +} +``` + +- If the ```Value``` property is evaluated before the operation is complete (```HasCompleted``` is false) throw ```InvalidOperationException```. + - The exception message should be: "The operation has not yet completed." + +```csharp +// start the operation. +ExampleOperation operation = await client.StartExampleOperationAsync(someParameter); + +if (!operation.HasCompleted) +{ + // attempt to get the Value property before the operation has completed. + ExampleOperationResult myResult = operation.Value; //throws InvalidOperationException. +} +else if (!operation.HasValue) +{ + // attempt to get the Value property after the operation has completed unsuccessfully. + ExampleOperationResult myResult = operation.Value; //throws RequestFailedException. +} +``` + +## SDK Feature Implementation + +### Configuration + +TODO: Add discussion on configuration environment variables to parallel that of other languages + +### Logging + +Request logging will be done automatically by the `HttpPipeline`. If a client library needs to add custom logging, follow the [same guidelines](implementation.md#general-logging) and mechanisms as the pipeline logging mechanism. If a client library wants to do custom logging, the designer of the library must ensure that the logging mechanism is pluggable in the same way as the `HttpPipeline` logging policy. + +{% include requirement/MUST id="dotnet-logging-follow-guidelines" %} follow [the logging section of the Azure SDK General Guidelines](implementation.md#general-logging) if logging directly (as opposed to through the `HttpPipeline`). + +#### EventSource + +{% include requirement/MUST id="dotnet-tracing-eventsource" %} use `EventSource` to produce diagnostic events. + +{% include requirement/MUST id="dotnet-tracing-eventsource-logging-guidelines" %} follow the logging guidelines when implementing an `EventSource`. + +{% include requirement/MUST id="dotnet-tracing-eventsource-single" %} have a single `EventSource` type per library. + +{% include requirement/MUST id="dotnet-tracing-eventsource-internal" %} define `EventSource` class as `internal sealed`. + +{% include requirement/MUST id="dotnet-tracing-eventsource-singleton" %} define and use a singleton instance of `EventSource`: + +{% include requirement/MUST id="dotnet-tracing-eventsource-traits" %} define `AzureEventSource` trait with value `true` to make the `EventSource` discoverable by listeners (you can use `AzureEventSourceListener.TraitName` `AzureEventSourceListener.TraitValue` constants): + +{% include requirement/MUST id="dotnet-tracing-eventsource-name" %} set `EventSource` name to package name replacing `.` with `-` (i.e. . `Azure-Core` for `Azure.Core` package) + +{% include requirement/MUST id="dotnet-tracing-eventsource-event-message" %} have `Message` property of EventAttribute set for all events. + +{% include requirement/MUST id="dotnet-tracing-eventsource-public-api" %} treat `EventSource` name, guid, event id and parameters as public API and follow the appropriate versioning rules. + +{% include requirement/SHOULD id="dotnet-tracing-eventsource-is-enabled" %} check IsEnabled property before doing expensive work (formatting parameters, calling ToString, allocations etc.) + +{% include requirement/MUSTNOT id="dotnet-tracing-eventsource-event-param-exception" %} define events with `Exception` parameters as they are not supported by `EventSource`. + +{% include requirement/MUST id="dotnet-tracing-eventsource-test" %} have a test that asserts `EventSource` name, guid and generates the manifest to verify that event source is correctly defined. + +{% include requirement/MUST id="dotnet-tracing-eventsource-test-events" %} test that expected events are produced when appropriate. `TestEventListener` class can be used to collect events. Make sure you mark the test as `[NonParallelizable]`. + +{% include requirement/SHOULD id="dotnet-tracing-eventsource-thrown-exceptions" %} avoid logging exceptions that are going to get thrown to user code anyway. + +{% include requirement/SHOULD id="dotnet-tracing-eventsource-event-size-limit" %} be aware of event size limit of 64K. + +``` +[Test] +public void MatchesNameAndGuid() +{ + // Arrange & Act + var eventSourceType = typeof(AzureCoreEventSource); + + // Assert + Assert.NotNull(eventSourceType); + Assert.AreEqual("Azure-Core", EventSource.GetName(eventSourceType)); + Assert.AreEqual(Guid.Parse("1015ab6c-4cd8-53d6-aec3-9b937011fa95"), EventSource.GetGuid(eventSourceType)); + Assert.IsNotEmpty(EventSource.GenerateManifest(eventSourceType, "assemblyPathToIncludeInManifest")); +} +``` + +Sample `EventSource` declaration: + +``` C# + +[EventSource(Name = EventSourceName)] +internal sealed class AzureCoreEventSource : EventSource +{ + private const string EventSourceName = "Azure-Core"; + + // Having event ids defined as const makes it easy to keep track of them + private const int MessageSentEventId = 0; + private const int ClientClosingEventId = 1; + + public static AzureCoreEventSource Shared { get; } = new AzureCoreEventSource(); + + private AzureCoreEventSource() : base(EventSourceName, EventSourceSettings.Default, AzureEventSourceListener.TraitName, AzureEventSourceListener.TraitValue) { } + + [NonEvent] + public void MessageSent(Guid clientId, string messageBody) + { + // We are calling Guid.ToString make sure anyone is listening before spending resources + if (IsEnabled(EventLevel.Informational, EventKeywords.None)) + { + MessageSent(clientId.ToString("N"), messageBody); + } + } + + // In this example we don't do any expensive parameter formatting so we can avoid extra method and IsEnabled check + + [Event(ClientClosingEventId, Level = EventLevel.Informational, Message = "Client {0} is closing the connection.")] + public void ClientClosing(string clientId) + { + WriteEvent(ClientClosingEventId, clientId); + } + + [Event(MessageSentEventId, Level = EventLevel.Informational, Message = "Client {0} sent message with body '{1}'")] + private void MessageSent(string clientId, string messageBody) + { + WriteEvent(MessageSentEventId, clientId, messageBody); + } +} +``` + +### Distributed Tracing {#dotnet-distributedtracing} + +{% include draft.html content="Guidance coming soon ..." %} + +TODO: Add guidance for distributed tracing implementation + +### Telemetry + +TODO: Add guidance regarding user agent strings + +## Ecosystem Integration + +### Integration with ASP.NET Core All Azure client libraries ship with a set of extension methods that provide integration with ASP.NET Core applications by registering clients with DependencyInjection container, flowing Azure SDK logs to ASP.NET Core logging subsystem and providing ability to use configuration subsystem for client configuration (for more examples see https://github.com/Azure/azure-sdk-for-net/tree/master/sdk/core/Microsoft.Extensions.Azure) @@ -285,7 +555,6 @@ public static IAzureClientBuilder. { @@ -109,6 +113,7 @@ namespace Azure.Data.Configuration { public ConfigurationClient(string connectionString); public ConfigurationClient(string connectionString, ConfigurationClientOptions options); + protected ConfigurationClient(); // for mocking public virtual Task> GetConfigurationSettingAsync(string key, CancellationToken cancellationToken = default); @@ -119,7 +124,7 @@ namespace Azure.Data.Configuration { } // options for configuring the client - public class ConfigurationClientOptions : HttpPipelineOptions { + public class ConfigurationClientOptions : ClientOptions { ... } } @@ -137,7 +142,9 @@ For example, the service client for the Application Configuration service is cal {% include requirement/MUST id="dotnet-client-namespace" %} see [Namespace Naming](#dotnet-namespace-naming) guidelines for how to choose the namespace for the client types. -### Service Client Constructors {#dotnet-client-ctor} +TODO: Mention client immutability here? + +#### Service Client Constructors {#dotnet-client-ctor} {% include requirement/MUST id="dotnet-client-constructor-minimal" %} provide a minimal constructor that takes only the parameters required to connect to the service. @@ -153,7 +160,30 @@ public class ConfigurationClient { {% include requirement/MUST id="dotnet-client-constructor-overloads" %} provide constructor overloads that allow specifying additional options such as credentials, a custom HTTP pipeline, or advanced configuration. -Custom pipeline and client-specific configuration are represented by an `options` parameter. The type of the parameter is typically a subclass of ```ClientOptions``` type. Guidelines for using `ClientOptions` can be found in [Using ClientOptions](#dotnet-usage-options) section below. +Custom pipeline and client-specific configuration are represented by an `options` parameter. The type of the parameter is typically a subclass of ```ClientOptions``` type, shown below. + +##### Using ClientOptions {#dotnet-usage-options} + +{% include requirement/MUST id="dotnet-http-pipeline-options" %} name subclasses of ```ClientOptions``` by adding _Options_ suffix to the name of the client type the options subclass is configuring. + +```csharp +// options for configuring ConfigurationClient +public class ConfigurationClientOptions : ClientOptions { + ... +} + +public class ConfigurationClient { + + public ConfigurationClient(string connectionString, ConfigurationClientOptions options); + ... +} +``` + +If the options type can be shared by multiple client types, name it with a plural or more general name. For example, the `BlobClientsOptions` class can be used by `BlobClient`, `BlobContainerClient`, and `BlobAccountClient`. + +{% include requirement/MUSTNOT id="dotnet-options-no-default-constructor" %} have a default constructor on the options type. + +Each overload constructor should take at least `version` parameter to specify the service version. See [Versioning](#dotnet-service-version-option) guidelines for details. For example, the `ConfigurationClient` type and its public constructors look as follows: @@ -165,6 +195,57 @@ public class ConfigurationClient { } ``` +##### Service Versions + +{% include requirement/MUST id="dotnet-versioning-highest-api" %} call the highest supported service API version by default. + +{% include requirement/MUST id="dotnet-versioning-select-api-version" %} allow the consumer to explicitly select a supported service API version when instantiating the service client. + +Use a constructor parameter called `version` on the client options type. + +* The `version` parameter must be the first parameter to all constructor overloads. +* The `version` parameter must be required, and default to the latest supported service version. +* The type of the `version` parameter must be `ServiceVersion`; an enum nested in the options type. +* The `ServiceVersion` enum must use explicit values starting from 1. +* `ServiceVersion` enum value 0 is reserved. When 0 is passed into APIs, ArgumentException should be thrown. + +For example, the following is a code snippet from the `ConfigurationClientOptions`: + +```csharp +public class ConfigurationClientOptions : ClientOptions { + + public ConfigurationClientOptions(ServiceVersion version = ServiceVersion.V2019_05_09) { + if (version == default) + throw new ArgumentException($"The service version {version} is not supported by this library."); + } + } + + public enum ServiceVersion { + V2019_05_09 = 1, + } + ... +} +``` + +{% include note.html content="Third-party reusable libraries shouldn't change behavior without an explicit decision by the developer. When developing libraries that are based on the Azure SDK, lock the library to a specific service version to avoid changes in behavior." %} + +{% include requirement/MUSTNOT id="dotnet-versioning-feature-flags" %} force consumers to test service API versions to check support for a feature. Use the _tester-doer_ .NET pattern to implement feature flags, or use `Nullable`. + +For example, if the client library supports two service versions, only one of which can return batches, the consumer might write the following code: + +```csharp +if (client.CanBatch) { + Response response = await client.GetBatch("some_key*"); + Guid? Guid = response.Result.Guid; +} else { + Response response1 = await client.GetAsync("some_key1"); + Response response2 = await client.GetAsync("some_key2"); + Response response3 = await client.GetAsync("some_key3"); +} +``` + +##### Mocking + {% include requirement/MUST id="dotnet-client-constructor-for-mocking" %} provide protected parameterless constructor for mocking. ```csharp @@ -189,9 +270,11 @@ public class ConfigurationClient { In mocks, using the virtual property instead of the parameter requires the property to be mocked to return the value before the constructor is called when the mock is created. In [Moq] this requires using the delegate parameter to create the mock, which may not be an obvious workaround. -See [Supporting Mocking](#dotnet-mocking) for details. +See [Support for Mocking](#dotnet-mocking) for details. + +#### Service Methods {#dotnet-client-methods} -### Service Methods {#dotnet-client-methods} + _Service methods_ are the methods on the client that invoke operations on the service. Here are the main service methods in the `ConfigurationClient`. They meet all the guidelines that are discussed below. @@ -212,35 +295,43 @@ public class ConfigurationClient { } ``` +##### Sync and Async + {% include requirement/MUST id="dotnet-service-methods-sync-and-async" %} provide both asynchronous and synchronous variants for all service methods. -Many developers want to port existing application to the Cloud. These application are often synchronous, and the cost of rewriting them to be asynchronous is usually prohibitive. Calling asynchronous APIs from synchronous methods can only be done through a technique called [_sync-over-async_, which can cause deadlocks](https://devblogs.microsoft.com/pfxteam/should-i-expose-synchronous-wrappers-for-asynchronous-methods/). Azure SDK is providing synchronous APIs to minimize friction when porting existing application to Azure. +Many developers want to port existing applications to the Cloud. These applications are often synchronous, and the cost of rewriting them to be asynchronous is usually prohibitive. Calling asynchronous APIs from synchronous methods can only be done through a technique called [_sync-over-async_, which can cause deadlocks](https://devblogs.microsoft.com/pfxteam/should-i-expose-synchronous-wrappers-for-asynchronous-methods/). The Azure SDK provides synchronous APIs to minimize friction when porting existing application to Azure. {% include requirement/MUST id="dotnet-service-methods-naming" %} ensure that the names of the asynchronous and the synchronous variants differ only by the _Async_ suffix. +##### Naming + +TODO: Add discussion to parallel method naming in other sections + +##### Cancellation + {% include requirement/MUST id="dotnet-service-methods-cancellation" %} ensure all service methods, both asynchronous and synchronous, take an optional `CancellationToken` parameter called _cancellationToken_. The token should be further passed to all calls that take a cancellation token. DO NOT check the token manually, except when running a significant amount of CPU-bound work within the library, e.g. a loop that can take more than a typical network call. +##### Mocking + {% include requirement/MUST id="dotnet-service-methods-virtual" %} make service methods virtual. -Virtual methods are used to support mocking. See [Supporting Mocking](#dotnet-mocking) for details. +Virtual methods are used to support mocking. See [Support for Mocking](#dotnet-mocking) for details. + +##### Return Types {% include requirement/MUST id="dotnet-service-methods-response-sync" %} return `Response` or `Response` from synchronous methods. -`T` represents the content of the response. For more information, see [Service Method Return Types](#dotnet-method-return). +`T` represents the content of the response, as described below. {% include requirement/MUST id="dotnet-service-methods-response-async" %} return `Task>` or `Task` from asynchronous methods that make network requests. There are two possible return types from asynchronous methods: `Task` and `ValueTask`. Your code will be doing a network request in the majority of cases. The `Task` variant is more appropriate for this use case. For more information, see [this blog post](https://devblogs.microsoft.com/dotnet/understanding-the-whys-whats-and-whens-of-valuetask/#user-content-should-every-new-asynchronous-api-return-valuetask--valuetasktresult). -`T` represents the content of the response. For more information, see [Service Method Return Types](#dotnet-method-return). +`T` represents the content of the response, as described below. -{% include requirement/MUST id="dotnet-service-methods-thread-safety" %} be thread-safe. All public members of the client type must be safe to call from multiple threads concurrently. - -### Service Method Return Types {#dotnet-method-return} - -As mentioned above, service methods will often return `Response`. The `T` can be either an unstructured payload (e.g. bytes of a storage blob) or a _model type_ representing deserialized response content. This section describes guidelines for the design of unstructured return types, _model types_, and all their transitive closure of public dependencies (i.e. the _model graph_). +The `T` can be either an unstructured payload (e.g. bytes of a storage blob) or a _model type_ representing deserialized response content. {% include requirement/MUST id="dotnet-service-return-unstructured-type" %} use one of the following return types to represent an unstructured payload: @@ -250,144 +341,25 @@ As mentioned above, service methods will often return `Response`. The `T` can {% include requirement/MUST id="dotnet-service-return-model-type" %} return a _model type_ if the content has a schema and can be deserialized. -For example, review the configuration service _model type_ below: - -```csharp -public sealed class ConfigurationSetting : IEquatable { - - public ConfigurationSetting(string key, string value, string label = default); - - public string ContentType { get; set; } - public string ETag { get; internal set; } - public string Key { get; set; } - public string Label { get; set; } - public DateTimeOffset LastModified { get; internal set; } - public bool Locked { get; internal set; } - public IDictionary Tags { get; } - public string Value { get; set; } - - public bool Equals(ConfigurationSetting other); - - [EditorBrowsable(EditorBrowsableState.Never)] - public override bool Equals(object obj); - [EditorBrowsable(EditorBrowsableState.Never)] - public override int GetHashCode(); - [EditorBrowsable(EditorBrowsableState.Never)] - public override string ToString(); -} -``` - -This model is returned from service methods as follows: - -```csharp -public class ConfigurationClient { - public virtual Task> GetAsync(...); - public virtual Response Get(...); - ... -} -``` - -{% include requirement/MUST id="dotnet-service-return-model-public-getters" %} ensure model public properties are get-only if they aren't intended to be changed by the user. - -Most output-only models can be fully read-only. Models that are used as both outputs and inputs (i.e. received from and sent to the service) typically have a mixture of read-only and read-write properties. - -For example, the `Locked` property of `ConfigurationSetting` is controlled by the service. It shouldn't be changed by the user. The `ContentType` property, by contrast, can be modified by the user. - -```csharp -public sealed class ConfigurationSetting : IEquatable { - - public string ContentType { get; set; } - - public bool Locked { get; internal set; } -} -``` - -Ensure you include an internal setter to allow for deserialization. For more information, see [JSON Serialization](#dotnet-usage-json). - -{% include requirement/MUST id="dotnet-service-models-prefer-structs" %} ensure model types are structs, if they meet the criteria for being structs. - -Good candidates for struct are types that are small and immutable, especially if they are often stored in arrays. See [.NET Framework Design Guidelines](https://docs.microsoft.com/dotnet/standard/design-guidelines/choosing-between-class-and-struct) for details. - -{% include requirement/SHOULD id="dotnet-service-models-basic-data-interfaces" %} implement basic data type interfaces on model types, per .NET Framework Design Guidelines. - -For example, implement `IEquatable`, `IComparable`, `IEnumerable`, etc. if applicable. - -{% include requirement/SHOULD id="dotnet-service-return-model-collections" %} use the following collection types for properties of model types: -- ```IReadOnlyList``` and ```IList``` for most collections -- ```IReadOnlyDictionary``` and ```IDictionary``` for lookup tables -- ```T[]```, ```Memory```, and ```ReadOnlyMemory``` when low allocations and perfromance are critical - -Note that this guidance does not apply to input parameters. Input parameters representing collections should follow standard [.NET Design Guidelines](https://docs.microsoft.com/dotnet/standard/design-guidelines/parameter-design), e.g. use ```IEnumerable``` is allowed. -Also, this guidance does not apply to return types of service method calls. These should be using ```Pageable``` and ```AsyncPageable``` discussed in [Service Method Return Types](#dotnet-method-return). - -{% include requirement/MAY id="dotnet-service-models-namespace" %} place output model types in _.Models_ subnamespace to avoid cluttering the main namespace with too many types. - -It is important for the main namespace of a client library to be clutter free. Some client libraries have a relatively small number of model types, and these should keep the model types in the main namespace. For example, model types of `Azure.Data.AppConfiguration` package are in the main namespace. On the other hand, model types of `Azure.Storage.Blobs` package are in _.Models_ subnamespace. - -```csharp -namespace Azure.Storage.Blobs { - public class BlobClient { ... } - public class BlobClientOptions { ... } - ... -} -namespace Azure.Storage.Blobs.Models { - ... - public class BlobContainerItem { ... } - public class BlobContainerProperties { ...} - ... -} -``` -{% include requirement/SHOULD id="dotnet-service-editor-browsable-state" %} apply the `[EditorBrowsable(EditorBrowsableState.Never)]` attribute to methods that the user isn't meant to call. - -Adding this attribute will hide the methods from being shown with IntelliSense. A user will almost never call `GetHashCode()` directly. `Equals(object)` is almost never called if the type implements `IEquatable` (which is preferred). Hide the `ToString()` method if it isn't overridden. - -```csharp -public sealed class ConfigurationSetting : IEquatable { - [EditorBrowsable(EditorBrowsableState.Never)] - public override bool Equals(object obj); - [EditorBrowsable(EditorBrowsableState.Never)] - public override int GetHashCode(); -} -``` - -{% include note.html content="Unlike client APIs, model type APIs aren't required to be thread-safe, as they're rarely shared between threads." %} - -{% include requirement/MUST id="dotnet-models-in-mocks" %} ensure all model types can be used in mocks. - -In practice, you need to provide public APIs to construct _model graphs_. See [Supporting Mocking](#dotnet-mocking) for details. - -### Returning Collections {#dotnet-paging} - -Many Azure REST APIs return collections of data in batches or pages. A client library will expose such APIs as special enumerable types ```Pageable``` or ```AsyncPageable```. -These types are located in the ```Azure.Core``` package. +For more information, see [Model Types](#dotnet-model-types) -For example, the configuration service returns collections of items as follows: - -```csharp -public class ConfigurationClient { - - // asynchronous API returning a collection of items - public virtual AsyncPageable> GetConfigurationSettingsAsync(...); - - // synchronous variant of the method above - public virtual Pageable GetConfigurationSettings(...); - ... -} -``` +##### Thread Safety -{% include requirement/MUST id="dotnet-pagination-ienumerable" %} return ```Pageable``` or ```AsyncPageable``` from service methods that return a collection of items. +{% include requirement/MUST id="dotnet-service-methods-thread-safety" %} be thread-safe. All public members of the client type must be safe to call from multiple threads concurrently. -### Service Method Parameters {#dotnet-parameters} +#### Service Method Parameters {#dotnet-parameters} Service methods fall into two main groups when it comes to the number and complexity of parameters they accept: - Service Methods with Simple Inputs, _simple methods_ for short -- Service Methods with Complex Inputs, _complext methods_ for short +- Service Methods with Complex Inputs, _complex methods_ for short _Simple methods_ are methods that take up to six parameters, with most of the parameters being simple BCL primitives. _Complex methods_ are methods that take large number of parameters and typically correspond to REST APIs with complex request payloads. _Simple methods_ should follow standard .NET Design Guidelines for parameter list and overload design. +TODO: Link to these guidelines in FDG3 + _Complex methods_ should use _option parameter_ to represent the request payload, and consider providing convenience simple overloads for most common scenarios. ```csharp @@ -419,7 +391,10 @@ public class BlobCreateOptions { If in common scenarios, users are likely to pass just a small subset of what the _options_ parameter represents, consider adding an overload with a parameter list representing just this subset. -#### Parameter Validation +TODO: There was a discussion here and the guidance was updated within the .NET team and across Java as well. Update these to reflect those decisions? +TODO: Add "Do name the options parameter type name with the _Options_ suffix ... ? + +##### Parameter Validation Service methods take two kinds of parameters: _service parameters_ and _client parameters_. _Service parameters_ are directly passed across the wire to the service. _Client parameters_ are used within the client library and aren't passed directly to the service. @@ -431,11 +406,32 @@ Common parameter validations include null checks, empty string checks, and range {% include requirement/MUST id="dotnet-params-test-devex" %} test the developer experience when invalid service parameters are passed in. Ensure clear error messages are generated by the client. If the developer experience is inadequate, work with the service team to correct the problem. -### Long Running Operations {#dotnet-longrunning} +#### Methods Returning Collections (Paging) {#dotnet-paging} + +Many Azure REST APIs return collections of data in batches or pages. A client library will expose such APIs as special enumerable types ```Pageable``` or ```AsyncPageable```. +These types are located in the ```Azure.Core``` package. + +For example, the configuration service returns collections of items as follows: + +```csharp +public class ConfigurationClient { + + // asynchronous API returning a collection of items + public virtual AsyncPageable> GetConfigurationSettingsAsync(...); + + // synchronous variant of the method above + public virtual Pageable GetConfigurationSettings(...); + ... +} +``` + +{% include requirement/MUST id="dotnet-pagination-ienumerable" %} return ```Pageable``` or ```AsyncPageable``` from service methods that return a collection of items. + +#### Methods Invoking Long Running Operations {#dotnet-longrunning} Some service operations, known as _Long Running Operations_ or _LROs_ take a long time (up to hours or days). Such operations do not return their result immediately, but rather are started, their progress is polled, and finally the result of the operation is retrieved. -Azure.Core library exposes an abstract type called ```Operation```, which represents such LROs and supports operations for polling and waiting for status changes, and retrieving the final operation result. +Azure.Core library exposes an abstract type called ```Operation```, which represents such LROs and supports operations for polling and waiting for status changes, and retrieving the final operation result. A service method invoking a long running operation will return a subclass of `Operation`, as shown below. ```csharp // the following type is located in Azure.Core @@ -456,6 +452,7 @@ public abstract class Operation { public abstract ValueTask> WaitForCompletionAsync(TimeSpan pollingInterval, CancellationToken cancellationToken); } ``` + Client libraries need to inherit from ```Operation``` not only to implement all abstract members, but also to provide a constructor required to access an existing LRO (an LRO initiated by a different process). ```csharp @@ -471,7 +468,7 @@ public class BlobBaseClient { } ``` -The `Operation` object can be used to poll for a response. +The following code snippet shows how an SDK consumer would use the `Operation` to poll for a response. ```csharp BlobBaseClient client = ... @@ -512,61 +509,191 @@ BlobBaseClient client = ... {% include requirement/MAY id="dotnet-lro-subclass" %} add additional APIs to subclasses of ```Operation```. For example, some subclasses add a constructor allowing to create an operation instance from a previously saved operation ID. Also, some subclasses are more granular states besides the IsCompleted and HasValue states that are present on the base class. -Guidelines for implementing `Operation` can be found in [Implementing Subtypes of Operation](#dotnet-implement-operation) section below. +##### Conditional Request Methods -### Supporting Mocking {#dotnet-mocking} +TODO: Add discussion for this -All client libraries must support mocking. Here is an example of how the `ConfigurationClient` can be mocked using [Moq] (a popular .NET mocking library): - -```csharp -// Create a mock response -var mockResponse = new Mock(); -// Create a client mock -var mock = new Mock(); -// Setup client method -mock.Setup(c => c.Get("Key", It.IsAny(), It.IsAny(), It.IsAny())) - .Returns(new Response(mockResponse.Object, ConfigurationModelFactory.ConfigurationSetting("Key", "Value"))); +##### Hierarchical Clients -// Use the client mock -ConfigurationClient client = mock.Object; -ConfigurationSetting setting = client.Get("Key"); -Assert.AreEqual("Value", setting.Value); -``` +TODO: Add discussion of hierarchical clients -Review the [full sample](https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/appconfiguration/Azure.Data.AppConfiguration/samples/Sample7_MockClient.md) in the GitHub repository. +### Supporting Types -{% include requirement/MUST id="dotnet-mocking-virtual-method" %} make all service methods virtual. +In addition to service client types, Azure SDK APIs provide and use other supporting types as well. -{% include requirement/MUST id="dotnet-mocking-constructor" %} provide protected parameterless constructor for mocking. +#### Model Types {#dotnet-model-types} -{% include requirement/MUST id="dotnet-mocking-factory-builder" %} provide factory or builder for constructing model graphs returned from virtual service methods. +This section describes guidelines for the design _model types_ and all their transitive closure of public dependencies (i.e. the _model graph_). A model type is a representation of a REST service's resource. -Model types shouldn't have public constructors. Instances of the model are typically returned from the client library, and are not constructed by the consumer of the library. Mock implementations need to create instances of model types. Implement a static class called `ModelFactory` in the same namespace as the model types: +For example, review the configuration service _model type_ below: ```csharp -public static class ConfigurationModelFactory { - public static ConfigurationSetting ConfigurationSetting(string key, string value, string label=default, string contentType=default, ETag eTag=default, DateTimeOffset? lastModified=default, bool? locked=default); - public static SettingBatch SettingBatch(ConfigurationSetting[] settings, string link, SettingSelector selector); -} -``` +public sealed class ConfigurationSetting : IEquatable { -{% include requirement/MUST id="dotnet-mocking-factory-builder-methods" %} hide older overloads and avoid ambiguity. + public ConfigurationSetting(string key, string value, string label = default); -When read-only properties are added to models and factory methods must be added to optionally set these properties, -you must hide the previous method and remove all default parameter values to avoid ambiguity: + public string ContentType { get; set; } + public string ETag { get; internal set; } + public string Key { get; set; } + public string Label { get; set; } + public DateTimeOffset LastModified { get; internal set; } + public bool Locked { get; internal set; } + public IDictionary Tags { get; } + public string Value { get; set; } + + public bool Equals(ConfigurationSetting other); + + [EditorBrowsable(EditorBrowsableState.Never)] + public override bool Equals(object obj); + [EditorBrowsable(EditorBrowsableState.Never)] + public override int GetHashCode(); + [EditorBrowsable(EditorBrowsableState.Never)] + public override string ToString(); +} +``` + +This model is returned from service methods as follows: ```csharp -public static class ConfigurationModelFactory { +public class ConfigurationClient { + public virtual Task> GetAsync(...); + public virtual Response Get(...); + ... +} +``` + +{% include requirement/MUST id="dotnet-service-return-model-public-getters" %} ensure model public properties are get-only if they aren't intended to be changed by the user. + +Most output-only models can be fully read-only. Models that are used as both outputs and inputs (i.e. received from and sent to the service) typically have a mixture of read-only and read-write properties. + +For example, the `Locked` property of `ConfigurationSetting` is controlled by the service. It shouldn't be changed by the user. The `ContentType` property, by contrast, can be modified by the user. + +```csharp +public sealed class ConfigurationSetting : IEquatable { + + public string ContentType { get; set; } + + public bool Locked { get; internal set; } +} +``` + +Ensure you include an internal setter to allow for deserialization. For more information, see [JSON Serialization](#dotnet-usage-json). + +{% include requirement/MUST id="dotnet-service-models-prefer-structs" %} ensure model types are structs, if they meet the criteria for being structs. + +Good candidates for struct are types that are small and immutable, especially if they are often stored in arrays. See [.NET Framework Design Guidelines](https://docs.microsoft.com/dotnet/standard/design-guidelines/choosing-between-class-and-struct) for details. + +TODO: Update link to FDG3 + +{% include requirement/SHOULD id="dotnet-service-models-basic-data-interfaces" %} implement basic data type interfaces on model types, per .NET Framework Design Guidelines. + +For example, implement `IEquatable`, `IComparable`, `IEnumerable`, etc. if applicable. + +{% include requirement/SHOULD id="dotnet-service-return-model-collections" %} use the following collection types for properties of model types: + +- ```IReadOnlyList``` and ```IList``` for most collections +- ```IReadOnlyDictionary``` and ```IDictionary``` for lookup tables +- ```T[]```, ```Memory```, and ```ReadOnlyMemory``` when low allocations and performance are critical + +Note that this guidance does not apply to input parameters. Input parameters representing collections should follow standard [.NET Design Guidelines](https://docs.microsoft.com/dotnet/standard/design-guidelines/parameter-design), e.g. use ```IEnumerable``` is allowed. +Also, this guidance does not apply to return types of service method calls. These should be using ```Pageable``` and ```AsyncPageable``` discussed in [Service Method Return Types](#dotnet-method-return). + +{% include requirement/MAY id="dotnet-service-models-namespace" %} place output model types in _.Models_ subnamespace to avoid cluttering the main namespace with too many types. + +It is important for the main namespace of a client library to be clutter free. Some client libraries have a relatively small number of model types, and these should keep the model types in the main namespace. For example, model types of `Azure.Data.AppConfiguration` package are in the main namespace. On the other hand, model types of `Azure.Storage.Blobs` package are in _.Models_ subnamespace. + +```csharp +namespace Azure.Storage.Blobs { + public class BlobClient { ... } + public class BlobClientOptions { ... } + ... +} +namespace Azure.Storage.Blobs.Models { + ... + public class BlobContainerItem { ... } + public class BlobContainerProperties { ...} + ... +} +``` + +{% include requirement/SHOULD id="dotnet-service-editor-browsable-state" %} apply the `[EditorBrowsable(EditorBrowsableState.Never)]` attribute to methods on the model type that the user isn't meant to call. + +Adding this attribute will hide the methods from being shown with IntelliSense. A user will almost never call `GetHashCode()` directly. `Equals(object)` is almost never called if the type implements `IEquatable` (which is preferred). Hide the `ToString()` method if it isn't overridden. + +```csharp +public sealed class ConfigurationSetting : IEquatable { [EditorBrowsable(EditorBrowsableState.Never)] - public static ConfigurationSetting ConfigurationSetting(string key, string value, string label, string contentType, ETag eTag, DateTimeOffset? lastModified, bool? locked) => - ConfigurationSetting(key, value, label, contentType, eTag, lastModified, locked, default); - public static ConfigurationSetting ConfigurationSetting(string key, string value, string label=default, string contentType=default, ETag eTag=default, DateTimeOffset? lastModified=default, bool? locked=default, int? ttl=default); + public override bool Equals(object obj); + [EditorBrowsable(EditorBrowsableState.Never)] + public override int GetHashCode(); } ``` +{% include note.html content="Unlike service clients, model types aren't required to be thread-safe, as they're rarely shared between threads." %} + +{% include requirement/MUST id="dotnet-models-in-mocks" %} ensure all model types can be used in mocks. + +In practice, you need to provide public APIs to construct _model graphs_. See [Support for Mocking](#dotnet-mocking) for details. + +##### Model Type Naming + +TODO: Add this discussion to parallel other sections + +#### Enumerations + +{% include requirement/MUST id="dotnet-enums" %} use an `enum` for parameters, properties, and return types when values are known. + +{% include requirement/MAY id="dotnet-enums-exception" %} use a `readonly struct` in place of an `enum` that declares well-known fields but can contain unknown values returned from the service, or user-defined values passed to the service. + +See [enumeration-like structure documentation](implementation.md#dotnet-enums) for implementation details. + +#### Using Azure Core Types {#dotnet-commontypes} + +The `Azure.Core` package provides common functionality for client libraries. Documentation and usage examples can be found in the [azure/azure-sdk-for-net](https://github.com/Azure/azure-sdk-for-net/tree/master/sdk/core/Azure.Core) repository. + +#### Using Primitive Types + +{% include requirement/MUST id="dotnet-primitives-etag" %} use `Azure.ETag` to represent ETags. + +The `Azure.ETag` type is located in `Azure.Core` package. + +{% include requirement/MUST id="dotnet-primitives-uri" %} use `System.Uri` to represent URIs. + +### Exceptions {#dotnet-errors} + +In .NET, throwing exceptions is how we communicate to library consumers that the services returned an error. + +{% include requirement/MUST id="dotnet-errors-response-failed" %} throw `RequestFailedException` or its subtype when a service method fails with non-success status code. + +The exception is available in ```Azure.Core``` package: + +```csharp +public class RequestFailedException : Exception { + + public RequestFailedException(int status, string message); + public RequestFailedException(int status, string message, Exception innerException); + + public int Status { get; } +} +``` + +{% include requirement/SHOULD id="dotnet-errors-response-exception-extensions" %} use `ResponseExceptionExtensions` to create `RequestFailedException` instances. + +The exception message should contain detailed response information. For example: + +```csharp +if (response.Status != 200) { + throw await response.CreateRequestFailedExceptionAsync(message); +} +``` + +{% include requirement/MUST id="dotnet-errors-use-response-failed-when-possible" %} use `RequestFailedException` or one of its subtypes where possible. + +Don't introduce new exception types unless there's a programmatic scenario for handling the new exception that's different than `RequestFailedException` + ### Authentication {#dotnet-authentication} -The client library consumer should construct a service client using just the constructor. After construction, service methods can be called successfully. The constructor parameters must take all parameters required to create a functioning client, including all information needed to authenticate with the service. +The client library consumer should construct a service client using just the constructor. After construction, service methods can successfully invoke service operations. The constructor parameters must take all parameters required to create a functioning client, including all information needed to authenticate with the service. The general constructor pattern refers to _binding parameters_. @@ -613,17 +740,7 @@ Credentials passed to the constructors must be read before every request (for ex Don't ask users to compose connection strings manually if they aren't available through the Azure portal. Connection strings are immutable. It's impossible for an application to roll over credentials when using connection strings. -### Enumerations - -{% include requirement/MUST id="dotnet-enums" %} use an `enum` for parameters, properties, and return types when values are known. - -{% include requirement/MAY id="dotnet-enums-exception" %} use a `readonly struct` in place of an `enum` that declares well-known fields but can contain unknown values returned from the service, or user-defined values passed to the service. - -See [enumeration-like structure documentation](implementation.md#dotnet-enums) for implementation details. - -## General Azure SDK Library Design - -### Namespace Naming {#dotnet-namespace-naming} +### Namespaces {#dotnet-namespace-naming} {% include requirement/MUST id="dotnet-namespaces-naming" %} adhere to the following scheme when choosing a namespace: `Azure..[.]` @@ -655,44 +772,61 @@ If you think a new group should be added to the list, contact [adparch]. See [model type guidelines](#dotnet-service-models-namespace) for details. -### Error Reporting {#dotnet-errors} +TODO: This set of namespaces needs to be updated. -{% include requirement/MUST id="dotnet-errors-response-failed" %} throw `RequestFailedException` or its subtype when a service method fails with non-success status code. +### Support for Mocking {#dotnet-mocking} -The exception is available in ```Azure.Core``` package: -```csharp -public class RequestFailedException : Exception { +All client libraries must support mocking to enable non-live testing of service clients by customers. - public RequestFailedException(int status, string message); - public RequestFailedException(int status, string message, Exception innerException); +Here is an example of how the `ConfigurationClient` can be mocked using [Moq] (a popular .NET mocking library): - public int Status { get; } -} +```csharp +// Create a mock response +var mockResponse = new Mock(); +// Create a client mock +var mock = new Mock(); +// Setup client method +mock.Setup(c => c.Get("Key", It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(new Response(mockResponse.Object, ConfigurationModelFactory.ConfigurationSetting("Key", "Value"))); + +// Use the client mock +ConfigurationClient client = mock.Object; +ConfigurationSetting setting = client.Get("Key"); +Assert.AreEqual("Value", setting.Value); ``` -{% include requirement/SHOULD id="dotnet-errors-response-exception-extensions" %} use `ResponseExceptionExtensions` to create `RequestFailedException` instances. +Review the [full sample](https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/appconfiguration/Azure.Data.AppConfiguration/samples/Sample7_MockClient.md) in the GitHub repository. -The exception message should contain detailed response information. For example: +{% include requirement/MUST id="dotnet-mocking-constructor" %} provide protected parameterless constructor for mocking. -```csharp -if (response.Status != 200) { - throw await response.CreateRequestFailedExceptionAsync(message); -} -``` +{% include requirement/MUST id="dotnet-mocking-virtual-method" %} make all service methods virtual. -{% include requirement/MUST id="dotnet-errors-use-response-failed-when-possible" %} use `RequestFailedException` or one of its subtypes where possible. +{% include requirement/MUST id="dotnet-mocking-factory-builder" %} provide factory or builder for constructing model graphs returned from virtual service methods. -Don't introduce new exception types unless there's a programmatic scenario for handling the new exception that's different than `RequestFailedException` +Model types shouldn't have public constructors. Instances of the model are typically returned from the client library, and are not constructed by the consumer of the library. Mock implementations need to create instances of model types. Implement a static class called `ModelFactory` in the same namespace as the model types: -### Logging +```csharp +public static class ConfigurationModelFactory { + public static ConfigurationSetting ConfigurationSetting(string key, string value, string label=default, string contentType=default, ETag eTag=default, DateTimeOffset? lastModified=default, bool? locked=default); + public static SettingBatch SettingBatch(ConfigurationSetting[] settings, string link, SettingSelector selector); +} +``` -Request logging will be done automatically by the `HttpPipeline`. If a client library needs to add custom logging, follow the [same guidelines](implementation.md#general-logging) and mechanisms as the pipeline logging mechanism. If a client library wants to do custom logging, the designer of the library must ensure that the logging mechanism is pluggable in the same way as the `HttpPipeline` logging policy. +{% include requirement/MUST id="dotnet-mocking-factory-builder-methods" %} hide older overloads and avoid ambiguity. -{% include requirement/MUST id="dotnet-logging-follow-guidelines" %} follow [the logging section of the Azure SDK General Guidelines](implementation.md#general-logging) if logging directly (as opposed to through the `HttpPipeline`). +When read-only properties are added to models and factory methods must be added to optionally set these properties, +you must hide the previous method and remove all default parameter values to avoid ambiguity: -#### Distributed Tracing {#dotnet-distributedtracing} +```csharp +public static class ConfigurationModelFactory { + [EditorBrowsable(EditorBrowsableState.Never)] + public static ConfigurationSetting ConfigurationSetting(string key, string value, string label, string contentType, ETag eTag, DateTimeOffset? lastModified, bool? locked) => + ConfigurationSetting(key, value, label, contentType, eTag, lastModified, locked, default); + public static ConfigurationSetting ConfigurationSetting(string key, string value, string label=default, string contentType=default, ETag eTag=default, DateTimeOffset? lastModified=default, bool? locked=default, int? ttl=default); +} +``` -{% include draft.html content="Guidance coming soon ..." %} +## Azure SDK Library Design ### Packaging {#dotnet-packaging} @@ -714,80 +848,23 @@ Use the following target setting in the `.csproj` file: netstandard2.0 ``` -### Dependencies {#dotnet-dependencies} - -{% include requirement/SHOULD id="dotnet-dependencies-minimize" %} minimize dependencies outside of the .NET Standard and `Azure.Core` packages. +#### Common Libraries -{% include requirement/MUSTNOT id="dotnet-dependencies-list" %} depend on any NuGet package except the following packages: - -* `Azure.*` packages from the [azure/azure-sdk-for-net] repository. -* `System.Text.Json`. -* `Microsoft.BCL.AsyncInterfaces`. -* packages produced by your own team. - -In the past, [JSON.NET] was commonly used for serialization and deserialization. Use the [System.Text.Json](https://www.nuget.org/packages/System.Text.Json/) -package that is now a part of the .NET platform instead. - -{% include requirement/MUSTNOT id="dotnet-dependencies-exposing" %} publicly expose types from dependencies unless the types follow these guidelines as well. +TODO: add this discussion ### Versioning {#dotnet-versioning} +#### Client Versions + {% include requirement/MUST id="dotnet-versioning-backwards-compatibility" %} be 100% backwards compatible with older versions of the same package. For detailed rules, see [.NET Breaking Changes](https://github.com/dotnet/runtime/blob/master/docs/coding-guidelines/breaking-change-rules.md). -{% include requirement/MUST id="dotnet-versioning-highest-api" %} call the highest supported service API version by default. - -{% include requirement/MUST id="dotnet-versioning-select-api-version" %} allow the consumer to explicitly select a supported service API version when instantiating the service client. - -Use a constructor parameter called `version` on the client options type. - -* The `version` parameter must be the first parameter to all constructor overloads. -* The `version` parameter must be required, and default to the latest supported service version. -* The type of the `version` parameter must be `ServiceVersion`; an enum nested in the options type. -* The `ServiceVersion` enum must use explicit values starting from 1. -* `ServiceVersion` enum value 0 is reserved. When 0 is passed into APIs, ArgumentException should be thrown. - -For example, the following is a code snippet from the `ConfigurationClientOptions`: - -```csharp -public class ConfigurationClientOptions : ClientOptions { - - public ConfigurationClientOptions(ServiceVersion version = ServiceVersion.V2019_05_09) { - if (version == default) - throw new ArgumentException($"The service version {version} is not supported by this library."); - } - } - - public enum ServiceVersion { - V2019_05_09 = 1, - } - ... -} -``` - -{% include note.html content="Third-party reusable libraries shouldn't change behavior without an explicit decision by the developer. When developing libraries that are based on the Azure SDK, lock the library to a specific service version to avoid changes in behavior." %} - {% include requirement/MUST id="dotnet-versioning-new-package" %} introduce a new package (with new assembly names, new namespace names, and new type names) if you must do an API breaking change. Breaking changes should happen rarely, if ever. Register your intent to do a breaking change with [adparch]. You'll need to have a discussion with the language architect before approval. -{% include requirement/MUSTNOT id="dotnet-versioning-feature-flags" %} force consumers to test service API versions to check support for a feature. Use the _tester-doer_ .NET pattern to implement feature flags, or use `Nullable`. - -For example, if the client library supports two service versions, only one of which can return batches, the consumer might write the following code: - -```csharp -if (client.CanBatch) { - Response response = await client.GetBatch("some_key*"); - Guid? Guid = response.Result.Guid; -} else { - Response response1 = await client.GetAsync("some_key1"); - Response response2 = await client.GetAsync("some_key2"); - Response response3 = await client.GetAsync("some_key3"); -} -``` - -#### Version Numbers {#dotnet-versionnumbers} +##### Package Version Numbers {#dotnet-versionnumbers} Consistent version number scheme allows consumers to determine what to expect from a new version of the library. @@ -809,278 +886,45 @@ Use _-beta._N_ suffix for beta package versions. For example, _1.0.0-beta.2_. {% include requirement/MUST id="dotnet-version-change-on-release" %} select a version number greater than the highest version number of any other released Track 1 package for the service in any other scope or language. -### Documentation {#dotnet-documentation} - -{% include requirement/MUST id="dotnet-docs-document-everything" %} document every exposed (public or protected) type and member within your library's code. - -{% include requirement/MUST id="dotnet-docs-docstrings" %} use [C# documentation comments](https://docs.microsoft.com/dotnet/csharp/language-reference/language-specification/documentation-comments) for reference documentation. - -See the [documentation guidelines]({{ site.baseurl }}/general_documentation.html) for language-independent guidelines for how to provide good documentation. - -## Common Type Usage {#dotnet-commontypes} - -### Using ClientOptions {#dotnet-usage-options} - -{% include requirement/MUST id="dotnet-http-pipeline-options" %} name subclasses of ```ClientOptions``` by adding _Options_ suffix to the name of the client type the options subclass is configuring. - -```csharp -// options for configuring ConfigurationClient -public class ConfigurationClientOptions : ClientOptions { - ... -} - -public class ConfigurationClient { - - public ConfigurationClient(string connectionString, ConfigurationClientOptions options); - ... -} -``` - -If the options type can be shared by multiple client types, name it with a plural or more general name. For example, the `BlobClientsOptions` class can be used by `BlobClient`, `BlobContainerClient`, and `BlobAccountClient`. - -{% include requirement/MUSTNOT id="dotnet-options-no-default-constructor" %} have a default constructor on the options type. - -Each overload constructor should take at least `version` parameter to specify the service version. See [Versioning](#dotnet-service-version-option) guidelines for details. - -### Using HttpPipeline {#dotnet-usage-httppipeline} - -The following example shows a typical way of using `HttpPipeline` to implement a service call method. The `HttpPipeline` will handle common HTTP requirements such as the user agent, logging, distributed tracing, retries, and proxy configuration. - -```csharp -public virtual async Task> AddAsync(ConfigurationSetting setting, CancellationToken cancellationToken = default) -{ - if (setting == null) throw new ArgumentNullException(nameof(setting)); - ... // validate other preconditions - - // Use HttpPipeline _pipeline filed of the client type to create new HTTP request - using (Request request = _pipeline.CreateRequest()) { - - // specify HTTP request line - request.Method = RequestMethod.Put; - request.Uri.Reset(_endpoint); - request.Uri.AppendPath(KvRoute, escape: false); - requast.Uri.AppendPath(key); - - // add headers - request.Headers.Add(IfNoneMatchWildcard); - request.Headers.Add(MediaTypeKeyValueApplicationHeader); - request.Headers.Add(HttpHeader.Common.JsonContentType); - request.Headers.Add(HttpHeader.Common.CreateContentLength(content.Length)); - - // add content - ReadOnlyMemory content = Serialize(setting); - request.Content = HttpPipelineRequestContent.Create(content); - - // send the request - var response = await Pipeline.SendRequestAsync(request).ConfigureAwait(false); - - if (response.Status == 200) { - // deserialize content - Response result = await CreateResponse(response, cancellationToken); - } - else - { - throw await response.CreateRequestFailedExceptionAsync(message); - } - } -} -``` - -For a more complete example, see the [configuration client](https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/appconfiguration/Azure.Data.AppConfiguration/src/ConfigurationClient.cs) implementation. - -### Using HttpPipelinePolicy - -The HTTP pipeline includes a number of policies that all requests pass through. Examples of policies include setting required headers, authentication, generating a request ID, and implementing proxy authentication. `HttpPipelinePolicy` is the base type of all policies (plugins) of the `HttpPipeline`. This section describes guidelines for designing custom policies. - -{% include requirement/MUST id="dotnet-http-pipeline-policy-inherit" %} inherit from `HttpPipelinePolicy` if the policy implementation calls asynchronous APIs. - -See an example [here](https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/core/Azure.Core/src/Pipeline/BearerTokenAuthenticationPolicy.cs). - -{% include requirement/MUST id="dotnet-sync-http-pipeline-policy-inherit" %} inherit from `HttpPipelineSynchronousPolicy` if the policy implementation calls only synchronous APIs. - -See an example [here](https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/core/Azure.Core/src/Pipeline/Internal/ClientRequestIdPolicy.cs). - -{% include requirement/MUST id="dotnet-http-pipeline-thread-safety" %} ensure `ProcessAsync` and `Process` methods are thread safe. - -`HttpMessage`, `Request`, and `Response` don't have to be thread-safe. - -### JSON Serialization {#dotnet-usage-json} - -{% include requirement/MUST id="dotnet-json-use-system-text-json" %} use [`System.Text.Json`](https://www.nuget.org/packages/System.Text.Json) package to write and read JSON content. - -{% include requirement/SHOULD id="dotnet-json-use-utf8jsonwriter" %} use `Utf8JsonWriter` to write JSON payloads: - -```csharp -var json = new Utf8JsonWriter(writer); -json.WriteStartObject(); -json.WriteString("value", setting.Value); -json.WriteString("content_type", setting.ContentType); -json.WriteEndObject(); -json.Flush(); -written = (int)json.BytesWritten; -``` - -{% include requirement/SHOULD id="dotnet-json-use-jsondocument" %} use `JsonDocument` to read JSON payloads: - -```csharp -using (JsonDocument json = await JsonDocument.ParseAsync(content, default, cancellationToken).ConfigureAwait(false)) -{ - JsonElement root = json.RootElement; - - var setting = new ConfigurationSetting(); - - // required property - setting.Key = root.GetProperty("key").GetString(); - - // optional property - if (root.TryGetProperty("last_modified", out var lastModified)) { - if(lastModified.Type == JsonValueType.Null) { - setting.LastModified = null; - } - else { - setting.LastModified = DateTimeOffset.Parse(lastModified.GetString()); - } - } - ... - - return setting; -} -``` - -{% include requirement/SHOULD id="dotnet-json-use-utf8jsonreader" %} consider using `Utf8JsonReader` to read JSON payloads. - -`Utf8JsonReader` is faster than `JsonDocument` but much less convenient to use. - -{% include requirement/MUST id="dotnet-json-serialization-resilience" %} make your serialization and deserialization code version resilient. - -Optional JSON properties should be deserialized into nullable model properties. - -### Implementing Subtypes of Operation {#dotnet-implement-operation} - -{% include requirement/MUST id="dotnet-lro-return" %} check the value of `HasCompleted` in subclass implementations of `UpdateStatus` and `UpdateStatusAsync` and immediately return the result of `GetRawResponse` if it is true. - -```csharp -public override Response UpdateStatus( - CancellationToken cancellationToken = default) => - UpdateStatusAsync(false, cancellationToken).EnsureCompleted(); - -public override async ValueTask UpdateStatusAsync( - CancellationToken cancellationToken = default) => - await UpdateStatusAsync(true, cancellationToken).ConfigureAwait(false); - -private async Task UpdateStatusAsync(bool async, CancellationToken cancellationToken) -{ - // Short-circuit when already completed (which improves mocking scenarios that won't have a client). - if (HasCompleted) - { - return GetRawResponse(); - } - - // some service operation call here, which the above check avoids if the operation has already completed. - ... -``` - -{% include requirement/MUST id="dotnet-lro-return" %} throw from methods on `Operation` subclasses in the following scenarios. - -* If an underlying service operation call from `UpdateStatus`, `WaitForCompletion`, or `WaitForCompletionAsync` throws, re-throw `RequestFailedException` or its subtype. -* If the operation completes with a non-success result, throw `RequestFailedException` or its subtype from `UpdateStatus`, `WaitForCompletion`, or `WaitForCompletionAsync`. - * Include any relevant error state information in the exception details. - -```csharp -private async ValueTask UpdateStatusAsync(bool async, CancellationToken cancellationToken) -{ - ... - - try - { - Response update = async - ? await _serviceClient.GetExampleResultAsync(new Guid(Id), cancellationToken).ConfigureAwait(false) - : _serviceClient.GetExampleResult(new Guid(Id), cancellationToken); - - _response = update.GetRawResponse(); - - if (update.Value.Status == OperationStatus.Succeeded) - { - // we need to first assign a value and then mark the operation as completed to avoid race conditions - _value = ConvertValue(update.Value.ExampleResult.PageResults, update.Value.ExampleResult.ReadResults); - _hasCompleted = true; - } - else if (update.Value.Status == OperationStatus.Failed) - { - _requestFailedException = await ClientCommon - .CreateExceptionForFailedOperationAsync(async, _diagnostics, _response, update.Value.AnalyzeResult.Errors) - .ConfigureAwait(false); - _hasCompleted = true; - - // the operation didn't throw, but it completed with a non-success result. - throw _requestFailedException; - } - } - catch (Exception e) - { - scope.Failed(e); - throw; - } - } - - return GetRawResponse(); -} -``` +### Dependencies {#dotnet-dependencies} -* If the ```Value``` property is evaluated after the operation failed (```HasValue``` is false and ```HasCompleted``` is true) throw the same exception as the one thrown when the operation failed. +{% include requirement/SHOULD id="dotnet-dependencies-minimize" %} minimize dependencies outside of the .NET Standard and `Azure.Core` packages. -```csharp -public override FooResult Value -{ - get - { - if (HasCompleted && !HasValue) -#pragma warning disable CA1065 // Do not raise exceptions in unexpected locations +{% include requirement/MUSTNOT id="dotnet-dependencies-list" %} depend on any NuGet package except the following packages: - // throw the exception captured when the operation failed. - throw _requestFailedException; +* `Azure.*` packages from the [azure/azure-sdk-for-net] repository. +* `System.Text.Json`. +* `Microsoft.BCL.AsyncInterfaces`. +* packages produced by your own team. -#pragma warning restore CA1065 // Do not raise exceptions in unexpected locations - else - return OperationHelpers.GetValue(ref _value); - } -} -``` +In the past, [JSON.NET] was commonly used for serialization and deserialization. Use the [System.Text.Json](https://www.nuget.org/packages/System.Text.Json/) +package that is now a part of the .NET platform instead. -* If the ```Value``` property is evaluated before the operation is complete (```HasCompleted``` is false) throw ```InvalidOperationException```. - * The exception message should be: "The operation has not yet completed." +{% include requirement/MUSTNOT id="dotnet-dependencies-exposing" %} publicly expose types from dependencies unless the types follow these guidelines as well. -```csharp -// start the operation. -ExampleOperation operation = await client.StartExampleOperationAsync(someParameter); +### Native Code -if (!operation.HasCompleted) -{ - // attempt to get the Value property before the operation has completed. - ExampleOperationResult myResult = operation.Value; //throws InvalidOperationException. -} -else if (!operation.HasValue) -{ - // attempt to get the Value property after the operation has completed unsuccessfully. - ExampleOperationResult myResult = operation.Value; //throws RequestFailedException. -} -``` +TODO: Add this discussion -### Primitive Types +### Documentation Comments {#dotnet-documentation} -{% include requirement/MUST id="dotnet-primitives-etag" %} use `Azure.ETag` to represent ETags. +{% include requirement/MUST id="dotnet-docs-document-everything" %} document every exposed (public or protected) type and member within your library's code. -The `Azure.ETag` type is located in `Azure.Core` package. +{% include requirement/MUST id="dotnet-docs-docstrings" %} use [C# documentation comments](https://docs.microsoft.com/dotnet/csharp/language-reference/language-specification/documentation-comments) for reference documentation. -{% include requirement/MUST id="dotnet-primitives-uri" %} use `System.Uri` to represent URIs. +See the [documentation guidelines]({{ site.baseurl }}/general_documentation.html) for language-independent guidelines for how to provide good documentation. -# Repository Guidelines {#dotnet-repository} +## Repository Guidelines {#dotnet-repository} {% include requirement/MUST id="dotnet-general-repository" %} locate all source code and README in the [azure/azure-sdk-for-net] GitHub repository. {% include requirement/MUST id="dotnet-general-engsys" %} follow Azure SDK engineering systems guidelines for working in the [azure/azure-sdk-for-net] GitHub repository. -## README {#dotnet-repository-readme} +### Documentation Style + +TODO: Add links to doc style guidance + +### README {#dotnet-repository-readme} {% include requirement/MUST id="dotnet-docs-readme" %} have a README.md file in the component root folder. @@ -1090,7 +934,7 @@ An example of a good `README.md` file can be found [here](https://github.com/Azu The contributor guide (`CONTRIBUTING.md`) should be a separate file linked to from the main component `README.md`. -## Samples {#dotnet-samples} +### Samples {#dotnet-samples} Each client library should have a quickstart guide with code samples. Developers like to learn about a library by looking at sample code; not by reading in-depth technology papers. @@ -1131,9 +975,11 @@ var client = new ConfigurationClient(connectionString); {% include requirement/MUST id="dotnet-samples-build" %} make sure all the samples build and run as part of the CI process. -# Commonly Overlooked .NET API Design Guidelines {#dotnet-appendix-overlookedguidelines} +TODO: Update guidance on samples to reflect what we do in most places. + +## Commonly Overlooked .NET API Design Guidelines {#dotnet-appendix-overlookedguidelines} -Some .NET Design Guidelines have been notoriously overlooked in existing Azure SDKs. This section serves as a way to highlight these guidelines. +Some .NET Design Guidelines have been notoriously overlooked in earlier Azure SDKs. This section serves as a way to highlight these guidelines. {% include requirement/SHOULDNOT id="dotnet-problems-too-many-types" %} have many types in the main namespace. Number of types is directly proportional to the perceived complexity of a library.