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.

Sign up for GitHub

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: 번쩍 관련 객체 정의 #508

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
+125 −0
Open

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

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c8890ea
feat(Lightning): Lightning 객체 정의
mikekks Dec 21, 2024
0b67697
feat(Tag): Tag 객체 정의
mikekks Dec 21, 2024
3152314
feat(TagType): TagType 객체 정의
mikekks Dec 21, 2024
f6bbe75
chore(Lightning): 최소 글자수 변경
mikekks Dec 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions main/src/main/java/org/sopt/makers/crew/main/entity/lightning/Lightning.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package org.sopt.makers.crew.main.entity.lightning;

import java.time.LocalDate;
import java.time.LocalDateTime;
import java.util.List;

import org.hibernate.annotations.Type;
import org.sopt.makers.crew.main.entity.common.BaseTimeEntity;
import org.sopt.makers.crew.main.entity.meeting.vo.ImageUrlVO;

import io.hypersistence.utils.hibernate.type.json.JsonBinaryType;
import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.Table;
import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Size;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.NoArgsConstructor;

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

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

넵 좋습니다 !!

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

@NotNull
Copy link
Member

Choose a reason for hiding this comment

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

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

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

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

@Column(name = "leaderUserId")
private Integer leaderUserId;
Comment on lines +34 to +35
Copy link
Member

Choose a reason for hiding this comment

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

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

/**
* 유저 id
*/
@Column(insertable = false, updatable = false)
private Integer userId;

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

  1. 모임 엔티티에서는 User user와 Integer userId를 모두 선언하면서, Integer userId를 insertable과 updatable 옵션으로 읽기 전용으로 관리하도록 설정하신 것 같습니다. 그런데 이번 번쩍 모임 엔티티에서는 이를 적용하지 않으신 이유가 궁금합니다!
  2. 기존 모임 엔티티에서 Integer userId를 추가로 두신 이유는, userId만 필요할 때 연관된 User 객체를 불러오지 않아도 되어서(쿼리가 1개 덜 발생) 성능 최적화를 위해 사용하신 것이 맞는지 궁금합니다!

Copy link
Member Author

@mikekks mikekks Dec 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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

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

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

  1. 무분별한 연관관계 사용
  2. 의도를 알 수 없는 코드

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

그때 당시 참고한 영상 첨부드립니다! 혹시 안보셨다면 추천드립니다 !!
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() 과 같은 코드에서는 쿼리가 발생하고요! 그래서 더 정확한 의도를 파악하기 어려운 것 같습니다...


@NotNull
@Size(min = 0, max = 30)
private String title;
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

image

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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


@Column(name = "imageURL", columnDefinition = "jsonb")
@Type(JsonBinaryType.class)
@NotNull
@Size(max = 1)
private List<ImageUrlVO> imageURL;
Comment on lines +43 to +45
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

넵 좋습니다!


@Column(columnDefinition = "TEXT")
private String desc;

@Column(name = "activityStartDate")
@NotNull
private LocalDate activityStartDate;

@Column(name = "activityEndDate")
@NotNull
private LocalDateTime activityEndDate;

@Column(name = "meetingTime")
private LocalDateTime meetingTime;

@Column(name = "meetingPlace")
@NotNull
private String meetingPlace;

@Column(name = "applicationStartDate")
@NotNull
private LocalDateTime applicationStartDate;

@Column(name = "applicationEndDate")
@NotNull
private LocalDateTime applicationEndDate;

private int capacity;

private String note;
Comment on lines +73 to +75
Copy link
Member

Choose a reason for hiding this comment

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

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

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

@Column(name = "note")
private String note;

Copy link
Member Author

Choose a reason for hiding this comment

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

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


@Column(name = "leaderDesc", columnDefinition = "TEXT")
private String leaderDesc;
}