Merge branch '32-transaction-boundaries' into 'master'

Resolve "Transaction boundaries"

Closes #32

See merge request genesys-pgr/application-blocks!43

auditlog/src/main/java/org/genesys/blocks/auditlog/component/AuditTrailInterceptor.java

@@ -28,8 +28,8 @@ import java.util.Date;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.Stack;
 import java.util.UUID;
-import java.util.concurrent.atomic.AtomicLong;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -115,18 +115,12 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ
 	/** The time formatter. */
 	private Format timeFormatter;
 
-	/** Place to store audit logs before storing them to db */
-	private static final ThreadLocal<Set<TransactionAuditLog>> auditLogs = new ThreadLocal<Set<TransactionAuditLog>>() {
-		@Override
-		protected Set<TransactionAuditLog> initialValue() {
-			return new HashSet<>();
-		};
-	};
 
-	private static final ThreadLocal<AtomicLong> transactionStack = new ThreadLocal<AtomicLong>() {
+	/** Place to store audit logs before storing them to db */
+	private static final ThreadLocal<Stack<Set<TransactionAuditLog>>> auditLogStack = new ThreadLocal<Stack<Set<TransactionAuditLog>>>() {
 		@Override
-		protected AtomicLong initialValue() {
-			return new AtomicLong(0l);
+		protected Stack<Set<TransactionAuditLog>> initialValue() {
+			return new Stack<Set<TransactionAuditLog>>();
 		};
 	};
 
@@ -599,12 +593,12 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ
 			return;
 		}
 		TransactionAuditLog change = auditTrailService.auditLogEntry(AuditAction.UPDATE, entity, id, propertyName, previousState, currentState, referencedEntity);
-		if (auditLogs.get().remove(change)) {
+		if (auditLogStack.get().peek().remove(change)) {
 			LOG.trace("Replacing exising changelog {}", change);
 		} else {
 			LOG.trace("Adding new changelog {}", change);
 		}
-		auditLogs.get().add(change);
+		auditLogStack.get().peek().add(change);
 	}
 
 	/**
@@ -619,12 +613,12 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ
 	 */
 	private void recordDelete(final Object entity, final Long id, final String propertyName, final String state, final Class<?> referencedEntity) {
 		TransactionAuditLog delete = auditTrailService.auditLogEntry(AuditAction.DELETE, entity, id, propertyName, state, null, referencedEntity);
-		if (auditLogs.get().remove(delete)) {
+		if (auditLogStack.get().peek().remove(delete)) {
 			LOG.trace("Replacing exising changelog {}", delete);
 		} else {
 			LOG.trace("Adding new changelog {}", delete);
 		}
-		auditLogs.get().add(delete);
+		auditLogStack.get().peek().add(delete);
 	}
 
 	/**
@@ -684,8 +678,10 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ
 
 	@Override
 	public void afterTransactionBegin(final Transaction tx) {
-		transactionStack.get().getAndIncrement();
-		LOG.trace("Starting transaction level={}", transactionStack.get().get());
+		// Push new auditLogs to the stack
+		auditLogStack.get().push(new HashSet<>());
+
+		LOG.trace("Starting transaction level={}", auditLogStack.get().size());
 
 		// tx.registerSynchronization(new Synchronization() {
 		//
@@ -705,19 +701,25 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ
 	// Called before a transaction is committed (but not before rollback).
 	@Override
 	public void beforeTransactionCompletion(final Transaction tx) {
-		final long level = transactionStack.get().get();
-		LOG.trace("beforeTransactionCompletion transaction level={} auditlogs={}", level, auditLogs.get().size());
+
+		final long level = auditLogStack.get().size();
+		Set<TransactionAuditLog> currentAuditLogs = auditLogStack.get().pop(); // pop
+
+		LOG.trace("beforeTransactionCompletion transaction level={} auditlogs={}", level, currentAuditLogs.size());
 
 		// LOG.trace("Before transaction completion status={} tx={}",
 		// tx.getLocalStatus(), tx);
 
-		if (level == 1 && auditLogs.get().size() > 0) {
-			LOG.trace("We have {} auditlogs", auditLogs.get().size());
-			auditLogs.get().stream().forEach(auditLog -> {
+		if (currentAuditLogs.size() > 0) {
+			LOG.trace("We have {} auditlogs", currentAuditLogs.size());
+			currentAuditLogs.stream().forEach(auditLog -> {
 				LOG.debug("Audit log to save: {}", auditLog);
 			});
-			this.auditTrailService.addAuditLogs(auditLogs.get());
-			auditLogs.get().clear();
+			if (tx.wasRolledBack()) {
+				LOG.warn("Transaction was rolled back. Audit logs likely won't be persisted");
+			}
+			this.auditTrailService.addAuditLogs(currentAuditLogs);
+			currentAuditLogs.clear(); // not required anymore
 		}
 
 		super.beforeTransactionCompletion(tx);
@@ -737,18 +739,15 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ
 	// Called after a transaction is committed or rolled back.
 	@Override
 	public void afterTransactionCompletion(Transaction tx) {
-		final long level = transactionStack.get().decrementAndGet();
-		LOG.trace("afterTransactionCompletion transaction level={} auditlogs={}", level, auditLogs.get().size());
+
+		final long level = auditLogStack.get().size();
+		LOG.trace("afterTransactionCompletion transaction level={}", level);
 		if (tx.wasCommitted()) {
-			LOG.trace("Transaction was committed, have {} audit logs", auditLogs.get().size());
+			LOG.trace("Transaction was committed, level={}", level);
 		} else if (tx.wasRolledBack()) {
-			LOG.trace("Transaction was rolled back, have {} audit logs", auditLogs.get().size());
+			LOG.trace("Transaction was rolled back, level={}", level);
 		}
 
-		if (level == 0) {
-			// clear pending logs if back to level 0
-			auditLogs.get().clear();
-		}
 		super.afterTransactionCompletion(tx);
 	}