Polishing.

Refine assignment flow and use early returns where possible. Cache empty MapSqlParameterSource. Reduce dependency on RelationalMappingContext using a lower-level abstraction signature. Simplify names. Use default value check from Commons. Fix log warning message. Add missing since tags.

Remove superfluous annotations and redundant code. Tweak documentation wording.

Closes #2003
Original pull request: #2005

Author: mp911de

Diff for: spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/IdGeneratingBeforeSaveCallback.java

@@ -1,89 +1,112 @@
 package org.springframework.data.jdbc.core.mapping;
 
-import java.util.Map;
 import java.util.Optional;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-import org.springframework.data.jdbc.repository.config.AbstractJdbcConfiguration;
+
+import org.springframework.data.mapping.PersistentProperty;
 import org.springframework.data.mapping.PersistentPropertyAccessor;
+import org.springframework.data.mapping.context.MappingContext;
 import org.springframework.data.relational.core.conversion.MutableAggregateChange;
 import org.springframework.data.relational.core.dialect.Dialect;
-import org.springframework.data.relational.core.mapping.RelationalMappingContext;
 import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
+import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
 import org.springframework.data.relational.core.mapping.event.BeforeSaveCallback;
 import org.springframework.data.relational.core.sql.SqlIdentifier;
+import org.springframework.data.util.ReflectionUtils;
+import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
 import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
 import org.springframework.util.Assert;
