From 24783303a7b6b2d1fd9c080bab00a1890b1a9e49 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Thu, 21 May 2015 15:10:51 +0200 Subject: [PATCH] Add possible matches for field access DirectFieldAccessor now support richer NotWritablePropertyException content, including dedicated error message and possible matches. Issue: SPR-13053 --- .../beans/PropertyMatches.java | 202 +++++++++++++----- .../beans/PropertyMatchesTests.java | 192 +++++++++++++++++ 2 files changed, 339 insertions(+), 55 deletions(-) create mode 100644 spring-beans/src/test/java/org/springframework/beans/PropertyMatchesTests.java diff --git a/spring-beans/src/main/java/org/springframework/beans/PropertyMatches.java b/spring-beans/src/main/java/org/springframework/beans/PropertyMatches.java index ea860ec1610..312abd8967d 100644 --- a/spring-beans/src/main/java/org/springframework/beans/PropertyMatches.java +++ b/spring-beans/src/main/java/org/springframework/beans/PropertyMatches.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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,11 +17,13 @@ package org.springframework.beans; import java.beans.PropertyDescriptor; +import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Collections; import java.util.List; import org.springframework.util.ObjectUtils; +import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; /** @@ -31,10 +33,11 @@ import org.springframework.util.StringUtils; * @author Alef Arendsen * @author Arjen Poutsma * @author Juergen Hoeller + * @author Stephane Nicoll * @since 2.0 * @see #forProperty(String, Class) */ -final class PropertyMatches { +abstract class PropertyMatches { //--------------------------------------------------------------------- // Static section @@ -60,7 +63,26 @@ final class PropertyMatches { * @param maxDistance the maximum property distance allowed for matches */ public static PropertyMatches forProperty(String propertyName, Class beanClass, int maxDistance) { - return new PropertyMatches(propertyName, beanClass, maxDistance); + return new BeanPropertyMatches(propertyName, beanClass, maxDistance); + } + + /** + * Create PropertyMatches for the given field property. + * @param propertyName the name of the field to find possible matches for + * @param beanClass the bean class to search for matches + */ + public static PropertyMatches forField(String propertyName, Class beanClass) { + return forField(propertyName, beanClass, DEFAULT_MAX_DISTANCE); + } + + /** + * Create PropertyMatches for the given field property. + * @param propertyName the name of the field to find possible matches for + * @param beanClass the bean class to search for matches + * @param maxDistance the maximum property distance allowed for matches + */ + public static PropertyMatches forField(String propertyName, Class beanClass, int maxDistance) { + return new FieldPropertyMatches(propertyName, beanClass, maxDistance); } @@ -74,13 +96,19 @@ final class PropertyMatches { /** - * Create a new PropertyMatches instance for the given property. + * Create a new PropertyMatches instance for the given property and possible matches. */ - private PropertyMatches(String propertyName, Class beanClass, int maxDistance) { + private PropertyMatches(String propertyName, String[] possibleMatches) { this.propertyName = propertyName; - this.possibleMatches = calculateMatches(BeanUtils.getPropertyDescriptors(beanClass), maxDistance); + this.possibleMatches = possibleMatches; } + /** + * Return the name of the requested property. + */ + public String getPropertyName() { + return propertyName; + } /** * Return the calculated possible matches. @@ -93,54 +121,7 @@ final class PropertyMatches { * Build an error message for the given invalid property name, * indicating the possible property matches. */ - public String buildErrorMessage() { - StringBuilder msg = new StringBuilder(); - msg.append("Bean property '"); - msg.append(this.propertyName); - msg.append("' is not writable or has an invalid setter method. "); - - if (ObjectUtils.isEmpty(this.possibleMatches)) { - msg.append("Does the parameter type of the setter match the return type of the getter?"); - } - else { - msg.append("Did you mean "); - for (int i = 0; i < this.possibleMatches.length; i++) { - msg.append('\''); - msg.append(this.possibleMatches[i]); - if (i < this.possibleMatches.length - 2) { - msg.append("', "); - } - else if (i == this.possibleMatches.length - 2){ - msg.append("', or "); - } - } - msg.append("'?"); - } - return msg.toString(); - } - - - /** - * Generate possible property alternatives for the given property and - * class. Internally uses the {@code getStringDistance} method, which - * in turn uses the Levenshtein algorithm to determine the distance between - * two Strings. - * @param propertyDescriptors the JavaBeans property descriptors to search - * @param maxDistance the maximum distance to accept - */ - private String[] calculateMatches(PropertyDescriptor[] propertyDescriptors, int maxDistance) { - List candidates = new ArrayList(); - for (PropertyDescriptor pd : propertyDescriptors) { - if (pd.getWriteMethod() != null) { - String possibleAlternative = pd.getName(); - if (calculateStringDistance(this.propertyName, possibleAlternative) <= maxDistance) { - candidates.add(possibleAlternative); - } - } - } - Collections.sort(candidates); - return StringUtils.toStringArray(candidates); - } + public abstract String buildErrorMessage(); /** * Calculate the distance between the given two Strings @@ -149,7 +130,7 @@ final class PropertyMatches { * @param s2 the second String * @return the distance value */ - private int calculateStringDistance(String s1, String s2) { + private static int calculateStringDistance(String s1, String s2) { if (s1.length() == 0) { return s2.length(); } @@ -184,4 +165,115 @@ final class PropertyMatches { return d[s1.length()][s2.length()]; } + private static class BeanPropertyMatches extends PropertyMatches { + + private BeanPropertyMatches(String propertyName, Class beanClass, int maxDistance) { + super(propertyName, calculateMatches(propertyName, + BeanUtils.getPropertyDescriptors(beanClass), maxDistance)); + } + + /** + * Generate possible property alternatives for the given property and + * class. Internally uses the {@code getStringDistance} method, which + * in turn uses the Levenshtein algorithm to determine the distance between + * two Strings. + * @param propertyDescriptors the JavaBeans property descriptors to search + * @param maxDistance the maximum distance to accept + */ + private static String[] calculateMatches(String propertyName, PropertyDescriptor[] propertyDescriptors, int maxDistance) { + List candidates = new ArrayList(); + for (PropertyDescriptor pd : propertyDescriptors) { + if (pd.getWriteMethod() != null) { + String possibleAlternative = pd.getName(); + if (calculateStringDistance(propertyName, possibleAlternative) <= maxDistance) { + candidates.add(possibleAlternative); + } + } + } + Collections.sort(candidates); + return StringUtils.toStringArray(candidates); + } + + + @Override + public String buildErrorMessage() { + String propertyName = getPropertyName(); + String[] possibleMatches = getPossibleMatches(); + StringBuilder msg = new StringBuilder(); + msg.append("Bean property '"); + msg.append(propertyName); + msg.append("' is not writable or has an invalid setter method. "); + + if (ObjectUtils.isEmpty(possibleMatches)) { + msg.append("Does the parameter type of the setter match the return type of the getter?"); + } + else { + msg.append("Did you mean "); + for (int i = 0; i < possibleMatches.length; i++) { + msg.append('\''); + msg.append(possibleMatches[i]); + if (i < possibleMatches.length - 2) { + msg.append("', "); + } + else if (i == possibleMatches.length - 2) { + msg.append("', or "); + } + } + msg.append("'?"); + } + return msg.toString(); + } + + } + + private static class FieldPropertyMatches extends PropertyMatches { + + private FieldPropertyMatches(String propertyName, Class beanClass, int maxDistance) { + super(propertyName, calculateMatches(propertyName, beanClass, maxDistance)); + } + + private static String[] calculateMatches(final String propertyName, Class beanClass, final int maxDistance) { + final List candidates = new ArrayList(); + ReflectionUtils.doWithFields(beanClass, new ReflectionUtils.FieldCallback() { + @Override + public void doWith(Field field) throws IllegalArgumentException, IllegalAccessException { + String possibleAlternative = field.getName(); + if (calculateStringDistance(propertyName, possibleAlternative) <= maxDistance) { + candidates.add(possibleAlternative); + } + } + }); + Collections.sort(candidates); + return StringUtils.toStringArray(candidates); + } + + + @Override + public String buildErrorMessage() { + String propertyName = getPropertyName(); + String[] possibleMatches = getPossibleMatches(); + StringBuilder msg = new StringBuilder(); + msg.append("Bean property '"); + msg.append(propertyName); + msg.append("' has no matching field. "); + + if (!ObjectUtils.isEmpty(possibleMatches)) { + msg.append("Did you mean "); + for (int i = 0; i < possibleMatches.length; i++) { + msg.append('\''); + msg.append(possibleMatches[i]); + if (i < possibleMatches.length - 2) { + msg.append("', "); + } + else if (i == possibleMatches.length - 2) { + msg.append("', or "); + } + } + msg.append("'?"); + } + return msg.toString(); + } + + } + } diff --git a/spring-beans/src/test/java/org/springframework/beans/PropertyMatchesTests.java b/spring-beans/src/test/java/org/springframework/beans/PropertyMatchesTests.java new file mode 100644 index 00000000000..c37810ac3d0 --- /dev/null +++ b/spring-beans/src/test/java/org/springframework/beans/PropertyMatchesTests.java @@ -0,0 +1,192 @@ +/* + * Copyright 2002-2015 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.beans; + +import org.junit.Test; + +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; + +/** + * Tests for {@link PropertyMatches}. + * + * @author Stephane Nicoll + */ +public class PropertyMatchesTests { + + @Test + public void simpleBeanPropertyTypo() { + PropertyMatches matches = PropertyMatches.forProperty("naem", SampleBeanProperties.class); + assertThat(matches.getPossibleMatches(), hasItemInArray("name")); + } + + @Test + public void complexBeanPropertyTypo() { + PropertyMatches matches = PropertyMatches.forProperty("desriptn", SampleBeanProperties.class); + assertThat(matches.getPossibleMatches(), emptyArray()); + } + + @Test + public void unknownBeanProperty() { + PropertyMatches matches = PropertyMatches.forProperty("unknown", SampleBeanProperties.class); + assertThat(matches.getPossibleMatches(), emptyArray()); + } + + @Test + public void severalMatchesBeanProperty() { + PropertyMatches matches = PropertyMatches.forProperty("counter", SampleBeanProperties.class); + assertThat(matches.getPossibleMatches(), hasItemInArray("counter1")); + assertThat(matches.getPossibleMatches(), hasItemInArray("counter2")); + assertThat(matches.getPossibleMatches(), hasItemInArray("counter3")); + } + + @Test + public void simpleBeanPropertyErrorMessage() { + PropertyMatches matches = PropertyMatches.forProperty("naem", SampleBeanProperties.class); + String msg = matches.buildErrorMessage(); + assertThat(msg, containsString("naem")); + assertThat(msg, containsString("name")); + assertThat(msg, containsString("setter")); + assertThat(msg, not(containsString("field"))); + } + + @Test + public void complexBeanPropertyErrorMessage() { + PropertyMatches matches = PropertyMatches.forProperty("counter", SampleBeanProperties.class); + String msg = matches.buildErrorMessage(); + assertThat(msg, containsString("counter")); + assertThat(msg, containsString("counter1")); + assertThat(msg, containsString("counter2")); + assertThat(msg, containsString("counter3")); + } + + @Test + public void simpleFieldPropertyTypo() { + PropertyMatches matches = PropertyMatches.forField("naem", SampleFieldProperties.class); + assertThat(matches.getPossibleMatches(), hasItemInArray("name")); + } + + @Test + public void complexFieldPropertyTypo() { + PropertyMatches matches = PropertyMatches.forField("desriptn", SampleFieldProperties.class); + assertThat(matches.getPossibleMatches(), emptyArray()); + } + + @Test + public void unknownFieldProperty() { + PropertyMatches matches = PropertyMatches.forField("unknown", SampleFieldProperties.class); + assertThat(matches.getPossibleMatches(), emptyArray()); + } + + @Test + public void severalMatchesFieldProperty() { + PropertyMatches matches = PropertyMatches.forField("counter", SampleFieldProperties.class); + assertThat(matches.getPossibleMatches(), hasItemInArray("counter1")); + assertThat(matches.getPossibleMatches(), hasItemInArray("counter2")); + assertThat(matches.getPossibleMatches(), hasItemInArray("counter3")); + } + + @Test + public void simpleFieldPropertyErrorMessage() { + PropertyMatches matches = PropertyMatches.forField("naem", SampleFieldProperties.class); + String msg = matches.buildErrorMessage(); + assertThat(msg, containsString("naem")); + assertThat(msg, containsString("name")); + assertThat(msg, containsString("field")); + assertThat(msg, not(containsString("setter"))); + } + + @Test + public void complexFieldPropertyErrorMessage() { + PropertyMatches matches = PropertyMatches.forField("counter", SampleFieldProperties.class); + String msg = matches.buildErrorMessage(); + assertThat(msg, containsString("counter")); + assertThat(msg, containsString("counter1")); + assertThat(msg, containsString("counter2")); + assertThat(msg, containsString("counter3")); + } + + + private static class SampleBeanProperties { + + private String name; + + private String description; + + private int counter1; + + private int counter2; + + private int counter3; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getDescription() { + return description; + } + + public void setDescription(String description) { + this.description = description; + } + + public int getCounter1() { + return counter1; + } + + public void setCounter1(int counter1) { + this.counter1 = counter1; + } + + public int getCounter2() { + return counter2; + } + + public void setCounter2(int counter2) { + this.counter2 = counter2; + } + + public int getCounter3() { + return counter3; + } + + public void setCounter3(int counter3) { + this.counter3 = counter3; + } + } + + + private static class SampleFieldProperties { + + private String name; + + private String description; + + private int counter1; + + private int counter2; + + private int counter3; + + } + +}