feat: 초기 엔티티 구축

[kth990303]

close #2

초기 엔티티를 구축했습니다.
도메인 프로퍼티 검증 로직은 별도로 추가하지 않았고, 엔티티 필드들만 추가해보았습니다.

누락된 프로퍼티, 오타, 수정되면 좋을 듯한 로직 있으면 편하게 리뷰 부탁드려요.
TODO 주석이 추가돼있으니 바로 merge하지 마시고 리뷰 부탁드립니다!

성현님 reviewers request에 추가가 안되네요... 그래서 우선 진우님이랑 설희님으로 request 요청했습니다.

[kth990303]

네이밍 고민입니다. TARDY라는 이름 괜찮은가요? 의견 부탁드려요

[bugoverdose]

[출석=ON_TIME, 지각=LATE] 혹은 [출석=ATTENDED, 지각=TARDY]는 어떨까요? 세트로 가져가고 싶어서요.

[ykc415]

[출석=ATTENDED, 지각=TARDY] 로 할까요 _ 쓰기 귀찮으니

[kth990303]

괜찮아보여서 반영했습니다~
혹시 다른 의견 있으면 코멘트 또 남겨주세요!

[xxeol2]

저도 ATTENDED/TARDY 좋아보여요!

[kth990303]

가장 고민됐던 부분입니다.
세션(Session)에는 17, 21과 같이 기수 여러 개를 띄워줘야 하는 프로퍼티를 포함해야 합니다.
단순 String으로 할 경우에, 시니어 회원이 참여했을 때 해당 프로퍼티 수정을 하드코딩으로 해야 합니다. 차라리 일대다 연관관계로 묶어서 처리하는 것이 낫다고 판단해서 이와 같이 설정해보았는데 이 부분 리뷰 부탁드려요.

[bugoverdose]

어 저는 Session에서 기수 정보는 하나만 오는걸로 이해했는데 아닌가요? 22기의 1주차 세션이면 기수 정보는 22만 되는 방식으로요. 그래서 OneToMany가 아니라 Long이나 Integer면 될 것 같았어요.

도메인 용어집의 예시 때문에 혼란이 온것 같아요. 17, 21을 17로 수정해야 할 것 같네요.

[ykc415]

저도 위와같이 Session에서 기수정보 하나만 오는걸로 이해했습니다.

[kth990303]

아 제가 오해했네요. 수정했습니다 :)

[kth990303]

이 부분을 도메인 로직에 넣는 것이 맞을까 고민했습니다.
따로 support나 config 패키지를 파서 하는 것이 나을까요?

+) 추가로, 이 로직은 관리자 암호화할 때에도 써야 되겠네요

[bugoverdose]

일단 여기서만 쓰인다면 지금처럼 가고, 필요에 따라 분리하는건 어떨까요?

[ykc415]

https://kotlinlang.org/docs/extensions.html#nullable-receiver
Kotlin 에서 이런 util function들은 보통 Extensions 로 빼서 여러곳에서 사용하는 편입니다.

[kth990303]

한가지 고민되는 점이 있다면 성현님께서 코멘트해주신 확장함수의 도입인데요.
확장함수를 이용하면 확실히 Password 외의 다른 암호화가 필요한 로직에서도 사용할 수 있어보입니다.

하지만 현재 도메인 규정 상 암호화가 필요한 로직이 Password에만 존재한다는 점때문에 조금 고민중이에요. 만약 확장함수로 fun String.sha256Encrypt() 와 같이 변경할 경우 Password 외에 다른 곳에서 사용될 가능성을 염두에 두어 Password VO에 해당 암호화 로직을 넣는 대신 별도의 패키지(ex. support)를 생성해서 로직을 만드는 게 좋아보이는데 다른 분들의 생각은 어떤지 궁금합니다.

이외에 혹시 다른 방법이 있거나 제가 잘 이해하지 못한 부분이 있다면 편하게 리뷰 부탁드립니다!

---

현재의 코드

data class Password(var value: String) {
    init {
        value = sha256Encrypt(value)
    }
}

fun sha256Encrypt(plainText: String): String = bytesToHex(SHA256.digest(plainText.toByteArray()))

