Skip to content

Commit

Permalink
finish unit testing FedoraTransactions and debug expiration, refactor…
Browse files Browse the repository at this point in the history 
… Transaction state to not rely on client code, Transaction#rollback and Transaction#commit now end internal sessions as appropriate, unit test Transaction
  • Loading branch information
@barmintor
barmintor committed May 29, 2013
1 parent e308b39 commit 291d0eb
Show file tree
Hide file tree
Showing 5 changed files with 191 additions and 43 deletions.
15 changes: 11 additions & 4 deletions fcrepo-http-api/src/main/java/org/fcrepo/api/FedoraTransactions.java
View file
Expand Up @@ -39,14 +39,22 @@ public class FedoraTransactions extends AbstractResource {
*/
private static Map<String, Transaction> TRANSACTIONS =
new ConcurrentHashMap<String, Transaction>();

public static final long REAP_INTERVAL = 1000;

@Scheduled(fixedRate=1000)
@Scheduled(fixedRate=REAP_INTERVAL)
public void removeAndRollbackExpired(){
synchronized(TRANSACTIONS){
Iterator<Entry<String, Transaction>> txs = TRANSACTIONS.entrySet().iterator();
while (txs.hasNext()){
Transaction tx = txs.next().getValue();
if (tx.getExpires().getTime() > System.currentTimeMillis()){
if (tx.getExpires().getTime() <= System.currentTimeMillis()){
try {
tx.rollback();
} catch (RepositoryException e) {
// TODO Not clear how to respond here
e.printStackTrace();
}
txs.remove();
}
}
Expand Down Expand Up @@ -97,7 +105,7 @@ public Transaction rollback(@PathParam("txid")
throw new RepositoryException("Transaction with id " + txid +
" is not available");
}
tx.setState(ROLLED_BACK);
tx.rollback();
return tx;
}

Expand All @@ -115,7 +123,6 @@ public Response createObjectInTransaction(@PathParam("txid")
final String path = toPath(pathlist);
objectService.createObject(tx.getSession(), path);
tx.updateExpiryDate();
tx.setState(DIRTY);
return Response.ok(path).build();
}

Expand Down
61 changes: 42 additions & 19 deletions fcrepo-http-api/src/test/java/org/fcrepo/api/FedoraTransactionsTest.java
View file
Expand Up @@ -4,6 +4,13 @@
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.verify;

import java.lang.reflect.Field;
import java.util.Date;
import java.util.Map;

import javax.jcr.RepositoryException;
import javax.jcr.Session;
Expand All @@ -15,15 +22,35 @@
import org.junit.Test;

public class FedoraTransactionsTest {

private static final String IS_A_TX = "foo";
private static final String NOT_A_TX = "bar";

FedoraTransactions resource;

Session mockSession;

Transaction mockTx;

@Before
public void setup() throws Exception {
resource = new FedoraTransactions();
mockSession = TestHelpers.mockSession(resource);
mockTx = mock(Transaction.class);
when(mockTx.getId()).thenReturn(IS_A_TX);
Field txsField = FedoraTransactions.class.getDeclaredField("TRANSACTIONS");
txsField.setAccessible(true);
@SuppressWarnings("unchecked")
Map<String, Transaction> txs = (Map<String, Transaction>) txsField.get(FedoraTransactions.class);
txs.put(IS_A_TX, mockTx);
}

@Test
public void testExpiration() throws Exception {
Date fiveSecondsAgo = new Date(System.currentTimeMillis() - 5000);
when(mockTx.getExpires()).thenReturn(fiveSecondsAgo);
resource.removeAndRollbackExpired();
verify(mockTx).rollback();
}

@Test
Expand All @@ -38,45 +65,41 @@ public void testCreateTx() throws Exception {

@Test
public void testGetTx() throws Exception {
Transaction tx = resource.createTransaction();
tx = resource.getTransaction(tx.getId());
Transaction tx = resource.getTransaction(IS_A_TX);

assertNotNull(tx);
assertNotNull(tx.getCreated());
assertNotNull(tx.getId());
assertTrue(tx.getState() == State.NEW);

try{
tx = resource.getTransaction(NOT_A_TX);
fail("Transaction retrieved for nonexistent id " + NOT_A_TX);
}catch(RepositoryException e){
// just checking that the exception occurs
}
}

@Test
public void testCommitTx() throws Exception {
Transaction tx = resource.createTransaction();
tx = resource.commit(tx.getId());
Transaction tx = resource.commit(IS_A_TX);

assertNotNull(tx);
assertNotNull(tx.getCreated());
assertNotNull(tx.getId());
assertTrue(tx.getState() == State.COMMITED);
verify(mockTx).commit();
try{
assertNull(resource.getTransaction(tx.getId()));
tx = resource.getTransaction(tx.getId());
fail("Transaction is available after commit");
}catch(RepositoryException e){
// just checking that the exception occurs
}

}

@Test
public void testRollbackTx() throws Exception {
Transaction tx = resource.createTransaction();
tx = resource.rollback(tx.getId());
Transaction tx = resource.rollback(IS_A_TX);

assertNotNull(tx);
assertNotNull(tx.getCreated());
assertNotNull(tx.getId());
assertTrue(tx.getState() == State.ROLLED_BACK);
verify(mockTx).rollback();
try{
assertNull(resource.getTransaction(tx.getId()));
fail("Transaction is available after rollback");
tx = resource.getTransaction(tx.getId());
fail("Transaction is available after commit");
}catch(RepositoryException e){
// just checking that the exception occurs
}
Expand Down
21 changes: 17 additions & 4 deletions fcrepo-http-api/src/test/java/org/fcrepo/integration/api/FedoraTransactionsIT.java
View file
Expand Up @@ -11,6 +11,7 @@
import org.codehaus.jackson.map.ObjectMapper;
import org.fcrepo.Transaction;
import org.fcrepo.Transaction.State;
import org.fcrepo.api.FedoraTransactions;
import org.junit.Test;

public class FedoraTransactionsIT extends AbstractResourceIT {
Expand Down Expand Up @@ -48,7 +49,11 @@ public void testCreateAndGetTransaction() throws Exception {

@Test
public void testCreateAndTimeoutTransaction() throws Exception {
/* create a tx */

long start = System.currentTimeMillis();
/* create a tx that will timeout in 10 ms */
long testTimeout = 10;
System.setProperty(Transaction.TIMEOUT_SYSTEM_PROPERTY, Long.toString(testTimeout));
HttpPost createTx = new HttpPost(serverAddress + "fcr:tx");
HttpResponse resp = execute(createTx);
assertTrue(resp.getStatusLine().getStatusCode() == 200);
Expand All @@ -64,16 +69,24 @@ public void testCreateAndTimeoutTransaction() throws Exception {
createTx.releaseConnection();

boolean expired = false;
long start = System.currentTimeMillis();
while (!expired && System.currentTimeMillis() - start < 3000) {
long diff = 0;
// the loop should be able to run for at least the tx reaping interval
while (!expired &&
(diff = (System.currentTimeMillis() - start)) < (2* FedoraTransactions.REAP_INTERVAL)) {
/* check that the tx is removed */
HttpGet getTx = new HttpGet(serverAddress + "fcr:tx/" + tx.getId());
resp = execute(getTx);
getTx.releaseConnection();
expired = (404 == resp.getStatusLine().getStatusCode());
}

assertTrue("Transaction did not expire", expired);
try {
assertTrue("Transaction did not expire", expired);
assertTrue(diff >= testTimeout);
} finally {
System.setProperty(Transaction.TIMEOUT_SYSTEM_PROPERTY,
Long.toString(Transaction.DEFAULT_TIMEOUT));
}
System.clearProperty("fcrepo4.tx.timeout");
}

Expand Down
61 changes: 45 additions & 16 deletions fcrepo-kernel/src/main/java/org/fcrepo/Transaction.java
View file
@@ -1,5 +1,6 @@
package org.fcrepo;

import java.util.Calendar;
import java.util.Date;
import java.util.UUID;

Expand All @@ -11,11 +12,20 @@
import javax.xml.bind.annotation.XmlRootElement;
import javax.xml.bind.annotation.XmlTransient;

import org.modeshape.jcr.JcrSession;

@XmlRootElement(name = "transaction")
@XmlAccessorType(XmlAccessType.FIELD)
public class Transaction {

public enum State {

// the default timeout is 3 minutes
public static final long DEFAULT_TIMEOUT =
3l * 60l * 1000l;

public static final String TIMEOUT_SYSTEM_PROPERTY =
"fcrepo4.tx.timeout";

public static enum State {
DIRTY, NEW, COMMITED, ROLLED_BACK;
}

Expand All @@ -29,7 +39,7 @@ public enum State {
private final Date created;

@XmlAttribute(name = "expires")
private Date expires;
private Calendar expires;

private State state = State.NEW;

Expand All @@ -45,6 +55,7 @@ public Transaction(Session session) {
this.session = session;
this.created = new Date();
this.id = UUID.randomUUID().toString();
this.expires = Calendar.getInstance();
this.updateExpiryDate();
}

Expand All @@ -60,35 +71,53 @@ public String getId() {
return id;
}

public void setState(State state) {
this.state = state;
}

public State getState() {
public State getState() throws RepositoryException {
if (this.session != null && this.session.hasPendingChanges()) {
return State.DIRTY;
}
return state;
}

public Date getExpires() {
return expires;
return expires.getTime();
}

public void commit() throws RepositoryException{
this.state = State.COMMITED;
public void commit() throws RepositoryException {
this.session.save();
this.state = State.COMMITED;
this.expire();
}

/**
* End the session, and mark for reaping
* @throws RepositoryException
*/
public void expire() throws RepositoryException {
this.session.logout();
this.expires.setTimeInMillis(System.currentTimeMillis());
}

/**
* Discard all unpersisted changes and expire
* @throws RepositoryException
*/
public void rollback() throws RepositoryException {
this.state = State.ROLLED_BACK;
this.session.refresh(false);
this.expire();
}


/**
* Roll forward the expiration date for recent activity
*/
public void updateExpiryDate() {
long duration;
if (System.getProperty("fcrepo4.tx.timeout") != null){
duration = Long.parseLong(System.getProperty("fcrepo4.tx.timeout"));
if (System.getProperty(TIMEOUT_SYSTEM_PROPERTY) != null){
duration = Long.parseLong(System.getProperty(TIMEOUT_SYSTEM_PROPERTY));
}else{
duration = 1000l * 60l * 3l;
duration = DEFAULT_TIMEOUT;
}
this.expires = new Date(System.currentTimeMillis() + duration);
this.expires.setTimeInMillis(System.currentTimeMillis() + duration);
}

}
76 changes: 76 additions & 0 deletions fcrepo-kernel/src/test/java/org/fcrepo/TransactionTest.java
View file
@@ -0,0 +1,76 @@
package org.fcrepo;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.verify;

import javax.jcr.RepositoryException;
import javax.jcr.Session;

import org.junit.Before;
import org.junit.Test;

public class TransactionTest {

private Transaction testObj;

private Session mockSession;

@Before
public void setUp(){
mockSession = mock(Session.class);
testObj = new Transaction(mockSession);
}

@Test
public void testRollback() throws RepositoryException {
testObj.rollback();
verify(mockSession).refresh(false);
verify(mockSession).logout();
assertEquals(Transaction.State.ROLLED_BACK, testObj.getState());
long update = testObj.getExpires().getTime();
assertTrue(update <= System.currentTimeMillis());
}

@Test
public void testCommit() throws RepositoryException {
testObj.commit();
verify(mockSession).save();
verify(mockSession).logout();
assertEquals(Transaction.State.COMMITED, testObj.getState());
long update = testObj.getExpires().getTime();
assertTrue(update <= System.currentTimeMillis());
}

@Test
public void testExpire() throws RepositoryException {
long orig = testObj.getExpires().getTime();
testObj.expire();
verify(mockSession, never()).save();
verify(mockSession).logout();
long update = testObj.getExpires().getTime();
assertTrue(update < orig);
assertTrue(update <= System.currentTimeMillis());
assertTrue(update < orig);
}

@Test
public void testExpiryUpdate() throws RepositoryException {
long orig = testObj.getExpires().getTime();
testObj.updateExpiryDate();
long update = testObj.getExpires().getTime();
assertTrue("Unexpected negative expiry delta: " + (update - orig),
update - orig >= 0);
}

@Test
public void testState() throws RepositoryException {
assertEquals(Transaction.State.NEW, testObj.getState());
when(mockSession.hasPendingChanges()).thenReturn(true, false);
assertEquals(Transaction.State.DIRTY, testObj.getState());
testObj.commit();
}
}

0 comments on commit 291d0eb

Please sign in to comment.