Merge pull request #4518 from geoadmin/fix_PB-1319_fix_departure_time…

…s_1h_off

PB-1319: corrected timestamp formats and adapted format_time() to fix erroneous 1h offset for departure times

chsdi/lib/opentransapi/opentransapi.py

@@ -3,30 +3,33 @@
 import requests
 import xml.etree.ElementTree as et
 from pytz import timezone
+from pytz import UTC
 from datetime import datetime
-from dateutil import tz
+from dateutil.parser import isoparse
 import re
 
 
-def format_time(str_date_time, fmt="%Y-%m-%dT%H:%M:%SZ"):
-    from_zone = tz.tzutc()
-    to_zone = tz.gettz('Europe/Zurich')
+def format_time(str_date_time):
+    # Though the documentation of the OJP 2.0 API is not too verbose on this point (see:
+    # https://opentransportdata.swiss/de/cookbook/ojpstopeventrequest/), it seems, the
+    # timestamps are always returned in some form of ISO 8601 datetime format.
+    # Using isoparse() should be able to handle all the needed cases, e.g.
+    # - "Z" as timezone designator
+    # - timezone offsets, e.g. "+01:00"
+    # - sometimes the returned timestamps have an unexpected number of
+    #   fractional seconds, e.g. 7 instead of 6, should be handled, too
+    local_tz = timezone('Europe/Zurich')
+    date_time = isoparse(str_date_time)
 
-    try:
-        date_time = datetime.strptime(str_date_time, fmt)
-    except ValueError:
-        # sometimes the timestamp of the OJP 2.0 API's response has 7 digits for the
-        # milliseconds. 6 are expected and only 6 can be handled by Python.
-        # Hence we need to safely truncate everything between the last . and
-        # the +01:00 part of the timestamp, e.g.:
-        # 2024-11-01T15:39:45.5348804+01:00
-        # Use regex to capture and truncate everything between the last '.' and
-        # the first '+' to 6 digits
-        truncated_date_time = re.sub(r'(\.\d{6})\d*(?=\+)', r'\1', str_date_time)
-        date_time = datetime.strptime(truncated_date_time, '%Y-%m-%dT%H:%M:%S.%f%z')
-    date_time_utc = date_time.replace(tzinfo=from_zone)
-    date_time_zurich = date_time_utc.astimezone(to_zone)
-    return date_time_zurich.strftime('%d/%m/%Y %H:%M')
+    # If for some reason we receive a timezone-naive timestamp, we assume it is a
+    # local time
+    if date_time.tzinfo is None:
+        date_time = date_time.replace(tzinfo=local_tz)
+
+    # Convert to local timezone explicitly
+    local_date_time = date_time.astimezone(local_tz)
+    # Return time in local time, as needed.
+    return local_date_time.strftime('%d/%m/%Y %H:%M')
 
 
 class OpenTrans:
@@ -36,11 +39,14 @@ def __init__(self, open_trans_api_key, open_trans_url):
         self.url = open_trans_url  # URL of API
         self.station_id = None
 
-    def get_departures(self, station_id, number_results=5, request_dt_time=False):
-        if not request_dt_time:
-            request_dt_time = datetime.now(timezone('Europe/Zurich')).strftime('%Y-%m-%dT%H:%M:%S')
+    def get_departures(self, station_id, number_results=5):
+        # Note: according to https://opentransportdata.swiss/de/cookbook/ojpstopeventrequest/
+        # the timestamps used in the OJPStopEventRequest should preferably be in UTC
+        # and MUST include the seconds, in order to prevent their code from trying to interpret
+        # the given times as a form of local time!
+        request_dt_time = datetime.now(UTC).strftime('%Y-%m-%dT%H:%M:%SZ')
         self.station_id = station_id
-        api_response_xml = self.send_post(station_id, request_dt_time, number_results)  # request_dt_time in format 2017-12-11T14:26:18Z
+        api_response_xml = self.send_post(station_id, request_dt_time, number_results)  # UTC!
         results = self.xml_to_array(api_response_xml)
         return results
 
@@ -80,7 +86,7 @@ def xml_to_array(self, xml_data):
             results.append({
                 'id': el_id,
                 'label': el_service_name,
-                'currentDate': format_time(el_current_date, fmt="%Y-%m-%dT%H:%M:%S.%f%z"),
+                'currentDate': format_time(el_current_date),
                 'departureDate': format_time(el_departure_date),
                 'estimatedDate': self._convert_estimated_date(el.find('.//ojp:ServiceDeparture/ojp:EstimatedTime', ns)),
                 'destinationName': el_destination_name,
@@ -92,6 +98,10 @@ def create_ojp_payload(self, station_id, request_dt_time, number_results=5):
         # ATTENTION: The value "swisstopo_Abfahrtsmonitor" for the RequestorRef
         # in the payload below is suggested by the OJP product owner.
         # Hence it MUST NOT be changed!
+
+        # Further ATTENTION:
+        # Timestamps used for RequestTimestamp and DepArrTime MUST be in UTC, see:
+        # https://opentransportdata.swiss/de/cookbook/ojpstopeventrequest/
         payload = f"""<?xml version="1.0" encoding="UTF-8"?>
         <OJP xmlns='http://www.vdv.de/ojp' xmlns:siri='http://www.siri.org.uk/siri' version='2.0' xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance' xsi:schemaLocation='http://www.vdv.de/ojp ../../../../OJP4/OJP.xsd'>
             <OJPRequest>

tests/integration/test_opentransapi.py

@@ -54,6 +54,14 @@ def test_stationboard(self, mock_requests):
         self.assertEqual(results[0]["destinationName"], "Hogwarts")
         self.assertEqual(results[0]["destinationId"], "ch:1:sloid:91178::3")
 
+    def test_format_time(self):
+        # assert, that several timestamp formats are correctly handled and transformed into the
+        # correct local time
+        self.assertEqual(format_time("2024-11-19T08:52:00Z"), "19/11/2024 09:52")
+        self.assertEqual(format_time("2024-11-19T09:52:00.123456789"), "19/11/2024 09:52")
+        self.assertEqual(format_time("2024-11-19T08:52:00.123456789Z"), "19/11/2024 09:52")
+        self.assertEqual(format_time("2024-11-19T08:52:00+01:00"), "19/11/2024 08:52")
+
     @requests_mock.Mocker()
     def test_stationboard_nonexisting_station(self, mock_requests):
         now = datetime.now(timezone('Europe/Zurich')).isoformat(timespec="microseconds")