확장함수를 이용한 코드

data class Password(var value: String) {
    init {
        value = value.sha256Encrypt()
    }
}

// 확장함수로 한다면 아래 함수는 Password 클래스에서 다른 클래스로 로직 이동시키는 것이 나을듯?
fun String.sha256Encrypt(): String = bytesToHex(SHA256.digest(this.toByteArray())) 

[ykc415]

내 좋습니다 해당 String 확장 함수는 extensions 패키지를 만들어 StringExtensions.kt 파일에 넣어두면 좋을것 같습니다.

[kth990303]

관리자 암호화 로직은 아직 작성안했습니다.
비밀번호 로직 위치 고민해보고 작성할 예정입니다.

[kth990303]

관리자 비밀번호 암호화 로직에 아래 고민이 있습니다.

관리자 역시 회원처럼 아이디와 비밀번호를 가지므로 Password 클래스를 사용하면 좋겠다 싶은데요.
문제는 이 Password 클래스가 member 패키지에만 있다는 점입니다.

현재 패키지 구조입니다.

따라서 아래와 같은 여러가지 방법이 있는데요.

1. administrator, member 상위 패키지인 user를 만들고 Password는 user패키지에 넣음.

• 이런 느낌으로 하여 비밀번호는 administrator, member 모두 쓸 수 있게

---

2. Password 클래스를 아예 member 패키지의 상위 패키지인 domain 패키지에 넣음

• 역시 Password 클래스는 administrator, member 모두 쓸 수 있음.
• Password가 상위 패키지인 도메인 패키지에 있는 게 맞는걸까? 출석, 기수회원 엔티티에는 사용되지도 않는다.

---

3. member Password, administrator Password 클래스 따로 만들음.

---

개인적으로 저는 1번이 끌리는데 다른 분들 의견도 들어보고 싶어요.
@ykc415 @bugoverdose @xxeol2
아니면 관리자는 일단 암호화 안한 상태로 냅두는 방법도 있을 것 같긴 합니다

[ykc415]

저는 복잡성을 줄이는 방향에서 2,3 번중 중 하나로 해결하면 된다고 생각합니다. 일단 admin 로직 분리 이전에는 2번이 좀더 맞는것 같기도 하네요

[bugoverdose]

개인적으로는 1안이 끌리는데 어떤 방법을 쓰든 큰 문제는 안될 것 같아요.

1안과 유사하지만 복잡성을 줄이는 방법으로
4안. administrator 패키지를 별도로 두지 말고, 일단 Administrator를 member 패키지에 추가하는 것도 방법일 것 같아요.

[xxeol2]

저도 뭘 해도 상관없는데 1안으로 하면 user도 좋은데 account도 괜찮은 것 같아요!
Password를 담은게 계정의 느낌이나서 ㅋㅋㅋ
물론 User도 좋습니다~!

[kth990303]

일단 1안으로 진행 및 user 패키지로 하겠습니다. 이후에 추가로 수정 필요할 경우 수정하면 좋을듯합니다 😄

[bugoverdose]

간단하게 대댓글로 리뷰 남겼슴다

[ykc415]

리뷰했습니다.

[ykc415]

https://kotlinlang.org/docs/extensions.html#nullable-receiver
Kotlin 에서 이런 util function들은 보통 Extensions 로 빼서 여러곳에서 사용하는 편입니다.

[kth990303]

리뷰 반영해봤는데 확인 부탁드립니다~
추가 request changes, 코멘트 적극 환영

[kth990303]

kotest 단위테스트를 작성해보았습니다.
비밀번호는 암호화된 값으로 반환한다와 같이 문장형으로 할지, 아니면 지금 형태가 괜찮은지 고민되는데 자유롭게 의견 부탁드립니다.

[bugoverdose]

개인적으로는 문장이 익숙한데 지금도 나쁘진 않은 것 같아요.
다만, 비밀번호 암호화 관련 테스트 케이스가 다양해지면 문구가 더 구체적이어야 할 것 같아요.

[ykc415]

네 나중에 테스트케이스가 복잡해지면 풀어서 설명해놓는게 좋을것 같습니다.

