Skip to content

Commit 1a19433

Browse files
gavinkinggavinking
committed
fix dupe error messages across _X and X_ class generation
1 parent 7dddc37 commit 1a19433

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
@@ -392,7 +391,6 @@ public TypeElement getTypeElementForFullyQualifiedName(String qualifiedName) {
392391
}
393392

394393
void markGenerated(Metamodel metamodel) {
395-
generatedModelClasses.add( metamodel );
396394
generatedClassNames.add( getFullyQualifiedClassName( metamodel ) );
397395
}
398396

@@ -432,7 +430,7 @@ public void mappingDocumentFullyXmlConfigured(boolean fullyXmlConfigured) {
432430
return persistenceUnitDefaultAccessType;
433431
}
434432

435-
public void setPersistenceUnitDefaultAccessType(AccessType persistenceUnitDefaultAccessType) {
433+
public void setPersistenceUnitDefaultAccessType(@Nullable AccessType persistenceUnitDefaultAccessType) {
436434
this.persistenceUnitDefaultAccessType = persistenceUnitDefaultAccessType;
437435
}
438436

‎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
@@ -1913,34 +1913,27 @@ private void addStaticQueryMethod(
19131913
boolean isNative) {
19141914
final var value = getAnnotationValue( mirror );
19151915
if ( value != null && value.getValue() instanceof String queryString ) {
1916+
final var validate = shouldValidateStaticQueryMethod( method );
19161917
final var methodReturnType =
19171918
unwrappedReturnTypeIfPossible( method, memberMethodType( method ).getReturnType() );
19181919
final var queryReturnType =
1919-
staticQueryReturnType( method, methodReturnType, mirror, value, queryString );
1920+
staticQueryReturnType( method, methodReturnType, mirror, value, queryString, validate );
19201921
if ( queryReturnType != null ) {
19211922
final var paramNames = staticQueryParameterNames( method );
19221923
final var paramTypes = parameterTypes( method );
1923-
final TypeElement entity;
1924-
if ( isNative ) {
1925-
entity = null;
1926-
if ( !repository || method.isDefault() ) {
1927-
validateNativeQueryHasNoDynamicAugmentation( method, mirror, paramTypes );
1924+
final var returnType = queryReturnType.validationReturnType;
1925+
final var entityType =
1926+
isNative ? null : querySelectionEntity( method, returnType, mirror, queryString, validate );
1927+
if ( validate ) {
1928+
final var type = entityType == null ? returnType : entityType.asType();
1929+
checkParameters( method, type, paramNames, paramTypes, mirror, value, queryString, isNative );
1930+
if ( isNative ) {
1931+
validateSql( method, mirror, queryString, paramNames, value );
1932+
}
1933+
else {
1934+
final var processedQuery = staticQueryValidationString( type, mirror, queryString );
1935+
validateHql( method, type, mirror, value, processedQuery, paramNames, paramTypes );
19281936
}
1929-
}
1930-
else {
1931-
entity = querySelectionEntity( method, queryReturnType.validationReturnType, mirror, queryString );
1932-
}
1933-
final var validationReturnType = entity == null ? queryReturnType.validationReturnType : entity.asType();
1934-
final var processedQuery =
1935-
staticQueryValidationString( validationReturnType, mirror, queryString );
1936-
checkParameters( method, validationReturnType,
1937-
paramNames, paramTypes, mirror, value, queryString, isNative );
1938-
if ( isNative ) {
1939-
validateSql( method, mirror, queryString, paramNames, value );
1940-
}
1941-
else {
1942-
validateHql( method, validationReturnType,
1943-
mirror, value, processedQuery, paramNames, paramTypes );
19441937
}
19451938
if ( canBindReferenceArguments( queryString, isNative, paramNames, paramTypes ) ) {
19461939
final var referenceParamNames = queryParameterNames( paramNames, paramTypes );
@@ -1953,12 +1946,12 @@ private void addStaticQueryMethod(
19531946
method.getSimpleName().toString(),
19541947
queryReturnType.statement,
19551948
isNative,
1956-
entity == null
1949+
entityType == null
19571950
? queryReturnType.resultTypeName
1958-
: entity.getQualifiedName().toString(),
1959-
entity == null
1951+
: entityType.getQualifiedName().toString(),
1952+
entityType == null
19601953
? queryReturnType.resultTypeClass
1961-
: entity.getQualifiedName().toString(),
1954+
: entityType.getQualifiedName().toString(),
19621955
paramTypes,
19631956
referenceParamNames,
19641957
referenceParamTypes,
@@ -1970,6 +1963,16 @@ private void addStaticQueryMethod(
19701963
}
19711964
}
19721965

1966+
/**
1967+
* For a repository method validation is done during generation of
1968+
* the {@code _Repo} class, and we don't need to duplicate it for
1969+
* the {@code Repo_} class which contains only static references.
1970+
*/
1971+
private boolean shouldValidateStaticQueryMethod(ExecutableElement method) {
1972+
return !repositoryQueryMetamodel
1973+
|| method.isDefault(); //TODO: is this necessary?
1974+
}
1975+
19731976
private String staticQueryValidationString(
19741977
@Nullable TypeMirror returnType,
19751978
AnnotationMirror mirror,
@@ -1991,17 +1994,18 @@ private String staticQueryValidationString(
19911994
TypeMirror methodReturnType,
19921995
AnnotationMirror mirror,
19931996
AnnotationValue value,
1994-
String queryString) {
1997+
String queryString,
1998+
boolean validate) {
19951999
final var statement = isMutationStatement( queryString );
19962000
return switch ( methodReturnType.getKind() ) {
19972001
case VOID -> statement
19982002
? new StaticQueryReturnType( true, null, null, methodReturnType )
1999-
: illegalStaticQueryReturnType( method, mirror, value );
2003+
: illegalStaticQueryReturnType( method, mirror, value, validate );
20002004
case BOOLEAN, BYTE, SHORT, INT, LONG, CHAR, FLOAT, DOUBLE -> {
20012005
if ( statement ) {
20022006
yield switch ( methodReturnType.getKind() ) {
20032007
case BOOLEAN, INT, LONG -> new StaticQueryReturnType( true, null, null, methodReturnType );
2004-
default -> illegalStaticQueryReturnType( method, mirror, value );
2008+
default -> illegalStaticQueryReturnType( method, mirror, value, validate );
20052009
};
20062010
}
20072011
else {
@@ -2018,7 +2022,9 @@ yield new StaticQueryReturnType( false,
20182022
}
20192023
else {
20202024
final var queryResultType = staticQueryResultType( (DeclaredType) methodReturnType );
2021-
resultType( method, queryResultType, mirror, value );
2025+
if ( validate ) {
2026+
resultType( method, queryResultType, mirror, value );
2027+
}
20222028
yield new StaticQueryReturnType( false,
20232029
typeAsString( queryResultType ),
20242030
returnTypeClass( queryResultType ),
@@ -2032,18 +2038,21 @@ yield new StaticQueryReturnType( false,
20322038
returnTypeClass( queryResultType ),
20332039
queryResultType );
20342040
}
2035-
default -> illegalStaticQueryReturnType( method, mirror, value );
2041+
default -> illegalStaticQueryReturnType( method, mirror, value, validate );
20362042
};
20372043
}
20382044

20392045
private @Nullable StaticQueryReturnType illegalStaticQueryReturnType(
20402046
ExecutableElement method,
20412047
AnnotationMirror mirror,
2042-
AnnotationValue value) {
2043-
message( method, mirror, value,
2044-
"static query method must return a referenceable query result type,"
2045-
+ " or a legal mutation result type: 'void', 'boolean', 'int', or 'long'",
2046-
Diagnostic.Kind.ERROR );
2048+
AnnotationValue value,
2049+
boolean validate) {
2050+
if ( validate ) {
2051+
message( method, mirror, value,
2052+
"static query method must return a referenceable query result type,"
2053+
+ " or a legal mutation result type: 'void', 'boolean', 'int', or 'long'",
2054+
Diagnostic.Kind.ERROR );
2055+
}
20472056
return null;
20482057
}
20492058

@@ -3121,17 +3130,26 @@ private record SelectedAttribute(String path, TypeMirror type) {
31213130
@Nullable TypeMirror returnType,
31223131
AnnotationMirror query,
31233132
String queryString) {
3133+
return querySelectionEntity( method, returnType, query, queryString, true );
3134+
}
3135+
3136+
private @Nullable TypeElement querySelectionEntity(
3137+
ExecutableElement method,
3138+
@Nullable TypeMirror returnType,
3139+
AnnotationMirror query,
3140+
String queryString,
3141+
boolean validate) {
31243142
final var explicitMethodSelect = hasSelectAnnotation( method );
31253143
if ( returnType == null ) {
3126-
if ( explicitMethodSelect ) {
3144+
if ( validate && explicitMethodSelect ) {
31273145
message( method, query,
31283146
"'@Select' requires a concrete query result type",
31293147
Diagnostic.Kind.ERROR );
31303148
}
31313149
return null;
31323150
}
31333151
if ( isMutationStatement( queryString ) ) {
3134-
if ( explicitMethodSelect ) {
3152+
if ( validate && explicitMethodSelect ) {
31353153
message( method, query,
31363154
"'@Select' may not be used on a mutation query",
31373155
Diagnostic.Kind.ERROR );
@@ -3143,23 +3161,23 @@ private record SelectedAttribute(String path, TypeMirror type) {
31433161
}
31443162
if ( returnType.getKind() == TypeKind.DECLARED
31453163
&& containsAnnotation( ((DeclaredType) returnType).asElement(), ENTITY ) ) {
3146-
if ( explicitMethodSelect ) {
3164+
if ( validate && explicitMethodSelect ) {
31473165
message( method, query,
31483166
"'@Select' may not be used on a '@Query' method that returns entity type '" + returnType + "'",
31493167
Diagnostic.Kind.ERROR );
31503168
}
31513169
return null;
31523170
}
31533171
if ( hasSelectClause( queryString ) ) {
3154-
if ( explicitMethodSelect ) {
3172+
if ( validate && explicitMethodSelect ) {
31553173
message( method, query,
31563174
"'@Select' may not be used on a '@Query' method with an explicit SELECT clause",
31573175
Diagnostic.Kind.ERROR );
31583176
}
31593177
return null;
31603178
}
31613179
final var entity = querySelectionEntity( queryString );
3162-
if ( entity == null ) {
3180+
if ( validate && entity == null ) {
31633181
message( method, query,
31643182
"repository method has no well-defined entity type",
31653183
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)