finally sort out handling of @id and @Version in query validator

gavinking · gavinking · commit 19d5895dd64a · 2024-09-14T12:29:10.000+02:00
also remove 'this' hacks made obsolete by Steve's work on core
diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/AbstractIdentifiableType.java b/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/AbstractIdentifiableType.java
@@ -423,11 +423,10 @@ else if ( id != null ) {
 			}
 			else {
 				assert type instanceof EmbeddableDomainType;
-				final EmbeddableDomainType<?> compositeType = (EmbeddableDomainType<?>) type;
 				return new EmbeddedSqmPathSource<>(
 						EntityIdentifierMapping.ID_ROLE_NAME,
 						(SqmPathSource) id,
-						compositeType,
+						(EmbeddableDomainType<?>) type,
 						Bindable.BindableType.SINGULAR_ATTRIBUTE,
 						id.isGeneric()
 				);
diff --git a/hibernate-core/src/main/java/org/hibernate/query/hql/internal/SemanticQueryBuilder.java b/hibernate-core/src/main/java/org/hibernate/query/hql/internal/SemanticQueryBuilder.java
@@ -2999,17 +2999,22 @@ public SqmPath<?> visitEntityIdReference(HqlParser.EntityIdReferenceContext ctx)
 
 		final SqmPath<?> sqmPath = consumeDomainPath( ctx.path() );
 		final DomainType<?> sqmPathType = sqmPath.getReferencedPathSource().getSqmPathType();
-
-		if ( sqmPathType instanceof IdentifiableDomainType<?> ) {
-			final SqmPathSource<?> identifierDescriptor = ( (IdentifiableDomainType<?>) sqmPathType ).getIdentifierDescriptor();
+		if ( sqmPathType instanceof IdentifiableDomainType<?> identifiableType ) {
+			final SqmPathSource<?> identifierDescriptor = identifiableType.getIdentifierDescriptor();
+			if ( identifierDescriptor == null ) {
+				// mainly for benefit of Hibernate Processor
+				throw new FunctionArgumentException( "Argument '" + sqmPath.getNavigablePath()
+						+ "' of 'id()' is a '" + identifiableType.getTypeName()
+						+ "' and does not have a well-defined '@Id' attribute" );
+			}
 			return sqmPath.get( identifierDescriptor.getPathName() );
 		}
 		else if ( sqmPath instanceof SqmAnyValuedSimplePath<?> ) {
 			return sqmPath.resolvePathPart( AnyKeyPart.KEY_NAME, true, processingStateStack.getCurrent().getCreationState() );
 		}
 		else {
 			throw new FunctionArgumentException( "Argument '" + sqmPath.getNavigablePath()
-					+ "' of 'id()' function does not resolve to an entity type" );
+					+ "' of 'id()' does not resolve to an entity type" );
 		}
 	}
 
