Skip to content

Allow adding registration directly in ResourceHandlerRegistry #34967

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

Conversation

Allan-QLB
Copy link

Allow to add ResourceHandlerRegistration directly to ResourceHandlerRegistry.
That allows to add subtypes of ResourceHandlerRegistration to create custom instance of ResourceHttpRequestHandler.
For example, we can add a custom registration which creates a custom handler reolving media type of resources without filename extension to "text/html".

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 30, 2025
@Allan-QLB Allan-QLB force-pushed the resource-handler-registery2 branch from 9188348 to 50d7a11 Compare May 30, 2025 10:38
protected @Nullable MediaType getMediaType(HttpServletRequest request, Resource resource) {
String filename = resource.getFilename();
String ext = StringUtils.getFilenameExtension(filename);
if (!StringUtils.hasText(ext)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion:
Currently, the custom handler assumes that resources without a file extension are of type text/html. However may not work well for different context type. To make it more flexible, maybe allowing the user to configure the type gives more control/flexibility and can support many use cases.

Copy link
Author

Choose a reason for hiding this comment

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

Suggestion: Currently, the custom handler assumes that resources without a file extension are of type text/html. However may not work well for different context type. To make it more flexible, maybe allowing the user to configure the type gives more control/flexibility and can support many use cases.

This is just an example to illustrate its use case.

Copy link
Contributor

@DhruvTheDev1 DhruvTheDev1 left a comment

Choose a reason for hiding this comment

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

  • This PR adds a new method to ResourceHandlerRegistry that lets developers add a fully customised ResourceHandlerRegistration instances directly.
  • Improves flexibility and reduces boilerplate config without altering the existing API.
  • Clean and useful edition for users that want more control.
  • Introduces overloaded addResourceHandler(ResourceHandlerRegistration registration) method that extends the API.

Suggestions

  • Currently the custom handler assumes resources without a file extension are of type text/html hence possible solutions:
    • Allowing the default type to be configurable helps support different use cases.
    • Look for a file with .html extension, serving if that is found or responding with a 404 error.
    • Log for a warning

@bclozel bclozel self-assigned this Jun 3, 2025
@bclozel
Copy link
Member

bclozel commented Jun 3, 2025

Thanks for the proposal but I don't think this is needed. Registering a custom ResourceResolver should get you much further with resource caching support and resource content rewriting.

@bclozel bclozel closed this Jun 3, 2025
@bclozel bclozel added in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants