Skip to content

Commit 6ae2b68

Browse files
hantsyhantsy
committed
@
fix: support Java records as JPA @embeddable, @EmbeddedId and @IdClass Fixes #2656: IllegalAccessException "Can not set final field" when a java.lang.Record is used as an @embeddable (@EmbeddedId / @Embedded) or as an @IdClass. Two code paths assumed a mutable bean with a no-arg constructor: 1. ClassDescriptor.preInitialize() only set RecordCopyPolicy / RecordInstantiationPolicy inside the !isMethodAccess branch, so method-accessed embeddable records (the default, since record accessors like id() are methods) never got record-aware policies and fell back to field reflection. 2. CMPPolicy.createPrimaryKeyInstance() built the id class via getConstructor().newInstance() (no-arg) then accessor.setValue(...). Records have neither a no-arg constructor nor writable fields. Changes: - ClassDescriptor: apply record-aware policies to any record class regardless of access type. - CMPPolicy: detect a record id class and build it via RecordInstantiationPolicy (canonical constructor), mapping values by record component name so descriptor field order is irrelevant. Tests: - TestRecordEmbeddable: @EmbeddedId/@Embedded records covering persist/find, merge, getIdentifier, and a custom no-arg constructor case (mirroring the original CopyId reproduction). - TestRecordIdClass: two-component and single-component records used as @IdClass, each with persist/find and getIdentifier. @
1 parent 1ef871b commit 6ae2b68

11 files changed

Lines changed: 596 additions & 30 deletions

File tree

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

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.eclipse.persistence.exceptions.DescriptorException;
2828
import org.eclipse.persistence.exceptions.ValidationException;
2929
import org.eclipse.persistence.internal.descriptors.ObjectBuilder;
30+
import org.eclipse.persistence.internal.descriptors.RecordInstantiationPolicy;
3031
import org.eclipse.persistence.internal.helper.DatabaseField;
3132
import org.eclipse.persistence.internal.identitymaps.CacheId;
3233
import org.eclipse.persistence.internal.security.PrivilegedAccessHelper;
@@ -38,8 +39,12 @@
3839
import org.eclipse.persistence.queries.UpdateObjectQuery;
3940

4041
import java.io.Serializable;
42+
import java.lang.reflect.RecordComponent;
43+
import java.util.ArrayList;
4144
import java.util.HashSet;
45+
import java.util.LinkedHashMap;
4246
import java.util.List;
47+
import java.util.Map;
4348
import java.util.Set;
4449

4550
/**
@@ -462,7 +467,10 @@ public Object createPrimaryKeyInstance(Object object, AbstractSession session) {
462467
return fieldValue;
463468
}
464469

465-
Object keyInstance = getPKClassInstance();
470+
Class<?> pkClass = getPKClass();
471+
boolean isRecord = pkClass != null && pkClass.isRecord();
472+
Object keyInstance = isRecord ? null : getPKClassInstance();
473+
Map<String, Object> recordValues = isRecord ? new LinkedHashMap<>() : null;
466474
Set<ObjectReferenceMapping> usedObjectReferenceMappings = new HashSet<>();
467475
for (int index = 0; index < pkElementArray.length; index++) {
468476
Object keyObj = object;
@@ -492,11 +500,35 @@ public Object createPrimaryKeyInstance(Object object, AbstractSession session) {
492500
fieldValue = mapping.getReferenceDescriptor().getCMPPolicy().createPrimaryKeyInstance(fieldValue, session);
493501
usedObjectReferenceMappings.add((ObjectReferenceMapping)mapping);
494502
}
495-
accessor.setValue(nestedKeyInstance, fieldValue);
503+
if (isRecord) {
504+
recordValues.put(accessor.getAttributeName(), fieldValue);
505+
} else {
506+
accessor.setValue(nestedKeyInstance, fieldValue);
507+
}
496508
}
497509
}
498510

499-
return keyInstance;
511+
return isRecord ? createRecordInstance(pkClass, recordValues) : keyInstance;
512+
}
513+
514+
/**
515+
* Build a record primary key class instance from its component values using
516+
* its canonical constructor. Records are immutable, so the default
517+
* no-arg-constructor + field-reflection approach used for bean id classes
518+
* cannot be applied.
519+
*
520+
* Values are looked up by record component name so that the field order in
521+
* the descriptor does not have to match the record component declaration
522+
* order.
523+
*/
524+
private Object createRecordInstance(Class<?> recordClass, Map<String, Object> valuesByName) {
525+
List<Object> values = new ArrayList<>(valuesByName.size());
526+
for (RecordComponent component : recordClass.getRecordComponents()) {
527+
values.add(valuesByName.get(component.getName()));
528+
}
529+
RecordInstantiationPolicy policy = new RecordInstantiationPolicy(recordClass);
530+
policy.setValues(values);
531+
return policy.buildNewInstance();
500532
}
501533

