Polishing.

Reference issues in tests comments.
Removed `DisabledOnDatabase`
IdGeneration default methods related to sequence generation are now internally consistent.
Formatting and naming.
IdGeneration offers simple support by default.
Fix exception in oracle integration test setup
Use SqlIdentifier for sequence names
Remove SEQUENCE id source
Added documentation

See #1923
Original pull request #1955

Author: schauder

pom.xml

@@ -1,5 +1,6 @@
 <?xml version="1.0" encoding="UTF-8" standalone="no"?>
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+		 xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
 
 	<modelVersion>4.0.0</modelVersion>
 

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

@@ -11,6 +11,7 @@
 import org.springframework.data.relational.core.mapping.RelationalMappingContext;
 import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
 import org.springframework.data.relational.core.mapping.event.BeforeSaveCallback;
+import org.springframework.data.relational.core.sql.SqlIdentifier;
 import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
 import org.springframework.util.Assert;
 
@@ -40,23 +41,25 @@ public IdGeneratingBeforeSaveCallback(
 
     @Override
     public Object onBeforeSave(Object aggregate, MutableAggregateChange<Object> aggregateChange) {
+
         Assert.notNull(aggregate, "The aggregate cannot be null at this point");
+
         RelationalPersistentEntity<?> persistentEntity = relationalMappingContext.getPersistentEntity(aggregate.getClass());
-        Optional<String> idTargetSequence = persistentEntity.getIdTargetSequence();
+        Optional<SqlIdentifier> idSequence = persistentEntity.getIdSequence();
 
         if (dialect.getIdGeneration().sequencesSupported()) {
 
             if (persistentEntity.getIdProperty() != null) {
-                idTargetSequence
-                  .map(s -> dialect.getIdGeneration().nextValueFromSequenceSelect(s))
+                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 (idTargetSequence.isPresent()) {
+            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

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/mapping/IdGeneratingBeforeSaveCallbackTest.java

@@ -14,7 +14,7 @@
 import org.springframework.data.relational.core.dialect.PostgresDialect;
 import org.springframework.data.relational.core.mapping.RelationalMappingContext;
 import org.springframework.data.relational.core.mapping.Table;
-import org.springframework.data.relational.core.mapping.TargetSequence;
+import org.springframework.data.relational.core.mapping.Sequence;
 import org.springframework.data.relational.core.sql.IdentifierProcessing;
 import org.springframework.jdbc.core.RowMapper;
 import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
@@ -26,68 +26,58 @@
  */
 class IdGeneratingBeforeSaveCallbackTest {
 
-    @Test
-    void test_mySqlDialect_sequenceGenerationIsNotSupported() {
-        // given
-        RelationalMappingContext relationalMappingContext = new RelationalMappingContext();
+    @Test // GH-1923
+    void mySqlDialectsequenceGenerationIsNotSupported() {
+
+		RelationalMappingContext relationalMappingContext = new RelationalMappingContext();
         MySqlDialect mySqlDialect = new MySqlDialect(IdentifierProcessing.NONE);
         NamedParameterJdbcOperations operations = mock(NamedParameterJdbcOperations.class);
 
-        // and
         IdGeneratingBeforeSaveCallback subject = new IdGeneratingBeforeSaveCallback(relationalMappingContext, mySqlDialect, operations);
 
         NoSequenceEntity entity = new NoSequenceEntity();
 
-        // when
         Object processed = subject.onBeforeSave(entity, MutableAggregateChange.forSave(entity));
 
-        // then
         Assertions.assertThat(processed).isSameAs(entity);
         Assertions.assertThat(processed).usingRecursiveComparison().isEqualTo(entity);
     }
 
-    @Test
-    void test_EntityIsNotMarkedWithTargetSequence() {
-        // given
-        RelationalMappingContext relationalMappingContext = new RelationalMappingContext();
+    @Test // GH-1923
+    void entityIsNotMarkedWithTargetSequence() {
+
+		RelationalMappingContext relationalMappingContext = new RelationalMappingContext();
         PostgresDialect mySqlDialect = PostgresDialect.INSTANCE;
         NamedParameterJdbcOperations operations = mock(NamedParameterJdbcOperations.class);
 
-        // and
         IdGeneratingBeforeSaveCallback subject = new IdGeneratingBeforeSaveCallback(relationalMappingContext, mySqlDialect, operations);
 
         NoSequenceEntity entity = new NoSequenceEntity();
 
-        // when
         Object processed = subject.onBeforeSave(entity, MutableAggregateChange.forSave(entity));
 
-        // then
         Assertions.assertThat(processed).isSameAs(entity);
         Assertions.assertThat(processed).usingRecursiveComparison().isEqualTo(entity);
     }
 
-    @Test
-    void test_EntityIdIsPopulatedFromSequence() {
-        // given
+    @Test // GH-1923
+    void entityIdIsPopulatedFromSequence() {
+
         RelationalMappingContext relationalMappingContext = new RelationalMappingContext();
         relationalMappingContext.getRequiredPersistentEntity(EntityWithSequence.class);
 
         PostgresDialect mySqlDialect = PostgresDialect.INSTANCE;
         NamedParameterJdbcOperations operations = mock(NamedParameterJdbcOperations.class);
 
-        // and
         long generatedId = 112L;
         when(operations.queryForObject(anyString(), anyMap(), any(RowMapper.class))).thenReturn(generatedId);
 
-        // and
         IdGeneratingBeforeSaveCallback subject = new IdGeneratingBeforeSaveCallback(relationalMappingContext, mySqlDialect, operations);
 
         EntityWithSequence entity = new EntityWithSequence();
 
-        // when
         Object processed = subject.onBeforeSave(entity, MutableAggregateChange.forSave(entity));
 
-        // then
         Assertions.assertThat(processed).isSameAs(entity);
         Assertions
           .assertThat(processed)
@@ -109,7 +99,7 @@ static class NoSequenceEntity {
     static class EntityWithSequence {
 
         @Id
-        @TargetSequence(value = "id_seq", schema = "public")
+        @Sequence(value = "id_seq", schema = "public")
         private Long id;
 
         private Long name;

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java

@@ -65,7 +65,7 @@
 import org.springframework.data.relational.core.mapping.Column;
 import org.springframework.data.relational.core.mapping.MappedCollection;
 import org.springframework.data.relational.core.mapping.Table;
-import org.springframework.data.relational.core.mapping.TargetSequence;
+import org.springframework.data.relational.core.mapping.Sequence;
 import org.springframework.data.relational.core.mapping.event.AbstractRelationalEvent;
 import org.springframework.data.relational.core.mapping.event.AfterConvertEvent;
 import org.springframework.data.relational.core.sql.LockMode;
@@ -126,9 +126,10 @@ public void savesAnEntity() {
 				"id_Prop = " + entity.getIdProp())).isEqualTo(1);
 	}
 
-	@Test
+	@Test // GH-1923
 	@EnabledOnFeature(value = TestDatabaseFeatures.Feature.SUPPORTS_SEQUENCES)
 	public void saveEntityWithTargetSequenceSpecified() {
+
 		EntityWithSequence first = entityWithSequenceRepository.save(new EntityWithSequence("first"));
 		EntityWithSequence second = entityWithSequenceRepository.save(new EntityWithSequence("second"));
 
@@ -139,9 +140,10 @@ public void saveEntityWithTargetSequenceSpecified() {
 		assertThat(second.getName()).isEqualTo("second");
 	}
 
-	@Test
+	@Test // GH-1923
 	@EnabledOnFeature(value = TestDatabaseFeatures.Feature.SUPPORTS_SEQUENCES)
 	public void batchInsertEntityWithTargetSequenceSpecified() {
+
 		Iterable<EntityWithSequence> results = entityWithSequenceRepository
 				.saveAll(List.of(new EntityWithSequence("first"), new EntityWithSequence("second")));
 
@@ -1862,7 +1864,7 @@ private static DummyEntity createEntity(String entityName, Consumer<DummyEntity>
 	static class EntityWithSequence {
 
 		@Id
-		@TargetSequence(sequence = "entity_sequence") private Long id;
+		@Sequence(sequence = "ENTITY_SEQUENCE") private Long id;
 
 		private String name;
 

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DisabledOnDatabase.java

@@ -47,4 +47,4 @@ CREATE TABLE ENTITY_WITH_SEQUENCE
     NAME VARCHAR(100)
 );
 
-CREATE SEQUENCE ENTITY_SEQUENCE START WITH 1 INCREMENT BY 1 NO MAXVALUE;
+CREATE SEQUENCE `ENTITY_SEQUENCE` START WITH 1 INCREMENT BY 1 NO MAXVALUE;

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DisabledOnDatabaseExecutionCondition.java

@@ -47,12 +47,12 @@ CREATE TABLE WITH_DELIMITED_COLUMN
     ID                      NUMBER GENERATED BY DEFAULT ON NULL AS IDENTITY PRIMARY KEY,
     "ORG.XTUNIT.IDENTIFIER" VARCHAR(100),
     STYPE                   VARCHAR(100)
-)
+);
 
 CREATE TABLE ENTITY_WITH_SEQUENCE
 (
-    ID BIGINT,
+    ID NUMBER,
     NAME VARCHAR(100)
 );
 
-CREATE SEQUENCE ENTITY_SEQUENCE START WITH 1 INCREMENT BY 1 NO MAXVALUE;
+CREATE SEQUENCE ENTITY_SEQUENCE START WITH 1 INCREMENT BY 1;

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mariadb.sql

@@ -55,4 +55,4 @@ CREATE TABLE ENTITY_WITH_SEQUENCE
     NAME VARCHAR(100)
 );
 
-CREATE SEQUENCE ENTITY_SEQUENCE START WITH 1 INCREMENT BY 1 NO MAXVALUE;
+CREATE SEQUENCE "ENTITY_SEQUENCE" START WITH 1 INCREMENT BY 1 NO MAXVALUE;

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-oracle.sql

@@ -15,8 +15,6 @@
  */
 package org.springframework.data.relational.core.conversion;
 
-import java.util.Optional;
-
 import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
 import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
 
@@ -42,12 +40,7 @@ public enum IdValueSource {
 	/**
 	 * There is no id property, and therefore no id value source.
 	 */
-	NONE,
-
-	/**
-	 * The id should be dervied from the database sequence
-	 */
-	SEQUENCE;
+	NONE;
 
 	/**
 	 * Returns the appropriate {@link IdValueSource} for the instance: {@link IdValueSource#NONE} when the entity has no
@@ -56,9 +49,8 @@ public enum IdValueSource {
 	 */
 	public static <T> IdValueSource forInstance(Object instance, RelationalPersistentEntity<T> persistentEntity) {
 
-		Optional<String> idTargetSequence = persistentEntity.getIdTargetSequence();
-		if (idTargetSequence.isPresent()) {
-			return IdValueSource.SEQUENCE;
+		if (persistentEntity.getIdSequence().isPresent()) {
+			return IdValueSource.PROVIDED;
 		}
 
 		Object idValue = persistentEntity.getIdentifierAccessor(instance).getIdentifier();

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-postgres.sql

@@ -18,8 +18,8 @@
 import java.util.Collection;
 import java.util.Collections;
 
-import org.springframework.data.relational.core.sql.IdentifierProcessing;
 import org.springframework.data.relational.core.sql.LockOptions;
+import org.springframework.data.relational.core.sql.SqlIdentifier;
 
 /**
  * An SQL dialect for DB2.
@@ -41,14 +41,20 @@ public boolean supportedForBatchOperations() {
 			return false;
 		}
 
-		/**
-		 * This workaround (non-ANSI SQL way of querying sequence) exists for the same reasons it exists for {@link HsqlDbDialect}
-		 *
-		 * @see HsqlDbDialect#getIdGeneration()#nextValueFromSequenceSelect(String)
-		 */
 		@Override
-		public String nextValueFromSequenceSelect(String sequenceName) {
-			return "SELECT NEXT VALUE FOR %s FROM SYSCAT.SEQUENCES LIMIT 1".formatted(sequenceName);
+		public boolean sequencesSupported() {
+			return true;
+		}
+
+		@Override
+		public String createSequenceQuery(SqlIdentifier sequenceName) {
+			/*
+			 * This workaround (non-ANSI SQL way of querying sequence) exists for the same reasons it exists for {@link HsqlDbDialect}
+			 *
+			 * @see HsqlDbDialect#getIdGeneration()#nextValueFromSequenceSelect(String)
+			 */
+			return "SELECT NEXT VALUE FOR %s FROM SYSCAT.SEQUENCES LIMIT 1"
+					.formatted(sequenceName.toSql(INSTANCE.getIdentifierProcessing()));
 		}
 	};
 
@@ -104,11 +110,6 @@ public Position getClausePosition() {
 		};
 	}
 
-	@Override
-	public IdentifierProcessing getIdentifierProcessing() {
-		return IdentifierProcessing.ANSI;
-	}
-
 	@Override
 	public Collection<Object> getConverters() {
 		return Collections.singletonList(TimestampAtUtcToOffsetDateTimeConverter.INSTANCE);