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

Conversation

haolingdong-msft
Copy link
Member

@haolingdong-msft haolingdong-msft commented Oct 24, 2024

PR shows the diff cases after adopting TCGC and summarized the cause of each diff.

TCGC dependencies are tracked in this issue: Azure/autorest.java#2931

@microsoft-github-policy-service microsoft-github-policy-service bot added the emitter:client:java Issue for the Java client emitter: @typespec/http-client-java label Oct 24, 2024
@azure-sdk
Copy link
Collaborator

No changes needing a change description found.

@azure-sdk
Copy link
Collaborator

You can try these changes here

🛝 Playground 🌐 Website 📚 Next docs

/**
* Repeatability Result header options.
*/
public enum RepeatabilityResult {
Copy link
Member Author

@haolingdong-msft haolingdong-msft Oct 25, 2024

Choose a reason for hiding this comment

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

Caused by this case1

/**
* Enum describing allowed operation states.
*/
public final class OperationState extends ExpandableStringEnum<OperationState> {
Copy link
Member Author

@haolingdong-msft haolingdong-msft Oct 25, 2024

Choose a reason for hiding this comment

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

Caused by case1 and case2

@@ -150,7 +150,7 @@ public final class Builtin implements JsonSerializable<Builtin> {
* @param encoded the encoded value to set.
*/
@Generated
public Builtin(boolean booleanProperty, String string, byte[] bytes, int intProperty, long safeint,
Copy link
Member Author

@haolingdong-msft haolingdong-msft Oct 25, 2024

Choose a reason for hiding this comment

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

This is expected as Builltin and Encoded are used in operation whose convenientAPI == false

@@ -63,7 +63,7 @@ enum OperationStateValues {
Failed,
}

@usage(Usage.input)
@usage(Usage.input | Usage.output)
Copy link
Member Author

Choose a reason for hiding this comment

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

update the test case as the definition is invalid.

* The LroOperationStatusError model.
*/
@Immutable
public final class LroOperationStatusError implements JsonSerializable<LroOperationStatusError> {
Copy link
Member Author

Choose a reason for hiding this comment

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

Caused by case1 and case2

Copy link
Member Author

Choose a reason for hiding this comment

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

Caused by case3

import com.cadl.multicontenttypes.implementation.MultipleContentTypesOnRequestsImpl;
import com.cadl.multicontenttypes.models.Resource;
import reactor.core.publisher.Mono;

/**
Copy link
Member Author

Choose a reason for hiding this comment

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

Caused by case7

@@ -71,6 +73,7 @@ interface MultipleContentTypesOnRequest {
@route("upload/multi-body-types")
@sharedRoute
@post
@convenientAPI(false)
Copy link
Member Author

Choose a reason for hiding this comment

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

Cause by case7. We agree to add @convenientAPI(false) to the operation.

@@ -92,8 +90,6 @@ public Mono<Response<Void>> uploadBytesWithSingleBodyTypeForMultiContentTypesWit
@ServiceMethod(returns = ReturnType.SINGLE)
public Mono<Response<Void>> uploadBytesWithMultiBodyTypesForMultiContentTypesWithResponse(String contentType,
BinaryData data, RequestOptions requestOptions) {
// Convenience API is not generated, as operation 'uploadBytesWithMultiBodyTypesForMultiContentTypes' is
Copy link
Member Author

@haolingdong-msft haolingdong-msft Oct 25, 2024

Choose a reason for hiding this comment

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

I'm working on the javadoc and error message when user does not set convenientAPI(false)


/**
* The actual serialized value for a RepeatabilityResult instance.
*/
Copy link
Member Author

Choose a reason for hiding this comment

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

Caused by case1

@@ -0,0 +1,150 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
Copy link
Member Author

Choose a reason for hiding this comment

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

Caused by case2

*/
public final class OperationState extends ExpandableStringEnum<OperationState> {
/**
* The operation has not started.
Copy link
Member Author

Choose a reason for hiding this comment

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

Caused by case2

/**
* Defines values for ClientType.
*/
public enum ClientType {
Copy link
Member Author

Choose a reason for hiding this comment

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

Caused by case8

/**
* Enum value accepted.
*/
ACCEPTED("accepted"),
Copy link
Member Author

Choose a reason for hiding this comment

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

Caused by case1

Copy link
Member Author

Choose a reason for hiding this comment

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

Caused by case3

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

…t-tcgc-access-usage

# Conflicts:
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/_specs_/azure/core/lro/rpc/models/OperationState.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/_specs_/azure/core/lro/rpc/models/ResourceOperationStatusGenerationResponseGenerationResultError.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/_specs_/azure/core/lro/standard/models/OperationState.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/_specs_/azure/core/lro/standard/models/OperationStatusError.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/_specs_/azure/core/lro/standard/models/ResourceOperationStatusUserExportedUserError.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/_specs_/azure/core/traits/models/RepeatabilityResult.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/com/cadl/longrunning/models/OperationState.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/literalservice/models/ModelOptionalLiteral.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/literalservice/models/PutRequestOptionalLiteralParam.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/longrunning/models/LroOperationStatusError.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/longrunning/models/OperationState.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/longrunning/models/RepeatabilityResult.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/protocolandconvenient/models/ResourceD.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/protocolandconvenient/models/ResourceH.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/specialheaders/models/RepeatabilityResult.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/union/models/OperationState.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/union/models/ResourceOperationStatusOperationStatusResultError.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/versioning/models/OperationState.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/tsptest/versioning/models/ResourceOperationStatusResourceExportedResourceError.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/type/property/optional/models/BooleanLiteralPropertyProperty.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/type/property/optional/models/FloatLiteralPropertyProperty.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/type/property/optional/models/IntLiteralPropertyProperty.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/type/property/optional/models/StringLiteralPropertyProperty.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/_specs_-azure-core-lro-rpc_apiview_properties.json
#	packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/_specs_-azure-core-lro-standard_apiview_properties.json
#	packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/_specs_-azure-core-traits_apiview_properties.json
#	packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/cadl-literalservice_apiview_properties.json
#	packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/cadl-longrunning_apiview_properties.json
#	packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/cadl-protocolandconvenient_apiview_properties.json
#	packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/cadl-specialheaders_apiview_properties.json
#	packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/cadl-union_apiview_properties.json
#	packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/cadl-versioning_apiview_properties.json
#	packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/specialheaders-repeatability_apiview_properties.json
#	packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/type-property-optional_apiview_properties.json
@@ -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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
emitter:client:java Issue for the Java client emitter: @typespec/http-client-java
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants