refactor: use ULID for notifications functionality (denoland#483)

Changes:
1. Notifications use ULIDs for their IDs.
2. Removes the `notifications` index from KV, as it seems unnecessary.
3. Removed the `createdAt` property from notifications.
4. Removes `newNotificationProps()`, as it now seems unnecessary.

Closes denoland#473
Towards denoland#470
CC @lino-levan (this might be a good reference to learn more about
ULIDs)

deno.json

@@ -4,6 +4,7 @@
     "init:stripe": "deno run --allow-read --allow-env --allow-net tools/init_stripe.ts",
     "db:dump": "deno run --allow-read --allow-env --unstable tools/dump_kv.ts",
     "db:seed": "deno run --allow-read --allow-env --allow-net --unstable tools/seed_submissions.ts",
+    "db:migrate": "deno run --allow-read --allow-env --allow-net --unstable tasks/db_migrate.ts",
     "db:reset": "deno run --allow-read --allow-env --unstable tools/reset_kv.ts",
     "start": "deno run --unstable -A --watch=static/,routes/ dev.ts",
     "test": "KV_PATH=:memory: deno test -A --unstable --parallel --coverage=./cov --doc",
@@ -30,7 +31,7 @@
     "@preact/signals-core": "https://esm.sh/*@preact/signals-core@1.2.3",
     "twind-preset-tailwind/": "https://esm.sh/@twind/preset-tailwind@1.1.4/",
     "twind-preset-ext": "https://esm.sh/@twind/preset-ext@1.0.7/",
-    "std/": "https://deno.land/std@0.188.0/",
+    "std/": "https://raw.githubusercontent.com/lino-levan/deno_std/feat-ulid/",
     "stripe": "./stripe.ts",
     "feed": "https://esm.sh/feed@4.2.2",
     "kv_oauth": "https://deno.land/x/deno_kv_oauth@v0.6.1/mod.ts",

islands/NotificationsList.tsx

@@ -5,6 +5,7 @@ import type { Notification } from "@/utils/db.ts";
 import { LINK_STYLES } from "@/utils/constants.ts";
 import { timeAgo } from "@/utils/display.ts";
 import { fetchValues } from "@/utils/islands.ts";
+import { decodeTime } from "std/ulid/mod.ts";
 
 function NotificationSummary(props: Notification) {
   return (
@@ -14,7 +15,7 @@ function NotificationSummary(props: Notification) {
           <strong>New {props.type}!</strong>
         </span>
         <span class="text-gray-500">
-          {" " + timeAgo(new Date(props.createdAt))}
+          {" " + timeAgo(new Date(decodeTime(props.id)))}
         </span>
         <br />
         <span>{props.text}</span>

routes/api/items/[id]/vote.ts

@@ -6,9 +6,9 @@ import {
   createVote,
   deleteVote,
   getItem,
-  newNotificationProps,
   newVoteProps,
 } from "@/utils/db.ts";
+import { monotonicUlid } from "std/ulid/mod.ts";
 import { errors } from "std/http/http_errors.ts";
 
 export const handler: Handlers<undefined, State> = {
@@ -28,11 +28,11 @@ export const handler: Handlers<undefined, State> = {
 
     if (item.userLogin !== sessionUser.login) {
       await createNotification({
+        id: monotonicUlid(),
         userLogin: item.userLogin,
         type: "vote",
         text: `${sessionUser.login} upvoted your post: ${item.title}`,
         originUrl: `/items/${itemId}`,
-        ...newNotificationProps(),
       });
     }
 

routes/api/me/notifications.ts

@@ -1,13 +1,13 @@
 // Copyright 2023 the Deno authors. All rights reserved. MIT license.
 import type { Handlers } from "$fresh/server.ts";
-import { collectValues, listNotificationsByUser } from "@/utils/db.ts";
+import { collectValues, listNotifications } from "@/utils/db.ts";
 import { getCursor } from "@/utils/http.ts";
 import { SignedInState } from "@/middleware/session.ts";
 
 export const handler: Handlers<undefined, SignedInState> = {
   async GET(req, ctx) {
     const url = new URL(req.url);
-    const iter = listNotificationsByUser(ctx.state.sessionUser.login, {
+    const iter = listNotifications(ctx.state.sessionUser.login, {
       cursor: getCursor(url),
       limit: 10,
       // Newest to oldest

routes/items/[id].tsx

@@ -9,13 +9,13 @@ import {
   getAreVotedByUser,
   getItem,
   newCommentProps,
-  newNotificationProps,
   Notification,
 } from "@/utils/db.ts";
 import { redirect } from "@/utils/http.ts";
 import Head from "@/components/Head.tsx";
 import { assertSignedIn, State } from "@/middleware/session.ts";
 import CommentsList from "@/islands/CommentsList.tsx";
+import { monotonicUlid } from "std/ulid/mod.ts";
 import { errors } from "std/http/http_errors.ts";
 
 /** @todo Move to `POST /api/comments` */
@@ -46,11 +46,11 @@ export const handler: Handlers<unknown, State> = {
 
     if (item.userLogin !== sessionUser.login) {
       const notification: Notification = {
+        id: monotonicUlid(),
         userLogin: item.userLogin,
         type: "comment",
         text: `${sessionUser.login} commented on your post: ${item.title}`,
         originUrl: `/items/${itemId}`,
-        ...newNotificationProps(),
       };
       await createNotification(notification);
     }

routes/notifications/[id].ts

@@ -2,13 +2,16 @@
 import type { RouteContext } from "$fresh/server.ts";
 import { deleteNotification, getNotification } from "@/utils/db.ts";
 import { redirect } from "@/utils/http.ts";
-import { SignedInState } from "@/middleware/session.ts";
+import type { SignedInState } from "@/middleware/session.ts";
 
 export default async function NotificationsNotificationPage(
   _req: Request,
   ctx: RouteContext<undefined, SignedInState>,
 ) {
-  const notification = await getNotification(ctx.params.id);
+  const notification = await getNotification({
+    id: ctx.params.id,
+    userLogin: ctx.state.sessionUser.login,
+  });
   if (notification === null) return await ctx.renderNotFound();
 
   await deleteNotification(notification);

tasks/db_migrate.ts

@@ -0,0 +1,36 @@
+// Copyright 2023 the Deno authors. All rights reserved. MIT license.
+/**
+ * This script is used to perform migration jobs on the database. These jobs
+ * can be performed on remote KV instances using
+ * {@link https://github.com/denoland/deno/tree/main/ext/kv#kv-connect|KV Connect}.
+ *
+ * This script will continually change over time for database migrations, as
+ * required.
+ *
+ * @example
+ * ```bash
+ * deno task db:migrate
+ * ```
+ */
+import { kv, type Notification } from "@/utils/db.ts";
+
+interface OldNotification extends Notification {
+  createdAt: Date;
+}
+
+if (!confirm("WARNING: The database will be migrated. Continue?")) Deno.exit();
+
+const promises = [];
+
+const iter1 = kv.list<OldNotification>({ prefix: ["notifications"] });
+for await (const { key } of iter1) promises.push(kv.delete(key));
+
+const iter2 = kv.list<OldNotification>({ prefix: ["notifications_by_user"] });
+for await (const { key } of iter2) promises.push(kv.delete(key));
+
+const results = await Promise.allSettled(promises);
+results.forEach((result) => {
+  if (result.status === "rejected") console.error(result);
+});
+
+kv.close();

utils/db.ts

@@ -144,83 +144,78 @@ export function listItemsByTime(options?: Deno.KvListOptions) {
 
 // Notification
 export interface Notification {
+  // Uses ULID
+  id: string;
   userLogin: string;
   type: string;
   text: string;
   originUrl: string;
-  // The below properties can be automatically generated upon notification creation
-  id: string;
-  createdAt: Date;
-}
-
-export function newNotificationProps(): Pick<Item, "id" | "createdAt"> {
-  return {
-    id: crypto.randomUUID(),
-    createdAt: new Date(),
-  };
 }
 
 /**
  * Creates a new notification in KV. Throws if the item already exists in one of the indexes.
  *
  * @example
  * ```ts
- * import { newNotificationProps, createNotification } from "@/utils/db.ts";
+ * import { createNotification } from "@/utils/db.ts";
+ * import { monotonicUlid } from "std/ulid/mod.ts";
  *
  * await createNotification({
+ *   id: monotonicUlid(),
  *   userLogin: "john_doe",
  *   type: "example-type",
  *   text: "Hello, world!",
  *   originUrl: "https://hunt.deno.land",
- *   ...newNotificationProps(),
  * });
  * ```
  */
 export async function createNotification(notification: Notification) {
-  const notificationsKey = ["notifications", notification.id];
-  const notificationsByUserKey = [
+  const key = [
     "notifications_by_user",
     notification.userLogin,
-    notification.createdAt.getTime(),
     notification.id,
   ];
 
   const res = await kv.atomic()
-    .check({ key: notificationsKey, versionstamp: null })
-    .check({ key: notificationsByUserKey, versionstamp: null })
-    .set(notificationsKey, notification)
-    .set(notificationsByUserKey, notification)
+    .check({ key: key, versionstamp: null })
+    .set(key, notification)
     .commit();
 
-  if (!res.ok) {
-    throw new Error(`Failed to create notification: ${notification}`);
-  }
+  if (!res.ok) throw new Error("Failed to create notification");
 }
 
-export async function deleteNotification(notification: Notification) {
-  const notificationsKey = ["notifications", notification.id];
-  const notificationsByUserKey = [
+export async function deleteNotification(
+  notification: Pick<Notification, "id" | "userLogin">,
+) {
+  const key = [
     "notifications_by_user",
     notification.userLogin,
-    notification.createdAt.getTime(),
     notification.id,
   ];
+  const notificationRes = await kv.get<Notification>(key);
+  if (notificationRes.value === null) {
+    throw new Deno.errors.NotFound("Notification not found");
+  }
 
   const res = await kv.atomic()
-    .delete(notificationsKey)
-    .delete(notificationsByUserKey)
+    .check(notificationRes)
+    .delete(key)
     .commit();
 
-  if (!res.ok) {
-    throw new Error(`Failed to delete notification: ${notification}`);
-  }
+  if (!res.ok) throw new Error("Failed to delete notification");
 }
 
-export async function getNotification(id: string) {
-  return await getValue<Notification>(["notifications", id]);
+export async function getNotification(
+  notification: Pick<Notification, "id" | "userLogin">,
+) {
+  return await getValue<Notification>([
+    "notifications_by_user",
+    notification.userLogin,
+    notification.id,
+  ]);
 }
 
-export function listNotificationsByUser(
+export function listNotifications(
   userLogin: string,
   options?: Deno.KvListOptions,
 ) {

utils/db_test.ts

@@ -29,10 +29,9 @@ import {
   listItemsByTime,
   listItemsByUser,
   listItemsVotedByUser,
-  listNotificationsByUser,
+  listNotifications,
   newCommentProps,
   newItemProps,
-  newNotificationProps,
   newUserProps,
   newVoteProps,
   Notification,
@@ -46,6 +45,7 @@ import {
   assertRejects,
 } from "std/testing/asserts.ts";
 import { DAY } from "std/datetime/constants.ts";
+import { monotonicUlid } from "std/ulid/mod.ts";
 
 export function genNewComment(): Comment {
   return {
@@ -76,11 +76,11 @@ export function genNewUser(): User {
 
 export function genNewNotification(): Notification {
   return {
+    id: monotonicUlid(),
     userLogin: crypto.randomUUID(),
     type: crypto.randomUUID(),
     text: crypto.randomUUID(),
     originUrl: crypto.randomUUID(),
-    ...newNotificationProps(),
   };
 }
 
@@ -245,40 +245,46 @@ Deno.test("[db] getDatesSince()", () => {
   ]);
 });
 
-Deno.test("[db] newNotificationProps()", () => {
-  const notificationProps = newNotificationProps();
-  assert(notificationProps.createdAt.getTime() <= Date.now());
-  assertEquals(typeof notificationProps.id, "string");
-});
-
-Deno.test("[db] (get/create/delete)Notification()", async () => {
-  const notification = genNewNotification();
-
-  assertEquals(await getNotification(notification.id), null);
-
-  await createNotification(notification);
-  await assertRejects(async () => await createNotification(notification));
-  assertEquals(await getNotification(notification.id), notification);
-
-  await deleteNotification(notification);
-  assertEquals(await getItem(notification.id), null);
-});
-
-Deno.test("[db] getNotificationsByUser()", async () => {
+Deno.test("[db] notifications", async () => {
   const userLogin = crypto.randomUUID();
   const notification1 = { ...genNewNotification(), userLogin };
   const notification2 = { ...genNewNotification(), userLogin };
 
-  assertEquals(await collectValues(listNotificationsByUser(userLogin)), []);
+  assertEquals(await getNotification(notification1), null);
+  assertEquals(await getNotification(notification2), null);
+  await assertRejects(
+    async () => await deleteNotification(notification1),
+    "Notification not found",
+  );
+  assertEquals(await collectValues(listNotifications(userLogin)), []);
   assertEquals(await ifUserHasNotifications(userLogin), false);
 
   await createNotification(notification1);
   await createNotification(notification2);
-  assertArrayIncludes(await collectValues(listNotificationsByUser(userLogin)), [
+  await assertRejects(
+    async () => await createNotification(notification1),
+    "Failed to create notification",
+  );
+
+  await assertEquals(await getNotification(notification1), notification1);
+  assertEquals(await getNotification(notification2), notification2);
+  assertEquals(await collectValues(listNotifications(userLogin)), [
     notification1,
     notification2,
   ]);
   assertEquals(await ifUserHasNotifications(userLogin), true);
+
+  await deleteNotification(notification1);
+  await deleteNotification(notification2);
+  await assertRejects(
+    async () => await deleteNotification(notification1),
+    "Failed to delete notification",
+  );
+
+  assertEquals(await getNotification(notification1), null);
+  assertEquals(await getNotification(notification2), null);
+  assertEquals(await collectValues(listNotifications(userLogin)), []);
+  assertEquals(await ifUserHasNotifications(userLogin), false);
 });
 
 Deno.test("[db] compareScore()", () => {