Conversation
There was a problem hiding this comment.
레포지토리 영역과 서비스 영역이 전반적으로 혼재되어 있습니다!
또 어떤 레포지토리에선 단건조회에 Optional을 반환, Entity를 반환, Error를 Throw 등 각 XXXRepository 마다 일관성이나 기준점이 없기에 해당 레포지토리를 사용하려는 다른 개발자의 입장에선 동작에 대한 예측이 힘들고, 상세 내부 구현을 모두 살펴볼수 밖에 없게 됩니다!
우선적으로 Repository에 일관성 있는 반환(Optional로 반환할지, Entity를 그대로 반환할지 etc...) 등과 같은 기준점을 잡으시고 일관성있게 통일하셔야 합니다!
이후 서비스 레이어의 영역과 레포지토리 레이어의 영역을 조금더 분리 하셔야 합니다.
Repository 는 최대한 복잡한 로직없이 단순하게 데이터의 "검색, 수정, 추가, 삭제" 에만 집중해야 합니다!
권한체크, 검색한 혹은 수정하려는 데이터가 없을시 에러를 throw 등의 역할은 서비스에서 수행하셔야 합니다!
| implementation("org.springframework:spring-aop:6.2.3") | ||
|
|
||
| implementation("org.aspectj:aspectjweaver:1.9.21") | ||
| implementation("org.aspectj:aspectjrt:1.9.21") | ||
| } |
There was a problem hiding this comment.
spring-aop 의존성은 spring-boot-starter-web 에 이미 포함되어 있습니다.
aspectjweaver, aspectjrt 의존성은 현재 사용하고 계시는 Spring Boot 3.4.4 권장 버전보다 낮은 버전으로 설정하신 이유가 있으실까요 ?
아래 URL을 통해서 Spring Boot 특정 버전에서 권장하는 디펜던시 버전이 명시되어 있습니다.
아래 URL에 포함된 의존성은 특정한 이유가 있는게 아니라면, 다음과 같이 버전을 명시적으로 설정 안하시는게 안전합니다. implementation("org.aspectj:aspectjweaver")
"io.spring.dependency-management" 플러그인으로 자동으로 호환 및 권장되는 버전이 설정됩니다.
https://mvnrepository.com/artifact/org.springframework.boot/spring-boot-starter-parent/3.4.4
https://mvnrepository.com/artifact/org.springframework.boot/spring-boot-dependencies/3.4.4
| @Before("serviceMethods()") | ||
| public void logBefore(JoinPoint joinPoint) { | ||
| String method = joinPoint.getSignature().toShortString(); | ||
| Object[] args = joinPoint.getArgs(); | ||
| log.info("▶▶ Start: {}", method); | ||
| log.info("▶▶ Args: {}", Arrays.toString(args)); | ||
| } | ||
|
|
||
| @AfterReturning(pointcut = "serviceMethods()", returning = "result") | ||
| public void logAfter(JoinPoint joinPoint, Object result) { | ||
| String method = joinPoint.getSignature().toShortString(); | ||
| log.info("✅ End: {}", method); | ||
| log.info("✅ Returned: {}", result); | ||
| } | ||
|
|
There was a problem hiding this comment.
AOP를 활용하여 보신 부분은 인상적입니다!
만약 매개변수나 리턴값이 기본 자료형 or DTO가 아닌 바이너리 데이터(이미지, 엑셀, etc...) 등이라면 식별하기 힘든 로그가 너무 길게 출력될 수 있습니다.
현재 단계와 테스트 환경에서는 인상적이고 좋은 시도입니다!
| @Override | ||
| public Optional<User> getUser(UUID id) { | ||
| return Optional.ofNullable(userRepository.loadFromFile().get(id)); | ||
| public Optional<UserResponseDto> getUser(UUID id) { | ||
| Optional<User> user = userRepository.getUser(id); | ||
|
|
||
| userStatusRepository.findAllStatus().forEach(userStatus -> { | ||
| if (userStatus.getUserId().equals(id)) { | ||
| user.get().setOnline(userStatus.isOnline(Instant.now())); | ||
| } | ||
| }); | ||
|
|
||
| BinaryContent profileImg = binaryContentRepository.findBinaryContentById(user.get().getId()); | ||
| user.get().setProfileImage(profileImg.getData()); | ||
|
|
||
| return Optional.of(createUserResponseDto(user.get())); | ||
| } | ||
|
|
There was a problem hiding this comment.
Optional 을 사용하여 반환하는 이유에 대해 조금더 고민하여 보시면 좋을 것 같습니다!
Optional 로 반환된 user를 isPresent 체크나 orElseXXX 등으로 체크나 예외를 던지는 등의 동작없이 get 하여 바로 사용한다면 그냥 null을 반환하는 것과 별다른 차이가 없게됩니다!
| @Setter | ||
| @Getter | ||
| @ToString | ||
| public class MessageResponseDto { | ||
| private UUID userId; | ||
| private String messageContent; | ||
| private UUID messageId; | ||
| private List<byte[]> messageFile; | ||
|
|
||
| public MessageResponseDto(Message message, List<byte[]> messageFile) { | ||
| this.userId = message.getSender(); | ||
| this.messageContent = message.getContent(); | ||
| this.messageId = message.getId(); | ||
| this.messageFile = messageFile; | ||
| } | ||
|
|
||
| public MessageResponseDto(Message message) { | ||
| this.userId = message.getSender(); | ||
| this.messageContent = message.getContent(); | ||
| this.messageId = message.getId(); | ||
| this.messageFile = new ArrayList<>(); | ||
| } | ||
| } |
There was a problem hiding this comment.
@DaTa, @Setter 는 주의해야 좋습니다!
MessageResponseDto 는 응답용 데이터 전송 객체이므로, 생성된 이후 상태가 변경될 필요가 없습니다.
현재 구현하신 구현 또한 생성자를 통해서 모두 멤버변수를 모두 초기화 하고 계시니 차후 의도치않은 불변성이 깨지는걸 방지하기 위해 불필요한 @Setter 사용은 안하시는걸 권장 드립니다.
현재 구현하신 생성자를 통한 초기화도 좋고, Lombok에 @builder 라는 빌더패턴을 지원하는 어노테이션, 그리고 팩토리 메서드를 통한 초기화도 있으니 세가지 방법 모두 한번 알아보시길 추천드립니다.
개발자별 스타일이나 선호도 차이지만 빌더패턴과 팩토리메서드 모두 자주쓰이는 유형이니 한번 알아보시면 좋습니다!
| public List<BinaryContent> findAllBinaryContentById(UUID id); | ||
|
|
||
| public boolean deleteBinaryContentById(UUID id); | ||
| } |
There was a problem hiding this comment.
현재 인터페이스 설계와 실제 구현 모두 id의 주체가 매우 혼동되어 있습니다 ㅠㅠ.
인터페이스만 보기엔 id가 BinaryContent 자체의 id 기준으로 동작되어야 합니다.
하지만 실제 구현은 검색에는 ownerId가, 삭제에는 BinaryContent의 고유 id 기준으로 동작하고 있습니다.
현재 구현에서는 아래와 같은 메서드명으로 변경하는게 적합하여 보입니다.
findProfileImageByOwnerId(UUID ownerId)
findAllAttachmentsByOwnerId(UUID ownerId)
Interface 만 보고도 동작이 예측 가능해야 합니다!
요구사항
기본 요구사항
Spring 프로젝트 초기화
Bean 선언 및 테스트
Spring 핵심 개념 이해하기
DiscodeitApplication에서는 @SpringBootApplication이 @componentscan을 통해 @component 계열을 찾아 Bean으로 등록하고
이를 IoC Container가 등록된 Bean을 통해 스스로 클래스들의 관계를 파악하여 객체의 생성, 초기화, 의존성 주입까지 스스로 동작하여 Service를 초기화를 진행함
DI 방법은 아래의 방법들이 있음
Lombok 적용
비즈니스 로직 고도화
추가 기능 요구사항
시간 타입 변경하기
새로운 도메인 추가하기
DTO 활용하기
UserService 고도화
AuthService 구현
ChannelService 고도화
MessageService 고도화
ReadStatusService 구현
UserStatusService 고도화
BinaryContentService 구현
새로운 도메인 Repository 구현체 구현
심화 요구사항
bean 다루기
스크린샷
User 정보 (profile 정보 포함)
Channel 정보 (public : 참여한 memberId 제외, private : 참여한 memberId)
Message 정보 (파일 포함)
FileRepository 구현체의 파일 저장 디렉토리
멘토에게