fix: Projections.constructor 매핑 과정 오류 해결#389
Conversation
WalkthroughThis PR adds a new projection interface for member certification counts and extends the ChallengeCertificationRepository with batch query methods. It refactors WeeklyRankingService to accept explicit date parameters and implements full weekly ranking recomputation using aggregated member point and certification data with a custom ranking algorithm. Minor query projection improvements are included. Changes
Sequence DiagramsequenceDiagram
actor Scheduler as WeeklyRankingScheduler
participant Service as WeeklyRankingService
participant WRRepo as WeeklyRankingRepository
participant MemberRepo as MemberRepository
participant PointRepo as PointTransactionQueryRepository
participant CertRepo as ChallengeCertificationRepository
Scheduler->>Service: updateWeeklyRanks(weekStart, weekEnd)
Service->>WRRepo: deleteByWeekStart(weekStart)
Service->>MemberRepo: findAll()
Service->>PointRepo: fetchEarnedPointsByMember()
Service->>CertRepo: findCertifiedCountByMemberIds(memberIds)
Service->>Service: Build WeeklyRanking entries<br/>(aggregate points, certs, compute image URLs)
Service->>Service: Sort by totalPoint (desc)<br/>and certificationCount (desc)
Service->>Service: Assign ranks<br/>(same rank for ties)
Service->>WRRepo: saveAll(weeklyRankings)
WRRepo-->>Service: Persist complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Unit Test Coverage Report
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/main/java/com/example/green/domain/certification/infra/projections/MemberCertifiedCountProjection.java (1)
3-7: Consider usingLongforgetCertifiedCount()return type.The JPQL
COUNT(c)returnsLong, but the projection declaresint. While Spring Data can perform the conversion, this may cause issues with large counts or null handling. UsingLongwould be more type-safe and consistent with the aggregate function's return type.public interface MemberCertifiedCountProjection { Long getMemberId(); - int getCertifiedCount(); + Long getCertifiedCount(); }src/main/java/com/example/green/domain/dashboard/rankingmodule/service/WeeklyRankingService.java (2)
94-107: Confirm that “competition ranking” (1,2,2,4) is the intended behavior for tiesThe current logic:
- Gives the same rank to members with identical
totalPointandcertificationCount.- Increments
rankCounterevery iteration, so ranks go like1,2,2,4for a2-waytie at second place (competition style), not1,2,2,3(dense style).If product requirements expect dense ranking, the loop needs to increment
rankCounteronly when the rank actually changes.
59-66: Validate projection type forcertifiedCountvs DB/query type
certifiedCountsis declared asMap<Long, Integer>and populated usingMemberCertifiedCountProjection::getCertifiedCount. In JPA/Querydsl,COUNTexpressions are typicallyLong. If the underlying query or DB column isLongbut the projection getter returnsInteger, it can lead to mapping issues or overflow for large counts.Please double-check that:
MemberCertifiedCountProjection#getCertifiedCount()isIntegeronly if your query already casts/truncates to an int-sized value, or- Otherwise, consider using
Longconsistently in the projection, map, and entity field.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/com/example/green/domain/certification/infra/projections/MemberCertifiedCountProjection.java(1 hunks)src/main/java/com/example/green/domain/certification/repository/ChallengeCertificationRepository.java(1 hunks)src/main/java/com/example/green/domain/dashboard/rankingmodule/repository/WeeklyRankingRepository.java(1 hunks)src/main/java/com/example/green/domain/dashboard/rankingmodule/service/WeeklyRankingScheduler.java(2 hunks)src/main/java/com/example/green/domain/dashboard/rankingmodule/service/WeeklyRankingService.java(4 hunks)src/main/java/com/example/green/domain/pointshop/item/infra/PointItemQueryExecutor.java(2 hunks)src/main/java/com/example/green/infra/client/CertificationClient.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: check
🔇 Additional comments (6)
src/main/java/com/example/green/domain/pointshop/item/infra/PointItemQueryExecutor.java (2)
11-11: LGTM! Required import for constructor-based projection.The import is correctly added to support the direct
Projections.constructorapproach used in the updatedfindItemByCursormethod.
54-61: PointItemResponse constructor signature is compatible with the projection.PointItemResponse is a Java record with a canonical constructor accepting (long pointItemId, String pointItemName, String thumbnailUrl, BigDecimal pointPrice, SellingStatus sellingStatus). The projection correctly passes 5 arguments in matching order. No further action needed.
src/main/java/com/example/green/domain/dashboard/rankingmodule/repository/WeeklyRankingRepository.java (1)
12-14: Derived delete query looks correct.The
deleteByWeekStartmethod follows Spring Data JPA naming conventions and will generate the appropriate delete query. Ensure the calling service method is annotated with@Transactionalto avoidInvalidDataAccessApiUsageException.Note: The
findByWeekStartmethod returningList<Tuple>is unusual for a derived query—this typically suggests a custom implementation inWeeklyRankingRepositoryCustom. If this is a derived query, it should returnList<WeeklyRanking>instead.src/main/java/com/example/green/infra/client/CertificationClient.java (1)
11-11: No functional changes.This file only has a whitespace formatting change (added blank line). No concerns.
src/main/java/com/example/green/domain/dashboard/rankingmodule/service/WeeklyRankingScheduler.java (1)
22-29: Date range calculation is correct.The logic properly computes last week's Monday-to-Sunday range. Since the job runs on Monday at 00:00,
minusWeeks(1).with(DayOfWeek.MONDAY)gives the previous Monday, and.with(DayOfWeek.SUNDAY)gives that week's Sunday.One minor consideration: if this method is ever called manually on a non-Monday, the calculation still works correctly due to
minusWeeks(1)anchoring to the previous week.src/main/java/com/example/green/domain/certification/repository/ChallengeCertificationRepository.java (1)
14-15: Derived query method naming is correct.The method
countChallengeCertificationByMemberMemberIdfollows Spring Data JPA naming conventions for navigating themember.memberIdproperty path.
| //여러 회원 인증 수 (한번에 조회) | ||
| @Query("SELECT c.member.memberId AS memberId, COUNT(c) AS certifiedCount " + | ||
| "FROM ChallengeCertification c " + | ||
| "WHERE c.member.memberId IN :memberIds " + | ||
| "GROUP BY c.member.memberId") | ||
| List<MemberCertifiedCountProjection> findCertifiedCountByMemberIds(@Param("memberIds") List<Long> memberIds); |
There was a problem hiding this comment.
Handle empty memberIds list edge case.
If memberIds is empty, the IN :memberIds clause may produce unexpected behavior or SQL errors depending on the database. Consider adding a guard in the service layer or using @Query with conditional logic.
Service-layer guard example:
if (memberIds.isEmpty()) {
return Collections.emptyList();
}
return repository.findCertifiedCountByMemberIds(memberIds);🤖 Prompt for AI Agents
In
src/main/java/com/example/green/domain/certification/repository/ChallengeCertificationRepository.java
around lines 17 to 22, the repository query uses IN :memberIds which can fail or
behave unexpectedly when memberIds is empty; add a guard in the calling service
method to check if the memberIds list is empty and immediately return
Collections.emptyList() (or equivalent) instead of calling
findCertifiedCountByMemberIds, or alternatively ensure the repository call is
only invoked with a non-empty list to avoid SQL errors.
| List<WeeklyRanking> weeklyRankings = allMembers.stream() | ||
| .map(member -> { | ||
| BigDecimal point = memberPoints.getOrDefault(member.getId(), BigDecimal.ZERO); | ||
| int certificationCount = certifiedCounts.getOrDefault(member.getId(), 0); | ||
| String profileImageUrl = (member.getProfile() != null) ? | ||
| member.getProfile().getProfileImageUrl() : null; | ||
|
|
||
| return WeeklyRanking.builder() | ||
| .memberId(member.getId()) | ||
| .memberName(member.getName()) | ||
| .profileImageUrl(profileImageUrl) | ||
| .totalPoint(point) // DB 저장/정렬용 | ||
| .certificationCount(certificationCount) | ||
| .weekStart(weekStart) | ||
| .weekEnd(weekEnd) | ||
| .rank(0) | ||
| .build(); | ||
| }) | ||
| .toList(); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "pom.xml" -o -name "build.gradle" | head -5Repository: GreenWiNit/backend
Length of output: 44
🏁 Script executed:
cat -n /tmp/src/main/java/com/example/green/domain/dashboard/rankingmodule/service/WeeklyRankingService.java | head -120Repository: GreenWiNit/backend
Length of output: 195
🏁 Script executed:
git ls-files | grep -i "weeklyrankingservice"Repository: GreenWiNit/backend
Length of output: 253
🏁 Script executed:
git ls-files | grep -E "(pom\.xml|build\.gradle)" | head -5Repository: GreenWiNit/backend
Length of output: 74
🏁 Script executed:
cat build.gradle | grep -A 5 -B 5 "java\|source\|target\|version" | head -30Repository: GreenWiNit/backend
Length of output: 895
🏁 Script executed:
cat -n src/main/java/com/example/green/domain/dashboard/rankingmodule/service/WeeklyRankingService.java | sed -n '50,110p'Repository: GreenWiNit/backend
Length of output: 2574
🌐 Web query:
Java 16 stream().toList() unmodifiable list documentation
💡 Result:
Yes — Java 16 added Stream.toList(), and its Javadoc states the returned List is unmodifiable: "The returned List is unmodifiable; calls to any mutator method will always cause UnsupportedOperationException to be thrown." [1]. Note the difference from Collectors.toList() (no guarantees about mutability) and Collectors.toUnmodifiableList() (also unmodifiable but disallows nulls). [2]
Sources:
- Stream.toList() Javadoc (Stream API). [1]
- Collectors (toList / toUnmodifiableList) Javadoc. [2]
stream().toList() returns an unmodifiable list; calling sort() will throw UnsupportedOperationException at runtime
On Java 16+, stream().toList() returns an unmodifiable list. Line 89 calls weeklyRankings.sort(...) on this unmodifiable list, which will fail at runtime with UnsupportedOperationException.
Replace with a mutable list:
- List<WeeklyRanking> weeklyRankings = allMembers.stream()
+ List<WeeklyRanking> weeklyRankings = allMembers.stream()
.map(member -> {
BigDecimal point = memberPoints.getOrDefault(member.getId(), BigDecimal.ZERO);
int certificationCount = certifiedCounts.getOrDefault(member.getId(), 0);
String profileImageUrl = (member.getProfile() != null) ?
member.getProfile().getProfileImageUrl() : null;
return WeeklyRanking.builder()
.memberId(member.getId())
.memberName(member.getName())
.profileImageUrl(profileImageUrl)
.totalPoint(point) // DB 저장/정렬용
.certificationCount(certificationCount)
.weekStart(weekStart)
.weekEnd(weekEnd)
.rank(0)
.build();
- })
- .toList();
+ })
+ .collect(Collectors.toCollection(ArrayList::new));🤖 Prompt for AI Agents
In
src/main/java/com/example/green/domain/dashboard/rankingmodule/service/WeeklyRankingService.java
around lines 68-87, the code uses stream().toList() which returns an
unmodifiable list on Java 16+, causing weeklyRankings.sort(...) later to throw
UnsupportedOperationException; change the collection to a mutable list (for
example, collect into an ArrayList via
.collect(Collectors.toCollection(ArrayList::new)) or wrap the result with new
ArrayList<>(...)) so weeklyRankings can be sorted in place.
#️⃣ 연관된 이슈
📝 작업 내용
📷 스크린샷
💬 리뷰 요구사항(선택)
📌 PR 진행 시 참고사항
P1: 꼭 반영해 주세요 (Request Changes) – 이슈나 취약점 관련P2: 반영을 고려해 주세요 (Comment) – 개선 의견P3: 단순 제안 (Chore)Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.