From a01de8354718b5ca7bc5532974dcab1e3e125e16 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 10 Sep 2012 12:56:20 +0200 Subject: [PATCH] DATACMNS-228 - AbstractMappingContext disregards Groovy meta fields. Refactored persistent field discovery by extracting the FieldFilter implementation and improving the way excluded fields are detected. We now exclude all fields named "this$*" as well as a metaClass field of type groovy.lang.MetaClass explicitly. --- spring-data-commons-core/pom.xml | 8 ++ .../context/AbstractMappingContext.java | 99 +++++++++++++++++-- .../AbstractMappingContextUnitTests.java | 20 ++++ .../mapping/context/FieldMatchUnitTests.java | 80 +++++++++++++++ 4 files changed, 199 insertions(+), 8 deletions(-) create mode 100644 spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/FieldMatchUnitTests.java diff --git a/spring-data-commons-core/pom.xml b/spring-data-commons-core/pom.xml index 636ee9462..a2d914269 100644 --- a/spring-data-commons-core/pom.xml +++ b/spring-data-commons-core/pom.xml @@ -161,6 +161,14 @@ 2.2.3U1 test + + + + org.codehaus.groovy + groovy-all + 1.8.6 + test + diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java b/spring-data-commons-core/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java index a9c6a5a05..68d5bfb9e 100644 --- a/spring-data-commons-core/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java +++ b/spring-data-commons-core/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java @@ -22,8 +22,8 @@ import java.beans.PropertyDescriptor; import java.lang.reflect.Field; import java.lang.reflect.Modifier; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -52,6 +52,7 @@ import org.springframework.data.util.TypeInformation; import org.springframework.util.Assert; import org.springframework.util.ReflectionUtils; import org.springframework.util.ReflectionUtils.FieldCallback; +import org.springframework.util.ReflectionUtils.FieldFilter; /** * Base class to build mapping metadata and thus create instances of {@link PersistentEntity} and @@ -69,8 +70,6 @@ public abstract class AbstractMappingContext, ApplicationContextAware, ApplicationEventPublisherAware, ApplicationListener { - private static final Set UNMAPPED_FIELDS = new HashSet(Arrays.asList("class", "this$0")); - private final ConcurrentMap, E> persistentEntities = new ConcurrentHashMap, E>(); private ApplicationContext applicationContext; @@ -280,11 +279,7 @@ public abstract class AbstractMappingContext UNMAPPED_FIELDS; + + static { + + Set matches = new HashSet(); + matches.add(new FieldMatch("class", null)); + matches.add(new FieldMatch("this\\$.*", null)); + matches.add(new FieldMatch("metaClass", "groovy.lang.MetaClass")); + + UNMAPPED_FIELDS = Collections.unmodifiableCollection(matches); + } + + /* + * (non-Javadoc) + * @see org.springframework.util.ReflectionUtils.FieldFilter#matches(java.lang.reflect.Field) + */ + public boolean matches(Field field) { + + if (Modifier.isStatic(field.getModifiers())) { + return false; + } + + for (FieldMatch candidate : UNMAPPED_FIELDS) { + if (candidate.matches(field)) { + return false; + } + } + + return true; + } + } + + /** + * Value object to help defining field eclusion based on name patterns and types. + * + * @since 1.4 + * @author Oliver Gierke + */ + static class FieldMatch { + + private final String namePattern; + private final String typeName; + + /** + * Creates a new {@link FieldMatch} for the given name pattern and type name. At least one of the paramters must not + * be {@literal null}. + * + * @param namePattern a regex pattern to match field names, can be {@literal null}. + * @param typeName the name of the type to exclude, can be {@literal null}. + */ + public FieldMatch(String namePattern, String typeName) { + + Assert.isTrue(!(namePattern == null && typeName == null), "Either name patter or type name must be given!"); + + this.namePattern = namePattern; + this.typeName = typeName; + } + + /** + * Returns whether the given {@link Field} matches the defined {@link FieldMatch}. + * + * @param field must not be {@literal null}. + * @return + */ + public boolean matches(Field field) { + + if (namePattern != null && !field.getName().matches(namePattern)) { + return false; + } + + if (typeName != null && !field.getType().getName().equals(typeName)) { + return false; + } + + return true; + } + } } diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java b/spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java index 1bf404e5e..167484cab 100644 --- a/spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java +++ b/spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/AbstractMappingContextUnitTests.java @@ -18,6 +18,7 @@ package org.springframework.data.mapping.context; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; import static org.mockito.Mockito.*; +import groovy.lang.MetaClass; import java.beans.PropertyDescriptor; import java.lang.reflect.Field; @@ -30,6 +31,7 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationEvent; import org.springframework.context.event.ContextRefreshedEvent; import org.springframework.data.mapping.Association; +import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PropertyPath; import org.springframework.data.mapping.model.AbstractPersistentProperty; import org.springframework.data.mapping.model.BasicPersistentEntity; @@ -116,6 +118,19 @@ public class AbstractMappingContextUnitTests { context.getPersistentEntity((TypeInformation) null); } + /** + * @see DATACMNS-228 + */ + @Test + public void doesNotCreatePersistentPropertyForGroovyMetaClass() { + + DummyMappingContext mappingContext = new DummyMappingContext(); + mappingContext.initialize(); + + PersistentEntity entity = mappingContext.getPersistentEntity(Sample.class); + assertThat(entity.getPersistentProperty("metaClass"), is(nullValue())); + } + class Person { String name; } @@ -124,6 +139,11 @@ public class AbstractMappingContextUnitTests { } + class Sample { + + MetaClass metaClass; + } + class DummyMappingContext extends AbstractMappingContext, DummyPersistenProperty> { diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/FieldMatchUnitTests.java b/spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/FieldMatchUnitTests.java new file mode 100644 index 000000000..185f1c770 --- /dev/null +++ b/spring-data-commons-core/src/test/java/org/springframework/data/mapping/context/FieldMatchUnitTests.java @@ -0,0 +1,80 @@ +/* + * Copyright 2012 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.context; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + +import org.junit.Test; +import org.springframework.data.mapping.context.AbstractMappingContext.FieldMatch; + +/** + * Unit tests for {@link FieldMatch}. Introduced for DATACMNS-228. + * + * @since 1.4 + * @author Oliver Gierke + */ +public class FieldMatchUnitTests { + + @Test(expected = IllegalArgumentException.class) + public void rejectsBothParametersBeingNull() { + + new FieldMatch(null, null); + } + + @Test + public void matchesFieldByConcreteNameAndType() throws Exception { + + FieldMatch match = new FieldMatch("name", "java.lang.String"); + assertThat(match.matches(Sample.class.getField("this$0")), is(false)); + assertThat(match.matches(Sample.class.getField("this$1")), is(false)); + assertThat(match.matches(Sample.class.getField("name")), is(true)); + } + + @Test + public void matchesFieldByNamePattern() throws Exception { + + FieldMatch match = new FieldMatch("this\\$.*", "java.lang.Object"); + assertThat(match.matches(Sample.class.getField("this$0")), is(true)); + assertThat(match.matches(Sample.class.getField("this$1")), is(true)); + assertThat(match.matches(Sample.class.getField("name")), is(false)); + } + + @Test + public void matchesFieldByNameOnly() throws Exception { + + FieldMatch match = new FieldMatch("this\\$.*", null); + assertThat(match.matches(Sample.class.getField("this$0")), is(true)); + assertThat(match.matches(Sample.class.getField("this$1")), is(true)); + assertThat(match.matches(Sample.class.getField("name")), is(false)); + } + + @Test + public void matchesFieldByTypeNameOnly() throws Exception { + + FieldMatch match = new FieldMatch(null, "java.lang.Object"); + assertThat(match.matches(Sample.class.getField("this$0")), is(true)); + assertThat(match.matches(Sample.class.getField("this$1")), is(true)); + assertThat(match.matches(Sample.class.getField("name")), is(false)); + } + + static class Sample { + + public Object this$0; + public Object this$1; + public String name; + } +}