Fix ready marker ordering on late subscriptions (#3454)

Signed-off-by: Jan N. Klug <[email protected]>
openhab · Mar 13, 2023 · c05fe8a · c05fe8a
1 parent 73b8a3a
commit c05fe8a
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 2 deletions.
diff --git a/...es/org.openhab.core/src/main/java/org/openhab/core/internal/service/ReadyServiceImpl.java b/...es/org.openhab.core/src/main/java/org/openhab/core/internal/service/ReadyServiceImpl.java
@@ -14,9 +14,9 @@
 
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.function.Consumer;
 
@@ -38,7 +38,7 @@ public class ReadyServiceImpl implements ReadyService {
     private final Logger logger = LoggerFactory.getLogger(ReadyServiceImpl.class);
     private static final ReadyMarkerFilter ANY = new ReadyMarkerFilter();
 
-    private final Set<ReadyMarker> markers = Collections.newSetFromMap(new ConcurrentHashMap<>());
+    private final Set<ReadyMarker> markers = Collections.synchronizedSet(new LinkedHashSet<>());
 
     private final Map<ReadyTracker, ReadyMarkerFilter> trackers = new HashMap<>();
     private final ReentrantReadWriteLock rwlTrackers = new ReentrantReadWriteLock(true);

diff --git a/...rg.openhab.core/src/test/java/org/openhab/core/internal/service/ReadyServiceImplTest.java b/...rg.openhab.core/src/test/java/org/openhab/core/internal/service/ReadyServiceImplTest.java
@@ -12,12 +12,17 @@
  */
 package org.openhab.core.internal.service;
 
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.ArgumentMatchers.isA;
 import static org.mockito.Mockito.*;
 
+import java.util.List;
+
 import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.hamcrest.Matchers;
 import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
 import org.openhab.core.service.ReadyMarker;
 import org.openhab.core.service.ReadyMarkerFilter;
 import org.openhab.core.service.ReadyService.ReadyTracker;
@@ -84,4 +89,17 @@ public void testUnregisterTracker() {
         verify(tracker).onReadyMarkerRemoved(isA(ReadyMarker.class));
         verifyNoMoreInteractions(tracker);
     }
+
+    @Test
+    public void testReadyMarkerOrderPreserved() {
+        ReadyTracker tracker = mock(ReadyTracker.class);
+        ReadyServiceImpl rs = new ReadyServiceImpl();
+        List<ReadyMarker> markers = List.of(new ReadyMarker("foo", "c"), new ReadyMarker("foo", "1"),
+                new ReadyMarker("foo", "a"), new ReadyMarker("foo", "3"));
+        markers.forEach(rs::markReady);
+        rs.registerTracker(tracker, new ReadyMarkerFilter().withType("foo"));
+        ArgumentCaptor<ReadyMarker> captor = ArgumentCaptor.forClass(ReadyMarker.class);
+        verify(tracker, times(4)).onReadyMarkerAdded(captor.capture());
+        assertThat(captor.getAllValues(), Matchers.contains(markers.toArray(ReadyMarker[]::new)));
+    }
 }