Skip to content

Commit dad8a30

Browse files
gavinkinggavinking
committed
fix dupe error messages across _X and X_ class generation
1 parent 9654d73 commit dad8a30

7 files changed

Lines changed: 204 additions & 47 deletions

File tree

‎tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/basic/DataTest.java‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import static org.hibernate.processor.test.util.TestUtil.assertMetamodelClassGeneratedFor;
2323
import static org.hibernate.processor.test.util.TestUtil.assertNoMetamodelClassGeneratedFor;
2424
import static org.hibernate.processor.test.util.TestUtil.getMetaModelSourceAsString;
25+
import static org.junit.jupiter.api.Assertions.assertEquals;
2526
import static org.junit.jupiter.api.Assertions.assertFalse;
2627
import static org.junit.jupiter.api.Assertions.assertTrue;
2728

@@ -164,16 +165,27 @@ void nonSequentialPositionalQueryParametersUseMeaningfulDiagnostic() throws Exce
164165
);
165166
assertFalse( task.call() );
166167
}
167-
final String messages = diagnostics.getDiagnostics()
168+
final var errorMessages = diagnostics.getDiagnostics()
168169
.stream()
169170
.filter( diagnostic -> diagnostic.getKind() == Diagnostic.Kind.ERROR )
170171
.map( diagnostic -> diagnostic.getMessage( Locale.ROOT ) )
171-
.collect( Collectors.joining( "\n" ) );
172+
.toList();
173+
final String messages = errorMessages.stream().collect( Collectors.joining( "\n" ) );
172174

173175
assertTrue( messages.contains(
174176
"positional query parameters must be numbered sequentially starting at ?1" ) );
175177
assertTrue( messages.contains(
176178
"query parameters must be either all named or all positional" ) );
179+
assertEquals( 1, countMessagesContaining( errorMessages,
180+
"positional query parameters must be numbered sequentially starting at ?1" ) );
181+
assertEquals( 1, countMessagesContaining( errorMessages,
182+
"query parameters must be either all named or all positional" ) );
183+
}
184+
185+
private static long countMessagesContaining(List<String> messages, String text) {
186+
return messages.stream()
187+
.filter( message -> message.contains( text ) )
188+
.count();
177189
}
178190

179191
private static File sourceFile(Class<?> type) {

‎tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/restriction/DataRestrictionTest.java‎

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import static java.nio.charset.Charset.defaultCharset;
2424
import static org.hibernate.processor.test.util.TestUtil.getMetaModelSourceAsString;
25+
import static org.junit.jupiter.api.Assertions.assertEquals;
2526
import static org.junit.jupiter.api.Assertions.assertFalse;
2627
import static org.junit.jupiter.api.Assertions.assertTrue;
2728

@@ -119,11 +120,12 @@ void invalidAutomaticDeleteUsesMeaningfulDiagnostics() throws Exception {
119120
);
120121
assertFalse( task.call() );
121122
}
122-
final String messages = diagnostics.getDiagnostics()
123+
final var errorMessages = diagnostics.getDiagnostics()
123124
.stream()
124125
.filter( diagnostic -> diagnostic.getKind() == Diagnostic.Kind.ERROR )
125126
.map( diagnostic -> diagnostic.getMessage( Locale.ROOT ) )
126-
.collect( Collectors.joining( "\n" ) );
127+
.toList();
128+
final String messages = errorMessages.stream().collect( Collectors.joining( "\n" ) );
127129

128130
assertTrue( messages.contains(
129131
"parameter of type 'jakarta.data.Limit' is not allowed on an automatic '@Delete' method" ) );
@@ -145,6 +147,51 @@ void invalidAutomaticDeleteUsesMeaningfulDiagnostics() throws Exception {
145147
assertTrue( messages.contains(
146148
"'@NativeQuery' methods may not declare Order or Sort parameters or '@OrderBy'; "
147149
+ "native SQL cannot be augmented with ordering" ) );
150+
assertEquals( 1, countMessagesContaining( errorMessages,
151+
"'@NativeQuery' methods may not declare Restriction or Range parameters" ) );
152+
assertEquals( 1, countMessagesContaining( errorMessages,
153+
"'@NativeQuery' methods may not declare Order or Sort parameters or '@OrderBy'" ) );
154+
}
155+
156+
@Test
157+
void nonRepositoryNativeQueryWithRestrictionUsesSingleDiagnostic() throws Exception {
158+
final DiagnosticCollector<JavaFileObject> diagnostics = new DiagnosticCollector<>();
159+
final var compiler = ToolProvider.getSystemJavaCompiler();
160+
try ( var fileManager = compiler.getStandardFileManager( diagnostics, Locale.ROOT, defaultCharset() ) ) {
161+
final var sourceFiles = List.of(
162+
sourceFile( DataRestrictionPublisher.class ),
163+
sourceFile( DataRestrictionBook.class ),
164+
sourceFile( InvalidNativeStaticQuery.class )
165+
);
166+
final var task = compiler.getTask(
167+
null,
168+
fileManager,
169+
diagnostics,
170+
List.of(
171+
"-d",
172+
TestUtil.getOutBaseDir( DataRestrictionTest.class ).getAbsolutePath(),
173+
"-processor",
174+
HibernateProcessor.class.getName()
175+
),
176+
null,
177+
fileManager.getJavaFileObjectsFromFiles( sourceFiles )
178+
);
179+
assertFalse( task.call() );
180+
}
181+
final var errorMessages = diagnostics.getDiagnostics()
182+
.stream()
183+
.filter( diagnostic -> diagnostic.getKind() == Diagnostic.Kind.ERROR )
184+
.map( diagnostic -> diagnostic.getMessage( Locale.ROOT ) )
185+
.toList();
186+
187+
assertEquals( 1, countMessagesContaining( errorMessages,
188+
"'@NativeQuery' methods may not declare Restriction or Range parameters" ) );
189+
}
190+
191+
private static long countMessagesContaining(List<String> messages, String text) {
192+
return messages.stream()
193+
.filter( message -> message.contains( text ) )
194+
.count();
148195
}
149196

