tsp, path allowReserved #2904
tsp, path allowReserved #2904
Conversation
| @route("/contoso/{group}") | ||
| @route("/contoso/") | ||
| interface ServerOp { | ||
| get(@path group: url): OkResponse | NoContentResponse; | ||
| get(@path(#{ allowReserved: true }) group: string): OkResponse | NoContentResponse; | ||
| } |
There was a problem hiding this comment.
0.59.0 requires either way
/contoso/{+group}withoutallowReserved(URI template)/contoso/withallowReserved. compiler or http lib would append thegroupto path (kind of the autoRoute)
There was a problem hiding this comment.
Do we want to add another test case using URI template? (/contoso/{+group})? Or cadl-ranch already has case to cover it?
There was a problem hiding this comment.
Timothee is adding a cadl-ranch test.
My local test, that allowReserved does not work if route written as @route("/contoso/{+group}"). Could be the http lib just don't want to support this (say it require we use URI template directly), or it could be a bug.
Same effect on playground (Swagger having x-ms-skip-url-encoding or not).
There was a problem hiding this comment.
Ok, it need to be (no @path)
@route("/contoso/{+group}")
interface ServerOp {
get(group: string): OkResponse | NoContentResponse;
}
There was a problem hiding this comment.
Timothee is adding a cadl-ranch test.
My local test, that
allowReserveddoes not work if route written as@route("/contoso/{+group}"). Could be the http lib just don't want to support this (say it require we use URI template directly), or it could be a bug.Same effect on playground (Swagger having
x-ms-skip-url-encodingor not).
This appears to be a bug in http lib. microsoft/typespec#4132
The getPathParamOptions gives undefined. But param.allowReserved is true. Will report a bug there.
Therefore I switched to param.allowReserved.
| // TODO: deprecate this logic of string/url for x-ms-skip-url-encoding | ||
| if ( | ||
| (param.type === "query" || param.type === "path") && | ||
| param.param.type.kind === "Scalar" && | ||
| schema instanceof UriSchema | ||
| ) { | ||
| extensions = { "x-ms-skip-url-encoding": true }; | ||
| extensions = extensions ?? {}; | ||
| extensions["x-ms-skip-url-encoding"] = true; | ||
| } |
There was a problem hiding this comment.
I didn't deprecate the old logic of uri for skip encoding, string for not skip encoding. I added TODO to these part.
Plan to check later. Let's have support for allowReserved first.
It should be OK to drop such logic in TCGC SdkPackage migration.
There was a problem hiding this comment.
Yeah, TCGC should handle it and returns correct collection format. I am integrating with latest TCGC. I will pay attention to the test case.
weidongxu-microsoft
requested review from
haolingdong-msft,
XiaofeiCao and
srnagar
as code owners
| subprocess.check_call(['tsp-client', '--generate-lock-file'], cwd=sdk_root) | ||
| subprocess.check_call(['tsp-client', 'generate-lock-file'], cwd=sdk_root) |
There was a problem hiding this comment.
tsp-client 0.19.0
$ tsp-client --help
Usage: tsp-client <COMMAND> [OPTIONS]
Use one of the supported commands to get started generating clients from a
TypeSpec project. This tool will default to using your current working directory
to generate clients in and will use it to look for relevant configuration files.
To specify a different directory, use the -o or --output-dir option.
Commands:
init Initialize the SDK project folder from a
tspconfig.yaml
sync Sync TypeSpec project specified in
tsp-location.yaml
generate Generate from a TypeSpec project
update Sync and generate from a TypeSpec project
convert Convert a swagger specification to TypeSpec
generate-lock-file Generate a lock file under the eng/ directory
from an existing emitter-package.json
sort-swagger <swagger-file> Sort a swagger file to be the same content order
with TypeSpec generated swagger
Options:
--version Show version number [boolean]
-d, --debug Enable debug logging [boolean]
-o, --output-dir Specify an alternate output directory for the generated
files. [string] [default: "."]
-y, --no-prompt Skip any interactive prompts. [boolean]
--help Show help [boolean]
fix #2896