Skip to content

Purge private classes and interfaces in @Bean methods #13490

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

Closed
dsyer opened this issue Jun 15, 2018 · 3 comments
Closed

Purge private classes and interfaces in @Bean methods #13490

dsyer opened this issue Jun 15, 2018 · 3 comments
Labels
status: duplicate A duplicate of another issue theme: performance Issues related to general performance

Comments

@dsyer
Copy link
Member

dsyer commented Jun 15, 2018

I would like Spring Boot autoconfiguration to be a public (or at least package-private) contract, so that I can use it to create functional bean registrations with all the same features. There's a barrier to doing that, which is that there are some private types that can't be used, even from a class in the same package.

There are a bunch of these in Spring Boot autoconfiguration, generally following one of two patterns. The first is a private class with a public interface, e.g. in ErrorMvcAutoConfiguration:

	@Bean
	public ErrorPageCustomizer errorPageCustomizer() {
		return new ErrorPageCustomizer(this.serverProperties);
	}

	private static class ErrorPageCustomizer implements ErrorPageRegistrar, Ordered {
		...
	}

The second is a private class with a private interface, e.g. in WebMvcAutoConfiguration:

	@Configuration
	@ConditionalOnEnabledResourceChain
	static class ResourceChainCustomizerConfiguration {

		@Bean
		public ResourceChainResourceHandlerRegistrationCustomizer resourceHandlerRegistrationCustomizer() {
			return new ResourceChainResourceHandlerRegistrationCustomizer();
		}

	}

	interface ResourceHandlerRegistrationCustomizer {

		void customize(ResourceHandlerRegistration registration);

	}

	private static class ResourceChainResourceHandlerRegistrationCustomizer
			implements ResourceHandlerRegistrationCustomizer {
		...
	}
@dsyer dsyer added the theme: performance Issues related to general performance label Jun 15, 2018
@dsyer
Copy link
Member Author

dsyer commented Jun 15, 2018

The second pattern is the most pernicious (the first can be worked around by using the public interface). In spring-boot-autoconfigure it occurs 3 times:

WebMvcAutoConfiguration.ResourceChainResourceHandlerRegistrationCustomizer
HazelcastJCacheCustomizationConfiguration.HazelcastPropertiesCustomizer
WebFluxAutoConfiguration.ResourceChainResourceHandlerRegistrationCustomizer

The first pattern is more widespread. I can generate a list if necessary.

@wilkinsona
Copy link
Member

Generally speaking, I think we should make things package-private and no more. However, there are a few types that I think should be made more visible than that.

For example, ErrorPageCustomizer is exposed by a public method on ErrorMvcAutoConfiguration. This, among other things, breaks the javadoc for ErrorMvcAutoConfiguration as we end up with a link to https://projects.spring.io/spring-boot/#/spring-boot-parent/spring-boot-autoconfigure/apidocs/org/springframework/boot/autoconfigure/web/servlet/error/ErrorMvcAutoConfiguration.ErrorPageCustomizer.html?is-external=true. It doesn't exist and redirects to https://spring.io/projects/spring-boot.

@snicoll snicoll added the status: waiting-for-triage An issue we've not yet triaged label Jun 15, 2018
dsyer pushed a commit to dsyer/spring-boot that referenced this issue Aug 23, 2018
Fixes spring-projectsgh-13490 (at least as far as spring-boot-autoconfigure is
concerned).
snicoll added a commit that referenced this issue Aug 27, 2018
* pr/13795:
  Make some nested private classes package private
@wilkinsona
Copy link
Member

I think #13795 has (largely) taken care of this. If we need further changes, such as those proposed in #14399, I think it makes sense to do so in small, targeted issues and PRs.

@wilkinsona wilkinsona added status: duplicate A duplicate of another issue and removed status: waiting-for-triage An issue we've not yet triaged theme: performance Issues related to general performance labels Sep 10, 2018
@dsyer dsyer added the theme: performance Issues related to general performance label Sep 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue theme: performance Issues related to general performance
Projects
None yet
Development

No branches or pull requests

3 participants