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

Added json.md - Serialization part #1083

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Jai2305
Copy link
Contributor

@Jai2305 Jai2305 commented Jul 15, 2024

Description

  • Added documentation json.md in guides folder for Json serialization with openSearch java Client.
  • Added code samples to guide users to use the toJsonString method added here and manual serialization with serialize method in JsonpSerializable
  • Added a Working Sample , created a new directory for json in /samples.

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.

@Jai2305
Copy link
Contributor Author

Jai2305 commented Jul 15, 2024

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.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
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.

Signed-off-by: Jai2305 <jainjai2305@gmail.com>
Copy link
Member

@dblock dblock left a 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

@@ -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))
Copy link
Member

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.

@@ -0,0 +1,59 @@
- [Working with Json](#working-with-json)
- [Serialization](#serialization)
- [Using toJsonString method](#using-tojsonstring-method)
Copy link
Member

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)

Copy link
Member

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.

- [Working with Json](#working-with-json)
- [Serialization](#serialization)
- [Using toJsonString method](#using-tojsonstring-method)
- [Manual serialization](#manually-serialize-by-providing-mapper-and-generator)
Copy link
Member

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"


# Working with Json

OpenSearch Java client seamlessly integrate with JSON providing serialization and deserialization capability.
Copy link
Member

Choose a reason for hiding this comment

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

integrates

, providing

For demonstration let's consider an instance of `SearchRequest`.

```java
public static void main(final String[] args) {
Copy link
Member

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

}

```

Copy link
Member

Choose a reason for hiding this comment

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

Remove extra lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants