Skip to content

Commit

Permalink
Optimizar proceso de creación/actualización de usuarios y resolver ra…
Browse files Browse the repository at this point in the history
…ce condition (#271)

Este PR aborda un problema crítico de race condition en la creación de
usuarios y mejora significativamente el performance general de nuestro
sistema de gestión de usuarios.

### Problema actual:

Actualmente, cuando se realizan múltiples llamadas concurrentes a
endpoints autenticados (por ejemplo, usando `await Promise.all([...])`)
para un usuario que se está autenticando por primera vez, puede ocurrir
una race condition. Esto resulta en un error de clave duplicada:

```
(error) 🚨 APPLICATION ERROR of: duplicate key value violates unique constraint "users_email_unique" Internal Server Error
```

Este error se produce porque múltiples procesos intentan crear el mismo
usuario simultáneamente, violando la restricción de unicidad en el
email.

### Cambios propuestos:

1. Reemplazar `updateUserProfileInfo` con `upsertUserProfileInfo`:
- Esta nueva función maneja tanto la creación como las actualizaciones
de usuarios en una sola operación atómica.
- Elimina la necesidad de verificar la existencia del usuario antes de
decidir si crear o actualizar.

2. Implementar una operación upsert utilizando `onConflictDoUpdate`:
- Esto previene las race conditions durante intentos concurrentes de
creación de usuarios.
- Si un usuario ya existe, la operación se convierte automáticamente en
una actualización, evitando así el error de clave duplicada.

### Beneficios:

- **Eliminación de errores:** Resuelve la race condition que causaba
errores de usuarios duplicados.
- **Mejora de performance:** Reduce el número de operaciones de base de
datos necesarias. Además esta es una operación critica dado que esta
operación se realiza en cada request.
  • Loading branch information
TextC0de authored Sep 23, 2024
1 parent e67f148 commit 4ea4e4b
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 45 deletions.
4 changes: 2 additions & 2 deletions src/authn/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { ORM_TYPE } from "~/datasources/db";
import { insertUsersSchema, USER } from "~/datasources/db/schema";
import {
findUserByID,
updateUserProfileInfo,
upsertUserProfileInfo,
} from "~/datasources/queries/users";
import { getUsername } from "~/datasources/queries/utils/createUsername";
import { unauthorizedError } from "~/errors";
Expand Down Expand Up @@ -141,7 +141,7 @@ export const upsertUserFromRequest = async ({

logger.info(`Updating profile Info for user ID: ${sub}`);

return updateUserProfileInfo(DB, profileInfo.data, logger);
return upsertUserProfileInfo(DB, profileInfo.data, logger);
};

export const logPossibleUserIdFromJWT = (request: Request, logger: Logger) => {
Expand Down
76 changes: 33 additions & 43 deletions src/datasources/queries/users.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { eq } from "drizzle-orm";
import { z } from "zod";

import { ORM_TYPE } from "~/datasources/db";
Expand All @@ -20,64 +19,55 @@ export const findUserByID = async (db: ORM_TYPE, id: string) => {
return result ? selectUsersSchema.parse(result) : null;
};

export const updateUserProfileInfo = async (
export const upsertUserProfileInfo = async (
db: ORM_TYPE,
parsedProfileInfo: z.infer<typeof insertUsersSchema>,
logger: Logger,
) => {
const result = await db.query.usersSchema.findFirst({
where: (u, { eq }) => eq(u.email, parsedProfileInfo.email.toLowerCase()),
});
logger.info(
`Upserting user profile info for ${parsedProfileInfo.email} (externalId: ${
parsedProfileInfo.externalId || "N/A"
})`,
);

const { email, externalId, name, imageUrl, isEmailVerified, publicMetadata } =
parsedProfileInfo;
const lowercaseEmail = email.trim().toLowerCase();

const upsertData: z.infer<typeof allowedUserUpdateForAuth> = {
externalId,
name,
imageUrl,
isEmailVerified,
publicMetadata: publicMetadata ?? {},
status: UserStatusEnum.active,
};

if (!result) {
logger.info("User not found — creating new user");
// we create the user
const createdUsers = await db
try {
const result = await db
.insert(usersSchema)
.values({
externalId: parsedProfileInfo.externalId,
email: parsedProfileInfo.email.trim().toLowerCase(),
...upsertData,
email: lowercaseEmail,
username: parsedProfileInfo.username ?? getUsername(),
name: parsedProfileInfo.name,
imageUrl: parsedProfileInfo.imageUrl,
isEmailVerified: parsedProfileInfo.isEmailVerified,
publicMetadata: parsedProfileInfo.publicMetadata ?? {},
status: UserStatusEnum.active,
})
.onConflictDoUpdate({
target: usersSchema.email,
set: allowedUserUpdateForAuth.parse(upsertData),
})
.returning();
const createdUser = createdUsers?.[0];

if (!createdUser) {
logger.error("Could not create user");
throw new Error("Could not create user");
}

return selectUsersSchema.parse(createdUser);
} else {
logger.info("User found — updating user");
// we update the user
const updateData = allowedUserUpdateForAuth.parse({
externalId: parsedProfileInfo.externalId,
name: parsedProfileInfo.name,
imageUrl: parsedProfileInfo.imageUrl,
isEmailVerified: parsedProfileInfo.isEmailVerified,
publicMetadata: parsedProfileInfo.publicMetadata ?? {},
status: UserStatusEnum.active,
});
const updatedUsers = await db
.update(usersSchema)
.set(updateData)
.where(eq(usersSchema.email, parsedProfileInfo.email))
.returning();
const updatedUser = updatedUsers?.[0];
const updatedUser = result[0];

if (!updatedUser) {
logger.error("Could not update user");
throw new Error("Could not update user");
throw new Error("User operation failed");
}

logger.info("User updated");
logger.info(result.length > 1 ? "User updated" : "New user created");

return selectUsersSchema.parse(updatedUser);
} catch (error) {
logger.error("Error in user operation", { error });
throw new Error("User operation failed");
}
};

0 comments on commit 4ea4e4b

Please sign in to comment.