Skip to content

[feat] 캡슐 CRUD 구현#22

Open
jucheonsu wants to merge 19 commits intomainfrom
feat/#16
Open

[feat] 캡슐 CRUD 구현#22
jucheonsu wants to merge 19 commits intomainfrom
feat/#16

Conversation

@jucheonsu
Copy link
Copy Markdown

@jucheonsu jucheonsu commented Jul 20, 2025

[feat] 캡슐 CRUD 구현

😺 Issue

✅ 작업 리스트

  • Capsule, Media 엔티티 및 1:N 매핑

  • CapsuleType enum과 DTO 작성

  • Repository, Service CRUD 구현

  • REST API 컨트롤러 작성 (파일 업로드 포함)

  • 커스텀 예외 및 글로벌 예외 처리

  • 단위 테스트 및 API 테스트 수행

⚙️ 작업 내용

  • 캡슐과 미디어 데이터를 계층 구조로 관리하고,
    REST API로 CRUD 기능 제공

  • 파일 업로드 및 예외 처리를 포함한 안정적 서비스 구현

  • 테스트로 기능 정상 동작 확인

📷 테스트 / 구현 내용

image

🔧 수정 내용

  • Controller에서 ResponseEntity return inline
    변경 및 코드 간소화

  • Service에서 user 조회 분리

  • Entity에서 Setter 제거 및 생성자 정리

  • DTO를 요청/응답으로 분리하고 builder 패턴 적용

  • Repository 주석 제거

  • 테스트 코드 리팩토링, 출력문 제거

  • SecurityUtil 활용해 유저 조회 로직 분리 및 print문 삭제

@jucheonsu jucheonsu self-assigned this Jul 20, 2025
@jucheonsu jucheonsu added 👾 feat 새 기능 / New features 🐹 천수 labels Jul 20, 2025
@jucheonsu jucheonsu linked an issue Jul 20, 2025 that may be closed by this pull request
@jucheonsu jucheonsu requested review from Vloeiolzlr, dohy-eon, hodoon, kjunh972 and ysw789 and removed request for hodoon July 20, 2025 12:20
@jucheonsu jucheonsu changed the title Feat/#16 [feat] 캡슐 CRUD 구현 Jul 20, 2025
Comment on lines +24 to +25
CapsuleResponseDto responseDto = capsuleService.createCapsule(requestDto);
return ResponseEntity.status(HttpStatus.CREATED).body(responseDto);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

비즈니스 레이어 메소드가 리턴 값이 있는 경우 ResponseEntity 리턴에 인라인으로 합쳐도 될 듯 합니다

return ResponseEntity.status(HttpStatus.CREATED).body(capsuleService.createCapsule(requestDto));

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

네 수정하겠습니다

import java.util.List;
import java.util.stream.Collectors;

