Skip to content

fix: Projections.constructor 매핑 과정 오류 해결#389

Merged
sanchaehwa merged 2 commits intomainfrom
fix/rankmodule
Nov 30, 2025
Merged

fix: Projections.constructor 매핑 과정 오류 해결#389
sanchaehwa merged 2 commits intomainfrom
fix/rankmodule

Conversation

@sanchaehwa
Copy link
Member

@sanchaehwa sanchaehwa commented Nov 30, 2025

#️⃣ 연관된 이슈

ex) close #Issue number

📝 작업 내용

이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)

📷 스크린샷

이미지

💬 리뷰 요구사항(선택)

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

ex) 메서드 명칭

📌 PR 진행 시 참고사항

  • 리뷰어는 좋은 코드 방향을 제시하되, 수정을 강요하지 않습니다.
  • 좋은 코드를 발견하면 칭찬과 격려를 아끼지 않습니다.
  • 리뷰는 Reviewer로 지정된 시점 기준으로 3일 이내에 진행해 주세요.
  • Comment 작성 시 아래 Prefix를 사용해 주세요:
    • P1: 꼭 반영해 주세요 (Request Changes) – 이슈나 취약점 관련
    • P2: 반영을 고려해 주세요 (Comment) – 개선 의견
    • P3: 단순 제안 (Chore)

Summary by CodeRabbit

  • New Features

    • Weekly rankings now incorporate certification counts alongside points for more comprehensive member rankings
  • Refactor

    • Enhanced ranking calculation algorithm to include certification metrics and improved data aggregation for weekly rankings

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 30, 2025

Walkthrough

This 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

Cohort / File(s) Summary
Certification Data Access
src/main/java/com/example/green/domain/certification/infra/projections/MemberCertifiedCountProjection.java, src/main/java/com/example/green/domain/certification/repository/ChallengeCertificationRepository.java
New projection interface with getMemberId() and getCertifiedCount() accessors. New repository adds countChallengeCertificationByMemberMemberId(Long memberId) and findCertifiedCountByMemberIds(List<Long> memberIds) with @Query aggregation.
Weekly Ranking Refactor
src/main/java/com/example/green/domain/dashboard/rankingmodule/repository/WeeklyRankingRepository.java, src/main/java/com/example/green/domain/dashboard/rankingmodule/service/WeeklyRankingScheduler.java, src/main/java/com/example/green/domain/dashboard/rankingmodule/service/WeeklyRankingService.java
Repository adds deleteByWeekStart(LocalDate weekStart). Scheduler computes last week's date range (Monday–Sunday) in Asia/Seoul timezone and passes it to service. Service signature updated to accept explicit weekStart and weekEnd parameters; now performs full weekly ranking recomputation with deletion, member enumeration, point/certification aggregation, ranking with tie handling, and persistence.
Query Projection Update
src/main/java/com/example/green/domain/pointshop/item/infra/PointItemQueryExecutor.java
Replaces previous projection with Projections.constructor-based approach for PointItemResponse and adds cursor limit.
Formatting
src/main/java/com/example/green/infra/client/CertificationClient.java
Whitespace-only change (blank line added before closing brace).

Sequence Diagram

sequenceDiagram
    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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • WeeklyRankingService.updateWeeklyRanks: Contains complex aggregation logic, ranking algorithm with tie handling, and multiple repository interactions; requires careful verification of data flow and correctness of point/certification totals.
  • ChallengeCertificationRepository.findCertifiedCountByMemberIds: Custom @Query with JPQL aggregation; verify query correctness and projection mapping.
  • Heterogeneous file changes: Multiple distinct concerns across 7 files (repository additions, service refactoring, query projection changes) demand context-switching during review.

Possibly related PRs

Suggested labels

dev:fix

Poem

🐰 Hop, hop! Rankings reborn anew,
Weekly batches sorted true,
Certifications counted with care,
Members ranked beyond compare! 🏆

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title references fixing 'Projections.constructor mapping errors', which directly relates to the main change in PointItemQueryExecutor.java where Projections.constructor-based projection was modified. This reflects a real and significant part of the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/rankmodule

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

Unit Test Coverage Report

Total Project Coverage 34.51%
File Coverage [31.76%]
WeeklyRankingService.java 34.31%
WeeklyRankingScheduler.java 0%

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (3)
src/main/java/com/example/green/domain/certification/infra/projections/MemberCertifiedCountProjection.java (1)

3-7: Consider using Long for getCertifiedCount() return type.

The JPQL COUNT(c) returns Long, but the projection declares int. While Spring Data can perform the conversion, this may cause issues with large counts or null handling. Using Long would 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 ties

The current logic:

  • Gives the same rank to members with identical totalPoint and certificationCount.
  • Increments rankCounter every iteration, so ranks go like 1,2,2,4 for a 2-way tie at second place (competition style), not 1,2,2,3 (dense style).

If product requirements expect dense ranking, the loop needs to increment rankCounter only when the rank actually changes.


59-66: Validate projection type for certifiedCount vs DB/query type

certifiedCounts is declared as Map<Long, Integer> and populated using MemberCertifiedCountProjection::getCertifiedCount. In JPA/Querydsl, COUNT expressions are typically Long. If the underlying query or DB column is Long but the projection getter returns Integer, it can lead to mapping issues or overflow for large counts.

Please double-check that:

  • MemberCertifiedCountProjection#getCertifiedCount() is Integer only if your query already casts/truncates to an int-sized value, or
  • Otherwise, consider using Long consistently in the projection, map, and entity field.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ee29161 and e6282d7.

📒 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.constructor approach used in the updated findItemByCursor method.


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 deleteByWeekStart method follows Spring Data JPA naming conventions and will generate the appropriate delete query. Ensure the calling service method is annotated with @Transactional to avoid InvalidDataAccessApiUsageException.

Note: The findByWeekStart method returning List<Tuple> is unusual for a derived query—this typically suggests a custom implementation in WeeklyRankingRepositoryCustom. If this is a derived query, it should return List<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 countChallengeCertificationByMemberMemberId follows Spring Data JPA naming conventions for navigating the member.memberId property path.

Comment on lines +17 to +22
//여러 회원 인증 수 (한번에 조회)
@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);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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.

Comment on lines +68 to 87
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();

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cd /tmp && find . -name "pom.xml" -o -name "build.gradle" | head -5

Repository: GreenWiNit/backend

Length of output: 44


🏁 Script executed:

cat -n /tmp/src/main/java/com/example/green/domain/dashboard/rankingmodule/service/WeeklyRankingService.java | head -120

Repository: 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 -5

Repository: GreenWiNit/backend

Length of output: 74


🏁 Script executed:

cat build.gradle | grep -A 5 -B 5 "java\|source\|target\|version" | head -30

Repository: 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.

@sanchaehwa sanchaehwa merged commit 9816715 into main Nov 30, 2025
2 checks passed
@sanchaehwa sanchaehwa deleted the fix/rankmodule branch November 30, 2025 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant