From 0c4fc4eea3cd916ea2ed1b4e5a10d60cb6710f4a Mon Sep 17 00:00:00 2001 From: rbygrave Date: Sat, 19 Jul 2014 17:49:03 +1200 Subject: [PATCH] Fix for #173 - Ability to use a MappedSuperClass without being enhanced as long as it doesn't have persistent fields --- pom.xml | 2 +- .../server/deploy/BeanDescriptorManager.java | 86 +++++++++++++------ .../deploy/meta/DeployBeanDescriptor.java | 6 +- .../deploy/meta/DeployBeanPropertyLists.java | 12 +-- .../deploy/parse/AnnotationAssocManys.java | 6 +- .../deploy/parse/AnnotationAssocOnes.java | 5 +- .../server/deploy/parse/AnnotationFields.java | 6 +- .../deploy/parse/DeployCreateProperties.java | 12 +-- .../deploy/TestNotEnhancedMappedSuper.java | 29 +++++++ .../tests/model/mappedsuper/ASimpleBean.java | 30 +++++++ .../mappedsuper/NotEnhancedMappedSuper.java | 32 +++++++ 11 files changed, 165 insertions(+), 61 deletions(-) create mode 100644 src/test/java/com/avaje/ebeaninternal/server/deploy/TestNotEnhancedMappedSuper.java create mode 100644 src/test/java/com/avaje/tests/model/mappedsuper/ASimpleBean.java create mode 100644 src/test/java/com/avaje/tests/model/mappedsuper/NotEnhancedMappedSuper.java diff --git a/pom.xml b/pom.xml index 8b053eaf4..e7bed8abf 100644 --- a/pom.xml +++ b/pom.xml @@ -94,7 +94,7 @@ org.avaje.ebeanorm avaje-ebeanorm-agent - 4.1.2 + 4.1.5 test diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanDescriptorManager.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanDescriptorManager.java index dca24d05d..a044a0deb 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanDescriptorManager.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanDescriptorManager.java @@ -1,6 +1,8 @@ package com.avaje.ebeaninternal.server.deploy; import java.io.Serializable; +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; @@ -11,7 +13,9 @@ import java.util.List; import java.util.Map; import java.util.Set; +import javax.persistence.MappedSuperclass; import javax.persistence.PersistenceException; +import javax.persistence.Transient; import javax.sql.DataSource; import org.slf4j.Logger; @@ -61,9 +65,7 @@ import com.avaje.ebeaninternal.server.idgen.UuidIdGenerator; import com.avaje.ebeaninternal.server.lib.util.Dnode; import com.avaje.ebeaninternal.server.reflect.BeanReflect; import com.avaje.ebeaninternal.server.reflect.BeanReflectFactory; -import com.avaje.ebeaninternal.server.reflect.BeanReflectGetter; import com.avaje.ebeaninternal.server.reflect.BeanReflectProperties; -import com.avaje.ebeaninternal.server.reflect.BeanReflectSetter; import com.avaje.ebeaninternal.server.reflect.EnhanceBeanReflectFactory; import com.avaje.ebeaninternal.server.type.TypeManager; @@ -1154,12 +1156,8 @@ public class BeanDescriptorManager implements BeanDescriptorMap { */ private void setScalarType(DeployBeanDescriptor deployDesc) { - Iterator it = deployDesc.propertiesAll(); - while (it.hasNext()) { - DeployBeanProperty prop = it.next(); - if (prop instanceof DeployBeanPropertyAssoc) { - - } else { + for (DeployBeanProperty prop : deployDesc.propertiesAll()) { + if (prop instanceof DeployBeanPropertyAssoc == false) { deployUtil.setScalarType(prop); } } @@ -1313,30 +1311,39 @@ public class BeanDescriptorManager implements BeanDescriptorMap { desc.setBeanReflect(beanReflect); desc.setProperties(reflectProps.getProperties()); - Iterator it = desc.propertiesAll(); - while (it.hasNext()) { - DeployBeanProperty prop = it.next(); + for (DeployBeanProperty prop : desc.propertiesAll()) { String propName = prop.getName(); - Integer pos = reflectProps.getPropertyIndex(propName); if (pos == null) { - throw new IllegalStateException("Property "+propName+" not found in "+reflectProps); - } - - BeanReflectGetter getter = beanReflect.getGetter(propName, pos.intValue()); - BeanReflectSetter setter = beanReflect.getSetter(propName, pos.intValue()); - prop.setGetter(getter); - prop.setSetter(setter); - prop.setPropertyIndex(pos.intValue()); - - if (getter == null) { - String m = "BeanReflectGetter for " + prop.getFullBeanName() + " was not found?"; - throw new RuntimeException(m); - + if (isPersistentField(prop)) { + throw new IllegalStateException("Property "+propName+" not found in "+reflectProps); + } + + } else { + int propertyIndex = pos.intValue(); + prop.setPropertyIndex(propertyIndex); + prop.setGetter(beanReflect.getGetter(propName, propertyIndex)); + prop.setSetter(beanReflect.getSetter(propName, propertyIndex)); } } } + /** + * Return true if this is a persistent field (not transient or static). + */ + private boolean isPersistentField(DeployBeanProperty prop) { + + Field field = prop.getField(); + int modifiers = field.getModifiers(); + if (Modifier.isStatic(modifiers) || Modifier.isTransient(modifiers)) { + return false; + } + if (field.isAnnotationPresent(Transient.class)) { + return false; + } + return true; + } + /** * DevNote: It is assumed that Embedded can contain version properties. It is * also assumed that Embedded beans do NOT themselves contain Embedded beans @@ -1408,6 +1415,7 @@ public class BeanDescriptorManager implements BeanDescriptorMap { * enhanced or all dynamically subclassed). */ private void checkInheritedClasses(Class beanClass) { + Class superclass = beanClass.getSuperclass(); if (Object.class.equals(superclass)) { // we got to the top of the inheritance @@ -1418,14 +1426,40 @@ public class BeanDescriptorManager implements BeanDescriptorMap { return; } if (!EntityBean.class.isAssignableFrom(superclass)) { + if (isMappedSuperWithNoProperties(superclass)) { + // ok to stop and treat just the same as Object.class + return; + } throw new IllegalStateException("Super type "+superclass+" is not enhanced?"); } - // recursively continue up the inheritance hierarchy checkInheritedClasses(superclass); } + /** + * Return true if this is a MappedSuperclass bean with no persistent properties. + * If so it is ok for it not to be enhanced. + */ + private boolean isMappedSuperWithNoProperties(Class beanClass) { + + MappedSuperclass annotation = beanClass.getAnnotation(MappedSuperclass.class); + if (annotation == null) { + return false; + } + Field[] fields = beanClass.getDeclaredFields(); + for (Field field : fields) { + if (Modifier.isStatic(field.getModifiers()) || Modifier.isTransient(field.getModifiers())) { + // ignore this field + } else if (field.isAnnotationPresent(Transient.class)) { + // ignore this field + } else { + return false; + } + } + return true; + } + /** * Comparator to sort the BeanDescriptors by name. */ diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/meta/DeployBeanDescriptor.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/meta/DeployBeanDescriptor.java index 16041a089..88856064b 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/meta/DeployBeanDescriptor.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/meta/DeployBeanDescriptor.java @@ -707,10 +707,10 @@ public class DeployBeanDescriptor { } /** - * Return an Iterator of all BeanProperty. + * Return a collection of all BeanProperty deployment information. */ - public Iterator propertiesAll() { - return propMap.values().iterator(); + public Collection propertiesAll() { + return propMap.values(); } /** diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/meta/DeployBeanPropertyLists.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/meta/DeployBeanPropertyLists.java index 95d0e054f..22ec56c41 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/meta/DeployBeanPropertyLists.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/meta/DeployBeanPropertyLists.java @@ -1,7 +1,6 @@ package com.avaje.ebeaninternal.server.deploy.meta; import java.util.ArrayList; -import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; @@ -76,18 +75,13 @@ public class DeployBeanPropertyLists { this.propertyMap = new LinkedHashMap(); - Iterator deployIt = deploy.propertiesAll(); - while (deployIt.hasNext()) { - DeployBeanProperty deployProp = deployIt.next(); - BeanProperty beanProp = createBeanProperty(owner, deployProp); + for (DeployBeanProperty prop : deploy.propertiesAll()) { + BeanProperty beanProp = createBeanProperty(owner, prop); propertyMap.put(beanProp.getName(), beanProp); } - Iterator it = propertyMap.values().iterator(); - int order = 0; - while (it.hasNext()) { - BeanProperty prop = it.next(); + for (BeanProperty prop : propertyMap.values()) { prop.setDeployOrder(order++); allocateToList(prop); } diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/parse/AnnotationAssocManys.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/parse/AnnotationAssocManys.java index d3aa8d830..891b69098 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/parse/AnnotationAssocManys.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/parse/AnnotationAssocManys.java @@ -1,7 +1,5 @@ package com.avaje.ebeaninternal.server.deploy.parse; -import java.util.Iterator; - import javax.persistence.JoinColumn; import javax.persistence.JoinColumns; import javax.persistence.JoinTable; @@ -44,9 +42,7 @@ public class AnnotationAssocManys extends AnnotationParser { * Parse the annotations. */ public void parse() { - Iterator it = descriptor.propertiesAll(); - while (it.hasNext()) { - DeployBeanProperty prop = it.next(); + for (DeployBeanProperty prop : descriptor.propertiesAll()) { if (prop instanceof DeployBeanPropertyAssocMany) { read((DeployBeanPropertyAssocMany) prop); } diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/parse/AnnotationAssocOnes.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/parse/AnnotationAssocOnes.java index 36276c08b..06be5c30b 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/parse/AnnotationAssocOnes.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/parse/AnnotationAssocOnes.java @@ -1,6 +1,5 @@ package com.avaje.ebeaninternal.server.deploy.parse; -import java.util.Iterator; import java.util.Map; import javax.persistence.Column; @@ -44,9 +43,7 @@ public class AnnotationAssocOnes extends AnnotationParser { */ public void parse() { - Iterator it = descriptor.propertiesAll(); - while (it.hasNext()) { - DeployBeanProperty prop = it.next(); + for (DeployBeanProperty prop : descriptor.propertiesAll()) { if (prop instanceof DeployBeanPropertyAssocOne) { readAssocOne((DeployBeanPropertyAssocOne) prop); } diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/parse/AnnotationFields.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/parse/AnnotationFields.java index 9da0b4f3d..6aa33235b 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/parse/AnnotationFields.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/parse/AnnotationFields.java @@ -1,7 +1,6 @@ package com.avaje.ebeaninternal.server.deploy.parse; import java.sql.Types; -import java.util.Iterator; import java.util.Map; import java.util.UUID; @@ -74,15 +73,12 @@ public class AnnotationFields extends AnnotationParser { */ public void parse() { - Iterator it = descriptor.propertiesAll(); - while (it.hasNext()) { - DeployBeanProperty prop = it.next(); + for (DeployBeanProperty prop : descriptor.propertiesAll()) { if (prop instanceof DeployBeanPropertyAssoc) { readAssocOne(prop); } else { readField(prop); } - } } diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/parse/DeployCreateProperties.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/parse/DeployCreateProperties.java index 7f66f50cc..c2e242334 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/parse/DeployCreateProperties.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/parse/DeployCreateProperties.java @@ -5,11 +5,13 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; -import java.util.Iterator; import javax.persistence.PersistenceException; import javax.persistence.Transient; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.avaje.ebean.annotation.ColumnHstore; import com.avaje.ebeaninternal.server.core.Message; import com.avaje.ebeaninternal.server.deploy.DetermineManyType; @@ -26,9 +28,6 @@ import com.avaje.ebeaninternal.server.type.ScalarTypePostgresHstore; import com.avaje.ebeaninternal.server.type.TypeManager; import com.avaje.ebeaninternal.server.type.reflect.CheckImmutableResponse; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - /** * Create the properties for a bean. *

