[이상경] 스프린트 미션 5, 6#72
Hidden character warning
Conversation
- Tag 컴포넌트 분리 및 태그 삭제 기능 구현 - 상품 등록 유효성 검사 강화 - 한글 IME 입력 처리 추가 - 상품 API 에러 처리 추가 - PATCH validation 적용
- client/server package.json 분리 - 프론트엔드와 백엔드 의존성 분리 - MongoDB 마이그레이션을 위한 프로젝트 구조 정리 - Prisma 디렉토리 생성 - 기존 MongoDB 관련 파일(.mongodb-backup) 백업 - 기존 루트 package.json 및 package-lock.json 유지
devbini
left a comment
devbini
left a comment
There was a problem hiding this comment.
🎉 고생 많으셨습니다!
단순히 API만 추가한 게 아니라 routes와 services/validators, 그리고 미들웨어 등 서버 코드의 역할을 나누려고 한 점이 특히 좋았습니다. (역할 분할)
이러한 역할 분할은 현업에서도 중요하게 생각하면서 많이 간과하는데, 좋은 습관이 자리잡고 있는 것 같아요.
특히 댓글 구현에 있어서 라우트는 분리하면서도 내부 생성/조회 로직은 공통화하려고 한 점이 좋았어요. 연결 필드 관련 개발 방식도 처음 관계형 데이터 모델을 다루는 단계에서 꽤 좋은 시도이자 경험치였다 생각합니다.
superstruct와 errorHandler를 따로 나눠서 검증과 에러처리를 나눠서 관리하고, seed 데이터도 검색/정렬/페이지네이션 등을 세밀하게 확인할 수 있도록 꽤 많이 준비해둔 점이 좋았습니다.
PR 본문에 적어주신 것처럼 cursor pagination을 직접 여러 페이지로 나눠 테스트해본 것도 인상적이었어요. 단순 CRUD만 확인하고 끝낸 게 아니라 실제 데이터 흐름을 보려고 한 점이 보였습니다. 이런 실험 결과와 과정은 나중에 큰 도움이 될 거라 생각해요.
다만, 그래도 실제 코드 흐름 기준으로는 프론트와 백엔드 응답 형태가 아직 하나로 이어지진 않는 것 같긴 해요. 일부 라우트나 CORS 설정처럼 화면에서 바로 티가 날 수 있는 부분부터 먼저 맞추면 전체 완성도가 훨씬 올라갈 것 같습니다! (실제로 내가 사용자라면 뭘 해볼까? 를 생각해보는 사고를 기르는거죠!!)
전반적으로... 구조화에 대한 감각이 있는 것 같아요.
이제는 조금 더 단단하게 다듬으면 다음 작업에서 훨씬 안정적으로 확장할 수 있을 것 같아요.
요구사항에 따라 세밀하게 관리하고 조절하는 능력만 천천히 기르는거죠!
이미 엔지니어링적 사고는 거의 잡혀가고 계신 것 같으니까요..!
너무나 고생 많으셨고, 파이팅입니다! 👍
추가로.. PR 본문에 적어주신 “구현 중 고민한 점”에 답을 남기면...
Comment 모델을 하나로 통합한 선택은 현재 단계에서는 합리적이라 생각은 하지만. 다만 productId, articleId가 모두 optional이 되면서 잘못된 상태가 들어올 여지가 생긴 것 같아요. 라우터나 서비스 레이어에서 “둘 중 하나만 연결” 규칙을 잡아주면 좋겠습니다.
지금은 안전 장치가 없어서- 공격이 가능한 경로가 생겨버렸다.. 라고도 생각이 되는 것 같아요.
댓글 공통 로직을 별도 파일로 뺀 것도 좋아요. 특히 parentKey는 중복을 줄이는 데 의미가 있지요.... 이제는 유효하지 않은 값 등 비정상 데이터를 정리하기만 해도 충분 해 보입니다.
cursor pagination은 위에서 이미 언급했지만.. 직접 테스트해본 점이 너무 좋았어요.
createdAt + id를 함께 쓰는 방식으로 정렬 안정성까지 챙기면 훨씬 프로덕션에 가까운 구현이 될 것 같습니다.
전반적으로 구현과 테스트, 공부 방향이 잘 잡혀있는 것 같아요.
어느 회사로 가게 될지, 벌써 그 회사가 부럽다는 생각이 드네요ㅎㅎ
| }) | ||
|
|
||
| // 백엔드 응답이 _id 또는 id인 경우 모두 대응 | ||
| navigate(`/items/${product._id || product.id}`) |
There was a problem hiding this comment.
🚩 Check Point
상품 등록 후 이동하는 페이지의 매개변수를 product._id로 쓰고 있네요?
제가 알기론 Prisma 데이터 응답 식별자가 _id가 아니라 id를 쓸 텐데
등록이 성공해도 이동하는 nav 주소가 undefined가 되는 불상사가 있을 수도 있을 것 같아요.
실제로 구동 시 문제가 없다면 괜찮겠지만
혹시 모르니 리뷰 남겨둘게요.
There was a problem hiding this comment.
🚩 Check Point
추가로 상품 등록 성공 후 /items/:id로 이동하려는 코드인데,
App.jsx에 저장 된 현재 라우터에는 /items/:id 경로가 없는 것 같아요-!
| const app = express() | ||
| const PORT = process.env.PORT || 3000 | ||
|
|
||
| app.use(express.json()) | ||
|
|
||
| app.use(productRoutes) | ||
| app.use(articleRoutes) | ||
| app.use(commentRoutes) | ||
|
|
||
| app.use(errorHandler) |
There was a problem hiding this comment.
☕ Thinking
음.. 문제는 없지만, 프론트와 백엔드가 각각 Vite dev server와 Express server로 다른 포트에서 뜨면 브라우저 요청이 CORS 정책에 막힐 수 있어요.
cors 패키지는 의존성에 들어가 있으니 import cors from 'cors' 후 app.use(cors())를 추가해두면 차후에 나올 문제를 미리 방지할 수 있지 않을까요?🤗
| <p>검색 결과가 없습니다.</p> | ||
| ) : ( | ||
| products.map((product) => ( | ||
| <ProductCard key={product._id} product={product} /> |
There was a problem hiding this comment.
☕ Thinking
상품 카드 렌더링에서 key={product._id}를 쓰고 있네요!
아까... 위쪽 다른 Check Point에서도 있던 문제랑 동일한데
현재 Prisma 기반 백엔드는 id를 내려주고 있어서 _id는 없어 보여요.
화면이 당장 깨지지는 않을 수 있지만... React 리스트 생성 키가 undefined가 되면 정렬이나 페이지 이동이 발생할 때 렌더링이 깨져버릴 수 있어 보여요!
| const skip = Number(offset) || 0 | ||
| const take = Number(pageSize) || 10 |
There was a problem hiding this comment.
☕ Thinking
이것도 문제는 아니지만,
숫자를 바로 DB로 던지고 있는 것 같아요. 페이지네이션 관련 데이터를..
정상적인 상황에서는 문제가 없지만, 실제 환경에서는 어떤 문제가 나올지 몰라요.
예를 들어- limit 데이터가 음수로 들어간다거나, 비정상적으로 엄청 큰 데이터가 들어간다던가.... 그러면 Prisma 에러가 나거나 서버가 너무 큰 조회를 하게 될 수 있어요.
숫자 변환 후 하한/상한을 검증하는 공통 함수를 하나 만들어두면 상품/게시글 목록 조회에서 같이 쓸 수 있을 것 같습니다.
또는 Math 라이브러리를 활용하면 아마 상/하 제한을 걸 수 있을텐데, 서버 리소스 보호를 생각하는 방법이 하나 더 있지 않을까 싶어 남겨둡니다ㅎㅎ
| content: true, | ||
| createdAt: true, | ||
| }, | ||
| orderBy: { createdAt: 'desc' }, |
There was a problem hiding this comment.
☕ Thinking
PR 본문에도 적어주신 부분인데, 현재 댓글 페이지네이션은 createdAt으로 정렬하면서 cursor는 id를 사용하고 있네요.
이건... 데이터가 적을 때는 잘 동작해 보일 수 있지만, 같은 createdAt을 가진 댓글이 여러 개 생기면 페이지 사이에서 순서가 꺠지거나 레이스컨디션이 생길 수도 있어요.
위에 주석으로 리팩토링 예정이라고 적어주신 것처럼, createdAt desc, id desc를 함께 쓰는 방식으로 정리하면 더 안정적일 것 같습니다 🙇🤗
| productId String? | ||
| product Product? @relation(fields: [productId], references: [id], onDelete: Cascade) | ||
| articleId String? | ||
| article Article? @relation(fields: [articleId], references: [id], onDelete: Cascade) |
There was a problem hiding this comment.
🚩 Check Point
현재 스키마 구조를 보면,
productId와 articleId가 모두 없는 댓글도 만들 수 있고, 반대로 둘 다 있는 댓글을 만들 수 있네요.
XOR 제약을 직접 표현하기 어렵다면, 댓글 생성 라우터에서 productId와 articleId 중 하나만 들어가도록 검증을 추가하거나, ProductComment, ArticleComment와 같이 모델을 분리하는 방법이 필요 해 보여요.
| }) | ||
|
|
||
| commentRoutes.post('/articles/:articleId/comments', async (req, res) => { | ||
| assert(req.body, CreateComment) |
There was a problem hiding this comment.
☕ Thinking
assert(req.body, CreateComment)가 try 밖에 있네요.
의도 된 건지 까먹은건지는 모르겠지만, 아래쪽에 다른 코드에도 동일한 내용이 보입니다.
async handler에서 던진 에러가 미들웨어로 넘어갈 수 있긴 하지만..
같은 파일 안에서 어떤 에러는 route catch에서 처리되고 어떤 에러는 전역 errorHandler로 넘어가는 구조가 된 것 같아요.
쉽게 말하면, 통일성이 떨어져 보이기도 하는 것 같아요.
검증하는 로직도 try 안으로 넣거나.. 별도의 검증 미들웨어를 따로 두면 에러 응답 형식이 더 일관되게 되어 유지보수적으로 이점이 생기지 않을까 싶습니다.
| res.sendStatus(404) | ||
| } else { | ||
| console.log(err) | ||
| res.status(500).send({ message: err.message }) |
There was a problem hiding this comment.
🚩 Check Point
음.. 이건 조금 위험해요.
요구사항에 500에 대한 응답 에러처리가 있어서 이렇게 해도 되는건지는 잘 모르겠지만..
500의 경우에는 DB 연결 내용이나 구현에 있어 치명적일 수 있는 서버 오류메시지가 사용자에게 그대로 전달되게 됩니다.
가능하면 message를 하드하게 별도로 만들거나, 에러핸들러를 따로 만들어서 관리하는 게 좋을 것 같아요.
블라인드 SQL 인젝션 등, 다양한 보안적 위험요소가 존재한답니다.
자세한 오류 로그는 서버 로그에만 남기는 게 좋아요! 🥹
2 participants
스프린트 미션 5
기본 요구사항
공통
프론트엔드 구현 요구사항
랜딩 페이지
중고마켓 페이지
라우팅
상품 목록
상품 등록 이동
상품 등록 페이지
페이지 구성
상품 등록
백엔드 구현 요구사항
중고마켓
Product 스키마
API (등록, 조회, 수정, 삭제)
API 에러 처리
서버 환경
프론트엔드 심화 요구사항
상품 등록 페이지
버튼 활성화
태그 기능
Validation
스프린트 미션 6
기본 요구사항
중고마켓
MongoDB → PostgreSQL 마이그레이션
공통
데이터베이스
API 에러 처리
자유게시판
Article 스키마
API (등록, 조회, 수정, 삭제)
게시글 등록 API를 만들어 주세요.
게시글 조회 API를 만들어 주세요.
게시글 수정 API를 만들어 주세요.
게시글 삭제 API를 만들어 주세요.
게시글 목록 조회 API를 만들어 주세요.
댓글
API (등록, 조회, 수정, 삭제)
댓글 등록 API를 만들어 주세요.
댓글 수정 API를 만들어 주세요.
댓글 삭제 API를 만들어 주세요.
댓글 목록 조회 API를 만들어 주세요.
중고마켓 / 자유게시판 API 분리
구현 중 고민한 점 (미션 6)
1. Comment 모델 설계
댓글은 상품 댓글과 게시글 댓글을 모두 지원해야 하므로
Comment모델에서productId,articleId를 선택적으로 가질 수 있도록 설계했습니다. 상품 댓글인 경우에는productId를, 게시글 댓글인 경우에는articleId를 연결하도록 했고, Product 또는 Article이 삭제될 때 연결된 댓글도 함께 삭제되도록onDelete: Cascade를 적용했습니다.2. 댓글 API 공통 로직 분리
상품 댓글과 게시글 댓글은 라우트는 분리되어 있지만, 내부 생성/조회 흐름은 거의 동일했기에 중복을 줄이기 위해 댓글 관련 공통 로직을
commentService.js로 분리해봤습니다. 댓글 생성 시[parentKey]: parentId형태를 사용해productId또는articleId를 동적으로 연결하도록 구현해서 상품 댓글과 게시글 댓글 등록 로직을 하나의 함수로 공통화했습니다.3. Cursor Pagination 검증
댓글 목록 조회에는 cursor 방식 페이지네이션을 적용했습니다. 단순히 댓글이 조회되는지만 확인하지 않고, seed 데이터에서 특정 상품과 게시글에 댓글을 여러 개 연결해 실제 페이지 이동이 정상적으로 동작하는지 확인했습니다. 현재는
createdAt desc정렬과id cursor를 사용하고 있습니다. 동작은 확인했지만, 같은 생성일자를 가진 데이터가 여러 개 있을 경우 더 안정적인 페이지네이션을 위해 추후createdAt + id를 함께 사용하는 방식으로 리팩토링 예정입니다.4. 응답 형식 정리
성공/실패 응답을 확인하기 쉽도록
message와code를 함께 반환하도록 했습니다. 삭제 API의 경우 처음에는204 No Content도 고려했지만, 성공 메시지와 code를 응답으로 내려주기 위해200 OK와 JSON 응답을 사용하는 방식으로 변경했습니다.5. 테스트 데이터 설계
API 테스트를 위해 작성한 seed 데이터는 Product, Article은 검색/정렬/페이지네이션을 확인할 수 있도록 여러 개의 데이터를 넣었고, Comment는 cursor pagination을 검증하기 위해 특정 상품과 게시글에 여러 개의 댓글이 연결되도록 구성했습니다. 이 과정을 통해 CRUD뿐만 아니라 실제 관계 데이터와 페이지네이션 흐름까지 확인할 수 있었습니다.
테스트 완료
limit=2로 조회하여 1페이지, 2페이지, 3페이지 모두 중복/누락 없이 조회되는지 확인limit=2로 조회하여 1페이지, 2페이지 모두 중복/누락 없이 조회되는지 확인nextCursor: null이 반환되는지 확인