Conversation
| boot: | ||
| admin: | ||
| client: | ||
| url: http://localhost:9091 |
There was a problem hiding this comment.
요구사항에서는 포트번호를 9090으로 하라고 되어있는데 9091로 하신 이유가 있을까요?
There was a problem hiding this comment.
이전에 스프링부트 애플리케이션 모니터링을 위해서 프로메테우스와 그라파나를 사용한 적이 있는데 프로메테우스의 기본 포트가 9090 포트라서 충돌이 발생해서 9091로 설정했습니다.
| dialect: org.hibernate.dialect.PostgreSQLDialect | ||
|
|
||
| server: | ||
| port: 8080 |
There was a problem hiding this comment.
application-prod.yaml에 없는 설정은 application.yaml에서 가져오기 때문에 profile을 prod로 실행해도 일부 설정은 들어올거에요.
특히, 로깅관련해서 Debug레벨까지 로깅하는거랑 데이터 바인드까지 하는건 성능상이나 보안상 문제가 될 수 있어보여요.
https://github.com/codeit-bootcamp-spring/3-sprint-mission/pull/194/files
application.yaml을 없애시거나 아니면 최소화해주세요.
그리고 prod의 설정은 최대한 보수적으로 설정해주세요.
그렇게 하는게 application-prod.yaml만 봐도 이행할 수 있어서 명식적이고 실수할 여지가 적어보여요.
| </encoder> | ||
| </appender> | ||
|
|
||
| <!-- 비즈니스 파일 로그 (business.log) --> |
There was a problem hiding this comment.
비즈니스 파일 로그는 어떤 경우에 남기는건가요? 잘 이해가 안되서 여쭤봅니다.
구분하신 의도도 설명해주시면 감사드립니다.
There was a problem hiding this comment.
UserService, MessageService, ChannelService에 있는 주요 비즈니스 로직을 로깅하기 위해 분리했습니다!
| <appender name="APP_FILE" class="ch.qos.logback.core.rolling.RollingFileAppender"> | ||
| <file>${LOG_PATH}/${LOG_FILE_NAME}-application.log</file> | ||
| <rollingPolicy class="ch.qos.logback.core.rolling.TimeBasedRollingPolicy"> | ||
| <fileNamePattern>${LOG_PATH}/${LOG_FILE_NAME}-application-%d{yyyy-MM-dd}.log</fileNamePattern> | ||
| <maxHistory>30</maxHistory> | ||
| </rollingPolicy> | ||
| <encoder> | ||
| <pattern>${FILE_LOG_PATTERN}</pattern> | ||
| <charset>UTF-8</charset> | ||
| </encoder> | ||
| </appender> |
There was a problem hiding this comment.
로깅 파일을 시간단위로 롤링하는 정책은 현업에서도 많이 쓰고 굉장히 잘 하신것 같아요.
하지만 파일이 너무 커져서 문제가 되는 경우도 있거든요.
그래서 보통은 최대 파일크기도 같이 설정으로 걸어둬요.
아래 파일 크기 + 시간 주기로 롤링하는 정책입니다.
<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>
There was a problem hiding this comment.
로깅이 쌓여서 로깅 파일 사이즈가 커진다는 생각을 미처 생각하지 못했었네요. 참고하겠습니다!
|
|
||
| @Test | ||
| @DisplayName("Private 채널 등록 프로세스가 모든 계층에서 올바르게 동작해야 한다.") | ||
| void completeCreatePrivateChannelIntegration() { |
There was a problem hiding this comment.
when 절은 최대한 짧게 작성하는게 좋습니다. 가급적 1줄로 작성하는게 제일 좋습니다. 과학 실험을 할때도 최대한 환경을 통제해놓고 한가지의 변경만 가하면서 변화들을 확인하잖아요. 마찬가지 이유로 then 부분은 최대한 심플하게 하는게 좋습니다.
given-when-then을 좀 더 명확하게 구분하기 위해서 각 절 내부에서는 빈 공백 줄바꿈은 최소화하고 절이 넘어갈때만 사용하는게 좋습니다.
@Test
@DisplayName("Private 채널 등록 시 채널, 참여자, ReadStatus가 올바르게 생성된다.")
void createPrivateChannel_shouldPersistAllRelatedData() {
// given
PrivateChannelDto request = new PrivateChannelDto(List.of(userId1, userId2));
// when
ChannelResponseDto result = channelService.createPrivateChannel(request);
// then
assertNotNull(result);
assertEquals(ChannelType.PRIVATE, result.type());
assertEquals(userId1, result.participants().get(0).id());
assertNull(result.lastMessageAt(), "초기에는 메시지가 없어야 하므로 lastMessageAt은 null이어야 합니다.");
Optional<Channel> foundChannel = channelRepository.findById(result.id());
assertTrue(foundChannel.isPresent(), "채널이 저장되어 있어야 합니다.");
List<ReadStatus> readStatuses = readStatusRepository.findAllByUserId(userId1);
assertEquals(1, readStatuses.size(), "유저 1명의 ReadStatus가 정확히 하나 생성되어야 합니다.");
}
| mockMvc.perform(post("/api/channels/public") | ||
| .contentType(MediaType.APPLICATION_JSON_VALUE) | ||
| .content(objectMapper.writeValueAsString(request))) | ||
| .andExpect(status().isCreated()) | ||
| .andExpect(jsonPath("$.id").value(channelId.toString())) | ||
| .andExpect(jsonPath("$.name").value("public")) | ||
| .andExpect(jsonPath("$.description").value("test channel")); |
There was a problem hiding this comment.
여기는 When과 then이 섞여있는걸로 보여요. 분리해주세요.
| Channel savedChannel = channelRepository.save(channel); | ||
| Optional<Channel> foundChannel = channelRepository.findById(savedChannel.getId()); |
There was a problem hiding this comment.
저장을 테스트 하고 싶으신건지, 조회를 테스트하고 싶은건지 명확하게 할 필요가 있다 생각합니다.
저장을 테스트하고 싶은거면 when에는 Channel savedChannel = channelRepository.save(channel); 남겨두시고
조회를 테스트하고 싶으신거면 Channel savedChannel = channelRepository.save(channel);을 given으로 옮겨주세요.
when 절에 이렇게 두가지 이상의 행동을 하는건 테스트의 변수를 너무 많이 만들어 이해하기 어렵고 테스트를 복잡하게 만듭니다.
| given(userRepository.findById(notExistId)).willReturn(Optional.empty()); | ||
|
|
||
| // when | ||
| assertThrows(NotFoundUserException.class, () -> userService.findById(notExistId)); |
There was a problem hiding this comment.
이것도 when이 아니라 when, then이라 생가됩니다. 아래처럼 고쳐주세요.
@Test
@DisplayName("등록되지 않은 ID로 사용자를 조회하면 NotFoundUserException이 발생해야 한다.")
void givenNonexistentUserId_whenFindById_thenThrowNotFoundUserException() {
// given
UUID notExistId = UUID.randomUUID();
given(userRepository.findById(notExistId)).willReturn(Optional.empty());
// when
Throwable thrown = catchThrowable(() -> userService.findById(notExistId));
// then
assertThat(thrown)
.isInstanceOf(NotFoundUserException.class)
.hasMessageContaining("사용자");
verify(userRepository).findById(notExistId);
verifyNoInteractions(userStatusRepository, userMapper); // 사용자 없으면 이후 로직 없어야 함
}
|
전체적으로 잘 작성해주셨습니다. 테스트 코드에서 when절에 then이 섞여 있는 코드가 일부 있습니다. 제가 일일이 다 코멘트 달지 않았습니다. 작성하셨던 코드 다시 확인 해보시면서 when절에 then이 섞여 있는거 아닌지 고민해봐주시면 감사드립니다. yaml 파일도 application.yaml의 설정이 profile을 "prod"로 설정해도 포함될 수 있다는 걸 주의하셔야 할 것 같아요. |
|
피드백 감사합니다! 테스트 코드 작성 부분은 아직 익숙하지 않아서 when then이 섞여 있는 부분도 많고 목적이 애매한 테스트 코드를 많이 작성한 것 같습니다. 리뷰 주신 부분은 미션8 pr에 커밋해서 올리겠습니다 |
기본 요구사항
프로파일 기반 설정 관리
application-dev.yaml,application-prod.yaml파일을 생성하세요.로그 관리
Lombok의
@Slf4j어노테이션을 활용해 로깅을 쉽게 추가할 수 있도록 구성하세요.application.yaml에 기본 로깅 레벨을 설정하세요.info레벨로 설정합니다.환경 별 적절한 로깅 레벨을 프로파일 별로 설정해보세요.
debug, 운영 환경에서는info레벨로 설정합니다.Spring Boot의 기본 로깅 구현체인 Logback의 설정 파일을 구성하세요.
logback-spring.xml파일을 생성하세요.다음 예시와 같은 로그 메시지를 출력하기 위한 로깅 패턴과 출력 방식을 커스터마이징하세요.
로그 출력 예시
콘솔과 파일에 동시에 로그를 기록하도록 설정하세요.
{프로젝트 루트}/.logs경로에 저장되도록 설정하세요.로그 파일은 일자별로 롤링되도록 구성하세요.
로그 파일은 30일간 보관하도록 구성하세요.
서비스 레이어와 컨트롤러 레이어의 주요 메소드에 로깅을 추가하세요.
로깅 레벨을 적절히 사용하세요: ERROR, WARN, INFO, DEBUG
다음과 같은 메소드에 로깅을 추가하세요:
예외 처리 고도화
com.sprint.mission.discodeit.exception[.{도메인}]DiscodeitException클래스를 정의하세요.-[x]
DiscodeitException을 상속하는 주요 도메인 별 메인 예외 클래스를 정의하세요.-[x] 도메인 메인 예외 클래스를 상속하는 구체적인 예외 클래스를 정의하세요.
UserNotFoundException,UserAlreadyExistException등 필요한 예외를 정의하세요.-[x] 기존에 구현했던 예외를 커스텀 예외로 대체하세요.
-
NoSuchElementException-
IllegalArgumentException- …
ErrorResponse를 통해 일관된 예외 응답을 정의하세요.ErrorResponse와@RestControllerAdvice를 활용해 예외를 처리하는 예외 핸들러를 구현하세요.ErrorResponse)을 가져야 합니다.유효성 검사
@NotNull,@NotBlank,@Size,@Email등@Valid를 사용해 요청 데이터를 검증하세요.MethodArgumentNotValidException을 전역 예외 핸들러에서 처리하세요.Actuator
/actuator/info/actuator/metrics/actuator/health/actuator/loggers단위 테스트
Mockito를 활용해 Repository 의존성을 모의(mock)하세요.BDDMockito를 활용해 테스트 가독성을 높이세요.슬라이스 테스트
test프로파일을 활성화 하세요.@EnableJpaAuditing을 추가하세요.커스텀 쿼리 메소드
페이징 및 정렬 메소드
컨트롤러 레이어의 슬라이스 테스트를 작성하세요.
@WebMvcTest를 활용해 테스트를 구현하세요.WebMvcTest에서 자동으로 등록되지 않는 유형의 Bean이 필요하다면 @import를 활용해 추가하세요.
통합 테스트
@SpringBootTest를 활용해 Spring 애플리케이션 컨텍스트를 로드하세요.@Transactional을 활용해 독립적으로 실행하세요.심화 요구사항
MDC를 활용한 로깅 고도화
MDCLoggingInterceptorcom.**.discodeit.configDiscodeit-Request-IDWebMvcConfigcom.**.discodeit.configSpring Boot Admin을 활용한 메트릭 가시화
Spring Boot Admin 서버를 구현할 모듈을 생성하세요.
admin 모듈의 메인 클래스에 @EnableAdminServer 어노테이션을 추가하고, 서버는 9091번 포트로 설정합니다.
admin서버 실행 후 localhost:9090/applications 에 접속해봅니다.discodeit 프로젝트에 Spring Boot Admin Client를 적용합니다.
-[x] 의존성 추가
테스트 커버리지 관리
build/reports/jacoco경로에서 확인할 수 있습니다.com.sprint.mission.discodeit.service.basic패키지에 대해서 60% 이상의 코드 커버리지를 달성하세요.스크린샷
멘토에게