From ab40236949f9c2dcf86b0456fc1236abe3af552a Mon Sep 17 00:00:00 2001 From: Ariel Morelli Andres Date: Thu, 22 May 2025 21:22:05 -0700 Subject: [PATCH] Prevent access to `EntityManager` when looking up `PersistenceProvider`. Signed-off-by: Ariel Morelli Andres Closes: #3425 Original pull request: #3885 --- .../jpa/provider/PersistenceProvider.java | 5 +- .../repository/query/AbstractJpaQuery.java | 1 - .../data/jpa/repository/support/Querydsl.java | 4 +- .../CrudMethodMetadataUnitTests.java | 6 +- ...ernateCurrentTenantIdentifierResolver.java | 49 +++++++++++ .../HibernateMultitenancyTests.java | 88 +++++++++++++++++++ .../AbstractStringBasedJpaQueryUnitTests.java | 4 + .../repository/query/NamedQueryUnitTests.java | 1 - .../query/NativeJpaQueryUnitTests.java | 8 +- ...positoryFragmentsContributorUnitTests.java | 4 + .../support/SimpleJpaRepositoryUnitTests.java | 6 +- .../src/test/resources/multitenancy-test.xml | 40 +++++++++ 12 files changed, 200 insertions(+), 16 deletions(-) create mode 100644 spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/HibernateCurrentTenantIdentifierResolver.java create mode 100644 spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/HibernateMultitenancyTests.java create mode 100644 spring-data-jpa/src/test/resources/multitenancy-test.xml diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/provider/PersistenceProvider.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/provider/PersistenceProvider.java index 9755f19f0..caabe955c 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/provider/PersistenceProvider.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/provider/PersistenceProvider.java @@ -56,6 +56,7 @@ import org.springframework.util.ConcurrentReferenceHashMap; * @author Jens Schauder * @author Greg Turnquist * @author Yuriy Tsarkov + * @author Ariel Morelli Andres (Atlassian US, Inc.) */ public enum PersistenceProvider implements QueryExtractor, ProxyIdAccessor, QueryComment { @@ -316,7 +317,7 @@ public enum PersistenceProvider implements QueryExtractor, ProxyIdAccessor, Quer } /** - * Determines the {@link PersistenceProvider} from the given {@link EntityManager}. If no special one can be + * Determines the {@link PersistenceProvider} from the given {@link EntityManagerFactory}. If no special one can be * determined {@link #GENERIC_JPA} will be returned. * * @param emf must not be {@literal null}. @@ -324,7 +325,7 @@ public enum PersistenceProvider implements QueryExtractor, ProxyIdAccessor, Quer */ public static PersistenceProvider fromEntityManagerFactory(EntityManagerFactory emf) { - Assert.notNull(emf, "EntityManager must not be null"); + Assert.notNull(emf, "EntityManagerFactory must not be null"); Class entityManagerType = emf.getPersistenceUnitUtil().getClass(); PersistenceProvider cachedProvider = CACHE.get(entityManagerType); diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java index f70210ff8..ad0cafba9 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractJpaQuery.java @@ -24,7 +24,6 @@ import jakarta.persistence.TupleElement; import jakarta.persistence.TypedQuery; import java.lang.reflect.Constructor; -import java.util.AbstractMap; import java.util.ArrayList; import java.util.List; import java.util.function.UnaryOperator; diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/Querydsl.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/Querydsl.java index da00e0536..1e493eb90 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/Querydsl.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/Querydsl.java @@ -40,7 +40,6 @@ import com.querydsl.jpa.JPQLQuery; import com.querydsl.jpa.JPQLTemplates; import com.querydsl.jpa.impl.AbstractJPAQuery; import com.querydsl.jpa.impl.JPAQuery; -import org.jspecify.annotations.Nullable; /** * Helper instance to ease access to Querydsl JPA query API. @@ -87,7 +86,8 @@ public class Querydsl { * Obtains the {@link JPQLTemplates} for the configured {@link EntityManager}. Can return {@literal null} to use the * default templates. * - * @return the {@link JPQLTemplates} for the configured {@link EntityManager}, {@link JPQLTemplates#DEFAULT} by default. + * @return the {@link JPQLTemplates} for the configured {@link EntityManager}, {@link JPQLTemplates#DEFAULT} by + * default. * @since 3.5 */ public JPQLTemplates getTemplates() { diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/CrudMethodMetadataUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/CrudMethodMetadataUnitTests.java index 52e217bb7..69e7379d1 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/CrudMethodMetadataUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/CrudMethodMetadataUnitTests.java @@ -17,9 +17,6 @@ package org.springframework.data.jpa.repository; import static org.mockito.Mockito.*; -import java.util.Collections; -import java.util.Map; - import jakarta.persistence.EntityManager; import jakarta.persistence.EntityManagerFactory; import jakarta.persistence.LockModeType; @@ -28,6 +25,9 @@ import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.CriteriaQuery; import jakarta.persistence.metamodel.Metamodel; +import java.util.Collections; +import java.util.Map; + import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/HibernateCurrentTenantIdentifierResolver.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/HibernateCurrentTenantIdentifierResolver.java new file mode 100644 index 000000000..436e99fb3 --- /dev/null +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/HibernateCurrentTenantIdentifierResolver.java @@ -0,0 +1,49 @@ +/* + * Copyright 2011-2025 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 + * + * https://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.repository; + +import java.util.Optional; + +import org.hibernate.context.spi.CurrentTenantIdentifierResolver; +import org.jspecify.annotations.Nullable; + +/** + * {@code CurrentTenantIdentifierResolver} instance for testing + * + * @author Ariel Morelli Andres (Atlassian US, Inc.) + */ +public class HibernateCurrentTenantIdentifierResolver implements CurrentTenantIdentifierResolver { + private static final ThreadLocal<@Nullable String> CURRENT_TENANT_IDENTIFIER = new ThreadLocal<>(); + + public static void setTenantIdentifier(String tenantIdentifier) { + CURRENT_TENANT_IDENTIFIER.set(tenantIdentifier); + } + + public static void removeTenantIdentifier() { + CURRENT_TENANT_IDENTIFIER.remove(); + } + + @Override + public String resolveCurrentTenantIdentifier() { + return Optional.ofNullable(CURRENT_TENANT_IDENTIFIER.get()) + .orElseThrow(() -> new IllegalArgumentException("Could not resolve current tenant identifier")); + } + + @Override + public boolean validateExistingCurrentSessions() { + return true; + } +} diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/HibernateMultitenancyTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/HibernateMultitenancyTests.java new file mode 100644 index 000000000..3de19e90d --- /dev/null +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/HibernateMultitenancyTests.java @@ -0,0 +1,88 @@ +/* + * Copyright 2011-2025 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 + * + * https://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.repository; + +import static org.assertj.core.api.Assertions.*; +import static org.assertj.core.api.Assumptions.*; + +import java.util.List; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.ComponentScan; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.FilterType; +import org.springframework.context.annotation.ImportResource; +import org.springframework.data.jpa.domain.sample.Role; +import org.springframework.data.jpa.provider.PersistenceProvider; +import org.springframework.data.jpa.repository.config.EnableJpaRepositories; +import org.springframework.data.jpa.repository.sample.RoleRepository; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.springframework.transaction.annotation.Transactional; + +import jakarta.persistence.EntityManager; + +/** + * Tests for repositories that use multi-tenancy. This tests verifies that repositories can be created an injected + * despite not having a tenant available at creation time + * + * @author Ariel Morelli Andres (Atlassian US, Inc.) + */ +@ExtendWith(SpringExtension.class) +@ContextConfiguration() +class HibernateMultitenancyTests { + + @Autowired RoleRepository roleRepository; + @Autowired EntityManager em; + + @AfterEach + void tearDown() { + HibernateCurrentTenantIdentifierResolver.removeTenantIdentifier(); + } + + @Test + void testPersistenceProviderFromFactoryWithoutTenant() { + PersistenceProvider provider = PersistenceProvider.fromEntityManagerFactory(em.getEntityManagerFactory()); + assumeThat(provider).isEqualTo(PersistenceProvider.HIBERNATE); + } + + @Test + void testRepositoryWithTenant() { + HibernateCurrentTenantIdentifierResolver.setTenantIdentifier("tenant-id"); + assertThatNoException().isThrownBy(() -> roleRepository.findAll()); + } + + @Test + void testRepositoryWithoutTenantFails() { + assertThatThrownBy(() -> roleRepository.findAll()).isInstanceOf(RuntimeException.class); + } + + @Transactional + List insertAndQuery() { + roleRepository.save(new Role("DRUMMER")); + roleRepository.flush(); + return roleRepository.findAll(); + } + + @ImportResource({ "classpath:multitenancy-test.xml" }) + @Configuration + @EnableJpaRepositories(basePackageClasses = HibernateRepositoryTests.class, considerNestedRepositories = true, + includeFilters = @ComponentScan.Filter(classes = { RoleRepository.class }, type = FilterType.ASSIGNABLE_TYPE)) + static class TestConfig {} +} diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQueryUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQueryUnitTests.java index 953203134..b254c41ce 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQueryUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQueryUnitTests.java @@ -18,6 +18,7 @@ package org.springframework.data.jpa.repository.query; import static org.mockito.Mockito.*; import jakarta.persistence.EntityManager; +import jakarta.persistence.EntityManagerFactory; import jakarta.persistence.metamodel.Metamodel; import java.lang.reflect.Method; @@ -52,6 +53,7 @@ import org.springframework.util.ReflectionUtils; * * @author Christoph Strobl * @author Mark Paluch + * @author Ariel Morelli Andres */ class AbstractStringBasedJpaQueryUnitTests { @@ -137,10 +139,12 @@ class AbstractStringBasedJpaQueryUnitTests { public EntityManager get() { EntityManager em = Mockito.mock(EntityManager.class); + EntityManagerFactory emf = Mockito.mock(EntityManagerFactory.class); Metamodel meta = mock(Metamodel.class); when(em.getMetamodel()).thenReturn(meta); when(em.getDelegate()).thenReturn(new Object()); // some generic jpa + when(em.getEntityManagerFactory()).thenReturn(emf); return em; } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/NamedQueryUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/NamedQueryUnitTests.java index 71bd266f0..b5a70c909 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/NamedQueryUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/NamedQueryUnitTests.java @@ -36,7 +36,6 @@ import org.mockito.quality.Strictness; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.provider.QueryExtractor; -import org.springframework.data.jpa.repository.QueryRewriter; import org.springframework.data.projection.ProjectionFactory; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.repository.core.RepositoryMetadata; diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/NativeJpaQueryUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/NativeJpaQueryUnitTests.java index c17cc49f9..776823916 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/NativeJpaQueryUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/NativeJpaQueryUnitTests.java @@ -71,11 +71,9 @@ class NativeJpaQueryUnitTests { queryExtractor); NativeJpaQuery query = new NativeJpaQuery(queryMethod, em, queryMethod.getRequiredDeclaredQuery(), - queryMethod.getDeclaredCountQuery(), - new JpaQueryConfiguration(QueryRewriterProvider.simple(), QueryEnhancerSelector.DEFAULT_SELECTOR, - ValueExpressionDelegate.create(), EscapeCharacter.DEFAULT)); - QueryProvider sql = query.getSortedQuery(Sort.by("foo", "bar"), - queryMethod.getResultProcessor().getReturnedType()); + queryMethod.getDeclaredCountQuery(), new JpaQueryConfiguration(QueryRewriterProvider.simple(), + QueryEnhancerSelector.DEFAULT_SELECTOR, ValueExpressionDelegate.create(), EscapeCharacter.DEFAULT)); + QueryProvider sql = query.getSortedQuery(Sort.by("foo", "bar"), queryMethod.getResultProcessor().getReturnedType()); assertThat(sql.getQueryString()).isEqualTo("SELECT e FROM Employee e order by e.foo asc, e.bar asc"); } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryFragmentsContributorUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryFragmentsContributorUnitTests.java index 7825534a3..f3634a37e 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryFragmentsContributorUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/JpaRepositoryFragmentsContributorUnitTests.java @@ -19,6 +19,7 @@ import static org.assertj.core.api.Assertions.*; import static org.mockito.Mockito.*; import jakarta.persistence.EntityManager; +import jakarta.persistence.EntityManagerFactory; import java.util.Iterator; @@ -40,6 +41,7 @@ import com.querydsl.core.types.EntityPath; * Unit tests for {@link JpaRepositoryFragmentsContributor}. * * @author Mark Paluch + * @author Ariel Morelli Andres */ class JpaRepositoryFragmentsContributorUnitTests { @@ -53,7 +55,9 @@ class JpaRepositoryFragmentsContributorUnitTests { when(entityPathResolver.createPath(any())).thenReturn((EntityPath) QCustomer.customer); EntityManager entityManager = mock(EntityManager.class); + EntityManagerFactory emf = mock(EntityManagerFactory.class); when(entityManager.getDelegate()).thenReturn(entityManager); + when(entityManager.getEntityManagerFactory()).thenReturn(emf); RepositoryComposition.RepositoryFragments fragments = contributor.contribute( AbstractRepositoryMetadata.getMetadata(QuerydslUserRepository.class), diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/SimpleJpaRepositoryUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/SimpleJpaRepositoryUnitTests.java index 3d17c347c..b720228bd 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/SimpleJpaRepositoryUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/support/SimpleJpaRepositoryUnitTests.java @@ -31,7 +31,6 @@ import jakarta.persistence.criteria.CriteriaQuery; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.Arrays; -import java.util.Optional; import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; @@ -44,7 +43,6 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; - import org.springframework.data.domain.PageRequest; import org.springframework.data.jpa.domain.Specification; import org.springframework.data.jpa.domain.sample.User; @@ -61,6 +59,7 @@ import org.springframework.transaction.annotation.Transactional; * @author Jens Schauder * @author Greg Turnquist * @author Yanming Zhou + * @author Ariel Morelli Andres (Atlassian US, Inc.) */ @ExtendWith(MockitoExtension.class) @MockitoSettings(strictness = Strictness.LENIENT) @@ -85,6 +84,9 @@ class SimpleJpaRepositoryUnitTests { void setUp() { when(em.getDelegate()).thenReturn(em); + when(em.getEntityManagerFactory()).thenReturn(entityManagerFactory); + + when(entityManagerFactory.getPersistenceUnitUtil()).thenReturn(persistenceUnitUtil); when(information.getJavaType()).thenReturn(User.class); when(em.getCriteriaBuilder()).thenReturn(builder); diff --git a/spring-data-jpa/src/test/resources/multitenancy-test.xml b/spring-data-jpa/src/test/resources/multitenancy-test.xml new file mode 100644 index 000000000..d1ff786d1 --- /dev/null +++ b/spring-data-jpa/src/test/resources/multitenancy-test.xml @@ -0,0 +1,40 @@ + + + + + + + + + + + + + org.springframework.data.jpa.repository.HibernateCurrentTenantIdentifierResolver + + + + + + + + + + + + + + + + + + + + + +