[xxeol2]

저도 더 복잡해지면 문장으로 명시해주는게 조금 더 명확할것같아요!
이 부분은 이후 개발하는 과정에서 요구사항이 명확해짐에따라 발전시켜나가면 좋을 것 같아요

[kth990303]

가장 고민됐던 부분입니다.
세션(Session)에는 17, 21과 같이 기수 여러 개를 띄워줘야 하는 프로퍼티를 포함해야 합니다.
단순 String으로 할 경우에, 시니어 회원이 참여했을 때 해당 프로퍼티 수정을 하드코딩으로 해야 합니다. 차라리 일대다 연관관계로 묶어서 처리하는 것이 낫다고 판단해서 이와 같이 설정해보았는데 이 부분 리뷰 부탁드려요.

제가 도메인 모델을 조금 오해했었어요.
위와 같이 수정했습니다.

[bugoverdose]

굿

[kth990303]

추가로 문득 든 생각인데, FK 없이 가는 것도 괜찮을 거 같다는 생각이 들긴 하는데 어떻게 생각하시나요?

장점을 뽑자면 FK 없이 간접참조로 진행하면 의존성 결합이 줄어들 수 있을 것 같아요. 단점을 뽑자면 N+1 문제나 정합성을 들 수 있을 것 같아요.

장단점이 명확해서 조금 고민되는 부분이네요

[bugoverdose]

JPA 연관관계 없이 generation_member_id라는 Long 타입 컬럼을 두자는 뜻 맞나요?

개인의 활동 점수 계산처럼 저희가 고려해야할 경우가 워낙 다양할 것 같아서 일단은 그렇게 가는 것도 좋을 것 같아요.

[ykc415]

저도 이부분은 취향차이라고 생각드는데 fk 없이가는게 좀더 쉬울것 같다는 생각은 드네요

[xxeol2]

저도 우선 FK없이 가는거 좋을 것 같아요!
이후에 작업 진행하다가 필요성을 느끼면 그 때 추가해도 늦지않을 것 같아요

[kth990303]

좋아요~ Fk 제거하는 쪽으로 수정하겠습니다

[bugoverdose]

개인적으로 "초기 엔티티 구축"이라 일단 지금처럼 가고 기능 구현으로 넘어가도 될 것 같아요.

어차피 API 구현하다보면 수정할게 생겨날 것 같아서요.

[bugoverdose]

개인적으로는 문장이 익숙한데 지금도 나쁘진 않은 것 같아요.
다만, 비밀번호 암호화 관련 테스트 케이스가 다양해지면 문구가 더 구체적이어야 할 것 같아요.

[bugoverdose]

굿

[bugoverdose]

nullable일 필요가 있나요?

GenerationMember 참조가 가능하다면 이 컬럼 없이 가보고,
만일 연관관계를 안쓴다면 generation_member_id와 member_id처럼 Long 컬럼을 두 개 두는건 어떨까 싶어요.

[kth990303]

아하 도메인 다이어그램이 있어서 추가했었는데, 생각해보니까 필요 시에 추가하자고 결론이 나왔었네요.
우선은 제거했습니다. 다른 분들도 괜찮은 의견 편하게 제시해주세요~

[bugoverdose]

JPA 연관관계 없이 generation_member_id라는 Long 타입 컬럼을 두자는 뜻 맞나요?

개인의 활동 점수 계산처럼 저희가 고려해야할 경우가 워낙 다양할 것 같아서 일단은 그렇게 가는 것도 좋을 것 같아요.

[bugoverdose]

이게 DB에 MALE, FEMALE처럼 저장되는건가요 아니면 "남자", "여자"로 저장되는건가요?

enum 쓸때 @Enumerated 어노테이션이 필요했던 것 같아서요.

Suggested change

	@Column(name = "gender", nullable = false, length = 10)
	val gender: Gender,
	@Enumerated(EnumType.STRING)
	@Column(name = "gender", nullable = false, length = 10)
	val gender: Gender,

[kth990303]

큰일날 뻔 했네요.
위와 같이 @Enumerated가 없으면 인덱스 숫자값이 저장됩니다.
바로 반영하겠습니다.

