From 94152c7c35e0a5e2a0d9be7fa11c0029ed2ea294 Mon Sep 17 00:00:00 2001 From: Sylvain Jermini Date: Sat, 7 Sep 2019 19:13:03 +0200 Subject: [PATCH] additional work for implementing an alternative of java-object-diff: generic compare --- src/main/java/alfio/util/ObjectDiffUtil.java | 44 ++++++++++++++++---- src/test/java/alfio/util/ObjectDiffTest.java | 40 +++++++++++------- 2 files changed, 62 insertions(+), 22 deletions(-) diff --git a/src/main/java/alfio/util/ObjectDiffUtil.java b/src/main/java/alfio/util/ObjectDiffUtil.java index 358343524a..2c69399a11 100644 --- a/src/main/java/alfio/util/ObjectDiffUtil.java +++ b/src/main/java/alfio/util/ObjectDiffUtil.java @@ -19,8 +19,12 @@ import alfio.model.Ticket; import lombok.AllArgsConstructor; import lombok.Getter; +import org.apache.commons.lang3.builder.CompareToBuilder; +import org.springframework.beans.BeanUtils; +import org.springframework.util.ReflectionUtils; import java.util.*; +import java.util.stream.Stream; /** * Replace https://github.com/SQiShER/java-object-diff . As our use case is way more restricted and limited. @@ -28,7 +32,10 @@ public class ObjectDiffUtil { public static List diff(Map before, Map after) { + return diffUntyped(before, after); + } + private static List diffUntyped(Map before, Map after) { var removed = new HashSet<>(before.keySet()); removed.removeAll(after.keySet()); @@ -43,32 +50,53 @@ public static List diff(Map before, Map removed.stream().map(k -> new Change(k, State.REMOVED, before.get(k), null)).forEach(changes::add); added.stream().map(k -> new Change(k, State.ADDED, null, after.get(k))).forEach(changes::add); - changedOrUntouched.stream().map(k -> { + changedOrUntouched.stream().forEach(k -> { var beforeValue = before.get(k); var afterValue = after.get(k); - return new Change(k, Objects.equals(beforeValue, afterValue) ? State.UNTOUCHED : State.CHANGED, beforeValue, afterValue); - }).forEach(changes::add); - + if(!Objects.equals(beforeValue, afterValue)) { + changes.add(new Change(k, State.CHANGED, beforeValue, afterValue)); + } + }); + changes.sort(Change::compareTo); return changes; } public static List diff(Ticket before, Ticket after) { - return List.of(); + + var beforeAsMap = new HashMap(); + var afterAsMap = new HashMap(); + Stream.of(BeanUtils.getPropertyDescriptors(Ticket.class)).forEach(propertyDescriptor -> { + var method = propertyDescriptor.getReadMethod(); + var name = propertyDescriptor.getName(); + if (method != null) { + beforeAsMap.put(name, ReflectionUtils.invokeMethod(method, before)); + afterAsMap.put(name, ReflectionUtils.invokeMethod(method, after)); + } + }); + return diffUntyped(beforeAsMap, afterAsMap); } @AllArgsConstructor @Getter - public static class Change { + public static class Change implements Comparable { private final String propertyName; private final State state; private final Object oldValue; private final Object newValue; + + @Override + public int compareTo(Change change) { + return new CompareToBuilder() + .append(propertyName, change.propertyName) + .append(state.ordinal(), change.state.ordinal()) + .append(oldValue, change.oldValue) + .append(newValue, change.newValue).toComparison(); + } } public enum State { ADDED, CHANGED, - REMOVED, - UNTOUCHED + REMOVED } } diff --git a/src/test/java/alfio/util/ObjectDiffTest.java b/src/test/java/alfio/util/ObjectDiffTest.java index 1ca57fa028..1070fd9be9 100644 --- a/src/test/java/alfio/util/ObjectDiffTest.java +++ b/src/test/java/alfio/util/ObjectDiffTest.java @@ -31,19 +31,23 @@ public class ObjectDiffTest { + ZonedDateTime now = ZonedDateTime.now(); + + Ticket preUpdateTicket = new Ticket(42, "42", now, 1, Ticket.TicketStatus.ACQUIRED.name(), 42, + "42", "full name", "full", "name", "email@email.com", + false, "en", + 0, 0, 0, 0, null, null); + + Ticket postUpdateTicket = new Ticket(42, "42", now, 1, Ticket.TicketStatus.CANCELLED.name(), 42, + "42", "full name", "full", "name", "email@email.com", + false, "en", + 0, 0, 0, 0, null, null); + + @Test public void diffTest() { - var now = ZonedDateTime.now(); - var preUpdateTicket = new Ticket(42, "42", now, 1, Ticket.TicketStatus.ACQUIRED.name(), 42, - "42", "full name", "full", "name", "email@email.com", - false, "en", - 0, 0, 0, 0, null, null); - var postUpdateTicket = new Ticket(42, "42", now, 1, Ticket.TicketStatus.CANCELLED.name(), 42, - "42", "full name", "full", "name", "email@email.com", - false, "en", - 0, 0, 0, 0, null, null); var postUpdateTicketFields = new HashMap(); postUpdateTicketFields.put("hello", "world"); @@ -60,6 +64,7 @@ public void diffTest() { changes.addAll(diffTicketFieldsVisitor.changes); Assert.assertEquals(2, changes.size()); + } @Test @@ -92,11 +97,18 @@ public void diffMapTest() { var untouchedElem = ObjectDiffUtil.diff(newElement, new HashMap<>(newElement)); - Assert.assertEquals(1, untouchedElem.size()); - Assert.assertEquals("new", untouchedElem.get(0).getPropertyName()); - Assert.assertEquals("element", untouchedElem.get(0).getNewValue()); - Assert.assertEquals("element", untouchedElem.get(0).getOldValue()); - Assert.assertEquals(ObjectDiffUtil.State.UNTOUCHED, untouchedElem.get(0).getState()); + Assert.assertEquals(0, untouchedElem.size()); + } + + @Test + public void testTicketDiff() { + var res = ObjectDiffUtil.diff(preUpdateTicket, postUpdateTicket); + + Assert.assertEquals(1, res.size()); + Assert.assertEquals("status", res.get(0).getPropertyName()); + Assert.assertEquals(Ticket.TicketStatus.CANCELLED, res.get(0).getNewValue()); + Assert.assertEquals(Ticket.TicketStatus.ACQUIRED, res.get(0).getOldValue()); + Assert.assertEquals(ObjectDiffUtil.State.CHANGED, res.get(0).getState()); } private static class FieldChangesSaver implements DiffNode.Visitor {