EntityConverterManager limitations and solutions

[tgeens]

First of all: I have a need for an abstraction for http-clients and was about to start my own, but then I found http-requests - wow, exactly what I had in mind, very nice job 👍

However, the EntityConverterManager is a bit unaccommodating. More specifically with the EntityReader and EntityWriter interfaces: they don't expose the type to be marshaled to/from. This prevents you from fully delegating this to another library, like for example Jackson.

When I implement the EntityReader interface, it looks like I have to hard-code what the marshaling type is ?

I should be able to deserialize json into a custom POJO, without writing a specialized EntityReader for this POJO, etc ...

EntityConverterManager converter = new EntityConverterManager(Arrays.asList(
                new JacksonEntityReader(new ObjectMapper())
        ));

        HttpEntity httpEntity = new HttpEntity(someInputStreamWithJson);
        MyCustomPojo response = converters.read(MyCustomPojo.class, httpEntity);

I have this actually working locally, by subclassing EntityConverterManager, overriding some methods and introducing new -Reader/-Writer interfaces that actually expose the type in the read/write methods. That is of course not ideal, it would be a lot nicer to have this in http-requests-core.

Are you interested in a PR that fixes all of that ?

It is fully possible to do this without losing backwards compatibility, but it makes the current EntityConverterManager, EntityWriter and EntityReader eventually @Deprecated.

[budjb]

Sure I'd love to see what you came up with!

…

On Thu, Apr 23, 2020, 3:20 AM Toon Geens ***@***.***> wrote: First of all: I have a need for an abstraction for http-clients and was about to start my own, but then I found http-requests - wow, exactly what I had in mind, very nice job 👍 However, the EntityConverterManager is a bit unaccommodating. More specifically with the EntityReader and EntityWriter interfaces: they don't expose the type to be marshaled to/from. This prevents you from fully delegating this to another library, like for example Jackson. When I implement the EntityReader interface, it looks like I have to hard-code what the marshaling type is ? I should be able to deserialize json into a custom POJO, without writing a specialized EntityReader for this POJO, etc ... EntityConverterManager converter = new EntityConverterManager(Arrays.asList( new JacksonEntityReader(new ObjectMapper()) )); HttpEntity httpEntity = new HttpEntity(someInputStreamWithJson); MyCustomPojo response = converters.read(MyCustomPojo.class, httpEntity); I have this actually working locally, by subclassing EntityConverterManager, overriding some methods and introducing new -Reader/-Writer interfaces that actually expose the type in the read/write methods. That is of course not ideal, it would be a lot nicer to have this in http-requests-core. Are you interested in a PR that fixes all of that ? It is fully possible to do this without losing backwards compatibility, but it makes the current EntityConverterManager, EntityWriter and EntityReader eventually @deprecated. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#23>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAKOWO7JTQLN2YDD3FS6BKLRN726TANCNFSM4MO3FVJQ> .

[tgeens]

I tried to minimize my customization, currently available here:

• HttpEntityReader - new interface with different read signature
  HttpEntityConverters - subclasses EntityConverterManager and overrides the read method to make use of the new HttpEntityReader interface, before delegating to the existing implementation.
• JacksonEntityConverter - Jackson based generic entity converter (both reader- and writer)
• HttpEntityConvertersTest - test case showing serialization and deserialization of custom POJOs using the generic JacksonEntityConverter

It's possible I have missed something, but I don't think that was possible before ?

If you agree on the general principles, I can create a PR for http-requests-core (and http-requests-jackson) and we can continue from there.

[tgeens]

While waiting for a reply, I'm going to fork the project and make the improvements I had in mind.

Ping me if you are interested and available to discuss integrating.

[budjb]

I want to make sure I understand the specific use case that led you to this solution. I believe what you're trying to do is perform some JSON marshalling via Jackson to custom POJO's, while currently those Jackson converters assume that we'll only be working with maps and lists. Is that correct?

[tgeens]

Correct.

As a user of this lib, I'm not expecting that I need to write/register entity-converters for straight forward POJOs

[budjb]

Ok! I'd like to aim for limiting the amount of changes to what already exists. From looking at this, it seems all that really needs to change on the existing interface, then, is to pass the class type that we need to marshall to in the read() method. The rest of the changes are a result of that one change it looks like.

I noticed the addition of the content type to the supports method, but it's not currently used. Do you think that would be beneficial?

[budjb]

@tgeens I have some changes staged that I believe address your use case. It follows your changes with a few differences, but overall I believe it should work. Let me know if you agree. I've got a few test cases to write around it before pushing it out.

[tgeens]

I noticed the addition of the content type to the supports method, but it's not currently used. Do you think that would be beneficial?

It's useful in scenario's where you can use a set of default converters that look at the content type. You don't want to attempt to use Jackson for application/xml, leave that for another entity-reader in the chain.

This would be similar to Spring RestTemplate and the Spring Boot HttpMessageConverters, if you would be familiar with that.

[tgeens]

Now that the EntityReader API gets changed - I'd suggest to keep parity in the EntityWriter#supports method.

Use case: you have your ConvertingHttpEntity with a specified content type, let's say application/xml.

Should you use the registered JacksonEntityWriter ? No, but currently there is not a good way out, besides fiddling with converter ordering. Instead, if the EntityWriter#supports method provides the target content-type, the JacksonEntityWriter can back off properly.