KEMBAR78
docs: Improve SpringQueryMap documentation by bombo-dev · Pull Request #1173 · spring-cloud/spring-cloud-openfeign · GitHub
Skip to content

Conversation

@bombo-dev
Copy link
Contributor

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.

related to #1163
cc. @OlgaMaciaszek Thanks for your guidance!

…ration In SpringQueryMap

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

Signed-off-by: Bombo-Dev <bombo96@chungwoon.ac.kr>
Copy link
Collaborator

@OlgaMaciaszek OlgaMaciaszek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @bombo-dev. LGTM.

@OlgaMaciaszek OlgaMaciaszek moved this to In Progress in 2023.0.6 Mar 14, 2025
@OlgaMaciaszek OlgaMaciaszek moved this to In Progress in 2024.0.1 Mar 14, 2025
@OlgaMaciaszek OlgaMaciaszek moved this to In Progress in 2025.0.0-M3 Mar 14, 2025
@OlgaMaciaszek OlgaMaciaszek added this to the 4.1.5 milestone Mar 14, 2025
@OlgaMaciaszek OlgaMaciaszek merged commit 219dbb7 into spring-cloud:4.1.x Mar 18, 2025
2 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in 2024.0.1 Mar 18, 2025
@github-project-automation github-project-automation bot moved this from In Progress to Done in 2025.0.0-M3 Mar 18, 2025
@github-project-automation github-project-automation bot moved this from In Progress to Done in 2023.0.6 Mar 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Done
Status: Done
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants