Merge pull request #1795 from serlo/fix-deadlock

Fix deadlock 2: the bug knight returns
serlo · Nov 4, 2024 · bb2705c · bb2705c
2 parents a5c6e2b + 173d7cd
commit bb2705c
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 7 deletions.
diff --git a/__tests__/schema/user/add-role.ts b/__tests__/schema/user/add-role.ts
@@ -127,6 +127,26 @@ test('updates the cache', async () => {
   })
 })
 
+test('Is successful even when user has already role', async () => {
+  await uuidQuery.shouldReturnData({
+    uuid: {
+      roles: {
+        nodes: [{ role: Role.Login, scope: Scope.Serlo }],
+      },
+    },
+  })
+  await mutation
+    .withInput({ username: regularUser.username, role: Role.Login })
+    .execute()
+  await uuidQuery.shouldReturnData({
+    uuid: {
+      roles: {
+        nodes: [{ role: Role.Login, scope: Scope.Serlo }],
+      },
+    },
+  })
+})
+
 test('fails when user is not authenticated', async () => {
   await mutation.forUnauthenticatedUser().shouldFailWithError('UNAUTHENTICATED')
 })

diff --git a/packages/server/src/schema/uuid/user/resolvers.ts b/packages/server/src/schema/uuid/user/resolvers.ts
@@ -321,20 +321,29 @@ export const resolvers: Resolvers = {
         throw new UserInputError('no user with given username')
       }
 
+      const userHasAlreadyRole = await database.fetchOptional(
+        `
+        SELECT 1
+          FROM role_user
+          WHERE user_id = ? 
+          AND role_id = ( SELECT id
+            FROM role
+            WHERE name = ?)`,
+        [id, generateRole(role, instance)],
+      )
+
+      if (userHasAlreadyRole) {
+        return { success: true, query: {} }
+      }
+
       await database.mutate(
         `
         INSERT INTO role_user (user_id, role_id)
         SELECT ?, role.id
         FROM role
         WHERE role.name = ?
-        AND NOT EXISTS (
-          SELECT 1
-          FROM role_user
-          WHERE role_user.user_id = ?
-          AND role_user.role_id = role.id
-        )
         `,
-        [id, generateRole(role, instance), id],
+        [id, generateRole(role, instance)],
       )
 
       await UuidResolver.removeCacheEntry({ id }, context)