Fix polyglot AppHost builder and callback generation#16691
Open
sebastienros wants to merge 4 commits intomainfrom
Open
Fix polyglot AppHost builder and callback generation#16691sebastienros wants to merge 4 commits intomainfrom
sebastienros wants to merge 4 commits intomainfrom
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16691Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16691" |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes several polyglot AppHost codegen/transport edge cases so generated Python/Java/Go AppHosts preserve the correct AppHost source path and callback behavior (positional JSON-RPC params, DTO/handle marshaling, and DTO writeback) matches the declared signatures.
Changes:
- Update ATS callback argument marshaling to use the declared CLR parameter type (not runtime type) for handles/arrays, with a new regression test.
- Improve Java JSON-RPC transport callback handling (array-style params + positional
pNargs) and ensure DTO values serialize as maps via aJsonSerializablecontract. - Adjust Python/Go builder defaults to respect
ASPIRE_APPHOST_FILEPATH(and fix Go callback DTO writeback), with focused generator regression tests + snapshot updates.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.RemoteHost.Tests/CallbackProxyTests.cs | Adds regression coverage for handle marshaling using declared callback parameter types. |
| src/Aspire.Hosting.RemoteHost/Ats/AtsMarshaller.cs | Introduces marshaling overload that honors declared CLR types when producing JSON. |
| src/Aspire.Hosting.RemoteHost/Ats/AtsCallbackProxyFactory.cs | Passes declared parameter types into marshalling during proxy invocation argument packing. |
| src/Aspire.Hosting.CodeGeneration.Python/PythonModuleBuilder.cs | Updates generated create_builder to accept/apply app_host_file_path and default it from ASPIRE_APPHOST_FILEPATH. |
| tests/Aspire.Hosting.CodeGeneration.Python.Tests/AtsPythonCodeGeneratorTests.cs | Adds regression assertions for Python create_builder defaults. |
| tests/Aspire.Hosting.CodeGeneration.Python.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.py | Snapshot update reflecting Python builder option defaults. |
| tests/Aspire.Hosting.CodeGeneration.Python.Tests/Snapshots/AtsGeneratedAspire.verified.py | Snapshot update reflecting Python builder option defaults. |
| src/Aspire.Hosting.CodeGeneration.Java/Resources/Transport.java | Enhances callback request parsing (array/object params) and DTO serialization hook. |
| src/Aspire.Hosting.CodeGeneration.Java/AtsJavaCodeGenerator.cs | Emits DTOs implementing JsonSerializable so they serialize as maps. |
| tests/Aspire.Hosting.CodeGeneration.Java.Tests/AtsJavaCodeGeneratorTests.cs | Adds regression tests for array callback params + DTO map serialization in generated Java. |
| tests/Aspire.Hosting.CodeGeneration.Java.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.java | Snapshot update for transport changes + DTO serialization behavior. |
| tests/Aspire.Hosting.CodeGeneration.Java.Tests/Snapshots/AtsGeneratedAspire.verified.java | Snapshot update for transport changes + DTO serialization behavior. |
| src/Aspire.Hosting.CodeGeneration.Go/Resources/base.go | Adds fallback struct-field decoding when full JSON unmarshal fails (used for DTO callback args). |
| src/Aspire.Hosting.CodeGeneration.Go/AtsGoCodeGenerator.cs | Emits Go callback shims that return mutated DTO args + fixes CreateBuilder defaults/omissions. |
| tests/Aspire.Hosting.CodeGeneration.Go.Tests/AtsGoCodeGeneratorTests.cs | Adds regression tests for Go CreateBuilder defaults and DTO callback writeback behavior. |
| tests/Aspire.Hosting.CodeGeneration.Go.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.go | Snapshot update for Go callback writeback + CreateBuilder defaults. |
| tests/Aspire.Hosting.CodeGeneration.Go.Tests/Snapshots/AtsGeneratedAspire.verified.go | Snapshot update for Go CreateBuilder defaults. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🎬 CLI E2E Test Recordings — 69 recordings uploaded (commit View all recordings
📹 Recordings uploaded automatically from CI run #25325202639 |
Co-authored-by: Copilot <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.
Summary
ASPIRE_APPHOST_FILEPATH.CreateBuilder(nil)sends zero-value options.eShop parity validation
The eShop polyglot AppHosts were validated from the external
dotnet/eshopclonesrc/eShop.AppHost.TypeScript/apphost.tssrc/eShop.AppHost.Python/apphost.pysrc/eShop.AppHost.Java/AppHost.javasrc/eShop.AppHost.Go/apphost.goAppHost source used for validation
src/eShop.AppHost.TypeScript/apphost.tssrc/eShop.AppHost.Python/apphost.pysrc/eShop.AppHost.Java/AppHost.javasrc/eShop.AppHost.Go/apphost.goValidation
dotnet test --project tests/Aspire.Hosting.CodeGeneration.Go.Tests/Aspire.Hosting.CodeGeneration.Go.Tests.csproj --no-launch-profile -- --filter-class "*.AtsGoCodeGeneratorTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"dotnet run --project src/Aspire.Cli/Aspire.Cli.csproj -- start --isolated --debug --no-build --non-interactive --format Json --apphost <file>.