HttpSecurityLambdaDsl and ServerHttpSecurityLambdaDsl are overly …

…eager when running against very old Spring Security versions (#402)

* Add precondition to guard recipe execution to only applicable Java source files

* Only perform replacement when desired target method exists

build.gradle.kts

@@ -150,6 +150,9 @@ dependencies {
     "testWithSpringBoot_2_1RuntimeOnly"("org.springframework.boot:spring-boot-actuator:2.1.0.RELEASE")
     "testWithSpringBoot_2_1RuntimeOnly"("org.springframework.data:spring-data-jpa:2.1.0.RELEASE")
     "testWithSpringBoot_2_1RuntimeOnly"("javax.persistence:javax.persistence-api:2.2")
+    "testWithSpringBoot_2_1RuntimeOnly"("org.springframework.security:spring-security-core:5.1.+")
+    "testWithSpringBoot_2_1RuntimeOnly"("org.springframework.security:spring-security-config:5.1.+")
+    "testWithSpringBoot_2_1RuntimeOnly"("org.springframework.security:spring-security-web:5.1.+")
 
     "testWithSpringBoot_2_2RuntimeOnly"("org.springframework.boot:spring-boot:2.2.+")
 

src/main/java/org/openrewrite/java/spring/boot2/ConvertToSecurityDslVisitor.java

@@ -16,20 +16,15 @@
 package org.openrewrite.java.spring.boot2;
 
 import org.openrewrite.Cursor;
-import org.openrewrite.SourceFile;
 import org.openrewrite.Tree;
 import org.openrewrite.internal.StringUtils;
 import org.openrewrite.java.JavaIsoVisitor;
 import org.openrewrite.java.MethodMatcher;
-import org.openrewrite.java.search.UsesType;
 import org.openrewrite.java.tree.*;
 import org.openrewrite.marker.Markers;
 import org.openrewrite.marker.Markup;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
+import java.util.*;
 
 import static java.util.Collections.emptyList;
 import static java.util.Objects.requireNonNull;
@@ -49,20 +44,18 @@ public ConvertToSecurityDslVisitor(String securityFqn, Collection<String> conver
         this.convertableMethods = convertableMethods;
     }
 
-    @Override
-    public boolean isAcceptable(SourceFile sourceFile, P p) {
-        return new UsesType<>(securityFqn, true).isAcceptable(sourceFile, p);
-    }
-
     @Override
     public J.MethodInvocation visitMethodInvocation(J.MethodInvocation initialMethod, P executionContext) {
         J.MethodInvocation method = super.visitMethodInvocation(initialMethod, executionContext);
         if (isApplicableMethod(method)) {
+            Optional<JavaType.Method> replacementMethod = findDesiredReplacement(method);
+            if (!replacementMethod.isPresent()) {
+                return method;
+            }
+
             final List<J.MethodInvocation> chain = computeAndMarkChain(method);
-            JavaType.FullyQualified httpSecurityType = method.getMethodType().getDeclaringType();
-            final String methodName = method.getSimpleName();
             final J.MethodInvocation m = method;
-            method = httpSecurityType.getMethods().stream().filter(aMethod -> methodName.equals(aMethod.getName()) && aMethod.getParameterTypes().size() == 1).findFirst().map(newMethodType -> {
+            method = replacementMethod.map(newMethodType -> {
                 String paramName = generateParamNameFromMethodName(m.getSimpleName());
                 return m
                         .withMethodType(newMethodType)
@@ -134,6 +127,20 @@ private boolean isApplicableMethod(J.MethodInvocation m) {
         return false;
     }
 
+    private Optional<JavaType.Method> findDesiredReplacement(J.MethodInvocation m) {
+        JavaType.Method methodType = m.getMethodType();
+        if (methodType == null) {
+            return Optional.empty();
+        }
+        JavaType.FullyQualified httpSecurityType = methodType.getDeclaringType();
+        return httpSecurityType.getMethods().stream()
+                .filter(availableMethod -> availableMethod.getName().equals(m.getSimpleName()) &&
+                        availableMethod.getParameterTypes().size() == 1 &&
+                        availableMethod.getParameterTypes().get(0) instanceof JavaType.FullyQualified &&
+                        "org.springframework.security.config.Customizer".equals(((JavaType.FullyQualified) availableMethod.getParameterTypes().get(0)).getFullyQualifiedName()))
+                .findFirst();
+    }
+
     public boolean isApplicableTopLevelMethodInvocation(J.MethodInvocation m) {
         if (isApplicableMethod(m)) {
             return true;

src/main/java/org/openrewrite/java/spring/boot2/HttpSecurityLambdaDsl.java

@@ -16,8 +16,10 @@
 package org.openrewrite.java.spring.boot2;
 
 import org.openrewrite.ExecutionContext;
+import org.openrewrite.Preconditions;
 import org.openrewrite.Recipe;
 import org.openrewrite.TreeVisitor;
+import org.openrewrite.java.search.UsesType;
 
 import java.util.Arrays;
 import java.util.Collection;
@@ -44,7 +46,10 @@ public String getDescription() {
 
     @Override
     public TreeVisitor<?, ExecutionContext> getVisitor() {
-        return new ConvertToSecurityDslVisitor<>(FQN_HTTP_SECURITY, APPLICABLE_METHOD_NAMES);
+        return Preconditions.check(
+                new UsesType<>(FQN_HTTP_SECURITY, true),
+                new ConvertToSecurityDslVisitor<>(FQN_HTTP_SECURITY, APPLICABLE_METHOD_NAMES)
+        );
     }
 
 }

src/main/java/org/openrewrite/java/spring/boot2/ServerHttpSecurityLambdaDsl.java

@@ -16,8 +16,10 @@
 package org.openrewrite.java.spring.boot2;
 
 import org.openrewrite.ExecutionContext;
+import org.openrewrite.Preconditions;
 import org.openrewrite.Recipe;
 import org.openrewrite.TreeVisitor;
+import org.openrewrite.java.search.UsesType;
 
 import java.util.Arrays;
 import java.util.Collection;
@@ -43,7 +45,10 @@ public String getDescription() {
 
     @Override
     public TreeVisitor<?, ExecutionContext> getVisitor() {
-        return new ConvertToSecurityDslVisitor<>(FQN_SERVER_HTTP_SECURITY, APPLICABLE_METHOD_NAMES);
+        return Preconditions.check(
+                new UsesType<>(FQN_SERVER_HTTP_SECURITY, true),
+                new ConvertToSecurityDslVisitor<>(FQN_SERVER_HTTP_SECURITY, APPLICABLE_METHOD_NAMES)
+        );
     }
 
 }

src/testWithSpringBoot_2_1/java/org/openrewrite/java/spring/boot2/LegacyHttpSecurityTest.java

@@ -0,0 +1,57 @@
+/*
+ * Copyright 2023 the original author or authors.
+ * <p>
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * https://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.openrewrite.java.spring.boot2;
+
+import org.junit.jupiter.api.Test;
+import org.openrewrite.java.JavaParser;
+import org.openrewrite.test.RecipeSpec;
+import org.openrewrite.test.RewriteTest;
+
+import static org.openrewrite.java.Assertions.java;
+
+class LegacyHttpSecurityTest implements RewriteTest {
+    @Override
+    public void defaults(RecipeSpec spec) {
+        spec.recipe(new HttpSecurityLambdaDsl())
+          .parser(JavaParser.fromJavaVersion()
+            .classpath("spring-beans", "spring-context", "spring-boot", "spring-security", "spring-web", "spring-core"));
+    }
+
+    @Test
+    void dontUseUnavailableMethods() {
+        //language=java
+        rewriteRun(
+          java(
+            """
+              import org.springframework.security.config.annotation.web.builders.HttpSecurity;
+              import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
+              import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
+              
+              @EnableWebSecurity
+              public class ConventionalSecurityConfig extends WebSecurityConfigurerAdapter {
+                  @Override
+                  protected void configure(HttpSecurity http) throws Exception {
+                      http
+                          .authorizeRequests()
+                              .antMatchers("/blog/**").permitAll()
+                              .anyRequest().authenticated();
+                  }
+              }
+              """
+          )
+        );
+    }
+}