amechama/sharkey
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

merge: Improvements and tweaks to latest note handling (resolves #744) (!688)

Browse Source 
View MR for information: https://activitypub.software/TransFem-org/Sharkey/-/merge_requests/688

Closes #744

Approved-by: dakkar <dakkar@thenautilus.net>
Approved-by: Marie <github@yuugi.dev>
This commit is contained in:
Marie 2024-10-15 21:50:32 +00:00
commit 7647aa637a
7 changed files with 262 additions and 106 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
packages/backend/src/core/CoreModule.ts
View File

@ -42,6 +42,7 @@ import { ModerationLogService } from './ModerationLogService.js';
import { NoteCreateService } from './NoteCreateService.js';
import { NoteEditService } from './NoteEditService.js';
import { NoteDeleteService } from './NoteDeleteService.js';
import { LatestNoteService } from './LatestNoteService.js';
import { NotePiningService } from './NotePiningService.js';
import { NoteReadService } from './NoteReadService.js';
import { NotificationService } from './NotificationService.js';
@ -185,6 +186,7 @@ const $ModerationLogService: Provider = { provide: 'ModerationLogService', useEx
const $NoteCreateService: Provider = { provide: 'NoteCreateService', useExisting: NoteCreateService };
const $NoteEditService: Provider = { provide: 'NoteEditService', useExisting: NoteEditService };
const $NoteDeleteService: Provider = { provide: 'NoteDeleteService', useExisting: NoteDeleteService };
const $LatestNoteService: Provider = { provide: 'LatestNoteService', useExisting: LatestNoteService };
const $NotePiningService: Provider = { provide: 'NotePiningService', useExisting: NotePiningService };
const $NoteReadService: Provider = { provide: 'NoteReadService', useExisting: NoteReadService };
const $NotificationService: Provider = { provide: 'NotificationService', useExisting: NotificationService };
@ -335,6 +337,7 @@ const $SponsorsService: Provider = { provide: 'SponsorsService', useExisting: Sp
NoteCreateService,
NoteEditService,
NoteDeleteService,
LatestNoteService,
NotePiningService,
NoteReadService,
NotificationService,
@ -481,6 +484,7 @@ const $SponsorsService: Provider = { provide: 'SponsorsService', useExisting: Sp
$NoteCreateService,
$NoteEditService,
$NoteDeleteService,
$LatestNoteService,
$NotePiningService,
$NoteReadService,
$NotificationService,
@ -628,6 +632,7 @@ const $SponsorsService: Provider = { provide: 'SponsorsService', useExisting: Sp
NoteCreateService,
NoteEditService,
NoteDeleteService,
LatestNoteService,
NotePiningService,
NoteReadService,
NotificationService,
@ -773,6 +778,7 @@ const $SponsorsService: Provider = { provide: 'SponsorsService', useExisting: Sp
$NoteCreateService,
$NoteEditService,
$NoteDeleteService,
$LatestNoteService,
$NotePiningService,
$NoteReadService,
$NotificationService,

139
packages/backend/src/core/LatestNoteService.ts Normal file
View File

@ -0,0 +1,139 @@
import { Inject, Injectable } from '@nestjs/common';
import { Not } from 'typeorm';
import { MiNote } from '@/models/Note.js';
import { isPureRenote } from '@/misc/is-renote.js';
import { SkLatestNote } from '@/models/LatestNote.js';
import { DI } from '@/di-symbols.js';
import type { LatestNotesRepository, NotesRepository } from '@/models/_.js';
import { LoggerService } from '@/core/LoggerService.js';
import Logger from '@/logger.js';
@Injectable()
export class LatestNoteService {
private readonly logger: Logger;
constructor(
@Inject(DI.notesRepository)
private notesRepository: NotesRepository,
@Inject(DI.latestNotesRepository)
private latestNotesRepository: LatestNotesRepository,
loggerService: LoggerService,
) {
this.logger = loggerService.getLogger('LatestNoteService');
}
handleUpdatedNoteBG(before: MiNote, after: MiNote): void {
this
.handleUpdatedNote(before, after)
.catch(err => this.logger.error('Unhandled exception while updating latest_note (after update):', err));
}
async handleUpdatedNote(before: MiNote, after: MiNote): Promise<void> {
// If the key didn't change, then there's nothing to update
if (SkLatestNote.areEquivalent(before, after)) return;
// Simulate update as delete + create
await this.handleDeletedNote(before);
await this.handleCreatedNote(after);
}
handleCreatedNoteBG(note: MiNote): void {
this
.handleCreatedNote(note)
.catch(err => this.logger.error('Unhandled exception while updating latest_note (after create):', err));
}
async handleCreatedNote(note: MiNote): Promise<void> {
// Ignore DMs.
// Followers-only posts are *included*, as this table is used to back the "following" feed.
if (note.visibility === 'specified') return;
// Ignore pure renotes
if (isPureRenote(note)) return;
// Compute the compound key of the entry to check
const key = SkLatestNote.keyFor(note);
// Make sure that this isn't an *older* post.
// We can get older posts through replies, lookups, updates, etc.
const currentLatest = await this.latestNotesRepository.findOneBy(key);
if (currentLatest != null && currentLatest.noteId >= note.id) return;
// Record this as the latest note for the given user
const latestNote = new SkLatestNote({
...key,
noteId: note.id,
});
await this.latestNotesRepository.upsert(latestNote, ['userId', 'isPublic', 'isReply', 'isQuote']);
}
handleDeletedNoteBG(note: MiNote): void {
this
.handleDeletedNote(note)
.catch(err => this.logger.error('Unhandled exception while updating latest_note (after delete):', err));
}
async handleDeletedNote(note: MiNote): Promise<void> {
// If it's a DM, then it can't possibly be the latest note so we can safely skip this.
if (note.visibility === 'specified') return;
// If it's a pure renote, then it can't possibly be the latest note so we can safely skip this.
if (isPureRenote(note)) return;
// Compute the compound key of the entry to check
const key = SkLatestNote.keyFor(note);
// Check if the deleted note was possibly the latest for the user
const existingLatest = await this.latestNotesRepository.findOneBy(key);
if (existingLatest == null || existingLatest.noteId !== note.id) return;
// Find the newest remaining note for the user.
// We exclude DMs and pure renotes.
const nextLatest = await this.notesRepository
.createQueryBuilder('note')
.select()
.where({
userId: key.userId,
visibility: key.isPublic
? 'public'
: Not('specified'),
replyId: key.isReply
? Not(null)
: null,
renoteId: key.isQuote
? Not(null)
: null,
})
.andWhere(`
(
note."renoteId" IS NULL
OR note.text IS NOT NULL
OR note.cw IS NOT NULL
OR note."replyId" IS NOT NULL
OR note."hasPoll"
OR note."fileIds" != '{}'
)
`)
.orderBy({ id: 'DESC' })
.getOne();
if (!nextLatest) return;
// Record it as the latest
const latestNote = new SkLatestNote({
...key,
noteId: nextLatest.id,
});
// When inserting the latest note, it's possible that another worker has "raced" the insert and already added a newer note.
// We must use orIgnore() to ensure that the query ignores conflicts, otherwise an exception may be thrown.
await this.latestNotesRepository
.createQueryBuilder('latest')
.insert()
.into(SkLatestNote)
.values(latestNote)
.orIgnore()
.execute();
}
}

38
packages/backend/src/core/NoteCreateService.ts
View File

@ -14,8 +14,7 @@ import { extractCustomEmojisFromMfm } from '@/misc/extract-custom-emojis-from-mf
import { extractHashtags } from '@/misc/extract-hashtags.js';
import type { IMentionedRemoteUsers } from '@/models/Note.js';
import { MiNote } from '@/models/Note.js';
import { SkLatestNote } from '@/models/LatestNote.js';
import type { ChannelFollowingsRepository, ChannelsRepository, FollowingsRepository, InstancesRepository, LatestNotesRepository, MiFollowing, MutingsRepository, NotesRepository, NoteThreadMutingsRepository, UserListMembershipsRepository, UserProfilesRepository, UsersRepository } from '@/models/_.js';
import type { ChannelFollowingsRepository, ChannelsRepository, FollowingsRepository, InstancesRepository, MiFollowing, MutingsRepository, NotesRepository, NoteThreadMutingsRepository, UserListMembershipsRepository, UserProfilesRepository, UsersRepository } from '@/models/_.js';
import type { MiDriveFile } from '@/models/DriveFile.js';
import type { MiApp } from '@/models/App.js';
import { concat } from '@/misc/prelude/array.js';
@ -63,7 +62,7 @@ import { isReply } from '@/misc/is-reply.js';
import { trackPromise } from '@/misc/promise-tracker.js';
import { isUserRelated } from '@/misc/is-user-related.js';
import { IdentifiableError } from '@/misc/identifiable-error.js';
import { isPureRenote } from '@/misc/is-renote.js';
import { LatestNoteService } from '@/core/LatestNoteService.js';
type NotificationType = 'reply' | 'renote' | 'quote' | 'mention';
@ -172,9 +171,6 @@ export class NoteCreateService implements OnApplicationShutdown {
@Inject(DI.notesRepository)
private notesRepository: NotesRepository,
@Inject(DI.latestNotesRepository)
private latestNotesRepository: LatestNotesRepository,
@Inject(DI.mutingsRepository)
private mutingsRepository: MutingsRepository,
@ -226,6 +222,7 @@ export class NoteCreateService implements OnApplicationShutdown {
private utilityService: UtilityService,
private userBlockingService: UserBlockingService,
private cacheService: CacheService,
private latestNoteService: LatestNoteService,
) { }
@bindThis
@ -531,8 +528,6 @@ export class NoteCreateService implements OnApplicationShutdown {
await this.notesRepository.insert(insert);
}
await this.updateLatestNote(insert);
return insert;
} catch (e) {
// duplicate key error
@ -815,6 +810,9 @@ export class NoteCreateService implements OnApplicationShutdown {
});
}
// Update the Latest Note index / following feed
this.latestNoteService.handleCreatedNoteBG(note);
// Register to search database
if (!user.noindex) this.index(note);
}
@ -1144,28 +1142,4 @@ export class NoteCreateService implements OnApplicationShutdown {
public onApplicationShutdown(signal?: string | undefined): void {
this.dispose();
}
private async updateLatestNote(note: MiNote) {
// Ignore DMs.
// Followers-only posts are *included*, as this table is used to back the "following" feed.
if (note.visibility === 'specified') return;
// Ignore pure renotes
if (isPureRenote(note)) return;
// Compute the compound key of the entry to check
const key = SkLatestNote.keyFor(note);
// Make sure that this isn't an *older* post.
// We can get older posts through replies, lookups, etc.
const currentLatest = await this.latestNotesRepository.findOneBy(key);
if (currentLatest != null && currentLatest.noteId >= note.id) return;
// Record this as the latest note for the given user
const latestNote = new SkLatestNote({
...key,
noteId: note.id,
});
await this.latestNotesRepository.upsert(latestNote, ['userId', 'isPublic', 'isReply', 'isQuote']);
}
}

78
packages/backend/src/core/NoteDeleteService.ts
View File

@ -3,12 +3,11 @@
* SPDX-License-Identifier: AGPL-3.0-only
*/
import { Brackets, In, Not } from 'typeorm';
import { Brackets, In } from 'typeorm';
import { Injectable, Inject } from '@nestjs/common';
import type { MiUser, MiLocalUser, MiRemoteUser } from '@/models/User.js';
import { MiNote, IMentionedRemoteUsers } from '@/models/Note.js';
import { SkLatestNote } from '@/models/LatestNote.js';
import type { InstancesRepository, LatestNotesRepository, NotesRepository, UsersRepository } from '@/models/_.js';
import type { InstancesRepository, NotesRepository, UsersRepository } from '@/models/_.js';
import { RelayService } from '@/core/RelayService.js';
import { FederatedInstanceService } from '@/core/FederatedInstanceService.js';
import { DI } from '@/di-symbols.js';
@ -20,12 +19,12 @@ import { GlobalEventService } from '@/core/GlobalEventService.js';
import { ApRendererService } from '@/core/activitypub/ApRendererService.js';
import { ApDeliverManagerService } from '@/core/activitypub/ApDeliverManagerService.js';
import { UserEntityService } from '@/core/entities/UserEntityService.js';
import { NoteEntityService } from '@/core/entities/NoteEntityService.js';
import { bindThis } from '@/decorators.js';
import { MetaService } from '@/core/MetaService.js';
import { SearchService } from '@/core/SearchService.js';
import { ModerationLogService } from '@/core/ModerationLogService.js';
import { isPureRenote, isQuote, isRenote } from '@/misc/is-renote.js';
import { isQuote, isRenote } from '@/misc/is-renote.js';
import { LatestNoteService } from '@/core/LatestNoteService.js';
@Injectable()
export class NoteDeleteService {
@ -39,14 +38,10 @@ export class NoteDeleteService {
@Inject(DI.notesRepository)
private notesRepository: NotesRepository,
@Inject(DI.latestNotesRepository)
private latestNotesRepository: LatestNotesRepository,
@Inject(DI.instancesRepository)
private instancesRepository: InstancesRepository,
private userEntityService: UserEntityService,
private noteEntityService: NoteEntityService,
private globalEventService: GlobalEventService,
private relayService: RelayService,
private federatedInstanceService: FederatedInstanceService,
@ -58,6 +53,7 @@ export class NoteDeleteService {
private notesChart: NotesChart,
private perUserNotesChart: PerUserNotesChart,
private instanceChart: InstanceChart,
private latestNoteService: LatestNoteService,
) {}
/**
@ -152,7 +148,7 @@ export class NoteDeleteService {
userId: user.id,
});
await this.updateLatestNote(note);
this.latestNoteService.handleDeletedNoteBG(note);
if (deleter && (note.userId !== deleter.id)) {
const user = await this.usersRepository.findOneByOrFail({ id: note.userId });
@ -235,66 +231,4 @@ export class NoteDeleteService {
this.apDeliverManagerService.deliverToUser(user, content, remoteUser);
}
}
private async updateLatestNote(note: MiNote) {
// If it's a DM, then it can't possibly be the latest note so we can safely skip this.
if (note.visibility === 'specified') return;
// If it's a pure renote, then it can't possibly be the latest note so we can safely skip this.
if (isPureRenote(note)) return;
// Compute the compound key of the entry to check
const key = SkLatestNote.keyFor(note);
// Check if the deleted note was possibly the latest for the user
const hasLatestNote = await this.latestNotesRepository.existsBy(key);
if (hasLatestNote) return;
// Find the newest remaining note for the user.
// We exclude DMs and pure renotes.
const nextLatest = await this.notesRepository
.createQueryBuilder('note')
.select()
.where({
userId: key.userId,
visibility: key.isPublic
? 'public'
: Not('specified'),
replyId: key.isReply
? Not(null)
: null,
renoteId: key.isQuote
? Not(null)
: null,
})
.andWhere(`
(
note."renoteId" IS NULL
OR note.text IS NOT NULL
OR note.cw IS NOT NULL
OR note."replyId" IS NOT NULL
OR note."hasPoll"
OR note."fileIds" != '{}'
)
`)
.orderBy({ id: 'DESC' })
.getOne();
if (!nextLatest) return;
// Record it as the latest
const latestNote = new SkLatestNote({
...key,
noteId: nextLatest.id,
});
// When inserting the latest note, it's possible that another worker has "raced" the insert and already added a newer note.
// We must use orIgnore() to ensure that the query ignores conflicts, otherwise an exception may be thrown.
await this.latestNotesRepository
.createQueryBuilder('latest')
.insert()
.into(SkLatestNote)
.values(latestNote)
.orIgnore()
.execute();
}
}

9
packages/backend/src/core/NoteEditService.ts
View File

@ -52,6 +52,7 @@ import { isReply } from '@/misc/is-reply.js';
import { trackPromise } from '@/misc/promise-tracker.js';
import { isUserRelated } from '@/misc/is-user-related.js';
import { IdentifiableError } from '@/misc/identifiable-error.js';
import { LatestNoteService } from '@/core/LatestNoteService.js';
type NotificationType = 'reply' | 'renote' | 'quote' | 'mention' | 'edited';
@ -214,6 +215,7 @@ export class NoteEditService implements OnApplicationShutdown {
private utilityService: UtilityService,
private userBlockingService: UserBlockingService,
private cacheService: CacheService,
private latestNoteService: LatestNoteService,
) { }
@bindThis
@ -558,7 +560,7 @@ export class NoteEditService implements OnApplicationShutdown {
}
setImmediate('post edited', { signal: this.#shutdownController.signal }).then(
() => this.postNoteEdited(note, user, data, silent, tags!, mentionedUsers!),
() => this.postNoteEdited(note, oldnote, user, data, silent, tags!, mentionedUsers!),
() => { /* aborted, ignore this */ },
);
@ -569,7 +571,7 @@ export class NoteEditService implements OnApplicationShutdown {
}
@bindThis
private async postNoteEdited(note: MiNote, user: {
private async postNoteEdited(note: MiNote, oldNote: MiNote, user: {
id: MiUser['id'];
username: MiUser['username'];
host: MiUser['host'];
@ -766,6 +768,9 @@ export class NoteEditService implements OnApplicationShutdown {
});
}
// Update the Latest Note index / following feed
this.latestNoteService.handleUpdatedNoteBG(oldNote, note);
// Register to search database
if (!user.noindex) this.index(note);
}

15
packages/backend/src/models/LatestNote.ts
View File

@ -82,4 +82,19 @@ export class SkLatestNote {
isQuote: isRenote(note) && isQuote(note),
};
}
/**
* Checks if two notes would produce equivalent compound keys.
*/
static areEquivalent(first: MiNote, second: MiNote): boolean {
const firstKey = SkLatestNote.keyFor(first);
const secondKey = SkLatestNote.keyFor(second);
return (
firstKey.userId === secondKey.userId &&
firstKey.isPublic === secondKey.isPublic &&
firstKey.isReply === secondKey.isReply &&
firstKey.isQuote === secondKey.isQuote
);
}
}

83
packages/backend/test/unit/models/LatestNote.ts
View File

@ -63,4 +63,87 @@ describe(SkLatestNote, () => {
expect(key.isQuote).toBeFalsy();
});
});
describe('areEquivalent', () => {
it('should return true when keys match', () => {
const first = new MiNote({ id: '1', userId: 'abc123', visibility: 'public', replyId: null, renoteId: null, fileIds: [] });
const second = new MiNote({ id: '2', userId: 'abc123', visibility: 'public', replyId: null, renoteId: null, fileIds: [] });
const result = SkLatestNote.areEquivalent(first, second);
expect(result).toBeTruthy();
});
it('should return true when keys match with different reply IDs', () => {
const first = new MiNote({ id: '1', userId: 'abc123', visibility: 'public', replyId: '3', renoteId: null, fileIds: [] });
const second = new MiNote({ id: '2', userId: 'abc123', visibility: 'public', replyId: '4', renoteId: null, fileIds: [] });
const result = SkLatestNote.areEquivalent(first, second);
expect(result).toBeTruthy();
});
it('should return true when keys match with different renote IDs', () => {
const first = new MiNote({ id: '1', userId: 'abc123', visibility: 'public', replyId: null, renoteId: '3', fileIds: ['1'] });
const second = new MiNote({ id: '2', userId: 'abc123', visibility: 'public', replyId: null, renoteId: '4', fileIds: ['1'] });
const result = SkLatestNote.areEquivalent(first, second);
expect(result).toBeTruthy();
});
it('should return true when keys match with different file counts', () => {
const first = new MiNote({ id: '1', userId: 'abc123', visibility: 'public', replyId: null, renoteId: null, fileIds: ['1'] });
const second = new MiNote({ id: '2', userId: 'abc123', visibility: 'public', replyId: null, renoteId: null, fileIds: ['1','2'] });
const result = SkLatestNote.areEquivalent(first, second);
expect(result).toBeTruthy();
});
it('should return true when keys match with different private visibilities', () => {
const first = new MiNote({ id: '1', userId: 'abc123', visibility: 'home', replyId: null, renoteId: null, fileIds: [] });
const second = new MiNote({ id: '2', userId: 'abc123', visibility: 'followers', replyId: null, renoteId: null, fileIds: [] });
const result = SkLatestNote.areEquivalent(first, second);
expect(result).toBeTruthy();
});
it('should return false when user ID differs', () => {
const first = new MiNote({ id: '1', userId: 'abc123', visibility: 'public', replyId: null, renoteId: null, fileIds: [] });
const second = new MiNote({ id: '2', userId: 'def456', visibility: 'public', replyId: null, renoteId: null, fileIds: [] });
const result = SkLatestNote.areEquivalent(first, second);
expect(result).toBeFalsy();
});
it('should return false when visibility differs', () => {
const first = new MiNote({ id: '1', userId: 'abc123', visibility: 'public', replyId: null, renoteId: null, fileIds: [] });
const second = new MiNote({ id: '2', userId: 'abc123', visibility: 'home', replyId: null, renoteId: null, fileIds: [] });
const result = SkLatestNote.areEquivalent(first, second);
expect(result).toBeFalsy();
});
it('should return false when reply differs', () => {
const first = new MiNote({ id: '1', userId: 'abc123', visibility: 'public', replyId: '1', renoteId: null, fileIds: [] });
const second = new MiNote({ id: '2', userId: 'abc123', visibility: 'public', replyId: null, renoteId: null, fileIds: [] });
const result = SkLatestNote.areEquivalent(first, second);
expect(result).toBeFalsy();
});
it('should return false when quote differs', () => {
const first = new MiNote({ id: '1', userId: 'abc123', visibility: 'public', replyId: null, renoteId: '3', fileIds: ['1'] });
const second = new MiNote({ id: '2', userId: 'abc123', visibility: 'public', replyId: null, renoteId: null, fileIds: [] });
const result = SkLatestNote.areEquivalent(first, second);
expect(result).toBeFalsy();
});
});
});