[그리디] 강동현 Spring Core (배포) 미션 제출합니다.#216
Conversation
| cd $PROJECT_DIR | ||
|
|
||
| echo "> Git Pull" | ||
| git pull origin ke-62 |
There was a problem hiding this comment.
요거는 다른 사람의 브랜치가 실수로 들어간 것 같습니다!
추가로 협업 시 브랜치 명은 보통 하드코딩하지 않는 편인데요,
환경이나 실행 주체에 따라 배포 결과가 달라진다면 아찔한 상황이 벌어질 수 있습니당
There was a problem hiding this comment.
미션 진행 당시 시간이 없어 고은님 코드를 사용하고 제 프로젝트에 맞게끔 적용하는 과정에서 검토를 놓쳤습니다....정말 부끄럽네요...ㅠㅠ
기존 코드에서 몇몇 부분들을 수정하여 리팩토링하였습니다!
- set -euo pipefail 을 적용하여 중간에 에러 발생 시 즉시 배포를 중단하도록 리팩토링하여 배포 시 안정성을 높였습니다.
- PROJECT_DIR="$SCRIPT_DIR/.."로 한 단계 위를 프로젝트 루트로 보고 스크립트가 scripts/ 같은 하위 폴더에 있어도 동작하도록 변경하였습니다.
- 브랜치를 동적으로 결정하도록 리팩토링 하였습니다.
- --ff-only를 사용하여 배포 브랜치가 뒤쳐져 있을때만 머지 가능하도록 변경했습니다!
- 찾아보니 서버의 프로세스가 없는 경우가 존재한다고 알게 되었습니다!(서버 최초 배포, 직전에 서버를 내린 경우 등) 따라서 CURRENT_PID="$(pgrep -f "${PROJECT_NAME}.*.jar" || true)" => 프로세스가 없어도 스크립트가 중단되지 않게 하였습니다.
- 그 외에 경로에 따옴표를 써서 경로에 공백이 있을 때에도 인식되도록 리팩토링하였습니다!
There was a problem hiding this comment.
정리 잘 해주셔서 감사합니다!
단순히 코드 수정에 그치지 않고 왜 문제가 될 수 있는지 → 어떻게 개선했는지가 명확해서 이해하기 쉬웠어요.
set -euo pipefail 적용이나 프로세스가 없는 경우까지 고려한 PID 처리 부분에서 고민하신 흔적이 잘 느껴졌습니다.
과정과 배운 점을 공유해주셔서 저도 많이 참고가 됐습니다 👍
| @Component | ||
| @Profile("test") | ||
| public class TestDataLoader implements CommandLineRunner { | ||
|
|
||
| public TestDataLoader() {} | ||
|
|
||
| @Override | ||
| public void run(String... args) { | ||
| } | ||
| } |
There was a problem hiding this comment.
헉 내용이 아무것도 없네요! 의도한 것일까요?
아마 미션에서는 아래와 같이 요구한 것으로 보아,
�Production용과 DataLoader와 Test용 TestDataLoader를 만드세요.
DataLoader에서는 사용자 정보만 초기화
TestDataLoader에서는 테스트에 필요한 사전 값 초기화
- 동현님의 schema.sql에 들어있는 테스트용 값을 이곳에서 초기화 해보는 것은 어떨까요?
- 추가로 Test용 데이터를 로드하는 코드는 prod 패키지보다 test 패키지가 적절할 것 같아 보입니다.
There was a problem hiding this comment.
테스트 데이터 로더가 테스트 환경에서만 실행되는 초기 데이터 세팅용 클래스인만큼, 흐름상 testdataloader에 넣는 것이 좋아보입니다!
기존에는 테스트코드 통과를 위해 도메인에 직접 테스트용 값을 넣어두어서, 리팩토링 시에 이전 미션에서 값을 넣어둔 탓에 테스트코드가 통과되었고 그걸 그대로 사용하였습니다.
말씀드린 부분은 납득하였고 동의합니다! 수정했습니다!
| if (!"ADMIN".equals(role)) { | ||
| writeError(response, ApiError.FORBIDDEN_ADMIN_ONLY); | ||
| writeError(response, ApiError.UNAUTHORIZED_MISSING_TOKEN); |
There was a problem hiding this comment.
이 부분은 토큰의 여부를 검사하고 있다기 보다
권한을 묻고 있는 것 같은데 missing token 에러를 던지신 이유가 있을까요?
There was a problem hiding this comment.
리팩토링 중 코드를 잘못봤습니다.. 이전 코드로 수정하였습니다!
|
|
||
| @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT) | ||
| @Sql(scripts = "/sql/truncate.sql", executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD) | ||
| @DirtiesContext(classMode = DirtiesContext.ClassMode.BEFORE_EACH_TEST_METHOD) |
There was a problem hiding this comment.
[참고]
이 어노테이션이 견본 코드이긴 하지만 성능이 안 좋아서,
시간이 되신다면 다른 방법으로 역할을 대신해보는 것도 좋을 것 같아요!
성능 최적화 관련 글
There was a problem hiding this comment.
@SQL로 DB를 비우고 있기 때문에 DirtiesContext가 필요없군요!!
지난번 리뷰에서 DirtiesContext의 장단점에 대해 리뷰를 받았었는데, 이번 미션에도 적용할 필요는 없어 보입니다!
수정했습니다!
| if ("admin@email.com".equals(email)) { | ||
| return jwtUtils.createToken("1", "어드민", Role.ADMIN.name()); | ||
| } | ||
| if ("brown@email.com".equals(email)) { | ||
| return jwtUtils.createToken("2", "브라운", Role.USER.name()); | ||
| } |
There was a problem hiding this comment.
또한 테스트용 고정 사용자(admin, brown)의 경우에는
토큰을 만들 때 DB를 조회하지 않고 바로 생성하도록 처리해
불필요한 DB 접근을 줄였습니다.(테스트용이기 때문에)
일반 사용자 인증 흐름은 기존 방식 그대로 유지했습니다.
PR에 써주신 내용대로 구현하신 의도는 어느 정도 이해했는데 이 부분을 말한 것이 맞겠죠??
이와 관련해 두 가지가 궁금합니다.
- 이번 미션에서 Profile(test / prod)을 분리하셨는데 적용하면서 느끼신 필요성 혹은 이점이 무엇이었는지 궁금합니다! 만약 없다고 생각하셨다면 이에 대해 설명해주셔도 좋아요!
- Profile을 통한 환경 분리 의도와 테스트용 분기를 프로덕트 코드에 직접 넣은 현재 구현은 서로 상충된다고 보이는데, 이 부분에 대해서는 어떻게 생각하셨는지 의견을 듣고 싶습니당
There was a problem hiding this comment.
-
테스트 환경과 운영 환경의 데이터 로딩을 분리하는 것은 이후 프로젝트를 안정적으로 관리하기 위해 필수적이라고 생각합니다! 이를 분리하지 않으면 테스트 수행 시마다 운영 환경에 불필요한 데이터가 유입되고, 그에 따른 불필요한 트래픽이 발생하여 관리에 부정적인 영향을 끼칠 것이라고 생각합니다!
-
지금 보니 초기 제출시에는 리팩토링이 불완전하게 이루어져 운영 환경과 테스트 환경의 분리가 의미없었다고 생각합니다. 해당 부분은
TestDataLoader에서 직접 데이터를 주입함으로써 문제를 해결했습니다!
# Conflicts: # build.gradle # src/main/java/roomescape/WebConfig.java # src/main/java/roomescape/auth/AdminAuthInterceptor.java # src/main/java/roomescape/auth/LoginMemberArgumentResolver.java # src/main/java/roomescape/member/MemberController.java # src/main/java/roomescape/member/MemberDao.java # src/main/java/roomescape/reservation/ReservationController.java # src/main/java/roomescape/theme/ThemeController.java # src/main/resources/application.properties # src/test/java/roomescape/MissionStepTest.java # src/test/resources/sql/truncate.sql
SANGHEEJEONG
left a comment
SANGHEEJEONG
left a comment
There was a problem hiding this comment.
죄송해요,, 넘 늦게 답변을 달았네요 ㅠㅠ
전반적으로 리뷰 반영이 잘 된 것 같아 코멘트 중에서 login 메서드 중복만 확인해주시구 연락주시면
이만 pr 머지하도록 하겠습니다
긴 스터디 동안 너무너무 수고 많으셨습니다 🥳
|
|
||
| @Component | ||
| @Profile("test") | ||
| public class TestDataLoader implements CommandLineRunner { |
| public Member login(String email, String password) { | ||
| return memberRepository.findByEmailAndPassword(email, password).orElseThrow(); | ||
| } | ||
|
|
||
| public Member login(String email, String password) { | ||
| return memberDao.findByEmailAndPassword(email, password); | ||
| } |
There was a problem hiding this comment.
요기 충돌 해결 과정에서 먼가 잘못된 것 같은데 확인해주시면 좋을 것 같아요!
3 participants
안녕하세요 상희님!
미션 설명에 앞서 여행과 창의학기제 일정으로 미션 제출이 늦어지는 부분 양해 봐주신 점 정말 감사드립니다!
잘부탁드립니다!
7단계 – JWT 분리 및 불필요한 DB 접근 최소화
JWT와 관련된 로직을
auth패키지로 분리하고,JwtUtils클래스로 정리했습니다.JwtUtils는 토큰 생성, 파싱, 쿠키 추출 역할만 담당하도록 구성했습니다.AuthConfig설정 클래스를 통해JwtUtils를 빈으로 등록했고,WebConfig,AdminAuthInterceptor,LoginMemberArgumentResolver,MemberController에서는해당 빈을 주입받아 사용하도록 수정했습니다.
또한 테스트용 고정 사용자(admin, brown)의 경우에는
토큰을 만들 때 DB를 조회하지 않고 바로 생성하도록 처리해
불필요한 DB 접근을 줄였습니다.(테스트용이기 때문에)
일반 사용자 인증 흐름은 기존 방식 그대로 유지했습니다.
8단계 – Profile과 Resource 분리
애플리케이션이 실행될 때 기본 데이터를 넣기 위해
DataLoader를 추가했습니다.@Profile("!test")에서는 실제 실행 환경에서 필요한 사용자(admin, brown)만 로딩하도록 했습니다.JWT 비밀키는 코드에 두지 않고
application.properties로 분리해 관리하도록 변경했습니다.9단계 – 배포 스크립트 추가
초기 환경을 세팅하기 위해
init.sh스크립트를 추가했습니다!이 스크립트는 저장소 클론, 디렉터리 생성, 실행에 필요한 파일 준비를 담당합니다.
배포 과정을 자동화하기 위해
deploy.sh스크립트도 추가했습니다.해당 스크립트는 최신 코드를 pull한 뒤 프로젝트를 빌드하고,
기존에 실행 중이던 프로세스를 종료한 후
최신 JAR 파일로 애플리케이션을 다시 실행합니다.