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

tsp, dpg, keep enum type for client parameters, not change it to string type #4911

Merged

Conversation

haolingdong-msft
Copy link
Member

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

Fix Azure/autorest.java#2974
related with https://gist.github.com/haolingdong-msft/df3f25a048baf83d8fabf5206aacc7db#8-clienttype-will-be-generated-after-adopting-tcgc

Existing code

We change enum type to string type in two classes:

  1. ProxyParameterMapper:
    if (settings.isDataPlaneClient()) {
    clientType = SchemaUtil.removeModelFromParameter(parameterRequestLocation, clientType);
    }
  2. ServiceClientMapper:
    if (settings.isDataPlaneClient()) {
    // mostly for Enum to String
    serviceClientPropertyClientType = SchemaUtil.removeModelFromParameter(RequestParameterLocation.URI,
    serviceClientPropertyClientType);
    }

Logic in this pr

Above classes calls SchemaUtil.removeModelFromParameter to remove enum type and change it to string type.
Currently I modify the logic in removeModelFromParameter() to not chang enum type to string type for dataplane path/uri parameters.

Would like to hear your thoughts on below: @weidongxu-microsoft @XiaofeiCao

  1. The logics can also be put in each Mapper, currently I centralized the logics to SchemaUtil class.
  2. We may also need to add a flag to control this because it will impact existing libraries. The flag can be named as path-parameter-enum-as-string.

Latest logics

After discussing with Weidong, we aggreed to put the logics into the mappers and keep the type in protocol method be enum as well.

  1. Define a protected function protected boolean isRemoveModelFromParameter(Parameter parameter, IType type) in both ProxyParameterMapper and ServiceClientMapper.
  2. Override the function in the subclass extends above mappers. In the override function, the logic will be not remove model from parameter if the type is enum and parameter location is client.

@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 30, 2024
@haolingdong-msft haolingdong-msft changed the title tsp, dpg, remove logics that model enum to string for client parameters tsp, dpg, keep enum type for client parameters, not model it to string Oct 30, 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

@haolingdong-msft haolingdong-msft force-pushed the host-parameter-enum-as-enum branch from 2d26fa8 to ee68228 Compare November 1, 2024 08:13
@haolingdong-msft haolingdong-msft changed the title tsp, dpg, keep enum type for client parameters, not model it to string tsp, dpg, keep enum type for path/uri parameters, not model it to string Nov 1, 2024
@haolingdong-msft haolingdong-msft changed the title tsp, dpg, keep enum type for path/uri parameters, not model it to string tsp, dpg, keep enum type for path/uri parameters, not change it to string type Nov 1, 2024
@haolingdong-msft haolingdong-msft changed the title tsp, dpg, keep enum type for path/uri parameters, not change it to string type tsp, dpg, keep enum type for client parameters, not change it to string type Nov 12, 2024
…s-enum

# Conflicts:
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/client/structure/service/ClientAClientBuilder.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/client/structure/service/ClientBClientBuilder.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/client/structure/service/FirstClientBuilder.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/client/structure/service/RenamedOperationClientBuilder.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/client/structure/service/ServiceClientClientBuilder.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/client/structure/service/TwoOperationGroupClientBuilder.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/client/structure/service/subnamespace/SecondClientBuilder.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/added/AddedClientBuilder.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/added/implementation/AddedClientImpl.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/added/implementation/InterfaceV2sImpl.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/added/models/Versions.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/madeoptional/MadeOptionalClientBuilder.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/madeoptional/implementation/MadeOptionalClientImpl.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/madeoptional/models/Versions.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/removed/RemovedClientBuilder.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/removed/implementation/RemovedClientImpl.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/removed/models/Versions.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/renamedfrom/RenamedFromClientBuilder.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/renamedfrom/implementation/NewInterfacesImpl.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/renamedfrom/implementation/RenamedFromClientImpl.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/renamedfrom/models/Versions.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/returntypechangedfrom/ReturnTypeChangedFromClientBuilder.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/returntypechangedfrom/implementation/ReturnTypeChangedFromClientImpl.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/typechangedfrom/TypeChangedFromClientBuilder.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/typechangedfrom/implementation/TypeChangedFromClientImpl.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/java/versioning/typechangedfrom/models/Versions.java
#	packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/versioning-added_apiview_properties.json
#	packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/versioning-madeoptional_apiview_properties.json
#	packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/versioning-removed_apiview_properties.json
#	packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/versioning-renamedfrom_apiview_properties.json
#	packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/versioning-returntypechangedfrom_apiview_properties.json
#	packages/http-client-java/generator/http-client-generator-test/src/main/resources/META-INF/versioning-typechangedfrom_apiview_properties.json
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/client/structure/ClientOperationGroupTests.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/client/structure/DefaultClientTests.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/client/structure/MultiClientTests.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/client/structure/OperationGroupClientTests.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/client/structure/RenameOperationTests.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/client/structure/service/generated/ClientAClientTestBase.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/client/structure/service/generated/FirstClientTestBase.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/client/structure/service/generated/RenamedOperationClientTestBase.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/client/structure/service/generated/ServiceClientClientTestBase.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/client/structure/service/generated/TwoOperationGroupClientTestBase.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/type/scalar/ScalarTests.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/versioning/added/AddedClientTests.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/versioning/added/generated/AddedClientTestBase.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/versioning/madeoptional/MadeOptionalClienTests.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/versioning/madeoptional/generated/MadeOptionalClientTestBase.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/versioning/removed/RemovedClientTests.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/versioning/removed/generated/RemovedClientTestBase.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/versioning/renamedfrom/RenamedFromClientTests.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/versioning/renamedfrom/generated/RenamedFromClientTestBase.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/versioning/returntypechangedfrom/generated/ReturnTypeChangedFromClientTestBase.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/versioning/typechangedfrom/TypeChangedFromClientTests.java
#	packages/http-client-java/generator/http-client-generator-test/src/test/java/versioning/typechangedfrom/generated/TypeChangedFromClientTestBase.java
@haolingdong-msft haolingdong-msft marked this pull request as ready for review November 15, 2024 06:30
@haolingdong-msft haolingdong-msft added this pull request to the merge queue Nov 15, 2024
Merged via the queue into microsoft:main with commit 08bda52 Nov 15, 2024
25 checks passed
@haolingdong-msft haolingdong-msft deleted the host-parameter-enum-as-enum branch November 15, 2024 09:52
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.

TSP, TCGC adoption, not change enum type to string type for client parameter
4 participants