diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCEnhancer.java b/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCEnhancer.java index 090776747..f5278aff7 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCEnhancer.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCEnhancer.java @@ -3261,15 +3261,14 @@ public class PCEnhancer { ifins.setTarget(code.nop()); // if (pcVersionInit != false) // return true - // else return false; + // else return null; // (returning null because we don't know the correct answer) loadManagedInstance(code, false); getfield(code, null, VERSION_INIT_STR); ifins = code.ifeq(); code.getstatic().setField(Boolean.class, "TRUE", Boolean.class); code.areturn(); ifins.setTarget(code.nop()); - code.getstatic().setField(Boolean.class, "FALSE", Boolean.class); - + code.constant().setNull(); } code.areturn(); return false; diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/VersionAttachStrategy.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/VersionAttachStrategy.java index 61b6046e4..31403f501 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/VersionAttachStrategy.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/VersionAttachStrategy.java @@ -18,12 +18,15 @@ */ package org.apache.openjpa.kernel; +import java.lang.reflect.Field; import java.util.Collection; import java.util.Iterator; import java.util.Map; import org.apache.openjpa.enhance.PersistenceCapable; +import org.apache.openjpa.enhance.Reflection; import org.apache.openjpa.enhance.StateManager; +import org.apache.openjpa.event.LifecycleEvent; import org.apache.openjpa.lib.util.Localizer; import org.apache.openjpa.meta.ClassMetaData; import org.apache.openjpa.meta.FieldMetaData; @@ -31,10 +34,9 @@ import org.apache.openjpa.meta.JavaTypes; import org.apache.openjpa.meta.ValueMetaData; import org.apache.openjpa.meta.ValueStrategies; import org.apache.openjpa.util.ApplicationIds; +import org.apache.openjpa.util.ImplHelper; import org.apache.openjpa.util.ObjectNotFoundException; import org.apache.openjpa.util.OptimisticException; -import org.apache.openjpa.util.ImplHelper; -import org.apache.openjpa.event.LifecycleEvent; /** * Handles attaching instances using version and primary key fields. @@ -175,9 +177,29 @@ class VersionAttachStrategy */ private void compareVersion(StateManagerImpl sm, PersistenceCapable pc) { Object version = pc.pcGetVersion(); - if (version == null) + // In the event that the version field is a primitive and it is the types default value, we can't differentiate + // between a value that was set to be the default, and one that defaulted to that value. + if (version != null + && JavaTypes.isPrimitiveDefault(version, sm.getMetaData().getVersionField().getTypeCode())) { + Field pcVersionInitField = null; + try { + pcVersionInitField = pc.getClass().getDeclaredField("pcVersionInit"); + Object pcField = Reflection.get(pc, pcVersionInitField); + if (pcField != null) { + boolean bool = (Boolean) pcField; + if (bool == false) { + // If this field if false, that means that the pcGetVersion returned a default value rather than + // and actual value. + version = null; + } + } + } catch (Exception e) { + // Perhaps this is an Entity that was enhanced before the pcVersionInit field was added. + } + } + if (version == null) { return; - + } // don't need to load unloaded fields since its implicitly // a single field value StoreManager store = sm.getBroker().getStoreManager(); diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/meta/JavaTypes.java b/openjpa-kernel/src/main/java/org/apache/openjpa/meta/JavaTypes.java index 36dce4713..a74dec15e 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/meta/JavaTypes.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/meta/JavaTypes.java @@ -435,4 +435,31 @@ public class JavaTypes { Array.set(array, idx, itr.next ()); return array; } + + /** + * Determine whether or not the provided Object value is the default for the provided typeCode. + * + * For example: If o = Integer(0) and typeCode = JavaTypes.INT, this method will return true. + */ + public static boolean isPrimitiveDefault(Object o, int typeCode) { + switch (typeCode) { + case BOOLEAN: + return ((Boolean) o).equals(Boolean.FALSE) ? true : false; + case BYTE: + return ((Byte) o) == 0 ? true : false; + case SHORT: + return ((Short) o) == 0 ? true : false; + case INT: + return ((Integer) o) == 0 ? true : false; + case LONG: + return ((Long) o) == 0L ? true : false; + case FLOAT: + return ((Float) o) == 0.0F ? true : false; + case CHAR: + return ((Character) o) == '\u0000' ? true : false; + case DOUBLE: + return ((Double) o) == 0.0d ? true : false; + } + return false; + } } diff --git a/openjpa-kernel/src/test/java/org/apache/openjpa/meta/TestJavaTypes.java b/openjpa-kernel/src/test/java/org/apache/openjpa/meta/TestJavaTypes.java new file mode 100644 index 000000000..c9c42baf0 --- /dev/null +++ b/openjpa-kernel/src/test/java/org/apache/openjpa/meta/TestJavaTypes.java @@ -0,0 +1,68 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.openjpa.meta; + +import junit.framework.TestCase; + +public class TestJavaTypes extends TestCase { + TypesHolder _types = new TypesHolder(); + + public void testIsPrimitiveDefault() { + assertTrue(JavaTypes.isPrimitiveDefault(_types.getBoolean(), JavaTypes.BOOLEAN)); + assertTrue(JavaTypes.isPrimitiveDefault(_types.getChar(), JavaTypes.CHAR)); + assertTrue(JavaTypes.isPrimitiveDefault(_types.getDouble(), JavaTypes.DOUBLE)); + assertTrue(JavaTypes.isPrimitiveDefault(_types.getInt(), JavaTypes.INT)); + assertTrue(JavaTypes.isPrimitiveDefault(_types.getLong(), JavaTypes.LONG)); + assertTrue(JavaTypes.isPrimitiveDefault(_types.getShort(), JavaTypes.SHORT)); + } + + class TypesHolder { + boolean _boolean; + short _short; + int _int; + long _long; + float _float; + double _double; + char _char; + + public Object getBoolean() { + return _boolean; + } + + public Object getShort() { + return _short; + } + + public Object getInt() { + return _int; + } + + public Object getLong() { + return _long; + } + + public Object getDouble() { + return _double; + } + + public Object getChar() { + return _char; + } + } +} diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/detach/IntVersionEntity.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/detach/IntVersionEntity.java index 0b73dafd7..9b1957cd6 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/detach/IntVersionEntity.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/detach/IntVersionEntity.java @@ -18,6 +18,8 @@ */ package org.apache.openjpa.persistence.detach; +import java.io.Serializable; + import javax.persistence.CascadeType; import javax.persistence.Entity; import javax.persistence.FetchType; @@ -26,7 +28,7 @@ import javax.persistence.OneToOne; import javax.persistence.Version; @Entity -public class IntVersionEntity { +public class IntVersionEntity implements Serializable { @Id private int id; diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/detach/IntegerVersionEntity.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/detach/IntegerVersionEntity.java new file mode 100644 index 000000000..ab98e9447 --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/detach/IntegerVersionEntity.java @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.openjpa.persistence.detach; + +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.Version; + +@Entity +public class IntegerVersionEntity { + + @Id + private int id; + private String name; + + @Version + private Integer version; + + public int getVersion() { + return version; + } + + public IntegerVersionEntity(int id) { + this.id = id; + } + + public int getId() { + return id; + } + + public void setId(int id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } +} diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/detach/TestMergeNoStateManager.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/detach/TestMergeNoStateManager.java new file mode 100644 index 000000000..ba6cd9764 --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/detach/TestMergeNoStateManager.java @@ -0,0 +1,129 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.openjpa.persistence.detach; + +import javax.persistence.OptimisticLockException; + +import org.apache.openjpa.persistence.OpenJPAEntityManagerSPI; +import org.apache.openjpa.persistence.test.SingleEMFTestCase; + +/** + * Added for OPENJPA-1896 + */ +public class TestMergeNoStateManager extends SingleEMFTestCase { + Object[] args = + new Object[] { TimestampVersionEntity.class, IntVersionEntity.class, NoVersionEntity.class, + IntegerVersionEntity.class, CLEAR_TABLES, "openjpa.Log", "SQL=trace" }; + + IntVersionEntity _ive; + NoVersionEntity _nve; + IntegerVersionEntity _integerVe; + + @Override + public void setUp() throws Exception { + super.setUp(args); + OpenJPAEntityManagerSPI em = emf.createEntityManager(); + try { + if (em.find(IntVersionEntity.class, 1) == null) { + em.getTransaction().begin(); + _ive = new IntVersionEntity(1); + _nve = new NoVersionEntity(1); + _integerVe = new IntegerVersionEntity(1); + + em.persist(_ive); + em.persist(_nve); + em.persist(_integerVe); + + em.getTransaction().commit(); + } + } finally { + em.close(); + } + } + + /** + * This test is commented out is it will fail. + */ +// public void testOLE() throws Exception { +// OpenJPAEntityManagerSPI em = emf.createEntityManager(); +// try { +// String updatedName = "updatedName_" + System.currentTimeMillis(); +// IntVersionEntity ive = em.find(IntVersionEntity.class, _ive.getId()); +// em.clear(); +// +// IntVersionEntity detachedIve = new IntVersionEntity(_ive.getId()); +// // Set the version to older than currently in the db to simulate having stale data +// detachedIve.setId(0); +// detachedIve.setName(updatedName); +// // serialize +// detachedIve = roundtrip(detachedIve); +// +// em.getTransaction().begin(); +// // This merge should throw an OLE since we have older version than current +// try { +// em.merge(detachedIve); +// throw new RuntimeException("Expected an OLE, but didn't get one!"); +// } catch (OptimisticLockException ole) { +// // expected +// } +// } finally { +// if (em.getTransaction().isActive()) { +// em.getTransaction().rollback(); +// } +// em.close(); +// } +// } + + public void test() throws Exception { + OpenJPAEntityManagerSPI em = emf.createEntityManager(); + try { + String updatedName = "updatedName_" + System.currentTimeMillis(); + IntVersionEntity detachedIve = new IntVersionEntity(_ive.getId()); + NoVersionEntity detachedNve = new NoVersionEntity(_nve.getId()); + IntegerVersionEntity detachedIntegerVe = new IntegerVersionEntity(_integerVe.getId()); + + detachedIntegerVe.setName(updatedName); + detachedNve.setName(updatedName); + detachedIve.setName(updatedName); + + em.getTransaction().begin(); + em.merge(detachedIntegerVe); + em.merge(detachedNve); + em.merge(detachedIve); + em.getTransaction().commit(); + + em.clear(); + + detachedIntegerVe = em.find(IntegerVersionEntity.class, _integerVe.getId()); + detachedNve = em.find(NoVersionEntity.class, _nve.getId()); + detachedIve = em.find(IntVersionEntity.class, _ive.getId()); + + // Make sure the updated values were persisted + assertEquals(detachedIntegerVe.getName(), updatedName); + assertEquals(detachedNve.getName(), updatedName); + assertEquals(detachedIve.getName(), updatedName); + + } finally { + if (em.getTransaction().isActive()) { + em.getTransaction().commit(); + } + em.close(); + } + } +}