DATACMNS-1157 - Enforce nullable/non-null API constraints on repository query methods.

We now validate query method invocations to check method parameters whether they accept null values and reject execution if null values are not supported. We derive nullability support from repository interfaces declared using Kotlin and Spring's NonNullApi/Nullable annotations.

We also check whether a query method can return null. If a query method returns null which isn't supposed to do so, then we throw EmptyResultDataAccessException to prevent null return values.

interface UserRepository extends Repository<User, String> {

  List<User> findByLastname(@Nullable String firstname);

  @Nullable
  User findByFirstnameAndLastname(String firstname, String lastname);
}

interface UserRepository : Repository<User, String> {

  fun findByLastname(username: String?): List<User>

  fun findByFirstnameAndLastname(firstname: String, lastname: String): User?
}

Original pull request: #241.

src/test/java/org/springframework/data/repository/core/support/ReactiveWrapperRepositoryFactorySupportUnitTests.java

@@ -20,7 +20,6 @@ import static org.mockito.Mockito.*;
 
 import io.reactivex.Completable;
 import io.reactivex.Maybe;
-import org.reactivestreams.Publisher;
 import reactor.core.publisher.Mono;
 import rx.Single;
 
@@ -33,6 +32,7 @@ import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
+import org.reactivestreams.Publisher;
 import org.springframework.data.repository.Repository;
 import org.springframework.data.repository.reactive.ReactiveSortingRepository;
 
@@ -67,10 +67,12 @@ public class ReactiveWrapperRepositoryFactorySupportUnitTests {
 		verify(backingRepo, times(0)).findById(1);
 	}
 
