[New] added test to check acquiring locks

Added a test which checks that a TX fails if a lock can not be acquired,
but succeeds before the timeout to acquire the lock is exceeded.

Further fixed the brittle tests by using a different semaphore to
synchronize the threads.

src/main/java/ch/eitchnet/xmlpers/api/PersistenceConstants.java

@@ -33,5 +33,5 @@ public class PersistenceConstants {
 	public static final String PROP_BASEPATH = PROP_PREFIX + "basePath";
 	public static final String PROP_DAO_FACTORY_CLASS = PROP_PREFIX + "daoFactoryClass";
 	public static final String PROP_XML_IO_MOD = PROP_PREFIX + "ioMode";
-	public static final String PROP_XML_LOCK_TIME_MILLIS = PROP_PREFIX + "lockTimeSeconds";
+	public static final String PROP_LOCK_TIME_MILLIS = PROP_PREFIX + "lockTimeSeconds";
 }

src/main/java/ch/eitchnet/xmlpers/impl/DefaultPersistenceManager.java

@@ -32,6 +32,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import ch.eitchnet.utils.helper.PropertiesHelper;
+import ch.eitchnet.utils.helper.StringHelper;
 import ch.eitchnet.xmlpers.api.IoMode;
 import ch.eitchnet.xmlpers.api.PersistenceConstants;
 import ch.eitchnet.xmlpers.api.PersistenceContextFactoryDelegator;
@@ -69,13 +70,14 @@ public class DefaultPersistenceManager implements PersistenceManager {
 				IoMode.DOM.name());
 		IoMode ioMode = IoMode.valueOf(ioModeS);
 		long lockTime = PropertiesHelper.getPropertyLong(properties, context,
-				PersistenceConstants.PROP_XML_LOCK_TIME_MILLIS, 10000L);
+				PersistenceConstants.PROP_LOCK_TIME_MILLIS, 10000L);
 
 		// set lock time on LockableObject
 		try {
 			Field lockTimeField = LockableObject.class.getDeclaredField("tryLockTime");//$NON-NLS-1$
 			lockTimeField.setAccessible(true);
 			lockTimeField.setLong(null, lockTime);
+			logger.info("Using a max lock acquire time of " + StringHelper.formatMillisecondsDuration(lockTime)); //$NON-NLS-1$
 		} catch (SecurityException | NoSuchFieldException | IllegalArgumentException | IllegalAccessException e) {
 			throw new RuntimeException("Failed to configure tryLockTime on LockableObject!", e); //$NON-NLS-1$
 		}

src/test/java/ch/eitchnet/xmlpers/test/LockingTest.java

@@ -21,8 +21,12 @@
  */
 package ch.eitchnet.xmlpers.test;
 
+import static ch.eitchnet.xmlpers.test.impl.TestConstants.TYPE_RES;
+import static ch.eitchnet.xmlpers.test.model.ModelBuilder.RES_TYPE;
 import static ch.eitchnet.xmlpers.test.model.ModelBuilder.createResource;
+import static ch.eitchnet.xmlpers.test.model.ModelBuilder.updateResource;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -35,6 +39,7 @@ import org.junit.Test;
 import ch.eitchnet.xmlpers.api.IoMode;
 import ch.eitchnet.xmlpers.api.PersistenceConstants;
 import ch.eitchnet.xmlpers.api.PersistenceTransaction;
+import ch.eitchnet.xmlpers.objref.IdOfSubTypeRef;
 import ch.eitchnet.xmlpers.objref.LockableObject;
 import ch.eitchnet.xmlpers.test.model.Resource;
 
