Skip to content
This repository was archived by the owner on Nov 21, 2023. It is now read-only.

[#28] 가입기능 구현 #38

Merged
merged 10 commits into from
Nov 18, 2021
Merged

[#28] 가입기능 구현 #38

merged 10 commits into from
Nov 18, 2021

Conversation

youngvly
Copy link
Contributor

@youngvly youngvly commented Nov 15, 2021

진행상황

  • 가입 시나리오
  • 로그인
  • 탈퇴 시나리오

가입 시나리오

1. 이름,비밀번호,email 입력 
   - email 중복검사
2. CREATED 상태로 가입.
   - email 인증코드 전송
3. email 인증코드 인증.
   - ACTIVATED 상태로 가입 완료.
   - 프로필생성, 타임라인생성 등,, 후액션 처리필요.
 4. 상태 변경시 UserStatusChangedEvent 발송 -> 나누는게 나을까요??
  • 환경변수 추가
    • ddd-secret 참고
  • user 테이블 수정 , auth_code 테이블 추가
  • auth_code 도메인?
    • crudRepository 등 복합키 미지원.
    • 커스텀 쿼리로 repo작성했습니다.
  • security로 인증 추가는 해두었는데, 로그인은 아직입니다 ^^,,
    • 로그인유저 권한 체크 필요하다면 @IsLoginUser 먼저 사용하시면 될것같아요
      user가 profile. freinds를 가지는게 맞을까,, profile이 user를 가지는게 맞을까??

논의필요

  • exception handle, global 수준 catch 필요할지,, exception 패키지 위치나, 응답 전략
    • core.exceptions 에 범용적인 예외는 위치, domain.exeptions에 각 도메인의미의 exception위치
  • 응답코드
    • 일단 응답코드 다양하게 활용하긴했는데, 200 + response body 식 혹은 응답코드 적절하게 활용 방식 중 어떤게 나을지??
    • 응답코드 적절하게 활용

  • 응답body 형식
    • json으로 무조건 묶는다 혹은 간단한 응답은(Boolean?) 내용만 내린다
    • json응답으로 통일

@youngvly youngvly added the enhancement New feature or request label Nov 15, 2021
@youngvly youngvly self-assigned this Nov 15, 2021
fun userStatusFlow(userStatusListener: UserStatusListener) = integrationFlow {
channel { publishSubscribe(Channels.USER_STATUS) }
handle<UserStatusChangedEvent> { event, _ ->
when (event.user.status) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

상태변경 이벤트 하나로 뭉쳐두긴했는데, 나누는게 나을까요??

Copy link
Contributor

Choose a reason for hiding this comment

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

상관 없을 것 같아요

Comment on lines +11 to +12
"spring.mail.username=***@gmail.com",
"spring.mail.password=***",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

발송 테스트용인데, 필요하시면 실제계정 입력해서 사용하시면 됩니다

Comment on lines +14 to +17
@NotEmpty
@Size(min = 8, max = 30, message = "비밀번호는 8자 이상 30자 미만이어야 합니다.")
@Pattern(regexp = "(?=.*[A-z])(?=.*[0-9])", message = "비밀번호는 영문자와 숫자가 포함되어야 합니다.")
val password: String,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

user가입에 관한 validation은 request단에 뒀습니다

@youngvly youngvly mentioned this pull request Nov 15, 2021
Copy link
Contributor

@chanhyeong chanhyeong left a comment

Choose a reason for hiding this comment

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

엄청 많네요;; 아직 다 보진 못했는데 여태까지 본거 코멘트드립니다

override fun save(authCode: AuthCode): AuthCode {
jdbcTemplate.update(
"""
REPLACE INTO auth_code (user_id, `code`, created_at, purpose)
Copy link
Contributor

Choose a reason for hiding this comment

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

코드가 더 길어지긴 하지만
replace 보단 on duplicate update 가 맞긴 하겠네요
(찾아보니 replace 는 기존 row 삭제 후 insert 를 다시 한다네요)

외부에 안나가니까 이대로 둬도 괜찮긴 합니다

Copy link
Contributor

Choose a reason for hiding this comment

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

추가로 replace 때마다 created_at 이 엎어쳐지는게 조금 이상하네요
삭제되는건 아직 구현되기 전인가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

인증코드 일회성이라 인증하고 바로삭제하면 되겠네요!
created_at 덮어쓰는건, 인증 제한시간?정도 제약들어가면 사용될값으로 봤어요 (지금은 따로 없습니다)
최초 created_at 히스토리가 필요없어서, deleted-insert 의도된부분 맞습니다!

@@ -0,0 +1,15 @@
package com.sns.commons.utils
Copy link
Contributor

Choose a reason for hiding this comment

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

utils 아래 파일명은 Booleans 와 같이 클래스s 가 나을 것 같습니다

import com.sns.user.component.authcode.domain.Purpose
import org.springframework.data.repository.NoRepositoryBean

@NoRepositoryBean
Copy link
Contributor

Choose a reason for hiding this comment

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

다른데서 이거 붙이고 있는건 CrudRepository<T, ID> 를 상속하고 있어서고 (이거 붙은건 spring data 에서 bean 등록해버려서)
여기는 붙이지 않으셔도 될거에요

import org.springframework.data.annotation.Id
import org.springframework.jdbc.core.RowMapper

data class AuthCode(
Copy link
Contributor

Choose a reason for hiding this comment

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

지금 이 상태면 CrudRepository 의 장점을 거의 사용하지 못하는데

composite key 로 가지 않고 별도 long 아이디로 가는 것도 괜찮을 것 같습니다 (아이디를 사용할 일이 없지만)
이건 unique key 로 등록해두고요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

고민했던 부분이긴한데, save도 upsert처럼 동작해야하고, select도 uniqueKey로 만 하면 되는거라 crudRepository 있어도 활용을 잘 안할것같아서 crudRrepo빼고 쿼리 작성으로 구현한 부분이긴 합니다! (인증코드에서 현재 기능 이상의 스펙이 요구될 것 같지 않구요..?)

) {

fun isCorrect(userId: String, code: String, purpose: Purpose): Boolean =
(this.userId == userId) and (this.code == code) and (this.purpose == purpose)
Copy link
Contributor

Choose a reason for hiding this comment

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

오 and 가 있는건 몰랐네요

WHERE user_id = :userId AND purpose = :purpose
LIMIT 1
""".trimIndent(),
mutableMapOf(
Copy link
Contributor

Choose a reason for hiding this comment

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

여기는 mutable 이어야하나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

파라미터로 mutable 받고있어서 컴파일에러났던것같아요!

@@ -10,4 +10,6 @@ class UserQueryService(
private val userRepository: UserRepository
) {
fun getById(id: String): User? = userRepository.findByIdOrNull(id)

fun getByEmail(email: String): User? = userRepository.findByInfoEmailAddressOrNull(email)
Copy link
Contributor

Choose a reason for hiding this comment

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

fun userStatusFlow(userStatusListener: UserStatusListener) = integrationFlow {
channel { publishSubscribe(Channels.USER_STATUS) }
handle<UserStatusChangedEvent> { event, _ ->
when (event.user.status) {
Copy link
Contributor

Choose a reason for hiding this comment

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

상관 없을 것 같아요


override fun configure(http: HttpSecurity?) {
if (http == null) return
http.authorizeRequests()
Copy link
Contributor

Choose a reason for hiding this comment

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

코멘트 드린게 많아서 ㅋㅋ 나중에 시간되시면 재미로 한 번 봐주세요
https://www.baeldung.com/kotlin/spring-security-dsl

youngvly and others added 3 commits November 16, 2021 10:13
 - response는 json으로
 - crudRepository 활용
 - controller에 함께 aggregator 위치. (1:1 매칭)
@youngvly youngvly merged commit 7b42fcb into main Nov 18, 2021
@youngvly youngvly deleted the feature/join branch November 18, 2021 10:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants