Merge pull request #3150 from logto-io/simeng-log-4166-remove-unnecessary-querying-before

refactor(core): remove duplicate findEntityById query before delete

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 () => {

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;

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',

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;
 

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);

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;