Handle duplicate group checks

src/lib/groups/checkGroupMemberships.ts

@@ -1,4 +1,4 @@
-import { EntityManager } from '@mikro-orm/mysql'
+import { EntityManager, UniqueConstraintViolationException } from '@mikro-orm/mysql'
 import Player from '../../entities/player'
 import PlayerGroup from '../../entities/player-group'
 import { getResultCacheOptions } from '../perf/getResultCacheOptions'
@@ -21,7 +21,7 @@ async function runMembershipChecksForGroups(em: EntityManager, player: Player, g
 
   for (const group of groups) {
     const eligible = await group.isPlayerEligible(em, player)
-    const isInGroup = player.groups.contains(group)
+    const isInGroup = player.groups.getIdentifiers().includes(group.id)
 
     const eligibleButNotInGroup = eligible && !isInGroup
     const inGroupButNotEligible = !eligible && isInGroup
@@ -60,21 +60,25 @@ export default async function checkGroupMemberships(em: EntityManager, player: P
 
   const redisKey = `checkMembership:${player.id}`
   let lockCreated: 'OK' | null = null
-  let shouldFlush = false
 
   try {
     lockCreated = await redis.set(redisKey, '1', 'EX', 30, 'NX')
     if (lockCreated) {
-      shouldFlush = await runMembershipChecksForGroups(em, player, groups)
+      const shouldFlush = await runMembershipChecksForGroups(em, player, groups)
+      if (shouldFlush) {
+        await em.flush()
+      }
     }
   } catch (err) {
+    if (err instanceof UniqueConstraintViolationException) {
+      console.info(`Duplicate group attempt for player ${player.id}`)
+      return
+    }
+
     console.error(`Failed checking memberships: ${(err as Error).message}`)
     captureException(err)
   } finally {
     if (lockCreated) {
-      if (shouldFlush) {
-        await em.flush()
-      }
       await redis.del(redisKey)
     }
   }

tests/services/_api/player-api/patch.test.ts

@@ -9,6 +9,7 @@ import PlayerGroupRule, { PlayerGroupRuleCastType, PlayerGroupRuleName } from '.
 import createAPIKeyAndToken from '../../../utils/createAPIKeyAndToken'
 import { randWord } from '@ngneat/falso'
 import PlayerGroup from '../../../../src/entities/player-group'
+import Redis from 'ioredis'
 
 describe('Player API service - patch', () => {
   it('should update a player\'s properties', async () => {
@@ -347,4 +348,46 @@ describe('Player API service - patch', () => {
     isPlayerEligibleSpy.mockRestore()
     consoleSpy.mockRestore()
   })
+
+  // this is more likely to happen with event/stat flushing, but easier to test it here
+  it('should handle unique constraint failures for groups', async () => {
+    const redisSetSpy = vi.spyOn(Redis.prototype, 'set').mockResolvedValue('OK')
+    const consoleSpy = vi.spyOn(console, 'info')
+
+    const [apiKey, token] = await createAPIKeyAndToken([APIKeyScope.WRITE_PLAYERS])
+
+    const rule = new PlayerGroupRule(PlayerGroupRuleName.GTE, 'props.currentLevel')
+    rule.castType = PlayerGroupRuleCastType.DOUBLE
+    rule.operands = ['60']
+
+    const group = await new PlayerGroupFactory().construct(apiKey.game).state(() => ({ rules: [rule] })).one()
+
+    const player = await new PlayerFactory([apiKey.game]).state((player) => ({
+      props: new Collection<PlayerProp>(player, [
+        new PlayerProp(player, 'collectibles', '0'),
+        new PlayerProp(player, 'currentLevel', '59')
+      ])
+    })).one()
+    await em.persistAndFlush([group, player])
+
+    await Promise.allSettled(['60', '61', '62', '63', '64', '65'].map((level) => {
+      return request(app)
+        .patch(`/v1/players/${player.id}`)
+        .send({
+          props: [
+            {
+              key: 'currentLevel',
+              value: level
+            }
+          ]
+        })
+        .auth(token, { type: 'bearer' })
+        .expect(200)
+    }))
+
+    expect(consoleSpy).toHaveBeenCalledWith(`Duplicate group attempt for player ${player.id}`)
+
+    redisSetSpy.mockRestore()
+    consoleSpy.mockRestore()
+  })
 })