DATAJDBC-407 Escaping case-sensitive columns

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-relational-parent</artifactId>
-	<version>1.1.0.BUILD-SNAPSHOT</version>
+	<version>1.1.0.DATAJDBC-407-SNAPSHOT</version>
 	<packaging>pom</packaging>
 
 	<name>Spring Data Relational Parent</name>

spring-data-jdbc-distribution/pom.xml

@@ -14,7 +14,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-relational-parent</artifactId>
-		<version>1.1.0.BUILD-SNAPSHOT</version>
+		<version>1.1.0.DATAJDBC-407-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

spring-data-jdbc/pom.xml

@@ -5,7 +5,7 @@
 	<modelVersion>4.0.0</modelVersion>
 
 	<artifactId>spring-data-jdbc</artifactId>
-	<version>1.1.0.BUILD-SNAPSHOT</version>
+	<version>1.1.0.DATAJDBC-407-SNAPSHOT</version>
 
 	<name>Spring Data JDBC</name>
 	<description>Spring Data module for JDBC repositories.</description>
@@ -14,7 +14,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-relational-parent</artifactId>
-		<version>1.1.0.BUILD-SNAPSHOT</version>
+		<version>1.1.0.DATAJDBC-407-SNAPSHOT</version>
 	</parent>
 
 	<properties>

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 <T> Object insert(T instance, Class<T> domainType, Identifier identifier)
 
 		operations.update( //
 				sql(domainType).getInsert(new HashSet<>(Arrays.asList(parameterSource.getParameterNames()))), //
-				parameterSource, //
+				sanitize(parameterSource), //
 				holder //
 		);
 
@@ -137,7 +138,7 @@ public <S> boolean update(S instance, Class<S> domainType) {
 		RelationalPersistentEntity<S> 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 <S> boolean update(S instance, Class<S> 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> T findById(Object id, Class<T> domainType) {
 		MapSqlParameterSource parameter = createIdParameterSource(id, domainType);
 
 		try {
-			return operations.queryForObject(findOneSql, parameter, (RowMapper<T>) getEntityRowMapper(domainType));
+			return operations.queryForObject(findOneSql, sanitize(parameter), (RowMapper<T>) getEntityRowMapper(domainType));
 		} catch (EmptyResultDataAccessException e) {
 			return null;
 		}
@@ -253,7 +254,7 @@ public <T> Iterable<T> findAllById(Iterable<?> ids, Class<T> domainType) {
 
 		String findAllInListSql = sql(domainType).getFindAllInList();
 
-		return operations.query(findAllInListSql, parameterSource, (RowMapper<T>) getEntityRowMapper(domainType));
+		return operations.query(findAllInListSql, sanitize(parameterSource), (RowMapper<T>) getEntityRowMapper(domainType));
 	}
 
 	/*
@@ -279,7 +280,7 @@ public Iterable<Object> findAllByPath(Identifier identifier,
 		RowMapper<?> rowMapper = path.isMap() ? this.getMapEntityRowMapper(path, identifier)
 				: this.getEntityRowMapper(path, identifier);
 
-		return operations.query(findAllByProperty, parameters, (RowMapper<Object>) rowMapper);
+		return operations.query(findAllByProperty, sanitize( parameters), (RowMapper<Object>) rowMapper);
 	}
 
 	/*
@@ -307,7 +308,7 @@ public <T> boolean existsById(Object id, Class<T> 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 <S, T> 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 <S, ID> ID getIdValueOrNull(S instance, RelationalPersistentEntity<S> persistentEntity) {

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));
 	}
 
 	/**

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;
+	}
 }

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<String> 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<Boolean, String> {
 

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.<database>} where
+ * This {@link ProfileValueSource} offers a set of keys {@code current.database.is.not.<database>} where
  * {@code <database> } 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;
 	}
 }

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
     )
-);
+);
+
+CREATE TABLE ENTITY_WITH_QUOTED_COLUMN_NAME
+(
+    "primaryKey" SERIAL PRIMARY KEY,
+    "ValueCol"   VARCHAR(20),
+    "test_@123"  VARCHAR(20)
+);

spring-data-relational/pom.xml

@@ -5,15 +5,15 @@
 	<modelVersion>4.0.0</modelVersion>
 
 	<artifactId>spring-data-relational</artifactId>
-	<version>1.1.0.BUILD-SNAPSHOT</version>
+	<version>1.1.0.DATAJDBC-407-SNAPSHOT</version>
 
 	<name>Spring Data Relational</name>
 	<description>Spring Data Relational support</description>
 
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-relational-parent</artifactId>
-		<version>1.1.0.BUILD-SNAPSHOT</version>
+		<version>1.1.0.DATAJDBC-407-SNAPSHOT</version>
 	</parent>
 
 	<properties>

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()));
 	}
 
 	/**

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;
+	}
+}