Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 58 additions & 18 deletions packages/api-core/src/util/buildContext.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,60 @@
import Logger from "@reactioncommerce/logger";
import ReactionError from "@reactioncommerce/reaction-error";

// Simple in-memory lock map to prevent concurrent account creation for the same user
// within a single Node.js process. This is intentionally minimal and can be replaced
// by a more advanced, distributed solution by swapping out the logic that calls
// `createAccount` (for example, via a custom context or plugin).
const accountCreationPromisesByUserId = new Map();

/**
* @name ensureAccountForUser
* @summary Ensure that an account exists for the given user ID, creating it if needed.
* Concurrent calls for the same user ID are coalesced so that only a single
* `createAccount` invocation happens per user at a time.
* @param {Object} context - GraphQL request context
* @param {String} userId - User ID for which to ensure an account exists
* @returns {Object|null} Account document or null if it could not be created/found
*/
async function ensureAccountForUser(context, userId) {
if (!userId || typeof context.auth?.accountByUserId !== "function") return null;

// Fast path: if an account already exists, return it immediately
const existingAccount = await context.auth.accountByUserId(context, userId);
if (existingAccount) return existingAccount;

// If we already have an in-flight creation for this user, await it instead of
// starting another one.
if (accountCreationPromisesByUserId.has(userId)) {
return accountCreationPromisesByUserId.get(userId);
}

const creationPromise = (async () => {
let account;
try {
Logger.debug(`Creating missing account for user ID ${userId}`);
account = await context.mutations.createAccount(context.getInternalContext(), {
emails: context.user.emails && context.user.emails.map((rec) => ({ ...rec, provides: rec.provides || "default" })),
name: context.user.name,
profile: context.user.profile || {},
userId
});
} catch (error) {
// We might have had a unique index error if account already exists due to timing
account = await context.auth.accountByUserId(context, userId);
if (!account) Logger.error(error, "Creating missing account failed");
} finally {
// Ensure we do not hold onto stale promises indefinitely
accountCreationPromisesByUserId.delete(userId);
}

return account || null;
})();

accountCreationPromisesByUserId.set(userId, creationPromise);
return creationPromise;
}

/**
* @name buildContext
* @method
Expand Down Expand Up @@ -60,24 +114,10 @@ export default async function buildContext(context, request = {}) {
let account;
let permissions;
if (userId && typeof context.auth.accountByUserId === "function") {
account = await context.auth.accountByUserId(context, userId);

// Create an account the first time a user makes a request
if (!account) {
try {
Logger.debug(`Creating missing account for user ID ${userId}`);
account = await context.mutations.createAccount(context.getInternalContext(), {
emails: context.user.emails && context.user.emails.map((rec) => ({ ...rec, provides: rec.provides || "default" })),
name: context.user.name,
profile: context.user.profile || {},
userId
});
} catch (error) {
// We might have had a unique index error if account already exists due to timing
account = await context.auth.accountByUserId(context, userId);
if (!account) Logger.error(error, "Creating missing account failed");
}
}
// Create an account the first time a user makes a request. Concurrent calls
// for the same user will wait for the same in-flight creation rather than
// attempting multiple inserts.
account = await ensureAccountForUser(context, userId);
if (typeof context.auth.permissionsByUserId === "function") {
permissions = await context.auth.permissionsByUserId(context, userId);
}
Expand Down
58 changes: 58 additions & 0 deletions packages/api-core/src/util/buildContext.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,61 @@ test("properly mutates the context object with user", async () => {
userId: fakeUser._id
});
});

test("coalesces concurrent account creation for the same user into a single createAccount call", async () => {
const userId = "CONCURRENT_USER_ID";
const concurrentUser = { _id: userId, emails: [{ address: "test@example.com" }], name: "Concurrent User", profile: {} };

let createdAccount = null;

const concurrentAccountByUserId = jest.fn().mockName("concurrentAccountByUserId").mockImplementation(async () => createdAccount);

const createAccount = jest.fn().mockName("createAccount").mockImplementation(async () => {
// Simulate async work and DB round trip
await new Promise((resolve) => setTimeout(resolve, 10));
createdAccount = { _id: "CONCURRENT_ACCOUNT_ID", userId };
return createdAccount;
});

const concurrentAuth = {
accountByUserId: concurrentAccountByUserId,
permissionsByUserId: jest.fn().mockResolvedValue([])
};

const makeContext = () => {
const ctx = {
auth: concurrentAuth,
collections: mockContext.collections,
getFunctionsOfType: mockContext.getFunctionsOfType,
mutations: {
createAccount
},
queries: {
primaryShopId: () => "PRIMARY_SHOP_ID"
},
userPermissions: [],
validatePermissions: mockContext.validatePermissions
};

// Minimal stub to satisfy buildContext's use of context.getInternalContext()
ctx.getInternalContext = function getInternalContext() {
return {
...this,
isInternalCall: true
};
};

return ctx;
};

const contexts = [makeContext(), makeContext(), makeContext(), makeContext(), makeContext()];

await Promise.all(contexts.map((ctx) => buildContext(ctx, { user: concurrentUser })));

expect(createAccount).toHaveBeenCalledTimes(1);
contexts.forEach((ctx) => {
expect(ctx.account).toEqual(createdAccount);
expect(ctx.accountId).toEqual(createdAccount._id);
expect(ctx.userId).toEqual(userId);
});
});
Loading