Conversation
이번에 받은 basic code로 부터 시작
|
|
||
| ### 숨김 파일 ### | ||
| .* | ||
| !.gitignore |
There was a problem hiding this comment.
log 파일들이나 관련 디렉토리는 커밋에 불필요하게 추가되는걸 막아야될 것 같아요.
희준님이 잘하셔서 링크 남깁니다.
https://github.com/codeit-bootcamp-spring/3-sprint-mission/pull/193/files#diff-bc37d034bad564583790a46f19d807abfe519c5671395fd494d8cce506c42947R29
| } | ||
|
|
||
| private Optional<BinaryContentCreateRequest> resolveProfileRequest(MultipartFile profileFile) { | ||
| if (profileFile.isEmpty()) { |
There was a problem hiding this comment.
resolveProfileRequest를 호출하는 쪽에서 Optional.ofNullable에서 꺼내서 호출해서요.
MultipartFile profileFile가 null인 경우도 고려는 해야할 것 같아요.
|
|
||
| public record LoginRequest( | ||
| @NotBlank | ||
| @NotNull |
There was a problem hiding this comment.
@NotBlank 내부에 이미 @NotNull이 포함되어 있어요. 그래서 @NotNull은 붙이실 필요 없으십니다.
| @RestControllerAdvice | ||
| public class GlobalExceptionHandler { | ||
|
|
||
| private final View error; |
There was a problem hiding this comment.
이게 쓰이는 부분이 없는것 같은데.. 왜 있는건가요?
| @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.
SpEL은 컴파일 시점에 정적 분석의 대상이 아니라서 코드에 문제가 있어도 발견되지 않아요. 그 상태로 배포가 나가서 실제 코드가 동작하는 순간에 확인이 되는데, 너무 리스크가 크다 생각합니다.
@Mapping을 저는 써본적이 없어서 리뷰 달면서 알게되었는데.. 잘 쓰면 유용한 면도 있을것 같다고 생각하는데요. 그만큼 리스크도 커 보이네요.
개인적으로는 이걸 쓰는 것보다 그냥 변환하는 코드를 작성하는게 훨씬 더 나아보입니다.
| + "FROM Message m " | ||
| + "WHERE m.channel.id = :channelId " | ||
| + "ORDER BY m.createdAt DESC LIMIT 1") | ||
| Optional<Instant> findLastMessageAtByChannelId(@Param("channelId") UUID channelId); |
There was a problem hiding this comment.
이것도
Optional<Message> findTopByChannelIdOrderByCreatedAtDesc(UUID channelId);
이렇게 작성하고 사용하는 쪽에서 아래와 같이 사용하는걸 권장드려요.
messageRepository.findTopByChannelIdOrderByCreatedAtDesc(channelId)
.map(Message::getCreatedAt);
단일 row의 경우, 전체 컬럼을 가져오는 것과 특정 컬럼만 가져오는 것 간의 성능 차이는 미미하고, JPA의 영속성 컨텍스트를 고려하면 전체 엔티티를 가져오는 것이 재사용성과 캐시 활용 측면에서 더 유리할 수 있어요.
|
|
||
| channelRepository.save(channel); | ||
| log.info("[Public 채널 생성 성공] 채널 이름 : {} ", name); | ||
| return channelMapper.toDto(channel); |
There was a problem hiding this comment.
channel의 id를 알수가 없어서 channel로 반환하면 안될거 같아요.
channelRepository.save(channel);에서 반환하는 값을 channelMapper.toDto()에 넣어서 반환해야해요. 그래야 id가 있을거에요.
| org.hibernate.SQL: debug | ||
| org.hibernate.orm.jdbc.bind: trace | ||
| com.sprint.mission.discodeit: info |
There was a problem hiding this comment.
운영 환경에서는 Debug랑 bind 옵션은 꺼야돼요.
성능상 문제가 되기도 하고 보안상 문제가 될 수 있습니다.
| <appender name="RollingFile" class="ch.qos.logback.core.rolling.RollingFileAppender"> | ||
| <file>${LOG_PATH}/discodeit.log</file> | ||
|
|
||
| <rollingPolicy class="ch.qos.logback.core.rolling.TimeBasedRollingPolicy"> | ||
| <fileNamePattern>${LOG_PATH}/discodeit.%d{yyyy-MM-dd}.log</fileNamePattern> | ||
| <maxHistory>30</maxHistory> | ||
| </rollingPolicy> | ||
|
|
||
| <encoder> | ||
| <pattern>${LOG_PATTERN}</pattern> | ||
| </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>
|
|
||
| @Test | ||
| @DisplayName("POST /channels - case : success") | ||
| void createChannelSuccess() throws Exception { |
There was a problem hiding this comment.
고치시야할게 몇개 보여서 여기에 한번에 다 남기겠습니다.
- 테스트 코드를 작성하실때, // given, // when, // then을 주석으로 달아서 명확하게 구분해주세요. 코드 가독성에 도움이 됩니다.
- given, when, then 절을 구분할때는 빈 공백 문자로 해줍니다. 그리고 각 절 내부에서는 불필요한 빈 공백 문자는 사용을 자제합니다. 마찬가지로 코드 가독성에 영향을 줍니다. 아래처럼 바꿔주세요.
- 아래 코드 부분은 when절과 then절이 섞여 있는걸로 보입니다.
// when
ResultActions result = mockMvc.perform(post("/api/channels/private")
.contentType(MediaType.APPLICATION_JSON)
.content(requestBody));
// then
result.andExpect(status().isCreated())
.andExpect(jsonPath("$.type").value(ChannelType.PRIVATE.name()));
- 여기에서
when()때문에 이부분을 when절로 착각하신것 같습니다. 하지만 제가 읽기론 이 테스트코드는 controller를 테스트하는 코드로 보입니다. 그렇다면 테스트 대상은mockMvc.perform(post("/api/channels/private")이 코드 부분이어야합니다.when()은 오히려 테스트를 위한 준비단계에 해당하므로 given절에 있는게 적절합니다.
There was a problem hiding this comment.
다른 코드 부분에도 비슷한 문제들이 있는데 굳이 코멘트 안 남기겠습니다. 확인 후 변경 부탁드립니다.
There was a problem hiding this comment.
넵 피드백 해주신 내용 최대한 spritn8에 반영해보도록 하겠습니다!
|
전체적으로는 잘 작성해주셨습니다. given: 테스트를 위한 준비 단계 |
기본 요구사항
프로파일 기반 설정 관리
로그 관리
Lombok의 @slf4j 어노테이션을 활용해 로깅을 쉽게 추가할 수 있도록 구성하세요.
application.yaml에 기본 로깅 레벨을 설정하세요.
기본적으로 info 레벨로 설정합니다.
환경 별 적절한 로깅 레벨을 프로파일 별로 설정해보세요.
SQL 로그를 보기위해 설정했던 레벨은 유지합니다.
우리가 작성한 프로젝트의 로그는 개발 환경에서 debug, 운영 환경에서는 info 레벨로 설정합니다.
Spring Boot의 기본 로깅 구현체인 Logback의 설정 파일을 구성하세요.
logback-spring.xml 파일을 생성하세요.
다음 예시와 같은 로그 메시지를 출력하기 위한 로깅 패턴과 출력 방식을 커스터마이징하세요.
로그 출력 예시
패턴
{년}-{월}-{일} {시}:{분}:{초}:{밀리초} [{스레드명}] {로그 레벨(5글자로 맞춤)} {로거 이름(최대 36글자)} - {로그 메시지}{줄바꿈}
예시
25-01-01 10:33:55.740 [main] DEBUG c.s.m.discodeit.DiscodeitApplication - Running with Sp
콘솔과 파일에 동시에 로그를 기록하도록 설정하세요.
파일은 {프로젝트 루트}/.logs 경로에 저장되도록 설정하세요.
로그 파일은 일자별로 롤링되도록 구성하세요.
로그 파일은 30일간 보관하도록 구성하세요.
서비스 레이어와 컨트롤러 레이어의 주요 메소드에 로깅을 추가하세요.
로깅 레벨을 적절히 사용하세요: ERROR, WARN, INFO, DEBUG
다음과 같은 메소드에 로깅을 추가하세요:
사용자 생성/수정/삭제
채널 생성/수정/삭제
메시지 생성/수정/삭제
파일 업로드/다운로드
[ ]
예외 처리 고도화
커스텀 예외를 설계하고 구현하세요.
패키지명: 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 등
Actuator
Spring Boot Actuator 의존성을 추가하세요.
기본 Actuator 엔트포인트를 설정하세요.
health, info, metrics, loggers
Actuator info를 위한 애플리케이션 정보를 추가하세요.
애플리케이션 이름: Discodeit
애플리케이션 버전: 1.7.0
자바 버전: 17
스프링 부트 버전: 3.4.0
주요 설정 정보
데이터소스: url, 드라이버 클래스 이름
jpa: ddl-auto
storage 설정: type, path
multipart 설정: max-file-size, max-request-size
Spring Boot 서버를 실행 후 각종 정보를 확인해보세요.
/actuator/info
/actuator/metrics
/actuator/health
/actuator/loggers
단위 테스트
###슬라이스 테스트
통합 테스트
심화 요구사항
MDC를 활용한 로깅 고도화
Spring Boot Admin을 활용한 메트릭 가시화
admin 모듈의 메인 클래스에 @EnableAdminServer 어노테이션을 추가하고, 서버는 9090번 포트로 설정합니다.
admin 서버 실행 후 localhost:9090/applications 에 접속해봅니다.
discodeit 프로젝트에 Spring Boot Admin Client를 적용합니다.
의존성을 추가합니다.
dependencies {
...
implementation 'de.codecentric:spring-boot-admin-starter-client:3.4.5
}
application.yml
spring:
application:
name: discodeit
...
boot:
admin:
client:
instance:
name: discodeit
...
application-dev.yml
spring:
application:
name: discodeit
...
boot:
admin:
client:
url: http://localhost:9090
...
application-prod.yml
spring:
application:
name: discodeit
...
boot:
admin:
client:
url: ${SPRING_BOOT_ADMIN_CLIENT_URL}
...
discodeit 서버를 실행하고, admin 대시보드에 discodeit 인스턴스가 추가되었는지 확인합니다.
admin 대시보드 화면을 조작해보면서 각종 메트릭 정보를 확인해보세요.
주요 API의 요청 횟수, 응답시간 등
서비스 정보
테스트 커버리지 관리
스크린 샷
멘토님에게
이번 미션은 base 코드를 기반으로 0-sprint-mission-s7-base에 작성하였습니다!