Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ else if ( !generator.generatesOnInsert() ) {
// the @PrePersist callback to happen first
generatedId = null;
}
else if ( generatedBeforeExecution ) {
else if ( generatedBeforeExecution && ( ! generator.allowAssignedIdentifiers() || persister.getIdentifier( entity, source ) == null ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

While I agree with the scope of the change, I don't believe this is the correct way to do it. This way, when allowAssignedIdentifier returns true and there is a non-null value we do not call generate at all.

I would say we want to still invoke your custom BeforeExecutionGenerator though, and pass the existing identifier through the currentValue parameter.

Copy link
Member

Choose a reason for hiding this comment

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

I would say we want to still invoke your custom BeforeExecutionGenerator though, and pass the existing identifier through the currentValue parameter.

Yes, I agree. I think the bug is that currentValue is null when it shouldn't be.

Copy link
Member

Choose a reason for hiding this comment

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

I think the bug is that currentValue is null when it shouldn't be.

[That's what I was trying to say in the comment I left on the issue, though now I realize my comment wasn't very clear.]

// go ahead and generate id, and then set it to
// the entity instance, so it will be available
// to the entity in the @PrePersist callback
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
* SPDX-License-Identifier: Apache-2.0
* Copyright Red Hat Inc. and Hibernate Authors
*/
package org.hibernate.orm.test.idgen.userdefined;

import jakarta.persistence.Entity;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.Id;
import jakarta.persistence.Table;
import org.hibernate.annotations.IdGeneratorType;
import org.hibernate.annotations.ValueGenerationType;
import org.hibernate.engine.spi.SharedSessionContractImplementor;
import org.hibernate.generator.BeforeExecutionGenerator;
import org.hibernate.generator.EventType;
import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.SessionFactory;
import org.hibernate.testing.orm.junit.SessionFactoryScope;
import org.junit.jupiter.api.Test;

import java.lang.annotation.Retention;
import java.lang.annotation.Target;
import java.util.EnumSet;
import java.util.UUID;

import static java.lang.annotation.ElementType.FIELD;
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import static org.assertj.core.api.Assertions.assertThat;

@SessionFactory
@DomainModel(annotatedClasses = BeforeExecutionGeneratorWithAssignedIdentifiersTest.Book.class)
class BeforeExecutionGeneratorWithAssignedIdentifiersTest {

@Test
void testAssignedValueNotOverridden(SessionFactoryScope scope) {
final String assignedId = "assigned-id";
final Book book = new Book();
book.id = assignedId;
scope.inTransaction( session -> session.persist( book ) );
assertThat( book.id ).isEqualTo( assignedId );
}

@Entity
@Table(name = "books")
static class Book {
@Id @GeneratedValue
@AssignableIdGenerator
String id;
}

@IdGeneratorType(IdGenerator.class)
@ValueGenerationType(generatedBy = IdGenerator.class)
@Retention(RUNTIME)
@Target({FIELD, METHOD})
public @interface AssignableIdGenerator {}

public static class IdGenerator implements BeforeExecutionGenerator {

@Override
public Object generate(
SharedSessionContractImplementor session,
Object owner,
Object currentValue,
EventType eventType) {
if ( currentValue != null ) {
return currentValue;
}
return UUID.randomUUID().toString();
}

@Override
public EnumSet<EventType> getEventTypes() {
return EnumSet.of( EventType.INSERT );
}

@Override
public boolean allowAssignedIdentifiers() {
return true;
}
}
}