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, unknown map to binary data #4738

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

Conversation

weidongxu-microsoft
Copy link
Contributor

@weidongxu-microsoft weidongxu-microsoft commented Oct 15, 2024

Fix #4669

Added use-object-for-unknown to both swagger/typespec flag. Default is false, meaning use BinaryData.
(we had 2 or 3 GA lib get affected, and may need to use this flag)

Pending issue Azure/autorest.java#2964

@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 15, 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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No logic change in this file. Just being a bit more restricted on imports Jackson (only do this when not in sss).

Comment on lines -32 to +36
return ClassType.OBJECT;
if (JavaSettings.getInstance().isUseObjectForUnknown()) {
return ClassType.OBJECT;
} else {
return ClassType.BINARY_DATA;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the main code change.

@@ -57,6 +57,10 @@ $generateScript = {
$tspOptions += " --option ""@typespec/http-client-java.api-version=2022-09-01"""
# exclude preview from service versions
$tspOptions += " --option ""@typespec/http-client-java.service-version-exclude-preview=true"""
} elseif ($tspFile -match "type[\\/]array" -or $tspFile -match "type[\\/]dictionary") {
# TODO https://github.com/Azure/autorest.java/issues/2964
Copy link
Member

Choose a reason for hiding this comment

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

Does released services have this case? If they have, do we need to hold on the releases on typespec-java until the issue fixed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We've done a test sync, no released lib has it. -- this is pretty rare for service to have body as raw array or dict, and rarer for the element to be unknown.

Copy link
Member

@haolingdong-msft haolingdong-msft left a comment

Choose a reason for hiding this comment

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

Overall look good to me. We may need to do a test on existing libraries before releasing this changes.

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.

[http-client-java] use BinaryData for typespec unknown
4 participants