Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip insert/update for @GeneratedValue annotated fields in @Embedded #3344

Merged
merged 8 commits into from
Mar 18, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package io.micronaut.data.jdbc.h2.embeddedAssociation
import io.micronaut.context.ApplicationContext
import io.micronaut.data.annotation.*
import io.micronaut.data.annotation.repeatable.JoinSpecifications
import io.micronaut.data.connection.jdbc.advice.DelegatingDataSource
import io.micronaut.data.jdbc.annotation.JdbcRepository
import io.micronaut.data.jdbc.h2.H2DBProperties
import io.micronaut.data.jdbc.h2.H2TestPropertyProvider
Expand All @@ -11,6 +12,7 @@ import io.micronaut.data.model.Pageable
import io.micronaut.data.model.Sort
import io.micronaut.data.model.query.builder.sql.Dialect
import io.micronaut.data.repository.CrudRepository
import io.micronaut.data.repository.GenericRepository
import io.micronaut.data.repository.jpa.JpaSpecificationExecutor
import io.micronaut.data.repository.jpa.criteria.PredicateSpecification
import io.micronaut.data.tck.entities.Order
Expand All @@ -21,6 +23,8 @@ import spock.lang.Specification

import jakarta.inject.Inject

import javax.sql.DataSource

@MicronautTest
@H2DBProperties
class EmbeddedAssociationJoinSpec extends Specification implements H2TestPropertyProvider {
Expand All @@ -40,6 +44,29 @@ class EmbeddedAssociationJoinSpec extends Specification implements H2TestPropert
@Inject
OneMainEntityEmRepository oneMainEntityEmRepository = applicationContext.getBean(OneMainEntityEmRepository)

@Shared
@Inject
MyMainEntityRepository myMainEntityRepository = applicationContext.getBean(MyMainEntityRepository)

void setup() {
def dataSource = DelegatingDataSource.unwrapDataSource(applicationContext.getBean(DataSource))
def connection = dataSource.connection
connection.prepareStatement("DROP TABLE IF EXISTS `my_main_entity`").execute()
connection.prepareStatement("""
CREATE TABLE `my_main_entity` (
`id` bigint primary key not null,
`value` text,
`example` text,
`part_text` text);
""").execute()
}

void cleanup() {
def dataSource = DelegatingDataSource.unwrapDataSource(applicationContext.getBean(DataSource))
def connection = dataSource.connection
connection.prepareStatement("DROP TABLE IF EXISTS `my_main_entity`")
}

void 'test one-to-one update'() {
given:
ChildEntity child = new ChildEntity(name: "child")
Expand Down Expand Up @@ -122,6 +149,41 @@ class EmbeddedAssociationJoinSpec extends Specification implements H2TestPropert
oem.id.one.em.assoc[0].name == "C"
oem.id.one.em.assoc[1].name == "D"
}

void 'test save/update embedded with @GeneratedValue'() {
when:"should not update field 'example'"
myMainEntityRepository.save(new MyMainEntity(id: 1L, example: "Test", value: "Val"))
def persistedEntity = myMainEntityRepository.findById(1L).orElse(null)
then:
persistedEntity
persistedEntity.value == "Val"
!persistedEntity.example
when:
myMainEntityRepository.update(new MyMainEntity(id: 1L, example: "Changed", value: "Val-Changed"))
def updatedEntity = myMainEntityRepository.findById(1L).orElse(null)
then:
updatedEntity
updatedEntity.value == "Val-Changed"
!updatedEntity.example

when:"should not update field 'part_text'"
myMainEntityRepository.save(new MyMainEntity(id: 2L, value: "Val1", part: new MyPart(text: "Test")))
persistedEntity = myMainEntityRepository.findById(2L).orElse(null)
then:
persistedEntity
persistedEntity.value == "Val1"
!persistedEntity.part.text
when:
myMainEntityRepository.update(new MyMainEntity(id: 2L, value: "Val2", part: new MyPart(text: "Changed")))
updatedEntity = myMainEntityRepository.findById(2L).orElse(null)
then:
updatedEntity
updatedEntity.value == "Val2"
!updatedEntity.part.text

cleanup:
myMainEntityRepository.deleteAll()
}
}

@JdbcRepository(dialect = Dialect.H2)
Expand Down Expand Up @@ -218,3 +280,35 @@ class MainEntityAssociation {
Long id
String name
}

@MappedEntity("my_main_entity")
class MyMainEntity {

@Id
Long id

@GeneratedValue
String example

String value

@Relation(value = Relation.Kind.EMBEDDED)
MyPart part = new MyPart()
}

@Embeddable
class MyPart {
@GeneratedValue
String text
}

@JdbcRepository(dialect = Dialect.H2)
interface MyMainEntityRepository extends GenericRepository<MyMainEntity, Long> {
Optional<MyMainEntity> findById(Long id)

MyMainEntity save(MyMainEntity entity)

MyMainEntity update(MyMainEntity entity)

void deleteAll()
}
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,28 @@ public static Optional<String> getPersistentPropertyPath(PersistentEntity entity
return entity.getPath(path);
}

