From aa1cd520690633722313633ce52813b4cb32f545 Mon Sep 17 00:00:00 2001
From: simeng-li <simeng@silverhand.io>
Date: Mon, 20 Feb 2023 11:37:39 +0800
Subject: [PATCH] refactor(core): remove duplicate findEntityById query before
 delete

remveo duplication findEntityById query before delete
---
 packages/core/src/routes/admin-user.test.ts  | 6 +++---
 packages/core/src/routes/admin-user.ts       | 2 --
 packages/core/src/routes/application.test.ts | 6 +++---
 packages/core/src/routes/application.ts      | 1 -
 packages/core/src/routes/resource.test.ts    | 7 +++++++
 packages/core/src/routes/resource.ts         | 1 -
 6 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/packages/core/src/routes/admin-user.test.ts b/packages/core/src/routes/admin-user.test.ts
index 2ebe72f6f..7979e5325 100644
--- a/packages/core/src/routes/admin-user.test.ts
+++ b/packages/core/src/routes/admin-user.test.ts
@@ -374,17 +374,17 @@ describe('adminUserRoutes', () => {
 
   it('DELETE /users/:userId should throw if user cannot be found', async () => {
     const notExistedUserId = 'notExistedUserId';
-    const mockedFindUserById = findUserById as jest.Mock;
-    mockedFindUserById.mockImplementationOnce((userId) => {
+
+    deleteUserById.mockImplementationOnce((userId) => {
       if (userId === notExistedUserId) {
         throw new Error(' ');
       }
     });
+
     await expect(userRequest.delete(`/users/${notExistedUserId}`)).resolves.toHaveProperty(
       'status',
       500
     );
-    expect(deleteUserById).not.toHaveBeenCalled();
   });
 
   it('DELETE /users/:userId/identities/:target should throw if user cannot be found', async () => {
diff --git a/packages/core/src/routes/admin-user.ts b/packages/core/src/routes/admin-user.ts
index 006794423..faded7c90 100644
--- a/packages/core/src/routes/admin-user.ts
+++ b/packages/core/src/routes/admin-user.ts
@@ -277,8 +277,6 @@ export default function adminUserRoutes<T extends AuthedRouter>(
         throw new RequestError('user.cannot_delete_self');
       }
 
-      await findUserById(userId);
-
       await deleteUserById(userId);
 
       ctx.status = 204;
diff --git a/packages/core/src/routes/application.test.ts b/packages/core/src/routes/application.test.ts
index fb7f4fa25..32bc9b093 100644
--- a/packages/core/src/routes/application.test.ts
+++ b/packages/core/src/routes/application.test.ts
@@ -9,6 +9,7 @@ const { jest } = import.meta;
 const { mockEsmWithActual } = createMockUtils(jest);
 
 const findApplicationById = jest.fn(async () => mockApplication);
+const deleteApplicationById = jest.fn();
 
 await mockEsmWithActual('@logto/core-kit', () => ({
   // eslint-disable-next-line unicorn/consistent-function-scoping
@@ -21,7 +22,7 @@ const tenantContext = new MockTenant(undefined, {
     findTotalNumberOfApplications: jest.fn(async () => ({ count: 10 })),
     findAllApplications: jest.fn(async () => [mockApplication]),
     findApplicationById,
-    deleteApplicationById: jest.fn(),
+    deleteApplicationById,
     insertApplication: jest.fn(
       async (body: CreateApplication): Promise<Application> => ({
         ...mockApplication,
@@ -224,8 +225,7 @@ describe('application route', () => {
   });
 
   it('DELETE /applications/:applicationId should throw if application not found', async () => {
-    const mockFindApplicationById = findApplicationById as jest.Mock;
-    mockFindApplicationById.mockRejectedValueOnce(new Error(' '));
+    deleteApplicationById.mockRejectedValueOnce(new Error(' '));
 
     await expect(applicationRequest.delete('/applications/foo')).resolves.toHaveProperty(
       'status',
diff --git a/packages/core/src/routes/application.ts b/packages/core/src/routes/application.ts
index 8998637d7..6e48557dd 100644
--- a/packages/core/src/routes/application.ts
+++ b/packages/core/src/routes/application.ts
@@ -132,7 +132,6 @@ export default function applicationRoutes<T extends AuthedRouter>(
     async (ctx, next) => {
       const { id } = ctx.guard.params;
       // Note: will need delete cascade when application is joint with other tables
-      await findApplicationById(id);
       await deleteApplicationById(id);
       ctx.status = 204;
 
diff --git a/packages/core/src/routes/resource.test.ts b/packages/core/src/routes/resource.test.ts
index 1829f3aac..9614c3292 100644
--- a/packages/core/src/routes/resource.test.ts
+++ b/packages/core/src/routes/resource.test.ts
@@ -150,6 +150,13 @@ describe('resource routes', () => {
     await expect(resourceRequest.delete('/resources/foo')).resolves.toHaveProperty('status', 204);
   });
 
+  it('DELETE /resources/:id should throw with invalid id', async () => {
+    const { deleteResourceById } = resources;
+    deleteResourceById.mockRejectedValueOnce(new Error('not found'));
+
+    await expect(resourceRequest.delete('/resources/foo')).resolves.toHaveProperty('status', 500);
+  });
+
   it('GET /resources/:id/scopes', async () => {
     const response = await resourceRequest.get('/resources/foo/scopes');
     expect(response.status).toEqual(200);
diff --git a/packages/core/src/routes/resource.ts b/packages/core/src/routes/resource.ts
index 42a80bd36..b503fc3c9 100644
--- a/packages/core/src/routes/resource.ts
+++ b/packages/core/src/routes/resource.ts
@@ -127,7 +127,6 @@ export default function resourceRoutes<T extends AuthedRouter>(
     koaGuard({ params: object({ id: string().min(1) }) }),
     async (ctx, next) => {
       const { id } = ctx.guard.params;
-      await findResourceById(id);
       await deleteResourceById(id);
       ctx.status = 204;