From 141f2d77acfcb5a3d0a3c824ba71c35f51e81ad7 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 8 Apr 2019 12:36:53 +0200 Subject: [PATCH] DATAJDBC-340 - Polishing. Encapsulate column caches in Columns type. Relax RelationalMappingContext to MappingContext with appropriate generics. Remove unused SQL generator source. Cache table and Id column objects. Simplify assertions. Consistently use naming pattern for named parameters. Migrate http URLs to https. Original pull request: #147. --- mvnw | 2 +- mvnw.cmd | 4 +- .../core/PersistentPropertyPathExtension.java | 40 +- .../data/jdbc/core/SqlContext.java | 15 +- .../data/jdbc/core/SqlGenerator.java | 469 +++++++++++------- .../data/jdbc/core/SqlGeneratorSource.java | 4 +- ...sistentPropertyPathExtensionUnitTests.java | 2 +- ...orContextBasedNamingStrategyUnitTests.java | 2 +- .../core/SqlGeneratorEmbeddedUnitTests.java | 2 +- ...GeneratorFixedNamingStrategyUnitTests.java | 2 +- .../data/jdbc/core/SqlGeneratorUnitTests.java | 20 +- 11 files changed, 347 insertions(+), 215 deletions(-) diff --git a/mvnw b/mvnw index 5551fde8..8b9da3b8 100755 --- a/mvnw +++ b/mvnw @@ -8,7 +8,7 @@ # "License"); you may not use this file except in compliance # with the License. You may obtain a copy of the License at # -# http://www.apache.org/licenses/LICENSE-2.0 +# 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 diff --git a/mvnw.cmd b/mvnw.cmd index e5cfb0ae..b3f99581 100755 --- a/mvnw.cmd +++ b/mvnw.cmd @@ -7,7 +7,7 @@ @REM "License"); you may not use this file except in compliance @REM with the License. You may obtain a copy of the License at @REM -@REM http://www.apache.org/licenses/LICENSE-2.0 +@REM https://www.apache.org/licenses/LICENSE-2.0 @REM @REM Unless required by applicable law or agreed to in writing, @REM software distributed under the License is distributed on an @@ -122,7 +122,7 @@ set WRAPPER_LAUNCHER=org.apache.maven.wrapper.MavenWrapperMain set DOWNLOAD_URL="https://repo.maven.apache.org/maven2/io/takari/maven-wrapper/0.4.2/maven-wrapper-0.4.2.jar" FOR /F "tokens=1,2 delims==" %%A IN (%MAVEN_PROJECTBASEDIR%\.mvn\wrapper\maven-wrapper.properties) DO ( - IF "%%A"=="wrapperUrl" SET DOWNLOAD_URL=%%B + IF "%%A"=="wrapperUrl" SET DOWNLOAD_URL=%%B ) @REM Extension to allow automatically downloading the maven-wrapper.jar from Maven-central diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/PersistentPropertyPathExtension.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/PersistentPropertyPathExtension.java index 32154adf..a9a5e757 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/PersistentPropertyPathExtension.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/PersistentPropertyPathExtension.java @@ -5,7 +5,7 @@ * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * 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, @@ -15,10 +15,8 @@ */ package org.springframework.data.jdbc.core; -import java.util.Objects; - import org.springframework.data.mapping.PersistentPropertyPath; -import org.springframework.data.relational.core.mapping.RelationalMappingContext; +import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; import org.springframework.lang.Nullable; @@ -34,10 +32,11 @@ import org.springframework.util.Assert; class PersistentPropertyPathExtension { private final RelationalPersistentEntity entity; - private final PersistentPropertyPath path; - private final RelationalMappingContext context; + private final @Nullable PersistentPropertyPath path; + private final MappingContext, RelationalPersistentProperty> context; - PersistentPropertyPathExtension(RelationalMappingContext context, RelationalPersistentEntity entity) { + PersistentPropertyPathExtension(MappingContext, RelationalPersistentProperty> context, + RelationalPersistentEntity entity) { Assert.notNull(context, "Context must not be null."); Assert.notNull(entity, "Entity must not be null."); @@ -47,15 +46,15 @@ class PersistentPropertyPathExtension { this.path = null; } - PersistentPropertyPathExtension(RelationalMappingContext context, + PersistentPropertyPathExtension(MappingContext, RelationalPersistentProperty> context, PersistentPropertyPath path) { Assert.notNull(context, "Context must not be null."); Assert.notNull(path, "Path must not be null."); - Assert.isTrue(!path.isEmpty(), "Path must not be empty."); + Assert.notNull(path.getBaseProperty(), "Path must not be empty."); this.context = context; - this.entity = Objects.requireNonNull(path.getBaseProperty()).getOwner(); + this.entity = path.getBaseProperty().getOwner(); this.path = path; } @@ -140,6 +139,8 @@ class PersistentPropertyPathExtension { */ String getReverseColumnName() { + Assert.state(path != null, "Path is null"); + return path.getRequiredLeafProperty().getReverseColumnName(); } @@ -160,6 +161,8 @@ class PersistentPropertyPathExtension { */ String getColumnName() { + Assert.state(path != null, "Path is null"); + return assembleColumnName(path.getRequiredLeafProperty().getColumnName()); } @@ -212,11 +215,9 @@ class PersistentPropertyPathExtension { String getTableAlias() { PersistentPropertyPathExtension tableOwner = getTableOwningAncestor(); - if (tableOwner.path == null) { - return null; - } - return tableOwner.assembleTableAlias(); + return tableOwner.path == null ? null : tableOwner.assembleTableAlias(); + } /** @@ -251,14 +252,13 @@ class PersistentPropertyPathExtension { */ private PersistentPropertyPathExtension getTableOwningAncestor() { - if (isEntity() && !isEmbedded()) { - return this; - } - return getParentPath().getTableOwningAncestor(); + return isEntity() && !isEmbedded() ? this : getParentPath().getTableOwningAncestor(); } private String assembleTableAlias() { + Assert.state(path != null, "Path is null"); + RelationalPersistentProperty leafProperty = path.getRequiredLeafProperty(); String prefix = isEmbedded() ? leafProperty.getEmbeddedPrefix() : leafProperty.getName(); @@ -274,6 +274,8 @@ class PersistentPropertyPathExtension { private String assembleColumnName(String suffix) { + Assert.state(path != null, "Path is null"); + if (path.getLength() <= 1) { return suffix; } @@ -286,11 +288,11 @@ class PersistentPropertyPathExtension { return getParentPath().assembleColumnName(embeddedPrefix + suffix); } + @SuppressWarnings("unchecked") private RelationalPersistentEntity getRequiredLeafEntity() { return path == null ? entity : context.getRequiredPersistentEntity(path.getRequiredLeafProperty().getActualType()); } - private String prefixWithTableAlias(String columnName) { String tableAlias = getTableAlias(); diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/SqlContext.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/SqlContext.java index 33346a99..aabb3707 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/SqlContext.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/SqlContext.java @@ -5,7 +5,7 @@ * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * 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, @@ -24,18 +24,25 @@ import org.springframework.data.relational.core.sql.Table; * Utility to get from path to SQL DSL elements. * * @author Jens Schauder + * @author Mark Paluch * @since 1.1 */ class SqlContext { private final RelationalPersistentEntity entity; + private final Table table; SqlContext(RelationalPersistentEntity entity) { this.entity = entity; + this.table = SQL.table(entity.getTableName()); } Column getIdColumn() { - return getTable().column(entity.getIdColumn()); + return table.column(entity.getIdColumn()); + } + + Table getTable() { + return table; } Table getTable(PersistentPropertyPathExtension path) { @@ -45,10 +52,6 @@ class SqlContext { return tableAlias == null ? table : table.as(tableAlias); } - Table getTable() { - return SQL.table(entity.getTableName()); - } - Column getColumn(PersistentPropertyPathExtension path) { return getTable(path).column(path.getColumnName()).as(path.getColumnAlias()); } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java index 958a7f16..16e1e92d 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/SqlGenerator.java @@ -18,6 +18,8 @@ package org.springframework.data.jdbc.core; import lombok.Value; import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; @@ -26,19 +28,35 @@ import java.util.Set; import java.util.function.Function; import java.util.regex.Pattern; import java.util.stream.Collectors; -import java.util.stream.Stream; import org.springframework.data.annotation.ReadOnlyProperty; import org.springframework.data.jdbc.repository.support.SimpleJdbcRepository; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.mapping.PropertyHandler; +import org.springframework.data.mapping.context.MappingContext; 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.*; +import org.springframework.data.relational.core.sql.AssignValue; +import org.springframework.data.relational.core.sql.Assignments; +import org.springframework.data.relational.core.sql.BindMarker; +import org.springframework.data.relational.core.sql.Column; +import org.springframework.data.relational.core.sql.Condition; +import org.springframework.data.relational.core.sql.Delete; +import org.springframework.data.relational.core.sql.DeleteBuilder; +import org.springframework.data.relational.core.sql.Expression; +import org.springframework.data.relational.core.sql.Expressions; +import org.springframework.data.relational.core.sql.Functions; +import org.springframework.data.relational.core.sql.Insert; +import org.springframework.data.relational.core.sql.InsertBuilder; +import org.springframework.data.relational.core.sql.SQL; +import org.springframework.data.relational.core.sql.Select; +import org.springframework.data.relational.core.sql.SelectBuilder; +import org.springframework.data.relational.core.sql.StatementBuilder; +import org.springframework.data.relational.core.sql.Table; +import org.springframework.data.relational.core.sql.Update; import org.springframework.data.relational.core.sql.render.SqlRenderer; import org.springframework.data.util.Lazy; -import org.springframework.data.util.StreamUtils; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -49,14 +67,17 @@ import org.springframework.util.Assert; * @author Yoichi Imai * @author Bastian Wilhelm * @author Oleksandr Kucher + * @author Mark Paluch */ class SqlGenerator { + private static final Pattern parameterPattern = Pattern.compile("\\W"); + private final RelationalPersistentEntity entity; - private final RelationalMappingContext mappingContext; - private final List columnNames = new ArrayList<>(); - private final List nonIdColumnNames = new ArrayList<>(); - private final Set readOnlyColumnNames = new HashSet<>(); + private final MappingContext, RelationalPersistentProperty> mappingContext; + + private final SqlContext sqlContext; + private final Columns columns; private final Lazy findOneSql = Lazy.of(this::createFindOneSql); private final Lazy findAllSql = Lazy.of(this::createFindAllSql); @@ -69,56 +90,19 @@ class SqlGenerator { private final Lazy deleteByIdSql = Lazy.of(this::createDeleteSql); private final Lazy deleteByListSql = Lazy.of(this::createDeleteByListSql); - private final SqlGeneratorSource sqlGeneratorSource; - private final Pattern parameterPattern = Pattern.compile("\\W"); - private final SqlContext sqlContext; - - SqlGenerator(RelationalMappingContext mappingContext, RelationalPersistentEntity entity, - SqlGeneratorSource sqlGeneratorSource) { + /** + * Create a new {@link SqlGenerator} given {@link RelationalMappingContext} and {@link RelationalPersistentEntity}. + * + * @param mappingContext must not be {@literal null}. + * @param entity must not be {@literal null}. + */ + SqlGenerator(RelationalMappingContext mappingContext, RelationalPersistentEntity entity) { this.mappingContext = mappingContext; this.entity = entity; - this.sqlGeneratorSource = sqlGeneratorSource; this.sqlContext = new SqlContext(entity); - initColumnNames(entity, ""); - } - - private void initColumnNames(RelationalPersistentEntity entity, String prefix) { - - entity.doWithProperties((PropertyHandler) property -> { - - // the referencing column of referenced entity is expected to be on the other side of the relation - if (!property.isEntity()) { - initSimpleColumnName(property, prefix); - } else if (property.isEmbedded()) { - initEmbeddedColumnNames(property, prefix); - } - }); - } - - private void initSimpleColumnName(RelationalPersistentProperty property, String prefix) { - - String columnName = prefix + property.getColumnName(); - - columnNames.add(columnName); - - if (!entity.isIdProperty(property)) { - nonIdColumnNames.add(columnName); - } - if (property.isAnnotationPresent(ReadOnlyProperty.class)) { - readOnlyColumnNames.add(columnName); - } - } - - private void initEmbeddedColumnNames(RelationalPersistentProperty property, String prefix) { - - final String embeddedPrefix = property.getEmbeddedPrefix(); - - final RelationalPersistentEntity embeddedEntity = mappingContext - .getRequiredPersistentEntity(property.getColumnType()); - - initColumnNames(embeddedEntity, prefix + embeddedPrefix); + this.columns = new Columns(entity, mappingContext); } /** @@ -157,85 +141,135 @@ class SqlGenerator { Assert.isTrue(keyColumn != null || !ordered, "If the SQL statement should be ordered a keyColumn to order by must be provided."); - SelectBuilder.SelectWhere baseSelect = createBaseSelect(keyColumn); + SelectBuilder.SelectWhere builder = selectBuilder( + keyColumn == null ? Collections.emptyList() : Collections.singleton(keyColumn)); - Table table = Table.create(entity.getTableName()); - SelectBuilder.SelectWhereAndOr withWhereClause = baseSelect - .where(table.column(columnName).isEqualTo(SQL.bindMarker(":" + columnName))); + Table table = getTable(); + SelectBuilder.SelectWhereAndOr withWhereClause = builder + .where(table.column(columnName).isEqualTo(getBindMarker(columnName))); - SelectBuilder.BuildSelect select; + Select select; if (ordered) { - select = withWhereClause.orderBy(table.column(keyColumn).as(keyColumn)); + select = withWhereClause.orderBy(table.column(keyColumn).as(keyColumn)).build(); } else { - select = withWhereClause; + select = withWhereClause.build(); } return render(select); } + /** + * Create a {@code SELECT COUNT(id) FROM … WHERE :id = …} statement. + * + * @return + */ String getExists() { return existsSql.get(); } + /** + * Create a {@code SELECT … FROM … WHERE :id = …} statement. + * + * @return + */ String getFindOne() { return findOneSql.get(); } + /** + * Create a {@code INSERT INTO … (…) VALUES(…)} statement. + * + * @return + */ String getInsert(Set additionalColumns) { return createInsertSql(additionalColumns); } + /** + * Create a {@code UPDATE … SET …} statement. + * + * @return + */ String getUpdate() { return updateSql.get(); } + /** + * Create a {@code SELECT COUNT(*) FROM …} statement. + * + * @return + */ String getCount() { return countSql.get(); } + /** + * Create a {@code DELETE FROM … WHERE :id = …} statement. + * + * @return + */ String getDeleteById() { return deleteByIdSql.get(); } + /** + * Create a {@code DELETE FROM … WHERE :ids in (…)} statement. + * + * @return + */ String getDeleteByList() { return deleteByListSql.get(); } + /** + * Create a {@code DELETE} query and optionally filter by {@link PersistentPropertyPath}. + * + * @param path can be {@literal null}. + * @return + */ + String createDeleteAllSql(@Nullable PersistentPropertyPath path) { + + Table table = getTable(); + + DeleteBuilder.DeleteWhere deleteAll = Delete.builder().from(table); + + if (path == null) { + return render(deleteAll.build()); + } + + return createDeleteByPathAndCriteria(new PersistentPropertyPathExtension(mappingContext, path), Column::isNotNull); + } + + /** + * Create a {@code DELETE} query and filter by {@link PersistentPropertyPath}. + * + * @param path must not be {@literal null}. + * @return + */ + String createDeleteByPath(PersistentPropertyPath path) { + return createDeleteByPathAndCriteria(new PersistentPropertyPathExtension(mappingContext, path), + filterColumn -> filterColumn.isEqualTo(getBindMarker("rootId"))); + } + private String createFindOneSql() { - SelectBuilder.SelectWhereAndOr withCondition = createBaseSelect() - .where(sqlContext.getIdColumn().isEqualTo(SQL.bindMarker(":id"))); + Select select = selectBuilder().where(getIdColumn().isEqualTo(getBindMarker("id"))) // + .build(); - return render(withCondition); - } - - private Stream getColumnNameStream(String prefix) { - - return StreamUtils.createStreamFromIterator(entity.iterator()) // - .flatMap(p -> getColumnNameStream(p, prefix)); - } - - private Stream getColumnNameStream(RelationalPersistentProperty p, String prefix) { - - if (p.isEntity()) { - return sqlGeneratorSource.getSqlGenerator(p.getType()).getColumnNameStream(prefix + p.getColumnName() + "_"); - } else { - return Stream.of(prefix + p.getColumnName()); - } + return render(select); } private String createFindAllSql() { - return render(createBaseSelect()); + return render(selectBuilder().build()); } - private SelectBuilder.SelectWhere createBaseSelect() { - - return createBaseSelect(null); + private SelectBuilder.SelectWhere selectBuilder() { + return selectBuilder(Collections.emptyList()); } - private SelectBuilder.SelectWhere createBaseSelect(@Nullable String keyColumn) { + private SelectBuilder.SelectWhere selectBuilder(Collection keyColumns) { - Table table = SQL.table(entity.getTableName()); + Table table = getTable(); List columnExpressions = new ArrayList<>(); @@ -257,12 +291,11 @@ class SqlGenerator { } } - if (keyColumn != null) { + for (String keyColumn : keyColumns) { columnExpressions.add(table.column(keyColumn).as(keyColumn)); } SelectBuilder.SelectAndFrom selectBuilder = StatementBuilder.select(columnExpressions); - SelectBuilder.SelectJoin baseSelect = selectBuilder.from(table); for (Join join : joinTables) { @@ -272,6 +305,12 @@ class SqlGenerator { return (SelectBuilder.SelectWhere) baseSelect; } + /** + * Create a {@link Column} for {@link PersistentPropertyPathExtension}. + * + * @param path + * @return + */ @Nullable Column getColumn(PersistentPropertyPathExtension path) { @@ -296,11 +335,9 @@ class SqlGenerator { } return sqlContext.getReverseColumn(path); - } return sqlContext.getColumn(path); - } @Nullable @@ -324,59 +361,42 @@ class SqlGenerator { private String createFindAllInListSql() { - SelectBuilder.SelectWhereAndOr withCondition = createBaseSelect() - .where(sqlContext.getIdColumn().in(SQL.bindMarker(":ids"))); + Select select = selectBuilder().where(getIdColumn().in(getBindMarker("ids"))).build(); - return render(withCondition); - } - - private String render(SelectBuilder.BuildSelect select) { - return SqlRenderer.create().render(select.build()); - } - - private String render(InsertBuilder.BuildInsert insert) { - return SqlRenderer.create().render(insert.build()); - } - - private String render(DeleteBuilder.BuildDelete delete) { - return SqlRenderer.create().render(delete.build()); - } - - private String render(UpdateBuilder.BuildUpdate update) { - return SqlRenderer.create().render(update.build()); + return render(select); } private String createExistsSql() { - Table table = sqlContext.getTable(); - Column idColumn = table.column(entity.getIdColumn()); + Table table = getTable(); - SelectBuilder.BuildSelect select = StatementBuilder // - .select(Functions.count(idColumn)) // + Select select = StatementBuilder // + .select(Functions.count(getIdColumn())) // .from(table) // - .where(idColumn.isEqualTo(SQL.bindMarker(":id"))); + .where(getIdColumn().isEqualTo(getBindMarker("id"))) // + .build(); return render(select); } private String createCountSql() { - Table table = SQL.table(entity.getTableName()); + Table table = getTable(); - SelectBuilder.BuildSelect select = StatementBuilder // + Select select = StatementBuilder // .select(Functions.count(Expressions.asterisk())) // - .from(table); + .from(table) // + .build(); return render(select); } private String createInsertSql(Set additionalColumns) { - Table table = SQL.table(entity.getTableName()); + Table table = getTable(); - LinkedHashSet columnNamesForInsert = new LinkedHashSet<>(nonIdColumnNames); + Set columnNamesForInsert = new LinkedHashSet<>(columns.getInsertableColumns()); columnNamesForInsert.addAll(additionalColumns); - columnNamesForInsert.removeIf(readOnlyColumnNames::contains); InsertBuilder.InsertIntoColumnsAndValuesWithBuild insert = Insert.builder().into(table); @@ -386,98 +406,76 @@ class SqlGenerator { InsertBuilder.InsertValuesWithBuild insertWithValues = null; for (String cn : columnNamesForInsert) { - insertWithValues = (insertWithValues == null ? insert : insertWithValues) - .values(SQL.bindMarker(":" + columnNameToParameterName(cn))); + insertWithValues = (insertWithValues == null ? insert : insertWithValues).values(getBindMarker(cn)); } - return render(insertWithValues == null ? insert : insertWithValues); + return render(insertWithValues == null ? insert.build() : insertWithValues.build()); } private String createUpdateSql() { - Table table = SQL.table(entity.getTableName()); + Table table = getTable(); - List assignments = columnNames.stream() // - .filter(s -> !s.equals(entity.getIdColumn())) // - .filter(s -> !readOnlyColumnNames.contains(s)) // + List assignments = columns.getUpdateableColumns() // + .stream() // .map(columnName -> Assignments.value( // table.column(columnName), // - SQL.bindMarker(":" + columnNameToParameterName(columnName)))) // + getBindMarker(columnName))) // .collect(Collectors.toList()); - UpdateBuilder.UpdateWhereAndOr update = Update.builder() // + Update update = Update.builder() // .table(table) // .set(assignments) // - .where(table.column(entity.getIdColumn()) - .isEqualTo(SQL.bindMarker(":" + columnNameToParameterName(entity.getIdColumn())))) // - ; + .where(getIdColumn().isEqualTo(getBindMarker(entity.getIdColumn()))) // + .build(); return render(update); } private String createDeleteSql() { - Table table = SQL.table(entity.getTableName()); + Table table = getTable(); - DeleteBuilder.DeleteWhereAndOr delete = Delete.builder().from(table) - .where(table.column(entity.getIdColumn()).isEqualTo(SQL.bindMarker(":id"))); + Delete delete = Delete.builder().from(table).where(getIdColumn().isEqualTo(SQL.bindMarker(":id"))) // + .build(); return render(delete); } - String createDeleteAllSql(@Nullable PersistentPropertyPath path) { - - Table table = SQL.table(entity.getTableName()); - - DeleteBuilder.DeleteWhere deleteAll = Delete.builder().from(table); - - if (path == null) { - return render(deleteAll); - } - return createDeleteByPathAndCriteria(new PersistentPropertyPathExtension(mappingContext, path), Column::isNotNull); - } - - private String createDeleteByListSql() { - - Table table = SQL.table(entity.getTableName()); - - DeleteBuilder.DeleteWhereAndOr delete = Delete.builder() // - .from(table) // - .where(table.column(entity.getIdColumn()).in(SQL.bindMarker(":ids"))); - - return render(delete); - } - - String createDeleteByPath(PersistentPropertyPath path) { - return createDeleteByPathAndCriteria(new PersistentPropertyPathExtension(mappingContext, path), - filterColumn -> filterColumn.isEqualTo(SQL.bindMarker(":rootId"))); - } - private String createDeleteByPathAndCriteria(PersistentPropertyPathExtension path, Function rootCondition) { Table table = SQL.table(path.getTableName()); - DeleteBuilder.DeleteWhere delete = Delete.builder() // + DeleteBuilder.DeleteWhere builder = Delete.builder() // .from(table); - - DeleteBuilder.DeleteWhereAndOr deleteWithWhere; + Delete delete; Column filterColumn = table.column(path.getReverseColumnName()); if (path.getLength() == 1) { - deleteWithWhere = delete // - .where(rootCondition.apply(filterColumn)); + delete = builder // + .where(rootCondition.apply(filterColumn)) // + .build(); } else { Condition condition = getSubselectCondition(path, rootCondition, filterColumn); - deleteWithWhere = delete.where(condition); + delete = builder.where(condition).build(); } - return render(deleteWithWhere); + + return render(delete); } - private Condition getSubselectCondition(PersistentPropertyPathExtension path, + /** + * Construct a {@link Select Sub-Select}. + * + * @param path + * @param rootCondition + * @param filterColumn + * @return + */ + private static Condition getSubselectCondition(PersistentPropertyPathExtension path, Function rootCondition, Column filterColumn) { PersistentPropertyPathExtension parentPath = path.getParentPath(); @@ -497,15 +495,144 @@ class SqlGenerator { return filterColumn.in(select); } - private String columnNameToParameterName(String columnName) { - return parameterPattern.matcher(columnName).replaceAll(""); + private String createDeleteByListSql() { + + Table table = getTable(); + + Delete delete = Delete.builder() // + .from(table) // + .where(getIdColumn().in(getBindMarker("ids"))) // + .build(); + + return render(delete); } + private String render(Select select) { + return SqlRenderer.create().render(select); + } + + private String render(Insert insert) { + return SqlRenderer.create().render(insert); + } + + private String render(Update update) { + return SqlRenderer.create().render(update); + } + + private String render(Delete delete) { + return SqlRenderer.create().render(delete); + } + + private Table getTable() { + return sqlContext.getTable(); + } + + private Column getIdColumn() { + return sqlContext.getIdColumn(); + } + + private static BindMarker getBindMarker(String columnName) { + return SQL.bindMarker(":" + parameterPattern.matcher(columnName).replaceAll("")); + } + + /** + * Value object representing a {@code JOIN} association. + */ @Value - class Join { + static class Join { Table joinTable; Column joinColumn; Column parentId; } + /** + * Value object encapsulating column name caches. + * + * @author Mark Paluch + */ + static class Columns { + + private final MappingContext, RelationalPersistentProperty> mappingContext; + + private final List columnNames = new ArrayList<>(); + private final List idColumnNames = new ArrayList<>(); + private final List nonIdColumnNames = new ArrayList<>(); + private final Set readOnlyColumnNames = new HashSet<>(); + private final Set insertableColumns; + private final Set updateableColumns; + + Columns(RelationalPersistentEntity entity, + MappingContext, RelationalPersistentProperty> mappingContext) { + + this.mappingContext = mappingContext; + + populateColumnNameCache(entity, ""); + + Set insertable = new LinkedHashSet<>(nonIdColumnNames); + insertable.removeAll(readOnlyColumnNames); + + this.insertableColumns = Collections.unmodifiableSet(insertable); + + Set updateable = new LinkedHashSet<>(columnNames); + + updateable.removeAll(idColumnNames); + updateable.removeAll(readOnlyColumnNames); + + this.updateableColumns = Collections.unmodifiableSet(updateable); + } + + private void populateColumnNameCache(RelationalPersistentEntity entity, String prefix) { + + entity.doWithProperties((PropertyHandler) property -> { + + // the referencing column of referenced entity is expected to be on the other side of the relation + if (!property.isEntity()) { + initSimpleColumnName(property, prefix); + } else if (property.isEmbedded()) { + initEmbeddedColumnNames(property, prefix); + } + }); + } + + private void initSimpleColumnName(RelationalPersistentProperty property, String prefix) { + + String columnName = prefix + property.getColumnName(); + + columnNames.add(columnName); + + if (!property.getOwner().isIdProperty(property)) { + nonIdColumnNames.add(columnName); + } else { + idColumnNames.add(columnName); + } + + if (!property.isWritable() || property.isAnnotationPresent(ReadOnlyProperty.class)) { + readOnlyColumnNames.add(columnName); + } + } + + private void initEmbeddedColumnNames(RelationalPersistentProperty property, String prefix) { + + String embeddedPrefix = property.getEmbeddedPrefix(); + + RelationalPersistentEntity embeddedEntity = mappingContext + .getRequiredPersistentEntity(property.getColumnType()); + + populateColumnNameCache(embeddedEntity, prefix + embeddedPrefix); + } + + /** + * @return Column names that can be used for {@code INSERT}. + */ + Set getInsertableColumns() { + return insertableColumns; + } + + /** + * @return Column names that can be used for {@code UPDATE}. + */ + Set getUpdateableColumns() { + return updateableColumns; + } + } } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/SqlGeneratorSource.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/SqlGeneratorSource.java index b66273a9..732d9807 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/SqlGeneratorSource.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/SqlGeneratorSource.java @@ -17,7 +17,6 @@ package org.springframework.data.jdbc.core; import lombok.RequiredArgsConstructor; -import java.util.HashMap; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -38,7 +37,6 @@ public class SqlGeneratorSource { SqlGenerator getSqlGenerator(Class domainType) { return sqlGeneratorCache.computeIfAbsent(domainType, - t -> new SqlGenerator(context, context.getRequiredPersistentEntity(t), this)); - + t -> new SqlGenerator(context, context.getRequiredPersistentEntity(t))); } } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/PersistentPropertyPathExtensionUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/PersistentPropertyPathExtensionUnitTests.java index 6e9f48dc..27350c3e 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/PersistentPropertyPathExtensionUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/PersistentPropertyPathExtensionUnitTests.java @@ -5,7 +5,7 @@ * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * 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, diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorContextBasedNamingStrategyUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorContextBasedNamingStrategyUnitTests.java index 03522ba1..4bf0a597 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorContextBasedNamingStrategyUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorContextBasedNamingStrategyUnitTests.java @@ -212,7 +212,7 @@ public class SqlGeneratorContextBasedNamingStrategyUnitTests { RelationalMappingContext context = new JdbcMappingContext(namingStrategy); RelationalPersistentEntity persistentEntity = context.getRequiredPersistentEntity(DummyEntity.class); - return new SqlGenerator(context, persistentEntity, new SqlGeneratorSource(context)); + return new SqlGenerator(context, persistentEntity); } @SuppressWarnings("unused") diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorEmbeddedUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorEmbeddedUnitTests.java index 67a20dcd..10184c76 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorEmbeddedUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorEmbeddedUnitTests.java @@ -47,7 +47,7 @@ public class SqlGeneratorEmbeddedUnitTests { SqlGenerator createSqlGenerator(Class type) { RelationalPersistentEntity persistentEntity = context.getRequiredPersistentEntity(type); - return new SqlGenerator(context, persistentEntity, new SqlGeneratorSource(context)); + return new SqlGenerator(context, persistentEntity); } @Test // DATAJDBC-111 diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorFixedNamingStrategyUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorFixedNamingStrategyUnitTests.java index b8a87d19..2dd17b91 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorFixedNamingStrategyUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorFixedNamingStrategyUnitTests.java @@ -189,7 +189,7 @@ public class SqlGeneratorFixedNamingStrategyUnitTests { RelationalMappingContext context = new JdbcMappingContext(namingStrategy); RelationalPersistentEntity persistentEntity = context.getRequiredPersistentEntity(DummyEntity.class); - return new SqlGenerator(context, persistentEntity, new SqlGeneratorSource(context)); + return new SqlGenerator(context, persistentEntity); } @SuppressWarnings("unused") diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorUnitTests.java index b2db5989..a7e6e064 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/SqlGeneratorUnitTests.java @@ -24,6 +24,7 @@ import java.util.Set; import org.assertj.core.api.SoftAssertions; import org.junit.Before; import org.junit.Test; + import org.springframework.data.annotation.Id; import org.springframework.data.annotation.ReadOnlyProperty; import org.springframework.data.jdbc.core.mapping.AggregateReference; @@ -45,6 +46,7 @@ import org.springframework.data.relational.core.sql.Table; * @author Greg Turnquist * @author Oleksandr Kucher * @author Bastian Wilhelm + * @author Mark Paluch */ public class SqlGeneratorUnitTests { @@ -61,7 +63,7 @@ public class SqlGeneratorUnitTests { RelationalPersistentEntity persistentEntity = context.getRequiredPersistentEntity(type); - return new SqlGenerator(context, persistentEntity, new SqlGeneratorSource(context)); + return new SqlGenerator(context, persistentEntity); } @Test // DATAJDBC-112 @@ -146,7 +148,7 @@ public class SqlGeneratorUnitTests { public void findAllByProperty() { // this would get called when ListParent is the element type of a Set - String sql = sqlGenerator.getFindAllByProperty("back-ref", null, false); + String sql = sqlGenerator.getFindAllByProperty("backref", null, false); assertThat(sql).contains("SELECT", // "dummy_entity.id1 AS id1", // @@ -159,14 +161,14 @@ 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", // - "WHERE dummy_entity.back-ref = :back-ref"); + "WHERE dummy_entity.backref = :backref"); } @Test // DATAJDBC-131, DATAJDBC-111 public void findAllByPropertyWithKey() { // this would get called when ListParent is th element type of a Map - String sql = sqlGenerator.getFindAllByProperty("back-ref", "key-column", false); + String sql = sqlGenerator.getFindAllByProperty("backref", "key-column", false); assertThat(sql).isEqualTo("SELECT dummy_entity.id1 AS id1, dummy_entity.x_name AS x_name, " // + "dummy_entity.x_other AS x_other, " // @@ -176,7 +178,7 @@ 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 " // - + "WHERE dummy_entity.back-ref = :back-ref"); + + "WHERE dummy_entity.backref = :backref"); } @Test(expected = IllegalArgumentException.class) // DATAJDBC-130 @@ -188,7 +190,7 @@ public class SqlGeneratorUnitTests { public void findAllByPropertyWithKeyOrdered() { // this would get called when ListParent is th element type of a Map - String sql = sqlGenerator.getFindAllByProperty("back-ref", "key-column", true); + String sql = sqlGenerator.getFindAllByProperty("backref", "key-column", true); assertThat(sql).isEqualTo("SELECT dummy_entity.id1 AS id1, dummy_entity.x_name AS x_name, " // + "dummy_entity.x_other AS x_other, " // @@ -198,7 +200,7 @@ 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 " // - + "WHERE dummy_entity.back-ref = :back-ref " + "ORDER BY key-column"); + + "WHERE dummy_entity.backref = :backref " + "ORDER BY key-column"); } @Test // DATAJDBC-264 @@ -294,14 +296,14 @@ public class SqlGeneratorUnitTests { final SqlGenerator sqlGenerator = createSqlGenerator(EntityWithReadOnlyProperty.class); - assertThat(sqlGenerator.getFindAllByProperty("back-ref", "key-column", true)).isEqualToIgnoringCase( // + assertThat(sqlGenerator.getFindAllByProperty("backref", "key-column", true)).isEqualToIgnoringCase( // "SELECT " // + "entity_with_read_only_property.x_id AS x_id, " // + "entity_with_read_only_property.x_name AS x_name, " // + "entity_with_read_only_property.x_read_only_value AS x_read_only_value, " // + "entity_with_read_only_property.key-column AS key-column " // + "FROM entity_with_read_only_property " // - + "WHERE entity_with_read_only_property.back-ref = :back-ref " // + + "WHERE entity_with_read_only_property.backref = :backref " // + "ORDER BY key-column" // ); }