From a3e38f982e9d71f415b0edaadae31e3b4bdb4e1d Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Fri, 8 Sep 2017 09:44:57 +0200 Subject: [PATCH] DATAJPA-1172 - Added logging to make swallowed exception visible. It looks like we could actually get rid of catching the exception and just let it propagate. But we are not confident enough in this to take such a risk so late in the release life cycle just to get better error reporting. Original pull request: #217. --- ...elExpressionStringQueryParameterBinder.java | 16 ++++++++++++++-- ...clipseLinkNamespaceUserRepositoryTests.java | 18 ++++++++++++++++++ .../OpenJpaNamespaceUserRepositoryTests.java | 17 +++++++++++++++++ .../jpa/repository/UserRepositoryTests.java | 8 ++++++++ 4 files changed, 57 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/springframework/data/jpa/repository/query/SpelExpressionStringQueryParameterBinder.java b/src/main/java/org/springframework/data/jpa/repository/query/SpelExpressionStringQueryParameterBinder.java index 8f1560d73..69928c0f6 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/SpelExpressionStringQueryParameterBinder.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/SpelExpressionStringQueryParameterBinder.java @@ -19,6 +19,8 @@ import java.util.List; import javax.persistence.Query; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.data.jpa.repository.query.JpaParameters.JpaParameter; import org.springframework.data.jpa.repository.query.StringQuery.ParameterBinding; import org.springframework.data.repository.query.EvaluationContextProvider; @@ -32,9 +34,12 @@ import org.springframework.util.Assert; * A {@link StringQueryParameterBinder} that is able to bind synthetic query parameters. * * @author Thomas Darimont + * @author Jens Schauder */ class SpelExpressionStringQueryParameterBinder extends StringQueryParameterBinder { + private static final Logger LOGGER = LoggerFactory.getLogger(SpelExpressionStringQueryParameterBinder.class); + private final StringQuery query; private final EvaluationContextProvider evaluationContextProvider; private final SpelExpressionParser parser; @@ -96,8 +101,15 @@ class SpelExpressionStringQueryParameterBinder extends StringQueryParameterBinde } } catch (IllegalArgumentException iae) { - // Since Eclipse doesn't reliably report whether a query has parameters - // we simply try to set the parameters and ignore possible failures. + // DATAJPA-1172 we don't expect this exception any more. + // But aren't sure enough to just stop catching it. + LOGGER.warn( // + String.format( // + "Setting the parameter with name '%s' and position '%s' lead to an exception.", // + binding.getName(), // + binding.getPosition() // + ), // + iae); } } } diff --git a/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java index d86e36b87..dd0a81245 100644 --- a/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/EclipseLinkNamespaceUserRepositoryTests.java @@ -15,6 +15,12 @@ */ package org.springframework.data.jpa.repository; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + +import javax.persistence.Query; + +import org.junit.Test; import org.springframework.data.jpa.repository.sample.UserRepository; import org.springframework.test.context.ContextConfiguration; @@ -23,6 +29,7 @@ import org.springframework.test.context.ContextConfiguration; * * @author Oliver Gierke * @author Thomas Darimont + * @author Jens Schauder */ @ContextConfiguration(value = "classpath:eclipselink.xml") public class EclipseLinkNamespaceUserRepositoryTests extends NamespaceUserRepositoryTests { @@ -74,4 +81,15 @@ public class EclipseLinkNamespaceUserRepositoryTests extends NamespaceUserReposi */ @Override public void findByElementCollectionAttribute() {} + + /** + * This test will fail once https://bugs.eclipse.org/bugs/show_bug.cgi?id=521915 is fixed. + */ + @Override + @Test // DATAJPA-1172 + public void queryProvidesCorrectNumberOfParametersForNativeQuery() { + + Query query = em.createNativeQuery("select 1 from User where firstname=? and lastname=?"); + assertThat(query.getParameters().size(), equalTo(0)); + } } diff --git a/src/test/java/org/springframework/data/jpa/repository/OpenJpaNamespaceUserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/OpenJpaNamespaceUserRepositoryTests.java index 56974e9fc..d0413d8f0 100644 --- a/src/test/java/org/springframework/data/jpa/repository/OpenJpaNamespaceUserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/OpenJpaNamespaceUserRepositoryTests.java @@ -16,6 +16,8 @@ package org.springframework.data.jpa.repository; import static org.hamcrest.CoreMatchers.*; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.*; import java.util.Arrays; @@ -24,6 +26,7 @@ import java.util.List; import javax.persistence.EntityManager; import javax.persistence.PersistenceContext; +import javax.persistence.Query; import javax.persistence.TypedQuery; import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.CriteriaQuery; @@ -40,6 +43,7 @@ import org.springframework.test.context.ContextConfiguration; * Testcase to run {@link UserRepository} integration tests on top of OpenJPA. * * @author Oliver Gierke + * @author Jens Schauder */ @ContextConfiguration("classpath:openjpa.xml") public class OpenJpaNamespaceUserRepositoryTests extends NamespaceUserRepositoryTests { @@ -93,4 +97,17 @@ public class OpenJpaNamespaceUserRepositoryTests extends NamespaceUserRepository */ @Override public void shouldFindUsersInNativeQueryWithPagination() {} + + + /** + * OpenJpa doesn't provide the correct values in the version referenced in this branch. + * Since the problem is already gone in the version referenced in master no bug was created. + */ + @Override + @Test // DATAJPA-1172 + public void queryProvidesCorrectNumberOfParametersForNativeQuery() { + + Query query = em.createNativeQuery("select 1 from User where firstname=? and lastname=?"); + assertThat(query.getParameters().size(), equalTo(0)); + } } diff --git a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java index 08d0ce0b6..f252358dc 100644 --- a/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -88,6 +88,7 @@ import com.google.common.base.Optional; * @author Kevin Raymond * @author Thomas Darimont * @author Mark Paluch + * @author Jens Schauder */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration("classpath:application-context.xml") @@ -2137,6 +2138,13 @@ public class UserRepositoryTests { assertThat(result.getContent().get(0), is(thirdUser)); } + @Test // DATAJPA-1172 + public void queryProvidesCorrectNumberOfParametersForNativeQuery() { + + Query query = em.createNativeQuery("select 1 from User where firstname=? and lastname=?"); + assertThat(query.getParameters(),hasSize(2)); + } + private Page executeSpecWithSort(Sort sort) { flushTestUsers();