-
Notifications
You must be signed in to change notification settings - Fork 24
[조현아] sprint7 #202
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: 조현아
Are you sure you want to change the base?
The head ref may contain hidden characters: "\uC870\uD604\uC544-sprint7"
[조현아] sprint7 #202
Conversation
- yaml 파일 분리 - logback 파일 추가 - AOP class 생성 - 프론트 코드 변경
# Conflicts: # src/main/java/com/sprint/mission/discodeit/dto/request/MessageCreateRequest.java
- UserServiceTest - ChannelServiceTest - MessageServiceTest
- message content 검증 메서드 private으로 변경 - privateChannel 참여자Ids list로 변경
- Repository 테스트 작성 - Controller 테스트 작성 - AppConfig 수정, 기존 main에서 어노테이션 제거 - test-yaml 수정
@Pattern(regexp = "^(?:\\w+\\.?)*\\w+@(?:\\w+\\.)+\\w+$", | ||
message = "이메일 형식이 올바르지 않습니다.") |
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.
정규표현식도 나쁘진않지만 @Email
이라는 애노테이션이 이미 존재합니다
이미 있는걸 굳이 또 만드실 필요는 없으니 이걸 쓰시길 권장드립니다.
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.
넵 수정하겠습니다!!
@Mapping(target = "participants", expression = "java(resolveParticipants(channel))") | ||
@Mapping(target = "lastMessageAt", expression = "java(resolveLastMessageAt(channel))") | ||
abstract public ChannelDto toDto(Channel channel); |
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.
SpEL은 컴파일 시점에 정적 분석의 대상이 아니라서 코드에 문제가 있어도 발견되지 않아요. 그 상태로 배포가 나가서 실제 코드가 동작하는 순간에 확인이 되는데, 너무 리스크가 크다 생각합니다.
@Mapping
을 저는 써본적이 없어서 리뷰 달면서 알게되었는데.. 잘 쓰면 유용한 면도 있을것 같다고 생각하는데요. 그만큼 리스크도 커 보이네요.
개인적으로는 이걸 쓰는 것보다 이전 코드가 훨씬 더 나아보입니다.
|
||
return messageMapper.toDto(savedMessage); | ||
return messageMapper.toDto(message); |
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.
이전에 return messageMapper.toDto(savedMessage);
가 저장된 결과를 반환하는 거라 더 좋은 코드로 보여요.
지금 코드에서 message 안에는 id 값이 null일거에요.
savedUser.setUserStatus(userStatus); //연관 관계 설정 | ||
|
||
return userMapper.toDto(savedUser); | ||
return userMapper.toDto(user); |
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.
여기도 마찬가지로 userRepository.save(user);
의 반환값을 userMapper.toDto()
의 인자로 넣어주는게 적절해보이네요.
<rollingPolicy class="ch.qos.logback.core.rolling.TimeBasedRollingPolicy"> | ||
<fileNamePattern>${LOG_PATH}/${APP_NAME}-error.%d{yyyy-MM-dd}.log</fileNamePattern> | ||
<maxHistory>30</maxHistory> | ||
</rollingPolicy> |
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.
로깅 파일을 시간단위로 롤링하는 정책은 현업에서도 많이 쓰고 굉장히 잘 하신것 같아요.
하지만 파일이 너무 커져서 문제가 되는 경우도 있거든요.
그래서 보통은 최대 파일크기도 같이 설정으로 걸어둬요.
아래 파일 크기 + 시간 주기로 롤링하는 정책입니다.
<appender name="FILE" class="ch.qos.logback.core.rolling.RollingFileAppender">
<file>./logs/app.log</file>
<rollingPolicy class="ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy">
<fileNamePattern>./logs/app.%d{yyyy-MM-dd}.%i.log</fileNamePattern>
<maxFileSize>100MB</maxFileSize> <!-- 파일 하나가 100MB 넘으면 새로 만듦 -->
<maxHistory>30</maxHistory> <!-- 최대 30일 보관 -->
<totalSizeCap>5GB</totalSizeCap> <!-- 전체 합이 5GB 넘으면 오래된 것부터 삭제 -->
</rollingPolicy>
<encoder>
<pattern>${ROLLING_PATTERN}</pattern>
</encoder>
</appender>
// When & Then | ||
mockMvc.perform(post("/api/channels/public") | ||
.contentType(MediaType.APPLICATION_JSON) | ||
.content(requestJson)) | ||
.andExpect(status().isCreated()) | ||
.andExpect(jsonPath("$.type").value(ChannelType.PUBLIC.name())) | ||
.andExpect(jsonPath("$.name").value("채널1")) | ||
.andExpect(jsonPath("$.description").value("테스트 채널입니다.")) | ||
.andDo(print()); |
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.
when과 then은 명확히 구분하는게 더 좋습니다.
// When
ResultActions result = mockMvc.perform(post("/api/channels/public")
.contentType(MediaType.APPLICATION_JSON)
.content(requestJson));
// Then
result.andExpect(status().isCreated())
.andExpect(jsonPath("$.type").value(ChannelType.PUBLIC.name()))
.andExpect(jsonPath("$.name").value("채널1"))
.andExpect(jsonPath("$.description").value("테스트 채널입니다."))
.andDo(print());
PrivateChannelCreateRequest request = new PrivateChannelCreateRequest(participantIds); | ||
ChannelDto channelDto = new ChannelDto(UUID.randomUUID(), ChannelType.PRIVATE, null, null, | ||
null, null); | ||
|
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.
빈 공백 문자가 given, when, then에서 사용되기 때문 내부에서 불필요하게 공백쓰는건 지양하는게 더 가독성이 좋습니다.
// When & Then | ||
assertThatThrownBy(() -> userService.delete(userId)) | ||
.isInstanceOf(UserNotFoundException.class); | ||
|
||
then(userRepository).should().findById(userId); | ||
then(userRepository).should(never()).deleteById(any()); | ||
then(userStatusRepository).should(never()).deleteByUserId(any()); |
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.
// When
Throwable thrown = catchThrowable(() -> userService.delete(userId));
// Then
assertThat(thrown)
.isInstanceOf(UserNotFoundException.class);
then(userRepository).should().findById(userId);
then(userRepository).should(never()).deleteById(any());
then(userStatusRepository).should(never()).deleteByUserId(any());
이렇게 하시면 더 좋을거 같아요.
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.
넵 리뷰해주신부분 반영해서 수정하겠습니다!! 감사합니다!
전체적으로 잘 작성하신 것 같습니다. |
요구사항
기본
기본 요구사항
프로파일 기반 설정 관리
application-dev.yaml
,application-prod.yaml
파일을 생성하세요.로그 관리
@Slf4j
어노테이션을 활용해 로깅을 쉽게 추가할 수 있도록 구성하세요.application.yaml
에 기본 로깅 레벨을 설정하세요.info
레벨로 설정합니다.debug
, 운영 환경에서는info
레벨로 설정합니다.logback-spring.xml
파일을 생성하세요.다음 예시와 같은 로그 메시지를 출력하기 위한 로깅 패턴과 출력 방식을 커스터마이징하세요.
로그 출력 예시
콘솔과 파일에 동시에 로그를 기록하도록 설정하세요.
{프로젝트 루트}/.logs
경로에 저장되도록 설정하세요.로그 파일은 일자별로 롤링되도록 구성하세요.
로그 파일은 30일간 보관하도록 구성하세요.
예외 처리 고도화
커스텀 예외를 설계하고 구현하세요.
패키지명:
com.sprint.mission.discodeit.exception[.{도메인}]
ErrorCode
Enum 클래스를 통해 예외 코드명과 메시지를 정의하세요.아래는 예시입니다. 필요하다고 판단되는 다양한 코드를 정의하세요.
예시
모든 예외의 기본이 되는
DiscodeitException
클래스를 정의하세요.클래스 다이어그램
details
는 예외 발생 상황에 대한 추가정보를 저장하기 위한 속성입니다.DiscodeitException
을 상속하는 주요 도메인 별 메인 예외 클래스를 정의하세요.UserException
,ChannelException
등도메인 메인 예외 클래스를 상속하는 구체적인 예외 클래스를 정의하세요.
UserNotFoundException
,UserAlreadyExistException
등 필요한 예외를 정의하세요.예시
기존에 구현했던 예외를 커스텀 예외로 대체하세요.
NoSuchElementException
IllegalArgumentException
ErrorResponse
를 통해 일관된 예외 응답을 정의하세요.클래스 다이어그램
int status
: HTTP 상태코드String exceptionType
: 발생한 예외의 클래스 이름앞서 정의한
ErrorResponse
와@RestControllerAdvice
를 활용해 예외를 처리하는 예외 핸들러를 구현하세요.ErrorResponse
)을 가져야 합니다.유효성 검사
@NotNull
,@NotBlank
,@Size
,@Email
등@Valid
를 사용해 요청 데이터를 검증하세요.MethodArgumentNotValidException
을 전역 예외 핸들러에서 처리하세요.Actuator
Discodeit
1.7.0
17
3.4.0
/actuator/info
/actuator/metrics
/actuator/health
/actuator/loggers
단위 테스트
Mockito
를 활용해 Repository 의존성을 모의(mock)하세요.BDDMockito
를 활용해 테스트 가독성을 높이세요.슬라이스 테스트
메시지
MessageRepositoryTest 테스트 클래스
채널 정보, 유저 정보, 커서 정보, 거기에 약간의 메시지 더미 데이터
1. 채널 ID와 생성 시간으로 메시지를 페이징하여 조회할 수 있다
2. 채널의 마지막 메시지 시간을 조회할 수 있다
3. 메시지가 없는 채널에서는 마지막 메시지 시간이 없다
4. 채널의 모든 메시지를 삭제할 수 있다
3번은 채널이 존재하긴 하는데 메시지가 없는, 즉 채널을 만들어놓고 어떠한 채팅도 하지 않은 빈 채널을 테스트하는 것이고
4번은 특정 채팅방을 삭제하면, 그 채팅방에 존재하던 모든 메시지가 삭제되는지를 테스트하면 되겠죠?
@DataJpaTest
를 활용해 테스트를 구현하세요.application-test.yaml
을 생성하세요.test
프로파일을 활성화 하세요.@EnableJpaAuditing
을 추가하세요.@WebMvcTest
를 활용해 테스트를 구현하세요.WebMvcTest
에서 자동으로 등록되지 않는 유형의 Bean이 필요하다면@Import
를 활용해 추가하세요.예시
주요 컨트롤러(User, Channel, Message)에 대해 최소 2개 이상(성공, 실패)의 테스트 케이스를 작성하세요.
MockMvc를 활용해 컨트롤러를 테스트하세요.
서비스 레이어를 모의(mock)하여 컨트롤러 로직만 테스트하세요.
JSON 응답을 검증하는 테스트를 포함하세요.
통합 테스트
@SpringBootTest
를 활용해 Spring 애플리케이션 컨텍스트를 로드하세요.@Transactional
을 활용해 독립적으로 실행하세요.심화
주요 변경사항
스크린샷
멘토에게