[ykc415]

리뷰 재확인했습니다

[ykc415]

네 나중에 테스트케이스가 복잡해지면 풀어서 설명해놓는게 좋을것 같습니다.

[bugoverdose]

몇 가지 커멘트 남겼는데, 일단은 이렇게 머지하고 진행해도 괜찮을 것 같아요.

[bugoverdose]

개인적으로는 1안이 끌리는데 어떤 방법을 쓰든 큰 문제는 안될 것 같아요.

1안과 유사하지만 복잡성을 줄이는 방법으로
4안. administrator 패키지를 별도로 두지 말고, 일단 Administrator를 member 패키지에 추가하는 것도 방법일 것 같아요.

[bugoverdose]

nullable일 필요가 있나 싶어요

[kth990303]

세션이 생성은 됐지만 출석을 아직 시작하지 않은 경우, 또는 출석 진행중이어서 endAttendTime이 없을 경우가 있으리라 판단했어서 nullable하게 했어요. nullable하지 않아도 괜찮을까요?

[bugoverdose]

제 기억에는 저번주 토요일에 짧게 옆의 FE+디자이너분들이랑 얘기했었을 때 저 컬럼을 아예 지우거나, 아니면 남기더라도 실제 출석 체크 관련 로직에는 써먹지 말자는 얘기가 나왔었어요.

저 시간 정보는 어드민이 참고만 하는거고, 실제로는 "출석 시작", "지각 시작", "완료?" 버튼을 눌렀을 때 출석 혹은 지각으로 저장되도록 하고요.

그래서 전 세션 시작할 때 기본 정보로 무조건 입력하는 정보라고 생각했었어요.

[bugoverdose]

기획 관련해서 나중에 더 얘기하고 확정지어야할 것 같으니 일단은 이대로 남겨놔도 괜찮을 듯요.

[xxeol2]

전 그 회의에서 진우님과 살짝 다르게 admin페이지에서 출석페이지를 띄웠다가 해당 페이지를 닫으면 그 닫은 시간이 endAttendTime에 저장된다고 이해했어요! 제가 이해한방향이 맞다면 해당 칼럼은 nullable이 맞을 것 같은데, 이 부분은 다음 전체회의에서 조금 더 명확히 하고 가면 좋을 것 같아요!!

[kth990303]

넵 일단 이대로 놓겠습니다~

[xxeol2]

수고 많으셨어요!!👏
제가 남긴 리뷰들은 대부분 취향차이 관련인 것 같아서 가볍게 봐주셔도 될 것 같아요 😆

[xxeol2]

저도 우선 FK없이 가는거 좋을 것 같아요!
이후에 작업 진행하다가 필요성을 느끼면 그 때 추가해도 늦지않을 것 같아요

[xxeol2]

position을 String말고 enum으로 하는건 어떻게생각하시나요!
개발자 / 디자이너 enum으로 가도 좋을 것 같아서 말씀드려요

[kth990303]

enum 좋네요 추가가능성을 고려하더라도 enum이 좋을 것 같네요 (클래스 복잡성 및 값을 꺼내야된다는 로직 추가가 증가하긴 하지만).
enum으로 변경하는거에 동의합니다~

[xxeol2]

subPosition도 enum으로 해도 좋을 것 같은데 어떻게 생각하시나요!
subPosition은 position보다는 변화가능성이 있지만 (향후 AI개발자 추가라던가..) 변화가능성이 커보이진 않아서요!

[kth990303]

이것도 동의합니다

[bugoverdose]

좋은 것 같아요. 만약 enum 맞지 않는게 추가되면 저희 백에서 enum 타입 추가하면 되니까 문제 없을 것 같아요.

다만, 이전 기수들에서 BE vs 백엔드 vs 서버처럼 같은 대상을 다양하게 지칭하는 경우도 있을 것 같은데, 혹시 문제가 될지 나중에 추가로 논의해봐도 좋을 것 같아요.

[kth990303]

디자이너의 경우 SubPosition이 없을 수도 있겠다는 생각도 드네요.
일단 subPosition의 목록은 아래와 같이 수정했습니다.

