Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Spring Core] (배포) 신혜빈 미션 제출합니다. #113

Open
wants to merge 65 commits into
base: shin378378
Choose a base branch
from

Conversation

shin378378
Copy link

@shin378378 shin378378 commented Jan 14, 2025

안녕하세요 :) 망초 리뷰어님!! ☘️☘️
벌써 마지막 pr이라니... 시원섭섭하네요 ㅜㅠ
주환님께 한 번쯤 pr받아보고 싶었는 데 마지막에 받게 되게 영광입니다 ><

방금 제 1번째 PR을 다시 보고 왔는데, 정말 엉망이더라고요 ㅎㅎ
아직 많이 부족하긴 하지만 그래도 여기까지 성장할 수 있었던 건 리뷰어님 덕분입니다 :)
마지막 pr도 잘 부탁드립니다!! ☺️

<구현과정>

7단계 - @Configuration

  • JWT 관련 로직을 roomescape와 같은 계층의 auth 패키지의 클래스로 분리
  • 불필요한 DB 접근을 최소화

8단계 - Profile과 Resource

  • schema.sql 대신 데이터베이스를 초기화 해주기 위해 실행하는 클래스를 만들기
  • DataLoader에서는 사용자 정보만 초기화
  • TestDataLoader에서는 테스트에 필요한 사전 값 초기화
  • token 생성에 필요한 비밀키값을 외부 파일로 분리

9단계 - 배포 스크립트 (아직 안 함, 이번 주 스터디에서 같이 한다고 공지 받음)

  • ec2나 서버에서 배포를 할 수 있게 배포 스크립트를 작성
  • 배포 스크립트의 시나리오
  • 초기 설정의 시나리오

<궁금한 점>

1) 메인클래스의 @ComponentScan(basePackages = {"roomescape", "jwt"}) 부분

  • 문제상황 1

JWT 관련 로직을 roomescape패키지와 같은 계층의 패키지의 클래스로 분리했는 데 코드가 돌아가지 않았고
메인코드(RoomescapeApplication.class)가 roomescape패키지 안에 있어서 이 패키지 밖에 있는 컴포넌트를 탐색하지 않기 때문이라는 점을 알게 되었습니다.
--> 해결
메인클래스(RoomescapeApplication.class) 위에 @ComponentScan(basePackages = {"jwt"})를 붙여 스프링 빈으로 등록할 컴포넌트들을 탐색하도록 지시해주었습니다.

  • 문제상황 2

하지만 이 컴포넌트를 붙인 이후로 http://localhost:8080/login 페이지가 열리지 않습니다.
--> 해결
임시방편으로 @ComponentScan(basePackages = {"roomescape", "jwt"})를 붙여주어 코드를 잘 돌아가게는 해두었습니다.

  • 질문

하지만 왜 "roomescape"도 붙여야하는 지 모르겠습니다.
@SpringBootApplication가 이미 roomescape 패키지 내에 위치하고 있는 데 굳이 "roomescape"를 붙여주어야 돌아가는 이유는 뭔가요?

2) Jwt의 DB 접근을 부분

  • 문제상황

이번 미션에서는 Jwt의 DB접근 부분을 최소화하라는 요구조건이 있었는 데요.
최대한 Jwt가 DB접근을 최소화하도록 구현했지만 JwtService클래스에서의 getMemberFromToken메소드 때문에 MemberRepository를 참조하게 됩니다.

  • 질문

토큰으로부터 멤버를 찾는 로직은 제가 생각할 땐 member패키지보단 jwt패키지에 있는 게 더 어울린다고 생각하는 데 리뷰어님 의견은 어떤 지 궁금합니다!! jwt패키지에 넣는 것과 member패키지에 넣는 것 이외에도 좋은 의견이 있으시면 편하게 말씀해주세요 :)

@shin378378 shin378378 changed the title . [Spring MVC] 신혜빈 미션 제출합니다. Jan 14, 2025
@shin378378 shin378378 changed the base branch from main to shin378378 January 14, 2025 20:13
@shin378378 shin378378 changed the title [Spring MVC] 신혜빈 미션 제출합니다. [Spring Core] (배포) 신혜빈 미션 제출합니다. Jan 15, 2025
Copy link

@3Juhwan 3Juhwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 혜빈님~ 망쵸입니다 😀
혜빈님이 미션 잘 진행하고 있단 얘기를 많이 들었어요. 저도 리뷰어로 함께 미션 진행해 보고 싶었는데, 마지막에 운 좋게 매칭되었네요! 재밌게 미션해 보아요~
몇 가지 코멘트 달았는데, 혜빈님 의견이 궁금한 것도 있어요. 읽어보고 답변해 주시면 감사할 것 같아요.

