Don't validate if the only change is to a @Version attribute

Signed-off-by: Jonathan Gallimore <[email protected]>

foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/queries/DatabaseQueryMechanism.java

@@ -1081,6 +1081,7 @@ && isExpressionQueryMechanism()) {// this is not a hand-coded call (custom SQL e
             // PERF: Avoid events if no listeners.
             if (eventManager.hasAnyEventListeners()) {
                 DescriptorEvent event = new DescriptorEvent(DescriptorEventManager.PreUpdateWithChangesEvent, writeQuery);
+                event.setChangeSet(changeSet);
                 eventManager.executeEvent(event);
 
                 // PreUpdateWithChangesEvent listeners may have altered the object - should recalculate the change set.

jpa/eclipselink.jpa.testapps/jpa.test.beanvalidation/src/main/java/org/eclipse/persistence/testing/models/jpa/beanvalidation/BeanValidationTableCreator.java

@@ -22,6 +22,7 @@ public BeanValidationTableCreator() {
         setName("BeanValidationEmployeeProject");
 
         addTableDefinition(buildProjectTable());
+        addTableDefinition(buildTaskTable());
         addTableDefinition(buildEmployeeTable());
         addTableDefinition(buildEmployeeProjectTable());
     }
@@ -53,6 +54,54 @@ public TableDefinition buildProjectTable() {
         return table;
     }
 
+    public TableDefinition buildTaskTable() {
+        TableDefinition table = new TableDefinition();
+        table.setName("CMP3_BV_TASK");
+
+        FieldDefinition fieldID = new FieldDefinition();
+        fieldID.setName("ID");
+        fieldID.setTypeName("NUMBER");
+        fieldID.setSize(19);
+        fieldID.setSubSize(0);
+        fieldID.setIsPrimaryKey(true);
+        fieldID.setIsIdentity(true);
+        fieldID.setShouldAllowNull(false);
+        table.addField(fieldID);
+
+        FieldDefinition fieldVersion = new FieldDefinition();
+        fieldVersion.setName("VERSION");
+        fieldVersion.setTypeName("NUMBER");
+        fieldVersion.setSize(19);
+        fieldVersion.setSubSize(0);
+        fieldVersion.setIsPrimaryKey(false);
+        fieldVersion.setIsIdentity(false);
+        fieldVersion.setShouldAllowNull(false);
+        table.addField(fieldVersion);
+
+        FieldDefinition fieldName = new FieldDefinition();
+        fieldName.setName("NAME");
+        fieldName.setTypeName("VARCHAR");
+        fieldName.setSize(20);
+        fieldName.setShouldAllowNull(true);
+        fieldName.setIsPrimaryKey(false);
+        fieldName.setUnique(false);
+        fieldName.setIsIdentity(false);
+        table.addField(fieldName);
+
+        FieldDefinition fieldPriority = new FieldDefinition();
+        fieldPriority.setName("PRIORITY");
+        fieldPriority.setTypeName("NUMBER");
+        fieldPriority.setSize(19);
+        fieldPriority.setSubSize(0);
+        fieldPriority.setIsPrimaryKey(false);
+        fieldPriority.setIsIdentity(false);
+        fieldPriority.setShouldAllowNull(false);
+        table.addField(fieldPriority);
+
+        return table;
+    }
+
+
     public TableDefinition buildEmployeeTable() {
         TableDefinition table = new TableDefinition();
         table.setName("CMP3_BV_EMPLOYEE");

jpa/eclipselink.jpa.testapps/jpa.test.beanvalidation/src/main/java/org/eclipse/persistence/testing/models/jpa/beanvalidation/Task.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright (c) 2009, 2022 Oracle and/or its affiliates. All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0,
+ * or the Eclipse Distribution License v. 1.0 which is available at
+ * http://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause
+ */
+
+// Contributors:
+
+package org.eclipse.persistence.testing.models.jpa.beanvalidation;
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.Id;
+import jakarta.persistence.Column;
+import jakarta.persistence.Version;
+import jakarta.validation.constraints.NotNull;
+
+@Entity(name="CMP3_BV_TASK")
+public class Task {
+
+    @Id
+    private int        id;
+
+    @Version
+    private int        version;
+
+    @NotNull
+    private String     name;
+
+    @Column
+    private int        priority;
+
+    public Task() {}
+
+
+    public int getId() {
+        return id;
+    }
+
+    public String getName() {
+        return name;
+    }
+
+    public void setName(final String name) {
+        this.name = name;
+    }
+
+
+    public int getPriority() {
+        return priority;
+    }
+
+
+    public void setPriority(final int priority) {
+        this.priority = priority;
+    }
+}

jpa/eclipselink.jpa.testapps/jpa.test.beanvalidation/src/main/resources/META-INF/persistence.xml

@@ -21,6 +21,7 @@
         <class>org.eclipse.persistence.testing.models.jpa.beanvalidation.Employee</class>
         <class>org.eclipse.persistence.testing.models.jpa.beanvalidation.Project</class>
         <class>org.eclipse.persistence.testing.models.jpa.beanvalidation.Address</class>
+        <class>org.eclipse.persistence.testing.models.jpa.beanvalidation.Task</class>
         <validation-mode>CALLBACK</validation-mode>
         <properties>
             <property name="jakarta.persistence.schema-generation.database.action" value="drop-and-create"/>

jpa/eclipselink.jpa.testapps/jpa.test.beanvalidation/src/test/java/org/eclipse/persistence/testing/tests/jpa/beanvalidation/BeanValidationJunitTest.java

@@ -14,20 +14,26 @@
 //      Marcel Valovy
 package org.eclipse.persistence.testing.tests.jpa.beanvalidation;
 
+import java.util.Vector;
+
 import jakarta.persistence.EntityManager;
 import jakarta.persistence.EntityManagerFactory;
+import jakarta.persistence.LockModeType;
+import jakarta.persistence.RollbackException;
 import jakarta.persistence.TypedQuery;
 import jakarta.validation.ConstraintViolation;
 import jakarta.validation.ConstraintViolationException;
 import junit.framework.Test;
 import junit.framework.TestSuite;
 import org.eclipse.persistence.logging.SessionLog;
 import org.eclipse.persistence.mappings.ForeignReferenceMapping;
+import org.eclipse.persistence.sessions.DatabaseRecord;
 import org.eclipse.persistence.testing.framework.jpa.junit.JUnitTestCase;
 import org.eclipse.persistence.testing.models.jpa.beanvalidation.Address;
 import org.eclipse.persistence.testing.models.jpa.beanvalidation.BeanValidationTableCreator;
 import org.eclipse.persistence.testing.models.jpa.beanvalidation.Employee;
 import org.eclipse.persistence.testing.models.jpa.beanvalidation.Project;
+import org.eclipse.persistence.testing.models.jpa.beanvalidation.Task;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -62,6 +68,9 @@ public static Test suite() {
         suite.addTest(new BeanValidationJunitTest("testTraversableResolverPreventsLoadingOfLazyRelationships"));
         suite.addTest(new BeanValidationJunitTest("testTraversableResolverPreventsTraversingRelationshipMultipleTimes"));
         suite.addTest(new BeanValidationJunitTest("testValidateChangedData"));
+        suite.addTest(new BeanValidationJunitTest("testPessimisticLockWithInvalidData"));
+        suite.addTest(new BeanValidationJunitTest("testPessimisticLockUpdateObjectWithInvalidData"));
+        suite.addTest(new BeanValidationJunitTest("testPessimisticLockUpdateObjectWithValidData"));
         return suite;
     }
 
@@ -390,6 +399,163 @@ public void testValidateChangedData() {
             closeEntityManager(em);
         }
     }
+
+    /**
+     * Strategy:
+     * 1. Update an Employee and related project to trigger validation on it
+     * 2. Find the object using its primary key, with a pessimistic lock
+     * 3. Do not change the object
+     * 4. End the transaction
+     * 5. The version field should be incremented in the database, but no other changes
+     * 6. No validation exceptions should be thrown
+     */
+    public void testPessimisticLockWithInvalidData() throws Exception {
+        try {
+            getDatabaseSession().executeNonSelectingSQL("insert into CMP3_BV_TASK values (900, 1, NULL, 1)");
+        } catch (Throwable t) {
+            getDatabaseSession().getSessionLog().logThrowable(SessionLog.WARNING, t);
+        }
+        clearCache();
+        Map<String, Object> props = new HashMap<>();
+        props.put("eclipselink.weaving", "false");
+        EntityManagerFactory factory = getEntityManagerFactory(props);
+        EntityManager em = factory.createEntityManager();
+        try {
+            beginTransaction(em);
+
+            Task task = em.find(Task.class, 900, LockModeType.PESSIMISTIC_FORCE_INCREMENT);
+
+            commitTransaction(em);
+
+            Vector resultSet = getDatabaseSession().executeSQL("select * from CMP3_BV_TASK where ID=900");
+            assertEquals(1, resultSet.size());
+
+            final DatabaseRecord dr = (DatabaseRecord) resultSet.firstElement();
+            assertEquals(900L, dr.get("ID"));
+            assertEquals(2L, dr.get("VERSION")); // should be incremented by the pessimistic lock
+            assertNull(dr.get("NAME")); // should be unchanged
+            assertEquals(1L, dr.get("PRIORITY")); // should be unchanged
+
+        } catch (RuntimeException ex) {
+            if (isTransactionActive(em)) {
+                rollbackTransaction(em);
+            }
+            throw ex;
+        } finally {
+            closeEntityManager(em);
+        }
+    }
+
+    /**
+     * Strategy:
+     * 1. Update an Employee and related project to trigger validation on it
+     * 2. Find the object using its primary key, with a pessimistic lock
+     * 3. Change the object with still invalid data
+     * 4. End the transaction
+     * 5. The version field should be incremented in the database, but no other changes
+     * 6. A validation exception should be thrown
+     */
+    public void testPessimisticLockUpdateObjectWithInvalidData() throws Exception {
+        try {
+            getDatabaseSession().executeNonSelectingSQL("insert into CMP3_BV_TASK values (901, 1, NULL, 1)");
+        } catch (Throwable t) {
+            getDatabaseSession().getSessionLog().logThrowable(SessionLog.WARNING, t);
+        }
+        clearCache();
+        Map<String, Object> props = new HashMap<>();
+        props.put("eclipselink.weaving", "false");
+        boolean gotConstraintViolations = false;
+        EntityManagerFactory factory = getEntityManagerFactory(props);
+        EntityManager em = factory.createEntityManager();
+        try {
+            beginTransaction(em);
+
+            Task task = em.find(Task.class, 901, LockModeType.PESSIMISTIC_FORCE_INCREMENT);
+            task.setPriority(2);
+
+            commitTransaction(em);
+        } catch (ConstraintViolationException e) {
+            assertTrue("Transaction not marked for roll back when ConstraintViolation is thrown", getRollbackOnly(em));
+            Set<ConstraintViolation<?>> constraintViolations = e.getConstraintViolations();
+            ConstraintViolation constraintViolation = constraintViolations.iterator().next();
+            Object invalidValue = constraintViolation.getInvalidValue();
+            System.out.println(invalidValue);
+            gotConstraintViolations = true;
+        } catch (RollbackException e) {
+            e.printStackTrace();
+            final ConstraintViolationException cve = (ConstraintViolationException) e.getCause();
+            Set<ConstraintViolation<?>> constraintViolations = cve.getConstraintViolations();
+            ConstraintViolation constraintViolation = constraintViolations.iterator().next();
+            assertEquals("must not be null", constraintViolation.getMessage());
+            gotConstraintViolations = true;
+        } finally {
+            if (isTransactionActive(em)) {
+                rollbackTransaction(em);
+            }
+            closeEntityManager(em);
+        }
+
+        assertTrue("Did not get Constraint Violation while persisting invalid data ", gotConstraintViolations);
+
+        Vector resultSet = getDatabaseSession().executeSQL("select * from CMP3_BV_TASK where ID=901");
+        assertEquals(1, resultSet.size());
+
+        final DatabaseRecord dr = (DatabaseRecord) resultSet.firstElement();
+        assertEquals(901L, dr.get("ID"));
+        assertEquals(1L, dr.get("VERSION")); // should be unchanged
+        assertNull(dr.get("NAME")); // should be unchanged
+        assertEquals(1L, dr.get("PRIORITY")); // should be unchanged
+    }
+
+    /**
+     * Strategy:
+     * 1. Update an Employee and related project to trigger validation on it
+     * 2. Find the object using its primary key, with a pessimistic lock
+     * 3. Change the object to have valid data
+     * 4. End the transaction
+     * 5. The version field should be incremented in the database along with the other changes
+     * 6. No validation exceptions should be thrown
+     */
+    public void testPessimisticLockUpdateObjectWithValidData() throws Exception {
+        try {
+            getDatabaseSession().executeNonSelectingSQL("insert into CMP3_BV_TASK values (902, 1, NULL, 1)");
+        } catch (Throwable t) {
+            getDatabaseSession().getSessionLog().logThrowable(SessionLog.WARNING, t);
+        }
+        clearCache();
+        Map<String, Object> props = new HashMap<>();
+        props.put("eclipselink.weaving", "false");
+        boolean gotConstraintViolations = false;
+        EntityManagerFactory factory = getEntityManagerFactory(props);
+        EntityManager em = factory.createEntityManager();
+        try {
+            beginTransaction(em);
+
+            Task task = em.find(Task.class, 902, LockModeType.PESSIMISTIC_FORCE_INCREMENT);
+            task.setPriority(2);
+            task.setName("Do some work");
+
+            commitTransaction(em);
+        } catch (ConstraintViolationException e) {
+            gotConstraintViolations = true;
+        } finally {
+            if (isTransactionActive(em)) {
+                rollbackTransaction(em);
+            }
+            closeEntityManager(em);
+        }
+
+        assertFalse("Got Constraint Violation while persisting valid data ", gotConstraintViolations);
+
+        Vector resultSet = getDatabaseSession().executeSQL("select * from CMP3_BV_TASK where ID=902");
+        assertEquals(1, resultSet.size());
+
+        final DatabaseRecord dr = (DatabaseRecord) resultSet.firstElement();
+        assertEquals(902L, dr.get("ID"));
+        assertEquals(2L, dr.get("VERSION")); // should be incremented by the pessimistic lock
+        assertEquals("Do some work", dr.get("NAME")); // new value
+        assertEquals(2L, dr.get("PRIORITY")); // new value
+    }
 
     //--------------------Helper Methods ---------------//
     private boolean isInstantiated(Object entityObject, String attributeName, org.eclipse.persistence.sessions.Project project) {

jpa/org.eclipse.persistence.jpa/src/main/java/org/eclipse/persistence/internal/jpa/metadata/listeners/BeanValidationListener.java

@@ -40,9 +40,11 @@
 import org.eclipse.persistence.descriptors.ClassDescriptor;
 import org.eclipse.persistence.descriptors.DescriptorEvent;
 import org.eclipse.persistence.descriptors.DescriptorEventAdapter;
+import org.eclipse.persistence.descriptors.DescriptorEventManager;
 import org.eclipse.persistence.descriptors.FetchGroupManager;
 import org.eclipse.persistence.internal.localization.ExceptionLocalization;
 import org.eclipse.persistence.internal.security.PrivilegedAccessHelper;
+import org.eclipse.persistence.internal.sessions.ObjectChangeSet;
 import org.eclipse.persistence.internal.sessions.UnitOfWorkImpl;
 import org.eclipse.persistence.mappings.DatabaseMapping;
 import org.eclipse.persistence.mappings.ForeignReferenceMapping;
@@ -93,7 +95,10 @@ public void aboutToUpdate(DescriptorEvent event) {
         // preUpdate is also generated for deleted objects that were modified in this UOW.
         // Do not perform preUpdate validation for such objects as preRemove would have already been called.
         if(!unitOfWork.isObjectDeleted(source)) {
-            validateOnCallbackEvent(event, "preUpdate", groupPreUpdate);
+            ObjectChangeSet changeSet = event.getChangeSet();
+            if (changeSet != null && (changeSet.isNew() || (changeSet.getChanges() != null && changeSet.getChanges().size() > 0))) {
+                validateOnCallbackEvent(event, "preUpdate", groupPreUpdate);
+            }
         }
     }