public class CapsuleDto {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Request와 Response처럼 역할이 완전히 다른 DTO를 함께 이너 클래스로 두는 것은 단일 책임 원칙에 위배된다는 생각이 들긴 합니다만
지금처럼 작은 프로젝트에서는 큰 문제 없을 듯 하므로 그대로 유지해도 무방해보입니다
다만 프로젝트 규모가 더 커지고, 한 도메인에서 사용되는 DTO 및 VO 클래스가 많아진다면 그 때는 역할 별로 DTO도 클래스를 분리하는 것을 추천합니다~

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

네 코드 리뷰 감사합니다

private String originalFileName;

public static MediaDto toDto(Media media) {
String cid = media.getCid();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

빌더에서 바로 초기화해도 될 것 같은데 따로 초기화 된 이유가 있을까요?

Comment on lines +29 to +38
private User getCurrentUser() {
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
if (authentication == null || !authentication.isAuthenticated()) {
throw new CustomException(ErrorCode.UNAUTHORIZED);
}
String userEmail = authentication.getName();

return userRepository.findByEmail(userEmail)
.orElseThrow(() -> new CustomException(ErrorCode.USER_NOT_FOUND));
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

사용자 정보를 불러오는 로직은 Capsule 과는 전혀 연관되지 않는데 별도의 서비스로 분리하는 게 어떨까요? 예를들어 UserService

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

캡슐 CRUD에 사용자의 로그인 정보를 불러오는 로직이 필요해서 추가했습니다. 만약 수정이 필요하다면 바로 수정하도록 하겠습니다.

capsule.setUser(currentUser);

Capsule savedCapsule = capsuleRepository.save(capsule);
System.out.println("Capsule 저장 완료, ID: " + savedCapsule.getId());
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

print문은 개발 때에만 잠깐 사용하고 실제 로깅은 Slf4j를 통해 하는 걸 추천합니다

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

네 감사합니다

Comment on lines +44 to +45
Capsule capsule = requestDto.toEntity();
capsule.setUser(currentUser);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Entity 변환 시 User를 인자로 전달해 객체 인스턴스 생성과 동시에 초기화 하는 것이 좋아보입니다
객체 인스턴스 생성 후 상태 변경은 객체 내부적으로만 수행하는 것이 적절하므로 setter 사용은 지양하는 것이 좋습니다

private User user;

@Builder
public Capsule(String title, CapsuleType type, String content, LocalDate openDate, User user) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

사용되지 않는 생성자는 제거해주세요

@ysw789
Copy link
Copy Markdown

ysw789 commented Jul 20, 2025

전체적으로 잘 설계하신 것 같습니다!
Media 관련 구현 사항은 아직 없는 것 같아 이 부분은 제외하고 참고 사항 알려드리겠습니다

  1. Builder 패턴 적용
    CapsuleDto.CapsuleRequestDto 에서 Capsule.builder()를 직접 호출하고 있는데, 객체 생성의 책임을 해당 도메인 엔티티에 위임하고 외부에서는 이를 호출하기만 하도록 하는 것이 적절합니다.

    // 현재: DTO에서 직접 빌더 호출
    public Capsule toEntity() {
        return Capsule.builder()
                .title(title)
                .type(type)
                .content(content)
                .openDate(openDate)
                .build();
    }
    
    // 개선: Capsule에 정적 팩토리 메소드 추가
    public class Capsule {
        public static Capsule from(CapsuleRequestDto dto, User user) {
            return Capsule.builder()
                    .title(dto.getTitle())
                    .type(dto.getType())
                    .content(dto.getContent())
                    .openDate(dto.getOpenDate())
                    .user(user)
                    .build();
        }
    }
  2. setter
    setter를 사용하게 되면 객체의 불변성을 해치고 객체 외부 어디에서나 상태를 변경할 수 있습니다. 객체의 상태가 의도치 않은 곳에서 변경될 경우 예상 불가한 오류가 발생할 수 있습니다.
    Capsule 클래스 내부에 있는 update() 메소드가 바로 모범 케이스입니다. update 작업 자체도 setter 통해서 외부에서 수행할 수 있을텐데 왜 내부적으로 했을까요? 이유는 객체 인스턴스 생성 후 상태 변경은 객체 내부적으로만 수행하는 것이 캡슐화를 준수하는 방식이기 때문입니다.

@jucheonsu
Copy link
Copy Markdown
Author

긴 시간동안 코드 리뷰 해주셔서 감사드립니다!
질문을 하나 드리자면 Setter를 모두 없애는 것이 코드 전체적으로 좋을까요?

@ysw789
Copy link
Copy Markdown

ysw789 commented Jul 20, 2025

긴 시간동안 코드 리뷰 해주셔서 감사드립니다! 질문을 하나 드리자면 Setter를 모두 없애는 것이 코드 전체적으로 좋을까요?

개인적으론 정말 특수한 경우를 제외하곤 setter 사용은 아예 지양하는 것이 좋다고 생각합니다

@jucheonsu
Copy link
Copy Markdown
Author

네 감사합니다 바로 수정해서 다시 올리도록 하겠습니다

@dohy-eon
Copy link
Copy Markdown
Member

변경사항은 확인 완료했고 스웨거 관련 어노테이션만 간단하게 추가했습니다. 확인 후 코멘트 남겨주세요

@jucheonsu
Copy link
Copy Markdown
Author

스웨거 관련 어노테이션 확인 완료했습니다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐹 천수 👾 feat 새 기능 / New features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feat] 캡슐 CRUD 구현

3 participants