+import org.springframework.util.ClassUtils;
+import org.springframework.util.NumberUtils;
 
 /**
- * Callback for generating ID via the database sequence. By default, it is registered as a bean in
- * {@link AbstractJdbcConfiguration}
+ * Callback for generating identifier values through a database sequence.
  *
  * @author Mikhail Polivakha
+ * @author Mark Paluch
+ * @since 3.5
+ * @see org.springframework.data.relational.core.mapping.Sequence
  */
 public class IdGeneratingBeforeSaveCallback implements BeforeSaveCallback<Object> {
 
 	private static final Log LOG = LogFactory.getLog(IdGeneratingBeforeSaveCallback.class);
+	private final static MapSqlParameterSource EMPTY_PARAMETERS = new MapSqlParameterSource();
 
-	private final RelationalMappingContext relationalMappingContext;
+	private final MappingContext<RelationalPersistentEntity<?>, ? extends RelationalPersistentProperty> mappingContext;
 	private final Dialect dialect;
 	private final NamedParameterJdbcOperations operations;
 
-	public IdGeneratingBeforeSaveCallback(RelationalMappingContext relationalMappingContext, Dialect dialect,
-			NamedParameterJdbcOperations namedParameterJdbcOperations) {
-		this.relationalMappingContext = relationalMappingContext;
+	public IdGeneratingBeforeSaveCallback(
+			MappingContext<RelationalPersistentEntity<?>, ? extends RelationalPersistentProperty> mappingContext,
+			Dialect dialect, NamedParameterJdbcOperations operations) {
+		this.mappingContext = mappingContext;
 		this.dialect = dialect;
-		this.operations = namedParameterJdbcOperations;
+		this.operations = operations;
 	}
 
 	@Override
 	public Object onBeforeSave(Object aggregate, MutableAggregateChange<Object> aggregateChange) {
 
-		Assert.notNull(aggregate, "The aggregate cannot be null at this point");
+		Assert.notNull(aggregate, "aggregate must not be null");
 
-		RelationalPersistentEntity<?> persistentEntity = relationalMappingContext.getPersistentEntity(aggregate.getClass());
+		RelationalPersistentEntity<?> entity = mappingContext.getRequiredPersistentEntity(aggregate.getClass());
 
-		if (!persistentEntity.hasIdProperty()) {
+		if (!entity.hasIdProperty()) {
 			return aggregate;
 		}
 
-		// we're doing INSERT and ID property value is not set explicitly by client
-		if (persistentEntity.isNew(aggregate) && !hasIdentifierValue(aggregate, persistentEntity)) {
-			return potentiallyFetchIdFromSequence(aggregate, persistentEntity);
-		} else {
+		RelationalPersistentProperty idProperty = entity.getRequiredIdProperty();
+		PersistentPropertyAccessor<Object> accessor = entity.getPropertyAccessor(aggregate);
+
+		if (!entity.isNew(aggregate) || hasIdentifierValue(idProperty, accessor)) {
 			return aggregate;
 		}
+
+		potentiallyFetchIdFromSequence(idProperty, entity, accessor);
+		return accessor.getBean();
 	}
 
-	private boolean hasIdentifierValue(Object aggregate, RelationalPersistentEntity<?> persistentEntity) {
-		Object identifier = persistentEntity.getIdentifierAccessor(aggregate).getIdentifier();
+	private boolean hasIdentifierValue(PersistentProperty<?> idProperty,
+			PersistentPropertyAccessor<Object> propertyAccessor) {
 
-		if (persistentEntity.getIdProperty().getType().isPrimitive()) {
-			return identifier instanceof Number num && num.longValue() != 0L;
-		} else {
-			return identifier != null;
+		Object identifier = propertyAccessor.getProperty(idProperty);
+
+		if (idProperty.getType().isPrimitive()) {
+
+			Object primitiveDefault = ReflectionUtils.getPrimitiveDefault(idProperty.getType());
+			return !primitiveDefault.equals(identifier);
 		}
+
+		return identifier != null;
 	}
 
-	private Object potentiallyFetchIdFromSequence(Object aggregate, RelationalPersistentEntity<?> persistentEntity) {
+	@SuppressWarnings("unchecked")
+	private void potentiallyFetchIdFromSequence(PersistentProperty<?> idProperty,
+			RelationalPersistentEntity<?> persistentEntity, PersistentPropertyAccessor<Object> accessor) {
+
 		Optional<SqlIdentifier> idSequence = persistentEntity.getIdSequence();
 
-		if (dialect.getIdGeneration().sequencesSupported()) {
-			idSequence.map(s -> dialect.getIdGeneration().createSequenceQuery(s)).ifPresent(sql -> {
-				Long idValue = operations.queryForObject(sql, Map.of(), (rs, rowNum) -> rs.getLong(1));
-				PersistentPropertyAccessor<Object> propertyAccessor = persistentEntity.getPropertyAccessor(aggregate);
-				propertyAccessor.setProperty(persistentEntity.getRequiredIdProperty(), idValue);
-			});
-		} else {
-			if (idSequence.isPresent()) {
-				LOG.warn("""
-						It seems you're trying to insert an aggregate of type '%s' annotated with @TargetSequence, but the problem is RDBMS you're
-						working with does not support sequences as such. Falling back to identity columns
-						""".formatted(aggregate.getClass().getName()));
-			}
+		if (idSequence.isPresent() && !dialect.getIdGeneration().sequencesSupported()) {
+			LOG.warn("""
+					Aggregate type '%s' is marked for sequence usage but configured dialect '%s'
+					does not support sequences. Falling back to identity columns.
+					""".formatted(persistentEntity.getType(), ClassUtils.getQualifiedName(dialect.getClass())));
+			return;
 		}
 
-		return aggregate;
+		idSequence.map(s -> dialect.getIdGeneration().createSequenceQuery(s)).ifPresent(sql -> {
+
+			Object idValue = operations.queryForObject(sql, EMPTY_PARAMETERS, (rs, rowNum) -> rs.getObject(1));
+
+			Class<?> targetType = ClassUtils.resolvePrimitiveIfNecessary(idProperty.getType());
+			if (idValue instanceof Number && Number.class.isAssignableFrom(targetType)) {
+				accessor.setProperty(idProperty,
+						NumberUtils.convertNumberToTargetClass((Number) idValue, (Class<? extends Number>) targetType));
+			} else {
+				accessor.setProperty(idProperty, idValue);
+			}
+		});
 	}
 }

Diff for: spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/AbstractJdbcConfiguration.java

@@ -126,13 +126,11 @@ public JdbcMappingContext jdbcMappingContext(Optional<NamingStrategy> namingStra
 	 * {@link #jdbcDialect(NamedParameterJdbcOperations)}.
 	 *
 	 * @return must not be {@literal null}.
+	 * @since 3.5
 	 */
 	@Bean
-	public IdGeneratingBeforeSaveCallback idGeneratingBeforeSaveCallback(
-		JdbcMappingContext mappingContext,
-		NamedParameterJdbcOperations operations,
-		Dialect dialect
-	) {
+	public IdGeneratingBeforeSaveCallback idGeneratingBeforeSaveCallback(JdbcMappingContext mappingContext,
+			NamedParameterJdbcOperations operations, Dialect dialect) {
 		return new IdGeneratingBeforeSaveCallback(mappingContext, dialect, operations);
 	}
 
@@ -224,8 +222,7 @@ public DataAccessStrategy dataAccessStrategyBean(NamedParameterJdbcOperations op
 
 		SqlGeneratorSource sqlGeneratorSource = new SqlGeneratorSource(context, jdbcConverter, dialect);
 		DataAccessStrategyFactory factory = new DataAccessStrategyFactory(sqlGeneratorSource, jdbcConverter, operations,
-				new SqlParametersFactory(context, jdbcConverter),
-				new InsertStrategyFactory(operations, dialect));
+				new SqlParametersFactory(context, jdbcConverter), new InsertStrategyFactory(operations, dialect));
 
 		return factory.create();
 	}