-
Notifications
You must be signed in to change notification settings - Fork 2
전반적인 도메인 로직 내에서 validation 체크 및 NPE 안정성 보강 #250
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
Changes from all commits
cbde362
fe516dc
6e365b9
510da44
84a88f2
cc3e589
36f5a58
c9f0463
77ec41d
59a4aee
da821bd
eb090a8
f883a22
37fc049
1562bbc
af910d3
b9e1687
bc960b8
ebfa48d
f279815
795a6e9
6ea148a
9e44526
b61bb05
50cba22
553dd16
5bc6c38
6341200
921b2ef
5d7fb7b
46e7c7a
34a04b5
6bd062b
caa0723
df0354e
572ddb0
b680c70
bc7d613
40f61d0
a0a4473
c5be4ad
a0fd7db
6a43347
53b4e11
1d40637
febbe32
e784335
50915bc
24f6e2e
9934b7d
2521b11
03357a6
affed22
0a9d3ab
6527121
7f0bf73
3dbbf21
da95311
3f0a973
08a58cd
5e01c1f
867e96d
533528e
1dad3f7
735cbf3
8a58f8a
7a89399
301188d
8e786f0
a5226a9
24695b6
ecd6305
1359898
2c7a1a2
20381b4
5de298d
56d4ffe
445393a
c0fa37c
348ff8a
e6b5d58
0373d2b
4cba953
95e6ff8
a63a58b
91ddf3a
f783688
13a6ca9
96f7f5f
22a2b70
a01febf
da43546
1e4f080
8af2cdb
f8e1232
76d795e
ee528cf
195f97e
3bc3eef
b7d6fb9
2c99aaa
ff671d9
d0157b1
05927d7
25ae7f0
c0576ab
5264184
5d270db
8d2f64f
9b6d103
b01684d
f410cab
9acbab2
28cf726
3c34ef3
4db5506
9716879
37b8e9c
04fe08c
0fc0ead
09a3dea
c8c936c
3a9d195
5951dc1
f85c830
a46f2ac
cd6da2b
c825149
26e233c
201b2b8
ec85327
c72ab75
e9372d1
29053d7
a1d61d4
d7ecbe2
d3d563a
64800d9
d2a4f2d
716093d
14dcdf9
4084362
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,8 @@ | ||
| package life.mosu.mosuserver.application.admin; | ||
|
|
||
| import life.mosu.mosuserver.domain.examapplication.repository.ExamApplicationJpaRepository; | ||
| import life.mosu.mosuserver.domain.refund.repository.RefundJpaRepository; | ||
| import life.mosu.mosuserver.domain.refund.repository.RefundFailureLogJpaRepository; | ||
| import life.mosu.mosuserver.domain.user.entity.UserRole; | ||
| import life.mosu.mosuserver.domain.user.repository.UserJpaRepository; | ||
| import life.mosu.mosuserver.presentation.admin.dto.DashBoardResponse; | ||
| import lombok.RequiredArgsConstructor; | ||
|
|
@@ -13,15 +14,19 @@ public class AdminDashboardService { | |
|
|
||
| private final ExamApplicationJpaRepository examApplicationJpaRepository; | ||
| private final UserJpaRepository userJpaRepository; | ||
| private final RefundJpaRepository refundJpaRepository; | ||
| private final RefundFailureLogJpaRepository refundFailureLogJpaRepository; | ||
|
|
||
| // 대시보드 정보 조회 | ||
| public DashBoardResponse getAll() { | ||
| Long applicationCounts = examApplicationJpaRepository.count(); | ||
| Long refundCounts = refundJpaRepository.count(); | ||
| Long userCounts = userJpaRepository.count(); | ||
| return new DashBoardResponse(applicationCounts, refundCounts, userCounts); | ||
| } | ||
|
|
||
| Long applicationCounts = examApplicationJpaRepository.countAll(); | ||
| Long refundAbortedCounts = refundFailureLogJpaRepository.count(); | ||
| Long userCounts = userJpaRepository.countByUserRoleNot(UserRole.ROLE_ADMIN); | ||
|
Comment on lines
+22
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chaincount 계열은 primitive 사용 권장 + 시그니처/의도 재확인
적용 가능한 최소 변경(박싱 제거): - Long applicationCounts = examApplicationJpaRepository.countAll();
- Long refundAbortedCounts = refundFailureLogJpaRepository.count();
- Long userCounts = userJpaRepository.countByUserRoleNot(UserRole.ROLE_ADMIN);
+ long applicationCounts = examApplicationJpaRepository.countAll();
+ long refundAbortedCounts = refundFailureLogJpaRepository.count();
+ long userCounts = userJpaRepository.countByUserRoleNot(UserRole.ROLE_ADMIN);읽기 전용 트랜잭션 부여(스냅샷 일관성 및 미세한 성능 이점): // add import:
import org.springframework.transaction.annotation.Transactional;
// annotate method:
@Transactional(readOnly = true)
public DashBoardResponse getAll() { ... }🏁 Script executed: #!/bin/bash
# Locate the repository interface for examApplicationJpaRepository
repo_file=$(fd --full-path "ExamApplicationJpaRepository.java")
echo "Found file: $repo_file"
# Show the countAll method signature
rg -n "countAll\s*\(" "$repo_file"
# Also confirm the return types for other count methods
rg -n "countByUserRoleNot\s*\(" "$repo_file"Length of output: 694 🏁 Script executed: #!/bin/bash
# Locate UserJpaRepository and show the signature of countByUserRoleNot
user_repo=$(fd --full-path "UserJpaRepository.java")
echo "UserJpaRepository found at: $user_repo"
rg -n "countByUserRoleNot" "$user_repo"
# Locate RefundFailureLogJpaRepository and show count signature
refund_repo=$(fd --full-path "RefundFailureLogJpaRepository.java")
echo "RefundFailureLogJpaRepository found at: $refund_repo"
rg -n "count" "$refund_repo" | head -n 10
# Check AdminDashboardService for existing @Transactional on getAll method
service_file=$(fd --full-path "AdminDashboardService.java")
echo "Service file: $service_file"
rg -n "@Transactional" "$service_file"
rg -n "public DashBoardResponse getAll" -n "$service_file"Length of output: 1690 🏁 Script executed: #!/bin/bash
# Show context around countAll in ExamApplicationJpaRepository to verify if it's custom or just alias
rg -n -A3 -B3 'countAll' src/main/java/life/mosu/mosuserver/domain/examapplication/repository/ExamApplicationJpaRepository.javaLength of output: 264 Use primitive
Locations:
Suggested changes: --- a/src/main/java/life/mosu/mosuserver/application/admin/AdminDashboardService.java
+++ b/src/main/java/life/mosu/mosuserver/application/admin/AdminDashboardService.java
@@ -20,6 +20,7 @@
import org.springframework.transaction.annotation.Transactional;
+@Transactional(readOnly = true)
public class AdminDashboardService {
public DashBoardResponse getAll() {
- Long applicationCounts = examApplicationJpaRepository.countAll();
- Long refundAbortedCounts = refundFailureLogJpaRepository.count();
- Long userCounts = userJpaRepository.countByUserRoleNot(UserRole.ROLE_ADMIN);
+ long applicationCounts = examApplicationJpaRepository.countAll();
+ long refundAbortedCounts = refundFailureLogJpaRepository.count();
+ long userCounts = userJpaRepository.countByUserRoleNot(UserRole.ROLE_ADMIN);
// ...
}
}And in the repository: // src/.../ExamApplicationJpaRepository.java
/**
* Counts exam applications with paymentStatus = DONE and not deleted.
*/
@Query("""
SELECT COUNT(p) FROM ExamApplication p
WHERE p.paymentStatus = 'DONE'
AND p.deleted = false
""")
long countDoneNonDeleted();🤖 Prompt for AI Agents |
||
|
|
||
| return DashBoardResponse.of( | ||
| applicationCounts, | ||
| refundAbortedCounts, | ||
| userCounts | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,29 +6,41 @@ | |||||||||||||||||||||||||||
| import java.util.Set; | ||||||||||||||||||||||||||||
| import java.util.stream.Collectors; | ||||||||||||||||||||||||||||
| import life.mosu.mosuserver.application.exam.cache.ExamQuotaCacheManager; | ||||||||||||||||||||||||||||
| import life.mosu.mosuserver.domain.application.repository.ApplicationJpaRepository; | ||||||||||||||||||||||||||||
| import life.mosu.mosuserver.domain.exam.entity.ExamJpaEntity; | ||||||||||||||||||||||||||||
| import life.mosu.mosuserver.domain.exam.entity.ExamJpaRepository; | ||||||||||||||||||||||||||||
| import life.mosu.mosuserver.domain.exam.entity.ExamStatus; | ||||||||||||||||||||||||||||
| import life.mosu.mosuserver.domain.examapplication.repository.ExamApplicationJpaRepository; | ||||||||||||||||||||||||||||
| import life.mosu.mosuserver.global.exception.CustomRuntimeException; | ||||||||||||||||||||||||||||
| import life.mosu.mosuserver.global.exception.ErrorCode; | ||||||||||||||||||||||||||||
| import life.mosu.mosuserver.presentation.application.dto.ApplicationRequest; | ||||||||||||||||||||||||||||
| import life.mosu.mosuserver.presentation.application.dto.ExamApplicationRequest; | ||||||||||||||||||||||||||||
| import lombok.RequiredArgsConstructor; | ||||||||||||||||||||||||||||
| import lombok.extern.slf4j.Slf4j; | ||||||||||||||||||||||||||||
| import org.springframework.stereotype.Component; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| @Slf4j | ||||||||||||||||||||||||||||
| @Component | ||||||||||||||||||||||||||||
| @RequiredArgsConstructor | ||||||||||||||||||||||||||||
| public class ApplicationValidator { | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| private final ExamJpaRepository examJpaRepository; | ||||||||||||||||||||||||||||
| private final ApplicationJpaRepository applicationJpaRepository; | ||||||||||||||||||||||||||||
| private final ExamApplicationJpaRepository examApplicationJpaRepository; | ||||||||||||||||||||||||||||
| private final ExamQuotaCacheManager examQuotaCacheManager; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| public void agreedToTerms(ApplicationRequest request) { | ||||||||||||||||||||||||||||
| if (!request.agreement().validateAgreement()) { | ||||||||||||||||||||||||||||
| throw new CustomRuntimeException(ErrorCode.NOT_AGREED_TO_TERMS); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
Comment on lines
+30
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add null safety check for the agreement field. The method logic is correct, but consider adding a null check for the agreement field to prevent potential NullPointerException. public void agreedToTerms(ApplicationRequest request) {
+ if (request.agreement() == null) {
+ throw new CustomRuntimeException(ErrorCode.NOT_AGREED_TO_TERMS);
+ }
if (!request.agreement().validateAgreement()) {
throw new CustomRuntimeException(ErrorCode.NOT_AGREED_TO_TERMS);
}
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| public void requestNoDuplicateExams(List<Long> examIds) { | ||||||||||||||||||||||||||||
| Set<Long> examIdSet = new HashSet<>(examIds); | ||||||||||||||||||||||||||||
| if (examIds.size() != examIdSet.size()) { | ||||||||||||||||||||||||||||
| throw new CustomRuntimeException(ErrorCode.EXAM_DUPLICATED); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| if (examIdSet.isEmpty()) { | ||||||||||||||||||||||||||||
| throw new CustomRuntimeException(ErrorCode.EXAM_NOT_APPLIED); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| public void examIdsAndLunchSelection(List<ExamApplicationRequest> requests) { | ||||||||||||||||||||||||||||
|
|
@@ -39,12 +51,9 @@ public void examIdsAndLunchSelection(List<ExamApplicationRequest> requests) { | |||||||||||||||||||||||||||
| List<Long> requestedExamIds = requests.stream() | ||||||||||||||||||||||||||||
| .map(ExamApplicationRequest::examId) | ||||||||||||||||||||||||||||
| .toList(); | ||||||||||||||||||||||||||||
| Set<Long> examIdSet = new HashSet<>(requestedExamIds); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| List<ExamJpaEntity> existingExams = examJpaRepository.findAllById(requestedExamIds); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (existingExams.size() != requestedExamIds.size()) { | ||||||||||||||||||||||||||||
| throw new CustomRuntimeException(ErrorCode.EXAM_NOT_FOUND); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| List<ExamJpaEntity> existingExams = examJpaRepository.findAllById(examIdSet); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
polyglot-k marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||
| lunchSelection(requests, existingExams); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
@@ -56,16 +65,18 @@ private void lunchSelection(List<ExamApplicationRequest> requests, | |||||||||||||||||||||||||||
| .map(ExamJpaEntity::getId) | ||||||||||||||||||||||||||||
| .collect(Collectors.toSet()); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| boolean hasInvalidLunchRequest = requests.stream() | ||||||||||||||||||||||||||||
| .anyMatch(req -> examsWithoutLunch.contains(req.examId()) && req.isLunchChecked()); | ||||||||||||||||||||||||||||
| requests.stream() | ||||||||||||||||||||||||||||
| .filter(req -> req.isLunchChecked() && examsWithoutLunch.contains(req.examId())) | ||||||||||||||||||||||||||||
| .findFirst() | ||||||||||||||||||||||||||||
| .ifPresent(req -> { | ||||||||||||||||||||||||||||
| throw new CustomRuntimeException(ErrorCode.LUNCH_SELECTION_INVALID); | ||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (hasInvalidLunchRequest) { | ||||||||||||||||||||||||||||
| throw new CustomRuntimeException(ErrorCode.LUNCH_SELECTION_INVALID); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| public void noDuplicateApplication(Long userId, List<Long> examIds) { | ||||||||||||||||||||||||||||
| boolean alreadyApplied = applicationJpaRepository.existsByUserIdAndExamIds(userId, examIds); | ||||||||||||||||||||||||||||
| boolean alreadyApplied = examApplicationJpaRepository.existsByUserIdAndExamIds(userId, | ||||||||||||||||||||||||||||
| examIds); | ||||||||||||||||||||||||||||
| if (alreadyApplied) { | ||||||||||||||||||||||||||||
| throw new CustomRuntimeException(ErrorCode.APPLICATION_SCHOOL_DUPLICATED); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| package life.mosu.mosuserver.application.caffeine; | ||
|
|
||
| import com.github.benmanes.caffeine.cache.Caffeine; | ||
| import java.util.Arrays; | ||
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
| import life.mosu.mosuserver.domain.caffeine.CacheGroup; | ||
| import life.mosu.mosuserver.domain.caffeine.CacheType; | ||
| import org.springframework.cache.Cache; | ||
| import org.springframework.cache.CacheManager; | ||
| import org.springframework.cache.annotation.EnableCaching; | ||
| import org.springframework.cache.caffeine.CaffeineCache; | ||
| import org.springframework.context.annotation.Bean; | ||
| import org.springframework.context.annotation.Configuration; | ||
| import org.springframework.context.annotation.Primary; | ||
|
|
||
| @EnableCaching | ||
| @Configuration | ||
| public class LocalCacheConfig { | ||
|
|
||
| @Bean | ||
| public LocalCacheManager localCacheManager() { | ||
| List<Cache> caches = Arrays.stream(CacheGroup.values()) | ||
| .filter(g -> g.getCacheType() == CacheType.LOCAL | ||
| || g.getCacheType() == CacheType.COMPOSITE) | ||
| .map(g -> new CaffeineCache( | ||
| g.getCacheName(), | ||
| Caffeine.newBuilder() | ||
| .recordStats() | ||
| .expireAfterWrite(g.getExpiredAfterWrite()) | ||
| .build() | ||
| )).collect(Collectors.toList()); | ||
|
|
||
| return new LocalCacheManager(caches); | ||
| } | ||
|
|
||
| @Bean | ||
| @Primary | ||
| public CacheManager appCacheManager(LocalCacheManager localCacheManager) { | ||
| return localCacheManager; | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.