Skip to content

Propagate Status and Error in typed handler model mapping#374

Merged
shenglol merged 2 commits into
mainfrom
shenglol/fix-model-mapping
Jun 24, 2026
Merged

Propagate Status and Error in typed handler model mapping#374
shenglol merged 2 commits into
mainfrom
shenglol/fix-model-mapping

Conversation

@shenglol

Copy link
Copy Markdown
Contributor

Summary

TypedResourceOperationHandler dropped the Resource.Status / Resource.Error fields (and ResourcePreview.Status) when mapping between the strongly-typed handler models and the untyped V2 contract models.

This silently broke the RELO (resource-based long-running operation) flow for any handler built on the typed base classes: a non-terminal status (e.g. "Running") or a terminal "Failed" + error set on the typed result never reached the untyped response. Because Get/Delete also funnel through ToResource (via ToNullableResource), polling never observed the status, so RELO appeared to complete synchronously.

LRO was unaffected, since LongRunningOperation is returned via passthrough rather than through these mappers.

Changes

  • ToResource (typed → untyped) and ToTypedResource (untyped → typed): copy Status and Error.
  • ToResourcePreview and ToTypedResourcePreview: copy Status (ResourcePreview has no Error field).
  • New project Azure.Deployments.Extensibility.AspNetCore.Tests.Unit with regression tests driving real typed handlers through the public interface and asserting Status/Error survive the round trip (create, get, delete, preview). These fail on the previous code and pass now.

Testing

  • New tests: 4/4 pass.
  • Full solution builds clean (purely additive change).

Rollback

Revert this commit. The change only adds field assignments in the model-mapping helpers, so reverting restores the prior behavior with no migration or state concerns.

TypedResourceOperationHandler dropped the Resource Status/Error fields (and ResourcePreview Status) when mapping between strongly-typed and untyped contract models. This silently broke the RELO (resource-based long-running operation) flow for typed handlers: a status/error set on the typed result never reached the untyped response, so the host never observed it.

Fix ToResource/ToTypedResource to copy Status and Error, and ToResourcePreview/ToTypedResourcePreview to copy Status (ResourcePreview has no Error). Add an AspNetCore.Tests.Unit project with regression tests covering create/get/delete/preview mapping.
Copilot AI review requested due to automatic review settings June 23, 2026 03:13
@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.27%. Comparing base (ea99ce1) to head (8e0a236).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #374      +/-   ##
==========================================
+ Coverage   45.04%   52.27%   +7.23%     
==========================================
  Files         108      108              
  Lines        2100     2106       +6     
  Branches      235      235              
==========================================
+ Hits          946     1101     +155     
+ Misses       1111      954     -157     
- Partials       43       51       +8     
Files with missing lines Coverage Δ
...pNetCore/Handlers/TypedResourceOperationHandler.cs 65.04% <100.00%> (+65.04%) ⬆️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a regression in the typed handler model mappers so RELO (resource-based long-running operations) can correctly propagate Resource.Status / Resource.Error (and ResourcePreview.Status) between strongly-typed handler models and the untyped V2 contract models.

Changes:

  • Propagate Status and Error in ToResource / ToTypedResource mappings.
  • Propagate Status in ToResourcePreview / ToTypedResourcePreview mappings.
  • Add a new unit test project with regression tests covering create/get/delete/preview round-trips via the public handler interfaces.

Reviewed changes

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

File Description
src/Azure.Deployments.Extensibility.AspNetCore/Handlers/TypedResourceOperationHandler.cs Adds Status/Error propagation to typed/untyped resource mapping helpers.
src/Azure.Deployments.Extensibility.AspNetCore.Tests.Unit/Handlers/TypedResourceMappingTests.cs Adds regression tests ensuring Status/Error survive typed↔untyped mapping via handler interfaces.
src/Azure.Deployments.Extensibility.AspNetCore.Tests.Unit/Azure.Deployments.Extensibility.AspNetCore.Tests.Unit.csproj Introduces new unit test project for AspNetCore typed mapping tests.
Azure.Deployments.Extensibility.sln Adds the new unit test project to the solution.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Azure.Deployments.Extensibility.sln Outdated
- Only set Error alongside a Failed status in mapping tests (the V2 contract requires error only when status is Failed); split create into non-terminal and failed cases and add untyped-to-typed (ToTypedResource/ToTypedResourcePreview) coverage.

- Add [assembly: AssemblyTrait(Category, Unit)] so the CI 'Category=Unit' filter runs these tests (fixes 0% patch coverage).

- Use the SDK-style project type GUID for the new test project in the solution.
@shenglol shenglol merged commit 286d742 into main Jun 24, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants