Skip to content

Conversation

@anuchandy
Copy link
Member

@anuchandy anuchandy commented Mar 18, 2025

This pull request focuses on enhancing the readability of ProxyMethodMapper by refactoring but without altering overall behavior.

  • Added ProxyMethod.newBuilder() to ensure immutability.
  • Added CollectionUtil, with utility methods to create immutable list/set/map.
  • Extracted and simplified the logic that create async and sync response method return type into a utility class - ResponseTypeFactory, so this can be reused across ProxyMethodMapper and ClientMethodMapper.
  • Extracted the logic that reads exception configurations from settings in to a standalone type - ExceptionSettingsUtil
  • Added ProxyMethodParameterProcessor type responsible for resolving all ProxyMethodParameter, so we can simplify the ProxyMethodMapper internals.
  • Added UniqueProxyMethodNameGenerator type so we can isolate the logic that generate unique method names, which again simplifies the ProxyMethodMapper internals.

The auto-rest.java test run for this commit passed the CI - CI run pr

@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 Mar 18, 2025
@github-actions
Copy link
Contributor

No changes needing a change description found.

@azure-sdk
Copy link
Collaborator

azure-sdk commented Mar 18, 2025

You can try these changes here

🛝 Playground 🌐 Website 🛝 VSCode Extension

Copy link
Contributor

@weidongxu-microsoft weidongxu-microsoft left a comment

Choose a reason for hiding this comment

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

LGTM

Any logic change (other than refactor) that you want us pay attention to?

@anuchandy anuchandy force-pushed the proxy-method-mapper-cleanup branch from 4f01318 to 5524def Compare March 20, 2025 23:59
@anuchandy
Copy link
Member Author

anuchandy commented Mar 21, 2025

LGTM

Any logic change (other than refactor) that you want us pay attention to?

Thanks, Weidong. There are a few conditions that have been inverted to improve readability, If you could skim over it.. the types are ResponseTypeFactory, and ProxyMethodParameterProcessor, to the best of my knowledge, this should not alter any expected behavior.

Copy link
Member

@XiaofeiCao XiaofeiCao left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@anuchandy anuchandy added this pull request to the merge queue Mar 21, 2025
Merged via the queue into microsoft:main with commit 7011df1 Mar 21, 2025
25 checks passed
@anuchandy anuchandy deleted the proxy-method-mapper-cleanup branch March 21, 2025 04:55
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.

4 participants