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, accessor to subclient #5027

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

http-client-java, accessor to subclient #5027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
73ba7b8
emitter, add enable-subclient option
weidongxu-microsoft Nov 5, 2024
ee6ece7
disable builder for subclient
weidongxu-microsoft Nov 5, 2024
573d4ae
cache for client
weidongxu-microsoft Nov 5, 2024
3080a8b
add subClients to ServiceClient
weidongxu-microsoft Nov 5, 2024
a27756c
stub for accessor
weidongxu-microsoft Nov 7, 2024
d377855
impl of stub
weidongxu-microsoft Nov 7, 2024
703d968
enhance builder and wrapper client
weidongxu-microsoft Nov 8, 2024
26357b0
Merge branch 'main' into http-client-java_subclient-accessor
weidongxu-microsoft Nov 8, 2024
6b1c7b4
use list for ordering
weidongxu-microsoft Nov 8, 2024
7f5530f
ProtocolTestWriter, do this for the 1st client with ClientBuilder
weidongxu-microsoft Nov 11, 2024
f073867
Merge branch 'main' into http-client-java_subclient-accessor
weidongxu-microsoft Nov 11, 2024
4299f4f
bug fix for TestBase
weidongxu-microsoft Nov 11, 2024
80e0b10
switch from getAccessorProperties to getMethodParameters, to make Cli…
weidongxu-microsoft Nov 11, 2024
da412d3
fix ServiceVersion argument no at the correct position
weidongxu-microsoft Nov 11, 2024
f201ee2
make subclient have same namespace of its parent
weidongxu-microsoft Nov 11, 2024
63edeb5
Merge branch 'main' into http-client-java_subclient-accessor
weidongxu-microsoft Nov 12, 2024
928eeb7
Merge branch 'main' into http-client-java_subclient-accessor
weidongxu-microsoft Nov 13, 2024
f1c5f9a
Merge branch 'main' into http-client-java_subclient-accessor
weidongxu-microsoft Nov 14, 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
258 changes: 137 additions & 121 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 @@ -496,106 +496,120 @@ export class CodeModelBuilder {

const sdkPackage = this.sdkContext.sdkPackage;
for (const client of sdkPackage.clients) {
let clientName = client.name;
let javaNamespace = this.getJavaNamespace(this.namespace);
const clientFullName = client.name;
const clientNameSegments = clientFullName.split(".");
if (clientNameSegments.length > 1) {
clientName = clientNameSegments.at(-1)!;
const clientSubNamespace = clientNameSegments.slice(0, -1).join(".");
javaNamespace = this.getJavaNamespace(this.namespace + "." + clientSubNamespace);
}

const codeModelClient = new CodeModelClient(clientName, client.doc ?? "", {
summary: client.summary,
language: {
default: {
namespace: this.namespace,
},
java: {
namespace: javaNamespace,
},
this.processClient(client);
}
}

private processClient(client: SdkClientType<SdkHttpOperation>): CodeModelClient {
let clientName = client.name;
let javaNamespace = this.getJavaNamespace(this.namespace);
const clientFullName = client.name;
const clientNameSegments = clientFullName.split(".");
if (clientNameSegments.length > 1) {
clientName = clientNameSegments.at(-1)!;
const clientSubNamespace = clientNameSegments.slice(0, -1).join(".");
javaNamespace = this.getJavaNamespace(this.namespace + "." + clientSubNamespace);
}

const codeModelClient = new CodeModelClient(clientName, client.doc ?? "", {
summary: client.summary,
language: {
default: {
namespace: this.namespace,
},
java: {
namespace: javaNamespace,
},
},

// at present, use global security definition
security: this.codeModel.security,
});
codeModelClient.crossLanguageDefinitionId = client.crossLanguageDefinitionId;
// at present, use global security definition
security: this.codeModel.security,
});
codeModelClient.crossLanguageDefinitionId = client.crossLanguageDefinitionId;

// versioning
const versions = client.apiVersions;
if (versions && versions.length > 0) {
if (!this.sdkContext.apiVersion || ["all", "latest"].includes(this.sdkContext.apiVersion)) {
this.apiVersion = versions[versions.length - 1];
} else {
this.apiVersion = versions.find((it: string) => it === this.sdkContext.apiVersion);
if (!this.apiVersion) {
throw new Error("Unrecognized api-version: " + this.sdkContext.apiVersion);
}
// versioning
const versions = client.apiVersions;
if (versions && versions.length > 0) {
if (!this.sdkContext.apiVersion || ["all", "latest"].includes(this.sdkContext.apiVersion)) {
this.apiVersion = versions[versions.length - 1];
} else {
this.apiVersion = versions.find((it: string) => it === this.sdkContext.apiVersion);
if (!this.apiVersion) {
throw new Error("Unrecognized api-version: " + this.sdkContext.apiVersion);
}
}

codeModelClient.apiVersions = [];
for (const version of this.getFilteredApiVersions(
this.apiVersion,
versions,
this.options["service-version-exclude-preview"],
)) {
const apiVersion = new ApiVersion();
apiVersion.version = version;
codeModelClient.apiVersions.push(apiVersion);
}
codeModelClient.apiVersions = [];
for (const version of this.getFilteredApiVersions(
this.apiVersion,
versions,
this.options["service-version-exclude-preview"],
)) {
const apiVersion = new ApiVersion();
apiVersion.version = version;
codeModelClient.apiVersions.push(apiVersion);
}
}

// client initialization
let baseUri = "{endpoint}";
let hostParameters: Parameter[] = [];
client.initialization.properties.forEach((initializationProperty) => {
if (initializationProperty.kind === "endpoint") {
let sdkPathParameters: SdkPathParameter[] = [];
if (initializationProperty.type.kind === "union") {
if (initializationProperty.type.variantTypes.length === 2) {
// only get the sdkPathParameters from the endpoint whose serverUrl is not {"endpoint"}
for (const endpointType of initializationProperty.type.variantTypes) {
if (endpointType.kind === "endpoint" && endpointType.serverUrl !== "{endpoint}") {
sdkPathParameters = endpointType.templateArguments;
baseUri = endpointType.serverUrl;
}
// client initialization
let baseUri = "{endpoint}";
let hostParameters: Parameter[] = [];
client.initialization.properties.forEach((initializationProperty) => {
if (initializationProperty.kind === "endpoint") {
let sdkPathParameters: SdkPathParameter[] = [];
if (initializationProperty.type.kind === "union") {
if (initializationProperty.type.variantTypes.length === 2) {
// only get the sdkPathParameters from the endpoint whose serverUrl is not {"endpoint"}
for (const endpointType of initializationProperty.type.variantTypes) {
if (endpointType.kind === "endpoint" && endpointType.serverUrl !== "{endpoint}") {
sdkPathParameters = endpointType.templateArguments;
baseUri = endpointType.serverUrl;
}
} else if (initializationProperty.type.variantTypes.length > 2) {
throw new Error("Multiple server url defined for one client is not supported yet.");
}
} else if (initializationProperty.type.kind === "endpoint") {
sdkPathParameters = initializationProperty.type.templateArguments;
baseUri = initializationProperty.type.serverUrl;
} else if (initializationProperty.type.variantTypes.length > 2) {
throw new Error("Multiple server url defined for one client is not supported yet.");
}

hostParameters = this.processHostParameters(sdkPathParameters);
codeModelClient.addGlobalParameters(hostParameters);
} else if (initializationProperty.type.kind === "endpoint") {
sdkPathParameters = initializationProperty.type.templateArguments;
baseUri = initializationProperty.type.serverUrl;
}
});

const clientContext = new ClientContext(
baseUri,
hostParameters,
codeModelClient.globalParameters!,
codeModelClient.apiVersions,
);

// preprocess operation groups and operations
// operations without operation group
const serviceMethodsWithoutSubClient = this.listServiceMethodsUnderClient(client);
let codeModelGroup = new OperationGroup("");
for (const serviceMethod of serviceMethodsWithoutSubClient) {
if (!this.needToSkipProcessingOperation(serviceMethod.__raw, clientContext)) {
codeModelGroup.addOperation(this.processOperation(serviceMethod, clientContext, ""));
}
hostParameters = this.processHostParameters(sdkPathParameters);
codeModelClient.addGlobalParameters(hostParameters);
}
if (codeModelGroup.operations?.length > 0) {
codeModelClient.operationGroups.push(codeModelGroup);
});

const clientContext = new ClientContext(
baseUri,
hostParameters,
codeModelClient.globalParameters!,
codeModelClient.apiVersions,
);

const enableSubclient: boolean = Boolean(this.options["enable-subclient"]);

// preprocess operation groups and operations
// operations without operation group
const serviceMethodsWithoutSubClient = this.listServiceMethodsUnderClient(client);
let codeModelGroup = new OperationGroup("");
for (const serviceMethod of serviceMethodsWithoutSubClient) {
if (!this.needToSkipProcessingOperation(serviceMethod.__raw, clientContext)) {
codeModelGroup.addOperation(this.processOperation(serviceMethod, clientContext, ""));
}
}
if (codeModelGroup.operations?.length > 0 || enableSubclient) {
codeModelClient.operationGroups.push(codeModelGroup);
}

const subClients = this.listSubClientsUnderClient(client, !enableSubclient);
if (enableSubclient) {
// subclient, no operation group
for (const subClient of subClients) {
const codeModelSubclient = this.processClient(subClient);
codeModelClient.addSubClient(codeModelSubclient);
}
} else {
Comment on lines +604 to +611
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most is refactor (extract code to processClient, as we now need to call it recursively).

Here is the main change, with the option, we would not do operation group, and every client is a client (or subclient).

// operations under operation groups
const subClients = this.listSubClientsUnderClient(client, true, true);
for (const subClient of subClients) {
const serviceMethods = this.listServiceMethodsUnderClient(subClient);
// operation group with no operation is skipped
Expand All @@ -611,48 +625,51 @@ export class CodeModelBuilder {
codeModelClient.operationGroups.push(codeModelGroup);
}
}
this.codeModel.clients.push(codeModelClient);
}

this.codeModel.clients.push(codeModelClient);

// postprocess for ServiceVersion
let apiVersionSameForAllClients = true;
let sharedApiVersions = undefined;
// postprocess for ServiceVersion
let apiVersionSameForAllClients = true;
let sharedApiVersions = undefined;
for (const client of this.codeModel.clients) {
const apiVersions = client.apiVersions;
if (!apiVersions) {
// client does not have apiVersions
apiVersionSameForAllClients = false;
} else if (!sharedApiVersions) {
// first client, set it to sharedApiVersions
sharedApiVersions = apiVersions;
} else {
apiVersionSameForAllClients = isEqual(sharedApiVersions, apiVersions);
}
if (!apiVersionSameForAllClients) {
break;
}
}
if (apiVersionSameForAllClients) {
const serviceVersion = getServiceVersion(this.codeModel);
for (const client of this.codeModel.clients) {
const apiVersions = client.apiVersions;
if (!apiVersions) {
// client does not have apiVersions
apiVersionSameForAllClients = false;
} else if (!sharedApiVersions) {
// first client, set it to sharedApiVersions
sharedApiVersions = apiVersions;
} else {
apiVersionSameForAllClients = isEqual(sharedApiVersions, apiVersions);
}
if (!apiVersionSameForAllClients) {
break;
}
client.serviceVersion = serviceVersion;
}
if (apiVersionSameForAllClients) {
const serviceVersion = getServiceVersion(this.codeModel);
for (const client of this.codeModel.clients) {
client.serviceVersion = serviceVersion;
}
} else {
for (const client of this.codeModel.clients) {
const apiVersions = client.apiVersions;
if (apiVersions) {
client.serviceVersion = getServiceVersion(client);
}
} else {
for (const client of this.codeModel.clients) {
const apiVersions = client.apiVersions;
if (apiVersions) {
client.serviceVersion = getServiceVersion(client);
}
}
}

return codeModelClient;
}

private listSubClientsUnderClient(
client: SdkClientType<SdkHttpOperation>,
includeNestedOperationGroups: boolean,
isRootClient: boolean,
Copy link
Contributor Author

@weidongxu-microsoft weidongxu-microsoft Nov 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This isRootClient param seems not needed. If a client does not have parent, it is the root client.

includeNestedSubClients: boolean,
): SdkClientType<SdkHttpOperation>[] {
const operationGroups: SdkClientType<SdkHttpOperation>[] = [];
const isRootClient = !client.parent;
const subClients: SdkClientType<SdkHttpOperation>[] = [];
for (const method of client.methods) {
if (method.kind === "clientaccessor") {
const subClient = method.response;
Expand All @@ -661,19 +678,18 @@ export class CodeModelBuilder {
subClient.name =
removeClientSuffix(client.name) + removeClientSuffix(pascalCase(subClient.name));
}
operationGroups.push(subClient);
if (includeNestedOperationGroups) {
subClients.push(subClient);
if (includeNestedSubClients) {
for (const operationGroup of this.listSubClientsUnderClient(
subClient,
includeNestedOperationGroups,
false,
includeNestedSubClients,
)) {
operationGroups.push(operationGroup);
subClients.push(operationGroup);
}
}
}
}
return operationGroups;
return subClients;
}

private listServiceMethodsUnderClient(
Expand Down
18 changes: 18 additions & 0 deletions packages/http-client-java/emitter/src/common/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ export interface Client extends Aspect, CrossLanguageDefinition {
security: Security;

serviceVersion?: ServiceVersion; // apiVersions is in

parent?: Client;
subClients: Array<Client>;
publicBuilder: boolean;
publicParentAccessor: boolean;
}

export class Client extends Aspect implements Client {
Expand All @@ -19,6 +24,9 @@ export class Client extends Aspect implements Client {

this.operationGroups = [];
this.security = new Security(false);
this.subClients = [];
this.publicBuilder = true;
this.publicParentAccessor = false;

this.applyTo(this, objectInitializer);
}
Expand All @@ -30,6 +38,16 @@ export class Client extends Aspect implements Client {
addGlobalParameters(parameters: Parameter[]) {
this.globals.push(...parameters);
}

addSubClient(subClient: Client) {
subClient.parent = this;
subClient.publicBuilder = false;
subClient.publicParentAccessor = true;
this.subClients.push(subClient);

// at present, sub client must have same namespace of its parent client
subClient.language.java!.namespace = this.language.java!.namespace;
}
}

export class ServiceVersion extends Metadata {
Expand Down
6 changes: 5 additions & 1 deletion packages/http-client-java/emitter/src/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ export interface EmitterOptions {

"group-etag-headers"?: boolean;

"enable-subclient"?: boolean;

"advanced-versioning"?: boolean;
"api-version"?: string;
"service-version-exclude-preview"?: boolean;
Expand Down Expand Up @@ -78,7 +80,7 @@ const EmitterOptionsSchema: JSONSchemaType<EmitterOptions> = {

"enable-sync-stack": { type: "boolean", nullable: true, default: true },
"stream-style-serialization": { type: "boolean", nullable: true, default: true },
"use-object-for-unknown": { type: "boolean", nullable: true, default: true },
"use-object-for-unknown": { type: "boolean", nullable: true, default: false },

// customization
"partial-update": { type: "boolean", nullable: true, default: false },
Expand All @@ -90,6 +92,8 @@ const EmitterOptionsSchema: JSONSchemaType<EmitterOptions> = {

"group-etag-headers": { type: "boolean", nullable: true },

"enable-subclient": { type: "boolean", nullable: true, default: false },
Copy link
Member

@haolingdong-msft haolingdong-msft Nov 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Do we want to set the option to true in our test to see the impact? (at least for some of the tests)

Copy link
Contributor Author

@weidongxu-microsoft weidongxu-microsoft Nov 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, we'd plan to set it to true for the subclient related tests (and for the whole unbranded too, if we separate the flavors).

Currently my cadl-ranch PR is not merged, so we don't have them now. (test case, the basic one, is written before the http-specs package, I would need to migrate it first; and we will need more tests on advanced cases)


"advanced-versioning": { type: "boolean", nullable: true, default: false },
"api-version": { type: "string", nullable: true },
"service-version-exclude-preview": { type: "boolean", nullable: true, default: false },
Expand Down
2 changes: 1 addition & 1 deletion ...nerator-core/src/main/java/com/microsoft/typespec/http/client/generator/core/Javagen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ protected JavaPackage writeToTemplates(CodeModel codeModel, Client client, JavaS
// Test
if (settings.isDataPlaneClient() && settings.isGenerateTests()) {
if (!client.getSyncClients().isEmpty()
&& client.getSyncClients().iterator().next().getClientBuilder() != null) {
&& client.getSyncClients().stream().anyMatch(c -> c.getClientBuilder() != null)) {
List<ServiceClient> serviceClients = client.getServiceClients();
if (CoreUtils.isNullOrEmpty(serviceClients)) {
serviceClients = Collections.singletonList(client.getServiceClient());
Expand Down
Loading
Loading