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.

Sign up for GitHub

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

Jump to bottom

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

Draft
wants to merge 33 commits into
base: main
Choose a base branch
from
Draft

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

Show file tree
Hide file tree
Changes from 20 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
173 changes: 114 additions & 59 deletions packages/http-client-java/emitter/src/code-model-builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
Response,
Schema,
SchemaResponse,
Schemas,

Check warning on line 33 in packages/http-client-java/emitter/src/code-model-builder.ts

View workflow job for this annotation

GitHub Actions / Lint 

'Schemas' is defined but never used. Allowed unused vars must match /^_/u
SchemaType,
Security,
SecurityScheme,
Expand Down Expand Up @@ -152,7 +153,9 @@
getUsage,
isStable,
modelIs,
processSchemaUsageFromSdkType,
pushDistinct,
trackSchemaUsage,
} from "./type-utils.js";
import {
getNamespace,
Expand Down Expand Up @@ -265,7 +268,8 @@

this.processModels();

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


this.deduplicateSchemaName();

Expand All @@ -280,7 +284,7 @@
parameter = this.createApiVersionParameter(arg.name, ParameterLocation.Uri);
} else {
const schema = this.processSchema(arg.type, arg.name);
this.trackSchemaUsage(schema, {
trackSchemaUsage(schema, {
usage: [SchemaContext.Input, SchemaContext.Output /*SchemaContext.Public*/],
});
parameter = new Parameter(arg.name, arg.doc ?? "", schema, {
Expand Down Expand Up @@ -391,38 +395,38 @@
usage: [SchemaContext.Public],
});
} else if (access === "internal") {
const schema = this.processSchema(model, model.name);

Check warning on line 398 in packages/http-client-java/emitter/src/code-model-builder.ts

View workflow job for this annotation

GitHub Actions / Lint 

'schema' is assigned a value but never used. Allowed unused vars must match /^_/u

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

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

Check warning on line 407 in packages/http-client-java/emitter/src/code-model-builder.ts

View workflow job for this annotation

GitHub Actions / Lint 

'schema' is assigned a value but never used. Allowed unused vars must match /^_/u

this.trackSchemaUsage(schema, {
usage: usage,
});
// trackSchemaUsage(schema, {
// usage: usage,
// });
}

processedSdkModels.add(model);
}
}
}

private processSchemaUsage() {
this.codeModel.schemas.objects?.forEach((it) => this.propagateSchemaUsage(it));
// 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));
}
// // 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
Expand Down Expand Up @@ -923,7 +927,7 @@

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 @@ -997,7 +1001,6 @@
}
}

// 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 @@ -1006,14 +1009,14 @@
});
}
}
if (finalSchema) {
this.trackSchemaUsage(finalSchema, { usage: [SchemaContext.Output] });
if (trackConvenienceApi) {
this.trackSchemaUsage(finalSchema, {
usage: [op.internalApi ? SchemaContext.Internal : SchemaContext.Public],
});
}
}
// 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 @@ -1193,13 +1196,13 @@
clientContext.addGlobalParameter(parameter);
}

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

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

Expand Down Expand Up @@ -1300,12 +1303,12 @@
},
);

this.trackSchemaUsage(requestConditionsSchema, { usage: [SchemaContext.Input] });
if (op.convenienceApi) {
this.trackSchemaUsage(requestConditionsSchema, {
usage: [op.internalApi ? SchemaContext.Internal : SchemaContext.Public],
});
}
// 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) {
Expand Down Expand Up @@ -1378,20 +1381,23 @@

const jsonMergePatch = operationIsJsonMergePatch(sdkHttpOperation);

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

this.trackSchemaUsage(schema, { usage: [SchemaContext.Input] });
// 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
// this.trackSchemaUsage(schema, {
// usage: [op.internalApi ? SchemaContext.Internal : SchemaContext.Public],
// });
// }

if (op.convenienceApi) {
// model/schema does not need to be Public or Internal, if it is not to be used in convenience API
this.trackSchemaUsage(schema, {
usage: [op.internalApi ? SchemaContext.Internal : SchemaContext.Public],
});
}

