-
Notifications
You must be signed in to change notification settings - Fork 207
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
base: main
Are you sure you want to change the base?
http-client-java, unknown map to binary data #4738
Conversation
No changes needing a change description found. |
You can try these changes here
|
There was a problem hiding this comment.
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).
return ClassType.OBJECT; | ||
if (JavaSettings.getInstance().isUseObjectForUnknown()) { | ||
return ClassType.OBJECT; | ||
} else { | ||
return ClassType.BINARY_DATA; | ||
} |
There was a problem hiding this comment.
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.
...n/java/com/type/property/additionalproperties/models/ExtendsUnknownAdditionalProperties.java
Show resolved
Hide resolved
@@ -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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this 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.
Fix #4669
Added
use-object-for-unknown
to both swagger/typespec flag. Default isfalse
, meaning useBinaryData
.(we had 2 or 3 GA lib get affected, and may need to use this flag)
Pending issue Azure/autorest.java#2964