Skip to content

Add hypermedia autoconfiguration for WebTestClient. #20372

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Add hypermedia autoconfiguration for WebTestClient. #20372

wants to merge 2 commits into from

Conversation

gregturn
Copy link
Contributor

@gregturn gregturn commented Mar 2, 2020

  • Provide CodecCustomizer for WebTestClient.

Related: #16020.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 2, 2020
wilkinsona
wilkinsona requested changes Mar 3, 2020
View reviewed changes
...ava/org/springframework/boot/test/autoconfigure/hateoas/HypermediaTestAutoConfiguration.java Outdated
public class HypermediaTestAutoConfiguration {

@Bean
@ConditionalOnMissingBean
Copy link
Member

Choose a reason for hiding this comment

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

This will back off if any other CodecCustomizer beans exist. I suspect that isn't what you want.

Copy link
Contributor Author

@gregturn gregturn Mar 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Certainly not.

I wanted to guard against the other branch that introduces WebClient support (#20374) and basically has the same type of bean. Just in another module. And of course, NOT backing off for just ANY CodecCustomizer.

Suggestions? Perhaps drop the backing off policy here and now, and then resolve such bean conflicts when we get to #20374?

Copy link
Member

Choose a reason for hiding this comment

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

The CodecCustomizer here and in #20374 seem to be identical. I suspect it could be defined in one place. If so, it should probably be in spring-boot-autoconfigure. It could then be included via spring.factories in the relevant test auto-configuration as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess I seemed to have issues (initially) with spring-boot-test-autoconfigure seeing things so I copied it.

Let me polish up #20374 applying some of these comments there. After that is merged, I can rebase this to use that.

...rg/springframework/boot/test/autoconfigure/hateoas/HypermediaTestAutoConfigurationTests.java Outdated
Comment on lines 19 to 20
import static org.assertj.core.api.Assertions.*;
import static org.mockito.Mockito.*;
Copy link
Member

Choose a reason for hiding this comment

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

We don't use wildcard imports in Spring Boot's codebase. Building the project locally should show any problems like these as they're enforced by Checkstyle and Spring Java Format.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

;)

...ava/org/springframework/boot/test/autoconfigure/hateoas/HypermediaTestAutoConfiguration.java Outdated
import com.fasterxml.jackson.databind.ObjectMapper;

/**
* @author Greg Turnquist
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a description of the class please? Something like the following:

Auto-configuration for using hypermedia with {@link WebTestClient}.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

wilkinsona
wilkinsona reviewed Mar 3, 2020
View reviewed changes
...ava/org/springframework/boot/test/autoconfigure/hateoas/HypermediaTestAutoConfiguration.java Outdated

@Bean
@ConditionalOnMissingBean
@ConditionalOnClass({WebTestClient.class})
Copy link
Member

@wilkinsona wilkinsona Mar 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think this will need for be @ConditionalOnClass for HypermediaMappingInformation, ObjectMapper, and perhaps Jackson2JsonDecoder as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@gregturn gregturn force-pushed the hateoas-webtestclient-support branch 2 times, most recently from ca1b4c8 to fd55ff3 Compare March 3, 2020 14:33
@gregturn
Copy link
Contributor Author

gregturn commented Mar 3, 2020

@wilkinsona applied all corrections.

@gregturn gregturn force-pushed the hateoas-webtestclient-support branch 4 times, most recently from e28140b to ab4f6f1 Compare March 3, 2020 15:24
@gregturn
Add hypermedia autoconfiguration for WebClient.
9bc160d 
* Provide CodecCustomizer to support WebClient.
* Add tests to verify one mediatype, multiple mediatypes, and custom mediatypes all properly register with WebClient and CodecConfigurer.

Related: spring-projects#16020
@gregturn gregturn mentioned this pull request Mar 3, 2020
Closed
gregturn added a commit to gregturn/spring-boot that referenced this pull request Mar 4, 2020
@gregturn
Verify that WebClient hypermedia autoconfig works for WebTestClient.
a8a7782 
* Write test cases that verify WebTestClient will pick up the same autoconfiguration beans created for WebClient.
* Verify it handles one mediatype, multiple mediatypes, and custom mediatypes.

Related: spring-projects#16020.
Resolves spring-projects#20372.
@gregturn gregturn force-pushed the hateoas-webtestclient-support branch from ab4f6f1 to a8a7782 Compare March 4, 2020 16:10
gregturn added a commit to gregturn/spring-boot that referenced this pull request Mar 4, 2020
@gregturn
Verify that WebClient hypermedia autoconfig works for WebTestClient.
93f1516 
* Write test cases that verify WebTestClient will pick up the same autoconfiguration beans created for WebClient.
* Verify it handles one mediatype, multiple mediatypes, and custom mediatypes.

Related: spring-projects#16020.
Resolves spring-projects#20372.
@gregturn gregturn force-pushed the hateoas-webtestclient-support branch from a8a7782 to 93f1516 Compare March 4, 2020 16:29
gregturn added a commit to gregturn/spring-boot that referenced this pull request Mar 4, 2020
@gregturn
Verify that WebClient hypermedia autoconfig works for WebTestClient.
f4bab0c 
* Write test cases that verify WebTestClient will pick up the same autoconfiguration beans created for WebClient.
* Verify it handles one mediatype, multiple mediatypes, and custom mediatypes.

Related: spring-projects#16020.
Resolves spring-projects#20372.
@gregturn gregturn force-pushed the hateoas-webtestclient-support branch from 93f1516 to f4bab0c Compare March 4, 2020 16:51
@gregturn gregturn mentioned this pull request Mar 4, 2020
Closed
@gregturn
Verify that WebClient hypermedia autoconfig works for WebTestClient.
91b181c 
* Write test cases that verify WebTestClient will pick up the same autoconfiguration beans created for WebClient.
* Verify it handles one mediatype, multiple mediatypes, and custom mediatypes.

Related: spring-projects#16020.
Resolves spring-projects#20372.
@gregturn gregturn force-pushed the hateoas-webtestclient-support branch from f4bab0c to 91b181c Compare March 4, 2020 16:53
@gregturn
Copy link
Contributor Author

gregturn commented Mar 4, 2020

After #20374, can you review this one, @wilkinsona?

@wilkinsona wilkinsona added the status: on-hold We can't start working on this issue yet label Mar 11, 2020
@gregturn gregturn mentioned this pull request Mar 23, 2020
Closed
@gregturn
Copy link
Contributor Author

Per our video chat,

@gregturn gregturn closed this Mar 23, 2020
@wilkinsona wilkinsona removed status: on-hold We can't start working on this issue yet status: waiting-for-triage An issue we've not yet triaged labels Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wilkinsona wilkinsona wilkinsona requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gregturn @wilkinsona @spring-projects-issues