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

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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
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

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 = 1, 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;
}
40 changes: 40 additions & 0 deletions main/src/main/java/org/sopt/makers/crew/main/entity/tag/Tag.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package org.sopt.makers.crew.main.entity.tag;

import org.sopt.makers.crew.main.entity.common.BaseTimeEntity;

import jakarta.persistence.Column;
import jakarta.persistence.EnumType;
import jakarta.persistence.Enumerated;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.validation.constraints.NotNull;

public class Tag 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

Choose a reason for hiding this comment

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

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

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;

/**
* @implSpec : 모임태그 or 번쩍태그 구분
* @implNote : 모임태그일 경우, lightningId == null
* @implNote : 번쩍태그일 경우, meetingId == null
* */
@Enumerated(EnumType.STRING)
@Column(name = "tagType")
@NotNull
private TagType tagType;

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

@Column(name = "lightningId")
@NotNull
private Integer lightningId;
Comment on lines +29 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.

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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


@NotNull
private String content;

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package org.sopt.makers.crew.main.entity.tag;

public enum TagType {
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

MEETING,
LIGHTING
}
Loading