From d2cd13996a9999eed4ce07aa83442e8a4271487e Mon Sep 17 00:00:00 2001 From: Michael Hirsch Date: Mon, 18 Jul 2016 12:48:50 +0200 Subject: [PATCH] fix initial creation of tenant when no current tenant and caching fix Signed-off-by: Michael Hirsch --- .../cache/CacheAutoConfiguration.java | 23 +++++---- .../hawkbit/cache/RedisConfiguration.java | 9 +++- .../repository/jpa/JpaSystemManagement.java | 47 +++++++++++++++---- .../security/SecurityContextTenantAware.java | 27 +++++++---- .../security/SystemSecurityContext.java | 8 +++- 5 files changed, 85 insertions(+), 29 deletions(-) diff --git a/hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/cache/CacheAutoConfiguration.java b/hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/cache/CacheAutoConfiguration.java index 6696c44ca..a170dfcd3 100644 --- a/hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/cache/CacheAutoConfiguration.java +++ b/hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/cache/CacheAutoConfiguration.java @@ -25,6 +25,7 @@ import org.springframework.cache.interceptor.CacheOperationInvocationContext; import org.springframework.cache.interceptor.SimpleCacheResolver; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Primary; /** * A configuration for configuring the spring {@link CacheManager} for specific @@ -33,9 +34,6 @@ import org.springframework.context.annotation.Configuration; * * This is done by providing a special {@link TenantCacheResolver} which * generates a cache name included the current tenant. - * - * - * */ @Configuration @EnableCaching @@ -51,18 +49,27 @@ public class CacheAutoConfiguration extends CachingConfigurerSupport { @Override @Bean @ConditionalOnMissingBean + @Primary public TenancyCacheManager cacheManager() { - return new TenantAwareCacheManager(new GuavaCacheManager(), tenantAware); + return new TenantAwareCacheManager(directCacheManager(), tenantAware); + } + + /** + * @return the direct cache manager to access without tenant aware check, + * cause in sometimes it's necessary to access the cache directly + * without having the current tenant, e.g. initial creation of + * tenant + */ + @Bean(name = "directCacheManager") + @ConditionalOnMissingBean(name = "directCacheManager") + public CacheManager directCacheManager() { + return new GuavaCacheManager(); } /** * A {@link SimpleCacheResolver} implementation which includes the * {@link TenantAware#getCurrentTenant()} into the cache name before * resolving it. - * - * - * - * */ public class TenantCacheResolver extends SimpleCacheResolver { diff --git a/hawkbit-cache-redis/src/main/java/org/eclipse/hawkbit/cache/RedisConfiguration.java b/hawkbit-cache-redis/src/main/java/org/eclipse/hawkbit/cache/RedisConfiguration.java index edc183b17..fc3364d81 100644 --- a/hawkbit-cache-redis/src/main/java/org/eclipse/hawkbit/cache/RedisConfiguration.java +++ b/hawkbit-cache-redis/src/main/java/org/eclipse/hawkbit/cache/RedisConfiguration.java @@ -15,6 +15,7 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties import org.springframework.cache.CacheManager; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Primary; import org.springframework.data.redis.cache.RedisCacheManager; import org.springframework.data.redis.connection.jedis.JedisConnectionFactory; import org.springframework.data.redis.core.RedisTemplate; @@ -50,8 +51,14 @@ public class RedisConfiguration { * @return the spring redis cache manager. */ @Bean + @Primary public CacheManager cacheManager() { - return new TenantAwareCacheManager(new RedisCacheManager(redisTemplate()), tenantAware); + return new TenantAwareCacheManager(directCacheManager(), tenantAware); + } + + @Bean(name = "directCacheManager") + public CacheManager directCacheManager() { + return new RedisCacheManager(redisTemplate()); } /** diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaSystemManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaSystemManagement.java index 1786dc976..f1d427af2 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaSystemManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaSystemManagement.java @@ -19,6 +19,7 @@ import org.eclipse.hawkbit.repository.Constants; import org.eclipse.hawkbit.repository.SystemManagement; import org.eclipse.hawkbit.repository.TenantStatsManagement; import org.eclipse.hawkbit.repository.exception.EntityNotFoundException; +import org.eclipse.hawkbit.repository.jpa.configuration.MultiTenantJpaTransactionManager; import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSetType; import org.eclipse.hawkbit.repository.jpa.model.JpaSoftwareModuleType; import org.eclipse.hawkbit.repository.jpa.model.JpaTenantMetaData; @@ -26,6 +27,7 @@ import org.eclipse.hawkbit.repository.model.DistributionSetType; import org.eclipse.hawkbit.repository.model.SoftwareModuleType; import org.eclipse.hawkbit.repository.model.TenantMetaData; import org.eclipse.hawkbit.repository.report.model.SystemUsageReport; +import org.eclipse.hawkbit.security.SystemSecurityContext; import org.eclipse.hawkbit.tenancy.TenantAware; import org.eclipse.persistence.config.PersistenceUnitProperties; import org.springframework.beans.factory.annotation.Autowired; @@ -33,11 +35,14 @@ import org.springframework.cache.annotation.CacheEvict; import org.springframework.cache.annotation.CachePut; import org.springframework.cache.annotation.Cacheable; import org.springframework.cache.interceptor.KeyGenerator; -import org.springframework.context.ApplicationContext; import org.springframework.data.jpa.repository.Modifying; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.TransactionDefinition; import org.springframework.transaction.annotation.Isolation; import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.DefaultTransactionDefinition; +import org.springframework.transaction.support.TransactionTemplate; import org.springframework.validation.annotation.Validated; /** @@ -108,7 +113,10 @@ public class JpaSystemManagement implements CurrentTenantCacheKeyGenerator, Syst private SystemManagementCacheKeyGenerator currentTenantCacheKeyGenerator; @Autowired - private ApplicationContext applicationContext; + private SystemSecurityContext systemSecurityContext; + + @Autowired + private PlatformTransactionManager txManager; @Override public SystemUsageReport getSystemUsageStatistics() { @@ -159,27 +167,48 @@ public class JpaSystemManagement implements CurrentTenantCacheKeyGenerator, Syst } @Override - @Cacheable(value = "tenantMetadata", key = "#tenant.toUpperCase()") + @Cacheable(value = "tenantMetadata", key = "#tenant.toUpperCase()", cacheManager = "directCacheManager") @Transactional(isolation = Isolation.READ_UNCOMMITTED) @Modifying public TenantMetaData getTenantMetadata(final String tenant) { final TenantMetaData result = tenantMetaDataRepository.findByTenantIgnoreCase(tenant); - // Create if it does not exist if (result == null) { try { currentTenantCacheKeyGenerator.getCreateInitialTenant().set(tenant); - cacheManager.getCache("currentTenant").evict(currentTenantKeyGenerator().generate(null, null)); - applicationContext.getBean("currentTenantKeyGenerator"); - return tenantMetaDataRepository.save(new JpaTenantMetaData(createStandardSoftwareDataSetup(), tenant)); + return createInitialTenantMetaData(tenant); + } finally { currentTenantCacheKeyGenerator.getCreateInitialTenant().remove(); } } - return result; } + /** + * Creating the initial tenant meta-data in a new transaction. Due the + * {@link MultiTenantJpaTransactionManager} is using the current tenant to + * set the necessary tenant discriminator to the query. This is not working + * if we don't have a current tenant set. Due the + * {@link #getTenantMetadata(String)} is maybe called without having a + * current tenant we need to re-open a new transaction so the + * {@link MultiTenantJpaTransactionManager} is called again and set the + * tenant for this transaction. + * + * @param tenant + * the tenant to be created + * @return the initial created {@link TenantMetaData} + */ + private TenantMetaData createInitialTenantMetaData(final String tenant) { + final DefaultTransactionDefinition def = new DefaultTransactionDefinition(); + def.setName("initial-tenant-creation"); + def.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW); + return systemSecurityContext.runAsSystemAsTenant( + () -> new TransactionTemplate(txManager, def).execute(status -> tenantMetaDataRepository + .save(new JpaTenantMetaData(createStandardSoftwareDataSetup(), tenant))), + tenant); + } + @Override public List findTenants() { return tenantMetaDataRepository.findAll().stream().map(md -> md.getTenant()).collect(Collectors.toList()); @@ -226,7 +255,7 @@ public class JpaSystemManagement implements CurrentTenantCacheKeyGenerator, Syst } @Override - @Cacheable(value = "currentTenant", keyGenerator = "currentTenantKeyGenerator") + @Cacheable(value = "currentTenant", keyGenerator = "currentTenantKeyGenerator", cacheManager = "directCacheManager") // set transaction to not supported, due we call this in // BaseEntity#prePersist methods // and it seems that JPA committing the transaction when executing this diff --git a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SecurityContextTenantAware.java b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SecurityContextTenantAware.java index 4b5fe7224..e67faa739 100644 --- a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SecurityContextTenantAware.java +++ b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SecurityContextTenantAware.java @@ -9,6 +9,7 @@ package org.eclipse.hawkbit.security; import java.util.Collection; +import java.util.Collections; import org.eclipse.hawkbit.im.authentication.TenantAwareAuthenticationDetails; import org.eclipse.hawkbit.tenancy.TenantAware; @@ -80,32 +81,37 @@ public class SecurityContextTenantAware implements TenantAware { @Override public boolean equals(final Object another) { - return delegate.equals(another); + if (delegate != null) { + return delegate.equals(another); + } else if (another == null) { + return true; + } + return false; } @Override public String toString() { - return delegate.toString(); + return (delegate != null) ? delegate.toString() : null; } @Override public int hashCode() { - return delegate.hashCode(); + return (delegate != null) ? delegate.hashCode() : null; } @Override public String getName() { - return delegate.getName(); + return (delegate != null) ? delegate.getName() : null; } @Override public Collection getAuthorities() { - return delegate.getAuthorities(); + return (delegate != null) ? delegate.getAuthorities() : Collections.emptyList(); } @Override public Object getCredentials() { - return delegate.getCredentials(); + return (delegate != null) ? delegate.getCredentials() : null; } @Override @@ -115,16 +121,19 @@ public class SecurityContextTenantAware implements TenantAware { @Override public Object getPrincipal() { - return delegate.getPrincipal(); + return (delegate != null) ? delegate.getPrincipal() : null; } @Override public boolean isAuthenticated() { - return delegate.isAuthenticated(); + return (delegate != null) ? delegate.isAuthenticated() : null; } @Override - public void setAuthenticated(final boolean isAuthenticated) throws IllegalArgumentException { + public void setAuthenticated(final boolean isAuthenticated) { + if (delegate == null) { + return; + } delegate.setAuthenticated(isAuthenticated); } } diff --git a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SystemSecurityContext.java b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SystemSecurityContext.java index c0ecb8ceb..9becae909 100644 --- a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SystemSecurityContext.java +++ b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SystemSecurityContext.java @@ -67,12 +67,16 @@ public class SystemSecurityContext { // Exception squid:S2221 - Callable declares Exception @SuppressWarnings("squid:S2221") public T runAsSystem(final Callable callable) { + return runAsSystemAsTenant(callable, tenantAware.getCurrentTenant()); + } + + public T runAsSystemAsTenant(final Callable callable, final String tenant) { final SecurityContext oldContext = SecurityContextHolder.getContext(); try { logger.debug("entering system code execution"); - return tenantAware.runAsTenant(tenantAware.getCurrentTenant(), () -> { + return tenantAware.runAsTenant(tenant, () -> { try { - setSystemContext(oldContext); + setSystemContext(SecurityContextHolder.getContext()); return callable.call(); } catch (final Exception e) { throw Throwables.propagate(e);