[Review] 2022/06/23 3차 PR (Issue Tracker)

이번 리뷰는 Brain 께서 해주셨다! 리뷰해주셔서 감사합니다 !!

https://github.com/codesquad-members-2022/issue-tracker/pull/124#issuecomment-1163833790

[Team-26][BE : Shine] Issue-Tracker 2주차 1회차 PR by zbqmgldjfh · Pull Request #124 · codesquad-members-2022/issue-tracke

 

1. 질문

과연 API에서의 Redirection 처리는 어떻게 해야할까?

이에 대하여 다음과 같이 답변해주셨다.

우선 회사에서 BE는 200번대 응답만 해주면 되는 것 같다!

Redirection 처리는 FE팀이 해주면 될것같다는 결론이다!!

 

2. 코드 리뷰

2-1) Nginx 컴파일 설치의 장점

apt-get을 통해 설치하는 방식은 NGINX 공식 repository를 통해 다운받기 때문에 안전하다고 합니다.
또한 페키지메니저를 통해 설치할 경우, Nginx를 사용하는 port를 방화벽에서 따로 열어줄 필요 없이 사용하는 포트들을 자동 등록하며, 로그로테이트 설정이 자동으로 되기 때문에 편하다는 장점이 있습니다.

 

반대로 직접 컴파일 해서 사용할경우 기본패키지에 추가되어있는 모듈 외의 추가적인 모듈을 사용하고 싶다면 소스파일을 추가하여 컴파일 할수 있다는 장점이 있다고 합니다.

기업들 같은 경우 자기 회사만의 특이사항을 만족시킬수 있어야 할탠데, 이런 면에서 장점이 있다고 생각합니다!!
https://extrememanual.net/9910

우분투 NGINX 컴파일 설치 방법 - 익스트림 매뉴얼

 

2-2) Equals로 재정의하기

이게 내가 생각을 잘못한것이... long (Primitive) 타입이라 생각해서 == 로 비교 했는데, Long으로 사용중이였다 ㅎㅎ

객체간의 비교는 equals()를 사용하는것이 좋다!

 

2-3) 변경 감지기능으로의 저장

Issue와 Comment 는 1:N의 관계인데, Comment는 Issue에 종속되어 있다.

@OneToMany(mappedBy = "issue", cascade = CascadeType.ALL, orphanRemoval = true)
private List<Comment> comments = new ArrayList<>();

 

따라서 부모인 Issue를 불러와 comment를 add해주면 변경감지로 인하여 Transaction이 끝날 시점에 flush가 되어 저장될것이라는 것이 나의 생각이였다.

 

이에 따른 테스트 코드를 다음과 같이 작성하였다.

@Test
@DisplayName("Issue에 1글자 이상의 댓글을 정상 등록시킨다.")
public void addComment_ValidContent_Success() {
    // given
    Issue testIssue = Issue.createBasic("test1");
    issueRepository.save(testIssue);

    User testUser = new User("test user", "zbqmgldjfh@gmail.com", "url");
    userRepository.save(testUser);

    Comment comment = Comment.builder()
            .description("댓글")
            .issue(testIssue)
            .user(testUser)
            .build();

    // when
    commentService.add(testIssue.getId(), comment.getDescription(), testUser.getEmail());
    em.flush();
    em.clear();

    // then
    List<Comment> comments = testIssue.getComments();
    assertThat(comments.size()).isEqualTo(1);
    assertThat(comments.get(0).getDescription()).isEqualTo("댓글");
}

 

3. 팀원들이 받은 리뷰

3-1) 날짜 관련 전역 설정하기

나도 처음 알게된 내용이며, 나또한 지금까지 모든 부분에서 @DataTimeFormat을 위와같이 설정해주고 있었다.

 

Date Type이 java.util.Date 이라면 application 설정에서 spring.jackson.date-format 지정으로 변경 관리할 수 있습니다.

이러한 방식은 java.util.Date or the java.util.Calendar 에서 사용할수가 있습니다!

 

하지만 현재 데이터 포맷은 LocalDate 이라 적용되지 않습니다.(LocalDateTime 도 마찬가지)
따라서 java.time.* 클래스 용도를 별도로 추가해주어야 합니다.

jackson2ObjectMapperBuilder 의 빈을 생성하고 ObjectMapper에 직렬화 포맷을 넣어 해결할 수 있다고 한다.

@Configuration
public class ContactAppConfig {

    private static final String dateFormat = "yyyy-MM-dd";
    private static final String dateTimeFormat = "yyyy-MM-dd HH:mm:ss";

