Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resolve package.json dependency, and update prisma 4 to 5 #132

Merged
merged 6 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3,547 changes: 2,381 additions & 1,166 deletions package-lock.json

Large diffs are not rendered by default.

14 changes: 9 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
"author": "",
"private": true,
"license": "UNLICENSED",
"engines": {
"node": ">=18.20.4",
"npm": ">=10.7.0"
},
"scripts": {
"prebuild": "rimraf dist",
"build": "nest build --config src/bootstrap/nest-cli.json",
Expand Down Expand Up @@ -46,7 +50,7 @@
"@nestjs/passport": "^9.0.3",
"@nestjs/platform-express": "^9.0.0",
"@nestjs/swagger": "^7.1.1",
"@prisma/client": "4.16.0",
"@prisma/client": "^5.18.0",
"@types/cookie-parser": "^1.4.3",
"@types/express-session": "^1.17.7",
"@types/morgan": "^1.9.4",
Expand All @@ -68,7 +72,6 @@
"nestjs-cls": "^4.4.0",
"passport": "^0.6.0",
"passport-jwt": "^4.0.1",
"prisma": "4.16.0",
"reflect-metadata": "^0.1.13",
"rimraf": "^3.0.2",
"rxjs": "^7.2.0"
Expand All @@ -81,7 +84,7 @@
"@types/express": "^4.17.13",
"@types/jest": "28.1.4",
"@types/node": "^16.18.23",
"@types/supertest": "^2.0.11",
"@types/supertest": "^6.0.2",
"@typescript-eslint/eslint-plugin": "^5.0.0",
"@typescript-eslint/parser": "^5.0.0",
"cross-env": "^7.0.3",
Expand All @@ -91,13 +94,14 @@
"jest": "28.1.2",
"lint-staged": "^13.2.3",
"prettier": "^2.3.2",
"prisma": "^5.18.0",
"source-map-support": "^0.5.20",
"supertest": "^6.1.3",
"supertest": "^7.0.0",
"ts-jest": "^28.0.5",
"ts-loader": "^9.2.3",
"ts-node": "^10.9.1",
"tsconfig-paths": "4.0.0",
"typescript": "^4.3.5"
"typescript": "^5.0.0"
},
"lint-staged": {
"*.ts": "eslint --fix",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 패치에 대한 간략한 리뷰입니다.

개선 사항 및 버그 리스크:

  1. Node.js 및 npm 버전 지정:

    • "engines" 필드를 추가하여 Node.js와 npm의 최소 버전을 명시한 것은 좋은 조치입니다. 이는 프로젝트의 호환성을 보장하는 데 유용합니다.
  2. 의존성 업데이트:

    • @prisma/clientprisma 의존성을 5.x 버전으로 업데이트한 것은 새로운 기능과 버그 수정을 활용할 수 있게 해줍니다. 그러나 이러한 주요 버전 변경이 호환성에 영향을 미칠 수 있으므로, 엔진 테스트 후 배포하는 것이 좋습니다.
    • supertest를 7.x로 업데이트한 것도 마찬가지입니다. 새로운 버전의 변경 사항을 확인하고 필요시 코드 수정이 필요할 수 있습니다.
  3. 타입스크립트 버전 변경:

    • typescript를 5.0으로 업데이트한 것은 현대적인 언어 기능을 사용하는 데 도움이 됩니다. 기존 코드가 변경된 API와 호환되는지 확인하는 테스트가 필요합니다.
  4. 문서화:

    • 변경된 의존성과 그에 관련된 이유를 문서화하면 다른 개발자들이 이해하는 데 도움이 됩니다.
  5. 테스트 커버리지:

    • 패치 후에 코드를 충분히 테스트해야 합니다. 특히 주요 라이브러리의 버전이 변경되었으므로, 예상치 못한 문제를 방지하기 위해 기존 테스트 케이스를 점검하는 것이 중요합니다.
  6. 레거시 의존성 제어:

    • 의존성을 업데이트한 후 이전 버전의 레거시 문제가 발생하지 않도록 주의해야 합니다. 각 라이브러리의 문서를 참고하여 마이그레이션 가이드를 검토하는 것이 좋습니다.

위 사항들을 고려하여 갱신된 코드를 잘 관리하고 테스트하세요!

Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
// import { AuthGuard, MockAuthGuard } from '../../common/guards/auth.guard'
import morgan = require('morgan');

let cachedServer: Server;

Check warning on line 12 in src/bootstrap/bootstrap.ts

View workflow job for this annotation

GitHub Actions / Lint

'cachedServer' is defined but never used

async function bootstrap() {
const { NODE_ENV } = process.env;

Check warning on line 15 in src/bootstrap/bootstrap.ts

View workflow job for this annotation

GitHub Actions / Lint

'NODE_ENV' is assigned a value but never used

const app = await NestFactory.create(AppModule);

Expand Down Expand Up @@ -60,8 +60,8 @@
}),
);

const prismaService = app.get(PrismaService);

Check warning on line 63 in src/bootstrap/bootstrap.ts

View workflow job for this annotation

GitHub Actions / Lint

'prismaService' is assigned a value but never used
await prismaService.enableShutdownHooks(app);
// await prismaService.enableShutdownHooks(app);
return app.listen(8000);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치에 대한 간단한 코드 리뷰입니다.

버그 위험

  1. 비정상적인 종료 처리: enableShutdownHooks가 주석 처리되면 애플리케이션에서 비정상 종료 시 데이터베이스 연결을 올바르게 종료하지 않을 수 있습니다. 이로 인해 리소스 누수나 데이터 손상이 발생할 수 있습니다.

개선 제안

  1. 주석의 의도 명시: 주석 처리된 코드는 그 이유를 명확히 설명하는 주석을 추가하는 것이 좋습니다. 예: // Shutdown hooks disabled for testing purposes.
  2. 로깅 추가: 코드 변경으로 인한 영향을 이해하기 위해 필요한 경우 로그를 추가하여 애플리케이션이 정상적으로 작동하는지 확인할 수 있습니다.

결론적으로, 이 변화를 고수하는 것이 아닌 원래 코드로 복구하는 것을 추천합니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰를 진행하겠습니다. 다음은 수정 사항에 대한 간단한 분석입니다.

  1. 불필요한 주석: const prismaService = app.get(PrismaService);에 대한 주석 처리가 의미가 불분명합니다. 필요하지 않다면 삭제하거나, 주석을 통해 해당 코드의 의도나 이유를 설명하는 것이 좋습니다.

  2. enableShutdownHooks의 변경:

    • prismaService.enableShutdownHooks(app); 대신 app.enableShutdownHooks();를 사용한 것으로 보입니다. 이 변경이 의도된 것이라면, Prisma 서비스의 종료 후크가 제대로 동작하는지 확인해야 합니다. 이로 인해 리소스 관리에 문제가 발생할 수 있습니다.
  3. process 임포트: import * as process from 'node:process';가 추가되었으나, 코드 안에서 실제로 어디에 사용되는지 명시되어 있지 않습니다. 사용되지 않는다면 이 줄은 삭제하는 것이 좋습니다.

  4. 서버 포트 하드코딩: app.listen(8000);에서 포트 번호가 하드코딩되어 있습니다. 설정 파일이나 환경 변수를 사용하는 방법으로 유연성을 높이는 것이 좋습니다.

  5. 에러 핸들링: bootstrap 함수 내에 에러 처리가 없습니다. 서버 시작 시 오류가 발생할 경우 적절한 핸들링을 추가하여 안정성을 높여야 합니다.

  6. 문서화 및 가독성: 전체적으로 함수와 코드 블록에 대한 문서화가 부족합니다. 어떤 역할을 하는지 설명하는 주석을 추가하면 이해도를 높일 수 있습니다.

이러한 점들을 고려하여 코드를 개선하는 것이 좋습니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 패치를 검토한 결과, 다음과 같은 검토 사항을 제안합니다:

  1. 의존성 제거: ServerPrismaService를 import 했으나 사용하지 않아서 코드가 깔끔해졌습니다. 불필요한 주석도 제거하면 좋겠습니다.

  2. 에러 핸들링: 애플리케이션 부트스트랩 과정에서 비동기 작업이 실패할 경우를 대비하여 에러 핸들링을 추가하는 것이 좋습니다. try-catch 블록을 사용해 에러를 적절히 처리할 수 있습니다.

  3. 환경 변수 세팅: NODE_ENV 변수가 사용되지 않는 대신, 환경 변수를 사용하는 설정이나 논리를 추가한다면 더 유용할 수 있습니다. 해당 변수를 어떻게 사용할지 검토해볼 필요가 있습니다.

  4. 포트 번호 상수화: 8000이라는 숫자를 코드 내에 하드코딩하기보다 상수로 정의하거나 설정 파일에서 읽어오는 것이 좋습니다.

  5. 로그 및 모니터링: Morgan을 import했지만 현재 사용되지 않으므로 로그미들웨어를 추가하고 로깅 기능을 활성화하는 것을 고려할 수 있습니다.

위 사항들을 참고하여 코드 품질을 개선하세요.

Expand Down
18 changes: 10 additions & 8 deletions src/prisma/middleware/prisma.timetable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,18 @@ export class TimetableMiddleware
await prisma.subject_lecture.update({
where: { id: id },
data: {
num_people: await prisma.timetable_timetable.count({
distinct: ['user_id'],
where: {
timetable_timetable_lectures: {
some: {
lecture_id: id,
num_people: (
await prisma.timetable_timetable.findMany({
distinct: ['user_id'],
where: {
timetable_timetable_lectures: {
some: {
lecture_id: id,
},
},
},
},
}),
})
).length,
},
});
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 패치 리뷰:

버그 위험

  1. 비동기 함수 처리: await prisma.timetable_timetable.findMany의 결과로 반환된 배열의 길이를 계산하는 구문이 문제가 될 수 있습니다. findMany는 만약 데이터가 없거나 잘못된 경우 예상 외의 결과를 초래할 수 있습니다.

  2. 성능 문제: findMany로 모든 관련 레코드를 가져온 후 길이를 확인하는 방식은 데이터베이스에 부담을 줄 수 있습니다. 대량의 데이터가 존재할 경우 성능 저하가 발생할 수 있습니다.

개선 제안

  1. 카운트 쿼리 사용: count 메서드를 사용하는 기존 접근 방식이 더 효율적입니다. num_people을 계산하기 위해 findMany를 사용할 필요가 없습니다. 원래 코드처럼 count를 사용하는 것이 좋습니다.

  2. 코드 가독성: 잘 정돈된 코드를 유지하기 위해, 중첩된 객체 구조에서 필요한 값만 남기는 쪽으로 리팩토링하는 것이 좋습니다.

  3. 주석 추가: 패치한 코드의 의도를 설명하는 주석을 추가하면 다른 개발자가 이해하는 데 도움이 됩니다.

결론적으로, 성능과 안정성을 고려하여 원래의 count를 사용하는 방식을 복원하는 것이 가장 바람직합니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 패치에 대한 리뷰는 다음과 같습니다:

  1. 버그 리스크:

    • prisma.timetable_timetable.findMany 메소드가 반환하는 수가 직접적으로 length를 통해 num_people에 할당되고 있습니다. 이 경우, 반환된 데이터가 배열이 아닐 경우 오류가 발생할 수 있습니다. 따라서 findMany의 결과가 항상 배열인지 확인해야 합니다.
  2. 성능:

    • findMany 호출은 모든 사용자를 가져오는 방식인데, 실제로 필요한 것은 고유한 사용자 수입니다. count를 사용하는 것이 더 효율적일 수 있는 경우도 고려해보세요.
  3. 가독성 개선:

    • 코드를 더 간결하게 만들기 위해 중복된 where 조건을 함수로 추출하거나, 불필요한 중괄호 수준을 줄이는 것도 좋습니다.
  4. 예외 처리:

    • await 구문에서 오류가 발생할 수 있으므로 적절한 예외 처리를 추가하는 것이 좋습니다.

개선된 코드 예시는 다음과 같습니다:

await prisma.subject_lecture.update({
  where: { id: id },
  data: {
    num_people: (await prisma.timetable_timetable.count({
      distinct: ['user_id'],
      where: {
        timetable_timetable_lectures: {
          some: {
            lecture_id: id,
          },
        },
      },
    })) ?? 0,
  },
});

이렇게 하는 것이 성능과 가독성을 동시에 향상시킬 수 있습니다.

Expand Down
36 changes: 20 additions & 16 deletions src/prisma/middleware/prisma.timetablelecture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,16 +108,18 @@ export class TimetableLectureMiddleware
await prisma.subject_lecture.update({
where: { id: lectureId },
data: {
num_people: await prisma.timetable_timetable.count({
distinct: ['user_id'],
where: {
timetable_timetable_lectures: {
some: {
lecture_id: lectureId,
num_people: (
await prisma.timetable_timetable.findMany({
distinct: ['user_id'],
where: {
timetable_timetable_lectures: {
some: {
lecture_id: 16155,
},
},
},
},
}),
})
).length,
},
});
});
Expand All @@ -134,16 +136,18 @@ export class TimetableLectureMiddleware
await prisma.subject_lecture.update({
where: { id: id },
data: {
num_people: await prisma.timetable_timetable.count({
distinct: ['user_id'],
where: {
timetable_timetable_lectures: {
some: {
lecture_id: id,
num_people: (
await prisma.timetable_timetable.findMany({
distinct: ['user_id'],
where: {
timetable_timetable_lectures: {
some: {
lecture_id: 16155,
},
},
},
},
}),
})
).length,
},
});
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰를 통해 몇 가지 버그 리스크와 개선 사항을 제안합니다.

  1. 하드코딩된 lecture_id:

    • lecture_id가 하드코딩되어 있습니다(예: lecture_id: 16155). 이는 코드의 유연성을 떨어뜨리고, 향후 변경 시 전체 코드를 수정해야 할 위험이 있습니다. 변수로 받아서 사용하는 것이 좋습니다.
  2. 성능 문제:

    • findMany로 사용자 수를 구하는 방식은 효율적이지 않을 수 있습니다. 만약 데이터베이스에서 매우 많은 레코드를 검색해야 한다면 성능 저하가 발생할 수 있습니다. 가능한 경우 count 쿼리를 사용하는 것이 더 나을 수 있습니다.
  3. 비동기 처리:

    • 비동기 호출 후 await을 사용하여 결과를 기다리고 있는데, 이러한 방식이 적절합니다. 하지만 이 과정에서 에러 핸들링이 누락되었으므로, try-catch 문으로 예외 처리를 추가하는 것이 필요합니다.
  4. 코드 중복 제거:

    • 두 개의 블록이 거의 동일하므로, 함수로 분리하여 코드 중복을 줄이는 것이 좋습니다. 이는 유지 관리성을 높입니다.
  5. 주석 및 문서화:

    • 각 주요 기능에 대해 주석을 추가하면 코드의 이해도를 높일 수 있습니다. 특히, 복잡한 비즈니스 로직이나 데이터 모델과 관련된 부분에 대한 설명이 필요합니다.

위의 포인트들을 고려하여 코드를 개선하면 더 안정적이고 유지 보수가 쉬운 소프트웨어를 만들 수 있을 것입니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰를 진행하겠습니다.

버그 리스크

  1. 성능 문제: findMany로 모든 사용자 ID를 조회한 후 length를 계산하는 방식은 데이터베이스에서 많은 데이터를 가져올 수 있습니다. 이는 성능 저하를 초래할 수 있습니다.
  2. 동시성 문제: 여러 사용자가 동시에 강의를 등록하거나 삭제하는 경우, num_people 수치가 정확하지 않을 수 있습니다. 트랜잭션을 고려해야 합니다.

개선 제안

  1. 카운트 최적화: 여전히 count() 메서드를 사용하는 것이 효율적입니다. count 구문으로 다시 변경하고, 해당 쿼리의 복잡성을 유지하세요.

    num_people: await prisma.timetable_timetable.count({
      distinct: ['user_id'],
      where: {
        timetable_timetable_lectures: {
          some: {
            lecture_id: lectureId,
          },
        },
      },
    }),
  2. 코드 중복 제거: 같은 코드 블록이 두 곳에 존재합니다. 이 부분을 함수로 분리하여 중복을 줄이면 가독성이 향상됩니다.

  3. 에러 핸들링: API 요청 시 에러 처리를 추가하여 안정성을 높이세요. 예를 들어, try-catch 블록을 이용할 수 있습니다.

  4. 비동기 처리 확인: await 키워드를 적절히 사용하고 있는지 확인하세요. 특히 의도한 비동기 동작이 제대로 작동하는지 점검이 필요합니다.

이러한 조정을 통해 코드의 효율성과 안정성을 높일 수 있을 것입니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치에 대한 간단한 코드 리뷰를 진행하겠습니다.

버그 리스크

  1. 비효율성: prisma.timetable_timetable.findMany를 호출하여 distinct된 사용자 수를 세는 방식은, 필요하지 않은 모든 데이터를 조회하므로 비효율적입니다. 대신 count 메서드를 사용해 직접 개수를 세는 것이 성능에 더 좋습니다.

  2. 응답 시간: 데이터의 양이 많을 경우, 이 방법은 응답 시간이 느려질 수 있습니다. 데이터베이스에서 대량의 결과를 가져오는 것보다 카운트 쿼리를 사용하는 것이 효율적입니다.

  3. 예외 처리 누락: await 구문 뒤에서 발생할 수 있는 예외에 대한 처리가 없습니다. 만약 DB 쿼리에서 오류가 발생하면, 적절한 예외 처리 없이는 어플리케이션이 크래시될 수 있습니다.

개선 제안

  1. percentUpdate: num_people을 업데이트할 때, findMany 대신 count를 사용하여 데이터베이스에 더 효율적으로 접근하도록 변경하는 것이 좋습니다.

    num_people: await prisma.timetable_timetable.count({
      distinct: ['user_id'],
      where: {
        timetable_timetable_lectures: {
          some: {
            lecture_id: lectureId,
          },
        },
      },
    }),
  2. 코드 중복 제거: num_people 업데이트 로직이 두 군데에서 중복됩니다. 이를 별도의 함수로 만들어 재사용성을 높이면 코드가 더 깔끔해집니다.

  3. 비동기 처리 고려: Promise가 실패할 경우를 대비해 try-catch 문을 사용하여 적절한 예외 처리 로직을 추가하는 것을 권장합니다.

이러한 점들을 고려하면 코드의 안정성과 효율성을 높일 수 있을 것입니다.

Expand Down
10 changes: 5 additions & 5 deletions src/prisma/prisma.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ export class PrismaService extends PrismaClient implements OnModuleInit {
Object.assign(this, extendedClient);
}

async enableShutdownHooks(app: INestApplication) {
this.$on('beforeExit', async () => {
await app.close();
});
}
// async enableShutdownHooks(app: INestApplication) {
// this.$on('beforeExit', async () => {
// await app.close();
// });
// }
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰:

  1. 주석 처리: enableShutdownHooks 메서드가 주석 처리되었습니다. 이 기능이 제거된 이유가 명확하지 않으므로, 추후 코드를 이해하는 데 혼란을 줄 수 있습니다. 해당 메서드의 필요성에 대해 논의되었는지 확인할 필요가 있습니다.

  2. 동기화 문제: 메서드가 비동기 방식으로 작성되어 있는데 주석 처리됨으로써 비동기 처리가 이루어지지 않는다면, 애플리케이션 종료 시 리소스 정리가 제대로 이루어지지 않을 위험이 있습니다. 만약 이 코드가 특정 사유로 인해 주석 처리된 것이라면 기록을 남기는 것이 좋습니다.

  3. 문서화: 코드 변경 사항이나 메서드가 비활성화된 이유에 대한 주석을 추가하면 유지보수에 도움을 줄 수 있습니다.

  4. 기능 모니터링: enableShutdownHooks가 중요하다면, alternative한 shutdown 제어 방법을 고려해야 할 수도 있습니다. 이 부분에 대한 대체 솔루션을 검토해 보세요.

전반적으로, 주석 처리된 코드의 필요성과 그에 따른 문서화가 필요합니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 패치에 대한 간단한 리뷰입니다.

코드 변경 사항

  • enableShutdownHooks 메서드가 삭제되었습니다.

위험 요소 및 개선 제안

  1. 종료 후킹 기능 제거:

    • enableShutdownHooks 메서드가 없어진 것은 애플리케이션의 종료 시 클린업을 수행하는 기능이 사라진 것입니다. 이는 리소스 누수나 비정상적인 종료 문제가 발생할 수 있습니다.
  2. 비동기 처리:

    • this.$on('beforeExit', async () => { await app.close(); });가 없는 상태에서, 비동기 호출 처리가 이루어지지 않게 되었습니다. 이로 인해 종료 시 필요한 작업이 생략될 수 있습니다.

개선 사항

  • 만약 shutdown hooks가 필요하다면 다시 추가하고, 메서드를 호출해야 할 순간에 적절히 사용할 수 있도록 설계해야 합니다.
  • 앱 종료 시 반드시 필요한 중요한 리소스 정리를 고려하십시오.

결과적으로, 이 패치로 인해 앱의 안정성이 영향을 받을 수 있으며, 관계된 리소스를 안전하게 관리하는 방법을 재검토해야 합니다.

15 changes: 0 additions & 15 deletions src/prisma/repositories/review.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,21 +257,6 @@ export class ReviewsRepository {
where: {
lecture: lectureFilter,
},
distinct: [
'id',
'course_id',
'lecture_id',
'content',
'grade',
'load',
'speech',
'writer_id',
'writer_label',
'updated_datetime',
'like',
'is_deleted',
'written_datetime',
],
});
return reviewsCount;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 패치를 검토한 결과, 다음과 같은 사항들을 지적할 수 있습니다.

  1. 중복 데이터 처리: distinct 배열을 제거함으로써 중복 데이터가 발생할 위험이 있습니다. 데이터베이스에서 고유한 레코드를 반환해야 하는 경우, 필요한 필드를 명시적으로 지정하는 것이 좋습니다.

  2. 성능 문제: distinct 필드가 없으면 불필요한 데이터가 많이 반환될 수 있어 성능 저하를 일으킬 수 있습니다. 특히 대규모 데이터셋을 다룰 때 이 점이 중요합니다.

  3. 기능 변경: 기존에 특정 필드의 중복을 방지하기 위한 로직이 있었을 가능성이 높습니다. 이 변경이 의도된 것인지 확인이 필요하며, 다른 부분에 부작용이 있을 수 있으니 유닛 테스트로 확인하는 것을 권장합니다.

  4. 코드 주석: 코드 변경의 이유나 배경에 대한 설명이 부족합니다. 향후 유지보수를 고려하여 변경 이유를 주석으로 남기는 것이 좋습니다.

  5. 테스트 케이스: 이 패치로 인해 영향을 받을 수 있는 기능에 대한 충분한 테스트가 이루어졌는지 검증해야 합니다. 새로운 로직이 비즈니스 요구사항에 맞는지 확인하기 위한 테스트 추가가 필요할 수 있습니다.

이러한 사항들을 고려하여 코드 개선 및 리펙토링을 진행하는 것이 좋겠습니다.

Expand Down
58 changes: 58 additions & 0 deletions test/prisma.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { INestApplication } from '@nestjs/common';
import { Test, TestingModule } from '@nestjs/testing';
import { AppModule } from '../src/app.module';
import { PrismaService } from '../src/prisma/prisma.service';
import { TranManager } from '../src/prisma/transactionManager';
import { CourseRepository } from '../src/prisma/repositories/course.repository';
import { ECourse } from '@src/common/entities/ECourse';
import { CoursesService } from '@src/modules/courses/courses.service';
import { session_userprofile } from '@prisma/client';

describe('AppController (e2e)', () => {
let app: INestApplication;

beforeEach(async () => {
const moduleFixture: TestingModule = await Test.createTestingModule({
imports: [AppModule],
}).compile();

app = moduleFixture.createNestApplication();
await app.init();
});

it('distinct count', async () => {
const prisma = app.get(PrismaService);
console.log('transaction test with tranManager');
const result = await prisma.timetable_timetable.groupBy({
by: ['user_id'],
where: {
timetable_timetable_lectures: {
some: {
lecture_id: 16155,
},
},
},
_count: {
user_id: true,
},
});
console.log(result);
}, 10000);

it('distinct count', async () => {
const prisma = app.get(PrismaService);
const result = (
await prisma.timetable_timetable.findMany({
distinct: ['user_id'],
where: {
timetable_timetable_lectures: {
some: {
lecture_id: 16155,
},
},
},
})
).length;
console.log(result);
});
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰를 진행하겠습니다.

코드 리뷰

  1. 중복 테스트 이름:

    • 두 개의 테스트가 모두 "distinct count"라는 동일한 이름을 사용하고 있습니다. 각 테스트에 고유한 이름을 부여하여 무엇을 테스트하는지 명확하게 하는 것이 좋습니다.
  2. 타입 안정성:

    • result 변수가 어떤 형태일지 구체적인 타입이 정의되지 않았습니다. TypeScript의 이점을 살려 타입을 명시적으로 지정하는 것이 좋습니다.
  3. 에러 처리:

    • 데이터베이스 요청에 대한 에러 처리가 없습니다. try-catch 블록을 추가하여 예외 상황에 적절히 대처할 수 있도록 해야 합니다.
  4. 테스트 환경 리셋:

    • beforeEach에서 매번 애플리케이션을 초기화하고 있지만, 필요한 경우 afterEach를 사용하여 리소스를 해제하거나 데이터베이스 상태를 원래대로 복원하는 것이 좋습니다.
  5. 로그 출력:

    • console.log를 사용하고 있는데, 테스트 결과가 진정으로 필요하지 않은 경우 이를 제거하거나, 실제 테스트 프레임워크의 로깅 기능을 사용하는 것이 좋습니다.
  6. 시간 제한 설정:

    • 첫 번째 테스트에는 시간 제한(10000ms)이 설정되어 있으나, 두 번째 테스트에서는 시간 제한이 없습니다. 일관성을 위해 모든 테스트에 대해 시간 제한을 고려해야 합니다.

개선 제안

  • 테스트 이름을 유니크하게 수정
  • 반환되는 값의 타입을 명시적으로 정의
  • 데이터베이스 호출에 대한 에러 처리를 추가
  • 리소스 청소 및 초기화를 위한 afterEach 구현
  • 로그 출력 관련 수정
  • 모든 테스트에 시간 제한 설정

위 사항들을 고려하여 코드를 수정하면 더욱 견고하고 유지보수가 용이한 테스트 코드가 될 것입니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰를 시작하겠습니다.

버그 리스크

  1. 수동 입력된 ID: lecture_id가 하드코딩되어 있습니다. 나중에 변경될 경우, 모든 관련 테스트를 수정해야 합니다.
  2. 테스트명 중복: 두 개의 테스트에서 동일한 이름 'distinct count'를 사용하고 있습니다. 이는 혼동을 초래할 수 있으니 각각 다른 이름으로 변경하는 것이 좋습니다.

개선 제안

  1. 테스트 목적 명확화: 각 테스트 케이스의 목적을 명확히 하기 위해 더 구체적인 설명을 추가하는 것이 좋습니다. 예를 들어 하나는 그룹화를 확인하는 테스트이고, 다른 하나는 distinct 결과의 길이를 검증하는 테스트로 세분화할 수 있습니다.
  2. 로그 수준 조정: console.log(result) 대신 Jest의 expect를 사용하여 결과를 검증하는 것이 바람직합니다. 예:
    expect(result).toBeGreaterThan(0); // 예시
  3. 전역 설정 고려: beforeEach 대신 beforeAll을 사용할 수 있습니다. 모든 테스트에서 응용프로그램을 새로 초기화할 필요가 없는 경우에는 성능이 향상됩니다.
  4. 타임아웃 설정: 첫 번째 테스트에만 타임아웃(10000ms)을 지정했습니다. 두 번째 테스트에서도 필요한 경우 이를 명시해주는 것이 좋습니다.

결론

테스트 코드에서의 명확성 및 유지보수성을 높이기 위한 몇 가지 개선사항을 제안했습니다. 코드 품질을 더욱 높일 수 있도록 리팩토링 해보세요.

Loading