@@ -3022,26 +3027,21 @@ public SqmExpression<?> visitEntityVersionExpression(HqlParser.EntityVersionExpr
 	public SqmPath<?> visitEntityVersionReference(HqlParser.EntityVersionReferenceContext ctx) {
 		final SqmPath<?> sqmPath = consumeDomainPath( ctx.path() );
 		final DomainType<?> sqmPathType = sqmPath.getReferencedPathSource().getSqmPathType();
-
-		if ( sqmPathType instanceof IdentifiableDomainType<?> ) {
-			@SuppressWarnings("unchecked")
-			final IdentifiableDomainType<Object> identifiableType = (IdentifiableDomainType<Object>) sqmPathType;
-			final SingularPersistentAttribute<Object, ?> versionAttribute = identifiableType.findVersionAttribute();
-			if ( versionAttribute == null ) {
-				throw new FunctionArgumentException(
-						String.format(
-								"Argument '%s' of 'version()' function resolved to entity type '%s' which does not have a '@Version' attribute",
-								sqmPath.getNavigablePath(),
-								identifiableType.getTypeName()
-						)
-				);
+		if ( sqmPathType instanceof IdentifiableDomainType<?> identifiableType ) {
+			if ( !identifiableType.hasVersionAttribute() ) {
+				throw new FunctionArgumentException( "Argument '" + sqmPath.getNavigablePath()
+						+ "' of 'version()' is a '" + identifiableType.getTypeName()
+						+ "' and does not have a '@Version' attribute" );
 			}
-
+			@SuppressWarnings("unchecked")
+			final SingularPersistentAttribute<Object, ?> versionAttribute =
+					(SingularPersistentAttribute<Object, ?>) identifiableType.findVersionAttribute();
 			return sqmPath.get( versionAttribute );
 		}
-
-		throw new FunctionArgumentException( "Argument '" + sqmPath.getNavigablePath()
-				+ "' of 'version()' function does not resolve to an entity type" );
+		else {
+			throw new FunctionArgumentException( "Argument '" + sqmPath.getNavigablePath()
+					+ "' of 'version()' does not resolve to an entity type" );
+		}
 	}
 
 	@Override
@@ -3060,35 +3060,29 @@ public SqmPath<?> visitEntityNaturalIdReference(HqlParser.EntityNaturalIdReferen
 
 		if ( sqmPathType instanceof IdentifiableDomainType<?> ) {
 			@SuppressWarnings("unchecked")
-			final IdentifiableDomainType<Object> identifiableType = (IdentifiableDomainType<? super Object>) sqmPathType;
+			final IdentifiableDomainType<Object> identifiableType = (IdentifiableDomainType<Object>) sqmPathType;
 			final List<? extends PersistentAttribute<Object, ?>> attributes = identifiableType.findNaturalIdAttributes();
 			if ( attributes == null ) {
-				throw new FunctionArgumentException(
-						String.format(
-								"Argument '%s' of 'naturalid()' function resolved to entity type '%s' which does not have a natural id",
-								sqmPath.getNavigablePath(),
-								identifiableType.getTypeName()
-						)
+				throw new FunctionArgumentException( "Argument '" + sqmPath.getNavigablePath()
+						+ "' of 'naturalid()' is a '" + identifiableType.getTypeName()
+						+ "' and does not have a natural id"
 				);
 			}
 			else if ( attributes.size() >1 ) {
-				throw new FunctionArgumentException(
-						String.format(
-								"Argument '%s' of 'naturalid()' function resolved to entity type '%s' which has a composite natural id",
-								sqmPath.getNavigablePath(),
-								identifiableType.getTypeName()
-						)
+				throw new FunctionArgumentException( "Argument '" + sqmPath.getNavigablePath()
+						+ "' of 'naturalid()' is a '" + identifiableType.getTypeName()
+						+ "' and has a composite natural id"
 				);
 			}
 
 			@SuppressWarnings("unchecked")
-			SingularAttribute<Object, ?> naturalIdAttribute
+			final SingularAttribute<Object, ?> naturalIdAttribute
 					= (SingularAttribute<Object, ?>) attributes.get(0);
 			return sqmPath.get( naturalIdAttribute );
 		}
 
 		throw new FunctionArgumentException( "Argument '" + sqmPath.getNavigablePath()
-				+ "' of 'naturalid()' function does not resolve to an entity type" );
+				+ "' of 'naturalid()' does not resolve to an entity type" );
 	}
 //
 //	@Override
diff --git a/tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/basic/Concrete.java b/tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/basic/Concrete.java
@@ -3,5 +3,5 @@
 import jakarta.data.repository.Repository;
 
 @Repository
-public interface Concrete extends IdOperations<Book> {
+public interface Concrete extends IdOperations<Thing> {
 }
diff --git a/tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/basic/DataTest.java b/tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/basic/DataTest.java
@@ -19,7 +19,7 @@
  */
 public class DataTest extends CompilationTest {
 	@Test
-	@WithClasses({ Author.class, Book.class, BookAuthorRepository.class, IdOperations.class, Concrete.class })
+	@WithClasses({ Author.class, Book.class, BookAuthorRepository.class, IdOperations.class, Concrete.class, Thing.class })
 	public void test() {
 		System.out.println( getMetaModelSourceAsString( Author.class ) );
 		System.out.println( getMetaModelSourceAsString( Book.class ) );
diff --git a/tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/basic/IdOperations.java b/tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/basic/IdOperations.java
@@ -23,11 +23,6 @@
 import jakarta.data.Sort;
 import jakarta.data.repository.Query;
 
-/**
- * This interface contains common operations for the NaturalNumbers and AsciiCharacters repositories.
- *
- * @param <T> type of entity.
- */
 public interface IdOperations<T> {
     @Query("where id(this) between ?1 and ?2")
     Stream<T> findByIdBetween(long minimum, long maximum, Sort<T> sort);
diff --git a/tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/basic/Thing.java b/tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/basic/Thing.java
@@ -0,0 +1,9 @@
+package org.hibernate.processor.test.data.basic;
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.Id;
+
+@Entity
+public class Thing {
+	@Id long id;
+}
diff --git a/tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/versioned/SpecialVersioned.java b/tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/versioned/SpecialVersioned.java
@@ -0,0 +1,7 @@
+package org.hibernate.processor.test.data.versioned;
+
+import jakarta.persistence.Entity;
+
+@Entity
+public class SpecialVersioned extends Versioned {
+}
diff --git a/tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/versioned/SpecialVersionedRepo.java b/tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/versioned/SpecialVersionedRepo.java
@@ -0,0 +1,17 @@
+package org.hibernate.processor.test.data.versioned;
+
+import jakarta.data.repository.Query;
+import jakarta.data.repository.Repository;
+
+@Repository
+public interface SpecialVersionedRepo {
+	@Query("where id(this) = ?1")
+	SpecialVersioned forId(long id);
+
+	@Query("where id(this) = ?1 and version(this) = ?2")
+	SpecialVersioned forIdAndVersion(long id, int version);
+
+	@Query("select count(this) from SpecialVersioned")
+	long count();
+
+}
diff --git a/tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/versioned/Versioned.java b/tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/versioned/Versioned.java
@@ -0,0 +1,11 @@
+package org.hibernate.processor.test.data.versioned;
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.Id;
+import jakarta.persistence.Version;
+
+@Entity
+public class Versioned {
+	@Id long id;
+	@Version int version;
+}
diff --git a/tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/versioned/VersionedRepo.java b/tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/versioned/VersionedRepo.java
@@ -0,0 +1,17 @@
+package org.hibernate.processor.test.data.versioned;
+
+import jakarta.data.repository.Query;
+import jakarta.data.repository.Repository;
+
+@Repository
+public interface VersionedRepo {
+	@Query("where id(this) = ?1")
+	Versioned forId(long id);
+
+	@Query("where id(this) = ?1 and version(this) = ?2")
+	Versioned forIdAndVersion(long id, int version);
+
+	@Query("select count(this) from Versioned")
+	long count();
+
+}
diff --git a/tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/versioned/VersionedTest.java b/tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/versioned/VersionedTest.java
@@ -0,0 +1,31 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.processor.test.data.versioned;
+
+import org.hibernate.processor.test.util.CompilationTest;
+import org.hibernate.processor.test.util.WithClasses;
+import org.junit.Test;
+
+import static org.hibernate.processor.test.util.TestUtil.assertMetamodelClassGeneratedFor;
+import static org.hibernate.processor.test.util.TestUtil.getMetaModelSourceAsString;
+
+/**
+ * @author Gavin King
+ */
+public class VersionedTest extends CompilationTest {
+	@Test
+	@WithClasses({ Versioned.class, VersionedRepo.class, SpecialVersioned.class, SpecialVersionedRepo.class })
+	public void test() {
+		System.out.println( getMetaModelSourceAsString( VersionedRepo.class ) );
+		assertMetamodelClassGeneratedFor( Versioned.class, true );
+		assertMetamodelClassGeneratedFor( Versioned.class );
+		assertMetamodelClassGeneratedFor( SpecialVersioned.class, true );
+		assertMetamodelClassGeneratedFor( SpecialVersioned.class );
+		assertMetamodelClassGeneratedFor( VersionedRepo.class );
+		assertMetamodelClassGeneratedFor( SpecialVersionedRepo.class );
+	}
+}
diff --git a/tooling/metamodel-generator/src/main/java/org/hibernate/processor/annotation/AnnotationMetaEntity.java b/tooling/metamodel-generator/src/main/java/org/hibernate/processor/annotation/AnnotationMetaEntity.java
@@ -71,11 +71,9 @@
 import static java.util.stream.Collectors.toList;
 import static javax.lang.model.util.ElementFilter.fieldsIn;
 import static javax.lang.model.util.ElementFilter.methodsIn;
-import static org.hibernate.grammars.hql.HqlLexer.AS;
 import static org.hibernate.grammars.hql.HqlLexer.FROM;
 import static org.hibernate.grammars.hql.HqlLexer.GROUP;
 import static org.hibernate.grammars.hql.HqlLexer.HAVING;
-import static org.hibernate.grammars.hql.HqlLexer.IDENTIFIER;
 import static org.hibernate.grammars.hql.HqlLexer.ORDER;
 import static org.hibernate.grammars.hql.HqlLexer.WHERE;
 import static org.hibernate.internal.util.StringHelper.qualify;
@@ -2408,47 +2406,22 @@ else if ( isInsertUpdateDelete(hql) ) {
 		}
 		else {
 			final HqlLexer hqlLexer = HqlParseTreeBuilder.INSTANCE.buildHqlLexer( hql );
-			String thisText = "";
 			final List<? extends Token> allTokens = hqlLexer.getAllTokens();
-			for (Token token : allTokens) {
-				if ( token.getType() == IDENTIFIER ) {
-					//TEMPORARY until HQL gets support for 'this'
-					final String text = token.getText();
-					if ( text.equalsIgnoreCase("this") ) {
-						thisText = " as " + text;
-					}
-					break;
-				}
-			}
 			for (int i = 0; i < allTokens.size(); i++) {
 				final Token token = allTokens.get(i);
 				switch ( token.getType() ) {
 					case FROM:
-						return thisText.isEmpty() || hasAlias(i, allTokens) ? hql
-								: new StringBuilder(hql)
-								.insert(allTokens.get(i+1).getStopIndex() + 1, thisText)
-								.toString();
+						return hql;
 					case WHERE:
 					case HAVING:
 					case GROUP:
 					case ORDER:
 						return new StringBuilder(hql)
-								.insert(token.getStartIndex(), "from " + entityType + thisText + " ")
+								.insert(token.getStartIndex(), "from " + entityType + " ")
 								.toString();
 				}
 			}
-			return hql + " from " + entityType + thisText;
-		}
-	}
-
-	private static boolean hasAlias(int i, List<? extends Token> allTokens) {
-		if ( allTokens.size() <= i+2 ) {
-			return false;
-		}
-		else {
-			final int nextTokenType = allTokens.get(i+2).getType();
-			return nextTokenType == IDENTIFIER
-				|| nextTokenType == AS;
+			return hql + " from " + entityType;
 		}
 	}
 
diff --git a/tooling/metamodel-generator/src/main/java/org/hibernate/processor/validation/MockEntityPersister.java b/tooling/metamodel-generator/src/main/java/org/hibernate/processor/validation/MockEntityPersister.java
@@ -12,6 +12,7 @@
 import org.hibernate.persister.entity.EntityPersister;
 import org.hibernate.persister.entity.Joinable;
 import org.hibernate.tuple.entity.EntityMetamodel;
+import org.hibernate.type.BasicType;
 import org.hibernate.type.Type;
 
 import java.io.Serializable;
@@ -113,27 +114,41 @@ public final Type getPropertyType(String propertyPath) {
 
 	abstract Type createPropertyType(String propertyPath);
 
+	/**
+	 * Override on subclasses!
+	 */
+	@Override
+	public String getIdentifierPropertyName() {
+		return getRootEntityPersister().identifierPropertyName();
+	}
+
+	protected abstract String identifierPropertyName();
+
+	/**
+	 * Override on subclasses!
+	 */
 	@Override
 	public Type getIdentifierType() {
-		//TODO: propertyType(getIdentifierPropertyName())
-		return typeConfiguration.getBasicTypeForJavaType(Long.class);
+		return getRootEntityPersister().identifierType();
 	}
 
+	protected abstract Type identifierType();
+
+	/**
+	 * Override on subclasses!
+	 */
 	@Override
-	public String getIdentifierPropertyName() {
-		//TODO: return the correct @Id property name
-		return "id";
+	public BasicType<?> getVersionType() {
+		return getRootEntityPersister().versionType();
 	}
 
+	protected abstract BasicType<?> versionType();
+
 	@Override
-	public String getRootEntityName() {
-		for (MockEntityPersister persister : factory.getMockEntityPersisters()) {
-			if (this != persister && !persister.isSamePersister(this)
-					&& persister.isSubclassPersister(this)) {
-				return persister.getRootEntityName();
-			}
-		}
-		return entityName;
+	public abstract String getRootEntityName();
+
+	public MockEntityPersister getRootEntityPersister() {
+		return factory.createMockEntityPersister(getRootEntityName());
 	}
 
 	@Override
@@ -195,7 +210,12 @@ public String toString() {
 
 	@Override
 	public int getVersionProperty() {
-		return -66;
+		return 0;
+	}
+
+	@Override
+	public boolean isVersioned() {
+		return true;
 	}
 
 	@Override
diff --git a/tooling/metamodel-generator/src/main/java/org/hibernate/processor/validation/MockSessionFactory.java b/tooling/metamodel-generator/src/main/java/org/hibernate/processor/validation/MockSessionFactory.java
diff --git a/tooling/metamodel-generator/src/main/java/org/hibernate/processor/validation/ProcessorSessionFactory.java b/tooling/metamodel-generator/src/main/java/org/hibernate/processor/validation/ProcessorSessionFactory.java

Original file line number	Diff line number	Diff line change
`@@ -3,5 +3,5 @@`
`3`	`3`	`import jakarta.data.repository.Repository;`
`4`	`4`
`5`	`5`	`@Repository`
`6`		`-public interface Concrete extends IdOperations<Book> {`
	`6`	`+public interface Concrete extends IdOperations<Thing> {`
`7`	`7`	`}`