RAC-462 feat : wish 분리 및 신청서 API #327
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,6 @@ | |
| import com.postgraduate.domain.member.user.application.dto.res.UserPossibleResponse; | ||
| import com.postgraduate.domain.member.user.application.mapper.UserMapper; | ||
| import com.postgraduate.domain.member.user.domain.entity.User; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Service; | ||
|
|
||
|
|
@@ -21,8 +20,7 @@ public UserInfoResponse getUserOriginInfo(User user) { | |
| } | ||
|
|
||
| public UserPossibleResponse checkSenior(User user) { | ||
| if (user.isSenior()) | ||
| return new UserPossibleResponse(true, user.getSocialId()); | ||
| return new UserPossibleResponse(false, user.getSocialId()); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ public class UserUtils { | |
|
|
||
| @PostConstruct | ||
| public void init() { | ||
| archiveUser = new User(-100L, -100L, null, "알수없음", "탈퇴한회원", profile, 0, null, false, null, null, false, false); | ||
| } | ||
|
|
||
| public User getArchiveUser() { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,27 @@ | ||||||||||||||||
| package com.postgraduate.domain.member.user.domain.entity; | ||||||||||||||||
|
|
||||||||||||||||
| import com.postgraduate.domain.member.user.domain.entity.constant.Role; | ||||||||||||||||
| import jakarta.persistence.*; | ||||||||||||||||
| import lombok.AllArgsConstructor; | ||||||||||||||||
| import lombok.Builder; | ||||||||||||||||
| import lombok.Getter; | ||||||||||||||||
| import lombok.NoArgsConstructor; | ||||||||||||||||
|
|
||||||||||||||||
| @Entity | ||||||||||||||||
| @Builder | ||||||||||||||||
| @AllArgsConstructor | ||||||||||||||||
| @NoArgsConstructor | ||||||||||||||||
| @Getter | ||||||||||||||||
| public class MemberRole { | ||||||||||||||||
| @Id | ||||||||||||||||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||||||||||||||||
| private Long roleId; | ||||||||||||||||
|
|
||||||||||||||||
| @Enumerated(EnumType.STRING) | ||||||||||||||||
| @Column(nullable = false) | ||||||||||||||||
| private Role role; | ||||||||||||||||
|
Comment on lines
+20
to
+22
There was a problem hiding this comment. 🛠️ Refactor suggestion Add validation to ensure role is never null While the column is marked as non-nullable in the database, the entity should validate this at the application level. Consider adding Bean Validation annotations. @Enumerated(EnumType.STRING)
@Column(nullable = false)
+@NotNull(message = "Role cannot be null")
private Role role;📝 Committable suggestion
Suggested change
|
||||||||||||||||
|
|
||||||||||||||||
| @ManyToOne(fetch = FetchType.LAZY) | ||||||||||||||||
| @JoinColumn(name = "user_id") | ||||||||||||||||
| private User user; | ||||||||||||||||
|
Comment on lines
+24
to
+26
There was a problem hiding this comment. 💡 Codebase verification Add index and foreign key constraint to improve query performance The suggestion to add an index and explicit foreign key constraint is valid as:
Regarding the transactional context concern:
Recommended changes:
🔗 Analysis chainConsider adding an index on the user_id foreign key The many-to-one relationship would benefit from an index on the foreign key column for better query performance, especially when querying roles by user. @ManyToOne(fetch = FetchType.LAZY)
-@JoinColumn(name = "user_id")
+@JoinColumn(name = "user_id", foreignKey = @ForeignKey(name = "fk_member_role_user"))
+@Index(name = "idx_member_role_user_id")
private User user;Also, be cautious with lazy loading. Ensure that the user field is always accessed within a transactional context to prevent LazyInitializationException. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check if @Transactional is properly used in services accessing MemberRole
ast-grep --pattern 'class $_ {
$$$
@Transactional
$$$
public $_ $_($$$) {
$$$
memberRole.getUser()
$$$
}
$$$
}'
Length of output: 137 Script: #!/bin/bash
# Let's first check for any references to MemberRole.getUser()
rg "memberRole.*getUser" -A 3
# Check for service classes that use MemberRole
rg "class.*Service.*\{" -A 5 | rg -B 5 "MemberRole"
# Check if there's any @Index annotation already on MemberRole
rg "@Index" src/main/java/com/postgraduate/domain/member/user/domain/entity/MemberRole.java
# Check for existing foreign key constraints in MemberRole
rg "@ForeignKey|@JoinColumn" src/main/java/com/postgraduate/domain/member/user/domain/entity/MemberRole.java
Length of output: 2361 Script: #!/bin/bash
# Let's check for @Transactional annotations in services that use MemberRole
rg "@Transactional" src/main/java/com/postgraduate/domain/member/user/domain/service/UserDeleteService.java
rg "@Transactional" src/main/java/com/postgraduate/domain/member/user/domain/service/UserUpdateService.java
rg "@Transactional" src/main/java/com/postgraduate/domain/member/user/domain/service/UserSaveService.java
# Check if there are any existing indexes in the entity
rg "Table|Index" src/main/java/com/postgraduate/domain/member/user/domain/entity/MemberRole.java
# Let's see the full MemberRole entity class to understand its structure
cat src/main/java/com/postgraduate/domain/member/user/domain/entity/MemberRole.java
Length of output: 1146 |
||||||||||||||||
| } | ||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |||||||||||||||
| import org.hibernate.annotations.*; | ||||||||||||||||
|
|
||||||||||||||||
| import java.time.LocalDateTime; | ||||||||||||||||
| import java.util.ArrayList; | ||||||||||||||||
| import java.util.List; | ||||||||||||||||
|
|
||||||||||||||||
| import static java.time.LocalDate.now; | ||||||||||||||||
|
|
@@ -46,10 +47,9 @@ public class User { | |||||||||||||||
| @Column(nullable = false) | ||||||||||||||||
| private int point; | ||||||||||||||||
|
|
||||||||||||||||
| @OneToMany(mappedBy = "user", fetch = FetchType.EAGER) | ||||||||||||||||
| @Builder.Default | ||||||||||||||||
| private List<MemberRole> roles = new ArrayList<>(); | ||||||||||||||||
|
Comment on lines
+50
to
+52
There was a problem hiding this comment. 🛠️ Refactor suggestion Consider using Currently, the Apply this diff to change the fetch type: - @OneToMany(mappedBy = "user", fetch = FetchType.EAGER)
+ @OneToMany(mappedBy = "user", fetch = FetchType.LAZY)📝 Committable suggestion
Suggested change
|
||||||||||||||||
|
|
||||||||||||||||
| @Column(nullable = false) | ||||||||||||||||
| private Boolean marketingReceive; | ||||||||||||||||
|
|
@@ -69,23 +69,29 @@ public class User { | |||||||||||||||
| @Builder.Default | ||||||||||||||||
| private boolean isDelete = false; | ||||||||||||||||
|
|
||||||||||||||||
| public boolean isJunior() { | ||||||||||||||||
| return roles.stream() | ||||||||||||||||
| .map(MemberRole::getRole) | ||||||||||||||||
| .toList() | ||||||||||||||||
| .contains(Role.USER); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| public boolean isSenior() { | ||||||||||||||||
| return roles.stream() | ||||||||||||||||
| .map(MemberRole::getRole) | ||||||||||||||||
| .toList() | ||||||||||||||||
| .contains(Role.SENIOR); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| public boolean isAdmin() { | ||||||||||||||||
| return roles.stream() | ||||||||||||||||
| .map(MemberRole::getRole) | ||||||||||||||||
| .toList() | ||||||||||||||||
| .contains(Role.ADMIN); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| public void updateRole(MemberRole memberRole) { | ||||||||||||||||
| roles.add(memberRole); | ||||||||||||||||
|
Comment on lines
+93
to
+94
There was a problem hiding this comment. Ensure idempotent role updates in Adding roles without checking for existing entries might introduce duplicates, especially if Apply this diff: public void updateRole(MemberRole memberRole) {
- roles.add(memberRole);
+ if (!roles.contains(memberRole)) {
+ roles.add(memberRole);
+ }
}Note: If you've converted 📝 Committable suggestion
Suggested change
|
||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| public void updateInfo(String profile, String nickName, String phoneNumber) { | ||||||||||||||||
|
|
@@ -143,8 +149,4 @@ public boolean isHardDelete() { | |||||||||||||||
| ) | ||||||||||||||||
| ); | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Critical: Null return in wishInfo() causes runtime risk in active endpoint
The method is actively used in the
/user/matching/{userId}endpoint where its result is directly added to the model. Returning null will cause issues when the template tries to access wishInfo attributes.AdminWithThymeLeafController.userMatching()adds the null response to model without null checkingImmediate action required to prevent runtime errors. Either:
🔗 Analysis chain
Critical: Replace null return with proper error handling.
Returning null with a TODO comment is problematic as it can lead to NullPointerException in consuming code. Consider these alternatives:
Here's a suggested improvement:
Let's check if this method is still being used:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 195
Script:
Length of output: 536