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

http-client-java, adopt TCGC usage and access #4854

Draft
wants to merge 33 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
41cf4bf
code to use TCGC usage and access
haolingdong-msft Sep 18, 2024
efc36ec
refine logic
haolingdong-msft Sep 18, 2024
f744324
Merge remote-tracking branch 'remote/main' into http-client-java-adop…
haolingdong-msft Sep 18, 2024
374e3a6
remove track usage logic in process response
haolingdong-msft Sep 19, 2024
e6612bf
logics for handling lro schema usage
haolingdong-msft Sep 19, 2024
3fe361f
Merge remote-tracking branch 'remote/main' into http-client-java-adop…
haolingdong-msft Sep 20, 2024
d989457
solve conflict
haolingdong-msft Sep 20, 2024
de378a1
solve conflict
haolingdong-msft Sep 20, 2024
1b17f84
Merge remote-tracking branch 'remote/main' into http-client-java-adop…
haolingdong-msft Sep 29, 2024
c3f65c7
remove usage tracking
haolingdong-msft Sep 30, 2024
14bdceb
do not track usage if it is api version
haolingdong-msft Oct 9, 2024
00ef14a
regen
haolingdong-msft Oct 10, 2024
6ec2098
Merge branch 'main' into http-client-java-adopt-tcgc-access-usage
haolingdong-msft Oct 17, 2024
40cb55c
Merge remote-tracking branch 'remote/main' into http-client-java-adop…
haolingdong-msft Oct 24, 2024
846439f
Merge remote-tracking branch 'remote/main' into http-client-java-adop…
haolingdong-msft Oct 24, 2024
005ab21
not generate the model if usageFlag === 0
haolingdong-msft Oct 24, 2024
272b8df
regen
haolingdong-msft Oct 24, 2024
515b134
update multiple content types case
haolingdong-msft Oct 24, 2024
e5947e3
Remove unnecessary trackSchemaUsage
haolingdong-msft Oct 25, 2024
8239160
update test case as the definition is invalid
haolingdong-msft Oct 25, 2024
1589891
update multi content type case
haolingdong-msft Oct 25, 2024
d1a6c12
remove enpty space
haolingdong-msft Oct 25, 2024
a5dadd5
update multi content type case
haolingdong-msft Oct 25, 2024
823f91a
regen
haolingdong-msft Oct 30, 2024
7215520
remove unnecessary trackSchemaUsage
haolingdong-msft Oct 30, 2024
a190200
regen
haolingdong-msft Oct 30, 2024
8d4bbb3
code clean up
haolingdong-msft Oct 30, 2024
d140128
code clean up
haolingdong-msft Oct 30, 2024
812a7d4
Merge remote-tracking branch 'remote/main' into http-client-java-adop…
haolingdong-msft Nov 15, 2024
813b1a6
regen
haolingdong-msft Nov 15, 2024
3ef0796
code clean up
haolingdong-msft Nov 15, 2024
cb58aa0
format
haolingdong-msft Nov 15, 2024
cde7d30
format
haolingdong-msft Nov 15, 2024
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
205 changes: 31 additions & 174 deletions packages/http-client-java/emitter/src/code-model-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,9 @@ import {
getUsage,
isStable,
modelIs,
processSchemaUsageFromSdkType,
pushDistinct,
trackSchemaUsage,
} from "./type-utils.js";
import {
getNamespace,
Expand Down Expand Up @@ -265,7 +267,7 @@ export class CodeModelBuilder {

this.processModels();

this.processSchemaUsage();
this.postProcessSchemaUsage();

this.deduplicateSchemaName();

Expand All @@ -280,9 +282,6 @@ export class CodeModelBuilder {
parameter = this.createApiVersionParameter(arg.name, ParameterLocation.Uri);
} else {
const schema = this.processSchema(arg.type, arg.name);
this.trackSchemaUsage(schema, {
usage: [SchemaContext.Input, SchemaContext.Output /*SchemaContext.Public*/],
});
parameter = new Parameter(arg.name, arg.doc ?? "", schema, {
implementation: ImplementationLocation.Client,
origin: "modelerfour:synthesized/host",
Expand Down Expand Up @@ -383,47 +382,21 @@ export class CodeModelBuilder {
// process sdk models
for (const model of sdkModels) {
if (!processedSdkModels.has(model)) {
// For this part, we only process models which explicitly set access and use decorator,
// otherwise, there will be isseus, e.g. for multipart models, we generate our own model, but TCGC returns the original multipart model with public access and input usage, so we will generate the original multipart model which is unexpected
const access = getAccess(model.__raw, accessCache);
if (access === "public") {
const schema = this.processSchema(model, "");

this.trackSchemaUsage(schema, {
usage: [SchemaContext.Public],
});
} else if (access === "internal") {
const schema = this.processSchema(model, model.name);

this.trackSchemaUsage(schema, {
usage: [SchemaContext.Internal],
});
if (access) {
this.processSchema(model, "");
}

const usage = getUsage(model.__raw, usageCache);
if (usage) {
const schema = this.processSchema(model, "");

this.trackSchemaUsage(schema, {
usage: usage,
});
this.processSchema(model, "");
}

processedSdkModels.add(model);
}
}
}

private processSchemaUsage() {
this.codeModel.schemas.objects?.forEach((it) => this.propagateSchemaUsage(it));

// post process for schema usage
this.codeModel.schemas.objects?.forEach((it) => this.resolveSchemaUsage(it));
this.codeModel.schemas.groups?.forEach((it) => this.resolveSchemaUsage(it));
this.codeModel.schemas.choices?.forEach((it) => this.resolveSchemaUsage(it));
this.codeModel.schemas.sealedChoices?.forEach((it) => this.resolveSchemaUsage(it));
this.codeModel.schemas.ors?.forEach((it) => this.resolveSchemaUsage(it));
this.codeModel.schemas.constants?.forEach((it) => this.resolveSchemaUsage(it));
}

private deduplicateSchemaName() {
// deduplicate model name
const nameCount = new Map<string, number>();
Expand Down Expand Up @@ -930,7 +903,7 @@ export class CodeModelBuilder {

op.responses?.forEach((r) => {
if (r instanceof SchemaResponse) {
this.trackSchemaUsage(r.schema, { usage: [SchemaContext.Paged] });
trackSchemaUsage(r.schema, { usage: [SchemaContext.Paged] });
}
});
break;
Expand Down Expand Up @@ -1004,7 +977,6 @@ export class CodeModelBuilder {
}
}

// track usage
if (pollingSchema) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did not remove this part because of this TCGC issue: Azure/typespec-azure#1752

this.trackSchemaUsage(pollingSchema, { usage: [SchemaContext.Output] });
if (trackConvenienceApi) {
Expand All @@ -1013,14 +985,6 @@ export class CodeModelBuilder {
});
}
}
if (finalSchema) {
this.trackSchemaUsage(finalSchema, { usage: [SchemaContext.Output] });
if (trackConvenienceApi) {
this.trackSchemaUsage(finalSchema, {
usage: [op.internalApi ? SchemaContext.Internal : SchemaContext.Public],
});
}
}

op.lroMetadata = new LongRunningMetadata(
true,
Expand Down Expand Up @@ -1199,14 +1163,6 @@ export class CodeModelBuilder {
if (parameterOnClient) {
clientContext.addGlobalParameter(parameter);
}

this.trackSchemaUsage(schema, { usage: [SchemaContext.Input] });

if (op.convenienceApi) {
this.trackSchemaUsage(schema, {
usage: [op.internalApi ? SchemaContext.Internal : SchemaContext.Public],
});
}
}
}

Expand Down Expand Up @@ -1307,13 +1263,6 @@ export class CodeModelBuilder {
},
);

this.trackSchemaUsage(requestConditionsSchema, { usage: [SchemaContext.Input] });
if (op.convenienceApi) {
this.trackSchemaUsage(requestConditionsSchema, {
usage: [op.internalApi ? SchemaContext.Internal : SchemaContext.Public],
});
}

// update group schema for properties
for (const parameter of request.parameters) {
if (
Expand Down Expand Up @@ -1385,20 +1334,11 @@ export class CodeModelBuilder {

const jsonMergePatch = operationIsJsonMergePatch(sdkHttpOperation);

const schemaIsPublicBeforeProcess =
schema instanceof ObjectSchema &&
(schema as SchemaUsage).usage?.includes(SchemaContext.Public);

this.trackSchemaUsage(schema, { usage: [SchemaContext.Input] });

if (op.convenienceApi) {
// model/schema does not need to be Public or Internal, if it is not to be used in convenience API
if (jsonMergePatch) {
// if it's json-merge-patch body, we need to track the body model's access, see case https://gist.github.com/haolingdong-msft/2d941f452551a6f96e141adbc38ac483#case7-flatten-body--json-merge-patch
this.trackSchemaUsage(schema, {
usage: [op.internalApi ? SchemaContext.Internal : SchemaContext.Public],
});
}

if (jsonMergePatch) {
this.trackSchemaUsage(schema, { usage: [SchemaContext.JsonMergePatch] });
}
if (op.convenienceApi && operationIsMultipart(sdkHttpOperation)) {
Expand Down Expand Up @@ -1433,14 +1373,6 @@ export class CodeModelBuilder {
return;
}

const schemaUsage = (schema as SchemaUsage).usage;
if (!schemaIsPublicBeforeProcess && schemaUsage?.includes(SchemaContext.Public)) {
// Public added in this op, change it to PublicSpread
// This means that if this op would originally add Public to this schema, it adds PublicSpread instead
schemaUsage?.splice(schemaUsage?.indexOf(SchemaContext.Public), 1);
this.trackSchemaUsage(schema, { usage: [SchemaContext.PublicSpread] });
}

if (op.convenienceApi && op.parameters) {
op.convenienceApi.requests = [];
const request = new Request({
Expand Down Expand Up @@ -1606,11 +1538,6 @@ export class CodeModelBuilder {
}
}

private findResponseBody(bodyType: Type): Type {
// find a type that possibly without http metadata like @statusCode
return this.getEffectiveSchemaType(bodyType);
}

private processResponse(
op: CodeModelOperation,
statusCode: number | HttpStatusCodeRange | "*",
Expand All @@ -1628,6 +1555,7 @@ export class CodeModelBuilder {
if (sdkResponse.headers) {
for (const header of sdkResponse.headers) {
const schema = this.processSchema(header.type, header.serializedName);

headers.push(
new HttpHeader(header.serializedName, schema, {
language: {
Expand All @@ -1642,7 +1570,6 @@ export class CodeModelBuilder {
}

const bodyType: SdkType | undefined = sdkResponse.type;
let trackConvenienceApi: boolean = Boolean(op.convenienceApi);

const unknownResponseBody =
sdkResponse.contentTypes &&
Expand Down Expand Up @@ -1671,10 +1598,6 @@ export class CodeModelBuilder {
} else if (bodyType) {
// schema (usually JSON)
let schema: Schema | undefined = undefined;
if (longRunning) {
// LRO uses the LroMetadata for poll/final result, not the response of activation request
trackConvenienceApi = false;
}
if (!schema) {
schema = this.processSchema(bodyType, op.language.default.name + "Response");
}
Expand Down Expand Up @@ -1719,16 +1642,6 @@ export class CodeModelBuilder {
}
} else {
op.addResponse(response);

if (response instanceof SchemaResponse) {
this.trackSchemaUsage(response.schema, { usage: [SchemaContext.Output] });

if (trackConvenienceApi) {
this.trackSchemaUsage(response.schema, {
usage: [op.internalApi ? SchemaContext.Internal : SchemaContext.Public],
});
}
}
}
}

Expand Down Expand Up @@ -1964,6 +1877,7 @@ export class CodeModelBuilder {
},
});
schema.crossLanguageDefinitionId = type.crossLanguageDefinitionId;
schema.usage = processSchemaUsageFromSdkType(type, schema.usage);
return this.codeModel.schemas.add(schema);
}

Expand Down Expand Up @@ -2066,6 +1980,8 @@ export class CodeModelBuilder {
(objectSchema as CrossLanguageDefinition).crossLanguageDefinitionId =
type.crossLanguageDefinitionId;
this.codeModel.schemas.add(objectSchema);
// TODO haoling: fix as any
objectSchema.usage = processSchemaUsageFromSdkType(type, objectSchema.usage) as any;
Copy link
Member Author

@haolingdong-msft haolingdong-msft Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

objectSchema.usage's type is @autorest\codemodel's SchemaContext type in packages\http-client-java\node_modules\@autorest\codemodel\dist\model\common\schemas\usage.d.ts,
while processSchemaUsageFromSdkType(type, objectSchema.usage) returned usage is our SchemaContext: in packages\http-client-java\emitter\src\common\schemas\usage.ts

The two models are different, hence I use as any here to convert first.

The ideal way may be create another ObjectSchema type in our emitter which extends @autorest/codemodel's ObjectSchema and change ObjectSchema.usage as our emitter's SchemaContext type. But not sure if it is too complex.


// cache this now before we accidentally recurse on this type.
if (!this.schemaCache.has(type)) {
Expand Down Expand Up @@ -2623,83 +2539,24 @@ export class CodeModelBuilder {

private _subscriptionParameter?: Parameter;

private propagateSchemaUsage(schema: Schema): void {
const processedSchemas = new Set<Schema>();

const innerApplySchemaUsage = (schema: Schema, schemaUsage: SchemaUsage) => {
this.trackSchemaUsage(schema, schemaUsage);
innerPropagateSchemaUsage(schema, schemaUsage);
};

const innerPropagateSchemaUsage = (schema: Schema, schemaUsage: SchemaUsage) => {
if (processedSchemas.has(schema)) {
return;
}

processedSchemas.add(schema);
if (schema instanceof ObjectSchema || schema instanceof GroupSchema) {
if (schemaUsage.usage || schemaUsage.serializationFormats) {
schema.properties?.forEach((p) => {
if (p.readOnly && schemaUsage.usage?.includes(SchemaContext.Input)) {
const schemaUsageWithoutInput = {
usage: schemaUsage.usage.filter((it) => it !== SchemaContext.Input),
serializationFormats: schemaUsage.serializationFormats,
};
innerApplySchemaUsage(p.schema, schemaUsageWithoutInput);
} else {
innerApplySchemaUsage(p.schema, schemaUsage);
}
});

if (schema instanceof ObjectSchema) {
schema.parents?.all?.forEach((p) => innerApplySchemaUsage(p, schemaUsage));
schema.parents?.immediate?.forEach((p) => innerApplySchemaUsage(p, schemaUsage));

if (schema.discriminator) {
// propagate access/usage to immediate children, if the schema is a discriminated model
// if the schema is not a discriminated model, its children likely not valid for the mode/API
// TODO: it does not handle the case that concrete model (kind: "type1") for the discriminated model have depth larger than 1 (e.g. kind: "type1" | "type2" in middle)
schema.children?.immediate?.forEach((c) => innerApplySchemaUsage(c, schemaUsage));
}

if (schema.discriminator?.property?.schema) {
innerApplySchemaUsage(schema.discriminator?.property?.schema, schemaUsage);
}
}
}
} else if (schema instanceof DictionarySchema) {
innerApplySchemaUsage(schema.elementType, schemaUsage);
} else if (schema instanceof ArraySchema) {
innerApplySchemaUsage(schema.elementType, schemaUsage);
} else if (schema instanceof OrSchema) {
schema.anyOf?.forEach((it) => innerApplySchemaUsage(it, schemaUsage));
} else if (schema instanceof ConstantSchema) {
innerApplySchemaUsage(schema.valueType, schemaUsage);
private postProcessSchemaUsage(): void {
const innerProcessUsage = (schema: Schema) => {
const usages = (schema as SchemaUsage).usage;
if (
usages &&
usages.includes(SchemaContext.Public) &&
usages.includes(SchemaContext.Internal)
) {
// If access contains both public and internal, remove internal
usages.splice(usages.indexOf(SchemaContext.Internal), 1);
}
};

// Exclude context that not to be propagated
const updatedSchemaUsage = (schema as SchemaUsage).usage?.filter(
(it) => it !== SchemaContext.Paged && it !== SchemaContext.PublicSpread,
);
const indexSpread = (schema as SchemaUsage).usage?.indexOf(SchemaContext.PublicSpread);
if (
updatedSchemaUsage &&
indexSpread &&
indexSpread >= 0 &&
!(schema as SchemaUsage).usage?.includes(SchemaContext.Public)
) {
// Propagate Public, if schema is PublicSpread
updatedSchemaUsage.push(SchemaContext.Public);
}
const schemaUsage = {
usage: updatedSchemaUsage,
serializationFormats: (schema as SchemaUsage).serializationFormats?.filter(
(it) => it !== KnownMediaType.Multipart,
),
};
// Propagate the usage of the initial schema itself
innerPropagateSchemaUsage(schema, schemaUsage);
this.codeModel.schemas.choices?.forEach(innerProcessUsage);
this.codeModel.schemas.objects?.forEach(innerProcessUsage);
this.codeModel.schemas.sealedChoices?.forEach(innerProcessUsage);
this.codeModel.schemas.constants?.forEach(innerProcessUsage);
this.codeModel.schemas.groups?.forEach(innerProcessUsage);
this.codeModel.schemas.ors?.forEach(innerProcessUsage);
}

private trackSchemaUsage(schema: Schema, schemaUsage: SchemaUsage): void {
Expand Down
7 changes: 7 additions & 0 deletions packages/http-client-java/emitter/src/external-schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import {
SdkType,
} from "@azure-tools/typespec-client-generator-core";
import { CrossLanguageDefinition } from "./common/client.js";
import { SchemaContext } from "./common/schemas/usage.js";
import { trackSchemaUsage } from "./type-utils.js";
import { getNamespace, pascalCase } from "./utils.js";

/*
Expand Down Expand Up @@ -369,6 +371,8 @@ export function getFileDetailsSchema(
processSchemaFunc,
);
}
// set the schema usage to public access, later we will have post processor to propagate operation's access to the model
trackSchemaUsage(fileDetailsSchema, { usage: [SchemaContext.Input, SchemaContext.Public] });
return fileDetailsSchema;
} else {
// property.type is bytes, create a File schema
Expand All @@ -388,6 +392,9 @@ export function getFileDetailsSchema(
addFilenameProperty(fileDetailsSchema, stringSchema);
addContentTypeProperty(fileDetailsSchema, stringSchema);
}
// set the schema usage to public access, later we will have post processor to propagate operation's access to the model
trackSchemaUsage(fileDetailsSchema, { usage: [SchemaContext.Input, SchemaContext.Public] });

return fileDetailsSchema;
}
}
Loading
Loading