Skip to content

Commit 5e7f2f5

Browse files
committed
Make duplicate Announces idempotent
Treat repeated Announce activities from the same actor for the same post as an existing share, even when the duplicate uses a different activity IRI. The share insert now ignores the actor/object uniqueness conflict and returns the existing row instead of surfacing a database error. Track whether a persisted share is new so inbox handling can suppress federation forwarding and notification side effects for duplicate Announces. Fixes #444 Assisted-by: Codex:gpt-5.5
1 parent 3b1fc3e commit 5e7f2f5

4 files changed

Lines changed: 324 additions & 20 deletions

File tree

CHANGES.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@ Version 0.7.12
66

77
To be released.
88

9+
- Fixed a federation bug where duplicate Announce activities from the same
10+
actor for the same post could fail with a database uniqueness error instead
11+
of being treated idempotently. [[#443], [#444]]
12+
13+
[#443]: https://github.com/fedify-dev/hollo/issues/443
14+
[#444]: https://github.com/fedify-dev/hollo/issues/444
15+
916

1017
Version 0.7.11
1118
--------------

src/federation/inbox.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,7 @@ export async function onPostShared(
586586
const object = await announce.getObject();
587587
if (!isPost(object)) return;
588588
const post = await persistSharingPost(db, announce, object, ctx.origin, ctx);
589+
if (post == null || !post.isNew) return;
589590
if (post?.sharingId != null) {
590591
await updatePostStats(db, { id: post.sharingId });
591592
}

src/federation/post.test.ts

Lines changed: 268 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,268 @@
1+
import {
2+
Announce,
3+
type InboxContext,
4+
Note,
5+
Person,
6+
PUBLIC_COLLECTION,
7+
} from "@fedify/fedify";
8+
import { and, eq } from "drizzle-orm";
9+
import { beforeEach, describe, expect, it, vi } from "vitest";
10+
import { cleanDatabase } from "../../tests/helpers";
11+
import { createAccount } from "../../tests/helpers/oauth";
12+
import db from "../db";
13+
import { accounts, follows, instances, posts, timelinePosts } from "../schema";
14+
import type { Uuid } from "../uuid";
15+
import { onPostShared } from "./inbox";
16+
import { persistSharingPost } from "./post";
17+
18+
async function seedRemoteAccount(username: string) {
19+
const id = crypto.randomUUID() as Uuid;
20+
const iri = `https://remote.test/@${username}`;
21+
await db
22+
.insert(instances)
23+
.values({
24+
host: "remote.test",
25+
software: "mastodon",
26+
softwareVersion: null,
27+
})
28+
.onConflictDoNothing();
29+
await db.insert(accounts).values({
30+
id,
31+
iri,
32+
type: "Person",
33+
name: username,
34+
handle: `@${username}@remote.test`,
35+
bioHtml: "",
36+
emojis: {},
37+
fieldHtmls: {},
38+
aliases: [],
39+
protected: false,
40+
inboxUrl: `${iri}/inbox`,
41+
followersUrl: `${iri}/followers`,
42+
sharedInboxUrl: "https://remote.test/inbox",
43+
featuredUrl: `${iri}/featured`,
44+
instanceHost: "remote.test",
45+
published: new Date(),
46+
});
47+
const account = await db.query.accounts.findFirst({
48+
where: eq(accounts.id, id),
49+
with: { owner: true },
50+
});
51+
if (account == null) throw new Error("Failed to seed remote account");
52+
return account;
53+
}
54+
55+
function createPerson(account: {
56+
handle: string;
57+
iri: string;
58+
followersUrl: string | null;
59+
}) {
60+
return new Person({
61+
id: new URL(account.iri),
62+
name: account.handle,
63+
inbox: new URL(`${account.iri}/inbox`),
64+
followers:
65+
account.followersUrl == null ? null : new URL(account.followersUrl),
66+
});
67+
}
68+
69+
function createAnnounce(id: string, actor: Person, object: string | Note) {
70+
return new Announce({
71+
id: new URL(id),
72+
actor,
73+
object: typeof object === "string" ? new URL(object) : object,
74+
to: PUBLIC_COLLECTION,
75+
});
76+
}
77+
78+
function createCtx() {
79+
const forwardActivity = vi.fn(async () => undefined);
80+
const ctx = {
81+
origin: "https://hollo.test",
82+
parseUri: () => null,
83+
forwardActivity,
84+
} as unknown as InboxContext<void>;
85+
return { ctx, forwardActivity };
86+
}
87+
88+
async function seedShareScenario() {
89+
const owner = await createAccount({ username: "hollo" });
90+
const author = await seedRemoteAccount("author");
91+
const sharer = await seedRemoteAccount("sharer");
92+
await db.insert(follows).values({
93+
iri: `https://hollo.test/@hollo#follows/${sharer.id}`,
94+
followingId: sharer.id,
95+
followerId: owner.id as Uuid,
96+
approved: new Date(),
97+
shares: true,
98+
notify: false,
99+
});
100+
const originalPostId = crypto.randomUUID() as Uuid;
101+
const originalPostIri = "https://remote.test/@author/posts/1";
102+
await db.insert(posts).values({
103+
id: originalPostId,
104+
iri: originalPostIri,
105+
type: "Note",
106+
accountId: author.id,
107+
visibility: "public",
108+
contentHtml: "<p>Shared once</p>",
109+
content: "Shared once",
110+
tags: {},
111+
emojis: {},
112+
sensitive: false,
113+
published: new Date(),
114+
updated: new Date(),
115+
});
116+
return {
117+
actor: createPerson(sharer),
118+
object: new Note({ id: new URL(originalPostIri) }),
119+
originalPostId,
120+
originalPostIri,
121+
sharer,
122+
};
123+
}
124+
125+
async function seedLocalPostShareScenario() {
126+
const author = await createAccount({ username: "hollo" });
127+
const sharer = await seedRemoteAccount("sharer");
128+
const originalPostId = crypto.randomUUID() as Uuid;
129+
const originalPostIri = `https://hollo.test/@hollo/${originalPostId}`;
130+
await db.insert(posts).values({
131+
id: originalPostId,
132+
iri: originalPostIri,
133+
type: "Note",
134+
accountId: author.id as Uuid,
135+
visibility: "public",
136+
contentHtml: "<p>Local post</p>",
137+
content: "Local post",
138+
tags: {},
139+
emojis: {},
140+
sensitive: false,
141+
published: new Date(),
142+
updated: new Date(),
143+
});
144+
return {
145+
actor: createPerson(sharer),
146+
object: new Note({ id: new URL(originalPostIri) }),
147+
originalPostIri,
148+
};
149+
}
150+
151+
describe("persistSharingPost", () => {
152+
beforeEach(async () => {
153+
await cleanDatabase();
154+
});
155+
156+
it("returns an existing share when the same actor announces the same post with another IRI", async () => {
157+
expect.assertions(5);
158+
const { actor, object, originalPostId, originalPostIri, sharer } =
159+
await seedShareScenario();
160+
161+
const first = await persistSharingPost(
162+
db,
163+
createAnnounce(
164+
"https://remote.test/@sharer/announces/1",
165+
actor,
166+
originalPostIri,
167+
),
168+
object,
169+
"https://hollo.test",
170+
{ account: sharer },
171+
);
172+
const second = await persistSharingPost(
173+
db,
174+
createAnnounce(
175+
"https://remote.test/@sharer/announces/2",
176+
actor,
177+
originalPostIri,
178+
),
179+
object,
180+
"https://hollo.test",
181+
{ account: sharer },
182+
);
183+
184+
const sharingPosts = await db.query.posts.findMany({
185+
where: and(
186+
eq(posts.accountId, sharer.id),
187+
eq(posts.sharingId, originalPostId),
188+
),
189+
});
190+
const timelineRows = await db.query.timelinePosts.findMany({
191+
where: eq(timelinePosts.postId, first!.id),
192+
});
193+
const originalPost = await db.query.posts.findFirst({
194+
where: eq(posts.id, originalPostId),
195+
});
196+
expect(first).not.toBeNull();
197+
expect(second?.id).toBe(first?.id);
198+
expect(sharingPosts).toHaveLength(1);
199+
expect(timelineRows).toHaveLength(1);
200+
expect(originalPost?.sharesCount).toBe(1);
201+
});
202+
203+
it("handles concurrent duplicate announces atomically", async () => {
204+
expect.assertions(5);
205+
const { actor, object, originalPostId, originalPostIri, sharer } =
206+
await seedShareScenario();
207+
208+
const [first, second] = await Promise.all([
209+
persistSharingPost(
210+
db,
211+
createAnnounce(
212+
"https://remote.test/@sharer/announces/1",
213+
actor,
214+
originalPostIri,
215+
),
216+
object,
217+
"https://hollo.test",
218+
{ account: sharer },
219+
),
220+
persistSharingPost(
221+
db,
222+
createAnnounce(
223+
"https://remote.test/@sharer/announces/2",
224+
actor,
225+
originalPostIri,
226+
),
227+
object,
228+
"https://hollo.test",
229+
{ account: sharer },
230+
),
231+
]);
232+
233+
const sharingPosts = await db.query.posts.findMany({
234+
where: and(
235+
eq(posts.accountId, sharer.id),
236+
eq(posts.sharingId, originalPostId),
237+
),
238+
});
239+
const timelineRows = await db.query.timelinePosts.findMany({
240+
where: eq(timelinePosts.postId, first!.id),
241+
});
242+
const originalPost = await db.query.posts.findFirst({
243+
where: eq(posts.id, originalPostId),
244+
});
245+
expect(first).not.toBeNull();
246+
expect(second?.id).toBe(first?.id);
247+
expect(sharingPosts).toHaveLength(1);
248+
expect(timelineRows).toHaveLength(1);
249+
expect(originalPost?.sharesCount).toBe(1);
250+
});
251+
252+
it("does not forward duplicate announces for a local post", async () => {
253+
expect.assertions(1);
254+
const { actor, object } = await seedLocalPostShareScenario();
255+
const { ctx, forwardActivity } = createCtx();
256+
257+
await onPostShared(
258+
ctx,
259+
createAnnounce("https://remote.test/@sharer/announces/1", actor, object),
260+
);
261+
await onPostShared(
262+
ctx,
263+
createAnnounce("https://remote.test/@sharer/announces/2", actor, object),
264+
);
265+
266+
expect(forwardActivity).toHaveBeenCalledOnce();
267+
});
268+
});

0 commit comments

Comments
 (0)