-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(core): apply pessimistic locking to prevent order modification race conditions #3398 #4071
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -97,24 +97,60 @@ function getDbConfig(): DataSourceOptions { | |
| port: process.env.CI ? +(process.env.E2E_POSTGRES_PORT || 5432) : 5432, | ||
| username: 'vendure', | ||
| password: 'password', | ||
| // Connection timeouts to prevent aborted connections | ||
| connectTimeout: 60000, | ||
| // Keep connections alive | ||
| keepConnectionAlive: true, | ||
| extra: { | ||
| // Connection pool settings | ||
| max: 10, | ||
| idleTimeoutMillis: 30000, | ||
| connectionTimeoutMillis: 60000, | ||
| }, | ||
| }; | ||
| case 'mariadb': | ||
| return { | ||
| synchronize: true, | ||
| type: 'mariadb', | ||
| host: '127.0.0.1', | ||
| port: process.env.CI ? +(process.env.E2E_MARIADB_PORT || 3306) : 3306, | ||
| username: 'root', | ||
| username: 'vendure', | ||
| password: 'password', | ||
| extra: { | ||
| // Ensure tables use InnoDB for locking support | ||
| initSql: "SET default_storage_engine=InnoDB;", | ||
| // Connection pool settings | ||
| connectionLimit: 10, | ||
| waitForConnections: true, | ||
| queueLimit: 0, | ||
| }, | ||
|
Comment on lines
+119
to
+126
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Check if initSql is used anywhere in the codebase
rg -i "initSql" --type ts --type jsRepository: vendurehq/vendure Length of output: 246 🏁 Script executed: # Check the surrounding code context for mysql2 configuration patterns
cd e2e-common && head -150 test-config.ts | tail -50Repository: vendurehq/vendure Length of output: 2087 🌐 Web query:
💡 Result: Short answer: mysql2 has no built‑in connection option named initSql / initSQL. To run initialization SQL when a connection is created, acquire the connection and run the statements (or listen for pool events and run them on new connections). See mysql2 docs for createPool/createConnection and examples, and the classic node-mysql pool "connection" event pattern for running session init statements. [1][2] References
🌐 Web query:
💡 Result: Use the TypeORM DataSource option extra to pass the driver's initial SQL command (the underlying mysql/mysql2 client will run it on connect). Example — set default storage engine to InnoDB: TypeScript: Notes:
Sources: Replace The option Change both occurrences from: initSql: "SET default_storage_engine=InnoDB;",to: init_command: "SET default_storage_engine=INNODB",This applies to lines 119-126 (MariaDB) and 141-148 (MySQL). 🤖 Prompt for AI Agents |
||
| // Connection timeouts to prevent aborted connections | ||
| connectTimeout: 60000, | ||
| acquireTimeout: 60000, | ||
| // Keep connections alive | ||
| keepConnectionAlive: true, | ||
| }; | ||
| case 'mysql': | ||
| return { | ||
| synchronize: true, | ||
| type: 'mysql', | ||
| host: '127.0.0.1', | ||
| port: process.env.CI ? +(process.env.E2E_MYSQL_PORT || 3306) : 3306, | ||
| username: 'root', | ||
| username: 'vendure', | ||
| password: 'password', | ||
| extra: { | ||
| // Ensure tables use InnoDB for locking support | ||
| initSql: "SET default_storage_engine=InnoDB;", | ||
| // Connection pool settings | ||
| connectionLimit: 10, | ||
| waitForConnections: true, | ||
| queueLimit: 0, | ||
| }, | ||
| // Connection timeouts to prevent aborted connections | ||
| connectTimeout: 60000, | ||
| acquireTimeout: 60000, | ||
| // Keep connections alive | ||
| keepConnectionAlive: true, | ||
| }; | ||
| case 'sqljs': | ||
| default: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| import { createTestEnvironment } from '@vendure/testing'; | ||
| import * as path from 'path'; | ||
| import { afterAll, beforeAll, describe, expect, it } from 'vitest'; | ||
|
|
||
| import { initialData } from '../../e2e-common/e2e-initial-data'; | ||
| import { TEST_SETUP_TIMEOUT_MS, testConfig } from '../../e2e-common/test-config'; | ||
|
|
||
| describe('Order race conditions', () => { | ||
| const { server, adminClient, shopClient } = createTestEnvironment(testConfig); | ||
|
|
||
| beforeAll(async () => { | ||
| await server.init({ | ||
| initialData, | ||
| productsCsvPath: path.join(__dirname, '../../e2e-common/fixtures/e2e-products-full.csv'), | ||
| customerCount: 1, | ||
| }); | ||
| await shopClient.asUserWithCredentials('[email protected]', 'test'); | ||
| }, TEST_SETUP_TIMEOUT_MS); | ||
|
|
||
| afterAll(async () => { | ||
| await server.destroy(); | ||
| }); | ||
|
|
||
| it('handles parallel modifications to the order correctly', async () => { | ||
| const ADD_ITEM_TO_ORDER = ` | ||
| mutation AddItemToOrder($productVariantId: ID!, $quantity: Int!) { | ||
| addItemToOrder(productVariantId: $productVariantId, quantity: $quantity) { | ||
| ... on Order { | ||
| id | ||
| totalQuantity | ||
| total | ||
| } | ||
| ... on ErrorResult { | ||
| errorCode | ||
| message | ||
| } | ||
| } | ||
| } | ||
| `; | ||
|
|
||
| const variantId = 'T_1'; // Laptop 13 inch 8GB | ||
| const quantityPerRequest = 1; | ||
| const concurrency = 5; | ||
|
|
||
| // Executing 5 Simulataneous requests to add an item | ||
| const promises: Array<Promise<any>> = []; | ||
| for (let i = 0; i < concurrency; i++) { | ||
| promises.push( | ||
| shopClient.query(ADD_ITEM_TO_ORDER, { | ||
| productVariantId: variantId, | ||
| quantity: quantityPerRequest, | ||
| }), | ||
| ); | ||
| } | ||
|
|
||
| await Promise.all(promises); | ||
|
|
||
| const { activeOrder } = await shopClient.query(` | ||
| query GetActiveOrder { activeOrder { totalQuantity } } | ||
| `); | ||
|
|
||
| // Si hay condición de carrera, el total será menor a 5 (algunas escrituras se sobrescribieron) | ||
| expect(activeOrder.totalQuantity).toBe(concurrency * quantityPerRequest); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.