Improved collection handling in Audit Log

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

@@ -400,7 +400,6 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ
 			LOG.trace("Class {} is not audited", pc.getOwner().getClass());
 			return;
 		}
-		LOG.trace("Collection remove: key={} coll={}", key, collection);
 
 		final Class<? extends Object> ownerClass = pc.getOwner().getClass();
 		final String propertyName = pc.getRole().substring(pc.getRole().lastIndexOf('.') + 1);
@@ -409,9 +408,22 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ
 
 		Collection<Object> deleted = new HashSet<>();
 
+		if (pc.getValue() == null) {
+			LOG.trace("onCollectionRemove is empty, no change", key);
+			return;
+		}
+		
 		if (pc.getValue() instanceof Collection<?>) {
 			deleted.addAll((Collection<?>) pc.getValue());
 		}
+		
+		if (deleted.size() == 0) {
+			LOG.trace("onCollectionRemove is empty, no change", key);
+			return;
+		}
+		
+		Serializable snap = pc.getStoredSnapshot();
+		LOG.trace("Collection remove: key={} coll={} snap={}", key, collection, snap);
 
 		// If remaining is EntityId, convert to ID only
 		Class<?> referencedEntity = null;
@@ -437,6 +449,10 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ
 			LOG.trace("Class {} is not audited", pc.getOwner().getClass());
 			return;
 		}
+		// if (pc.empty()) {
+		// LOG.trace("onCollectionUpdate is empty, no change", key);
+		// return;
+		// }
 		LOG.trace("Collection update: key={} coll={}", key, collection);
 
 		final Class<? extends Object> ownerClass = pc.getOwner().getClass();
