Add Pageable to applicable controllers to prevent issue in PagedResourcesAssembler

[donovanmuller]

Fixes #1382

If Spring Boot were to be updated to 1.5.3, it would bring in spring-data-commons:1.13.3.RELEASE which includes a fix for DATACMNS-1042 (Self link created with pagination request parameters) in spring-projects/spring-data-commons@a8ff4ea. This commit breaks certain controllers using the PagedResourcesAssembler in spring-cloud-dataflow-server-core.

For example, the following exception will be thrown (using the local server for testing) when requesting http://localhost:9393/apps:

2017-04-22 22:24:45.941 ERROR 2253 --- [nio-9393-exec-8] o.s.c.d.s.c.RestControllerAdvice         : Caught exception while handling a request

java.lang.IllegalArgumentException: Page size must not be less than one!
	at org.springframework.data.domain.AbstractPageRequest.<init>(AbstractPageRequest.java:47) ~[spring-data-commons-1.13.3.RELEASE.jar:na]
	at org.springframework.data.domain.PageRequest.<init>(PageRequest.java:63) ~[spring-data-commons-1.13.3.RELEASE.jar:na]
	at org.springframework.data.web.PagedResourcesAssembler.addPaginationLinks(PagedResourcesAssembler.java:222) ~[spring-data-commons-1.13.3.RELEASE.jar:na]
	at org.springframework.data.web.PagedResourcesAssembler.createResource(PagedResourcesAssembler.java:205) ~[spring-data-commons-1.13.3.RELEASE.jar:na]
	at org.springframework.data.web.PagedResourcesAssembler.toResource(PagedResourcesAssembler.java:115) ~[spring-data-commons-1.13.3.RELEASE.jar:na]
	at org.springframework.cloud.dataflow.server.controller.AppRegistryController.list(AppRegistryController.java:...) ~[classes/:na]
...

The reason this happens is because a new PageImpl, with default values of 0 for page and page size, is being shipped to the PagedResourcesAssembler. This (new) line in PagedResourcesAssembler creates a new PageRequest with the passed in Pageable and is where the size < 1 check blows up because size == 0.

So we need to add a Pageable param (with sane defaults provided by PageableHandlerMethodArgumentResolver) to the affected controller methods and use that when building the PageImpl.

Also changed the default page size to 2000 on the REST templates to match DEFAULT_MAX_PAGE_SIZE, since 10000 would be converted to 2000 anyway.

[donovanmuller]

A before and after for StreamDefinitionController#listRelated

[ilayaperumalg]

@donovanmuller I haven't been able to reproduce the issue with spring-data-commons:1.13.3.RELEASE. it definitely makes sense to have Pageable injected into PageImpl. Merging this change.

[ilayaperumalg]

The appRegistry.findAll() can be reused from what we computed above in the same method.

[ilayaperumalg]

will fix while merging.

[ilayaperumalg]

@donovanmuller Thank you for the contribution. Merged the PR as 2184068 along with eae98f4

[sabbyanandan]

Thanks, @donovanmuller and @ilayaperumalg. Unfortunately, this commit has a side effect on Flo's pagination (#1397). We may have to revert the commit and investigate the applicability of PR and it's side effects post 1.2.0.

[trisberg]

Revert PR: #1400

Let's make these changes after 1.2.0.RELEASE

[ilayaperumalg]

The root cause of this issue is that AppRegistry doesn't have findAll(Pageable) method in its implementation. Since AppRegistryController always returns PagedResources, the UI that expects the paginated resources from AppRegistryController goes into the loop.

As a follow up, I have created this story: #1401

[kalvarez2]

So, what version of spring-boot does work?

[kalvarez2]

so... I am trying to write an implementation of spring-cloud-deployer-spi at version 1.2.0.RELEASE. It is an spring-boot app, so my parent is spring-boot-starter-parent at version "1.4.6.RELEASE", also trried at version "1.5.2.RELEASE". The app boots just fine, but then I am getting this error when trying to use the UI... what version should I set on my pom for the parent or for -spi to get this error to go away?

[vijaykalidindi]

Same problem here. Is there any workaround while the fix is being worked on?

[SuperMohit]

Hi,
I am facing same issue. Is there work around? or which version should I use to make this work?

[sabbyanandan]

Hi, @kalvarez2, @vijaykalidindi, @SuperMohit. The fix is underway via #1545. Once this is merged (hopefully today), we will update the relevant bits associated with this PR and release it in the upcoming 1.2.2.RELEASE.

Thanks for the patience.

[ilayaperumalg]

This can be closed as #1545 is merged with the changes from this PR.