[chore] JPA entity 작성#13
Conversation
buzz0331
left a comment
There was a problem hiding this comment.
고생하셨습니다~~ 엔티티 갯수가 어마무시하네요 🥹
| @OneToMany(mappedBy = "aliasJpaEntity", cascade = CascadeType.ALL, orphanRemoval = true) | ||
| private List<CategoryJpaEntity> categories = new ArrayList<>(); |
There was a problem hiding this comment.
P1: 카테고리와 칭호는 1 : 1 매핑이였던 것 같습니다!
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "alias_id", nullable = false) | ||
| private AliasJpaEntity aliasJpaEntity; |
There was a problem hiding this comment.
P1: 이 부분도 1 : 1 매핑으로 바뀌어야 할 것 같네요
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "room_id") | ||
| private RoomJpaEntity roomJpaEntity; | ||
|
|
||
| private Integer page; | ||
|
|
||
| @Column(name = "is_overview",nullable = false) | ||
| private boolean isOverview; | ||
| } |
There was a problem hiding this comment.
P3: 매핑 관계를 나타내는 Column이 가장 아래에 있는 것이 좋을 것 같은데 어떻게 생각하시나욥
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "book_id") | ||
| private BookJpaEntity bookJpaEntity; | ||
|
|
||
| @Column(length = 50, nullable = false) | ||
| private String title; | ||
|
|
||
| @Column(length = 230, nullable = false) | ||
| private String description; | ||
|
|
||
| @Column(name = "is_public", nullable = false) | ||
| private boolean isPublic; | ||
|
|
||
| @Column(length = 4) | ||
| private Integer password; | ||
|
|
||
| @Column(name = "room_percentage",nullable = false) | ||
| private double roomPercentage; | ||
|
|
||
| @Column(name = "start_date",nullable = false) | ||
| private LocalDate startDate; | ||
|
|
||
| @Column(name = "end_date",nullable = false) | ||
| private LocalDate endDate; | ||
|
|
There was a problem hiding this comment.
P3: 여기서도 마찬가지로 매핑 관계를 나타내는 Column이 가장 아래에 있는 것이 보기 편할 것 같아요
| @Column(name = "item_name",length = 70, nullable = false) | ||
| private String itemName; | ||
|
|
||
| private int count; |
There was a problem hiding this comment.
P3: 마찬가지로 디폴트 값 있어도 될 것 같아요!
| package konkuk.thip.global.entity; | ||
|
|
||
| public enum JpaEntityStatus { | ||
| ACTIVE, INACTIVE, EXPIRED | ||
| } |
| import lombok.*; | ||
|
|
||
| @Entity | ||
| @Table(name = "tag") |
There was a problem hiding this comment.
P2: 테이블이름 복수형으로 일치시키는게 좋을 것 같습니다~
| import lombok.*; | ||
|
|
||
| @Entity | ||
| @Table(name = "saved_book") |
| import lombok.*; | ||
|
|
||
| @Entity | ||
| @Table(name = "saved_feed") |
…able = false) 누락 수정 (#11)
There was a problem hiding this comment.
Pull Request Overview
This PR introduces new JPA entity classes based on the provided ERD to support various domain models such as feeds, books, rooms, posts, comments, etc., and updates the CI workflow to perform a clean build.
- Added multiple entity classes with proper JPA relationships and cascade settings.
- Introduced composite key classes for entities with embedded ids.
- Updated the CI workflow to run a clean build before testing.
Reviewed Changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| SavedFeedJpaEntity.java | New JPA entity for saved feeds with user and feed associations. |
| SavedBookJpaEntity.java | New JPA entity for saved books with user and book associations. |
| RoomJpaEntity.java | New JPA entity for rooms with various attributes and a relation to BookJpaEntity. |
| RecordJpaEntity.java | New JPA entity with a shared primary key mapping to PostJpaEntity, including room association. |
| RecentSearchJpaEntity.java | New JPA entity for storing recent searches with search type and user relations. |
| PostLikeJpaEntity.java | New JPA entity handling post-like associations between posts and users. |
| PostJpaEntity.java | New JPA entity for posts including multiple one-to-one and one-to-many mappings. |
| NotificationJpaEntity.java | New JPA entity for notifications with basic attributes and user mapping. |
| FollowingJpaEntityId.java | New embeddable identifier for followings. |
| FollowingJpaEntity.java | New JPA entity for following relationships using a composite key. |
| FeedJpaEntity.java | New JPA entity for feeds with a shared primary key mapping and book association. |
| ContentJpaEntity.java | New JPA entity for content associated with posts. |
| CommentLikeJpaEntity.java | New JPA entity for comment likes linking users and comments. |
| CommentJpaEntity.java | New JPA entity for comments supporting nested replies. |
| CategoryJpaEntity.java | New JPA entity for categories with relation to an alias and tags. |
| BookJpaEntity.java | New JPA entity for books with various attributes and room associations. |
| AttendanceCheckJpaEntityId.java | New embeddable identifier for attendance checks. |
| AttendanceCheckJpaEntity.java | New JPA entity for attendance checks with room and user associations. |
| AliasJpaEntity.java | New JPA entity for aliases with basic attributes. |
| .github/workflows/ci-workflow.yml | Updated CI workflow to perform a clean build using Gradle. |
Comments suppressed due to low confidence (1)
src/main/java/konkuk/thip/entity/CategoryJpaEntity.java:29
- [nitpick] The field name 'aliasForCategoryJpaEntity' is overly verbose; consider renaming it to 'alias' to improve clarity.
private AliasJpaEntity aliasForCategoryJpaEntity;
| @Column(name = "is_public", nullable = false) | ||
| private boolean isPublic; | ||
|
|
||
| @Column(length = 4) |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
리뷰사항 반영해서
추가적으로 칭호와 카테고리를 1대1 매핑으로 수정하면서 칭호의 1대1매핑이 카테고리,유저로 두개로 늘어나면서 동시에 양방향 매핑을 하려다 보니, 중복 매핑 오류가 발생해서 칭호와 카테고리, 칭호와 유저의 1대1매핑을 단방향 매핑으로 수정했습니다 성준님코드 머지해서 패키지 구조에 jpa entity이동했습니다! 애매한것들이 많아서 확인해주시면 감사하겠습니닷! |
There was a problem hiding this comment.
erd 작성시 의논했던 database 제약사항들까지 꼼꼼하게 작성해주셨네요!! 정말 고생하셨습니다!
다만 코드를 보면서 양방향 연관관계에 대해서 @buzz0331, @hd0rable 두분과 조금 더 논의하고 싶다는 생각이 들었습니다.
이전에 양방향 매핑의 사용을 데이터 참조가 아니라, 데이터 삭제시로 한정한다면 n+1 문제가 발생하지 않고, jpa cascade 속성을 활용해 데이터 삭제를 편하게 할 수 있다는 얘기가 나왔었는데,
예를 들어 BookJpaEntity -> Book 으로의 매핑 시에
List roomIds 를 구성하기 위해 매번 BookJpaEntity와 연관관계를 맺고있는 RoomJpaEntity를 찾아 List roomIds에 추가해줘야하는 문제점이 있습니다.
이러면 결국 매번 도메인 엔티티 조회시마다 n+1 문제가 발생한다고 생각합니다.
(살짝 찾아보니 fetch join, DTO 전용 쿼리 등 차선책이 있는거 같지만 배보다 배꼽이 더 커지는거 같다는 생각입니다)
또한 api의 비즈니스 로직을 application 패키지와 domain 엔티티가 담당하게 되는데, jpa 의 양방향 매핑관계를 사용하여 delete를 진행하는 경우, 사실상 application 패키지는 request를 바로 영속성 adapter로 던지고 jpa의 양방향 매핑관계의 스펙에 맞춰 delete를 진행하게 되는게 이게 맞는건가 라는 생각도 들었습니다.
jpa 의 스펙을 어떻게 하면 잘 이용할까 보다는 도메인 중심적으로 생각하는게 어떨까 싶습니다!!
관련해서 고민해보면서 찾아본 reference들 공유드립니다!
https://www.inflearn.com/community/questions/1146350/%EB%8F%84%EB%A9%94%EC%9D%B8-%EA%B0%9D%EC%B2%B4%EC%99%80-%EC%98%81%EC%86%8D%EC%84%B1-%EA%B0%9D%EC%B2%B4%EB%A5%BC-%EA%B5%AC%EB%B6%84%ED%95%98%EA%B2%8C-%EB%90%98%EB%A9%B4?srsltid=AfmBOoqNWka37RGQodCtH22cU53EPdRCDRM7HJNS8aqdaSSjd6Ie6tCx
위 링크들을 읽다보니 멘토님이 왜 jpa를 단순히 DB table -> java object 의 변환용으로만 생각하시는지 살짝 공감이 되는거같기도,,
|
음 저두.. 이태껏 양방향관계로만 해와서 별 다른 생각없이 양방향 매핑관계로 엔티티 코드를 작성했는데 단방향 매핑으로 구현한다면 도메인과 영속성 엔티티 분리가 분리되서 좀더 도메인 중심적으로 구현하는게 좋을것같아보이긴합니다..! 다만 우려되는점은 제가 jpa엔티티를 패키지구조로 나누면서 엔티티가 어느 도메인으로 들어가야하는지 애매한것들이 많아서요.. 이 엔티티가 어디 도메인에 속해있는지 이해관계가 잘 되어있지않다면 구현할때 착오를 많이 일으킬 것 같긴합니다.. 어쩔수없는걸까요??🥲🥲 |
buzz0331
left a comment
There was a problem hiding this comment.
성준님 리뷰 확인했습니다! 음.. 저는 저희 ERD가 애초에 테이블간의 매핑이 연쇄적인 것이 많아서 추후에 하나의 엔티티의 삭제가 일어날 때, 그와 매핑되어 있는 삭제가 연쇄적으로 일어나는 것을 우리가 감당하는데에 부담을 느껴서 양방향 매핑을 고려한거였습니다. 하지만, 생각해보니 이 또한 jpa 스펙에 의존해서 헥사고날 아키텍처의 본질과 멀어지는 느낌이네요 🥹 그럼 일단 양방향 매핑은 없애는 걸로 하고 추후에 스케줄러 또는 이벤트 큐를 통한 비동기 삭제 전략를 도입해보는 걸로 하죠!
@hd0rable 이 부분은 저희가 이론적으로만 도메인을 크게 나누고 api 명세를 한 상태라 현 상황에서는 명확하게 경계를 구분하기는 어려워 보여요. 우선은, 희진님 판단하에 엔티티를 어느정도 나누고, 방학 시작하고 api 개발을 본격적으로 시작하기 전에 한번 모여서 각 도메인에 대해서 상의해보면 좋을 것 같아요~ |
#️⃣ 연관된 이슈
📝 작업 내용
현재 설계된 ERD를 보고 JPA entity를 작성했습니다
(mappedBy = "xxx", cascade = CascadeType.ALL, orphanRemoval = true)속성을 부여했습니다(fetch = FetchType.LAZY)속성을 부여했습니다.@Entity @Table(name = "xxx") @Getter @NoArgsConstructor(access = AccessLevel.PROTECTED) @AllArgsConstructor @Builder공통으로 적용됩니다value를 가지는Alias,Category,Tag의 경우 DBMS 예약어인value와 같아 구문 오류가 나는 것을 방지 하기 위하여 칼럼이름을해당엔티티_value로 명시해줬습니다.📸 스크린샷
💬 리뷰 요구사항
검토하긴했는데 혹시나 빠진 연관관계가 있는지 봐주시면 감사하겠습니닷!!
📌 PR 진행 시 이러한 점들을 참고해 주세요