From c4b9a03e4e32b020dd6294ffe05e6a6055decd33 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 19 Jul 2016 09:35:47 +0200 Subject: [PATCH] DATAJPA-929 - Further guards against null values returned from ManagedType. Extracted the functionality to check for JPA managed types into a dedicated wrapper for the JPA Metamodel. This allows all clients to benefit from the null guards we have to put in place due to [0]. Adapted the fix introduced for DATAJPA-904 to make use of the newly created infrastructure, too. [0] https://hibernate.atlassian.net/browse/HHH-10968 --- .../mapping/JpaPersistentPropertyImpl.java | 30 +----- .../repository/query/AbstractJpaQuery.java | 14 ++- .../query/AbstractStringBasedJpaQuery.java | 23 +---- .../data/jpa/util/JpaMetamodel.java | 93 +++++++++++++++++++ .../CrudMethodMetadataUnitTests.java | 3 + .../JpaQueryLookupStrategyUnitTests.java | 3 + .../repository/query/NamedQueryUnitTests.java | 4 + .../query/SimpleJpaQueryUnitTests.java | 5 +- .../JpaRepositoryFactoryUnitTests.java | 3 + 9 files changed, 130 insertions(+), 48 deletions(-) create mode 100644 src/main/java/org/springframework/data/jpa/util/JpaMetamodel.java diff --git a/src/main/java/org/springframework/data/jpa/mapping/JpaPersistentPropertyImpl.java b/src/main/java/org/springframework/data/jpa/mapping/JpaPersistentPropertyImpl.java index 358763ef0..55eb5fde7 100644 --- a/src/main/java/org/springframework/data/jpa/mapping/JpaPersistentPropertyImpl.java +++ b/src/main/java/org/springframework/data/jpa/mapping/JpaPersistentPropertyImpl.java @@ -37,11 +37,11 @@ import javax.persistence.OneToOne; import javax.persistence.OrderColumn; import javax.persistence.Transient; import javax.persistence.Version; -import javax.persistence.metamodel.ManagedType; import javax.persistence.metamodel.Metamodel; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.data.annotation.AccessType.Type; +import org.springframework.data.jpa.util.JpaMetamodel; import org.springframework.data.mapping.Association; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.model.AnnotationBasedPersistentProperty; @@ -91,7 +91,7 @@ class JpaPersistentPropertyImpl extends AnnotationBasedPersistentProperty associationTargetType; private final boolean updateable; - private final Set> managedTypes; + private final JpaMetamodel metamodel; /** * Creates a new {@link JpaPersistentPropertyImpl} @@ -112,7 +112,7 @@ class JpaPersistentPropertyImpl extends AnnotationBasedPersistentProperty> getManagedTypes(Metamodel metamodel) { - - Set> managedTypes = metamodel.getManagedTypes(); - Set> types = new HashSet>(managedTypes.size()); - - for (ManagedType managedType : metamodel.getManagedTypes()) { - - Class type = managedType.getJavaType(); - - if (type != null) { - types.add(type); - } - } - - return Collections.unmodifiableSet(types); - } } diff --git a/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java index b8ec62d3f..4b7c7b6ac 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2015 the original author or authors. + * Copyright 2008-2016 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. @@ -35,6 +35,7 @@ import org.springframework.data.jpa.repository.query.JpaQueryExecution.Procedure import org.springframework.data.jpa.repository.query.JpaQueryExecution.SingleEntityExecution; import org.springframework.data.jpa.repository.query.JpaQueryExecution.SlicedExecution; import org.springframework.data.jpa.repository.query.JpaQueryExecution.StreamExecution; +import org.springframework.data.jpa.util.JpaMetamodel; import org.springframework.data.repository.query.ParametersParameterAccessor; import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.data.repository.query.ResultProcessor; @@ -50,6 +51,7 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { private final JpaQueryMethod method; private final EntityManager em; + private final JpaMetamodel metamodel; /** * Creates a new {@link AbstractJpaQuery} from the given {@link JpaQueryMethod}. @@ -65,6 +67,7 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { this.method = method; this.em = em; + this.metamodel = new JpaMetamodel(em.getMetamodel()); } /* @@ -84,6 +87,15 @@ public abstract class AbstractJpaQuery implements RepositoryQuery { return em; } + /** + * Returns the {@link JpaMetamodel}. + * + * @return + */ + protected JpaMetamodel getMetamodel() { + return metamodel; + } + /* * (non-Javadoc) * @see org.springframework.data.repository.query.RepositoryQuery#execute(java.lang.Object[]) diff --git a/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java index 2972a3992..7a0392edb 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java @@ -18,7 +18,6 @@ package org.springframework.data.jpa.repository.query; import javax.persistence.EntityManager; import javax.persistence.Query; import javax.persistence.Tuple; -import javax.persistence.metamodel.ManagedType; import org.springframework.data.repository.query.EvaluationContextProvider; import org.springframework.data.repository.query.ParameterAccessor; @@ -138,25 +137,9 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery { ResultProcessor resultFactory = getQueryMethod().getResultProcessor(); ReturnedType returnedType = resultFactory.getReturnedType(); - return returnedType.isProjecting() && !isJpaManaged(returnedType.getReturnedType(), em) + getMetamodel().isJpaManaged(returnedType.getReturnedType()); + + return returnedType.isProjecting() && !getMetamodel().isJpaManaged(returnedType.getReturnedType()) ? em.createQuery(queryString, Tuple.class) : em.createQuery(queryString); } - - /** - * Returns whether the given type is managed by the given {@link EntityManager} - * @param type must not be {@literal null}. - * @param em must not be {@literal null}. - * - * @return - */ - private static boolean isJpaManaged(Class type, EntityManager em) { - - for (ManagedType managedType : em.getMetamodel().getManagedTypes()) { - if (managedType.getJavaType().equals(type)) { - return true; - } - } - - return false; - } } diff --git a/src/main/java/org/springframework/data/jpa/util/JpaMetamodel.java b/src/main/java/org/springframework/data/jpa/util/JpaMetamodel.java new file mode 100644 index 000000000..19dcb9182 --- /dev/null +++ b/src/main/java/org/springframework/data/jpa/util/JpaMetamodel.java @@ -0,0 +1,93 @@ +/* + * Copyright 2016 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 + * + * http://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.util; + +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +import javax.persistence.metamodel.ManagedType; +import javax.persistence.metamodel.Metamodel; + +import org.springframework.util.Assert; + +/** + * Wrapper around the JPA {@link Metamodel} to be able to apply some fixes against bugs in provider implementations. + * + * @author Oliver Gierke + */ +public class JpaMetamodel { + + private final Metamodel metamodel; + + private Collection> managedTypes; + + /** + * Creates a new {@link JpaMetamodel} for the given JPA {@link Metamodel}. + * + * @param metamodel must not be {@literal null}. + */ + public JpaMetamodel(Metamodel metamodel) { + + Assert.notNull(metamodel, "Metamodel must not be null!"); + + this.metamodel = metamodel; + } + + /** + * Returns whether the given type is managed by the backing JPA {@link Metamodel}. + * + * @param type must not be {@literal null}. + * @return + */ + public boolean isJpaManaged(Class type) { + + Assert.notNull(type, "Type must not be null!"); + + return getManagedTypes().contains(type); + } + + /** + * Returns all types managed by the backing {@link Metamodel}. Skips {@link ManagedType} instances that return + * {@literal null} for calls to {@link ManagedType#getJavaType()}. + * + * @return all managed types. + * @see https://hibernate.atlassian.net/browse/HHH-10968 + */ + private Collection> getManagedTypes() { + + if (managedTypes != null) { + return managedTypes; + } + + Set> managedTypes = metamodel.getManagedTypes(); + Set> types = new HashSet>(managedTypes.size()); + + for (ManagedType managedType : metamodel.getManagedTypes()) { + + Class type = managedType.getJavaType(); + + if (type != null) { + types.add(type); + } + } + + this.managedTypes = Collections.unmodifiableSet(types); + + return this.managedTypes; + } +} diff --git a/src/test/java/org/springframework/data/jpa/repository/CrudMethodMetadataUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/CrudMethodMetadataUnitTests.java index 7f34b6b4d..3302f930e 100644 --- a/src/test/java/org/springframework/data/jpa/repository/CrudMethodMetadataUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/CrudMethodMetadataUnitTests.java @@ -28,6 +28,7 @@ import javax.persistence.LockModeType; import javax.persistence.TypedQuery; import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.CriteriaQuery; +import javax.persistence.metamodel.Metamodel; import org.junit.Before; import org.junit.Test; @@ -56,6 +57,7 @@ public class CrudMethodMetadataUnitTests { @Mock JpaEntityInformation information; @Mock TypedQuery typedQuery; @Mock javax.persistence.Query query; + @Mock Metamodel metamodel; RoleRepository repository; @@ -64,6 +66,7 @@ public class CrudMethodMetadataUnitTests { when(information.getJavaType()).thenReturn(Role.class); + when(em.getMetamodel()).thenReturn(metamodel); when(em.getDelegate()).thenReturn(em); when(em.getEntityManagerFactory()).thenReturn(emf); when(emf.createEntityManager()).thenReturn(em); diff --git a/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java index 22ce45689..70e48a3cb 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java @@ -24,6 +24,7 @@ import java.lang.reflect.Method; import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; +import javax.persistence.metamodel.Metamodel; import org.junit.Before; import org.junit.Rule; @@ -61,6 +62,7 @@ public class JpaQueryLookupStrategyUnitTests { @Mock EntityManagerFactory emf; @Mock QueryExtractor extractor; @Mock NamedQueries namedQueries; + @Mock Metamodel metamodel; @Mock ProjectionFactory projectionFactory; public @Rule ExpectedException exception = ExpectedException.none(); @@ -68,6 +70,7 @@ public class JpaQueryLookupStrategyUnitTests { @Before public void setUp() { + when(em.getMetamodel()).thenReturn(metamodel); when(em.getEntityManagerFactory()).thenReturn(emf); when(emf.createEntityManager()).thenReturn(em); when(em.getDelegate()).thenReturn(em); diff --git a/src/test/java/org/springframework/data/jpa/repository/query/NamedQueryUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/NamedQueryUnitTests.java index ad371db40..68e9cbc93 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/NamedQueryUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/NamedQueryUnitTests.java @@ -22,6 +22,7 @@ import java.lang.reflect.Method; import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; +import javax.persistence.metamodel.Metamodel; import org.junit.Before; import org.junit.Test; @@ -49,6 +50,8 @@ public class NamedQueryUnitTests { @Mock QueryExtractor extractor; @Mock EntityManager em; @Mock EntityManagerFactory emf; + @Mock Metamodel metamodel; + ProjectionFactory projectionFactory = new SpelAwareProxyProjectionFactory(); Method method; @@ -61,6 +64,7 @@ public class NamedQueryUnitTests { when(metadata.getDomainType()).thenReturn((Class) String.class); when(metadata.getReturnedDomainClass(method)).thenReturn((Class) String.class); + when(em.getMetamodel()).thenReturn(metamodel); when(em.getEntityManagerFactory()).thenReturn(emf); when(emf.createEntityManager()).thenReturn(em); } diff --git a/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java index f84d67252..c7abc6f42 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2015 the original author or authors. + * Copyright 2008-2016 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. @@ -28,6 +28,7 @@ import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; import javax.persistence.Tuple; import javax.persistence.TypedQuery; +import javax.persistence.metamodel.Metamodel; import org.junit.Before; import org.junit.Rule; @@ -75,6 +76,7 @@ public class SimpleJpaQueryUnitTests { @Mock TypedQuery typedQuery; @Mock RepositoryMetadata metadata; @Mock ParameterBinder binder; + @Mock Metamodel metamodel; ProjectionFactory factory = new SpelAwareProxyProjectionFactory(); @@ -84,6 +86,7 @@ public class SimpleJpaQueryUnitTests { @SuppressWarnings({ "rawtypes", "unchecked" }) public void setUp() throws SecurityException, NoSuchMethodException { + when(em.getMetamodel()).thenReturn(metamodel); when(em.createQuery(anyString())).thenReturn(query); when(em.createQuery(anyString(), eq(Long.class))).thenReturn(typedQuery); when(em.getEntityManagerFactory()).thenReturn(emf); diff --git a/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryFactoryUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryFactoryUnitTests.java index bb76e1b58..4b327cf50 100644 --- a/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryFactoryUnitTests.java @@ -24,6 +24,7 @@ import java.io.Serializable; import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; +import javax.persistence.metamodel.Metamodel; import org.junit.Before; import org.junit.Test; @@ -55,12 +56,14 @@ public class JpaRepositoryFactoryUnitTests { JpaRepositoryFactory factory; @Mock EntityManager entityManager; + @Mock Metamodel metamodel; @Mock @SuppressWarnings("rawtypes") JpaEntityInformation entityInformation; @Mock EntityManagerFactory emf; @Before public void setUp() { + when(entityManager.getMetamodel()).thenReturn(metamodel); when(entityManager.getEntityManagerFactory()).thenReturn(emf); when(entityManager.getDelegate()).thenReturn(entityManager); when(emf.createEntityManager()).thenReturn(entityManager);