Skip to content
This repository was archived by the owner on Oct 31, 2024. It is now read-only.

Commit b8965e6

Browse files
committed
fix: Removes null and undefined values from top level payload
The EJSON.stringify library does not remove null and undefined values, resulting in payloads that can contain sort, projection, etc set to an explicit undefined. EJSON turns these into "null" and then MongoDB cannot take the default value. The fix for this is to explicitly drop any null or undefined values, but only at the top level, as updates and aggregation pipelines need access to proper EJSON serialization. Fixes #1
1 parent 96f0d9c commit b8965e6

File tree

3 files changed

+34
-3
lines changed

3 files changed

+34
-3
lines changed

README.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -300,8 +300,8 @@ Requests via `fetch()` have their resposne codes checked against the [Data API E
300300

301301
# FAQ
302302

303-
- **Why is `mongodb` in the dependencies?** The short answer is [TypeScript requires it](https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html#dependencies). The mongodb dependency is types-only and will not be included in your built lambda when using `tsc`, `rollup`, `webpack`, and other bundling tools. Unfortunately, to re-export the types, it must be included as a regular dependency instead of dev-dependency. You can verify that mongo is not included by looking at the [CommonJS build](https://www.npmjs.com/package/@taskless/mongo-data-api?activeTab=code).
304-
- **Why is `node-fetch`'s `fetch` not of the correct type?** The short answer is that `node-fetch`'s `fetch` isn't a true `fetch`. To work around this, you can either use [`cross-fetch`](https://github.com/lquixada/cross-fetch) which types the `fetch` API through a type assertion, or [perform the type assertion yourself](https://github.com/lquixada/cross-fetch/blob/main/index.d.ts): `fetch: _fetch as typeof fetch`. It's not ideal, but with proper `fetch` coming to node.js, it's a small inconvienence in the short term.
303+
- **Why is `mongodb` in the dependencies?** [TypeScript requires it](https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html#dependencies), however, the mongodb dependency is types-only and will not be included in your built lambda when using `tsc`, `rollup`, `webpack`, etc. You can verify that mongo is not included by looking at the [CommonJS build](https://www.npmjs.com/package/@taskless/mongo-data-api?activeTab=code).
304+
- **Why is `node-fetch`'s `fetch` not of the correct type?** `node-fetch`'s `fetch` isn't a true `fetch` and wasn't typed as one. To work around this, you can either use [`cross-fetch`](https://github.com/lquixada/cross-fetch) which types the `fetch` API through a type assertion, or [perform the type assertion yourself](https://github.com/lquixada/cross-fetch/blob/main/index.d.ts): `fetch: _fetch as typeof fetch`. It's not ideal, but with proper `fetch` coming to node.js, it's a small inconvienence in the short term.
305305

306306
# License
307307

src/client.ts

+10-1
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,15 @@ export type MongoClientConstructorOptions = {
6666
headers?: OutgoingHttpHeaders | Headers;
6767
};
6868

69+
/**
70+
* Removes empty keys from an object at the top level. EJSON.stringify does not drop
71+
* undefined values in serialization, so we need to explicitly remove any keys with
72+
* null/undefined without recursing further into the object.
73+
*/
74+
const removeEmptyKeys = (object: Record<string, unknown>) => {
75+
return Object.fromEntries(Object.entries(object).filter(([_, v]) => v));
76+
};
77+
6978
/**
7079
* Create a MongoDB-like client for communicating with the Atlas Data API
7180
*
@@ -433,7 +442,7 @@ export class Collection<TSchema = Document> {
433442
collection: this.name,
434443
database: this.database.name,
435444
dataSource,
436-
...body,
445+
...removeEmptyKeys(body),
437446
}),
438447
});
439448

test/mocks/handlers.ts

+22
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,19 @@ function first<T>(value: T | T[] | readonly T[]) {
117117
return asArray(value)[0];
118118
}
119119

120+
const cannotBeNull: Record<string, string[]> = {
121+
find: ["filter", "projection"],
122+
findOne: ["filter", "projection", "sort", "limit", "skip"],
123+
insertOne: [],
124+
insertMany: [],
125+
updateOne: ["upsert"],
126+
updateMany: ["upsert"],
127+
replaceOne: ["upsert"],
128+
deleteOne: [],
129+
deleteMany: [],
130+
aggregate: [],
131+
};
132+
120133
export const handlers = [
121134
rest.post(`${BASE_URL}/action/:action`, async (request, response, ctx) => {
122135
const body: {
@@ -158,6 +171,15 @@ export const handlers = [
158171
);
159172
}
160173

174+
// attempting to pass "null" for an optional top level field results in a 400 from mongo
175+
const nonNullables =
176+
cannotBeNull[request.params.action as keyof typeof cannotBeNull] ?? [];
177+
for (const key of nonNullables) {
178+
if (body[key] === null) {
179+
return response(ctx.status(400), ctx.json({ error: "Bad Request" }));
180+
}
181+
}
182+
161183
// find payload for use case
162184
const action = first(request.params?.action) ?? "";
163185
const conditions = payloads[action];

0 commit comments

Comments
 (0)