Loosen validation around different configurations for Durable Object

.changeset/loose-glasses-poke.md

@@ -0,0 +1,7 @@
+---
+"miniflare": patch
+---
+
+Loosen validation around different configurations for Durable Object
+
+Allow durable objects to have `enableSql`, `unsafeUniqueKey` and `unsafePreventEviction` configurations set to `undefined` even if the same durable objects are defined with those configurations set to different values (this allows workers using external durable objects not to have to duplicate such configurations in their options)

packages/miniflare/src/index.ts

@@ -331,63 +331,114 @@ function getDurableObjectClassNames(
 	allWorkerOpts: PluginWorkerOptions[]
 ): DurableObjectClassNames {
 	const serviceClassNames: DurableObjectClassNames = new Map();
-	for (const workerOpts of allWorkerOpts) {
-		const workerServiceName = getUserServiceName(workerOpts.core.name);
-		for (const designator of Object.values(
-			workerOpts.do.durableObjects ?? {}
-		)) {
-			const {
-				className,
-				// Fallback to current worker service if name not defined
-				serviceName = workerServiceName,
-				enableSql,
-				unsafeUniqueKey,
-				unsafePreventEviction,
-				container,
-			} = normaliseDurableObject(designator);
-			// Get or create `Map` mapping class name to optional unsafe unique key
-			let classNames = serviceClassNames.get(serviceName);
-			if (classNames === undefined) {
-				classNames = new Map();
-				serviceClassNames.set(serviceName, classNames);
-			}
-			if (classNames.has(className)) {
-				// If we've already seen this class in this service, make sure the
-				// unsafe unique keys and unsafe prevent eviction values match
-				const existingInfo = classNames.get(className);
-				if (existingInfo?.enableSql !== enableSql) {
-					throw new MiniflareCoreError(
-						"ERR_DIFFERENT_STORAGE_BACKEND",
-						`Different storage backends defined for Durable Object "${className}" in "${serviceName}": ${JSON.stringify(
-							enableSql
-						)} and ${JSON.stringify(existingInfo?.enableSql)}`
-					);
+
+	const allDurableObjects = allWorkerOpts
+		.flatMap((workerOpts) => {
+			const workerServiceName = getUserServiceName(workerOpts.core.name);
+
+			return Object.values(workerOpts.do.durableObjects ?? {}).map(
+				(workerDODesignator) => {
+					const doInfo = normaliseDurableObject(workerDODesignator);
+					if (doInfo.serviceName === undefined) {
+						// Fallback to current worker service if name not defined
+						doInfo.serviceName = workerServiceName;
+					}
+					return {
+						doInfo,
+						workerRawName: workerOpts.core.name,
+					};
 				}
-				if (existingInfo?.unsafeUniqueKey !== unsafeUniqueKey) {
-					throw new MiniflareCoreError(
-						"ERR_DIFFERENT_UNIQUE_KEYS",
-						`Multiple unsafe unique keys defined for Durable Object "${className}" in "${serviceName}": ${JSON.stringify(
-							unsafeUniqueKey
-						)} and ${JSON.stringify(existingInfo?.unsafeUniqueKey)}`
-					);
+			);
+		})
+		// We sort the list of durable objects because we want the durable objects without a scriptName or a scriptName
+		// that matches the raw worker's name (meaning that they are defined within their worker) to be processed first
+		.sort(({ doInfo, workerRawName }) =>
+			doInfo.scriptName === undefined || doInfo.scriptName === workerRawName
+				? -1
+				: 0
+		)
+		.map(({ doInfo }) => doInfo);
+
+	for (const doInfo of allDurableObjects) {
+		const { className, serviceName, container, ...doConfigs } = doInfo;
+		// We know that the service name is always defined (since if it is not we do default it to the current worker service)
+		assert(serviceName);
+		// Get or create `Map` mapping class name to optional unsafe unique key
+		let classNames = serviceClassNames.get(serviceName);
+		if (classNames === undefined) {
+			classNames = new Map();
+			serviceClassNames.set(serviceName, classNames);
+		}
+
+		if (classNames.has(className)) {
+			// If we've already seen this class in this service, make sure the
+			// unsafe unique keys and unsafe prevent eviction values match
+			const existingInfo = classNames.get(className);
+
+			const isDoUnacceptableDiff = (
+				field: Extract<
+					keyof typeof doConfigs,
+					"enableSql" | "unsafeUniqueKey" | "unsafePreventEviction"
+				>
+			) => {
+				if (!existingInfo) {
+					return false;
 				}
-				if (existingInfo?.unsafePreventEviction !== unsafePreventEviction) {
-					throw new MiniflareCoreError(
-						"ERR_DIFFERENT_PREVENT_EVICTION",
-						`Multiple unsafe prevent eviction values defined for Durable Object "${className}" in "${serviceName}": ${JSON.stringify(
-							unsafePreventEviction
-						)} and ${JSON.stringify(existingInfo?.unsafePreventEviction)}`
-					);
+
+				const same = existingInfo[field] === doConfigs[field];
+				if (same) {
+					return false;
 				}
-			} else {
-				// Otherwise, just add it
-				classNames.set(className, {
-					enableSql,
-					unsafeUniqueKey,
-					unsafePreventEviction,
-					container,
-				});
+
+				const oneIsUndefined =
+					existingInfo[field] === undefined || doConfigs[field] === undefined;
+
+				// If one of the configurations is `undefined` (either the current one or the existing one) then there we
+				// want to consider this as an acceptable difference since we might be in a potentially valid situation in
+				// which worker A defines a DO with a config, while worker B simply uses the DO from worker A but without
+				// providing the configuration (thus leaving it `undefined`) (this for example is exactly what Wrangler does
+				// with the implicitly defined `enableSql` flag)
+				if (oneIsUndefined) {
+					return false;
+				}
+
+				return true;
+			};
+
+			if (isDoUnacceptableDiff("enableSql")) {
+				throw new MiniflareCoreError(
+					"ERR_DIFFERENT_STORAGE_BACKEND",
+					`Different storage backends defined for Durable Object "${className}" in "${serviceName}": ${JSON.stringify(
+						doConfigs.enableSql
+					)} and ${JSON.stringify(existingInfo?.enableSql)}`
+				);
+			}
+
+			if (isDoUnacceptableDiff("unsafeUniqueKey")) {
+				throw new MiniflareCoreError(
+					"ERR_DIFFERENT_UNIQUE_KEYS",
+					`Multiple unsafe unique keys defined for Durable Object "${className}" in "${serviceName}": ${JSON.stringify(
+						doConfigs.unsafeUniqueKey
+					)} and ${JSON.stringify(existingInfo?.unsafeUniqueKey)}`
+				);
 			}
+
+			if (isDoUnacceptableDiff("unsafePreventEviction")) {
+				throw new MiniflareCoreError(
+					"ERR_DIFFERENT_PREVENT_EVICTION",
+					`Multiple unsafe prevent eviction values defined for Durable Object "${className}" in "${serviceName}": ${JSON.stringify(
+						doConfigs.unsafePreventEviction
+					)} and ${JSON.stringify(existingInfo?.unsafePreventEviction)}`
+				);
+			}
+		} else {
+			// Otherwise, just add it
+			classNames.set(className, {
+				enableSql: doConfigs.enableSql,
+				unsafeUniqueKey: doConfigs.unsafeUniqueKey,
+				unsafePreventEviction: doConfigs.unsafePreventEviction,
+				container,
+			});
 		}
 	}
 	return serviceClassNames;

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

@@ -482,3 +482,124 @@ test("colo-local actors", async (t) => {
 	res = await stub.fetch("http://localhost");
 	t.is(await res.text(), "body:thing2");
 });
+
+test("multiple workers with DO conflicting useSQLite booleans cause options error", async (t) => {
+	const mf = new Miniflare({
+		workers: [
+			{
+				modules: true,
+				name: "worker-a",
+				script: "export default {}",
+			},
+		],
+	});
+
+	t.teardown(() => mf.dispose());
+
+	await t.throwsAsync(
+		async () => {
+			await mf.setOptions({
+				workers: [
+					{
+						modules: true,
+						name: "worker-c",
+						script: "export default {}",
+						durableObjects: {
+							MY_DO: {
+								className: "MyDo",
+								scriptName: "worker-a",
+								useSQLite: false,
+							},
+						},
+					},
+					{
+						modules: true,
+						name: "worker-a",
+						script: `
+							import { DurableObject } from "cloudflare:workers";
+
+							export class MyDo extends DurableObject {}
+
+							export default { }
+						`,
+						durableObjects: {
+							MY_DO: {
+								className: "MyDo",
+								scriptName: undefined,
+								useSQLite: true,
+							},
+						},
+					},
+					{
+						modules: true,
+						name: "worker-b",
+						script: "export default {}",
+						durableObjects: {
+							MY_DO: {
+								className: "MyDo",
+								scriptName: "worker-a",
+								useSQLite: false,
+							},
+						},
+					},
+				],
+			});
+		},
+		{
+			instanceOf: Error,
+			message:
+				'Different storage backends defined for Durable Object "MyDo" in "core:user:worker-a": false and true',
+		}
+	);
+});
+
+test("multiple workers with DO useSQLite true and undefined does not cause options error", async (t) => {
+	const mf = new Miniflare({
+		workers: [
+			{
+				modules: true,
+				name: "worker-a",
+				script: "export default {}",
+			},
+		],
+	});
+
+	t.teardown(() => mf.dispose());
+
+	await t.notThrowsAsync(async () => {
+		await mf.setOptions({
+			workers: [
+				{
+					modules: true,
+					name: "worker-a",
+					script: `
+							import { DurableObject } from "cloudflare:workers";
+
+							export class MyDo extends DurableObject {}
+
+							export default { }
+						`,
+					durableObjects: {
+						MY_DO: {
+							className: "MyDo",
+							scriptName: undefined,
+							useSQLite: true,
+						},
+					},
+				},
+				{
+					modules: true,
+					name: "worker-b",
+					script: "export default {}",
+					durableObjects: {
+						MY_DO: {
+							className: "MyDo",
+							scriptName: "worker-a",
+							useSQLite: undefined,
+						},
+					},
+				},
+			],
+		});
+	});
+});