Skip to content

Commit 31ea920

Browse files
tchapitchapi
andauthored
Perf improvements for counts (#291)
Co-authored-by: tchapi <regbasket@gmail.com>
1 parent 71da975 commit 31ea920

9 files changed

Lines changed: 131 additions & 21 deletions

File tree

src/Controller/Admin/AddressBookController.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,13 @@ public function addressBooks(ManagerRegistry $doctrine, #[MapEntity(id: 'userId'
2626
$principal = $doctrine->getRepository(Principal::class)->findOneByUri($principalUri);
2727
$addressbooks = $doctrine->getRepository(AddressBook::class)->findByPrincipalUri($principalUri);
2828

29+
$cardCounts = $doctrine->getRepository(AddressBook::class)->countCardsByAddressBook(
30+
array_map(fn (AddressBook $addressbook) => $addressbook->getId(), $addressbooks)
31+
);
32+
2933
return $this->render('addressbooks/index.html.twig', [
3034
'addressbooks' => $addressbooks,
35+
'cardCounts' => $cardCounts,
3136
'principal' => $principal,
3237
'userId' => $userId,
3338
]);

src/Controller/Admin/CalendarController.php

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,30 +30,31 @@ public function calendars(ManagerRegistry $doctrine, UrlGeneratorInterface $rout
3030
$principalUri = $user->getPrincipalUri();
3131

3232
$principal = $doctrine->getRepository(Principal::class)->findOneByUri($principalUri);
33-
$allCalendars = $doctrine->getRepository(CalendarInstance::class)->findByPrincipalUri($principalUri);
33+
$allCalendars = $doctrine->getRepository(CalendarInstance::class)->findByPrincipalUriWithCalendars($principalUri);
3434

3535
$subscriptions = $doctrine->getRepository(CalendarSubscription::class)->findByPrincipalUri($principalUri);
3636

37+
$objectCounts = $doctrine->getRepository(CalendarInstance::class)->countObjectsByCalendar(
38+
array_map(fn (CalendarInstance $instance) => $instance->getCalendar()->getId(), $allCalendars)
39+
);
40+
3741
// Separate shared calendars
3842
$calendars = [];
3943
$shared = [];
4044
$auto = [];
4145
foreach ($allCalendars as $calendar) {
46+
$compoundObject = [
47+
'entity' => $calendar,
48+
'uri' => $router->generate('dav', ['path' => 'calendars/'.$username.'/'.$calendar->getUri()], UrlGeneratorInterface::ABSOLUTE_URL),
49+
'objectCount' => $objectCounts[$calendar->getCalendar()->getId()],
50+
];
51+
4252
if ($calendar->isAutomaticallyGenerated()) {
43-
$auto[] = [
44-
'entity' => $calendar,
45-
'uri' => $router->generate('dav', ['path' => 'calendars/'.$username.'/'.$calendar->getUri()], UrlGeneratorInterface::ABSOLUTE_URL),
46-
];
53+
$auto[] = $compoundObject;
4754
} elseif (!$calendar->isShared()) {
48-
$calendars[] = [
49-
'entity' => $calendar,
50-
'uri' => $router->generate('dav', ['path' => 'calendars/'.$username.'/'.$calendar->getUri()], UrlGeneratorInterface::ABSOLUTE_URL),
51-
];
55+
$calendars[] = $compoundObject;
5256
} else {
53-
$shared[] = [
54-
'entity' => $calendar,
55-
'uri' => $router->generate('dav', ['path' => 'calendars/'.$username.'/'.$calendar->getUri()], UrlGeneratorInterface::ABSOLUTE_URL),
56-
];
57+
$shared[] = $compoundObject;
5758
}
5859
}
5960

src/Controller/Admin/UserController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ public function userDelete(ManagerRegistry $doctrine, Request $request, #[MapEnt
152152
$principalUri = $user->getPrincipalUri();
153153

154154
// Remove calendars and addressbooks
155-
$calendars = $doctrine->getRepository(CalendarInstance::class)->findByPrincipalUri($principalUri);
155+
$calendars = $doctrine->getRepository(CalendarInstance::class)->findByPrincipalUriWithCalendars($principalUri);
156156
foreach ($calendars ?? [] as $instance) {
157157
// We're only removing the calendar objects / changes / and calendar if the deleted user is an owner,
158158
// which means that the underlying calendar instance should not have another principal as owner.

src/Controller/Api/ApiController.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ public function getUserCalendars(Request $request, int $userId, ManagerRegistry
168168
return $this->json(['status' => 'error', 'message' => 'Principal Not Found', 'timestamp' => $this->getTimestamp()], 404);
169169
}
170170

171-
$allCalendars = $doctrine->getRepository(CalendarInstance::class)->findByPrincipalUri($principalUri);
171+
$allCalendars = $doctrine->getRepository(CalendarInstance::class)->findByPrincipalUriWithCalendars($principalUri);
172172
$allSubscriptions = $doctrine->getRepository(CalendarSubscription::class)->findByPrincipalUri($principalUri);
173173

174174
$calendars = [];
@@ -243,7 +243,7 @@ public function getUserCalendarDetails(Request $request, int $userId, int $calen
243243
return $this->json(['status' => 'error', 'message' => 'Principal Not Found', 'timestamp' => $this->getTimestamp()], 404);
244244
}
245245

246-
$allCalendars = $doctrine->getRepository(CalendarInstance::class)->findByPrincipalUri($principalUri);
246+
$allCalendars = $doctrine->getRepository(CalendarInstance::class)->findByPrincipalUriWithCalendars($principalUri);
247247

248248
$calendar_details = [];
249249
foreach ($allCalendars as $calendar) {

src/Entity/AddressBook.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@
22

33
namespace App\Entity;
44

5+
use App\Repository\AddressBookRepository;
56
use Doctrine\Common\Collections\ArrayCollection;
67
use Doctrine\Common\Collections\Collection;
78
use Doctrine\ORM\Mapping as ORM;
89
use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity;
910
use Symfony\Component\Validator\Constraints as Assert;
1011

11-
#[ORM\Entity()]
12+
#[ORM\Entity(repositoryClass: AddressBookRepository::class)]
1213
#[ORM\Table(name: 'addressbooks')]
1314
#[UniqueEntity(fields: ['principalUri', 'uri'], errorPath: 'uri', message: 'form.uri.unique')]
1415
class AddressBook
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<?php
2+
3+
namespace App\Repository;
4+
5+
use App\Entity\AddressBook;
6+
use App\Entity\Card;
7+
use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
8+
use Doctrine\Persistence\ManagerRegistry;
9+
10+
/**
11+
* @method AddressBook|null find($id, $lockMode = null, $lockVersion = null)
12+
* @method AddressBook|null findOneBy(array $criteria, array $orderBy = null)
13+
* @method AddressBook[] findAll()
14+
* @method AddressBook[] findBy(array $criteria, array $orderBy = null, $limit = null, $offset = null)
15+
*/
16+
class AddressBookRepository extends ServiceEntityRepository
17+
{
18+
public function __construct(ManagerRegistry $registry)
19+
{
20+
parent::__construct($registry, AddressBook::class);
21+
}
22+
23+
/**
24+
* Counts the cards of several address books at once, so that listing a principal's address
25+
* books costs a single query instead of one per address book.
26+
*
27+
* @param int[] $addressBookIds
28+
*
29+
* @return array<int, int> count per address book id, including the address books that hold nothing
30+
*/
31+
public function countCardsByAddressBook(array $addressBookIds): array
32+
{
33+
$counts = array_fill_keys($addressBookIds, 0);
34+
35+
if (!$addressBookIds) {
36+
return $counts;
37+
}
38+
39+
$results = $this->getEntityManager()->getRepository(Card::class)
40+
->createQueryBuilder('c')
41+
->select('IDENTITY(c.addressBook) AS addressBookId, COUNT(c.id) AS count')
42+
->where('c.addressBook IN (:addressBookIds)')
43+
->setParameter('addressBookIds', $addressBookIds)
44+
->groupBy('c.addressBook')
45+
->getQuery()
46+
->getResult();
47+
48+
foreach ($results as $result) {
49+
$counts[(int) $result['addressBookId']] = (int) $result['count'];
50+
}
51+
52+
return $counts;
53+
}
54+
}

src/Repository/CalendarInstanceRepository.php

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,23 @@ public function __construct(ManagerRegistry $registry)
2323
parent::__construct($registry, CalendarInstance::class);
2424
}
2525

26+
/**
27+
* Returns every instance the principal has, with its `calendars` row already loaded. Every
28+
* caller reads it, so leaving it lazy costs one extra query per calendar.
29+
*
30+
* @return CalendarInstance[]
31+
*/
32+
public function findByPrincipalUriWithCalendars(string $principalUri): array
33+
{
34+
return $this->createQueryBuilder('c')
35+
->addSelect('cal')
36+
->join('c.calendar', 'cal')
37+
->where('c.principalUri = :principalUri')
38+
->setParameter('principalUri', $principalUri)
39+
->getQuery()
40+
->getResult();
41+
}
42+
2643
/**
2744
* @return CalendarInstance[] Returns an array of CalendarInstance objects
2845
*/
@@ -118,6 +135,38 @@ public function findAllSchedulingObjectsForCalendar(int $calendarInstanceId, str
118135
->getResult();
119136
}
120137

138+
/**
139+
* Counts the objects of several calendars at once, so that listing a principal's calendars
140+
* costs a single query instead of one per calendar.
141+
*
142+
* @param int[] $calendarIds
143+
*
144+
* @return array<int, int> count per calendar id, including the calendars that hold nothing
145+
*/
146+
public function countObjectsByCalendar(array $calendarIds): array
147+
{
148+
$counts = array_fill_keys($calendarIds, 0);
149+
150+
if (!$calendarIds) {
151+
return $counts;
152+
}
153+
154+
$results = $this->getEntityManager()->getRepository(CalendarObject::class)
155+
->createQueryBuilder('o')
156+
->select('IDENTITY(o.calendar) AS calendarId, COUNT(o.id) AS count')
157+
->where('o.calendar IN (:calendarIds)')
158+
->setParameter('calendarIds', $calendarIds)
159+
->groupBy('o.calendar')
160+
->getQuery()
161+
->getResult();
162+
163+
foreach ($results as $result) {
164+
$counts[(int) $result['calendarId']] = (int) $result['count'];
165+
}
166+
167+
return $counts;
168+
}
169+
121170
/**
122171
* Get counts of calendar objects by component type for a calendar instance.
123172
*

templates/addressbooks/index.html.twig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
</div>
2424
</div>
2525
<p class="mb-1">{{ addressbook.description }}</p>
26-
<small>{{ "addressbooks.uri"|trans }} : <code>{{ addressbook.uri }}</code> — {{ "addressbooks.contacts"|trans({'%count%': addressbook.cards|length}) }}</small>
26+
<small>{{ "addressbooks.uri"|trans }} : <code>{{ addressbook.uri }}</code> — {{ "addressbooks.contacts"|trans({'%count%': cardCounts[addressbook.id]}) }}</small>
2727
<div class="btn-group btn-group-sm mt-3 d-flex d-md-none" role="group">
2828
<a href="{{ path('addressbook_edit',{userId: userId, id: addressbook.id})}}" class="btn btn-sm btn-outline-primary">✎ {{ "edit"|trans }}</a>
2929
<a href="#"

templates/calendars/index.html.twig

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
{% if constant('\\App\\Entity\\Calendar::COMPONENT_NOTES') in calendar.calendar.components %}<span class="badge bg-info">{{ "calendars.component.notes"|trans }}</span>{% endif %}
4646
{% if constant('\\App\\Entity\\Calendar::COMPONENT_TODOS') in calendar.calendar.components %}<span class="badge bg-primary-subtle">{{ "calendars.component.todos"|trans }}</span>{% endif %}
4747
{% endif %}
48-
— {{ "calendars.entries"|trans({'%count%': calendar.calendar.objects|length}) }}
48+
— {{ "calendars.entries"|trans({'%count%': compoundObject.objectCount}) }}
4949
</small>
5050
<div class="btn-group btn-group-sm mt-3 d-flex d-md-none" role="group">
5151
{% if not calendar.isPublic() %}
@@ -103,7 +103,7 @@
103103
{% if constant('\\App\\Entity\\Calendar::COMPONENT_NOTES') in calendar.calendar.components %}<span class="badge bg-info">{{ "calendars.component.notes"|trans }}</span>{% endif %}
104104
{% if constant('\\App\\Entity\\Calendar::COMPONENT_TODOS') in calendar.calendar.components %}<span class="badge bg-primary-subtle">{{ "calendars.component.todos"|trans }}</span>{% endif %}
105105
{% endif %}
106-
— {{ "calendars.entries"|trans({'%count%': calendar.calendar.objects|length}) }}
106+
— {{ "calendars.entries"|trans({'%count%': compoundObject.objectCount}) }}
107107
</small>
108108
<div class="btn-group btn-group-sm mt-3 d-flex d-md-none" role="group">
109109
<a href="{{ path('calendar_edit',{userId: userId, id: calendar.id})}}" class="btn btn-outline-primary">✎ {{ "edit"|trans }}</a>
@@ -147,7 +147,7 @@
147147
{% if constant('\\App\\Entity\\Calendar::COMPONENT_NOTES') in calendar.calendar.components %}<span class="badge bg-info">{{ "calendars.component.notes"|trans }}</span>{% endif %}
148148
{% if constant('\\App\\Entity\\Calendar::COMPONENT_TODOS') in calendar.calendar.components %}<span class="badge bg-primary-subtle">{{ "calendars.component.todos"|trans }}</span>{% endif %}
149149
{% endif %}
150-
— {{ "calendars.entries"|trans({'%count%': calendar.calendar.objects|length}) }}
150+
— {{ "calendars.entries"|trans({'%count%': compoundObject.objectCount}) }}
151151
</small>
152152
<div class="btn-group btn-group-sm mt-3 d-flex d-md-none" role="group">
153153
<a href="{{ path('calendar_edit',{userId: userId, id: calendar.id})}}" class="btn btn-outline-primary">✎ {{ "edit"|trans }}</a>

0 commit comments

Comments
 (0)