Skip to content

Commit 1f57aae

Browse files
gavinkinggavinking
committed
HHH-19364 simplify generated code to take better advantage of Specification
1. don't build a List of Orders when using Specification 2. use Specification.createQuery() if possible (non-reactive case)
1 parent 72e5c54 commit 1f57aae

File tree

4 files changed

+96
-58
lines changed

4 files changed

+96
-58
lines changed

‎tooling/metamodel-generator/src/main/java/org/hibernate/processor/annotation/AbstractCriteriaMethod.java

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -80,19 +80,28 @@ String specificationType() {
8080

8181
@Override
8282
void createQuery(StringBuilder declaration) {
83-
declaration
84-
.append(localSessionName())
85-
.append(".")
86-
.append(createQueryMethod())
87-
.append('(');
88-
if (isUsingSpecification() ) {
83+
final boolean specification = isUsingSpecification();
84+
if ( specification && !isReactive() ) {
8985
declaration
90-
.append("_spec.buildCriteria(_builder)");
86+
.append("_spec.createQuery(")
87+
.append(localSessionName())
88+
.append(")\n");
9189
}
9290
else {
93-
declaration.append("_query");
91+
declaration
92+
.append(localSessionName())
93+
.append(".")
94+
.append(createQueryMethod())
95+
.append('(');
96+
if ( specification ) {
97+
declaration
98+
.append("_spec.buildCriteria(_builder)");
99+
}
100+
else {
101+
declaration.append("_query");
102+
}
103+
declaration.append(")\n");
94104
}
95-
declaration.append(")\n");
96105
}
97106

98107
@Override

‎tooling/metamodel-generator/src/main/java/org/hibernate/processor/annotation/AbstractQueryMethod.java

Lines changed: 58 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,11 @@ public String getPropertyName() {
9494

9595
abstract boolean isNullable(int index);
9696

97+
boolean initiallyUnwrapped() {
98+
return !isUsingEntityManager() // a TypedQuery from EntityManager is not a SelectionQuery
99+
|| isUsingSpecification() && !isReactive(); // SelectionSpecification.createQuery() returns SelectionQuery
100+
}
101+
97102
List<String> parameterTypes() {
98103
return paramTypes.stream()
99104
.map(paramType -> isOrderParam(paramType) && paramType.endsWith("[]")
@@ -275,19 +280,6 @@ else if ( jakartaPageRequest ) {
275280
}
276281
}
277282

278-
void handleSorting(
279-
StringBuilder declaration, List<String> paramTypes,
280-
@Nullable String containerType) {
281-
if ( !isJakartaCursoredPage(containerType)
282-
&& hasOrdering(paramTypes) ) {
283-
if ( !isUsingSpecification() ) {
284-
throw new AssertionError();
285-
}
286-
declaration
287-
.append("\t_spec.resort(_orders);\n");
288-
}
289-
}
290-
291283
void handlePageParameters(
292284
StringBuilder declaration, List<String> paramTypes,
293285
@Nullable String containerType) {
@@ -547,22 +539,34 @@ private void totalResults(StringBuilder declaration, List<String> paramTypes) {
547539
}
548540
}
549541

550-
void collectOrdering(StringBuilder declaration, List<String> paramTypes) {
542+
void collectOrdering(StringBuilder declaration, List<String> paramTypes, @Nullable String containerType) {
551543
if ( hasOrdering(paramTypes) ) {
552544
if ( returnTypeName != null ) {
553-
declaration
554-
.append("\tvar _orders = new ")
555-
.append(annotationMetaEntity.importType("java.util.ArrayList"))
556-
.append("<")
557-
.append(annotationMetaEntity.importType(HIB_ORDER))
558-
.append("<? super ")
559-
.append(annotationMetaEntity.importType(returnTypeName))
560-
.append(">>();\n");
545+
final boolean cursoredPage = isJakartaCursoredPage( containerType );
546+
final String add;
547+
if ( cursoredPage ) {
548+
// we need to collect them together in a List
549+
declaration
550+
.append("\tvar _orders = new ")
551+
.append(annotationMetaEntity.importType("java.util.ArrayList"))
552+
.append("<")
553+
.append(annotationMetaEntity.importType(HIB_ORDER))
554+
.append("<? super ")
555+
.append(annotationMetaEntity.importType(returnTypeName))
556+
.append(">>();\n");
557+
add = "_orders.add";
558+
}
559+
else {
560+
add = "_spec.sort";
561+
}
562+
561563
// static orders declared using @OrderBy must come first
562564
for ( OrderBy orderBy : orderBys ) {
563565
annotationMetaEntity.staticImport(HIB_SORT_DIRECTION, "*");
564566
declaration
565-
.append("\t_orders.add(")
567+
.append("\t")
568+
.append(add)
569+
.append('(')
566570
.append(annotationMetaEntity.staticImport(HIB_ORDER, orderBy.descending ? "desc" : "asc"))
567571
.append('(')
568572
.append(annotationMetaEntity.importType(returnTypeName))
@@ -585,28 +589,46 @@ void collectOrdering(StringBuilder declaration, List<String> paramTypes) {
585589
.append("\tfor (var _sort : ")
586590
.append(name)
587591
.append(") {\n")
588-
.append("\t\t_orders.add(_sort);\n")
592+
.append("\t\t")
593+
.append(add)
594+
.append("(_sort);\n")
589595
.append("\t}\n");
590596
}
591597
else if ( type.startsWith(HIB_ORDER) ) {
592598
declaration
593-
.append("\t_orders.add(")
599+
.append("\t")
600+
.append(add)
601+
.append('(')
594602
.append(name)
595603
.append(");\n");
596604
}
597605
else if ( type.startsWith(LIST + "<" + HIB_ORDER) ) {
598-
declaration
599-
.append("\t_orders.addAll(")
600-
.append(name)
601-
.append(");\n");
606+
if ( cursoredPage ) {
607+
declaration
608+
.append("\t_orders.addAll(")
609+
.append(name)
610+
.append(");\n");
611+
}
612+
else {
613+
declaration
614+
.append("\tfor (var _sort : ")
615+
.append(name)
616+
.append(") {\n")
617+
.append("\t\t")
618+
.append(add)
619+
.append("(_sort);\n")
620+
.append("\t}\n");
621+
}
602622
}
603623
else if ( type.startsWith(JD_ORDER) ) {
604624
annotationMetaEntity.staticImport(HIB_SORT_DIRECTION, "*");
605625
declaration
606626
.append("\tfor (var _sort : ")
607627
.append(name)
608628
.append(".sorts()) {\n")
609-
.append("\t\t_orders.add(")
629+
.append("\t\t")
630+
.append(add)
631+
.append('(')
610632
.append(annotationMetaEntity.staticImport(HIB_ORDER, "asc"))
611633
.append('(')
612634
.append(annotationMetaEntity.importType(returnTypeName))
@@ -624,7 +646,9 @@ else if ( type.startsWith(JD_SORT) && type.endsWith("...") ) {
624646
.append("\tfor (var _sort : ")
625647
.append(name)
626648
.append(") {\n")
627-
.append("\t\t_orders.add(")
649+
.append("\t\t")
650+
.append(add)
651+
.append('(')
628652
.append(annotationMetaEntity.staticImport(HIB_ORDER, "asc"))
629653
.append('(')
630654
.append(annotationMetaEntity.importType(returnTypeName))
@@ -638,7 +662,9 @@ else if ( type.startsWith(JD_SORT) && type.endsWith("...") ) {
638662
else if ( type.startsWith(JD_SORT) ) {
639663
annotationMetaEntity.staticImport(HIB_SORT_DIRECTION, "*");
640664
declaration
641-
.append("\t_orders.add(")
665+
.append("\t")
666+
.append(add)
667+
.append('(')
642668
.append(annotationMetaEntity.staticImport(HIB_ORDER, "asc"))
643669
.append('(')
644670
.append(annotationMetaEntity.importType(returnTypeName))

‎tooling/metamodel-generator/src/main/java/org/hibernate/processor/annotation/CriteriaFinderMethod.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,12 @@ void executeQuery(StringBuilder declaration, List<String> paramTypes) {
5656
.append('\n');
5757
createSpecification( declaration );
5858
handleRestrictionParameters( declaration, paramTypes );
59-
collectOrdering( declaration, paramTypes );
60-
handleSorting( declaration, paramTypes, containerType );
59+
collectOrdering( declaration, paramTypes, containerType );
6160
tryReturn( declaration, paramTypes, containerType );
6261
castResult( declaration );
6362
createQuery( declaration );
6463
handlePageParameters( declaration, paramTypes, containerType );
65-
boolean unwrapped = !isUsingEntityManager();
64+
boolean unwrapped = initiallyUnwrapped();
6665
unwrapped = enableFetchProfile( declaration, unwrapped );
6766
execute( declaration, paramTypes, unwrapped );
6867
}
@@ -83,7 +82,9 @@ private void execute(StringBuilder declaration, List<String> paramTypes, boolean
8382

8483
@Override
8584
String createQueryMethod() {
86-
return isUsingEntityManager() || isReactive() || isUnspecializedQueryType(containerType)
85+
return isUsingEntityManager()
86+
|| isReactive()
87+
|| isUnspecializedQueryType(containerType)
8788
? "createQuery"
8889
: "createSelectionQuery";
8990
}

‎tooling/metamodel-generator/src/main/java/org/hibernate/processor/annotation/QueryMethod.java

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -94,15 +94,14 @@ public String getAttributeDeclarationString() {
9494
preamble( declaration, paramTypes );
9595
createSpecification( declaration );
9696
handleRestrictionParameters( declaration, paramTypes );
97-
collectOrdering( declaration, paramTypes );
98-
handleSorting( declaration, paramTypes, containerType );
97+
collectOrdering( declaration, paramTypes, containerType );
9998
chainSession( declaration );
10099
tryReturn( declaration, paramTypes, containerType );
101100
castResult( declaration );
102101
createQuery( declaration );
103102
setParameters( declaration, paramTypes, "");
104103
handlePageParameters( declaration, paramTypes, containerType );
105-
execute( declaration, !isUsingEntityManager() );
104+
execute( declaration, initiallyUnwrapped() );
106105
convertExceptions( declaration );
107106
chainSessionEnd( isUpdate, declaration );
108107
closingBrace( declaration );
@@ -117,20 +116,23 @@ String specificationType() {
117116

118117
@Override
119118
void createQuery(StringBuilder declaration) {
120-
if ( isUsingSpecification() ) {
121-
declaration
122-
.append(localSessionName())
123-
.append(".createQuery(_spec.buildCriteria(")
124-
.append(localSessionName());
119+
final boolean specification = isUsingSpecification();
120+
if ( specification ) {
125121
if ( isReactive() ) {
126-
declaration.append(".getFactory()");
122+
declaration
123+
.append(localSessionName())
124+
.append(".createQuery(_spec.buildCriteria(")
125+
.append(localSessionName())
126+
.append(".getFactory().getCriteriaBuilder()))\n");
127+
}
128+
else {
129+
declaration
130+
.append("_spec.createQuery(")
131+
.append(localSessionName())
132+
.append(")\n");
127133
}
128-
declaration
129-
.append(".getCriteriaBuilder()") // TODO no such method in Reactive!
130-
.append( "))\n" );
131134
}
132135
else {
133-
// can't use Specification
134136
declaration
135137
.append(localSessionName())
136138
.append('.')

0 commit comments

Comments
 (0)