johan/FoundKey
1
0
Fork 0
forked from FoundKeyGang/FoundKey
Code Issues Pull requests Projects Releases Packages Wiki Activity

server: refactor to group deletion deliveries

Browse source 
The `quiet` argument has been removed from `deleteNote` (or `deleteNotes`
respectively) since it was not used anywhere and it does not seem a good
idea to not update statistics in some cases.

This should also fix an issue where cascade deletions mean that statistics
are not properly updated or streaming clients not properly informed of
deletions. This case was seemingly not considered before, even though
there was some handling for cascade deleted notes.

This is going to improve how cascade deletion impacts the delivery queue,
because cascade-deleted notes will now be grouped for delivery.

Changelog: Fixed
This commit is contained in:
Johann150 2023-03-11 19:38:56 +01:00
parent 383ea40704
commit 6a17dcf4de
Signed by untrusted user: Johann150
GPG key ID: 9EE6577A2A06F8F1
5 changed files with 99 additions and 75 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
packages/backend/src/remote/activitypub/kernel/delete/note.ts
View file

@ -1,5 +1,5 @@
import { CacheableRemoteUser } from '@/models/entities/user.js';
import { deleteNote } from '@/services/note/delete.js';
import { deleteNotes } from '@/services/note/delete.js';
import { getApLock } from '@/misc/app-lock.js';
import { deleteMessage } from '@/services/messages/delete.js';
import { DbResolver } from '@/remote/activitypub/db-resolver.js';
@ -29,7 +29,7 @@ export default async function(actor: CacheableRemoteUser, uri: string): Promise<
return 'skip: cant delete other actors note';
}
await deleteNote(actor, note);
await deleteNotes([note], actor);
return 'ok: note deleted';
}
} finally {

4
packages/backend/src/remote/activitypub/kernel/undo/announce.ts
View file

@ -1,6 +1,6 @@
import { Notes } from '@/models/index.js';
import { CacheableRemoteUser } from '@/models/entities/user.js';
import { deleteNote } from '@/services/note/delete.js';
import { deleteNotes } from '@/services/note/delete.js';
import { IAnnounce, getApId } from '@/remote/activitypub/type.js';
export const undoAnnounce = async (actor: CacheableRemoteUser, activity: IAnnounce): Promise<string> => {
@ -13,6 +13,6 @@ export const undoAnnounce = async (actor: CacheableRemoteUser, activity: IAnnoun
if (!note) return 'skip: no such Announce';
await deleteNote(actor, note);
await deleteNotes([note], actor);
return 'ok: deleted';
};

6
packages/backend/src/server/api/endpoints/notes/delete.ts
View file

@ -1,4 +1,4 @@
import { deleteNote } from '@/services/note/delete.js';
import { deleteNotes } from '@/services/note/delete.js';
import { Users } from '@/models/index.js';
import { SECOND, HOUR } from '@/const.js';
import define from '@/server/api/define.js';
@ -47,6 +47,6 @@ export default define(meta, paramDef, async (ps, user) => {
throw new ApiError('ACCESS_DENIED');
}
// Fetch the note owner, since the current user may be an admin or moderator.
await deleteNote(await Users.findOneByOrFail({ id: note.userId }), note);
// Here, we do not provide the current user because it may be an admin/moderator.
await deleteNotes([note]);
});

8
packages/backend/src/server/api/endpoints/notes/unrenote.ts
View file

@ -1,4 +1,4 @@
import { deleteNote } from '@/services/note/delete.js';
import { deleteNotes } from '@/services/note/delete.js';
import { Notes, Users } from '@/models/index.js';
import { SECOND, HOUR } from '@/const.js';
import define from '@/server/api/define.js';
@ -48,7 +48,7 @@ export default define(meta, paramDef, async (ps, user) => {
renoteId: note.id,
});
for (const note of renotes) {
deleteNote(await Users.findOneByOrFail({ id: user.id }), note);
}
if (renotes.length === 0) return;
await deleteNotes(renotes, user);
});

152
packages/backend/src/services/note/delete.ts
View file

