[전승현] sprint5#105
Hidden character warning
Conversation
[전승현]Sprint2
전승현/스프린트미션3
joonfluence
left a comment
There was a problem hiding this comment.
전체적으로 Entity~Controller 레이어 분리, DTO 설계, File/JCF Repository 이중 구현 등 기본기가 잘 갖춰져 있습니다. 몇 가지 버그와 설계 개선 포인트를 코멘트로 남깁니다.
- P1: 2건 (반드시 수정 필요)
- P2: 4건 (적극 고려 권장)
- P3: 2건 (웬만하면 반영)
- P4: 1건
- P5: 1건
p5. .DS_Store, .gradle/, .discodeit/*.ser 바이너리/빌드 파일이 커밋에 포함되어 있습니다. .gitignore에 추가하고 git rm --cached로 트래킹을 해제하세요.
| BinaryContent binaryContent = new BinaryContent(fileName, (long) bytes.length, contentType, bytes); | ||
| return binaryContentRepository.save(binaryContent).getId(); | ||
| }) | ||
| .orElse(null); |
There was a problem hiding this comment.
p1. 프로필 이미지를 변경하지 않고 다른 정보(이름, 이메일 등)만 수정할 때 optionalProfileCreateRequest가 비어 있으면 nullableProfileId가 null이 되어 기존 프로필이 사라집니다.
기존 프로필을 유지하도록 수정해야 합니다:
| .orElse(null); | |
| .orElse(user.getProfileId()); |
|
|
||
| @ExceptionHandler(Exception.class) | ||
| public ResponseEntity<String> handlerException(Exception e){ | ||
| return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(e.getMessage()); |
There was a problem hiding this comment.
p1. Exception.class 폴백 핸들러가 400 Bad Request를 반환하고 있습니다. NullPointerException, RuntimeException 등 서버 내부 오류도 클라이언트 잘못으로 응답됩니다. 폴백은 500이어야 합니다.
| return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(e.getMessage()); | |
| return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(e.getMessage()); |
|
|
||
|
|
||
| @RequestMapping( | ||
| path="/create", |
There was a problem hiding this comment.
p2. URI에 동사(/create, /find, /findAll, /update, /delete)가 포함되어 있습니다. RESTful API에서는 HTTP 메서드가 동작을 나타내고, URI는 리소스만 표현해야 합니다.
예시:
POST /api/users(생성)GET /api/users/{userId}(조회)GET /api/users(전체 조회)PUT /api/users/{userId}(수정)DELETE /api/users/{userId}(삭제)
리소스명도 복수형(/api/user → /api/users, /api/channel → /api/channels)으로 통일하면 좋겠습니다. 다른 Controller(ChannelController, MessageController, BinaryContentController 등)에도 동일하게 적용해주세요.
|
|
||
| String newUsername = userUpdateRequest.newUsername(); | ||
| String newEmail = userUpdateRequest.newEmail(); | ||
| if (userRepository.existsByEmail(newEmail)) { |
There was a problem hiding this comment.
p2. update() 메서드에서 자기 자신의 email/username도 중복으로 감지됩니다. 비밀번호만 변경하더라도 기존 email이 이미 존재하므로 예외가 발생합니다.
현재 사용자를 제외하고 중복 체크해야 합니다:
userRepository.findByEmail(newEmail)
.filter(existing -> !existing.getId().equals(userId))
.ifPresent(existing -> {
throw new IllegalArgumentException("User with email " + newEmail + " already exists");
});| method = RequestMethod.GET | ||
| ) | ||
| public ResponseEntity<ChannelDto> find( | ||
| @RequestParam UUID channelId |
There was a problem hiding this comment.
p2. 리소스 식별자는 @RequestParam(쿼리 파라미터 ?channelId=xxx) 대신 @PathVariable(경로 파라미터 /channels/{channelId})을 사용하는 것이 RESTful 관례입니다.
@GetMapping("/{channelId}")
public ResponseEntity<ChannelDto> find(@PathVariable UUID channelId) {UserController, MessageController, ReadStatusController 등 다른 컨트롤러에도 동일하게 적용해주세요.
| @RequestBody LoginRequest loginRequest | ||
| ){ | ||
| User loginUser =authService.login(loginRequest); | ||
| return ResponseEntity.ok(loginUser); |
There was a problem hiding this comment.
p2. User Entity를 직접 반환하면 password 필드가 JSON 응답에 포함됩니다. UserDto처럼 민감 정보를 제외한 응답 DTO를 사용하거나, User Entity의 password 필드에 @JsonIgnore를 추가해주세요.
| } | ||
|
|
||
| dependencies { | ||
| implementation 'org.springframework.boot:spring-boot-starter-web' |
There was a problem hiding this comment.
p3. PR 체크리스트에서 Swagger 문서화를 완료했다고 표시했지만, springdoc-openapi 의존성이 build.gradle에 없습니다. Swagger UI가 동작하지 않을 것 같습니다.
| implementation 'org.springframework.boot:spring-boot-starter-web' | |
| implementation 'org.springframework.boot:spring-boot-starter-web' | |
| implementation 'org.springdoc:springdoc-openapi-starter-webmvc-ui:2.7.0' |
| @RequestBody PublicChannelCreateRequest publicChannelCreateRequest | ||
| ){ | ||
| Channel publicChannel= channelService.create(publicChannelCreateRequest); | ||
| return ResponseEntity.ok(publicChannel); |
There was a problem hiding this comment.
p3. 리소스 생성 API는 200 OK 대신 201 Created를 반환하는 것이 HTTP 표준입니다. UserController.create()에서는 올바르게 201을 사용하고 있으니, 나머지 Controller의 create 메서드도 맞춰주세요.
| return ResponseEntity.ok(publicChannel); | |
| return ResponseEntity.status(HttpStatus.CREATED).body(publicChannel); |
| public class BinaryContentController { | ||
| private final BinaryContentService binaryContentService; | ||
|
|
||
| @RequestMapping( |
There was a problem hiding this comment.
p4. @RequestMapping(path="/find", method = RequestMethod.GET) 대신 @GetMapping("/find") 등 단축 어노테이션을 사용하면 코드가 간결해집니다. 모든 Controller에 적용 가능합니다.
요구사항
기본
-[x]Spring Data JPA 적용하기
-[x]엔티티 정의하기
-[x] JPA 주요 어노테이션을 활용해 ERD, 연관관계 매핑 정보를 도메인 모델에 반영해보세요.
-[x] ERD의 외래키 제약 조건과 연관관계 매핑 정보의 부모-자식 관계를 고려해 영속성 전이와 고아 객체를 정의하세요.
-[x]DTO 적극 도입하기
-[x]BinaryContent 저장 로직 고도화
-[x]페이징과 정렬
심화
-[x] N+1 문제가 발생하는 쿼리를 찾고 해결해보세요.
-[x] 프로덕션 환경에서는 OSIV를 비활성화하는 경우가 많습니다. 이때 서비스 레이어의 조회 메소드에서 발생할 수 있는 문제를 식별하고, 읽기 전용 트랜잭션을 활용해 문제를 해결해보세요.
-[x]페이지네이션 최적화
-[x] Entity와 DTO를 매핑하는 보일러플레이트 코드를 MapStruct 라이브러리를 활용해 간소화해보세요.
###PR첨부내용
-[x] Entity를 Controller 까지 그대로 노출했을 때 발생할 수 있는 문제점에 대해 정리해보세요. DTO를 적극 도입했을 때 보일러플레이트 코드가 많아지지만, 그럼에도 불구하고 어떤 이점이 있는지 알 수 있을거에요.(이 내용은 PR에 첨부해주세요.)
-> 예를 들어 User 엔티티를 controller에서 그대로 노출하게되면 비밀번호같은 공개되어야하지않은 정보가 공개될수가있다
-- [x] 오프셋 페이지네이션과 커서 페이지네이션 방식의 차이에 대해 정리해보세요.
- 오프셋 페이지네이션:LIMIT와 OFFSET 쿼리를 사용하여 특정 페이지의 데이터를 가져옴
- 동작 방식→ 앞에서부터 100개를 건너뛰고(OFFSET) 그 다음 10개를 가져와(LIMIT)라는 식
- 문제점
- 성능 저하: 뒤 페이지로 갈수록 데이터베이스는 앞부분의 데이터를 모두 읽어서 버려야하므로 속도가 급격하게 느려짐
- 데이터 중복/누락:조회를 하던 도중에 새로운 데이터가 추가되면 페이지 경계가 밀리면서 이전페이지에서 봤던 데이터가 다음 페이지에 또 나올 수 있다
주요 변경사항
스크린샷
멘토에게
-Postman/Swagger에서는 괜찮았는데 웹 실행 시 쿼리가 폭발하는 것이, 단순히 웹 페이지에서 여러 API를 동시에 호출해서 발생하는 현상인지, 아니면 제가 작성한 엔티티 연관 관계 때문에 발생하는 구조적 결함인지 판단하기 어렵습니다. 멘토님은 실무에서 이런 대량의 쿼리 로그를 분석할 때 어떤 포인트부터 디버깅하시는지 궁금합니다.
-ChannelMapper같은 경우 mapStruct를 쓰지않는게 더 가독성이 좋아보이는데 굳이 써야하는지 궁금합니다
-저번리뷰대로 gitignore 수정했는데 맞게 수정된건지 여쭤보고싶습니다
-현재 controller service repository등의 기능별 패키지 구조를 유지하고 있는데 프로젝트 규모가 커짐에 따라 도메인별로 패키지를 나누는 방식으로 전환하는 시점이 언제쯤이 적당한지 여쭤보고싶습니다
-브랜치를 하나 새로파서 pr을 새로 남겻어야햇는데 제이름브렌치로 해서 pr생성이 안됬습니다. 다음 미션부터 주의하겠습니다
-한글 브랜치명 인코딩 문제로 제 레포지토리 메인에서 404가 뜨는 경우가 있으나, PR의 Files changed 탭에서는 코드가 정상 확인됩니다->이또한 한글브랜치명 인코딩문제인지 확인이되지않습니다