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

feat : redis 연동 및 로그인 #19

Merged
merged 23 commits into from
Jun 15, 2024
Merged

Conversation

GaBaljaintheroom
Copy link
Contributor

@GaBaljaintheroom GaBaljaintheroom commented Jun 5, 2024

😋 작업한 내용

🙏 PR Point

  • jwt 관련 의존성과 코드가 api 모듈 내부에 있었는데, user-api 모듈에서 jwt 관련 코드가 필요합니다.
  • user-api 모듈에서 api 모듈을 의존하면 양방향으로 의존하게 되어 오류가 발생하기 때문에 jwt 관련 의존성과 코드를 user-api로 이동하였습니다.
  • api 모듈에서는 security 의존성에 관련된 코드를 남겨두었습니다.

  • postgresql에서 json 형식으로 저장할 때 json 타입과 jsonb 타입 중 jsonb 타입으로 하였습니다.
    • jsonb 타입이 인덱스를 걸 수 있으며, 공백을 보존하지 않고, 객체 키의 순서를 보장하지 않으며 중복 객체 key 또한 저장하지 않습니다. 또한 추가적인 파싱을 하지 않아 json 타입보다 처리 속도가 빨라 선택하였습니다. (json 타입은 입력된 텍스트에 대한 정확한 복사본을 저장하기 때문에, 처리할 때마다 매번 파싱을 다시 해야만 합니다., jsonb는 decopmose된 바이너리 형식으로 저장)
    • posgresql json docs

👍 관련 이슈


@GaBaljaintheroom GaBaljaintheroom added feat add feature labels Jun 5, 2024
@GaBaljaintheroom GaBaljaintheroom self-assigned this Jun 5, 2024
Copy link
Contributor

@devmizz devmizz left a comment

Choose a reason for hiding this comment

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

Redis 구현하느라 수고 많으셨어요!
준수님이 말씀해주신 부분 살펴보고, api -> user-api로 옮기신 부분들 살펴봤는데, 조금은 어색한 거 같아요!

가령, AuthenticatedUser라는 Record class는 Security Context에 담겨있는 유저 정보를 받아오는 클래스 역할인데, user-api 모듈이 아닌 show-api와 같은 다른 api 모듈에서 사용할 일이 없나? 싶은 생각이 드네요.

JWTGenerator 같은 경우, user-api에서만 사용할 가능성이 높지만, 저는 조금 더 범용성 있는 클래스로서 취급해줘야 한다고 생각해요. 저희가 JWT를 쓰다가 얼마든지 다른 토큰을 쓸 수 있는 문제거든요. 의존성을 현재 api -> user-api로 하고 있지만, api에 구현체가 있더라도 user-api에서 잘 갖다 쓸 수 있는 방법이 있습니다! DI를 여기서 어떻게 사용할 수 있을지 한 번 고민해보세요! (난이도는 정말 높긴 하겠지만, 해내시면 성취가 아주..👍)

도저히 못하시겠다면 언제든지 편하게 질문 주십쇼!

@GaBaljaintheroom
Copy link
Contributor Author

찬기님 저희 회의한 바탕으로 수정해 보았습니다.

근데 SocialCredentials 부분을 봐주시면 감사하겠습니다.
User를 save할 때 Insert 쿼리 후 update 쿼리가 발생하는데,SocialCredentials 필드 때문입니다.
List 필드에서 add가 되어서 JPA에서 변경감지가 되는 형식으로 되어서 그런것 같습니다.
이 부분은 어쩔 수 없는거 같다고 판단을 내렸습니다..(하루 종일 찾아봤는데 update 쿼리가 발생할 수 밖에 없는 것 같아요...)

Infrastructrure 모듈 의존성을 변경했는데, 정말 말씀대로 주입이 되는게 너무 신기했습니다. 감사합니다! 이게 IoC;;;
JWT 관련 인증 인가 코드는 common-api로 분리 했습니다.

Copy link
Contributor

@devmizz devmizz left a comment

Choose a reason for hiding this comment

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

고칠 게 많으셨을텐데 너무 고생 많으셨어요! 물론 제가 그만큼 또 드려서 죄송하긴 하지만... 파이팅 하십쇼!

우선 일급 컬렉션 쪽은 구조를 한 번만 더 고민해보시고 얘기 나눴으면 좋겠어요! 뭔가 좀 어색합니다. 그리고 업데이트 쿼리는.. 일단 예상 가는 문제가 코드에 있긴 한데, 다른 수정 사항부터 같이 검토해보시죠!

app/api/build.gradle Outdated Show resolved Hide resolved
app/api/build.gradle Show resolved Hide resolved
app/api/common-api/build.gradle Show resolved Hide resolved
app/infrastructure/build.gradle Outdated Show resolved Hide resolved
@GaBaljaintheroom
Copy link
Contributor Author

GaBaljaintheroom commented Jun 11, 2024

SocialCredential을 변경해 보았는데, 이게 찬기님이 생각하신 방법인지는 잘 모르겠어요.
제가 이해한 바로는 APPLE, GOOGLE, KAKAO에 따라 식별 형식이 변경되었을 때 대응을 어떻게 해야하면 좋을까란 생각에서 json 형식으로 나온 것 같은데,
일단 기본적으로 SocailLoginType과 소셜 로그인에서 주는 식별자는 공통으로 가져야 한다고 생각이 들었고, 이후 변경되었을 때를 대비하여 sealed 클래스를 선언해주었습니다.

@GaBaljaintheroom
Copy link
Contributor Author

GaBaljaintheroom commented Jun 14, 2024

  1. User 엔티티 Nullable 삭제 (LocalDate.min 으로 하면 Postgresql에서 저장할 수 있는 범위를 넘어가서 0.1.1로 저장을 하였습니다.)
  2. refreshToken 탈취 우려하여 비교 로직을 추가 했습니다.
  3. Infrastructure 모듈 내부를 분리하였습니다.
  4. login 로직을 dto 분리하여 수정하였는데, 이런 흐름이 맞는지 확인해 주시면 감사하겠습니다.
  5. blacklist 방식으로 AT가 로그아웃 되어있는지 검증 로직을 추가했습니다.

로그인 요청 값은 아마 변경될 수 있지 않을까 생각합니다. (nickname, fcm 토큰 등)

Copy link
Contributor

@devmizz devmizz left a comment

Choose a reason for hiding this comment

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

말씀드렸던 거 거의 다 반영된 거 같아서 좋네요! 수고하셨어요! merge까지 얼마 안 남았다!!

@GaBaljaintheroom GaBaljaintheroom merged commit 7c1ef11 into develop Jun 15, 2024
1 check passed
@GaBaljaintheroom GaBaljaintheroom deleted the feat-redis-setting branch June 15, 2024 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat add feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants