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

Provide full control over the registration of resolvers and transformers in ResourceHandlerRegistration [SPR-12124] #16740

Closed
spring-projects-issues opened this issue Aug 26, 2014 · 16 comments
Assignees
Labels
in: web type: enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Aug 26, 2014

Guillaume DROUET opened SPR-12124 and commented

ResourceHandlerRegistration is designed to be extended, however it is currently not possible to specify any other implementation. In some cases, we may desire to specialize methods like getResourceResolvers and getResourceTransformers (which are already in a protected access level). For instance, we can intercept all created ResourceResolver and ResourceTransformer instances in order to know what elements will be executed later in the chains of responsibility.

So the ResourceHandlerRegistry should expose an API like this:

ObjectFactory<ResourceHandlerRegistration> factory = ...;
registry.resourceHandlerRegistrationFactory(factory).addResourceHandler("/**");


Affects: 4.1 RC2

Referenced from: commits fbfb7a3, 0b02551

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 26, 2014

Rossen Stoyanchev commented

Is your end goal to check what resolvers and transformers are registered? For example we could make some changes in WebMvcConfigurationSupport to make it easier to access this information after all resolvers and transformers have been registered.

Or otherwise we could provide a protected method to allow creating a custom ResourceHandlerRegistry and from there a custom ResourceHandlerRegistration. At least that's the originally intended way.

A registry.resourceHandlerRegistrationFactory approach might not be effective since the registry can be invoked from any number of WebMvcConfigurer instances (in the same, modular app) in which case an ObjectFactory may be registered too late.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 26, 2014

Guillaume DROUET commented

Sure if you can make those changes it's perfect, I did not dare to offer this solution because it requires broader changes.

I was suspecting the issue of multi-WebMvcConfigurer instances but I was not sure of it. It would be ok for "local" usage but could be disturbing for user who desires to apply the factory to all instances creation.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 26, 2014

Guillaume DROUET commented

To answer your first question, the response is no.

Actually I'm trying to create a spring support for an resource pipeline framework that needs to implement ResourceResolver and ResourceTransformer. The main problem I'm facing is that the PathResourceResolver is too encapsulated to let the spring user that I am to delegate localization to the underlying library. I'm looking for a way to customize this kind of resolver added by default by spring. Maybe there is another direction we should take to solve this issue...

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 27, 2014

Rossen Stoyanchev commented

I'm trying to create a spring support for an resource pipeline framework

OK that gives me a better idea. The route of extending ResourceHandlerRegistration requires extending WebMvcConfigurationSupport and that's not ideal to impose as a requirement on applications -- what if they want to extend it or another framework wants to do the same? A less intrusive approach is to simply let applications register resolvers and transformers you provide as usual. You could even provide some wrapper class (a registrar or builder) to help with those registrations.

The main obstacle I see is that getResourceResolvers registers PathResourceResolver unconditionally (I think that's what you're pointing out). We could make that conditional (like we do with the caching resolver) basically checking if the last resolver is a PathResourceResolver or a sub-class of it. We can also make PathResourceResolver more friendly to extension (what exactly do you need there with regards to localization?)

Does that make sense? I can have those changes pretty soon for you to try.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 27, 2014

Rossen Stoyanchev commented

Alright, I've made some changes (see this commit) that will let you register your own PathResourceResolver sub-class and I've also made it more extensible.

This should solve the problem you had with registering your own PathResourceResolver and also lets you register the complete list of resolvers and transformers. It would be great if you could try this and confirm it works for your purposes. Thanks!

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 27, 2014

Rossen Stoyanchev commented

Modified title (was "Create a factory for ResourceHandlerRegistration").

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 27, 2014

Rossen Stoyanchev commented

Resolving for now but you can still comment or re-open if necessary.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 27, 2014

Guillaume DROUET commented

Hi,

thanks for replying. You post your commit while I was writing a comment! Here's the state of my mind before starting to play with your modifications. Also thank you for quick response and your contribution.

