Fix TCSObjectEvent aggregation

When aggregating TCSObjectEvents for RMI clients, actually aggregate the oldest and youngest events properly instead of keeping only the youngest one. Merged-by: Martin Grzenia <[email protected]>
openTCS · Oct 31, 2024 · c464ea8 · c464ea8
1 parent 27ceb1a
commit c464ea8
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 13 deletions.
diff --git a/opentcs-documentation/src/docs/release-notes/changelog.adoc b/opentcs-documentation/src/docs/release-notes/changelog.adoc
@@ -24,6 +24,7 @@ This change log lists the most relevant changes for past releases in reverse chr
 *** Add missing _required_ markers for request and response bodies.
 *** Include a vehicle's 'sufficiently recharged' and 'fully recharged' energy levels when requesting vehicle data.
 * Bugs fixed:
+** When aggregating ``TCSObjectEvent``s for RMI clients, actually aggregate the oldest and youngest events properly instead of keeping only the youngest one.
 ** Ask user for confirmation before overwriting files when using the _Save Model As..._ menu item in the Model Editor application.
 * Changes affecting developers:
 ** Update JUnit to 5.11.2.

diff --git a/...l-extension-rmi-services/src/main/java/org/opentcs/kernel/extensions/rmi/EventBuffer.java b/...l-extension-rmi-services/src/main/java/org/opentcs/kernel/extensions/rmi/EventBuffer.java
@@ -53,12 +53,9 @@ public void onEvent(Object event) {
     requireNonNull(event, "event");
     synchronized (events) {
       if (eventFilter.test(event)) {
-        if (replacesLatestEvent(event)) {
-          // If the event is considered a replacement for the latest buffered event, simply remove
-          // the latest event (before adding the replacement to the queue of events).
-          events.removeLast();
+        if (!tryMergeWithPreviousEvent(event)) {
+          events.add(event);
         }
-        events.add(event);
 
         // If the client is waiting for an event, wake it up, since there is one now.
         if (waitingClient) {
@@ -130,20 +127,39 @@ public void setEventFilter(
     }
   }
 
-  private boolean replacesLatestEvent(Object event) {
+  /**
+   * If possible, merge the given new event with the previous one in the buffer.
+   *
+   * @param event The new event.
+   * @return <code>true</code> if the new event was merged with the previous one.
+   */
+  private boolean tryMergeWithPreviousEvent(Object event) {
     if (!(event instanceof TCSObjectEvent currentEvent)
-        || !(events.peekLast() instanceof TCSObjectEvent latestEvent)) {
+        || !(events.peekLast() instanceof TCSObjectEvent previousEvent)) {
       return false;
     }
 
     if (currentEvent.getType() != TCSObjectEvent.Type.OBJECT_MODIFIED
-        || latestEvent.getType() != TCSObjectEvent.Type.OBJECT_MODIFIED) {
+        || previousEvent.getType() != TCSObjectEvent.Type.OBJECT_MODIFIED) {
       return false;
     }
 
-    return Objects.equals(
+    if (!Objects.equals(
         currentEvent.getCurrentObjectState().getReference(),
-        latestEvent.getCurrentObjectState().getReference()
+        previousEvent.getCurrentObjectState().getReference()
+    )) {
+      return false;
+    }
+
+    events.removeLast();
+    events.add(
+        new TCSObjectEvent(
+            currentEvent.getCurrentObjectState(),
+            previousEvent.getPreviousObjectState(),
+            TCSObjectEvent.Type.OBJECT_MODIFIED
+        )
     );
+
+    return true;
   }
 }
diff --git a/...tension-rmi-services/src/test/java/org/opentcs/kernel/extensions/rmi/EventBufferTest.java b/...tension-rmi-services/src/test/java/org/opentcs/kernel/extensions/rmi/EventBufferTest.java
@@ -7,6 +7,7 @@
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.theInstance;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 
 import java.util.List;
@@ -119,9 +120,25 @@ void aggregateConsecutiveTcsObjectEventsForSameObjects() {
 
     List<Object> result = eventBuffer.getEvents(0);
     assertThat(result, hasSize(3));
-    assertThat(result.get(0), is(equalTo(event1)));
-    assertThat(result.get(1), is(equalTo(event4)));
-    assertThat(result.get(2), is(equalTo(event6)));
+    assertThat(result.get(0), is(theInstance(event1)));
+
+    assertThat(
+        ((TCSObjectEvent) result.get(1)).getPreviousObjectState(),
+        is(theInstance(vehicle))
+    );
+    assertThat(
+        ((TCSObjectEvent) result.get(1)).getCurrentObjectState(),
+        is(theInstance(vehicleC))
+    );
+
+    assertThat(
+        ((TCSObjectEvent) result.get(2)).getPreviousObjectState(),
+        is(theInstance(pointA))
+    );
+    assertThat(
+        ((TCSObjectEvent) result.get(2)).getCurrentObjectState(),
+        is(theInstance(pointC))
+    );
   }
 
   @Test