    @Bean
    public Jackson2ObjectMapperBuilderCustomizer jsonCustomizer() {
        return builder -> {
            builder.simpleDateFormat(dateTimeFormat);
            builder.serializers(new LocalDateSerializer(DateTimeFormatter.ofPattern(dateFormat)));
            builder.serializers(new LocalDateTimeSerializer(DateTimeFormatter.ofPattern(dateTimeFormat)));
        };
    }

}

https://www.baeldung.com/spring-boot-formatting-json-dates

Formatting JSON Dates in Spring Boot | Baeldung

 

3-2) 도메인을 더생각한 네이밍 생각해보기

제 3자의 입장에서 MilestoneController에서 받는 status가 무엇일까? 란 생각을 해봤다.

최종적으로 "아! close, open"이겠구나 라고 생각은 했는데, 한번에 떠오르지는 않았다.

 

좀더 적절한 도메인 네이밍을 찾도록 노력해보자!!

 

3-3) 검증이 필요한 상황

Q) 사용자 입력(request body 또는 PathVariable 또는 Query Parameter) 의 유효성 검사는 주로 어디서 하는가?

1. 컨트롤러에서만 한다. (객체라면 @Valid, 위 경우 milestoneId가 음수인지 체크를 컨트롤러에서 한다. )
2. 서비스에서 한다
3. 컨트롤러, 서비스 양쪽에서 모두한다.
4. 그때끄때 달라요~

Dion은 4번 정답이 없는 부분이라고 하셨다. 이런 부분은 항상 선택의 기로에서 고민을 많이해야하는 것 같다.

 

3-4) Builder가 꼭 필요한가?

사실 나도 Builder를 즐겨 쓰기는 했는데, 한번쯤 고민해볼 문제인것 같다.

 

우선 나도 방어적인 Builder패턴을 적용하는 방식으로, 예를 들어 Assert.notNull() 과 같이 생성자 부분에 한번은 막을수 있도록 처리한 후,Builder를 사용한다.

이렇게 방어적으로 사용하면 Null값이 전달되는 빌더페턴의 문제를 막을수가 있다.

 

다만 위와 같이 간단한 부분에서 빌더가 필요할까?

인자 5개라? 이정도는 생성자를 통해서 넘겨줄만 한걸까?

사실 내 기준상 생성자를 통해 생성기에 적절한 인자의 수는 3개까지가 적당하고 생각은 하는데....

따라서 나였어도 위와같이 Builder를 사용했을것 같다만...

3-5) Entity에서 DTO를 모르도록

보통 나같은 경우 DTO에서는 Entity를 알고있고, Entity는 DTO를 모르도록 설계를 한다.

Entity는 최대한 다른곳에 대한 의존성 없는 순수한 상태를 유지해야 한다 생각한다.

 

이런 면에서 나는 RestAPI를 만들때도 Swagger를 사용하기도 매우 싫어 RestDocs를 사용한다.

 

위 코드는 Entity 내부에 DTO를 받아 Entity로 변환하는 과정인데,

1) DTO를 직접 받기 보다는, 값을 전부 꺼내서 받는다.

2) DTO 자체에 Entity 변환 ObjectMapper를 처리한다

 

위와같은 방식으로 처리할 것 같다.

 

3-6) 테스트 마다의 DB의 독립성 보장하기

음 일단 나는 이부분을 상요하고 있지 않았었는데, 쿠킴 덕분에 좋은 내용을 배워간다.

일단 두분의 대화 내용은 다음과 같다.

 

그럼 어떤문제 떄문에 이러한 방식을 사용해야 할까?

서비스 코드에는 @Transactional 을 이용하지 않고 테스트 코드에만 @Transactional 을 이용한다면 개발자의 실수로 런타임에서만 LazyInitailizationException이 발생하는 문제가 생길수 있다.

 

이러한 문제를 막기 위해 애당초 test 코드에서도 @Transactional을 없에서 Test코드에서도 일괄적으로 에러가 발생하게 만드는 것 이다.

다만 이렇게 되면 테스트코드가 영속성 컨텍스트 안에 있지 않다보니 문제가 발생한다...

 

지연로딩시 문제가 된다.

이러한 부부은 Fetch Join으로 해결할수가 있다.

 

https://dev.to/henrykeys/don-t-use-transactional-in-tests-40eb

Don’t Use @Transactional in Tests

https://javabom.tistory.com/103

JPA 사용시 테스트 코드에서 @Transactional 주의하기