From 91ddda0c3f97858e75cbf44ee23054559dd0daeb Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 11 Feb 2020 11:02:55 +0100 Subject: [PATCH] DATAJDBC-479 - Polishing. Fixes compiler error from rebase rebase. Removes IdentifierProcessingAdapter since it wasn't used anymore Merged UnquotedDialect with NonQuotingDialect since both served the same purpose of a simple Dialect for testing. Formatting. Original pull request: #187. --- .../data/jdbc/core/convert/SqlGenerator.java | 13 +++--- .../jdbc/core/convert/SqlGeneratorSource.java | 4 +- .../convert/IdentifierProcessingAdapter.java | 46 ------------------- ...tedDialect.java => NonQuotingDialect.java} | 14 +++--- ...orContextBasedNamingStrategyUnitTests.java | 1 - .../SqlGeneratorEmbeddedUnitTests.java | 40 ++++++++++++---- .../core/convert/SqlGeneratorUnitTests.java | 12 ++--- .../data/jdbc/testing/NonQuotingDialect.java | 35 -------------- .../data/relational/core/sql/Table.java | 2 - .../core/sql/render/ColumnVisitor.java | 11 ++--- .../core/sql/render/JoinVisitor.java | 2 + 11 files changed, 57 insertions(+), 123 deletions(-) delete mode 100644 spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/IdentifierProcessingAdapter.java rename spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/{UnquotedDialect.java => NonQuotingDialect.java} (72%) delete mode 100644 spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/NonQuotingDialect.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 31700022..b5d55d9a 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 @@ -62,7 +62,6 @@ class SqlGenerator { static final SqlIdentifier ROOT_ID_PARAMETER = SqlIdentifier.unquoted("rootId"); private static final Pattern parameterPattern = Pattern.compile("\\W"); - private final JdbcConverter converter; private final RelationalPersistentEntity entity; private final MappingContext, RelationalPersistentProperty> mappingContext; private final RenderContext renderContext; @@ -93,10 +92,10 @@ class SqlGenerator { * @param entity must not be {@literal null}. * @param dialect must not be {@literal null}. */ - SqlGenerator(RelationalMappingContext mappingContext, JdbcConverter converter,RelationalPersistentEntity entity, Dialect dialect) { + SqlGenerator(RelationalMappingContext mappingContext, JdbcConverter converter, RelationalPersistentEntity entity, + Dialect dialect) { this.mappingContext = mappingContext; - this.converter = converter; this.entity = entity; this.sqlContext = new SqlContext(entity); this.sqlRenderer = SqlRenderer.create(new RenderContextFactory(dialect).createRenderContext()); @@ -406,7 +405,8 @@ class SqlGenerator { return (SelectBuilder.SelectWhere) baseSelect; } - private SelectBuilder.SelectOrdered selectBuilder(Collection keyColumns, Sort sort, Pageable pageable) { + private SelectBuilder.SelectOrdered selectBuilder(Collection keyColumns, Sort sort, + Pageable pageable) { SelectBuilder.SelectOrdered sortable = this.selectBuilder(keyColumns); sortable = applyPagination(pageable, sortable); @@ -426,8 +426,9 @@ class SqlGenerator { SelectBuilder.SelectLimitOffset limitable = (SelectBuilder.SelectLimitOffset) select; SelectBuilder.SelectLimitOffset limitResult = limitable.limitOffset(pageable.getPageSize(), pageable.getOffset()); - Assert.state(limitResult instanceof SelectBuilder.SelectOrdered, - String.format("The result of applying the limit-clause must be of type SelectOrdered in order to apply the order-by-clause but is of type %s.", select.getClass())); + Assert.state(limitResult instanceof SelectBuilder.SelectOrdered, String.format( + "The result of applying the limit-clause must be of type SelectOrdered in order to apply the order-by-clause but is of type %s.", + select.getClass())); return (SelectBuilder.SelectOrdered) limitResult; } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGeneratorSource.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGeneratorSource.java index f8352a85..933d0986 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGeneratorSource.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGeneratorSource.java @@ -48,8 +48,8 @@ public class SqlGeneratorSource { } SqlGenerator getSqlGenerator(Class domainType) { + return CACHE.computeIfAbsent(domainType, - t -> new SqlGenerator(context, converter, - context.getRequiredPersistentEntity(t), dialect)); + t -> new SqlGenerator(context, converter, context.getRequiredPersistentEntity(t), dialect)); } } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/IdentifierProcessingAdapter.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/IdentifierProcessingAdapter.java deleted file mode 100644 index a20be2c0..00000000 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/IdentifierProcessingAdapter.java +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Copyright 2020 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.data.jdbc.core.convert; - -import org.springframework.data.relational.core.dialect.AbstractDialect; -import org.springframework.data.relational.core.dialect.Dialect; -import org.springframework.data.relational.core.dialect.HsqlDbDialect; -import org.springframework.data.relational.core.dialect.LimitClause; -import org.springframework.data.relational.core.sql.IdentifierProcessing; - -/** - * {@link Dialect} adapter that delegates to the given {@link IdentifierProcessing}. - * - * @author Mark Paluch - */ -public class IdentifierProcessingAdapter extends AbstractDialect implements Dialect { - - private final IdentifierProcessing identifierProcessing; - - public IdentifierProcessingAdapter(IdentifierProcessing identifierProcessing) { - this.identifierProcessing = identifierProcessing; - } - - @Override - public LimitClause limit() { - return HsqlDbDialect.INSTANCE.limit(); - } - - @Override - public IdentifierProcessing getIdentifierProcessing() { - return identifierProcessing; - } -} diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/UnquotedDialect.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/NonQuotingDialect.java similarity index 72% rename from spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/UnquotedDialect.java rename to spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/NonQuotingDialect.java index 83551779..b1672e5f 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/UnquotedDialect.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/NonQuotingDialect.java @@ -1,5 +1,5 @@ /* - * Copyright 2020 the original author or authors. + * Copyright 2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,19 +20,19 @@ import org.springframework.data.relational.core.dialect.Dialect; import org.springframework.data.relational.core.dialect.HsqlDbDialect; import org.springframework.data.relational.core.dialect.LimitClause; import org.springframework.data.relational.core.sql.IdentifierProcessing; -import org.springframework.data.relational.core.sql.IdentifierProcessing.LetterCasing; -import org.springframework.data.relational.core.sql.IdentifierProcessing.Quoting; /** * Simple {@link Dialect} that provides unquoted {@link IdentifierProcessing}. * * @author Mark Paluch + * @author Milan Milanov + * @author Jens Schauder */ -public class UnquotedDialect extends AbstractDialect implements Dialect { +public class NonQuotingDialect extends AbstractDialect implements Dialect { - public static final UnquotedDialect INSTANCE = new UnquotedDialect(); + public static final NonQuotingDialect INSTANCE = new NonQuotingDialect(); - private UnquotedDialect() {} + private NonQuotingDialect() {} @Override public LimitClause limit() { @@ -41,6 +41,6 @@ public class UnquotedDialect extends AbstractDialect implements Dialect { @Override public IdentifierProcessing getIdentifierProcessing() { - return IdentifierProcessing.create(new Quoting(""), LetterCasing.AS_IS); + return IdentifierProcessing.create(new IdentifierProcessing.Quoting(""), IdentifierProcessing.LetterCasing.AS_IS); } } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorContextBasedNamingStrategyUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorContextBasedNamingStrategyUnitTests.java index 7bcc13d9..8834fe8b 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorContextBasedNamingStrategyUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorContextBasedNamingStrategyUnitTests.java @@ -28,7 +28,6 @@ import org.junit.Test; import org.springframework.data.annotation.Id; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.jdbc.core.mapping.PersistentPropertyPathTestUtils; -import org.springframework.data.jdbc.testing.NonQuotingDialect; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.mapping.NamingStrategy; import org.springframework.data.relational.core.mapping.RelationalMappingContext; diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorEmbeddedUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorEmbeddedUnitTests.java index 15cb6d77..fb62c756 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorEmbeddedUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorEmbeddedUnitTests.java @@ -22,11 +22,9 @@ import org.assertj.core.api.SoftAssertions; import org.junit.Before; import org.junit.Ignore; import org.junit.Test; - import org.springframework.data.annotation.Id; import org.springframework.data.jdbc.core.PropertyPathTestingUtils; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; -import org.springframework.data.jdbc.testing.NonQuotingDialect; import org.springframework.data.relational.core.mapping.Column; import org.springframework.data.relational.core.mapping.Embedded; import org.springframework.data.relational.core.mapping.Embedded.OnEmpty; @@ -35,6 +33,7 @@ import org.springframework.data.relational.core.mapping.RelationalMappingContext import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.sql.Aliased; import org.springframework.data.relational.core.sql.SqlIdentifier; +import org.springframework.lang.Nullable; /** * Unit tests for the {@link SqlGenerator} in a context of the {@link Embedded} annotation. @@ -203,8 +202,15 @@ public class SqlGeneratorEmbeddedUnitTests { public void columnForEmbeddedProperty() { assertThat(generatedColumn("embeddable.test", DummyEntity.class)) // - .extracting(c -> c.getName(), c -> c.getTable().getName(), c -> getAlias(c.getTable()), this::getAlias) - .containsExactly(SqlIdentifier.unquoted("test"), SqlIdentifier.unquoted("dummy_entity"), null, + .extracting( // + c -> c.getName(), // + c -> c.getTable().getName(), // + c -> getAlias(c.getTable()), // + this::getAlias) // + .containsExactly( // + SqlIdentifier.unquoted("test"), // + SqlIdentifier.unquoted("dummy_entity"), // + null, // SqlIdentifier.unquoted("test")); } @@ -227,8 +233,15 @@ public class SqlGeneratorEmbeddedUnitTests { public void columnForPrefixedEmbeddedProperty() { assertThat(generatedColumn("prefixedEmbeddable.test", DummyEntity.class)) // - .extracting(c -> c.getName(), c -> c.getTable().getName(), c -> getAlias(c.getTable()), this::getAlias) - .containsExactly(SqlIdentifier.unquoted("prefix_test"), SqlIdentifier.unquoted("dummy_entity"), null, + .extracting( // + c -> c.getName(), // + c -> c.getTable().getName(), // + c -> getAlias(c.getTable()), // + this::getAlias) // + .containsExactly( // + SqlIdentifier.unquoted("prefix_test"), // + SqlIdentifier.unquoted("dummy_entity"), // + null, // SqlIdentifier.unquoted("prefix_test")); } @@ -268,9 +281,16 @@ public class SqlGeneratorEmbeddedUnitTests { public void columnForEmbeddedWithReferenceProperty() { assertThat(generatedColumn("embedded.other.value", DummyEntity2.class)) // - .extracting(c -> c.getName(), c -> c.getTable().getName(), c -> getAlias(c.getTable()), this::getAlias) - .containsExactly(SqlIdentifier.unquoted("value"), SqlIdentifier.unquoted("other_entity"), - SqlIdentifier.quoted("prefix_other"), SqlIdentifier.unquoted("prefix_other_value")); + .extracting( // + c -> c.getName(), // + c -> c.getTable().getName(), // + c -> getAlias(c.getTable()), // + this::getAlias) // + .containsExactly( // + SqlIdentifier.unquoted("value"), // + SqlIdentifier.unquoted("other_entity"), // + SqlIdentifier.quoted("prefix_other"), // + SqlIdentifier.unquoted("prefix_other_value")); } private SqlGenerator.Join generateJoin(String path, Class type) { @@ -278,6 +298,7 @@ public class SqlGeneratorEmbeddedUnitTests { .getJoin(new PersistentPropertyPathExtension(context, PropertyPathTestingUtils.toPath(path, type, context))); } + @Nullable private SqlIdentifier getAlias(Object maybeAliased) { if (maybeAliased instanceof Aliased) { @@ -287,6 +308,7 @@ public class SqlGeneratorEmbeddedUnitTests { } private org.springframework.data.relational.core.sql.Column generatedColumn(String path, Class type) { + return createSqlGenerator(type) .getColumn(new PersistentPropertyPathExtension(context, PropertyPathTestingUtils.toPath(path, type, context))); } 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 8b81be2e..a266b025 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 @@ -37,7 +37,6 @@ import org.springframework.data.jdbc.core.mapping.AggregateReference; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.jdbc.core.mapping.PersistentPropertyPathTestUtils; import org.springframework.data.jdbc.testing.AnsiDialect; -import org.springframework.data.jdbc.testing.NonQuotingDialect; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.dialect.Dialect; import org.springframework.data.relational.core.mapping.Column; @@ -47,9 +46,6 @@ 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.sql.Aliased; -import org.springframework.data.relational.core.sql.IdentifierProcessing; -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.SqlIdentifier; import org.springframework.data.relational.core.sql.Table; import org.springframework.data.relational.domain.Identifier; @@ -243,8 +239,8 @@ public class SqlGeneratorUnitTests { "FROM dummy_entity ", // "LEFT OUTER JOIN referenced_entity AS ref ON ref.dummy_entity = dummy_entity.id1", // "LEFT OUTER JOIN second_level_referenced_entity AS ref_further ON ref_further.referenced_entity = ref.x_l1id", // - "OFFSET 40 ROWS", // - "FETCH FIRST 20 ROWS ONLY"); + "OFFSET 40", // + "LIMIT 20"); } @Test // DATAJDBC-101 @@ -264,8 +260,8 @@ public class SqlGeneratorUnitTests { "LEFT OUTER JOIN referenced_entity AS ref ON ref.dummy_entity = dummy_entity.id1", // "LEFT OUTER JOIN second_level_referenced_entity AS ref_further ON ref_further.referenced_entity = ref.x_l1id", // "ORDER BY x_name ASC", // - "OFFSET 30 ROWS", // - "FETCH FIRST 10 ROWS ONLY"); + "OFFSET 30", // + "LIMIT 10"); } @Test // DATAJDBC-131, DATAJDBC-111 diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/NonQuotingDialect.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/NonQuotingDialect.java deleted file mode 100644 index b706239b..00000000 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/NonQuotingDialect.java +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright 2019 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.data.jdbc.testing; - - -import org.springframework.data.relational.core.sql.IdentifierProcessing; - -/** - * The ANSI standard dialect, but without quoting the identifiers. - * - * @author Milan Milanov - * @since 2.0 - */ -public class NonQuotingDialect extends AnsiDialect { - - public static final NonQuotingDialect INSTANCE = new NonQuotingDialect(); - - @Override - public IdentifierProcessing getIdentifierProcessing() { - return IdentifierProcessing.NONE; - } -} diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Table.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Table.java index 98dad433..38055dad 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Table.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Table.java @@ -36,12 +36,10 @@ public class Table extends AbstractSegment { private final SqlIdentifier name; Table(String name) { - super(); this.name = SqlIdentifier.unquoted(name); } Table(SqlIdentifier name) { - super(); this.name = name; } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/ColumnVisitor.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/ColumnVisitor.java index 6ac108cd..bf22cb31 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/ColumnVisitor.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/ColumnVisitor.java @@ -49,15 +49,12 @@ class ColumnVisitor extends TypedSubtreeVisitor { Delegation leaveMatched(Column segment) { SqlIdentifier column = context.getNamingStrategy().getName(segment); - StringBuilder builder = new StringBuilder(); - if (considerTablePrefix && tableName != null) { - builder.append(NameRenderer.render(context, SqlIdentifier.from(tableName, column))); - } else { - builder.append(NameRenderer.render(context, segment)); - } + CharSequence name = considerTablePrefix && tableName != null + ? NameRenderer.render(context, SqlIdentifier.from(tableName, column)) + : NameRenderer.render(context, segment); - target.onRendered(builder); + target.onRendered(name); return super.leaveMatched(segment); } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/JoinVisitor.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/JoinVisitor.java index e5934b58..9f3b4acc 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/JoinVisitor.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/JoinVisitor.java @@ -38,6 +38,7 @@ class JoinVisitor extends TypedSubtreeVisitor { private boolean hasSeenCondition = false; JoinVisitor(RenderContext context, RenderTarget parent) { + this.context = context; this.parent = parent; this.conditionVisitor = new ConditionVisitor(context); @@ -87,6 +88,7 @@ class JoinVisitor extends TypedSubtreeVisitor { Delegation leaveNested(Visitable segment) { if (segment instanceof Condition) { + inCondition = false; if (hasSeenCondition) {