@@ -58,10 +57,7 @@ public class DeployCreateProperties { desc.sortProperties(); // check the transient properties... - Iterator it = desc.propertiesAll(); - - while (it.hasNext()) { - DeployBeanProperty prop = it.next(); + for (DeployBeanProperty prop : desc.propertiesAll()) { if (prop.isTransient()) { if (prop.getWriteMethod() == null || prop.getReadMethod() == null) { // Typically a helper method ... this is expected diff --git a/src/test/java/com/avaje/ebeaninternal/server/deploy/TestNotEnhancedMappedSuper.java b/src/test/java/com/avaje/ebeaninternal/server/deploy/TestNotEnhancedMappedSuper.java new file mode 100644 index 000000000..731883cd0 --- /dev/null +++ b/src/test/java/com/avaje/ebeaninternal/server/deploy/TestNotEnhancedMappedSuper.java @@ -0,0 +1,29 @@ +package com.avaje.ebeaninternal.server.deploy; + + +import org.junit.Assert; +import org.junit.Test; + +import com.avaje.ebean.BaseTestCase; +import com.avaje.ebean.Ebean; +import com.avaje.tests.model.basic.ResetBasicData; +import com.avaje.tests.model.mappedsuper.ASimpleBean; + +public class TestNotEnhancedMappedSuper extends BaseTestCase { + + @Test + public void simpleBean_mappedSuperNotEnhanced_ok() { + +// //GlobalProperties.put("ebean.search.packages", "com.avaje.tests.model.mappedsuper"); + + ResetBasicData.reset(); + + ASimpleBean bean = new ASimpleBean(); + bean.setName("junk"); + + Ebean.save(bean); + + Assert.assertNotNull(bean.getId()); + } + +} diff --git a/src/test/java/com/avaje/tests/model/mappedsuper/ASimpleBean.java b/src/test/java/com/avaje/tests/model/mappedsuper/ASimpleBean.java new file mode 100644 index 000000000..1123034fa --- /dev/null +++ b/src/test/java/com/avaje/tests/model/mappedsuper/ASimpleBean.java @@ -0,0 +1,30 @@ +package com.avaje.tests.model.mappedsuper; + +import javax.persistence.Entity; +import javax.persistence.Id; + +@Entity +public class ASimpleBean extends NotEnhancedMappedSuper { + + @Id + Long id; + + String name; + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + +} diff --git a/src/test/java/com/avaje/tests/model/mappedsuper/NotEnhancedMappedSuper.java b/src/test/java/com/avaje/tests/model/mappedsuper/NotEnhancedMappedSuper.java new file mode 100644 index 000000000..2f9405f3b --- /dev/null +++ b/src/test/java/com/avaje/tests/model/mappedsuper/NotEnhancedMappedSuper.java @@ -0,0 +1,32 @@ +package com.avaje.tests.model.mappedsuper; + +import javax.persistence.MappedSuperclass; +import javax.persistence.Transient; + +@MappedSuperclass +public abstract class NotEnhancedMappedSuper { + + public static String SOMETHING = "Hello"; + + private transient Long one; + + @Transient + private Long two; + + public Long getOne() { + return one; + } + + public void setOne(Long one) { + this.one = one; + } + + public Long getTwo() { + return two; + } + + public void setTwo(Long two) { + this.two = two; + } + +}