-
Notifications
You must be signed in to change notification settings - Fork 183
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
Added json.md - Serialization part #1083
Conversation
In response to @dblock's ask for documentation I have added json.md in guides folder with code samples demonstrating the usage of toJsonString. |
guides/json.md
Outdated
``` | ||
### Using toJsonString method | ||
For classes implementing `PlainJsonSerializable`, which extends `JsonpSerializable`, a default `toJsonString` method is provided. | ||
This method utilizes the default mapper and generator from the JsonpUtils class within the package to streamline the serialization process. |
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 method utilizes the default mapper and generator from the JsonpUtils class within the package to streamline the serialization process. | |
This implementation uses `jakarta.json.spi.JsonProvider` SPI to discover the available JSON provider instance from the classpath and to create a new mapper. The `JsonpUtils` utility class streamlines this serialization process. |
2c1f442
to
30b0be2
Compare
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.
Looks great, I have a bunch of nits, please.
Also link the new doc from USER_GUIDE
CHANGELOG.md
Outdated
@@ -16,6 +16,7 @@ This section is for maintaining a changelog for all breaking changes for the cli | |||
- Document HTTP/2 support ([#330](https://github.com/opensearch-project/opensearch-java/pull/330)) | |||
- Add support for phase_took & search_pipeline request params ([#1036](https://github.com/opensearch-project/opensearch-java/pull/1036)) | |||
- Add an interface PlainJsonSerializable inherit from JsonpSerializable with a default method streamlining serialization ([#1064](https://github.com/opensearch-project/opensearch-java/pull/1064)) | |||
- Add a new file json.md in guides and SerializationBasics in samples/json ([#1083](https://github.com/opensearch-project/opensearch-java/pull/1083)) |
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.
Nit: I would collapse the two changelog items and just say that we added request serialization to JSON.
guides/json.md
Outdated
@@ -0,0 +1,59 @@ | |||
- [Working with Json](#working-with-json) | |||
- [Serialization](#serialization) | |||
- [Using toJsonString method](#using-tojsonstring-method) |
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.
Using toJsonString
(quote, remove method)
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.
Update TOC to match body.
guides/json.md
Outdated
- [Working with Json](#working-with-json) | ||
- [Serialization](#serialization) | ||
- [Using toJsonString method](#using-tojsonstring-method) | ||
- [Manual serialization](#manually-serialize-by-providing-mapper-and-generator) |
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.
Capitalize sections consistently, so "Manual Serialization"
guides/json.md
Outdated
|
||
# Working with Json | ||
|
||
OpenSearch Java client seamlessly integrate with JSON providing serialization and deserialization capability. |
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.
integrates
, providing
guides/json.md
Outdated
For demonstration let's consider an instance of `SearchRequest`. | ||
|
||
```java | ||
public static void main(final String[] args) { |
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.
remove main, keep the body of it
guides/json.md
Outdated
} | ||
|
||
``` | ||
|
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.
Remove extra lines.
eb9bc92
to
ecce2f0
Compare
Signed-off-by: Jai2305 <jainjai2305@gmail.com>
Signed-off-by: Jai2305 <jainjai2305@gmail.com> (cherry picked from commit c40e486) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Merged, thank you! |
(cherry picked from commit c40e486) Signed-off-by: Jai2305 <jainjai2305@gmail.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
toJsonString
method added here and manual serialization with serialize method inJsonpSerializable
Issues Resolved
Documentation for Json serialization with code samples
Work samples demonstrating logging with toJsonString method.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.