diff --git a/common/taskana-common/src/main/java/pro/taskana/common/internal/util/ObjectAttributeChangeDetector.java b/common/taskana-common/src/main/java/pro/taskana/common/internal/util/ObjectAttributeChangeDetector.java index ff3889ecd..69bf514e6 100644 --- a/common/taskana-common/src/main/java/pro/taskana/common/internal/util/ObjectAttributeChangeDetector.java +++ b/common/taskana-common/src/main/java/pro/taskana/common/internal/util/ObjectAttributeChangeDetector.java @@ -1,12 +1,13 @@ package pro.taskana.common.internal.util; +import static pro.taskana.common.internal.util.CheckedFunction.wrap; + import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Objects; import java.util.Optional; -import java.util.function.Predicate; import java.util.stream.Collectors; import org.json.JSONObject; @@ -23,82 +24,76 @@ public class ObjectAttributeChangeDetector { * @param newObject the new object for the comparison * @param The generic type parameter * @return the details of all changed fields as JSON string - * @throws SystemException when any parameter is null + * @throws SystemException when any parameter is null or the class of oldObject and newObject do + * not match */ public static String determineChangesInAttributes(T oldObject, T newObject) { - List fields = new ArrayList<>(); - - if (Objects.isNull(oldObject) || Objects.isNull(newObject)) { + if (oldObject == null || newObject == null) { throw new SystemException( "Null was provided as a parameter. Please provide two objects of the same type"); } - Class currentClass = oldObject.getClass(); - - if (List.class.isAssignableFrom(currentClass)) { - + Class objectClass = oldObject.getClass(); + if (List.class.isAssignableFrom(objectClass)) { return compareLists(oldObject, newObject); - - } else { - - retrieveFields(fields, currentClass); } - Predicate> areFieldsNotEqual = - fieldAndValuePairTriplet -> - !Objects.equals( - fieldAndValuePairTriplet.getMiddle(), fieldAndValuePairTriplet.getRight()); - Predicate> isFieldNotCustomAttributes = - fieldAndValuePairTriplet -> - !fieldAndValuePairTriplet.getLeft().getName().equals("customAttributes"); + // this has to be checked after we deal with List data types, because + // we want to allow different implementations of the List interface to work as well. + if (!oldObject.getClass().equals(newObject.getClass())) { + throw new SystemException( + String.format( + "The classes differ between the oldObject(%s) and newObject(%s). " + + "In order to detect changes properly they should not differ.", + oldObject.getClass().getName(), newObject.getClass().getName())); + } List changedAttributes = - fields.stream() + retrieveAllFields(objectClass).stream() .peek(field -> field.setAccessible(true)) - .map( - CheckedFunction.wrap( - field -> Triplet.of(field, field.get(oldObject), field.get(newObject)))) - .filter(areFieldsNotEqual.and(isFieldNotCustomAttributes)) - .map( - fieldAndValuePairTriplet -> { - JSONObject changedAttribute = new JSONObject(); - changedAttribute.put("fieldName", fieldAndValuePairTriplet.getLeft().getName()); - changedAttribute.put( - "oldValue", - Optional.ofNullable(fieldAndValuePairTriplet.getMiddle()).orElse("")); - changedAttribute.put( - "newValue", - Optional.ofNullable(fieldAndValuePairTriplet.getRight()).orElse("")); - return changedAttribute; - }) + .filter(field -> !"customAttributes".equals(field.getName())) + .map(wrap(field -> Triplet.of(field, field.get(oldObject), field.get(newObject)))) + .filter(t -> !Objects.equals(t.getMiddle(), t.getRight())) + .map(t -> generateChangedAttribute(t.getLeft(), t.getMiddle(), t.getRight())) .collect(Collectors.toList()); JSONObject changes = new JSONObject(); changes.put("changes", changedAttributes); - return changes.toString(); } - private static void retrieveFields(List fields, Class currentClass) { + private static JSONObject generateChangedAttribute( + Field field, Object oldValue, Object newValue) { + JSONObject changedAttribute = new JSONObject(); + changedAttribute.put("fieldName", field.getName()); + changedAttribute.put( + "oldValue", Optional.ofNullable(oldValue).map(JSONObject::wrap).orElse("")); + changedAttribute.put( + "newValue", Optional.ofNullable(newValue).map(JSONObject::wrap).orElse("")); + return changedAttribute; + } + + private static List retrieveAllFields(Class currentClass) { + List fields = new ArrayList<>(); while (currentClass.getSuperclass() != null) { fields.addAll(Arrays.asList(currentClass.getDeclaredFields())); currentClass = currentClass.getSuperclass(); } + return fields; } private static String compareLists(T oldObject, T newObject) { - if (!oldObject.equals(newObject)) { - JSONObject changedAttribute = new JSONObject(); - - changedAttribute.put("oldValue", oldObject); - changedAttribute.put("newValue", newObject); - - JSONObject changes = new JSONObject(); - - changes.put("changes", changedAttribute); - - return changes.toString(); + if (oldObject.equals(newObject)) { + return ""; } - return ""; + + JSONObject changedAttribute = new JSONObject(); + changedAttribute.put("oldValue", oldObject); + changedAttribute.put("newValue", newObject); + + JSONObject changes = new JSONObject(); + changes.put("changes", changedAttribute); + + return changes.toString(); } } diff --git a/history/taskana-simplehistory-provider/src/test/java/acceptance/events/task/CreateHistoryEventOnTaskTransferAccTest.java b/history/taskana-simplehistory-provider/src/test/java/acceptance/events/task/CreateHistoryEventOnTaskTransferAccTest.java index 475ce54d4..9ec7eab67 100644 --- a/history/taskana-simplehistory-provider/src/test/java/acceptance/events/task/CreateHistoryEventOnTaskTransferAccTest.java +++ b/history/taskana-simplehistory-provider/src/test/java/acceptance/events/task/CreateHistoryEventOnTaskTransferAccTest.java @@ -148,20 +148,24 @@ class CreateHistoryEventOnTaskTransferAccTest extends AbstractAccTest { TaskHistoryEvent event = historyService.getTaskHistoryEvent(eventId); assertThat(event.getDetails()).isNotNull(); JSONArray changes = new JSONObject(event.getDetails()).getJSONArray("changes"); - assertThat(changes.length()).isPositive(); - for (int i = 0; i < changes.length(); i++) { + boolean foundField = false; + for (int i = 0; i < changes.length() && !foundField; i++) { JSONObject change = changes.getJSONObject(i); if (change.get("fieldName").equals("workbasketSummary")) { - String oldWorkbasketStr = (String) change.get("oldValue"); - String newWorkbasketStr = (String) change.get("newValue"); + foundField = true; + String oldWorkbasketStr = change.get("oldValue").toString(); + String newWorkbasketStr = change.get("newValue").toString(); WorkbasketService workbasketService = taskanaEngine.getWorkbasketService(); Workbasket oldWorkbasket = workbasketService.getWorkbasket(expectedOldValue); - assertThat(oldWorkbasket.asSummary()).hasToString(oldWorkbasketStr); + assertThat(oldWorkbasketStr) + .isEqualTo(JSONObject.wrap(oldWorkbasket.asSummary()).toString()); Workbasket newWorkbasket = workbasketService.getWorkbasket(expectedNewValue); - assertThat(newWorkbasket.asSummary()).hasToString(newWorkbasketStr); + assertThat(newWorkbasketStr) + .isEqualTo(JSONObject.wrap(newWorkbasket.asSummary()).toString()); } } + assertThat(foundField).describedAs("changes do not contain field 'workbasketSummary'").isTrue(); assertThat(event.getId()).startsWith("THI:"); assertThat(event.getOldValue()).isEqualTo(expectedOldValue);