Affects: 5.3.x
This is not an issue with spring framework specifically, but with a combination of spring-core, spring-context-indexer and tomcat-jakartaee-migration.
We're using spring-context-indexer and are trying to migrate to Jakarta EE using the tomcat tool. The problem is that tomcat-jakartaee-migration rewrites all references to specific javax classes to use jakarta, which includes javax.annotation.ManagedBean and javax.inject.Named. We never use those annotations in the codebase to define / inject spring beans, but because those also get rewritten it effectively makes indexer incompatible due to this condition:
private boolean indexSupportsIncludeFilter(TypeFilter filter) {
if (filter instanceof AnnotationTypeFilter) {
Class<? extends Annotation> annotation = ((AnnotationTypeFilter) filter).getAnnotationType();
return (AnnotationUtils.isAnnotationDeclaredLocally(Indexed.class, annotation) ||
annotation.getName().startsWith("javax."));
^^^^^^
}
if (filter instanceof AssignableTypeFilter) {
Class<?> target = ((AssignableTypeFilter) filter).getTargetType();
return AnnotationUtils.isAnnotationDeclaredLocally(Indexed.class, target);
}
return false;
}
where the rewriter doesn't catch this string literal.
it would be a weird ask, but is it possible to add a condition that explicitly checks for the default javax.annotation.ManagedBean and javax.inject.Named annotations along side with startsWith("javax.") or an additional startsWith("jakarta.")?
Comment From: snicoll
The context indexer is deprecated so we won't make any further change to it. It looks like you're asking to add jakarta in 5.3.x which doesn't really make sense since support for Jakarta has been added in 6.x onwards.
I also don't understand why you'd have to use the component indexer for your migration. This looks like a temporary step or are you trying to use the Jakarata namespace with Spring 5.3.x? That isn't supported.
Comment From: gavlyukovskiy
I also don't understand why you'd have to use the component indexer for your migration.
We're not using it for migration specifically. We've been using it for a while now together with Spring 5.3.
This looks like a temporary step or are you trying to use the Jakarata namespace with Spring 5.3.x? That isn't supported.
Yes, that's partly what we do - we have an application that's deployed to Tomcat 9 and uses Spring 5.3. We want to upgrade to Tomcat 10 first without also upgrading every library to Jakarta version. The Tomcat 10 has a migration tool (tomcat-jakartaee-migration) or the legacy javax war can be deployed to webapps-javaee which runs this tool internally.
This migration tool rewrites most references to javax to be jakarta in the compiled jar(s), which also includes spring-core. So effectively we're having Spring 5.3 with all references to javax.annotation.ManagedBean replaced to jakarta.annotation.ManagedBean and so on, but this migration tool doesn't rewrite "javax." to "jakarta." because it's not fully qualified name and some javax annotations aren't part of Jakarta EE (e.g. jsr305 annotations).
So the problem is that when we run the spring-core 5.3 it has default 2 filters
this.includeFilters.add(new AnnotationTypeFilter(
((Class<? extends Annotation>) ClassUtils.forName("javax.annotation.ManagedBean", cl))
this.includeFilters.add(new AnnotationTypeFilter(
((Class<? extends Annotation>) ClassUtils.forName("javax.inject.Named", cl)), false));
which are rewritten to jakarta.annotation.ManagedBean and jakarta.inject.Named. Because of that the condition
(AnnotationUtils.isAnnotationDeclaredLocally(Indexed.class, annotation) ||
annotation.getName().startsWith("javax."));
no longer returns true and indexer file is completely ignored. If that condition was
(AnnotationUtils.isAnnotationDeclaredLocally(Indexed.class, annotation) ||
annotation.getName().equals("javax.annotation.ManagedBean")) ||
annotation.getName().equals("javax.inject.Named")) ||
annotation.getName().startsWith("javax."));
then the tool would've handled it correctly. I thought it would make sense to add this additional condition that would be backward compatible and that will not introduce any issues for existing users.
I understand that it's a ask for a deprecated component (we're trying to migrate off it as well), but because those are the default filters makes it more difficult for us to migrate to Jakarta and Spring 6 using the Tomcat 10 migration path.
Comment From: bclozel
Thanks for the feedback @gavlyukovskiy
While we appreciate Tomcat's efforts to bring apps on board with the jakarta namespace, we have never run our 5.3.x test suite against this use case (bytecode transformation with tomcat-jakartaee-migration). As a result, it's hard for us to offer any guarantee about this runtime profile. Spring performs reflection in various places, so I'm not 100% sure this would work as expected.
As far as I understand, Tomcat 9.x and 10.0.x are very much the same, besides the namespace change. The Tomcat team maintained both branches in sync. As a result, testing your existing application with Tomcat 10.0.x and this namespace change is... not validating much from my perspective. Unless many applications are running on the same Tomcat and you're preparing a migration in steps?
Besides the deprecation notice that @snicoll already shared, we are very late in the cycle here. We could have considered such a change earlier, but at this time we must face the timeline: Spring Framework 5.3.x is about to be EOL for its OSS support and our last OSS release is scheduled for next month. We would rather not introduce brittle changes in that last release, as we don't want to risk any regression there. So far we have scheduled one regression fix and one bug fix - the rest are tasks and documentation fixes.
It's far from ideal, but since you're willing to modify Spring's bytecode on-the-fly, maybe overriding this class (which is only used at compile time) for this purpose?
I'm declining this enhancement as we won't do this change for the next and final 5.3.x release. I hope you'll understand our position - best of luck with your upgrades!
Comment From: gavlyukovskiy
As a result, testing your existing application with Tomcat 10.0.x and this namespace change is... not validating much from my perspective.
The problem is that we manage the standalone Tomcat 10 installation separately from the code, so we're not trying to test that code itself works, but be able to have a gradual rollout to Tomcat 9 -> Tomcat 10 without modifying the code that's still compiled against Tomcat 9.
Unless many applications are running on the same Tomcat and you're preparing a migration in steps?
That's what we have in our setup 🙃 So we wanted to keep Tomcat 10 upgrade separately from code changes and also be able to upgrade libraries like Jersey and Hibernate Validator separately from Spring 6 update.
It's far from ideal, but since you're willing to modify Spring's bytecode on-the-fly, maybe overriding this class (which is only used at compile time) for this purpose?
I think we might need to do that. Thank you for the suggestion. One other thing I've considered is overriding every @ComponentScan to not use default filters and only include Component.class.
We would rather not introduce brittle changes in that last release, as we don't want to risk any regression there.
The change I proposed cannot break the backward compatibility - for all existing users that would be just unnecessary if check that is guaranteed to return the same result as startsWith (it can also be after startsWith, but it's a couple of nanoseconds 🤷).
But I understand why you don't want to introduce such a change, thank you for the suggestions on how we can solve that differently!