DATACMNS-1114 - Introduced usage of nullable annotations for API validation.

Marked all packages with Spring Frameworks @NonNullApi. Added Spring's @Nullable to methods, parameters and fields that take or produce null values. Adapted using code to make sure the IDE can evaluate the null flow properly. Fixed Javadoc in places where an invalid null handling policy was advertised. Strengthened null requirements for types that expose null-instances.

Removed null handling from converters for JodaTime and ThreeTenBP. Introduced factory methods Page.empty() and Page.empty(Pageable). Introduced default methods getRequiredGetter(), …Setter() and …Field() on PersistentProperty to allow non-nullable lookups of members. The same for TypeInformation.getrequiredActualType(), …SuperTypeInformation().

Tweaked PersistentPropertyCreator.addPropertiesForRemainingDescriptors() to filter unsuitable PropertyDescriptors before actually trying to create a Property instance from them as the new stronger nullability requirements would cause exceptions downstream.

Lazy.get() now expects a non-null return value. Clients being able to cope with null need to call ….orElse(…).

Original pull request: #232.
This commit is contained in:
Oliver Gierke
2017-06-27 08:41:16 +02:00
parent d9b16d8a27
commit 049970874d
234 changed files with 2274 additions and 1179 deletions

View File

@@ -47,6 +47,7 @@ public class SortUnitTests {
*
* @throws Exception
*/
@SuppressWarnings("null")
@Test(expected = IllegalArgumentException.class)
public void preventsNullProperties() throws Exception {
@@ -90,14 +91,15 @@ public class SortUnitTests {
public void allowsCombiningSorts() {
Sort sort = Sort.by("foo").and(Sort.by("bar"));
assertThat(sort).containsExactly(new Sort.Order("foo"), new Sort.Order("bar"));
assertThat(sort).containsExactly(Order.by("foo"), Order.by("bar"));
}
@Test
public void handlesAdditionalNullSort() {
Sort sort = Sort.by("foo").and(null);
assertThat(sort).containsExactly(new Sort.Order("foo"));
Sort sort = Sort.by("foo").and(Sort.unsorted());
assertThat(sort).containsExactly(Order.by("foo"));
}
@Test // DATACMNS-281, DATACMNS-1021
@@ -108,7 +110,7 @@ public class SortUnitTests {
@Test // DATACMNS-281, DATACMNS-1021
public void orderDoesNotIgnoreCaseByDefault() {
assertThat(new Order(Direction.ASC, "foo").isIgnoreCase()).isFalse();
assertThat(Order.by("foo").isIgnoreCase()).isFalse();
assertThat(Order.asc("foo").isIgnoreCase()).isFalse();
assertThat(Order.desc("foo").isIgnoreCase()).isFalse();
}
@@ -123,8 +125,8 @@ public class SortUnitTests {
@Test // DATACMNS-436
public void ordersWithDifferentIgnoreCaseDoNotEqual() {
Order foo = new Order("foo");
Order fooIgnoreCase = new Order("foo").ignoreCase();
Order foo = Order.by("foo");
Order fooIgnoreCase = Order.by("foo").ignoreCase();
assertThat(foo).isNotEqualTo(fooIgnoreCase);
assertThat(foo.hashCode()).isNotEqualTo(fooIgnoreCase.hashCode());
@@ -132,28 +134,28 @@ public class SortUnitTests {
@Test // DATACMNS-491
public void orderWithNullHandlingHintNullsFirst() {
assertThat(new Order("foo").nullsFirst().getNullHandling()).isEqualTo(NULLS_FIRST);
assertThat(Order.by("foo").nullsFirst().getNullHandling()).isEqualTo(NULLS_FIRST);
}
@Test // DATACMNS-491
public void orderWithNullHandlingHintNullsLast() {
assertThat(new Order("foo").nullsLast().getNullHandling()).isEqualTo(NULLS_LAST);
assertThat(Order.by("foo").nullsLast().getNullHandling()).isEqualTo(NULLS_LAST);
}
@Test // DATACMNS-491
public void orderWithNullHandlingHintNullsNative() {
assertThat(new Order("foo").nullsNative().getNullHandling()).isEqualTo(NATIVE);
assertThat(Order.by("foo").nullsNative().getNullHandling()).isEqualTo(NATIVE);
}
@Test // DATACMNS-491
public void orderWithDefaultNullHandlingHint() {
assertThat(new Order("foo").getNullHandling()).isEqualTo(NATIVE);
assertThat(Order.by("foo").getNullHandling()).isEqualTo(NATIVE);
}
@Test // DATACMNS-908
public void createsNewOrderForDifferentProperty() {
Order source = new Order(Direction.DESC, "foo").nullsFirst().ignoreCase();
Order source = Order.desc("foo").nullsFirst().ignoreCase();
Order result = source.withProperty("bar");
assertThat(result.getProperty()).isEqualTo("bar");
@@ -163,6 +165,7 @@ public class SortUnitTests {
}
@Test
@SuppressWarnings("null")
public void preventsNullDirection() {
assertThatExceptionOfType(IllegalArgumentException.class)//

View File

@@ -39,9 +39,7 @@ public class DistanceUnitTests {
@Test // DATACMNS-437
public void defaultsMetricToNeutralOne() {
assertThat(new Distance(2.5).getMetric()).isEqualTo((Metric) Metrics.NEUTRAL);
assertThat(new Distance(2.5, null).getMetric()).isEqualTo((Metric) Metrics.NEUTRAL);
}
@Test // DATACMNS-437

View File

@@ -68,11 +68,4 @@ public class PropertyMatchUnitTests {
assertThat(match.matches("this$1", Object.class)).isTrue();
assertThat(match.matches("name", String.class)).isFalse();
}
static class Sample {
public Object this$0;
public Object this$1;
public String name;
}
}

View File

@@ -25,7 +25,7 @@ import java.lang.annotation.Target;
import java.util.Map;
import java.util.Optional;
import javax.annotation.Nullable;
import org.springframework.lang.Nullable;
import org.junit.Before;
import org.junit.Test;

View File

@@ -39,11 +39,13 @@ public class ProxyProjectionFactoryUnitTests {
ProjectionFactory factory = new ProxyProjectionFactory();
@SuppressWarnings("null")
@Test(expected = IllegalArgumentException.class) // DATACMNS-630
public void rejectsNullProjectionType() {
factory.createProjection(null);
}
@SuppressWarnings("null")
@Test(expected = IllegalArgumentException.class) // DATACMNS-630
public void rejectsNullProjectionTypeWithSource() {
factory.createProjection(null, new Object());
@@ -51,7 +53,7 @@ public class ProxyProjectionFactoryUnitTests {
@Test // DATACMNS-630
public void returnsNullForNullSource() {
assertThat(factory.createProjection(CustomerExcerpt.class, null)).isNull();
assertThat(factory.createNullableProjection(CustomerExcerpt.class, null)).isNull();
}
@Test // DATAREST-221, DATACMNS-630

View File

@@ -32,9 +32,8 @@ import org.springframework.data.domain.Persistable;
@RunWith(MockitoJUnitRunner.class)
public class PersistableEntityInformationUnitTests {
@SuppressWarnings({ "rawtypes",
"unchecked" }) static final PersistableEntityInformation metadata = new PersistableEntityInformation(
Persistable.class);
@SuppressWarnings({ "rawtypes", "unchecked" }) //
static final PersistableEntityInformation metadata = new PersistableEntityInformation(PersistableEntity.class);
@Mock Persistable<Long> persistable;
@@ -68,16 +67,13 @@ public class PersistableEntityInformationUnitTests {
assertThat(info.getJavaType()).isEqualTo(PersistableEntity.class);
}
@SuppressWarnings("serial")
static class PersistableEntity implements Persistable<Long> {
public Long getId() {
return null;
}
public boolean isNew() {
return false;
}
}

View File

@@ -15,6 +15,7 @@
*/
package org.springframework.data.repository.init;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*;
import java.util.Collection;
@@ -23,6 +24,7 @@ import java.util.Collections;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.AdditionalAnswers;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.ApplicationEvent;
import org.springframework.context.ApplicationEventPublisher;
@@ -92,6 +94,7 @@ public class ResourceReaderRepositoryInitializerUnitTests {
throws Exception {
when(reader.readFrom(any(), any())).thenReturn(reference);
when(productRepository.save(any())).then(AdditionalAnswers.returnsFirstArg());
ResourceReaderRepositoryPopulator populator = new ResourceReaderRepositoryPopulator(reader);
populator.setResources(resource);

View File

@@ -15,6 +15,7 @@
*/
package org.springframework.data.repository.support;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*;
import static org.springframework.data.repository.support.RepositoryInvocationTestUtils.*;
@@ -26,6 +27,7 @@ import java.util.Optional;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.AdditionalAnswers;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
import org.springframework.core.convert.support.GenericConversionService;
@@ -56,6 +58,9 @@ public class CrudRepositoryInvokerUnitTests {
@Test // DATACMNS-589, DATAREST-216
public void invokesRedeclaredSave() {
when(orderRepository.save(any())).then(AdditionalAnswers.returnsFirstArg());
getInvokerFor(orderRepository, expectInvocationOnType(OrderRepository.class)).invokeSave(new Order());
}

View File

@@ -16,9 +16,11 @@
package org.springframework.data.repository.support;
import static org.assertj.core.api.Assertions.*;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*;
import java.lang.reflect.Method;
import java.util.Optional;
import org.junit.Before;
import org.junit.Test;
@@ -177,13 +179,13 @@ public class DomainClassConverterUnitTests {
converter.setApplicationContext(initContextWithRepo());
@SuppressWarnings("rawtypes")
DomainClassConverter.ToIdConverter toIdConverter = (ToIdConverter) ReflectionTestUtils.getField(converter,
Optional<ToIdConverter> toIdConverter = (Optional<ToIdConverter>) ReflectionTestUtils.getField(converter,
"toIdConverter");
Method method = Wrapper.class.getMethod("foo", User.class);
TypeDescriptor target = TypeDescriptor.nested(new MethodParameter(method, 0), 0);
assertThat(toIdConverter.matches(SUB_USER_TYPE, target)).isFalse();
assertThat(toIdConverter).map(it -> it.matches(SUB_USER_TYPE, target)).hasValue(false);
}
private ApplicationContext initContextWithRepo() {

View File

@@ -15,6 +15,7 @@
*/
package org.springframework.data.repository.support;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*;
import static org.springframework.data.repository.support.RepositoryInvocationTestUtils.*;
@@ -65,6 +66,8 @@ public class PaginginAndSortingRepositoryInvokerUnitTests {
RepositoryWithRedeclaredFindAllWithPageable repository = mock(RepositoryWithRedeclaredFindAllWithPageable.class);
Method method = RepositoryWithRedeclaredFindAllWithPageable.class.getMethod("findAll", Pageable.class);
when(repository.findAll(any(Pageable.class))).thenReturn(Page.empty());
getInvokerFor(repository, expectInvocationOf(method)).invokeFindAll(PageRequest.of(0, 10));
getInvokerFor(repository, expectInvocationOf(method)).invokeFindAll(Pageable.unpaged());
}

View File

@@ -23,7 +23,6 @@ import static org.springframework.data.repository.support.RepositoryInvocationTe
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.List;
import java.util.Optional;
@@ -31,12 +30,12 @@ import java.util.Optional;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.AdditionalAnswers;
import org.mockito.junit.MockitoJUnitRunner;
import org.springframework.core.convert.ConversionFailedException;
import org.springframework.core.convert.ConversionService;
import org.springframework.core.convert.support.GenericConversionService;
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;
@@ -45,7 +44,6 @@ import org.springframework.data.repository.Repository;
import org.springframework.data.repository.core.RepositoryMetadata;
import org.springframework.data.repository.core.support.DefaultRepositoryMetadata;
import org.springframework.data.repository.query.Param;
import org.springframework.data.repository.support.CrudRepositoryInvokerUnitTests.Person;
import org.springframework.data.repository.support.CrudRepositoryInvokerUnitTests.PersonRepository;
import org.springframework.data.repository.support.RepositoryInvocationTestUtils.VerifyingMethodInterceptor;
import org.springframework.format.support.DefaultFormattingConversionService;
@@ -60,8 +58,6 @@ import org.springframework.util.MultiValueMap;
@RunWith(MockitoJUnitRunner.class)
public class ReflectionRepositoryInvokerUnitTests {
static final Page<Person> EMPTY_PAGE = new PageImpl<>(Collections.emptyList());
ConversionService conversionService;
@Before
@@ -75,6 +71,8 @@ public class ReflectionRepositoryInvokerUnitTests {
ManualCrudRepository repository = mock(ManualCrudRepository.class);
Method method = ManualCrudRepository.class.getMethod("save", Domain.class);
when(repository.save(any())).then(AdditionalAnswers.returnsFirstArg());
getInvokerFor(repository, expectInvocationOf(method)).invokeSave(new Domain());
}
@@ -118,6 +116,8 @@ public class ReflectionRepositoryInvokerUnitTests {
Method method = RepoWithFindAllWithSort.class.getMethod("findAll", Sort.class);
RepoWithFindAllWithSort repository = mock(RepoWithFindAllWithSort.class);
when(repository.findAll(any())).thenReturn(Page.empty());
getInvokerFor(repository, expectInvocationOf(method)).invokeFindAll(Pageable.unpaged());
getInvokerFor(repository, expectInvocationOf(method)).invokeFindAll(PageRequest.of(0, 10));
getInvokerFor(repository, expectInvocationOf(method)).invokeFindAll(Sort.unsorted());
@@ -130,6 +130,8 @@ public class ReflectionRepositoryInvokerUnitTests {
Method method = RepoWithFindAllWithPageable.class.getMethod("findAll", Pageable.class);
RepoWithFindAllWithPageable repository = mock(RepoWithFindAllWithPageable.class);
when(repository.findAll(any())).thenReturn(Page.empty());
getInvokerFor(repository, expectInvocationOf(method)).invokeFindAll(Pageable.unpaged());
getInvokerFor(repository, expectInvocationOf(method)).invokeFindAll(PageRequest.of(0, 10));
}

View File

@@ -41,8 +41,8 @@ public class AnnotationDetectionFieldCallbackUnitTests {
AnnotationDetectionFieldCallback callback = new AnnotationDetectionFieldCallback(Autowired.class);
ReflectionUtils.doWithFields(Sample.class, callback);
assertThat(callback.getType()).hasValue(String.class);
assertThat(callback.getValue(new Sample("foo"))).hasValue("foo");
assertThat(callback.getType()).isEqualTo(String.class);
assertThat(callback.<Object> getValue(new Sample("foo"))).isEqualTo("foo");
}
@Test // DATACMNS-616
@@ -51,8 +51,8 @@ public class AnnotationDetectionFieldCallbackUnitTests {
AnnotationDetectionFieldCallback callback = new AnnotationDetectionFieldCallback(Autowired.class);
ReflectionUtils.doWithFields(Empty.class, callback);
assertThat(callback.getType()).isNotPresent();
assertThat(callback.getValue(new Empty())).isNotPresent();
assertThat(callback.getType()).isNull();
assertThat(callback.<Object> getValue(new Empty())).isNull();
}
@Value

View File

@@ -124,7 +124,6 @@ public class VersionUnitTests {
assertThat(version.compareTo(nextBugfix)).isLessThan(0);
assertThat(version.compareTo(nextBuild)).isLessThan(0);
assertThat(version.compareTo(null)).isGreaterThan(0);
assertThat(nextMajor.compareTo(version)).isGreaterThan(0);
assertThat(nextMinor.compareTo(version)).isGreaterThan(0);
assertThat(nextBugfix.compareTo(version)).isGreaterThan(0);

View File

@@ -143,12 +143,12 @@ public class PageableHandlerMethodArgumentResolverUnitTests extends PageableDefa
PageableHandlerMethodArgumentResolver resolver = getResolver();
resolver.setFallbackPageable(Pageable.unpaged());
assertSupportedAndResult(supportedMethodParameter, null, new ServletWebRequest(new MockHttpServletRequest()),
resolver);
assertSupportedAndResult(supportedMethodParameter, Pageable.unpaged(),
new ServletWebRequest(new MockHttpServletRequest()), resolver);
}
@Test // DATACMNS-477
public void returnsNullIfFallbackIsUnpagedAndOnlyPageIsGiven() throws Exception {
public void returnsFallbackIfOnlyPageIsGiven() throws Exception {
PageableHandlerMethodArgumentResolver resolver = getResolver();
resolver.setFallbackPageable(Pageable.unpaged());
@@ -156,11 +156,12 @@ public class PageableHandlerMethodArgumentResolverUnitTests extends PageableDefa
MockHttpServletRequest request = new MockHttpServletRequest();
request.addParameter("page", "20");
assertThat(resolver.resolveArgument(supportedMethodParameter, null, new ServletWebRequest(request), null)).isNull();
assertThat(resolver.resolveArgument(supportedMethodParameter, null, new ServletWebRequest(request), null))
.isEqualTo(Pageable.unpaged());
}
@Test // DATACMNS-477
public void returnsNullIfFallbackIsUnpagedAndOnlySizeIsGiven() throws Exception {
public void returnsFallbackIfFallbackIsUnpagedAndOnlySizeIsGiven() throws Exception {
PageableHandlerMethodArgumentResolver resolver = getResolver();
resolver.setFallbackPageable(Pageable.unpaged());
@@ -168,7 +169,8 @@ public class PageableHandlerMethodArgumentResolverUnitTests extends PageableDefa
MockHttpServletRequest request = new MockHttpServletRequest();
request.addParameter("size", "10");
assertThat(resolver.resolveArgument(supportedMethodParameter, null, new ServletWebRequest(request), null)).isNull();
assertThat(resolver.resolveArgument(supportedMethodParameter, null, new ServletWebRequest(request), null))
.isEqualTo(Pageable.unpaged());
}
@Test // DATACMNS-563