Can you provide more details about your application and environment? You marked affects 2.0.1 and 2.1.0, so which jars are you using? The openjpa-2.x.x.jar does not include the BVAL API or implementation. If you grab the latest openjpa-all-2.1.0-SNAPSHOT.jar then that includes the Apache Geronimo Validation API and Apache Bean Validation implementation.

Donald Woods
added a comment - 13/Sep/10 13:56 Can you provide more details about your application and environment? You marked affects 2.0.1 and 2.1.0, so which jars are you using? The openjpa-2.x.x.jar does not include the BVAL API or implementation. If you grab the latest openjpa-all-2.1.0-SNAPSHOT.jar then that includes the Apache Geronimo Validation API and Apache Bean Validation implementation.

So after looking at your example, I'm not sure why you opened a JIRA....
You're using Hibernate Validator 4.0.2.GA, which we support.
The Employee.name is marked with @NotNull.
But in your code snippet above, you are setting the Name, so what is the behavior you're expecting? @NotNull should only throw an exception if you didn't call setName() before trying to merge/persist the entity.....

Donald Woods
added a comment - 13/Sep/10 14:04 So after looking at your example, I'm not sure why you opened a JIRA....
You're using Hibernate Validator 4.0.2.GA, which we support.
The Employee.name is marked with @NotNull.
But in your code snippet above, you are setting the Name, so what is the behavior you're expecting? @NotNull should only throw an exception if you didn't call setName() before trying to merge/persist the entity.....

Oliver Ringel
added a comment - 13/Sep/10 14:32 OK, I think I described the issue not clear enough.
As you said right, @NotNull should only throw an exception, if name is not set.
The problem is, that I get an exception (javax.validation.ConstraintViolationException),
although I called setName().
Using persist instead of merge is working as expected. I added added test method to
demonstrate this.

Can you attach your stack trace? I just ran into a scenario where an entity not using Validation is causing the Apache Bean Validation provider to throw an exception due to entity.hashCode() throwing a NPE....

Donald Woods
added a comment - 13/Sep/10 22:11 Can you attach your stack trace? I just ran into a scenario where an entity not using Validation is causing the Apache Bean Validation provider to throw an exception due to entity.hashCode() throwing a NPE....

Oliver Ringel
added a comment - 02/Mar/11 19:22 I finally found the place in the source code where the data gets lost.
If you merge a new entity the BrokerImpl first tries to attach the entity via the AttachManager.
The AttachManager uses an AttachStrategy to persist the new entity.
Here ist the code snippet from the org.apache.openjpa.kernel.AttachStrategy class with my comments
protected StateManagerImpl persist(AttachManager manager,
PersistenceCapable pc, ClassMetaData meta, Object appId,
boolean explicit) {
PersistenceCapable newInstance;
if (!manager.getCopyNew())
newInstance = pc; <--- calling this would fix the issue
else if (appId == null)
{
newInstance = pc.pcNewInstance(null, false); <--- but this is called, pc.pcNewInstance returns an new instance, person.name is set to null
}
else
newInstance = pc.pcNewInstance(null, appId, false);
return (StateManagerImpl) manager.getBroker().persist <-- this calls finally BrokerImpl.persistInternal(...) and fails because person.name is null
(newInstance, appId, explicit, manager.getBehavior());
}
For my testcases this issue could be fixed by replacing the first if statement with "if (manager.getCopyNew())".
I'm absolutely not sure, if it is really that easy. I guess not.
Maybe another good place to fix the problem is EntityManagerImpl.merge() which also sets the AttachManager's copyNew flag.
Please could someone with more insight into openjpa verify this solution.
Thanks.

The main problem with your proposed solution is that it would change the way merge works. EntityManager.merge creates a new copy of the entity and it's the copy of the entity that becomes part of the persistence context - not the entity you passed in. Your change would eliminate the copy.

I think the real issue here is that the data hasn't been copied into the new instance before validation occurs. I'm not familiar with this code path, and I haven't had a chance to try your testcase yet, but I'd start by looking at the code where we copy the fields into a new instance.

Michael Dick
added a comment - 08/Mar/11 21:41 The main problem with your proposed solution is that it would change the way merge works. EntityManager.merge creates a new copy of the entity and it's the copy of the entity that becomes part of the persistence context - not the entity you passed in. Your change would eliminate the copy.
I think the real issue here is that the data hasn't been copied into the new instance before validation occurs. I'm not familiar with this code path, and I haven't had a chance to try your testcase yet, but I'd start by looking at the code where we copy the fields into a new instance.

Hi Michael,
thank you for your answer. I thought that it would not be so easy to solve this issue. Unfortunately.

I agree with you that the main problem here is that a copy action is missing and I guess AttachStrategy.persist is the right place to add some kind of copy functionality.

