From 3fbb31c3c1f85011db9cc616dab0ef113342d7dd Mon Sep 17 00:00:00 2001 From: Luca Casonato Date: Thu, 27 Apr 2023 16:59:02 +0200 Subject: [PATCH] feat(kv): return ok bool from atomic commit (#18873) --- cli/tests/unit/kv_test.ts | 39 ++++++++++++++-------------- cli/tsc/dts/lib.deno.unstable.d.ts | 41 ++++++++++++++++++++---------- ext/kv/01_db.ts | 8 +++--- 3 files changed, 52 insertions(+), 36 deletions(-) diff --git a/cli/tests/unit/kv_test.ts b/cli/tests/unit/kv_test.ts index 0af8f338d2..0dc1690aad 100644 --- a/cli/tests/unit/kv_test.ts +++ b/cli/tests/unit/kv_test.ts @@ -66,6 +66,7 @@ dbTest("basic read-write-delete and versionstamps", async (db) => { assertEquals(result1.versionstamp, null); const setRes = await db.set(["a"], "b"); + assert(setRes.ok); assertEquals(setRes.versionstamp, "00000000000000010000"); const result2 = await db.get(["a"]); assertEquals(result2.key, ["a"]); @@ -183,7 +184,7 @@ dbTest("compare and mutate", async (db) => { .check({ key: ["t"], versionstamp: currentValue.versionstamp }) .set(currentValue.key, "2") .commit(); - assert(res); + assert(res.ok); assertEquals(res.versionstamp, "00000000000000020000"); const newValue = await db.get(["t"]); @@ -194,7 +195,7 @@ dbTest("compare and mutate", async (db) => { .check({ key: ["t"], versionstamp: currentValue.versionstamp }) .set(currentValue.key, "3") .commit(); - assertEquals(res, null); + assert(!res.ok); const newValue2 = await db.get(["t"]); assertEquals(newValue2.versionstamp, "00000000000000020000"); @@ -206,7 +207,7 @@ dbTest("compare and mutate not exists", async (db) => { .check({ key: ["t"], versionstamp: null }) .set(["t"], "1") .commit(); - assert(res); + assert(res.ok); const newValue = await db.get(["t"]); assertEquals(newValue.versionstamp, "00000000000000010000"); @@ -216,7 +217,7 @@ dbTest("compare and mutate not exists", async (db) => { .check({ key: ["t"], versionstamp: null }) .set(["t"], "2") .commit(); - assertEquals(res, null); + assert(!res.ok); }); dbTest("atomic mutation helper (sum)", async (db) => { @@ -264,7 +265,7 @@ dbTest("compare multiple and mutate", async (db) => { .set(currentValue1.key, "3") .set(currentValue2.key, "4") .commit(); - assert(res); + assert(res.ok); const newValue1 = await db.get(["t1"]); assertEquals(newValue1.versionstamp, "00000000000000030000"); @@ -280,7 +281,7 @@ dbTest("compare multiple and mutate", async (db) => { .set(newValue1.key, "5") .set(newValue2.key, "6") .commit(); - assertEquals(res2, null); + assert(!res2.ok); const newValue3 = await db.get(["t1"]); assertEquals(newValue3.versionstamp, "00000000000000030000"); @@ -296,7 +297,7 @@ dbTest("atomic mutation ordering (set before delete)", async (db) => { .set(["a"], "2") .delete(["a"]) .commit(); - assert(res); + assert(res.ok); const result = await db.get(["a"]); assertEquals(result.value, null); }); @@ -307,7 +308,7 @@ dbTest("atomic mutation ordering (delete before set)", async (db) => { .delete(["a"]) .set(["a"], "2") .commit(); - assert(res); + assert(res.ok); const result = await db.get(["a"]); assertEquals(result.value, "2"); }); @@ -316,7 +317,7 @@ dbTest("atomic mutation type=set", async (db) => { const res = await db.atomic() .mutate({ key: ["a"], value: "1", type: "set" }) .commit(); - assert(res); + assert(res.ok); const result = await db.get(["a"]); assertEquals(result.value, "1"); }); @@ -326,7 +327,7 @@ dbTest("atomic mutation type=set overwrite", async (db) => { const res = await db.atomic() .mutate({ key: ["a"], value: "2", type: "set" }) .commit(); - assert(res); + assert(res.ok); const result = await db.get(["a"]); assertEquals(result.value, "2"); }); @@ -336,7 +337,7 @@ dbTest("atomic mutation type=delete", async (db) => { const res = await db.atomic() .mutate({ key: ["a"], type: "delete" }) .commit(); - assert(res); + assert(res.ok); const result = await db.get(["a"]); assertEquals(result.value, null); }); @@ -345,7 +346,7 @@ dbTest("atomic mutation type=delete no exists", async (db) => { const res = await db.atomic() .mutate({ key: ["a"], type: "delete" }) .commit(); - assert(res); + assert(res.ok); const result = await db.get(["a"]); assertEquals(result.value, null); }); @@ -355,7 +356,7 @@ dbTest("atomic mutation type=sum", async (db) => { const res = await db.atomic() .mutate({ key: ["a"], value: new Deno.KvU64(1n), type: "sum" }) .commit(); - assert(res); + assert(res.ok); const result = await db.get(["a"]); assertEquals(result.value, new Deno.KvU64(11n)); }); @@ -364,7 +365,7 @@ dbTest("atomic mutation type=sum no exists", async (db) => { const res = await db.atomic() .mutate({ key: ["a"], value: new Deno.KvU64(1n), type: "sum" }) .commit(); - assert(res); + assert(res.ok); const result = await db.get(["a"]); assert(result.value); assertEquals(result.value, new Deno.KvU64(1n)); @@ -375,7 +376,7 @@ dbTest("atomic mutation type=sum wrap around", async (db) => { const res = await db.atomic() .mutate({ key: ["a"], value: new Deno.KvU64(10n), type: "sum" }) .commit(); - assert(res); + assert(res.ok); const result = await db.get(["a"]); assertEquals(result.value, new Deno.KvU64(9n)); @@ -423,7 +424,7 @@ dbTest("atomic mutation type=min", async (db) => { const res = await db.atomic() .mutate({ key: ["a"], value: new Deno.KvU64(5n), type: "min" }) .commit(); - assert(res); + assert(res.ok); const result = await db.get(["a"]); assertEquals(result.value, new Deno.KvU64(5n)); @@ -439,7 +440,7 @@ dbTest("atomic mutation type=min no exists", async (db) => { const res = await db.atomic() .mutate({ key: ["a"], value: new Deno.KvU64(1n), type: "min" }) .commit(); - assert(res); + assert(res.ok); const result = await db.get(["a"]); assert(result.value); assertEquals(result.value, new Deno.KvU64(1n)); @@ -477,7 +478,7 @@ dbTest("atomic mutation type=max", async (db) => { const res = await db.atomic() .mutate({ key: ["a"], value: new Deno.KvU64(5n), type: "max" }) .commit(); - assert(res); + assert(res.ok); const result = await db.get(["a"]); assertEquals(result.value, new Deno.KvU64(10n)); @@ -493,7 +494,7 @@ dbTest("atomic mutation type=max no exists", async (db) => { const res = await db.atomic() .mutate({ key: ["a"], value: new Deno.KvU64(1n), type: "max" }) .commit(); - assert(res); + assert(res.ok); const result = await db.get(["a"]); assert(result.value); assertEquals(result.value, new Deno.KvU64(1n)); diff --git a/cli/tsc/dts/lib.deno.unstable.d.ts b/cli/tsc/dts/lib.deno.unstable.d.ts index f169e0254b..8613da2ab5 100644 --- a/cli/tsc/dts/lib.deno.unstable.d.ts +++ b/cli/tsc/dts/lib.deno.unstable.d.ts @@ -1545,6 +1545,10 @@ declare namespace Deno { * relative significance of the types can be found in documentation for the * {@linkcode Deno.KvKeyPart} type. * + * Keys have a maximum size of 2048 bytes serialized. If the size of the key + * exceeds this limit, an error will be thrown on the operation that this key + * was passed to. + * * @category KV */ export type KvKey = readonly KvKeyPart[]; @@ -1758,10 +1762,16 @@ declare namespace Deno { /** @category KV */ export interface KvCommitResult { + ok: true; /** The versionstamp of the value committed to KV. */ versionstamp: string; } + /** @category KV */ + export interface KvCommitError { + ok: false; + } + /** **UNSTABLE**: New API, yet to be vetted. * * A check to perform as part of a {@linkcode Deno.AtomicOperation}. The check @@ -1803,11 +1813,13 @@ declare namespace Deno { * * The `commit` method of an atomic operation returns a value indicating * whether checks passed and mutations were performed. If the operation failed - * because of a failed check, the return value will be `null`. If the + * because of a failed check, the return value will be a + * {@linkcode Deno.KvCommitError} with an `ok: false` property. If the * operation failed for any other reason (storage error, invalid value, etc.), * an exception will be thrown. If the operation succeeded, the return value - * will be a {@linkcode Deno.KvCommitResult} object containing the - * versionstamp of the value committed to KV. + * will be a {@linkcode Deno.KvCommitResult} object with a `ok: true` property + * and the versionstamp of the value committed to KV. + * * @category KV */ @@ -1857,17 +1869,19 @@ declare namespace Deno { /** * Commit the operation to the KV store. Returns a value indicating whether * checks passed and mutations were performed. If the operation failed - * because of a failed check, the return value will be `null`. If the - * operation failed for any other reason (storage error, invalid value, - * etc.), an exception will be thrown. If the operation succeeded, the - * return value will be a {@linkcode Deno.KvCommitResult} object containing - * the versionstamp of the value committed to KV. + * because of a failed check, the return value will be a {@linkcode + * Deno.KvCommitError} with an `ok: false` property. If the operation failed + * for any other reason (storage error, invalid value, etc.), an exception + * will be thrown. If the operation succeeded, the return value will be a + * {@linkcode Deno.KvCommitResult} object with a `ok: true` property and the + * versionstamp of the value committed to KV. * - * If the commit returns `null`, one may create a new atomic operation with - * updated checks and mutations and attempt to commit it again. See the note - * on optimistic locking in the documentation for {@linkcode Deno.AtomicOperation}. + * If the commit returns `ok: false`, one may create a new atomic operation + * with updated checks and mutations and attempt to commit it again. See the + * note on optimistic locking in the documentation for + * {@linkcode Deno.AtomicOperation}. */ - commit(): Promise; + commit(): Promise; } /** **UNSTABLE**: New API, yet to be vetted. @@ -1901,7 +1915,8 @@ declare namespace Deno { * maximum length of 64 KiB after serialization. Serialization of both keys * and values is somewhat opaque, but one can usually assume that the * serialization of any value is about the same length as the resulting string - * of a JSON serialization of that same value. + * of a JSON serialization of that same value. If theses limits are exceeded, + * an exception will be thrown. * * @category KV */ diff --git a/ext/kv/01_db.ts b/ext/kv/01_db.ts index da29a09521..0dd6ba83a2 100644 --- a/ext/kv/01_db.ts +++ b/ext/kv/01_db.ts @@ -116,7 +116,7 @@ class Kv { [], ); if (versionstamp === null) throw new TypeError("Failed to set value"); - return { versionstamp }; + return { ok: true, versionstamp }; } async delete(key: Deno.KvKey) { @@ -266,7 +266,7 @@ class AtomicOperation { return this; } - async commit(): Promise { + async commit(): Promise { const versionstamp = await core.opAsync( "op_kv_atomic_write", this.#rid, @@ -274,8 +274,8 @@ class AtomicOperation { this.#mutations, [], // TODO(@losfair): enqueue ); - if (versionstamp === null) return null; - return { versionstamp }; + if (versionstamp === null) return { ok: false }; + return { ok: true, versionstamp }; } then() {