@@ -46,7 +51,8 @@ public class LockingTest extends AbstractPersistenceTest {
 
 	private static final String BASE_PATH = "target/db/LockingTest/"; //$NON-NLS-1$
 
-	long waitForWorkersTime = LockableObject.getLockTime() + 2000L;
+	private long waitForWorkersTime;
+	private boolean run;
 
 	@BeforeClass
 	public static void beforeClass() {
@@ -57,18 +63,21 @@ public class LockingTest extends AbstractPersistenceTest {
 	public void before() {
 		Properties properties = new Properties();
 		properties.setProperty(PersistenceConstants.PROP_BASEPATH, BASE_PATH + IoMode.DOM.name());
+		properties.setProperty(PersistenceConstants.PROP_LOCK_TIME_MILLIS, Long.toString(500L));
 		setup(properties);
+
+		this.waitForWorkersTime = LockableObject.getLockTime() + (long) ((double) this.getWaitForWorkersTime() * .2);
 	}
 
 	@Test
 	public void shouldLockObjects() throws InterruptedException {
 
-		List<Worker> workers = new ArrayList<>(5);
+		List<CreateResourceWorker> workers = new ArrayList<>(5);
 
+		String resoureId = "worker"; //$NON-NLS-1$
 		for (int i = 0; i < 5; i++) {
-
+			String workerName = resoureId + "_" + i; //$NON-NLS-1$
-			String workerName = "worker_" + i; //$NON-NLS-1$
+			CreateResourceWorker worker = new CreateResourceWorker(workerName, workerName);
-			Worker worker = new Worker(workerName, workerName);
 			worker.start();
 			workers.add(worker);
 			logger.info("Setup thread " + worker.getName()); //$NON-NLS-1$
@@ -80,14 +89,13 @@ public class LockingTest extends AbstractPersistenceTest {
 	}
 
 	@Test
-	public void shouldFailIfLockNotAcquirable() throws InterruptedException {
+	public void shouldFailIfResourceAlreadyExists() throws InterruptedException {
 
-		List<Worker> workers = new ArrayList<>(5);
+		List<CreateResourceWorker> workers = new ArrayList<>(5);
 
+		String resourceId = "createWorkerRes"; //$NON-NLS-1$
 		for (int i = 0; i < 5; i++) {
-
+			CreateResourceWorker worker = new CreateResourceWorker(resourceId, resourceId);
-			String workerName = "workerRes"; //$NON-NLS-1$
-			Worker worker = new Worker(workerName, workerName);
 			worker.start();
 			workers.add(worker);
 			logger.info("Setup thread " + worker.getName()); //$NON-NLS-1$
@@ -98,18 +106,41 @@ public class LockingTest extends AbstractPersistenceTest {
 		assertEquals("Only one thread should be able to perform the TX!", 1, nrOfSuccess); //$NON-NLS-1$
 	}
 
-	private int runWorkers(List<Worker> workers) throws InterruptedException {
+	@Test
+	public void shouldFailUpdateIfLockNotAcquirable() throws InterruptedException {
 
-		synchronized (this) {
+		// prepare workers
-			this.notifyAll();
+		List<UpdateResourceWorker> workers = new ArrayList<>(5);
+		String resourceId = "updatWorkerRes"; //$NON-NLS-1$
+		for (int i = 0; i < 5; i++) {
+			String workerName = resourceId + "_" + i; //$NON-NLS-1$
+			UpdateResourceWorker worker = new UpdateResourceWorker(workerName, resourceId);
+			worker.start();
+			workers.add(worker);
+			logger.info("Setup thread " + worker.getName()); //$NON-NLS-1$
 		}
 
-		for (Worker worker : workers) {
+		// create resource which is to be updated
-			worker.join(this.waitForWorkersTime + 2000L);
+		try (PersistenceTransaction tx = this.persistenceManager.openTx()) {
+			Resource resource = createResource(resourceId);
+			tx.getObjectDao().add(resource);
+		}
+
+		int nrOfSuccess = runWorkers(workers);
+
+		assertEquals("Only one thread should be able to perform the TX!", 1, nrOfSuccess); //$NON-NLS-1$
+	}
+
+	private int runWorkers(List<? extends AbstractWorker> workers) throws InterruptedException {
+
+		this.setRun(true);
+
+		for (AbstractWorker worker : workers) {
+			worker.join(this.getWaitForWorkersTime() + 2000L);
 		}
 
 		int nrOfSuccess = 0;
-		for (Worker worker : workers) {
+		for (AbstractWorker worker : workers) {
 			if (worker.isSuccess())
 				nrOfSuccess++;
 		}
@@ -117,22 +148,34 @@ public class LockingTest extends AbstractPersistenceTest {
 		return nrOfSuccess;
 	}
 
-	public class Worker extends Thread {
+	public long getWaitForWorkersTime() {
+		return this.waitForWorkersTime;
+	}
 
-		private boolean success;
+	public boolean isRun() {
-		private String resourceId;
+		return this.run;
+	}
 
-		public Worker(String name, String resourceId) {
+	public void setRun(boolean run) {
+		this.run = run;
+	}
+
+	public abstract class AbstractWorker extends Thread {
+
+		protected boolean success;
+		protected String resourceId;
+
+		public AbstractWorker(String name, String resourceId) {
 			super(name);
 			this.resourceId = resourceId;
 		}
 
 		public void run() {
 
-			synchronized (LockingTest.this) {
+			logger.info("Waiting for ok to work..."); //$NON-NLS-1$
+			while (!LockingTest.this.isRun()) {
 				try {
-					logger.info("Waiting for ok to work..."); //$NON-NLS-1$
+					Thread.sleep(10L);
-					LockingTest.this.wait();
 				} catch (InterruptedException e) {
 					throw new RuntimeException(e);
 				}
@@ -140,23 +183,55 @@ public class LockingTest extends AbstractPersistenceTest {
 
 			logger.info("Starting work..."); //$NON-NLS-1$
 			try (PersistenceTransaction tx = LockingTest.this.persistenceManager.openTx()) {
+				doWork(tx);
 
-				Resource resource = createResource(this.resourceId);
+				try {
-				tx.getObjectDao().add(resource);
+					Thread.sleep(getWaitForWorkersTime());
-			}
+				} catch (InterruptedException e) {
-			this.success = true;
+					throw new RuntimeException(e);
+				}
 
-			try {
+				this.success = true;
-				Thread.sleep(LockingTest.this.waitForWorkersTime);
-			} catch (InterruptedException e) {
-				throw new RuntimeException(e);
 			}
 
 			logger.info("Work completed."); //$NON-NLS-1$
 		}
 
+		protected abstract void doWork(PersistenceTransaction tx);
+
 		public boolean isSuccess() {
 			return this.success;
 		}
 	}
+
+	public class CreateResourceWorker extends AbstractWorker {
+
+		public CreateResourceWorker(String name, String resourceId) {
+			super(name, resourceId);
+		}
+
+		@Override
+		protected void doWork(PersistenceTransaction tx) {
+			Resource resource = createResource(this.resourceId);
+			tx.getObjectDao().add(resource);
+		}
+	}
+
+	public class UpdateResourceWorker extends AbstractWorker {
+
+		public UpdateResourceWorker(String name, String resourceId) {
+			super(name, resourceId);
+		}
+
+		@Override
+		protected void doWork(PersistenceTransaction tx) {
+
+			IdOfSubTypeRef objectRef = tx.getObjectRefCache().getIdOfSubTypeRef(TYPE_RES, RES_TYPE, this.resourceId);
+			Resource resource = tx.getObjectDao().queryById(objectRef);
+			assertNotNull(resource);
+			updateResource(resource);
+
+			tx.getObjectDao().update(resource);
+		}
+	}
 }