I started looking into the source code of the enhanced class from the test. PersistenceCapable declares a public method to copy fields (pcCopyFields(...)).
Unfortunately you can't use in AttachStrategy.persist, because the entity to persist has no statemanager at this point (an exception InvalidStateException will be the result).

My suggestion is to modify the class enhancement and remove the StateManager check in pcCopyFields (I don't see the need for this check).
Afterwards you can use pcCopyFields in AttachStrategy.persist by adding something like

Oliver Ringel
added a comment - 10/Mar/11 13:43 Hi Michael,
thank you for your answer. I thought that it would not be so easy to solve this issue. Unfortunately.
I agree with you that the main problem here is that a copy action is missing and I guess AttachStrategy.persist is the right place to add some kind of copy functionality.
I started looking into the source code of the enhanced class from the test. PersistenceCapable declares a public method to copy fields (pcCopyFields(...)).
Unfortunately you can't use in AttachStrategy.persist, because the entity to persist has no statemanager at this point (an exception InvalidStateException will be the result).
My suggestion is to modify the class enhancement and remove the StateManager check in pcCopyFields (I don't see the need for this check).
Afterwards you can use pcCopyFields in AttachStrategy.persist by adding something like
...
if (manager.getCopyNew())
{
int[] fields = new int[meta.getFields().length];
for (int i = 0; i < fields.length; i++)
fields[i] = i;
newInstance.pcCopyFields(pc, fields);
}
...
This solution works for my testcase.
As an alternative you can modify (or add an additional) newInstance to copy the data.
I have no real experience with OpenJPA. Perhaps there is a much better solution.

2004 OpenJpaTestcase TRACE [main] openjpa.Runtime - An exception occurred while ending the transaction. This exception will be re-thrown.
<openjpa-2.2.0-SNAPSHOT-r422266:1078811 fatal store error> org.apache.openjpa.util.StoreException: The transaction cannot be committed, because it was already marked for rollback only. The transaction will be rolled back instead. The cause of the rollback-only status is reported in the embedded stack.
at org.apache.openjpa.kernel.LocalManagedRuntime.commit(LocalManagedRuntime.java:89)
at org.apache.openjpa.kernel.BrokerImpl.commit(BrokerImpl.java:1497)
at org.apache.openjpa.kernel.DelegatingBroker.commit(DelegatingBroker.java:933)
at org.apache.openjpa.persistence.EntityManagerImpl.commit(EntityManagerImpl.java:565)
at testcase.openjpa.OpenJpaTest.testValidation(OpenJpaTest.java:24)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:76)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
at org.apache.maven.surefire.junit4.JUnit4TestSet.execute(JUnit4TestSet.java:59)
at org.apache.maven.surefire.suite.AbstractDirectoryTestSuite.executeTestSet(AbstractDirectoryTestSuite.java:115)
at org.apache.maven.surefire.suite.AbstractDirectoryTestSuite.execute(AbstractDirectoryTestSuite.java:102)
at org.apache.maven.surefire.Surefire.run(Surefire.java:180)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at org.apache.maven.surefire.booter.SurefireBooter.runSuitesInProcess(SurefireBooter.java:350)
at org.apache.maven.surefire.booter.SurefireBooter.main(SurefireBooter.java:1021)
Caused by: javax.validation.ConstraintViolationException: A validation constraint failure occurred for class "testcase.openjpa.Person".
at org.apache.openjpa.persistence.validation.ValidatorImpl.validate(ValidatorImpl.java:282)
at org.apache.openjpa.validation.ValidatingLifecycleEventManager.fireEvent(ValidatingLifecycleEventManager.java:122)
at org.apache.openjpa.kernel.BrokerImpl.fireLifecycleEvent(BrokerImpl.java:790)
at org.apache.openjpa.kernel.BrokerImpl.persistInternal(BrokerImpl.java:2606)
at org.apache.openjpa.kernel.BrokerImpl.persist(BrokerImpl.java:2544)
at org.apache.openjpa.kernel.AttachStrategy.persist(AttachStrategy.java:95)
at org.apache.openjpa.kernel.VersionAttachStrategy.attach(VersionAttachStrategy.java:102)
at org.apache.openjpa.kernel.AttachManager.attach(AttachManager.java:251)
at org.apache.openjpa.kernel.AttachManager.attach(AttachManager.java:104)
at org.apache.openjpa.kernel.BrokerImpl.attach(BrokerImpl.java:3433)
at org.apache.openjpa.kernel.DelegatingBroker.attach(DelegatingBroker.java:1214)
at org.apache.openjpa.persistence.EntityManagerImpl.merge(EntityManagerImpl.java:873)
at testcase.openjpa.OpenJpaTest.testValidation(OpenJpaTest.java:22)
... 26 more

Oliver Ringel
added a comment - 10/Mar/11 17:36 Hi Rick,
this is the stack trace from my testcase.
2004 OpenJpaTestcase TRACE [main] openjpa.Runtime - An exception occurred while ending the transaction. This exception will be re-thrown.
<openjpa-2.2.0-SNAPSHOT-r422266:1078811 fatal store error> org.apache.openjpa.util.StoreException: The transaction cannot be committed, because it was already marked for rollback only. The transaction will be rolled back instead. The cause of the rollback-only status is reported in the embedded stack.
at org.apache.openjpa.kernel.LocalManagedRuntime.commit(LocalManagedRuntime.java:89)
at org.apache.openjpa.kernel.BrokerImpl.commit(BrokerImpl.java:1497)
at org.apache.openjpa.kernel.DelegatingBroker.commit(DelegatingBroker.java:933)
at org.apache.openjpa.persistence.EntityManagerImpl.commit(EntityManagerImpl.java:565)
at testcase.openjpa.OpenJpaTest.testValidation(OpenJpaTest.java:24)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:76)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
at org.apache.maven.surefire.junit4.JUnit4TestSet.execute(JUnit4TestSet.java:59)
at org.apache.maven.surefire.suite.AbstractDirectoryTestSuite.executeTestSet(AbstractDirectoryTestSuite.java:115)
at org.apache.maven.surefire.suite.AbstractDirectoryTestSuite.execute(AbstractDirectoryTestSuite.java:102)
at org.apache.maven.surefire.Surefire.run(Surefire.java:180)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at org.apache.maven.surefire.booter.SurefireBooter.runSuitesInProcess(SurefireBooter.java:350)
at org.apache.maven.surefire.booter.SurefireBooter.main(SurefireBooter.java:1021)
Caused by: javax.validation.ConstraintViolationException: A validation constraint failure occurred for class "testcase.openjpa.Person".
at org.apache.openjpa.persistence.validation.ValidatorImpl.validate(ValidatorImpl.java:282)
at org.apache.openjpa.validation.ValidatingLifecycleEventManager.fireEvent(ValidatingLifecycleEventManager.java:122)
at org.apache.openjpa.kernel.BrokerImpl.fireLifecycleEvent(BrokerImpl.java:790)
at org.apache.openjpa.kernel.BrokerImpl.persistInternal(BrokerImpl.java:2606)
at org.apache.openjpa.kernel.BrokerImpl.persist(BrokerImpl.java:2544)
at org.apache.openjpa.kernel.AttachStrategy.persist(AttachStrategy.java:95)
at org.apache.openjpa.kernel.VersionAttachStrategy.attach(VersionAttachStrategy.java:102)
at org.apache.openjpa.kernel.AttachManager.attach(AttachManager.java:251)
at org.apache.openjpa.kernel.AttachManager.attach(AttachManager.java:104)
at org.apache.openjpa.kernel.BrokerImpl.attach(BrokerImpl.java:3433)
at org.apache.openjpa.kernel.DelegatingBroker.attach(DelegatingBroker.java:1214)
at org.apache.openjpa.persistence.EntityManagerImpl.merge(EntityManagerImpl.java:873)
at testcase.openjpa.OpenJpaTest.testValidation(OpenJpaTest.java:22)
... 26 more

