Fix accessing superclass fields in checkExists() (#799)

* Fix accessing superclass fields in checkExists()

JpaTransactionManagerImpl doesn't respect @Id fields in mapped superclasses.
Replace calls to getDeclaredId() and getDeclaredField() with superclass
friendly counterparts.
This commit is contained in:
Michael Muller 2020-09-11 13:45:51 -04:00 committed by GitHub
parent baa59b1f55
commit 86565f237d
2 changed files with 38 additions and 7 deletions

View file

@ -391,7 +391,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
private static ImmutableSet<EntityId> getEntityIdsFromEntity(
EntityType<?> entityType, Object entity) {
if (entityType.hasSingleIdAttribute()) {
String idName = entityType.getDeclaredId(entityType.getIdType().getJavaType()).getName();
String idName = entityType.getId(entityType.getIdType().getJavaType()).getName();
Object idValue = getFieldValue(entity, idName);
return ImmutableSet.of(new EntityId(idName, idValue));
} else {
@ -402,7 +402,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
private static ImmutableSet<EntityId> getEntityIdsFromSqlKey(
EntityType<?> entityType, Object sqlKey) {
if (entityType.hasSingleIdAttribute()) {
String idName = entityType.getDeclaredId(entityType.getIdType().getJavaType()).getName();
String idName = entityType.getId(entityType.getIdType().getJavaType()).getName();
return ImmutableSet.of(new EntityId(idName, sqlKey));
} else {
return getEntityIdsFromIdContainer(entityType, sqlKey);
@ -429,7 +429,7 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
private static Object getFieldValue(Object object, String fieldName) {
try {
Field field = object.getClass().getDeclaredField(fieldName);
Field field = getField(object.getClass(), fieldName);
field.setAccessible(true);
return field.get(object);
} catch (NoSuchFieldException | IllegalAccessException e) {
@ -437,6 +437,21 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
}
}
/** Gets the field definition from clazz or any superclass. */
private static Field getField(Class clazz, String fieldName) throws NoSuchFieldException {
try {
// Note that we have to use getDeclaredField() for this, getField() just finds public fields.
return clazz.getDeclaredField(fieldName);
} catch (NoSuchFieldException e) {
Class base = clazz.getSuperclass();
if (base != null) {
return getField(base, fieldName);
} else {
throw e;
}
}
}
private static class TransactionInfo {
EntityManager entityManager;
boolean inTransaction = false;

View file

@ -37,6 +37,8 @@ import java.util.List;
import java.util.NoSuchElementException;
import java.util.Set;
import java.util.stream.Stream;
import javax.persistence.Embeddable;
import javax.persistence.MappedSuperclass;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.TestTemplate;
import org.junit.jupiter.api.extension.RegisterExtension;
@ -65,7 +67,7 @@ public class TransactionManagerTest {
.withClock(fakeClock)
.withDatastoreAndCloudSql()
.withOfyTestEntities(TestEntity.class)
.withJpaUnitTestEntities(TestEntity.class)
.withJpaUnitTestEntities(TestEntity.class, TestEntityBase.class)
.build();
TransactionManagerTest() {}
@ -324,17 +326,31 @@ public class TransactionManagerTest {
entities.forEach(TransactionManagerTest::assertEntityNotExist);
}
/**
* We put the id field into a base class to test that id fields can be discovered in a base class.
*/
@MappedSuperclass
@Embeddable
private static class TestEntityBase extends ImmutableObject {
@Id @javax.persistence.Id protected String name;
TestEntityBase(String name) {
this.name = name;
}
TestEntityBase() {}
}
@Entity(name = "TxnMgrTestEntity")
@javax.persistence.Entity(name = "TestEntity")
private static class TestEntity extends ImmutableObject {
@Id @javax.persistence.Id private String name;
private static class TestEntity extends TestEntityBase {
private String data;
private TestEntity() {}
private TestEntity(String name, String data) {
this.name = name;
super(name);
this.data = data;
}