Skip to content

fix: require transactional query context for createOrGetExistingAsync utility method #628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wschurman wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix: require transactional query context for createOrGetExistingAsync utility method #628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
51 changes: 0 additions & 51 deletions packages/entity-database-adapter-knex/src/__integration-tests__/EntityCreationUtils-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,57 +33,6 @@ describe(createWithUniqueConstraintRecoveryAsync, () => {
});

describe.each([true, false])('is parallel creations %p', (parallel) => {
it('recovers when the same entity is created twice outside of transaction', async () => {
const vc1 = new ViewerContext(createKnexIntegrationTestEntityCompanionProvider(knexInstance));

const args = {
name: 'unique',
};

let createdEntities: [PostgresUniqueTestEntity, PostgresUniqueTestEntity];
if (parallel) {
createdEntities = await Promise.all([
createWithUniqueConstraintRecoveryAsync(
vc1,
PostgresUniqueTestEntity,
PostgresUniqueTestEntity.getByNameAsync,
args,
PostgresUniqueTestEntity.createWithNameAsync,
args,
),
createWithUniqueConstraintRecoveryAsync(
vc1,
PostgresUniqueTestEntity,
PostgresUniqueTestEntity.getByNameAsync,
args,
PostgresUniqueTestEntity.createWithNameAsync,
args,
),
]);
} else {
createdEntities = [
await createWithUniqueConstraintRecoveryAsync(
vc1,
PostgresUniqueTestEntity,
PostgresUniqueTestEntity.getByNameAsync,
args,
PostgresUniqueTestEntity.createWithNameAsync,
args,
),
await createWithUniqueConstraintRecoveryAsync(
vc1,
PostgresUniqueTestEntity,
PostgresUniqueTestEntity.getByNameAsync,
args,
PostgresUniqueTestEntity.createWithNameAsync,
args,
),
];
}

expect(createdEntities[0].getID()).toEqual(createdEntities[1].getID());
});

it('recovers when the same entity is created twice within same transaction', async () => {
const vc1 = new ViewerContext(createKnexIntegrationTestEntityCompanionProvider(knexInstance));

Expand Down
34 changes: 12 additions & 22 deletions packages/entity/src/utils/EntityCreationUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,25 +46,18 @@ export async function createOrGetExistingAsync<
queryContext?: EntityTransactionalQueryContext,
) => Promise<TEntity>,
createArgs: TCreateArgs,
queryContext?: EntityTransactionalQueryContext,
queryContext: EntityTransactionalQueryContext,
): Promise<TEntity> {
if (!queryContext) {
const maybeEntity = await getAsync(viewerContext, getArgs);
if (maybeEntity) {
return maybeEntity;
}
} else {
// This is done in a nested transaction since entity may negatively cache load results per-transaction (when configured).
// Without it, it would
// 1. load the entity in the current query context, negatively cache it
// 2. then try to create it in the nested transaction, which may fail due to a unique constraint error
// 3. then try to load the entity again in the current query context, which would return null due to negative cache
const maybeEntity = await queryContext.runInNestedTransactionAsync((nestedQueryContext) =>
getAsync(viewerContext, getArgs, nestedQueryContext),
);
if (maybeEntity) {
return maybeEntity;
}
// This is done in a nested transaction since entity may negatively cache load results per-transaction (when configured).
// Without it, it would
// 1. load the entity in the current query context, negatively cache it
// 2. then try to create it in the nested transaction, which may fail due to a unique constraint error
// 3. then try to load the entity again in the current query context, which would return null due to negative cache
const maybeEntity = await queryContext.runInNestedTransactionAsync((nestedQueryContext) =>
getAsync(viewerContext, getArgs, nestedQueryContext),
);
if (maybeEntity) {
return maybeEntity;
}
return await createWithUniqueConstraintRecoveryAsync(
viewerContext,
Expand Down Expand Up @@ -118,12 +111,9 @@ export async function createWithUniqueConstraintRecoveryAsync<
queryContext?: EntityTransactionalQueryContext,
) => Promise<TEntity>,
createArgs: TCreateArgs,
queryContext?: EntityTransactionalQueryContext,
queryContext: EntityTransactionalQueryContext,
): Promise<TEntity> {
try {
if (!queryContext) {
return await createAsync(viewerContext, createArgs);
}
return await queryContext.runInNestedTransactionAsync((nestedQueryContext) =>
createAsync(viewerContext, createArgs, nestedQueryContext),
);
Expand Down
Loading