From 261bc2ad6acf974933267bbfcc2586c286cccad0 Mon Sep 17 00:00:00 2001 From: Marten Deinum Date: Mon, 17 Jan 2022 07:54:07 +0100 Subject: [PATCH 1/3] Fix regression in BeanPropertyRowMapper.underscoreName(String) Commit 6316a35 introduced a regression for property names starting with multiple uppercase letters (such as setEMail(...)). This commit fixes that regression and includes an additional test to cover this case. See gh-27929 Closes gh-27941 --- .../jdbc/core/BeanPropertyRowMapper.java | 5 +- .../jdbc/core/AbstractRowMapperTests.java | 16 +++- .../jdbc/core/BeanPropertyRowMapperTests.java | 14 +++- .../jdbc/core/test/EmailPerson.java | 78 +++++++++++++++++++ 4 files changed, 108 insertions(+), 5 deletions(-) create mode 100644 spring-jdbc/src/test/java/org/springframework/jdbc/core/test/EmailPerson.java diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/core/BeanPropertyRowMapper.java b/spring-jdbc/src/main/java/org/springframework/jdbc/core/BeanPropertyRowMapper.java index 446ef64c5e4..5399e9c4741 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/core/BeanPropertyRowMapper.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/core/BeanPropertyRowMapper.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -273,7 +273,8 @@ public class BeanPropertyRowMapper implements RowMapper { } StringBuilder result = new StringBuilder(); - for (int i = 0; i < name.length(); i++) { + result.append(Character.toLowerCase(name.charAt(0))); + for (int i = 1; i < name.length(); i++) { char c = name.charAt(i); if (Character.isUpperCase(c)) { result.append('_').append(Character.toLowerCase(c)); diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/core/AbstractRowMapperTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/core/AbstractRowMapperTests.java index 601bbdfd7a1..bee0bade090 100644 --- a/spring-jdbc/src/test/java/org/springframework/jdbc/core/AbstractRowMapperTests.java +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/core/AbstractRowMapperTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -30,6 +30,7 @@ import org.springframework.beans.PropertyAccessorFactory; import org.springframework.jdbc.core.test.ConcretePerson; import org.springframework.jdbc.core.test.ConstructorPerson; import org.springframework.jdbc.core.test.DatePerson; +import org.springframework.jdbc.core.test.EmailPerson; import org.springframework.jdbc.core.test.Person; import org.springframework.jdbc.core.test.SpacePerson; import org.springframework.jdbc.datasource.SingleConnectionDataSource; @@ -97,6 +98,14 @@ public abstract class AbstractRowMapperTests { assertThat(bw.getPropertyValue("balance")).isEqualTo(new BigDecimal("1234.56")); } + protected void verifyPerson(EmailPerson person) { + assertThat(person.getName()).isEqualTo("Bubba"); + assertThat(person.getAge()).isEqualTo(22L); + assertThat(person.getBirth_date()).usingComparator(Date::compareTo).isEqualTo(new java.util.Date(1221222L)); + assertThat(person.getBalance()).isEqualTo(new BigDecimal("1234.56")); + assertThat(person.getEMail()).isEqualTo("hello@world.info"); + } + protected enum MockType {ONE, TWO, THREE}; @@ -136,19 +145,22 @@ public abstract class AbstractRowMapperTests { given(resultSet.getDate(3)).willReturn(new java.sql.Date(1221222L)); given(resultSet.getBigDecimal(4)).willReturn(new BigDecimal("1234.56")); given(resultSet.getObject(4)).willReturn(new BigDecimal("1234.56")); + given(resultSet.getString(5)).willReturn("hello@world.info"); given(resultSet.wasNull()).willReturn(type == MockType.TWO); - given(resultSetMetaData.getColumnCount()).willReturn(4); + given(resultSetMetaData.getColumnCount()).willReturn(5); given(resultSetMetaData.getColumnLabel(1)).willReturn( type == MockType.THREE ? "Last Name" : "name"); given(resultSetMetaData.getColumnLabel(2)).willReturn("age"); given(resultSetMetaData.getColumnLabel(3)).willReturn("birth_date"); given(resultSetMetaData.getColumnLabel(4)).willReturn("balance"); + given(resultSetMetaData.getColumnLabel(5)).willReturn("e_mail"); given(resultSet.findColumn("name")).willReturn(1); given(resultSet.findColumn("age")).willReturn(2); given(resultSet.findColumn("birth_date")).willReturn(3); given(resultSet.findColumn("balance")).willReturn(4); + given(resultSet.findColumn("e_mail")).willReturn(5); jdbcTemplate = new JdbcTemplate(); jdbcTemplate.setDataSource(new SingleConnectionDataSource(connection, false)); diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/core/BeanPropertyRowMapperTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/core/BeanPropertyRowMapperTests.java index 6e1f84a632d..c61bab0a297 100644 --- a/spring-jdbc/src/test/java/org/springframework/jdbc/core/BeanPropertyRowMapperTests.java +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/core/BeanPropertyRowMapperTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2022 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. @@ -24,6 +24,7 @@ import org.springframework.beans.TypeMismatchException; import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.jdbc.core.test.ConcretePerson; import org.springframework.jdbc.core.test.DatePerson; +import org.springframework.jdbc.core.test.EmailPerson; import org.springframework.jdbc.core.test.ExtendedPerson; import org.springframework.jdbc.core.test.Person; import org.springframework.jdbc.core.test.SpacePerson; @@ -134,4 +135,15 @@ public class BeanPropertyRowMapperTests extends AbstractRowMapperTests { mock.verifyClosed(); } + @Test + public void testQueryWithUnderscoreAndPersonWithMultipleAdjacentUppercaseLettersInPropertyName() throws Exception { + Mock mock = new Mock(); + List result = mock.getJdbcTemplate().query( + "select name, age, birth_date, balance, e_mail from people", + new BeanPropertyRowMapper<>(EmailPerson.class)); + assertThat(result.size()).isEqualTo(1); + verifyPerson(result.get(0)); + mock.verifyClosed(); + } + } diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/core/test/EmailPerson.java b/spring-jdbc/src/test/java/org/springframework/jdbc/core/test/EmailPerson.java new file mode 100644 index 00000000000..f9c6996f7ab --- /dev/null +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/core/test/EmailPerson.java @@ -0,0 +1,78 @@ +/* + * Copyright 2002-2022 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.jdbc.core.test; + +import java.math.BigDecimal; +import java.util.Date; + +/** + * @author Thomas Risberg + * @author Marten Deinum + */ +public class EmailPerson { + + private String name; + + private long age; + + private Date birth_date; + + private BigDecimal balance; + + private String eMail; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public long getAge() { + return age; + } + + public void setAge(long age) { + this.age = age; + } + + public Date getBirth_date() { + return birth_date; + } + + public void setBirth_date(Date birth_date) { + this.birth_date = birth_date; + } + + public BigDecimal getBalance() { + return balance; + } + + public void setBalance(BigDecimal balance) { + this.balance = balance; + } + + public void setEMail(String email) { + this.eMail=email; + } + + public String getEMail() { + return this.eMail; + } + +} From 420ff46b2acfc91a945a12a1ae0ea11f61b43e3e Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Mon, 17 Jan 2022 16:46:00 +0100 Subject: [PATCH 2/3] Polishing --- .../jdbc/core/BeanPropertyRowMapperTests.java | 47 ++++++++++--------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/core/BeanPropertyRowMapperTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/core/BeanPropertyRowMapperTests.java index c61bab0a297..8cb5cd45d79 100644 --- a/spring-jdbc/src/test/java/org/springframework/jdbc/core/BeanPropertyRowMapperTests.java +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/core/BeanPropertyRowMapperTests.java @@ -31,74 +31,77 @@ import org.springframework.jdbc.core.test.SpacePerson; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatNoException; /** + * Tests for {@link BeanPropertyRowMapper}. + * * @author Thomas Risberg * @author Juergen Hoeller + * @author Sam Brannen */ -public class BeanPropertyRowMapperTests extends AbstractRowMapperTests { +class BeanPropertyRowMapperTests extends AbstractRowMapperTests { @Test @SuppressWarnings({"unchecked", "rawtypes"}) - public void testOverridingDifferentClassDefinedForMapping() { + void overridingDifferentClassDefinedForMapping() { BeanPropertyRowMapper mapper = new BeanPropertyRowMapper(Person.class); assertThatExceptionOfType(InvalidDataAccessApiUsageException.class).isThrownBy(() -> mapper.setMappedClass(Long.class)); } @Test - public void testOverridingSameClassDefinedForMapping() { + void overridingSameClassDefinedForMapping() { BeanPropertyRowMapper mapper = new BeanPropertyRowMapper<>(Person.class); - mapper.setMappedClass(Person.class); + assertThatNoException().isThrownBy(() -> mapper.setMappedClass(Person.class)); } @Test - public void testStaticQueryWithRowMapper() throws Exception { + void staticQueryWithRowMapper() throws Exception { Mock mock = new Mock(); List result = mock.getJdbcTemplate().query( "select name, age, birth_date, balance from people", new BeanPropertyRowMapper<>(Person.class)); - assertThat(result.size()).isEqualTo(1); + assertThat(result).hasSize(1); verifyPerson(result.get(0)); mock.verifyClosed(); } @Test - public void testMappingWithInheritance() throws Exception { + void mappingWithInheritance() throws Exception { Mock mock = new Mock(); List result = mock.getJdbcTemplate().query( "select name, age, birth_date, balance from people", new BeanPropertyRowMapper<>(ConcretePerson.class)); - assertThat(result.size()).isEqualTo(1); + assertThat(result).hasSize(1); verifyPerson(result.get(0)); mock.verifyClosed(); } @Test - public void testMappingWithNoUnpopulatedFieldsFound() throws Exception { + void mappingWithNoUnpopulatedFieldsFound() throws Exception { Mock mock = new Mock(); List result = mock.getJdbcTemplate().query( "select name, age, birth_date, balance from people", new BeanPropertyRowMapper<>(ConcretePerson.class, true)); - assertThat(result.size()).isEqualTo(1); + assertThat(result).hasSize(1); verifyPerson(result.get(0)); mock.verifyClosed(); } @Test - public void testMappingWithUnpopulatedFieldsNotChecked() throws Exception { + void mappingWithUnpopulatedFieldsNotChecked() throws Exception { Mock mock = new Mock(); List result = mock.getJdbcTemplate().query( "select name, age, birth_date, balance from people", new BeanPropertyRowMapper<>(ExtendedPerson.class)); - assertThat(result.size()).isEqualTo(1); - ExtendedPerson bean = result.get(0); - verifyPerson(bean); + assertThat(result).hasSize(1); + verifyPerson(result.get(0)); mock.verifyClosed(); } @Test - public void testMappingWithUnpopulatedFieldsNotAccepted() throws Exception { + void mappingWithUnpopulatedFieldsNotAccepted() throws Exception { Mock mock = new Mock(); assertThatExceptionOfType(InvalidDataAccessApiUsageException.class).isThrownBy(() -> mock.getJdbcTemplate().query("select name, age, birth_date, balance from people", @@ -106,7 +109,7 @@ public class BeanPropertyRowMapperTests extends AbstractRowMapperTests { } @Test - public void testMappingNullValue() throws Exception { + void mappingNullValue() throws Exception { BeanPropertyRowMapper mapper = new BeanPropertyRowMapper<>(Person.class); Mock mock = new Mock(MockType.TWO); assertThatExceptionOfType(TypeMismatchException.class).isThrownBy(() -> @@ -114,34 +117,34 @@ public class BeanPropertyRowMapperTests extends AbstractRowMapperTests { } @Test - public void testQueryWithSpaceInColumnNameAndLocalDateTime() throws Exception { + void queryWithSpaceInColumnNameAndLocalDateTime() throws Exception { Mock mock = new Mock(MockType.THREE); List result = mock.getJdbcTemplate().query( "select last_name as \"Last Name\", age, birth_date, balance from people", new BeanPropertyRowMapper<>(SpacePerson.class)); - assertThat(result.size()).isEqualTo(1); + assertThat(result).hasSize(1); verifyPerson(result.get(0)); mock.verifyClosed(); } @Test - public void testQueryWithSpaceInColumnNameAndLocalDate() throws Exception { + void queryWithSpaceInColumnNameAndLocalDate() throws Exception { Mock mock = new Mock(MockType.THREE); List result = mock.getJdbcTemplate().query( "select last_name as \"Last Name\", age, birth_date, balance from people", new BeanPropertyRowMapper<>(DatePerson.class)); - assertThat(result.size()).isEqualTo(1); + assertThat(result).hasSize(1); verifyPerson(result.get(0)); mock.verifyClosed(); } @Test - public void testQueryWithUnderscoreAndPersonWithMultipleAdjacentUppercaseLettersInPropertyName() throws Exception { + void queryWithUnderscoreInColumnNameAndPersonWithMultipleAdjacentUppercaseLettersInPropertyName() throws Exception { Mock mock = new Mock(); List result = mock.getJdbcTemplate().query( "select name, age, birth_date, balance, e_mail from people", new BeanPropertyRowMapper<>(EmailPerson.class)); - assertThat(result.size()).isEqualTo(1); + assertThat(result).hasSize(1); verifyPerson(result.get(0)); mock.verifyClosed(); } From 420c4f3df3809f5d394fd54a338ea0a1d91a14fe Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Mon, 17 Jan 2022 16:47:16 +0100 Subject: [PATCH 3/3] Explicitly test BeanPropertyRowMapper.underscoreName(String) See gh-27929 --- .../jdbc/core/BeanPropertyRowMapperTests.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/core/BeanPropertyRowMapperTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/core/BeanPropertyRowMapperTests.java index 8cb5cd45d79..99e9eb41627 100644 --- a/spring-jdbc/src/test/java/org/springframework/jdbc/core/BeanPropertyRowMapperTests.java +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/core/BeanPropertyRowMapperTests.java @@ -19,6 +19,8 @@ package org.springframework.jdbc.core; import java.util.List; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.springframework.beans.TypeMismatchException; import org.springframework.dao.InvalidDataAccessApiUsageException; @@ -149,4 +151,18 @@ class BeanPropertyRowMapperTests extends AbstractRowMapperTests { mock.verifyClosed(); } + @ParameterizedTest + @CsvSource({ + "age, age", + "lastName, last_name", + "Name, name", + "FirstName, first_name", + "EMail, e_mail", + "URL, u_r_l", // likely undesirable, but that's the status quo + }) + void underscoreName(String input, String expected) { + BeanPropertyRowMapper mapper = new BeanPropertyRowMapper<>(Object.class); + assertThat(mapper.underscoreName(input)).isEqualTo(expected); + } + }