150197
private static File sourceFile(Class<?> type) {

‎tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/restriction/InvalidNativeStaticQuery.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
* Copyright Red Hat Inc. and Hibernate Authors
4+
*/
5+
package org.hibernate.processor.test.data.restriction;
6+
7+
import jakarta.data.restrict.Restriction;
8+
import jakarta.persistence.EntityAgent;
9+
import jakarta.persistence.query.JakartaQuery;
10+
import jakarta.persistence.query.NativeQuery;
11+
12+
import java.util.List;
13+
14+
interface InvalidNativeStaticQuery {
15+
record Summary(String isbn, String title) {
16+
}
17+
18+
@JakartaQuery("from DataRestrictionBook")
19+
List<DataRestrictionBook> books();
20+
21+
@JakartaQuery("from DataRestrictionBook")
22+
List<Summary> summaries();
23+
24+
@NativeQuery("select * from data_restriction_book")
25+
List<DataRestrictionBook> nat(EntityAgent agent, Restriction<DataRestrictionBook> restriction);
26+
}

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@ public final class Context {
9999
private boolean dataEventPackageAvailable;
100100

101101
// keep track of all classes for which model have been generated
102-
private final Set<Metamodel> generatedModelClasses = new HashSet<>();
103102
private final Set<String> generatedClassNames = new HashSet<>();
104103

105104
// keep track of which named queries have been checked
@@ -396,7 +395,6 @@ public TypeElement getTypeElementForFullyQualifiedName(String qualifiedName) {
396395
}
397396

398397
void markGenerated(Metamodel metamodel) {
399-
generatedModelClasses.add( metamodel );
400398
generatedClassNames.add( getFullyQualifiedClassName( metamodel ) );
401399
}
402400

@@ -436,7 +434,7 @@ public AccessType getPersistenceUnitDefaultAccessType() {
436434
return persistenceUnitDefaultAccessType;
437435
}
438436

439-
public void setPersistenceUnitDefaultAccessType(AccessType persistenceUnitDefaultAccessType) {
437+
public void setPersistenceUnitDefaultAccessType(@Nullable AccessType persistenceUnitDefaultAccessType) {
440438
this.persistenceUnitDefaultAccessType = persistenceUnitDefaultAccessType;
441439
}
442440

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

Lines changed: 58 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1907,34 +1907,27 @@ private void addStaticQueryMethod(
19071907
boolean isNative) {
19081908
final var value = getAnnotationValue( mirror );
19091909
if ( value != null && value.getValue() instanceof String queryString ) {
1910+
final var validate = shouldValidateStaticQueryMethod( method );
19101911
final var methodReturnType =
19111912
unwrappedReturnTypeIfPossible( method, memberMethodType( method ).getReturnType() );
19121913
final var queryReturnType =
1913-
staticQueryReturnType( method, methodReturnType, mirror, value, queryString );
1914+
staticQueryReturnType( method, methodReturnType, mirror, value, queryString, validate );
19141915
if ( queryReturnType != null ) {
19151916
final var paramNames = staticQueryParameterNames( method );
19161917
final var paramTypes = parameterTypes( method );
1917-
final TypeElement entity;
1918-
if ( isNative ) {
1919-
entity = null;
1920-
if ( !repository || method.isDefault() ) {
1921-
validateNativeQueryHasNoDynamicAugmentation( method, mirror, paramTypes );
1918+
final var returnType = queryReturnType.validationReturnType;
1919+
final var entityType =
1920+
isNative ? null : querySelectionEntity( method, returnType, mirror, queryString, validate );
1921+
if ( validate ) {
1922+
final var type = entityType == null ? returnType : entityType.asType();
1923+
checkParameters( method, type, paramNames, paramTypes, mirror, value, queryString, isNative );
1924+
if ( isNative ) {
1925+
validateSql( method, mirror, queryString, paramNames, value );
1926+
}
1927+
else {
1928+
final var processedQuery = staticQueryValidationString( type, mirror, queryString );
1929+
validateHql( method, type, mirror, value, processedQuery, paramNames, paramTypes );
19221930
}
1923-
}
1924-
else {
1925-
entity = querySelectionEntity( method, queryReturnType.validationReturnType, mirror, queryString );
1926-
}
1927-
final var validationReturnType = entity == null ? queryReturnType.validationReturnType : entity.asType();
1928-
final var processedQuery =
1929-
staticQueryValidationString( validationReturnType, mirror, queryString );
1930-
checkParameters( method, validationReturnType,
1931-
paramNames, paramTypes, mirror, value, queryString, isNative );
1932-
if ( isNative ) {
1933-
validateSql( method, mirror, queryString, paramNames, value );
1934-
}
1935-
else {
1936-
validateHql( method, validationReturnType,
1937-
mirror, value, processedQuery, paramNames, paramTypes );
19381931
}
19391932
if ( canBindReferenceArguments( queryString, isNative, paramNames, paramTypes ) ) {
19401933
final var referenceParamNames = queryParameterNames( paramNames, paramTypes );
@@ -1947,12 +1940,12 @@ private void addStaticQueryMethod(
19471940
method.getSimpleName().toString(),
19481941
queryReturnType.statement,
19491942
isNative,
1950-
entity == null
1943+
entityType == null
19511944
? queryReturnType.resultTypeName
1952-
: entity.getQualifiedName().toString(),
1953-
entity == null
1945+
: entityType.getQualifiedName().toString(),
1946+
entityType == null
19541947
? queryReturnType.resultTypeClass
1955-
: entity.getQualifiedName().toString(),
1948+
: entityType.getQualifiedName().toString(),
19561949
paramTypes,
19571950
referenceParamNames,
19581951
referenceParamTypes,
@@ -1964,6 +1957,16 @@ private void addStaticQueryMethod(
19641957
}
19651958
}
19661959

1960+
/**
1961+
* For a repository method validation is done during generation of
1962+
* the {@code _Repo} class, and we don't need to duplicate it for
1963+
* the {@code Repo_} class which contains only static references.
1964+
*/
1965+
private boolean shouldValidateStaticQueryMethod(ExecutableElement method) {
1966+
return !repositoryQueryMetamodel
1967+
|| method.isDefault(); //TODO: is this necessary?
1968+
}
1969+
19671970
private String staticQueryValidationString(
19681971
@Nullable TypeMirror returnType,
19691972
AnnotationMirror mirror,
@@ -1985,17 +1988,18 @@ private String staticQueryValidationString(
19851988
TypeMirror methodReturnType,
19861989
AnnotationMirror mirror,
19871990
AnnotationValue value,
1988-
String queryString) {
1991+
String queryString,
1992+
boolean validate) {
19891993
final var statement = isMutationStatement( queryString );
19901994
return switch ( methodReturnType.getKind() ) {
19911995
case VOID -> statement
19921996
? new StaticQueryReturnType( true, null, null, methodReturnType )
1993-
: illegalStaticQueryReturnType( method, mirror, value );
1997+
: illegalStaticQueryReturnType( method, mirror, value, validate );
19941998
case BOOLEAN, BYTE, SHORT, INT, LONG, CHAR, FLOAT, DOUBLE -> {
19951999
if ( statement ) {
19962000
yield switch ( methodReturnType.getKind() ) {
19972001
case BOOLEAN, INT, LONG -> new StaticQueryReturnType( true, null, null, methodReturnType );
1998-
default -> illegalStaticQueryReturnType( method, mirror, value );
2002+
default -> illegalStaticQueryReturnType( method, mirror, value, validate );
19992003
};
20002004
}
20012005
else {
@@ -2012,7 +2016,9 @@ yield new StaticQueryReturnType( false,
20122016
}
20132017
else {
20142018
final var queryResultType = staticQueryResultType( (DeclaredType) methodReturnType );
2015-
resultType( method, queryResultType, mirror, value );
2019+
if ( validate ) {
2020+
resultType( method, queryResultType, mirror, value );
2021+
}
20162022
yield new StaticQueryReturnType( false,
20172023
typeAsString( queryResultType ),
20182024
returnTypeClass( queryResultType ),
@@ -2026,18 +2032,21 @@ yield new StaticQueryReturnType( false,
20262032
returnTypeClass( queryResultType ),
20272033
queryResultType );
20282034
}
2029-
default -> illegalStaticQueryReturnType( method, mirror, value );
2035+
default -> illegalStaticQueryReturnType( method, mirror, value, validate );
20302036
};
20312037
}
20322038

20332039
private @Nullable StaticQueryReturnType illegalStaticQueryReturnType(
20342040
ExecutableElement method,
20352041
AnnotationMirror mirror,
2036-
AnnotationValue value) {
2037-
message( method, mirror, value,
2038-
"static query method must return a referenceable query result type,"
2039-
+ " or a legal mutation result type: 'void', 'boolean', 'int', or 'long'",
2040-
Diagnostic.Kind.ERROR );
2042+
AnnotationValue value,
2043+
boolean validate) {
2044+
if ( validate ) {
2045+
message( method, mirror, value,
2046+
"static query method must return a referenceable query result type,"
2047+
+ " or a legal mutation result type: 'void', 'boolean', 'int', or 'long'",
2048+
Diagnostic.Kind.ERROR );
2049+
}
20412050
return null;
20422051
}
20432052

@@ -3114,17 +3123,26 @@ private record SelectedAttribute(String path, TypeMirror type) {
31143123
@Nullable TypeMirror returnType,
31153124
AnnotationMirror query,
31163125
String queryString) {
3126+
return querySelectionEntity( method, returnType, query, queryString, true );
3127+
}
3128+
3129+
private @Nullable TypeElement querySelectionEntity(
3130+
ExecutableElement method,
3131+
@Nullable TypeMirror returnType,
3132+
AnnotationMirror query,
3133+
String queryString,
3134+
boolean validate) {
31173135
final var explicitMethodSelect = hasSelectAnnotation( method );
31183136
if ( returnType == null ) {
3119-
if ( explicitMethodSelect ) {
3137+
if ( validate && explicitMethodSelect ) {
31203138
message( method, query,
31213139
"'@Select' requires a concrete query result type",
31223140
Diagnostic.Kind.ERROR );
31233141
}
31243142
return null;
31253143
}
31263144
if ( isMutationStatement( queryString ) ) {
3127-
if ( explicitMethodSelect ) {
3145+
if ( validate && explicitMethodSelect ) {
31283146
message( method, query,
31293147
"'@Select' may not be used on a mutation query",
31303148
Diagnostic.Kind.ERROR );
@@ -3136,23 +3154,23 @@ private record SelectedAttribute(String path, TypeMirror type) {
31363154
}
31373155
if ( returnType.getKind() == TypeKind.DECLARED
31383156
&& containsAnnotation( ((DeclaredType) returnType).asElement(), ENTITY ) ) {
3139-
if ( explicitMethodSelect ) {
3157+
if ( validate && explicitMethodSelect ) {
31403158
message( method, query,
31413159
"'@Select' may not be used on a '@Query' method that returns entity type '" + returnType + "'",
31423160
Diagnostic.Kind.ERROR );
31433161
}
31443162
return null;
31453163
}
31463164
if ( hasSelectClause( queryString ) ) {
3147-
if ( explicitMethodSelect ) {
3165+
if ( validate && explicitMethodSelect ) {
31483166
message( method, query,
31493167
"'@Select' may not be used on a '@Query' method with an explicit SELECT clause",
31503168
Diagnostic.Kind.ERROR );
31513169
}
31523170
return null;
31533171
}
31543172
final var entity = querySelectionEntity( queryString );
3155-
if ( entity == null ) {
3173+
if ( validate && entity == null ) {
31563174
message( method, query,
31573175
"repository method has no well-defined entity type",
31583176
Diagnostic.Kind.ERROR );

‎tooling/metamodel-generator/src/test/java/org/hibernate/processor/test/staticquery/InvalidNativeStaticQuery.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
* Copyright Red Hat Inc. and Hibernate Authors
4+
*/
5+
package org.hibernate.processor.test.staticquery;
6+
7+
import jakarta.persistence.EntityAgent;
8+
import jakarta.persistence.query.NativeQuery;
9+
import org.hibernate.query.restriction.Restriction;
10+
11+
import java.util.List;
12+
13+
interface InvalidNativeStaticQuery {
14+
@NativeQuery("select * from Book")
15+
List<Book> nat(EntityAgent agent, Restriction<Book> restriction);
16+
}

0 commit comments

Comments
 (0)