enum class SubPosition(val value: String) {
    BE("백엔드"),
    FE("프론트엔드"),
    ANDROID("안드로이드"),
    IOS("IOS"),
    DESIGNER("디자이너"), // 일단은 넣어둠.
}

그리고 아래 컬럼도 nullable=false를 제거하는 방향으로 택했습니다.

@Column(name = "sub_position", length = 30) // nullable = false 제거
val subPosition: SubPosition,

[xxeol2]

최초에는 score가 100으로 초기화되기때문에 이수가능성도 true로 초기화해도 좋을 것 같아요!
그러고 매주 세션을 진행할 때 score를 업데이트 해주면서 특정 점수 이하인 회원의 값을 false로 바꾸는 방식은 어떻게 생각하시나요?

[kth990303]

오 예리한 지적 좋습니다~

[bugoverdose]

설명 들어보니 맞는 것 같아요. 처음에는 boolean의 초기값이 false라는 점이 자연스러워보였는데, 100점이니 true로 시작하는게 도메인 규칙에 부합하는 것 같네요.

한편으로는 아예 isCompletable라는 컬럼은 없애버리고, 조회 시점에 score를 토대로 isCompletable 값을 계산하는건 어떨까도 싶네요. 그러면 두 컬럼 간에 불일치가 예방될 것 같아서요.

[xxeol2]

진우님 말씀에 완전 동의합니다!!
다만 걱정이 되는건

1. 이전 기수정보중에 활동점수가 남아있지 않는 정보
2. 이수가능 커트라인 점수가 기수별로 다를 경우
   이 두가지 경우인데,
   1번의 경우 임의로 100점으로 설정해도 좋을 것 같고
   2번의 경우는 사실 isCompletable 칼럼의 전재유무와 상관없이 신경써야할부분이기때문에
   해당 내용들을 나중에 개발진행하며 체크해도 괜찮을 것 같아요!

[bugoverdose]

2번이 좀 아찔하네요. 기수별로 수료 커트라인을 별도로 관리해야 한다면 기수(generation) 테이블을 별도로 만들고 커트라인 컬럼을 관리해야 할 것도 같네요.

일단은 지금처럼 갈까요?

[xxeol2]

2번의 경우는 일단 고려하지않고 지금처럼 가거나 앞서 진우님이 말씀하신것처럼 isCompletable 없애는 방향으로 가는게 좋을 것 같아요!
커트라인이 바뀌는건 동아리 회칙이 변하는 특수한경우니까요~!

[kth990303]

한편으로는 아예 isCompletable라는 컬럼은 없애버리고, 조회 시점에 score를 토대로 isCompletable 값을 계산하는건 어떨까도 싶네요. 그러면 두 컬럼 간에 불일치가 예방될 것 같아서요.

음 저는 일단 isCompletable 컬럼을 없애지는 않는 방향으로 했습니다.
나중에 도메인로직(점수변경이 있을 때 isCompletable 여부 변경하는 로직)을 추가하는 방향으로 하거나, 이후에 문제가 생기면 없애도 될 것 같아서 우선 var isCompletable: Boolean = true로만 변경해둘게요~

[xxeol2]

전 그 회의에서 진우님과 살짝 다르게 admin페이지에서 출석페이지를 띄웠다가 해당 페이지를 닫으면 그 닫은 시간이 endAttendTime에 저장된다고 이해했어요! 제가 이해한방향이 맞다면 해당 칼럼은 nullable이 맞을 것 같은데, 이 부분은 다음 전체회의에서 조금 더 명확히 하고 가면 좋을 것 같아요!!

[xxeol2]

저도 더 복잡해지면 문장으로 명시해주는게 조금 더 명확할것같아요!
이 부분은 이후 개발하는 과정에서 요구사항이 명확해짐에따라 발전시켜나가면 좋을 것 같아요

[xxeol2]

저도 뭘 해도 상관없는데 1안으로 하면 user도 좋은데 account도 괜찮은 것 같아요!
Password를 담은게 계정의 느낌이나서 ㅋㅋㅋ
물론 User도 좋습니다~!