From 271108459adac1536598c5c871d5a7ba5b6c1943 Mon Sep 17 00:00:00 2001 From: Sylvain Jermini Date: Sun, 8 Sep 2019 15:50:47 +0200 Subject: [PATCH] remove java object diff, wire in our implementation --- build.gradle | 1 - .../manager/TicketReservationManager.java | 52 ++++----------- src/main/java/alfio/util/ObjectDiffUtil.java | 16 +++-- src/test/java/alfio/util/ObjectDiffTest.java | 65 ++----------------- 4 files changed, 25 insertions(+), 109 deletions(-) diff --git a/build.gradle b/build.gradle index 48b044c5d2..09b85bc02b 100644 --- a/build.gradle +++ b/build.gradle @@ -159,7 +159,6 @@ dependencies { compile 'com.atlassian.commonmark:commonmark-ext-gfm-tables:0.13.0' compile 'com.ryantenney.passkit4j:passkit4j:2.0.1' compile 'com.github.ben-manes.caffeine:caffeine:2.7.0' - compile 'de.danielbechler:java-object-diff:0.95' compile 'com.github.scribejava:scribejava-core:5.0.0' compile 'ch.digitalfondue.vatchecker:vatchecker:1.2' compile 'ch.digitalfondue.basicxlsx:basicxlsx:0.5.1' diff --git a/src/main/java/alfio/manager/TicketReservationManager.java b/src/main/java/alfio/manager/TicketReservationManager.java index 0d1ecd0450..d815165e69 100644 --- a/src/main/java/alfio/manager/TicketReservationManager.java +++ b/src/main/java/alfio/manager/TicketReservationManager.java @@ -52,9 +52,6 @@ import alfio.repository.user.UserRepository; import alfio.util.*; import ch.digitalfondue.npjt.AffectedRowCountAndKey; -import de.danielbechler.diff.ObjectDifferBuilder; -import de.danielbechler.diff.node.DiffNode; -import de.danielbechler.diff.node.Visit; import lombok.extern.log4j.Log4j2; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Validate; @@ -1421,49 +1418,22 @@ boolean isTicketBeingReassigned(Ticket original, UpdateTicketOwnerForm updated, } private void auditUpdateTicket(Ticket preUpdateTicket, Map preUpdateTicketFields, Ticket postUpdateTicket, Map postUpdateTicketFields, int eventId) { - DiffNode diffTicket = ObjectDifferBuilder.buildDefault().compare(postUpdateTicket, preUpdateTicket); - DiffNode diffTicketFields = ObjectDifferBuilder.buildDefault().compare(postUpdateTicketFields, preUpdateTicketFields); - FieldChangesSaver diffTicketVisitor = new FieldChangesSaver(preUpdateTicket, postUpdateTicket); - FieldChangesSaver diffTicketFieldsVisitor = new FieldChangesSaver(preUpdateTicketFields, postUpdateTicketFields); - diffTicket.visit(diffTicketVisitor); - diffTicketFields.visit(diffTicketFieldsVisitor); - - List> changes = new ArrayList<>(diffTicketVisitor.changes); - changes.addAll(diffTicketFieldsVisitor.changes); + List diffTicket = ObjectDiffUtil.diff(preUpdateTicket, postUpdateTicket); + List diffTicketFields = ObjectDiffUtil.diff(preUpdateTicketFields, postUpdateTicketFields); + + List> changes = Stream.concat(diffTicket.stream(), diffTicketFields.stream()).map(change -> { + var v = new HashMap(); + v.put("propertyName", change.getPropertyName()); + v.put("state", change.getState()); + v.put("oldValue", change.getOldValue()); + v.put("newValue", change.getNewValue()); + return v; + }).collect(Collectors.toList()); auditingRepository.insert(preUpdateTicket.getTicketsReservationId(), null, eventId, Audit.EventType.UPDATE_TICKET, new Date(), Audit.EntityType.TICKET, Integer.toString(preUpdateTicket.getId()), changes); } - - private static class FieldChangesSaver implements DiffNode.Visitor { - - private final Object preBase; - private final Object postBase; - - private final List> changes = new ArrayList<>(); - - - FieldChangesSaver(Object preBase, Object postBase) { - this.preBase = preBase; - this.postBase = postBase; - } - - @Override - public void node(DiffNode node, Visit visit) { - if(node.hasChanges() && node.getState() != DiffNode.State.UNTOUCHED && !node.isRootNode()) { - Object baseValue = node.canonicalGet(preBase); - Object workingValue = node.canonicalGet(postBase); - HashMap change = new HashMap<>(); - change.put("propertyName", node.getPath().toString()); - change.put("state", node.getState()); - change.put("oldValue", baseValue); - change.put("newValue", workingValue); - changes.add(change); - } - } - } - private boolean isAdmin(Optional userDetails) { return userDetails.flatMap(u -> u.getAuthorities().stream().map(a -> Role.fromRoleName(a.getAuthority())).filter(Role.ADMIN::equals).findFirst()).isPresent(); } diff --git a/src/main/java/alfio/util/ObjectDiffUtil.java b/src/main/java/alfio/util/ObjectDiffUtil.java index 2c69399a11..bc93a2755d 100644 --- a/src/main/java/alfio/util/ObjectDiffUtil.java +++ b/src/main/java/alfio/util/ObjectDiffUtil.java @@ -32,10 +32,14 @@ public class ObjectDiffUtil { public static List diff(Map before, Map after) { - return diffUntyped(before, after); + return diffUntyped(before, after, "/{", "}"); } - private static List diffUntyped(Map before, Map after) { + private static String formatPropertyName(String k, String propertyNameBefore, String propertyNameAfter) { + return new StringBuilder(propertyNameBefore).append(k).append(propertyNameAfter).toString(); + } + + private static List diffUntyped(Map before, Map after, String propertyNameBefore, String propertyNameAfter) { var removed = new HashSet<>(before.keySet()); removed.removeAll(after.keySet()); @@ -48,13 +52,13 @@ private static List diffUntyped(Map before, Map af var changes = new ArrayList(); - 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); + removed.stream().map(k -> new Change(formatPropertyName(k, propertyNameBefore, propertyNameAfter), State.REMOVED, before.get(k), null)).forEach(changes::add); + added.stream().map(k -> new Change(formatPropertyName(k, propertyNameBefore, propertyNameAfter), State.ADDED, null, after.get(k))).forEach(changes::add); changedOrUntouched.stream().forEach(k -> { var beforeValue = before.get(k); var afterValue = after.get(k); if(!Objects.equals(beforeValue, afterValue)) { - changes.add(new Change(k, State.CHANGED, beforeValue, afterValue)); + changes.add(new Change(formatPropertyName(k, propertyNameBefore, propertyNameAfter), State.CHANGED, beforeValue, afterValue)); } }); changes.sort(Change::compareTo); @@ -73,7 +77,7 @@ public static List diff(Ticket before, Ticket after) { afterAsMap.put(name, ReflectionUtils.invokeMethod(method, after)); } }); - return diffUntyped(beforeAsMap, afterAsMap); + return diffUntyped(beforeAsMap, afterAsMap, "/", ""); } @AllArgsConstructor diff --git a/src/test/java/alfio/util/ObjectDiffTest.java b/src/test/java/alfio/util/ObjectDiffTest.java index 1070fd9be9..9af3a44f5f 100644 --- a/src/test/java/alfio/util/ObjectDiffTest.java +++ b/src/test/java/alfio/util/ObjectDiffTest.java @@ -17,16 +17,11 @@ package alfio.util; import alfio.model.Ticket; -import de.danielbechler.diff.ObjectDifferBuilder; -import de.danielbechler.diff.node.DiffNode; -import de.danielbechler.diff.node.Visit; import org.junit.Assert; import org.junit.Test; import java.time.ZonedDateTime; -import java.util.ArrayList; import java.util.HashMap; -import java.util.List; import java.util.Map; public class ObjectDiffTest { @@ -43,30 +38,6 @@ public class ObjectDiffTest { false, "en", 0, 0, 0, 0, null, null); - - @Test - public void diffTest() { - - - - var postUpdateTicketFields = new HashMap(); - postUpdateTicketFields.put("hello", "world"); - var preUpdateTicketFields = new HashMap(); - - DiffNode diffTicket = ObjectDifferBuilder.buildDefault().compare(postUpdateTicket, preUpdateTicket); - DiffNode diffTicketFields = ObjectDifferBuilder.buildDefault().compare(postUpdateTicketFields, preUpdateTicketFields); - FieldChangesSaver diffTicketVisitor = new FieldChangesSaver(preUpdateTicket, postUpdateTicket); - FieldChangesSaver diffTicketFieldsVisitor = new FieldChangesSaver(preUpdateTicketFields, postUpdateTicketFields); - diffTicket.visit(diffTicketVisitor); - diffTicketFields.visit(diffTicketFieldsVisitor); - - List> changes = new ArrayList<>(diffTicketVisitor.changes); - changes.addAll(diffTicketFieldsVisitor.changes); - - Assert.assertEquals(2, changes.size()); - - } - @Test public void diffMapTest() { Map empty = Map.of(); @@ -76,21 +47,21 @@ public void diffMapTest() { var newElement = Map.of("new", "element"); var newElementRes = ObjectDiffUtil.diff(emptyAfter, newElement); Assert.assertEquals(1, newElementRes.size()); - Assert.assertEquals("new", newElementRes.get(0).getPropertyName()); + Assert.assertEquals("/{new}", newElementRes.get(0).getPropertyName()); Assert.assertEquals("element", newElementRes.get(0).getNewValue()); Assert.assertEquals(null, newElementRes.get(0).getOldValue()); Assert.assertEquals(ObjectDiffUtil.State.ADDED, newElementRes.get(0).getState()); var removedElementRes = ObjectDiffUtil.diff(newElement, empty); Assert.assertEquals(1, removedElementRes.size()); - Assert.assertEquals("new", removedElementRes.get(0).getPropertyName()); + Assert.assertEquals("/{new}", removedElementRes.get(0).getPropertyName()); Assert.assertEquals(null, removedElementRes.get(0).getNewValue()); Assert.assertEquals("element", removedElementRes.get(0).getOldValue()); Assert.assertEquals(ObjectDiffUtil.State.REMOVED, removedElementRes.get(0).getState()); var changedElem = ObjectDiffUtil.diff(newElement, Map.of("new", "changed")); Assert.assertEquals(1, changedElem.size()); - Assert.assertEquals("new", changedElem.get(0).getPropertyName()); + Assert.assertEquals("/{new}", changedElem.get(0).getPropertyName()); Assert.assertEquals("changed", changedElem.get(0).getNewValue()); Assert.assertEquals("element", changedElem.get(0).getOldValue()); Assert.assertEquals(ObjectDiffUtil.State.CHANGED, changedElem.get(0).getState()); @@ -105,37 +76,9 @@ public void testTicketDiff() { var res = ObjectDiffUtil.diff(preUpdateTicket, postUpdateTicket); Assert.assertEquals(1, res.size()); - Assert.assertEquals("status", res.get(0).getPropertyName()); + 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 { - - private final Object preBase; - private final Object postBase; - - private final List> changes = new ArrayList<>(); - - - FieldChangesSaver(Object preBase, Object postBase) { - this.preBase = preBase; - this.postBase = postBase; - } - - @Override - public void node(DiffNode node, Visit visit) { - if(node.hasChanges() && node.getState() != DiffNode.State.UNTOUCHED && !node.isRootNode()) { - Object baseValue = node.canonicalGet(preBase); - Object workingValue = node.canonicalGet(postBase); - HashMap change = new HashMap<>(); - change.put("propertyName", node.getPath().toString()); - change.put("state", node.getState()); - change.put("oldValue", baseValue); - change.put("newValue", workingValue); - changes.add(change); - } - } - } }