@@ -578,6 +594,10 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ
 	private void recordChange(final Object entity, final Long id, final String propertyName, final String previousState, final String currentState,
 			final Class<?> referencedEntity) {
 
+		if (StringUtils.equals(previousState, currentState)) {
+			LOG.trace("No state change {}.{} {}=={}", entity.getClass(), id, previousState, currentState);
+			return;
+		}
 		TransactionAuditLog change = auditTrailService.auditLogEntry(AuditAction.UPDATE, entity, id, propertyName, previousState, currentState, referencedEntity);
 		if (auditLogs.get().remove(change)) {
 			LOG.trace("Replacing exising changelog {}", change);

auditlog/src/test/java/org/genesys/blocks/auditlog/model/ExampleAuditedEntity.java

@@ -16,8 +16,15 @@
 
 package org.genesys.blocks.auditlog.model;
 
+import java.util.List;
+import java.util.Set;
+
+import javax.persistence.CollectionTable;
 import javax.persistence.Column;
+import javax.persistence.ElementCollection;
 import javax.persistence.Entity;
+import javax.persistence.FetchType;
+import javax.persistence.JoinColumn;
 import javax.persistence.ManyToOne;
 import javax.persistence.Table;
 
@@ -37,6 +44,16 @@ public class ExampleAuditedEntity extends BasicModel {
 	@ManyToOne(optional = true)
 	private ExampleAuditedEntity reference;
 
+	@Column(name = "item", nullable = false, length = 9)
+	@ElementCollection(fetch = FetchType.LAZY)
+	@CollectionTable(name = "entity_list", joinColumns = @JoinColumn(name = "entityId", referencedColumnName = "id"))
+	private List<Long> list;
+
+	@Column(name = "item", nullable = false, length = 9)
+	@ElementCollection(fetch = FetchType.LAZY)
+	@CollectionTable(name = "entity_set", joinColumns = @JoinColumn(name = "entityId", referencedColumnName = "id"))
+	private Set<String> set;
+
 	public String getName() {
 		return name;
 	}
@@ -52,4 +69,20 @@ public class ExampleAuditedEntity extends BasicModel {
 	public ExampleAuditedEntity getReference() {
 		return reference;
 	}
+
+	public List<Long> getList() {
+		return list;
+	}
+	
+	public void setList(List<Long> list) {
+		this.list = list;
+	}
+	
+	public Set<String> getSet() {
+		return set;
+	}
+	
+	public void setSet(Set<String> set) {
+		this.set = set;
+	}
 }

auditlog/src/test/java/org/genesys/blocks/auditlog/service/AuditTrailServiceTest.java

@@ -24,6 +24,7 @@ import static org.junit.Assert.assertThat;
 
 import java.util.List;
 
+import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 
 import org.genesys.blocks.auditlog.model.AuditAction;
@@ -73,7 +74,8 @@ public class AuditTrailServiceTest extends ServiceTest {
 	@Test
 	public void saveRefEnt() {
 		final Long entity = new Long(42);
-		final List<AuditLog> logs = auditTrailService.addAuditLogs(Sets.newHashSet(auditTrailService.auditLogEntry(AuditAction.UPDATE, entity, 1, "a", null, "new", AuditLog.class)));
+		final List<AuditLog> logs = auditTrailService.addAuditLogs(Sets.newHashSet(auditTrailService.auditLogEntry(AuditAction.UPDATE, entity, 1, "a", null, "new",
+			AuditLog.class)));
 		AuditLog log = logs.get(0);
 
 		assertThat(log.getId(), not(nullValue()));
@@ -94,8 +96,8 @@ public class AuditTrailServiceTest extends ServiceTest {
 
 		entity.setName("Test 2");
 		entity = exampleAuditedEntityService.save(entity);
-		assertThat(listAuditLogs(entity), hasSize(1));
 		// printAuditLogs(entity);
+		assertThat(listAuditLogs(entity), hasSize(1));
 	}
 
 	@Test
@@ -160,13 +162,13 @@ public class AuditTrailServiceTest extends ServiceTest {
 		ClassPK classPk = classPkService.getClassPk(ExampleAuditedEntity.class);
 		assertThat(auditLogRepository.get(classPk, null), is(nullValue()));
 		assertThat(auditLogRepository.get(classPk, -1l), is(nullValue()));
-		
+
 		Object lookup = auditLogRepository.get(classPk, entity.getId());
 		assertThat(lookup, not(nullValue()));
 		assertThat(lookup.getClass(), equalTo(ExampleAuditedEntity.class));
 		assertThat(((ExampleAuditedEntity) lookup).getId(), is(entity.getId()));
 	}
-	
+
 	@Test
 	public void testReferencedEntityAuditLog() {
 		ExampleAuditedEntity refEntity = new ExampleAuditedEntity();
@@ -180,11 +182,71 @@ public class AuditTrailServiceTest extends ServiceTest {
 		entity.setReference(refEntity);
 		entity = exampleAuditedEntityService.save(entity);
 		assertThat(entity.getReference(), not(nullValue()));
-		
+
+		// printAuditLogs(entity);
 		assertThat(listAuditLogs(entity), hasSize(1));
 		assertThat(listAuditLogs(entity).get(0).getPreviousEntity(), is(nullValue()));
 		assertThat(listAuditLogs(entity).get(0).getNewEntity(), not(nullValue()));
 		assertThat(listAuditLogs(entity).get(0).getNewEntity().getClass(), equalTo(ExampleAuditedEntity.class));
-		assertThat(((ExampleAuditedEntity)listAuditLogs(entity).get(0).getNewEntity()).getId(), is(refEntity.getId()));
+		assertThat(((ExampleAuditedEntity) listAuditLogs(entity).get(0).getNewEntity()).getId(), is(refEntity.getId()));
 	}
+
+	@Test
+	public void testEmbeddedCollections() {
+		ExampleAuditedEntity entity = new ExampleAuditedEntity();
+		entity.setName("Test 1");
+		entity.setList(Lists.newArrayList(1L, 2L, 3L));
+		entity.setSet(Sets.newHashSet("zebra", "banana", "piper"));
+		entity = exampleAuditedEntityService.save(entity);
+		// printAuditLogs(entity);
+		assertThat(listAuditLogs(entity), hasSize(0));
+
+		entity.getList().clear();
+		entity.getList().addAll(Lists.newArrayList(2L, 3L, 4L));
+		entity = exampleAuditedEntityService.save(entity);
+		// printAuditLogs(entity);
+		assertThat(listAuditLogs(entity), hasSize(1));
+		AuditLog lastLog = lastAuditLog(entity);
+		assertThat(lastLog.getAction(), is(AuditAction.UPDATE));
+		assertThat(lastLog.getPropertyName(), is("list"));
+		assertThat(lastLog.getPreviousState(), is("[1, 2, 3]"));
+		assertThat(lastLog.getNewState(), is("[2, 3, 4]"));
+
+		entity.getSet().clear();
+		entity = exampleAuditedEntityService.save(entity);
+		// printAuditLogs(entity);
+		assertThat(listAuditLogs(entity), hasSize(2));
+		lastLog = lastAuditLog(entity);
+		assertThat(lastLog.getAction(), is(AuditAction.UPDATE));
+		assertThat(lastLog.getPropertyName(), is("set"));
+		assertThat(lastLog.getPreviousState(), is("[zebra, banana, piper]"));
+		assertThat(lastLog.getNewState(), is("[]"));
+
+		entity.getSet().clear();
+		entity.getSet().add("c++");
+		entity = exampleAuditedEntityService.save(entity);
+		// printAuditLogs(entity);
+		assertThat(listAuditLogs(entity), hasSize(3));
+		lastLog = lastAuditLog(entity);
+		assertThat(lastLog.getAction(), is(AuditAction.UPDATE));
+		assertThat(lastLog.getPropertyName(), is("set"));
+		assertThat(lastLog.getPreviousState(), is("[]"));
+		assertThat(lastLog.getNewState(), is("[c++]"));
+
+		assertThat(entity.getList(), hasSize(3));
+		assertThat(entity.getSet(), hasSize(1));
+
+		entity.getSet().clear();
+		entity.getSet().add("c++");
+		entity = exampleAuditedEntityService.save(entity);
+		// printAuditLogs(entity);
+		assertThat(listAuditLogs(entity), hasSize(3));
+		lastLog = lastAuditLog(entity);
+		assertThat(lastLog.getAction(), is(AuditAction.UPDATE));
+		assertThat(lastLog.getPropertyName(), is("set"));
+		assertThat(lastLog.getPreviousState(), is("[]"));
+		assertThat(lastLog.getNewState(), is("[c++]"));
+
+	}
+
 }

auditlog/src/test/java/org/genesys/blocks/auditlog/test/BaseTest.java

@@ -88,5 +88,6 @@ public abstract class BaseTest {
 			LOG.debug(auditLog.getId() + " " + auditLog.getAction() + " " + auditLog.getLogDate() + " " + auditLog.getClassPk().getClassname() + "<" + auditLog.getEntityId() + ">."
 					+ auditLog.getPropertyName() + " = " + auditLog.getPreviousState() + " -> " + auditLog.getNewState());
 		}
+		LOG.debug("\n\n");
 	}
 }