From 8609f0c51be8d726578d91325ccfcc4a738b94e6 Mon Sep 17 00:00:00 2001 From: Laurens Westerlaken Date: Thu, 19 Sep 2024 17:26:23 +0200 Subject: [PATCH] Update `HttpStatus` to `HttpStatusCode` in `ResponseEntityExceptionHandler` implementations (#589) * Initial testcase * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Fix botched imports after faulty code suggestion * WIP * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * WIP - might have to revert * Use MethodMatcher and check arguments; expand tests * Apply suggestions from code review * A little bit closer * Properly apply initializer * All cases covered, expect recipe now requires 2 cycles * Fix some minor issues * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Review feedback * Polish `visitMethodDeclaration` * Drop unnecessary `visitReturn` * Drop now unused import * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Move recipe to framework and include it in framework 6 migration * Limit test classpath to spring-web and spring-webmvc --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Tim te Beek --- build.gradle.kts | 1 + ...tionHandlerHttpStatusToHttpStatusCode.java | 116 ++++++++++ .../META-INF/rewrite/spring-framework-60.yml | 1 + ...HandlerHttpStatusToHttpStatusCodeTest.java | 200 ++++++++++++++++++ 4 files changed, 318 insertions(+) create mode 100644 src/main/java/org/openrewrite/java/spring/framework/MigrateResponseEntityExceptionHandlerHttpStatusToHttpStatusCode.java create mode 100644 src/testWithSpringBoot_3_0/java/org/openrewrite/java/spring/framework/MigrateResponseEntityExceptionHandlerHttpStatusToHttpStatusCodeTest.java diff --git a/build.gradle.kts b/build.gradle.kts index 0fbbc9c24..d99348007 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -240,6 +240,7 @@ dependencies { "testWithSpringBoot_3_0RuntimeOnly"("org.springframework.boot:spring-boot-starter-test:3.0.+") "testWithSpringBoot_3_0RuntimeOnly"("org.springframework:spring-context:6.0.+") "testWithSpringBoot_3_0RuntimeOnly"("org.springframework:spring-web:6.0.+") + "testWithSpringBoot_3_0RuntimeOnly"("org.springframework:spring-webmvc:6.0.+") "testWithSpringBoot_3_0RuntimeOnly"("org.springframework.batch:spring-batch-core:4.+") "testWithSpringBoot_3_0RuntimeOnly"("org.springframework.batch:spring-batch-core:5.+") "testWithSpringBoot_3_0RuntimeOnly"("org.springframework.batch:spring-batch-infrastructure:4.+") diff --git a/src/main/java/org/openrewrite/java/spring/framework/MigrateResponseEntityExceptionHandlerHttpStatusToHttpStatusCode.java b/src/main/java/org/openrewrite/java/spring/framework/MigrateResponseEntityExceptionHandlerHttpStatusToHttpStatusCode.java new file mode 100644 index 000000000..07355736c --- /dev/null +++ b/src/main/java/org/openrewrite/java/spring/framework/MigrateResponseEntityExceptionHandlerHttpStatusToHttpStatusCode.java @@ -0,0 +1,116 @@ +/* + * Copyright 2024 the original author or authors. + *

+ * 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 + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * 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.framework; + +import org.openrewrite.ExecutionContext; +import org.openrewrite.Preconditions; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.internal.ListUtils; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.search.UsesType; +import org.openrewrite.java.tree.*; + +import static java.util.Collections.singletonList; + +public class MigrateResponseEntityExceptionHandlerHttpStatusToHttpStatusCode extends Recipe { + + private static final String HTTP_STATUS_FQ = "org.springframework.http.HttpStatus"; + private static final String HTTP_STATUS_CODE_FQ = "org.springframework.http.HttpStatusCode"; + private static final String RESPONSE_ENTITY_EXCEPTION_HANDLER_FQ = "org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler"; + private static final MethodMatcher HANDLER_METHOD = new MethodMatcher( + "org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler *(..)", true); + + @Override + public String getDisplayName() { + return "Migrate `ResponseEntityExceptionHandler` from HttpStatus to HttpStatusCode"; + } + + @Override + public String getDescription() { + return "With Spring 6 `HttpStatus` was replaced by `HttpStatusCode` in most method signatures in the `ResponseEntityExceptionHandler`."; + } + + @Override + public TreeVisitor getVisitor() { + return Preconditions.check( + new UsesType<>(RESPONSE_ENTITY_EXCEPTION_HANDLER_FQ, true), + new JavaIsoVisitor() { + + @Override + public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) { + J.MethodDeclaration m = method; + if (HANDLER_METHOD.matches(m.getMethodType())) { + JavaType javaTypeHttpStatusCode = JavaType.buildType(HTTP_STATUS_CODE_FQ); + //noinspection DataFlowIssue + JavaType.Method met = m.getMethodType().withParameterTypes(ListUtils.map(m.getMethodType().getParameterTypes(), + type -> TypeUtils.isAssignableTo(HTTP_STATUS_FQ, type) ? javaTypeHttpStatusCode : type)); + if (met == m.getMethodType()) { + // There was no parameter to change + return m; + } + + m = m.withMethodType(met); + m = m.withParameters(ListUtils.map(m.getParameters(), var -> { + if (var instanceof J.VariableDeclarations) { + J.VariableDeclarations v = (J.VariableDeclarations) var; + J.VariableDeclarations.NamedVariable declaredVar = v.getVariables().get(0); + if (declaredVar.getVariableType() != null && TypeUtils.isAssignableTo(HTTP_STATUS_FQ, v.getType())) { + J.Identifier newName = declaredVar.getName().withType(javaTypeHttpStatusCode); + if (newName.getFieldType() != null) { + newName = newName.withFieldType(newName.getFieldType().withType(javaTypeHttpStatusCode)); + } + declaredVar = declaredVar + .withName(newName) + .withVariableType(declaredVar.getVariableType().withOwner(met)); + return v.withVariables(singletonList(declaredVar.withType(javaTypeHttpStatusCode))) + .withTypeExpression(TypeTree.build("HttpStatusCode").withType(javaTypeHttpStatusCode)); + } + } + return var; + })); + } + updateCursor(m); + maybeAddImport(HTTP_STATUS_CODE_FQ); + maybeRemoveImport(HTTP_STATUS_FQ); + return super.visitMethodDeclaration(m, ctx); + } + + @Override + public J.Identifier visitIdentifier(J.Identifier identifier, ExecutionContext ctx) { + J.Identifier ident = super.visitIdentifier(identifier, ctx); + J.MethodDeclaration methodScope = getCursor().firstEnclosing(J.MethodDeclaration.class); + if (methodScope != null) { + for (Statement stmt : methodScope.getParameters()) { + if (stmt instanceof J.VariableDeclarations) { + J.VariableDeclarations vd = (J.VariableDeclarations) stmt; + for (J.VariableDeclarations.NamedVariable var : vd.getVariables()) { + if (var.getName().getSimpleName().equals(ident.getSimpleName())) { + if (!TypeUtils.isOfType(var.getName().getType(), ident.getType())) { + ident = ident.withType(var.getName().getType()); + } + } + } + } + } + } + return super.visitIdentifier(ident, ctx); + } + } + ); + } +} diff --git a/src/main/resources/META-INF/rewrite/spring-framework-60.yml b/src/main/resources/META-INF/rewrite/spring-framework-60.yml index aae00fcf2..cded8b6bf 100644 --- a/src/main/resources/META-INF/rewrite/spring-framework-60.yml +++ b/src/main/resources/META-INF/rewrite/spring-framework-60.yml @@ -30,6 +30,7 @@ recipeList: - org.openrewrite.java.spring.framework.MigrateSpringAssert - org.openrewrite.apache.httpclient5.UpgradeApacheHttpClient_5 - org.openrewrite.java.spring.framework.HttpComponentsClientHttpRequestFactoryReadTimeout + - org.openrewrite.java.spring.framework.MigrateResponseEntityExceptionHandlerHttpStatusToHttpStatusCode --- type: specs.openrewrite.org/v1beta/recipe diff --git a/src/testWithSpringBoot_3_0/java/org/openrewrite/java/spring/framework/MigrateResponseEntityExceptionHandlerHttpStatusToHttpStatusCodeTest.java b/src/testWithSpringBoot_3_0/java/org/openrewrite/java/spring/framework/MigrateResponseEntityExceptionHandlerHttpStatusToHttpStatusCodeTest.java new file mode 100644 index 000000000..f810679af --- /dev/null +++ b/src/testWithSpringBoot_3_0/java/org/openrewrite/java/spring/framework/MigrateResponseEntityExceptionHandlerHttpStatusToHttpStatusCodeTest.java @@ -0,0 +1,200 @@ +/* + * Copyright 2024 the original author or authors. + *

+ * 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 + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * 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.framework; + +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.java.JavaParser; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +class MigrateResponseEntityExceptionHandlerHttpStatusToHttpStatusCodeTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new MigrateResponseEntityExceptionHandlerHttpStatusToHttpStatusCode()) + .parser(JavaParser.fromJavaVersion().classpath("spring-web", "spring-webmvc")); + } + + @DocumentExample + @Test + void migrateHttpStatustoHttpStatusCode() { + //language=java + rewriteRun( + java( + """ + import org.springframework.http.HttpHeaders; + import org.springframework.http.HttpStatus; + import org.springframework.http.ResponseEntity; + import org.springframework.web.context.request.WebRequest; + import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; + + class GlobalExceptionHandler extends ResponseEntityExceptionHandler { + + @Override + protected ResponseEntity handleExceptionInternal(Exception ex, Object body, HttpHeaders headers, HttpStatus status, WebRequest request) { + // Imagine we log or manipulate the status here somehow + return super.handleExceptionInternal(ex, body, headers, status, request); + } + } + """, + """ + import org.springframework.http.HttpHeaders; + import org.springframework.http.HttpStatusCode; + import org.springframework.http.ResponseEntity; + import org.springframework.web.context.request.WebRequest; + import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; + + class GlobalExceptionHandler extends ResponseEntityExceptionHandler { + + @Override + protected ResponseEntity handleExceptionInternal(Exception ex, Object body, HttpHeaders headers, HttpStatusCode status, WebRequest request) { + // Imagine we log or manipulate the status here somehow + return super.handleExceptionInternal(ex, body, headers, status, request); + } + } + """ + ) + ); + } + + @Test + void changeTypeOfValueCallAndRemoveImport() { + //language=java + rewriteRun( + java( + """ + import org.springframework.http.HttpHeaders; + import org.springframework.http.HttpStatus; + import org.springframework.http.ResponseEntity; + import org.springframework.web.context.request.WebRequest; + import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; + + class GlobalExceptionHandler extends ResponseEntityExceptionHandler { + + @Override + protected ResponseEntity handleExceptionInternal(Exception ex, Object body, HttpHeaders headers, HttpStatus status, WebRequest request) { + int value = status.value(); + return super.handleExceptionInternal(ex, body, headers, status, request); + } + } + """, + """ + import org.springframework.http.HttpHeaders; + import org.springframework.http.HttpStatusCode; + import org.springframework.http.ResponseEntity; + import org.springframework.web.context.request.WebRequest; + import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; + + class GlobalExceptionHandler extends ResponseEntityExceptionHandler { + + @Override + protected ResponseEntity handleExceptionInternal(Exception ex, Object body, HttpHeaders headers, HttpStatusCode status, WebRequest request) { + int value = status.value(); + return super.handleExceptionInternal(ex, body, headers, status, request); + } + } + """ + ) + ); + } + + @Test + void shouldNotChangeEnumUsageAndImport() { + //language=java + rewriteRun( + java( + """ + import org.springframework.http.HttpHeaders; + import org.springframework.http.HttpStatus; + import org.springframework.http.ResponseEntity; + import org.springframework.web.context.request.WebRequest; + import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; + + class GlobalExceptionHandler extends ResponseEntityExceptionHandler { + + @Override + protected ResponseEntity handleExceptionInternal(Exception ex, Object body, HttpHeaders headers, HttpStatus status, WebRequest request) { + HttpStatus enumValue = HttpStatus.INTERNAL_SERVER_ERROR; + return super.handleExceptionInternal(ex, body, headers, enumValue, request); + } + } + """, + """ + import org.springframework.http.HttpHeaders; + import org.springframework.http.HttpStatus; + import org.springframework.http.HttpStatusCode; + import org.springframework.http.ResponseEntity; + import org.springframework.web.context.request.WebRequest; + import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; + + class GlobalExceptionHandler extends ResponseEntityExceptionHandler { + + @Override + protected ResponseEntity handleExceptionInternal(Exception ex, Object body, HttpHeaders headers, HttpStatusCode status, WebRequest request) { + HttpStatus enumValue = HttpStatus.INTERNAL_SERVER_ERROR; + return super.handleExceptionInternal(ex, body, headers, enumValue, request); + } + } + """ + ) + ); + } + + @Test + void noSuperCallChangeMethodSignature() { + //language=java + rewriteRun( + java( + """ + import org.springframework.http.HttpHeaders; + import org.springframework.http.HttpStatus; + import org.springframework.http.ResponseEntity; + import org.springframework.web.context.request.WebRequest; + import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; + + class GlobalExceptionHandler extends ResponseEntityExceptionHandler { + + @Override + protected ResponseEntity handleExceptionInternal( + Exception ex, Object body, HttpHeaders headers, HttpStatus status, WebRequest request) { + return ResponseEntity.ok().build(); + } + } + """, + """ + import org.springframework.http.HttpHeaders; + import org.springframework.http.HttpStatusCode; + import org.springframework.http.ResponseEntity; + import org.springframework.web.context.request.WebRequest; + import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; + + class GlobalExceptionHandler extends ResponseEntityExceptionHandler { + + @Override + protected ResponseEntity handleExceptionInternal( + Exception ex, Object body, HttpHeaders headers, HttpStatusCode status, WebRequest request) { + return ResponseEntity.ok().build(); + } + } + """ + ) + ); + } + +}