From 61c6438bc31fa4817bef6c8a3647dc559b21b2ed Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Mon, 29 Nov 2021 12:01:51 +0100 Subject: [PATCH] Polishing. Simplified the code structure. Ensured backward compatibility by recreating some methods often immediately deprecating them. Moved new classes to the places where they belong, so that the package ...core.sql.render depends on ...core.dialect and not the other way round. This causes dependency cycles because dependencies in the other direction already exists. This will be properly fixed by #1105. For now the offending classes are ignored by the DependencyTests. See #777 See #1105 Polishing --- .../data/jdbc/core/convert/SqlGenerator.java | 6 +-- ...JdbcAggregateTemplateIntegrationTests.java | 4 +- .../DefaultDataAccessStrategyUnitTests.java | 2 +- .../core/convert/SqlGeneratorUnitTests.java | 11 ++--- .../data/relational/core/dialect/Dialect.java | 9 ++-- .../core/dialect/InsertRenderContext.java | 16 +++++++ .../core/dialect/InsertRenderContexts.java | 25 +++++++++++ .../core/dialect/InsertWithDefaultValues.java | 16 +++---- .../core/dialect/RenderContextFactory.java | 25 ++++++----- .../core/dialect/SqlServerDialect.java | 10 +---- .../core/mapping/InsertDefaultValues.java | 28 ------------ .../core/sql/render/InsertRenderContext.java | 18 -------- .../sql/render/InsertStatementVisitor.java | 43 ++++++++++--------- .../core/sql/render/RenderContext.java | 12 +++++- .../core/sql/render/SimpleRenderContext.java | 9 +++- .../core/sql/render/SqlRenderer.java | 10 +++++ 16 files changed, 132 insertions(+), 112 deletions(-) create mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/InsertRenderContext.java create mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/InsertRenderContexts.java delete mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/InsertDefaultValues.java delete mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/InsertRenderContext.java diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java index 5ecca365..80fa379a 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java @@ -93,14 +93,12 @@ class SqlGenerator { SqlGenerator(RelationalMappingContext mappingContext, JdbcConverter converter, RelationalPersistentEntity entity, Dialect dialect) { - final RenderContextFactory renderContextFactory = new RenderContextFactory(dialect); - this.mappingContext = mappingContext; this.entity = entity; this.sqlContext = new SqlContext(entity); - this.sqlRenderer = SqlRenderer.create(renderContextFactory.createRenderContext()); + this.renderContext = new RenderContextFactory(dialect).createRenderContext(); + this.sqlRenderer = SqlRenderer.create(renderContext); this.columns = new Columns(entity, mappingContext, converter); - this.renderContext = renderContextFactory.createRenderContext(); } /** diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java index c7c339c7..7c3f0c4f 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java @@ -888,9 +888,11 @@ public class JdbcAggregateTemplateIntegrationTests { assertThat(loaded.testTime).isEqualTo(entity.testTime); } - @Test // DATAJDBC-557 + @Test // GH-777 public void insertWithIdOnly() { + WithIdOnly entity = new WithIdOnly(); + assertThat(template.save(entity).id).isNotNull(); } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java index 48464a0f..5098823c 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java @@ -219,7 +219,7 @@ public class DefaultDataAccessStrategyUnitTests { assertThat(generatedId).isEqualTo(GENERATED_ID); - verify(namedJdbcOperations).update(eq("INSERT INTO \"DUMMY_ENTITY\" VALUES ()"), + verify(namedJdbcOperations).update(eq("INSERT INTO \"DUMMY_ENTITY\" VALUES (DEFAULT)"), paramSourceCaptor.capture(), any(KeyHolder.class)); } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java index 5b0fa054..363d9c9d 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java @@ -398,18 +398,19 @@ class SqlGeneratorUnitTests { SqlGenerator sqlGenerator = createSqlGenerator(IdOnlyEntity.class, PostgresDialect.INSTANCE); - String insertSqlStatement = sqlGenerator.getInsert(emptySet()); + String insert = sqlGenerator.getInsert(emptySet()); - assertThat(insertSqlStatement).endsWith(" VALUES (DEFAULT) "); + assertThat(insert).endsWith(" VALUES (DEFAULT)"); } - @Test //DATAJDBC-557 + @Test // GH-777 void gerInsertForEmptyColumnListMsSqlServer() { + SqlGenerator sqlGenerator = createSqlGenerator(IdOnlyEntity.class, SqlServerDialect.INSTANCE); - String insertSqlStatement = sqlGenerator.getInsert(emptySet()); + String insert = sqlGenerator.getInsert(emptySet()); - assertThat(insertSqlStatement).endsWith(" DEFAULT VALUES "); + assertThat(insert).endsWith(" DEFAULT VALUES"); } @Test // DATAJDBC-334 diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java index c66fe997..5febb8c5 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java @@ -112,11 +112,12 @@ public interface Dialect { } /** - * @return an appropriate {@link InsertWithDefaultValues } for that specific dialect. + * @return an appropriate {@link InsertRenderContext} for that specific dialect. * for most of the Dialects the default implementation will be valid, but, for - * example, in case of {@link SqlServerDialect} it is not + * example, in case of {@link SqlServerDialect} it is not. + * @since 2.4 */ - default InsertWithDefaultValues getSqlInsertWithDefaultValues() { - return new InsertWithDefaultValues() {}; + default InsertRenderContext getInsertRenderContext() { + return InsertRenderContexts.DEFAULT; } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/InsertRenderContext.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/InsertRenderContext.java new file mode 100644 index 00000000..e02c3934 --- /dev/null +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/InsertRenderContext.java @@ -0,0 +1,16 @@ +package org.springframework.data.relational.core.dialect; + +import org.springframework.data.relational.core.sql.Insert; +import org.springframework.data.relational.core.sql.render.RenderContext; + +/** + * This interface encapsulates the details about how to process {@link Insert} SQL statement + * + * @see RenderContext + * @author Mikhail Polivakha + * @since 2.4 + */ +public interface InsertRenderContext { + + String getDefaultValuesInsertPart(); +} diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/InsertRenderContexts.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/InsertRenderContexts.java new file mode 100644 index 00000000..352a7502 --- /dev/null +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/InsertRenderContexts.java @@ -0,0 +1,25 @@ +package org.springframework.data.relational.core.dialect; + +/** + * In the scope of Insert with default values SQL statement, for example {@literal INSERT INTO SCHEMA.TABLE VALUES + * (DEFAULT)} this enum represents the default values part in different {@link Dialect}s + * + * @author Mikhail Polivakha + * @since 2.4 + */ +public enum InsertRenderContexts implements InsertRenderContext { + + DEFAULT(" VALUES (DEFAULT)"), // + MS_SQL_SERVER(" DEFAULT VALUES"); + + private final String defaultInsertPart; + + InsertRenderContexts(String defaultInsertPart) { + this.defaultInsertPart = defaultInsertPart; + } + + public String getDefaultValuesInsertPart() { + return defaultInsertPart; + } + +} diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/InsertWithDefaultValues.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/InsertWithDefaultValues.java index 4710ab79..20045086 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/InsertWithDefaultValues.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/InsertWithDefaultValues.java @@ -1,17 +1,15 @@ package org.springframework.data.relational.core.dialect; -import org.springframework.data.relational.core.mapping.InsertDefaultValues; - /** * This interface aggregates information about an Insert with default values statement. + * * @author Mikhail Polivakha + * @since 2.4 */ public interface InsertWithDefaultValues { - /** - * @return the part of the sql statement, that follows after INSERT INTO table - */ - default String getDefaultInsertPart() { - return InsertDefaultValues.DEFAULT.getDefaultInsertPart(); - } -} \ No newline at end of file + /** + * @return the part of the sql statement, that follows after INSERT INTO table + */ + String getDefaultInsertPart(); +} diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/RenderContextFactory.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/RenderContextFactory.java index b35014a5..c9dcc065 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/RenderContextFactory.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/RenderContextFactory.java @@ -16,7 +16,6 @@ package org.springframework.data.relational.core.dialect; import org.springframework.data.relational.core.sql.IdentifierProcessing; -import org.springframework.data.relational.core.sql.render.InsertRenderContext; import org.springframework.data.relational.core.sql.render.NamingStrategies; import org.springframework.data.relational.core.sql.render.RenderContext; import org.springframework.data.relational.core.sql.render.RenderNamingStrategy; @@ -78,19 +77,23 @@ public class RenderContextFactory { static class DialectRenderContext implements RenderContext { private final RenderNamingStrategy renderNamingStrategy; - private final SelectRenderContext selectRenderContext; private final Dialect renderingDialect; + private final SelectRenderContext selectRenderContext; + private final InsertRenderContext insertRenderContext; - DialectRenderContext(RenderNamingStrategy renderNamingStrategy, Dialect renderingDialect, SelectRenderContext selectRenderContext) { + DialectRenderContext(RenderNamingStrategy renderNamingStrategy, Dialect renderingDialect, + SelectRenderContext selectRenderContext) { Assert.notNull(renderNamingStrategy, "RenderNamingStrategy must not be null"); Assert.notNull(renderingDialect, "renderingDialect must not be null"); - Assert.notNull(renderingDialect.getIdentifierProcessing(), "IdentifierProcessing of renderingDialect must not be null"); + Assert.notNull(renderingDialect.getIdentifierProcessing(), + "IdentifierProcessing of renderingDialect must not be null"); Assert.notNull(selectRenderContext, "SelectRenderContext must not be null"); this.renderNamingStrategy = renderNamingStrategy; this.renderingDialect = renderingDialect; this.selectRenderContext = selectRenderContext; + this.insertRenderContext = renderingDialect.getInsertRenderContext(); } /* @@ -111,6 +114,11 @@ public class RenderContextFactory { return renderingDialect.getIdentifierProcessing(); } + @Override + public SelectRenderContext getSelect() { + return getSelectRenderContext(); + } + /* * (non-Javadoc) * @see org.springframework.data.relational.core.sql.render.RenderContext#getSelect() @@ -122,12 +130,7 @@ public class RenderContextFactory { @Override public InsertRenderContext getInsertRenderContext() { - return new InsertRenderContext() { - @Override - public String getInsertDefaultValuesPartSQL() { - return renderingDialect.getSqlInsertWithDefaultValues().getDefaultInsertPart(); - } - }; + return insertRenderContext; } } -} \ No newline at end of file +} 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 cf3bcbbc..11ae0d3c 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 @@ -15,7 +15,6 @@ */ package org.springframework.data.relational.core.dialect; -import org.springframework.data.relational.core.mapping.InsertDefaultValues; import org.springframework.data.relational.core.sql.IdentifierProcessing; import org.springframework.data.relational.core.sql.LockOptions; import org.springframework.data.relational.core.sql.render.SelectRenderContext; @@ -154,12 +153,7 @@ public class SqlServerDialect extends AbstractDialect { } @Override - public InsertWithDefaultValues getSqlInsertWithDefaultValues() { - return new InsertWithDefaultValues() { - @Override - public String getDefaultInsertPart() { - return InsertDefaultValues.MS_SQL_SERVER.getDefaultInsertPart(); - } - }; + public InsertRenderContext getInsertRenderContext() { + return InsertRenderContexts.MS_SQL_SERVER; } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/InsertDefaultValues.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/InsertDefaultValues.java deleted file mode 100644 index 8a9299ed..00000000 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/InsertDefaultValues.java +++ /dev/null @@ -1,28 +0,0 @@ -package org.springframework.data.relational.core.mapping; - -import org.springframework.data.relational.core.dialect.Dialect; -import org.springframework.data.relational.core.dialect.InsertWithDefaultValues; - -/** - * In the scope of Insert with default values SQL statement, for example - * INSERT INTO SCHEMA.TABLE VALUES (DEFAULT) - * this enum represents the default values part in different {@link Dialect}s - * - * @author Mikhail Polivakha - * @see InsertWithDefaultValues - */ -public enum InsertDefaultValues { - - DEFAULT(" VALUES (DEFAULT) "), - MS_SQL_SERVER(" DEFAULT VALUES "); - - private final String defaultInsertPart; - - InsertDefaultValues(String defaultInsertPart) { - this.defaultInsertPart = defaultInsertPart; - } - - public String getDefaultInsertPart() { - return defaultInsertPart; - } -} \ No newline at end of file diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/InsertRenderContext.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/InsertRenderContext.java deleted file mode 100644 index 6824d8c7..00000000 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/InsertRenderContext.java +++ /dev/null @@ -1,18 +0,0 @@ -package org.springframework.data.relational.core.sql.render; - -import org.springframework.data.relational.core.mapping.InsertDefaultValues; -import org.springframework.data.relational.core.sql.Insert; - -/** - * This interface encapsulates the details about how to - * process {@link Insert} SQL statement - * - * @see RenderContext - * @author Mikhail Polivakha - */ -public interface InsertRenderContext { - - default String getInsertDefaultValuesPartSQL() { - return InsertDefaultValues.DEFAULT.getDefaultInsertPart(); - } -} \ No newline at end of file diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/InsertStatementVisitor.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/InsertStatementVisitor.java index 75cf4407..472914c5 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/InsertStatementVisitor.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/InsertStatementVisitor.java @@ -15,7 +15,6 @@ */ package org.springframework.data.relational.core.sql.render; -import org.jetbrains.annotations.NotNull; import org.springframework.data.relational.core.sql.Column; import org.springframework.data.relational.core.sql.Insert; import org.springframework.data.relational.core.sql.Into; @@ -98,24 +97,6 @@ class InsertStatementVisitor extends DelegatingVisitor implements PartRenderer { return Delegation.retain(); } - private void addInsertValuesIfPresentElseDefault() { - if (values.length() != 0) { - builder.append(" VALUES (").append(values).append(")"); - } else { - addInsertWithDefaultValuesToBuilder(); - } - } - - private void addInsertColumnsIfPresent() { - if (columns.length() != 0) { - builder.append(" (").append(columns).append(")"); - } - } - - private void addInsertWithDefaultValuesToBuilder() { - builder.append(renderContext.getInsertRenderContext().getInsertDefaultValuesPartSQL()); - } - /* * (non-Javadoc) * @see org.springframework.data.relational.core.sql.render.PartRenderer#getRenderedPart() @@ -125,8 +106,28 @@ class InsertStatementVisitor extends DelegatingVisitor implements PartRenderer { return builder; } - @NotNull + private void addInsertValuesIfPresentElseDefault() { + + if (values.length() != 0) { + builder.append(" VALUES (").append(values).append(")"); + } else { + addInsertWithDefaultValuesToBuilder(); + } + } + + private void addInsertColumnsIfPresent() { + + if (columns.length() != 0) { + builder.append(" (").append(columns).append(")"); + } + } + + private void addInsertWithDefaultValuesToBuilder() { + builder.append(renderContext.getInsertRenderContext().getDefaultValuesInsertPart()); + } + private ColumnVisitor createColumnVisitor(RenderContext context) { + return new ColumnVisitor(context, false, it -> { if (columns.length() != 0) { @@ -137,8 +138,8 @@ class InsertStatementVisitor extends DelegatingVisitor implements PartRenderer { }); } - @NotNull private IntoClauseVisitor createIntoClauseVisitor(RenderContext context) { + return new IntoClauseVisitor(context, it -> { if (into.length() != 0) { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/RenderContext.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/RenderContext.java index 2d40ef51..38a1c210 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/RenderContext.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/RenderContext.java @@ -15,6 +15,7 @@ */ package org.springframework.data.relational.core.sql.render; +import org.springframework.data.relational.core.dialect.InsertRenderContext; import org.springframework.data.relational.core.sql.IdentifierProcessing; /** @@ -43,8 +44,17 @@ public interface RenderContext { /** * @return the {@link SelectRenderContext}. + * @deprecated Use {@link #getInsertRenderContext()} instead. */ - SelectRenderContext getSelectRenderContext(); + @Deprecated + SelectRenderContext getSelect(); + + /** + * @return the {@link SelectRenderContext}. + */ + default SelectRenderContext getSelectRenderContext() { + return getSelect(); + } /** * @return the {@link InsertRenderContext} diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SimpleRenderContext.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SimpleRenderContext.java index 3886d5c4..552067a4 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SimpleRenderContext.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SimpleRenderContext.java @@ -15,6 +15,8 @@ */ package org.springframework.data.relational.core.sql.render; +import org.springframework.data.relational.core.dialect.InsertRenderContext; +import org.springframework.data.relational.core.dialect.InsertRenderContexts; import org.springframework.data.relational.core.sql.IdentifierProcessing; /** @@ -36,6 +38,11 @@ final class SimpleRenderContext implements RenderContext { return IdentifierProcessing.NONE; } + @Override + public SelectRenderContext getSelect() { + return getSelectRenderContext(); + } + @Override public SelectRenderContext getSelectRenderContext() { return DefaultSelectRenderContext.INSTANCE; @@ -43,7 +50,7 @@ final class SimpleRenderContext implements RenderContext { @Override public InsertRenderContext getInsertRenderContext() { - return new InsertRenderContext() {}; + return InsertRenderContexts.DEFAULT; } public RenderNamingStrategy getNamingStrategy() { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SqlRenderer.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SqlRenderer.java index 9c93fe68..93999346 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SqlRenderer.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SqlRenderer.java @@ -69,6 +69,16 @@ public class SqlRenderer implements Renderer { return create().render(select); } + /** + * Renders a {@link Insert} statement into its SQL representation. + * + * @param insert must not be {@literal null}. + * @return the rendered statement. + */ + public static String toString(Insert insert) { + return create().render(insert); + } + /** * Renders a {@link Update} statement into its SQL representation. *