refactor: use ULIDs for items (denoland#514)

Please pay attention to the migration script, as it's responsible for migrating all items and votes. I've also changed all instances of `monotonicUlid()` to `ulid()` at there appears to be a bug that prevents `monotonicUlid()` from respecting the `seedTime` parameter. The migration can be simulated as follows: 1. Switch to **main** branch. 2. Run `deno run db:reset`. 3. Run `deno run db:seed`. 4. Run `deno task start` and navigate to `http://localhost:8000`. 6. Vote some items. 7. Note the voted items and the order of the items feed. 8. Switch to **ulid-items** branch. 9. Run `deno run db:migrate`. 10. Run `deno task start` and navigate to `http://localhost:8000`. 11. Check that the voted items and the order of the items feed are identical to those previously noted. Closes denoland#475 Closes denoland#476
dreamcatcher-tech · Sep 8, 2023 · 23be24c · 23be24c
1 parent 4109f6c
commit 23be24c
Show file tree

Hide file tree

Showing 8 changed files with 140 additions and 120 deletions.
diff --git a/components/ItemSummary.tsx b/components/ItemSummary.tsx
@@ -2,6 +2,7 @@
 import VoteButton from "@/islands/VoteButton.tsx";
 import type { Item } from "@/utils/db.ts";
 import UserPostedAt from "./UserPostedAt.tsx";
+import { decodeTime } from "std/ulid/mod.ts";
 
 export interface ItemSummaryProps {
   item: Item;
@@ -33,7 +34,7 @@ export default function ItemSummary(props: ItemSummaryProps) {
         </p>
         <UserPostedAt
           userLogin={props.item.userLogin}
-          createdAt={props.item.createdAt}
+          createdAt={new Date(decodeTime(props.item.id))}
         />
       </div>
     </div>

diff --git a/routes/api/comments.ts b/routes/api/comments.ts
@@ -4,7 +4,7 @@ import { errors } from "std/http/http_errors.ts";
 import { createComment, createNotification, getItem } from "@/utils/db.ts";
 import { redirect } from "@/utils/http.ts";
 import { assertSignedIn, State } from "@/middleware/session.ts";
-import { monotonicUlid } from "std/ulid/mod.ts";
+import { ulid } from "std/ulid/mod.ts";
 
 export const handler: Handlers<undefined, State> = {
   async POST(req, ctx) {
@@ -25,15 +25,15 @@ export const handler: Handlers<undefined, State> = {
 
     const { sessionUser } = ctx.state;
     await createComment({
-      id: monotonicUlid(),
+      id: ulid(),
       userLogin: sessionUser.login,
       itemId: itemId,
       text,
     });
 
     if (item.userLogin !== sessionUser.login) {
       await createNotification({
-        id: monotonicUlid(),
+        id: ulid(),
         userLogin: item.userLogin,
         type: "comment",
         text: `${sessionUser.login} commented on your post: ${item.title}`,

diff --git a/routes/api/items/[id]/vote.ts b/routes/api/items/[id]/vote.ts
@@ -8,7 +8,7 @@ import {
   getItem,
   newVoteProps,
 } from "@/utils/db.ts";
-import { monotonicUlid } from "std/ulid/mod.ts";
+import { ulid } from "std/ulid/mod.ts";
 import { errors } from "std/http/http_errors.ts";
 
 export const handler: Handlers<undefined, State> = {
@@ -28,7 +28,7 @@ export const handler: Handlers<undefined, State> = {
 
     if (item.userLogin !== sessionUser.login) {
       await createNotification({
-        id: monotonicUlid(),
+        id: ulid(),
         userLogin: item.userLogin,
         type: "vote",
         text: `${sessionUser.login} upvoted your post: ${item.title}`,

diff --git a/routes/api/items/index.ts b/routes/api/items/index.ts
@@ -1,16 +1,17 @@
-import { collectValues, listItemsByTime } from "@/utils/db.ts";
+import { collectValues, listItems } from "@/utils/db.ts";
 import { getCursor } from "@/utils/http.ts";
 import { type Handlers } from "$fresh/server.ts";
-import { createItem, type Item, newItemProps } from "@/utils/db.ts";
+import { createItem, type Item } from "@/utils/db.ts";
 import { redirect } from "@/utils/http.ts";
 import { assertSignedIn, State } from "@/middleware/session.ts";
 import { errors } from "std/http/http_errors.ts";
+import { ulid } from "std/ulid/mod.ts";
 
 // Copyright 2023 the Deno authors. All rights reserved. MIT license.
 export const handler: Handlers<undefined, State> = {
   async GET(req) {
     const url = new URL(req.url);
-    const iter = listItemsByTime({
+    const iter = listItems({
       cursor: getCursor(url),
       limit: 10,
       reverse: true,
@@ -33,10 +34,11 @@ export const handler: Handlers<undefined, State> = {
     }
 
     const item: Item = {
+      id: ulid(),
       userLogin: ctx.state.sessionUser.login,
       title,
       url,
-      ...newItemProps(),
+      score: 0,
     };
     await createItem(item);
     return redirect("/items/" + item.id);

diff --git a/tasks/db_migrate.ts b/tasks/db_migrate.ts
@@ -12,32 +12,60 @@
  * deno task db:migrate
  * ```
  */
-import { type Comment, createComment, kv } from "@/utils/db.ts";
-import { monotonicUlid } from "std/ulid/mod.ts";
+import {
+  createItem,
+  createVote,
+  deleteVote,
+  type Item,
+  kv,
+  User,
+} from "@/utils/db.ts";
+import { ulid } from "std/ulid/mod.ts";
 
-interface OldComment extends Comment {
+interface OldItem extends Item {
   createdAt: Date;
 }
 
 if (!confirm("WARNING: The database will be migrated. Continue?")) Deno.exit();
 
-const promises = [];
-
-const iter = kv.list<OldComment>({ prefix: ["comments_by_item"] });
-for await (const { key, value } of iter) {
-  if (!value.createdAt) continue;
-  promises.push(kv.delete(key));
-  promises.push(createComment({
-    id: monotonicUlid(value.createdAt.getTime()),
-    userLogin: value.userLogin,
-    itemId: value.itemId,
-    text: value.text,
-  }));
+const iter1 = kv.list<OldItem>({ prefix: ["items"] });
+for await (const oldItemEntry of iter1) {
+  if (oldItemEntry.value.createdAt) {
+    const newItem = {
+      id: ulid(new Date(oldItemEntry.value.createdAt).getTime()),
+      userLogin: oldItemEntry.value.userLogin,
+      url: oldItemEntry.value.url,
+      title: oldItemEntry.value.title,
+      score: oldItemEntry.value.score,
+    };
+    await createItem(newItem);
+    const iter2 = kv.list<User>({
+      prefix: ["users_voted_for_item", oldItemEntry.value.id],
+    });
+    for await (const userEntry of iter2) {
+      await deleteVote({
+        itemId: oldItemEntry.value.id,
+        userLogin: userEntry.value.login,
+      });
+      await deleteVote({
+        itemId: newItem.id,
+        userLogin: userEntry.value.login,
+      });
+      await createVote({
+        itemId: newItem.id,
+        userLogin: userEntry.value.login,
+        createdAt: new Date(),
+      });
+    }
+    await kv.delete(oldItemEntry.key);
+  }
 }
 
-const results = await Promise.allSettled(promises);
-results.forEach((result) => {
-  if (result.status === "rejected") console.error(result);
-});
+const iter3 = kv.list<OldItem>({ prefix: ["items_by_user"] });
+const promises = [];
+for await (const { key, value } of iter3) {
+  if (value.createdAt) promises.push(kv.delete(key));
+}
+await Promise.all(promises);
 
 kv.close();
diff --git a/tasks/db_seed.ts b/tasks/db_seed.ts
@@ -1,11 +1,7 @@
 // Copyright 2023 the Deno authors. All rights reserved. MIT license.
 // Description: Seeds the kv db with Hacker News stories
-import {
-  createItem,
-  createUser,
-  newItemProps,
-  newUserProps,
-} from "@/utils/db.ts";
+import { createItem, createUser, newUserProps } from "@/utils/db.ts";
+import { ulid } from "std/ulid/mod.ts";
 
 // Reference: https://github.com/HackerNews/API
 const API_BASE_URL = `https://hacker-news.firebaseio.com/v0`;
@@ -36,7 +32,7 @@ const stories = await Promise.all(
   storiesResponses.map((r) => r.json()),
 ) as Story[];
 const items = stories.map(({ by: userLogin, title, url, score, time }) => ({
-  ...newItemProps(),
+  id: ulid(),
   userLogin,
   title,
   url,

diff --git a/utils/db.ts b/utils/db.ts
@@ -1,4 +1,5 @@
 // Copyright 2023 the Deno authors. All rights reserved. MIT license.
+import { decodeTime } from "std/ulid/mod.ts";
 import { chunk } from "std/collections/chunk.ts";
 
 const KV_PATH_KEY = "KV_PATH";
@@ -54,87 +55,88 @@ export function formatDate(date: Date) {
 
 // Item
 export interface Item {
+  // Uses ULID
+  id: string;
   userLogin: string;
   title: string;
   url: string;
-  // The below properties can be automatically generated upon item creation
-  id: string;
-  createdAt: Date;
   score: number;
 }
 
-export function newItemProps(): Pick<Item, "id" | "score" | "createdAt"> {
-  return {
-    id: crypto.randomUUID(),
-    score: 0,
-    createdAt: new Date(),
-  };
-}
-
 /**
  * Creates a new item in KV. Throws if the item already exists in one of the indexes.
  *
  * @example
  * ```ts
- * import { newItemProps, createItem } from "@/utils/db.ts";
+ * import { createItem } from "@/utils/db.ts";
+ * import { ulid } from "std/ulid/mod.ts";
  *
  * await createItem({
+ *   id: ulid(),
  *   userLogin: "john_doe",
  *   title: "example-title",
  *   url: "https://example.com",
- *   ...newItemProps(),
+ *   score: 0,
  * });
  * ```
  */
 export async function createItem(item: Item) {
   const itemsKey = ["items", item.id];
-  const itemsByTimeKey = ["items_by_time", item.createdAt.getTime(), item.id];
   const itemsByUserKey = ["items_by_user", item.userLogin, item.id];
-  const itemsCountKey = ["items_count", formatDate(item.createdAt)];
+  const itemsCountKey = [
+    "items_count",
+    formatDate(new Date(decodeTime(item.id))),
+  ];
 
   const res = await kv.atomic()
     .check({ key: itemsKey, versionstamp: null })
-    .check({ key: itemsByTimeKey, versionstamp: null })
     .check({ key: itemsByUserKey, versionstamp: null })
     .set(itemsKey, item)
-    .set(itemsByTimeKey, item)
     .set(itemsByUserKey, item)
     .sum(itemsCountKey, 1n)
     .commit();
 
-  if (!res.ok) throw new Error(`Failed to create item: ${item}`);
+  if (!res.ok) throw new Error("Failed to create item");
 }
 
 export async function deleteItem(item: Item) {
   const itemsKey = ["items", item.id];
-  const itemsByTimeKey = ["items_by_time", item.createdAt.getTime(), item.id];
   const itemsByUserKey = ["items_by_user", item.userLogin, item.id];
+  const [itemsRes, itemsByUserRes] = await kv.getMany<Item[]>([
+    itemsKey,
+    itemsByUserKey,
+  ]);
+  if (itemsRes.value === null) throw new Deno.errors.NotFound("Item not found");
+  if (itemsByUserRes.value === null) {
+    throw new Deno.errors.NotFound("Item by user not found");
+  }
 
   const res = await kv.atomic()
+    .check(itemsRes)
+    .check(itemsByUserRes)
     .delete(itemsKey)
-    .delete(itemsByTimeKey)
     .delete(itemsByUserKey)
     .commit();
 
-  if (!res.ok) throw new Error(`Failed to delete item: ${item}`);
+  if (!res.ok) throw new Error("Failed to delete item");
 }
 
 export async function getItem(id: string) {
   const res = await kv.get<Item>(["items", id]);
   return res.value;
 }
 
+export function listItems(options?: Deno.KvListOptions) {
+  return kv.list<Item>({ prefix: ["items"] }, options);
+}
+
 export function listItemsByUser(
   userLogin: string,
   options?: Deno.KvListOptions,
 ) {
   return kv.list<Item>({ prefix: ["items_by_user", userLogin] }, options);
 }
 
-export function listItemsByTime(options?: Deno.KvListOptions) {
-  return kv.list<Item>({ prefix: ["items_by_time"] }, options);
-}
-
 // Notification
 export interface Notification {
   // Uses ULID
@@ -151,10 +153,10 @@ export interface Notification {
  * @example
  * ```ts
  * import { createNotification } from "@/utils/db.ts";
- * import { monotonicUlid } from "std/ulid/mod.ts";
+ * import { ulid } from "std/ulid/mod.ts";
  *
  * await createNotification({
- *   id: monotonicUlid(),
+ *   id: ulid(),
  *   userLogin: "john_doe",
  *   type: "example-type",
  *   text: "Hello, world!",
@@ -309,7 +311,6 @@ export async function createVote(vote: Vote) {
     vote.itemId,
     vote.userLogin,
   ];
-  const itemByTimeKey = ["items_by_time", item.createdAt.getTime(), item.id];
   const itemByUserKey = ["items_by_user", item.userLogin, item.id];
   const votesCountKey = ["votes_count", formatDate(vote.createdAt)];
 
@@ -321,7 +322,6 @@ export async function createVote(vote: Vote) {
     .check({ key: itemVotedByUserKey, versionstamp: null })
     .check({ key: userVotedForItemKey, versionstamp: null })
     .set(itemKey, item)
-    .set(itemByTimeKey, item)
     .set(itemByUserKey, item)
     .set(itemVotedByUserKey, item)
     .set(userVotedForItemKey, user)
@@ -352,14 +352,14 @@ export async function deleteVote(vote: Omit<Vote, "createdAt">) {
   const user = userRes.value;
   if (item === null) throw new Deno.errors.NotFound("Item not found");
   if (user === null) throw new Deno.errors.NotFound("User not found");
-  if (itemVotedByUserRes.value === null) {
+  /** @todo Uncomment after ULID-items migration */
+  /* if (itemVotedByUserRes.value === null) {
     throw new Deno.errors.NotFound("Item voted by user not found");
   }
   if (userVotedForItemRes.value === null) {
     throw new Deno.errors.NotFound("User voted for item not found");
-  }
+  } */
 
-  const itemByTimeKey = ["items_by_time", item.createdAt.getTime(), item.id];
   const itemByUserKey = ["items_by_user", item.userLogin, item.id];
 
   item.score--;
@@ -370,7 +370,6 @@ export async function deleteVote(vote: Omit<Vote, "createdAt">) {
     .check(itemVotedByUserRes)
     .check(userVotedForItemRes)
     .set(itemKey, item)
-    .set(itemByTimeKey, item)
     .set(itemByUserKey, item)
     .delete(itemVotedByUserKey)
     .delete(userVotedForItemKey)