Fix for #173 - Ability to use a MappedSuperClass without being enhanced as long as it doesn't have persistent fields

pom.xml

@@ -94,7 +94,7 @@
     <dependency>
       <groupId>org.avaje.ebeanorm</groupId>
       <artifactId>avaje-ebeanorm-agent</artifactId>
-      <version>4.1.2</version>
+      <version>4.1.5</version>
       <scope>test</scope>
     </dependency>
         

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<DeployBeanProperty> 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<DeployBeanProperty> 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.
    */

src/main/java/com/avaje/ebeaninternal/server/deploy/meta/DeployBeanDescriptor.java

@@ -707,10 +707,10 @@ public class DeployBeanDescriptor<T> {
   }
 
   /**
-   * Return an Iterator of all BeanProperty.
+   * Return a collection of all BeanProperty deployment information.
    */
-  public Iterator<DeployBeanProperty> propertiesAll() {
-    return propMap.values().iterator();
+  public Collection<DeployBeanProperty> propertiesAll() {
+    return propMap.values();
   }
 
   /**

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<String, BeanProperty>();
 
-    Iterator<DeployBeanProperty> 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<BeanProperty> it = propertyMap.values().iterator();
-
     int order = 0;
-    while (it.hasNext()) {
-      BeanProperty prop = it.next();
+    for (BeanProperty prop : propertyMap.values()) {
       prop.setDeployOrder(order++);
       allocateToList(prop);
     }

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<DeployBeanProperty> it = descriptor.propertiesAll();
-		while (it.hasNext()) {
-			DeployBeanProperty prop = it.next();
+		for (DeployBeanProperty prop : descriptor.propertiesAll()) {
 			if (prop instanceof DeployBeanPropertyAssocMany<?>) {
 				read((DeployBeanPropertyAssocMany<?>) prop);
 			}

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<DeployBeanProperty> it = descriptor.propertiesAll();
-        while (it.hasNext()) {
-            DeployBeanProperty prop = it.next();
+      for (DeployBeanProperty prop : descriptor.propertiesAll()) {
             if (prop instanceof DeployBeanPropertyAssocOne<?>) {
                 readAssocOne((DeployBeanPropertyAssocOne<?>) prop);
             }

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<DeployBeanProperty> it = descriptor.propertiesAll();
-    while (it.hasNext()) {
-      DeployBeanProperty prop = it.next();
+    for (DeployBeanProperty prop : descriptor.propertiesAll()) {
       if (prop instanceof DeployBeanPropertyAssoc<?>) {
         readAssocOne(prop);
       } else {
         readField(prop);
       }
-
     }
   }
 

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.
  * <p>
@@ -58,10 +57,7 @@ public class DeployCreateProperties {
     desc.sortProperties();
 
     // check the transient properties...
-    Iterator<DeployBeanProperty> 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

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());
+  }
+  
+}

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;
+  }
+  
+}

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;
+  }
+  
+}