-	@Test // DATACMNS-836
+	@Test // DATACMNS-836, DATACMNS-1154
 	public void callsRxJava1MethodOnBaseImplementationWithExactArguments() {
 
 		Serializable id = 1L;
+		when(backingRepo.existsById(id)).thenReturn(Mono.just(true));
+
 		RxJava1ConvertingRepository repository = factory.getRepository(RxJava1ConvertingRepository.class);
 		repository.existsById(id);
 		repository.existsById((Long) id);
@@ -78,10 +80,12 @@ public class ReactiveWrapperRepositoryFactorySupportUnitTests {
 		verify(backingRepo, times(2)).existsById(id);
 	}
 
-	@Test // DATACMNS-836, DATACMNS-1063
+	@Test // DATACMNS-836, DATACMNS-1063, DATACMNS-1154
 	@SuppressWarnings("unchecked")
 	public void callsRxJava1MethodOnBaseImplementationWithTypeConversion() {
 
+		when(backingRepo.existsById(any(Publisher.class))).thenReturn(Mono.just(true));
+
 		Single<Long> ids = Single.just(1L);
 
 		RxJava1ConvertingRepository repository = factory.getRepository(RxJava1ConvertingRepository.class);
@@ -90,20 +94,23 @@ public class ReactiveWrapperRepositoryFactorySupportUnitTests {
 		verify(backingRepo, times(1)).existsById(any(Publisher.class));
 	}
 
-	@Test // DATACMNS-988
+	@Test // DATACMNS-988, DATACMNS-1154
 	public void callsRxJava2MethodOnBaseImplementationWithExactArguments() {
 
 		Long id = 1L;
+		when(backingRepo.findById(id)).thenReturn(Mono.just(true));
+
 		RxJava2ConvertingRepository repository = factory.getRepository(RxJava2ConvertingRepository.class);
 		repository.findById(id);
 
 		verify(backingRepo, times(1)).findById(id);
 	}
 
-	@Test // DATACMNS-988
+	@Test // DATACMNS-988, DATACMNS-1154
 	public void callsRxJava2MethodOnBaseImplementationWithTypeConversion() {
 
 		Serializable id = 1L;
+		when(backingRepo.deleteById(id)).thenReturn(Mono.empty());
 
 		RxJava2ConvertingRepository repository = factory.getRepository(RxJava2ConvertingRepository.class);
 		repository.deleteById(id);

src/test/java/org/springframework/data/repository/core/support/RepositoryFactorySupportUnitTests.java

@@ -39,7 +39,9 @@ import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.springframework.beans.factory.support.DefaultListableBeanFactory;
 import org.springframework.core.SpringVersion;
+import org.springframework.dao.EmptyResultDataAccessException;
 import org.springframework.data.domain.Page;
+import org.springframework.data.domain.PageImpl;
 import org.springframework.data.domain.PageRequest;
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Sort;
@@ -55,6 +57,7 @@ import org.springframework.data.repository.core.support.RepositoryComposition.Re
 import org.springframework.data.repository.query.RepositoryQuery;
 import org.springframework.data.repository.sample.User;
 import org.springframework.data.util.Version;
+import org.springframework.lang.Nullable;
 import org.springframework.scheduling.annotation.Async;
 import org.springframework.scheduling.annotation.AsyncAnnotationBeanPostProcessor;
 import org.springframework.test.util.ReflectionTestUtils;
@@ -142,6 +145,8 @@ public class RepositoryFactorySupportUnitTests {
 
 		CustomRepository repository = factory.getRepository(CustomRepository.class);
 		Pageable pageable = PageRequest.of(0, 10);
+
+		when(backingRepo.findAll(pageable)).thenReturn(new PageImpl<>(Collections.emptyList()));
 		repository.findAll(pageable);
 
 		verify(backingRepo, times(1)).findAll(pageable);
@@ -299,6 +304,52 @@ public class RepositoryFactorySupportUnitTests {
 		verifyZeroInteractions(backingRepo);
 	}
 
+	@Test // DATACMNS-1154
+	public void considersRequiredReturnValue() {
+
+		KotlinUserRepository repository = factory.getRepository(KotlinUserRepository.class);
+
+		assertThatThrownBy(() -> repository.findById("")).isInstanceOf(EmptyResultDataAccessException.class)
+				.hasMessageContaining("Result must not be null!");
+		assertThat(repository.findByUsername("")).isNull();
+	}
+
+	@Test // DATACMNS-1154
+	public void considersRequiredParameter() {
+
+		ObjectRepository repository = factory.getRepository(ObjectRepository.class);
+
+		assertThatThrownBy(() -> repository.findByClass(null)).isInstanceOf(IllegalArgumentException.class)
+				.hasMessageContaining("must not be null!");
+	}
+
+	@Test // DATACMNS-1154
+	public void shouldAllowVoidMethods() {
+
+		ObjectRepository repository = factory.getRepository(ObjectRepository.class, backingRepo);
+
+		repository.deleteAll();
+
+		verify(backingRepo).deleteAll();
+	}
+
+	@Test // DATACMNS-1154
+	public void considersRequiredKotlinParameter() {
+
+		KotlinUserRepository repository = factory.getRepository(KotlinUserRepository.class);
+
+		assertThatThrownBy(() -> repository.findById(null)).isInstanceOf(IllegalArgumentException.class)
+				.hasMessageContaining("must not be null!");
+	}
+
+	@Test // DATACMNS-1154
+	public void considersRequiredKotlinNullableParameter() {
+
+		KotlinUserRepository repository = factory.getRepository(KotlinUserRepository.class);
+
+		assertThat(repository.findByOptionalId(null)).isNull();
+	}
+
 	private ConvertingRepository prepareConvertingRepository(final Object expectedValue) {
 
 		when(factory.queryOne.execute(Mockito.any(Object[].class))).then(invocation -> {
@@ -330,10 +381,13 @@ public class RepositoryFactorySupportUnitTests {
 
 	interface ObjectRepository extends Repository<Object, Object>, ObjectRepositoryCustom {
 
+		@Nullable
 		Object findByClass(Class<?> clazz);
 
+		@Nullable
 		Object findByFoo();
 
+		@Nullable
 		Object save(Object entity);
 
 		static String staticMethod() {
@@ -347,7 +401,10 @@ public class RepositoryFactorySupportUnitTests {
 
 	interface ObjectRepositoryCustom {
 
+		@Nullable
 		Object findById(Object id);
+
+		void deleteAll();
 	}
 
 	interface PlainQueryCreationListener extends QueryCreationListener<RepositoryQuery> {

src/test/java/org/springframework/data/util/NullableUtilsUnitTests.java

@@ -0,0 +1,119 @@
+/*
+ * Copyright 2017 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
+ *
+ *      http://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.data.util;
+
+import static org.assertj.core.api.Assertions.*;
+
+import java.lang.annotation.ElementType;
+import java.lang.reflect.Method;
+
+import org.junit.Test;
+import org.springframework.core.MethodParameter;
+import org.springframework.data.util.nonnull.NullableAnnotatedType;
+import org.springframework.data.util.nonnull.packagelevel.NonNullOnPackage;
+import org.springframework.data.util.nonnull.type.Jsr305NonnullAnnotatedType;
+import org.springframework.data.util.nonnull.type.NonAnnotatedType;
+import org.springframework.data.util.nonnull.type.NonNullableParameters;
+import org.springframework.util.ReflectionUtils;
+
+/**
+ * Unit tests for {@link NullableUtils}.
+ *
+ * @author Mark Paluch
+ */
+public class NullableUtilsUnitTests {
+
+	@Test // DATACMNS-1154
+	public void packageAnnotatedShouldConsiderNonNullAnnotation() {
+
+		Method method = ReflectionUtils.findMethod(NonNullOnPackage.class, "nonNullReturnValue");
+
+		assertThat(NullableUtils.isNonNull(method, ElementType.METHOD)).isTrue();
+		assertThat(NullableUtils.isNonNull(method, ElementType.PARAMETER)).isTrue();
+		assertThat(NullableUtils.isNonNull(method, ElementType.PACKAGE)).isFalse();
+	}
+
+	@Test // DATACMNS-1154
+	public void packageAnnotatedShouldConsiderNonNullAnnotationForClass() {
+
+		assertThat(NullableUtils.isNonNull(NonNullOnPackage.class, ElementType.PARAMETER)).isTrue();
+		assertThat(NullableUtils.isNonNull(NonNullOnPackage.class, ElementType.PACKAGE)).isFalse();
+	}
+
+	@Test // DATACMNS-1154
+	public void packageAnnotatedShouldConsiderNonNullAnnotationForMethod() {
+
+		assertThat(NullableUtils.isNonNull(NonNullOnPackage.class, ElementType.PARAMETER)).isTrue();
+		assertThat(NullableUtils.isNonNull(NonNullOnPackage.class, ElementType.PACKAGE)).isFalse();
+	}
+
+	@Test // DATACMNS-1154
+	public void shouldConsiderJsr305NonNullParameters() {
+
+		assertThat(NullableUtils.isNonNull(NonNullableParameters.class, ElementType.PARAMETER)).isTrue();
+		assertThat(NullableUtils.isNonNull(NonNullableParameters.class, ElementType.FIELD)).isFalse();
+	}
+
+	@Test // DATACMNS-1154
+	public void shouldConsiderJsr305NonNullAnnotation() {
+
+		assertThat(NullableUtils.isNonNull(Jsr305NonnullAnnotatedType.class, ElementType.PARAMETER)).isTrue();
+		assertThat(NullableUtils.isNonNull(Jsr305NonnullAnnotatedType.class, ElementType.FIELD)).isTrue();
+	}
+
+	@Test // DATACMNS-1154
+	public void shouldConsiderNonAnnotatedTypeNullable() {
+
+		assertThat(NullableUtils.isNonNull(NonAnnotatedType.class, ElementType.PARAMETER)).isFalse();
+		assertThat(NullableUtils.isNonNull(NonAnnotatedType.class, ElementType.FIELD)).isFalse();
+	}
+
+	@Test // DATACMNS-1154
+	public void shouldConsiderParametersWithoutNullableAnnotation() {
+
+		Method method = ReflectionUtils.findMethod(NullableAnnotatedType.class, "nonNullMethod", String.class);
+
+		MethodParameter returnValue = new MethodParameter(method, -1);
+		MethodParameter parameter = new MethodParameter(method, 0);
+
+		assertThat(NullableUtils.isExplicitNullable(returnValue)).isFalse();
+		assertThat(NullableUtils.isExplicitNullable(parameter)).isFalse();
+	}
+
+	@Test // DATACMNS-1154
+	public void shouldConsiderParametersNullableAnnotation() {
+
+		Method method = ReflectionUtils.findMethod(NullableAnnotatedType.class, "nullableReturn");
+
+		assertThat(NullableUtils.isExplicitNullable(new MethodParameter(method, -1))).isTrue();
+	}
+
+	@Test // DATACMNS-1154
+	public void shouldConsiderParametersJsr305NullableMetaAnnotation() {
+
+		Method method = ReflectionUtils.findMethod(NullableAnnotatedType.class, "jsr305NullableReturn");
+
+		assertThat(NullableUtils.isExplicitNullable(new MethodParameter(method, -1))).isTrue();
+	}
+
+	@Test // DATACMNS-1154
+	public void shouldConsiderParametersJsr305NonnullAnnotation() {
+
+		Method method = ReflectionUtils.findMethod(NullableAnnotatedType.class, "jsr305NullableReturnWhen");
+
+		assertThat(NullableUtils.isExplicitNullable(new MethodParameter(method, -1))).isTrue();
+	}
+}

src/test/java/org/springframework/data/util/ReflectionUtilsUnitTests.java

@@ -23,6 +23,8 @@ import java.lang.reflect.Field;
 import org.junit.Before;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.core.MethodParameter;
+import org.springframework.data.repository.sample.User;
 import org.springframework.data.util.ReflectionUtils.DescribedFieldFilter;
 import org.springframework.util.ReflectionUtils.FieldFilter;
 
@@ -106,6 +108,42 @@ public class ReflectionUtilsUnitTests {
 		assertThat(ReflectionUtils.findConstructor(ConstructorDetection.class, null, "test")).isNotPresent();
 	}
 
+	@Test // DATACMNS-1154
+	public void discoversNoReturnType() throws Exception {
+
+		MethodParameter parameter = new MethodParameter(DummyInterface.class.getDeclaredMethod("noReturnValue"), -1);
+		assertThat(ReflectionUtils.isNullable(parameter)).isTrue();
+	}
+
+	@Test // DATACMNS-1154
+	public void discoversNullableReturnType() throws Exception {
+
+		MethodParameter parameter = new MethodParameter(DummyInterface.class.getDeclaredMethod("nullableReturnValue"), -1);
+		assertThat(ReflectionUtils.isNullable(parameter)).isTrue();
+	}
+
+	@Test // DATACMNS-1154
+	public void discoversNonNullableReturnType() throws Exception {
+
+		MethodParameter parameter = new MethodParameter(DummyInterface.class.getDeclaredMethod("mandatoryReturnValue"), -1);
+		assertThat(ReflectionUtils.isNullable(parameter)).isFalse();
+	}
+
+	@Test // DATACMNS-1154
+	public void discoversNullableParameter() throws Exception {
+
+		MethodParameter parameter = new MethodParameter(
+				DummyInterface.class.getDeclaredMethod("nullableParameter", User.class), 0);
+		assertThat(ReflectionUtils.isNullable(parameter)).isTrue();
+	}
+
+	@Test // DATACMNS-1154
+	public void discoversNonNullablePrimitiveParameter() throws Exception {
+
+		MethodParameter parameter = new MethodParameter(DummyInterface.class.getDeclaredMethod("primitive", int.class), 0);
+		assertThat(ReflectionUtils.isNullable(parameter)).isFalse();
+	}
+
 	static class Sample {
 
 		public String field;

src/test/java/org/springframework/data/util/nonnull/NullableAnnotatedType.java

@@ -0,0 +1,37 @@
+/*
+ * Copyright 2017 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
+ *
+ *      http://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.data.util.nonnull;
+
+import javax.annotation.meta.When;
+
+import org.springframework.lang.Nullable;
+
+/**
+ * @author Mark Paluch
+ */
+public interface NullableAnnotatedType {
+
+	String nonNullMethod(String parameter);
+
+	@Nullable
+	String nullableReturn();
+
+	@javax.annotation.Nullable
+	String jsr305NullableReturn();
+
+	@javax.annotation.Nonnull(when = When.MAYBE)
+	String jsr305NullableReturnWhen();
+}

src/test/java/org/springframework/data/util/nonnull/packagelevel/NonNullOnPackage.java

@@ -0,0 +1,24 @@
+/*
+ * Copyright 2017 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
+ *
+ *      http://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.data.util.nonnull.packagelevel;
+
+/**
+ * @author Mark Paluch
+ */
+public interface NonNullOnPackage {
+
+	String nonNullReturnValue();
+}

src/test/java/org/springframework/data/util/nonnull/packagelevel/package-info.java

@@ -0,0 +1,3 @@
+
+@org.springframework.lang.NonNullApi
+package org.springframework.data.util.nonnull.packagelevel;

src/test/java/org/springframework/data/util/nonnull/type/CustomAnnotatedType.java

@@ -0,0 +1,22 @@
+/*
+ * Copyright 2017 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
+ *
+ *      http://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.data.util.nonnull.type;
+
+/**
+ * @author Mark Paluch
+ */
+@CustomNonNullAnnotation
+public interface CustomAnnotatedType {}

src/test/java/org/springframework/data/util/nonnull/type/CustomNonNullAnnotation.java

@@ -0,0 +1,34 @@
+/*
+ * Copyright 2017 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
+ *
+ *      http://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.data.util.nonnull.type;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+import javax.annotation.Nonnull;
+import javax.annotation.meta.TypeQualifierDefault;
+
+/**
+ * @author Mark Paluch
+ */
+@Target(ElementType.TYPE)
+@Retention(RetentionPolicy.RUNTIME)
+@Nonnull
+@TypeQualifierDefault({ ElementType.FIELD })
+@interface CustomNonNullAnnotation {
+}

src/test/java/org/springframework/data/util/nonnull/type/Jsr305NonnullAnnotatedType.java

@@ -0,0 +1,24 @@
+/*
+ * Copyright 2017 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
+ *
+ *      http://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.data.util.nonnull.type;
+
+import javax.annotation.Nonnull;
+
+/**
+ * @author Mark Paluch
+ */
+@Nonnull
+public interface Jsr305NonnullAnnotatedType {}

src/test/java/org/springframework/data/util/nonnull/type/NonAnnotatedType.java

@@ -0,0 +1,21 @@
+/*
+ * Copyright 2017 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
+ *
+ *      http://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.data.util.nonnull.type;
+
+/**
+ * @author Mark Paluch
+ */
+public class NonAnnotatedType {}

src/test/java/org/springframework/data/util/nonnull/type/NonNullableParameters.java

@@ -0,0 +1,24 @@
+/*
+ * Copyright 2017 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
+ *
+ *      http://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.data.util.nonnull.type;
+
+import javax.annotation.ParametersAreNonnullByDefault;
+
+/**
+ * @author Mark Paluch
+ */
+@ParametersAreNonnullByDefault
+public interface NonNullableParameters {}