diff --git a/pom.xml b/pom.xml index 2220303342..eeb1b282b8 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-407-SNAPSHOT pom Spring Data Relational Parent diff --git a/spring-data-jdbc-distribution/pom.xml b/spring-data-jdbc-distribution/pom.xml index 71b9a3c782..fc168e241d 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-407-SNAPSHOT ../pom.xml diff --git a/spring-data-jdbc/pom.xml b/spring-data-jdbc/pom.xml index d33566bc88..557c3580a6 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-407-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-407-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 3f54d1d3f0..61a21c6fbb 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 @@ -37,6 +37,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; @@ -120,7 +121,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 // ); @@ -137,7 +138,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; } /* @@ -148,7 +149,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); } @@ -218,7 +219,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; } @@ -253,7 +254,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)); } /* @@ -279,7 +280,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); } /* @@ -307,7 +308,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; @@ -349,6 +350,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 c388b46234..360051841c 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 @@ -597,6 +597,28 @@ public void shouldDeleteChainOfMapsWithoutIds() { }); } + @Test // DATAJDBC-407 + @IfProfileValue(name = "current.database", value = "postgres") + public void saveAndLoadEntityWithQuotedColumnName() { + + EntityWithQuotedColumnName entity = new EntityWithQuotedColumnName(); + entity.id = 42L; + entity.name = "some value"; + entity.name2 = "name2"; + + EntityWithQuotedColumnName saved = template.save(entity); + + EntityWithQuotedColumnName reloaded = template.findById(saved.id, EntityWithQuotedColumnName.class); + + assertThat(reloaded.name).isEqualTo("some value"); + + reloaded.name = "new value"; + + template.save(reloaded); + + template.deleteById(reloaded.id, EntityWithQuotedColumnName.class); + } + private static NoIdMapChain4 createNoIdMapTree() { NoIdMapChain4 chain4 = new NoIdMapChain4(); @@ -870,4 +892,11 @@ JdbcAggregateOperations operations(ApplicationEventPublisher publisher, Relation return new JdbcAggregateTemplate(publisher, context, converter, dataAccessStrategy); } } + + @Data + static class EntityWithQuotedColumnName { + @Id @Column("\"test_@id\"") private Long id; + @Column("\"test_@123\"") private String name; + @Column("\"ValueCol\"") private String name2; + } } 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 f9d876dd45..c4fa6c122d 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 @@ -33,6 +33,7 @@ import org.springframework.data.convert.ReadingConverter; import org.springframework.data.convert.WritingConverter; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; +import org.springframework.data.relational.core.mapping.Column; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.jdbc.core.JdbcOperations; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; @@ -132,6 +133,24 @@ public void considersConfiguredWriteConverter() { assertThat(paramSourceCaptor.getValue().getValue("flag")).isEqualTo("T"); } + @Test // DATAJDBC-407 + public void additionalParametersForCaseSensitiveColumnNames() { + + ArgumentCaptor sqlCaptor = ArgumentCaptor.forClass(String.class); + + accessStrategy.insert(new EntityWithCaseSensitiveColumnName(ORIGINAL_ID, "val", "val2"), EntityWithCaseSensitiveColumnName.class, additionalParameters); + + verify(namedJdbcOperations).update(sqlCaptor.capture(), paramSourceCaptor.capture(), any(KeyHolder.class)); + + assertThat(sqlCaptor.getValue()) // + .containsSequence("INSERT INTO entity_with_case_sensitive_column_name (", "\"primaryKey\"", ") VALUES (", ":primaryKey", ")") // + .containsSequence("INSERT INTO entity_with_case_sensitive_column_name (", "\"ValueCol\"", ") VALUES (", ":ValueCol", ")") // + .containsSequence("INSERT INTO entity_with_case_sensitive_column_name (", "\"test_@123\"", ") VALUES (", ":test_123", ")"); + assertThat(paramSourceCaptor.getValue().getValue("primaryKey")).isEqualTo(ORIGINAL_ID); + assertThat(paramSourceCaptor.getValue().getValue("ValueCol")).isEqualTo("val"); + assertThat(paramSourceCaptor.getValue().getValue("test_123")).isEqualTo("val2"); + } + @RequiredArgsConstructor private static class DummyEntity { @@ -145,6 +164,13 @@ private static class EntityWithBoolean { boolean flag; } + @AllArgsConstructor + static class EntityWithCaseSensitiveColumnName { + @Id @Column("\"primaryKey\"") Long id; + @Column("\"ValueCol\"") String name; + @Column("\"test_@123\"") String name2; + } + @WritingConverter enum BooleanToStringConverter implements Converter { 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-postgres.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql index 20fbd20a43..845131d538 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-postgres.sql @@ -303,4 +303,11 @@ CREATE TABLE NO_ID_MAP_CHAIN0 NO_ID_MAP_CHAIN3_KEY, NO_ID_MAP_CHAIN2_KEY ) -); \ No newline at end of file +); + +CREATE TABLE ENTITY_WITH_QUOTED_COLUMN_NAME +( + "primaryKey" SERIAL PRIMARY KEY, + "ValueCol" VARCHAR(20), + "test_@123" VARCHAR(20) +); diff --git a/spring-data-relational/pom.xml b/spring-data-relational/pom.xml index fa7f2f4873..b56d0e31c7 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-407-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-407-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 85f75a7df6..ff87ff56b6 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 @@ -21,6 +21,7 @@ import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.util.Lazy; +import org.springframework.data.relational.core.sql.SqlUtils; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -191,7 +192,8 @@ public String getColumnName() { * @throws IllegalStateException when called on an empty path. */ public String getColumnAlias() { - return columnAlias.get(); + + 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..e1b0c161fa --- /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) ? name : 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..cb5001eb7e --- /dev/null +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/SqlUtilsUnitTests.java @@ -0,0 +1,43 @@ +/* + * 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"); + } + + @Test + public void caseSensitiveName() { + assertThat(SqlUtils.sanitizeName("\"CaseSensitiveName@\"")).isEqualTo("CaseSensitiveName"); + } +}