fix: ensure RecordCopyPolicy and RecordInstantiationPolicy set for all embeddable records

foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/descriptors/CMPPolicy.java

@@ -27,6 +27,7 @@
 import org.eclipse.persistence.exceptions.DescriptorException;
 import org.eclipse.persistence.exceptions.ValidationException;
 import org.eclipse.persistence.internal.descriptors.ObjectBuilder;
+import org.eclipse.persistence.internal.descriptors.RecordInstantiationPolicy;
 import org.eclipse.persistence.internal.helper.DatabaseField;
 import org.eclipse.persistence.internal.identitymaps.CacheId;
 import org.eclipse.persistence.internal.security.PrivilegedAccessHelper;
@@ -38,6 +39,7 @@
 import org.eclipse.persistence.queries.UpdateObjectQuery;
 
 import java.io.Serializable;
+import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -462,7 +464,10 @@ public Object createPrimaryKeyInstance(Object object, AbstractSession session) {
             return  fieldValue;
         }
 
-        Object keyInstance = getPKClassInstance();
+        Class<?> pkClass = getPKClass();
+        boolean isRecord = pkClass != null && pkClass.isRecord();
+        Object keyInstance = isRecord ? null : getPKClassInstance();
+        List<Object> recordValues = isRecord ? new ArrayList<>(pkElementArray.length) : null;
         Set<ObjectReferenceMapping> usedObjectReferenceMappings = new HashSet<>();
         for (int index = 0; index < pkElementArray.length; index++) {
             Object keyObj = object;
@@ -492,11 +497,27 @@ public Object createPrimaryKeyInstance(Object object, AbstractSession session) {
                     fieldValue = mapping.getReferenceDescriptor().getCMPPolicy().createPrimaryKeyInstance(fieldValue, session);
                     usedObjectReferenceMappings.add((ObjectReferenceMapping)mapping);
                 }
-                accessor.setValue(nestedKeyInstance, fieldValue);
+                if (isRecord) {
+                    recordValues.add(fieldValue);
+                } else {
+                    accessor.setValue(nestedKeyInstance, fieldValue);
+                }
             }
         }
 
