[댓글 기능] 효오 미션 제출합니다.

[hyojaekim]

No description provided.

[woowahanCU]

오오! 좋네요 ㅎㅎ 💯

[woowahanCU]

인자가 달라질 경우에는 어떻게 작성하는 것이 좋을까요?

[woowahanCU]

customException을 작성하여 좀더 상세하게 예외상황을 전달하는 것이 좋을거 같아요.

[hyojaekim]

급하게 하느라 누락된 부분을 다시 반영하였습니다.!

[woowahanCU]

Article은 converter를 사용하지 않네요?

[woowahanCU]

시그니처를 맞춰주는 게 좋을거 같아요. ArticleService는 id를 리턴하고 Usersevice는 void 리턴인데요.
일반적으로 save를 하면 persistence entity를 리턴하니,
Service에서는 dto를 받아서 dto로 리턴하는 것은 어떨까요

[woowahanCU]

지금처럼 모든 method가 사용된다면 Class에 붙여두고 readOnly만 별도로 설정해줘도 되겠네요.

[woowahanCU]

메서드를 분리하여 재사용하는 것은 어떨까요

    @Transactional(readOnly = true)
    public ArticleDto findById(Long articleId) {
        return ArticleDto.of(findById(articleId));
    }

    @Transactional
    public UserDto findAuthor(long articleId) {
        User user = findById(articleId).getUser();
        return new UserDto(user.getEmail(),
                user.getName(),
                user.getPassword());
    }

    @Transactional(readOnly = true)
    public Article findById(Long articleId) {
        return articleRepository.findById(articleId)
                .orElseThrow(() -> new NotExistArticleIdException("존재하지 않는 Article 입니다."));
    }

[woowahanCU]

UserRepository를 직접 의존 주입하기 보다는, UserService에 메시지를 보내서 받는 것은 어떨까요

[woowahanCU]

SpringData의 pageable을 활용하는 것은 어떨까요

[woowahanCU]

input과 output이 명확해야 테스트하기가 용이합니다.
void 리턴하는 메서드가 비즈니스 로직에 해당할 경우 한번 고민해보세요.
권한체크와 관련한 단위 테스트 코드도 같이 작성해보는 것도 좋겠네요.

[woowahanCU]

CommentWriter를 찾는다기보다는 수정권한에 대한 체크를 담당하고 있네요.
그리고 이 메서드는 단일 책임 원칙 위반으로 보입니다.
아래 로직은 메서드로 분리하여 재사용하는 것은 어떨까요

[hyojaekim]

권한에 대한 체크를 하는 의미를 담기 위해서 메소드 명을 변경하였습니다.

[woowahanCU]

CommentService가 각 repository와 연관관계가 맺어져있는데요. 결합도를 낮출 수는 없을까요?

[woowahanCU]

예외가 발생하지 않네요.
times(2)로 둬야 테스트가 성공합니다.

[woowahanCU]

email, password의 제약을 잡지는 못하고 있네요.
User 생성자에서 유효성을 검증해야 좋을거 같아요.

[woowahanCU]

get, post 요청 등을 메서드 분리하거나 공통된 URI를 상수처리하여 중복을 없앨 수 있을거 같아요
body에 들어가는 부분도 메서드로 분리하면 재사용할 수 있지 않을까요

    private <T> BodyInserters.FormInserter<String> mapBy(Class<T> classType, String... parameters) {
        BodyInserters.FormInserter<String> body = BodyInserters.fromFormData(Strings.EMPTY, Strings.EMPTY);

        for (int i = 1; i < classType.getDeclaredFields().length; i++) {
            body.with(classType.getDeclaredFields()[i].getName(), parameters[i - 1]);
        }
        return body;
    }

[woowahanCU]

article에 Id가 없어 NPE가 발생하지 않을까요

[woowahanCU]

email이 없어서 NPE가 발생하네요.

[woowahanCU]

Interceptor에서 체킹하고 있어서 이 테스트는 깨지네요.

[woowahanCU]

안녕하세요! 리뷰어 CU입니다.
짧은 시간내에 깔끔하게 잘 작성하셨네요. 👍
몇가지 피드백 남겼으니 확인해보시고 궁금한 부분있으시면 DM 남겨주세요.

[woowahanCU]

현재는 8자리 이상이기만 하면 통과되네요.
email validator를 보시면 ConstraintValidator를 상속받아서 구현한 것을 볼 수 있습니다.
password의 경우에도 ConstraintValidator를 상속받아서 customValidator를 작성할 수 있어요.
그럼 도메인 모델의 구현코드에서 상당부분 해당 로직을 숨기고 비즈니스 로직만 표현이 가능해지겠네요.
참고만 해두세용 :)

https://www.baeldung.com/registration-password-strength-and-rules

[woowahanCU]

하나의 테스트는 하나의 개념만 담는 것이 좋다고 생각합니다.
현재는 email에 대한 유효성 위반인지 password에 대한 유효성 위반인지 해당 테스트로 확인이 불가하네요.
아래와 같이 작성하는 것은 어떨까요

    @Test
    void 올바르지_않은_password_User_생성() {
        String email = "woowahanCU@woowahan.com";
        String name = "cu";
        String password = "p";

        assertThat(getValidator(email, name, password))
                .extracting(ConstraintViolation::getMessage)
                .containsOnly("정규 표현식 \"^([a-zA-Z0-9!@#$%^&*]{8,})$\" 패턴과 일치해야 합니다.");
    }
...
    private Set<ConstraintViolation<User>> getValidator(String email, String name, String password) {
        return validator.validate(new User(email, name, password));
    }

[woowahanCU]

전체 테스트를 하면 사실상 처음 테스트에서 요청하여 저장된 값에 대해서만 테스트하는 구조네요.
이 후 테스트에서 수행하는 initialWork에서 추가했던 값은 사용하지 않을거 같아요.
auto increment때문에 이런 구성을 하셨을까요..

SpringBoot는 schema.sql, data.sql을 사용하여 DB를 초기화할 수 있어요. 이를 통해 더미데이터를 구성하여도 좋을거 같구요
https://docs.spring.io/spring-boot/docs/current/reference/html/howto-database-initialization.html#howto-initialize-a-database-using-spring-jdbc

유지보수 등의 이슈가 고민된다면 ApplicationRunner 등을 활용하여 Application이 구동되는 시점에 Repository 등의 의존성을 주입받은 Bean을 활용하여 데이터를 추가하는 방법도 있겠네요

https://yuien.tistory.com/entry/스프링-시작시점에서-프로그램-동작할-수-있게-하는-방법
https://www.daleseo.com/spring-boot-runners/

@Transactional
@Component
public class DataLoader {
    
    @Autowired
    private UserRepository userRepository;

    public void loadData() {
        loadUserData();
    }

    public void loadUserData() {
        ...
        userRepository.save(user);
    }
}

...

@Component
public class DataLoaderBootstrap implements ApplicationListener<ContextRefreshedEvent> {
    @Autowired
    private DataLoader dataLoader;

    @Override
    public void onApplicationEvent(ContextRefreshedEvent event) {
        dataLoader.loadData();
    }
}

[woowahanCU]

👍

[woowahanCU]

피드백 반영 잘해주셨네요! 👍
몇가지 피드백 남겼으니 확인해보시고 궁금한 점 있으면 DM 남겨주세요.
그리고 저번에 잘못 피드백 남겼던 부분이 있었는데요. 이상하다 싶으면 알려주세요 😅
이번 미션은 여기서 Merge할게요~