다음은 질문에 대한 답변이에요!


질문1: 왜 "roomescape"도 붙여야하는 지 모르겠습니다. @SpringBootApplication가 이미 roomescape 패키지 내에 위치하고 있는 데 굳이 "roomescape"를 붙여주어야 돌아가는 이유는 뭔가요?


저도 @SpringBootApplication가 이미 roomescape 패키지 내에 위치하고 있는데, 왜 컴포넌트 스캔을 안하는지 궁금하네요 🙄 그래도 어쩌겠어요.. 스프링이 그렇게 동작하는걸요. 알고 있는 지식을 확실히 하기 위해 한 가지 가설을 세웠어요!

@SpringBootApplication@ComponentScan이 중첩되어 있으면 @ComponentScan 적용된다.

@SpringBootApplication 내부에 @ComponentScan를 포함하고 있는 걸 아시나요?

@SpringBootConfiguration
@EnableAutoConfiguration
@ComponentScan(excludeFilters = { @Filter(type = FilterType.CUSTOM, classes = TypeExcludeFilter.class),
		@Filter(type = FilterType.CUSTOM, classes = AutoConfigurationExcludeFilter.class) })
public @interface SpringBootApplication {

@ComponentScan을 좀더 자세히 봐볼까요? 다음은 @ComponentScan javadocs의 일부에요!

스크린샷 2025-01-16 오후 12 07 28

basePackages에 명시한 패키지는 컴포넌트 스캔을 시작하는 지점이에요. 만약 이 옵션이 명시되어 있지 않다면, 어노테이션이 붙은 클래스의 패키지부터 재귀적으로 스캔한다고 하네요.

@SpringBootApplication 안에 @ComponentScan은 옵션이 명시되어 있지 않아서, Application.class가 속한 패키지부터 스캔한다는 걸 알 수 있죠. 이것은 경험적으로도 모두 알고 있을거예요!

하지만 이 클래스에 @ComponentScan를 추가하고 basePackages 옵션까지 명시하면 위 내용이 적용되지 않는 것 같아요. @ComponentScan가 중첩되면서 @SpringBootApplication 내부에 있는 @ComponentScan가 무시된 게 아닐가 싶어요.

좀더 확인하고 싶어서 간단한 프로젝트를 만들어서 확인해 봤어요! 패키지 구조는 다음과 같이 작성했어요.

스크린샷 2025-01-16 오후 12 13 44

Application을 구동하는 시점에 등록된 빈을 확인해 보았어요.

@SpringBootApplication
// @ComponentScan(basePackages = "com.example.live")
public class DemoApplication {

    private static final Logger log = LoggerFactory.getLogger(DemoApplication.class);

    @Autowired
    private ApplicationContext ac;

    public static void main(String[] args) {
        SpringApplication.run(DemoApplication.class, args);
    }

    @PostConstruct
    public void init() {
        log.info("빈 개수: {}", ac.getBeanDefinitionCount());
        for (String name : ac.getBeanDefinitionNames()) {
            log.info("빈 이름: {}", name);
        }
    }
}

@ComponentScanbasePackages 옵션을 변경해 보며 로그를 진행했어요.

  1. 옵션을 적용하지 않은 경우
  • 빈 개수: 56
  • 포함한 애플리케이션 빈: demoApplication, catService
  1. basePackages = "com.example.live" 옵션을 적용한 경우
  • 빈 개수: 56
  • 포함한 애플리케이션 빈: demoApplication, personService
  1. basePackages = "com.example.demo; com.example.live" 옵션을 적용한 경우
  • 빈 개수: 57
  • 포함한 애플리케이션 빈: demoApplication, catService, personService

정확히 하기 위해서는 좀더 파봐야 할 것 같은데 이 정도면 충분하다고 생각해요!


질문2: 토큰으로부터 멤버를 찾는 로직은 제가 생각할 땐 member패키지보단 jwt패키지에 있는 게 더 어울린다고 생각하는 데 리뷰어님 의견은 어떤 지 궁금합니다!! jwt패키지에 넣는 것과 member패키지에 넣는 것 이외에도 좋은 의견이 있으시면 편하게 말씀해주세요 :)


저의 개인적인 생각을 공유해 볼게요!

member와 jwt 패키지 둘 중에 선택하자면 jwt를 선택하겠어요. 토큰이 member와 jwt 중에 어떤 거에 가까울까요? 저는 jwt라고 생각해요! 토큰이라는 워딩이 jwt 토큰에서 나왔기 때문이에요.

토큰으로부터 멤버를 찾는 로직를 member에 둔다고 가정하면 member 도메인은 토큰 도메인을 알게 되어요. member는 그 자체로 순수한 도메인이어야 하는데 토큰을 아는 게 바람직할까요?

만약 jwt 인증 방식에서 세션 방식으로 변경한다면 어떻게 될까요? member 도메인 쪽에도 변경이 발생할 거예요. 인증 방식의 변경이 member 도메인에도 영향을 미치는 것은 바람직하지 않다고 생각해요. 그래서 저는 jwt 패키지를 선택할 것 같아요!

추가로 얘기해 보고 싶은 게 있다면 코멘트 또는 디스코드 DM을 보내주세요 😀

Comment on lines +16 to +19
@Value("${jwt.secret}")
public void setSecretKey(String secretKey) {
this.secretKey = secretKey;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메서드의 인자에 값을 주입할 수 있네요? 이렇게도 할 수 있군요~ 하나 배웠어요!
질문이에요. 이렇게 하면 어떤 장점이 있나요? 또 다른 방법은 무엇이 있을까요?

import java.util.Date;

public class JwtUtils {
private static final long EXPIRATION_TIME = 86400000;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

상수 분리 👍👍 86400000은 하루를 밀리초로 표현한 것 같아요. 다음은 제안사항인데 혜빈님의 의견이 궁금해요!

86400000의 단위가 밀리초라는 걸 표현하면 어떨까요?

Suggested change
private static final long EXPIRATION_TIME = 86400000;
private static final long EXPIRATION_TIME_MILLIS = 86400000;

86400000가 하루라는 걸 표현하면 어떨까요?

Suggested change
private static final long EXPIRATION_TIME = 86400000;
private static final long EXPIRATION_TIME_MILLIS = 1000 * 60 * 60 * 24;

Comment on lines +7 to +17
@Configuration
public class JwtConfiguration {

@Value("${jwt.secret}")
private String secretKey;

@Bean
public JwtUtils jwtUtils() {
return new JwtUtils();
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요구사항대로 @Configuration@Bean을 사용해서 빈 등록을 잘 해주었어요~

추가로 빈(Bean) 자동 등록과 수동 등록에 대해 알아볼까요?

  • 둘은 어떤 차이가 있을까요?
  • 어떤 장단점이 있을까요?

현재 jwt 패키지에서 등록하는 빈은 자동 등록과 수동 등록이 혼재되어 있어요. 일관성 있게 수정해 볼까요?

Comment on lines +11 to +17
@Component
@Profile("test")
public class TestDataLoader implements ApplicationRunner {

@Autowired
private EntityManager entityManager;

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Profile에 따라 초기 데이터를 잘 세팅했네요 👍 프로젝트할 때 Profile 다루는 건 문제 없겠어요!

제안 사항이에요! TestDataLoader는 테스트에 필요한 데이터를 세팅하는 클래스 같아요. 테스트에 필요한 클래스가 프로덕트 코드보다는 테스트 코드 영역으로 옮기면 좋을 것 같아요!

Comment on lines +29 to +34
Member member = memberRepository.findById(userId).get();
if (member == null) {
throw new IllegalArgumentException("토큰으로부터 유저를 찾을 수 없습니다.");
}
return member;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional의 orElseThrow 메서드를 사용해서 가독성을 개선해 볼까요?

Comment on lines +23 to +36
@Override
public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) {
String token = extractTokenFromCookies(request.getCookies());
if (token == null) {
throw new ResponseStatusException(HttpStatus.UNAUTHORIZED, "토큰을 찾을 수 없습니다.");
}

Member member = jwtService.getMemberFromToken(token);
if (!"ADMIN".equals(member.getRole())) {
throw new ResponseStatusException(HttpStatus.UNAUTHORIZED, "사용자를 찾을 수 없습니다.");
}

return true;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

미션 요구사항: JWT 토큰에는 사용자 식별 정보와 권한 정보가 들어갑니다. 만약 이 두 정보만 필요하다면 DB 접근이 필요하지 않습니다.

현재는 권한을 확인하기 위해 DB에 접근하고 있어요! DB 접근이 꼭 필요할까요? DB 접근 없이 권한 확인하도록 수정해 볼까요?
또한 DB에 접근하는 것이 좋을까요? 안 좋을까요? 필요한 경우는 어떤 상황이고, 그렇지 않은 상황은 언제일까요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants