more null checking, and some refactoring

tooling/metamodel-generator/src/main/java/org/hibernate/jpamodelgen/annotation/AbstractFinderMethod.java

@@ -175,8 +175,10 @@ public abstract class AbstractFinderMethod extends AbstractQueryMethod  {
 				.append(" ")
 				.append(methodName);
 		parameters( paramTypes, declaration ) ;
-		declaration
+		declaration.append(" {\n");
-				.append(" {\n");
+	}
+
+	void tryReturn(StringBuilder declaration) {
 		if (dataRepository) {
 			declaration
 					.append("\ttry {\n\t");

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

@@ -323,6 +323,10 @@ public abstract class AbstractQueryMethod implements MetaAttribute {
 
 	abstract boolean singleResult();
 
+	static void closingBrace(StringBuilder declaration) {
+		declaration.append("\n}");
+	}
+
 	@Nullable String getSortableEntityClass() {
 		return returnTypeName;
 	}

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

@@ -64,6 +64,18 @@ public class CriteriaFinderMethod extends AbstractFinderMethod {
 		final StringBuilder declaration = new StringBuilder();
 		comment( declaration );
 		modifiers( declaration );
+		preamble( declaration, paramTypes );
+		nullChecks( paramTypes, declaration );
+		createQuery( declaration );
+		where( declaration, paramTypes );
+		orderBy( paramTypes, declaration );
+		executeQuery( declaration, paramTypes );
+		convertExceptions( declaration );
+		closingBrace( declaration );
+		return declaration.toString();
+	}
+
+	private void preamble(StringBuilder declaration, List<String> paramTypes) {
 		declaration
 				.append(returnType())
 				.append(" ")
@@ -71,15 +83,6 @@ public class CriteriaFinderMethod extends AbstractFinderMethod {
 		parameters(paramTypes, declaration);
 		declaration
 				.append(" {");
-		nullChecks(paramTypes, declaration);
-		createQuery(declaration);
-		where(declaration, paramTypes);
-		orderBy(paramTypes, declaration);
-		executeQuery(declaration, paramTypes);
-		convertExceptions( declaration );
-		declaration
-				.append("\n}");
-		return declaration.toString();
 	}
 
 	private void executeQuery(StringBuilder declaration, List<String> paramTypes) {
@@ -161,14 +164,18 @@ public class CriteriaFinderMethod extends AbstractFinderMethod {
 			final String paramName = paramNames.get(i);
 			final String paramType = paramTypes.get(i);
 			if ( !isNullable(i) && !isPrimitive(paramType) ) {
+				nullCheck( declaration, paramName );
+			}
+		}
+	}
+
+	private static void nullCheck(StringBuilder declaration, String paramName) {
 		declaration
 				.append("\n\tif (")
 				.append(paramName)
-						.append(" == null) throw new IllegalArgumentException(\"Null \" + ")
+				.append(" == null) throw new IllegalArgumentException(\"Null ")
 				.append(paramName)
-						.append(");");
+				.append("\");");
-			}
-		}
 	}
 
 	private void orderBy(List<String> paramTypes, StringBuilder declaration) {

tooling/metamodel-generator/src/main/java/org/hibernate/jpamodelgen/annotation/IdFinderMethod.java

@@ -8,12 +8,15 @@ package org.hibernate.jpamodelgen.annotation;
 
 import java.util.List;
 
+import static org.hibernate.jpamodelgen.util.TypeUtils.isPrimitive;
+
 /**
  * @author Gavin King
  */
 public class IdFinderMethod extends AbstractFinderMethod {
 
 	private final String paramName;
+	private final String paramType;
 
 	public IdFinderMethod(
 			AnnotationMetaEntity annotationMetaEntity,
@@ -27,16 +30,18 @@ public class IdFinderMethod extends AbstractFinderMethod {
 			boolean dataRepository) {
 		super( annotationMetaEntity, methodName, entity, belongsToDao, sessionType, sessionName, fetchProfiles,
 				paramNames, paramTypes, addNonnullAnnotation, dataRepository );
-		this.paramName = idParameterName( paramNames, paramTypes );
+		int idParameter = idParameter(paramNames, paramTypes);
+		this.paramName = paramNames.get(idParameter);
+		this.paramType = paramTypes.get(idParameter);
 	}
 
-	private static String idParameterName(List<String> paramNames, List<String> paramTypes) {
+	private static int idParameter(List<String> paramNames, List<String> paramTypes) {
 		for (int i = 0; i < paramNames.size(); i ++ ) {
 			if ( !isSessionParameter( paramTypes.get(i) ) ) {
-				return paramNames.get(i);
+				return i;
 			}
 		}
-		return ""; // should never occur!
+		return -1; // should never occur!
 	}
 
 	@Override
@@ -54,6 +59,10 @@ public class IdFinderMethod extends AbstractFinderMethod {
 		final StringBuilder declaration = new StringBuilder();
 		comment( declaration );
 		preamble( declaration );
+		if ( paramName != null && !isPrimitive(paramType) ) {
+			nullCheck( declaration, paramName );
+		}
+		tryReturn( declaration );
 		if ( fetchProfiles.isEmpty() ) {
 			findWithNoFetchProfiles( declaration );
 		}
@@ -61,7 +70,7 @@ public class IdFinderMethod extends AbstractFinderMethod {
 			findWithFetchProfiles( declaration );
 		}
 		convertExceptions( declaration );
-		declaration.append("\n}");
+		closingBrace( declaration );
 		return declaration.toString();
 	}
 
@@ -94,4 +103,13 @@ public class IdFinderMethod extends AbstractFinderMethod {
 					.append("\n");
 		}
 	}
+
+	private static void nullCheck(StringBuilder declaration, String parameterName) {
+		declaration
+				.append("\tif (")
+				.append(parameterName)
+				.append(" == null) throw new IllegalArgumentException(\"Null ")
+				.append(parameterName)
+				.append("\");\n");
+	}
 }

tooling/metamodel-generator/src/main/java/org/hibernate/jpamodelgen/annotation/LifecycleMethod.java

@@ -47,19 +47,30 @@ public class LifecycleMethod implements MetaAttribute {
 
 	@Override
 	public String getAttributeDeclarationString() {
-		StringBuilder declaration = new StringBuilder()
+		StringBuilder declaration = new StringBuilder();
-				.append("\n@Override\npublic void ")
+		preamble(declaration);
-				.append(methodName)
+		nullCheck(declaration);
-				.append('(');
+		declaration.append("\ttry {\n");
-		notNull( declaration );
+		delegateCall(declaration);
-		final boolean isInsert = operationName.equals("insert");
+		if ( operationName.equals("insert") ) {
+			convertException(declaration,
+					"jakarta.persistence.EntityExistsException",
+					"jakarta.data.exceptions.EntityExistsException");
+		}
+		else {
+			convertException(declaration,
+					"jakarta.persistence.OptimisticLockException",
+					"jakarta.data.exceptions.OptimisticLockingFailureException");
+		}
+		convertException(declaration,
+				"jakarta.persistence.PersistenceException",
+				"jakarta.data.exceptions.DataException");
+		declaration.append("}");
+		return declaration.toString();
+	}
+
+	private void delegateCall(StringBuilder declaration) {
 		declaration
-				.append(annotationMetaEntity.importType(entity))
-				.append(' ')
-				.append(parameterName)
-				.append(')')
-				.append(" {\n")
-				.append("\ttry {\n")
 				.append("\t\t")
 				.append(sessionName)
 				.append('.')
@@ -68,27 +79,41 @@ public class LifecycleMethod implements MetaAttribute {
 				.append(parameterName)
 				.append(')')
 				.append(";\n")
-				.append("\t}\n")
+				.append("\t}\n");
+	}
+
+	private void preamble(StringBuilder declaration) {
+		declaration
+				.append("\n@Override\npublic void ")
+				.append(methodName)
+				.append('(');
+		notNull(declaration);
+		declaration
+				.append(annotationMetaEntity.importType(entity))
+				.append(' ')
+				.append(parameterName)
+				.append(')')
+				.append(" {\n");
+	}
+
+	private void nullCheck(StringBuilder declaration) {
+		declaration
+				.append("\tif (")
+				.append(parameterName)
+				.append(" == null) throw new IllegalArgumentException(\"Null ")
+				.append(parameterName)
+				.append("\");\n");
+	}
+
+	private void convertException(StringBuilder declaration, String exception, String convertedException) {
+		declaration
 				.append("\tcatch (")
-				.append(annotationMetaEntity.importType(isInsert
+				.append(annotationMetaEntity.importType(exception))
-						? "jakarta.persistence.EntityExistsException"
-						: "jakarta.persistence.OptimisticLockException"))
 				.append(" exception) {\n")
 				.append("\t\tthrow new ")
-				.append(annotationMetaEntity.importType(isInsert
+				.append(annotationMetaEntity.importType(convertedException))
-						? "jakarta.data.exceptions.EntityExistsException"
-						: "jakarta.data.exceptions.OptimisticLockingFailureException"))
 				.append("(exception);\n")
-				.append("\t}\n")
+				.append("\t}\n");
-				.append("\tcatch (")
-				.append(annotationMetaEntity.importType("jakarta.persistence.PersistenceException"))
-				.append(" exception) {\n")
-				.append("\t\tthrow new ")
-				.append(annotationMetaEntity.importType("jakarta.data.exceptions.DataException"))
-				.append("(exception);\n")
-				.append("\t}\n")
-				.append("}");
-		return declaration.toString();
 	}
 
 	private void notNull(StringBuilder declaration) {

tooling/metamodel-generator/src/main/java/org/hibernate/jpamodelgen/annotation/NaturalIdFinderMethod.java

@@ -47,6 +47,7 @@ public class NaturalIdFinderMethod extends AbstractFinderMethod {
 		final StringBuilder declaration = new StringBuilder();
 		comment( declaration );
 		preamble( declaration );
+		tryReturn( declaration );
 		unwrapSession( declaration );
 		if ( isReactive() ) {
 			findReactively( declaration );
@@ -55,7 +56,7 @@ public class NaturalIdFinderMethod extends AbstractFinderMethod {
 			findBlockingly( declaration );
 		}
 		convertExceptions( declaration );
-		declaration.append("\n}");
+		closingBrace( declaration );
 		return declaration.toString();
 	}
 
@@ -83,8 +84,7 @@ public class NaturalIdFinderMethod extends AbstractFinderMethod {
 	}
 
 	private void findReactively(StringBuilder declaration) {
-		boolean composite = paramTypes.stream()
+		boolean composite = isComposite();
-				.filter(type -> !isSessionParameter(type)).count()>1;
 		declaration
 				.append(".find(");
 		if (composite) {
@@ -131,4 +131,8 @@ public class NaturalIdFinderMethod extends AbstractFinderMethod {
 		declaration.append(");");
 	}
 
+	private boolean isComposite() {
+		return paramTypes.stream()
+				.filter(type -> !isSessionParameter(type)).count() > 1;
+	}
 }

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

@@ -80,31 +80,18 @@ public class QueryMethod extends AbstractQueryMethod {
 		final StringBuilder declaration = new StringBuilder();
 		comment( declaration );
 		modifiers( paramTypes, declaration );
-		declaration
+		preamble( declaration, returnType, paramTypes );
-				.append(returnType)
+		tryReturn( declaration );
-				.append(" ")
+		castResult( declaration, returnType );
-				.append(methodName);
+		createQuery( declaration );
-		parameters( paramTypes, declaration );
+		boolean unwrapped = setParameters( paramTypes, declaration );
-		declaration
+		execute( declaration, unwrapped );
-				.append(" {\n");
+		convertExceptions( declaration );
-		if (dataRepository) {
+		closingBrace( declaration );
-			declaration
+		return declaration.toString();
-					.append("\ttry {\n\t");
-		}
-		declaration
-				.append("\t");
-		if ( returnTypeName == null || !returnTypeName.equals("void") ) {
-			declaration
-					.append("return ");
-		}
-		if ( isNative && returnTypeName != null && containerTypeName == null
-				&& isUsingEntityManager() ) {
-			// EntityManager.createNativeQuery() does not return TypedQuery,
-			// so we need to cast to the entity type
-			declaration.append("(")
-					.append(returnType)
-					.append(") ");
 	}
+
+	private void createQuery(StringBuilder declaration) {
 		declaration
 				.append(sessionName)
 				.append(isNative ? ".createNativeQuery" : ".createQuery")
@@ -117,11 +104,40 @@ public class QueryMethod extends AbstractQueryMethod {
 					.append(".class");
 		}
 		declaration.append(")");
-		boolean unwrapped = setParameters( paramTypes, declaration );
+	}
-		execute( declaration, unwrapped );
+
-		convertExceptions( declaration );
+	private void castResult(StringBuilder declaration, StringBuilder returnType) {
-		declaration.append("\n}");
+		if ( isNative && returnTypeName != null && containerTypeName == null
-		return declaration.toString();
+				&& isUsingEntityManager() ) {
+			// EntityManager.createNativeQuery() does not return TypedQuery,
+			// so we need to cast to the entity type
+			declaration.append("(")
+					.append(returnType)
+					.append(") ");
+		}
+	}
+
+	private void tryReturn(StringBuilder declaration) {
+		if (dataRepository) {
+			declaration
+					.append("\ttry {\n\t");
+		}
+		declaration
+				.append("\t");
+		if ( returnTypeName == null || !returnTypeName.equals("void") ) {
+			declaration
+					.append("return ");
+		}
+	}
+
+	private void preamble(StringBuilder declaration, StringBuilder returnType, List<String> paramTypes) {
+		declaration
+				.append(returnType)
+				.append(" ")
+				.append(methodName);
+		parameters( paramTypes, declaration );
+		declaration
+				.append(" {\n");
 	}
 
 	private void execute(StringBuilder declaration, boolean unwrapped) {