From 58e24fd942623a8894c12d2558715a1bb32023a8 Mon Sep 17 00:00:00 2001 From: eduardpiazza <91827007+eduardpiazza@users.noreply.github.com> Date: Fri, 24 Jul 2026 14:59:50 +0200 Subject: [PATCH] fix: preserve duplicate external appointment slots --- CHANGELOG.md | 4 + lib/Backend/BCSabreImpl.php | 189 +++++++++++++++++++++--- lib/Backend/BackendUtils.php | 1 + lib/Backend/ExternalSlotCapacity.php | 65 ++++++++ tests/Unit/ExternalSlotCapacityTest.php | 85 +++++++++++ 5 files changed, 325 insertions(+), 19 deletions(-) create mode 100644 lib/Backend/ExternalSlotCapacity.php create mode 100644 tests/Unit/ExternalSlotCapacityTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index bfabeebf..3e8d3d69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## Unreleased +### Fixed +- External mode keeps duplicate source slots available until each individual slot is booked - #668 + ## v2.7.4 - 2026-06-17 ### Added - Add support for Nextcloud 34 - #665 diff --git a/lib/Backend/BCSabreImpl.php b/lib/Backend/BCSabreImpl.php index 0543c3fa..1326d805 100644 --- a/lib/Backend/BCSabreImpl.php +++ b/lib/Backend/BCSabreImpl.php @@ -111,8 +111,18 @@ function queryRangePast($calIds, $end, $only_empty, $delete, $delete_test = fals * @return int 0=no events, 1=at least 1 * @noinspection PhpDocMissingThrowsInspection */ - private function checkRangeTR(int $start_ts, int $end_ts, string $calId, \DateTimeZone $utz, array $settings): int + private function checkRangeTR( + int $start_ts, + int $end_ts, + string $calId, + string $srcId, + string $srcUri, + \DateTimeZone $utz, + array $settings + ): int { + $slot_start_ts = $start_ts; + $slot_end_ts = $end_ts; $start = new \DateTime('@' . $start_ts); $start->setTimezone($utz); @@ -126,16 +136,34 @@ private function checkRangeTR(int $start_ts, int $end_ts, string $calId, \DateTi ? ['CATEGORIES:' . BackendUtils::APPT_CAT] : [] ]; + $capacity = new ExternalSlotCapacity(); + $sourceRangeStart = new \DateTime('@' . $queryConfig['start']); + $sourceRangeStart->setTimezone($utz); + $this->registerExternalSourceOccurrences( + $capacity, + $srcId, + $queryConfig, + $sourceRangeStart, + $queryConfig['end'] + ); + // we need to adjust/modify $start and $end by 1 sec because of "<=" and ">=" comparisons(instead of just "<" and ">" ) in the buildBusyTree function $start->modify('+1 second'); $start_ts = $start->getTimestamp(); $end_ts--; // -1 second - $booked_tree = $this->buildBusyTree([$calId], $queryConfig, $start, $start_ts, $end_ts, true); + $booked_tree = $this->buildBusyTree( + [$calId], + $queryConfig, + $start, + $start_ts, + $end_ts, + false, + $capacity + ); - // if $booked_tree is NOT null then there was a match(intersection) - return $booked_tree === null ? 0 : 1; + return $capacity->hasBookingConflict($srcUri, $slot_start_ts, $slot_end_ts, $booked_tree) ? 1 : 0; } @@ -169,10 +197,10 @@ private function queryRangeTR(string $calIds, \DateTime $start, \DateTime $end, ? ['CATEGORIES:' . BackendUtils::APPT_CAT] : [] ]; - $booked_tree = $this->buildBusyTree([$dstId], $queryConfig, $start, $start_ts, $end_ts, false); - // Get free/available spots $str_out = ''; + $slots = []; + $capacity = new ExternalSlotCapacity(); // '_'ts_mode(1byte)ses_time(4bytes)dates(8bytes)uri(no extension) $ses_info = '_1' . pack("L", time()); @@ -267,13 +295,14 @@ private function queryRangeTR(string $calIds, \DateTime $start, \DateTime $end, if ($s_ts > $start_ts) { $e_ts = $it->getDtEnd()->getTimestamp(); - if (AVLIntervalTree::lookUp($booked_tree, - $s_ts, $e_ts) === null) { - - $str_out .= $ts_pref . $s_ts - . ($showET ? ":" . $e_ts : "") - . ':' . $this->utils->encrypt($ses_info . pack("LL", $s_ts, $e_ts) . substr($row['uri'], 0, -4), $key) . $atl . ','; - } + $capacity->addSource($row['uri'], $s_ts, $e_ts); + $slots[] = [ + 'uri' => $row['uri'], + 'start' => $s_ts, + 'end' => $e_ts, + 'prefix' => $ts_pref, + 'title' => $atl, + ]; } $it->next(); } @@ -290,9 +319,103 @@ private function queryRangeTR(string $calIds, \DateTime $start, \DateTime $end, } $vo->destroy(); } + + $booked_tree = $this->buildBusyTree( + [$dstId], + $queryConfig, + $start, + $start_ts, + $end_ts, + false, + $capacity + ); + + foreach ($slots as $slot) { + if (!$capacity->isAvailable($slot['uri'], $slot['start'], $slot['end'], $booked_tree)) { + continue; + } + + $str_out .= $slot['prefix'] . $slot['start'] + . ($showET ? ":" . $slot['end'] : "") + . ':' . $this->utils->encrypt( + $ses_info . pack("LL", $slot['start'], $slot['end']) . substr($slot['uri'], 0, -4), + $key + ) . $slot['title'] . ','; + } return $str_out !== '' ? substr($str_out, 0, -1) : null; } + private function registerExternalSourceOccurrences( + ExternalSlotCapacity $capacity, + string $srcId, + array $queryConfig, + \DateTime $start, + int $end_ts + ): void { + $utz = $start->getTimezone(); + $iter = $this->fastQuery( + [$srcId], + $queryConfig['start'], + $queryConfig['end'], + $queryConfig['props'], + ['uri'] + ); + + foreach ($iter as $row) { + $cd = $row['calendardata']; + if (strpos($cd, "\r\nTRANSP:TRANSPARENT\r\n", 22) === false) { + continue; + } + + /** @var \Sabre\VObject\Component\VCalendar $vo */ + $vo = Reader::read($cd); + /** @var \Sabre\VObject\Component\VEvent $evt */ + $evt = $vo->VEVENT; + + if (!$evt->DTSTART->hasTime() + || $evt->DTSTART->isFloating() + || (isset($evt->CLASS) && $evt->CLASS->getValue() !== 'PUBLIC')) { + $vo->destroy(); + continue; + } + + if (isset($evt->RRULE)) { + try { + $it = new EventIterator($vo->getByUID($evt->UID->getValue()), null, $utz); + } catch (NoInstancesException $e) { + $vo->destroy(); + continue; + } + $it->fastForward($start); + } else { + $it = new FakeIterator($evt, $utz); + } + + $count = 0; + while ($it->valid() && $count < 384) { + $count++; + $_evt = $it->getEventObject(); + if (isset($_evt->STATUS) && $_evt->STATUS->getValue() === 'CANCELLED') { + $it->next(); + continue; + } + + $source_start_ts = $it->getDtStart()->getTimestamp(); + if ($source_start_ts >= $end_ts) { + break; + } + + $source_end_ts = $it->getDtEnd()->getTimestamp(); + if ($source_end_ts >= $queryConfig['start']) { + $capacity->addSource($row['uri'], $source_start_ts, $source_end_ts); + } + $it->next(); + } + + $vo->destroy(); + } + } + /** * @return int 0=ok, -1=error, 1=taken */ @@ -418,7 +541,8 @@ private function buildBusyTree(array $calIds, \DateTime $start, int $start_ts, int $end_ts, - bool $returnAfterFirstMatch = false): AVLIntervalNode|null + bool $returnAfterFirstMatch = false, + ExternalSlotCapacity|null $capacity = null): AVLIntervalNode|null { $utz = $start->getTimezone(); @@ -478,13 +602,27 @@ private function buildBusyTree(array $calIds, ($_evt->DTEND && $_evt->DTEND->hasTime())) ) { // an all-day event - $s_ts = $it->getDtStart()->getTimestamp(); - $e_ts = $s_ts + 86400; + $raw_start_ts = $it->getDtStart()->getTimestamp(); + $raw_end_ts = $raw_start_ts + 86400; + $s_ts = $raw_start_ts; + $e_ts = $raw_end_ts; } else { // an event with end-time or multi-day duration - $s_ts = $it->getDtStart()->getTimestamp() - $beforeBufferSec; - $e_ts = $it->getDtEnd()->getTimestamp() + $afterBufferSec; + $raw_start_ts = $it->getDtStart()->getTimestamp(); + $raw_end_ts = $it->getDtEnd()->getTimestamp(); + $s_ts = $raw_start_ts - $beforeBufferSec; + $e_ts = $raw_end_ts + $afterBufferSec; } + + $sourceUri = isset($_evt->{BackendUtils::X_APPT_SOURCE}) + ? $_evt->{BackendUtils::X_APPT_SOURCE}->getValue() + : null; + if ($capacity !== null + && $capacity->registerDestination($sourceUri, $raw_start_ts, $raw_end_ts)) { + $it->next(); + continue; + } + // start1 <= end2 && start2 <= end1 if ($start_ts <= $e_ts && $s_ts <= $end_ts) { @@ -1014,6 +1152,11 @@ function setAttendee($userId, $calId, $uri, $info) $parts['3_before_dte'] . $dt->setTimestamp($info['ext_end'])->format(self::TIME_FORMAT_NO_Z) . $parts['4_last']; + $booking = Reader::read($d); + $booking->VEVENT->add(BackendUtils::X_APPT_SOURCE, $srcUri); + $d = $booking->serialize(); + $booking->destroy(); + // Special "lock" uid $lock_uid = "LOCK_" . hash("tiger128,4", $info['ext_start'] . $info['ext_end'] . $info['ext_src_uri']); @@ -1075,7 +1218,15 @@ function setAttendee($userId, $calId, $uri, $info) // for external and template modes we need to re-check the time range and update the lock_uid to "real" uid or delete the lock_uid if the time range is "taken" if ($ts_mode === BackendUtils::CLS_TS_MODE_EXTERNAL) { - $trc = $this->checkRangeTR($info['ext_start'], $info['ext_end'], $calId, $utz, $settings); + $trc = $this->checkRangeTR( + $info['ext_start'], + $info['ext_end'], + $calId, + $srcId, + $srcUri, + $utz, + $settings + ); } else { // template mode $dt->setTimestamp($info['tmpl_start_ts']); diff --git a/lib/Backend/BackendUtils.php b/lib/Backend/BackendUtils.php index 9a251d88..93c2733b 100644 --- a/lib/Backend/BackendUtils.php +++ b/lib/Backend/BackendUtils.php @@ -30,6 +30,7 @@ class BackendUtils const TZI_PROP = "X-TZI"; // original description const X_DSR = "X-APPT-DSR"; + const X_APPT_SOURCE = "X-APPT-SOURCE"; // ################################################################# // WARNING: most of constants are used in V2 migration diff --git a/lib/Backend/ExternalSlotCapacity.php b/lib/Backend/ExternalSlotCapacity.php new file mode 100644 index 00000000..1fde3f8e --- /dev/null +++ b/lib/Backend/ExternalSlotCapacity.php @@ -0,0 +1,65 @@ +sources[$uri][$this->occurrenceKey($start, $end)] = true; + } + + public function claim(string $uri, int $start, int $end): bool + { + $key = $this->occurrenceKey($start, $end); + if (!isset($this->sources[$uri][$key])) { + return false; + } + + $this->claims[$uri][$key] = true; + return true; + } + + public function registerDestination(string|null $sourceUri, int $start, int $end): bool + { + return $sourceUri !== null && $this->claim($sourceUri, $start, $end); + } + + public function isClaimed(string $uri, int $start, int $end): bool + { + return isset($this->claims[$uri][$this->occurrenceKey($start, $end)]); + } + + public function isAvailable( + string $uri, + int $start, + int $end, + AVLIntervalNode|null $busyTree + ): bool { + $key = $this->occurrenceKey($start, $end); + + return isset($this->sources[$uri][$key]) + && !isset($this->claims[$uri][$key]) + && AVLIntervalTree::lookUp($busyTree, $start, $end) === null; + } + + public function hasBookingConflict( + string $uri, + int $start, + int $end, + AVLIntervalNode|null $busyTree + ): bool { + return !$this->isAvailable($uri, $start, $end, $busyTree); + } + + private function occurrenceKey(int $start, int $end): string + { + return $start . ':' . $end; + } +} diff --git a/tests/Unit/ExternalSlotCapacityTest.php b/tests/Unit/ExternalSlotCapacityTest.php new file mode 100644 index 00000000..7a8eba7d --- /dev/null +++ b/tests/Unit/ExternalSlotCapacityTest.php @@ -0,0 +1,85 @@ +addSource("slot-$i.ics", 1000, 2000); + } + + $this->assertTrue($capacity->claim('slot-0.ics', 1000, 2000)); + + $available = 0; + for ($i = 0; $i < 100; $i++) { + if ($capacity->isAvailable("slot-$i.ics", 1000, 2000, null)) { + $available++; + } + } + + $this->assertSame(99, $available); + } + + public function testSameSourceRecurrenceInstancesAreIndependent(): void + { + $capacity = new ExternalSlotCapacity(); + $capacity->addSource('weekly.ics', 1000, 2000); + $capacity->addSource('weekly.ics', 3000, 4000); + + $this->assertTrue($capacity->claim('weekly.ics', 1000, 2000)); + $this->assertFalse($capacity->isAvailable('weekly.ics', 1000, 2000, null)); + $this->assertTrue($capacity->isAvailable('weekly.ics', 3000, 4000, null)); + } + + public function testUnknownOrMovedClaimFallsBackToBlockerClassification(): void + { + $capacity = new ExternalSlotCapacity(); + $capacity->addSource('slot.ics', 1000, 2000); + + $this->assertFalse($capacity->claim('slot.ics', 1100, 2100)); + $this->assertFalse($capacity->claim('missing.ics', 1000, 2000)); + } + + public function testNormalBusyIntervalBlocksEveryDuplicateSlot(): void + { + $capacity = new ExternalSlotCapacity(); + $capacity->addSource('slot-a.ics', 1000, 2000); + $capacity->addSource('slot-b.ics', 1000, 2000); + $busyTree = null; + (new AVLIntervalTree())->insert($busyTree, 900, 2100); + + $this->assertFalse($capacity->isAvailable('slot-a.ics', 1000, 2000, $busyTree)); + $this->assertFalse($capacity->isAvailable('slot-b.ics', 1000, 2000, $busyTree)); + } + + public function testDestinationClaimsOnlyItsMatchingDuplicate(): void + { + $capacity = new ExternalSlotCapacity(); + $capacity->addSource('slot-a.ics', 1000, 2000); + $capacity->addSource('slot-b.ics', 1000, 2000); + + $this->assertTrue($capacity->registerDestination('slot-a.ics', 1000, 2000)); + $this->assertFalse($capacity->isAvailable('slot-a.ics', 1000, 2000, null)); + $this->assertTrue($capacity->isAvailable('slot-b.ics', 1000, 2000, null)); + $this->assertFalse($capacity->registerDestination(null, 1000, 2000)); + $this->assertFalse($capacity->registerDestination('slot-a.ics', 1100, 2100)); + } + + public function testClaimedOccurrenceIsABookingConflict(): void + { + $capacity = new ExternalSlotCapacity(); + $capacity->addSource('slot.ics', 1000, 2000); + + $this->assertFalse($capacity->hasBookingConflict('slot.ics', 1000, 2000, null)); + $this->assertTrue($capacity->claim('slot.ics', 1000, 2000)); + $this->assertTrue($capacity->hasBookingConflict('slot.ics', 1000, 2000, null)); + $this->assertTrue($capacity->hasBookingConflict('missing.ics', 1000, 2000, null)); + } +}