From 45e961c7ffd0cb29a907da82438de4433790ffa2 Mon Sep 17 00:00:00 2001 From: romanetar Date: Fri, 23 Oct 2026 19:24:44 +0200 Subject: [PATCH 1/3] fix(summit): compute dates_with_events with one query per summit instead of one per day Summit::getSummitDaysWithEvents ran one COUNT per summit day on every serialization that includes the default dates_with_events relation. It now reads the start/end of the published events once and applies the same per day overlap test (summit timezone bounds converted to UTC) in PHP. The resulting day list is cached per summit, keyed with the summit dates, for CACHE_API_RESPONSE_SUMMIT_DATES_WITH_EVENTS_LIFETIME seconds (default 300, 0 disables it). SQL errors are no longer swallowed, and the duplicated dates_with_events entry in SummitSerializer::$allowed_relations is removed. --- .env.example | 1 + .../Summit/SummitSerializer.php | 1 - app/Models/Foundation/Summit/Summit.php | 75 +++++++---- config/cache_api_response.php | 1 + tests/SummitDaysWithEventsTest.php | 126 ++++++++++++++++++ 5 files changed, 177 insertions(+), 27 deletions(-) create mode 100644 tests/SummitDaysWithEventsTest.php diff --git a/.env.example b/.env.example index af492e5760..3e81fbc7ab 100644 --- a/.env.example +++ b/.env.example @@ -26,6 +26,7 @@ REDIS_DB=0 REDIS_PASSWORD= CACHE_DRIVER=file +CACHE_API_RESPONSE_SUMMIT_DATES_WITH_EVENTS_LIFETIME=300 SESSION_DRIVER=redis SESSION_COOKIE_DOMAIN= diff --git a/app/ModelSerializers/Summit/SummitSerializer.php b/app/ModelSerializers/Summit/SummitSerializer.php index 564e6d342d..7002614cb4 100644 --- a/app/ModelSerializers/Summit/SummitSerializer.php +++ b/app/ModelSerializers/Summit/SummitSerializer.php @@ -198,7 +198,6 @@ class SummitSerializer extends SilverStripeSerializer 'badge_types', 'badge_features_types', 'badge_access_level_types', - 'dates_with_events', 'supported_currencies', ]; diff --git a/app/Models/Foundation/Summit/Summit.php b/app/Models/Foundation/Summit/Summit.php index f2a7dfc414..c1d393d810 100644 --- a/app/Models/Foundation/Summit/Summit.php +++ b/app/Models/Foundation/Summit/Summit.php @@ -6024,39 +6024,62 @@ public function getSummitDays(): array public function getSummitDaysWithEvents(): array { $days = $this->getSummitDays(); - $list = []; - foreach ($days as $day) { - $begin = clone($day); - $begin = $begin->setTime(0, 0, 0)->setTimezone(new \DateTimeZone('UTC')); - $end = clone($day); - $end = $end->setTime(23, 59, 59)->setTimezone(new \DateTimeZone('UTC')); - $count = 0; - try { - $sql = <<computeSummitDaysWithEvents($days); + }; + + if ($lifetime <= 0) { + $with_events = $compute(); + } else { + // the key carries the summit dates, so a stale list is never served after they change + $key = sprintf( + "summit_%s_dates_with_events_%s_%s", + $this->id, + $this->begin_date->getTimestamp(), + $this->end_date->getTimestamp() + ); + $with_events = Cache::remember($key, $lifetime, $compute); + } + + return array_values(array_filter($days, function (DateTime $day) use ($with_events) { + return in_array($day->format('Y-m-d'), $with_events); + })); + } + + /** + * Single query over the published events, the per day overlap test runs in PHP + * @param DateTime[] $days summit days (summit timezone) + * @return string[] Y-m-d of the days that have at least one published event + */ + private function computeSummitDaysWithEvents(array $days): array + { + $sql = <<= :begin) SQL; - $stmt = $this->prepareRawSQL($sql, [ - 'summit_id' => $this->id, - 'begin' => $begin->format("Y-m-d H:i:s"), - 'end' => $end->format('Y-m-d H:i:s'), - ]); - $res = $stmt->executeQuery(); - $res = $res->fetchFirstColumn(); - $count = count($res) > 0 ? $res[0] : 0; - } catch (\Exception $ex) { - Log::debug($ex); - $count = 0; - } - if ($count > 0) { - $list[] = $day; + $stmt = $this->prepareRawSQL($sql, ['summit_id' => $this->id]); + $events = $stmt->executeQuery()->fetchAllAssociative(); + + $list = []; + foreach ($days as $day) { + $begin = clone($day); + $begin = $begin->setTime(0, 0, 0)->setTimezone(new \DateTimeZone('UTC'))->format("Y-m-d H:i:s"); + $end = clone($day); + $end = $end->setTime(23, 59, 59)->setTimezone(new \DateTimeZone('UTC'))->format("Y-m-d H:i:s"); + foreach ($events as $event) { + // UTC "Y-m-d H:i:s" strings, same overlap test the SQL used to run + if ($event['StartDate'] <= $end && $event['EndDate'] >= $begin) { + $list[] = $day->format('Y-m-d'); + break; + } } } - return $list; } diff --git a/config/cache_api_response.php b/config/cache_api_response.php index cb728400e5..03be725410 100644 --- a/config/cache_api_response.php +++ b/config/cache_api_response.php @@ -20,4 +20,5 @@ 'get_summits_response_lifetime' => env('CACHE_API_RESPONSE_GET_SUMMITS_LIFETIME', 600), 'file_url_lifetime' => env('CACHE_API_RESPONSE_FILE_URL_LIFETIME', 3600), 'event_has_access_lifetime' => env('CACHE_API_RESPONSE_EVENT_HAS_ACCESS_LIFETIME', 600), + 'summit_dates_with_events_lifetime' => env('CACHE_API_RESPONSE_SUMMIT_DATES_WITH_EVENTS_LIFETIME', 300), ]; \ No newline at end of file diff --git a/tests/SummitDaysWithEventsTest.php b/tests/SummitDaysWithEventsTest.php new file mode 100644 index 0000000000..e12827eb91 --- /dev/null +++ b/tests/SummitDaysWithEventsTest.php @@ -0,0 +1,126 @@ +getSummitDays() as $day) { + $begin = (clone $day)->setTime(0, 0, 0)->setTimezone(new \DateTimeZone('UTC')); + $end = (clone $day)->setTime(23, 59, 59)->setTimezone(new \DateTimeZone('UTC')); + $count = DB::selectOne( + "SELECT COUNT(ID) AS QTY FROM SummitEvent WHERE SummitID = ? AND Published = 1 AND StartDate <= ? AND EndDate >= ?", + [$summit->getId(), $end->format('Y-m-d H:i:s'), $begin->format('Y-m-d H:i:s')] + )->QTY; + if ($count > 0) $res[] = $day->format('Y-m-d'); + } + return $res; + } + + private function daysWithEvents(Summit $summit): array + { + return array_map(function ($day) { + return $day->format('Y-m-d'); + }, $summit->getSummitDaysWithEvents()); + } + + public function testMatchesPerDayImplementation(): void + { + $expected = $this->legacyDaysWithEvents(self::$summit); + $this->assertNotEmpty($expected); + $this->assertSame($expected, $this->daysWithEvents(self::$summit)); + } + + public function testEventCrossingDaysCountsOnBothDaysAndDaysWithoutEventsAreSkipped(): void + { + $summit_id = self::$summit->getId(); + DB::update("UPDATE SummitEvent SET Published = 0 WHERE SummitID = ?", [$summit_id]); + $days = self::$summit->getSummitDays(); + + // one event from the evening of day 2 to the morning of day 3, summit timezone + $start = (clone $days[1])->setTime(22, 0, 0)->setTimezone(new \DateTimeZone('UTC')); + $end = (clone $days[2])->setTime(2, 0, 0)->setTimezone(new \DateTimeZone('UTC')); + $id = DB::selectOne("SELECT ID FROM SummitEvent WHERE SummitID = ? LIMIT 1", [$summit_id])->ID; + DB::update( + "UPDATE SummitEvent SET Published = 1, StartDate = ?, EndDate = ? WHERE ID = ?", + [$start->format('Y-m-d H:i:s'), $end->format('Y-m-d H:i:s'), $id] + ); + + $expected = [$days[1]->format('Y-m-d'), $days[2]->format('Y-m-d')]; + $this->assertSame($expected, $this->daysWithEvents(self::$summit)); + $this->assertSame($expected, $this->legacyDaysWithEvents(self::$summit)); + } + + public function testRunsASingleSelectRegardlessOfTheNumberOfDays(): void + { + $this->assertGreaterThan(5, count(self::$summit->getSummitDays())); + $conn = self::$em->getConnection(); + $selects = function () use ($conn): int { + return intval($conn->fetchAssociative("SHOW SESSION STATUS LIKE 'Com_select'")['Value']); + }; + $before = $selects(); + $this->daysWithEvents(self::$summit); + $this->assertSame(1, $selects() - $before); + } + + public function testCachedListIsServedWithinTheLifetime(): void + { + Config::set('cache_api_response.summit_dates_with_events_lifetime', 300); + Cache::flush(); + $first = $this->daysWithEvents(self::$summit); + $this->assertNotEmpty($first); + DB::update("UPDATE SummitEvent SET Published = 0 WHERE SummitID = ?", [self::$summit->getId()]); + $this->assertSame($first, $this->daysWithEvents(self::$summit)); + Config::set('cache_api_response.summit_dates_with_events_lifetime', 0); + $this->assertSame([], $this->daysWithEvents(self::$summit)); + Cache::flush(); + } +} From a9d2d0a3ffdccd91044d02cfbbb128b9ddcd2a50 Mon Sep 17 00:00:00 2001 From: romanetar Date: Fri, 23 Oct 2026 19:37:44 +0200 Subject: [PATCH 2/3] fix(summit): include the summit timezone in the dates_with_events cache key --- app/Models/Foundation/Summit/Summit.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/Models/Foundation/Summit/Summit.php b/app/Models/Foundation/Summit/Summit.php index c1d393d810..bb80597da6 100644 --- a/app/Models/Foundation/Summit/Summit.php +++ b/app/Models/Foundation/Summit/Summit.php @@ -6034,10 +6034,11 @@ public function getSummitDaysWithEvents(): array if ($lifetime <= 0) { $with_events = $compute(); } else { - // the key carries the summit dates, so a stale list is never served after they change + // the key carries the summit dates and timezone, so a stale list is never served after they change $key = sprintf( - "summit_%s_dates_with_events_%s_%s", + "summit_%s_dates_with_events_%s_%s_%s", $this->id, + preg_replace('/[^A-Za-z0-9]/', '_', strval($this->getTimeZoneId())), $this->begin_date->getTimestamp(), $this->end_date->getTimestamp() ); From 318939a8c7dad23fb073b945ad98e5e727ad6e7c Mon Sep 17 00:00:00 2001 From: romanetar Date: Fri, 23 Oct 2026 19:37:52 +0200 Subject: [PATCH 3/3] fix(summit): ignore published events without dates when computing dates_with_events --- app/Models/Foundation/Summit/Summit.php | 8 +++++++- tests/SummitDaysWithEventsTest.php | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/app/Models/Foundation/Summit/Summit.php b/app/Models/Foundation/Summit/Summit.php index bb80597da6..756245a506 100644 --- a/app/Models/Foundation/Summit/Summit.php +++ b/app/Models/Foundation/Summit/Summit.php @@ -6075,7 +6075,13 @@ private function computeSummitDaysWithEvents(array $days): array $end = $end->setTime(23, 59, 59)->setTimezone(new \DateTimeZone('UTC'))->format("Y-m-d H:i:s"); foreach ($events as $event) { // UTC "Y-m-d H:i:s" strings, same overlap test the SQL used to run - if ($event['StartDate'] <= $end && $event['EndDate'] >= $begin) { + // a published event may lack dates, SQL never matched those rows (NULL comparison) + if ( + $event['StartDate'] !== null && + $event['EndDate'] !== null && + $event['StartDate'] <= $end && + $event['EndDate'] >= $begin + ) { $list[] = $day->format('Y-m-d'); break; } diff --git a/tests/SummitDaysWithEventsTest.php b/tests/SummitDaysWithEventsTest.php index e12827eb91..a7d01692d5 100644 --- a/tests/SummitDaysWithEventsTest.php +++ b/tests/SummitDaysWithEventsTest.php @@ -99,6 +99,22 @@ public function testEventCrossingDaysCountsOnBothDaysAndDaysWithoutEventsAreSkip $this->assertSame($expected, $this->legacyDaysWithEvents(self::$summit)); } + public function testPublishedEventsWithoutDatesAreIgnored(): void + { + $summit_id = self::$summit->getId(); + DB::update("UPDATE SummitEvent SET Published = 0 WHERE SummitID = ?", [$summit_id]); + $days = self::$summit->getSummitDays(); + $end = (clone $days[2])->setTime(2, 0, 0)->setTimezone(new \DateTimeZone('UTC')); + $id = DB::selectOne("SELECT ID FROM SummitEvent WHERE SummitID = ? LIMIT 1", [$summit_id])->ID; + DB::update( + "UPDATE SummitEvent SET Published = 1, StartDate = NULL, EndDate = ? WHERE ID = ?", + [$end->format('Y-m-d H:i:s'), $id] + ); + + $this->assertSame([], $this->legacyDaysWithEvents(self::$summit)); + $this->assertSame([], $this->daysWithEvents(self::$summit)); + } + public function testRunsASingleSelectRegardlessOfTheNumberOfDays(): void { $this->assertGreaterThan(5, count(self::$summit->getSummitDays()));