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

[feature/12] feat: 온보딩 선택을 등록하는 API 구현 #14

Merged
merged 10 commits into from
Jul 22, 2024

Conversation

injoon2019
Copy link
Collaborator

@injoon2019 injoon2019 commented Jul 20, 2024

💡 이슈 번호

close: #12

✨ 작업 내용

  • 온보딩 선택을 등록하는 API 구현했습니다
  • user 테이블을 생성했습니다.

🚀 전달 사항

  • jpa 관련 질문 남겼어요!
  • fk는 안걸어도 괜찮을까요?

@injoon2019 injoon2019 self-assigned this Jul 20, 2024
Comment on lines +7 to +18
kotlin("plugin.allopen") version "1.5.31"
kotlin("plugin.noarg") version "1.5.31"
}

allOpen {
annotation("javax.persistence.Entity")
annotation("javax.persistence.Convert")
}

noArg {
annotation("javax.persistence.Entity")
annotation("javax.persistence.Convert")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

코틀린은 디폴트가 final인데, jpa는 프록시를 생성해야 하기 때문에 상속이 가능해야하잖아요. 그래서 open으로 상속이 가능하게 바꿔주는 설정입니다

Comment on lines +38 to +39
implementation("io.springfox:springfox-boot-starter:3.0.0")
implementation("io.springfox:springfox-swagger-ui:3.0.0")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

swagger 관련 설정입니다!

Comment on lines 15 to 16
@Column(name = "user_id")
var userId: Long,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

요거 생각이 안나서 이렇게 했는데 낼 수정해놓을게요!

Comment on lines 18 to 21
@Column(name = "profile_select")
@Convert(converter = ProfileSelectConverter::class)
var profileSelect: ProfileSelect,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이 저장되는 데이터를 별도로 검색하는 요구 조건이 없고, 통째로 저장하고 통째로 꺼내쓸 것 같아서 json으로 db에 저장하게 했어요

Copy link
Member

Choose a reason for hiding this comment

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

지금은 좋은 것 같아요!
그런데 혹시 나중에 '매칭할 때 키워드를 기반으로 매칭한다' 와 같은 조건이 생긴다면 어떻게 해야할지 생각해봐야할 것 같아요 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

mysql 5.8이후 부터인가 json 안에 특정 값의 index도 가능하고, 값에 대해 쿼리도 가능하더라고요! 추후 마이그레이션도 좋습니다!

Comment on lines +1 to +4
CREATE TABLE user_profile (
id BIGINT AUTO_INCREMENT PRIMARY KEY,
user_id BIGINT NOT NULL,
profile_select JSON,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ddl 쿼리들을 원래 어떻게 관리하셨나요? 일단 여기에 저장해뒀습니다!

Copy link
Member

Choose a reason for hiding this comment

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

그냥 따로 기록해놨었어요! 여기에 저장해두는거 좋아요!

Comment on lines 9 to 10
@Component
class ProfileFacade(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

파사드 클래스 써보셨나요?
controller -> facde -> service -> repository

이렇게 레이어가 생기게 했어요. 이유는 서비스 레이어 (service, component)간에 참조가 생기지 않게하기 위해서 많이 쓰는데 어떻게 쓰시나요? 단순 mvc를 할때는 사실 불필요할 수 있어서 의견 내주세요!

Copy link
Member

Choose a reason for hiding this comment

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

저는 파사드 클래스 써보진 않았어요!
서비스 레이어 간에 참조가 생기지 않게한다는 것은 service에서는 domain과 repository만 의존하도록 하고 dto의 참조를 끊기 위해서인가요?
이렇게 하는 이유가 dto가 변경될 가능성이 높아서 service까지 영향을 미치지 못하게 하기 위함인가요?!

저는 파사드 클래스 쓰는거 좋아요!
그런데 dto가 controller 패키지에 있으면 controller <-> facade 간의 양방향 의존성이 생기는데, dto를 facade에 넣는 것은 어떤가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

레이어드 아키텍처 때문인데요, 예를들어 컨트롤러가 컨트롤러를 참조하면 이상하듯 서비스가 서비스를 참조해도 괜찮냐? 하시는 분들이 있더라고요. 서비스가 서비스를 호출하면 잠재적으로 재귀호출이 문제가 될 수 있어서요. 또 추후 문제이기도 하지만 서비스 레이어만 있으면 aop를 적용할때도 self invocation 문제가 생겨 결국 서비스가 서비스 호출하는 경우도 생기고요! dto를 facade에 넣는건 아주 좋네요!

다만 지금 12 <- 15 <- 17을 바라보고 있어서, 그럼 17번에서 위치이동 빼먹지 않고 시킬게요!

Copy link
Member

Choose a reason for hiding this comment

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

아하 다른 서비스 참조가 생기지 않게하기 위함이군요!
좋습니다👍👍

Comment on lines +45 to +46
implementation("io.github.microutils:kotlin-logging:3.0.5")
implementation("org.slf4j:slf4j-simple:2.0.7")
Copy link
Collaborator Author

@injoon2019 injoon2019 Jul 21, 2024

Choose a reason for hiding this comment

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

코틀린 로그 설정이에요. 로그 남기고 싶은 클래스에서

    private val log = KotlinLogging.logger {  }

    log.info { "로그입니다" }

이렇게 쓰시면 돼요

Comment on lines +19 to +25
@Transactional
fun saveProfile(userProfile: UserProfile): UserProfile {
val user = userRepository.findByIdOrNull(1L) // TODO User 회원 가입 기능 구현후 수정

userProfile.user = user
return profileRepository.save(userProfile)
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

jpa 객체 매핑이 잘 생각나지 않는데..

image
이렇게 1차 캐시에 없으면 null이 나더라고요. 원래 이랬던가요?? 사실 user는 id만 있어도 되는건데, 불필요하게 조회하는 느낌이 들었어요

Copy link
Member

Choose a reason for hiding this comment

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

오류가 저 api를 호출할 때 나는건가요?
우선 user_profile 테이블에서 user_id가 not null로 설정되어 있어서 save 할 때 null 이면 무조건 오류가 나는거 아닐까요!
그리고 어짜피 프로필을 등록하려는 user를 검증하려면 결국 user를 한 번은 조회해야 하지 않나요?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

네 api 호출때요! 아하 넵넵 그럼 문제가 없는걸로요!

@injoon2019 injoon2019 changed the title feat: 온보딩 선택을 등록하는 API 구현 [feature/12] feat: 온보딩 선택을 등록하는 API 구현 Jul 21, 2024
Copy link
Member

@miseongk miseongk left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!!
초기 세팅을 해주셔서 이후에 개발을 편하게 할 수 있을 것 같네요 ☺️

fk는 안걸어도 괜찮을까요?

fk를 걸지 말자는 의견이신건가요?! 저는 안걸고 해본적이 없어서 안걸었을 때의 장점을 아직 잘 모르겠어요! fk를 걸지 않으면 UserProfile에서 user_id에 null을 허용할 수 있다는 것 같은데 이렇게 해도 안전한지, 오히려 신경써야 할 포인트가 늘어나는게 아닌가 생각이 들었어요.
실제 회사에서는 fk를 잘 안건다고는 들었어서 이거는 인준님이 결정해주셔도 좋을 것 같아요!

Comment on lines 7 to 10
val job: String,
val smoking: String,
val alcohol: String,
val religion: String,
Copy link
Member

Choose a reason for hiding this comment

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

입력값에 대한 validation은 따로 하지 않아도 괜찮은가요?
저는 그동안 spring validation으로 처리했었는데 보통 어떻게 하는지 궁금해요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

요거는 하는분들마다 다른거 같아요! validation 체크를 하면 보통 길이에 대해 체크하셨나요?

Copy link
Member

Choose a reason for hiding this comment

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

네! 보통 Null, 길이, 숫자 체크했었어요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아하 요거는 그럼 뒤에 추가해볼게요!

Comment on lines 9 to 10
@Component
class ProfileFacade(
Copy link
Member

Choose a reason for hiding this comment

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

저는 파사드 클래스 써보진 않았어요!
서비스 레이어 간에 참조가 생기지 않게한다는 것은 service에서는 domain과 repository만 의존하도록 하고 dto의 참조를 끊기 위해서인가요?
이렇게 하는 이유가 dto가 변경될 가능성이 높아서 service까지 영향을 미치지 못하게 하기 위함인가요?!

저는 파사드 클래스 쓰는거 좋아요!
그런데 dto가 controller 패키지에 있으면 controller <-> facade 간의 양방향 의존성이 생기는데, dto를 facade에 넣는 것은 어떤가요?

) {

fun saveProfile(profileDto: RegisterProfileRequestDto) {
validateProfile(profileDto)
Copy link
Member

Choose a reason for hiding this comment

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

Facade 클래스에서 하는 일은 dto를 검증하는 일이 되겠군요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

맞아요 그리고 여러 서비스에서 조회한 것들을 합치고, 로직이 들어갑니다. 또 외부 api를 호출하는 클래스를 호출하기도하고요

@JoinColumn(name = "user_id")
var user: User? = null,

@Column(name = "profile_select")
Copy link
Member

Choose a reason for hiding this comment

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

컬럼에 name을 지정해주지 않아도 자동으로 camel case가 snake case로 매핑이 돼서 저는 굳이 명시하지 않아도 괜찮을 것 같아요! 어떻게 하는게 좋으신가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

저도 좋습니다!

Comment on lines 25 to 31
@Column(name = "created_at", updatable = false)
@CreationTimestamp
val createdAt: LocalDateTime = LocalDateTime.now(),

@Column(name = "updated_at")
@UpdateTimestamp
var updatedAt: LocalDateTime = LocalDateTime.now()
Copy link
Member

@miseongk miseongk Jul 22, 2024

Choose a reason for hiding this comment

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

찾아보니까 직접 초기화하지 않고

@Column(name = "created_at", updatable = false)
@CreationTimestamp
val createdAt: LocalDateTime? = null,

@Column(name = "updated_at")
@UpdateTimestamp
var updatedAt: LocalDateTime? = null

이렇게 해준다고 하네요!

그리고 createdAt과 updatedAt은 거의 모든 엔티티에 공동으로 사용될 것 같은데 BaseEntity를 만들어서 상속해서 사용하는 것은 어떤가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

createdAt과 updatedAt을 not null로 만들려고 했어요! not null로 하고 LocalDateTime.now() 를 넣어주지 않으면 스프링 코드에서 객체를 만들때 매번 넣어줘야 하는 번거로움이 있을거라 디폴트로 넣어줬어요!

BaseEntity 쓰는거 좋습니다!

Copy link
Member

Choose a reason for hiding this comment

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

아하 그렇군요!
@CreationTimestamp를 쓰면 결과적으로 값이 null이 아니게 되는데 not null로 만들려고 하시는 이유가 있나요?? 궁금해서요!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

db에서 테이블 정의와 엔티티 정의를 맞추기 위함이에요!

Comment on lines +19 to +25
@Transactional
fun saveProfile(userProfile: UserProfile): UserProfile {
val user = userRepository.findByIdOrNull(1L) // TODO User 회원 가입 기능 구현후 수정

userProfile.user = user
return profileRepository.save(userProfile)
}
Copy link
Member

Choose a reason for hiding this comment

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

오류가 저 api를 호출할 때 나는건가요?
우선 user_profile 테이블에서 user_id가 not null로 설정되어 있어서 save 할 때 null 이면 무조건 오류가 나는거 아닐까요!
그리고 어짜피 프로필을 등록하려는 user를 검증하려면 결국 user를 한 번은 조회해야 하지 않나요?!

Comment on lines +24 to +25
@OneToOne(mappedBy = "user", cascade = [CascadeType.ALL])
var userProfile: UserProfile? = null,
Copy link
Member

Choose a reason for hiding this comment

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

여기서 @OneToOne을 걸면 User와 UserProfile이 양방향 연관관계가 되는데, 굳이 양방향 관계를 가질 필요가 없을 것 같아요!
그래서 제거해도 될 것 같은데 어떠신가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

저도 없는게 좋습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Caused by: javax.persistence.PersistenceException: [PersistenceUnit: default] Unable to build Hibernate SessionFactory; nested exception is org.hibernate.MappingException: Could not determine type for: com.nexters.bottles.user.domain.User, at table: user_profile, for columns: [org.hibernate.mapping.Column(user_id)]

요거 없앴더니 이렇게 에러가 나요!

Copy link
Member

Choose a reason for hiding this comment

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

음 혹시 어떤부분을 없앤건가요? User에서 24,25 line 삭제했는데 저렇게 나오나요?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

네! 제가 이해를 잘 못했나 일단 요것도 뒤에서 챙겨볼게요!

Comment on lines 13 to 21
private val objectMapper = ObjectMapper().registerModule(
KotlinModule.Builder()
.withReflectionCacheSize(512)
.configure(KotlinFeature.NullToEmptyCollection, false)
.configure(KotlinFeature.NullToEmptyMap, false)
.configure(KotlinFeature.NullIsSameAsDefault, false)
.configure(KotlinFeature.StrictNullChecks, false)
.build()
)
Copy link
Member

Choose a reason for hiding this comment

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

이부분 JacksonConfig 내용과 동일한데 꼭 필요한건가요??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

objectMapper를 등록하는 부분은 필요한데 kotlinModule 설정 부분은 여러 클래스에서 쓸 수 있게 공통화 했어요 감사해요!

Comment on lines +1 to +4
CREATE TABLE user_profile (
id BIGINT AUTO_INCREMENT PRIMARY KEY,
user_id BIGINT NOT NULL,
profile_select JSON,
Copy link
Member

Choose a reason for hiding this comment

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

그냥 따로 기록해놨었어요! 여기에 저장해두는거 좋아요!

Comment on lines 18 to 21
@Column(name = "profile_select")
@Convert(converter = ProfileSelectConverter::class)
var profileSelect: ProfileSelect,

Copy link
Member

Choose a reason for hiding this comment

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

지금은 좋은 것 같아요!
그런데 혹시 나중에 '매칭할 때 키워드를 기반으로 매칭한다' 와 같은 조건이 생긴다면 어떻게 해야할지 생각해봐야할 것 같아요 🤔

Comment on lines 8 to 9
val smoking: String,
val alcohol: String,
Copy link
Member

Choose a reason for hiding this comment

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

스크린샷 2024-07-22 오후 5 41 38

저희 프로필 조회할 때는 무조건 위의 표에서 오른쪽 워딩으로 조회되는데 디비에 저장할 때 바꿔서 저장하는 것은 어떤가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오.. 감사합니다!

[Feature/15] 온보딩 선택지를 조회하는 api를 구현한다
Copy link
Member

@miseongk miseongk left a comment

Choose a reason for hiding this comment

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

확인했습니다!! 👍

@injoon2019 injoon2019 merged commit 237ea74 into develop Jul 22, 2024
1 check passed
@injoon2019 injoon2019 deleted the feature/12 branch July 22, 2024 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

온보딩 선택을 등록하는 API를 구현한다
2 participants