Add a pathPrefix setting for services mounted under a prefix - #23
Closed
kubukoz wants to merge 2 commits into
Closed
Add a pathPrefix setting for services mounted under a prefix#23kubukoz wants to merge 2 commits into
kubukoz wants to merge 2 commits into
Conversation
The generated client uses each operation's `@http` URI verbatim. That is right when the model describes the whole path, but a service is often mounted under a prefix the model does not mention — a server framework deriving one from a trait, or a reverse proxy — and then every generated request misses it. `pathPrefix` is prepended to each operation's URI. It reaches the Storybook mocks too, split into literal segments, since the mock router matches segment by segment and would otherwise stop matching the client it is standing in for. Normalization accepts what people will actually write: a leading slash is optional, a trailing one is ignored, and `"/"` or whitespace means no prefix. Without that, `"/internal/v1/"` would emit `v1//things`. Deliberately a plain string rather than anything trait-derived: which prefix a deployment sits behind is not a property of the model, and a trait-aware version would have to encode one framework's conventions. Note for release: `generate` gains a parameter. The default value keeps it source-compatible, but it is a binary break, so this wants a minor bump rather than a patch. MiMa cannot see it — the 0.4 baseline has no published artifacts yet.
`generate` gained a `pathPrefix` parameter in the previous commit. The default
value keeps it source-compatible, but a defaulted parameter still changes the
JVM signature, so the two-argument overload disappears and MiMa reports it
against the `0.4.0` baseline:
* static method generate(Model,Set)String in class TsCodegenPlugin
does not have a correspondent in current version
That is a real binary break, not a false positive, so the next release is a
minor one. sbt-typelevel checks the release tag against `tlBaseVersion`, so
this has to move before `v0.5.0` can be tagged.
A filter would have been the wrong tool here. `generate` is the documented
programmatic entry point, so the break is genuinely user-visible — unlike the
`TsWriter` filter dropped when 0.4 opened, which excused a change to a class no
caller could reach.
Member
Author
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.
The generated client builds its URL from each operation's
@httpURI verbatim. That is correct when the model describes the whole path, but a service is often mounted under a prefix the model doesn't mention — a server framework that derives one from a trait, or a reverse proxy — and then every generated request misses it and 404s.This adds a
pathPrefixsetting that is prepended to each operation's URI.{ "plugins": { "ts-codegen": { "outFile": "generated.ts", "pathPrefix": "/internal/v1" } } }Notes on the design
It applies to the Storybook mocks too. The mock router matches path segments one at a time, so the prefix arrives there as literal segments (
{ literal: 'internal' }, { literal: 'v1' }, …) rather than as one concatenated string. Without that the mocks would silently stop matching the very client they stand in for.A plain string, not trait-derived. Which prefix a deployment sits behind isn't a property of the model, and a trait-aware version would have to bake in one framework's conventions. A caller that does have a convention (a trait, a service
versionfield) can compute the string and pass it.Normalization accepts what people actually write. A leading slash is optional, a trailing one is ignored, and
"/"or whitespace means no prefix — so"internal/v1","/internal/v1"and"/internal/v1/"are equivalent. Without it"/internal/v1/"would emitv1//things. There's also an edge case where a URI of just/combined with a prefix must not leave a trailing slash.Default is
"", which is a no-op: the committedtypecheck/src/generated.tssample is byte-identical.Surfaces
All four the existing
excludeServicessetting covers: the smithy-build plugin, the CLI, the sbt plugin (tsCodegenPathPrefix), and the README.Two wiring details worth a look in review:
excludeServices; the sbt plugin sends an empty string for excludes when only a prefix is set. The existing parse already ignored a 4th arg, so nothing changes for current callers.tsCodegenPathPrefixis folded into the sbt plugin's cache key — otherwise changing only the prefix wouldn't re-run codegen.Release note: base version moved to 0.5
generategains a parameter. The default value keeps it source-compatible, but a defaulted parameter still changes the JVM signature, so the two-arg overload disappears — and MiMa caught it against the published0.4.0:So the second commit bumps
tlBaseVersionto0.5. I went with the bump rather than aProblemFilters.exclude:generateis the documented programmatic entry point, so the break is genuinely user-visible, unlike theTsWriterfilter that was dropped when 0.4 opened (a class no caller could reach). Happy to switch to a filter if you would rather keep the next release a patch, but I do not think this one qualifies.Worth flagging that this needs a
v0.5.0tag when it ships — sbt-typelevel checks the release tag againsttlBaseVersion.Verification
scalafmtCheckAll,scalafmtSbtCheck,headerCheckAll,docall green, andmimaReportBinaryIssuesgreen with the 0.5 base version (mimaPreviousArtifactsis empty against an unpublished baseline, which is the point of the bump).git statusrather thantsCodegenSampleCheck— seetsCodegenSampleCheckcan never fail: it regenerates the file it is checking #22 for why that check can't be trusted for this.