fix: correct sort order of pickups and drop-offs

2026-03-16 12:03:00 +01:00
parent f99a2d99a2
commit 44a9de7db7
4 changed files with 331 additions and 9 deletions
@@ -62,23 +62,87 @@ class PickupLoader extends AbstractLoader
    public function patchPickupsDetails(Travel $travel): void
    {
        $travel->pickups = $this->patchAndSortPickups($travel->pickups);
-        $travel->dropOffs = $this->patchAndSortPickups($travel->dropOffs);
+        $travel->dropOffs = $this->patchAndOrderDropOffs($travel->dropOffs, $travel->pickups);
    }

    private function patchAndSortPickups(array $pickups): array
    {
-        foreach ($pickups as $pickupId => $pickup) {
-            $pickupData = $this->loadById($pickupId);
+        $this->enrichPickupDetails($pickups);
+        uasort($pickups, fn (Pickup $a, Pickup $b): int => $this->comparePickupsByTime($a, $b));
+
+        return $pickups;
+    }
+
+    /**
+     * Enriches drop-off details and determines their order.
+     *
+     * When drop-offs carry their own time data, they are sorted chronologically.
+     * Otherwise the return journey order is derived by reversing the chronologically
+     * sorted outbound pickup order. Any drop-offs without a matching pickup ID are
+     * appended at the end.
+     *
+     * @param array<int, Pickup> $dropOffs      Drop-offs indexed by ID
+     * @param array<int, Pickup> $sortedPickups Already sorted pickups indexed by ID
+     *
+     * @return array<int, Pickup> Ordered drop-offs
+     */
+    private function patchAndOrderDropOffs(array $dropOffs, array $sortedPickups): array
+    {
+        $this->enrichPickupDetails($dropOffs);
+
+        if ($this->hasTimeData($dropOffs)) {
+            uasort($dropOffs, fn (Pickup $a, Pickup $b): int => $this->comparePickupsByTime($a, $b));
+
+            return $dropOffs;
+        }
+
+        // Fallback: order drop-offs in reverse of the sorted pickup route
+        $reversedPickupIds = array_reverse(array_keys($sortedPickups));
+
+        $ordered = [];
+        foreach ($reversedPickupIds as $id) {
+            if (isset($dropOffs[$id])) {
+                $ordered[$id] = $dropOffs[$id];
+            }
+        }
+
+        // Append any drop-offs not present in pickups
+        foreach ($dropOffs as $id => $dropOff) {
+            if (false === isset($ordered[$id])) {
+                $ordered[$id] = $dropOff;
+            }
+        }
+
+        return $ordered;
+    }
+
+    /**
+     * @param array<int, Pickup> $pickups
+     */
+    private function hasTimeData(array $pickups): bool
+    {
+        foreach ($pickups as $pickup) {
+            if (null !== $pickup->time) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * @param array<int, Pickup> $pickups Pickups indexed by ID (modified in place)
+     */
+    private function enrichPickupDetails(array $pickups): void
+    {
+        foreach ($pickups as $pickup) {
+            $pickupData = $this->loadById($pickup->id);

            $pickup->code = $pickupData->code;
            $pickup->postalCode = $pickupData->postalCode;
            $pickup->city = $pickupData->city;
            $pickup->street = $pickupData->street;
        }
-
-        uasort($pickups, fn (Pickup $a, Pickup $b): int => $this->comparePickupsByTime($a, $b));
-
-        return $pickups;
    }

    private function comparePickupsByTime(Pickup $a, Pickup $b): int
@@ -267,8 +267,9 @@ class TravelParser extends AbstractParser
    public function getPickups(Crawler $node, ?\DateTimeImmutable $defaultDate = null, bool $isOutbound = true): array
    {
        $pickups = [];
+        $timeOnlyIds = [];

-        $node->each(function (Crawler $pickupNode) use (&$pickups, $defaultDate, $isOutbound) {
+        $node->each(function (Crawler $pickupNode) use (&$pickups, &$timeOnlyIds, $defaultDate, $isOutbound) {
            $pickupId = (int) $pickupNode->attr('idbuspro');

            $pickup = new Pickup();
@@ -285,12 +286,19 @@ class TravelParser extends AbstractParser

            // parse date and time only for direction 'to' indicated by provided default date
            if (null !== $defaultDate) {
-                $pickup->time = $this->stringToDateTimeFuzzy($pickupNode->attr('zeit'), $defaultDate);
+                $timeValue = $pickupNode->attr('zeit');
+                $pickup->time = $this->stringToDateTimeFuzzy($timeValue, $defaultDate);
+
+                if (null !== $timeValue && 1 === preg_match('/^\d{2}:\d{2}$/', $timeValue)) {
+                    $timeOnlyIds[] = $pickupId;
+                }
            }

            $pickups[$pickupId] = $pickup;
        });

+        $this->adjustOvernightTimes($pickups, $timeOnlyIds);
+
        return $pickups;
    }

@@ -440,4 +448,43 @@ class TravelParser extends AbstractParser
            }
        }
    }
+
+    /**
+     * Adjusts time-only pickup entries for overnight bus routes.
+     *
+     * When a bus route crosses midnight, time-only entries (e.g. "01:00") are
+     * initially stamped with the departure date. This method detects overnight
+     * scenarios by comparing time-only entries against the latest full-datetime
+     * entry and adds one day where needed.
+     *
+     * @param array<int, Pickup> $pickups     Pickups indexed by ID (modified in place)
+     * @param array<int>         $timeOnlyIds IDs of pickups parsed from time-only values
+     */
+    private function adjustOvernightTimes(array $pickups, array $timeOnlyIds): void
+    {
+        if (true === empty($timeOnlyIds)) {
+            return;
+        }
+
+        // Find latest datetime among full-datetime entries
+        $latestFullDateTime = null;
+        foreach ($pickups as $id => $pickup) {
+            if (null !== $pickup->time && false === in_array($id, $timeOnlyIds, true)) {
+                if (null === $latestFullDateTime || $pickup->time > $latestFullDateTime) {
+                    $latestFullDateTime = $pickup->time;
+                }
+            }
+        }
+
+        if (null === $latestFullDateTime) {
+            return;
+        }
+
+        // Adjust time-only entries that fall before the latest full-datetime
+        foreach ($timeOnlyIds as $id) {
+            if (isset($pickups[$id]) && null !== $pickups[$id]->time && $pickups[$id]->time < $latestFullDateTime) {
+                $pickups[$id]->time = $pickups[$id]->time->modify('+1 day');
+            }
+        }
+    }
 }
@@ -0,0 +1,138 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\BusProNet\XmlLoader;
+
+use App\BusProNet\Model\Pickup;
+use App\BusProNet\Model\Travel;
+use App\BusProNet\XmlLoader\PickupLoader;
+use PHPUnit\Framework\TestCase;
+
+class PickupLoaderTest extends TestCase
+{
+    private PickupLoader $loader;
+
+    protected function setUp(): void
+    {
+        $this->loader = $this->createPartialMock(PickupLoader::class, ['loadById']);
+        $this->loader->method('loadById')->willReturnCallback(function (int $id): Pickup {
+            $pickup = new Pickup();
+            $pickup->id = $id;
+            $pickup->code = 'Z'.$id;
+            $pickup->city = 'City '.$id;
+            $pickup->postalCode = '0000'.$id;
+            $pickup->street = 'Street '.$id;
+
+            return $pickup;
+        });
+    }
+
+    public function testDropOffsAreOrderedInReverseOfSortedPickups(): void
+    {
+        $travel = new Travel();
+
+        // Pickups with times: 10 (18:00), 20 (20:00), 30 (22:00)
+        $p10 = new Pickup();
+        $p10->id = 10;
+        $p10->time = new \DateTimeImmutable('2029-12-30 18:00');
+
+        $p20 = new Pickup();
+        $p20->id = 20;
+        $p20->time = new \DateTimeImmutable('2029-12-30 20:00');
+
+        $p30 = new Pickup();
+        $p30->id = 30;
+        $p30->time = new \DateTimeImmutable('2029-12-30 22:00');
+
+        // Pickups in non-sorted order
+        $travel->pickups = [30 => $p30, 10 => $p10, 20 => $p20];
+
+        // Drop-offs in arbitrary XML order
+        $d10 = new Pickup();
+        $d10->id = 10;
+        $d20 = new Pickup();
+        $d20->id = 20;
+        $d30 = new Pickup();
+        $d30->id = 30;
+
+        $travel->dropOffs = [20 => $d20, 30 => $d30, 10 => $d10];
+
+        $this->loader->patchPickupsDetails($travel);
+
+        // Pickups should be sorted chronologically: 10, 20, 30
+        $this->assertSame([10, 20, 30], array_keys($travel->pickups));
+
+        // Drop-offs should be in reverse order: 30, 20, 10
+        $this->assertSame([30, 20, 10], array_keys($travel->dropOffs));
+    }
+
+    public function testDropOffsNotInPickupsAreAppendedAtEnd(): void
+    {
+        $travel = new Travel();
+
+        $p10 = new Pickup();
+        $p10->id = 10;
+        $p10->time = new \DateTimeImmutable('2029-12-30 18:00');
+
+        $p20 = new Pickup();
+        $p20->id = 20;
+        $p20->time = new \DateTimeImmutable('2029-12-30 20:00');
+
+        $travel->pickups = [10 => $p10, 20 => $p20];
+
+        // Drop-offs include ID 99 which has no matching pickup
+        $d10 = new Pickup();
+        $d10->id = 10;
+        $d20 = new Pickup();
+        $d20->id = 20;
+        $d99 = new Pickup();
+        $d99->id = 99;
+
+        $travel->dropOffs = [99 => $d99, 10 => $d10, 20 => $d20];
+
+        $this->loader->patchPickupsDetails($travel);
+
+        // Drop-offs: reversed pickups (20, 10), then unmatched (99)
+        $this->assertSame([20, 10, 99], array_keys($travel->dropOffs));
+    }
+
+    public function testDropOffsWithTimeDataAreSortedChronologically(): void
+    {
+        $travel = new Travel();
+
+        $p10 = new Pickup();
+        $p10->id = 10;
+        $p10->time = new \DateTimeImmutable('2029-12-30 18:00');
+
+        $p20 = new Pickup();
+        $p20->id = 20;
+        $p20->time = new \DateTimeImmutable('2029-12-30 20:00');
+
+        $p30 = new Pickup();
+        $p30->id = 30;
+        $p30->time = new \DateTimeImmutable('2029-12-30 22:00');
+
+        $travel->pickups = [30 => $p30, 10 => $p10, 20 => $p20];
+
+        // Drop-offs with their own time data (return journey times)
+        $d10 = new Pickup();
+        $d10->id = 10;
+        $d10->time = new \DateTimeImmutable('2030-01-05 14:00');
+
+        $d20 = new Pickup();
+        $d20->id = 20;
+        $d20->time = new \DateTimeImmutable('2030-01-05 12:00');
+
+        $d30 = new Pickup();
+        $d30->id = 30;
+        $d30->time = new \DateTimeImmutable('2030-01-05 10:00');
+
+        $travel->dropOffs = [10 => $d10, 20 => $d20, 30 => $d30];
+
+        $this->loader->patchPickupsDetails($travel);
+
+        // Drop-offs should be sorted by time, not by reversed pickup order
+        $this->assertSame([30, 20, 10], array_keys($travel->dropOffs));
+    }
+}
@@ -208,4 +208,77 @@ class TravelParserTest extends TestCase

        $this->assertNull($travel->status); // No status_hin node should result in null
    }
