refactor!: generated clients inherit ConnectorClientBase (#88) by daviburg · Pull Request #90 · Azure/Connectors-NET-SDK

daviburg · 2026-05-02T02:11:19Z

Summary

Generated connector clients now inherit from ConnectorClientBase instead of implementing IDisposable directly. This eliminates ~1,500 lines of duplicated boilerplate across 11 connector files.

Breaking changes

Constructor signature: new optional ConnectorClientOptions parameter (position 3), HttpClient moves to position 4
Exception types: per-connector exceptions (e.g., Office365ConnectorException) replaced by unified ConnectorException
Namespace: Microsoft.Azure.Connectors.DirectClient.{Connector} → Microsoft.Azure.Connectors.Sdk.{Connector}
Navigator class: DirectClientConnectors → SdkConnectors

SDK core changes

New type	Purpose
`ConnectorClientBase`	Convenience constructors, `CallConnectorAsync`, `ResolveUrl` (SSRF), shared JSON options
`TokenCredentialTokenProvider`	Adapts `Azure.Core.TokenCredential` to `ITokenProvider`
`ConnectorException`	Unified exception with `ConnectorName`, `Operation`, `StatusCode`, `ResponseBody`

Generated files

All 11 connector files regenerated via updated CodefulSdkGenerator (not hand-edited). Each client now inherits ConnectorClientBase, overrides ConnectorName, and accepts optional ConnectorClientOptions.

Related PRs

BPM generator PR #15600409 — generator template changes
Azure/Connectors-NET-LSP#67 — namespace update
Azure/Connectors-NET-Samples#32 — namespace update

Test results

218/218 tests pass ✅

Closes #88

Copilot

Pull request overview

Refactors the generated connector clients to inherit from ConnectorClientBase (instead of implementing IDisposable and duplicating HTTP/auth/retry/JSON logic per connector), and aligns namespaces/exceptions/tests/docs with the new shared SDK infrastructure.

Changes:

Introduces shared SDK infrastructure (ConnectorClientBase, ConnectorException, TokenCredentialTokenProvider) and migrates generated clients to use it.
Regenerates 11 connector extension files to inherit the base class, switch namespaces to Microsoft.Azure.Connectors.Sdk.*, and unify exceptions to ConnectorException.
Updates unit tests and docs to reference the new namespaces and exception type.

Reviewed changes

Copilot reviewed 20 out of 33 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/Microsoft.Azure.Connectors.Sdk.Tests/TriggerCallbackPayloadTests.cs	Updates Office365 namespace import to `Microsoft.Azure.Connectors.Sdk.Office365`.
tests/Microsoft.Azure.Connectors.Sdk.Tests/TeamsClientTests.cs	Updates Teams namespace + asserts unified `ConnectorException`.
tests/Microsoft.Azure.Connectors.Sdk.Tests/SmtpClientTests.cs	Updates Smtp namespace + renames/asserts unified `ConnectorException`.
tests/Microsoft.Azure.Connectors.Sdk.Tests/SharepointonlineClientTests.cs	Updates SharePointOnline namespace + unified exception assertions/construction.
tests/Microsoft.Azure.Connectors.Sdk.Tests/OnedriveforbusinessClientTests.cs	Updates OneDrive for Business namespace + unified exception assertions/construction.
tests/Microsoft.Azure.Connectors.Sdk.Tests/Office365usersClientTests.cs	Updates Office365Users namespace + unified exception assertions.
tests/Microsoft.Azure.Connectors.Sdk.Tests/Office365TriggerPayloadTests.cs	Updates Office365 namespace import.
tests/Microsoft.Azure.Connectors.Sdk.Tests/Office365ClientTests.cs	Updates Office365 namespace + unified exception assertions.
tests/Microsoft.Azure.Connectors.Sdk.Tests/MsgraphgroupsanduserClientTests.cs	Updates MsGraphGroupsAndUser namespace + unified exception assertions.
tests/Microsoft.Azure.Connectors.Sdk.Tests/MqClientTests.cs	Updates MQ namespace + renames/asserts unified `ConnectorException`.
tests/Microsoft.Azure.Connectors.Sdk.Tests/KustoClientTests.cs	Updates Kusto namespace + unified exception assertions/construction.
tests/Microsoft.Azure.Connectors.Sdk.Tests/DynamicSchemaTests.cs	Updates Teams namespace import.
tests/Microsoft.Azure.Connectors.Sdk.Tests/ConnectorConstantsTests.cs	Updates constants test to use `SdkConnectors` and new namespaces.
tests/Microsoft.Azure.Connectors.Sdk.Tests/AzureloganalyticsClientTests.cs	Updates Azure Log Analytics namespace + unified exception assertions/construction.
tests/Microsoft.Azure.Connectors.Sdk.Tests/AzureblobClientTests.cs	Updates Azure Blob namespace + unified exception assertions/construction.
src/Microsoft.Azure.Connectors.Sdk/Generated/TeamsExtensions.cs	Migrates Teams client to `ConnectorClientBase`, new namespace, unified exception.
src/Microsoft.Azure.Connectors.Sdk/Generated/SmtpExtensions.cs	Migrates Smtp client to `ConnectorClientBase`, new namespace, unified exception.
src/Microsoft.Azure.Connectors.Sdk/Generated/SharepointonlineExtensions.cs	Migrates SharePointOnline client to `ConnectorClientBase`, new namespace, unified exception.
src/Microsoft.Azure.Connectors.Sdk/Generated/OnedriveforbusinessExtensions.cs	Migrates OneDrive for Business client to `ConnectorClientBase`, new namespace, unified exception.
src/Microsoft.Azure.Connectors.Sdk/Generated/Office365usersExtensions.cs	Migrates Office365Users client to `ConnectorClientBase`, new namespace, unified exception.
src/Microsoft.Azure.Connectors.Sdk/Generated/Office365Extensions.cs	Migrates Office365 client to `ConnectorClientBase`, new namespace, unified exception.
src/Microsoft.Azure.Connectors.Sdk/Generated/MsgraphgroupsanduserExtensions.cs	Migrates MsGraphGroupsAndUser client to `ConnectorClientBase`, new namespace, unified exception.
src/Microsoft.Azure.Connectors.Sdk/Generated/MqExtensions.cs	Migrates MQ client to `ConnectorClientBase`, new namespace, unified exception.
src/Microsoft.Azure.Connectors.Sdk/Generated/KustoExtensions.cs	Migrates Kusto client to `ConnectorClientBase`, new namespace, unified exception.
src/Microsoft.Azure.Connectors.Sdk/Generated/AzureloganalyticsExtensions.cs	Migrates Azure Log Analytics client to `ConnectorClientBase`, new namespace, unified exception.
src/Microsoft.Azure.Connectors.Sdk/Generated/AzureblobExtensions.cs	Migrates Azure Blob client to `ConnectorClientBase`, new namespace, unified exception.
src/Microsoft.Azure.Connectors.Sdk/Generated/ManagedConnectors.cs	Renames registry type to `SdkConnectors` and updates usage comments/namespaces.
src/Microsoft.Azure.Connectors.Sdk/Generated/ConnectorNames.cs	Updates header text (and minor doc text) to match new SDK branding.
src/Microsoft.Azure.Connectors.Sdk/ConnectorException.cs	Adds unified exception type with connector/operation/status/body.
src/Microsoft.Azure.Connectors.Sdk/ConnectorClientBase.cs	Adds shared constructors + CallConnectorAsync + SSRF-protected URL resolution.
src/Microsoft.Azure.Connectors.Sdk/Authentication/TokenCredentialTokenProvider.cs	Adds adapter from `Azure.Core.TokenCredential` to `ITokenProvider`.
README.md	Updates docs to describe new base/exceptions/options and adds a configuration sample.
CHANGELOG.md	Documents breaking changes and new shared types in Unreleased.

Copilot

Pull request overview

Copilot reviewed 20 out of 33 changed files in this pull request and generated 4 comments.

Copilot

Pull request overview

Copilot reviewed 20 out of 33 changed files in this pull request and generated 5 comments.

Copilot

Pull request overview

Copilot reviewed 22 out of 36 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 22 out of 36 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 22 out of 36 changed files in this pull request and generated no new comments.

Breaking change: generated connector clients now inherit from ConnectorClientBase instead of implementing IDisposable directly. SDK core changes: - ConnectorClientBase: add convenience constructors (connectionRuntimeUrl + TokenCredential), CallConnectorAsync, ResolveUrl with SSRF protection, shared ApiHubScopes and JsonOptions - TokenCredentialTokenProvider: adapts Azure.Core.TokenCredential to ITokenProvider - ConnectorException: unified exception with ConnectorName, Operation, StatusCode, ResponseBody (replaces per-connector exception types) Generated files (regenerated via updated CodefulSdkGenerator): - All 11 clients inherit ConnectorClientBase, override ConnectorName - Accept optional ConnectorClientOptions for retry/timeout - Namespace changed: DirectClient.{Connector} -> Sdk.{Connector} - DirectClientConnectors -> SdkConnectors Tests updated for ConnectorException, new namespaces, SdkConnectors. Closes #88

- Remove unused System.Net.Http.Headers using from ConnectorClientBase - Add guard in ResolveUrl for empty _connectionRuntimeUrl - Fix README Quick Start namespace to Sdk.Office365 - Add Operation to ConnectorException description in README - Fix formatting in 5 test files (constructor arg wrapping)

- Make managedIdentityClientId parameter nullable (string?) - Guard ResolveUrl against empty _connectionRuntimeUrl for relative paths - ApplyBaseUri: only set BaseUri when caller did not provide one - Add 'using System;' to README Quick Start snippet - Update DirectClient namespace references in GENERATION.md, Generated/README.md, and trigger-registration SKILL.md - Regenerated all 11 connectors via updated CodefulSdkGenerator

Use Uri.TryCreate with UriKind.Absolute and throw ArgumentException with actionable message instead of letting UriFormatException propagate.

Generated files use #nullable disable so string? triggers CS8632. Regenerated all 11 connectors with string parameter that forwards to the nullable-enabled base class.

vukelich

Minor ideas

vukelich · 2026-05-06T04:37:30Z

+        }
+
+        /// <inheritdoc />
+        public async Task<string> GetAccessTokenAsync(string[] scopes, CancellationToken cancellationToken = default)


