Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rest - filter string constant path http properties when using @autoRoute #6326

Merged
merged 2 commits into from
Mar 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .chronus/changes/rest-prop-filtering-2025-2-7-9-50-32.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: fix
packages:
- "@typespec/rest"
---

Updates `@autoRoute` behavior to apply same HttpOperationParameter filtering to HttpProperty
12 changes: 12 additions & 0 deletions packages/rest/src/rest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ function autoRouteProducer(
const routePath = getRoutePath(program, operation)?.path;
const segments = [...parentSegments, ...(routePath ? [routePath] : [])];
const filteredParameters: HttpOperationParameter[] = [];
const filteredParamProperties = new Set<ModelProperty>();
const paramOptions = {
...(options?.paramOptions ?? {}),
verbSelector: getResourceOperationHttpVerb,
Expand Down Expand Up @@ -146,10 +147,21 @@ function autoRouteProducer(

// Push all usable parameters to the filtered list
filteredParameters.push(httpParam);
filteredParamProperties.add(httpParam.param);
}

// Replace the original parameters with filtered set
parameters.parameters = filteredParameters;
// Remove any header/query/path/cookie properties that aren't in the filtered list
for (let i = parameters.properties.length - 1; i >= 0; i--) {
const httpProp = parameters.properties[i];
if (!["header", "query", "path", "cookie"].includes(httpProp.kind)) {
continue;
}
if (!filteredParamProperties.has(httpProp.property)) {
parameters.properties.splice(i, 1);
}
}

// Add the operation's own segment if present
addSegmentFragment(program, operation, segments);
Expand Down
60 changes: 52 additions & 8 deletions packages/rest/test/routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ describe("rest: routes", () => {
});

it("autoRoute operations filter out path parameters with a string literal type", async () => {
const routes = await getRoutesFor(
const operations = await getOperations(
`
model ThingId {
@path
Expand All @@ -167,16 +167,60 @@ describe("rest: routes", () => {
}
`,
);

deepStrictEqual(routes, [
{
verb: "get",
path: "/subscriptions/{subscriptionId}/providers/Microsoft.Things/things/{thingId}",
params: ["subscriptionId", "thingId"],
},
expect(operations.length).toBe(1);
const operation = operations[0];
expect(operation.verb).toBe("get");
expect(operation.path).toBe(
"/subscriptions/{subscriptionId}/providers/Microsoft.Things/things/{thingId}",
);
const operationParams = operation.parameters.parameters;
const operationProps = operation.parameters.properties;
expect(operationParams.map((p) => p.name)).toEqual(["subscriptionId", "thingId"]);
expect(operationProps.map((p) => (p as any).options.name)).toEqual([
"subscriptionId",
"thingId",
]);
});

it("autoRoute operations does not filter out non-param http properties", async () => {
const operations = await getOperations(
`
model ThingId {
@path
@segment("things")
thingId: string;
}

@autoRoute
interface Things {
@put
op WithFilteredParam(
@path
@segment("subscriptions")
subscriptionId: string,

@path
@segment("providers")
provider: "Microsoft.Things",
...ThingId,

@header contentType: "text/plain",

@body body: "Test Content"
): string;
}
`,
);
expect(operations.length).toBe(1);
const operation = operations[0];
expect(operation.verb).toBe("put");
expect(operation.path).toBe(
"/subscriptions/{subscriptionId}/providers/Microsoft.Things/things/{thingId}",
);
const operationProps = operation.parameters.properties;
expect(operationProps.map((p) => p.kind)).toEqual(["path", "path", "contentType", "body"]);
});

it("emit diagnostic if passing arguments to autoroute decorators", async () => {
const [_, diagnostics] = await compileOperations(`
@autoRoute("/test") op test(): string;
Expand Down
Loading