+
+    public function testGetPickupsAdjustsOvernightTimes(): void
+    {
+        $xml = '<?xml version="1.0" encoding="utf-8"?>
+<zustiege>
+    <zustieg idbuspro="1" zeit="30.12.2029 18:00" preis="0,00" />
+    <zustieg idbuspro="2" zeit="30.12.2029 20:30" preis="0,00" />
+    <zustieg idbuspro="3" zeit="23:00" preis="0,00" />
+    <zustieg idbuspro="4" zeit="01:00" preis="0,00" />
+</zustiege>';
+
+        $crawler = new Crawler($xml);
+        $defaultDate = new \DateTimeImmutable('2029-12-30');
+
+        $pickups = $this->parser->getPickups(
+            $crawler->filterXPath('//zustiege/zustieg'),
+            $defaultDate,
+            true
+        );
+
+        // Full-datetime entries keep their original date
+        $this->assertSame('2029-12-30 18:00', $pickups[1]->time->format('Y-m-d H:i'));
+        $this->assertSame('2029-12-30 20:30', $pickups[2]->time->format('Y-m-d H:i'));
+
+        // Time-only "23:00" is after latest full-datetime (20:30), so stays on same day
+        $this->assertSame('2029-12-30 23:00', $pickups[3]->time->format('Y-m-d H:i'));
+
+        // Time-only "01:00" is before latest full-datetime (20:30), so gets +1 day
+        $this->assertSame('2029-12-31 01:00', $pickups[4]->time->format('Y-m-d H:i'));
+    }
+
+    public function testGetPickupsDoesNotAdjustWhenAllEntriesAreFullDatetime(): void
+    {
+        $xml = '<?xml version="1.0" encoding="utf-8"?>
+<zustiege>
+    <zustieg idbuspro="1" zeit="30.12.2029 18:00" preis="0,00" />
+    <zustieg idbuspro="2" zeit="30.12.2029 20:30" preis="0,00" />
+</zustiege>';
+
+        $crawler = new Crawler($xml);
+        $defaultDate = new \DateTimeImmutable('2029-12-30');
+
+        $pickups = $this->parser->getPickups(
+            $crawler->filterXPath('//zustiege/zustieg'),
+            $defaultDate,
+            true
+        );
+
+        $this->assertSame('2029-12-30 18:00', $pickups[1]->time->format('Y-m-d H:i'));
+        $this->assertSame('2029-12-30 20:30', $pickups[2]->time->format('Y-m-d H:i'));
+    }
+
+    public function testGetPickupsDoesNotAdjustWhenAllEntriesAreTimeOnly(): void
+    {
+        $xml = '<?xml version="1.0" encoding="utf-8"?>
+<zustiege>
+    <zustieg idbuspro="1" zeit="18:00" preis="0,00" />
+    <zustieg idbuspro="2" zeit="01:00" preis="0,00" />
+</zustiege>';
+
+        $crawler = new Crawler($xml);
+        $defaultDate = new \DateTimeImmutable('2029-12-30');
+
+        $pickups = $this->parser->getPickups(
+            $crawler->filterXPath('//zustiege/zustieg'),
+            $defaultDate,
+            true
+        );
+
+        // No full-datetime reference entries, so no adjustment happens
+        $this->assertSame('2029-12-30 18:00', $pickups[1]->time->format('Y-m-d H:i'));
+        $this->assertSame('2029-12-30 01:00', $pickups[2]->time->format('Y-m-d H:i'));
+    }
 }