From d1766018fd20ee1a552a0bdd11edef5e616df123 Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Mon, 8 Jan 2024 17:22:23 +0100 Subject: [PATCH] HHH-17623 Test and fix use of association in @OrderBy --- .../mapping/internal/AbstractDomainPath.java | 16 +- .../ast/FkDomainPathContinuation.java | 33 ++- .../orm/test/orderby/OrderByToOneTest.java | 204 ++++++++++++++++++ 3 files changed, 221 insertions(+), 32 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/orderby/OrderByToOneTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/AbstractDomainPath.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/AbstractDomainPath.java index 085fafc63b..a0058ef080 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/AbstractDomainPath.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/AbstractDomainPath.java @@ -12,6 +12,7 @@ import java.util.List; import org.hibernate.metamodel.mapping.BasicValuedModelPart; import org.hibernate.metamodel.mapping.EmbeddableValuedModelPart; import org.hibernate.metamodel.mapping.EntityValuedModelPart; +import org.hibernate.metamodel.mapping.ForeignKeyDescriptor; import org.hibernate.metamodel.mapping.ModelPart; import org.hibernate.metamodel.mapping.SelectableMapping; import org.hibernate.metamodel.mapping.ordering.ast.DomainPath; @@ -163,19 +164,8 @@ public abstract class AbstractDomainPath implements DomainPath { subPart = ( (EntityValuedModelPart) referenceModelPart ).getEntityMappingType().getIdentifierMapping(); } else { - subPart = ( (EntityValuedModelPart) referenceModelPart ).findSubPart( modelPartName ); - if ( subPart == null && referenceModelPart instanceof ToOneAttributeMapping ) { - // this is the case of sort by to-one attribute inside an embedded item, - // at this stage the foreign key descriptor should have been set on the attribute mapping, - // so we can generate a sub part valid for the order-by generation - ToOneAttributeMapping toOneAttribute = (ToOneAttributeMapping) referenceModelPart; - String foreignKeyModelPart = toOneAttribute.getAttributeName() + "." - + toOneAttribute.getTargetKeyPropertyName(); - - if ( modelPartName.equals( foreignKeyModelPart ) ) { - subPart = toOneAttribute.findSubPart( toOneAttribute.getTargetKeyPropertyName() ); - } - } + // Default to using the foreign key of an entity valued model part + subPart = ( (EntityValuedModelPart) referenceModelPart ).findSubPart( ForeignKeyDescriptor.PART_NAME ); } apply( subPart, diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/ordering/ast/FkDomainPathContinuation.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/ordering/ast/FkDomainPathContinuation.java index 8fedb5591f..571a69bbf3 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/ordering/ast/FkDomainPathContinuation.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/ordering/ast/FkDomainPathContinuation.java @@ -21,15 +21,18 @@ public class FkDomainPathContinuation extends DomainPathContinuation { private final Set possiblePaths; public FkDomainPathContinuation( - NavigablePath navigablePath, DomainPath lhs, + NavigablePath navigablePath, + DomainPath lhs, ToOneAttributeMapping referencedModelPart) { super( navigablePath, lhs, referencedModelPart ); this.possiblePaths = referencedModelPart.getTargetKeyPropertyNames(); } public FkDomainPathContinuation( - NavigablePath navigablePath, DomainPath lhs, - ModelPart referencedModelPart, Set possiblePaths) { + NavigablePath navigablePath, + DomainPath lhs, + ModelPart referencedModelPart, + Set possiblePaths) { super( navigablePath, lhs, referencedModelPart ); this.possiblePaths = possiblePaths; } @@ -40,25 +43,17 @@ public class FkDomainPathContinuation extends DomainPathContinuation { String identifier, boolean isTerminal, TranslationContext translationContext) { - HashSet furtherPaths = new LinkedHashSet<>( possiblePaths.size() ); - for ( String path : possiblePaths ) { - if ( !path.startsWith( name ) ) { - return new DomainPathContinuation( - navigablePath.append( name ), - this, - // unfortunately at this stage the foreign key descriptor could not be set - // on the attribute mapping yet, so we need to defer the sub part extraction later - referencedModelPart - ); - } - - furtherPaths.add( path.substring( name.length() + 1 ) ); - } - - if ( furtherPaths.isEmpty() ) { + if ( !possiblePaths.contains( name ) ) { throw new PathResolutionException( "Domain path of type `" + referencedModelPart.getPartMappingType() + "` -> `" + name + "`" ); } + final HashSet furtherPaths = new HashSet<>(); + for ( String possiblePath : possiblePaths ) { + if ( possiblePath.startsWith( name ) && possiblePath.length() > name.length() + && possiblePath.charAt( name.length() ) == '.' ) { + furtherPaths.add( possiblePath.substring( name.length() + 2 ) ); + } + } return new FkDomainPathContinuation( navigablePath.append( name ), this, diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/orderby/OrderByToOneTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/orderby/OrderByToOneTest.java new file mode 100644 index 0000000000..38eac1f579 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/orderby/OrderByToOneTest.java @@ -0,0 +1,204 @@ +package org.hibernate.orm.test.orderby; + +import java.util.ArrayList; +import java.util.List; + +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.JiraKey; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import jakarta.persistence.CascadeType; +import jakarta.persistence.Entity; +import jakarta.persistence.FetchType; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; +import jakarta.persistence.JoinColumn; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.OneToMany; +import jakarta.persistence.OrderBy; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * @author Christian Beikov + */ +@DomainModel( + annotatedClasses = { + OrderByToOneTest.Task.class, + OrderByToOneTest.TaskVersion.class, + OrderByToOneTest.User.class + } +) +@SessionFactory +public class OrderByToOneTest { + + @BeforeEach + protected void prepareTest(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + User u1 = new User( 1L, "u1" ); + session.persist( u1 ); + User u2 = new User( 2L, "u2" ); + session.persist( u2 ); + + Task t = new Task(); + t.setId( 1L ); + TaskVersion tv1 = new TaskVersion(); + tv1.setName( "tv1" ); + tv1.setAssignee( u2 ); + List versions = new ArrayList<>(); + versions.add( tv1 ); + t.setTaskVersions( versions ); + tv1.setTask( t ); + + TaskVersion tv2 = new TaskVersion(); + tv2.setName( "tv2" ); + tv2.setAssignee( u1 ); + t.getTaskVersions().add( tv2 ); + tv2.setTask( t ); + session.persist( t ); + } + ); + } + + @AfterEach + protected void cleanupTest(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + session.createMutationQuery( "delete from TaskVersion" ).executeUpdate(); + session.createMutationQuery( "delete from UUser" ).executeUpdate(); + session.createMutationQuery( "delete from Task" ).executeUpdate(); + } + ); + } + + @Test + @JiraKey("HHH-17623") + public void testOrderByToOne(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + final Task task = session.createQuery( "from Task t", Task.class ).getSingleResult(); + final List taskVersions = task.getTaskVersions(); + assertEquals( 2, taskVersions.size() ); + assertEquals( "tv2", taskVersions.get( 0 ).getName() ); + assertEquals( "tv1", taskVersions.get( 1 ).getName() ); + } + ); + } + + @Entity(name = "Task") + public static class Task { + + @Id + private Long id; + + @OneToMany(mappedBy = "task", cascade = CascadeType.ALL, fetch = FetchType.EAGER) + @OrderBy("assignee ASC") + private List taskVersions; + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public List getTaskVersions() { + return taskVersions; + } + + public void setTaskVersions(List taskVersions) { + this.taskVersions = taskVersions; + } + } + + @Entity(name = "TaskVersion") + public static class TaskVersion { + + @Id + @GeneratedValue + private Long id; + + @ManyToOne(fetch = FetchType.EAGER) + @JoinColumn(name = "assignee", nullable = true) + private User assignee; + + @ManyToOne(fetch = FetchType.EAGER) + @JoinColumn(name = "task_id", nullable = false) + private Task task; + + private String name; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public Task getTask() { + return task; + } + + public void setTask(Task task) { + this.task = task; + } + + public User getAssignee() { + return assignee; + } + + public void setAssignee(User assignee) { + this.assignee = assignee; + } + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + } + + @Entity(name = "UUser") + public static class User { + + @Id + private Long id; + + private String name; + + public User() { + } + + public User(Long id, String name) { + this.id = id; + this.name = name; + } + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } + +}