micronaut-projects / micronaut-data

Ahead of Time Data Repositories
Apache License 2.0
459 stars 194 forks source link

Invalid query parameters when using Pageable as argument to declarative http client #219

Open Uniqen opened 4 years ago

Uniqen commented 4 years ago

It would be very usable if Pageable was converted to the correct query parameters when used with the declarative http client. For example for an API-gateway that should just forward the request or when writing tests. This would also make it possible to share a common interface between Controller and Client.

The problem as I see it is to make it a non breaking change or is there a way to configure how the query parameters is resolved that I don't know about?

Steps to Reproduce

  1. Create Controller with Pageable as argument
    @Controller("/pets")
    class PetController {
    @Get("/{?pageable*}") // No really needed but for symmetrical 
    List<NameDTO> all(Pageable pageable) {
        return petRepository.list(pageable);
    }
    }
  2. Create corresponding declarative http client
    @Client("/pets")
    public interface PetClient {
    @Get("/{?pageable*}")
    List<NameDTO> all(Pageable pageable);
    }
  3. Use client to invoke controller.
    List<NameDTO> results = petClient.all(Pageable.from(1,1).order("name", DESC));

    Expected Behaviour

    The pageable should be sent using the query parameters page, size and sort: GET /parent/?size=1&sort=name%2CDESC&page=1

Actual Behaviour

The query string is invalid (both page (sent as number) and sort) plus som additional irrelevant parameters. GET /pets/?sort=io.micronaut.data.model.DefaultSort%40f331e9d7&sorted=true&number=1&orderBy=io.micronaut.data.model.Sort%24Order%40f331e999&offset=1&size=1

And then an exception is raised on the server side since the sorting is invalid:

21:23:05.541 [pool-2-thread-2] ERROR i.m.h.s.netty.RoutingInBoundHandler - Unexpected error occurred: Cannot sort on non-existent property path: io.micronaut.data.model.DefaultSort@f331e9d7
java.lang.IllegalArgumentException: Cannot sort on non-existent property path: io.micronaut.data.model.DefaultSort@f331e9d7
    at io.micronaut.data.model.query.builder.AbstractSqlLikeQueryBuilder.lambda$buildOrderBy$35(AbstractSqlLikeQueryBuilder.java:1310)
    at java.util.Optional.orElseThrow(Optional.java:290)
    at io.micronaut.data.model.query.builder.AbstractSqlLikeQueryBuilder.buildOrderBy(AbstractSqlLikeQueryBuilder.java:1310)
    at io.micronaut.data.hibernate.operations.HibernateJpaOperations.lambda$findAll$6(HibernateJpaOperations.java:252)
    at io.micronaut.transaction.support.AbstractSynchronousTransactionManager.executeRead(AbstractSynchronousTransactionManager.java:157)
    at io.micronaut.data.hibernate.operations.HibernateJpaOperations.findAll(HibernateJpaOperations.java:245)
    at io.micronaut.data.runtime.intercept.DefaultFindAllInterceptor.intercept(DefaultFindAllInterceptor.java:52)
    at io.micronaut.data.runtime.intercept.DefaultFindAllInterceptor.intercept(DefaultFindAllInterceptor.java:36)
    at io.micronaut.data.intercept.DataIntroductionAdvice.intercept(DataIntroductionAdvice.java:78)
    at io.micronaut.aop.chain.MethodInterceptorChain.proceed(MethodInterceptorChain.java:69)
    at example.repositories.PetRepository$Intercepted.list(Unknown Source)
    at example.controllers.PetController.all(PetController.java:24)
...

Environment Information

Example Application

graemerocher commented 4 years ago

This would be nice. Currently the client implementation is not flexible enough to allow custom encoding of arguments. So we would need an improvement in Micronaut Core first to implement this.

Uniqen commented 4 years ago

I've duplicated the Pageable (and DefaultPageable, Sort, DefaultSort) as a workaround until it is resolved. This still blocks sharing interface between Client and Controller but at least makes it easy to use in tests and api gateway.

@Controller("/pets")
class PetGatewayController {
    @Get("/{?pageable*}")
    List<NameDTO> all(Pageable pageable) {
        return petClient.all(PageableQuery.from(pageable));
    }
}

@Client("/pets")
public interface PetClient {
    @Get("/{?pageable*}")
    List<NameDTO> all(PageableQuery pageable);
}

//Usage in tests is the same as Pageable
PageableQuery pageable = PageableQuery.from(1, 1)
    .order("name", DESC);
List<NameDTO> results = petClient.all(pageable);

If anyone else is interested in the workaround see commit: https://github.com/Uniqen/micronaut-predator/commit/37147347b4e1c6d59d2816e658c3ba9d6ed623d0

njimenezotto commented 3 years ago

Another option is to pass to the @Client the values of the Pageable like this:

@QueryValue("size") int size,
@QueryValue("number") int number

And the @Controller can still declare the

@QueryValue("pageable") Pageable page
regine-chan commented 12 months ago

Any updates on this? Would be terrific to be able to share interface between controllers and clients.

graemerocher commented 12 months ago

In Micronaut 4 we added support for custom message writers so maybe it is possible now

Hc747 commented 1 month ago

Bumping this - any updates for Pageable parameters within @Controller methods?