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

Inconsistent Pageable Sort serialization between openfeign and spring mvc #675

Open
philippeu opened this issue Jan 29, 2022 · 8 comments
Open

Comments

@philippeu
Copy link

  • spring boot version: 2.6.3
  • spring cloud version: 2021.0.0 (spring cloud openfeign 3.1.0)

spring serializes Sort as an object

"sort": { "empty": true, "sorted": false, "unsorted": true }

when SortJacksonModule is enabled (feign.autoconfiguration.jackson.enabled: true) Sort is serialize as an array by SortJsonComponent.SortSerializer.

"sort": []

since SortJacksonModule is in the spring context the controller response includes the Sort array creating inconsistency between the controller response with and without SortJacksonModule

@OlgaMaciaszek
Copy link
Collaborator

Hello @philippeu, thanks for reporting this. I was able to reproduce the issue. Will work on it.

@OlgaMaciaszek
Copy link
Collaborator

@philippeu Have looked into this further. Actually, the data that comes from the sort json of this kind: { "empty": true, "sorted": false, "unsorted": true } is not enough to construct a viable Sort object. If you do not set feign.autoconfiguration.jackson.enabled: true, you will simply get an exception, cause it cannot be deserialised. That's probably why @cbezmen has opted for returning null in such a scenario. Could you provide some more details regarding your usecase?

@OlgaMaciaszek OlgaMaciaszek added waiting for feedback and removed bug Something isn't working labels Feb 23, 2022
@philippeu
Copy link
Author

Hi Olga,

I have a service returning Pageable.
by default MappingJackson2HttpMessageConverter, serialize Sort object to (using PageImpl)

"sort": { "empty": true, "sorted": false, "unsorted": true }

This service is calling another one via a feign client which is also returning a Pageable

feign.autoconfiguration.jackson.enabled must be set to true in order for the feign client to deserialize the pagination.

once feign jackson is enabled, it registered the SortJacksonModule deserializer and it can read the paginated response.
this is good

the SortJacksonModule serializer is also registered in ObjectMapper of MappingJackson2HttpMessageConverter and then used to serialize the Sort object to

"sort": []

so services having feign jackson enabled do not provide the same response format (sort is serialized as an array) than the ones having feign jackson disabled (sort is serialized as an object).

I work around that by providing a CustomSortJacksonModule serializer and CustomSortJsonComponent.

@OlgaMaciaszek
Copy link
Collaborator

OlgaMaciaszek commented Feb 23, 2022

@philippeu Could you please provide the code of CustomSortJacksonModule serializer and CustomSortJsonComponent? It will be easier for us to understand your use-case then.

@philippeu
Copy link
Author

Here they are:

public class CustomSortJacksonModule extends SortJacksonModule {

    @Override
    public String getModuleName() {
        return "SortModule";
    }

    @Override
    public Version version() {
        return new Version(0, 1, 0, "", null, null);
    }

    @Override
    public void setupModule(SetupContext context) {
        var serializers = new SimpleSerializers();
        serializers.addSerializer(Sort.class, new CustomSortJsonComponent.SortSerializer());
        context.addSerializers(serializers);

        var deserializers = new SimpleDeserializers();
        deserializers.addDeserializer(Sort.class, new CustomSortJsonComponent.SortDeserializer());
        context.addDeserializers(deserializers);
    }
}
public class CustomSortJsonComponent {
    public static class SortSerializer extends JsonSerializer<Sort> {
        private record CustomSort(boolean empty, boolean sorted, boolean unsorted) {};

        @Override
        public void serialize(Sort value, JsonGenerator gen, SerializerProvider serializers) throws IOException {
            gen.writeObject(new CustomSort(value.isEmpty(), value.isSorted(), value.isUnsorted()));
        }

        @Override
        public Class<Sort> handledType() {
            return Sort.class;
        }
    }

    public static class SortDeserializer extends JsonDeserializer<Sort> {
        @Override
        public Sort deserialize(JsonParser jsonParser, DeserializationContext deserializationContext) throws IOException {
            var treeNode = jsonParser.getCodec().readTree(jsonParser);
            if (treeNode.isArray()) {
                var arrayNode = (ArrayNode) treeNode;
                var orders = new ArrayList<Sort.Order>();
                arrayNode.forEach(jsonNode -> orders.add(new Sort.Order(Sort.Direction.valueOf(jsonNode.get("direction").textValue()), jsonNode.get("property").textValue())));
                return Sort.by(orders);
            }
            return null;
        }

        @Override
        public Class<Sort> handledType() {
            return Sort.class;
        }
    }
}

@philippeu
Copy link
Author

a better solution would be to have an specific object mapper for feign not interfering with spring one maybe.

@OlgaMaciaszek
Copy link
Collaborator

@philippeu - thanks for providing the samples. I thought you meant deserialising, cause when a Sort body is passed on, unless you provide an Encoder bean different from PageableSpringEncoder, it will not be serialised, but passed as request params instead. The current serialisation provides the bits necessary to do the sorting. Since you have a viable workaround, we're going to wait with this to see if there's any more interest in it within the community.

@philippeu
Copy link
Author

ok, thanks Olga!

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

No branches or pull requests

3 participants