[우아한명지코스] 이성은 Spring Core 배포 7, 8, 9 단계 미션 제출합니다.

[selee1012]

안녕하세요 우아한명지코스 이성은입니다. 리뷰 요청이 조금 늦어진 점 죄송합니다.
배우고 고민해 본 부분들 작성해 보았습니다.

배운 부분

• 이번 단계에서 @configuration, @bean, @value, @Profile, @activeprofiles 등 다양한 어노테이션을 익혔습니다.
• @component로 등록하던 빈을 @configuration + @bean 구조로 나눌 수 있다는 걸 배웠습니다.
• CommandLineRunner로 시작 시점에 초기 데이터를 넣는 DataLoader 개념과, @Profile로 특정 환경에만 필요한 데이터를 적용하는 방법을 배웠습니다 -> 이때 @Profile을 지정하지 않은 빈은 공통적으로 필요한 부분이라고 이해했는데 맞을까요?

공부하고 싶은 부분

• 이번에 작성한 배포 스크립트를 바탕으로 CI/CD 파이프라인을 구축해 볼 수 있지 않을까 생각해 보았습니다

고민한 점

• 시크릿 키는 한 번 정해지면 바뀌면 안 된다고 생각해 불변(final)으로 두고 싶었습니다. 그런데 @value를 필드에 직접 주입하면 가변 필드가 되어 final을 쓸 수 없었습니다. 그래서 JwtConfig의 @bean 메서드 파라미터로 값을 받아 JwtUtils 생성자로 넘기고, JwtUtils에서 final 필드로 보관해 불변을 지켰습니다.
• 테스트 때문에 분리해서 구현했지만 @component가 아니라 "@configuration + @bean 구조가 이 상황에서 맞을까?"도 고민했습니다. -> @component 구조가 더 간단하다고 느꼈는데 @configuration + @bean 구조의 장점이 보이지 않는다면 @component를 쓰는게 맞다고 생각했는데 사실 이 부분에서 분리했을때의 장점을 못 느꼈습니다. 이 부분에서 과제랑 별개로 실제 구현 관점에서 어떻게 생각하시는지 여쭙고 싶습니다.

[Minuring]

안녕하세요 성은님! 미션 잘 진행해 주셨네요.
제 의견이 틀렸을 수 있으니, 같이 토론한다 생각하고 진행해주시면 좋을 것 같아요!

---

@Profile을 지정하지 않은 빈은 공통적으로 필요한 부분이라고 이해했는데 맞을까요?

네, 프로필에 상관없이 공통적으로 사용되는 빈들은 @Profile을 지정하지 않을테니까요!

---

(생략) 그래서 JwtConfig의 @bean 메서드 파라미터로 값을 받아 JwtUtils 생성자로 넘기고, JwtUtils에서 final 필드로 보관해 불변을 지켰습니다.

원하신대로 불변도 지켰고, 오히려 더 좋은 설계라고 생각합니다.

1. JWTUtils를 테스트할 때 시크릿 키를 완전히 통제할 수 있습니다.
2. 스프링에 대한 의존이 사라짐 -> 스프링 없이 단위 테스트 가능

---

@Component 구조가 더 간단하다고 느꼈는데 ...(생략)... 이 부분에서 과제랑 별개로 실제 구현 관점에서 어떻게 생각하시는지 여쭙고 싶습니다.

@Component를 사용하는 경우

지금 미션에서 정의한 컴포넌트들처럼,
직접 수정할 수 있고(@Component를 컴파일할 수 있고), 생성과정이 단순하다면 @Component가 당연히 편리하고 직관적일 것 같습니다.

@Configuration과 @Bean을 사용하는 경우

1. 외부 라이브러리같은 경우 직접 코드에 @Component를 삽입할 수 없습니다.
   이런 경우 해당 라이브러리의 컴포넌트를 빈으로 등록하기 위해 @Configuration, @Bean이 필요할 것 같아요. 7단계 요구사항이 의도한 상황이 이런 상황으로 보이네요.

2. 또는 생성 과정이 복잡한 경우가 있을 것 같습니다. 예시 코드 출처

@Configuration
public class SecurityConfig {
    
    @Bean
    public PasswordEncoder passwordEncoder() {
        return new BCryptPasswordEncoder();
    }
    
