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

Increase integration potential of msgpack-jackson #867

Open
extbe opened this issue Jan 8, 2025 · 5 comments
Open

Increase integration potential of msgpack-jackson #867

extbe opened this issue Jan 8, 2025 · 5 comments

Comments

@extbe
Copy link

extbe commented Jan 8, 2025

Hello MessagePack Team!

I use MessagePack Jackson integration in my project and have an issue integrating it with other libraries that rely on Jackson API.

The MessagePackFactory overrides only a part of JsonFactory#createParser methods which limits the integration potential because not all libraries create parser from InputStream or byte[].

What do you think about expanding the list of overriden createParser methods and thus increasing the integration potential?

@extbe extbe changed the title Increase integration potenital of msgpack-jackson Increase integration potential of msgpack-jackson Jan 8, 2025
@komamitsu
Copy link
Member

@extbe Which createParser() do you actually need?

@extbe
Copy link
Author

extbe commented Jan 12, 2025

The one that accepts Reader as a parameter

public JsonParser createParser(Reader r) throws IOException, JsonParseException {
    //...
}

https://github.com/FasterXML/jackson-core/blob/0d2b0f39200d466f49f1abb06d9027053d41483d/src/main/java/com/fasterxml/jackson/core/JsonFactory.java#L1242

@komamitsu
Copy link
Member

@extbe Reader is for char stream while MessagePack is a binary format. So, I don't think Reader works well with MessagePack.

@extbe
Copy link
Author

extbe commented Jan 15, 2025

@komamitsu This is what I'm trying to solve.

I'm trying to integrate Feign, Jackson and MessagePack. MessagePack and Feign both provide Jackson integration. Feign relies on createParser(Reader r) method which seems logical because JSON is a text format. MessagePack is a binary format, and now I also think that it is logical to not support that method.

Feign is a good declarative wrapper for HTTP clients and MessagePack is a good binary format, so I would like to make them compatible.

Initially I was thinking that the correct way is to support that method in msgpack-jackson as it works behind the Jackson API, but now I'm not sure that it will be the right decision. It seems like the integration should be placed in a separate library so that none of the existing APIs will be affected.

Since msgpack-jackson works behind the Jackson API, but implements only a part of the API, would you agree putting the integration library into msgpack-java? The library will integrate msgpack-jackson and Feign taking into account the implementation details of msgpack-jackson.

@komamitsu
Copy link
Member

@extbe

Feign relies on createParser(Reader r) method

I guess it's true only when using the bundled JacksonEncoder and JacksonDecoder. I'm not familiar with Feign, but I'm wondering if you can create own MessagePackEncoder and MessagePackDecoder with Jackson that handles only binary data.

I recommend creating a separate integration library like https://github.com/komamitsu/retrofit-converter-msgpack, as I think msgpack-java should be as simple as possible.

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

No branches or pull requests

2 participants