From 1145382fd2b833a5b5dad124c8f66395a95e99f4 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 | 8 ++- .../config/JpaRepositoryConfigExtension.java | 4 ++ .../data/jpa/util/JpaMetamodel.java | 12 ++++- .../jpa/util/JpaMetamodelCacheCleanup.java | 40 ++++++++++++++ ...MetamodelCacheCleanupIntegrationTests.java | 53 +++++++++++++++++++ .../data/jpa/util/JpaMetamodelUnitTests.java | 10 ++++ 6 files changed, 123 insertions(+), 4 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 d2dfebc1e..42628a441 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 eff006672..4593bbc6d 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) @@ -192,6 +193,9 @@ public class JpaRepositoryConfigExtension extends RepositoryConfigurationExtensi return contextDefinition; }, registry, JPA_CONTEXT_BEAN_NAME, source); + + registerLazyIfNotAlreadyRegistered(() -> 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 d64f6ae53..fe14caf56 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)); + } }