diff --git a/.env.example b/.env.example index af492e576..3e81fbc7a 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 564e6d342..7002614cb 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 f2a7dfc41..756245a50 100644 --- a/app/Models/Foundation/Summit/Summit.php +++ b/app/Models/Foundation/Summit/Summit.php @@ -6024,39 +6024,69 @@ 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 and timezone, so a stale list is never served after they change + $key = sprintf( + "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() + ); + $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 + // 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; + } } } - return $list; } diff --git a/config/cache_api_response.php b/config/cache_api_response.php index cb728400e..03be72541 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 000000000..a7d01692d --- /dev/null +++ b/tests/SummitDaysWithEventsTest.php @@ -0,0 +1,142 @@ +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 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())); + $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(); + } +}