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 52b4572a1..5b056f928 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
@@ -118,7 +118,10 @@ public class PCEnhancer {
public static final int ENHANCE_INTERFACE = 2 << 1;
public static final int ENHANCE_PC = 2 << 2;
- private static final String PRE = "pc";
+ public static final String PRE = "pc";
+ public static final String ISDETACHEDSTATEDEFINITIVE = PRE
+ + "isDetachedStateDefinitive";
+
private static final Class PCTYPE = PersistenceCapable.class;
private static final String SM = PRE + "StateManager";
private static final Class SMTYPE = StateManager.class;
@@ -2999,12 +3002,27 @@ public class PCEnhancer {
*/
private void addIsDetachedMethod()
throws NoSuchMethodException {
- // public boolean pcIsDetached ()
+ // public boolean pcIsDetached()
BCMethod method = _pc.declareMethod(PRE + "IsDetached",
Boolean.class, null);
method.makePublic();
Code code = method.getCode(true);
- writeIsDetachedMethod(code);
+ boolean needsDefinitiveMethod = writeIsDetachedMethod(code);
+ code.calculateMaxStack();
+ code.calculateMaxLocals();
+ if (!needsDefinitiveMethod)
+ return;
+
+ // private boolean pcIsDetachedStateDefinitive()
+ // return false;
+ // auxilliary enhancers may change the return value of this method
+ // if their specs consider detached state definitive
+ method = _pc.declareMethod(ISDETACHEDSTATEDEFINITIVE, boolean.class,
+ null);
+ method.makePrivate();
+ code = method.getCode(true);
+ code.constant().setValue(false);
+ code.ireturn();
code.calculateMaxStack();
code.calculateMaxLocals();
}
@@ -3012,14 +3030,17 @@ public class PCEnhancer {
/**
* Creates the body of the pcIsDetached() method to determine if an
* instance is detached.
+ *
+ * @return true if we need a pcIsDetachedStateDefinitive method, false
+ * otherwise
*/
- private void writeIsDetachedMethod(Code code)
+ private boolean writeIsDetachedMethod(Code code)
throws NoSuchMethodException {
// not detachable: return Boolean.FALSE
if (!_meta.isDetachable()) {
code.getstatic().setField(Boolean.class, "FALSE", Boolean.class);
code.areturn();
- return;
+ return false;
}
// if (sm != null)
@@ -3067,7 +3088,7 @@ public class PCEnhancer {
ifins.setTarget(target);
notdeser.setTarget(target);
code.areturn();
- return;
+ return false;
}
}
@@ -3077,9 +3098,9 @@ public class PCEnhancer {
if (notdeser != null)
notdeser.setTarget(target);
- // allow users with version fields to manually construct a "detached"
- // instance, so check version before taking into account non-existent
- // detached state
+ // allow users with version or auto-assigned pk fields to manually
+ // construct a "detached" instance, so check these before taking into
+ // account non-existent detached state
// consider detached if version is non-default
FieldMetaData version = _meta.getVersionField();
@@ -3094,41 +3115,13 @@ public class PCEnhancer {
ifins.setTarget(code.getstatic().setField(Boolean.class, "FALSE",
Boolean.class));
code.areturn();
- return;
- }
-
- // no detached state: if instance uses detached state and it's not
- // synthetic or the instance is not serializable or the state isn't
- // transient, must not be detached
- if (state == null
- && (!ClassMetaData.SYNTHETIC.equals(_meta.getDetachedState())
- || !Serializable.class.isAssignableFrom(_meta.getDescribedType())
- || !_repos.getConfiguration().getDetachStateInstance().
- isDetachedStateTransient())) {
- // return Boolean.FALSE
- code.getstatic().setField(Boolean.class, "FALSE", Boolean.class);
- code.areturn();
- return;
- }
-
- // no detached state: if instance uses detached state (and must be
- // synthetic and transient in serializable instance at this point),
- // not detached if state not set to DESERIALIZED
- if (state == null) {
- // if (pcGetDetachedState () == null) // instead of DESERIALIZED
- // return Boolean.FALSE;
- loadManagedInstance(code, false);
- code.invokevirtual().setMethod(PRE + "GetDetachedState",
- Object.class, null);
- ifins = code.ifnonnull();
- code.getstatic().setField(Boolean.class, "FALSE", Boolean.class);
- code.areturn();
- ifins.setTarget(code.nop());
+ return false;
}
// consider detached if auto-genned primary keys are non-default
ifins = null;
JumpInstruction ifins2 = null;
+ boolean hasAutoAssignedPK = false;
if (state != Boolean.TRUE
&& _meta.getIdentityType() == ClassMetaData.ID_APPLICATION) {
// for each pk field:
@@ -3162,13 +3155,65 @@ public class PCEnhancer {
}
}
- // give up; we just don't know
- target = code.constant().setNull();
+ // create artificial target to simplify
+ target = code.nop();
if (ifins != null)
ifins.setTarget(target);
if (ifins2 != null)
ifins2.setTarget(target);
+
+ // if has auto-assigned pk and we get to this point, must have default
+ // value, so must be new instance
+ if (hasAutoAssignedPK) {
+ code.getstatic().setField(Boolean.class, "FALSE", Boolean.class);
+ code.areturn();
+ return false;
+ }
+
+ // if detached state is not definitive, just give up now and return
+ // null so that the runtime will perform a DB lookup to determine
+ // whether we're detached or new
+ code.aload().setThis();
+ code.invokespecial().setMethod(ISDETACHEDSTATEDEFINITIVE, boolean.class,
+ null);
+ ifins = code.ifne();
+ code.constant().setNull();
code.areturn();
+ ifins.setTarget(code.nop());
+
+ // no detached state: if instance uses detached state and it's not
+ // synthetic or the instance is not serializable or the state isn't
+ // transient, must not be detached
+ if (state == null
+ && (!ClassMetaData.SYNTHETIC.equals(_meta.getDetachedState())
+ || !Serializable.class.isAssignableFrom(_meta.getDescribedType())
+ || !_repos.getConfiguration().getDetachStateInstance().
+ isDetachedStateTransient())) {
+ // return Boolean.FALSE
+ code.getstatic().setField(Boolean.class, "FALSE", Boolean.class);
+ code.areturn();
+ return true;
+ }
+
+ // no detached state: if instance uses detached state (and must be
+ // synthetic and transient in serializable instance at this point),
+ // not detached if state not set to DESERIALIZED
+ if (state == null) {
+ // if (pcGetDetachedState () == null) // instead of DESERIALIZED
+ // return Boolean.FALSE;
+ loadManagedInstance(code, false);
+ code.invokevirtual().setMethod(PRE + "GetDetachedState",
+ Object.class, null);
+ ifins = code.ifnonnull();
+ code.getstatic().setField(Boolean.class, "FALSE", Boolean.class);
+ code.areturn();
+ ifins.setTarget(code.nop());
+ }
+
+ // give up; we just don't know
+ code.constant().setNull();
+ code.areturn();
+ return true;
}
/**
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 7fa741f7b..c01dcb2fa 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
@@ -69,19 +69,12 @@ class VersionAttachStrategy
public Object attach(AttachManager manager, Object toAttach,
ClassMetaData meta, PersistenceCapable into, OpenJPAStateManager owner,
ValueMetaData ownerMeta, boolean explicit) {
-
- // VersionAttachStrategy is invoked in the case where no more
- // intelligent strategy could be found; let's be more lenient
- // about new vs. detached record determination.
- if (into == null)
- into = findFromDatabase(manager, toAttach);
-
BrokerImpl broker = manager.getBroker();
PersistenceCapable pc = ImplHelper.toPersistenceCapable(toAttach,
meta.getRepository().getConfiguration());
boolean embedded = ownerMeta != null && ownerMeta.isEmbeddedPC();
- boolean isNew = !broker.isDetached(pc) && into == null;
+ boolean isNew = !broker.isDetached(pc);
Object version = null;
StateManagerImpl sm;
diff --git a/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties b/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties
index cfeac3d4f..5c8d73732 100644
--- a/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties
+++ b/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/localizer.properties
@@ -116,7 +116,9 @@ not-managed: The given instance "{0}" is not managed by this context.
trans-not-managed: This broker is not configured to use managed transactions.
bad-detached-op: You cannot perform operation {0} on detached object "{1}". \
This operation only applies to managed objects.
-persist-detached: Attempt to persist detached object "{0}".
+persist-detached: Attempt to persist detached object "{0}". If this is a new \
+ instance, make sure any versino and/or auto-generated primary key fields are \
+ null/default when persisting.
null-value: The field "{0}" of instance "{1}" contained a null value; \
the metadata for this field specifies that nulls are illegal.
change-identity: Attempt to change a primary key field of an instance that \
@@ -396,4 +398,4 @@ cant-serialize-pessimistic-broker: Serialization not allowed for brokers with \
an active datastore (pessimistic) transaction.
cant-serialize-connected-broker: Serialization not allowed for brokers with \
an active connection to the database.
-no-interface-metadata: No metadata was found for managed interface {0}.
\ No newline at end of file
+no-interface-metadata: No metadata was found for managed interface {0}.
diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/detachment/TestAttachConstructedCopy.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/detachment/TestAttachConstructedCopy.java
new file mode 100755
index 000000000..1af43571a
--- /dev/null
+++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/detachment/TestAttachConstructedCopy.java
@@ -0,0 +1,72 @@
+/*
+ * 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.detachment;
+
+import javax.persistence.EntityManager;
+
+import junit.textui.TestRunner;
+import org.apache.openjpa.persistence.test.SingleEMFTestCase;
+
+/**
+ * Test that manually constructing instances with existing primary key values
+ * and attaching them works.
+ *
+ * @author Abe White
+ */
+public class TestAttachConstructedCopy
+ extends SingleEMFTestCase {
+
+ public void setUp() {
+ setUp(Record.class);
+ }
+
+ public void testAttachConstructedCopyWithGeneratedPKAndNoVersion() {
+ Record record = new Record();
+ record.setContent("orig");
+
+ EntityManager em = emf.createEntityManager();
+ em.getTransaction().begin();
+ em.persist(record);
+ em.getTransaction().commit();
+ em.close();
+ int id = record.getId();
+
+ Record copy = new Record();
+ copy.setId(id);
+ copy.setContent("new");
+
+ em = emf.createEntityManager();
+ em.getTransaction().begin();
+ record = em.merge(copy);
+ assertTrue(record != copy);
+ assertEquals("new", record.getContent());
+ em.getTransaction().commit();
+ em.close();
+
+ em = emf.createEntityManager();
+ record = em.find(Record.class, id);
+ assertEquals("new", record.getContent());
+ em.close();
+ }
+
+ public static void main(String[] args) {
+ TestRunner.run(TestAttachConstructedCopy.class);
+ }
+}
+
diff --git a/openjpa-project/src/doc/manual/ref_guide_remote.xml b/openjpa-project/src/doc/manual/ref_guide_remote.xml
index 0320be631..0ed8ca611 100644
--- a/openjpa-project/src/doc/manual/ref_guide_remote.xml
+++ b/openjpa-project/src/doc/manual/ref_guide_remote.xml
@@ -173,7 +173,10 @@ attached instance's corresponding fields to null.
If the instance has a Version field,
OpenJPA will consider the object detached if the version field has a non-default
-value, and new otherwise.
+value, and new otherwise. Similarly, if the instance has
+GeneratedValue primary key fields, OpenJPA will consider the
+object detached if any of these fields have non-default values, and new
+otherwise.
When attaching null fields in these cases, OpenJPA cannot distinguish between a