DATAJPA-617 - Avoid potential transaction rollbacks during named query lookups.

We now use a dedicated EntityManager for the named query lookup in NamedQuery.hasNamedQuery(…). Previously we used the given EntityManager to lookup the named query which will cause the transaction to be rolled back due to the Exception thrown in the case that the named query couldn't be found.

Added fast path to NamedQuery.lookupFrom(…) that now uses NamedQuery.hasNamedQuery(…) to test if a query exists or not. Pulled the check for the presence of a named count query from doCreateCountQuery(…) up to the NamedCountQuery constructor. This makes sure that the check is only performed once.

Original pull request: #110.
This commit is contained in:
Thomas Darimont
2014-10-23 14:58:36 +02:00
committed by Oliver Gierke
parent bacf6cce26
commit c585ed3c3b
4 changed files with 43 additions and 16 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2008-2013 the original author or authors.
* Copyright 2008-2014 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.
@@ -29,6 +29,7 @@ import org.springframework.data.repository.query.RepositoryQuery;
* Implementation of {@link RepositoryQuery} based on {@link javax.persistence.NamedQuery}s.
*
* @author Oliver Gierke
* @author Thomas Darimont
*/
final class NamedQuery extends AbstractJpaQuery {
@@ -43,6 +44,7 @@ final class NamedQuery extends AbstractJpaQuery {
private final String countQueryName;
private final String countProjection;
private final QueryExtractor extractor;
private final boolean namedCountQueryIsPresent;
/**
* Creates a new {@link NamedQuery}.
@@ -58,16 +60,14 @@ final class NamedQuery extends AbstractJpaQuery {
Parameters<?, ?> parameters = method.getParameters();
// Let's see if the referenced named query exists
em.createNamedQuery(queryName);
if (parameters.hasSortParameter()) {
throw new IllegalStateException(String.format("Finder method %s is backed " + "by a NamedQuery and must "
+ "not contain a sort parameter as we cannot modify the query! Use @Query instead!", method));
}
boolean weNeedToCreateCountQuery = !hasNamedQuery(em, countQueryName)
&& method.getParameters().hasPageableParameter();
this.namedCountQueryIsPresent = hasNamedQuery(em, countQueryName);
boolean weNeedToCreateCountQuery = !namedCountQueryIsPresent && method.getParameters().hasPageableParameter();
boolean cantExtractQuery = !this.extractor.canExtractQuery();
if (weNeedToCreateCountQuery && cantExtractQuery) {
@@ -88,12 +88,20 @@ final class NamedQuery extends AbstractJpaQuery {
*/
private static boolean hasNamedQuery(EntityManager em, String queryName) {
/*
* @see DATAJPA-617
* we have to use a dedicated em for the lookups to avoid a potential rollback of the running tx.
*/
EntityManager lookupEm = em.getEntityManagerFactory().createEntityManager();
try {
em.createNamedQuery(queryName);
lookupEm.createNamedQuery(queryName);
return true;
} catch (IllegalArgumentException e) {
LOG.debug("Did not find named query {}", queryName);
return false;
} finally {
lookupEm.close();
}
}
@@ -109,6 +117,10 @@ final class NamedQuery extends AbstractJpaQuery {
LOG.debug("Looking up named query {}", queryName);
if (!hasNamedQuery(em, queryName)) {
return null;
}
try {
RepositoryQuery query = new NamedQuery(method, em);
LOG.debug("Found named query {}!", queryName);
@@ -139,7 +151,7 @@ final class NamedQuery extends AbstractJpaQuery {
EntityManager em = getEntityManager();
TypedQuery<Long> countQuery = null;
if (hasNamedQuery(em, countQueryName)) {
if (namedCountQueryIsPresent) {
countQuery = em.createNamedQuery(countQueryName, Long.class);
} else {
Query query = createQuery(values);

View File

@@ -23,6 +23,7 @@ import java.util.Collections;
import java.util.Map;
import javax.persistence.EntityManager;
import javax.persistence.EntityManagerFactory;
import javax.persistence.LockModeType;
import javax.persistence.TypedQuery;
import javax.persistence.criteria.CriteriaBuilder;
@@ -44,11 +45,13 @@ import org.springframework.data.jpa.repository.support.JpaRepositoryFactory;
* Integratio test for lock support.
*
* @author Oliver Gierke
* @author Thomas Darimont
*/
@RunWith(MockitoJUnitRunner.class)
public class CrudMethodMetadataIntegrationTests {
@Mock EntityManager em;
@Mock EntityManagerFactory emf;
@Mock CriteriaBuilder builder;
@Mock CriteriaQuery<Role> criteriaQuery;
@Mock JpaEntityInformation<Role, Integer> information;
@@ -62,6 +65,9 @@ public class CrudMethodMetadataIntegrationTests {
when(information.getJavaType()).thenReturn(Role.class);
when(em.getEntityManagerFactory()).thenReturn(emf);
when(emf.createEntityManager()).thenReturn(em);
JpaRepositoryFactory factory = new JpaRepositoryFactory(em) {
@Override
@SuppressWarnings("unchecked")

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2008-2011 the original author or authors.
* Copyright 2008-2014 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.
@@ -21,6 +21,7 @@ import static org.mockito.Mockito.*;
import java.lang.reflect.Method;
import javax.persistence.EntityManager;
import javax.persistence.EntityManagerFactory;
import org.junit.Before;
import org.junit.Test;
@@ -36,16 +37,15 @@ import org.springframework.data.repository.query.QueryCreationException;
* Unit tests for {@link NamedQuery}.
*
* @author Oliver Gierke
* @author Thomas Darimont
*/
@RunWith(MockitoJUnitRunner.class)
public class NamedQueryUnitTests {
@Mock
RepositoryMetadata metadata;
@Mock
QueryExtractor extractor;
@Mock
EntityManager em;
@Mock RepositoryMetadata metadata;
@Mock QueryExtractor extractor;
@Mock EntityManager em;
@Mock EntityManagerFactory emf;
Method method;
@@ -56,6 +56,9 @@ public class NamedQueryUnitTests {
method = SampleRepository.class.getMethod("foo", Pageable.class);
when(metadata.getDomainType()).thenReturn((Class) String.class);
when(metadata.getReturnedDomainClass(method)).thenReturn((Class) String.class);
when(em.getEntityManagerFactory()).thenReturn(emf);
when(emf.createEntityManager()).thenReturn(em);
}
@Test(expected = QueryCreationException.class)

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2008-2011 the original author or authors.
* Copyright 2008-2014 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.
@@ -23,6 +23,7 @@ import java.io.IOException;
import java.io.Serializable;
import javax.persistence.EntityManager;
import javax.persistence.EntityManagerFactory;
import org.junit.Before;
import org.junit.Test;
@@ -43,6 +44,7 @@ import org.springframework.transaction.annotation.Transactional;
* Unit test for {@code JpaRepositoryFactory}.
*
* @author Oliver Gierke
* @author Thomas Darimont
*/
@RunWith(MockitoJUnitRunner.class)
public class JpaRepositoryFactoryUnitTests {
@@ -51,6 +53,7 @@ public class JpaRepositoryFactoryUnitTests {
@Mock EntityManager entityManager;
@Mock @SuppressWarnings("rawtypes") JpaEntityInformation metadata;
@Mock EntityManagerFactory emf;
@Before
public void setUp() {
@@ -67,6 +70,9 @@ public class JpaRepositoryFactoryUnitTests {
};
factory.setQueryLookupStrategyKey(Key.CREATE_IF_NOT_FOUND);
when(entityManager.getEntityManagerFactory()).thenReturn(emf);
when(emf.createEntityManager()).thenReturn(entityManager);
}
/**