SqlIdentifierParameterSource now sanitizes identifier names

pom.xml

@@ -5,7 +5,7 @@
 
 <groupId>org.springframework.data</groupId>
 <artifactId>spring-data-relational-parent</artifactId>
-<version>3.1.0-SNAPSHOT</version>
+<version>3.1.0-1406-delimited-identifiers-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>3.1.0-SNAPSHOT</version>
+<version>3.1.0-1406-delimited-identifiers-SNAPSHOT</version>
 <relativePath>../pom.xml</relativePath>
 </parent>
 

spring-data-jdbc/pom.xml

@@ -6,7 +6,7 @@
 <modelVersion>4.0.0</modelVersion>
 
 <artifactId>spring-data-jdbc</artifactId>
-<version>3.1.0-SNAPSHOT</version>
+<version>3.1.0-1406-delimited-identifiers-SNAPSHOT</version>
 
 <name>Spring Data JDBC</name>
 <description>Spring Data module for JDBC repositories.</description>
@@ -15,7 +15,7 @@
 <parent>
 <groupId>org.springframework.data</groupId>
 <artifactId>spring-data-relational-parent</artifactId>
-<version>3.1.0-SNAPSHOT</version>
+<version>3.1.0-1406-delimited-identifiers-SNAPSHOT</version>
 </parent>
 
 <properties>

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BindParameterNameSanitizer.java

@@ -0,0 +1,37 @@
+/*
+ * Copyright 2023 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 java.util.regex.Pattern;
+
+/**
+ * Sanitizes the name of bind parameters, so they don't contain any illegal characters.
+ *
+ * @author Jens Schauder
+ *
+ * @since 3.0
+ */
+enum BindParameterNameSanitizer {
+INSTANCE;
+
+private static final Pattern parameterPattern = Pattern.compile("\\W");
+
+String sanitize(String rawName) {
+
+return parameterPattern.matcher(rawName).replaceAll("");
+}
+}

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java

