Merge pull request #6 from nuxeo/GERONIMO-6805
GERONIMO-6805: Use a time SPI abstraction to provide time to the transaction manager
diff --git a/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/CurrentTimeMsProvider.java b/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/CurrentTimeMsProvider.java
new file mode 100644
index 0000000..6b3426b
--- /dev/null
+++ b/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/CurrentTimeMsProvider.java
@@ -0,0 +1,32 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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
+ *
+ * http://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.apache.geronimo.transaction.manager;
+
+/**
+ * An abstraction of a time provider with millisecond resolution.
+ */
+@FunctionalInterface
+public interface CurrentTimeMsProvider {
+
+ /**
+ * Returns the current time, in milliseconds.
+ *
+ * @return the current time, in milliseconds
+ */
+ long now();
+
+}
diff --git a/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/SystemCurrentTime.java b/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/SystemCurrentTime.java
new file mode 100644
index 0000000..3d23e9f
--- /dev/null
+++ b/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/SystemCurrentTime.java
@@ -0,0 +1,38 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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
+ *
+ * http://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.apache.geronimo.transaction.manager;
+
+/**
+ * A time provider returning the standard {@link System#currentTimeMillis()}.
+ */
+public class SystemCurrentTime implements CurrentTimeMsProvider {
+
+ /**
+ * A time provider returning the standard {@link System#currentTimeMillis()}.
+ */
+ public static final CurrentTimeMsProvider INSTANCE = new SystemCurrentTime();
+
+ private SystemCurrentTime() {
+ // singleton
+ }
+
+ @Override
+ public long now() {
+ return System.currentTimeMillis();
+ }
+
+}
diff --git a/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionImpl.java b/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionImpl.java
index 0b8ad6b..e40c37a 100644
--- a/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionImpl.java
+++ b/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionImpl.java
@@ -50,6 +50,7 @@
private final TransactionManagerImpl txManager;
private final Xid xid;
+ private final CurrentTimeMsProvider timeProvider;
private final long timeout;
private final List<Synchronization> syncList = new ArrayList<Synchronization>(5);
private final List<Synchronization> interposedSyncList = new ArrayList<Synchronization>(3);
@@ -66,10 +67,19 @@
this(txManager.getXidFactory().createXid(), txManager, transactionTimeoutMilliseconds);
}
+ TransactionImpl(TransactionManagerImpl txManager, long transactionTimeoutMilliseconds, CurrentTimeMsProvider timeProvider) throws SystemException {
+ this(txManager.getXidFactory().createXid(), txManager, transactionTimeoutMilliseconds, timeProvider);
+ }
+
TransactionImpl(Xid xid, TransactionManagerImpl txManager, long transactionTimeoutMilliseconds) throws SystemException {
+ this(xid, txManager, transactionTimeoutMilliseconds, null);
+ }
+
+ TransactionImpl(Xid xid, TransactionManagerImpl txManager, long transactionTimeoutMilliseconds, CurrentTimeMsProvider timeProvider) throws SystemException {
this.txManager = txManager;
this.xid = xid;
- this.timeout = transactionTimeoutMilliseconds + TransactionTimer.getCurrentTime();
+ this.timeProvider = timeProvider == null ? SystemCurrentTime.INSTANCE : timeProvider;
+ this.timeout = transactionTimeoutMilliseconds + this.timeProvider.now();
try {
txManager.getTransactionLog().begin(xid);
} catch (LogException e) {
@@ -87,6 +97,7 @@
this.txManager = txManager;
this.xid = xid;
status = Status.STATUS_PREPARED;
+ timeProvider = SystemCurrentTime.INSTANCE;
//TODO is this a good idea?
this.timeout = Long.MAX_VALUE;
}
@@ -208,7 +219,7 @@
Xid branchId = txManager.getXidFactory().createBranch(xid, resourceManagers.size() + 1);
xaRes.start(branchId, XAResource.TMNOFLAGS);
//Set the xaresource timeout in seconds to match the time left in this tx.
- xaRes.setTransactionTimeout((int)(timeout - TransactionTimer.getCurrentTime())/1000);
+ xaRes.setTransactionTimeout((int)(timeout - timeProvider.now())/1000);
activeXaResources.put(xaRes, addBranchXid(xaRes, branchId));
return true;
} catch (XAException e) {
@@ -262,7 +273,7 @@
beforePrepare();
try {
- if (TransactionTimer.getCurrentTime() > timeout) {
+ if (timeProvider.now() > timeout) {
markRollbackCause(new Exception("Transaction has timed out"));
status = Status.STATUS_MARKED_ROLLBACK;
}
diff --git a/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionManagerImpl.java b/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionManagerImpl.java
index 1db17f0..81c975f 100644
--- a/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionManagerImpl.java
+++ b/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionManagerImpl.java
@@ -60,6 +60,7 @@
private final TransactionLog transactionLog;
private final XidFactory xidFactory;
private final int defaultTransactionTimeoutMilliseconds;
+ private final CurrentTimeMsProvider timeProvider;
private final ThreadLocal<Long> transactionTimeoutMilliseconds = new ThreadLocal<Long>();
private final ThreadLocal<Transaction> threadTx = new ThreadLocal<Transaction>();
private final ConcurrentHashMap<Transaction, Thread> associatedTransactions = new ConcurrentHashMap<Transaction, Thread>();
@@ -95,10 +96,16 @@
}
public TransactionManagerImpl(int defaultTransactionTimeoutSeconds, XidFactory xidFactory, TransactionLog transactionLog) throws XAException {
+ this(defaultTransactionTimeoutSeconds, xidFactory, transactionLog, null);
+ }
+
+ public TransactionManagerImpl(int defaultTransactionTimeoutSeconds, XidFactory xidFactory,
+ TransactionLog transactionLog, CurrentTimeMsProvider timeProvider) throws XAException {
if (defaultTransactionTimeoutSeconds <= 0) {
throw new IllegalArgumentException("defaultTransactionTimeoutSeconds must be positive: attempted value: " + defaultTransactionTimeoutSeconds);
}
this.defaultTransactionTimeoutMilliseconds = defaultTransactionTimeoutSeconds * 1000;
+ this.timeProvider = timeProvider;
if (transactionLog == null) {
this.transactionLog = new UnrecoverableLog();
@@ -132,7 +139,7 @@
fireThreadAssociated(tx);
activeCount.getAndIncrement();
}
- }
+ }
private void unassociate() {
Transaction tx = getTransaction();
@@ -168,7 +175,7 @@
if (getStatus() != Status.STATUS_NO_TRANSACTION) {
throw new NotSupportedException("Nested Transactions are not supported");
}
- TransactionImpl tx = new TransactionImpl(this, getTransactionTimeoutMilliseconds(transactionTimeoutMilliseconds));
+ TransactionImpl tx = new TransactionImpl(this, getTransactionTimeoutMilliseconds(transactionTimeoutMilliseconds), timeProvider);
// timeoutTimer.schedule(tx, getTransactionTimeoutMilliseconds(transactionTimeoutMilliseconds));
try {
associate(tx);
@@ -197,7 +204,7 @@
if (!(tx instanceof TransactionImpl)) {
throw new InvalidTransactionException("Cannot resume foreign transaction: " + tx);
}
-
+
associate((TransactionImpl) tx);
}
}
@@ -224,7 +231,7 @@
}
public Object getTransactionKey() {
- TransactionImpl tx = (TransactionImpl) getTransaction();
+ TransactionImpl tx = (TransactionImpl) getTransaction();
return tx == null ? null: tx.getTransactionKey();
}
@@ -286,7 +293,7 @@
if (transactionTimeoutMilliseconds < 0) {
throw new SystemException("transaction timeout must be positive or 0 to reset to default");
}
- return new TransactionImpl(xid, this, getTransactionTimeoutMilliseconds(transactionTimeoutMilliseconds));
+ return new TransactionImpl(xid, this, getTransactionTimeoutMilliseconds(transactionTimeoutMilliseconds), timeProvider);
}
public void commit(Transaction tx, boolean onePhase) throws XAException {
diff --git a/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionTimer.java b/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionTimer.java
deleted file mode 100644
index c26aad0..0000000
--- a/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionTimer.java
+++ /dev/null
@@ -1,64 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You 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
- *
- * http://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.apache.geronimo.transaction.manager;
-
-import java.security.AccessController;
-import java.security.PrivilegedAction;
-
-/**
- * TODO improve shutdown
- *
- * @version $Revision$ $Date$
- */
-public class TransactionTimer {
- private static volatile long currentTime;
-
- private static class CurrentTime extends Thread {
- protected CurrentTime() {
- currentTime = System.currentTimeMillis();
- setContextClassLoader(null);
- }
-
- public void run() {
- for (; ;) {
- currentTime = System.currentTimeMillis();
- try {
- Thread.sleep(1000);
- } catch (InterruptedException e) {
- // Ignore exception
- }
- }
- }
- }
-
- static {
- AccessController.doPrivileged(new PrivilegedAction() {
- public Object run() {
- CurrentTime tm = new CurrentTime();
- tm.setDaemon(true);
- tm.start();
- return null;
- }
- });
- }
-
- public static long getCurrentTime() {
- return currentTime;
- }
-
-}
diff --git a/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/TransactionManagerImplTest.java b/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/TransactionManagerImplTest.java
index c4e26a4..a4d632d 100644
--- a/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/TransactionManagerImplTest.java
+++ b/geronimo-transaction/src/test/java/org/apache/geronimo/transaction/manager/TransactionManagerImplTest.java
@@ -45,19 +45,29 @@
TransactionManagerImpl tm;
+ long timeDelta;
+
protected void setUp() throws Exception {
tm = createTransactionManager();
+ timeDelta = 0;
}
private TransactionManagerImpl createTransactionManager() throws XAException {
return new TransactionManagerImpl(10,
- new XidFactoryImpl("WHAT DO WE CALL IT?".getBytes()), transactionLog);
+ new XidFactoryImpl("WHAT DO WE CALL IT?".getBytes()), transactionLog, new TestTimeProvider());
}
protected void tearDown() throws Exception {
tm = null;
}
+ public class TestTimeProvider implements CurrentTimeMsProvider {
+ @Override
+ public long now() {
+ return System.currentTimeMillis() + timeDelta;
+ }
+
+ }
public void testNoResourcesCommit() throws Exception {
assertEquals(Status.STATUS_NO_TRANSACTION, tm.getStatus());
tm.begin();
@@ -312,8 +322,7 @@
long timeout = tm.getTransactionTimeoutMilliseconds(0L);
tm.setTransactionTimeout((int)timeout/4000);
tm.begin();
- System.out.println("Test to sleep for " + timeout + " millisecs");
- Thread.sleep(timeout);
+ timeDelta += timeout;
try
{
tm.commit();
@@ -325,34 +334,33 @@
// Now test if the default timeout is active
tm.begin();
- System.out.println("Test to sleep for " + (timeout/2) + " millisecs");
- Thread.sleep((timeout/2));
+ timeDelta += timeout/2;
tm.commit();
// Its a failure if exception occurs.
- }
-
+ }
+
// resume throws InvalidTransactionException on completed tx (via commit)
public void testResume1() throws Exception {
Transaction tx;
assertEquals(Status.STATUS_NO_TRANSACTION, tm.getStatus());
- tm.begin();
+ tm.begin();
assertEquals(Status.STATUS_ACTIVE, tm.getStatus());
tx = tm.getTransaction();
assertNotNull(tx);
assertEquals(Status.STATUS_ACTIVE, tx.getStatus());
-
+
tm.commit();
assertEquals(Status.STATUS_NO_TRANSACTION, tm.getStatus());
assertNull(tm.getTransaction());
-
+
try {
tm.resume(tx);
fail();
} catch (InvalidTransactionException e) {
// expected
- }
+ }
}
-
+
// resume throws InvalidTransactionException on completed tx (via rollback)
public void testResume2() throws Exception {
Transaction tx;
@@ -373,16 +381,16 @@
tm.rollback();
assertEquals(Status.STATUS_NO_TRANSACTION, tm.getStatus());
- assertNull(tm.getTransaction());
-
+ assertNull(tm.getTransaction());
+
try {
tm.resume(tx);
fail();
} catch (InvalidTransactionException e) {
// expected
- }
+ }
}
-
+
// resume work on null tx
public void testResume3() throws Exception {
Transaction tx;
@@ -400,16 +408,16 @@
// tx should be null
tx = tm.suspend();
assertNull(tx);
-
+
try {
tm.resume(tx);
} catch (InvalidTransactionException e) {
// null is considered valid so we don't expect InvalidTransactionException here
e.printStackTrace();
fail();
- }
+ }
}
-
+
// resume works on any valid tx
public void testResume4() throws Exception {
Transaction tx;
@@ -428,10 +436,10 @@
// tx is considered valid so we don't expect InvalidTransactionException here
e.printStackTrace();
fail();
- }
+ }
tm.commit();
assertEquals(Status.STATUS_NO_TRANSACTION, tm.getStatus());
- assertNull(tm.getTransaction());
+ assertNull(tm.getTransaction());
}
}
