Cleanup

CHANGELOG.md

@@ -1,4 +1,11 @@
-## Unrelesed
+## 0.15.11
+
+- Remove option doLock from `.sync()`
+- Remove unused code from `key.ts`
+- Add benchmark for getting data that does not exist
+- Rename internal function `toAbsolutePath` to `toNormalizedAbsolutePath` and
+  clarify code with additional comments
+- Adds various missing test to reduce the risk for regression bugs
 
 ## 0.15.10
 

deno.json

@@ -1,6 +1,6 @@
 {
   "name": "@cross/kv",
-  "version": "0.15.10",
+  "version": "0.15.11",
   "exports": {
     ".": "./mod.ts",
     "./cli": "./src/cli/mod.ts"

src/lib/key.ts

@@ -67,20 +67,13 @@ export class KVKeyInstance {
   private key: KVQuery | KVKey;
   private isQuery: boolean;
   public byteLength?: number;
-  hasData: boolean = false;
   constructor(
-    key: KVQuery | KVKey | Uint8Array | DataView,
+    key: KVQuery | KVKey | DataView,
     isQuery: boolean = false,
     validate: boolean = true,
   ) {
-    if (key instanceof Uint8Array) {
-      this.key = this.fromUint8Array(
-        new DataView(key.buffer, key.byteOffset, key.byteLength),
-      );
-      this.hasData = true;
-    } else if (key instanceof DataView) {
+    if (key instanceof DataView) {
       this.key = this.fromUint8Array(key);
-      this.hasData = true;
     } else {
       this.key = key;
     }
@@ -134,6 +127,7 @@ export class KVKeyInstance {
       keyArray.set(bytes, keyOffset);
       keyOffset += bytes.length;
     }
+
     return keyArray;
   }
 
@@ -225,6 +219,15 @@ export class KVKeyInstance {
             'Ranges must have only "from" and/or "to" keys',
           );
         }
+
+        // Check for not mixing number from with string to and vice versa
+        if (
+          (typeof element.from === "number" &&
+            typeof element.to === "string") ||
+          (typeof element.from === "string" && typeof element.to === "number")
+        ) {
+          throw new TypeError("Cannot mix string and number in ranges");
+        }
       }
 
       if (

src/lib/kv.ts

@@ -291,26 +291,20 @@ export class KV extends EventEmitter {
    * - Automatically run on adding data
    * - Can be manually triggered for full consistency before data retrieval (iterate(), listAll(), get())
    *
-   * @param doLock - (Optional) Locks the database before synchronization. Defaults to true. Always true unless called internally.
-   *
    * @emits sync - Emits an event with the synchronization result:
    *   - `result`: "ready" | "blocked" | "success" | "ledgerInvalidated" | "error"
    *   - `error`: Error object (if an error occurred) or null
    *
    * @throws {Error} If an unexpected error occurs during synchronization.
    */
-  public async sync(doLock = false): Promise<KVSyncResult> {
+  public async sync(): Promise<KVSyncResult> {
     // Throw if database isn't open
     this.ensureOpen();
 
     // Synchronization Logic (with lock if needed)
     let result: KVSyncResult["result"] = "ready";
     let error: Error | null = null;
-    let lockSucceeded = false; // Keeping track as we only want to unlock the database later, if the locking operation succeeded
     try {
-      if (doLock) await this.ledger?.lock();
-      lockSucceeded = true;
-
       const newTransactions = await this.ledger?.sync(this.disableIndex);
 
       if (newTransactions === null) { // Ledger invalidated
@@ -336,7 +330,6 @@ export class KV extends EventEmitter {
       result = "error";
       error = new Error("Error during ledger sync", { cause: syncError });
     } finally {
-      if (doLock && lockSucceeded) await this.ledger?.unlock();
       // @ts-ignore .emit exists
       this.emit("sync", { result, error });
     }

src/lib/ledger.ts

@@ -2,7 +2,7 @@ import {
   ensureFile,
   rawOpen,
   readAtPosition,
-  toAbsolutePath,
+  toNormalizedAbsolutePath,
   writeAtPosition,
 } from "./utils/file.ts";
 import {
@@ -71,7 +71,7 @@ export class KVLedger {
   };
 
   constructor(filePath: string, maxCacheSizeMBytes: number) {
-    this.dataPath = toAbsolutePath(filePath);
+    this.dataPath = toNormalizedAbsolutePath(filePath);
     this.cache = new KVLedgerCache(maxCacheSizeMBytes * 1024 * 1024);
   }
 

src/lib/transaction.ts

@@ -133,8 +133,9 @@ export class KVTransaction {
     this.operation = operation;
     this.timestamp = timestamp;
     if (value) {
-      this.data = new Uint8Array(encode(value));
-      this.hash = await sha1(this.data!);
+      const valueData = new Uint8Array(encode(value));
+      this.data = valueData;
+      this.hash = await sha1(valueData);
     }
   }
 

src/lib/utils/file.ts

@@ -3,11 +3,17 @@ import { CurrentRuntime, Runtime } from "@cross/runtime";
 import { cwd, isDir, isFile, mkdir } from "@cross/fs";
 import { dirname, isAbsolute, join, resolve } from "@std/path";
 
-export function toAbsolutePath(filename: string): string {
+export function toNormalizedAbsolutePath(filename: string): string {
   let filePath;
   if (isAbsolute(filename)) {
+    // Even if the input is already an absolute path, it might not be normalized:
+    // - It could contain ".." or "." segments that need to be resolved.
+    // - It might have extra separators that need to be cleaned up.
+    // - It might not use the correct separator for the current OS.
     filePath = resolve(filename);
   } else {
+    // If the input is relative, combine it with the current working directory
+    // and then resolve to get a fully normalized absolute path.
     filePath = resolve(join(cwd(), filename));
   }
   return filePath;

test/key.test.ts

@@ -212,3 +212,98 @@ test("KVKeyInstance: parse error on invalid range format", () => {
     KVKeyInstance.parse("users.>=abc.<=123", false);
   }, TypeError);
 });
+
+test("KVKeyInstance: Throws if trying to use something other than an array for a key", () => {
+  assertThrows(
+    () => new KVKeyInstance("users.data.user123" as unknown as KVKey),
+    TypeError,
+    "Key must be an array",
+  );
+});
+
+test("KVKeyInstance: Throws if trying to use something other than a string or number for a key element", () => {
+  assertThrows(
+    // @ts-ignore expected to be wrong
+    () => new KVKeyInstance(["users", null]),
+    TypeError,
+    "Key ranges are only allowed in queries",
+  );
+});
+
+test("KVKeyInstance: Throws if trying to use an object with extra properties for a key element", () => {
+  assertThrows(
+    // @ts-ignore extected to be wrong
+    () => new KVKeyInstance(["users", { from: 100, extra: "value" }], true),
+    TypeError,
+    "Ranges must have only",
+  );
+});
+
+test("KVKeyInstance: Throws if trying to mix strings and numbers in a range for a key element", () => {
+  assertThrows(
+    () => new KVKeyInstance(["users", { from: "abc", to: 123 }], true),
+    TypeError,
+    "Cannot mix string and number in ranges",
+  );
+});
+
+test("KVKeyInstance: Throws if trying to use an object with invalid range values for a key element", () => {
+  assertThrows(
+    () => new KVKeyInstance(["users", { from: 123, to: "abc" }], true, true),
+    TypeError,
+    "Cannot mix string and number in ranges",
+  );
+});
+
+test("KVKeyInstance: throws when parsing a key with empty elements", () => {
+  assertThrows(
+    () => KVKeyInstance.parse("users..profile", false),
+    TypeError,
+    "Key ranges are only allowed in queries",
+  );
+});
+
+test("KVKeyInstance: stringify and parse with empty elements", () => {
+  // Empty object represents an empty element
+  const queryWithEmpty: KVQuery = ["users", {}, "profile"];
+  const queryWithRangeAndEmpty: KVQuery = ["data", { from: 10, to: 20 }, {}];
+
+  const parsedKey = KVKeyInstance.parse("users..profile", true);
+  assertEquals(parsedKey, queryWithEmpty);
+
+  const queryInstance = new KVKeyInstance(queryWithRangeAndEmpty, true);
+  assertEquals(queryInstance.stringify(), "data.>=#10<=#20.");
+
+  const parsedQuery = KVKeyInstance.parse("data.>=#10<=#20.", true);
+  assertEquals(parsedQuery, queryWithRangeAndEmpty);
+});
+
+test("KVKeyInstance: parse with leading dots should throw", () => {
+  assertThrows(
+    () => KVKeyInstance.parse(`.data.>=a<=z`, false),
+    TypeError,
+    "Ranges are not allowed in keys.",
+  );
+});
+
+test("KVKeyInstance: stringify and parse mixed-type keys", () => {
+  const mixedKey: KVKey = ["users", 123, "profile"];
+  const instance = new KVKeyInstance(mixedKey);
+
+  const stringified = instance.stringify();
+  assertEquals(stringified, "users.#123.profile");
+
+  const parsed = KVKeyInstance.parse(stringified, false);
+  assertEquals(parsed, mixedKey);
+});
+
+test("KVKeyInstance: toUint8Array and fromUint8Array roundtrip", () => {
+  const key: KVKey = ["users", "john_doe", 42];
+  const instance = new KVKeyInstance(key);
+  const uint8Array = instance.toUint8Array();
+
+  const dataView = new DataView(uint8Array.buffer);
+  const newKeyInstance = new KVKeyInstance(dataView);
+
+  assertEquals(newKeyInstance.get(), key);
+});

test/kv.bench.ts

@@ -18,16 +18,15 @@ async function setupDenoKV() {
 
 const crossStore = await setupKV();
 const denoStore = await setupDenoKV();
-let crossIterTransaction = 0;
-let denoIterTransaction = 0;
 
 await Deno.bench("cross_kv_set_100_atomic", async () => {
   await crossStore.beginTransaction();
   for (let i = 0; i < 100; i++) {
-    await crossStore.set(["testKey", crossIterTransaction++], {
+    const randomUUID = crypto.randomUUID();
+    await crossStore.set(["testKey", randomUUID], {
       data: {
         data: "testData",
-        i: crossIterTransaction,
+        i: randomUUID,
         more: {
           "test": "data",
           "with1": new Date(),
@@ -47,10 +46,11 @@ await Deno.bench("cross_kv_set_100_atomic", async () => {
 await Deno.bench("deno_kv_set_100_atomic", async () => {
   const at = denoStore.atomic();
   for (let i = 0; i < 100; i++) {
-    at.set(["testKey", denoIterTransaction++], {
+    const randomUUID = crypto.randomUUID();
+    at.set(["testKey", randomUUID], {
       data: {
         data: "testData",
-        i: denoIterTransaction,
+        i: randomUUID,
         more: {
           "test": "data",
           "with1": new Date(),
@@ -108,9 +108,17 @@ await Deno.bench("deno_kv_set", async () => {
 });
 
 await Deno.bench("cross_kv_get", async () => {
-  await crossStore.get(["testKey", 3]);
+  await crossStore.get(["testKey2", 3]);
 });
 
 await Deno.bench("deno_kv_get", async () => {
-  await denoStore.get(["testKey", 3]);
+  await denoStore.get(["testKey2", 3]);
+});
+
+await Deno.bench("cross_kv_get_nonexisting", async () => {
+  await crossStore.get(["testKey2", "eh"]);
+});
+
+await Deno.bench("deno_kv_get_nonexisting", async () => {
+  await denoStore.get(["testKey2", "eh"]);
 });

test/kv.test.ts

@@ -427,7 +427,7 @@ test("KV: watch functionality - basic matching", async () => {
   });
 
   await kvStore.set(watchedKey, { name: "Alice", age: 30 });
-  await kvStore.sync(true); // Manual sync to trigger the watch callback
+  await kvStore.sync();
 
   assertEquals(receivedTransaction!.key, watchedKey);
   assertEquals(receivedTransaction!.data, { name: "Alice", age: 30 });
@@ -451,7 +451,7 @@ test("KV: watch functionality - recursive matching", async () => {
   await kvStore.set(["users", "user1"], "Alice");
   await kvStore.set(["users", "user2"], "Bob");
   await kvStore.set(["data", "other"], "Not watched");
-  await kvStore.sync(true); // Not needed, but trigger to ensure no duplicate calls occurr
+  await kvStore.sync();
 
   assertEquals(receivedTransactions.length, 2);
   assertEquals(receivedTransactions[0].data, "Alice");
@@ -537,7 +537,7 @@ test("KV: watch functionality - no match", async () => {
 
   // Add data under a different key
   await kvStore.set(["data", "something"], "else");
-  await kvStore.sync(true);
+  await kvStore.sync();
 
   assertEquals(callbackCalled, false, "Callback should not have been called");