I tracked down the source of the problem. When merging a new entity, OpenJPA persists a new "empty" entity, sets (or retrieves) an ID for the new entity, and then merges in the object passed in. The problem is that the during the persist of the empty entity, a PRE_PERSIST lifecycle event gets fired, causing validation to occur. I've found that not only is validation occurring prematurely, but OpenJPA violates the JPA specification for the PrePersist callback as well.

<jpa 1.0 & 2.0 spec - section 3.5.2>
For entities to which the merge operation has been applied and causes the creation of newly managed instances, the PrePersist callback methods will be invoked for the managed instance after the entity state has been copied to it.
<jpa 1.0 & 2.0 spec - section 3.5.2/>

By adding this simple callback to the Person class, it showed that a PrePersist callback occurred before the entity was fully populated.
class Person {
...

@PrePersist
public void prePersist()

{
System.out.println("PrePersist getName()=" + this.getName());
}

}

Result: PrePersist getName()=null

I think the solution will be to disable this callback for the case where a new entity is created specifically for the purposes of a merge. Working on a fix...

Jeremy Bauer
added a comment - 10/Mar/11 19:06 I tracked down the source of the problem. When merging a new entity, OpenJPA persists a new "empty" entity, sets (or retrieves) an ID for the new entity, and then merges in the object passed in. The problem is that the during the persist of the empty entity, a PRE_PERSIST lifecycle event gets fired, causing validation to occur. I've found that not only is validation occurring prematurely, but OpenJPA violates the JPA specification for the PrePersist callback as well.
<jpa 1.0 & 2.0 spec - section 3.5.2>
For entities to which the merge operation has been applied and causes the creation of newly managed instances, the PrePersist callback methods will be invoked for the managed instance after the entity state has been copied to it.
<jpa 1.0 & 2.0 spec - section 3.5.2/>
By adding this simple callback to the Person class, it showed that a PrePersist callback occurred before the entity was fully populated.
class Person {
...
@PrePersist
public void prePersist()
{
System.out.println("PrePersist getName()=" + this.getName());
}
}
Result: PrePersist getName()=null
I think the solution will be to disable this callback for the case where a new entity is created specifically for the purposes of a merge. Working on a fix...

