Skip to content

Commit

Permalink
fix: ✏️ 총 지출 총합 값의 정수 범위 초과 케이스를 고려하여 타입 수정 (#133)
Browse files Browse the repository at this point in the history
* fix: target_amount_dto 원시 타입으로 수정

* fix: spending_search_res daily_total_amount int -> long

* fix: daily_total_amount total_spending 계산 시, 반환 타입 long으로 수정

* fix: recent_target_amount_res 필드 원시 타입으로 수정

* style: spending_service 총 지출 금액 조회 메서드 인접하게 위치 수정

* fix: 총 지출 금액 dto amount 필드 int -> long

* style: target_amount_usecase 함수 호출 스니펫과 return 문 사이 공백 추가

* fix: 가장 최근 목표 금액의 amount가 0인 경우가 존재할 수 있으므로, 타입을 래퍼 타입으로 수정

* feat: total_spending_amount year_month 반환 메서드 추가

* fix: int 타입 long으로 변환 및 코드 정리

* fix: target amount update int -> long 타입 수정

* fix: querydsl total_spending_amount 기본 타입 expression 정의

* test: target amount controller unit test get_id() null 에러 핸들링

* test: max integer 범위 초과한 경우 diff_amount, total_spending 출력 확인

* fix: to_year_month_map 동시성 환경을 고려하여, concurrent_map 타입으로 반환
  • Loading branch information
psychology50 authored Jul 16, 2024
1 parent 3bc5ea6 commit 127cf5c
Show file tree
Hide file tree
Showing 13 changed files with 101 additions and 87 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/open-api-code-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ permissions:

on:
pull_request:
types: [ opened, synchronize ]
types: [ opened ]

jobs:
test:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public record Daily(
@Schema(description = "일")
int day,
@Schema(description = "일별 총 지출 금액")
int dailyTotalAmount,
long dailyTotalAmount,
@Schema(description = "개별 지출 내역")
List<Individual> individuals
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@ public class TargetAmountDto {
@Schema(title = "목표 금액의 amount 유효성 검사를 위한 요청 파라미터", hidden = true)
public record AmountParam(
@Schema(description = "등록하려는 목표 금액 (0이상의 정수)", example = "100000", requiredMode = Schema.RequiredMode.REQUIRED)
@NotNull(message = "amount 값은 필수입니다.")
@Min(value = 0, message = "amount 값은 0 이상이어야 합니다.")
Integer amount
int amount
) {

}
Expand All @@ -40,44 +39,28 @@ public record DateParam(
@Schema(title = "목표 금액 및 총 지출 금액 조회 응답")
public record WithTotalSpendingRes(
@Schema(description = "조회 년도", example = "2024", requiredMode = Schema.RequiredMode.REQUIRED)
@NotNull(message = "year 값은 필수입니다.")
Integer year,
int year,
@Schema(description = "조회 월", example = "5", requiredMode = Schema.RequiredMode.REQUIRED)
@NotNull(message = "month 값은 필수입니다.")
Integer month,
int month,
@Schema(description = "목표 금액", requiredMode = Schema.RequiredMode.REQUIRED)
@NotNull(message = "targetAmountDetail 값은 필수입니다.")
TargetAmountInfo targetAmountDetail,
@Schema(description = "총 지출 금액", example = "100000", requiredMode = Schema.RequiredMode.REQUIRED)
@NotNull(message = "totalSpending 값은 필수입니다.")
Integer totalSpending,
long totalSpending,
@Schema(description = "목표 금액과 총 지출 금액의 차액(총 치줄 금액 - 목표 금액). 양수면 초과, 음수면 절약", example = "-50000", requiredMode = Schema.RequiredMode.REQUIRED)
@NotNull(message = "diffAmount 값은 필수입니다.")
Integer diffAmount
long diffAmount
) {
}

@Schema(title = "목표 금액 상세 정보")
public record TargetAmountInfo(
@Schema(description = "목표 금액 pk. 실제 저장된 데이터가 아니라면 -1", example = "1", requiredMode = Schema.RequiredMode.REQUIRED)
@NotNull(message = "id 값은 필수입니다.")
Long id,
long id,
@Schema(description = "목표 금액. -1이면 설정한 목표 금액이 존재하지 않음을 의미한다.", example = "50000", requiredMode = Schema.RequiredMode.REQUIRED)
@NotNull(message = "amount 값은 필수입니다.")
Integer amount,
int amount,
@Schema(description = "사용자 확인 여부", example = "true", requiredMode = Schema.RequiredMode.REQUIRED)
boolean isRead
) {
public TargetAmountInfo {
if (id == null) {
id = -1L;
}

if (amount == null) {
amount = -1;
}
}

/**
* {@link TargetAmount} -> {@link TargetAmountInfo} 변환하는 메서드 <br/>
* 만약, 인자로 들어온 값이 null이라면 모든 값을 -1로 초기화한 더미 데이터를 반환한다.
Expand All @@ -95,29 +78,29 @@ public record RecentTargetAmountRes(
@Schema(description = "최근 목표 금액 존재 여부로써 데이터가 존재하지 않으면 false, 존재하면 true", example = "true", requiredMode = Schema.RequiredMode.REQUIRED)
boolean isPresent,
@Schema(description = "최근 목표 금액 년도 정보. isPresent가 false인 경우 필드가 존재하지 않는다.", example = "2024", requiredMode = Schema.RequiredMode.REQUIRED)
@JsonInclude(JsonInclude.Include.NON_NULL)
Integer year,
@JsonInclude(JsonInclude.Include.NON_DEFAULT)
int year,
@Schema(description = "최근 목표 금액 월 정보. isPresent가 false인 경우 필드가 존재하지 않는다.", example = "6", requiredMode = Schema.RequiredMode.REQUIRED)
@JsonInclude(JsonInclude.Include.NON_NULL)
Integer month,
@JsonInclude(JsonInclude.Include.NON_DEFAULT)
int month,
@Schema(description = "최근 목표 금액 정보. isPresent가 false인 경우 필드가 존재하지 않는다.", requiredMode = Schema.RequiredMode.REQUIRED)
@JsonInclude(JsonInclude.Include.NON_NULL)
Integer amount
) {
public RecentTargetAmountRes {
if (!isPresent) {
assert year == null;
assert month == null;
assert year == 0;
assert month == 0;
assert amount == null;
}
}

public static RecentTargetAmountRes notPresent() {
return new RecentTargetAmountRes(false, null, null, null);
return new RecentTargetAmountRes(false, 0, 0, null);
}

public static RecentTargetAmountRes of(Integer year, Integer month, Integer amount) {
return (amount.equals(-1)) ? new RecentTargetAmountRes(false, null, null, null) : new RecentTargetAmountRes(true, year, month, amount);
public static RecentTargetAmountRes of(int year, int month, int amount) {
return (amount == -1) ? new RecentTargetAmountRes(false, 0, 0, null) : new RecentTargetAmountRes(true, year, month, amount);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public static SpendingSearchRes.Individual toSpendingSearchResIndividual(Spendin
/**
* 하루 지출 내역의 총 금액을 계산하는 메서드
*/
private static int calculateDailyTotalAmount(List<Spending> spendings) {
return spendings.stream().mapToInt(Spending::getAmount).sum();
private static long calculateDailyTotalAmount(List<Spending> spendings) {
return spendings.stream().mapToLong(Spending::getAmount).sum();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public class TargetAmountMapper {
* @param totalSpending {@link TotalSpendingAmount} : 값이 없을 경우 null
*/
public static TargetAmountDto.WithTotalSpendingRes toWithTotalSpendingResponse(TargetAmount targetAmount, TotalSpendingAmount totalSpending, LocalDate date) {
Integer totalSpendingAmount = (totalSpending != null) ? totalSpending.totalSpending() : 0;
long totalSpendingAmount = (totalSpending != null) ? totalSpending.totalSpending() : 0L;

return createWithTotalSpendingRes(targetAmount, totalSpendingAmount, date);
}
Expand All @@ -38,8 +38,8 @@ public static List<TargetAmountDto.WithTotalSpendingRes> toWithTotalSpendingResp
LocalDate startAt = getOldestDate(targetAmounts);
int monthLength = (endAt.getYear() - startAt.getYear()) * 12 + (endAt.getMonthValue() - startAt.getMonthValue());

Map<YearMonth, TargetAmount> targetAmountsByDates = toYearMonthMap(targetAmounts, targetAmount -> YearMonth.of(targetAmount.getCreatedAt().getYear(), targetAmount.getCreatedAt().getMonthValue()), Function.identity());
Map<YearMonth, Integer> totalSpendingAmounts = toYearMonthMap(totalSpendings, totalSpendingAmount -> YearMonth.of(totalSpendingAmount.year(), totalSpendingAmount.month()), TotalSpendingAmount::totalSpending);
Map<YearMonth, TargetAmount> targetAmountsByDates = toYearMonthMap(targetAmounts, ta -> YearMonth.from(ta.getCreatedAt()), Function.identity());
Map<YearMonth, Long> totalSpendingAmounts = toYearMonthMap(totalSpendings, TotalSpendingAmount::getYearMonth, TotalSpendingAmount::totalSpending);

return createWithTotalSpendingResponses(targetAmountsByDates, totalSpendingAmounts, startAt, monthLength).stream()
.sorted(Comparator.comparing(TargetAmountDto.WithTotalSpendingRes::year).reversed()
Expand All @@ -53,42 +53,39 @@ public static List<TargetAmountDto.WithTotalSpendingRes> toWithTotalSpendingResp
* @return {@link TargetAmountDto.RecentTargetAmountRes}
*/
public static TargetAmountDto.RecentTargetAmountRes toRecentTargetAmountResponse(Optional<TargetAmount> targetAmount) {
if (targetAmount.isEmpty()) {
return TargetAmountDto.RecentTargetAmountRes.notPresent();
}

Integer year = targetAmount.get().getCreatedAt().getYear();
Integer month = targetAmount.get().getCreatedAt().getMonthValue();
Integer amount = targetAmount.get().getAmount();

return TargetAmountDto.RecentTargetAmountRes.of(year, month, amount);
return targetAmount.map(ta -> {
LocalDate createdAt = ta.getCreatedAt().toLocalDate();
return TargetAmountDto.RecentTargetAmountRes.of(createdAt.getYear(), createdAt.getMonthValue(), ta.getAmount());
}).orElseGet(TargetAmountDto.RecentTargetAmountRes::notPresent);
}

private static List<TargetAmountDto.WithTotalSpendingRes> createWithTotalSpendingResponses(Map<YearMonth, TargetAmount> targetAmounts, Map<YearMonth, Integer> totalSpendings, LocalDate startAt, int monthLength) {
private static List<TargetAmountDto.WithTotalSpendingRes> createWithTotalSpendingResponses(Map<YearMonth, TargetAmount> targetAmounts, Map<YearMonth, Long> totalSpendings, LocalDate startAt, int monthLength) {
List<TargetAmountDto.WithTotalSpendingRes> withTotalSpendingResponses = new ArrayList<>(monthLength + 1);
LocalDate date = startAt;

for (int i = 0; i < monthLength + 1; i++) {
LocalDate date = startAt.plusMonths(i);
YearMonth yearMonth = YearMonth.of(date.getYear(), date.getMonthValue());
YearMonth yearMonth = YearMonth.from(date);

TargetAmount targetAmount = targetAmounts.getOrDefault(yearMonth, null);
Integer totalSpending = totalSpendings.getOrDefault(yearMonth, 0);
TargetAmount targetAmount = targetAmounts.get(yearMonth);
Long totalSpending = totalSpendings.getOrDefault(yearMonth, 0L);

withTotalSpendingResponses.add(createWithTotalSpendingRes(targetAmount, totalSpending, date));
date = date.plusMonths(1);
}

return withTotalSpendingResponses;
}

private static TargetAmountDto.WithTotalSpendingRes createWithTotalSpendingRes(TargetAmount targetAmount, Integer totalSpending, LocalDate date) {
private static TargetAmountDto.WithTotalSpendingRes createWithTotalSpendingRes(TargetAmount targetAmount, Long totalSpending, LocalDate date) {
TargetAmountDto.TargetAmountInfo targetAmountInfo = TargetAmountDto.TargetAmountInfo.from(targetAmount);
long diffAmount = (targetAmountInfo.amount() == -1) ? 0 : totalSpending - (long) targetAmountInfo.amount();

return TargetAmountDto.WithTotalSpendingRes.builder()
.year(date.getYear())
.month(date.getMonthValue())
.targetAmountDetail(targetAmountInfo)
.totalSpending(totalSpending)
.diffAmount((targetAmountInfo.amount() == -1) ? 0 : totalSpending - targetAmountInfo.amount())
.diffAmount(diffAmount)
.build();
}

Expand All @@ -112,12 +109,6 @@ private static LocalDate getOldestDate(List<TargetAmount> targetAmounts) {
* @param valueMapper : Value로 변환할 Function
*/
private static <T, U> Map<YearMonth, U> toYearMonthMap(List<T> list, Function<T, YearMonth> keyMapper, Function<T, U> valueMapper) {
return list.stream().collect(
Collectors.toMap(
keyMapper,
valueMapper,
(existing, replacement) -> existing
)
);
return list.stream().collect(Collectors.toConcurrentMap(keyMapper, valueMapper, (existing, replacement) -> existing));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public TargetAmount createTargetAmount(String key, Long userId, LocalDate date)
}

@Transactional
public TargetAmount updateTargetAmount(Long targetAmountId, Integer amount) {
public TargetAmount updateTargetAmount(Long targetAmountId, int amount) {
TargetAmount targetAmount = targetAmountService.readTargetAmount(targetAmountId)
.orElseThrow(() -> new TargetAmountErrorException(TargetAmountErrorCode.NOT_FOUND_TARGET_AMOUNT));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public TargetAmountDto.TargetAmountInfo createTargetAmount(Long userId, int year
public TargetAmountDto.WithTotalSpendingRes getTargetAmountAndTotalSpending(Long userId, LocalDate date) {
TargetAmount targetAmount = targetAmountSearchService.readTargetAmountThatMonth(userId, date);
Optional<TotalSpendingAmount> totalSpending = spendingSearchService.readTotalSpendingAmountByUserIdThatMonth(userId, date);

return TargetAmountMapper.toWithTotalSpendingResponse(targetAmount, totalSpending.orElse(null), date);
}

Expand All @@ -56,7 +57,7 @@ public TargetAmountDto.RecentTargetAmountRes getRecentTargetAmount(Long userId)
}

@Transactional
public TargetAmountDto.TargetAmountInfo updateTargetAmount(Long targetAmountId, Integer amount) {
public TargetAmountDto.TargetAmountInfo updateTargetAmount(Long targetAmountId, int amount) {
TargetAmount targetAmount = targetAmountSaveService.updateTargetAmount(targetAmountId, amount);

return TargetAmountDto.TargetAmountInfo.from(targetAmount);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
import kr.co.pennyway.api.apis.ledger.dto.TargetAmountDto;
import kr.co.pennyway.api.apis.ledger.usecase.TargetAmountUseCase;
import kr.co.pennyway.api.config.WebConfig;
import kr.co.pennyway.api.config.fixture.UserFixture;
import kr.co.pennyway.api.config.supporter.WithSecurityMockUser;
import kr.co.pennyway.domain.domains.target.domain.TargetAmount;
import kr.co.pennyway.domain.domains.target.exception.TargetAmountErrorCode;
import org.junit.jupiter.api.*;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
import org.springframework.boot.test.mock.mockito.MockBean;
Expand All @@ -30,7 +31,6 @@
@WebMvcTest(controllers = {TargetAmountController.class}, excludeFilters = {
@ComponentScan.Filter(type = FilterType.ASSIGNABLE_TYPE, classes = WebConfig.class)})
@ActiveProfiles("test")
@TestClassOrder(ClassOrderer.OrderAnnotation.class)
public class TargetAmountControllerUnitTest {
@Autowired
private MockMvc mockMvc;
Expand Down Expand Up @@ -111,8 +111,8 @@ void putTargetAmountWithNegativeAmount() throws Exception {
@WithSecurityMockUser
void putTargetAmountWithValidRequest() throws Exception {
// given
Integer amount = 100000;
given(targetAmountUseCase.updateTargetAmount(1L, amount)).willReturn(TargetAmountDto.TargetAmountInfo.from(TargetAmount.of(amount, UserFixture.GENERAL_USER.toUser())));
int amount = 100000;
given(targetAmountUseCase.updateTargetAmount(1L, amount)).willReturn(TargetAmountDto.TargetAmountInfo.from(null));

// when
ResultActions result = performPutTargetAmount(1L, amount);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import kr.co.pennyway.api.config.fixture.SpendingFixture;
import kr.co.pennyway.api.config.fixture.TargetAmountFixture;
import kr.co.pennyway.api.config.fixture.UserFixture;
import kr.co.pennyway.domain.domains.spending.service.SpendingService;
import kr.co.pennyway.domain.domains.target.domain.TargetAmount;
import kr.co.pennyway.domain.domains.target.service.TargetAmountService;
import kr.co.pennyway.domain.domains.user.domain.User;
Expand All @@ -31,6 +32,7 @@
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.user;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch;
import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

@Slf4j
Expand All @@ -46,6 +48,10 @@ public class TargetAmountIntegrationTest extends ExternalApiDBTestConfig {

@Autowired
private TargetAmountService targetAmountService;

@Autowired
private SpendingService spendingService;

@PersistenceContext
private EntityManager em;

Expand Down Expand Up @@ -140,6 +146,26 @@ void getTargetAmountAndTotalSpendingNotFound() throws Exception {
result.andDo(print()).andExpect(status().isNotFound());
}

@Test
@DisplayName("당월 지출 금액의 총합이 int 범위를 초과해도 200 OK 응답을 반환한다.")
@Transactional
void getTargetAmountAndTotalSpendingWithIntOverflow() throws Exception {
// given
User user = userService.createUser(UserFixture.GENERAL_USER.toUser());
TargetAmount targetAmount = targetAmountService.createTargetAmount(TargetAmountFixture.GENERAL_TARGET_AMOUNT.toTargetAmount(user));
spendingService.createSpending(SpendingFixture.MAX_SPENDING.toSpending(user));
spendingService.createSpending(SpendingFixture.MAX_SPENDING.toSpending(user));

// when
ResultActions result = performGetTargetAmountAndTotalSpending(user, LocalDate.now());

// then
result.andDo(print())
.andExpect(status().isOk())
.andExpect(jsonPath("$.data.targetAmount.totalSpending").value("4294967294"))
.andExpect(jsonPath("$.data.targetAmount.diffAmount").value(String.valueOf(4294967294L - (long) targetAmount.getAmount())));
}

private ResultActions performGetTargetAmountAndTotalSpending(User requestUser, LocalDate date) throws Exception {
UserDetails userDetails = SecurityUserDetails.from(requestUser);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@

public enum SpendingFixture {
GENERAL_SPENDING(10000, SpendingCategory.FOOD, LocalDateTime.now(), "카페인 수혈", "아메리카노 1잔"),
CUSTOM_CATEGORY_SPENDING(10000, SpendingCategory.CUSTOM, LocalDateTime.now(), "커스텀 카페인 수혈", "아메리카노 1잔");
CUSTOM_CATEGORY_SPENDING(10000, SpendingCategory.CUSTOM, LocalDateTime.now(), "커스텀 카페인 수혈", "아메리카노 1잔"),
MAX_SPENDING(Integer.MAX_VALUE, SpendingCategory.HOBBY, LocalDateTime.now(), "부가티", "이것이 인생");

private final int amount;
private final SpendingCategory category;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,28 @@
package kr.co.pennyway.domain.domains.spending.dto;

import java.time.YearMonth;

/**
* 사용자의 해당 년/월 총 지출 금액을 담는 DTO
*/
public record TotalSpendingAmount(
Integer year,
Integer month,
Integer totalSpending
int year,
int month,
long totalSpending
) {
public TotalSpendingAmount(Integer year, Integer month, Integer totalSpending) {
public TotalSpendingAmount(int year, int month, long totalSpending) {
this.year = year;
this.month = month;
this.totalSpending = totalSpending;
}

/**
* YearMonth 객체로 변환하는 메서드
*
* @return 해당 년/월을 나타내는 YearMonth 객체
*/
public YearMonth getYearMonth() {
return YearMonth.of(year, month);
}

}
Loading

0 comments on commit 127cf5c

Please sign in to comment.