OPENJPA-71: resolved inefficiency with array types and AbstractPCData

git-svn-id: https://svn.apache.org/repos/asf/incubator/openjpa/trunk@515987 13f79535-47bb-0310-9956-ffa450edef68

openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractPCData.java

@@ -23,7 +23,6 @@ import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.Iterator;
-import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 
@@ -45,6 +44,7 @@ public abstract class AbstractPCData
     implements PCData {
 
     public static final Object NULL = new Object();
+    private static final Object[] EMPTY_ARRAY = new Object[0];
 
     /**
      * Return the loaded field mask.
@@ -97,12 +97,16 @@ public abstract class AbstractPCData
                 }
                 return m2;
             case JavaTypes.ARRAY:
-                List l = (List) data;
+                int length = Array.getLength(data);
                 Object a = Array.newInstance(fmd.getElement().getDeclaredType(),
-                    l.size());
+                    length);
-                for (int i = 0; i < l.size(); i++) {
+                if (isImmutableType(fmd.getElement())) {
-                    Array.set(a, i, toNestedField(sm, fmd.getElement(),
+                    System.arraycopy(data, 0, a, 0, length);
-                        l.get(i), fetch, context));
+                } else {
+                    for (int i = 0; i < length; i++) {
+                        Array.set(a, i, toNestedField(sm, fmd.getElement(),
+                            Array.get(data, i), fetch, context));
+                    }
                 }
                 return a;
             default:
@@ -220,22 +224,53 @@ public abstract class AbstractPCData
                 Object a = val;
                 int length = Array.getLength(a);
                 if (length == 0)
-                    return Collections.EMPTY_LIST;
+                    return EMPTY_ARRAY;
-                List l = null;
+
-                for (int i = 0; i < length; i++) {
+                Object dataArray = Array.newInstance(
-                    val = toNestedData(fmd.getElement(), Array.get(a, i), ctx);
+                    fmd.getElement().getDeclaredType(), length);
-                    if (val == NULL)
+                if (isImmutableType(fmd.getElement())) {
-                        return NULL;
+                    System.arraycopy(a, 0, dataArray, 0, length);
-                    if (l == null)
+                } else {
-                        l = new ArrayList(length);
+                    for (int i = 0; i < length; i++) {
-                    l.add(val);
+                        val = toNestedData(fmd.getElement(), Array.get(a, i),
+                            ctx);
+                        Array.set(dataArray, i, val);
+                    }
                 }
-                return l;
+                return dataArray;
             default:
                 return toNestedData(fmd, val, ctx);
         }
     }
 
+    private boolean isImmutableType(ValueMetaData element) {
+        switch (element.getDeclaredTypeCode()) {
+            case JavaTypes.BOOLEAN:
+            case JavaTypes.BYTE:
+            case JavaTypes.CHAR:
+            case JavaTypes.DOUBLE:
+            case JavaTypes.FLOAT:
+            case JavaTypes.INT:
+            case JavaTypes.LONG:
+            case JavaTypes.SHORT:
+            case JavaTypes.STRING:
+            case JavaTypes.NUMBER:
+            case JavaTypes.BOOLEAN_OBJ:
+            case JavaTypes.BYTE_OBJ:
+            case JavaTypes.CHAR_OBJ:
+            case JavaTypes.DOUBLE_OBJ:
+            case JavaTypes.FLOAT_OBJ:
+            case JavaTypes.INT_OBJ:
+            case JavaTypes.LONG_OBJ:
+            case JavaTypes.SHORT_OBJ:
+            case JavaTypes.BIGDECIMAL:
+            case JavaTypes.BIGINTEGER:
+                return true;
+            default:
+                return false;
+        }
+    }
+
     /**
      * Transform the given nested value to a cachable value. Return
      * {@link #NULL} if the value cannot be cached.

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestArrayFieldsInDataCache.java

@@ -0,0 +1,99 @@
+package org.apache.openjpa.persistence.datacache;
+
+import java.util.Map;
+import java.util.Arrays;
+import javax.persistence.EntityManager;
+
+import org.apache.openjpa.persistence.test.SingleEMTest;
+import org.apache.openjpa.persistence.simple.AllFieldTypes;
+import org.apache.openjpa.persistence.OpenJPAPersistence;
+import org.apache.openjpa.datacache.DataCache;
+import org.apache.openjpa.kernel.PCData;
+import org.apache.openjpa.meta.ClassMetaData;
+
+public class TestArrayFieldsInDataCache
+    extends SingleEMTest {
+
+    private static final String[] STRINGS = new String[]{ "a", "b", "c" };
+    private static final int[] INTS = new int[]{ 1, 2, 3 };
+
+    private Object jpaOid;
+    private Object internalOid;
+
+    public TestArrayFieldsInDataCache() {
+        super(AllFieldTypes.class);
+    }
+
+    @Override
+    protected void setEMFProps(Map props) {
+        super.setEMFProps(props);
+        props.put("openjpa.DataCache", "true");
+        props.put("openjpa.RemoteCommitProvider", "sjvm");
+    }
+
+    @Override
+    protected boolean clearDatabaseInSetUp() {
+        return true;
+    }
+
+    @Override
+    public void setUp() throws Exception {
+        super.setUp();
+
+        EntityManager em = emf.createEntityManager();
+        em.getTransaction().begin();
+        AllFieldTypes aft = new AllFieldTypes();
+        aft.setArrayOfStrings(STRINGS);
+        aft.setArrayOfInts(INTS);
+        em.persist(aft);
+        em.getTransaction().commit();
+
+        // get the external and internal forms of the ID for cache
+        // interrogation and data validation
+        jpaOid = OpenJPAPersistence.cast(em).getObjectId(aft);
+        internalOid = OpenJPAPersistence.toBroker(em).getObjectId(aft);
+
+        em.close();
+    }
+
+    public void testArrayOfStrings() {
+        // check that the data cache contains an efficient representation
+        DataCache cache = OpenJPAPersistence.cast(emf).getStoreCache()
+            .getDelegate();
+        PCData data = cache.get(internalOid);
+        ClassMetaData meta = OpenJPAPersistence.getMetaData(emf,
+            AllFieldTypes.class);
+        Object cachedFieldData =
+            data.getData(meta.getField("arrayOfStrings").getIndex());
+        assertTrue(cachedFieldData.getClass().isArray());
+        assertEquals(String.class,
+            cachedFieldData.getClass().getComponentType());
+
+        // make sure that the returned results are correct
+        em = emf.createEntityManager();
+        AllFieldTypes aft = em.find(AllFieldTypes.class, jpaOid);
+        assertTrue(Arrays.equals(STRINGS, aft.getArrayOfStrings()));
+        assertNotSame(STRINGS, aft.getArrayOfStrings());
+        em.close();
+    }
+
+    public void testArrayOfInts() {
+        // check that the data cache contains an efficient representation
+        DataCache cache = OpenJPAPersistence.cast(emf).getStoreCache()
+            .getDelegate();
+        PCData data = cache.get(internalOid);
+        ClassMetaData meta = OpenJPAPersistence.getMetaData(emf,
+            AllFieldTypes.class);
+        Object cachedFieldData =
+            data.getData(meta.getField("arrayOfInts").getIndex());
+        assertTrue(cachedFieldData.getClass().isArray());
+        assertEquals(int.class, cachedFieldData.getClass().getComponentType());
+
+        // make sure that the returned results are correct
+        em = emf.createEntityManager();
+        AllFieldTypes aft = em.find(AllFieldTypes.class, jpaOid);
+        assertTrue(Arrays.equals(INTS, aft.getArrayOfInts()));
+        assertNotSame(INTS, aft.getArrayOfInts());
+        em.close();
+    }
+}

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/simple/AllFieldTypes.java

@@ -20,6 +20,8 @@ import java.util.HashSet;
 import java.util.Set;
 import javax.persistence.Entity;
 
+import org.apache.openjpa.persistence.PersistentCollection;
+
 @Entity
 public class AllFieldTypes {
 
@@ -36,6 +38,9 @@ public class AllFieldTypes {
     private Set<String> setOfStrings = new HashSet<String>();
     private String[] arrayOfStrings;
 
+    @PersistentCollection
+    private int[] arrayOfInts;
+
     public void setShortField(short shortField) {
         this.shortField = shortField;
     }
@@ -131,5 +136,13 @@ public class AllFieldTypes {
     public String[] getArrayOfStrings() {
         return this.arrayOfStrings;
     }
+
+    public void setArrayOfInts(int[] arrayOfInts) {
+        this.arrayOfInts = arrayOfInts;
+    }
+
+    public int[] getArrayOfInts() {
+        return arrayOfInts;
+    }
 }
 

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SingleEMTest.java

@@ -65,6 +65,16 @@ public abstract class SingleEMTest extends TestCase {
 
             props.put("openjpa.MetaDataFactory", "jpa(Types=" + str + ")");
         }
+
+        if (clearDatabaseInSetUp()) {
+            props.put("openjpa.jdbc.SynchronizeMappings",
+                "buildSchema(ForeignKeys=true," +
+                    "SchemaAction='add,deleteTableContents')");
+        }
+    }
+
+    protected boolean clearDatabaseInSetUp() {
+        return false;
     }
 
     public void setUp() throws Exception {

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PersistentCollection.java

@@ -24,7 +24,9 @@ import javax.persistence.CascadeType;
 import javax.persistence.FetchType;
 
 /**
- * Metadata annotation for a persistent collection field.
+ * Metadata annotation for a persistent collection field. This should be
+ * used to annotate array field types as well as fields of type
+ * {@link java.util.Collection}.
  *
  * @author Abe White
  * @since 0.4.0