General suggestion: I recommend always having CancellationToken cancellationToken be a required parameter with very few exceptional circumstances. Allowing it to be unused by the caller can introduce unexpected hangs, soft-locks, and degraded performance.

This suggestion is speaking with my own .NET experience and preferences, and it is not aligned with the Azure SDK .NET guidelines.

I paid the significant cost of updating thousands of lines of the Azure MCP server code to plumb through CancellationToken parameters during our drive to release a remote HTTP mode.
It took me
a lot
of time

vukelich · 2026-05-06T04:41:47Z

+        {
+            ArgumentNullException.ThrowIfNull(scopes);
+
+            if (scopes.Length == 0)


Nit: checking the length, but not the contents of the array. Do we care about null or empty string array elements?

vukelich · 2026-05-06T04:43:05Z

+            var context = new TokenRequestContext(scopes);
+            var token = await this._credential
+                .GetTokenAsync(context, cancellationToken)
+                .ConfigureAwait(continueOnCapturedContext: false);


Why include ConfigureAwait(false)?

Copilot AI review requested due to automatic review settings May 2, 2026 02:11

daviburg requested a review from a team as a code owner May 2, 2026 02:11

daviburg mentioned this pull request May 2, 2026

refactor!: update namespace from DirectClient to Sdk (#88) Azure/Connectors-NET-LSP#67

Closed

Copilot started reviewing on behalf of daviburg May 2, 2026 02:11 View session

daviburg mentioned this pull request May 2, 2026

refactor!: update namespace from DirectClient to Sdk (#88) Azure/Connectors-NET-Samples#32

Closed

Copilot AI reviewed May 2, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings May 2, 2026 04:04

Copilot started reviewing on behalf of daviburg May 2, 2026 04:05 View session

Copilot AI reviewed May 2, 2026

View reviewed changes

daviburg self-assigned this May 2, 2026

daviburg requested review from anuchandy, Copilot and vukelich May 2, 2026 04:38

Copilot started reviewing on behalf of daviburg May 4, 2026 17:15 View session

Copilot AI reviewed May 4, 2026

View reviewed changes

Comment thread src/Microsoft.Azure.Connectors.Sdk/ConnectorClientBase.cs Outdated

Comment thread src/Microsoft.Azure.Connectors.Sdk/ConnectorClientBase.cs

Comment thread README.md

Comment thread src/Microsoft.Azure.Connectors.Sdk/ConnectorClientBase.cs

Comment thread README.md

daviburg requested a review from Copilot May 4, 2026 17:33

Copilot started reviewing on behalf of daviburg May 4, 2026 17:35 View session

Copilot AI reviewed May 4, 2026

View reviewed changes

Comment thread src/Microsoft.Azure.Connectors.Sdk/ConnectorClientBase.cs Outdated

daviburg requested a review from Copilot May 4, 2026 17:42

Copilot started reviewing on behalf of daviburg May 4, 2026 17:46 View session

Copilot AI reviewed May 4, 2026

View reviewed changes

daviburg requested a review from Copilot May 4, 2026 19:06

Copilot started reviewing on behalf of daviburg May 4, 2026 19:08 View session

Copilot AI reviewed May 4, 2026

View reviewed changes

daviburg enabled auto-merge (squash) May 4, 2026 19:13

anuchandy previously approved these changes May 5, 2026

View reviewed changes

daviburg added 4 commits May 5, 2026 09:08

fix: add blank line after Client Base heading for markdown lint

a4e1852

daviburg added 2 commits May 5, 2026 09:08

fix: validate connectionRuntimeUrl as absolute URI in ApplyBaseUri

6c3053b

Use Uri.TryCreate with UriKind.Absolute and throw ArgumentException with actionable message instead of letting UriFormatException propagate.

fix: regenerate with string (not string?) for nullable-disabled files

f4fd5e4

Generated files use #nullable disable so string? triggers CS8632. Regenerated all 11 connectors with string parameter that forwards to the nullable-enabled base class.

daviburg dismissed anuchandy’s stale review via f4fd5e4 May 5, 2026 16:09

daviburg force-pushed the feature/client-options-inherit branch from d296a8e to f4fd5e4 Compare May 5, 2026 16:09

daviburg disabled auto-merge May 5, 2026 16:22

daviburg merged commit f315d17 into main May 5, 2026
10 checks passed

daviburg deleted the feature/client-options-inherit branch May 5, 2026 16:22

vukelich reviewed May 6, 2026

View reviewed changes

Conversation

daviburg commented May 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Breaking changes

SDK core changes

Generated files

Related PRs

Test results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

vukelich left a comment

Choose a reason for hiding this comment

Uh oh!

vukelich May 6, 2026

Choose a reason for hiding this comment

Uh oh!

vukelich May 6, 2026

Choose a reason for hiding this comment

Uh oh!

vukelich May 6, 2026

Choose a reason for hiding this comment

Uh oh!

vukelich May 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

daviburg commented May 2, 2026 •

edited

Loading