From e1c8671254e08a4aaa33b47b7ad36f2bef0e06a9 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Fri, 26 Oct 2018 16:30:18 +0200 Subject: [PATCH] DATAJPA-1446 - Clear JpaMetamodel cache when application context is closed. We now register a dedicated Spring Bean to wipe the static cache in JpaMetamodel to avoid memory from leaking in scenarios where ApplicationContexts are started and closed very often (e.g. integration tests). Heavily inspired by the work Sylvere Richard (@Nowheresly) has done in #301 but the key responsibility of wiping moved away from the MappingCOntext implementation. Original pull request #301. --- .../data/jpa/repository/Modifying.java | 6 ++- .../config/JpaRepositoryConfigExtension.java | 3 ++ .../data/jpa/util/JpaMetamodel.java | 12 ++++- .../jpa/util/JpaMetamodelCacheCleanup.java | 40 ++++++++++++++ ...MetamodelCacheCleanupIntegrationTests.java | 53 +++++++++++++++++++ .../data/jpa/util/JpaMetamodelUnitTests.java | 10 ++++ 6 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 src/main/java/org/springframework/data/jpa/util/JpaMetamodelCacheCleanup.java create mode 100644 src/test/java/org/springframework/data/jpa/util/JpaMetamodelCacheCleanupIntegrationTests.java diff --git a/src/main/java/org/springframework/data/jpa/repository/Modifying.java b/src/main/java/org/springframework/data/jpa/repository/Modifying.java index 48158a7d9..7d0770aa7 100644 --- a/src/main/java/org/springframework/data/jpa/repository/Modifying.java +++ b/src/main/java/org/springframework/data/jpa/repository/Modifying.java @@ -22,11 +22,15 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; /** - * Indicates a method should be regarded as modifying query. + * Indicates a query method should be considered as modifying query as that changes the way it needs to be executed. + * This annotation is only considered if used on actual query methods (either derived or manually defined through a + * {@link Query} annotation). It's not applied on custom implementation methods as they already have control over the + * underlying data access APIs. * * @author Oliver Gierke * @author Christoph Strobl * @author Nicolas Cirigliano + * @see Query */ @Retention(RetentionPolicy.RUNTIME) @Target({ ElementType.METHOD, ElementType.ANNOTATION_TYPE }) diff --git a/src/main/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtension.java b/src/main/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtension.java index 79128c49d..d7749d140 100644 --- a/src/main/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtension.java +++ b/src/main/java/org/springframework/data/jpa/repository/config/JpaRepositoryConfigExtension.java @@ -75,6 +75,7 @@ public class JpaRepositoryConfigExtension extends RepositoryConfigurationExtensi private static final Class PAB_POST_PROCESSOR = PersistenceAnnotationBeanPostProcessor.class; private static final String DEFAULT_TRANSACTION_MANAGER_BEAN_NAME = "transactionManager"; private static final String ENABLE_DEFAULT_TRANSACTIONS_ATTRIBUTE = "enableDefaultTransactions"; + private static final String JPA_METAMODEL_CACHE_CLEANUP_CLASSNAME = "org.springframework.data.jpa.util.JpaMetamodelCacheCleanup"; /* * (non-Javadoc) @@ -187,6 +188,8 @@ public class JpaRepositoryConfigExtension extends RepositoryConfigurationExtensi contextDefinition.setAutowireMode(AbstractBeanDefinition.AUTOWIRE_CONSTRUCTOR); registerIfNotAlreadyRegistered(contextDefinition, registry, JPA_CONTEXT_BEAN_NAME, source); + registerIfNotAlreadyRegistered(new RootBeanDefinition(JPA_METAMODEL_CACHE_CLEANUP_CLASSNAME), registry, + JPA_METAMODEL_CACHE_CLEANUP_CLASSNAME, source); } /* diff --git a/src/main/java/org/springframework/data/jpa/util/JpaMetamodel.java b/src/main/java/org/springframework/data/jpa/util/JpaMetamodel.java index e51d5e846..e551d2763 100644 --- a/src/main/java/org/springframework/data/jpa/util/JpaMetamodel.java +++ b/src/main/java/org/springframework/data/jpa/util/JpaMetamodel.java @@ -16,9 +16,9 @@ package org.springframework.data.jpa.util; import java.util.Collection; -import java.util.HashMap; import java.util.Map; import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; import javax.persistence.metamodel.EntityType; import javax.persistence.metamodel.ManagedType; @@ -34,10 +34,11 @@ import org.springframework.util.Assert; * * @author Oliver Gierke * @author Mark Paluch + * @author Sylvère Richard */ public class JpaMetamodel { - private static final Map CACHE = new HashMap<>(4); + private static final Map CACHE = new ConcurrentHashMap<>(4); private final Metamodel metamodel; @@ -95,6 +96,13 @@ public class JpaMetamodel { .orElse(false); } + /** + * Wipes the static cache of {@link Metamodel} to {@link JpaMetamodel}. + */ + static void clear() { + CACHE.clear(); + } + /** * Returns the {@link SingularAttribute} representing the identifier of the given {@link EntityType} if it contains a * singular one. diff --git a/src/main/java/org/springframework/data/jpa/util/JpaMetamodelCacheCleanup.java b/src/main/java/org/springframework/data/jpa/util/JpaMetamodelCacheCleanup.java new file mode 100644 index 000000000..5aa864fec --- /dev/null +++ b/src/main/java/org/springframework/data/jpa/util/JpaMetamodelCacheCleanup.java @@ -0,0 +1,40 @@ +/* + * Copyright 2018 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 org.springframework.beans.factory.DisposableBean; +import org.springframework.context.ApplicationContext; + +/** + * Simple component to be reigstered as Spring bean to clear the {@link JpaMetamodel} cache to avoid a memory leak in + * applications bootstrapping multiple {@link ApplicationContext}s. + * + * @author Oliver Gierke + * @author Sylvère Richard + * @see org.springframework.data.jpa.repository.config.JpaRepositoryConfigExtension#registerBeansForRoot(org.springframework.beans.factory.support.BeanDefinitionRegistry, + * org.springframework.data.repository.config.RepositoryConfigurationSource) + */ +class JpaMetamodelCacheCleanup implements DisposableBean { + + /* + * (non-Javadoc) + * @see org.springframework.beans.factory.DisposableBean#destroy() + */ + @Override + public void destroy() throws Exception { + JpaMetamodel.clear(); + } +} diff --git a/src/test/java/org/springframework/data/jpa/util/JpaMetamodelCacheCleanupIntegrationTests.java b/src/test/java/org/springframework/data/jpa/util/JpaMetamodelCacheCleanupIntegrationTests.java new file mode 100644 index 000000000..72f848ecc --- /dev/null +++ b/src/test/java/org/springframework/data/jpa/util/JpaMetamodelCacheCleanupIntegrationTests.java @@ -0,0 +1,53 @@ +/* + * Copyright 2018 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 static org.assertj.core.api.Assertions.*; + +import javax.persistence.metamodel.Metamodel; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.context.support.GenericApplicationContext; + +/** + * Integration tests for {@link JpaMetamodelCacheCleanup}. + * + * @author Oliver Gierke + */ +@RunWith(MockitoJUnitRunner.class) +public class JpaMetamodelCacheCleanupIntegrationTests { + + @Mock Metamodel metamodel; + + @Test // DATAJPA-1446 + public void wipesJpaMetamodelCacheOnApplicationContextClose() { + + JpaMetamodel model = JpaMetamodel.of(metamodel); + + try (GenericApplicationContext context = new GenericApplicationContext()) { + + context.registerBean(JpaMetamodelCacheCleanup.class); + context.refresh(); + + assertThat(model).isSameAs(JpaMetamodel.of(metamodel)); + } + + assertThat(model).isNotSameAs(JpaMetamodel.of(metamodel)); + } +} diff --git a/src/test/java/org/springframework/data/jpa/util/JpaMetamodelUnitTests.java b/src/test/java/org/springframework/data/jpa/util/JpaMetamodelUnitTests.java index 36ae80cf9..e051825d8 100644 --- a/src/test/java/org/springframework/data/jpa/util/JpaMetamodelUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/util/JpaMetamodelUnitTests.java @@ -47,4 +47,14 @@ public class JpaMetamodelUnitTests { assertThat(JpaMetamodel.of(metamodel).isSingleIdAttribute(Object.class, "id", Object.class)).isFalse(); } + + @Test // DATAJPA-1446 + public void cacheIsEffectiveUnlessCleared() { + + JpaMetamodel model = JpaMetamodel.of(metamodel); + assertThat(model).isEqualTo(JpaMetamodel.of(metamodel)); + + JpaMetamodel.clear(); + assertThat(model).isNotEqualTo(JpaMetamodel.of(metamodel)); + } }