502534
/**

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

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3967,38 +3967,41 @@ public void preInitialize(AbstractSession session) throws DescriptorException {
39673967
// PERF: Check if the class "itself" was weaved.
39683968
// If weaved avoid reflection, use clone copy and empty new.
39693969
if (Arrays.asList(getJavaClass().getInterfaces()).contains(PersistenceObject.class)) {
3970-
// Cloning is only auto set for field access, as method access
3971-
// may not have simple fields, same with empty new and reflection get/set.
3972-
boolean isMethodAccess = false;
3973-
for (DatabaseMapping mapping: getMappings()) {
3974-
if (mapping.isUsingMethodAccess()) {
3975-
// Ok for lazy 1-1s
3976-
if (!mapping.isOneToOneMapping() || !((ForeignReferenceMapping)mapping).usesIndirection()) {
3977-
isMethodAccess = true;
3970+
// Records are immutable and cannot be woven, so they always need
3971+
// record-aware copy and instantiation policies, regardless of access type.
3972+
if (javaClass != null && javaClass.isRecord()) {
3973+
if (this.copyPolicy == null) {
3974+
setCopyPolicy(new RecordCopyPolicy());
3975+
}
3976+
if (!isAbstract() && this.instantiationPolicy == null) {
3977+
setInstantiationPolicy(new RecordInstantiationPolicy(javaClass));
3978+
}
3979+
} else {
3980+
// Cloning is only auto set for field access, as method access
3981+
// may not have simple fields, same with empty new and reflection get/set.
3982+
boolean isMethodAccess = false;
3983+
for (DatabaseMapping mapping: getMappings()) {
3984+
if (mapping.isUsingMethodAccess()) {
3985+
// Ok for lazy 1-1s
3986+
if (!mapping.isOneToOneMapping() || !((ForeignReferenceMapping)mapping).usesIndirection()) {
3987+
isMethodAccess = true;
3988+
}
3989+
} else if (!mapping.isWriteOnly()) {
3990+
// Avoid reflection.
3991+
mapping.setAttributeAccessor(new PersistenceObjectAttributeAccessor(mapping.getAttributeName()));
39783992
}
3979-
} else if (!mapping.isWriteOnly()) {
3980-
// Avoid reflection.
3981-
mapping.setAttributeAccessor(new PersistenceObjectAttributeAccessor(mapping.getAttributeName()));
39823993
}
3983-
}
3984-
if (!isMethodAccess) {
3985-
if (this.copyPolicy == null) {
3986-
if (javaClass != null && javaClass.isRecord()) {
3987-
setCopyPolicy(new RecordCopyPolicy());
3988-
} else {
3994+
if (!isMethodAccess) {
3995+
if (this.copyPolicy == null) {
39893996
setCopyPolicy(new PersistenceEntityCopyPolicy());
39903997
}
3991-
}
3992-
if (!isAbstract()) {
3993-
try {
3994-
if (this.instantiationPolicy == null) {
3995-
if (javaClass != null && javaClass.isRecord()) {
3996-
setInstantiationPolicy(new RecordInstantiationPolicy(javaClass));
3997-
} else {
3998-
setInstantiationPolicy(new PersistenceObjectInstantiationPolicy((PersistenceObject)getJavaClass().getConstructor().newInstance()));
3999-
}
3998+
if (!isAbstract() && this.instantiationPolicy == null) {
3999+
try {
4000+
setInstantiationPolicy(new PersistenceObjectInstantiationPolicy((PersistenceObject)getJavaClass().getConstructor().newInstance()));
4001+
} catch (Exception ignore) {
4002+
// No-arg constructor may not exist.
40004003
}
4001-
} catch (Exception ignore) { }
4004+
}
40024005
}
40034006
}
40044007
}

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

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
/*
2+
* Copyright (c) 2024 Oracle and/or its affiliates. All rights reserved.
3+
*
4+
* This program and the accompanying materials are made available under the
5+
* terms of the Eclipse Public License v. 2.0 which is available at
6+
* http://www.eclipse.org/legal/epl-2.0,
7+
* or the Eclipse Distribution License v. 1.0 which is available at
8+
* http://www.eclipse.org/org/documents/edl-v10.php.
9+
*
10+
* SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause
11+
*/
12+
13+
package org.eclipse.persistence.jpa.test.record;
14+
15+
import jakarta.persistence.EntityManager;
16+
import jakarta.persistence.EntityManagerFactory;
17+
18+
import org.eclipse.persistence.jpa.test.framework.DDLGen;
19+
import org.eclipse.persistence.jpa.test.framework.Emf;
20+
import org.eclipse.persistence.jpa.test.framework.EmfRunner;
21+
import org.eclipse.persistence.jpa.test.record.model.RecordEntity;
22+
import org.eclipse.persistence.jpa.test.record.model.RecordId;
23+
import org.eclipse.persistence.jpa.test.record.model.RecordValue;
24+
import org.junit.Test;
25+
import org.junit.runner.RunWith;
26+
27+
import java.util.UUID;
28+
29+
import static org.junit.Assert.assertEquals;
30+
import static org.junit.Assert.assertNotNull;
31+
import static org.junit.Assert.assertTrue;
32+
33+
/**
34+
* Tests for {@code java.lang.Record} used as JPA {@code @Embeddable} types.
35+
* <p>
36+
* This test covers:
37+
* <ul>
38+
* <li>{@code @EmbeddedId} with a record type</li>
39+
* <li>{@code @Embedded} attribute with a record type</li>
40+
* <li>Persist, find, and merge operations</li>
41+
* </ul>
42+
* <p>
43+
* Prior to the fix for <a href="https://github.com/eclipse-ee4j/eclipselink/issues/2656">#2656</a>,
44+
* EclipseLink threw {@code IllegalAccessException: Can not set final field} when
45+
* method-accessed embeddable records were processed. The root cause was that
46+
* {@code RecordCopyPolicy} and {@code RecordInstantiationPolicy} were only
47+
* initialized for field-accessed descriptors, not method-accessed ones.
48+
*/
49+
@RunWith(EmfRunner.class)
50+
public class TestRecordEmbeddable {
51+
52+
@Emf(createTables = DDLGen.DROP_CREATE, classes = {
53+
RecordEntity.class, RecordId.class, RecordValue.class })
54+
private EntityManagerFactory emf;
55+
56+
@Test
57+
public void testPersistAndFind() {
58+
EntityManager em = emf.createEntityManager();
59+
try {
60+
em.getTransaction().begin();
61+
62+
UUID uuid = UUID.randomUUID();
63+
RecordId id = new RecordId(uuid);
64+
RecordValue value = new RecordValue("test-description", 42);
65+
RecordEntity entity = new RecordEntity(id, "test-entity", value);
66+
67+
em.persist(entity);
68+
em.getTransaction().commit();
69+
70+
// Clear to force a fresh read from the database
71+
em.clear();
72+
73+
RecordEntity found = em.find(RecordEntity.class, id);
74+
assertNotNull("Entity should be found after persist", found);
75+
assertEquals("test-entity", found.getName());
76+
assertNotNull("Embedded value should not be null", found.getValue());
77+
assertEquals("test-description", found.getValue().description());
78+
assertEquals(42, found.getValue().amount());
79+
} finally {
80+
if (em.getTransaction().isActive()) {
81+
em.getTransaction().rollback();
82+
}
83+
em.close();
84+
}
85+
}
86+
87+
@Test
88+
public void testMerge() {
89+
EntityManager em = emf.createEntityManager();
90+
try {
91+
// Persist
92+
em.getTransaction().begin();
93+
UUID uuid = UUID.randomUUID();
94+
RecordId id = new RecordId(uuid);
95+
RecordValue value = new RecordValue("original", 10);
96+
RecordEntity entity = new RecordEntity(id, "original-name", value);
97+
em.persist(entity);
98+
em.getTransaction().commit();
99+
em.clear();
100+
101+
// Merge with updated values
102+
em.getTransaction().begin();
103+
RecordValue updatedValue = new RecordValue("updated", 20);
104+
RecordEntity updated = new RecordEntity(id, "updated-name", updatedValue);
105+
RecordEntity merged = em.merge(updated);
106+
em.getTransaction().commit();
107+
108+
assertNotNull("Merged entity should not be null", merged);
109+
assertEquals("updated-name", merged.getName());
110+
assertEquals("updated", merged.getValue().description());
111+
assertEquals(20, merged.getValue().amount());
112+
} finally {
113+
if (em.getTransaction().isActive()) {
114+
em.getTransaction().rollback();
115+
}
116+
em.close();
117+
}
118+
}
119+
120+
@Test
121+
public void testGetIdentifier() {
122+
UUID uuid = UUID.randomUUID();
123+
RecordId id = new RecordId(uuid);
124+
RecordEntity entity = new RecordEntity(id, "test-entity", new RecordValue("desc", 1));
125+
126+
Object identifier = emf.getPersistenceUnitUtil().getIdentifier(entity);
127+
128+
assertNotNull("Identifier should not be null", identifier);
129+
assertTrue("Identifier should be a RecordId", identifier instanceof RecordId);
130+
assertEquals("Identifier should equal the entity's embedded id", id, identifier);
131+
}
132+
133+
@Test
134+
public void testPersistAndFindWithNoArgConstructor() {
135+
EntityManager em = emf.createEntityManager();
136+
try {
137+
em.getTransaction().begin();
138+
139+
// Mirror the original #2656 reproduction: the @EmbeddedId record
140+
// is created through its custom no-arg constructor.
141+
RecordId id = new RecordId();
142+
RecordValue value = new RecordValue("no-arg-description", 7);
143+
RecordEntity entity = new RecordEntity(id, "no-arg-entity", value);
144+
145+
em.persist(entity);
146+
em.getTransaction().commit();
147+
em.clear();
148+
149+
RecordEntity found = em.find(RecordEntity.class, id);
150+
assertNotNull("Entity should be found after persist", found);
151+
assertEquals("no-arg-entity", found.getName());
152+
assertEquals("no-arg-description", found.getValue().description());
153+
} finally {
154+
if (em.getTransaction().isActive()) {
155+
em.getTransaction().rollback();
156+
}
157+
em.close();
158+
}
159+
}
160+
}

0 commit comments

Comments
 (0)