DATACMNS-801 - Make sure PropertyReferenceException does not expose duplicate potential matches.

We now use a Set to collect potential matches from fields and accessors.

Polished assertions and JavaDoc in PropertyPath along the way.
This commit is contained in:
Oliver Gierke
2016-01-12 08:39:44 +01:00
parent f6a3d7b1ba
commit dd70b52a73
3 changed files with 125 additions and 15 deletions

View File

@@ -16,6 +16,7 @@
package org.springframework.data.mapping;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Stack;
@@ -53,7 +54,7 @@ public class PropertyPath implements Iterable<PropertyPath> {
* @param owningType must not be {@literal null}.
*/
PropertyPath(String name, Class<?> owningType) {
this(name, ClassTypeInformation.from(owningType), null);
this(name, ClassTypeInformation.from(owningType), Collections.<PropertyPath> emptyList());
}
/**
@@ -65,8 +66,9 @@ public class PropertyPath implements Iterable<PropertyPath> {
*/
PropertyPath(String name, TypeInformation<?> owningType, List<PropertyPath> base) {
Assert.hasText(name);
Assert.notNull(owningType);
Assert.hasText(name, "Name must not be null or empty!");
Assert.notNull(owningType, "Owning type must not be null!");
Assert.notNull(base, "Perviously found properties must not be null!");
String propertyName = name.matches(ALL_UPPERCASE) ? name : StringUtils.uncapitalize(name);
TypeInformation<?> propertyType = owningType.getProperty(propertyName);

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2012-2015 the original author or authors.
* Copyright 2012-2016 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.
@@ -15,9 +15,11 @@
*/
package org.springframework.data.mapping;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import org.springframework.beans.PropertyMatches;
import org.springframework.data.util.TypeInformation;
@@ -38,20 +40,21 @@ public class PropertyReferenceException extends RuntimeException {
private final String propertyName;
private final TypeInformation<?> type;
private final List<PropertyPath> alreadyResolvedPath;
private final List<String> propertyMatches;
private final Set<String> propertyMatches;
/**
* Creates a new {@link PropertyReferenceException}.
*
* @param propertyName the name of the property not found on the given type.
* @param type the type the property could not be found on.
* @param alreadyResolvedPah the previously calculated {@link PropertyPath}s.
* @param propertyName the name of the property not found on the given type, must not be {@literal null} or empty.
* @param type the type the property could not be found on, must not be {@literal null}.
* @param alreadyResolvedPah the previously calculated {@link PropertyPath}s, must not be {@literal null}.
*/
public PropertyReferenceException(String propertyName, TypeInformation<?> type,
List<PropertyPath> alreadyResolvedPah) {
Assert.hasText(propertyName);
Assert.notNull(type);
Assert.hasText(propertyName, "Property name must not be null!");
Assert.notNull(type, "Type must not be null!");
Assert.notNull(alreadyResolvedPah, "Already resolved paths must not be null!");
this.propertyName = propertyName;
this.type = type;
@@ -71,12 +74,21 @@ public class PropertyReferenceException extends RuntimeException {
/**
* Returns the type the property could not be found on.
*
* @return the type
* @return will never be {@literal null}.
*/
public TypeInformation<?> getType() {
return type;
}
/**
* Returns the properties that the invalid property might have been meant to be referred to.
*
* @return will never be {@literal null}.
*/
Collection<String> getPropertyMatches() {
return propertyMatches;
}
/*
* (non-Javadoc)
* @see java.lang.Throwable#getMessage()
@@ -114,7 +126,7 @@ public class PropertyReferenceException extends RuntimeException {
* Returns whether the given {@link PropertyReferenceException} has a deeper resolution depth (i.e. a longer path of
* already resolved properties) than the current exception.
*
* @param exception
* @param exception must not be {@literal null}.
* @return
*/
public boolean hasDeeperResolutionDepthThan(PropertyReferenceException exception) {
@@ -128,9 +140,9 @@ public class PropertyReferenceException extends RuntimeException {
* @param type must not be {@literal null}.
* @return
*/
private static List<String> detectPotentialMatches(String propertyName, Class<?> type) {
private static Set<String> detectPotentialMatches(String propertyName, Class<?> type) {
List<String> result = new ArrayList<String>();
Set<String> result = new HashSet<String>();
result.addAll(Arrays.asList(PropertyMatches.forField(propertyName, type).getPossibleMatches()));
result.addAll(Arrays.asList(PropertyMatches.forProperty(propertyName, type).getPossibleMatches()));

View File

@@ -0,0 +1,96 @@
/*
* Copyright 2016 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.mapping;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.springframework.data.util.ClassTypeInformation;
import org.springframework.data.util.TypeInformation;
/**
* Unit tests for {@link PropertyReferenceException}
*
* @author Oliver Gierke
*/
public class PropertyReferenceExceptionUnitTests {
static final TypeInformation<Sample> TYPE_INFO = ClassTypeInformation.from(Sample.class);
static final List<PropertyPath> NO_PATHS = Collections.emptyList();
public @Rule ExpectedException exception = ExpectedException.none();
@Test
public void rejectsNullPropertyName() {
exception.expect(IllegalArgumentException.class);
exception.expectMessage("name");
new PropertyReferenceException(null, TYPE_INFO, NO_PATHS);
}
@Test
public void rejectsNullType() {
exception.expect(IllegalArgumentException.class);
exception.expectMessage("Type");
new PropertyReferenceException("nme", null, NO_PATHS);
}
@Test
public void rejectsNullPaths() {
exception.expect(IllegalArgumentException.class);
exception.expectMessage("paths");
new PropertyReferenceException("nme", TYPE_INFO, null);
}
/**
* @see DATACMNS-801
*/
@Test
public void exposesPotentialMatch() {
PropertyReferenceException exception = new PropertyReferenceException("nme", TYPE_INFO, NO_PATHS);
Collection<String> matches = exception.getPropertyMatches();
assertThat(matches, hasSize(1));
assertThat(matches, hasItem("name"));
}
static class Sample {
String name;
public String getName() {
return name;
}
public void setName(String name) {
this.name = name;
}
}
}