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

Support Spring Data Pageable in Feign Client #26

Closed
karlnicholas opened this issue May 25, 2018 · 24 comments
Closed

Support Spring Data Pageable in Feign Client #26

karlnicholas opened this issue May 25, 2018 · 24 comments
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@karlnicholas
Copy link
Contributor

The pageable support that made it into Spring Cloud Netflix version of Feign didn't make it into this "fork" or version of the project. This is the version of Feign I get when I use Spring Boot 2.X, but I want/need to have feign have the Pageable support.

spring-cloud/spring-cloud-netflix#556

Can this PR associated with this issue be ported over here?

@ryanjbaxter
Copy link
Contributor

That PR was never merged and the issue is still open.

@karlnicholas
Copy link
Contributor Author

Hmm. Well, are people not using Feign generally? I'm thinking it's good for having ribbon included but maybe the better practice is to use zuul and RestTemplates and not use Feign?

@ryanjbaxter
Copy link
Contributor

There are people who are using Feign, and we are still actively maintaining Feign as part of Spring Cloud. However activity in the OpenFeign project has dropped and they are looking for contributors.

@karlnicholas
Copy link
Contributor Author

How does one get in touch with AdrianCole for contributing?

@ryanjbaxter
Copy link
Contributor

Gitter is probably best

@karlnicholas
Copy link
Contributor Author

@ryanjbaxter okay. Thanks. I started a gitter account but haven't tried to reach out yet. I worked on building current state of Netflix/feign/spring-cloud-openfeign but it seems the Netflix is a couple revisions back (2.0.0 instead of 2.0.2) and think it's best to wait a bit before looking at this again.

@ryanjbaxter
Copy link
Contributor

What do you mean Netflix is a couple revisions back?

@karlnicholas
Copy link
Contributor Author

Yea, the spring-cloud-netflix pom refers to the spring-cloud-build pom as the parent pom but refers to version 2.0.0-BUILD-SNAPSHOT. However the spring-cloud-build pom is on 2.0.2-BUILD-SNAPSHOT. It's the same for the spring-cloud-openfeign pom. I loaded and built the spring-cloud-build locally and updated the Netflix and Openfeign though I was a little concerned about submitting a patch to update the pom when I don't know what else might need to be done to bring the projects up to date. For example, the latest version for an application I was working on was 2.0.1.RELEASE and I could not go to 2.0.2.RELEASE because the Finchley.RC1 dependency needs 2.0.1.RELEASE. So at this point getting everything built locally so I could work on adding Pageable support to the openfeign project was getting a little daunting especially considering that the Netflix unit tests didn't pass right away on my windows machine. I had windows unit test issues for the feign (not openfeign) project but submitted a patch last weekend which they accepted. I wasn't worried so much about figuring out how to develop the Netflix stuff on a windows platform as much as the whole 2.0.0/2.0.1/2.0.2/Finchley.RC1 version resolution and figured maybe looking at Pageable support for Openfeign can wait until things settle down a little bit.

@ryanjbaxter
Copy link
Contributor

Are you referring to the Boot version?

@karlnicholas
Copy link
Contributor Author

I assume so but mostly I'm referring to the version numbers of the pom.xml's in question.

@ryanjbaxter
Copy link
Contributor

Oh I see what you are saying, spring-cloud-netflix and openfeign are still using spring-cloud-build 2.0.0.BUILD-SNAPSHOT. I will get that changes today, although it shouldn't make a huge difference.

@ryanjbaxter
Copy link
Contributor

Should be all set now

@karlnicholas
Copy link
Contributor Author

Okay, let me have a look.

@Tcharl
Copy link
Contributor

Tcharl commented Aug 6, 2018

Hi Guys,

What's new on this one? This is kind of a blocker for spring-cloud addicts...

@ryanjbaxter
Copy link
Contributor

It’s labeled as help wanted, so we are looking to the community to add support

@karlnicholas
Copy link
Contributor Author

Nothing new here. I put it on the back burner and rewrote everything in RestTemplate. RestTemplate seems to have more implicit Pivotal support since Feign is a Netflix project.

@Tcharl
Copy link
Contributor

Tcharl commented Aug 7, 2018

But Feign as better support when using eureka/netflix/ribbon than RestTempl: using resttemplate when dealing with multiple service instance is a pain with plain old mvc.

Also, the solution evoked in spring-cloud/spring-cloud-netflix#556 by @IsNull seems to work out of the box. It look simple to integrate as there's a PR associated.

@Julyandyou
Copy link

Support Spring Data Pageable in Feign Client #556
According to @IsNull's operation, Still unable to run ....

@ryanjbaxter
Copy link
Contributor

Closed via 6e0e636

@ryanjbaxter ryanjbaxter added this to the 2.1.1.RELEASE milestone Mar 6, 2019
@morth
Copy link

morth commented Apr 16, 2019

For people that are having difficulties to get this working after upgrading to Greenwich.SR1:

Query encoding won't work when you annotate your Pageable with @RequestParam as supposed in spring-cloud/spring-cloud-netflix#556 in your Feign client - you'll have to use a plain Pageable as you do in your controller method.

@roll57
Copy link

roll57 commented Apr 26, 2019

Using a plain Pageable for POST request with a body will not work.

I guess it is still required in that case to apply a workaround with a custom annotation and having a bean extending AnnotatedParameterProcessor

@luizhenriquesantana
Copy link

Just add this to your controller:

@Bean public Module pageJacksonModule() { return new PageJacksonModule(); }

@Hollerweger
Copy link

Hollerweger commented Oct 29, 2020

I still see an issue here. The PageJacksonModule Bean that is registered in org.springframework.cloud.openfeign.FeignClientsConfiguration is not registered as a Module in ObjectMapper and missing when it tries to map Page and therefore fails.
When i try to register the Bean again then it fails because the Bean is already registered by FeignClientsConfiguration.
@Bean public Module pageJacksonModule() { return new PageJacksonModule(); }

Registering the PageJacksonModule in the objecMapper within a Controller solved the issue for me:

@Autowired
private ObjectMapper objectMapper;

@PostConstruct
public void addObjectMapperModules() {
	objectMapper.registerModule(new PageJacksonModule());
}

@zteater
Copy link

zteater commented Nov 6, 2020

PageJacksonModule was not enough to get this working for me. I was seeing this error:

Caused by: org.springframework.http.converter.HttpMessageConversionException: Type definition error: [simple type, class org.springframework.data.domain.Sort]; nested exception is com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot construct instance of `org.springframework.data.domain.Sort` (no Creators, like default constructor, exist): cannot deserialize from Object value (no delegate- or property-based Creator)

Registering Sort with Jackson resolved it though:

@Autowired
private ObjectMapper objectMapper;

@PostConstruct
public void addObjectMapperModules() {
	objectMapper.registerModule(new PageJacksonModule());
	objectMapper.registerModule(new SortJacksonModule());
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

9 participants