[deutschebahn] Fixed order of stops (#12406)

* Fixed order of stops
* As execution was made synchronous in #12235 timeout is not needed any more

Signed-off-by: Sönke Küper <soenkekueper@gmx.de>

bundles/org.openhab.binding.deutschebahn/src/main/java/org/openhab/binding/deutschebahn/internal/DeutscheBahnTimetableHandler.java

@@ -129,13 +129,7 @@ public class DeutscheBahnTimetableHandler extends BaseBridgeHandler {
         this.executorService = executorService == null ? this.scheduler : executorService;
     }
 
-    private List<TimetableStop> loadTimetable() {
+    private List<TimetableStop> loadTimetable(TimetableLoader currentLoader) {
-        final TimetableLoader currentLoader = this.loader;
-        if (currentLoader == null) {
-            this.updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.HANDLER_INITIALIZING_ERROR);
-            return Collections.emptyList();
-        }
-
         try {
             final List<TimetableStop> stops = currentLoader.getTimetableStops();
             this.updateStatus(ThingStatus.ONLINE);
@@ -171,7 +165,7 @@ public class DeutscheBahnTimetableHandler extends BaseBridgeHandler {
             }
 
             final EventType eventSelection = stopFilter == TimetableStopFilter.ARRIVALS ? EventType.ARRIVAL
-                    : EventType.ARRIVAL;
+                    : EventType.DEPARTURE;
 
             this.loader = new TimetableLoader( //
                     api, //
@@ -247,7 +241,7 @@ public class DeutscheBahnTimetableHandler extends BaseBridgeHandler {
         }
         final GroupedThings groupedThings = this.groupThingsPerPosition();
         currentLoader.setStopCount(groupedThings.getMaxPosition());
-        final List<TimetableStop> timetableStops = this.loadTimetable();
+        final List<TimetableStop> timetableStops = this.loadTimetable(currentLoader);
         if (timetableStops.isEmpty()) {
             updateThingsToUndefined(groupedThings);
             return;

bundles/org.openhab.binding.deutschebahn/src/test/java/org/openhab/binding/deutschebahn/internal/DeutscheBahnTimetableHandlerTest.java

@@ -50,18 +50,18 @@ import org.openhab.core.types.UnDefType;
 @NonNullByDefault
 public class DeutscheBahnTimetableHandlerTest implements TimetablesV1ImplTestHelper {
 
-    private static Configuration createConfig() {
+    private static Configuration createConfig(String trainFilter) {
         final Configuration config = new Configuration();
         config.put("accessToken", "letMeIn");
         config.put("evaNo", "8000226");
-        config.put("trainFilter", "all");
+        config.put("trainFilter", trainFilter);
         return config;
     }
 
-    private static Bridge mockBridge() {
+    private static Bridge mockBridge(String trainFilter) {
         final Bridge bridge = mock(Bridge.class);
         when(bridge.getUID()).thenReturn(new ThingUID(DeutscheBahnBindingConstants.TIMETABLE_TYPE, "timetable"));
-        when(bridge.getConfiguration()).thenReturn(createConfig());
+        when(bridge.getConfiguration()).thenReturn(createConfig(trainFilter));
 
         final List<Thing> things = new ArrayList<>();
         things.add(DeutscheBahnTrainHandlerTest.mockThing(1));
@@ -76,9 +76,9 @@ public class DeutscheBahnTimetableHandlerTest implements TimetablesV1ImplTestHel
         return bridge;
     }
 
-    private DeutscheBahnTimetableHandler createAndInitHandler(final ThingHandlerCallback callback, final Bridge bridge)
+    private DeutscheBahnTimetableHandler createAndInitHandler(final ThingHandlerCallback callback, final Bridge bridge,
-            throws Exception {
+            String dataDirectory) throws Exception {
-        return createAndInitHandler(callback, bridge, createApiWithTestdata().getApiFactory());
+        return createAndInitHandler(callback, bridge, createApiWithTestdata(dataDirectory).getApiFactory());
     }
 
     private DeutscheBahnTimetableHandler createAndInitHandler( //
@@ -103,10 +103,10 @@ public class DeutscheBahnTimetableHandlerTest implements TimetablesV1ImplTestHel
 
     @Test
     public void testUpdateChannels() throws Exception {
-        final Bridge bridge = mockBridge();
+        final Bridge bridge = mockBridge("all");
         final ThingHandlerCallback callback = mock(ThingHandlerCallback.class);
 
-        final DeutscheBahnTimetableHandler handler = createAndInitHandler(callback, bridge);
+        final DeutscheBahnTimetableHandler handler = createAndInitHandler(callback, bridge, "/timetablesData");
 
         try {
             verify(callback).statusUpdated(eq(bridge), argThat(arg -> arg.getStatus().equals(ThingStatus.UNKNOWN)));
@@ -120,6 +120,44 @@ public class DeutscheBahnTimetableHandlerTest implements TimetablesV1ImplTestHel
         }
     }
 
+    @Test
+    public void testStopsAreOrderedByDeparture() throws Exception {
+        final Bridge bridge = mockBridge("departures");
+        final ThingHandlerCallback callback = mock(ThingHandlerCallback.class);
+
+        final DeutscheBahnTimetableHandler handler = createAndInitHandler(callback, bridge,
+                "/timetablesDataDifferentOrder");
+
+        try {
+            verify(callback).statusUpdated(eq(bridge), argThat(arg -> arg.getStatus().equals(ThingStatus.UNKNOWN)));
+            verify(callback).statusUpdated(eq(bridge), argThat(arg -> arg.getStatus().equals(ThingStatus.ONLINE)));
+
+            verifyThingUpdated(bridge, 0, "-5296516961807204721-2108160906-5");
+            verifyThingUpdated(bridge, 1, "-8364795265993682073-2108160911-6");
+        } finally {
+            handler.dispose();
+        }
+    }
+
+    @Test
+    public void testStopsAreOrderedByArrival() throws Exception {
+        final Bridge bridge = mockBridge("arrivals");
+        final ThingHandlerCallback callback = mock(ThingHandlerCallback.class);
+
+        final DeutscheBahnTimetableHandler handler = createAndInitHandler(callback, bridge,
+                "/timetablesDataDifferentOrder");
+
+        try {
+            verify(callback).statusUpdated(eq(bridge), argThat(arg -> arg.getStatus().equals(ThingStatus.UNKNOWN)));
+            verify(callback).statusUpdated(eq(bridge), argThat(arg -> arg.getStatus().equals(ThingStatus.ONLINE)));
+
+            verifyThingUpdated(bridge, 0, "-8364795265993682073-2108160911-6");
+            verifyThingUpdated(bridge, 1, "-5296516961807204721-2108160906-5");
+        } finally {
+            handler.dispose();
+        }
+    }
+
     private void verifyThingUpdated(final Bridge bridge, int offset, String stopId) {
         final Thing train = bridge.getThings().get(offset);
         final DeutscheBahnTrainHandler childHandler = (DeutscheBahnTrainHandler) train.getHandler();
@@ -128,7 +166,7 @@ public class DeutscheBahnTimetableHandlerTest implements TimetablesV1ImplTestHel
 
     @Test
     public void testUpdateTrainsToUndefinedIfNoDataWasProvided() {
-        final Bridge bridge = mockBridge();
+        final Bridge bridge = mockBridge("all");
         final ThingHandlerCallback callback = mock(ThingHandlerCallback.class);
 
         final TimetablesV1ApiStub stubWithError = TimetablesV1ApiStub.createWithException();
@@ -158,7 +196,7 @@ public class DeutscheBahnTimetableHandlerTest implements TimetablesV1ImplTestHel
 
     @Test
     public void testUpdateTrainsToUndefinedIfNotEnoughDataWasProvided() {
-        final Bridge bridge = mockBridge();
+        final Bridge bridge = mockBridge("all");
         final ThingHandlerCallback callback = mock(ThingHandlerCallback.class);
 
         // Bridge contains 3 trains, but Timetable contains only 1 items, so two trains has to be updated to undef

bundles/org.openhab.binding.deutschebahn/src/test/java/org/openhab/binding/deutschebahn/internal/timetable/TimetablesV1ImplTestHelper.java

@@ -33,9 +33,19 @@ public interface TimetablesV1ImplTestHelper {
 
     /**
      * Creates an {@link TimetablesApiTestModule} that uses http response data from file system.
+     * Uses default-testdata from directory /timetablesData
      */
     public default TimetablesApiTestModule createApiWithTestdata() throws Exception {
-        final URL timetablesData = getClass().getResource("/timetablesData");
+        return this.createApiWithTestdata("/timetablesData");
+    }
+
+    /**
+     * Creates an {@link TimetablesApiTestModule} that uses http response data from file system.
+     * 
+     * @param dataDirectory Directory within test-resources containing the stub-data.
+     */
+    public default TimetablesApiTestModule createApiWithTestdata(String dataDirectory) throws Exception {
+        final URL timetablesData = getClass().getResource(dataDirectory);
         assertNotNull(timetablesData);
         final File testDataDir = new File(timetablesData.toURI());
         final TimetableStubHttpCallable httpStub = new TimetableStubHttpCallable(testDataDir);

bundles/org.openhab.binding.deutschebahn/src/test/resources/timetablesDataDifferentOrder/plan/8000226/210816/09.xml

@@ -0,0 +1,16 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<timetable station='Lehrte'>
+  <!-- Modified testdata where first train arrives before the second one and departs after second train. -->
+  <s id="-8364795265993682073-2108160911-6">
+    <tl f="S" t="p" o="800244" c="S" n="99031"/>
+    <ar pt="2108160934" pp="13" l="3" ppth="Celle|Ehlershausen|Otze|Burgdorf|Aligse"/>
+    <dp pt="2108160937" pp="13" l="3" ppth="Sehnde|Algermissen|Harsum|Hildesheim Hbf"/>
+  </s>
+
+  <s id="-5296516961807204721-2108160906-5">
+    <tl f="S" t="p" o="800244" c="S" n="99030"/>
+    <ar pt="2108160935" pp="14" l="3" ppth="Hildesheim Hbf|Harsum|Algermissen|Sehnde"/>
+    <dp pt="2108160936" pp="14" l="3" ppth="Aligse|Burgdorf|Otze|Ehlershausen|Celle"/>
+  </s>
+
+</timetable>