diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetter.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetter.java index e3c40f18a..700a6fd4b 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetter.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetter.java @@ -15,7 +15,12 @@ */ package org.springframework.data.jpa.repository.query; -import static org.springframework.data.jpa.repository.query.QueryParameterSetter.ErrorHandling.*; +import static org.springframework.data.jpa.repository.query.QueryParameterSetter.ErrorHandling.LENIENT; + +import jakarta.persistence.Parameter; +import jakarta.persistence.Query; +import jakarta.persistence.TemporalType; +import jakarta.persistence.criteria.ParameterExpression; import java.lang.reflect.Proxy; import java.util.Collections; @@ -25,13 +30,9 @@ import java.util.Map; import java.util.Set; import java.util.function.Function; -import jakarta.persistence.Parameter; -import jakarta.persistence.Query; -import jakarta.persistence.TemporalType; -import jakarta.persistence.criteria.ParameterExpression; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.hibernate.query.TypedParameterValue; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -79,19 +80,26 @@ interface QueryParameterSetter { public void setParameter(BindableQuery query, JpaParametersParameterAccessor accessor, ErrorHandling errorHandling) { - Object value = valueExtractor.apply(accessor); - if (temporalType != null) { + Object extractedValue = valueExtractor.apply(accessor); + + final Date value; + if (extractedValue instanceof TypedParameterValue) { + value = (Date) ((TypedParameterValue) extractedValue).getValue(); + } else { + value = (Date) extractedValue; + } + // One would think we can simply use parameter to identify the parameter we want to set. // But that does not work with list valued parameters. At least Hibernate tries to bind them by name. // TODO: move to using setParameter(Parameter, value) when https://hibernate.atlassian.net/browse/HHH-11870 is // fixed. if (parameter instanceof ParameterExpression) { - errorHandling.execute(() -> query.setParameter((Parameter) parameter, (Date) value, temporalType)); + errorHandling.execute(() -> query.setParameter((Parameter) parameter, value, temporalType)); } else if (query.hasNamedParameters() && parameter.getName() != null) { - errorHandling.execute(() -> query.setParameter(parameter.getName(), (Date) value, temporalType)); + errorHandling.execute(() -> query.setParameter(parameter.getName(), value, temporalType)); } else { Integer position = parameter.getPosition(); @@ -101,12 +109,14 @@ interface QueryParameterSetter { || query.registerExcessParameters() // || errorHandling == LENIENT)) { - errorHandling.execute(() -> query.setParameter(parameter.getPosition(), (Date) value, temporalType)); + errorHandling.execute(() -> query.setParameter(parameter.getPosition(), value, temporalType)); } } } else { + final Object value = valueExtractor.apply(accessor); + if (parameter instanceof ParameterExpression) { errorHandling.execute(() -> query.setParameter((Parameter) parameter, value)); } else if (query.hasNamedParameters() && parameter.getName() != null) { diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/MySqlStoredProcedureIntegrationTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/MySqlStoredProcedureIntegrationTests.java index 4112d1825..aa011c432 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/MySqlStoredProcedureIntegrationTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/MySqlStoredProcedureIntegrationTests.java @@ -16,8 +16,13 @@ package org.springframework.data.jpa.repository.procedures; -import static org.assertj.core.api.Assertions.*; +import static org.assertj.core.api.Assertions.assertThat; +import jakarta.persistence.Entity; +import jakarta.persistence.EntityManagerFactory; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; +import jakarta.persistence.NamedStoredProcedureQuery; import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; @@ -25,13 +30,7 @@ import lombok.NoArgsConstructor; import java.util.List; import java.util.Properties; -import jakarta.persistence.Entity; -import jakarta.persistence.EntityManagerFactory; -import jakarta.persistence.GeneratedValue; -import jakarta.persistence.Id; -import jakarta.persistence.NamedStoredProcedureQuery; import javax.sql.DataSource; -import jakarta.transaction.Transactional; import org.hibernate.dialect.MySQL8Dialect; import org.junit.jupiter.api.Test; @@ -54,6 +53,7 @@ import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.annotation.EnableTransactionManagement; +import org.springframework.transaction.annotation.Transactional; import org.testcontainers.containers.MySQLContainer; import com.mysql.cj.jdbc.MysqlDataSource; @@ -154,7 +154,8 @@ public class MySqlStoredProcedureIntegrationTests { resultClasses = Employee.class) public static class Employee { - @Id @GeneratedValue private Integer id; + @Id + @GeneratedValue private Integer id; private String name; } @@ -194,7 +195,7 @@ public class MySqlStoredProcedureIntegrationTests { static class Config { @SuppressWarnings("resource") - @Bean(initMethod = "start") + @Bean(initMethod = "start", destroyMethod = "stop") public MySQLContainer container() { return new MySQLContainer<>("mysql:8.0.24") // diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureIntegrationTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureIntegrationTests.java index 5243cfce5..cbd325a2e 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureIntegrationTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureIntegrationTests.java @@ -16,15 +16,7 @@ package org.springframework.data.jpa.repository.procedures; -import static org.assertj.core.api.Assertions.*; - -import lombok.AllArgsConstructor; -import lombok.Data; -import lombok.NoArgsConstructor; - -import java.math.BigDecimal; -import java.util.List; -import java.util.Properties; +import static org.assertj.core.api.Assertions.assertThat; import jakarta.persistence.Entity; import jakarta.persistence.EntityManagerFactory; @@ -33,8 +25,15 @@ import jakarta.persistence.Id; import jakarta.persistence.NamedStoredProcedureQuery; import jakarta.persistence.ParameterMode; import jakarta.persistence.StoredProcedureParameter; +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; + +import java.math.BigDecimal; +import java.util.List; +import java.util.Properties; + import javax.sql.DataSource; -import jakarta.transaction.Transactional; import org.hibernate.dialect.PostgreSQL91Dialect; import org.junit.jupiter.api.Test; @@ -58,6 +57,7 @@ import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.annotation.EnableTransactionManagement; +import org.springframework.transaction.annotation.Transactional; import org.testcontainers.containers.PostgreSQLContainer; /** @@ -159,7 +159,8 @@ public class PostgresStoredProcedureIntegrationTests { resultClasses = Employee.class) public static class Employee { - @Id @GeneratedValue private Integer id; + @Id + @GeneratedValue private Integer id; private String name; } @@ -197,11 +198,11 @@ public class PostgresStoredProcedureIntegrationTests { static class Config { @SuppressWarnings("resource") - @Bean(initMethod = "start") + @Bean(initMethod = "start", destroyMethod = "stop") public PostgreSQLContainer container() { return new PostgreSQLContainer<>("postgres:9.6.12") // - .withUsername("postgres"); + .withUsername("postgres"); } @Bean diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureNullHandlingIntegrationTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureNullHandlingIntegrationTests.java new file mode 100644 index 000000000..3b7ea6e04 --- /dev/null +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureNullHandlingIntegrationTests.java @@ -0,0 +1,166 @@ +/* + * Copyright 2015-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.data.jpa.repository.procedures; + +import jakarta.persistence.Entity; +import jakarta.persistence.EntityManagerFactory; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import lombok.AccessLevel; +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; + +import java.util.Date; +import java.util.Properties; +import java.util.UUID; + +import javax.sql.DataSource; + +import org.hibernate.dialect.PostgreSQL91Dialect; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.postgresql.ds.PGSimpleDataSource; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.ComponentScan; +import org.springframework.context.annotation.FilterType; +import org.springframework.core.io.ClassPathResource; +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Temporal; +import org.springframework.data.jpa.repository.config.EnableJpaRepositories; +import org.springframework.data.jpa.repository.query.Procedure; +import org.springframework.jdbc.datasource.init.DataSourceInitializer; +import org.springframework.jdbc.datasource.init.ResourceDatabasePopulator; +import org.springframework.orm.jpa.AbstractEntityManagerFactoryBean; +import org.springframework.orm.jpa.JpaTransactionManager; +import org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean; +import org.springframework.orm.jpa.vendor.HibernateJpaVendorAdapter; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.annotation.EnableTransactionManagement; +import org.springframework.transaction.annotation.Transactional; +import org.testcontainers.containers.PostgreSQLContainer; + +/** + * Testcase to verify {@link org.springframework.jdbc.object.StoredProcedure}s properly handle null values. + * + * @author Greg Turnquist + */ +@Transactional +@ExtendWith(SpringExtension.class) +@ContextConfiguration(classes = PostgresStoredProcedureNullHandlingIntegrationTests.Config.class) +public class PostgresStoredProcedureNullHandlingIntegrationTests { + + @Autowired TestModelRepository repository; + + @Test // 2544 + void invokingNullOnNonTemporalStoredProcedureParameterShouldWork() { + repository.countUuid(null); + } + + @Test // 2544 + void invokingNullOnTemporalStoredProcedureParameterShouldWork() { + repository.countLocalDate(null); + } + + @Data + @AllArgsConstructor + @NoArgsConstructor(access = AccessLevel.PROTECTED) + @Entity + public class TestModel { + + @Id + @GeneratedValue(strategy = GenerationType.AUTO) private long id; + private UUID uuid; + private Date date; + } + + @Transactional + public interface TestModelRepository extends JpaRepository { + + @Procedure("countByUuid") + void countUuid(UUID this_uuid); + + @Procedure("countByLocalDate") + void countLocalDate(@Temporal Date localDate); + } + + @EnableJpaRepositories(considerNestedRepositories = true, + includeFilters = @ComponentScan.Filter(type = FilterType.ASSIGNABLE_TYPE, classes = TestModelRepository.class)) + @EnableTransactionManagement + static class Config { + + @Bean(initMethod = "start", destroyMethod = "stop") + public PostgreSQLContainer container() { + + return new PostgreSQLContainer<>("postgres:9.6.12") // + .withUsername("postgres"); + } + + @Bean + public DataSource dataSource(PostgreSQLContainer container) { + + PGSimpleDataSource dataSource = new PGSimpleDataSource(); + dataSource.setUrl(container.getJdbcUrl()); + dataSource.setUser(container.getUsername()); + dataSource.setPassword(container.getPassword()); + + return dataSource; + } + + @Bean + public AbstractEntityManagerFactoryBean entityManagerFactory(DataSource dataSource) { + + LocalContainerEntityManagerFactoryBean factoryBean = new LocalContainerEntityManagerFactoryBean(); + factoryBean.setDataSource(dataSource); + factoryBean.setPersistenceUnitRootLocation("simple-persistence"); + factoryBean.setJpaVendorAdapter(new HibernateJpaVendorAdapter()); + factoryBean.setPackagesToScan(this.getClass().getPackage().getName()); + + Properties properties = new Properties(); + properties.setProperty("hibernate.hbm2ddl.auto", "create"); + properties.setProperty("hibernate.dialect", PostgreSQL91Dialect.class.getCanonicalName()); + properties.setProperty("hibernate.proc.param_null_passing", "true"); + properties.setProperty("hibernate.globally_quoted_identifiers", "true"); + properties.setProperty("hibernate.globally_quoted_identifiers_skip_column_definitions", "true"); + factoryBean.setJpaProperties(properties); + + return factoryBean; + } + + @Bean + PlatformTransactionManager transactionManager(EntityManagerFactory entityManagerFactory) { + return new JpaTransactionManager(entityManagerFactory); + } + + @Bean + DataSourceInitializer initializer(DataSource dataSource) { + + DataSourceInitializer initializer = new DataSourceInitializer(); + initializer.setDataSource(dataSource); + + ClassPathResource script = new ClassPathResource("scripts/postgres-nullable-stored-procedures.sql"); + ResourceDatabasePopulator populator = new ResourceDatabasePopulator(script); + populator.setSeparator(";;"); + initializer.setDatabasePopulator(populator); + + return initializer; + } + } +} diff --git a/spring-data-jpa/src/test/resources/scripts/postgres-nullable-stored-procedures.sql b/spring-data-jpa/src/test/resources/scripts/postgres-nullable-stored-procedures.sql new file mode 100644 index 000000000..03a619176 --- /dev/null +++ b/spring-data-jpa/src/test/resources/scripts/postgres-nullable-stored-procedures.sql @@ -0,0 +1,41 @@ +CREATE TABLE test_model +( + ID numeric NOT NULL, + uuid UUID, + local_date DATE, + CONSTRAINT test_model_pk PRIMARY KEY (ID) +);; + +CREATE OR REPLACE FUNCTION countByUuid(this_uuid uuid) + RETURNS int + LANGUAGE 'plpgsql' +AS +$BODY$ +DECLARE + c integer; +BEGIN + SELECT count(*) + INTO c + FROM test_model + WHERE test_model.uuid = this_uuid; + RETURN c; +END; +$BODY$ +;; + +CREATE OR REPLACE FUNCTION countByLocalDate(this_local_date DATE) + RETURNS int + LANGUAGE 'plpgsql' +AS +$BODY$ +DECLARE + c integer; +BEGIN + SELECT count(*) + INTO c + FROM test_model + WHERE test_model.local_date = this_local_date; + RETURN c; +END; +$BODY$ +;;