Load spring-security-web dependency from resources

[knutwannheden]

No description provided.

[knutwannheden]

@timtebeek The nightly org.openrewrite.java.spring.boot3.UpgradeSpringBoot_3_2 flagship recipe run against spring-projects/spring-webflow-samples keeps failing with an error:

java.lang.IllegalStateException: Unable to construct Java17Parser.
  org.openrewrite.java.Java17Parser$Builder.build(Java17Parser.java:96)
  org.openrewrite.java.Java17Parser$Builder.build(Java17Parser.java:63)
  org.openrewrite.java.internal.template.JavaTemplateParser.compileTemplate(JavaTemplateParser.java:257)
  org.openrewrite.java.internal.template.JavaTemplateParser.lambda$parseBlockStatements$9(JavaTemplateParser.java:176)
  org.openrewrite.java.internal.template.JavaTemplateParser.lambda$cacheIfContextFree$14(JavaTemplateParser.java:287)
  org.openrewrite.java.internal.template.JavaTemplateParser.cache(JavaTemplateParser.java:308)
  org.openrewrite.java.internal.template.JavaTemplateParser.cacheIfContextFree(JavaTemplateParser.java:287)
  org.openrewrite.java.internal.template.JavaTemplateParser.parseBlockStatements(JavaTemplateParser.java:171)
  org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1.maybeReplaceStatement(JavaTemplateJavaExtension.java:469)
  org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1.visitNewClass(JavaTemplateJavaExtension.java:448)
  org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1.visitNewClass(JavaTemplateJavaExtension.java:56)
  org.openrewrite.java.tree.J$NewClass.acceptJava(J.java:4449)
  org.openrewrite.java.tree.J.accept(J.java:59)
  org.openrewrite.TreeVisitor.visit(TreeVisitor.java:250)
  org.openrewrite.TreeVisitor.visit(TreeVisitor.java:151)
  org.openrewrite.java.JavaTemplate.apply(JavaTemplate.java:101)
  ...

I traced it back to the UpdateRequestCache recipe as updated in this PR. But there are a lot of other recipes which also load Spring dependencies from the classpath rather than from resources. While I think this PR would fix the issue we are seeing in the SaaS, I suspect there could be many more like it and I am unsure why we don't see this more often.

I think we should investigate which Spring dependencies we have on the classpath and which ones we need to load from resources and then adjust the recipes accordingly.

To investigate this I reproduced it with the CLI and attached with the debugger (with breakpoint on IllegalStateException).

[timtebeek]

Thanks for the tough work of debugging and pinpointing the issue; do I understand correctly you'd like me to take over to apply this pattern at scale across this repository to switch from dependencies on the classpath to dependencies taken from resources?

[knutwannheden]

Thanks for the tough work of debugging and pinpointing the issue; do I understand correctly you'd like me to take over to apply this pattern at scale across this repository to switch from dependencies on the classpath to dependencies taken from resources?

@timtebeek That wasn't really my intention, but I would appreciate if you could do that. I just noticed in today's flagship recipe repo run that the error was gone, which is how I remembered this PR. By the looks of it you've already addressed the issue. Thanks for that!

Possibly we can give some more thought to how we can avoid this type of issue in the future. The problem, as far as I can tell, is that we in recipe repos sometimes have testImplementation dependencies, which the JavaTemplates require and then cause the recipes to fail in production, because the dependency is unavailable then. I don't know if we can enforce this somehow, other than by discipline.

[timtebeek]

Good to hear the error's gone! And no problem picking this up; I wanted to see this fixed too. :)

As for how to prevent this in the future; hard to say what's best; we could build another check into the test framework we have, or add a recipe to org.openrewrite.recipes.OpenRewriteBestPractices that flags this on PRs.

[nmck257]

Would it be too bold to simply remove the classpath option in favor of classpathFromResources?

In recipe code, the set of dependencies which would both be on the production classpath and also would likely be used in a recipe seems quite small, and this couples the version of the jar used by the recipe execution to the version of the jar which injects type metadata, which could drift and create subtle issues over time.

In test code, this mechanism is useful (especially in rewrite-spring) -- maybe we somehow enforce that the classpath option is only available in test code?

Such as:

JavaParser.fromJavaVersion().classpathSupplier(ClasspathSuppliers.fromResources("spring-web"))
// ClasspathSuppliers published in rewrite-java

JavaParser.fromJavaVersion().classpathSupplier(TestClasspathSuppliers.fromClasspath("spring-web"))
// TestClasspathSuppliers published in rewrite-test
// Though having something specific to java in rewrite-test might shift the current dependency model...
// maybe rewrite-java could be a compile-only dependency of rewrite-test?