@@ -66,7 +66,6 @@ class SqlGenerator {
 static final SqlIdentifier IDS_SQL_PARAMETER = SqlIdentifier.unquoted("ids");
 static final SqlIdentifier ROOT_ID_PARAMETER = SqlIdentifier.unquoted("rootId");
 
-private static final Pattern parameterPattern = Pattern.compile("\\W");
 private final RelationalPersistentEntity<?> entity;
 private final MappingContext<RelationalPersistentEntity<?>, RelationalPersistentProperty> mappingContext;
 private final RenderContext renderContext;
@@ -159,7 +158,7 @@ private Condition getSubselectCondition(PersistentPropertyPathExtension path,
 }
 
 private BindMarker getBindMarker(SqlIdentifier columnName) {
-return SQL.bindMarker(":" + parameterPattern.matcher(renderReference(columnName)).replaceAll(""));
+return SQL.bindMarker(":" + BindParameterNameSanitizer.INSTANCE.sanitize(renderReference(columnName)));
 }
 
 /**

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlIdentifierParameterSource.java

@@ -68,7 +68,7 @@ void addValue(SqlIdentifier name, Object value) {
 void addValue(SqlIdentifier identifier, Object value, int sqlType) {
 
 identifiers.add(identifier);
-String name = identifier.getReference(identifierProcessing);
+String name = BindParameterNameSanitizer.INSTANCE.sanitize(identifier.getReference(identifierProcessing));
 namesToValues.put(name, value);
 registerSqlType(name, sqlType);
 }

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlParametersFactory.java

@@ -95,8 +95,8 @@ <T> SqlIdentifierParameterSource forInsert(T instance, Class<T> domainType, Iden
  */
 <T> SqlIdentifierParameterSource forUpdate(T instance, Class<T> domainType) {
 
-return getParameterSource(instance, getRequiredPersistentEntity(domainType), "", RelationalPersistentProperty::isInsertOnly,
-dialect.getIdentifierProcessing());
+return getParameterSource(instance, getRequiredPersistentEntity(domainType), "",
+RelationalPersistentProperty::isInsertOnly, dialect.getIdentifierProcessing());
 }
 
 /**

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlParametersFactoryTest.java

@@ -38,6 +38,7 @@
 import org.springframework.data.jdbc.core.mapping.JdbcMappingContext;
 import org.springframework.data.relational.core.conversion.IdValueSource;
 import org.springframework.data.relational.core.dialect.AnsiDialect;
+import org.springframework.data.relational.core.mapping.Column;
 import org.springframework.data.relational.core.mapping.RelationalMappingContext;
 import org.springframework.data.relational.core.sql.SqlIdentifier;
 import org.springframework.jdbc.core.JdbcOperations;
@@ -147,6 +148,21 @@ void considersConfiguredWriteConverterForIdValueObjects_onWrite() {
 assertThat(sqlParameterSource.getValue("value")).isEqualTo(value);
 }
 
+@Test // GH-1405
+void parameterNamesGetSanitized() {
+
+WithIllegalCharacters entity = new WithIllegalCharacters(23L,"aValue");
+
+SqlIdentifierParameterSource sqlParameterSource = sqlParametersFactory.forInsert(entity, WithIllegalCharacters.class,
+Identifier.empty(), IdValueSource.PROVIDED);
+
+assertThat(sqlParameterSource.getValue("id")).isEqualTo(23L);
+assertThat(sqlParameterSource.getValue("value")).isEqualTo("aValue");
+
+assertThat(sqlParameterSource.getValue("i.d")).isNull();
+assertThat(sqlParameterSource.getValue("val&ue")).isNull();
+}
+
 @WritingConverter
 enum IdValueToStringConverter implements Converter<IdValue, String> {
 
@@ -212,6 +228,16 @@ private static class DummyEntity {
 @Id private final Long id;
 }
 
+@AllArgsConstructor
+private static class WithIllegalCharacters {
+
+@Column("i.d")
+@Id Long id;
+
+@Column("val&ue")
+String value;
+}
+
 private SqlParametersFactory createSqlParametersFactoryWithConverters(List<?> converters) {
 
 BasicJdbcConverter converter = new BasicJdbcConverter(context, relationResolver,

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java

@@ -69,7 +69,9 @@
 import org.springframework.data.jdbc.testing.EnabledOnFeature;
 import org.springframework.data.jdbc.testing.TestConfiguration;
 import org.springframework.data.jdbc.testing.TestDatabaseFeatures;
+import org.springframework.data.relational.core.mapping.Column;
 import org.springframework.data.relational.core.mapping.MappedCollection;
+import org.springframework.data.relational.core.mapping.Table;
 import org.springframework.data.relational.core.mapping.event.AbstractRelationalEvent;
 import org.springframework.data.relational.core.mapping.event.AfterConvertEvent;
 import org.springframework.data.relational.core.sql.LockMode;
@@ -102,6 +104,7 @@
  * @author Chirag Tailor
  * @author Diego Krupitza
  * @author Christopher Klein
+ * @author Mikhail Polivakha
  */
 @Transactional
 @TestExecutionListeners(value = AssumeFeatureTestExecutionListener.class, mergeMode = MERGE_WITH_DEFAULTS)
@@ -113,6 +116,8 @@ public class JdbcRepositoryIntegrationTests {
 @Autowired MyEventListener eventListener;
 @Autowired RootRepository rootRepository;
 
+@Autowired WithDelimitedColumnRepository withDelimitedColumnRepository;
+
 private static DummyEntity createDummyEntity() {
 
 DummyEntity entity = new DummyEntity();
@@ -1238,6 +1243,23 @@ void fetchByExampleFluentOnlyInstantOneValueAsSimple() {
 assertThat(match.get().getName()).contains(two.getName());
 }
 
+@Test // GH-1405
+void withDelimitedColumnTest() {
+
+WithDelimitedColumn withDelimitedColumn = new WithDelimitedColumn();
+withDelimitedColumn.setType("TYPICAL");
+withDelimitedColumn.setIdentifier("UR-123");
+
+WithDelimitedColumn saved = withDelimitedColumnRepository.save(withDelimitedColumn);
+
+assertThat(saved.getId()).isNotNull();
+
+Optional<WithDelimitedColumn> inDatabase = withDelimitedColumnRepository.findById(saved.getId());
+
+assertThat(inDatabase).isPresent();
+assertThat(inDatabase.get().getIdentifier()).isEqualTo("UR-123");
+}
+
 private Root createRoot(String namePrefix) {
 
 return new Root(null, namePrefix,
@@ -1361,10 +1383,17 @@ interface DummyEntityRepository extends CrudRepository<DummyEntity, Long>, Query
 List<DummyEntity> findByEnumType(Direction direction);
 }
 
+interface RootRepository extends ListCrudRepository<Root, Long> {
+List<Root> findAllByOrderByIdAsc();
+}
+
+interface WithDelimitedColumnRepository extends CrudRepository<WithDelimitedColumn, Long> { }
+
 @Configuration
 @Import(TestConfiguration.class)
 static class Config {
 
+
 @Autowired JdbcRepositoryFactory factory;
 
 @Bean
@@ -1382,6 +1411,9 @@ RootRepository rootRepository() {
 return factory.getRepository(RootRepository.class);
 }
 
+@Bean
+WithDelimitedColumnRepository withDelimitedColumnRepository() { return factory.getRepository(WithDelimitedColumnRepository.class); }
+
 @Bean
 NamedQueries namedQueries() throws IOException {
 
@@ -1404,15 +1436,11 @@ public ExtensionAwareQueryMethodEvaluationContextProvider extensionAware(List<Ev
 
 return extensionAwareQueryMethodEvaluationContextProvider;
 }
-
 @Bean
 public EvaluationContextExtension evaluationContextExtension() {
 return new MyIdContextProvider();
 }
-}
 
-interface RootRepository extends ListCrudRepository<Root, Long> {
-List<Root> findAllByOrderByIdAsc();
 }
 
 @Value
@@ -1424,6 +1452,14 @@ static class Root {
 @MappedCollection(idColumn = "ROOT_ID", keyColumn = "ROOT_KEY") List<Intermediate> intermediates;
 }
 
+@Data
+@Table("WITH_DELIMITED_COLUMN")
+static class WithDelimitedColumn {
+@Id Long id;
+@Column("ORG.XTUNIT.IDENTIFIER") String identifier;
+@Column ("STYPE") String type;
+}
+
 @Value
 static class Intermediate {
 

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-db2.sql

@@ -2,6 +2,7 @@ DROP TABLE dummy_entity;
 DROP TABLE ROOT;
 DROP TABLE INTERMEDIATE;
 DROP TABLE LEAF;
+DROP TABLE WITH_DELIMITED_COLUMN;
 
 CREATE TABLE dummy_entity
 (
@@ -37,3 +38,10 @@ CREATE TABLE LEAF
  INTERMEDIATE_ID BIGINT,
  INTERMEDIATE_KEY INTEGER
 );
+
+CREATE TABLE WITH_DELIMITED_COLUMN
+(
+ ID BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY,
+ "ORG.XTUNIT.IDENTIFIER" VARCHAR(100),
+ STYPE VARCHAR(100)
+);

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-h2.sql

@@ -32,3 +32,10 @@ CREATE TABLE LEAF
  INTERMEDIATE_ID BIGINT,
  INTERMEDIATE_KEY INTEGER
 );
+
+CREATE TABLE WITH_DELIMITED_COLUMN
+(
+ ID BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY,
+ "ORG.XTUNIT.IDENTIFIER" VARCHAR(100),
+ STYPE VARCHAR(100)
+);

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-hsql.sql

@@ -32,3 +32,10 @@ CREATE TABLE LEAF
  INTERMEDIATE_ID BIGINT,
  INTERMEDIATE_KEY INTEGER
 );
+
+CREATE TABLE WITH_DELIMITED_COLUMN
+(
+ ID BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY,
+ "ORG.XTUNIT.IDENTIFIER" VARCHAR(100),
+ STYPE VARCHAR(100)
+);

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mariadb.sql

@@ -32,3 +32,10 @@ CREATE TABLE LEAF
  INTERMEDIATE_ID BIGINT,
  INTERMEDIATE_KEY INTEGER
 );
+
+CREATE TABLE WITH_DELIMITED_COLUMN
+(
+ ID BIGINT AUTO_INCREMENT PRIMARY KEY,
+ `ORG.XTUNIT.IDENTIFIER` VARCHAR(100),
+ STYPE VARCHAR(100)
+);

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mssql.sql

@@ -2,6 +2,7 @@ DROP TABLE IF EXISTS dummy_entity;
 DROP TABLE IF EXISTS ROOT;
 DROP TABLE IF EXISTS INTERMEDIATE;
 DROP TABLE IF EXISTS LEAF;
+DROP TABLE IF EXISTS WITH_DELIMITED_COLUMN;
 
 CREATE TABLE dummy_entity
 (
@@ -37,3 +38,10 @@ CREATE TABLE LEAF
  INTERMEDIATE_ID BIGINT,
  INTERMEDIATE_KEY INTEGER
 );
+
+CREATE TABLE WITH_DELIMITED_COLUMN
+(
+ ID BIGINT IDENTITY PRIMARY KEY,
+ "ORG.XTUNIT.IDENTIFIER" VARCHAR(100),
+ STYPE VARCHAR(100)
+);

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-mysql.sql

@@ -36,3 +36,9 @@ CREATE TABLE LEAF
  INTERMEDIATE_KEY INTEGER
 );
 
+CREATE TABLE WITH_DELIMITED_COLUMN
+(
+ ID BIGINT AUTO_INCREMENT PRIMARY KEY,
+ `ORG.XTUNIT.IDENTIFIER` VARCHAR(100),
+ STYPE VARCHAR(100)
+);

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-oracle.sql

@@ -2,6 +2,7 @@ DROP TABLE DUMMY_ENTITY CASCADE CONSTRAINTS PURGE;
 DROP TABLE ROOT CASCADE CONSTRAINTS PURGE;
 DROP TABLE INTERMEDIATE CASCADE CONSTRAINTS PURGE;
 DROP TABLE LEAF CASCADE CONSTRAINTS PURGE;
+DROP TABLE WITH_DELIMITED_COLUMN CASCADE CONSTRAINTS PURGE;
 
 CREATE TABLE DUMMY_ENTITY
 (
@@ -37,3 +38,10 @@ CREATE TABLE LEAF
  INTERMEDIATE_ID NUMBER,
  INTERMEDIATE_KEY NUMBER
 );
+
+CREATE TABLE WITH_DELIMITED_COLUMN
+(
+ ID NUMBER GENERATED BY DEFAULT ON NULL AS IDENTITY PRIMARY KEY,
+ "ORG.XTUNIT.IDENTIFIER" VARCHAR(100),
+ STYPE VARCHAR(100)
+)

spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryIntegrationTests-postgres.sql

@@ -2,6 +2,7 @@ DROP TABLE dummy_entity;
 DROP TABLE ROOT;
 DROP TABLE INTERMEDIATE;
 DROP TABLE LEAF;
+DROP TABLE WITH_DELIMITED_COLUMN;
 
 CREATE TABLE dummy_entity
 (
@@ -37,3 +38,10 @@ CREATE TABLE LEAF
  "INTERMEDIATE_ID" BIGINT,
  "INTERMEDIATE_KEY" INTEGER
 );
+
+CREATE TABLE "WITH_DELIMITED_COLUMN"
+(
+ ID BIGINT IDENTITY PRIMARY KEY,
+ "ORG.XTUNIT.IDENTIFIER" VARCHAR(100),
+ STYPE VARCHAR(100)
+);