In fact the user needs to register any resolver and transformers himself so that's the only way to mix many frameworks inside spring, and that's I'm trying to do. As you mentioned, PathResourceResolver is always added.

You can check if the last element IS-A PathResourceResolver, but indeed it needs to be easily extended. Another approach would be to consider a family for each ResourceResolver. Elements inside the chain of responsibility are ordered by family (cache -> custom -> versionning -> localization). With an enumeration of families, the user can inject any ResourceResolver (which is in PATH_RESOLVER family) without the requirement to create a sub-class of PathResourceResolver.

But let's focus on what I need to do regarding localization. I don't really have a problem with resolveResource() calls. Actually, I can add a custom resolver that uses the underlying framework to create the Resource and return it, ignoring the end of the chain. The problem appears when I resolve a public URL with resolveUrlPath. Because the custom resolver returns a Resource with a particular name that the PathResourceResolver does not know it, he fails to return a non-null value. Why I need to go to the end of the chain in that case? Because I want to delegate to spring the version strategy computation. However, VersionStrategy is just before ResourcePathResolver and I don't have the possibility to return a Resource resolved by another framework. Even if I can add a custom ResourcePathResolver, there is another point: the hash is added to the URL at a different position expected by the underlying framework so it can fail to resolve the public URL. At this stage, I'm not sure I can't deal with it if I can add a custom PathResourceResolver. Let me try to do it first with your changes and I will back with new feedbacks.

Thanks!

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 27, 2014

Rossen Stoyanchev commented

Sounds right to me. Your custom ResourceResolver can extend PathResourceResolver and override the new method getResource(String resourcePath, Resource location), Effectively replaces the default PathResourceResolver while still letting the VersionResourceResolver do its job. Give that a try.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 28, 2014

Guillaume DROUET commented

Hi,

ok it works perfectly now. I will share you the sample once commited on github.

Thanks!

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 28, 2014

Rossen Stoyanchev commented

Okay, good to hear. BTW we are still in the making some tweaks to the config API but the principle remains the same so it shouldn't be too hard for you to adapt. Stay tuned.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 28, 2014

Guillaume DROUET commented

Just for information, tweaks are planned until the GA avalaibility or later?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 28, 2014

Rossen Stoyanchev commented

The short answer is those are last minute tweaks before GA. The long answer is generally we don't change public APIs once released. An important exception can be a minor release just after GA, in case something important gets reported with the justification that the cost of change is still low compared to the cost of keeping the issue forever. Once applications start depending on a public API the cost goes up.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 28, 2014

Guillaume DROUET commented

Ok thanks.

Here is my spring support: https://github.com/wuic/wuic-extensions/tree/wuic-0.5.x-snapshot/spring
And a sample is available here: https://github.com/wuic/wuic-samples/tree/wuic-0.5.x-snapshot/spring-sample

Web UI Compressor is an open source project for JVM which helps developers to manage, optimize, package and deploy automatically their web resources. The core module provides several features (server/client caching, servlet, filter, resource aggregation, ...) and automagically discovers extensions like yuicompressor or ehcache in classpath. There is also template helpers for thymeleaf or JSP.

Regarding spring support: internally, it's designed as a chain of responsibility which skips caching and GZIP because this is already done by the spring framework. Then the user can delegate to WUIC resource management with a particular PathResourceResolver, version computation (based on CRC32) and applies additional stuffs like minification.

If you can take a look and give me your feedback I'll appreciate ;)

Thanks!

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 28, 2014

Rossen Stoyanchev commented

Alright I'll have a look, thanks!

Just a quick note that I've committed some changes to the Java config (see descriptions of 0b0255 and ae48b5). You'll need to adapt to them but it should be fairly easy to do so. Mainly cosmetic API changes, nothing has changed conceptually so I don't expect further issues once you fix compilation issues.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 29, 2014

Guillaume DROUET commented

Ok thanks for notification, I've updated the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web type: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants