feat: 번쩍 관련 객체 정의 #508

mikekks · 2024-12-21T13:05:57Z

👩‍💻 Contents

번쩍 관련 객체를 정의했습니다. 아래 내용보시면서 봐주시면 좋을 것 같습니다! 이외에도 변수명, 타입 등등 여러 피드백 환영입니다!
https://www.notion.so/sopt-makers/11de266d58294416a221d38b5a614310?pvs=4

📝 Review Note

📣 Related Issue

closed feat: 번쩍 객체 정의 #507

✅ 점검사항

docker-compose.yml 파일에 마이그레이션 한 API의 포워딩을 변경해줬나요?
Spring Secret 값을 수정하거나 추가했다면 Github Secret에서 수정을 해줬나요?
Nestjs Secret 값을 수정하거나 추가했다면 Docker-Compose.yml 파일 및 인스턴스 내부의 .env 파일을 수정했나요?

height · 2024-12-21T13:05:59Z

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

hoonyworld

고생하셨습니다 민규님!

몇가지 수정하면 좋을 것 같은 부분이 있어서 코멘트 남겼습니다

hoonyworld · 2024-12-23T11:24:42Z

main/src/main/java/org/sopt/makers/crew/main/entity/lightning/Lightning.java

+	@GeneratedValue(strategy = GenerationType.IDENTITY)
+	private Integer id;
+
+	@NotNull


이 @NotNull에 대해서 TMI 하나 있습니다. (지식 공유입니다 ㅎㅎ)

저번에 지적해주신대로 NotNull이 데이터베이스에 SQL 쿼리를 보내기 전에 예외를 발생시켜서 저도 다른 프로젝트에서 개인적으로 잘 사용하고 있었습니다!!

그런데 Oracle DB에서는 해당 어노테이션이 적용이 안되는 이슈가 있더라고요 😭(nullable인 상태로 칼럼이 만들어짐)
그래서 앞으로 실무에서 사용을 하려면 미리 JPA Entity 생성 테스트를 해보는게 좋은 것 같다는 생각이 들었습니다!

hoonyworld · 2024-12-23T11:36:06Z

main/src/main/java/org/sopt/makers/crew/main/entity/lightning/Lightning.java

+	@Column(name = "leaderUserId")
+	private Integer leaderUserId;


/** * 개설한 유저 */ @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "userId", nullable = false) private User user; /** * 유저 id */ @Column(insertable = false, updatable = false) private Integer userId;

모임 테이블의 일부를 가져왔습니다! 이와 관련해서 대해 몇가지 궁금한 점이 있습니다.

모임 엔티티에서는 User user와 Integer userId를 모두 선언하면서, Integer userId를 insertable과 updatable 옵션으로 읽기 전용으로 관리하도록 설정하신 것 같습니다. 그런데 이번 번쩍 모임 엔티티에서는 이를 적용하지 않으신 이유가 궁금합니다!

기존 모임 엔티티에서 Integer userId를 추가로 두신 이유는, userId만 필요할 때 연관된 User 객체를 불러오지 않아도 되어서(쿼리가 1개 덜 발생) 성능 최적화를 위해 사용하신 것이 맞는지 궁금합니다!

흠.. 일단 기존 Meeting 구조는 레거시라 정확한 의도를 모르겠습니다..!

1번에 대해 말씀드리자면, 첨부해주신 코드도 그렇고 사실 예전 엔티티 구조에서는 양방향 의존이 굉장히 많이 있었습니다. 그로인해 알 수 없는 쿼리가 굉장히 많이 발생하였고, 더티체킹에도 문제가 많았습니다.

그러한 문제의 원인으로 생각했던 것이 아래와 같습니다.

무분별한 연관관계 사용

의도를 알 수 없는 코드

그 후 객체지향에 대해 공부하면서 웬만하면 연관관계를 걸지 않되, 라이프사이클이 아예 같은 객체에 한해서 연관관계를 고민한다. 라는 법칙을 가지고 엔티티를 정리하는 작업을 거친 것이 현재 엔티티들인데요. 그래서 이후에 만들어진 엔티티는 최대한 연관관계를 걸지 않고 있습니다. 다만 첨부해주신 코드는 연관관계를 없애면 사이드이펙트가 너무 커서 현재는 방치해둔 상황입니다..!

그때 당시 참고한 영상 첨부드립니다! 혹시 안보셨다면 추천드립니다 !!
https://www.youtube.com/watch?v=dJ5C4qRqAgA&t=5208s
https://www.youtube.com/watch?v=6q0-IT5J0nI&t=920s

그리고 2번에 대해서 말씀드려보겠습니다. fk 만 필요할때, meeting.getUser().getId() 를 하면 쿼리가 안 생기는 것으로 알고 있습니다! meeting.getUser().getName() 과 같은 코드에서는 쿼리가 발생하고요! 그래서 더 정확한 의도를 파악하기 어려운 것 같습니다...

hoonyworld · 2024-12-23T11:38:13Z

main/src/main/java/org/sopt/makers/crew/main/entity/lightning/Lightning.java

+
+	@NotNull
+	@Size(min = 0, max = 30)
+	private String title;


min 값을 1로 설정하는 것이 어떨까요?
min 값이 0이면 DB에 빈 문자열로 제목이 저장될 가능성이 있어 바람직하지 않을 것 같습니다. (UI/UX 측면에서도요!)

그리고 이참에 Meeting 테이블의 title에도 Size 어노테이션을 붙이면 좋을 것 같아요!!
(일반 모임도 제목이 30자 제한이더라고요!)

커밋에 따라 리뷰를 달다보니 마지막 커밋에 반영이 된 걸 못봤네요 😭
Size 관련 리뷰만 답변 달아주시면 감사하겠습니다!

아 네넵 좋습니다! 다만 프론트에서 처리하는게 먼저 되어야 할 것 같네요! 프론트랑 싱크맞춰보겠습니다!

hoonyworld · 2024-12-23T11:44:07Z

main/src/main/java/org/sopt/makers/crew/main/entity/lightning/Lightning.java

+	@NotNull
+	@Size(max = 1)
+	private List<ImageUrlVO> imageURL;


이 부분도 Meeting 에서도 이참에 정책을 통일했으면 좋겠습니다 ㅎㅎ

넵 좋습니다!

hoonyworld · 2024-12-23T11:47:58Z

main/src/main/java/org/sopt/makers/crew/main/entity/lightning/Lightning.java

+	private int capacity;
+
+	private String note;


이 부분에서 @column 어노테이션이 빠진 것 같아요! Meeting 테이블의 코드를 첨부하겠습니다.

@Column(name = "capacity", nullable = false) private Integer capacity; @Column(name = "note") private String note;

사실 모두 소문자라 그대로 뒀는데 수정하겠습니다!

hoonyworld · 2024-12-23T11:54:20Z

main/src/main/java/org/sopt/makers/crew/main/entity/tag/TagType.java

@@ -0,0 +1,6 @@
+package org.sopt.makers.crew.main.entity.tag;
+
+public enum TagType {


나중에 필요하면 @Getter나 @requiredargsconstructor를 추가해서 상수를 관리하면 좋을 것 같습니다!

네넵! 나중에 필요할 때 하면 좋을 것 같아요!

hoonyworld · 2024-12-23T11:55:25Z

main/src/main/java/org/sopt/makers/crew/main/entity/tag/Tag.java

+	@Column(name = "meetingId")
+	@NotNull
+	private Integer meetingId;
+
+	@Column(name = "lightningId")
+	@NotNull
+	private Integer lightningId;


위에서 모임태그일 경우, lightningId == null, 번쩍태그일 경우, meetingId == null이라고 말씀하셨는데, 그렇다면 두 필드에서 NotNull 어노테이션을 빼야할 것 같습니다!

엇 그러네요..! 감사합니다 !!

hoonyworld · 2024-12-23T11:57:07Z

main/src/main/java/org/sopt/makers/crew/main/entity/lightning/Lightning.java

+@Getter
+@NoArgsConstructor(access = AccessLevel.PROTECTED)
+@Table(name = "lightning")
+public class Lightning extends BaseTimeEntity {


생성자(or 정적 팩토리 메서드)가 빠진 것 같습니다! 해당 부분도 밑에 정의해주시면 좋을 것 같아요!

넵 좋습니다 !!

hoonyworld · 2024-12-23T11:57:35Z

main/src/main/java/org/sopt/makers/crew/main/entity/tag/Tag.java

+import jakarta.persistence.Id;
+import jakarta.validation.constraints.NotNull;
+
+public class Tag extends BaseTimeEntity {


테이블 설정시 필요한 어노테이션과 생성자(or 정적 팩토리 메서드) 선언이 빠진 것 같습니다!

추가로 Tag 엔티티는 번쩍 View에서 봤을 때 어느 부분에 사용하기 위한 엔티티일까요? 제가 이해를 못했습니다...

이 부분은 개인적으로 연락주셔도 좋을 것 같습니다!

엇 그러네요! 추가하겠습니다!

이게 아마 제가 첨부한 노션에서 보셔야 있습니다! 아직 화면에는 없습니다...

mikekks added 3 commits December 21, 2024 22:04

feat(Lightning): Lightning 객체 정의

c8890ea

feat(Tag): Tag 객체 정의

0b67697

feat(TagType): TagType 객체 정의

3152314

mikekks added the 🎁 feature 새로운 기능 label Dec 21, 2024

mikekks requested a review from hoonyworld December 21, 2024 13:05

mikekks self-assigned this Dec 21, 2024

pull-request-size bot added the size/L label Dec 21, 2024

chore(Lightning): 최소 글자수 변경

f6bbe75

hoonyworld requested changes Dec 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: 번쩍 관련 객체 정의 #508

feat: 번쩍 관련 객체 정의 #508

mikekks commented Dec 21, 2024

height bot commented Dec 21, 2024

hoonyworld left a comment

hoonyworld Dec 23, 2024

hoonyworld Dec 23, 2024

mikekks Dec 23, 2024 •

edited

Loading

hoonyworld Dec 23, 2024

hoonyworld Dec 23, 2024

hoonyworld Dec 23, 2024

mikekks Dec 23, 2024

hoonyworld Dec 23, 2024

mikekks Dec 23, 2024

hoonyworld Dec 23, 2024

mikekks Dec 23, 2024

hoonyworld Dec 23, 2024

mikekks Dec 23, 2024

hoonyworld Dec 23, 2024

mikekks Dec 23, 2024

hoonyworld Dec 23, 2024

mikekks Dec 23, 2024

hoonyworld Dec 23, 2024

hoonyworld Dec 23, 2024

mikekks Dec 23, 2024

mikekks Dec 23, 2024

		@@ -0,0 +1,6 @@
		package org.sopt.makers.crew.main.entity.tag;

		public enum TagType {

feat: 번쩍 관련 객체 정의 #508

Are you sure you want to change the base?

feat: 번쩍 관련 객체 정의 #508

Conversation

mikekks commented Dec 21, 2024

👩‍💻 Contents

📝 Review Note

📣 Related Issue

✅ 점검사항

height bot commented Dec 21, 2024

hoonyworld left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikekks Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikekks Dec 23, 2024 •

edited

Loading