Skip to content

Commit 0d02b07

Browse files
st3inyst3iny
committed
perf: cache pre-fetched mailboxes on the HTTP level
Signed-off-by: Richard Steinmetz <[email protected]>
1 parent 3e94377 commit 0d02b07

File tree

7 files changed

+127
-7
lines changed

7 files changed

+127
-7
lines changed

‎lib/Controller/MessagesController.php‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ public function __construct(string $appName,
125125
* @param string $filter
126126
* @param int|null $limit
127127
* @param string $view returns messages in requested view ('singleton' or 'threaded')
128+
* @param string|null $v Cache buster version to guarantee unique urls (will trigger HTTP caching if set)
128129
*
129130
* @return JSONResponse
130131
*
@@ -136,7 +137,8 @@ public function index(int $mailboxId,
136137
?int $cursor = null,
137138
?string $filter = null,
138139
?int $limit = null,
139-
?string $view = null): JSONResponse {
140+
?string $view = null,
141+
?string $v = null): JSONResponse {
140142
try {
141143
$mailbox = $this->mailManager->getMailbox($this->currentUserId, $mailboxId);
142144
$account = $this->accountService->find($this->currentUserId, $mailbox->getAccountId());
@@ -145,7 +147,7 @@ public function index(int $mailboxId,
145147
}
146148

147149
$this->logger->debug("loading messages of mailbox <$mailboxId>");
148-
$sort = $this->preferences->getPreference($this->currentUserId, 'sort-order', 'newest') === 'newest' ? IMailSearch::ORDER_NEWEST_FIRST: IMailSearch::ORDER_OLDEST_FIRST;
150+
$sort = $this->preferences->getPreference($this->currentUserId, 'sort-order', 'newest') === 'newest' ? IMailSearch::ORDER_NEWEST_FIRST : IMailSearch::ORDER_OLDEST_FIRST;
149151

150152
$view = $view === 'singleton' ? IMailSearch::VIEW_SINGLETON : IMailSearch::VIEW_THREADED;
151153

@@ -159,9 +161,12 @@ public function index(int $mailboxId,
159161
$this->currentUserId,
160162
$view
161163
);
162-
return new JSONResponse(
163-
$messages
164-
);
164+
165+
$response = new JSONResponse($messages);
166+
if ($v !== null && $v !== '') {
167+
$response->cacheFor(7 * 24 * 3600, false, true);
168+
}
169+
return $response;
165170
}
166171

167172
/**

‎lib/Db/Mailbox.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,15 @@ public function getStats(): MailboxStats {
141141
return new MailboxStats($this->getMessages(), $this->getUnseen());
142142
}
143143

144+
public function getCacheBuster(): string {
145+
return hash('md5', implode('|', [
146+
(string)$this->getId(),
147+
$this->getSyncNewToken() ?? 'null',
148+
$this->getSyncChangedToken() ?? 'null',
149+
$this->getSyncVanishedToken() ?? 'null',
150+
]));
151+
}
152+
144153
#[\Override]
145154
#[ReturnTypeWillChange]
146155
public function jsonSerialize() {
@@ -160,6 +169,7 @@ public function jsonSerialize() {
160169
'unread' => $this->unseen,
161170
'myAcls' => $this->myAcls,
162171
'shared' => $this->shared === true,
172+
'cacheBuster' => $this->getCacheBuster(),
163173
];
164174
}
165175
}

‎src/components/Mailbox.vue‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,7 @@ export default {
274274
const envelopes = await this.mainStore.fetchEnvelopes({
275275
mailboxId: mailbox.databaseId,
276276
limit: this.initialPageSize,
277+
includeCacheBuster: true,
277278
})
278279
this.syncedMailboxes.add(mailbox.databaseId)
279280
logger.debug(`Prefetched ${envelopes.length} envelopes for folder ${mailbox.displayName} (${mailbox.databaseId})`)

‎src/service/MessageService.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export function fetchEnvelope(accountId, id) {
3232
})
3333
}
3434

35-
export function fetchEnvelopes(accountId, mailboxId, query, cursor, limit, sort, view) {
35+
export function fetchEnvelopes(accountId, mailboxId, query, cursor, limit, sort, view, cacheBuster) {
3636
const url = generateUrl('/apps/mail/api/messages')
3737
const params = {
3838
mailboxId,
@@ -53,6 +53,9 @@ export function fetchEnvelopes(accountId, mailboxId, query, cursor, limit, sort,
5353
if (view) {
5454
params.view = view
5555
}
56+
if (cacheBuster) {
57+
params.v = cacheBuster
58+
}
5659

5760
return axios
5861
.get(url, {

‎src/store/mainStore/actions.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -662,6 +662,7 @@ export default function mainStoreActions() {
662662
mailboxId,
663663
query,
664664
addToUnifiedMailboxes = true,
665+
includeCacheBuster = false,
665666
}) {
666667
return handleHttpAuthErrors(async () => {
667668
const mailbox = this.getMailbox(mailboxId)
@@ -709,7 +710,7 @@ export default function mainStoreActions() {
709710
}),
710711
),
711712
),
712-
)(mailbox.accountId, mailboxId, query, undefined, PAGE_SIZE, this.getPreference('sort-order'), this.getPreference('layout-message-view'))
713+
)(mailbox.accountId, mailboxId, query, undefined, PAGE_SIZE, this.getPreference('sort-order'), this.getPreference('layout-message-view'), includeCacheBuster ? mailbox.cacheBuster : undefined)
713714
})
714715
},
715716
async fetchNextEnvelopePage({

‎src/tests/unit/store/actions.spec.js‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,4 +646,45 @@ describe('Vuex store actions', () => {
646646

647647
expect(removeEnvelope).toBeFalsy()
648648
})
649+
650+
it('includes a cache buster if requested', async() => {
651+
const account = {
652+
id: 13,
653+
personalNamespace: 'INBOX.',
654+
mailboxes: [],
655+
}
656+
657+
store.addAccountMutation(account)
658+
store.addMailboxMutation({
659+
account,
660+
mailbox: {
661+
id: 'INBOX',
662+
name: 'INBOX',
663+
databaseId: 21,
664+
accountId: 13,
665+
specialRole: 'inbox',
666+
cacheBuster: 'abcdef123',
667+
},
668+
})
669+
670+
store.addEnvelopesMutation = jest.fn()
671+
672+
MessageService.fetchEnvelopes.mockResolvedValueOnce([])
673+
674+
await store.fetchEnvelopes({
675+
mailboxId: 21,
676+
includeCacheBuster: true,
677+
})
678+
679+
expect(MessageService.fetchEnvelopes).toHaveBeenCalledWith(
680+
13, // account id
681+
21, // mailbox id
682+
undefined, // query
683+
undefined, // ???
684+
20, // PAGE_SIZE
685+
undefined, // sort ordre
686+
undefined, // layout
687+
'abcdef123', // cache buster
688+
)
689+
})
649690
})

‎tests/Unit/Controller/MessagesControllerTest.php‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
use OCA\Mail\Contracts\IUserPreferences;
2525
use OCA\Mail\Controller\MessagesController;
2626
use OCA\Mail\Db\MailAccount;
27+
use OCA\Mail\Db\Mailbox;
28+
use OCA\Mail\Db\Message as DbMessage;
2729
use OCA\Mail\Db\Tag;
2830
use OCA\Mail\Exception\ClientException;
2931
use OCA\Mail\Exception\ServiceException;
@@ -1161,4 +1163,61 @@ public function testGetDkim() {
11611163
$this->assertInstanceOf(JSONResponse::class, $actualResponse);
11621164
$this->assertEquals(['valid' => true], $actualResponse->getData());
11631165
}
1166+
1167+
public static function provideCacheBusterData(): array {
1168+
return [
1169+
[null, false],
1170+
['', false],
1171+
['abcdef123', true],
1172+
];
1173+
}
1174+
1175+
/** @dataProvider provideCacheBusterData */
1176+
public function testIndexCacheBuster(?string $cacheBuster, bool $expectCaching): void {
1177+
$mailbox = new Mailbox();
1178+
$mailbox->setAccountId(100);
1179+
$this->mailManager->expects(self::once())
1180+
->method('getMailbox')
1181+
->with($this->userId, 100)
1182+
->willReturn($mailbox);
1183+
$mailAccount = new MailAccount();
1184+
$account = new Account($mailAccount);
1185+
$this->accountService->expects(self::once())
1186+
->method('find')
1187+
->with($this->userId, 100)
1188+
->willReturn($account);
1189+
1190+
$this->userPreferences->expects(self::once())
1191+
->method('getPreference')
1192+
->with($this->userId, 'sort-order', 'newest')
1193+
->willReturnArgument(2);
1194+
1195+
$messages = [
1196+
new DbMessage(),
1197+
new DbMessage(),
1198+
];
1199+
$this->mailSearch->expects(self::once())
1200+
->method('findMessages')
1201+
->with(
1202+
$account,
1203+
$mailbox,
1204+
'DESC',
1205+
null,
1206+
null,
1207+
null,
1208+
$this->userId,
1209+
'threaded',
1210+
)->willReturn($messages);
1211+
1212+
$actualResponse = $this->controller->index(100, null, null, null, null, $cacheBuster);
1213+
1214+
$cacheForHeader = $actualResponse->getHeaders()['Cache-Control'] ?? null;
1215+
$this->assertNotNull($cacheForHeader);
1216+
if ($expectCaching) {
1217+
$this->assertEquals('private, max-age=604800, immutable', $cacheForHeader);
1218+
} else {
1219+
$this->assertEquals('no-cache, no-store, must-revalidate', $cacheForHeader);
1220+
}
1221+
1222+
}
11641223
}

0 commit comments

Comments
 (0)