Repository navigation
Use TypeSpec-generated Queue dispatch metadata - #2831
Draft
iscai-msft wants to merge 32 commits into
Draft
iscai-msft wants to merge 32 commits into
iscai-msft wants to merge 32 commits into
Conversation
… toolchain check Adds a copy-pasted, unmodified output of an internal pilot/prototype TypeSpec emitter (@azure-tools/typespec-azurite-emitter-pilot, see companion PR Azure/typespec-azure#5614), run against a small toy "Queue-like" fixture, plus a Mocha test (tagged @loki so it runs under the existing npm test / npm run test:in-memory scripts, no server/storage backend required) asserting the generated route metadata and handler interface carry the structural information this repo's real dispatch.middleware.ts / IQueueHandler.ts rely on. Running the generated code through this repo's own `npm run build` caught a real bug in the pilot emitter: operations.ts referenced model types (request/ response bodies) without importing them from models.ts, which has since been fixed upstream and is reflected in the regenerated files here. `npm run build`, `npm run lint`, and the new Mocha test all pass with these files in place. This does not wire anything into src/queue/generated/** or Azurite's real dispatch pipeline -- it's purely to let the Azurite team run/inspect the pilot's generated code with this repo's own toolchain. Not intended to be merged as-is. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
iscai-msft
pushed a commit
to iscai-msft/typespec-azure
that referenced
this pull request
Oct 5, 2026
Running the pilot emitter's generated output through a real end-to-end check against Azure/Azurite's own `tsc` build (see Azure/Azurite#2831) surfaced a compile error: operations.ts referenced model types used as request/response bodies (e.g. a model body for a PUT, or a model response body) without importing them from models.ts, failing with TS2304 "Cannot find name". Adds `collectModelRefs` in type-ref.ts to walk a ServerTypeRef (including through array element types) and collect referenced model names, and uses it in render-operations.ts to emit a single `import type { ... } from "./models.js";` line when needed. Adds a unit test in render.test.ts and an e2e regression test in e2e.test.ts. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
This explicitly non-mergeable pilot requires human evaluation before any merge decision.
Review effort: Balanced
Findings: None
What changed in this PR
Adds an isolated TypeSpec-emitter pilot to check generated artifacts with Azurite’s existing TypeScript toolchain, without changing production services.
Changes:
- Adds a toy Queue-like fixture and documentation overlay.
- Includes generated models, operation metadata, and handler interfaces.
- Adds six structural tests; validation was not rerun during this review.
| File | Description |
|---|---|
| typespec-emitter-pilot/README.md | Explains pilot purpose, evidence, and limitations. |
| typespec-emitter-pilot/generated/operations.ts | Defines operation types and routing metadata. |
| typespec-emitter-pilot/generated/models.ts | Defines fixture data models. |
| typespec-emitter-pilot/generated/handlers.ts | Defines typed handlers with request context. |
| typespec-emitter-pilot/fixture/base.tsp | Models three toy Queue-like operations. |
| typespec-emitter-pilot/fixture/azurite.tsp | Adds emulator-specific documentation augments. |
| tests/typespec-emitter-pilot/generatedArtifacts.test.ts | Checks metadata and handler structure. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Addresses feedback that simply dropping generated files alongside existing code doesn't prove anything actually uses them. Adds: - server/dispatcher.ts: a hand-written Express dispatcher whose routes are built purely from the generated operations metadata table (method, path template, parameter bindings) rather than hand-written route strings, and which shapes real HTTP responses (status/headers/body) using the generated per-status OperationResponseMetadata. - server/inMemoryQueueHandler.ts: a hand-written InMemoryQueueHandler class implementing IServiceHandler -- real (if minimal) in-memory business logic filling in the generated handler interface, analogous to Azurite's real hand-written src/queue/handlers/QueueHandler.ts. - server/createPilotServer.ts: wires the two into a real express.Express app. - tests/typespec-emitter-pilot/pilotServer.e2e.test.ts: starts that app on an ephemeral port and drives it with real HTTP requests (global fetch), proving the full path -- HTTP request -> generated-metadata dispatch -> hand-written handler logic -> generated-metadata response shaping -> real HTTP response -- actually works end to end. Wiring this up for real (rather than just asserting structural shape) surfaced one more metadata gap: response metadata only carried header wire names, not the TS property name a dispatcher needs to read off a handler's typed result object. Fixed in the companion Azure/typespec-azure PR (OperationResponseMetadata.headers now carries name/wireName pairs); the generated operations.ts here is regenerated with that fix. npm run build, npm run lint, and all 10 Mocha tests (6 structural + 4 new end-to-end) pass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CI showed Azurite_Docker and Azurite_Windows_Docker failing: both Dockerfiles
only `COPY src ./src` and `COPY tests ./tests` into the build image, so the
top-level `typespec-emitter-pilot/` directory (containing the generated
artifacts and the hand-written server layer) was missing entirely, and
`npm run build` inside the container failed with `TS2307: Cannot find
module` for every file that imported from it (plus cascading TS7006
implicit-any errors in the same files once their imports failed to resolve).
Rather than patching both Dockerfiles to add another COPY line, move
`typespec-emitter-pilot/{generated,server,fixture,README.md}` to
`src/queue/typespecPilot/` so they're automatically included by the existing
`COPY src ./src` step and by `npm run build`/`npm run lint` with no build
config changes anywhere. Updated the two test files' relative imports
accordingly (tests/typespec-emitter-pilot/*.test.ts -> ../../src/queue/
typespecPilot/...) and the README's self-references.
Verified locally: `npm run build`, `npm run lint`, and all 10 Mocha tests
pass with this layout (confirmed the original failure reproduces locally by
temporarily removing the directory from its old root-level location, and
that moving it under src/ fixes it).
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…atcher/handler to depend on it Replaces the toy 3-operation fixture with the real, unmodified Azure Storage Queue TypeSpec (specification/storage/data-plane/QueueStorage/, see fixture/storage-queue-real/PROVENANCE.md for exact source commit) plus a real azurite.tsp overlay citing this repo's own documented swagger/queue.md customizations. - Vendored the real spec + overlay; generated models.ts/operations.ts/handlers.ts with the pilot emitter (Azure/typespec-azure#5614) - all 17 real Queue operations, zero diagnostics, zero skipped. - Rewrote server/dispatcher.ts for real routing: queue-scoped vs service-scoped operation classification (documenting the queueName client-scoping gap this surfaced - see README's "What this surfaced"), literal-query-string route disambiguation (?comp=metadata, ?comp=acl, ?peekonly=true, etc.), and real numeric parameter names. - Added server/realQueueHandler.ts: in-memory IServiceHandler implementation of all 17 real operations, replacing the toy inMemoryQueueHandler.ts. - Rewrote both test suites against the real spec's shape: generatedArtifacts.test.ts adds structural assertions (route metadata, parameter bindings, response header wire-name mapping, overlay @@doc override, handler shape, and an explicit test pinning down the queueName-absent-from-metadata gap) citing specific Azurite files compared against (dispatch.middleware.ts, IQueueHandler.ts, Context.ts). pilotServer.e2e.test.ts drives real HTTP requests through the real routes, including same-path/same-verb disambiguation and a full message lifecycle. - Rewrote the pilot README to describe the real-spec vendoring, the queueName-client-scoping gap (the most important finding), the literal-query dispatcher workaround, and updated scope/out-of-scope notes. npm run build / npm test (lint + full mocha suite, incl. the 13 pilot tests) all green. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…n base spec by hash instead of vendoring Replaces the previous parallel, in-memory reimplementation (realQueueHandler.ts) and its hand-rolled Express-route dispatcher (dispatcher.ts) with a single new file, pilotDispatchMiddleware.ts, that matches requests to one of the 17 pilot-emitter-generated operations and invokes Azurite's REAL, unmodified QueueHandler / ServiceHandler / MessagesHandler / MessageIdHandler business logic against real in-memory Loki/MemoryExtentStore persistence, reusing Azurite's real queueStorageContext middleware, Context/ QueueStorageContext holder objects, and error/end middleware unchanged throughout. Also stops vendoring the real Storage Queue TypeSpec source files (main.tsp/models.tsp/routes.tsp/client.tsp) in this repo. Only the Azurite-authored azurite.tsp overlay is committed; the base spec is pinned by commit hash in PROVENANCE.md, with instructions for fetching it to regenerate, mirroring the real migration design where Azurite never owns a copy of the Storage TypeSpec it doesn't author. README.md and PROVENANCE.md rewritten to describe this architecture and cite exactly which real Azurite files are reused unmodified vs. newly written. Verified: tsc --noEmit clean, npm run build clean, eslint clean, and all 14 typespec-emitter-pilot tests pass in isolation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
iscai-msft
changed the title
Leftover path references from the earlier directory rename were missed in the previous commit; this just catches up README.md and the two test files' comments/citations to the renamed fixture/storage-queue/ directory. No behavior change (14/14 tests still pass). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…t server
Previously this integration added two new hand-written files
(createPilotServer.ts, pilotDispatchMiddleware.ts, ~600 lines) running a
separate Express app alongside Azurite's real server. That didn't actually
exercise Azurite's real request pipeline end to end.
This commit replaces that with a minimal, true in-place integration:
- src/queue/generated/middleware/dispatch.middleware.ts (the real,
AutoRest-generated file - the one piece of Azurite's real Queue pipeline
whose only job is choosing context.operation, with no involvement in
body/header (de)serialization is patched to select the matched operation
using @azure-tools/typespec-azurite-emitter's generated operations.ts
metadata instead of the original msRest.OperationSpec-based matching.
Resource classification (queue vs. messages vs. message-id) is derived
generically from the emitter's new interfaceName field plus the
operation's path shape - not a hardcoded per-operation-name table.
Same-path/verb collisions (e.g. Create vs SetMetadata) are disambiguated
by a specificity score (most matched literal/required conditions wins),
mirroring the original dispatcher's behavior.
- createPilotServer.ts and pilotDispatchMiddleware.ts are deleted entirely -
no parallel server remains.
- generated/{models,operations,handlers}.ts regenerated against the updated
emitter (adds interfaceName to every operation).
- tests/typespec-emitter-pilot/pilotServer.e2e.test.ts rewritten to boot the
REAL QueueServer via the existing QueueTestServerFactory test harness and
drive it with a real @azure/storage-queue SDK client (real shared-key
auth, real XML wire format) - proving the full real pipeline (auth,
patched dispatch, real deserializer/handler/persistence/serializer
middleware) works end to end.
- tests/typespec-emitter-pilot/dispatchMiddleware.test.ts added: focused,
HTTP-free unit tests of the patched dispatchMiddleware function covering
same-bucket/verb disambiguation, unroutable-request rejection, and the
documented GetUserDelegationKey gap (no real Operation enum member/handler
exists for it in Azurite today).
- generatedArtifacts.test.ts updated to assert interfaceName values and to
reference the real patched dispatch.middleware.ts instead of the deleted
pilotDispatchMiddleware.ts.
- README.md rewritten to describe the simpler, single-file-patch
architecture.
Verified: full existing real Queue test suite (92 tests covering auth, CORS,
SAS, special naming, messages, and all Queue/Service APIs) passes unchanged
against the patched dispatcher - no regressions. All 21 typespec-emitter-pilot
tests pass. tsc --noEmit and eslint are clean.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
EOF
)
Apply the Azurite overlay's AccessPolicy optionality changes, regenerate Queue artifacts, and simplify the integration documentation/tests around wiring-only changes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved compatibility regressions affect supported Queue requests, persisted settings, and response behavior.
0 open findings
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
iscai-msft
commented
Oct 8, 2026
iscai-msft
left a comment
iscai-msft
left a comment
Member
Author
There was a problem hiding this comment.
A few reading cues for the generated metadata and old/new Queue compatibility boundaries.
| import { ServiceSetPropertiesMetadata, ServiceGetPropertiesMetadata, ServiceGetStatisticsMetadata, GetUserDelegationKeyMetadata, ServiceListQueuesSegmentMetadata, QueueCreateMetadata, QueueGetPropertiesMetadata, QueueDeleteMetadata, QueueSetMetadataMetadata, QueueGetAccessPolicyMetadata, QueueSetAccessPolicyMetadata, MessagesDequeueMetadata, MessagesClearMetadata, MessagesEnqueueMetadata, MessagesPeekMetadata, MessageIdUpdateMetadata, MessageIdDeleteMetadata } from "./operations"; | ||
|
|
||
| export type { OperationMetadata, ServiceMetadata } from "../runtime/serializationRuntime"; | ||
| export const serviceMetadata: ServiceMetadata = defineServiceMetadata({ operations: [ServiceSetPropertiesMetadata, ServiceGetPropertiesMetadata, ServiceGetStatisticsMetadata, GetUserDelegationKeyMetadata, ServiceListQueuesSegmentMetadata, QueueCreateMetadata, QueueGetPropertiesMetadata, QueueDeleteMetadata, QueueSetMetadataMetadata, QueueGetAccessPolicyMetadata, QueueSetAccessPolicyMetadata, MessagesDequeueMetadata, MessagesClearMetadata, MessagesEnqueueMetadata, MessagesPeekMetadata, MessageIdUpdateMetadata, MessageIdDeleteMetadata], xmlModels: [QueueServicePropertiesXmlMetadata, LoggingXmlMetadata, RetentionPolicyXmlMetadata, MetricsXmlMetadata, CorsRuleXmlMetadata, QueueServiceStatsXmlMetadata, GeoReplicationXmlMetadata, KeyInfoXmlMetadata, UserDelegationKeyXmlMetadata, ListQueuesResponseXmlMetadata, QueueItemXmlMetadata, SignedIdentifiersXmlMetadata, SignedIdentifierXmlMetadata, AccessPolicyXmlMetadata, ReceivedMessagesXmlMetadata, ReceivedMessageXmlMetadata, QueueMessageXmlMetadata, ListOfSentMessageXmlMetadata, SentMessageXmlMetadata, PeekedMessagesXmlMetadata, PeekedMessageXmlMetadata] }); |
Member
Author
There was a problem hiding this comment.
This is the generated service manifest: it only gathers references to descriptors owned by models.ts and operations.ts, so there isn’t a second mapping graph to keep in sync.
|
|
||
| import { serviceMetadata } from "./metadata"; | ||
|
|
||
| const runtime = createSerializationRuntime(serviceMetadata); |
Member
Author
There was a problem hiding this comment.
This thin binding is the generated/handwritten boundary: service-specific metadata stays generated, while the shared wire behavior lives in runtime/serializationRuntime.ts.
| const method = handler[methodName] as unknown as GeneratedHandlerMethod; | ||
|
|
||
| method | ||
| .call(handler, context.handlerParameters, { |
Member
Author
There was a problem hiding this comment.
The pilot bridge deliberately stops here: it selects the generated IServiceHandler method and passes the generated parameter object through unchanged; middleware still owns HTTP conversion.
| // We assume handlerPath always exists for every generated operation in generated code | ||
| const handlerPath = getHandlerByOperation(context.operation)!; | ||
| const metadata = getGeneratedOperation(context.operation); | ||
| const handlerParameters = adaptGeneratedParameters( |
Member
Author
There was a problem hiding this comment.
This is a temporary migration seam for production QueueServer: it reshapes the new flat generated parameters into the split positional/options contract used by the existing handlerMappers and handlers.
| const legacyHeadOperations: readonly DispatchOperationMetadata[] = [ | ||
| // Keep the two HEAD routes from generated/artifacts/specifications.ts until TypeSpec declares them. | ||
| { | ||
| name: "Queue_GetPropertiesWithHead", |
Member
Author
There was a problem hiding this comment.
These two HEAD routes stay on the legacy path because they aren’t in the pinned Queue TypeSpec yet; the filter below drops the fallback automatically once generated metadata includes them.
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.
What this is
Companion draft PR for Azure/typespec-azure#5614. It demonstrates that Queue artifacts generated by
@azure-tools/typespec-azurite-emitterfrom the real pinned Azure Storage Queue TypeSpec plus Azurite'sazurite.tspoverlay can drive Azurite's real Queue dispatch path and the first generated serializer/deserializer slice.What's changed
src/queue/typespecPilot/fixture/storage-queue/azurite.tsp, which references the pinned Storage Queue TypeSpec commit documented inPROVENANCE.md.@@maxValuefor the visibility-timeout customization and@@makeOptionalforAccessPolicy.start,AccessPolicy.expiry, andAccessPolicy.permission.@@clientNameto align generated operation metadata names with Azurite's existingOperationenum names for currently supported Queue operations.models.ts,operations.ts,handlers.ts, andserialization.tsfor all 17 real Queue operations.src/queue/generated/middleware/dispatch.middleware.tsso it matches requests against the new generated operation metadata and assigns the existingOperationenum.deserializer.middleware.tsandserializer.middleware.tsto prefer TypeSpec-generateddeserializeRequest/serializeResponsehelpers for the no-body Queue slice, falling back to existing generated specs for unmigrated XML body operations.@azure/storage-queue.Handwritten wiring only
The emitter generates service/protocol metadata, no-body request/response serialization helpers, and honors TCGC operation names. The temporary coexistence bridge in Azurite is still
Operation[operation] -> generated operation metadata/helper, with no explicit old-name/new-name mapping table.The dispatcher still uses Azurite's existing
context.dispatchPatternrouting and the same method/path/query/header matching shape as the old generated dispatcher. Serializer/deserializer middleware now uses generated helper functions when available and falls back to existing generated specs for operations not yet covered by TypeSpec-generated XML body serialization.GetUserDelegationKeystill generates metadata from the Storage Queue TypeSpec, but remains unrouted because Azurite currently has no Queue handler orOperationenum member for it.Validation
Local validation passed:
npm run buildnpm test -- --grep "TypeSpec emitter pilot|patched dispatch.middleware.ts"(24 passing)npm run test:queue(92 passing)