From a7d0306e75299387b693eb87901f27d4d9a424df Mon Sep 17 00:00:00 2001 From: Kai Zimmermann Date: Fri, 8 Apr 2016 05:37:21 +0200 Subject: [PATCH 1/5] Removed unnecessary transaction exceptions. Signed-off-by: Kai Zimmermann --- .../MultiTenantJpaTransactionManager.java | 38 ++++++++----------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/MultiTenantJpaTransactionManager.java b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/MultiTenantJpaTransactionManager.java index 7b97f5037..1a519ea5b 100644 --- a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/MultiTenantJpaTransactionManager.java +++ b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/MultiTenantJpaTransactionManager.java @@ -11,7 +11,6 @@ package org.eclipse.hawkbit; import javax.persistence.EntityManager; import javax.transaction.Transaction; -import org.eclipse.hawkbit.repository.RolloutManagement; import org.eclipse.hawkbit.repository.SystemManagement; import org.eclipse.hawkbit.repository.exception.TenantNotExistException; import org.eclipse.hawkbit.tenancy.TenantAware; @@ -38,38 +37,31 @@ public class MultiTenantJpaTransactionManager extends JpaTransactionManager { protected void doBegin(final Object transaction, final TransactionDefinition definition) { super.doBegin(transaction, definition); + // ignore transactions on tenant independent calls + if (isTenantManagement(definition) && isCurrentTenantKeyGenerator(definition)) { + return; + } + final EntityManagerHolder emHolder = (EntityManagerHolder) TransactionSynchronizationManager .getResource(getEntityManagerFactory()); final EntityManager em = emHolder.getEntityManager(); - if (notTenantManagement(definition) && notCurrentTenantKeyGenerator(definition) - && notRolloutScheduler(definition) && notGetOrCreateTenantMetadata(definition)) { - - final String currentTenant = tenantAware.getCurrentTenant(); - if (currentTenant == null) { - throw new TenantNotExistException("Tenant Unknown. Canceling transaction."); - } - - em.setProperty(PersistenceUnitProperties.MULTITENANT_PROPERTY_DEFAULT, currentTenant.toUpperCase()); + final String currentTenant = tenantAware.getCurrentTenant(); + if (currentTenant == null) { + throw new TenantNotExistException("Tenant Unknown. Canceling transaction."); } + + em.setProperty(PersistenceUnitProperties.MULTITENANT_PROPERTY_DEFAULT, currentTenant.toUpperCase()); + } - private boolean notGetOrCreateTenantMetadata(final TransactionDefinition definition) { - return !definition.getName() - .startsWith(SystemManagement.class.getCanonicalName() + ".getOrCreateTenantMetadata"); - } - - private boolean notRolloutScheduler(final TransactionDefinition definition) { - return !definition.getName().startsWith(RolloutManagement.class.getCanonicalName() + ".rolloutScheduler"); - } - - private boolean notCurrentTenantKeyGenerator(final TransactionDefinition definition) { - return !definition.getName() + private boolean isCurrentTenantKeyGenerator(final TransactionDefinition definition) { + return definition.getName() .startsWith(SystemManagement.class.getCanonicalName() + ".currentTenantKeyGenerator"); } - private boolean notTenantManagement(final TransactionDefinition definition) { - return !definition.getName().startsWith(SystemManagement.class.getCanonicalName() + ".deleteTenant") + private boolean isTenantManagement(final TransactionDefinition definition) { + return definition.getName().startsWith(SystemManagement.class.getCanonicalName() + ".deleteTenant") && !definition.getName().startsWith(SystemManagement.class.getCanonicalName() + ".findTenants"); } } From e3de03206d540bea56b5bc1b70edc6d915798fc5 Mon Sep 17 00:00:00 2001 From: Kai Zimmermann Date: Fri, 8 Apr 2016 05:59:34 +0200 Subject: [PATCH 2/5] Fixed condition --- .../main/java/org/eclipse/hawkbit/ui/HawkbitUI.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/HawkbitUI.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/HawkbitUI.java index f4c26a960..4acd6e887 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/HawkbitUI.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/HawkbitUI.java @@ -48,15 +48,13 @@ import com.vaadin.ui.VerticalLayout; import com.vaadin.ui.themes.ValoTheme; /** - * Vaadin management UI. - * - * - * + * hawkBit Management UI. * */ -@SuppressWarnings("serial") @Title("hawkBit Update Server") public class HawkbitUI extends DefaultHawkbitUI implements DetachListener { + private static final long serialVersionUID = 1L; + private static final Logger LOG = LoggerFactory.getLogger(HawkbitUI.class); private static final String EMPTY_VIEW = ""; @@ -154,6 +152,8 @@ public class HawkbitUI extends DefaultHawkbitUI implements DetachListener { } final Navigator navigator = new Navigator(this, content); navigator.addViewChangeListener(new ViewChangeListener() { + private static final long serialVersionUID = 1L; + @Override public boolean beforeViewChange(final ViewChangeEvent event) { return true; @@ -233,6 +233,7 @@ public class HawkbitUI extends DefaultHawkbitUI implements DetachListener { } private class ManagementViewProvider implements ViewProvider { + private static final long serialVersionUID = 1L; @Override public String getViewName(final String viewAndParameters) { From 0c800d31490bf66810b382f7f6df22562f642261 Mon Sep 17 00:00:00 2001 From: Kai Zimmermann Date: Fri, 8 Apr 2016 06:13:16 +0200 Subject: [PATCH 3/5] Fixed condition --- .../org/eclipse/hawkbit/MultiTenantJpaTransactionManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/MultiTenantJpaTransactionManager.java b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/MultiTenantJpaTransactionManager.java index 1a519ea5b..ac6bc8806 100644 --- a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/MultiTenantJpaTransactionManager.java +++ b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/MultiTenantJpaTransactionManager.java @@ -38,7 +38,7 @@ public class MultiTenantJpaTransactionManager extends JpaTransactionManager { super.doBegin(transaction, definition); // ignore transactions on tenant independent calls - if (isTenantManagement(definition) && isCurrentTenantKeyGenerator(definition)) { + if (isTenantManagement(definition) || isCurrentTenantKeyGenerator(definition)) { return; } From cf8e4fd2fd55909a8fe66c43a2e59d422da1596e Mon Sep 17 00:00:00 2001 From: Kai Zimmermann Date: Fri, 8 Apr 2016 06:50:27 +0200 Subject: [PATCH 4/5] Revert "Fixed condition" This reverts commit e3de03206d540bea56b5bc1b70edc6d915798fc5. --- .../main/java/org/eclipse/hawkbit/ui/HawkbitUI.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/HawkbitUI.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/HawkbitUI.java index 4acd6e887..f4c26a960 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/HawkbitUI.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/HawkbitUI.java @@ -48,13 +48,15 @@ import com.vaadin.ui.VerticalLayout; import com.vaadin.ui.themes.ValoTheme; /** - * hawkBit Management UI. + * Vaadin management UI. + * + * + * * */ +@SuppressWarnings("serial") @Title("hawkBit Update Server") public class HawkbitUI extends DefaultHawkbitUI implements DetachListener { - private static final long serialVersionUID = 1L; - private static final Logger LOG = LoggerFactory.getLogger(HawkbitUI.class); private static final String EMPTY_VIEW = ""; @@ -152,8 +154,6 @@ public class HawkbitUI extends DefaultHawkbitUI implements DetachListener { } final Navigator navigator = new Navigator(this, content); navigator.addViewChangeListener(new ViewChangeListener() { - private static final long serialVersionUID = 1L; - @Override public boolean beforeViewChange(final ViewChangeEvent event) { return true; @@ -233,7 +233,6 @@ public class HawkbitUI extends DefaultHawkbitUI implements DetachListener { } private class ManagementViewProvider implements ViewProvider { - private static final long serialVersionUID = 1L; @Override public String getViewName(final String viewAndParameters) { From 6e85c724bf79b7a7d54148f228419b192b2191d6 Mon Sep 17 00:00:00 2001 From: Kai Zimmermann Date: Mon, 25 Apr 2016 14:09:38 +0200 Subject: [PATCH 5/5] Removed the method exception from transaction manager as they where not needed anymore. The bean in systemmanagement should also not open a transaction. Signed-off-by: Kai Zimmermann --- .../MultiTenantJpaTransactionManager.java | 16 ---------------- .../hawkbit/repository/SystemManagement.java | 3 +-- .../hawkbit/tenancy/MultiTenancyEntityTest.java | 16 ++++++++++++++++ 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/MultiTenantJpaTransactionManager.java b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/MultiTenantJpaTransactionManager.java index ac6bc8806..a45c846a9 100644 --- a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/MultiTenantJpaTransactionManager.java +++ b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/MultiTenantJpaTransactionManager.java @@ -11,7 +11,6 @@ package org.eclipse.hawkbit; import javax.persistence.EntityManager; import javax.transaction.Transaction; -import org.eclipse.hawkbit.repository.SystemManagement; import org.eclipse.hawkbit.repository.exception.TenantNotExistException; import org.eclipse.hawkbit.tenancy.TenantAware; import org.eclipse.persistence.config.PersistenceUnitProperties; @@ -37,11 +36,6 @@ public class MultiTenantJpaTransactionManager extends JpaTransactionManager { protected void doBegin(final Object transaction, final TransactionDefinition definition) { super.doBegin(transaction, definition); - // ignore transactions on tenant independent calls - if (isTenantManagement(definition) || isCurrentTenantKeyGenerator(definition)) { - return; - } - final EntityManagerHolder emHolder = (EntityManagerHolder) TransactionSynchronizationManager .getResource(getEntityManagerFactory()); final EntityManager em = emHolder.getEntityManager(); @@ -54,14 +48,4 @@ public class MultiTenantJpaTransactionManager extends JpaTransactionManager { em.setProperty(PersistenceUnitProperties.MULTITENANT_PROPERTY_DEFAULT, currentTenant.toUpperCase()); } - - private boolean isCurrentTenantKeyGenerator(final TransactionDefinition definition) { - return definition.getName() - .startsWith(SystemManagement.class.getCanonicalName() + ".currentTenantKeyGenerator"); - } - - private boolean isTenantManagement(final TransactionDefinition definition) { - return definition.getName().startsWith(SystemManagement.class.getCanonicalName() + ".deleteTenant") - && !definition.getName().startsWith(SystemManagement.class.getCanonicalName() + ".findTenants"); - } } diff --git a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/SystemManagement.java b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/SystemManagement.java index 5577e1d61..c1cee7223 100644 --- a/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/SystemManagement.java +++ b/hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/SystemManagement.java @@ -164,6 +164,7 @@ public class SystemManagement { * @return the {@link CurrentTenantKeyGenerator} */ @Bean + @Transactional(propagation = Propagation.NOT_SUPPORTED) public CurrentTenantKeyGenerator currentTenantKeyGenerator() { return new CurrentTenantKeyGenerator(); } @@ -206,7 +207,6 @@ public class SystemManagement { @NotNull @PreAuthorize(SpringEvalExpressions.HAS_AUTH_SYSTEM_ADMIN + SpringEvalExpressions.HAS_AUTH_OR + SpringEvalExpressions.IS_SYSTEM_CODE) - // tenant independent public List findTenants() { return tenantMetaDataRepository.findAll().stream().map(md -> md.getTenant()).collect(Collectors.toList()); } @@ -221,7 +221,6 @@ public class SystemManagement { @Transactional @Modifying @PreAuthorize(SpringEvalExpressions.HAS_AUTH_SYSTEM_ADMIN) - // tenant independent public void deleteTenant(@NotNull final String tenant) { cacheManager.evictCaches(tenant); cacheManager.getCache("currentTenant").evict(currentTenantKeyGenerator().generate(null, null)); diff --git a/hawkbit-repository/src/test/java/org/eclipse/hawkbit/tenancy/MultiTenancyEntityTest.java b/hawkbit-repository/src/test/java/org/eclipse/hawkbit/tenancy/MultiTenancyEntityTest.java index 1dd4c9823..67b9b5a26 100644 --- a/hawkbit-repository/src/test/java/org/eclipse/hawkbit/tenancy/MultiTenancyEntityTest.java +++ b/hawkbit-repository/src/test/java/org/eclipse/hawkbit/tenancy/MultiTenancyEntityTest.java @@ -78,6 +78,22 @@ public class MultiTenancyEntityTest extends AbstractIntegrationTest { assertThat(findTargetsForTenant).hasSize(1); } + @Test + @Description(value = "Ensures that tenant with proper permissions can read and delete other tenants.") + @WithUser(tenantId = "mytenant", allSpPermissions = true) + public void deleteAnotherTenantPossible() throws Exception { + // create target for another tenant + final String anotherTenant = "anotherTenant"; + final String controllerAnotherTenant = "anotherController"; + createTargetForTenant(controllerAnotherTenant, anotherTenant); + + assertThat(systemManagement.findTenants()).as("Expected number if tenants before deletion is").hasSize(3); + + systemManagement.deleteTenant(anotherTenant); + + assertThat(systemManagement.findTenants()).as("Expected number if tenants after deletion is").hasSize(2); + } + @Test @Description(value = "Ensures that tenant metadata is retrieved for the current tenant.") @WithUser(tenantId = "mytenant", autoCreateTenant = false, allSpPermissions = true)