Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package org.sopt.app.application.rank;

import java.util.EnumMap;
import java.util.Map;
import lombok.RequiredArgsConstructor;
import org.sopt.app.domain.enums.SoptPart;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.stereotype.Component;

@Component
@RequiredArgsConstructor
public class AuthPartMemberCountReader {

private final JdbcTemplate jdbcTemplate;

public Map<SoptPart, Long> getCurrentGenerationPartMemberCounts(Long generation) {
String sql = """
SELECT part, COUNT(*) AS member_count
FROM auth_prod.user_activity_histories
WHERE generation = ?
AND is_sopt = true
AND role = 'MEMBER'
AND part IN ('IOS', 'ANDROID', 'DESIGN', 'PLAN', 'SERVER', 'WEB')
GROUP BY part
""";

return jdbcTemplate.query(sql, rs -> {
Map<SoptPart, Long> result = new EnumMap<>(SoptPart.class);

while (rs.next()) {
result.put(
SoptPart.valueOf(rs.getString("part")),
rs.getLong("member_count")
);
}

return result;
}, generation);
}
Comment on lines +20 to +43
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@table(schema = "auth_prod", name = "user_activity_histories") 로 엔티티 자체를 만들어서 JPA로 사용하는 방식보다 이렇게 JdbcTemplate을 사용하는 방식이 더 빠르게 작업할 수 있으셔서 이렇게 하신건가요??

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

넵 맞습니다! 이번 조회가 auth_prod.user_activity_histories에 대한 단순 집계성 쿼리이고, 최종적으로 필요한 값도 엔티티 자체가 아니라 part별 count 값이라서JPA 엔티티/리포지토리를 추가하는 것보다 JdbcTemplate이 더 적절하다고 판단했습니다.

향후에 해당 값이나 auth의 데이터를 더 자주 활용하게 된다면 그때에 확장을 고려해보는 게 어떨까 해서 이렇게 작성해보았습니다.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

지금 다시 보면서 생각났는데, auth_prod를 하드 코딩할 경우 dev 환경에서 문제가 발생할 수 있겠네요,,,

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

이 부분 반영해서 다시 올려두었습니다! 내부 야물 파일 설정 부분들 모두 수정 완료하였고, 이 내용들 정리해서 알려드릴게요 🫡

}
Original file line number Diff line number Diff line change
@@ -1,32 +1,90 @@
package org.sopt.app.application.rank;

import static java.util.Map.Entry.comparingByValue;

import java.math.BigDecimal;
import java.math.RoundingMode;
import java.util.Comparator;
import java.util.EnumMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import lombok.AccessLevel;
import lombok.RequiredArgsConstructor;
import org.sopt.app.application.soptamp.SoptampPointInfo.PartRank;
import org.sopt.app.application.soptamp.SoptampUserInfo;
import org.sopt.app.domain.enums.Part;

import org.sopt.app.domain.enums.SoptPart;

@RequiredArgsConstructor(access = AccessLevel.PUBLIC)
public class SoptampPartRankCalculator {

private final List<SoptampUserInfo> userInfos;
private static final BigDecimal ZERO_POINT = BigDecimal.ZERO.setScale(2, RoundingMode.HALF_UP);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

소수점 몇째 자리까지 사용하는 지를 별도 상수로 빼서 관리해도 좋을 것 같아요! 하단의

