diff --git a/pom.xml b/pom.xml index ee6570e14e..fd1b6ed5ce 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-relational-parent - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-381-SNAPSHOT pom Spring Data Relational Parent diff --git a/spring-data-jdbc-distribution/pom.xml b/spring-data-jdbc-distribution/pom.xml index f6d4373844..876181efa1 100644 --- a/spring-data-jdbc-distribution/pom.xml +++ b/spring-data-jdbc-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-381-SNAPSHOT ../pom.xml diff --git a/spring-data-jdbc/pom.xml b/spring-data-jdbc/pom.xml index cfb1e9443b..6fc94897e8 100644 --- a/spring-data-jdbc/pom.xml +++ b/spring-data-jdbc/pom.xml @@ -5,7 +5,7 @@ 4.0.0 spring-data-jdbc - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-381-SNAPSHOT Spring Data JDBC Spring Data module for JDBC repositories. @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-381-SNAPSHOT diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java index 6c7f2bdeb4..461ce0e204 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java @@ -36,6 +36,7 @@ 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.SqlUtils; import org.springframework.data.relational.domain.Identifier; import org.springframework.jdbc.core.RowMapper; import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; @@ -119,7 +120,7 @@ public Object insert(T instance, Class domainType, Identifier identifier) operations.update( // sql(domainType).getInsert(new HashSet<>(Arrays.asList(parameterSource.getParameterNames()))), // - parameterSource, // + sanitize(parameterSource), // holder // ); @@ -136,7 +137,7 @@ public boolean update(S instance, Class domainType) { RelationalPersistentEntity persistentEntity = getRequiredPersistentEntity(domainType); return operations.update(sql(domainType).getUpdate(), - getParameterSource(instance, persistentEntity, "", Predicates.includeAll())) != 0; + sanitize(getParameterSource(instance, persistentEntity, "", Predicates.includeAll()))) != 0; } /* @@ -147,7 +148,7 @@ public boolean update(S instance, Class domainType) { public void delete(Object id, Class domainType) { String deleteByIdSql = sql(domainType).getDeleteById(); - MapSqlParameterSource parameter = createIdParameterSource(id, domainType); + MapSqlParameterSource parameter = sanitize(createIdParameterSource(id, domainType)); operations.update(deleteByIdSql, parameter); } @@ -217,7 +218,7 @@ public T findById(Object id, Class domainType) { MapSqlParameterSource parameter = createIdParameterSource(id, domainType); try { - return operations.queryForObject(findOneSql, parameter, (RowMapper) getEntityRowMapper(domainType)); + return operations.queryForObject(findOneSql, sanitize(parameter), (RowMapper) getEntityRowMapper(domainType)); } catch (EmptyResultDataAccessException e) { return null; } @@ -248,7 +249,7 @@ public Iterable findAllById(Iterable ids, Class domainType) { String findAllInListSql = sql(domainType).getFindAllInList(); - return operations.query(findAllInListSql, parameterSource, (RowMapper) getEntityRowMapper(domainType)); + return operations.query(findAllInListSql, sanitize(parameterSource), (RowMapper) getEntityRowMapper(domainType)); } /* @@ -274,7 +275,7 @@ public Iterable findAllByPath(Identifier identifier, RowMapper rowMapper = path.isMap() ? this.getMapEntityRowMapper(path, identifier) : this.getEntityRowMapper(path, identifier); - return operations.query(findAllByProperty, parameters, (RowMapper) rowMapper); + return operations.query(findAllByProperty, sanitize( parameters), (RowMapper) rowMapper); } /* @@ -302,7 +303,7 @@ public boolean existsById(Object id, Class domainType) { String existsSql = sql(domainType).getExists(); MapSqlParameterSource parameter = createIdParameterSource(id, domainType); - Boolean result = operations.queryForObject(existsSql, parameter, Boolean.class); + Boolean result = operations.queryForObject(existsSql,sanitize( parameter), Boolean.class); Assert.state(result != null, "The result of an exists query must not be null"); return result; @@ -344,6 +345,24 @@ private MapSqlParameterSource getParameterSource(S instance, RelationalPe return parameters; } + private MapSqlParameterSource sanitize(MapSqlParameterSource parameterSource) { + + MapSqlParameterSource sanitized = new MapSqlParameterSource(); + + for (String parameterName : parameterSource.getParameterNames()) { + + String sanitizedName = SqlUtils.sanitizeName(parameterName); + + sanitized.addValue( // + sanitizedName, // + parameterSource.getValue(parameterName), // + parameterSource.getSqlType(parameterName), // + parameterSource.getTypeName(parameterName)); // + } + + return sanitized; + } + @Nullable @SuppressWarnings("unchecked") private ID getIdValueOrNull(S instance, RelationalPersistentEntity persistentEntity) { 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 c24d32ad8c..d5eeb8b922 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 @@ -136,7 +136,7 @@ private static Condition getSubselectCondition(PersistentPropertyPathExtension p } private static BindMarker getBindMarker(String columnName) { - return SQL.bindMarker(":" + parameterPattern.matcher(columnName).replaceAll("")); + return SQL.bindMarker(":" + SqlUtils.sanitizeName(columnName)); } /** 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 8c35785fee..21caf98e3d 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 @@ -35,7 +35,6 @@ import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; - import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.annotation.Bean; @@ -597,6 +596,26 @@ public void shouldDeleteChainOfMapsWithoutIds() { }); } + @Test // DATAJDBC-381 + @IfProfileValue(name = "current.database", value = "mysql") + public void saveAndLoadEntityWithKeywordAsColumnName() { + + WithKeywordColumn entity = new WithKeywordColumn(); + entity.virtual = "some value"; + + WithKeywordColumn saved = template.save(entity); + + WithKeywordColumn reloaded = template.findById(saved.id, WithKeywordColumn.class); + + assertThat(reloaded.virtual).isEqualTo("some value"); + + reloaded.virtual = "other value"; + + template.save(reloaded); + + template.deleteById(reloaded.id, WithKeywordColumn.class); + } + private static NoIdMapChain4 createNoIdMapTree() { NoIdMapChain4 chain4 = new NoIdMapChain4(); @@ -870,4 +889,12 @@ static class NoIdMapChain4 { String fourValue; Map chain3 = new HashMap<>(); } + + @Data + static class WithKeywordColumn { + + @Id private Long id; + + @Column("`virtual`") private String virtual; + } } 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 4ca8aa891e..2211afca45 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 @@ -247,7 +247,7 @@ public void getInsertForQuotedColumnName() { String insert = sqlGenerator.getInsert(emptySet()); assertThat(insert) - .isEqualTo("INSERT INTO entity_with_quoted_column_name " + "(\"test_@123\") " + "VALUES (:test_123)"); + .isEqualTo("INSERT INTO entity_with_quoted_column_name " + "(\"test_@123\") " + "VALUES (:test_123_)"); } @Test // DATAJDBC-266 @@ -291,8 +291,8 @@ public void getUpdateForQuotedColumnName() { String update = sqlGenerator.getUpdate(); - assertThat(update).isEqualTo("UPDATE entity_with_quoted_column_name " + "SET \"test_@123\" = :test_123 " - + "WHERE entity_with_quoted_column_name.\"test_@id\" = :test_id"); + assertThat(update).isEqualTo("UPDATE entity_with_quoted_column_name " + "SET \"test_@123\" = :test_123_ " + + "WHERE entity_with_quoted_column_name.\"test_@id\" = :test_id_"); } @Test // DATAJDBC-324 diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DatabaseProfileValueSource.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DatabaseProfileValueSource.java index 776a2ea42a..f8ec8039bc 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DatabaseProfileValueSource.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/DatabaseProfileValueSource.java @@ -18,10 +18,11 @@ import org.springframework.test.annotation.ProfileValueSource; /** - * This {@link ProfileValueSource} offers a single set of keys {@code current.database.is.not.} where + * This {@link ProfileValueSource} offers a set of keys {@code current.database.is.not.} where * {@code } is a database as used in active profiles to enable integration tests to run with a certain * database. The value returned for these keys is {@code "true"} or {@code "false"} depending on if the database is - * actually the one currently used by integration tests. + * actually the one currently used by integration tests. Additionally it offers the key {@code current.database} which + * holds the database value. * * @author Jens Schauder */ @@ -37,10 +38,14 @@ public class DatabaseProfileValueSource implements ProfileValueSource { @Override public String get(String key) { - if (!key.startsWith("current.database.is.not.")) { - return null; + if (key.startsWith("current.database.is.not.")) { + return Boolean.toString(!key.endsWith(currentDatabase)).toLowerCase(); } - return Boolean.toString(!key.endsWith(currentDatabase)).toLowerCase(); + if (key.startsWith("current.database")) { + return currentDatabase; + } + + return null; } } diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mysql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mysql.sql index 5747ba0b6b..965317f36a 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mysql.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-mysql.sql @@ -282,4 +282,10 @@ CREATE TABLE NO_ID_MAP_CHAIN0 NO_ID_MAP_CHAIN3_KEY, NO_ID_MAP_CHAIN2_KEY ) +); + +CREATE TABLE WITH_KEYWORD_COLUMN +( + id BIGINT AUTO_INCREMENT PRIMARY KEY, + `VIRTUAL` VARCHAR(200) ); \ No newline at end of file diff --git a/spring-data-relational/pom.xml b/spring-data-relational/pom.xml index fa7f2f4873..b473f3f0c3 100644 --- a/spring-data-relational/pom.xml +++ b/spring-data-relational/pom.xml @@ -5,7 +5,7 @@ 4.0.0 spring-data-relational - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-381-SNAPSHOT Spring Data Relational Spring Data Relational support @@ -13,7 +13,7 @@ org.springframework.data spring-data-relational-parent - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-381-SNAPSHOT diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/PersistentPropertyPathExtension.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/PersistentPropertyPathExtension.java index 0f7bce65a8..570fc4ae3f 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/PersistentPropertyPathExtension.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/PersistentPropertyPathExtension.java @@ -20,6 +20,7 @@ import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.mapping.context.MappingContext; +import org.springframework.data.relational.core.sql.SqlUtils; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -189,7 +190,7 @@ public String getColumnName() { */ public String getColumnAlias() { - return prefixWithTableAlias(getColumnName()); + return SqlUtils.sanitizeName(prefixWithTableAlias(getColumnName())); } /** diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/SqlUtils.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/SqlUtils.java new file mode 100644 index 0000000000..3a12dd8ca8 --- /dev/null +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/SqlUtils.java @@ -0,0 +1,51 @@ +/* + * 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.relational.core.sql; + +import lombok.experimental.UtilityClass; + +import java.util.regex.Pattern; + +import org.springframework.util.StringUtils; + +/** + * Utility class for SQL related functions. + * + * @author Jens Schauder + * @since 1.1 + */ +@UtilityClass +public class SqlUtils { + + private static final Pattern parameterPattern = Pattern.compile("\\W"); + + /** + * Sanitizes a name so that the result maybe used for example as a bind parameter name or an alias. This is done by + * removing all special characters and if any where present appending and '_' in order to avoid resulting with a + * keyword. + * + * @param name as used for a table or a column. It may contain special characters like quotes. + */ + public String sanitizeName(String name) { + + if (StringUtils.isEmpty(name)) { + return name; + } + + String sanitized = parameterPattern.matcher(name).replaceAll(""); + return sanitized.equals(name) ? sanitized : sanitized + "_"; + } +} diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/SqlUtilsUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/SqlUtilsUnitTests.java new file mode 100644 index 0000000000..37cda8ef34 --- /dev/null +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/SqlUtilsUnitTests.java @@ -0,0 +1,38 @@ +/* + * 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.relational.core.sql; + +import static org.assertj.core.api.Assertions.*; + +import org.junit.Test; + +/** + * Unit tests for SqlUtils. + * + * @author Jens Schauder + */ +public class SqlUtilsUnitTests { + + @Test + public void simpleName() { + assertThat(SqlUtils.sanitizeName("simple")).isEqualTo("simple"); + } + + @Test + public void specialCharactersGetTrimmedName() { + assertThat(SqlUtils.sanitizeName("`this might be a keyword`")).isEqualTo("thismightbeakeyword_"); + } +}