Attaching patch for 2.1.x release that prevents the persist path from firing a premature pre-persist event on new entities produced in a merge operation. I am seeing some jUnit failures that test order/look schema related rather than related to this change. I am looking into those failures and producing additional test cases.

Jeremy Bauer
added a comment - 11/Mar/11 03:53 Attaching patch for 2.1.x release that prevents the persist path from firing a premature pre-persist event on new entities produced in a merge operation. I am seeing some jUnit failures that test order/look schema related rather than related to this change. I am looking into those failures and producing additional test cases.

Great! I'm glad it fixed the problem. I just checked the code into the 2.1.x stream as well.

Yes, the has*Listeners methods in the VLEM do look suspect. Thanks for reporting it. I'll do some verification and will open a new JIRA if it turns out to be a bug. I'm about 99% certain it is... Both pre-persist and pre-update validation are enabled by default. So, the bug will not surface unless non-default validation groups are specified. Looks like we are missing a test variation or three.

Jeremy Bauer
added a comment - 15/Mar/11 16:46 Great! I'm glad it fixed the problem. I just checked the code into the 2.1.x stream as well.
Yes, the has*Listeners methods in the VLEM do look suspect. Thanks for reporting it. I'll do some verification and will open a new JIRA if it turns out to be a bug. I'm about 99% certain it is... Both pre-persist and pre-update validation are enabled by default. So, the bug will not surface unless non-default validation groups are specified. Looks like we are missing a test variation or three.

It looks like the hasPersistListeners and hasUpdateListeners methods in the VLEM are implemented simply for the sake of consistency and, as far as I can tell, are not actually called within OpenJPA. Event type checking is all within the fireEvent method itself and testing shows it to work as expected. The hasDeleteListeners method (which is implemented correctly) does get called by JDBCStoreQuery for specific in-memory operations. For the sake of consistency, and in the event that an external plugin may be calling these methods (they are public), I'll check in the simple code fix and some additional test cases that further verify validation is working as expected, based on the event type and specified validation group (or lack thereof). This JIRA will be used to make these changes.

Jeremy Bauer
added a comment - 16/Mar/11 02:17 It looks like the hasPersistListeners and hasUpdateListeners methods in the VLEM are implemented simply for the sake of consistency and, as far as I can tell, are not actually called within OpenJPA. Event type checking is all within the fireEvent method itself and testing shows it to work as expected. The hasDeleteListeners method (which is implemented correctly) does get called by JDBCStoreQuery for specific in-memory operations. For the sake of consistency, and in the event that an external plugin may be calling these methods (they are public), I'll check in the simple code fix and some additional test cases that further verify validation is working as expected, based on the event type and specified validation group (or lack thereof). This JIRA will be used to make these changes.

Committed has*Listeners fix + additional testcases to trunk under rev 1082259. I don't think this additional change necessarily needs to be backported since it doesn't appear to cause any real problem, but it certainly could be.

Roger Keays
added a comment - 08/Apr/12 05:38 Or more accurately, when the merge cascades more than one relationship. i.e. the non-null field is
Invoice.Customer.Contact.name
and I call
merge(invoice)
The Customer validates okay, but validation annotations on the Contact all fail because the fields are null.

According to Albert's comment on 02/02/2012 and the svn commit messages, this problem was resolved in the 2.2.0 release. Are you still experiencing a similar problem with 2.2.0? Or, were you just asking?

Kevin Sutter
added a comment - 09/Apr/12 19:43 According to Albert's comment on 02/02/2012 and the svn commit messages, this problem was resolved in the 2.2.0 release. Are you still experiencing a similar problem with 2.2.0? Or, were you just asking?

Roger Keays
added a comment - 08/Sep/12 07:26 I've hit this bug again with cascade merging. Validations on the nested object fail because the fields are all null. Maybe the fireEvents flag isn't properly handled when cascading?