-        return keyInstance;
+        return isRecord ? createRecordInstance(pkClass, recordValues) : keyInstance;
+    }
+
+    /**
+     * Build a record primary key class instance from its component values using
+     * its canonical constructor. Records are immutable, so the default
+     * no-arg-constructor + field-reflection approach used for bean id classes
+     * cannot be applied.
+     */
+    private Object createRecordInstance(Class<?> recordClass, List<?> values) {
+        RecordInstantiationPolicy policy = new RecordInstantiationPolicy(recordClass);
+        policy.setValues(values);
+        return policy.buildNewInstance();
     }
 
     /**

foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/descriptors/ClassDescriptor.java

@@ -3967,38 +3967,41 @@ public void preInitialize(AbstractSession session) throws DescriptorException {
         // PERF: Check if the class "itself" was weaved.
         // If weaved avoid reflection, use clone copy and empty new.
         if (Arrays.asList(getJavaClass().getInterfaces()).contains(PersistenceObject.class)) {
-            // Cloning is only auto set for field access, as method access
-            // may not have simple fields, same with empty new and reflection get/set.
-            boolean isMethodAccess = false;
-            for (DatabaseMapping mapping: getMappings()) {
-                if (mapping.isUsingMethodAccess()) {
-                    // Ok for lazy 1-1s
-                    if (!mapping.isOneToOneMapping() || !((ForeignReferenceMapping)mapping).usesIndirection()) {
-                        isMethodAccess = true;
+            // Records are immutable and cannot be woven, so they always need
+            // record-aware copy and instantiation policies, regardless of access type.
+            if (javaClass != null && javaClass.isRecord()) {
+                if (this.copyPolicy == null) {
+                    setCopyPolicy(new RecordCopyPolicy());
+                }
+                if (!isAbstract() && this.instantiationPolicy == null) {
+                    setInstantiationPolicy(new RecordInstantiationPolicy(javaClass));
+                }
+            } else {
+                // Cloning is only auto set for field access, as method access
+                // may not have simple fields, same with empty new and reflection get/set.
+                boolean isMethodAccess = false;
+                for (DatabaseMapping mapping: getMappings()) {
+                    if (mapping.isUsingMethodAccess()) {
+                        // Ok for lazy 1-1s
+                        if (!mapping.isOneToOneMapping() || !((ForeignReferenceMapping)mapping).usesIndirection()) {
+                            isMethodAccess = true;
+                        }
+                    } else if (!mapping.isWriteOnly()) {
+                        // Avoid reflection.
+                        mapping.setAttributeAccessor(new PersistenceObjectAttributeAccessor(mapping.getAttributeName()));
                     }
-                } else if (!mapping.isWriteOnly()) {
-                    // Avoid reflection.
-                    mapping.setAttributeAccessor(new PersistenceObjectAttributeAccessor(mapping.getAttributeName()));
                 }
-            }
-            if (!isMethodAccess) {
-                if (this.copyPolicy == null) {
-                    if (javaClass != null && javaClass.isRecord()) {
-                        setCopyPolicy(new RecordCopyPolicy());
-                    } else {
+                if (!isMethodAccess) {
+                    if (this.copyPolicy == null) {
                         setCopyPolicy(new PersistenceEntityCopyPolicy());
                     }
-                }
-                if (!isAbstract()) {
-                    try {
-                        if (this.instantiationPolicy == null) {
-                            if (javaClass != null && javaClass.isRecord()) {
-                                setInstantiationPolicy(new RecordInstantiationPolicy(javaClass));
-                            } else {
-                                setInstantiationPolicy(new PersistenceObjectInstantiationPolicy((PersistenceObject)getJavaClass().getConstructor().newInstance()));
-                            }
+                    if (!isAbstract() && this.instantiationPolicy == null) {
+                        try {
+                            setInstantiationPolicy(new PersistenceObjectInstantiationPolicy((PersistenceObject)getJavaClass().getConstructor().newInstance()));
+                        } catch (Exception ignore) {
+                            // No-arg constructor may not exist.
                         }
-                    } catch (Exception ignore) { }
+                    }
                 }
             }
         }

foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/mappings/AggregateMapping.java

@@ -117,7 +117,8 @@ protected void buildAggregateModifyQuery(ObjectLevelModifyQuery sourceQuery, Obj
         // from a back up clone since a map key mapping does not map a field
         // on the source queries backup clone.
         if (sourceQuery.getSession().isUnitOfWork() && ! isMapKeyMapping()) {
-            Object backupAttributeValue = getAttributeValueFromBackupClone(sourceQuery.getBackupClone());
+            Object backupClone = sourceQuery.getBackupClone();
+            Object backupAttributeValue = backupClone == null ? null : getAttributeValueFromBackupClone(backupClone);
             if (backupAttributeValue == null) {
                 backupAttributeValue = getObjectBuilder(sourceAttributeValue, sourceQuery.getSession()).buildNewInstance();
             }
@@ -671,6 +672,12 @@ public void mergeChangesIntoObject(Object target, ChangeRecord changeRecord, Obj
         if (source != null) {
             sourceAggregate = getAttributeValueFromObject(source);
         }
+        // Records are immutable, so the aggregate cannot be updated field-by-field.
+        // Replace it with a clone of the source aggregate instead.
+        if (sourceAggregate instanceof Record) {
+            setAttributeValueInObject(target, referenceDescriptor.getCopyPolicy().buildClone(sourceAggregate, targetSession));
+            return;
+        }
         ObjectBuilder objectBuilder = getObjectBuilderForClass(aggregateChangeSet.getClassType(mergeManager.getSession()), mergeManager.getSession());
         //Bug#4719341  Always obtain aggregate attribute value from the target object regardless of new or not
         Object targetAggregate = getAttributeValueFromObject(target);

jpa/eclipselink.jpa.test.jse/src/it/java/org/eclipse/persistence/jpa/test/record/TestNestedRecordEmbeddable.java

@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) 2024 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
+ */
+
+package org.eclipse.persistence.jpa.test.record;
+
+import jakarta.persistence.EntityManager;
+import jakarta.persistence.EntityManagerFactory;
+
+import org.eclipse.persistence.jpa.test.framework.DDLGen;
+import org.eclipse.persistence.jpa.test.framework.Emf;
+import org.eclipse.persistence.jpa.test.framework.EmfRunner;
+import org.eclipse.persistence.jpa.test.record.model.LocationEntity;
+import org.eclipse.persistence.jpa.test.record.model.NestedActivityRecord;
+import org.eclipse.persistence.jpa.test.record.model.NestedCargoEntity;
+import org.eclipse.persistence.jpa.test.record.model.NestedDeliveryRecord;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+/**
+ * Tests for a nested {@code @Embedded} record used inside another {@code @Embeddable} record.
+ */
+@RunWith(EmfRunner.class)
+public class TestNestedRecordEmbeddable {
+
+    @Emf(createTables = DDLGen.DROP_CREATE, classes = {
+            NestedCargoEntity.class, NestedDeliveryRecord.class, NestedActivityRecord.class, LocationEntity.class })
+    private EntityManagerFactory emf;
+
+    @Test
+    public void testPersistNestedEmbeddedRecord() {
+        EntityManager em = emf.createEntityManager();
+        try {
+            em.getTransaction().begin();
+
+            LocationEntity location = new LocationEntity(100L, "CNHKG");
+            em.persist(location);
+
+            NestedActivityRecord activity = new NestedActivityRecord("LOAD", location);
+            NestedDeliveryRecord delivery = new NestedDeliveryRecord("IN_TRANSIT", activity);
+            NestedCargoEntity cargo = new NestedCargoEntity(1L, delivery, "ABC123");
+
+            em.persist(cargo);
+            em.getTransaction().commit();
+            em.clear();
+
+            NestedCargoEntity found = em.find(NestedCargoEntity.class, 1L);
+            assertNotNull("Cargo should be found after persist", found);
+            assertEquals("ABC123", found.getTrackingId());
+            assertNotNull("Nested delivery should not be null", found.getDelivery());
+            assertEquals("LOAD", found.getDelivery().nextExpectedActivity().type());
+            assertEquals("CNHKG", found.getDelivery().nextExpectedActivity().location().getName());
+        } finally {
+            if (em.getTransaction().isActive()) {
+                em.getTransaction().rollback();
+            }
+            em.close();
+        }
+    }
+}

