[Spring 6] Add a recipe to switch from HttpStatus to HttpStatusCode for WebMVC customizations

[iuliiasobolevska]

What problem are you trying to solve?

When updating from org.springframework:spring-webmvc from 5.3.27 -> 6.0.9, we had to update method overrides for classes extending ResponseEntityExceptionHandler due to method signature changes from this commit. ResponseEntityExceptionHandler is just one example of a class that was changed in that commit.

What precondition(s) should be checked before applying this recipe?

For org.springframework:spring-webmvc v.6.0.0 and higher. Should be included in the Spring Boot 3 recipe.

Describe the situation before applying the recipe

import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.client.HttpStatusCodeException;
import org.springframework.web.context.request.WebRequest;
import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler;

import javax.servlet.http.HttpServletRequest;

@ControllerAdvice
public class GlobalExceptionHandler extends ResponseEntityExceptionHandler {
    private final HttpServletRequest servletRequest;

    public GlobalExceptionHandler(HttpServletRequest servletRequest) {
        this.servletRequest = servletRequest;
    }

    @Override
    protected ResponseEntity<Object> handleExceptionInternal(Exception ex, Object body, HttpHeaders headers, HttpStatus status, WebRequest request) {
        ErrorResponse errorResponse = new ErrorResponse(status, "An error occurred while trying to handle the incoming request.", servletRequest, ex);
        return new ResponseEntity<>(errorResponse, errorResponse.getHttpStatus());
    }

    @ExceptionHandler(HttpStatusCodeException.class)
    protected ResponseEntity<Object> handleHttpClientError(HttpStatusCodeException ex) {
        ErrorResponse errorResponse = new ErrorResponse(ex.getStatusCode(),
                "An error occurred while calling SomeService API.",
                ex.getResponseBodyAsString(),
                servletRequest);

        return new ResponseEntity<>(errorResponse, errorResponse.getHttpStatus());
    }
}

Describe the situation after applying the recipe

import jakarta.servlet.http.HttpServletRequest;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.HttpStatusCode;
import org.springframework.http.ResponseEntity;
import org.springframework.lang.Nullable;
import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.client.HttpStatusCodeException;
import org.springframework.web.context.request.WebRequest;
import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler;

@ControllerAdvice
public class GlobalExceptionHandler extends ResponseEntityExceptionHandler {
    private final HttpServletRequest servletRequest;

    public GlobalExceptionHandler(HttpServletRequest servletRequest) {
        this.servletRequest = servletRequest;
    }

    @Override
    protected ResponseEntity<Object> handleExceptionInternal(Exception ex, @Nullable Object body, HttpHeaders headers, HttpStatusCode statusCode, WebRequest request) {
        ErrorResponse errorResponse = new ErrorResponse(HttpStatus.resolve(statusCode.value()), "An error occurred while trying to handle the incoming request.", servletRequest, ex);
        return new ResponseEntity<>(errorResponse, errorResponse.getHttpStatus());
    }

    @ExceptionHandler(HttpStatusCodeException.class)
    protected ResponseEntity<Object> handleHttpClientError(HttpStatusCodeException ex) {
        ErrorResponse errorResponse = new ErrorResponse(HttpStatus.resolve(ex.getStatusCode().value()),
                "An error occurred while calling SomeService API.",
                ex.getResponseBodyAsString(),
                servletRequest);

        return new ResponseEntity<>(errorResponse, errorResponse.getHttpStatus());
    }
}

Are you interested in contributing this recipe to OpenRewrite?

Maybe, won't have bandwidth in the next couple of weeks so can't fully commit.

[timtebeek]

Hi @iuliiasobolevska ; welcome back and thank you for reporting this issue here! Indeed it's a change that we have not yet provided a recipe for; thanks for including the link the original commit for context. That commit links to this issue, which also gives some more background.

philwebb suggested that we can introduce a new interface HttpStatusCode, which is implemented by HttpStatus. Code that currently returns a HttpStatus will return this new HttpStatusCode instead, and we will deprecate the methods that return the raw status codes. All methods that accepts the raw integer values will still be available, we will only deprecate the integer returning methods.

If I understand you correctly any time you extend ResponseEntityExceptionHandler, or classes similarly affected, one has to update the signature of the overridden method to now accept a HttpStatusCode instead of an HttpStatus enum. And any usage of the statusCode might need to be updated where appropriate.

In your example, where is the ErrorResponse class coming from? Because I'm seeing one from Spring , but that's an interface with a builder that accepts a HttpStatusCode, rather than the constructor you're calling with a resolved HttpStatus enum. Just to be sure we're working towards the right solution here with the given before/after samples.

Understandable that you can't fully commit to a recipe implementation just yet; I'll add it to the backlog and anyone is then welcome to help out.

[iuliiasobolevska]

If I understand you correctly any time you extend ResponseEntityExceptionHandler, or classes similarly affected, one has to update the signature of the overridden method to now accept a HttpStatusCode instead of an HttpStatus enum. And any usage of the statusCode might need to be updated where appropriate.

yes, that's correct

In your example, where is the ErrorResponse class coming from?

oh my bad and a bad example, I forgot that it's an internal class:

@JsonInclude(NON_EMPTY)
public final class ErrorResponse {
    private final String timestamp;
    private final HttpStatus status;
    private final String message;
    private final String debugMessage;
    private final String path;
... // constructors, getters, setters
}

Anyways, it was just for illustration purposes to highlight that usages of statusCode need to be updated as well (as you pointed out).