Fix race conditions in poll voting

- Fix race condition in vote counting by using repository.countVotes() instead
  of manual increment
- Add CAS  support in KvRepository for concurrent vote handling
- Add comprehensive tests for vote(), countVoters(), and countVotes() methods
- Add polls prefix to KvStoreRepositoryPrefixes
- Upgrade Fedify to 1.8.0-dev.910

Co-Authored-By: Claude <noreply@anthropic.com>

Author: dahlia

CHANGES.md

@@ -18,6 +18,7 @@ To be released.
      -  Added `SessionPublishOptionsWithQuestion` interface.
      -  Added `Bot.onVote` event.
      -  Added `VoteEventHandler` type.
+     -  Added `KvStoreRepositoryPrefixes.polls` option.
 
  -  Added `@fedify/botkit/repository` module that provides repository
     implementations for BotKit.
@@ -32,7 +33,7 @@ To be released.
      -  Added `Create` class.
      -  Added `MemoryCachedRepository` class.
 
- -  Upgraded Fedify to 1.7.1.
+ -  Upgraded Fedify to 1.8.0.
 
 [#7]: https://github.com/fedify-dev/botkit/issues/7
 [#8]: https://github.com/fedify-dev/botkit/pull/8

deno.json

@@ -20,7 +20,7 @@
     "./text": "./src/text.ts"
   },
   "imports": {
-    "@fedify/fedify": "jsr:@fedify/fedify@^1.7.1",
+    "@fedify/fedify": "jsr:@fedify/fedify@^1.8.0-dev.910+8a000b1c",
     "@fedify/markdown-it-hashtag": "jsr:@fedify/markdown-it-hashtag@^0.3.0",
     "@fedify/markdown-it-mention": "jsr:@fedify/markdown-it-mention@^0.3.0",
     "@logtape/logtape": "jsr:@logtape/logtape@^1.0.0",

deno.lock

@@ -3,4 +3,4 @@ packages:
 - docs
 
 catalog:
-  "@fedify/fedify": ^1.7.1
+  "@fedify/fedify": ^1.8.0-dev.910

pnpm-lock.yaml

@@ -673,12 +673,16 @@ export class BotImpl<TContextData> implements Bot<TContextData> {
       messageClasses.includes(replyTarget.class) &&
       object.name != null
     ) {
-      if (create.actorId?.href === session.actorId.href) return;
+      if (
+        create.actorId == null || create.actorId.href === session.actorId.href
+      ) {
+        return;
+      }
+      const actorId = create.actorId;
       const actor = await create.getActor(ctx);
       if (actor == null) return;
-      const pollMessage = await this.repository.getMessage(
-        replyTarget.values.id as Uuid,
-      );
+      const messageId = replyTarget.values.id as Uuid;
+      const pollMessage = await this.repository.getMessage(messageId);
       if (!(pollMessage instanceof Create)) return;
       const question = await pollMessage.getObject(ctx);
       if (
@@ -702,34 +706,37 @@ export class BotImpl<TContextData> implements Bot<TContextData> {
       }
       const option = object.name.toString();
       if (!options.includes(option)) return;
-      const updatedOptionNotes: Note[] = [...optionNotes];
-      let i = 0;
-      for (const note of updatedOptionNotes) {
-        if (note.name === option) {
-          const replies = await note.getReplies(ctx);
-          if (replies != null && replies.totalItems != null) {
-            updatedOptionNotes[i] = note.clone({
-              replies: replies.clone({
-                // FIXME: This way of updating vote count is not only inefficient,
-                // but also can lead to incorrect counts if multiple votes are
-                // cast at the same time.
-                totalItems: replies.totalItems + 1,
-              }),
-            });
-          }
-        }
-        i++;
-      }
-      const updatedQuestion = question.clone({
-        inclusiveOptions: multiple ? updatedOptionNotes : [],
-        exclusiveOptions: !multiple ? updatedOptionNotes : [],
-      });
-      const updatedPollMessage = pollMessage.clone({
-        object: updatedQuestion,
-      });
+      let updatedQuestion: Question = question;
+      let updatedPollMessage = pollMessage;
+      await this.repository.vote(messageId, actorId, option);
       await this.repository.updateMessage(
         replyTarget.values.id as Uuid,
-        () => updatedPollMessage,
+        async () => {
+          const votes = await this.repository.countVotes(messageId);
+          const updatedOptionNotes: Note[] = [...optionNotes];
+          let i = 0;
+          for (const note of updatedOptionNotes) {
+            if (note.name != null) {
+              const replies = await note.getReplies(ctx);
+              if (replies != null && replies.totalItems != null) {
+                updatedOptionNotes[i] = note.clone({
+                  replies: replies.clone({
+                    totalItems: votes[note.name.toString()],
+                  }),
+                });
+              }
+            }
+            i++;
+          }
+          updatedQuestion = question.clone({
+            inclusiveOptions: multiple ? updatedOptionNotes : [],
+            exclusiveOptions: !multiple ? updatedOptionNotes : [],
+            voters: await this.repository.countVoters(messageId),
+          });
+          return updatedPollMessage = pollMessage.clone({
+            object: updatedQuestion,
+          });
+        },
       );
       const message = await createMessage(updatedQuestion, session, {});
       const vote: Vote<TContextData> = {

pnpm-workspace.yaml

@@ -689,5 +689,85 @@ for (const name in factories) {
       await repo.removeFollowee(followeeId);
       assert.deepStrictEqual(await repo.getFollowee(followeeId), undefined);
     });
+
+    test("poll voting", async () => {
+      const messageId1 = "01945678-1234-7890-abcd-ef0123456789";
+      const messageId2 = "01945678-5678-7890-abcd-ef0123456789";
+      const voter1 = new URL("https://example.com/ap/actor/alice");
+      const voter2 = new URL("https://example.com/ap/actor/bob");
+      const voter3 = new URL("https://example.com/ap/actor/charlie");
+
+      // Initially, no votes exist
+      assert.deepStrictEqual(await repo.countVoters(messageId1), 0);
+      assert.deepStrictEqual(await repo.countVotes(messageId1), {});
+      assert.deepStrictEqual(await repo.countVoters(messageId2), 0);
+      assert.deepStrictEqual(await repo.countVotes(messageId2), {});
+
+      // Single voter, single option
+      await repo.vote(messageId1, voter1, "option1");
+      assert.deepStrictEqual(await repo.countVoters(messageId1), 1);
+      assert.deepStrictEqual(await repo.countVotes(messageId1), {
+        "option1": 1,
+      });
+
+      // Same voter votes for same option again (should be ignored)
+      await repo.vote(messageId1, voter1, "option1");
+      assert.deepStrictEqual(await repo.countVoters(messageId1), 1);
+      assert.deepStrictEqual(await repo.countVotes(messageId1), {
+        "option1": 1,
+      });
+
+      // Same voter votes for different option (multiple choice)
+      await repo.vote(messageId1, voter1, "option2");
+      assert.deepStrictEqual(await repo.countVoters(messageId1), 1);
+      assert.deepStrictEqual(await repo.countVotes(messageId1), {
+        "option1": 1,
+        "option2": 1,
+      });
+
+      // Different voter votes for same option
+      await repo.vote(messageId1, voter2, "option1");
+      assert.deepStrictEqual(await repo.countVoters(messageId1), 2);
+      assert.deepStrictEqual(await repo.countVotes(messageId1), {
+        "option1": 2,
+        "option2": 1,
+      });
+
+      // Third voter votes for new option
+      await repo.vote(messageId1, voter3, "option3");
+      assert.deepStrictEqual(await repo.countVoters(messageId1), 3);
+      assert.deepStrictEqual(await repo.countVotes(messageId1), {
+        "option1": 2,
+        "option2": 1,
+        "option3": 1,
+      });
+
+      // Votes for different message should be separate
+      await repo.vote(messageId2, voter1, "optionA");
+      await repo.vote(messageId2, voter2, "optionB");
+      assert.deepStrictEqual(await repo.countVoters(messageId2), 2);
+      assert.deepStrictEqual(await repo.countVotes(messageId2), {
+        "optionA": 1,
+        "optionB": 1,
+      });
+
+      // Original message votes should remain unchanged
+      assert.deepStrictEqual(await repo.countVoters(messageId1), 3);
+      assert.deepStrictEqual(await repo.countVotes(messageId1), {
+        "option1": 2,
+        "option2": 1,
+        "option3": 1,
+      });
+
+      // Test with empty options (edge case)
+      await repo.vote(messageId1, voter1, "");
+      assert.deepStrictEqual(await repo.countVoters(messageId1), 3);
+      assert.deepStrictEqual(await repo.countVotes(messageId1), {
+        "option1": 2,
+        "option2": 1,
+        "option3": 1,
+        "": 1,
+      });
+    });
   });
 }