Skip to content

Commit ddfc722

Browse files
mike182ukmike182uk
authored
Improved search perf by removing redundant field computation (#1471)
ref https://linear.app/ghost/issue/BER-3071 - Removed redundant `followerCount` computation by the search view - Updated the account view to conditional query for counts when needed
1 parent d83dde2 commit ddfc722

File tree

6 files changed

+137
-70
lines changed

6 files changed

+137
-70
lines changed

‎src/http/api/account.controller.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ export class AccountController {
6868

6969
const viewContext = {
7070
requestUserAccount: siteDefaultAccount,
71+
includeCounts: true,
7172
};
7273

7374
if (handle === CURRENT_USER_KEYWORD) {

‎src/http/api/search.controller.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ export type AccountSearchResult = Pick<
1515
| 'name'
1616
| 'handle'
1717
| 'avatarUrl'
18-
| 'followerCount'
1918
| 'followedByMe'
2019
| 'blockedByMe'
2120
| 'domainBlockedByMe'
@@ -27,7 +26,6 @@ function toSearchResult(dto: AccountDTO): AccountSearchResult {
2726
'name',
2827
'handle',
2928
'avatarUrl',
30-
'followerCount',
3129
'followedByMe',
3230
'blockedByMe',
3331
'domainBlockedByMe',

‎src/http/api/views/account.search.view.integration.test.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,6 @@ describe('AccountSearchView', () => {
172172
expect(accounts[0].avatarUrl).toBe(
173173
account.avatarUrl ? account.avatarUrl.toString() : null,
174174
);
175-
expect(accounts[0].followerCount).toBe(0);
176175
expect(accounts[0].followedByMe).toBe(false);
177176
expect(accounts[0].blockedByMe).toBe(false);
178177
expect(accounts[0].domainBlockedByMe).toBe(false);
@@ -762,7 +761,6 @@ describe('AccountSearchView', () => {
762761
expect(result!.avatarUrl).toBe(
763762
account.avatarUrl ? account.avatarUrl.toString() : null,
764763
);
765-
expect(result!.followerCount).toBeGreaterThanOrEqual(0);
766764
expect(result!.followedByMe).toBe(false);
767765
expect(result!.blockedByMe).toBe(false);
768766
expect(result!.domainBlockedByMe).toBe(false);

‎src/http/api/views/account.search.view.ts‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,6 @@ export class AccountSearchView {
9898
'accounts.avatar_url',
9999
)
100100
.where(whereClause)
101-
// Compute followerCount
102-
.select(
103-
this.db.raw(
104-
'(SELECT COUNT(*) FROM follows WHERE follows.following_id = accounts.id) as follower_count',
105-
),
106-
)
107101
// Compute followedByMe
108102
.select(
109103
this.db.raw(`
@@ -170,7 +164,6 @@ export class AccountSearchView {
170164
name: result.name || '',
171165
handle: getAccountHandle(result.domain, result.username),
172166
avatarUrl: result.avatar_url || null,
173-
followerCount: Number(result.follower_count),
174167
followedByMe: result.followed_by_me === 1,
175168
// blockedByMe and domainBlockedByMe are always false since we filter them out
176169
blockedByMe: false,

‎src/http/api/views/account.view.integration.test.ts‎

Lines changed: 63 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,33 @@ describe('AccountView', () => {
104104
expect(view).toBeNull();
105105
});
106106

107+
it('should return zero counts when includeCounts is false', async () => {
108+
const [account] = await fixtureManager.createInternalAccount();
109+
const [otherAccount] = await fixtureManager.createInternalAccount();
110+
111+
const post = Post.createFromData(account, {
112+
type: PostType.Article,
113+
audience: Audience.Public,
114+
});
115+
116+
post.addLike(account); // will cause likeCount to be 1
117+
118+
await postRepository.save(post); // will cause postCount to be 1
119+
120+
await fixtureManager.createFollow(otherAccount, account); // will cause followerCount to be 1
121+
await fixtureManager.createFollow(account, otherAccount); // will cause followingCount to be 1
122+
123+
const view = await accountView.viewById(account.id!, {
124+
includeCounts: false,
125+
});
126+
127+
expect(view).not.toBeNull();
128+
expect(view!.postCount).toBe(0);
129+
expect(view!.followerCount).toBe(0);
130+
expect(view!.followingCount).toBe(0);
131+
expect(view!.likedCount).toBe(0);
132+
});
133+
107134
it('should include the number of posts for the account', async () => {
108135
const [account] = await fixtureManager.createInternalAccount();
109136

@@ -114,7 +141,9 @@ describe('AccountView', () => {
114141
}),
115142
);
116143

117-
const view = await accountView.viewById(account.id!);
144+
const view = await accountView.viewById(account.id!, {
145+
includeCounts: true,
146+
});
118147

119148
expect(view).not.toBeNull();
120149
expect(view!.postCount).toBe(1);
@@ -130,7 +159,9 @@ describe('AccountView', () => {
130159
post.addLike(account);
131160
await postRepository.save(post);
132161

133-
const view = await accountView.viewById(account.id!);
162+
const view = await accountView.viewById(account.id!, {
163+
includeCounts: true,
164+
});
134165

135166
expect(view).not.toBeNull();
136167
expect(view!.likedCount).toBe(1);
@@ -146,7 +177,9 @@ describe('AccountView', () => {
146177
post.addRepost(account);
147178
await postRepository.save(post);
148179

149-
const view = await accountView.viewById(account.id!);
180+
const view = await accountView.viewById(account.id!, {
181+
includeCounts: true,
182+
});
150183

151184
expect(view).not.toBeNull();
152185
expect(view!.postCount).toBe(2);
@@ -158,7 +191,9 @@ describe('AccountView', () => {
158191

159192
await fixtureManager.createFollow(site2Account, siteAccount);
160193

161-
const view = await accountView.viewById(siteAccount.id!);
194+
const view = await accountView.viewById(siteAccount.id!, {
195+
includeCounts: true,
196+
});
162197

163198
expect(view).not.toBeNull();
164199
expect(view!.id).toBe(siteAccount.id);
@@ -172,7 +207,9 @@ describe('AccountView', () => {
172207

173208
await fixtureManager.createFollow(siteAccount, site2Account);
174209

175-
const view = await accountView.viewById(siteAccount.id!);
210+
const view = await accountView.viewById(siteAccount.id!, {
211+
includeCounts: true,
212+
});
176213

177214
expect(view).not.toBeNull();
178215
expect(view!.id).toBe(siteAccount.id);
@@ -189,6 +226,7 @@ describe('AccountView', () => {
189226

190227
const view = await accountView.viewById(siteAccount.id!, {
191228
requestUserAccount: requestUserAccount!,
229+
includeCounts: true,
192230
});
193231

194232
expect(view).not.toBeNull();
@@ -207,6 +245,7 @@ describe('AccountView', () => {
207245

208246
const view = await accountView.viewById(siteAccount.id!, {
209247
requestUserAccount: requestUserAccount!,
248+
includeCounts: true,
210249
});
211250

212251
expect(view).not.toBeNull();
@@ -376,7 +415,9 @@ describe('AccountView', () => {
376415
}),
377416
);
378417

379-
const view = await accountView.viewByApId(account.apId.toString());
418+
const view = await accountView.viewByApId(account.apId.toString(), {
419+
includeCounts: true,
420+
});
380421

381422
expect(view).not.toBeNull();
382423
expect(view!.postCount).toBe(1);
@@ -392,7 +433,9 @@ describe('AccountView', () => {
392433
post.addLike(account);
393434
await postRepository.save(post);
394435

395-
const view = await accountView.viewByApId(account.apId.toString());
436+
const view = await accountView.viewByApId(account.apId.toString(), {
437+
includeCounts: true,
438+
});
396439

397440
expect(view).not.toBeNull();
398441
expect(view!.likedCount).toBe(1);
@@ -408,7 +451,9 @@ describe('AccountView', () => {
408451
post.addRepost(account);
409452
await postRepository.save(post);
410453

411-
const view = await accountView.viewByApId(account.apId.toString());
454+
const view = await accountView.viewByApId(account.apId.toString(), {
455+
includeCounts: true,
456+
});
412457

413458
expect(view).not.toBeNull();
414459
expect(view!.postCount).toBe(2);
@@ -422,6 +467,7 @@ describe('AccountView', () => {
422467

423468
const view = await accountView.viewByApId(
424469
siteAccount.apId.toString(),
470+
{ includeCounts: true },
425471
);
426472

427473
expect(view).not.toBeNull();
@@ -438,6 +484,7 @@ describe('AccountView', () => {
438484

439485
const view = await accountView.viewByApId(
440486
siteAccount.apId.toString(),
487+
{ includeCounts: true },
441488
);
442489

443490
expect(view).not.toBeNull();
@@ -455,7 +502,10 @@ describe('AccountView', () => {
455502

456503
const view = await accountView.viewByApId(
457504
siteAccount.apId.toString(),
458-
{ requestUserAccount: requestUserAccount! },
505+
{
506+
requestUserAccount: requestUserAccount!,
507+
includeCounts: true,
508+
},
459509
);
460510

461511
expect(view).not.toBeNull();
@@ -474,7 +524,10 @@ describe('AccountView', () => {
474524

475525
const view = await accountView.viewByApId(
476526
siteAccount.apId.toString(),
477-
{ requestUserAccount: requestUserAccount! },
527+
{
528+
requestUserAccount: requestUserAccount!,
529+
includeCounts: true,
530+
},
478531
);
479532

480533
expect(view).not.toBeNull();

0 commit comments

Comments
 (0)