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.

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);
 				}
 			}
 		}

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));
+	}
 }

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));
+	}
 }

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<User> executeSpecWithSort(Sort sort) {
 
 		flushTestUsers();