Commit 4f1122a1 authored by Matija Obreza's avatar Matija Obreza

Sort set values before writing audit log change

- Prevents "a, d, c" -> "d, c, a" -> "a, b, c"
parent 87e12c2d
...@@ -427,7 +427,7 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ ...@@ -427,7 +427,7 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ
deleted = convertEntityId(deleted); deleted = convertEntityId(deleted);
} }
LOG.trace("prev=" + deleted.toString() + " curr=" + null); LOG.trace("prev={} curr=null", deleted);
recordDelete(pc.getOwner(), (Long) key, propertyName, deleted.toString(), referencedEntity); recordDelete(pc.getOwner(), (Long) key, propertyName, deleted.toString(), referencedEntity);
} }
...@@ -466,8 +466,10 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ ...@@ -466,8 +466,10 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ
Collection<?> previous = null; Collection<?> previous = null;
final Serializable snap = pc.getStoredSnapshot(); final Serializable snap = pc.getStoredSnapshot();
if (snap instanceof Map<?, ?>) { if (snap instanceof Map<?, ?>) {
// LOG.trace("Snap keys: {}", ((Map) snap).keySet());
// LOG.trace("Snap vals: {}", ((Map) snap).values());
final Map<?, ?> snapMap = (Map<?, ?>) snap; final Map<?, ?> snapMap = (Map<?, ?>) snap;
previous = snapMap.values(); previous = snapMap.keySet();
} else if (snap instanceof Collection<?>) { } else if (snap instanceof Collection<?>) {
final Collection<?> snapList = (Collection<?>) snap; final Collection<?> snapList = (Collection<?>) snap;
previous = snapList; previous = snapList;
...@@ -484,8 +486,29 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ ...@@ -484,8 +486,29 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ
remaining = convertEntityId(remaining); remaining = convertEntityId(remaining);
} }
LOG.trace("prev=" + previous + " curr=" + remaining); LOG.trace("prev={} curr={}", previous, remaining);
recordChange(pc.getOwner(), (Long) key, propertyName, previous.toString(), remaining.toString(), referencedEntity); recordChange(pc.getOwner(), (Long) key, propertyName, collectionToStringSorted(previous), collectionToStringSorted(remaining), referencedEntity);
}
/**
* Produce a string representation of the collection. Set elements are sorted,
* other collection types return `#toString`.
*
* @param collection the collection
* @return the string
*/
private String collectionToStringSorted(Collection<?> collection) {
if (collection == null) {
return null;
}
if (collection instanceof Set<?>) {
LOG.trace("Converting to sorted list {} -> {}", collection, collection.stream().sorted().map(Object::toString).collect(Collectors.joining(", ", "[", "]")));
return collection.stream().sorted().map(Object::toString).collect(Collectors.joining(", ", "[", "]"));
} else {
LOG.trace("Not sorting {}: {}", collection.getClass(), collection);
}
return collection.toString();
} }
/** /**
...@@ -501,7 +524,7 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ ...@@ -501,7 +524,7 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ
// Field // Field
final Field field = ReflectionUtils.findField(class1, propertyName); final Field field = ReflectionUtils.findField(class1, propertyName);
if (field != null) { if (field != null) {
LOG.trace("Found field: " + field + "\n\ttype=" + field.getType() + "\n\tgeneric=" + field.getGenericType() + "\n\tgenericTN=" + field.getGenericType().getTypeName()); LOG.trace("Found field: {}\n\ttype={}\n\tgeneric={}\n\tgenericTN={}", field, field.getType(), field.getGenericType(), field.getGenericType().getTypeName());
final ResolvableType t = ResolvableType.forField(field, class1); final ResolvableType t = ResolvableType.forField(field, class1);
if (t.hasGenerics()) { if (t.hasGenerics()) {
LOG.trace("\tResoved={} returning={}", t.toString(), t.resolveGeneric(0)); LOG.trace("\tResoved={} returning={}", t.toString(), t.resolveGeneric(0));
...@@ -516,7 +539,7 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ ...@@ -516,7 +539,7 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ
try { try {
final Method method = class1.getMethod("get" + StringUtils.capitalize(propertyName)); final Method method = class1.getMethod("get" + StringUtils.capitalize(propertyName));
if (method != null) { if (method != null) {
LOG.trace("Didn't find field, found the method: " + method.getReturnType().getTypeParameters()[0]); LOG.trace("Didn't find field, found the method: {}", method.getReturnType().getTypeParameters()[0]);
} }
} catch (SecurityException | NoSuchMethodException e) { } catch (SecurityException | NoSuchMethodException e) {
e.printStackTrace(); e.printStackTrace();
...@@ -785,12 +808,12 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ ...@@ -785,12 +808,12 @@ public class AuditTrailInterceptor extends EmptyInterceptor implements Initializ
if (getter != null) { if (getter != null) {
final Object result = getter.invoke(entity); final Object result = getter.invoke(entity);
if (result == null) { if (result == null) {
LOG.trace(entity + " is transient, has " + methodName + " == null"); LOG.trace("{} is transient, has {} == null", entity, methodName);
return true; return true;
} else if (result instanceof Number) { } else if (result instanceof Number) {
final Number r = (Number) result; final Number r = (Number) result;
if (r.longValue() < 0) { if (r.longValue() < 0) {
LOG.trace(entity + " is transient, has " + methodName + " = " + result + " < 0"); LOG.trace("{} is transient, has {} = {} < 0", entity, methodName, result);
return true; return true;
} else { } else {
// LOG.trace(entity + " is not transient, has " + methodName + " = " + result + // LOG.trace(entity + " is not transient, has " + methodName + " = " + result +
......
...@@ -242,15 +242,15 @@ public class AuditTrailServiceTest extends ServiceTest { ...@@ -242,15 +242,15 @@ public class AuditTrailServiceTest extends ServiceTest {
assertThat(lastLog.getPreviousState(), is("[1, 2, 3]")); assertThat(lastLog.getPreviousState(), is("[1, 2, 3]"));
assertThat(lastLog.getNewState(), is("[2, 3, 4]")); assertThat(lastLog.getNewState(), is("[2, 3, 4]"));
entity.getSet().clear(); entity.getSet().remove("piper");
entity = exampleAuditedEntityService.save(entity); entity = exampleAuditedEntityService.save(entity);
// printAuditLogs(entity); // printAuditLogs(entity);
assertThat(listAuditLogs(entity), hasSize(2)); assertThat(listAuditLogs(entity), hasSize(2));
lastLog = lastAuditLog(entity); lastLog = lastAuditLog(entity);
assertThat(lastLog.getAction(), is(AuditAction.UPDATE)); assertThat(lastLog.getAction(), is(AuditAction.UPDATE));
assertThat(lastLog.getPropertyName(), is("set")); assertThat(lastLog.getPropertyName(), is("set"));
assertThat(lastLog.getPreviousState(), is("[zebra, banana, piper]")); assertThat(lastLog.getPreviousState(), is("[banana, piper, zebra]"));
assertThat(lastLog.getNewState(), is("[]")); assertThat(lastLog.getNewState(), is("[banana, zebra]"));
entity.getSet().clear(); entity.getSet().clear();
entity.getSet().add("c++"); entity.getSet().add("c++");
...@@ -260,7 +260,7 @@ public class AuditTrailServiceTest extends ServiceTest { ...@@ -260,7 +260,7 @@ public class AuditTrailServiceTest extends ServiceTest {
lastLog = lastAuditLog(entity); lastLog = lastAuditLog(entity);
assertThat(lastLog.getAction(), is(AuditAction.UPDATE)); assertThat(lastLog.getAction(), is(AuditAction.UPDATE));
assertThat(lastLog.getPropertyName(), is("set")); assertThat(lastLog.getPropertyName(), is("set"));
assertThat(lastLog.getPreviousState(), is("[]")); assertThat(lastLog.getPreviousState(), is("[banana, zebra]"));
assertThat(lastLog.getNewState(), is("[c++]")); assertThat(lastLog.getNewState(), is("[c++]"));
assertThat(entity.getList(), hasSize(3)); assertThat(entity.getList(), hasSize(3));
...@@ -274,7 +274,7 @@ public class AuditTrailServiceTest extends ServiceTest { ...@@ -274,7 +274,7 @@ public class AuditTrailServiceTest extends ServiceTest {
lastLog = lastAuditLog(entity); lastLog = lastAuditLog(entity);
assertThat(lastLog.getAction(), is(AuditAction.UPDATE)); assertThat(lastLog.getAction(), is(AuditAction.UPDATE));
assertThat(lastLog.getPropertyName(), is("set")); assertThat(lastLog.getPropertyName(), is("set"));
assertThat(lastLog.getPreviousState(), is("[]")); assertThat(lastLog.getPreviousState(), is("[banana, zebra]"));
assertThat(lastLog.getNewState(), is("[c++]")); assertThat(lastLog.getNewState(), is("[c++]"));
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment