feat: release 3 improvements by mvantellingen · Pull Request #379 · labd/commercetools-node-mock

mvantellingen · 2026-03-04T06:41:37Z

This pull request introduces several improvements and fixes to the @labdigital/commercetools-mock package, focusing on performance, error handling, and logging. The most significant changes include major optimizations for custom object and resource lookups, unified logging via Pino, and enhanced error response consistency. Additionally, there are fixes for category creation and DELETE request handling, as well as updates to dependencies.

Performance Optimizations

Optimized custom object lookups by container and key from O(n) to O(1, log n) using in-memory and indexed SQLite queries, including schema migration and fast existence checks. [1] [2]
Improved resource creation performance by removing redundant fetches, caching project keys, and reusing resources in POST handlers.

Logging and Error Handling

Unified logging to use Pino (via Fastify), with support for custom logger instances and pretty output in the standalone server.
Centralized error handling: all error responses are now routed through a central handler, ensuring consistent logging and response bodies when not in silent mode.

Bug Fixes

Fixed category creation when specifying a parent by key instead of id, addressing a ResourceIdentifier error.
Fixed Fastify rejecting DELETE requests with empty JSON bodies by adding a tolerant content-type parser.

Dependency Updates

Added new dependencies for logging: pino, pino-pretty, and their transitive dependencies in package.json and pnpm-lock.yaml. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18] [19] [20] [21] [22] [23] [24] [25] [26] [27] [28]

Configuration

Updated .changeset/config.json to ignore the mvantellingen package.- fix: resolve category parent reference by key instead of only by id

feat: log error responses to console when silent option is false
perf: eliminate redundant re-fetch after resource creation
perf: optimize custom object lookups and add storage count method
refactor: route all error responses through central error handler
feat: unify logging to use Pino via Fastify's built-in logger
fix: handle DELETE requests with empty JSON body

Previously, creating a category with a parent specified by key (instead of id) would fail. Now uses getReferenceFromResourceIdentifier to properly resolve the parent reference from either id or key.

Adds console.error logging for both CommercetoolsError and unhandled errors in the Fastify error handler when silent mode is disabled. Also adds ENABLE_LOGGING env var support to the standalone server.

- Return the object directly after insert in both InMemory and SQLite storage backends instead of re-fetching by id - Cache known project keys in SQLite to skip repeated INSERT+SELECT - Reuse the already-created resource in the service POST handler instead of re-fetching from storage via _expandWithId

- Add secondary index for custom objects (container+key) in both InMemoryStorage and SQLiteStorage for O(1)/O(log n) lookups - Add count() method to storage backends for efficient existence checks - Short-circuit review statistics when no reviews exist - Add SQLite schema migration for new container/co_key columns - Replace full-scan all()+find() with indexed getByContainerAndKey()

Convert direct reply.status(4xx).send() calls to throw CommercetoolsError instead. This ensures all error responses go through the central error handler, providing consistent response bodies with statusCode, message, and errors fields.

Replace console.error calls with structured request.log.error() logging. Add support for passing a custom Pino logger instance via the new logger option on CommercetoolsMockOptions. The standalone server now uses pino-pretty for human-readable output. Re-exports FastifyBaseLogger type for consumers.

Add a custom content-type parser to tolerate empty bodies in requests with Content-Type: application/json. This fixes issues with SDKs that send DELETE requests with the JSON content type header but no body.

changeset-bot · 2026-03-04T06:41:42Z

🦋 Changeset detected

Latest commit: d8d4067

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copilot

Pull request overview

This PR bundles several improvements to the @labdigital/commercetools-mock library: performance optimizations, bug fixes, a unified logging approach, and better error handling consistency.

Changes:

Performance & correctness: Eliminates redundant re-fetches after resource creation, adds count() and getByContainerAndKey() storage methods, adds a secondary in-memory index for custom objects, and adds container/co_key indexed columns to SQLite.
Error handling: Routes all 4xx/5xx responses through the Fastify error handler by converting direct reply.status().send() calls to throw new CommercetoolsError(...), enabling centralized logging.
Bug fixes: Fixes category parent resolution by key, fixes Fastify rejecting DELETE requests with an empty JSON body, and unifies logging via Fastify's built-in pino logger with an optional custom logger instance.

Reviewed changes

Copilot reviewed 29 out of 30 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`src/storage/sqlite.ts`	Adds `container`/`co_key` columns, schema migration, `count()`, `getByContainerAndKey()`, project cache, and direct-return optimization
`src/storage/in-memory.ts`	Adds `_customObjectIndex` secondary index, `count()`, `getByContainerAndKey()`, and direct-return optimization
`src/storage/abstract.ts`	Adds abstract `count()` and `getByContainerAndKey()` methods
`src/repositories/abstract.ts`	Adds `expand()` helper method to avoid re-fetching
`src/repositories/category/index.ts`	Fixes parent reference resolution to support key-based lookup
`src/repositories/custom-object.ts`	Replaces O(n) scan with `getByContainerAndKey()` indexed lookup
`src/lib/review-statistics.ts`	Short-circuits review stats when no reviews exist using `count()`
`src/services/abstract.ts`	Converts all `reply.status(4xx).send()` to `CommercetoolsError` throws; refactors `post()` to avoid re-fetch
`src/services/cart.ts`, `custom-object.ts`, `my-cart.ts`, `my-customer.ts`, `order.ts`, `project.ts`, `shipping-method.ts`	Converts direct reply error responses to `CommercetoolsError`
`src/ctMock.ts`	Adds custom logger support, empty JSON body parser, and error logging in the error handler
`src/server.ts`	Uses Fastify logger for startup message; supports `ENABLE_LOGGING` env var with pino-pretty
`src/index.ts`	Exports `FastifyBaseLogger` type
`package.json`	Adds `pino` and `pino-pretty` as runtime dependencies
`src/services/category.test.ts`	Adds tests for category parent-by-key creation
`src/services/my-customer.test.ts`	Updates test expectation to include new `statusCode` field
`.changeset/*.md`	Multiple changeset entries covering all changes

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

Copilot · 2026-03-04T06:50:18Z

package.json

 		"light-my-request": "6.6.0",
 		"msw": "2.12.10",
+		"pino": "^10.3.1",
+		"pino-pretty": "^13.1.3",


pino-pretty is listed as a runtime dependency but it is only used in src/server.ts (the standalone development server), not in the library's public API. Library consumers that install this package will have pino-pretty and all its transitive dependencies pulled in unnecessarily. It should be moved to devDependencies. Users of the standalone server who need pretty-printing can install it themselves, or the documentation can note that pino-pretty is an optional peer dependency of the standalone server.

Copilot · 2026-03-04T06:50:18Z

src/storage/sqlite.ts

+		}
+		if (!columnNames.has("co_key")) {
+			this.db.exec("ALTER TABLE resources ADD COLUMN co_key TEXT");
+		}


The schema migration adds the container and co_key columns to existing rows as NULL, but does not backfill them with data extracted from the data JSON column. Any custom objects already stored in an existing SQLite database will have NULL values for these columns, making them invisible to getByContainerAndKey(). The migration should include an UPDATE statement to populate these columns from the existing JSON, such as:

UPDATE resources SET container = json_extract(data, '$.container'), co_key = json_extract(data, '$.key') WHERE type_id = 'key-value-document'

Suggested change

}

}

// Backfill container and co_key for existing custom objects so that

// getByContainerAndKey() can find them. We only touch key-value-document

// resources, and only where either column is still NULL.

this.db.exec(

"UPDATE resources " +

"SET container = json_extract(data, '$.container'), " +

" co_key = json_extract(data, '$.key') " +

"WHERE type_id = 'key-value-document' " +

" AND (container IS NULL OR co_key IS NULL)",

);

Copilot · 2026-03-04T06:50:18Z

src/storage/sqlite.ts

+		const row = this.stmtCountResources.get(projectKey, typeId) as {
+			cnt: number;
+		};
+		return row.cnt;


The new count() abstract method is added to AbstractStorage and implemented in both InMemoryStorage and SQLiteStorage, but it has no dedicated tests in src/storage/storage.test.ts. Given that the storage test suite is thorough and runs against both the in-memory and SQLite backends, a test for count() should be added to ensure correctness and prevent regressions (e.g., testing that it returns 0 for an empty store, the correct count after adding resources, and after deleting resources).

Suggested change

const row = this.stmtCountResources.get(projectKey, typeId) as {

cnt: number;

};

return row.cnt;

const row = this.stmtCountResources.get(projectKey, typeId) as

| {

cnt: number;

}

| undefined;

return row?.cnt ?? 0;

Copilot · 2026-03-04T06:50:18Z

src/storage/sqlite.ts

+	async getByContainerAndKey(
+		projectKey: string,
+		container: string,
+		key: string,
+	): Promise<CustomObject | null> {
+		const row = this.stmtGetResourceByContainerAndKey.get(
+			projectKey,
+			container,
+			key,
+		) as { data: string } | undefined;
+		if (row) {
+			return JSON.parse(row.data) as CustomObject;
+		}
+		return null;
+	}


The new getByContainerAndKey() method added to AbstractStorage, InMemoryStorage, and SQLiteStorage is not directly tested in src/storage/storage.test.ts. The storage tests are the canonical place to validate both backends work identically. Tests should cover at minimum: successfully finding a custom object by container and key, returning null when not found, and verifying the index is kept consistent after a delete operation (i.e., the object cannot be found after deletion).

Copilot · 2026-03-04T06:50:19Z

src/storage/sqlite.ts

+	 * ALTER TABLE ADD COLUMN is a no-op if the column already exists in SQLite
+	 * when using IF NOT EXISTS (not supported), so we catch errors instead.


The docstring on _migrateSchema() says "we catch errors instead" to handle the case where a column already exists, but the actual implementation uses PRAGMA table_info() to check for column existence before attempting ALTER TABLE. No errors are caught in the implementation. The docstring should be corrected to accurately describe the approach used.

Suggested change

* ALTER TABLE ADD COLUMN is a no-op if the column already exists in SQLite

* when using IF NOT EXISTS (not supported), so we catch errors instead.

* Since SQLite does not support `ALTER TABLE ... ADD COLUMN IF NOT EXISTS`,

* we first inspect the existing schema via `PRAGMA table_info(resources)`

* and only add the `container` and `co_key` columns if they are missing.

Copilot · 2026-03-04T06:50:19Z

src/storage/sqlite.ts

 		this.db = new DatabaseSync(filename);
 		this.db.exec(SCHEMA);
+
+		// Migration: add container and co_key columns for existing databases
+		this._migrateSchema();


The initialization order in the SQLiteStorage constructor is incorrect for existing databases. The SCHEMA string includes CREATE INDEX IF NOT EXISTS idx_resources_container_key ON resources (project_key, container, co_key) WHERE type_id = 'key-value-document', but the container and co_key columns don't exist in databases created by earlier versions. When this.db.exec(SCHEMA) is called first, the index creation will fail with a "no such column" error for old databases — the subsequent _migrateSchema() call never runs.

The fix is to split the SCHEMA so that the base table and existing index definitions run first, then call _migrateSchema() (which adds the new columns if needed), and then create the new index. Alternatively, move the index creation into _migrateSchema() itself, guarding it with a check similar to how the columns are checked.

Copilot · 2026-03-04T06:50:19Z

.changeset/optimize-custom-object-lookup.md

+- Use `json_extract` in SQLiteStorage for direct SQL lookup instead of loading all rows
+- Replace the full-scan `all()` + `find()` in `CustomObjectRepository.getWithContainerAndKey()` with the new indexed lookup


The changeset description states "Use json_extract in SQLiteStorage for direct SQL lookup instead of loading all rows," but the actual implementation adds dedicated container and co_key columns with a SQL index — it does not use json_extract. The changeset description is inaccurate and should be updated to reflect the actual approach (dedicated indexed columns).

…inate redundant clones Replace raw Map with StorageMap in InMemoryStorage so values are automatically deep-cloned on insertion and retrieval. This removes the need for scattered cloneObject calls across the storage layer, repositories, and expand logic while ensuring stored data remains isolated from external mutations.

- Move pino-pretty to devDependencies (only used by standalone server) - Replace container/co_key columns with json_extract expression index - Remove unnecessary _migrateSchema() (no released databases to migrate) - Make SQLiteStorage.count() handle undefined row defensively - Add tests for count() and getByContainerAndKey() in storage.test.ts - Fix changeset description to match actual implementation

mvantellingen added 7 commits March 3, 2026 22:05

fix: resolve category parent reference by key instead of only by id

82214aa

Previously, creating a category with a parent specified by key (instead of id) would fail. Now uses getReferenceFromResourceIdentifier to properly resolve the parent reference from either id or key.

feat: log error responses to console when silent option is false

ac3dfcd

Adds console.error logging for both CommercetoolsError and unhandled errors in the Fastify error handler when silent mode is disabled. Also adds ENABLE_LOGGING env var support to the standalone server.

fix: handle DELETE requests with empty JSON body

bce45a7

Add a custom content-type parser to tolerate empty bodies in requests with Content-Type: application/json. This fixes issues with SDKs that send DELETE requests with the JSON content type header but no body.

Copilot AI review requested due to automatic review settings March 4, 2026 06:41

Copilot started reviewing on behalf of mvantellingen March 4, 2026 06:42 View session

chore: dont thank me too much

14e0241

Copilot AI reviewed Mar 4, 2026

View reviewed changes

mvantellingen added 2 commits March 4, 2026 08:03

mvantellingen force-pushed the feature/release-3-improvements branch from 7e408df to d8d4067 Compare March 4, 2026 07:39

mvantellingen merged commit 85d332d into main Mar 4, 2026
17 checks passed

mvantellingen deleted the feature/release-3-improvements branch March 4, 2026 07:44

github-actions bot mentioned this pull request Mar 4, 2026

Release new version (beta) #378

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: release 3 improvements#379

feat: release 3 improvements#379
mvantellingen merged 10 commits intomainfrom
feature/release-3-improvements

mvantellingen commented Mar 4, 2026 •

edited

Loading

Uh oh!

changeset-bot bot commented Mar 4, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-		}
+		}
+		// Backfill container and co_key for existing custom objects so that
+		// getByContainerAndKey() can find them. We only touch key-value-document
+		// resources, and only where either column is still NULL.
+		this.db.exec(
+			"UPDATE resources " +
+				"SET container = json_extract(data, '$.container'), " +
+				"    co_key = json_extract(data, '$.key') " +
+				"WHERE type_id = 'key-value-document' " +
+				"  AND (container IS NULL OR co_key IS NULL)",
+		);

		* ALTER TABLE ADD COLUMN is a no-op if the column already exists in SQLite
		* when using IF NOT EXISTS (not supported), so we catch errors instead.

-	 * ALTER TABLE ADD COLUMN is a no-op if the column already exists in SQLite
-	 * when using IF NOT EXISTS (not supported), so we catch errors instead.
+	 * Since SQLite does not support `ALTER TABLE ... ADD COLUMN IF NOT EXISTS`,
+	 * we first inspect the existing schema via `PRAGMA table_info(resources)`
+	 * and only add the `container` and `co_key` columns if they are missing.

		- Use `json_extract` in SQLiteStorage for direct SQL lookup instead of loading all rows
		- Replace the full-scan `all()` + `find()` in `CustomObjectRepository.getWithContainerAndKey()` with the new indexed lookup

Conversation

mvantellingen commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance Optimizations

Logging and Error Handling

Bug Fixes

Dependency Updates

Configuration

Uh oh!

changeset-bot bot commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mvantellingen commented Mar 4, 2026 •

edited

Loading

changeset-bot bot commented Mar 4, 2026 •

edited

Loading