From 7d91c8ed3a1b16eb3f152f0d2051c358a7bbb99f Mon Sep 17 00:00:00 2001 From: John Blum Date: Thu, 2 Mar 2017 11:18:51 -0800 Subject: [PATCH] SGF-598 - Review and polish. --- .../GemfireTransactionManager.java | 83 +++++++++++++++---- ...ireTransactionManagerIntegrationTests.java | 8 +- 2 files changed, 73 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/springframework/data/gemfire/transaction/GemfireTransactionManager.java b/src/main/java/org/springframework/data/gemfire/transaction/GemfireTransactionManager.java index 2d5fd188..b998fc3c 100644 --- a/src/main/java/org/springframework/data/gemfire/transaction/GemfireTransactionManager.java +++ b/src/main/java/org/springframework/data/gemfire/transaction/GemfireTransactionManager.java @@ -20,6 +20,7 @@ package org.springframework.data.gemfire.transaction; import static org.springframework.data.gemfire.transaction.GemfireTransactionManager.CacheHolder.newCacheHolder; import static org.springframework.data.gemfire.transaction.GemfireTransactionManager.CacheTransactionObject.newCacheTransactionObject; +import java.util.Optional; import java.util.concurrent.TimeUnit; import org.apache.geode.cache.CacheTransactionManager; @@ -58,11 +59,17 @@ import org.springframework.util.Assert; * * @author Costin Leau * @author John Blum + * @see org.apache.geode.CopyHelper#copy(Object) * @see org.apache.geode.cache.GemFireCache#setCopyOnRead(boolean) * @see org.apache.geode.cache.CacheTransactionManager - * @see org.apache.geode.cache.Region#get(Object) - * @see org.apache.geode.CopyHelper#copy(Object) + * @see org.apache.geode.cache.Region + * @see org.apache.geode.cache.TransactionId + * @see org.springframework.beans.factory.InitializingBean + * @see org.springframework.transaction.PlatformTransactionManager + * @see org.springframework.transaction.TransactionDefinition * @see org.springframework.transaction.support.AbstractPlatformTransactionManager + * @see org.springframework.transaction.support.ResourceTransactionManager + * @see org.springframework.transaction.support.TransactionSynchronizationManager * @see #setCopyOnRead(boolean) */ @SuppressWarnings("unused") @@ -185,7 +192,7 @@ public class GemfireTransactionManager extends AbstractPlatformTransactionManage protected Object doSuspend(Object transaction) throws TransactionException { if (getCacheTransactionManager().suspend() != null) { TransactionSynchronizationManager.unbindResource(getCache()); - return ((CacheTransactionObject) transaction).setAndReturnExistingHolder(null); + return ((CacheTransactionObject) transaction).setAndGetExistingHolder(null); } return null; @@ -225,8 +232,7 @@ public class GemfireTransactionManager extends AbstractPlatformTransactionManage } catch (IllegalStateException e) { throw new NoTransactionException( - "No transaction is associated with the current thread; are multiple transaction managers present?", - e); + "No transaction is associated with the current thread; are multiple transaction managers present?", e); } } @@ -330,87 +336,134 @@ public class GemfireTransactionManager extends AbstractPlatformTransactionManage return getCache(); } + /*** + * Sets the timeout used to wait for the GemFire cache transaction to resume. + * + * @param resumeWaitTime long value with the timeout used to wait for the GemFire cache transaction to resume. + * @see org.apache.geode.cache.CacheTransactionManager#tryResume(TransactionId, long, TimeUnit) + */ public void setResumeWaitTime(Long resumeWaitTime) { this.resumeWaitTime = resumeWaitTime; } + /*** + * Returns the timeout used to wait for the GemFire cache transaction to resume. + * + * @return the long value with the timeout used to wait for the GemFire cache transaction to resume. + * @see org.apache.geode.cache.CacheTransactionManager#tryResume(TransactionId, long, TimeUnit) + */ protected Long getResumeWaitTime() { return this.resumeWaitTime; } + /** + * Determines whether the user specified a wait time for resuming a GemFire cache transaction. + * + * @return a boolean value to indicate whether the user specified a wait time + * for resuming a GemFire cache transaction. + * @see org.apache.geode.cache.CacheTransactionManager#tryResume(TransactionId, long, TimeUnit) + * @see #getResumeWaitTime() + */ protected boolean isResumeWaitTimeSet() { Long resumeWaitTime = getResumeWaitTime(); return (resumeWaitTime != null && resumeWaitTime > 0); } + /** + * Sets the {@link TimeUnit} used in the wait timeout when resuming a GemFire cache transaction. + * + * @param resumeWaitTimeUnit {@link TimeUnit} used in the wait timeout when resuming a GemFire cache transaction. + * @see org.apache.geode.cache.CacheTransactionManager#tryResume(TransactionId, long, TimeUnit) + */ public void setResumeWaitTimeUnit(TimeUnit resumeWaitTimeUnit) { this.resumeWaitTimeUnit = resumeWaitTimeUnit; } + /** + * Returns the {@link TimeUnit} used in the wait timeout when resuming a GemFire cache transaction. + * + * Defaults to {@link TimeUnit#SECONDS}. + * + * @return the {@link TimeUnit} used in the wait timeout when resuming a GemFire cache transaction. + * @see org.apache.geode.cache.CacheTransactionManager#tryResume(TransactionId, long, TimeUnit) + */ protected TimeUnit getResumeWaitTimeUnit() { - TimeUnit localResumeWaitTimeUnit = this.resumeWaitTimeUnit; - return (localResumeWaitTimeUnit != null ? localResumeWaitTimeUnit : DEFAULT_RESUME_WAIT_TIME_UNIT); + return Optional.ofNullable(this.resumeWaitTimeUnit).orElse(DEFAULT_RESUME_WAIT_TIME_UNIT); } /** - * GemfireTM local transaction object. + * GemFire local transaction object. * * @author Costin Leau + * @author John Blum */ protected static class CacheTransactionObject { private CacheHolder cacheHolder; + /* (non-Javadoc) */ static CacheTransactionObject newCacheTransactionObject(CacheHolder cacheHolder) { CacheTransactionObject transactionObject = new CacheTransactionObject(); transactionObject.setHolder(cacheHolder); return transactionObject; } + /* (non-Javadoc) */ boolean isHolding() { return (getHolder() != null); } + /* (non-Javadoc) */ CacheHolder getHolder() { return this.cacheHolder; } + /* (non-Javadoc) */ void setHolder(CacheHolder holder) { this.cacheHolder = holder; } - CacheHolder setAndGetHolder(CacheHolder holder) { - setHolder(holder); - return getHolder(); - } - - CacheHolder setAndReturnExistingHolder(CacheHolder cacheHolder) { + /* (non-Javadoc) */ + CacheHolder setAndGetExistingHolder(CacheHolder cacheHolder) { CacheHolder existingHolder = getHolder(); setHolder(cacheHolder); return existingHolder; } + + /* (non-Javadoc) */ + CacheHolder setAndGetHolder(CacheHolder holder) { + setHolder(holder); + return getHolder(); + } } + /** + * Holder of GemFire cache transaction state. + */ protected static class CacheHolder { private boolean rollbackOnly = false; private TransactionId transactionId; + /* (non-Javadoc) */ static CacheHolder newCacheHolder(TransactionId transactionId) { CacheHolder cacheHolder = new CacheHolder(); cacheHolder.transactionId = transactionId; return cacheHolder; } - public boolean isRollbackOnly() { + /* (non-Javadoc) */ + boolean isRollbackOnly() { return this.rollbackOnly; } + /* (non-Javadoc) */ void setRollbackOnly() { this.rollbackOnly = true; } + /* (non-Javadoc) */ TransactionId getTransactionId() { return this.transactionId; } diff --git a/src/test/java/org/springframework/data/gemfire/transaction/GemfireTransactionManagerIntegrationTests.java b/src/test/java/org/springframework/data/gemfire/transaction/GemfireTransactionManagerIntegrationTests.java index d13b8885..5b8a8d91 100644 --- a/src/test/java/org/springframework/data/gemfire/transaction/GemfireTransactionManagerIntegrationTests.java +++ b/src/test/java/org/springframework/data/gemfire/transaction/GemfireTransactionManagerIntegrationTests.java @@ -58,7 +58,7 @@ import org.springframework.util.Assert; GemfireTransactionManagerIntegrationTests.GemfireTransactionManagerIntegrationTestsConfiguration.class) public class GemfireTransactionManagerIntegrationTests { - protected static final String GEMFIRE_LOG_LEVEL = "warning"; + static final String GEMFIRE_LOG_LEVEL = "warning"; @Resource(name = "Example") @SuppressWarnings("unused") @@ -100,7 +100,9 @@ public class GemfireTransactionManagerIntegrationTests { } @Bean - SuspendAndResumeCacheTransactionsRepository suspendAndResumeCacheTransactionsRepository(GemFireCache gemFireCache) { + SuspendAndResumeCacheTransactionsRepository suspendAndResumeCacheTransactionsRepository( + GemFireCache gemFireCache) { + return new SuspendAndResumeCacheTransactionsRepository(gemFireCache.getRegion("Example")); } @@ -150,7 +152,7 @@ public class GemfireTransactionManagerIntegrationTests { static class SuspendAndResumeCacheTransactionsRepository { @SuppressWarnings("all") - private Region example; + Region example; SuspendAndResumeCacheTransactionsRepository(Region example) { Assert.notNull(example, "Region must not be null");