From d64cfd15ceb8ba5655149966436f52cd0226f84c Mon Sep 17 00:00:00 2001 From: "Greg L. Turnquist" Date: Mon, 19 Dec 2022 11:00:53 -0600 Subject: [PATCH] Handle nulls in stored procedure parameters properly. Hibernate supports TypedParameterValue in queries, but not stored procedures. Until https://github.com/hibernate/hibernate-orm/pull/5438 is adopted, we have to dereference such parameters on our end first. Closes #2544. --- .../query/QueryParameterSetter.java | 20 ++- .../MySqlStoredProcedureIntegrationTests.java | 4 +- ...stgresStoredProcedureIntegrationTests.java | 4 +- ...ProcedureNullHandlingIntegrationTests.java | 166 ++++++++++++++++++ .../JpaParametersParameterAccessorTests.java | 13 +- .../postgres-nullable-stored-procedures.sql | 41 +++++ 6 files changed, 237 insertions(+), 11 deletions(-) create mode 100644 src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureNullHandlingIntegrationTests.java create mode 100644 src/test/resources/scripts/postgres-nullable-stored-procedures.sql diff --git a/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetter.java b/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetter.java index 1f12c6fa1..7c0d58746 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetter.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/QueryParameterSetter.java @@ -15,7 +15,7 @@ */ 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 java.lang.reflect.Proxy; import java.util.Collections; @@ -27,11 +27,13 @@ import java.util.function.Function; import javax.persistence.Parameter; import javax.persistence.Query; +import javax.persistence.StoredProcedureQuery; import javax.persistence.TemporalType; import javax.persistence.criteria.ParameterExpression; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.hibernate.jpa.TypedParameterValue; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -83,7 +85,21 @@ interface QueryParameterSetter { public void setParameter(BindableQuery query, JpaParametersParameterAccessor accessor, ErrorHandling errorHandling) { - Object value = valueExtractor.apply(accessor); + final Object value; + + // TODO: When https://github.com/hibernate/hibernate-orm/pull/5438 is merged we should be able to drop this. + if (query.getQuery() instanceof StoredProcedureQuery) { + + Object extractedValue = valueExtractor.apply(accessor); + + if (extractedValue instanceof TypedParameterValue) { + value = ((TypedParameterValue) extractedValue).getValue(); + } else { + value = extractedValue; + } + } else { + value = valueExtractor.apply(accessor); + } if (temporalType != null) { diff --git a/src/test/java/org/springframework/data/jpa/repository/procedures/MySqlStoredProcedureIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/procedures/MySqlStoredProcedureIntegrationTests.java index e295a9a87..f21848b69 100644 --- a/src/test/java/org/springframework/data/jpa/repository/procedures/MySqlStoredProcedureIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/procedures/MySqlStoredProcedureIntegrationTests.java @@ -31,7 +31,6 @@ import javax.persistence.GeneratedValue; import javax.persistence.Id; import javax.persistence.NamedStoredProcedureQuery; import javax.sql.DataSource; -import javax.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; @@ -194,7 +194,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/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureIntegrationTests.java index eb55a222b..3e6701fe4 100644 --- a/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureIntegrationTests.java @@ -34,7 +34,6 @@ import javax.persistence.NamedStoredProcedureQuery; import javax.persistence.ParameterMode; import javax.persistence.StoredProcedureParameter; import javax.sql.DataSource; -import javax.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; /** @@ -197,7 +197,7 @@ 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") // diff --git a/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureNullHandlingIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureNullHandlingIntegrationTests.java new file mode 100644 index 000000000..49a60ccfc --- /dev/null +++ b/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 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.persistence.Entity; +import javax.persistence.EntityManagerFactory; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +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/src/test/java/org/springframework/data/jpa/repository/query/JpaParametersParameterAccessorTests.java b/src/test/java/org/springframework/data/jpa/repository/query/JpaParametersParameterAccessorTests.java index 17ea638fe..79e3ab578 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/JpaParametersParameterAccessorTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/JpaParametersParameterAccessorTests.java @@ -1,8 +1,10 @@ package org.springframework.data.jpa.repository.query; -import static org.assertj.core.api.Assertions.*; -import static org.mockito.ArgumentMatchers.*; -import static org.mockito.Mockito.*; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import java.lang.reflect.Method; @@ -44,7 +46,8 @@ class JpaParametersParameterAccessorTests { Method withNativeQuery = SampleRepository.class.getMethod("withNativeQuery", Integer.class); Object[] values = { null }; JpaParameters parameters = new JpaParameters(withNativeQuery); - JpaParametersParameterAccessor accessor = PersistenceProvider.GENERIC_JPA.getParameterAccessor(parameters, values, em); + JpaParametersParameterAccessor accessor = PersistenceProvider.GENERIC_JPA.getParameterAccessor(parameters, values, + em); bind(parameters, accessor); @@ -71,7 +74,7 @@ class JpaParametersParameterAccessorTests { private void bind(JpaParameters parameters, JpaParametersParameterAccessor accessor) { - ParameterBinderFactory.createBinder(parameters) + ParameterBinderFactory.createBinder(parameters) // .bind( // QueryParameterSetter.BindableQuery.from(query), // accessor, // diff --git a/src/test/resources/scripts/postgres-nullable-stored-procedures.sql b/src/test/resources/scripts/postgres-nullable-stored-procedures.sql new file mode 100644 index 000000000..03a619176 --- /dev/null +++ b/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$ +;;