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

Fix response examples with union not showing up correctly #4046

Merged
merged 19 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from 13 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: fix
packages:
- "@typespec/compiler"
---

Fix type comparison of literal and scalar when in projection context
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: fix
packages:
- "@typespec/openapi3"
---

Fix issue where operation example woudld produce an empty object when `@body`/`@bodyRoot` was used
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: fix
packages:
- "@typespec/openapi3"
---

Fix operation response body examples showing up for each response.
8 changes: 8 additions & 0 deletions .chronus/changes/fix-response-examples-2024-6-30-22-22-9.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: feature
packages:
- "@typespec/http"
---

API: Expose `properties: HttpProperty[]` on operation parameter and response which reference all the property of intrest(Body, statusCode, contentType, implicitBodyProperty, etc.)
timotheeguerin marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 2 additions & 2 deletions packages/compiler/src/core/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7874,7 +7874,7 @@ export function createChecker(program: Program): Checker {

function isNumericAssignableToNumericScalar(source: Numeric, target: Scalar) {
// if the target does not derive from numeric, then it can't be assigned a numeric literal
if (!areScalarsRelated(target, getStdType("numeric"))) {
if (!areScalarsRelated((target.projectionBase as any) ?? target, getStdType("numeric"))) {
return false;
}

Expand Down Expand Up @@ -7902,7 +7902,7 @@ export function createChecker(program: Program): Checker {
}

function isStringLiteralRelatedTo(source: StringLiteral | StringTemplate, target: Scalar) {
if (!areScalarsRelated(target, getStdType("string"))) {
if (!areScalarsRelated((target.projectionBase as any) ?? target, getStdType("string"))) {
return false;
}
if (source.kind === "StringTemplate") {
Expand Down
53 changes: 29 additions & 24 deletions packages/http/src/http-property.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ export type HttpProperty =

export interface HttpPropertyBase {
readonly property: ModelProperty;
/** Path from the root of the operation parameters/returnType to the property. */
readonly path: (string | number)[];
}

export interface HeaderProperty extends HttpPropertyBase {
Expand Down Expand Up @@ -78,14 +80,17 @@ export interface GetHttpPropertyOptions {
/**
* Find the type of a property in a model
*/
export function getHttpProperty(
function getHttpProperty(
program: Program,
property: ModelProperty,
path: (string | number)[],
options: GetHttpPropertyOptions = {}
): [HttpProperty, readonly Diagnostic[]] {
const diagnostics: Diagnostic[] = [];
function createResult<T extends HttpProperty>(opts: T): [T, readonly Diagnostic[]] {
return [{ ...opts, property } as any, diagnostics];
function createResult<T extends Omit<HttpProperty, "path" | "property">>(
opts: T
): [HttpProperty & T, readonly Diagnostic[]] {
return [{ ...opts, property, path } as any, diagnostics];
}

const annotations = {
Expand All @@ -106,10 +111,9 @@ export function getHttpProperty(
name: property.name,
type: "path",
},
property,
});
}
return [{ kind: "bodyProperty", property }, []];
return createResult({ kind: "bodyProperty" });
} else if (defined.length > 1) {
diagnostics.push(
createDiagnostic({
Expand All @@ -122,24 +126,24 @@ export function getHttpProperty(

if (annotations.header) {
if (annotations.header.name.toLowerCase() === "content-type") {
return createResult({ kind: "contentType", property });
return createResult({ kind: "contentType" });
} else {
return createResult({ kind: "header", options: annotations.header, property });
return createResult({ kind: "header", options: annotations.header });
}
} else if (annotations.query) {
return createResult({ kind: "query", options: annotations.query, property });
return createResult({ kind: "query", options: annotations.query });
} else if (annotations.path) {
return createResult({ kind: "path", options: annotations.path, property });
return createResult({ kind: "path", options: annotations.path });
} else if (annotations.statusCode) {
return createResult({ kind: "statusCode", property });
return createResult({ kind: "statusCode" });
} else if (annotations.body) {
return createResult({ kind: "body", property });
return createResult({ kind: "body" });
} else if (annotations.bodyRoot) {
return createResult({ kind: "bodyRoot", property });
return createResult({ kind: "bodyRoot" });
} else if (annotations.multipartBody) {
return createResult({ kind: "multipartBody", property });
return createResult({ kind: "multipartBody" });
}
compilerAssert(false, `Unexpected http property type`, property);
compilerAssert(false, `Unexpected http property type`);
timotheeguerin marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -161,33 +165,34 @@ export function resolvePayloadProperties(
}

const visited = new Set();
const queue = new Queue<[Model, ModelProperty | undefined]>([[type, undefined]]);
const queue = new Queue<[Model, (string | number)[]]>([[type, []]]);

while (!queue.isEmpty()) {
const [model, rootOpt] = queue.dequeue();
const [model, path] = queue.dequeue();
visited.add(model);

for (const property of walkPropertiesInherited(model)) {
const root = rootOpt ?? property;
const propPath = [...path, property.name];

if (!isVisible(program, property, visibility)) {
continue;
}

let httpProperty = diagnostics.pipe(getHttpProperty(program, property, options));
let httpProperty = diagnostics.pipe(getHttpProperty(program, property, propPath, options));
if (shouldTreatAsBodyProperty(httpProperty, visibility)) {
httpProperty = { kind: "bodyProperty", property };
httpProperty = { kind: "bodyProperty", property, path: propPath };
}
httpProperties.set(property, httpProperty);
if (
property !== root &&
path.length > 0 &&
(httpProperty.kind === "body" ||
httpProperty.kind === "bodyRoot" ||
httpProperty.kind === "multipartBody")
) {
const parent = httpProperties.get(root);
if (parent?.kind === "bodyProperty") {
httpProperties.delete(root);
for (const [name, prop] of httpProperties) {
if (prop?.kind === "bodyProperty") {
httpProperties.delete(name);
}
}
chrisradek marked this conversation as resolved.
Show resolved Hide resolved
}
if (httpProperty.kind === "body" || httpProperty.kind === "multipartBody") {
Expand All @@ -200,7 +205,7 @@ export function resolvePayloadProperties(
type.properties.size > 0 &&
!visited.has(property.type)
) {
queue.enqueue([property.type, root]);
queue.enqueue([property.type, propPath]);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions packages/http/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export { HttpPartOptions } from "../generated-defs/TypeSpec.Http.Private.js";
export * from "./auth.js";
export * from "./content-types.js";
export * from "./decorators.js";
export type { HttpProperty } from "./http-property.js";
export * from "./metadata.js";
export * from "./operations.js";
export * from "./parameters.js";
Expand Down
1 change: 1 addition & 0 deletions packages/http/src/parameters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ function getOperationParametersForVerb(
const body = resolvedBody;

return diagnostics.wrap({
properties: metadata,
parameters,
verb,
body,
Expand Down
3 changes: 2 additions & 1 deletion packages/http/src/responses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,10 @@ function processResponseType(
response.responses.push({
body: resolvedBody,
headers,
properties: metadata,
});
} else {
response.responses.push({ headers });
response.responses.push({ headers, properties: metadata });
}
responses.set(statusCode, response);
}
Expand Down
8 changes: 7 additions & 1 deletion packages/http/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
Tuple,
Type,
} from "@typespec/compiler";
import { HeaderProperty } from "./http-property.js";
import { HeaderProperty, HttpProperty } from "./http-property.js";

/**
* @deprecated use `HttpOperation`. To remove in November 2022 release.
Expand Down Expand Up @@ -332,6 +332,9 @@ export type HttpOperationRequestBody = HttpOperationBody;
export type HttpOperationResponseBody = HttpOperationBody;

export interface HttpOperationParameters {
/** Http properties */
readonly properties: HttpProperty[];

parameters: HttpOperationParameter[];

body?: HttpOperationBody | HttpOperationMultipartBody;
Expand Down Expand Up @@ -441,6 +444,9 @@ export interface HttpOperationResponse {
}

export interface HttpOperationResponseContent {
/** Http properties for this response */
readonly properties: HttpProperty[];

headers?: Record<string, ModelProperty>;
body?: HttpOperationBody | HttpOperationMultipartBody;
}
Expand Down
Loading
Loading