/**
* Checks whether a given property is considered generated based on its annotations and relationship with its owner property.
*
* @param entity the persistent entity that owns the property
* @param ownerProperty the property that owns the given property. This means
* when we are doing traversal in case it is an association. If not
* an association then ownerProperty will be the same as property.
* @param property the property to check
* @return true if the property is considered generated, false otherwise
*/
public static boolean isPropertyGenerated(PersistentEntity entity, PersistentProperty ownerProperty, PersistentProperty property) {
boolean generated = property.isGenerated();
if (generated) {
if (ownerProperty instanceof Association association) {
generated = association.isEmbedded();
} else if (!entity.equals(property.getOwner())) {
generated = false;
}
}
return generated;
}

private static PersistentProperty getJoinColumnAssocIdentity(PersistentProperty property, PersistentEntity associatedEntity) {
AnnotationMetadata propertyAnnotationMetadata = property.getAnnotationMetadata();
AnnotationValue<JoinColumns> joinColumnsAnnotationValue = propertyAnnotationMetadata.getAnnotation(JoinColumns.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1631,6 +1631,13 @@ public int getParameterIndex() {
QueryPropertyPath propertyPath = entry.getKey();
if (entry.getValue() instanceof BindingParameter bindingParameter) {
traversePersistentProperties(propertyPath.getAssociations(), propertyPath.getProperty(), (associations, property) -> {

boolean generated = PersistentEntityUtils.isPropertyGenerated(entity,
propertyPath.getProperty(), property);
if (generated) {
return;
}

String tableAlias = propertyPath.getTableAlias();
if (tableAlias != null) {
queryString.append(tableAlias).append(DOT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,13 @@ public JsonDataType getJsonDataType() {
QueryPropertyPath propertyPath = entry.getKey();
if (entry.getValue() instanceof BindingParameter bindingParameter) {
PersistentEntityUtils.traversePersistentProperties(propertyPath.getPropertyPath(), traverseEmbedded(), (associations, property) -> {

boolean generated = PersistentEntityUtils.isPropertyGenerated(entity,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is code that filters persistent properties before building update query, but one of these are embedded and we need to check each fields inside embedded here.

propertyPath.getProperty(), property);
if (generated) {
return;
}

String tableAlias = propertyPath.getTableAlias();
if (tableAlias != null) {
queryString.append(tableAlias).append(DOT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import io.micronaut.data.model.Pageable;
import io.micronaut.data.model.Pageable.Mode;
import io.micronaut.data.model.PersistentEntity;
import io.micronaut.data.model.PersistentEntityUtils;
import io.micronaut.data.model.PersistentProperty;
import io.micronaut.data.model.PersistentPropertyPath;
import io.micronaut.data.model.naming.NamingStrategy;
Expand Down Expand Up @@ -1026,7 +1027,8 @@ public int getParameterIndex() {

for (PersistentProperty prop : persistentProperties) {
traversePersistentProperties(prop, (associations, property) -> {
if (prop.isGenerated()) {
boolean generated = PersistentEntityUtils.isPropertyGenerated(entity, prop, property);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we somehow fix the prop.isGenerated to return true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does not look like. We have for example Author(id) which is generated in context of Author entity insert/update. But, when doing Book insert/update then it is not generated and it needs to be updatable. This is why we need more context to check if field is generated ie. involved in insert/update.

if (generated) {
String columnName = getMappedName(namingStrategy, associations, property);
if (escape) {
columnName = quote(columnName);
Expand All @@ -1035,7 +1037,7 @@ public int getParameterIndex() {
return;
}

addWriteExpression(values, prop);
addWriteExpression(values, property);

String key = String.valueOf(values.size());
String[] path = asStringPath(associations, property);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -861,7 +861,8 @@ public JsonDataType getJsonDataType() {

for (PersistentProperty prop : persistentProperties) {
PersistentEntityUtils.traversePersistentProperties(Collections.emptyList(), prop, (associations, property) -> {
if (prop.isGenerated()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this check was not valid
if (prop.isGenerated()) {
because prop is property being traversed (either single property or association) and I think we should check property that is result of prop traversal.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right

boolean generated = PersistentEntityUtils.isPropertyGenerated(entity, prop, property);
if (generated) {
String columnName = getMappedName(namingStrategy, associations, property);
if (escape) {
columnName = quote(columnName);
Expand All @@ -870,7 +871,7 @@ public JsonDataType getJsonDataType() {
return;
}

addWriteExpression(values, prop);
addWriteExpression(values, property);

String key = String.valueOf(values.size());
String[] path = asStringPath(associations, property);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@
*/
package io.micronaut.data.tck.entities;

import io.micronaut.data.annotation.GeneratedValue;
import io.micronaut.data.annotation.AutoPopulated;
import io.micronaut.data.annotation.Id;
import io.micronaut.data.annotation.MappedEntity;

import java.util.UUID;

@MappedEntity
public class UuidChildEntity {
@GeneratedValue
@AutoPopulated
@Id
private UUID uuid;

Expand Down
Loading