Azure / typespec-azure

About TypeSpec Azure Libraries
https://azure.github.io/typespec-azure/
MIT License
14 stars 40 forks source link

[tcgc] design for Java emitter to influence the default access property of SdkInitializationType and SdkClientAccessor #1702

Open weidongxu-microsoft opened 5 days ago

weidongxu-microsoft commented 5 days ago

Upon https://github.com/Azure/typespec-azure/issues/1696

Context

In TCGC, I take

In Java, there is currently 2 pattern when accessing a subclient.

  1. "client builder pattern" as
    
    // client
    Client client = new ClientBuilder().buildClient();

// subclient via builder (equivalent to client initialization in other languages) SubClient subclient = new ClientBuilder() .parameter() .buildSubClient();


I assume it maps to TCGC

// subclient initialization type subclient.initialization.access=public;

// client access method from (parent) client client..access=internal;


2. "client accessor pattern" as
```java
// client
Client client = new ClientBuilder().buildClient();

// subclient via client accessor
SubClient subclient = client.getSubClient(<param>);

I assume it maps to TCGC

// subclient initialization type
subclient.initialization.access=internal;

// client access method from (parent) client
client.<clientaccessor_method>.access=public;

At present, DPG and unbranded by default uses "client builder pattern". And we also need certain configure to switch to "client accessor pattern" for some modules (e.g. Azure Face API, unbranded OpenAI)

And currently the TCGC value of SdkInitializationType.access and SdkClientAccessor.access is not what Java expected. Therefore, Java emitter does not use the TCGC value for now. It would infer the client/subclient from whether the client has parent client.

PS: Java emitter would still treat the SdkInitializationType.model.access as "internal" or "none", as Java uses its properties directly and not the model (meaning there is currently no ##ClientOptions class for Java).

With "access" in @clientInitialization

Afterward, Java emitter must always use the TCGC value, as it cannot tell whether service has overridden the default @clientInitialization's "access" value.

Problem to solve

Java emitter would still want to affect TCGC to provide the expect default value (when service did not set the "access"). And this default is different from other languages.

The intention is to avoid existing DPG from TypeSpec always adding mulitple lines of @clientInitialization(<interface>, access=public, scope="java"), to make subclient.initialization.access=public for Java.

Possible solutions

Java emitter may provide a e.g. defaultClientInitializationAccess as TCGC context. Default internal for all languages, so that without client.tsp override, subclient is not directly initializable (without its parent client). And Java may provide public to TCGC context, for the required situations. <-- I assume it also switch the SdkClientAccessor.access to internal on its parent client accessor method

tadelesh commented 1 day ago

i think currently, client accessor method is always set with internal cause we do not support public client initialization before @clientInitialization. i agree with the access value you mentioned in two situations, and it should be fixed in tcgc. also, we may need a way to set client accessor's accessibility. cc: @iscai-msft

iscai-msft commented 1 day ago
  1. Client-Builder-Pattern: Yes, this is a correct mapping to me
  2. Client-Accessor-Pattern: Currently, we don't handle the client accessor pattern, our typing system is just designed so it will eventually handle it. The thinking is if you want the client accessor pattern, you need to explicitly define it in your client.tsp. I think this issue was lost in our repo reordering, so I recreated it here. In this case, the subclient's initialization.access can be either internal or public. For example, in storage, the subclient ContainerClient would have initilization.access === "public" and <client-accessor-method>.access === "public", since you can either use a client accessor to create it, or you can directly create it yourself.
  3. SdkInitializationType.model.access: I'm adding a ClientInitialization usage to usage flags, so languages like Java and Python can filter out the generation of it

I'm not fully understanding how TCGC isn't setting the correct access: are you talking about in the client-builder-pattern, or the client-accessor-pattern? We don't have full support for client-accessor-pattern so that is definitely true that we should add support for it. My question is: is Java not getting the correct values for client-builder-pattern? Because that would be a bug since we do support the client-builder-pattern.

weidongxu-microsoft commented 18 hours ago

What I mean is that today (I am aware that TCGC didn't give full information at this time, so emitter of different language kind of handling this client-subclient in their own best practice), .NET and Python is actually generate the code according to the Client-Accessor-Pattern.

E.g. .NET Face API FaceAdministrationClient.GetLargeFaceListClient() https://github.com/Azure/azure-sdk-for-net/blob/d68cd74453a6eec91135f72fefaf2bc851be4238/sdk/face/Azure.AI.Vision.Face/src/Generated/FaceAdministrationClient.cs#L101-L110 meaning <client-accessor-method>.access === "public"

Python Face API FaceAdministrationClient.large_face_list https://github.com/Azure/azure-sdk-for-python/blob/fc3fb85f5ce4a8cf7b915e626a52c232b350f69a/sdk/face/azure-ai-vision-face/generated_tests/test_face_administration_large_face_list_operations.py#L19C44-L19C45 I take this .large_face_list in python also means <client-accessor-method>.access === "public" (though it may not be a method)?

All of above is done without service ever add op getLargeFaceList() in client.tsp for client accessor support. Emitter just generate code according to their best practice for the client-subclient.

It is only Java generating code as Client-Builder-Pattern (we can say this be same as Client-Constructor-Pattern for Python/.NET, that the subclient can be initialized directly without the need to first initialize its parent client).


This is before we allow service to explicitly set the access on @clientInitialization.

After support of optional access in @clientInitialization, service would be able to explicitly set the access to some client, and language emitter can no longer ignore TCGC and do their own default behavior (as emitter won't know in which case that default get overridden by this access).

Hence now we need TCGC always gives correct information. And the problem is that this information for default (when service didn't add access) would be almost opossite in Java and Python/.NET.