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

Add serializeToString() method #213

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

Conversation

dadoonet
Copy link
Member

This method serializeToString() helps to generate the json String representation of any response or request object.
This could be useful for debugging or for other usages like consuming the search response objects within a JavaScript based frontend.

This will allow generating Json String documents out of the box from any of the response/request objects.
@dadoonet dadoonet added the Category: Enhancement New feature or request label Mar 21, 2022
@dadoonet dadoonet requested a review from swallez March 21, 2022 17:23
@dadoonet dadoonet self-assigned this Mar 21, 2022
@dadoonet
Copy link
Member Author

@swallez I think we should try to think something even more useful.
Let me know if it's doable.

I'd love to have the method serializeToString() available for every request or response object out of the box.

Instead of writing:

SearchResponse<Person> resp = ...;
String json = new JacksonJsonpMapper().serializeToString(resp);

We could write:

SearchResponse<Person> resp = ...;
String json = resp.toJson();

Would it be good for you?

@swallez
Copy link
Member

swallez commented Mar 24, 2022

Would it be good for you?

I'd like to limit the number of framework-related methods on API classes to the bare minimum, and I don't expect serializeToString to be widely used. So better keep it on the mapper object (similar to what exists on Jackson's ObjectMapper).

@frank-montyne
Copy link

I don't really agree with the quote of @swallez '...and I don't expect serializeToString to be widely used....'
The 'serializeToString' method is very useful for people not using Jackson but Gson for example. Even more useful would be to make the Json library used pluggable. Thanks for taking this into consideration!

@rharsh-arkin
Copy link

@dadoonet When can we expect this to get merged?

@dadoonet
Copy link
Member Author

@swallez I think that we can now use:

JsonpUtils.toJsonString(response, jacksonJsonpMapper);

So should we close this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants