Uh oh!
There was an error while loading. Please reload this page.
[cdac] Add default (empty-version) contract registration logic - #129827
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a convention in cdac contract registration where an empty-string version ("") represents a default implementation that can be used only when the target does not advertise any version for that contract. The production CachingContractRegistry and the test registries are updated to honor this fallback behavior, and new unit tests are added to validate the rule.
Changes:
- Document the empty-string default registration convention on
ContractRegistry.Register. - Implement default (empty-version) fallback resolution in
CachingContractRegistry.TryGetContractwhen the target does not advertise a contract version. - Update test contract registries to follow the same rule and add new unit tests covering the fallback and version-skew behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/tests/UnitTests/DefaultContractRegistrationTests.cs | Adds unit tests for empty-version default registration and version-skew behavior (currently targets the test registry). |
| src/native/managed/cdac/tests/TestInfrastructure/TestPlaceholderTarget.cs | Updates the test contract registry to fall back to "" registration only when no version is advertised. |
| src/native/managed/cdac/tests/DataGenerator/TestTarget.cs | Simplifies the DataGenerator test registry wiring and makes it honor the "" default resolution behavior. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/CachingContractRegistry.cs | Implements the production fallback logic: use "" registration only when the target doesn’t advertise a version. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs | Documents the empty-string fallback convention on Register. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Establishes a convention for registering a contract implementation that is used
when the target does not advertise a version for that contract:
- Register("", creator) registers an implementation under the empty-string version.
- In TryGetContract, when the target does not advertise a version for the contract,
the registry falls back to the empty-string registration if one is present.
- When the target does advertise a version, an exact-version match is still required
and a missing implementation fails (the fallback does not kick in), preserving
version-skew detection.
This lets host-side contracts (which have no target-side data and are not advertised
by the target's contract descriptor) be wired into the existing
ContractRegistry.Register(version, creator) API without adding a new method.
Adds unit tests that exercise the production CachingContractRegistry through a real
ContractDescriptorTarget (extending ContractDescriptorBuilder.TryCreateTarget to
accept custom contract registrations).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>24cd158 to
48adc33CompareTagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
## Summary Extracts the **default (empty-version) contract registration logic** from #129675 into a standalone change. This establishes a convention on `ContractRegistry` for registering a contract implementation that is used as a fallback when the target does not advertise a version for that contract: - **`Register("", creator)`** registers an implementation under the **empty-string version**. - In `TryGetContract`, when the target does **not** advertise a version for the contract, the registry falls back to the empty-string registration if one is present. - When the target **does** advertise a version, an exact-version match is still required, and a missing implementation fails (the fallback does **not** kick in) — preserving version-skew detection. ## Changes - `CachingContractRegistry.cs` — implement the empty-version fallback in `TryGetContract`. - `ContractDescriptorBuilder.cs` — `TryCreateTarget` now accepts optional `contractRegistrations` (defaulting to `CoreCLRContracts.Register`) so a test can register a contract implementation. - `ContractRegistrationTests.cs` (new) — exercises the production `CachingContractRegistry` through a **real** `ContractDescriptorTarget` (via the public `Target.Contracts` API), across all standard architectures: - no advertised version → falls back to the default (`""`) registration - no advertised version and no default registration → fails - advertised version → the versioned registration wins over the default - advertised version with no matching implementation → version-skew failure (does **not** fall back to the default) ## Testing All cdac unit tests pass: `Microsoft.Diagnostics.DataContractReader.Tests` 2611 passed / 16 skipped / 0 failed (including the 16 new test cases), and `DataGenerator.Tests` 34/34. Co-authored-by: Max Charlamb <maxcharlamb@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Extracts the default (empty-version) contract registration logic from #129675 into a standalone change.
This establishes a convention on
ContractRegistryfor registering a contract implementation that is used as a fallback when the target does not advertise a version for that contract:Register("", creator)registers an implementation under the empty-string version.TryGetContract, when the target does not advertise a version for the contract, the registry falls back to the empty-string registration if one is present.Changes
CachingContractRegistry.cs— implement the empty-version fallback inTryGetContract.ContractDescriptorBuilder.cs—TryCreateTargetnow accepts optionalcontractRegistrations(defaulting toCoreCLRContracts.Register) so a test can register a contract implementation.ContractRegistrationTests.cs(new) — exercises the productionCachingContractRegistrythrough a realContractDescriptorTarget(via the publicTarget.ContractsAPI), across all standard architectures:"") registrationTesting
All cdac unit tests pass:
Microsoft.Diagnostics.DataContractReader.Tests2611 passed / 16 skipped / 0 failed (including the 16 new test cases), andDataGenerator.Tests34/34.