            BigDecimal averagePoint = memberCount == 0 ? ZERO_POINT
                : BigDecimal.valueOf(totalScore)
                    .divide(BigDecimal.valueOf(memberCount), 2, RoundingMode.HALF_UP);

같은 부분도 상수로 컨트롤하면 정책 변경에 유연하게 대처하기 좋을 것 같습니다~

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

오호 이부분 반영해두었습니다! 좋은 의견 너무 감사합니다 🙇‍♀️ 518bdbe


private final PartScores partScores = new PartScores();
private final List<SoptampUserInfo> userInfos;
private final Map<SoptPart, Long> partMemberCounts;

public List<PartRank> calculatePartRank() {
userInfos.forEach(this::calculatePartScore);
return Part.getPartsByReturnOrder().stream().map(part -> PartRank.builder()
PartScores partScores = new PartScores();
userInfos.forEach(userInfo -> addPartScore(userInfo, partScores));

Map<Part, BigDecimal> averagePoints = calculateAveragePoints(partScores);
Map<Part, Integer> ranks = calculateRanks(averagePoints);

return Part.getPartsByReturnOrder().stream()
.map(part -> PartRank.builder()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

userInfos.forEach(userInfo -> addPartScore(userInfo, partScores)); 에서 foreach로 상태를 변경하는 것이 좀 애매하다면 이렇게 할 수도 있겠네요

    userInfos.stream()
    .filter(userInfo -> SoptPart.toPart(userInfo.getPart()) != null) 
    .collect(Collectors.groupingBy(
        userInfo -> SoptPart.toPart(userInfo.getPart()),
        Collectors.summingInt(SoptampUserInfo::getTotalPoints) 
    ));

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

오 좋은 의견 감사합니다 👀 말씀해주신 방식처럼 groupingBy로 바로 파트별 총점을 만드는 것도 가능할 것 같다고 생각했습니다.

다만 이번에는 기존 PartScores가 총점 누적 역할을 하고 있어서, 그 부분은 유지하고 평균 점수 계산과 rank 계산만 변경하는 쪽은 어떨까 싶었습니다. 또 groupingBy로 바꾸면 SoptPart -> Part 변환과 null 필터링이 collector 안에 함께 들어가게 되어서, 현재처럼 총점 누적 / 평균 점수 계산 / rank 계산 단계를 나누는 구조도 어떨까 하여 이렇게 구성해보았는데, 혹시 재헌님 생각은 어떠실까요?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

요게 가독성이 좀 떨어지시면 이 그룹핑 로직 자체를 메서드로 묶어서 처리할 수도 있을 것 같긴해요!
다만 이건 민경님이 보시기에 더 잘 읽힐 것 같은 방향으로 진행하셔도 될 것 같습니다!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

그룹핑 로직 관련해서 수정해보았습니다! 🫡 fefd033

.part(part.getPartName())
.rank(partScores.getRank(part))
.points(partScores.getPoints(part))
.build()).toList();
.rank(ranks.get(part))
.points(averagePoints.get(part))
.build())
.toList();
}

private void addPartScore(SoptampUserInfo userInfo, PartScores partScores) {
Part part = SoptPart.toPart(userInfo.getPart());
if (part == null) {
return;
}
partScores.addPartScore(part, userInfo.getTotalPoints());
}

private void calculatePartScore(SoptampUserInfo userInfo) {
Part.getAllParts().stream()
.filter(part -> userInfo.getNickname().startsWith(part.getPartName()))
.forEach(part -> partScores.addPartScore(part, userInfo.getTotalPoints()));
private Map<Part, BigDecimal> calculateAveragePoints(PartScores partScores) {
Map<Part, BigDecimal> averagePoints = new EnumMap<>(Part.class);

for (Part part : Part.getPartsByReturnOrder()) {
long totalScore = partScores.getPoints(part);
long memberCount = partMemberCounts.getOrDefault(SoptPart.valueOf(part.name()), 0L);

BigDecimal averagePoint = memberCount == 0 ? ZERO_POINT
: BigDecimal.valueOf(totalScore)
.divide(BigDecimal.valueOf(memberCount), 2, RoundingMode.HALF_UP);

averagePoints.put(part, averagePoint);
}

return averagePoints;
}

private Map<Part, Integer> calculateRanks(Map<Part, BigDecimal> averagePoints) {
List<Entry<Part, BigDecimal>> sortedParts = averagePoints.entrySet().stream()
.sorted(comparingByValue(Comparator.reverseOrder()))
.toList();

Map<Part, Integer> ranks = new EnumMap<>(Part.class);
BigDecimal previousPoint = null;
int currentRank = 0;

for (int i = 0; i < sortedParts.size(); i++) {
Entry<Part, BigDecimal> entry = sortedParts.get(i);

if (previousPoint == null || entry.getValue().compareTo(previousPoint) != 0) {
currentRank = i + 1;
previousPoint = entry.getValue();
}

ranks.put(entry.getKey(), currentRank);
}

return ranks;
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.sopt.app.application.soptamp;

import java.math.BigDecimal;
import lombok.*;

@NoArgsConstructor(access = AccessLevel.PRIVATE)
Expand Down Expand Up @@ -31,6 +32,6 @@ public static Main of(Integer rank, SoptampUserInfo soptampUserInfo) {
public static class PartRank {
private String part;
private Integer rank;
private Long points;
private BigDecimal points;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import java.util.List;
import java.util.Map;
import lombok.Getter;
import lombok.RequiredArgsConstructor;
import org.sopt.app.common.exception.BadRequestException;
import org.sopt.app.common.response.ErrorCode;
Expand All @@ -20,6 +21,7 @@ public class SoptampUserFinder {

private final SoptampUserRepository soptampUserRepository;

@Getter
@Value("${sopt.current.generation}")
private Long currentGeneration;

Expand Down
13 changes: 9 additions & 4 deletions src/main/java/org/sopt/app/facade/RankFacade.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.sopt.app.common.exception.BadRequestException;
import org.sopt.app.common.response.ErrorCode;
import org.sopt.app.domain.enums.Part;
import org.sopt.app.domain.enums.SoptPart;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.data.redis.core.ZSetOperations.TypedTuple;
import org.springframework.stereotype.Service;
Expand All @@ -20,6 +21,7 @@ public class RankFacade {

private final SoptampUserFinder soptampUserFinder;
private final RankCacheService rankCacheService;
private final AuthPartMemberCountReader authPartMemberCountReader;

@Value("${makers.app.soptamp.appjam-mode:false}")
private boolean appjamMode;
Expand Down Expand Up @@ -102,7 +104,8 @@ public List<PartRank> findAllPartRanks() {
throw new BadRequestException(ErrorCode.INVALID_APPJAM_SEASON_REQUEST);
}
List<SoptampUserInfo> soptampUserInfos = soptampUserFinder.findAllOfCurrentGeneration();
SoptampPartRankCalculator soptampPartRankCalculator = new SoptampPartRankCalculator(soptampUserInfos);
Map<SoptPart, Long> partMemberCounts = authPartMemberCountReader.getCurrentGenerationPartMemberCounts(soptampUserFinder.getCurrentGeneration());
SoptampPartRankCalculator soptampPartRankCalculator = new SoptampPartRankCalculator(soptampUserInfos, partMemberCounts);
return soptampPartRankCalculator.calculatePartRank();
}

Expand All @@ -112,10 +115,12 @@ public PartRank findPartRank(Part part) {
throw new BadRequestException(ErrorCode.INVALID_APPJAM_SEASON_REQUEST);
}
List<SoptampUserInfo> soptampUserInfos = soptampUserFinder.findAllOfCurrentGeneration();
SoptampPartRankCalculator soptampPartRankCalculator = new SoptampPartRankCalculator(soptampUserInfos);
Map<SoptPart, Long> partMemberCounts = authPartMemberCountReader.getCurrentGenerationPartMemberCounts(soptampUserFinder.getCurrentGeneration());
SoptampPartRankCalculator soptampPartRankCalculator = new SoptampPartRankCalculator(soptampUserInfos, partMemberCounts);
return soptampPartRankCalculator.calculatePartRank().stream()
.filter(partRank -> partRank.getPart().equals(part.getPartName()))
.findFirst().orElseThrow();
.filter(partRank -> partRank.getPart().equals(part.getPartName()))
.findFirst()
.orElseThrow();
}

@Transactional(readOnly = true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,4 +154,12 @@ public static PlatformUserInfoResponse getPlatformUserInfoResponse(
);
}

public static final Map<SoptPart, Long> PART_MEMBER_COUNT_MAP = Map.of(
SoptPart.PLAN, 10L,
SoptPart.DESIGN, 10L,
SoptPart.WEB, 10L,
SoptPart.IOS, 10L,
SoptPart.ANDROID, 10L,
SoptPart.SERVER, 30L
);
}
Loading
Loading