From 0b90c67a8bb2932037c4311e8afb7d85a6c3902e Mon Sep 17 00:00:00 2001 From: Patrick Linskey Date: Thu, 8 Mar 2007 09:34:52 +0000 Subject: [PATCH] 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 --- .../apache/openjpa/kernel/AbstractPCData.java | 67 ++++++++++--- .../datacache/TestArrayFieldsInDataCache.java | 99 +++++++++++++++++++ .../persistence/simple/AllFieldTypes.java | 13 +++ .../persistence/test/SingleEMTest.java | 10 ++ .../persistence/PersistentCollection.java | 4 +- 5 files changed, 176 insertions(+), 17 deletions(-) create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestArrayFieldsInDataCache.java diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractPCData.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractPCData.java index bec62db18..a6a50d5ef 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractPCData.java +++ b/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()); - for (int i = 0; i < l.size(); i++) { - Array.set(a, i, toNestedField(sm, fmd.getElement(), - l.get(i), fetch, context)); + length); + if (isImmutableType(fmd.getElement())) { + System.arraycopy(data, 0, a, 0, length); + } 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; - List l = null; - for (int i = 0; i < length; i++) { - val = toNestedData(fmd.getElement(), Array.get(a, i), ctx); - if (val == NULL) - return NULL; - if (l == null) - l = new ArrayList(length); - l.add(val); + return EMPTY_ARRAY; + + Object dataArray = Array.newInstance( + fmd.getElement().getDeclaredType(), length); + if (isImmutableType(fmd.getElement())) { + System.arraycopy(a, 0, dataArray, 0, length); + } else { + for (int i = 0; i < length; i++) { + 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. diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestArrayFieldsInDataCache.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/datacache/TestArrayFieldsInDataCache.java new file mode 100644 index 000000000..99aa7652d --- /dev/null +++ b/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(); + } +} \ No newline at end of file diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/simple/AllFieldTypes.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/simple/AllFieldTypes.java index 2e7f14218..70fc7bfbc 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/simple/AllFieldTypes.java +++ b/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 setOfStrings = new HashSet(); 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; + } } diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SingleEMTest.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SingleEMTest.java index 1ad126c2f..30c0dce7d 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/test/SingleEMTest.java +++ b/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 { diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PersistentCollection.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PersistentCollection.java index 0d3fbf1b0..07f064ef4 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PersistentCollection.java +++ b/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