Roy Golan has uploaded a new change for review. Change subject: coreL Inject TransactionManager instead of JNDI lookups ......................................................................
coreL Inject TransactionManager instead of JNDI lookups TransactionSupport is now injected into command base. No for jndi lookups which envolvs syncs on context lookup. Also create DummyTransactionSupport for unit tests so no need for EjbStrategy Class Rule Change-Id: Ia2a73887c76c85b17db6a3c2683d257aece10b59 Signed-off-by: Roy Golan <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterSyncJob.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/DummyTransactionSupport.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GlusterSyncJobTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/transaction/TransactionSupport.java 8 files changed, 67 insertions(+), 10 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/37/35637/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java index c8137a7..4b7b343 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java @@ -115,6 +115,9 @@ @Inject private BackendInternal backend; + @Inject + private TransactionSupport transactionSupport; + /* Multiplier used to convert GB to bytes or vice versa. */ protected static final long BYTES_IN_GB = 1024 * 1024 * 1024; private static final String DEFAULT_TASK_KEY = "DEFAULT_TASK_KEY"; @@ -2315,4 +2318,8 @@ protected MacPoolManagerStrategy getMacPool() { return MacPoolPerDcSingleton.getInstance().poolForDataCenter(getStoragePoolId()); } + + public TransactionSupport getTransactionSupport() { + return transactionSupport; + } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterSyncJob.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterSyncJob.java index 28909e0..b1c3e1e 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterSyncJob.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterSyncJob.java @@ -12,6 +12,7 @@ import org.ovirt.engine.core.bll.Backend; import org.ovirt.engine.core.bll.job.ExecutionHandler; +import org.ovirt.engine.core.bll.utils.Injector; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.SetNonOperationalVdsParameters; import org.ovirt.engine.core.common.action.VdcActionType; @@ -192,7 +193,7 @@ } private void removeServerFromDb(final VDS server) { - TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { + getTransactionSupport().executeInNewTransaction(new TransactionMethod<Void>() { @Override public Void runInTransaction() { @@ -205,6 +206,10 @@ }); } + protected TransactionSupport getTransactionSupport() { + return Injector.get(TransactionSupport.class); + } + /** * We need to be particularly careful about what servers we remove from the DB. A newly added (bootstrapped) server * gets peer probed after it's first reboot, and we don't want to accidentally remove such legitimate servers just diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java index f5730d0..d1ca514 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java @@ -52,7 +52,6 @@ import org.ovirt.engine.core.utils.linq.Predicate; import org.ovirt.engine.core.utils.threadpool.ThreadPoolUtil; import org.ovirt.engine.core.utils.transaction.TransactionMethod; -import org.ovirt.engine.core.utils.transaction.TransactionSupport; public abstract class StorageDomainCommandBase<T extends StorageDomainParametersBase> extends StorageHandlingCommandBase<T> { @@ -463,7 +462,7 @@ } protected void executeInNewTransaction(TransactionMethod<?> method) { - TransactionSupport.executeInNewTransaction(method); + getTransactionSupport().executeInNewTx(method); } private static final class LastTimeUsedAsMasterComp implements Comparator<StorageDomain>, Serializable { diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/DummyTransactionSupport.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/DummyTransactionSupport.java new file mode 100644 index 0000000..04f5137 --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/DummyTransactionSupport.java @@ -0,0 +1,16 @@ +package org.ovirt.engine.core; + +import org.infinispan.transaction.tm.DummyTransactionManager; +import org.ovirt.engine.core.utils.transaction.TransactionSupport; + +import javax.transaction.TransactionManager; + +public class DummyTransactionSupport extends TransactionSupport { + public DummyTransactionSupport() { + super.init(); + } + + @Override public TransactionManager getTransactionManager() { + return DummyTransactionManager.getInstance(); + } +} diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GlusterSyncJobTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GlusterSyncJobTest.java index 663da38..7c698df 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GlusterSyncJobTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GlusterSyncJobTest.java @@ -26,6 +26,7 @@ import org.mockito.Mockito; import org.mockito.runners.MockitoJUnitRunner; import org.mockito.verification.VerificationMode; +import org.ovirt.engine.core.DummyTransactionSupport; import org.ovirt.engine.core.bll.utils.ClusterUtils; import org.ovirt.engine.core.bll.utils.GlusterUtil; import org.ovirt.engine.core.common.AuditLogType; @@ -270,6 +271,7 @@ doNothing().when(glusterManager).acquireLock(CLUSTER_ID); doNothing().when(glusterManager).releaseLock(CLUSTER_ID); doReturn(glusterUtil).when(glusterManager).getGlusterUtil(); + doReturn(new DummyTransactionSupport()).when(glusterManager).getTransactionSupport(); } private ArgumentMatcher<VDSParametersBase> isRemovedServer() { diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommandTest.java index 1eeaad1..44a5483 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommandTest.java @@ -22,6 +22,7 @@ import org.mockito.invocation.InvocationOnMock; import org.mockito.runners.MockitoJUnitRunner; import org.mockito.stubbing.Answer; +import org.ovirt.engine.core.DummyTransactionSupport; import org.ovirt.engine.core.bll.context.CommandContext; import org.ovirt.engine.core.bll.context.CompensationContext; import org.ovirt.engine.core.bll.interfaces.BackendInternal; @@ -87,6 +88,7 @@ AttachStorageDomainToPoolCommand<AttachStorageDomainToPoolParameters> cmd = spy(new AttachStorageDomainToPoolCommand<AttachStorageDomainToPoolParameters>(params)); + when(cmd.getTransactionSupport()).thenReturn(new DummyTransactionSupport()); doReturn(dbFacade).when(cmd).getDbFacade(); doNothing().when(cmd).attemptToActivateDomain(); doReturn(Collections.emptyList()).when(cmd).connectHostsInUpToDomainStorageServer(); diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java index cd1ed46..3d2c542 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java @@ -12,12 +12,12 @@ import java.util.List; import org.junit.Before; -import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; +import org.ovirt.engine.core.DummyTransactionSupport; import org.ovirt.engine.core.bll.context.CompensationContext; import org.ovirt.engine.core.bll.interfaces.BackendInternal; import org.ovirt.engine.core.common.action.StorageDomainPoolParametersBase; @@ -47,6 +47,7 @@ import org.ovirt.engine.core.dao.VmStaticDAO; import org.ovirt.engine.core.utils.MockEJBStrategyRule; import org.ovirt.engine.core.utils.ejb.BeanType; +import org.ovirt.engine.core.utils.transaction.TransactionSupport; @RunWith(MockitoJUnitRunner.class) public class DeactivateStorageDomainCommandTest { @@ -77,12 +78,14 @@ @Mock private VmDynamicDAO vmDynamicDAO; + private TransactionSupport transactionSupport = new DummyTransactionSupport(); + StoragePoolIsoMap map = new StoragePoolIsoMap(); private StorageDomain domain = null; - @ClassRule - public static MockEJBStrategyRule mockEjbRule = new MockEJBStrategyRule(); +// @ClassRule +// public static MockEJBStrategyRule mockEjbRule = new MockEJBStrategyRule(); StorageDomainPoolParametersBase params = new StorageDomainPoolParametersBase(Guid.newGuid(), Guid.newGuid()); DeactivateStorageDomainCommand<StorageDomainPoolParametersBase> cmd = @@ -93,6 +96,7 @@ doReturn(dbFacade).when(cmd).getDbFacade(); when(dbFacade.getStoragePoolDao()).thenReturn(storagePoolDAO); when(dbFacade.getStorageDomainDao()).thenReturn(storageDomainDAO); + when(cmd.getTransactionSupport()).thenReturn(transactionSupport); } @Test diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/transaction/TransactionSupport.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/transaction/TransactionSupport.java index f09f4cf..7486770 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/transaction/TransactionSupport.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/transaction/TransactionSupport.java @@ -1,6 +1,9 @@ package org.ovirt.engine.core.utils.transaction; +import javax.annotation.PostConstruct; +import javax.annotation.Resource; import javax.ejb.TransactionRolledbackLocalException; +import javax.inject.Singleton; import javax.transaction.HeuristicMixedException; import javax.transaction.HeuristicRollbackException; import javax.transaction.NotSupportedException; @@ -12,21 +15,31 @@ import javax.transaction.TransactionManager; import org.ovirt.engine.core.compat.TransactionScopeOption; -import org.ovirt.engine.core.utils.ejb.ContainerManagedResourceType; -import org.ovirt.engine.core.utils.ejb.EjbUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +@Singleton public class TransactionSupport { + @Resource(lookup = "java:jboss/TransactionManager") + private TransactionManager tm; + private static TransactionSupport instance; private static final Logger log = LoggerFactory.getLogger(TransactionSupport.class); + + @PostConstruct + protected void init() { + instance = this; // to keep support for static methods and for not breaking tests. TODO make the code use only injected instances + } + + public TransactionManager getTransactionManager() { + return tm; + } /** * JBoss specific location of TransactionManager */ private static TransactionManager findTransactionManager() { - TransactionManager tm = EjbUtils.findResource(ContainerManagedResourceType.TRANSACTION_MANAGER); - return tm; + return instance.getTransactionManager(); } /** @@ -188,7 +201,16 @@ /** * Forces "REQUIRES_NEW" and executes given code in that scope + * Prefer this instance method by injecting the TransactionSuport bean */ + public <T> T executeInNewTx(TransactionMethod<T> code) { + return executeInNewTransaction(code); + } + + /** + * Forces "REQUIRES_NEW" and executes given code in that scope + */ + public static <T> T executeInNewTransaction(TransactionMethod<T> code) { T result = null; Transaction transaction = null; -- To view, visit http://gerrit.ovirt.org/35637 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia2a73887c76c85b17db6a3c2683d257aece10b59 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Roy Golan <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
