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

Add statement about of Pageable to documentation #26

Open
terribleherbst opened this issue Mar 11, 2019 · 5 comments
Open

Add statement about of Pageable to documentation #26

terribleherbst opened this issue Mar 11, 2019 · 5 comments
Labels
Feature Request Feature request. Spring Relates to the SpringConverter

Comments

@terribleherbst
Copy link

Setup information
hikaku version: 2.0.0-SNAPSHOT
implementation converter: SpringConverter

Describe the bug
Spring data supports the idea of Pageable. A pageable is an object build by request parameter, supporting page, size and sort (parameter names are changeable). These parameters might be part of an api, but are not explizit mapped to query parameter. The documentation says "Query Parameter based on an object" are not supported, but I think a Pageable should be mentioned explizitly.

Expected behavior
Add support for Pageable or add a note that Pageable are currently not supported in hikaku.

Code samples:
Can you provide specifications or code snippets?

public ResponseEntity<Person> getPersons(
        @RequestParam(name = "language", required = false) String language,
        @PageableDefault Pageable pageable) {
...
}
@terribleherbst terribleherbst added the Bug Something isn't working label Mar 11, 2019
@cc-jhr cc-jhr added Feature Request Feature request. and removed Bug Something isn't working labels Mar 12, 2019
@cc-jhr
Copy link
Collaborator

cc-jhr commented Mar 13, 2019

Note: Both interface and annotation are provided by org.springframework.data:spring-data-rest-core

@cc-jhr cc-jhr added the Spring Relates to the SpringConverter label Mar 13, 2019
@jrehwaldt
Copy link

jrehwaldt commented Jul 19, 2019

I'd like to see something like this as well, but not specific for Pageable. We implemented our own HandlerMethodArgumentResolver (I think Spring Data does the same for Pageable), which takes a request and creates instances that may be used as controller method arguments.

What I'd like to see is a HikakuConfig taking a list of something like DifferenceEvaluator from XmlUnit. Each spotted difference is passed through all DifferenceEvalutators and can be altered there. This way, a PagaeableDifferenceEvaluator would be written that checks if Pageable is a method argument when the spec requires the three query parameters page, size and sort.

What do you guys think?

Edit: Actually, all current ignoreXxx config options could be easily implemented this way.

@cc-jhr
Copy link
Collaborator

cc-jhr commented Jul 19, 2019

Hi @jrehwaldt,
thank you for giving feedback on this topic.

First of all, as stated in the list of currently not supported features in the README.md of the spring converter, query parameter based on classes/interfaces are currently not supported in general. So depending on the solution for that, this might solve the cases mentioned above already. It might make sense to implement the general approach before we focus on specific interfaces or classes.

From my perspective the PagaeableDifferenceEvaluator idea wouldn't work. Hikaku is designed to convert a specification or an implementation to an internal domain model of a REST endpoint. After that the model for the specification is compared to the model of the implementation in the core. At the time of the actual comparison, there is therefore no additional information about the implementation, such as class types and the like. This is intentional.

However I like the idea of creating rules dynamically for the config to ignore specific bits, instead of static rules.

@cc-jhr
Copy link
Collaborator

cc-jhr commented Aug 11, 2019

Created an issue for the general case. See #47

@jrehwaldt
Copy link

As I see it the proposal from #47 unfortunately doesn't solve either the general nor the original issue.

  • Pageable is an interface, which you excluded from your proposal
  • The proposal assumes a strict mapping from query parameter to an object property, but the latter should be an implementation detail and not bound to external names. Imho, a query parameter sort could be mapped to sortOrder internally.

A truly general solution would be to allow deriving arbitrary parameters from unknown objects declared in endpoint mappings, something like (Object) -> Set<Parameter>, with Parameter being any of these.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Feature request. Spring Relates to the SpringConverter
Projects
None yet
Development

No branches or pull requests

3 participants