fix: address pr comments

Author: teresalves

.changeset/funny-eggs-divide.md

@@ -2,4 +2,4 @@
 "miniflare": minor
 ---
 
-Add bulk gets for Workers KV
+Add Miniflare Workers KV bulk get support

packages/miniflare/src/workers/kv/constants.ts

@@ -34,6 +34,7 @@ export const SiteBindings = {
 // This ensures edge caching of Workers Sites files is disabled, and the latest
 // local version is always served.
 export const SITES_NO_CACHE_PREFIX = "$__MINIFLARE_SITES__$/";
+export const MAX_BULK_GET_KEYS = 100;
 
 export function encodeSitesKey(key: string): string {
 	// `encodeURIComponent()` ensures `ETag`s used by `@cloudflare/kv-asset-handler`

packages/miniflare/src/workers/kv/namespace.worker.ts

@@ -12,7 +12,7 @@ import {
 	PUT,
 	RouteHandler,
 } from "miniflare:shared";
-import { KVHeaders, KVLimits, KVParams } from "./constants";
+import { KVHeaders, KVLimits, KVParams, MAX_BULK_GET_KEYS } from "./constants";
 import {
 	decodeKey,
 	decodeListOptions,
@@ -22,7 +22,6 @@ import {
 	validatePutOptions,
 } from "./validator.worker";
 
-const MAX_BULK_GET_KEYS = 100;
 interface KVParams {
 	key: string;
 }
@@ -78,8 +77,8 @@ function secondsToMillis(seconds: number): number {
 
 async function processKeyValue(
 	obj: KeyValueEntry<unknown> | null,
-	type: string = "text",
-	withMetadata: boolean = false
+	type: "text" | "json" = "text",
+	withMetadata = false
 ) {
 	const decoder = new TextDecoder();
 	let decodedValue = "";
@@ -92,19 +91,18 @@ async function processKeyValue(
 
 	let val = null;
 	try {
-		val =
-			obj?.value == null
-				? null
-				: type === "json"
-					? JSON.parse(decodedValue)
-					: decodedValue;
+		val = !obj?.value
+			? null
+			: type === "json"
+				? JSON.parse(decodedValue)
+				: decodedValue;
 	} catch (err: any) {
 		throw new HttpError(
 			400,
-			"At least of of the requested keys corresponds to a non-JSON value"
+			"At least one of the requested keys corresponds to a non-JSON value"
 		);
 	}
-	if (val == null) {
+	if (val === null) {
 		return null;
 	}
 	if (withMetadata) {
@@ -137,11 +135,13 @@ export class KVNamespaceObject extends MiniflareDurableObject {
 			const keys: string[] = parsedBody.keys;
 			const type = parsedBody?.type;
 			if (type && type !== "text" && type !== "json") {
-				return new Response("", { status: 400 });
+				return new Response(`Type ${type} is invalid`, { status: 400 });
 			}
 			const obj: { [key: string]: any } = {};
 			if (keys.length > MAX_BULK_GET_KEYS) {
-				return new Response("", { status: 400 });
+				return new Response(`Accepting a max of 100 keys, got ${keys.length}`, {
+					status: 400,
+				});
 			}
 			for (const key of keys) {
 				validateGetOptions(key, { cacheTtl: parsedBody?.cacheTtl });

packages/miniflare/test/plugins/kv/index.spec.ts

@@ -6,6 +6,7 @@ import consumers from "stream/consumers";
 import { Macro, ThrowsExpectation } from "ava";
 import {
 	KV_PLUGIN_NAME,
+	MAX_BULK_GET_KEYS,
 	Miniflare,
 	MiniflareOptions,
 	ReplaceWorkersTypes,
@@ -139,7 +140,7 @@ test("bulk get: check max keys", async (t) => {
 	const { kv } = t.context;
 	await kv.put("key1", "value1");
 	const keyArray = [];
-	for (let i = 0; i <= 100; i++) {
+	for (let i = 0; i <= MAX_BULK_GET_KEYS; i++) {
 		keyArray.push(`key${i}`);
 	}
 	try {
@@ -167,7 +168,7 @@ test("bulk get: request json type", async (t) => {
 	} catch (error: any) {
 		t.is(
 			error.message,
-			"KV GET_BULK failed: 400 At least of of the requested keys corresponds to a non-JSON value"
+			"KV GET_BULK failed: 400 At least one of the requested keys corresponds to a non-JSON value"
 		);
 	}
 });

packages/miniflare/test/test-shared/miniflare.ts

@@ -45,7 +45,7 @@ export function namespace<T>(ns: string, binding: T): Namespaced<T> {
 					if (result instanceof Promise) {
 						return result.then((res) => {
 							// KV.get([a,b,c]) would be prefixed with ns, so we strip this prefix from response.
-							// Map keys => [ns-a, ns-b, ns-c] -> [a,b,c]
+							// Map keys => [{ns}{a}, {ns}{b}, {ns}{b}] -> [a,b,c]
 							if (res instanceof Map) {
 								const newResult = new Map<string, unknown>();
 								for (const [key, value] of res) {