if (jsonMergePatch) {
// TODO haoling: consider move it out
this.trackSchemaUsage(schema, { usage: [op.internalApi ? SchemaContext.Internal : SchemaContext.Public] });
this.trackSchemaUsage(schema, { usage: [SchemaContext.JsonMergePatch] });
}
if (op.convenienceApi && operationIsMultipart(sdkHttpOperation)) {
Expand Down Expand Up @@ -1423,16 +1429,20 @@
if (sdkType.isGeneratedName) {
schema.language.default.name = pascalCase(op.language.default.name) + "PatchRequest";
}
// // TODO haoling: maybe put to a global post processing
// this.postProcessSchemaUsage(schema);
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] });
}
// const schemaUsage = (schema as SchemaUsage).usage;
// trackSchemaUsage(schema, { usage: schemaUsage });
// 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);
// trackSchemaUsage(schema, { usage: [SchemaContext.Public] });
// }


if (op.convenienceApi && op.parameters) {
op.convenienceApi.requests = [];
Expand Down Expand Up @@ -1621,6 +1631,10 @@
if (sdkResponse.headers) {
for (const header of sdkResponse.headers) {
const schema = this.processSchema(header.type, header.serializedName);
// // TODO haoling: why header schema do not need usage tracking? no usage tracking for header schema?
// trackSchemaUsage(schema, {
// usage: [SchemaContext.None]
// });
headers.push(
new HttpHeader(header.serializedName, schema, {
language: {
Expand Down Expand Up @@ -1714,13 +1728,18 @@
op.addResponse(response);

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

if (trackConvenienceApi) {
this.trackSchemaUsage(response.schema, {
usage: [op.internalApi ? SchemaContext.Internal : SchemaContext.Public],
});
}
}
}
}
}
Expand Down Expand Up @@ -1957,6 +1976,7 @@
},
});
schema.crossLanguageDefinitionId = type.crossLanguageDefinitionId;
schema.usage = processSchemaUsageFromSdkType(type, schema.usage);
return this.codeModel.schemas.add(schema);
}

Expand Down Expand Up @@ -2059,6 +2079,8 @@
(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 •
edited
Loading

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 @@ -2618,7 +2640,7 @@
const processedSchemas = new Set<Schema>();

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

Expand Down Expand Up @@ -2693,6 +2715,39 @@
innerPropagateSchemaUsage(schema, schemaUsage);
}

// private postProcessSchemaUsage(schemaUsage: SchemaUsage): void {
// if (schemaUsage.usage?.includes(SchemaContext.Public) && schemaUsage.usage?.includes(SchemaContext.Internal)) {
// // remove internal
// schemaUsage.usage.splice(schemaUsage.usage.indexOf(SchemaContext.Internal), 1);
// }
// }

private postProcessSchemaUsage(): void {
const innerProcessUsage = (schema: Schema) => {
const usages = (schema as SchemaUsage).usage;
// if (usages && usages.includes(SchemaContext.Public) && usages.includes(SchemaContext.Internal)) { // TODO haoling: add check to apply only to json-merge-patch and multipart
// // remove internal
// if (usages.includes(SchemaContext.JsonMergePatch) || schema.serializationFormats?.includes(KnownMediaType.Multipart)) {
// usages.splice(usages.indexOf(SchemaContext.Internal), 1);
// }
// }
if (usages && usages.includes(SchemaContext.Public) && usages.includes(SchemaContext.Internal)) { // TODO haoling: add check to apply only to json-merge-patch and multipart
// remove internal
usages.splice(usages.indexOf(SchemaContext.Internal), 1);
}
// if (usages && usages.includes(SchemaContext.None)) {
// // no usage
// (schema as SchemaUsage).usage = [];
// }
};
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 {
if (
schema instanceof ObjectSchema ||
Expand Down
3 changes: 3 additions & 0 deletions packages/http-client-java/emitter/src/common/schemas/usage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ export enum SchemaContext {

/** Schema is used in json-merge-patch operation */
JsonMergePatch = "json-merge-patch",

/** Schema is not used */
None = "None"
}

export interface SchemaUsage {
Expand Down
7 changes: 7 additions & 0 deletions packages/http-client-java/emitter/src/external-schemas.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {
} from "@azure-tools/typespec-client-generator-core";
import { CrossLanguageDefinition } from "./common/client.js";
import { getNamespace, pascalCase } from "./utils.js";
import { trackSchemaUsage } from "./type-utils.js";
import { SchemaContext } from "./common/schemas/usage.js";

/*
* These schema need to reflect
Expand Down Expand Up @@ -317,6 +319,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 @@ -336,6 +340,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