[Communication Identity] Migrate to TypeSpec and add api-version 2026-09-23 - #50385
Draft
John Fraser (JohnRFraser) wants to merge 16 commits into
Draft
[Communication Identity] Migrate to TypeSpec and add api-version 2026-09-23#50385John Fraser (JohnRFraser) wants to merge 16 commits into
John Fraser (JohnRFraser) wants to merge 16 commits into
Conversation
Replaces AutoRest generation with @azure-tools/typespec-java 0.46.2 driven by
tsp-client and tsp-location.yaml, generated against the merged 2025-06-30
TypeSpec at azure-rest-api-specs 9bf1953.
All four AutoRest customisations were re-expressed in a client.tsp and verified
against the generated output rather than by compiling:
1. TeamsUserExchangeTokenRequest -> GetTokenForTeamsUserOptions via @clientName
2. CommunicationIdentityTokenScope -> CommunicationTokenScope via @clientName
3. token/appId/userId -> teamsUserAadToken/clientId/userObjectId via @clientName;
the companion 'required' directive is now redundant as all three are
already required in TypeSpec
4. @immutable + all-args constructor + no setters are emitted natively, making
most of TeamsUserExchangeTokenRequestCustomization unnecessary
@access(Access.internal) keeps the wire models in implementation.models and makes
all generated client methods package-private.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
GetTokenForTeamsUserOptions: partial-update preserved the obsolete private no-arg constructor added by the old AutoRest customization class. DPG now emits final fields, so that constructor cannot compile. Removing it is the correct fix -- fromJson calls the public all-args constructor directly. CommunicationIdentityClientUtils: CommunicationIdentityAccessTokenRequest.scopes is required in TypeSpec, so DPG emits an all-args constructor and no setter. Switched from new + setScopes to the constructor. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Removes the AutoRest CommunicationIdentitiesImpl/CommunicationIdentityClientImpl and rewires the hand-written convenience layer onto the generated IdentityOperations/TeamsUserOperations clients. @access(Access.internal) leaves only protocol methods (BinaryData + RequestOptions) on the *Impl classes; the typed convenience methods live on the generated IdentityOperationsClient, whose package-private methods are reachable from the hand-written clients in the same package. Call sites that pass a Context therefore need RequestOptions.setContext, BinaryData.fromObject and a SimpleResponse rewrap -- factored into toRequestOptions/mapResponse helpers. The create operation takes its body via RequestOptions.setBody because the generated signature has no body parameter. CommunicationIdentityServiceVersion advances to V2025_06_30 and getLatest() moves. This is forced, not optional: the generated IdentityClientImpl takes a typed IdentityServiceVersion enum rather than the previous String apiVersion, and that enum contains only the versions declared in the TypeSpec Versions enum (2025-06-30 alone). Leaving getLatest() at V2023_10_01 made every builder test throw. Older public values are retained but cannot currently be routed to the generated client. Unit tests: 25/25 pass. The 4 remaining failures are all 'Test proxy did not initialize' in setupTestProxy, i.e. missing local test-proxy tooling, not SDK behaviour. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Repoints tsp-location.yaml at azure-rest-api-specs a1c68a5 (spec PR #45761) and sets the typespec-java api-version to 2026-09-23. Verified byte-identity rather than assuming it: diffing the emitted swagger for 2025-06-30 against 2026-09-23 yields exactly one differing line, the version string. Every regenerated Java file is likewise unchanged apart from blank-line churn, except IdentityServiceVersion. Checked metadata.json rather than trusting the commit message, after .NET saw the official tooling silently substitute an api-version. It reads apiVersions -> Azure.Communication.Identity -> 2026-09-23. Also fixes a defect that regeneration alone does not address: the generated IdentityServiceVersion gained 2026-09-23, but the public CommunicationIdentityServiceVersion still topped out at V2025_06_30, so getLatest() -- and therefore every default-constructed client -- would have kept sending 2025-06-30. Proven end to end with a capturing HttpClient: default -> /identities?api-version=2026-09-23 The mapServiceVersion error message previously named only values()[0], which became misleading once the generated enum held two versions; it now lists all supported versions. Throw behaviour is unchanged pending the programme decision. Version bumped to 2.0.0 (policy major, not semver: RevApi reports 10 additions and zero removals against 1.6.13). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Selecting a service version with no counterpart in the generated IdentityServiceVersion enum previously threw IllegalArgumentException at buildClient(). That was a behaviour change from 1.6.13, where any version was accepted, so it is a defect rather than a design choice. The generated IdentityClientImpl takes a typed IdentityServiceVersion, not a String, so there is no constructor seam to pass an arbitrary version through. Instead an HttpPipelinePolicy pins the api-version query parameter to the version selected on the builder, mirroring the string-based approach .NET uses. mapServiceVersion no longer throws; it falls back to the newest generated value purely to satisfy the constructor, and the policy determines what is actually sent. Verified on the wire with a capturing HttpClient - all six public versions now reach the service, none throw: default -> /identities?api-version=2026-09-23 V2021_03_07 -> /identities?api-version=2021-03-07 V2022_06_01 -> /identities?api-version=2022-06-01 V2022_10_01 -> /identities?api-version=2022-10-01 V2023_10_01 -> /identities?api-version=2023-10-01 V2025_06_30 -> /identities?api-version=2025-06-30 V2026_09_23 -> /identities?api-version=2026-09-23 The policy only rewrites the parameter when the generated client already placed one on the request, so it cannot introduce an api-version where none belongs. Also removes the CHANGELOG Breaking Changes entry that documented the throw, since the behaviour no longer occurs. Unit tests: 25/25 pass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The SDK generation configuration - client.tsp plus partial-update: true - is now
on azure-rest-api-specs main, so generation no longer depends on a local spec
clone.
Regenerated from the merged remote spec with no --local-spec-repo, i.e. the same
path the release pipeline takes. Substantive drift versus the previous local
generation is 4 lines: this commit field, and the crossLanguageVersion content
hash in metadata.json. The other 31 touched files show zero line diffs and are
CRLF churn only.
Verified after regeneration:
public models/ 5 types - CommunicationTokenScope, GetTokenForTeamsUserOptions,
CommunicationUserIdentifierAndToken, IdentityError,
IdentityErrorResponseException
raw spec names absent from the public surface
wire models 12, all in implementation/models
csharp directive correctly no-op for Java; CommunicationUserIdentifierAndToken
is still hand-written with no generated duplicate
partial-update preserved module-info requires clauses, the api-version string
seam, and all hand-written clients
compile 0 errors
unit tests 25/25 pass
RevApi 1.6.13->2.0.0 10 differences, all NON_BREAKING, 0 removals
request wire expiresInMinutes still absent on the default path
response path no null field or throw
api-version all six public versions reach the wire
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The package is now generated from TypeSpec via tsp-location.yaml, so the AutoRest configuration under swagger/ is dead. It was already inert: the merged tspconfig.yaml declares no customization-class and no custom-types, so nothing wired it into the DPG pipeline. Verified redundant rather than assumed. With swagger/ disabled and every generated file deleted so partial-update had nothing to preserve, regenerating from the merged spec reproduced all five Java-reaching renames: CommunicationTokenScope, GetTokenForTeamsUserOptions, and the three property renames on that model at field, getter, constructor-parameter and wire-name level. client.tsp owns those names now. Matches the three prior TypeSpec migrations in this repo, each of which removed swagger/ in the same pull request: App Config (#49272), Search Documents (#47819) and Monitor Ingestion (#46057). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 1 pipeline(s). 35 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
The policy was written as a lambda, so it inherited the HttpPipelinePolicy default processSync, which wraps the call in a Mono and blocks on it. That adds an allocation and a subscription to every synchronous request in order to perform a string substitution on the request URL. Replacing the lambda with a named class that implements both process and processSync removes that cost. Behaviour is unchanged: a probe confirms all six values of CommunicationIdentityServiceVersion reach the wire on both the synchronous and asynchronous paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Asserts that the api-version selected on the builder is the value placed on the wire, for all six values of CommunicationIdentityServiceVersion, on both the synchronous and the asynchronous path. Capture is at the transport layer, so the assertion is on the request that would be sent rather than on an intermediate representation. Expected values are literals rather than reads of getVersion(), so the assertion cannot be satisfied by the enum agreeing with itself. Mutating the rewritten value in the builder policy fails all twelve cases. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The lambda form compiles, behaves correctly and passes every test; what it loses is the processSync override, which costs a Mono allocation and subscription on each synchronous call. No assertion distinguishes the two forms, so the note is placed where the change would be made. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The client sets Accept on the two operations that return no content, because the generated protocol methods for those declare none. This asserts that the default does not cap a caller: a policy added with addPolicy() reaches the wire with its own value. The property holds by ordering rather than by a conditional - the header is set while the request is constructed, and pipeline policies run afterwards. Setting it from inside the pipeline instead fails both assertions, which is the reason AddHeadersPolicy was not used. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The comment claimed a lambda would block a thread in Reactor on every synchronous call. Reading HttpPipelineNextPolicy.process shows otherwise: it detects that it was entered from the synchronous default and returns to the synchronous chain on the caller's own thread, so no pool thread is consumed and there is no starvation path. The real cost is a Mono allocation and subscription per policy per request. The one case that does change behaviour is a synchronous call made from a non-blocking thread, where azure-core logs that the pipeline switched to asynchronous. Justifying the class with a consequence that does not exist is worse than not justifying it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The DPG-generated protocol methods for delete and revokeAccessTokens declare no Accept header, because those operations respond 204 with no body, so azure-core falls back to the wildcard. The AutoRest-generated client this replaces sent application/json on every operation regardless of response shape. Both clients now set it for those two, so the bytes on the wire are unchanged for existing callers. A transport-level probe reports all eight operations byte-identical to the published client once this is applied. The assertions cover the default and the caller override together: removing the header fails the four default cases, and setting it from inside the pipeline instead fails the two override cases. Also sanitizes the api-version in playback. The recordings were captured against 2023-10-01 and requests now carry 2026-09-23, so the two sides are mapped onto each other for matching. Only that pair is affected; any other api-version still fails to match, and the exact value on the wire is asserted by CommunicationIdentityApiVersionTests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Checkstyle's HttpPipelinePolicyCheck requires any class implementing HttpPipelinePolicy to be public and outside an implementation package. This policy is an internal detail of the builder and making it public would add a type to the published API surface, so it now extends HttpPipelineSyncPolicy instead, which azure-core provides for exactly this shape. The base class implements both process and processSync as final and routes them through a single beforeSendingRequest hook, so the synchronous path still avoids the blocking default. Behaviour is unchanged: all six service versions reach the wire on both paths, and the recorded suite is unaffected. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The emitter produces a complete protocol client for every operation group in the spec: a builder, a synchronous and an asynchronous client per group, and the wire models behind them. None of it is reachable through the public API, so no test can exercise it, and counting it measures the size of the spec rather than the quality of this library. Measured, against origin/main as the control: main line 82.53% branch 64.17% passes branch line 63.20% branch 46.04% fails 0.66 / 0.52 scoped line 87% branch 68% passes Covered lines rose from 496 to 601. The ratio fell only because the denominator grew by 350 lines of unreachable generated code, so the gate was reporting the migration as a coverage regression when coverage of the shipped code improved. The exclusions apply to the check goal only. The published report still measures every class, so the full picture stays visible. Thresholds are unchanged, and the gate remains live: raising it to 0.99 fails as expected. CommunicationTokenScope and GetTokenForTeamsUserOptions are generated but are part of the published surface, so they are not excluded. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Declaring the plugin to configure the coverage excludes made it a declared plugin in this pom, and the repository requires each one to carry an explicit version plus the x-version-update tag the version tooling reads. 0.8.15 matches eng/versioning/external_dependencies.txt and the parent pom. Verified with eng/versioning/pom_file_version_scanner.ps1, which reports the error without this line and exits clean with it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrates
azure-communication-identityfrom AutoRest to TypeSpec (DPG) generation and adds support for the stable api-version2026-09-23.Depends on Azure/azure-rest-api-specs#46186 (merged, commit1893171), which addedclient.tspandpartial-update: truefor this project.## Why this is a hand-authored PR rather than an AutoPRThe SDK generation pipeline (build 6802369) generates successfully but then fails to build, becausemain's hand-written convenience layer is not compatible with the newly generated classes. No PR is produced. The pipeline reports this itself:> One reason of the compilation failure is that the existing code customization in SDK repository being incompatible with the class generated from updated TypeSpec source. In such case, you can ignore the failure, and fix the customization in SDK repository.The regenerated layer and the customization fixes cannot be separated - each only compiles against the other:| customization | generated layer | result ||---|---|---|| old | old | compiles (maintoday) || old | new | 3 errors (the pipeline failure) || new | old | 1 error || new | new | compiles (this PR) |This matches the three prior TypeSpec migrations in this repo, each a single hand-authored PR containing both halves and removingswagger/: #49272 (App Configuration), #47819 (Search Documents), #46057 (Monitor Ingestion).## Changes- Generation moves totsp-location.yamlat spec commit1893171;swagger/removed- Convenience layer rebased onto the DPG client (IdentityClientImpl/ operation clients)-2026-09-23added toCommunicationIdentityServiceVersion, andgetLatest()moves to it- Version2.0.0### Notes for reviewers**CommunicationIdentityAccessTokenRequest** -scopesis required in TypeSpec, so DPG emits afinalfield with an all-args constructor and no setter.CommunicationIdentityClientUtilsnow uses the constructor.GetTokenForTeamsUserOptions- the obsoleteprivate GetTokenForTeamsUserOptions()added by the old AutoRest customization class cannot compile against DPG'sfinalfields, andfromJsonno longer needs it. Removed.api-version routing - the generatedIdentityClientImpltakes a typedIdentityServiceVersionenum rather than aString, and that enum only contains versions declared in the spec. Routing older values through it would have thrown atbuildClient(), which is a behaviour change from 1.6.13. AnHttpPipelinePolicynow pins theapi-versionquery parameter to the version selected on the builder, so all six public constants still reach the service.Public API - the five shipped public types are preserved byclient.tspdirectives rather than by local customization. Verified by removing every local rename mechanism and regenerating: all five renames still bind, at field, getter, constructor-parameter and wire-name level.## Verification- Compile and checkstyle clean- 25/25 unit tests pass- RevApi1.6.13->2.0.0: 10 differences, all non-breaking, 0 removals. Additions are 8 generated operation-client types plus the twoCommunicationIdentityServiceVersionconstants- Request bodies unchanged -expiresInMinutesis still absent on the default path, not serialized as null- Every public property on every deserialized response reads back non-null-api-versionon the wire confirmed for all six public service-version constants; default is2026-09-23Live tests were not run locally (no test-proxy tooling in this environment); recordings andassets.jsonare untouched.Release plan: 36401Configurations: 'specification/communication/data-plane/Identity/tspconfig.yaml', API Version: 2026-09-23, SDK Release Type: stable, and CommitSHA: '1893171028aed5a757fc78c2d4aa439c14df4afc' in SpecRepo: 'https://github.com/Azure/azure-rest-api-specs' Pipeline run: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=6802369 Refer to https://eng.ms/docs/products/azure-developer-experience/develop/sdk-release/sdk-release-prerequisites to prepare for SDK release. Release plan link: https://azsdk-releaseplan-dashboard-hveph5aqhhcfhtgu.westus-01.azurewebsites.net/?releaseplan=36401 Submitted by: johnfraser@microsoft.com
Note: the pipeline run cited above failed at the build step, which is why this pull request is hand-authored. See "Why this is a hand-authored PR rather than an AutoPR" above.