diff --git a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitAttendeesApiController.php b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitAttendeesApiController.php index c0c2a5c12..3d0f84f24 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitAttendeesApiController.php +++ b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitAttendeesApiController.php @@ -286,6 +286,9 @@ public function getAttendee($summit_id, $attendee_id) parameters: [ new OA\Parameter(name: 'id', in: 'path', required: true, description: 'Summit ID', schema: new OA\Schema(type: 'string')), new OA\Parameter(name: 'attendee_id', in: 'path', required: true, description: 'Attendee ID', schema: new OA\Schema(type: 'integer')), + new OA\Parameter(name: 'expand', description: 'Expand relations', in: 'query', schema: new OA\Schema(type: 'string')), + new OA\Parameter(name: 'fields', description: 'Fields to return', in: 'query', schema: new OA\Schema(type: 'string')), + new OA\Parameter(name: 'relations', description: 'Relations to include', in: 'query', schema: new OA\Schema(type: 'string')), ], responses: [ new OA\Response( @@ -310,10 +313,20 @@ public function getAttendeeSchedule($summit_id, $attendee_id) $attendee = CheckAttendeeStrategyFactory::build(CheckAttendeeStrategyFactory::Own, $this->resource_server_context)->check($attendee_id, $summit); if (is_null($attendee)) return $this->error404(); + $attendee_schedule_events = $attendee->getSchedule(); + $this->event_repository->preloadEventRelations + ( + array_map(fn($attendee_schedule) => $attendee_schedule->getEvent(), $attendee_schedule_events) + ); + $schedule = []; - foreach ($attendee->getSchedule() as $attendee_schedule) { - if (!$summit->isEventOnSchedule($attendee_schedule->getEvent()->getId())) continue; - $schedule[] = SerializerRegistry::getInstance()->getSerializer($attendee_schedule)->serialize(); + foreach ($attendee_schedule_events as $attendee_schedule) { + $schedule[] = SerializerRegistry::getInstance()->getSerializer($attendee_schedule)->serialize + ( + SerializerUtils::getExpand(), + SerializerUtils::getFields(), + SerializerUtils::getRelations() + ); } return $this->ok($schedule); diff --git a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitMembersApiController.php b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitMembersApiController.php index 5d4e25954..f5e6cc734 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitMembersApiController.php +++ b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitMembersApiController.php @@ -26,6 +26,7 @@ use models\exceptions\ValidationException; use models\main\IMemberRepository; use models\oauth2\IResourceServerContext; +use models\summit\ISummitEventRepository; use models\summit\ISummitRepository; use ModelSerializers\SerializerRegistry; use services\model\ISummitService; @@ -60,12 +61,18 @@ final class OAuth2SummitMembersApiController extends OAuth2ProtectedController */ private $rsvp_service; + /** + * @var ISummitEventRepository + */ + private $event_repository; + /** * OAuth2SummitMembersApiController constructor. * @param IMemberRepository $member_repository * @param ISummitRepository $summit_repository * @param ISummitService $summit_service * @param ISummitRSVPService $rsvp_service + * @param ISummitEventRepository $event_repository * @param IResourceServerContext $resource_server_context */ public function __construct @@ -74,6 +81,7 @@ public function __construct ISummitRepository $summit_repository, ISummitService $summit_service, ISummitRSVPService $rsvp_service, + ISummitEventRepository $event_repository, IResourceServerContext $resource_server_context ) { @@ -82,6 +90,7 @@ public function __construct $this->repository = $member_repository; $this->summit_service = $summit_service; $this->rsvp_service = $rsvp_service; + $this->event_repository = $event_repository; } use RequestProcessor; @@ -317,9 +326,14 @@ public function getMemberScheduleSummitEvents($summit_id, $member_id) $current_member = $this->resource_server_context->getCurrentUser(); if (is_null($current_member)) return $this->error403(); + $schedule_events = $current_member->getScheduleBySummit($summit); + $this->event_repository->preloadEventRelations + ( + array_map(fn($schedule_event) => $schedule_event->getEvent(), $schedule_events) + ); + $schedule = []; - foreach ($current_member->getScheduleBySummit($summit) as $schedule_event) { - if (!$summit->isEventOnSchedule($schedule_event->getEvent()->getId())) continue; + foreach ($schedule_events as $schedule_event) { $schedule[] = SerializerRegistry::getInstance()->getSerializer($schedule_event)->serialize ( SerializerUtils::getExpand(), diff --git a/app/Http/Middleware/Doctrine/QueryTimingCollector.php b/app/Http/Middleware/Doctrine/QueryTimingCollector.php index f20c70b86..98bf9d9e4 100644 --- a/app/Http/Middleware/Doctrine/QueryTimingCollector.php +++ b/app/Http/Middleware/Doctrine/QueryTimingCollector.php @@ -15,15 +15,24 @@ class QueryTimingCollector public static float $totalMs = 0.0; public static int $count = 0; - public static function record(float $startedAt): void + /** + * SQL text of every statement executed since reset(), or null while not capturing. Off by + * default: it exists so a test can tell which statements a request repeats, not just how many. + * @var string[]|null + */ + public static ?array $statements = null; + + public static function record(float $startedAt, ?string $sql = null): void { self::$totalMs += (microtime(true) - $startedAt) * 1000.0; self::$count++; + if (self::$statements !== null && $sql !== null) self::$statements[] = $sql; } public static function reset(): void { self::$totalMs = 0.0; self::$count = 0; + if (self::$statements !== null) self::$statements = []; } } diff --git a/app/Http/Middleware/Doctrine/QueryTimingMiddleware.php b/app/Http/Middleware/Doctrine/QueryTimingMiddleware.php index c0581efac..d8aa26416 100644 --- a/app/Http/Middleware/Doctrine/QueryTimingMiddleware.php +++ b/app/Http/Middleware/Doctrine/QueryTimingMiddleware.php @@ -43,7 +43,7 @@ public function query(string $sql): DBALResult try { return parent::query($sql); } finally { - QueryTimingCollector::record($start); + QueryTimingCollector::record($start, $sql); } } @@ -53,25 +53,30 @@ public function exec(string $sql): int try { return parent::exec($sql); } finally { - QueryTimingCollector::record($start); + QueryTimingCollector::record($start, $sql); } } public function prepare(string $sql): DBALStatement { - return new QueryTimingStatement(parent::prepare($sql)); + return new QueryTimingStatement(parent::prepare($sql), $sql); } } class QueryTimingStatement extends AbstractStatementMiddleware { + public function __construct(DBALStatement $statement, private string $sql) + { + parent::__construct($statement); + } + public function execute($params = null): DBALResult { $start = microtime(true); try { return parent::execute($params); } finally { - QueryTimingCollector::record($start); + QueryTimingCollector::record($start, $this->sql); } } } diff --git a/app/Models/Foundation/Main/Member.php b/app/Models/Foundation/Main/Member.php index 6ae4a1cd5..87c62d1d0 100644 --- a/app/Models/Foundation/Main/Member.php +++ b/app/Models/Foundation/Main/Member.php @@ -1451,7 +1451,9 @@ public function getRSVPInvitations(Summit $summit) public function getScheduleBySummit(Summit $summit) { - $query = $this->createQuery("SELECT s from models\main\SummitMemberSchedule s + // select the event too: it is read for every row when the schedule is serialized, + // and would otherwise be lazy-loaded one query per row + $query = $this->createQuery("SELECT s, e from models\main\SummitMemberSchedule s JOIN s.member m JOIN s.event e JOIN e.summit su WHERE su.id = :summit_id and m.id = :member_id and e.published = 1 "); diff --git a/app/Models/Foundation/Summit/Repositories/ISummitEventRepository.php b/app/Models/Foundation/Summit/Repositories/ISummitEventRepository.php index 3eaf2ff71..b20f7b221 100644 --- a/app/Models/Foundation/Summit/Repositories/ISummitEventRepository.php +++ b/app/Models/Foundation/Summit/Repositories/ISummitEventRepository.php @@ -38,6 +38,13 @@ public function getPublishedOnSameTimeFrame(IPublishableEvent $event): array; */ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Order $order = null); + /** + * Batch-preloads the relations the event serializers read per event, so serializing a list + * of events does not lazy-load them once per event. + * @param SummitEvent[] $events + */ + public function preloadEventRelations(array $events): void; + /** * @param PagingInfo $paging_info * @param Filter|null $filter diff --git a/app/Repositories/Summit/DoctrineSummitEventRepository.php b/app/Repositories/Summit/DoctrineSummitEventRepository.php index 8d285b37d..f431808cc 100644 --- a/app/Repositories/Summit/DoctrineSummitEventRepository.php +++ b/app/Repositories/Summit/DoctrineSummitEventRepository.php @@ -751,7 +751,35 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord if (isset($byId[$id])) $data[] = $byId[$id]; } - // Batch-preload Tag rows for every event on the page so the serializer's + $this->preloadEventRelations($data); + + if ($shuffleResults) shuffle($data); + + $end = time() - $start; + Log::debug("DoctrineSummitEventRepository::getAllByPage", ['seconds'=>$end]); + + return new PagingResponse + ( + $total, + $paging_info->getPerPage(), + $paging_info->getCurrentPage(), + $paging_info->getLastPage($total), + $data + ); + + } + + /** + * Batch-preloads the relations the event serializers read per event (tags, sponsors, and for + * presentations the selection status, speaker assignment order and materials), so serializing + * N events does not lazy-load them N times. + * @param SummitEvent[] $events + */ + public function preloadEventRelations(array $events): void + { + $ids = array_map(fn(SummitEvent $event) => $event->getId(), $events); + + // Batch-preload Tag rows for every given event so the serializer's // foreach (\$event->getTags()) does not lazy-load the collection per event. // SELECT e, t fetch-joins tags onto each SummitEvent. if (!empty($ids)) { @@ -762,7 +790,7 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord 'WHERE e.id IN (:ids)' )->setParameter('ids', $ids)->getResult(); } catch (\Exception $ex) { - Log::warning('DoctrineSummitEventRepository::getAllByPage tags preload failed', [ + Log::warning('DoctrineSummitEventRepository::preloadEventRelations tags preload failed', [ 'error' => $ex->getMessage(), ]); } @@ -775,21 +803,21 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord 'WHERE e.id IN (:ids)' )->setParameter('ids', $ids)->getResult(); } catch (\Exception $ex) { - Log::warning('DoctrineSummitEventRepository::getAllByPage sponsors preload failed', [ + Log::warning('DoctrineSummitEventRepository::preloadEventRelations sponsors preload failed', [ 'error' => $ex->getMessage(), ]); } } - // Targeted N+1 preload for Presentation rows on this page: + // Targeted N+1 preload for Presentation rows in the given set: // one DQL query that loads PresentationSpeakerAssignment + PresentationSpeaker + Member. // After this, identity-map lookups by the serializer no longer trigger: // - per-speaker Member SELECT (was 56 queries on a typical /events page) // - per-speaker Presentation_Speakers composite-key SELECT (was 19) // Net: ~75 queries collapsed into 1, ~216ms DB time saved per request. - // Only runs when there is at least one Presentation in the page. + // Only runs when there is at least one Presentation in the given set. $presentationIds = []; - foreach ($data as $event) { + foreach ($events as $event) { if ($event instanceof Presentation) $presentationIds[] = $event->getId(); } if (!empty($presentationIds)) { @@ -797,7 +825,7 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord $em = $this->getEntityManager(); // Batch-preload SummitSelectedPresentation rows for all presentations - // on this page. getSelectionStatus() on each Presentation then uses the + // in the given set. getSelectionStatus() on each Presentation then uses the // preloaded data instead of firing its own DQL per presentation. // Filters match getSelectionStatus()'s constants exactly. try { @@ -822,13 +850,13 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord $pid = $sp->getPresentation()->getId(); $byPresentation[$pid][] = $sp; } - foreach ($data as $event) { + foreach ($events as $event) { if ($event instanceof Presentation && method_exists($event, 'setPreloadedSessionSelections')) { $event->setPreloadedSessionSelections($byPresentation[$event->getId()] ?? []); } } } catch (\Exception $ex) { - Log::warning('DoctrineSummitEventRepository::getAllByPage selection-status preload failed', [ + Log::warning('DoctrineSummitEventRepository::preloadEventRelations selection-status preload failed', [ 'error' => $ex->getMessage(), ]); } @@ -863,32 +891,17 @@ public function getAllByPage(PagingInfo $paging_info, Filter $filter = null, Ord 'WHERE p.id IN (:ids)' )->setParameter('ids', $presentationIds)->getResult(); } catch (\Exception $ex) { - Log::warning('DoctrineSummitEventRepository::getAllByPage materials preload failed', [ + Log::warning('DoctrineSummitEventRepository::preloadEventRelations materials preload failed', [ 'error' => $ex->getMessage(), ]); } } catch (\Exception $ex) { - Log::warning('DoctrineSummitEventRepository::getAllByPage speaker+member preload failed', [ + Log::warning('DoctrineSummitEventRepository::preloadEventRelations speaker+member preload failed', [ 'error' => $ex->getMessage(), ]); } } - - if ($shuffleResults) shuffle($data); - - $end = time() - $start; - Log::debug("DoctrineSummitEventRepository::getAllByPage", ['seconds'=>$end]); - - return new PagingResponse - ( - $total, - $paging_info->getPerPage(), - $paging_info->getCurrentPage(), - $paging_info->getLastPage($total), - $data - ); - } /** diff --git a/tests/oauth2/OAuth2SummitScheduleQueryCountTest.php b/tests/oauth2/OAuth2SummitScheduleQueryCountTest.php new file mode 100644 index 000000000..9827314a8 --- /dev/null +++ b/tests/oauth2/OAuth2SummitScheduleQueryCountTest.php @@ -0,0 +1,358 @@ +current_group = IGroup::TrackChairs; + parent::setUp(); + self::$defaultMember = self::$member; + self::insertSummitTestData(); + } + + protected function tearDown(): void + { + $this->reloadEntityManager(); + self::clearSummitTestData(); + parent::tearDown(); + } + + /** + * Each simulated request closes the model entity manager at its end, and the next one opens a + * new one in the registry, so the instance the test kept is stale after a request. + */ + private function reloadEntityManager(): void + { + self::$em = Registry::getManager(SilverstripeBaseModel::EntityManager); + if (!self::$em->isOpen()) { + self::$em = Registry::resetManager(SilverstripeBaseModel::EntityManager); + } + } + + /** + * Puts the first $count published presentations of the fixture on the member's schedule. + */ + private function fillSchedule(int $count): void + { + $this->reloadEntityManager(); + $member = self::$em->find(Member::class, self::$defaultMember->getId()); + $scheduled = 0; + foreach (self::$presentations as $presentation) { + if ($scheduled >= $count) break; + $event = self::$em->find(SummitEvent::class, $presentation->getId()); + if (is_null($event) || !$event->isPublished()) continue; + if (!$member->isOnSchedule($event)) $member->add2Schedule($event); + $scheduled++; + } + self::$em->flush(); + $this->assertSame($count, $scheduled, 'precondition: the fixture has enough published events'); + } + + /** + * The resource server context memoizes the current user for the request. The test process serves + * several requests with the same context, so the Member it kept is detached by the clear() above. + */ + private function forgetCachedCurrentUser(): void + { + // the controllers and the serializer registry may each hold their own context instance + $registry_property = new \ReflectionProperty(SerializerRegistry::getInstance(), 'resource_server_context'); + $contexts = [App::make(IResourceServerContext::class), $registry_property->getValue(SerializerRegistry::getInstance())]; + foreach ($contexts as $context) { + foreach (['cachedCurrentUser' => null, 'cachedCurrentUserResolved' => false, 'groupsSynched' => false, 'fieldsSynched' => false] as $property => $value) { + $reflection = new \ReflectionProperty($context, $property); + $reflection->setValue($context, $value); + } + } + } + + /** + * @return array{0: int, 1: array, 2: string[]} number of queries the request issued, decoded payload, captured SQL statements + */ + private function requestSchedule(string $action, array $params, ?array $query = null): array + { + // Evict everything so the request loads the events fresh, like a real one. Fixture entities + // still hold their in-memory collections, which would hide the lazy loads under test. + self::$em->clear(); + $this->forgetCachedCurrentUser(); + + QueryTimingCollector::$statements = []; + QueryTimingCollector::reset(); + $response = $this->action( + "GET", + $action, + $params, + $query ?? ['relations' => self::Relations], + [], + [], + $this->getAuthHeaders() + ); + $count = QueryTimingCollector::$count; + $statements = QueryTimingCollector::$statements; + QueryTimingCollector::$statements = null; + $this->reloadEntityManager(); + + $this->assertResponseStatus(200); + $payload = json_decode($response->getContent(), true); + $this->assertIsArray($payload); + + return [$count, $payload, $statements]; + } + + private function tableCounts(array $statements): array + { + $counts = []; + foreach ($statements as $sql) { + if (!preg_match('/^\s*SELECT\b.*?\bFROM\s+`?(\w+)`?/is', $sql, $m)) continue; + $counts[$m[1]] = ($counts[$m[1]] ?? 0) + 1; + } + return $counts; + } + + /** + * @return array preloaded table => [statements for the small schedule, for the big one] + */ + private function scalingPreloadedTables(array $small_sql, array $big_sql): array + { + $small = $this->tableCounts($small_sql); + $big = $this->tableCounts($big_sql); + $scaling = []; + foreach (self::PreloadedTables as $table) { + if (($small[$table] ?? 0) !== ($big[$table] ?? 0)) $scaling[$table] = [$small[$table] ?? 0, $big[$table] ?? 0]; + } + return $scaling; + } + + /** + * @return int[] + */ + private function ids(array $events): array + { + return array_map(fn(array $event) => $event['id'], $events); + } + + public function testMemberSchedulePreloadsRelationsOnceForAllEvents(): void + { + $action = "OAuth2SummitMembersApiController@getMemberScheduleSummitEvents"; + $params = ['id' => self::$summit->getId(), 'member_id' => 'me']; + + $this->fillSchedule(self::SmallSchedule); + $this->requestSchedule($action, $params); // warm-up: metadata and query caches + [, $small, $small_sql] = $this->requestSchedule($action, $params); + $this->assertCount(self::SmallSchedule, $small['data']); + $this->assertNotEmpty($small_sql); + + $this->fillSchedule(self::BigSchedule); + [, $big, $big_sql] = $this->requestSchedule($action, $params); + $this->assertCount(self::BigSchedule, $big['data']); + + $this->assertSame( + [], + $this->scalingPreloadedTables($small_sql, $big_sql), + sprintf( + 'The member schedule queried these preloaded tables once per event (statements for %d events, for %d).', + self::SmallSchedule, self::BigSchedule + ) + ); + } + + public function testAttendeeSchedulePreloadsRelationsOnceForAllEvents(): void + { + $action = "OAuth2SummitAttendeesApiController@getAttendeeSchedule"; + $params = ['id' => self::$summit->getId(), 'attendee_id' => 'me']; + + $this->fillSchedule(self::SmallSchedule); + $this->requestSchedule($action, $params); // warm-up: metadata and query caches + [, $small, $small_sql] = $this->requestSchedule($action, $params); + $this->assertCount(self::SmallSchedule, $small); + $this->assertNotEmpty($small_sql); + + $this->fillSchedule(self::BigSchedule); + [, $big, $big_sql] = $this->requestSchedule($action, $params); + $this->assertCount(self::BigSchedule, $big); + + $this->assertSame( + [], + $this->scalingPreloadedTables($small_sql, $big_sql), + sprintf( + 'The attendee schedule queried these preloaded tables once per event (statements for %d events, for %d).', + self::SmallSchedule, self::BigSchedule + ) + ); + } + + /** + * The schedule of the fixture member serialized the way the endpoints did before the batch + * preloads: row by row, with nothing preloaded, so every relation is lazy-loaded per event. + * @return array[] + */ + private function legacySchedulePayload(): array + { + $this->reloadEntityManager(); + self::$em->clear(); + $this->forgetCachedCurrentUser(); + + $member = self::$em->find(Member::class, self::$defaultMember->getId()); + $summit = self::$em->find(Summit::class, self::$summit->getId()); + + $payload = []; + foreach ($member->getScheduleBySummit($summit) as $schedule_event) { + $payload[] = SerializerRegistry::getInstance()->getSerializer($schedule_event)->serialize(); + } + // same normalization the response goes through: json types, no object instances + return json_decode(json_encode($payload), true); + } + + /** + * Guards the comparison against comparing two empty shapes: the relations the preloads feed + * have to be in the payload for it to prove anything. + */ + private function assertPayloadCarriesPreloadedRelations(array $event): void + { + foreach (['id', 'tags', 'sponsors', 'speakers', 'media_uploads'] as $key) { + $this->assertArrayHasKey($key, $event); + } + } + + public function testMemberSchedulePayloadIsTheSameAsWithoutPreloads(): void + { + $this->fillSchedule(self::SmallSchedule); + + [, $actual] = $this->requestSchedule + ( + "OAuth2SummitMembersApiController@getMemberScheduleSummitEvents", + ['id' => self::$summit->getId(), 'member_id' => 'me'], + [] + ); + $expected = $this->legacySchedulePayload(); + + $this->assertCount(self::SmallSchedule, $expected); + $this->assertPayloadCarriesPreloadedRelations($expected[0]); + $this->assertEquals($expected, $actual['data']); + } + + public function testAttendeeSchedulePayloadIsTheSameAsWithoutPreloads(): void + { + $this->fillSchedule(self::SmallSchedule); + + [, $actual] = $this->requestSchedule + ( + "OAuth2SummitAttendeesApiController@getAttendeeSchedule", + ['id' => self::$summit->getId(), 'attendee_id' => 'me'], + [] + ); + $expected = $this->legacySchedulePayload(); + + $this->assertCount(self::SmallSchedule, $expected); + $this->assertPayloadCarriesPreloadedRelations($expected[0]); + $this->assertEquals($expected, $actual); + } + + /** + * The handlers used to drop, one by one, the events that are not published through + * Summit::isEventOnSchedule(). getScheduleBySummit() already filters on e.published, so an event + * that was published when it was scheduled and was unpublished since must still be left out. + */ + public function testUnpublishedEventsAreStillLeftOutOfTheSchedule(): void + { + $this->fillSchedule(self::SmallSchedule); + + $this->reloadEntityManager(); + self::$em->clear(); + $unpublished = self::$em->find(SummitEvent::class, self::$presentations[0]->getId()); + $unpublished->unPublish(); + self::$em->flush(); + + [, $member_schedule] = $this->requestSchedule + ( + "OAuth2SummitMembersApiController@getMemberScheduleSummitEvents", + ['id' => self::$summit->getId(), 'member_id' => 'me'], + [] + ); + [, $attendee_schedule] = $this->requestSchedule + ( + "OAuth2SummitAttendeesApiController@getAttendeeSchedule", + ['id' => self::$summit->getId(), 'attendee_id' => 'me'], + [] + ); + + $this->assertCount(self::SmallSchedule - 1, $member_schedule['data']); + $this->assertCount(self::SmallSchedule - 1, $attendee_schedule); + $this->assertNotContains($unpublished->getId(), $this->ids($member_schedule['data'])); + $this->assertNotContains($unpublished->getId(), $this->ids($attendee_schedule)); + } + + public function testMemberAndAttendeeSchedulesReturnTheSameEvents(): void + { + $this->fillSchedule(self::SmallSchedule); + + [, $member_schedule] = $this->requestSchedule( + "OAuth2SummitMembersApiController@getMemberScheduleSummitEvents", + ['id' => self::$summit->getId(), 'member_id' => 'me'] + ); + [, $attendee_schedule] = $this->requestSchedule( + "OAuth2SummitAttendeesApiController@getAttendeeSchedule", + ['id' => self::$summit->getId(), 'attendee_id' => 'me'] + ); + + $this->assertSame($this->ids($member_schedule['data']), $this->ids($attendee_schedule)); + } +}