@ -14,14 +14,88 @@ import { notesChart, perUserNotesChart, instanceChart } from '@/services/chart/i
import { DeliverManager } from '@/remote/activitypub/deliver-manager.js';
import { countSameRenotes } from '@/misc/count-same-renotes.js';
import { registerOrFetchInstanceDoc } from '../register-or-fetch-instance-doc.js';
import { deliverToRelays } from '../relay.js';
import { deliverMultipleToRelays } from '../relay.js';
/**
* Delete your note.
* @param user author
* @param note note to be deleted
* Delete several notes of the same user.
* @param notes Array of notes to be deleted.
* @param user Author of the notes. Will be fetched if not provided.
*/
export async function deleteNote(user: { id: User['id']; uri: User['uri']; host: User['host']; }, note: Note, quiet = false): Promise<void> {
export async function deleteNotes(notes: Note[], user?: User): Promise<void> {
if (notes.length === 0) return;
const fetchedUser = user ?? await Users.findOneByOrFail({ id: notes[0].userId });
const cascadingNotes = await Promise.all(
notes.map(note => findCascadingNotes(note))
).then(res => res.flat());
// perform side effects for notes and cascaded notes
await Promise.all(
notes.concat(cascadingNotes)
.map(note => deletionSideEffects(note, fetchedUser))
);
// Compute delivery content for later.
// It is important that this is done before deleting notes from
// the database since we may need some information from parent
// notes that cause this one to be cascade-deleted.
let content = await Promise.all(
notes.concat(cascadingNotes)
// only deliver for local notes that are not local-only
.filter(note => note.userHost == null && !note.localOnly)
.map(async note => {
let renote: Note | null = null;
// if the deleted note is a renote
if (foundkey.entities.isPureRenote(note)) {
renote = await Notes.findOneBy({ id: note.renoteId });
}
return renderActivity(renote
? renderUndo(renderAnnounce(renote.uri || `${config.url}/notes/${renote.id}`, note), fetchedUser)
: renderDelete(renderTombstone(`${config.url}/notes/${note.id}`), fetchedUser));
})
);
// Compute addressing information.
// Since we do not send any actual content, we send all note deletions to everyone.
const manager = new DeliverManager(fetchedUser, content);
manager.addFollowersRecipe();
manager.addEveryone();
// Check mentioned users, since not all may have a shared inbox.
await Promise.all(
notes.concat(cascadingNotes)
.map(note => getMentionedRemoteUsers(note))
)
.then(remoteUsers => {
remoteUsers.flat()
.forEach(remoteUser => manager.addDirectRecipe(remoteUser))
});
// Actually delete notes from the database.
// It is important that this is done before delivering the activities.
// Otherwise there might be a race condition where we tell someone
// the note exists and they can successfully fetch it.
await Notes.delete({
id: In(notes.map(x => x.id)),
userId: fetchedUser.id,
});
// deliver the previously computed content
await Promise.all([
manager.execute(),
deliverMultipleToRelays(user, content),
]);
}
/**
* Perform side effects of deletion, such as updating statistics.
* Does not actually delete the note itself.
* @param note The soon to be deleted note.
* @param user The author of said note.
*/
async function deletionSideEffects(note: Note, user: User): Promise<void> {
const deletedAt = new Date();
// If this is the only renote of this note by this user
@ -34,48 +108,18 @@ export async function deleteNote(user: { id: User['id']; uri: User['uri']; host:
await Notes.decrement({ id: note.replyId }, 'repliesCount', 1);
}
if (!quiet) {
publishNoteStream(note.id, 'deleted', { deletedAt });
publishNoteStream(note.id, 'deleted', { deletedAt });
// deliver delete activity of note itself for local posts
if (Users.isLocalUser(user) && !note.localOnly) {
let renote: Note | null = null;
// update statistics
notesChart.update(note, false);
perUserNotesChart.update(user, note, false);
// if deleted note is renote
if (foundkey.entities.isPureRenote(note)) {
renote = await Notes.findOneBy({ id: note.renoteId });
}
const content = renderActivity(renote
? renderUndo(renderAnnounce(renote.uri || `${config.url}/notes/${renote.id}`, note), user)
: renderDelete(renderTombstone(`${config.url}/notes/${note.id}`), user));
deliverToConcerned(user, note, content);
}
// also deliver delete activity to cascaded notes
const cascadingNotes = await findCascadingNotes(note);
for (const cascadingNote of cascadingNotes) {
const content = renderActivity(renderDelete(renderTombstone(`${config.url}/notes/${cascadingNote.id}`), cascadingNote.user));
deliverToConcerned(cascadingNote.user, cascadingNote, content);
}
// update statistics
notesChart.update(note, false);
perUserNotesChart.update(user, note, false);
if (Users.isRemoteUser(user)) {
registerOrFetchInstanceDoc(user.host).then(i => {
Instances.decrement({ id: i.id }, 'notesCount', 1);
instanceChart.updateNote(i.host, note, false);
});
}
if (Users.isRemoteUser(user)) {
registerOrFetchInstanceDoc(user.host).then(i => {
Instances.decrement({ id: i.id }, 'notesCount', 1);
instanceChart.updateNote(i.host, note, false);
});
}
await Notes.delete({
id: note.id,
userId: user.id,
});
}
/**
@ -143,23 +187,3 @@ async function getMentionedRemoteUsers(note: Note): Promise<IRemoteUser[]> {
}) as IRemoteUser[];
}
async function deliverToConcerned(user: { id: ILocalUser['id']; host: null; }, note: Note, content: any): Promise<void> {
const manager = new DeliverManager(user, content);
const remoteUsers = await getMentionedRemoteUsers(note);
for (const remoteUser of remoteUsers) {
manager.addDirectRecipe(remoteUser);
}
if (['public', 'home', 'followers'].includes(note.visibility)) {
manager.addFollowersRecipe();
}
if (['public', 'home'].includes(note.visibility)) {
manager.addEveryone();
}
await manager.execute();
deliverToRelays(user, content);
}