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

Fully Support Null Fields when Using Custom ObjectSerializer #29238

Merged
merged 3 commits into from
Jun 7, 2022

Conversation

alzimmermsft
Copy link
Member

Description

Fixes #28610

Resolves a gap in functionality where a custom ObjectSerializer would be used to convert a customer owned type into a Map<String, Object> which would be serialized by JacksonAdapter to be sent to the service. This didn't handle null key-value pairs properly as JacksonAdapter is configured, and will continue to be configured, to ignore null values, but this defeated the purpose of the custom ObjectSerializer to allow for custom handling to be applied to customer owned types.

Now, if an ObjectSerializer is included in the SearchClient it will be used to convert the document into a JSON string and be injected directly into the serialized JSON, giving full control of document serialization to a customer if they choose, or need, to use it. This should also add small performance improvement as before documents would be converted to JSON to be converted into a Map<String, Object> and would then later be converted back to JSON when sending the final request. Customers not using ObjectSerializer will continue to see the same runtime behaviors as before where null field values aren't included in the document sent to the service.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

@alzimmermsft alzimmermsft added Search Client This issue points to a problem in the data-plane of the library. labels Jun 6, 2022
@alzimmermsft alzimmermsft self-assigned this Jun 6, 2022
@alzimmermsft
Copy link
Member Author

/azp run java - search - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@alzimmermsft
Copy link
Member Author

/azp run java - search - tests

@alzimmermsft alzimmermsft marked this pull request as ready for review June 7, 2022 13:24
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Comment on lines +76 to +77
boolean startsWithCurlyBrace = documentJson.startsWith("{");
boolean endsWithCurlyBrace = documentJson.endsWith("}");
Copy link
Contributor

Choose a reason for hiding this comment

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

Is content expected to be trimmed? I.e. is \t\t{ ... }\n\n possible ?

Copy link
Member Author

Choose a reason for hiding this comment

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

It should be trimmed given that it's serialization of the object without passing through other calls.

@alzimmermsft alzimmermsft merged commit 1710bc4 into Azure:main Jun 7, 2022
@alzimmermsft alzimmermsft deleted the AzSearch_NullingFields branch June 7, 2022 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. Search
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Search SDK Cannot Sends Null Field Values with Document Merge or MergeOrUpload
3 participants