DATACMNS-387 - Improvements in null handling in PartTree area.

We're now rejecting invalid constructor arguments handed to ClassTypeInformation, Part, PartTree and PropertyPath. Beyond that we skip the creation of a Part for an empty path segment, so that you don't end up with an invalid Part instance for a findAllByOrderByFooAsc.
This commit is contained in:
Oliver Gierke
2013-10-27 16:28:54 +01:00
parent ad622d8164
commit 0d2bf97d94
7 changed files with 92 additions and 18 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2011-2012 the original author or authors.
* Copyright 2011-2013 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.
@@ -52,7 +52,6 @@ public class PropertyPath implements Iterable<PropertyPath> {
* @param owningType must not be {@literal null}.
*/
PropertyPath(String name, Class<?> owningType) {
this(name, ClassTypeInformation.from(owningType), null);
}
@@ -132,7 +131,6 @@ public class PropertyPath implements Iterable<PropertyPath> {
* @see #hasNext()
*/
public PropertyPath next() {
return next;
}
@@ -143,7 +141,6 @@ public class PropertyPath implements Iterable<PropertyPath> {
* @return
*/
public boolean hasNext() {
return next != null;
}
@@ -167,7 +164,6 @@ public class PropertyPath implements Iterable<PropertyPath> {
* @return
*/
public boolean isCollection() {
return isCollection;
}
@@ -241,7 +237,6 @@ public class PropertyPath implements Iterable<PropertyPath> {
* @return
*/
public static PropertyPath from(String source, Class<?> type) {
return from(source, ClassTypeInformation.from(type));
}
@@ -254,6 +249,9 @@ public class PropertyPath implements Iterable<PropertyPath> {
*/
public static PropertyPath from(String source, TypeInformation<?> type) {
Assert.hasText(source, "Source must not be null or empty!");
Assert.notNull(type, "TypeInformation must not be null or empty!");
List<String> iteratorSource = new ArrayList<String>();
Matcher matcher = SPLITTER.matcher("_" + source);

View File

@@ -24,6 +24,7 @@ import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.springframework.data.mapping.PropertyPath;
import org.springframework.util.Assert;
import org.springframework.util.StringUtils;
/**
@@ -46,25 +47,27 @@ public class Part {
* Creates a new {@link Part} from the given method name part, the {@link Class} the part originates from and the
* start parameter index.
*
* @param part must not be {@literal null}.
* @param source must not be {@literal null}.
* @param clazz must not be {@literal null}.
*/
public Part(String part, Class<?> clazz) {
this(part, clazz, false);
public Part(String source, Class<?> clazz) {
this(source, clazz, false);
}
/**
* Creates a new {@link Part} from the given method name part, the {@link Class} the part originates from and the
* start parameter index.
*
* @param part must not be {@literal null}.
* @param source must not be {@literal null}.
* @param clazz must not be {@literal null}.
* @param alwaysIgnoreCase
*/
public Part(String part, Class<?> clazz, boolean alwaysIgnoreCase) {
public Part(String source, Class<?> clazz, boolean alwaysIgnoreCase) {
String partToUse = detectAndSetIgnoreCase(part);
Assert.hasText(source, "Part source must not be null or emtpy!");
Assert.notNull(clazz, "Type must not be null!");
String partToUse = detectAndSetIgnoreCase(source);
if (alwaysIgnoreCase && ignoreCase != IgnoreCaseType.ALWAYS) {
this.ignoreCase = IgnoreCaseType.WHEN_POSSIBLE;
}

View File

@@ -185,7 +185,9 @@ public class PartTree implements Iterable<OrPart> {
String[] split = split(source, "And");
for (String part : split) {
children.add(new Part(part, domainClass, alwaysIgnoreCase));
if (StringUtils.hasText(part)) {
children.add(new Part(part, domainClass, alwaysIgnoreCase));
}
}
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2011 the original author or authors.
* Copyright 2011-2013 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.
@@ -29,6 +29,7 @@ import java.util.Set;
import java.util.WeakHashMap;
import org.springframework.core.GenericTypeResolver;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
/**
@@ -60,11 +61,13 @@ public class ClassTypeInformation<S> extends TypeDiscoverer<S> {
* Simple factory method to easily create new instances of {@link ClassTypeInformation}.
*
* @param <S>
* @param type
* @param type must not be {@literal null}.
* @return
*/
public static <S> TypeInformation<S> from(Class<S> type) {
Assert.notNull(type, "Type must not be null!");
Reference<TypeInformation<?>> cachedReference = CACHE.get(type);
TypeInformation<?> cachedTypeInfo = cachedReference == null ? null : cachedReference.get();
@@ -80,10 +83,12 @@ public class ClassTypeInformation<S> extends TypeDiscoverer<S> {
/**
* Creates a {@link TypeInformation} from the given method's return type.
*
* @param method
* @param method must not be {@literal null}.
* @return
*/
public static <S> TypeInformation<S> fromReturnTypeOf(Method method) {
Assert.notNull(method, "Method must not be null!");
return new ClassTypeInformation(method.getDeclaringClass()).createInfo(method.getGenericReturnType());
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2011-2012 the original author or authors.
* Copyright 2011-2013 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.
@@ -17,6 +17,7 @@ package org.springframework.data.mapping;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
import static org.springframework.data.mapping.PropertyPath.*;
import java.util.Iterator;
import java.util.Map;
@@ -307,6 +308,51 @@ public class PropertyPathUnitTests {
assertThat(path.next().getSegment(), is("UUID"));
}
/**
* @see DATACMNS-381
*/
public void exposesPreviouslyReferencedPathInExceptionMessage() {
exception.expect(PropertyReferenceException.class);
exception.expectMessage("bar"); // missing variable
exception.expectMessage("String"); // type
exception.expectMessage("Bar.user.name"); // previously referenced path
PropertyPath.from("userNameBar", Bar.class);
}
/**
* @see DATACMNS-387
*/
@Test(expected = IllegalArgumentException.class)
public void rejectsNullSource() {
from(null, Foo.class);
}
/**
* @see DATACMNS-387
*/
@Test(expected = IllegalArgumentException.class)
public void rejectsEmptySource() {
from("", Foo.class);
}
/**
* @see DATACMNS-387
*/
@Test(expected = IllegalArgumentException.class)
public void rejectsNullClass() {
from("foo", (Class<?>) null);
}
/**
* @see DATACMNS-387
*/
@Test(expected = IllegalArgumentException.class)
public void rejectsNullTypeInformation() {
from("foo", (TypeInformation<?>) null);
}
private class Foo {
String userName;

View File

@@ -374,6 +374,18 @@ public class PartTreeUnitTests {
assertThat(new PartTree("findByAnders", Product.class), is(notNullValue()));
}
/**
* @see DATACMNS-387
*/
@Test
public void buildsPartTreeFromEmptyPredicateCorrectly() {
PartTree tree = new PartTree("findAllByOrderByLastnameAsc", User.class);
assertThat(tree.getParts(), is(emptyIterable()));
assertThat(tree.getSort(), is(new Sort(Direction.ASC, "lastname")));
}
private static void assertType(Iterable<String> sources, Type type, String property) {
assertType(sources, type, property, 1, true);
}

View File

@@ -277,6 +277,14 @@ public class ClassTypeInformationUnitTests {
assertThat(categoryIdInfo, is((TypeInformation) from(Long.class)));
}
/**
* @see DATACMNS-387
*/
@Test(expected = IllegalArgumentException.class)
public void rejectsNullClass() {
from(null);
}
static class StringMapContainer extends MapContainer<String> {
}