From 8c017fc56b7e23760a33d910c075e3d56623b5f9 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Thu, 30 Jan 2025 10:27:15 +0100 Subject: [PATCH] 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 --- pom.xml | 3 +- .../IdGeneratingBeforeSaveCallback.java | 11 ++-- .../IdGeneratingBeforeSaveCallbackTest.java | 36 +++++-------- .../JdbcRepositoryIntegrationTests.java | 10 ++-- .../data/jdbc/testing/DisabledOnDatabase.java | 27 ---------- .../DisabledOnDatabaseExecutionCondition.java | 36 ------------- ...JdbcRepositoryIntegrationTests-mariadb.sql | 2 +- .../JdbcRepositoryIntegrationTests-oracle.sql | 6 +-- ...dbcRepositoryIntegrationTests-postgres.sql | 2 +- .../core/conversion/IdValueSource.java | 14 ++--- .../relational/core/dialect/Db2Dialect.java | 27 +++++----- .../relational/core/dialect/H2Dialect.java | 13 ++--- .../core/dialect/HsqlDbDialect.java | 21 ++++---- .../relational/core/dialect/IdGeneration.java | 46 ++++++++++------ .../core/dialect/MariaDbDialect.java | 13 +---- .../relational/core/dialect/MySqlDialect.java | 17 ++++-- .../core/dialect/OracleDialect.java | 16 +++--- .../core/dialect/PostgresDialect.java | 28 +++++----- .../core/dialect/SqlServerDialect.java | 13 ++--- .../BasicRelationalPersistentEntity.java | 50 ++++++++---------- .../EmbeddedRelationalPersistentEntity.java | 2 +- .../mapping/RelationalPersistentEntity.java | 2 +- .../relational/core/mapping/Sequence.java | 46 ++++++++++++++++ .../core/mapping/TargetSequence.java | 43 --------------- ...icRelationalPersistentEntityUnitTests.java | 52 ++++++++++++------- .../modules/ROOT/partials/id-generation.adoc | 4 +- 26 files changed, 250 insertions(+), 290 deletions(-) delete mode 100644 spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DisabledOnDatabase.java delete mode 100644 spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DisabledOnDatabaseExecutionCondition.java create mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/Sequence.java delete mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/TargetSequence.java diff --git a/pom.xml b/pom.xml index 028dfdbd..aceb2aab 100644 --- a/pom.xml +++ b/pom.xml @@ -1,5 +1,6 @@ - + 4.0.0 diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/IdGeneratingBeforeSaveCallback.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/IdGeneratingBeforeSaveCallback.java index 0f8de428..f40a8abe 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/IdGeneratingBeforeSaveCallback.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/IdGeneratingBeforeSaveCallback.java @@ -11,6 +11,7 @@ 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.event.BeforeSaveCallback; +import org.springframework.data.relational.core.sql.SqlIdentifier; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; import org.springframework.util.Assert; @@ -40,15 +41,17 @@ public class IdGeneratingBeforeSaveCallback implements BeforeSaveCallback aggregateChange) { + Assert.notNull(aggregate, "The aggregate cannot be null at this point"); + RelationalPersistentEntity persistentEntity = relationalMappingContext.getPersistentEntity(aggregate.getClass()); - Optional idTargetSequence = persistentEntity.getIdTargetSequence(); + Optional 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 propertyAccessor = persistentEntity.getPropertyAccessor(aggregate); @@ -56,7 +59,7 @@ public class IdGeneratingBeforeSaveCallback implements BeforeSaveCallback results = entityWithSequenceRepository .saveAll(List.of(new EntityWithSequence("first"), new EntityWithSequence("second"))); @@ -1862,7 +1864,7 @@ public class JdbcRepositoryIntegrationTests { static class EntityWithSequence { @Id - @TargetSequence(sequence = "entity_sequence") private Long id; + @Sequence(sequence = "ENTITY_SEQUENCE") private Long id; private String name; diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DisabledOnDatabase.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DisabledOnDatabase.java deleted file mode 100644 index c83ec900..00000000 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DisabledOnDatabase.java +++ /dev/null @@ -1,27 +0,0 @@ -package org.springframework.data.jdbc.testing; - -import java.lang.annotation.Documented; -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -import org.junit.jupiter.api.extension.ExtendWith; -import org.springframework.test.context.junit.jupiter.EnabledIf; - -/** - * Annotation that allows to disable a particular test to be executed on a particular database - * - * @author Mikhail Polivakha - */ -@Target({ElementType.TYPE, ElementType.METHOD}) -@Retention(RetentionPolicy.RUNTIME) -@Documented -@ExtendWith(DisabledOnDatabaseExecutionCondition.class) -public @interface DisabledOnDatabase { - - /** - * The database on which the test is not supposed to run on - */ - DatabaseType database(); -} diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DisabledOnDatabaseExecutionCondition.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DisabledOnDatabaseExecutionCondition.java deleted file mode 100644 index 17f9bfdf..00000000 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DisabledOnDatabaseExecutionCondition.java +++ /dev/null @@ -1,36 +0,0 @@ -package org.springframework.data.jdbc.testing; - -import org.apache.commons.lang3.ArrayUtils; -import org.junit.jupiter.api.extension.ConditionEvaluationResult; -import org.junit.jupiter.api.extension.ExecutionCondition; -import org.junit.jupiter.api.extension.ExtensionContext; -import org.springframework.context.ApplicationContext; -import org.springframework.core.annotation.MergedAnnotation; -import org.springframework.core.annotation.MergedAnnotations; -import org.springframework.test.context.junit.jupiter.SpringExtension; - -/** - * {@link ExecutionCondition} for the {@link DisabledOnDatabase} annotation - * - * @author Mikhail Polivakha - */ -public class DisabledOnDatabaseExecutionCondition implements ExecutionCondition { - - @Override - public ConditionEvaluationResult evaluateExecutionCondition(ExtensionContext context) { - ApplicationContext applicationContext = SpringExtension.getApplicationContext(context); - - MergedAnnotation disabledOnDatabaseMergedAnnotation = MergedAnnotations - .from(context.getRequiredTestMethod(), MergedAnnotations.SearchStrategy.DIRECT) - .get(DisabledOnDatabase.class); - - DatabaseType database = disabledOnDatabaseMergedAnnotation.getEnum("database", DatabaseType.class); - - if (ArrayUtils.contains(applicationContext.getEnvironment().getActiveProfiles(), database.getProfile())) { - return ConditionEvaluationResult.disabled( - "The test method '%s' is disabled for '%s' because of the @DisabledOnDatabase annotation".formatted(context.getRequiredTestMethod().getName(), database) - ); - } - return ConditionEvaluationResult.enabled("The test method '%s' is enabled".formatted(context.getRequiredTestMethod())); - } -} diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mariadb.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mariadb.sql index 19ebad8b..23d3ad72 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mariadb.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mariadb.sql @@ -47,4 +47,4 @@ CREATE TABLE ENTITY_WITH_SEQUENCE NAME VARCHAR(100) ); -CREATE SEQUENCE ENTITY_SEQUENCE START WITH 1 INCREMENT BY 1 NO MAXVALUE; \ No newline at end of file +CREATE SEQUENCE `ENTITY_SEQUENCE` START WITH 1 INCREMENT BY 1 NO MAXVALUE; \ No newline at end of file diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-oracle.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-oracle.sql index 179ac5ab..428ff48f 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-oracle.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-oracle.sql @@ -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; \ No newline at end of file +CREATE SEQUENCE ENTITY_SEQUENCE START WITH 1 INCREMENT BY 1; \ No newline at end of file diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-postgres.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-postgres.sql index 14dff059..42e69437 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-postgres.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-postgres.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; \ No newline at end of file +CREATE SEQUENCE "ENTITY_SEQUENCE" START WITH 1 INCREMENT BY 1 NO MAXVALUE; \ No newline at end of file diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/IdValueSource.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/IdValueSource.java index 0ee53848..047fa435 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/IdValueSource.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/IdValueSource.java @@ -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 IdValueSource forInstance(Object instance, RelationalPersistentEntity persistentEntity) { - Optional idTargetSequence = persistentEntity.getIdTargetSequence(); - if (idTargetSequence.isPresent()) { - return IdValueSource.SEQUENCE; + if (persistentEntity.getIdSequence().isPresent()) { + return IdValueSource.PROVIDED; } Object idValue = persistentEntity.getIdentifierAccessor(instance).getIdentifier(); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java index 2658cf5c..118aef52 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java @@ -18,8 +18,8 @@ package org.springframework.data.relational.core.dialect; 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 class Db2Dialect extends AbstractDialect { 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 class Db2Dialect extends AbstractDialect { }; } - @Override - public IdentifierProcessing getIdentifierProcessing() { - return IdentifierProcessing.ANSI; - } - @Override public Collection getConverters() { return Collections.singletonList(TimestampAtUtcToOffsetDateTimeConverter.INSTANCE); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java index cf8f69d4..aaab1cb7 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java @@ -40,6 +40,9 @@ public class H2Dialect extends AbstractDialect { * Singleton instance. */ public static final H2Dialect INSTANCE = new H2Dialect(); + private static final IdentifierProcessing IDENTIFIER_PROCESSING = IdentifierProcessing.create(Quoting.ANSI, + LetterCasing.UPPER_CASE); + private static final IdGeneration ID_GENERATION = IdGeneration.create(IDENTIFIER_PROCESSING); protected H2Dialect() {} @@ -101,7 +104,7 @@ public class H2Dialect extends AbstractDialect { @Override public IdentifierProcessing getIdentifierProcessing() { - return IdentifierProcessing.create(Quoting.ANSI, LetterCasing.UPPER_CASE); + return IDENTIFIER_PROCESSING; } @Override @@ -117,12 +120,6 @@ public class H2Dialect extends AbstractDialect { @Override public IdGeneration getIdGeneration() { - return new IdGeneration() { - - @Override - public String nextValueFromSequenceSelect(String sequenceName) { - return "SELECT NEXT VALUE FOR %s".formatted(sequenceName); - } - }; + return ID_GENERATION; } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java index 51e7079f..d893bffc 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java @@ -15,6 +15,8 @@ */ package org.springframework.data.relational.core.dialect; +import org.springframework.data.relational.core.sql.SqlIdentifier; + /** * A {@link Dialect} for HsqlDb. * @@ -70,16 +72,17 @@ public class HsqlDbDialect extends AbstractDialect { public IdGeneration getIdGeneration() { return new IdGeneration() { - /** - * One may think that this is an over-complication, but it is actually not. - * There is no a direct way to query the next value for the sequence, only to use it as an expression - * inside other queries (SELECT/INSERT). Therefore, such a workaround is required - * - * @see The way JOOQ solves this problem - */ @Override - public String nextValueFromSequenceSelect(String sequenceName) { - return "SELECT NEXT VALUE FOR %s AS msq FROM INFORMATION_SCHEMA.SEQUENCES LIMIT 1".formatted(sequenceName); + public String createSequenceQuery(SqlIdentifier sequenceName) { + /* + * One may think that this is an over-complication, but it is actually not. + * There is no a direct way to query the next value for the sequence, only to use it as an expression + * inside other queries (SELECT/INSERT). Therefore, such a workaround is required + * + * @see The way JOOQ solves this problem + */ + return "SELECT NEXT VALUE FOR %s AS msq FROM INFORMATION_SCHEMA.SEQUENCES LIMIT 1" + .formatted(sequenceName.toSql(getIdentifierProcessing())); } }; } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/IdGeneration.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/IdGeneration.java index 738f3ec5..8b943233 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/IdGeneration.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/IdGeneration.java @@ -18,20 +18,30 @@ package org.springframework.data.relational.core.dialect; import java.sql.Connection; import java.sql.PreparedStatement; +import org.springframework.data.relational.core.sql.IdentifierProcessing; import org.springframework.data.relational.core.sql.SqlIdentifier; /** - * Encapsulates various properties that are related to ID generation process and specific to - * given {@link Dialect} + * Encapsulates various properties that are related to ID generation process and specific to given {@link Dialect} * * @author Jens Schauder * @author Chirag Tailor * @author Mikhail Polivakha - * * @since 2.1 */ public interface IdGeneration { + static IdGeneration create(IdentifierProcessing identifierProcessing) { + + return new IdGeneration() { + + @Override + public String createSequenceQuery(SqlIdentifier sequenceName) { + return IdGeneration.createSequenceQuery(sequenceName.toSql(identifierProcessing)); + } + }; + } + /** * A default instance working for many databases and equivalent to Spring Data JDBCs behavior before version 2.1. */ @@ -62,13 +72,6 @@ public interface IdGeneration { return id.getReference(); } - /** - * @return {@literal true} in case the sequences are supported by the underlying database, {@literal false} otherwise - */ - default boolean sequencesSupported() { - return true; - } - /** * Does the driver support id generation for batch operations. *

@@ -82,15 +85,28 @@ public interface IdGeneration { return true; } + /** + * @return {@literal true} in case the sequences are supported by the underlying database, {@literal false} otherwise + * @since 3.5 + */ + default boolean sequencesSupported() { + return true; + } + /** * The SQL statement that allows retrieving the next value from the passed sequence * - * @param sequenceName the sequence name to get the enxt value for + * @param sequenceName the sequence name to get the next value for * @return SQL string + * @since 3.5 */ - default String nextValueFromSequenceSelect(String sequenceName) { - throw new UnsupportedOperationException( - "Currently, there is no support for sequence generation for %s dialect. If you need it, please, submit a ticket".formatted(this.getClass().getSimpleName()) - ); + default String createSequenceQuery(SqlIdentifier sequenceName) { + + String nameString = sequenceName.toString(); + return createSequenceQuery(nameString); + } + + static String createSequenceQuery(String nameString) { + return "SELECT NEXT VALUE FOR" + nameString; } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/MariaDbDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/MariaDbDialect.java index 93c4261d..98a3115f 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/MariaDbDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/MariaDbDialect.java @@ -18,7 +18,6 @@ package org.springframework.data.relational.core.dialect; import java.util.Arrays; import java.util.Collection; -import org.springframework.data.annotation.Id; import org.springframework.data.relational.core.sql.IdentifierProcessing; /** @@ -36,19 +35,11 @@ public class MariaDbDialect extends MySqlDialect { @Override public Collection getConverters() { - return Arrays.asList( - TimestampAtUtcToOffsetDateTimeConverter.INSTANCE, - NumberToBooleanConverter.INSTANCE); + return Arrays.asList(TimestampAtUtcToOffsetDateTimeConverter.INSTANCE, NumberToBooleanConverter.INSTANCE); } @Override public IdGeneration getIdGeneration() { - return new IdGeneration() { - - @Override - public String nextValueFromSequenceSelect(String sequenceName) { - return "SELECT NEXTVAL(%s)".formatted(sequenceName); - } - }; + return IdGeneration.create(getIdentifierProcessing()); } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/MySqlDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/MySqlDialect.java index 323e4723..dd04c117 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/MySqlDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/MySqlDialect.java @@ -18,10 +18,12 @@ package org.springframework.data.relational.core.dialect; import java.util.Arrays; import java.util.Collection; +import org.jetbrains.annotations.NotNull; import org.springframework.data.relational.core.sql.IdentifierProcessing; -import org.springframework.data.relational.core.sql.LockOptions; import org.springframework.data.relational.core.sql.IdentifierProcessing.LetterCasing; import org.springframework.data.relational.core.sql.IdentifierProcessing.Quoting; +import org.springframework.data.relational.core.sql.LockOptions; +import org.springframework.data.relational.core.sql.SqlIdentifier; import org.springframework.util.Assert; /** @@ -131,10 +133,7 @@ public class MySqlDialect extends AbstractDialect { @Override public Collection getConverters() { - return Arrays.asList( - TimestampAtUtcToOffsetDateTimeConverter.INSTANCE, - NumberToBooleanConverter.INSTANCE - ); + return Arrays.asList(TimestampAtUtcToOffsetDateTimeConverter.INSTANCE, NumberToBooleanConverter.INSTANCE); } @Override @@ -144,12 +143,20 @@ public class MySqlDialect extends AbstractDialect { @Override public IdGeneration getIdGeneration() { + return new IdGeneration() { @Override public boolean sequencesSupported() { return false; } + + @Override + public String createSequenceQuery(@NotNull SqlIdentifier sequenceName) { + throw new UnsupportedOperationException( + "Currently, there is no support for sequence generation for %s dialect. If you need it, please, submit a ticket" + .formatted(this.getClass().getSimpleName())); + } }; } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java index eafd8cf5..7f654610 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java @@ -15,13 +15,14 @@ */ package org.springframework.data.relational.core.dialect; -import org.springframework.core.convert.converter.Converter; -import org.springframework.data.convert.WritingConverter; -import org.springframework.data.relational.core.sql.SqlIdentifier; +import static java.util.Arrays.*; import java.util.Collection; -import static java.util.Arrays.*; +import org.jetbrains.annotations.NotNull; +import org.springframework.core.convert.converter.Converter; +import org.springframework.data.convert.WritingConverter; +import org.springframework.data.relational.core.sql.SqlIdentifier; /** * An SQL dialect for Oracle. @@ -50,8 +51,8 @@ public class OracleDialect extends AnsiDialect { } @Override - public String nextValueFromSequenceSelect(String sequenceName) { - return "SELECT %s.nextval FROM DUAL".formatted(sequenceName); + public String createSequenceQuery(@NotNull SqlIdentifier sequenceName) { + return "SELECT %s.nextval FROM DUAL".formatted(sequenceName.toSql(INSTANCE.getIdentifierProcessing())); } }; @@ -64,7 +65,8 @@ public class OracleDialect extends AnsiDialect { @Override public Collection getConverters() { - return asList(TimestampAtUtcToOffsetDateTimeConverter.INSTANCE, NumberToBooleanConverter.INSTANCE, BooleanToIntegerConverter.INSTANCE); + return asList(TimestampAtUtcToOffsetDateTimeConverter.INSTANCE, NumberToBooleanConverter.INSTANCE, + BooleanToIntegerConverter.INSTANCE); } @WritingConverter diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java index a5ba6b67..6979c365 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java @@ -55,6 +55,16 @@ public class PostgresDialect extends AbstractDialect { private static final Set> POSTGRES_SIMPLE_TYPES = Set.of(UUID.class, URL.class, URI.class, InetAddress.class, Map.class); + private IdentifierProcessing identifierProcessing = IdentifierProcessing.create(Quoting.ANSI, + LetterCasing.LOWER_CASE); + private IdGeneration idGeneration = new IdGeneration() { + + @Override + public String createSequenceQuery(SqlIdentifier sequenceName) { + return "SELECT nextval('%s')".formatted(sequenceName.toSql(getIdentifierProcessing())); + } + }; + protected PostgresDialect() {} private static final LimitClause LIMIT_CLAUSE = new LimitClause() { @@ -131,10 +141,10 @@ public class PostgresDialect extends AbstractDialect { // without schema String tableName = last.toSql(this.identifierProcessing); - return switch (lockOptions.getLockMode()) { - case PESSIMISTIC_WRITE -> "FOR UPDATE OF " + tableName; - case PESSIMISTIC_READ -> "FOR SHARE OF " + tableName; - }; + return switch (lockOptions.getLockMode()) { + case PESSIMISTIC_WRITE -> "FOR UPDATE OF " + tableName; + case PESSIMISTIC_READ -> "FOR SHARE OF " + tableName; + }; } @Override @@ -145,7 +155,7 @@ public class PostgresDialect extends AbstractDialect { @Override public IdentifierProcessing getIdentifierProcessing() { - return IdentifierProcessing.create(Quoting.ANSI, LetterCasing.LOWER_CASE); + return identifierProcessing; } @Override @@ -160,12 +170,6 @@ public class PostgresDialect extends AbstractDialect { @Override public IdGeneration getIdGeneration() { - return new IdGeneration() { - - @Override - public String nextValueFromSequenceSelect(String sequenceName) { - return "SELECT nextval('%s')".formatted(sequenceName); - } - }; + return idGeneration; } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerDialect.java index 2a8a1e2e..36f0381f 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerDialect.java @@ -17,6 +17,7 @@ package org.springframework.data.relational.core.dialect; import org.springframework.data.relational.core.sql.IdentifierProcessing; import org.springframework.data.relational.core.sql.LockOptions; +import org.springframework.data.relational.core.sql.SqlIdentifier; import org.springframework.data.relational.core.sql.render.SelectRenderContext; import org.springframework.data.util.Lazy; @@ -36,6 +37,9 @@ public class SqlServerDialect extends AbstractDialect { */ public static final SqlServerDialect INSTANCE = new SqlServerDialect(); + private static final IdentifierProcessing IDENTIFIER_PROCESSING = IdentifierProcessing + .create(IdentifierProcessing.Quoting.ANSI, IdentifierProcessing.LetterCasing.AS_IS); + private static final IdGeneration ID_GENERATION = new IdGeneration() { @Override @@ -44,14 +48,11 @@ public class SqlServerDialect extends AbstractDialect { } @Override - public String nextValueFromSequenceSelect(String sequenceName) { - return "SELECT NEXT VALUE FOR %s".formatted(sequenceName); + public String createSequenceQuery(SqlIdentifier sequenceName) { + return IdGeneration.createSequenceQuery(sequenceName.toSql(IDENTIFIER_PROCESSING)); } }; - private static final IdentifierProcessing IDENTIFIER_PROCESSING = IdentifierProcessing - .create(IdentifierProcessing.Quoting.ANSI, IdentifierProcessing.LetterCasing.AS_IS); - protected SqlServerDialect() {} @Override @@ -86,7 +87,7 @@ public class SqlServerDialect extends AbstractDialect { @Override public String getLock(LockOptions lockOptions) { - + return switch (lockOptions.getLockMode()) { case PESSIMISTIC_WRITE -> "WITH (UPDLOCK, ROWLOCK)"; case PESSIMISTIC_READ -> "WITH (HOLDLOCK, ROWLOCK)"; diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentEntity.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentEntity.java index 75a501a3..cc84a950 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentEntity.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentEntity.java @@ -17,7 +17,8 @@ package org.springframework.data.relational.core.mapping; import java.util.Optional; -import org.jetbrains.annotations.NotNull; +import org.springframework.core.annotation.MergedAnnotation; +import org.springframework.core.annotation.MergedAnnotations; import org.springframework.data.mapping.model.BasicPersistentEntity; import org.springframework.data.relational.core.sql.SqlIdentifier; import org.springframework.data.util.Lazy; @@ -47,7 +48,7 @@ class BasicRelationalPersistentEntity extends BasicPersistentEntity tableName; private final @Nullable Expression tableNameExpression; - private final Lazy idTargetSequenceName; + private final Lazy idSequenceName; private final Lazy> schemaName; private final @Nullable Expression schemaNameExpression; @@ -91,7 +92,7 @@ class BasicRelationalPersistentEntity extends BasicPersistentEntity extends BasicPersistentEntity getIdTargetSequence() { - return idTargetSequenceName.getOptional(); + public Optional getIdSequence() { + return idSequenceName.getOptional(); } @Override @@ -174,33 +175,28 @@ class BasicRelationalPersistentEntity extends BasicPersistentEntity", getType()); } - private @Nullable String determineTargetSequenceName() { + private @Nullable SqlIdentifier determineSequenceName() { + RelationalPersistentProperty idProperty = getIdProperty(); - if (idProperty != null && idProperty.isAnnotationPresent(TargetSequence.class)) { - TargetSequence requiredAnnotation = idProperty.getRequiredAnnotation(TargetSequence.class); - if (!StringUtils.hasText(requiredAnnotation.sequence()) && !StringUtils.hasText(requiredAnnotation.value())) { - throw new IllegalStateException(""" - For the persistent entity '%s' the @TargetSequence annotation was specified for the @Id, however, neither - the value() nor the sequence() attributes are specified - """ - ); - } else { - String sequenceFullyQualifiedName = getSequenceName(requiredAnnotation); - if (StringUtils.hasText(requiredAnnotation.schema())) { - return String.join(".", requiredAnnotation.schema(), sequenceFullyQualifiedName); - } - return sequenceFullyQualifiedName; + if (idProperty != null && idProperty.isAnnotationPresent(Sequence.class)) { + + Sequence requiredAnnotation = idProperty.getRequiredAnnotation(Sequence.class); + + MergedAnnotation targetSequence = MergedAnnotations.from(requiredAnnotation) + .get(Sequence.class); + + String sequence = targetSequence.getString("sequence"); + String schema = targetSequence.getString("schema"); + + SqlIdentifier sequenceIdentifier = SqlIdentifier.quoted(sequence); + if (StringUtils.hasText(schema)) { + sequenceIdentifier = SqlIdentifier.from(SqlIdentifier.quoted(schema), sequenceIdentifier); } + + return sequenceIdentifier; } else { return null; } } - - @NotNull - private static String getSequenceName(TargetSequence requiredAnnotation) { - return Optional.of(requiredAnnotation.sequence()) - .filter(s -> !s.isBlank()) - .orElse(requiredAnnotation.value()); - } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/EmbeddedRelationalPersistentEntity.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/EmbeddedRelationalPersistentEntity.java index 433e9e25..5e23e1d5 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/EmbeddedRelationalPersistentEntity.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/EmbeddedRelationalPersistentEntity.java @@ -57,7 +57,7 @@ class EmbeddedRelationalPersistentEntity implements RelationalPersistentEntit } @Override - public Optional getIdTargetSequence() { + public Optional getIdSequence() { return Optional.empty(); } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntity.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntity.java index 33344513..025026a8 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntity.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntity.java @@ -58,5 +58,5 @@ public interface RelationalPersistentEntity extends MutablePersistentEntity getIdTargetSequence(); + Optional getIdSequence(); } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/Sequence.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/Sequence.java new file mode 100644 index 00000000..28cf43da --- /dev/null +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/Sequence.java @@ -0,0 +1,46 @@ +package org.springframework.data.relational.core.mapping; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import org.springframework.core.annotation.AliasFor; + +/** + * Specify the sequence from which the value for the {@link org.springframework.data.annotation.Id} should be fetched + * + * @author Mikhail Polivakha + */ +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.FIELD) +@Documented +public @interface Sequence { + + /** + * The name of the sequence from which the id should be fetched + */ + @AliasFor("sequence") + String value() default ""; + + /** + * Alias for {@link #value()} + */ + @AliasFor("value") + String sequence() default ""; + + /** + * Schema where the sequence reside. Technically, this attribute is not necessarily the schema. It just represents the + * location/namespace, where the sequence resides. For instance, in Oracle databases the schema and user are often + * used interchangeably, so {@link #schema() schema} attribute may represent an Oracle user as well. + *

+ * The final name of the sequence to be queried for the next value will be constructed by the concatenation of schema + * and sequence : + * + * 

+	 * schema().sequence()
+	 *
+ */ + String schema() default ""; +} diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/TargetSequence.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/TargetSequence.java deleted file mode 100644 index be16bcfc..00000000 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/TargetSequence.java +++ /dev/null @@ -1,43 +0,0 @@ -package org.springframework.data.relational.core.mapping; - -import java.lang.annotation.Documented; -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -import org.springframework.core.annotation.AliasFor; - -/** - * Specify the sequence from which the value for the {@link org.springframework.data.annotation.Id} - * should be fetched - * - * @author Mikhail Polivakha - */ -@Retention(RetentionPolicy.RUNTIME) -@Target(ElementType.FIELD) -@Documented -public @interface TargetSequence { - - /** - * The name of the sequence from which the id should be fetched - */ - String value() default ""; - - /** - * Alias for {@link #value()} - */ - @AliasFor("value") - String sequence() default ""; - - /** - * Schema where the sequence reside. - * Technically, this attribute is not necessarily the schema. It just represents the location/namespace, - * where the sequence resides. For instance, in Oracle databases the schema and user are often used - * interchangeably, so {@link #schema() schema} attribute may represent an Oracle user as well. - *

- * The final name of the sequence to be queried for the next value will be constructed by the concatenation - * of schema and sequence : 

schema().sequence()
- */ - String schema() default ""; -} diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentEntityUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentEntityUnitTests.java index a63c2d11..a6afba6d 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentEntityUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentEntityUnitTests.java @@ -58,32 +58,40 @@ class BasicRelationalPersistentEntityUnitTests { assertThat(entity.getTableName()).isEqualTo(quoted("dummy_sub_entity")); } - @Test - void entityWithNotargetSequence() { + @Test // GH-1923 + void entityWithNoSequence() { + RelationalPersistentEntity entity = mappingContext.getRequiredPersistentEntity(DummySubEntity.class); - assertThat(entity.getIdTargetSequence()).isEmpty(); + assertThat(entity.getIdSequence()).isEmpty(); } - @Test + @Test // GH-1923 void determineSequenceName() { - RelationalPersistentEntity persistentEntity = mappingContext.getPersistentEntity(EntityWithSequence.class); - assertThat(persistentEntity.getIdTargetSequence()).isPresent().hasValue("my_seq"); + RelationalPersistentEntity persistentEntity = mappingContext + .getRequiredPersistentEntity(EntityWithSequence.class); + + assertThat(persistentEntity.getIdSequence()).contains(SqlIdentifier.quoted("my_seq")); } - @Test + @Test // GH-1923 void determineSequenceNameFromValue() { - RelationalPersistentEntity persistentEntity = mappingContext.getPersistentEntity(EntityWithSequenceValueAlias.class); - assertThat(persistentEntity.getIdTargetSequence()).isPresent().hasValue("my_seq"); + RelationalPersistentEntity persistentEntity = mappingContext + .getRequiredPersistentEntity(EntityWithSequenceValueAlias.class); + + assertThat(persistentEntity.getIdSequence()).contains(SqlIdentifier.quoted("my_seq")); } - @Test + @Test // GH-1923 void determineSequenceNameWithSchemaSpecified() { - RelationalPersistentEntity persistentEntity = mappingContext.getPersistentEntity(EntityWithSequenceAndSchema.class); - assertThat(persistentEntity.getIdTargetSequence()).isPresent().hasValue("public.my_seq"); + RelationalPersistentEntity persistentEntity = mappingContext + .getRequiredPersistentEntity(EntityWithSequenceAndSchema.class); + + assertThat(persistentEntity.getIdSequence()) + .contains(SqlIdentifier.from(SqlIdentifier.quoted("public"), SqlIdentifier.quoted("my_seq"))); } @Test // DATAJDBC-294 @@ -203,8 +211,9 @@ class BasicRelationalPersistentEntityUnitTests { @Id private Long id; } - @Table(schema = "#{T(org.springframework.data.relational.core.mapping." - + "BasicRelationalPersistentEntityUnitTests$EntityWithSchemaAndTableSpelExpression).desiredSchemaName}", + @Table( + schema = "#{T(org.springframework.data.relational.core.mapping." + + "BasicRelationalPersistentEntityUnitTests$EntityWithSchemaAndTableSpelExpression).desiredSchemaName}", name = "#{T(org.springframework.data.relational.core.mapping." + "BasicRelationalPersistentEntityUnitTests$EntityWithSchemaAndTableSpelExpression).desiredTableName}") private static class EntityWithSchemaAndTableSpelExpression { @@ -213,10 +222,11 @@ class BasicRelationalPersistentEntityUnitTests { public static String desiredSchemaName = "HELP_ME_OBI_WON"; } - @Table(schema = "#{T(org.springframework.data.relational.core.mapping." - + "BasicRelationalPersistentEntityUnitTests$LittleBobbyTables).desiredSchemaName}", + @Table( + schema = "#{T(org.springframework.data.relational.core.mapping." + + "BasicRelationalPersistentEntityUnitTests$LittleBobbyTables).desiredSchemaName}", name = "#{T(org.springframework.data.relational.core.mapping." - + "BasicRelationalPersistentEntityUnitTests$LittleBobbyTables).desiredTableName}") + + "BasicRelationalPersistentEntityUnitTests$LittleBobbyTables).desiredTableName}") private static class LittleBobbyTables { @Id private Long id; public static String desiredTableName = "Robert'); DROP TABLE students;--"; @@ -232,19 +242,21 @@ class BasicRelationalPersistentEntityUnitTests { @Table("entity_with_sequence") static class EntityWithSequence { @Id - @TargetSequence(sequence = "my_seq") Long id; + @Sequence(sequence = "my_seq") Long id; } @Table("entity_with_sequence_value_alias") static class EntityWithSequenceValueAlias { @Id - @Column("myId") @TargetSequence(value = "my_seq") Long id; + @Column("myId") + @Sequence(value = "my_seq") Long id; } @Table("entity_with_sequence_and_schema") static class EntityWithSequenceAndSchema { @Id - @Column("myId") @TargetSequence(sequence = "my_seq", schema = "public") Long id; + @Column("myId") + @Sequence(sequence = "my_seq", schema = "public") Long id; } @Table() diff --git a/src/main/antora/modules/ROOT/partials/id-generation.adoc b/src/main/antora/modules/ROOT/partials/id-generation.adoc index e4f91b83..b654291a 100644 --- a/src/main/antora/modules/ROOT/partials/id-generation.adoc +++ b/src/main/antora/modules/ROOT/partials/id-generation.adoc @@ -6,7 +6,9 @@ The ID of an entity must be annotated with Spring Data's https://docs.spring.io/ When your database has an auto-increment column for the ID column, the generated value gets set in the entity after inserting it into the database. -Spring Data does not attempt to insert values of identifier columns when the entity is new and the identifier value defaults to its initial value. +If you annotate the id additionally with `@Sequence` a database sequence will be used to obtain values for the id. + +Otherwise Spring Data does not attempt to insert values of identifier columns when the entity is new and the identifier value defaults to its initial value. That is `0` for primitive types and `null` if the identifier property uses a numeric wrapper type such as `Long`. xref:repositories/core-concepts.adoc#is-new-state-detection[Entity State Detection] explains in detail the strategies to detect whether an entity is new or whether it is expected to exist in your database.