Skip to content

Commit

Permalink
Refine ModelAttributeMethodProcessor Kotlin exception handling
Browse files Browse the repository at this point in the history
This commit refines ModelAttributeMethodProcessor Kotlin exception
handling in order to throw a proper MethodArgumentNotValidException
instead of a NullPointerException when Kotlin null-safety constraints
are not fulfilled, translating to an HTTP error with 400 status code
(Bad Request) instead of 500 (Internal Server Error).

Closes gh-23846
  • Loading branch information
sdeleuze committed Feb 15, 2023
1 parent 979118c commit 569df6e
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2023 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.
Expand All @@ -16,6 +16,7 @@

package org.springframework.web.bind;

import java.lang.reflect.Executable;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
Expand All @@ -42,13 +43,18 @@
*
* @author Rossen Stoyanchev
* @author Juergen Hoeller
* @author Sebastien Deleuze
* @since 3.1
*/
@SuppressWarnings("serial")
public class MethodArgumentNotValidException extends BindException implements ErrorResponse {

@Nullable
private final MethodParameter parameter;

@Nullable
private final Executable executable;

private final ProblemDetail body;


Expand All @@ -60,6 +66,19 @@ public class MethodArgumentNotValidException extends BindException implements Er
public MethodArgumentNotValidException(MethodParameter parameter, BindingResult bindingResult) {
super(bindingResult);
this.parameter = parameter;
this.executable = null;
this.body = ProblemDetail.forStatusAndDetail(getStatusCode(), "Invalid request content.");
}

/**
* Constructor for {@link MethodArgumentNotValidException}.
* @param executable the executable that failed validation
* @param bindingResult the results of the validation
*/
public MethodArgumentNotValidException(Executable executable, BindingResult bindingResult) {
super(bindingResult);
this.parameter = null;
this.executable = executable;
this.body = ProblemDetail.forStatusAndDetail(getStatusCode(), "Invalid request content.");
}

Expand All @@ -83,9 +102,16 @@ public final MethodParameter getParameter() {

@Override
public String getMessage() {
StringBuilder sb = new StringBuilder("Validation failed for argument [")
.append(this.parameter.getParameterIndex()).append("] in ")
.append(this.parameter.getExecutable().toGenericString());
StringBuilder sb = new StringBuilder("Validation failed ");
if (this.parameter != null) {
sb.append("for argument [")
.append(this.parameter.getParameterIndex()).append("] in ")
.append(this.parameter.getExecutable().toGenericString());
}
else {
sb.append("in ")
.append(this.executable.toGenericString());
}
BindingResult bindingResult = getBindingResult();
if (bindingResult.getErrorCount() > 1) {
sb.append(" with ").append(bindingResult.getErrorCount()).append(" errors");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2023 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.
Expand Down Expand Up @@ -36,6 +36,7 @@
import org.springframework.beans.BeanInstantiationException;
import org.springframework.beans.BeanUtils;
import org.springframework.beans.TypeMismatchException;
import org.springframework.core.KotlinDetector;
import org.springframework.core.MethodParameter;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
Expand All @@ -47,6 +48,7 @@
import org.springframework.validation.BindException;
import org.springframework.validation.BindingResult;
import org.springframework.validation.Errors;
import org.springframework.validation.ObjectError;
import org.springframework.validation.SmartValidator;
import org.springframework.validation.Validator;
import org.springframework.validation.annotation.ValidationAnnotationUtils;
Expand Down Expand Up @@ -329,7 +331,21 @@ public Object getTarget() {
throw new MethodArgumentNotValidException(parameter, result);
}

return BeanUtils.instantiateClass(ctor, args);
try {
return BeanUtils.instantiateClass(ctor, args);
}
catch (BeanInstantiationException ex) {
Throwable cause = ex.getCause();
if (KotlinDetector.isKotlinType(ctor.getDeclaringClass()) && cause instanceof NullPointerException) {
BindingResult result = binder.getBindingResult();
ObjectError error = new ObjectError(ctor.getName(), cause.getMessage());
result.addError(error);
throw new MethodArgumentNotValidException(ctor, result);
}
else {
throw ex;
}
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* Copyright 2002-2023 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.springframework.web.method.annotation

import org.assertj.core.api.Assertions
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.mockito.ArgumentMatchers.*
import org.mockito.BDDMockito
import org.mockito.Mockito
import org.springframework.core.MethodParameter
import org.springframework.core.annotation.SynthesizingMethodParameter
import org.springframework.web.bind.MethodArgumentNotValidException
import org.springframework.web.bind.support.WebDataBinderFactory
import org.springframework.web.bind.support.WebRequestDataBinder
import org.springframework.web.context.request.ServletWebRequest
import org.springframework.web.method.support.ModelAndViewContainer
import org.springframework.web.testfixture.servlet.MockHttpServletRequest
import kotlin.annotation.AnnotationTarget.*

/**
* Kotlin test fixture for [ModelAttributeMethodProcessor].
*
* @author Sebastien Deleuze
*/
class ModelAttributeMethodProcessorKotlinTests {

private lateinit var container: ModelAndViewContainer

private lateinit var processor: ModelAttributeMethodProcessor

private lateinit var param: MethodParameter


@BeforeEach
fun setup() {
container = ModelAndViewContainer()
processor = ModelAttributeMethodProcessor(false)
var method = ModelAttributeHandler::class.java.getDeclaredMethod("test",Param::class.java)
param = SynthesizingMethodParameter(method, 0)
}

@Test
fun resolveArgumentWithValue() {
val mockRequest = MockHttpServletRequest().apply { addParameter("a", "b") }
val requestWithParam = ServletWebRequest(mockRequest)
val factory = Mockito.mock<WebDataBinderFactory>()
BDDMockito.given(factory.createBinder(any(), any(), eq("param")))
.willAnswer { WebRequestDataBinder(it.getArgument(1)) }
Assertions.assertThat(processor.resolveArgument(this.param, container, requestWithParam, factory)).isEqualTo(Param("b"))
}

@Test
fun throwMethodArgumentNotValidExceptionWithNull() {
val mockRequest = MockHttpServletRequest().apply { addParameter("a", null) }
val requestWithParam = ServletWebRequest(mockRequest)
val factory = Mockito.mock<WebDataBinderFactory>()
BDDMockito.given(factory.createBinder(any(), any(), eq("param")))
.willAnswer { WebRequestDataBinder(it.getArgument(1)) }
Assertions.assertThatThrownBy {
processor.resolveArgument(this.param, container, requestWithParam, factory)
}.isInstanceOf(MethodArgumentNotValidException::class.java)
.hasMessageContaining("parameter a")
}

private data class Param(val a: String)

@Suppress("UNUSED_PARAMETER")
private class ModelAttributeHandler {
fun test(param: Param) { }
}

}

0 comments on commit 569df6e

Please sign in to comment.