jpa/eclipselink.jpa.test.jse/src/it/java/org/eclipse/persistence/jpa/test/record/TestRecordEmbeddable.java

@@ -0,0 +1,132 @@
+/*
+ * Copyright (c) 2024 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
+ */
+
+package org.eclipse.persistence.jpa.test.record;
+
+import jakarta.persistence.EntityManager;
+import jakarta.persistence.EntityManagerFactory;
+
+import org.eclipse.persistence.jpa.test.framework.DDLGen;
+import org.eclipse.persistence.jpa.test.framework.Emf;
+import org.eclipse.persistence.jpa.test.framework.EmfRunner;
+import org.eclipse.persistence.jpa.test.record.model.RecordEntity;
+import org.eclipse.persistence.jpa.test.record.model.RecordId;
+import org.eclipse.persistence.jpa.test.record.model.RecordValue;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import java.util.UUID;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Tests for {@code java.lang.Record} used as JPA {@code @Embeddable} types.
+ * <p>
+ * This test covers:
+ * <ul>
+ *   <li>{@code @EmbeddedId} with a record type</li>
+ *   <li>{@code @Embedded} attribute with a record type</li>
+ *   <li>Persist, find, and merge operations</li>
+ * </ul>
+ * <p>
+ * Prior to the fix for <a href="https://github.com/eclipse-ee4j/eclipselink/issues/2656">#2656</a>,
+ * EclipseLink threw {@code IllegalAccessException: Can not set final field} when
+ * method-accessed embeddable records were processed. The root cause was that
+ * {@code RecordCopyPolicy} and {@code RecordInstantiationPolicy} were only
+ * initialized for field-accessed descriptors, not method-accessed ones.
+ */
+@RunWith(EmfRunner.class)
+public class TestRecordEmbeddable {
+
+    @Emf(createTables = DDLGen.DROP_CREATE, classes = {
+            RecordEntity.class, RecordId.class, RecordValue.class })
+    private EntityManagerFactory emf;
+
+    @Test
+    public void testPersistAndFind() {
+        EntityManager em = emf.createEntityManager();
+        try {
+            em.getTransaction().begin();
+
+            UUID uuid = UUID.randomUUID();
+            RecordId id = new RecordId(uuid);
+            RecordValue value = new RecordValue("test-description", 42);
+            RecordEntity entity = new RecordEntity(id, "test-entity", value);
+
+            em.persist(entity);
+            em.getTransaction().commit();
+
+            // Clear to force a fresh read from the database
+            em.clear();
+
+            RecordEntity found = em.find(RecordEntity.class, id);
+            assertNotNull("Entity should be found after persist", found);
+            assertEquals("test-entity", found.getName());
+            assertNotNull("Embedded value should not be null", found.getValue());
+            assertEquals("test-description", found.getValue().description());
+            assertEquals(42, found.getValue().amount());
+        } finally {
+            if (em.getTransaction().isActive()) {
+                em.getTransaction().rollback();
+            }
+            em.close();
+        }
+    }
+
+    @Test
+    public void testMerge() {
+        EntityManager em = emf.createEntityManager();
+        try {
+            // Persist
+            em.getTransaction().begin();
+            UUID uuid = UUID.randomUUID();
+            RecordId id = new RecordId(uuid);
+            RecordValue value = new RecordValue("original", 10);
+            RecordEntity entity = new RecordEntity(id, "original-name", value);
+            em.persist(entity);
+            em.getTransaction().commit();
+            em.clear();
+
+            // Merge with updated values
+            em.getTransaction().begin();
+            RecordValue updatedValue = new RecordValue("updated", 20);
+            RecordEntity updated = new RecordEntity(id, "updated-name", updatedValue);
+            RecordEntity merged = em.merge(updated);
+            em.getTransaction().commit();
+
+            assertNotNull("Merged entity should not be null", merged);
+            assertEquals("updated-name", merged.getName());
+            assertEquals("updated", merged.getValue().description());
+            assertEquals(20, merged.getValue().amount());
+        } finally {
+            if (em.getTransaction().isActive()) {
+                em.getTransaction().rollback();
+            }
+            em.close();
+        }
+    }
+
+    @Test
+    public void testGetIdentifier() {
+        UUID uuid = UUID.randomUUID();
+        RecordId id = new RecordId(uuid);
+        RecordEntity entity = new RecordEntity(id, "test-entity", new RecordValue("desc", 1));
+
+        Object identifier = emf.getPersistenceUnitUtil().getIdentifier(entity);
+
+        assertNotNull("Identifier should not be null", identifier);
+        assertTrue("Identifier should be a RecordId", identifier instanceof RecordId);
+        assertEquals("Identifier should equal the entity's embedded id", id, identifier);
+    }
+}