From fd7d5a4ab18d137831b9f81f5fc2c509ae2b367c Mon Sep 17 00:00:00 2001
From: Gao Sun <gao@silverhand.io>
Date: Thu, 12 Oct 2023 14:56:34 +0800
Subject: [PATCH] refactor(core): fix id guard issue for post api

---
 packages/core/src/utils/SchemaRouter.test.ts | 44 +++++++++++---------
 packages/core/src/utils/SchemaRouter.ts      |  5 ++-
 2 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/packages/core/src/utils/SchemaRouter.test.ts b/packages/core/src/utils/SchemaRouter.test.ts
index 38ad0fb53..fc721cecd 100644
--- a/packages/core/src/utils/SchemaRouter.test.ts
+++ b/packages/core/src/utils/SchemaRouter.test.ts
@@ -10,27 +10,33 @@ import { createRequester, createTestPool } from './test-utils.js';
 const { jest } = import.meta;
 
 type CreateSchema = {
-  id?: string;
+  id: string;
+  name?: string;
 };
 
 type Schema = {
   id: string;
+  name: string;
 };
 
 describe('SchemaRouter', () => {
-  const schema: GeneratedSchema<'id', CreateSchema, Schema> = {
-    table: 'test_table',
+  const schema: GeneratedSchema<'id' | 'name', CreateSchema, Schema> = {
+    table: 'test_tables',
     tableSingular: 'test_table',
     fields: {
       id: 'id',
+      name: 'name',
     },
-    fieldKeys: ['id'],
-    createGuard: z.object({ id: z.string().optional() }),
-    guard: z.object({ id: z.string() }),
-    updateGuard: z.object({ id: z.string().optional() }),
+    fieldKeys: ['id', 'name'],
+    createGuard: z.object({ id: z.string(), name: z.string().optional() }),
+    guard: z.object({ id: z.string(), name: z.string() }),
+    updateGuard: z.object({ id: z.string(), name: z.string() }).partial(),
   };
-  const entities = [{ id: 'test' }, { id: 'test2' }] as const satisfies readonly Schema[];
-  const actions: SchemaActions<'id', CreateSchema, Schema> = {
+  const entities = [
+    { id: '1', name: 'test' },
+    { id: '2', name: 'test2' },
+  ] as const satisfies readonly Schema[];
+  const actions: SchemaActions<'name', CreateSchema, Schema> = {
     queries: new SchemaQueries(createTestPool(), schema),
     get: jest.fn().mockResolvedValue([entities.length, entities]),
     getById: jest.fn(async (id) => {
@@ -40,8 +46,8 @@ describe('SchemaRouter', () => {
       }
       return entity;
     }),
-    post: jest.fn(async () => ({ id: 'test_new' })),
-    patchById: jest.fn(async (id, data) => ({ id, ...data })),
+    post: jest.fn(async () => ({ id: 'new', name: 'test_new' })),
+    patchById: jest.fn(async (id, data) => ({ id, name: 'name_patch_default', ...data })),
     deleteById: jest.fn(),
   };
   const schemaRouter = new SchemaRouter(schema, actions);
@@ -76,11 +82,11 @@ describe('SchemaRouter', () => {
       const response = await request.post(baseRoute).send({});
 
       expect(actions.post).toHaveBeenCalledWith({});
-      expect(response.body).toStrictEqual({ id: 'test_new' });
+      expect(response.body).toStrictEqual({ id: 'new', name: 'test_new' });
     });
 
     it('should throw with invalid input body', async () => {
-      const response = await request.post(baseRoute).send({ id: 1 });
+      const response = await request.post(baseRoute).send({ name: 1 });
 
       expect(response.status).toEqual(400);
     });
@@ -88,15 +94,15 @@ describe('SchemaRouter', () => {
 
   describe('getById', () => {
     it('should be able to get an entity by id', async () => {
-      const response = await request.get(`${baseRoute}/test`);
+      const response = await request.get(`${baseRoute}/1`);
 
-      expect(actions.getById).toHaveBeenCalledWith('test');
+      expect(actions.getById).toHaveBeenCalledWith('1');
       expect(response.body).toStrictEqual(entities[0]);
     });
 
     // This test case is actually nice-to-have. It's not required for the router to work.
     it('should throw with invalid id', async () => {
-      const response = await request.get(`${baseRoute}/2`);
+      const response = await request.get(`${baseRoute}/foo`);
 
       expect(response.status).toEqual(404);
     });
@@ -104,10 +110,10 @@ describe('SchemaRouter', () => {
 
   describe('patchById', () => {
     it('should be able to patch an entity by id', async () => {
-      const response = await request.patch(`${baseRoute}/test`).send({ id: 'test_new' });
+      const response = await request.patch(`${baseRoute}/test`).send({ name: 'test_new' });
 
-      expect(actions.patchById).toHaveBeenCalledWith('test', { id: 'test_new' });
-      expect(response.body).toStrictEqual({ id: 'test_new' });
+      expect(actions.patchById).toHaveBeenCalledWith('test', { name: 'test_new' });
+      expect(response.body).toStrictEqual({ id: 'test', name: 'test_new' });
       expect(response.status).toEqual(200);
     });
   });
diff --git a/packages/core/src/utils/SchemaRouter.ts b/packages/core/src/utils/SchemaRouter.ts
index 2777d5565..a0c10adae 100644
--- a/packages/core/src/utils/SchemaRouter.ts
+++ b/packages/core/src/utils/SchemaRouter.ts
@@ -1,4 +1,4 @@
-import { type SchemaLike, type GeneratedSchema } from '@logto/schemas';
+import { type SchemaLike, type GeneratedSchema, type Guard } from '@logto/schemas';
 import { generateStandardId, type OmitAutoSetFields } from '@logto/shared';
 import Router, { type IRouterParamContext } from 'koa-router';
 import { z } from 'zod';
@@ -119,7 +119,8 @@ export default class SchemaRouter<
     this.post(
       '/',
       koaGuard({
-        body: schema.createGuard,
+        // eslint-disable-next-line no-restricted-syntax -- `.omit()` doesn't play well for generic types
+        body: schema.createGuard.omit({ id: true }) as Guard<Omit<CreateSchema, 'id'>>,
         response: schema.guard,
         status: [201, 422],
       }),