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

docs: Improve SpringQueryMap documentation #1163

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bombo-dev
Copy link

Problem

When using the @SpringQueryMap annotation, it was difficult to discover the existence of the QueryMapEncoder class.
The reference path is as follows:
@SpringQueryMap -> @QueryMap -> @Param Usage All

Additionally, when using @SpringQueryMap, the default injected implementation is FieldQueryMapEncoder.
However, in FeignClientsConfiguration, the injected bean can change depending on the @ConditionalOnClass condition.

@Bean
@ConditionalOnClass(name = "org.springframework.data.domain.Pageable")
@ConditionalOnMissingBean
public QueryMapEncoder feignQueryMapEncoderPageable() {
	PageableSpringQueryMapEncoder queryMapEncoder = new PageableSpringQueryMapEncoder();
	if (springDataWebProperties != null) {
		queryMapEncoder.setPageParameter(springDataWebProperties.getPageable().getPageParameter());
		queryMapEncoder.setSizeParameter(springDataWebProperties.getPageable().getSizeParameter());
		queryMapEncoder.setSortParameter(springDataWebProperties.getSort().getSortParameter());
	}
	return queryMapEncoder;
}

Although FieldQueryMapEncoder is marked as deprecated in QueryMapEncoder, it still functions as the default implementation.

Solution

I have explicitly specified this behavior in the annotation documentation.
I encountered this issue while debugging, and it took me a long time to identify the cause.
By adding this information, I hope others can find the relevant details more quickly.

Consideration

I initially considered adding the note:

"The default implementation is FieldQueryMapEncoder, but it can change to PageableSpringQueryMapEncoder depending on the conditions."

However, since this behavior is subject to change, I decided not to include it.
If necessary, I am open to adding this detail.

…onfiguration In SpringQueryMap

- Added @see references to feign.QueryMapEncoder and FeignClientsConfiguration
- Improved documentation for better discoverability

Signed-off-by: Bombo-Dev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants