-
Notifications
You must be signed in to change notification settings - Fork 22
[오하영] Sprint12 #67
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
base: express-오하영
Are you sure you want to change the base?
[오하영] Sprint12 #67
The head ref may contain hidden characters: "express-\uC624\uD558\uC601"
Conversation
reach0908
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생많으셨습니다. 코멘트들 확인해주세요!
점점 코드들의 체계가 잡혀가는게 눈에 보이는 것 같아서 좋네요. 항상 제 개인적인 관점에서 피드백을 주다보니 다른 관점의 코드들도 오픈소스들을 많이 참고해보시면 좋을 것 같습니다!!
| import "dotenv/config"; | ||
| import passport from "passport"; | ||
| import "./config/passport"; | ||
| import "./configs/passport.config"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2]
파일로 관리하는 것도 좋지만 개인적으로 저는 app.ts를 쭉 읽어가면서 어떠한 설정들이 해당 앱에 적용되어있는가를 읽는 것도 괜찮아서 config들을 모아두기만 하고 전략들만 한번에 import 해와서 하단에서 passport.use() 로 직접 app.ts 파일에서 명시하는 편이 더 좋다고 생각합니다!
그리고 나중에 선언의 순서가 중요한 것들이 몇몇 있어서 이렇게 통으로 import하면 디버깅이 힘들어질 수도 있습니다!
| return await prisma.product.findMany(options); | ||
| const { where } = options; | ||
|
|
||
| const [totalCount, products] = await Promise.all([ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P)]
이 부분의 경우에는 같은 options로 검사하는데, 그냥 사용하는쪽에서 products.length를 계산하는게 더욱 효율적일 것 같습니다.
findAll과 count는 별도로 분리하고 사용이 필요한 경우에만 같이 부르면 좋을 것 같아요!
| const s3 = new S3Client({ | ||
| region: "ap-northeast-2", | ||
| credentials: { | ||
| accessKeyId: process.env.AWS_ACCESS_KEY_ID as string, | ||
| secretAccessKey: process.env.AWS_SECRET_ACCESS_KEY as string, | ||
| }, | ||
| }); | ||
|
|
||
| const upload = multer({ | ||
| storage: multerS3({ | ||
| s3: s3, | ||
| bucket: process.env.AWS_BUCKET_NAME as string, | ||
| key: ( | ||
| req: Request, | ||
| file: Express.Multer.File, | ||
| cb: (error: any, key?: string) => void | ||
| ) => { | ||
| // ?access=private 인 경우 업로드 위치를 private/ 폴더로 지정 | ||
| const isPrivate = req.query.access === "private"; | ||
| const folder = isPrivate ? "private/" : "public/"; | ||
| cb(null, `${folder}${Date.now()}_${file.originalname}`); | ||
| }, | ||
| }), | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1]
라우터에는 라우터와 관련된 파일들만 있으면 좋을 것 같습니다. 해당 코드는 별도의 s3를 위한 util로 분리해보면 어떨까요?
|
추가 코멘트에 대해 통합테스트의 경우에는 도메인을 넘나드는 로직들이 유기적으로 구성되어 테스트가 필요할 때 구성하는 편입니다. supertest를 통한 e2e테스트의 경우에는 사용자 플로우(시나리오)에 맞게 동작하는가를 중점적으로 테스트를 하고 있습니다. 첨언해서 개인적으로는 정말로 앱에 있어 중요한 로직(인증/인가, 주요서비스 로직)은 테스트 주도개발을 진행하고, 나머지 테스트의 경우 여유가 되면 진행합니다! |
요구사항
기본
공통백엔드 배포프로젝트 구조 및 환경 설정
development) 및 배포(production) 환경 설정을 구분하고, 환경 변수를 사용해 관리합니다.AWS S3를 이용한 파일 업로드 시스템 구축
multer-s3라이브러리를 사용하여 이미지 업로드 미들웨어를 S3로 변경합니다.AWS RDS를 사용한 데이터베이스 관리
AWS EC2에서의 애플리케이션 운영
pm2를 사용하여 애플리케이션을 백그라운드에서 실행시킵니다.백엔드 테스트 구현
jest.config.js)을 만들고 기본 설정을 하세요.async/await와done콜백을 사용하여 비동기 코드의 완료를 테스트하세요.describe와test블록을 사용하여 테스트 케이스를 그룹화하고 정리하세요.심화
테스트 구현
상품 이미지 업로드
AWS Route 53을 활용한 도메인 관리
SSL 인증서를 통한 HTTPS 연결 구현
주요 변경사항
render->aws ec2로 변경스크린샷
테스트 커버리지 측정 결과
멘토에게
supertest를 사용하는 통합테스트 과정에 속하는게 맞나요?