    @Bean
    public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
        http
            .csrf().disable()
            .authorizeHttpRequests()
            .requestMatchers("/public/**").permitAll()
            .anyRequest().authenticated()
            .and()
            .formLogin()
            .loginPage("/login")
            .defaultSuccessUrl("/dashboard");
            
        return http.build();
    }
}

만약 위와 같이 설정이 복잡한 경우, 설정그 자체만으로 하나의 큰 책임이라고도 볼 수 있습니다.
이런 경우에는 객체는 주어진 설정대로 잘 동작하는 책임, 그리고 설정을 구성하는 책임으로 나눠 설계할 수도 있을 것 같아요. 또한 이렇게 되면, 테스트 시 설정부분을 마음대로 제어할 수도 있을거고요!

[Minuring]

만약 프로덕션 데이터에 "어드민" 또는 "브라운"이 변경되거나 없다면 테스트가 실패할 수 있을 것 같아요.
또한 다른 개발자가 테스트를 이해하려고 할 때, 해당 데이터가 존재한다는 암묵적 전제를 알아야 해요.

이처럼 테스트가 프로덕션 데이터에 의존하게 되면 어떤 단점들이 있을까요?

참고 : self contained test

[selee1012]

이름에 대한 외부 의존성이 생겨서 좋지 않을 것 같습니다. 외부 이름을 변경해야 할 때 수정해야 하는 코드도 늘어나고요
이 부분 수정해보겠습니다!

[Minuring]

data.sql, CommandLineRunner 또는 애플리케이션 외부와 같이 여러 방법들이 있을텐데요,
만약 실제로 이렇게 데이터 초기화를 해야하는 상황이라면 어떤 방법을 택하실 것 같나요?

[selee1012]

음 상황에 따라 다를거 같은데요
이미 배포를 했고 유지보수나 운영 혹은 서버를 띄워놓고 개발을 진행하는 단계에서는 애플리케이션 외부에서 진행할 것 같습니다. 이미 RDS나 docker 등 외부에서 DB를 띄워 놨을 확률이 높아서 그쪽에서 관리하는게 편할 것 같아요
어디에도 배포하지 않은 상태로 로컬에서 띄우는 거라면 sql혹은 CommandLineRunner을 쓸 거 같은데요
자바에 호환해야 하는 것들이 많아보이면 CommandLineRunner를 없으면 sql(이게 제일 쉽게 구축할 수 있을것 같아요)을 쓸것 같습니다

[Minuring]

set -e 를 최상단에 추가함으로써, 실행 실패 시 그 다음 명령어를 실행하지 않도록 해도 좋을 것 같아요!

[selee1012]

잘 몰랐던 설정이었는데 추가하면 헛도는 시간을 절약할 수 있겠네요! 사용해 보겠습니다!

[Minuring]

요구사항
JWT 관련 로직을 roomescape와 같은 계층의 auth 패키지의 클래스로 분리하세요.

해당 요구사항이 요구하는 바는 아래와 같은 패키지 형태인 것 같아요.

├── java/
│   ├── auth/
│   └── roomescape/
└── resources/

또한 이렇게 분리했을 때, 어떤 문제가 발생하고 그 이유는 무엇일까요?
실제 서비스 코드라고 생각했을 때 어떤 상황과 유사할까요?

[selee1012]

auth를 roomscape와 같은 계층에 두면 현재는 방탈출 예약 기능만 제공하지만 비슷한 단위의 다른 도메인 기능을 제공할 때 auth를 공용으로 사용하기 용이하고 실제로 그럴 가능성은 꽤 높다고 생각됩니다.
auth가 roomscape 내부에 다른것에 의존하면 단점이 있을 수 있지만 현재는 그렇지 않기 때문에 auth를 밖으로 빼는게 맞는것 같네요
수정하겠습니다

[selee1012]

전체적으로 리뷰 반영해서 고민해보고 수정하였습니다
이외에 한 가지 생각했던 부분은 TestDataLoader를 객체를 생성하는 형식으로 변경하는 과정에서 DataLoader가 test에서도 실행돼 회원이 중복 생성되는 문제가 있어서, DataLoader에 @Profile("!test")를 붙여 분리하였습니다