fix(database): run cleanOrderColumns updates sequentially

Fix concurrent UPDATEs on the same join table that can deadlock Postgres
when renumbering order and inverse order columns.

Refs: https://github.com/strapi/strapi/issues/26131
This commit is contained in:
Ben Irvin
2026-04-27 15:15:01 +02:00
parent f78edc3bc9
commit 191f366923
2 changed files with 150 additions and 1 deletions
@@ -376,7 +376,9 @@ const cleanOrderColumns = async ({
}
};
return Promise.all([updateOrderColumn(), updateInverseOrderColumn()]);
// Run updates in a deterministic order to avoid lock cycles on the same join table.
await updateOrderColumn();
await updateInverseOrderColumn();
};
export {
+147
View File
@@ -29,6 +29,7 @@ const omitRegistrationToken = omit(['registrationToken']);
* 14. Updates a user (not found)
* 15. Finds a user (not found)
* 16. Finds a list of users (missing user)
* — Concurrent role updates (join order gaps, regression #26131)
*/
describe('Admin User CRUD (api)', () => {
@@ -396,4 +397,150 @@ describe('Admin User CRUD (api)', () => {
});
expect(res.body.data.results).toHaveLength(0);
});
describe('Concurrent admin user role updates (regression #26131)', () => {
const concurrencyData = {
users: [],
roles: [],
sharedRole: undefined,
keepRole: undefined,
};
const createAdminUser = async ({ email, roles }) => {
const res = await rq({
url: '/admin/users',
method: 'POST',
body: {
email,
firstname: 'concurrency',
lastname: 'test',
roles,
},
});
expect(res.statusCode).toBe(201);
return res.body.data;
};
const updateAdminUserRoles = async (id, roles) => {
return rq({
url: `/admin/users/${id}`,
method: 'PUT',
body: { roles },
});
};
const getUserRolesJoinTable = () => {
return strapi.db.metadata.get('admin::user').attributes.roles.joinTable;
};
const forceGapInSharedRoleOrder = async () => {
const joinTable = getUserRolesJoinTable();
const { name, inverseJoinColumn, inverseOrderColumnName } = joinTable;
const rows = await strapi.db
.getConnection()
.from(name)
.select(['id', inverseOrderColumnName])
.where(inverseJoinColumn.name, concurrencyData.sharedRole.id)
.orderBy(inverseOrderColumnName, 'asc');
expect(rows).toHaveLength(2);
const firstOrder = Number(rows[0][inverseOrderColumnName]);
const secondOrder = Number(rows[1][inverseOrderColumnName]);
if (secondOrder !== firstOrder + 2) {
await strapi.db
.getConnection()
.from(name)
.where('id', rows[1].id)
.update({ [inverseOrderColumnName]: firstOrder + 2 });
}
const updatedRows = await strapi.db
.getConnection()
.from(name)
.select([inverseOrderColumnName])
.where(inverseJoinColumn.name, concurrencyData.sharedRole.id)
.orderBy(inverseOrderColumnName, 'asc');
const updatedOrders = updatedRows.map((row) => Number(row[inverseOrderColumnName]));
expect(updatedOrders).toEqual([firstOrder, firstOrder + 2]);
};
beforeAll(async () => {
const timestamp = Date.now();
concurrencyData.sharedRole = await utils.createRole({
name: `concurrency-shared-role-${timestamp}`,
description: 'Role shared by users in concurrent update test',
});
concurrencyData.keepRole = await utils.createRole({
name: `concurrency-keep-role-${timestamp}`,
description: 'Role that keeps users assigned while shared role is removed',
});
concurrencyData.roles.push(concurrencyData.sharedRole, concurrencyData.keepRole);
const userA = await createAdminUser({
email: `concurrency-user-a-${timestamp}@strapi.io`,
roles: [concurrencyData.sharedRole.id, concurrencyData.keepRole.id],
});
const userB = await createAdminUser({
email: `concurrency-user-b-${timestamp}@strapi.io`,
roles: [concurrencyData.sharedRole.id, concurrencyData.keepRole.id],
});
concurrencyData.users.push(userA, userB);
});
afterAll(async () => {
if (concurrencyData.users.length > 0) {
await utils.deleteUsersById(concurrencyData.users.map((user) => user.id));
}
if (concurrencyData.roles.length > 0) {
await utils.deleteRolesById(concurrencyData.roles.map((role) => role.id));
}
});
test('concurrent role removals succeed when join order has gaps', async () => {
const [userA, userB] = concurrencyData.users;
const removeSharedRes = await updateAdminUserRoles(userB.id, [concurrencyData.keepRole.id]);
expect(removeSharedRes.statusCode).toBe(200);
const addSharedBackRes = await updateAdminUserRoles(userB.id, [
concurrencyData.sharedRole.id,
concurrencyData.keepRole.id,
]);
expect(addSharedBackRes.statusCode).toBe(200);
await forceGapInSharedRoleOrder();
for (let attempt = 0; attempt < 8; attempt += 1) {
const [removeARes, removeBRes] = await Promise.all([
updateAdminUserRoles(userA.id, [concurrencyData.keepRole.id]),
updateAdminUserRoles(userB.id, [concurrencyData.keepRole.id]),
]);
expect(removeARes.statusCode).toBe(200);
expect(removeBRes.statusCode).toBe(200);
const [addARes, addBRes] = await Promise.all([
updateAdminUserRoles(userA.id, [
concurrencyData.sharedRole.id,
concurrencyData.keepRole.id,
]),
updateAdminUserRoles(userB.id, [
concurrencyData.sharedRole.id,
concurrencyData.keepRole.id,
]),
]);
expect(addARes.statusCode).toBe(200);
expect(addBRes.statusCode).toBe(200);
await forceGapInSharedRoleOrder();
}
});
});
});