[BOM-1157] feat: 마이페이지 월간 리포트 API 구현#783
Hidden character warning
Conversation
There was a problem hiding this comment.
K1 - 그냥 보세요
옆 파일 트리에서 보이는 것처럼 generated 하위 파일들은 OpenAPI codegen으로 자동 생성된 코드입니다.
API는 spec 레포의 ~.tsp 파일에 정의하고 있으며, 생성 코드가 한 패키지에 계속 쌓이지 않도록 tsp 파일명을 기준으로 하위 패키지가 분리되도록 설정했습니다.
| @Validated | ||
| @Tag(name = "MonthlyReport", description = "the MonthlyReport API") | ||
| @Generated("org.openapitools.codegen.languages.SpringCodegen") |
There was a problem hiding this comment.
k3
기존 컨벤션인 애노테이션 길이 순 정렬을 고려했습니다.
@Validated와 @Tag는 API 동작과 문서화를 위한 애노테이션이고, @Generated는 해당 파일이 OpenAPI codegen으로 생성되었음을 나타내는 메타 정보입니다.
따라서 @Generated의 의미가 더 잘 드러나도록 일반 애노테이션 정렬 규칙과는 별개로 항상 선언부 바로 위, 즉 애노테이션 블록의 마지막에 위치하도록 했습니다.
| @JsonValue | ||
| public String getValue() { | ||
| return value; | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return String.valueOf(value); | ||
| } | ||
|
|
||
| @JsonCreator | ||
| public static ChangeDirection fromValue(String value) { | ||
| for (ChangeDirection b : ChangeDirection.values()) { | ||
| if (b.value.equals(value)) { | ||
| return b; | ||
| } | ||
| } | ||
| throw new IllegalArgumentException("Unexpected value '" + value + "'"); | ||
| } |
There was a problem hiding this comment.
K2
이 부분은 OpenAPI codegen이 enum의 JSON 직렬화/역직렬화를 보장하기 위해 생성하는 코드입니다.
스펙에 정의된 enum 문자열 값과 Java enum 상수를 명시적으로 매핑해야 해서 value, @JsonValue, @JsonCreator, fromValue가 함께 생성됩니다.
직접 작성했다면 더 단순하게 만들 수도 있지만, codegen 결과물에서는 enum 값 매핑을 안전하게 처리하기 위해 생기는 어쩔 수 없는 부분입니다.
| public static FrequentReadNewsletterResponse of( | ||
| int rank, | ||
| Long newsletterId, | ||
| String name, | ||
| long readCount | ||
| ) { | ||
| return new FrequentReadNewsletterResponse( | ||
| rank, | ||
| newsletterId, | ||
| name, | ||
| readCount | ||
| ); | ||
| } |
There was a problem hiding this comment.
K1
OpenAPI codegen으로 자동 생성되는 응답 DTO는 매퍼/호출부에서 기존과 동일한 생성 방식을 유지하기 위해 codegen 템플릿 차원에서 필드 개수에 따라 from(...)이나 of(...)를 함께 생성되도록 했습니다.
There was a problem hiding this comment.
P4
codegen 템플릿 차원에서 설정한 것이라면, 생성자 필드와 정적 팩터리 메서드 파라미터가 서로 동일할 것 같은데요 (맞나요?ㅎㅎ)
만약 기본 생성자와 정적 팩터리 메서드의 인자가 서로 똑같다면 new FrequentReadNewsletterResponse(...) 방식과 FrequentReadNewsletterResponse.of(...) 방식의 차이가 뭔지 잘 모르겠어서요..~! 기존과 동일한 생성 방식을 유지하기 위함이라고 하시긴 했지만..
저는 기본 생성자와 똑같은 인자로 정적 팩터리 메서드를 만든 경험이 거의 없었어서, 조금 어색하게 느껴졌던 것 같아요!
'이미 존재하는 기본 생성자로 만들 수 있는데 왜 똑같은 역할을 하는 메서드를 또 정의하는거지?' 이런 느낌이랄까..
There was a problem hiding this comment.
차이가 없는게 맞습니다,,
컨벤션이 record 생성 시 new 대신 of / from을 쓰는 거라서, 최대한 기존 컨벤션, 기존에 코드 작성하던 방식에 영향을 안주기 위해 추가한 것입니다,,
There was a problem hiding this comment.
K1 응답 매퍼 클래스를 따로 둔 이유
이게 Spec-first 방식에서 우리 컨벤션과 가장 달라지게 하는 부분이라고 생각합니다,,
generated 하위 모든 파일은 OpenAPI codegen으로 매 빌드마다 generated/openapi/ 아래 새로 생성됩니다.
빌드 시 cleanGeneratedOpenApi → openApiGenerate → addOpenApiResponseFactories 순으로 동작하며, 마지막 단계에서 record 컴포넌트와 1:1로 매칭되는 of(...) / from(...)만 자동 주입됩니다.
이 구조상 도메인 → 응답 변환 로직을 응답 DTO 안에 둘 수 없습니다.
- generated 파일에 직접 추가 → cleanGeneratedOpenApi가 매번 삭제
- pojo.mustache 수정 → 템플릿은 도메인 타입, 구성을 모름
- 후처리 스크립트 확장 → 도메인별 분기를 빌드 스크립트가 알아야 하는 잘못된 의존 방향
- 현재 응답 DTO에서
from()이나of()가 자동 생성되도록 한 것은 codegen 이후 후처리 스크립트에 의해 생깁니다. 해당 스크립트는gradle/openapi-generated-format.gradle에 위치해 있습니다. - 후처리 스크립트에서 각 DTO에 맞는 생성 로직을 작성하기 위해서는 모든 분기를 해당 파일에 작성해야 합니다..
- 현재 응답 DTO에서
- 서비스/컨트롤러 인라인 → 응답 표현 관심사(rounding scale, null direction, 1-base ranking 등)가 도메인 계층으로 누수
따라서 응답 변환 로직은 별도 매퍼 클래스(@component)에 응집하는 것이 현재 codegen 구조에서 최선이라 생각했습니다.
There was a problem hiding this comment.
이 부분은 많이 아쉽긴 하네여 .. codegen을 사용하는 이상 매퍼 클래스 외에는 다른 대안이 떠오르지 않네요 흠 ㅜㅜ
There was a problem hiding this comment.
현재 구조에서는 별도 매퍼로 분리하는 방향이 가장 현실적인 것 같네요..
| MonthlyPeriod currentPeriod = MonthlyPeriod.from(targetMonth); | ||
| MonthlyPeriod previousPeriod = MonthlyPeriod.from(targetMonth.minusMonths(1)); | ||
|
|
||
| ReadCountComparison readCountComparison = countReads(member, previousPeriod, currentPeriod); |
There was a problem hiding this comment.
k5
한번의 쿼리로 저번 달과 이번 달의 count 수를 찾아올 수 있도록 했습니다.
There was a problem hiding this comment.
k5
codegen 방식으로 자동 생성된 API 인터페이스나 DTO를 처음으로 사용해보는 PR이라 제대로 요청이 간다는 것을 검증하기 위해 작성했습니다.
There was a problem hiding this comment.
k1 기존 article 데이터를 기반으로 article_read_history 데이터를 백필하는 게 나을지 고민됩니다.
article 테이블의 데이터는 주기적으로 삭제되기 때문에 전체 과거 읽기 이력을 완벽하게 복원할 수는 없습니다.
그럼에도 백필을 하지 않으면 월간 리포트 도입 직후에는 기존 사용자의 읽기 활동이 전혀 반영되지 않아 리포트가 비어 보일 수 있습니다.
따라서 완전한 과거 데이터 복원보다는, 남아 있는 데이터 범위 안에서 초기 리포트 경험을 보완하는 용도로 백필하는 것이 낫다고 판단했습니다.
이 방향이 괜찮은지, 아니면 신규 이력부터(6월부터)만 article_read_history에 쌓는 방식이 더 적절할지 확인 부탁드립니다.
There was a problem hiding this comment.
전체 읽은 아티클 수 기반으로 랭킹 등의 경쟁요소가 추가된다면 특정 날짜부터 집계하는게 나을 것 같지만, 현재로서 관련 경쟁요소는 추가될 계획은 없어보여서 저도 현재 방식이 괜찮다고 생각합니다!
그래도 지금까지 읽은 내역 중 남아있는 데이터에 한해서는, 그동안의 읽기 기록을 인정하고 반영해주는게 사용자 입장에서도 좋을 것 같아요
There was a problem hiding this comment.
현재 기능 범위에서는 백필하는 방향이 더 좋아 보입니다.
새로운 기능이 오픈되었을 때 첫 화면이 완전히 비어있는 것보다는, 최근 기록이라도 유저에게 바로 노출되는 것이 훨씬 긍정적일 것 같아요.
근데 이 데이터 포함해서 앞으로 백필할 데이터도 회원 탈퇴시에 삭제해 줘야겠네요.
There was a problem hiding this comment.
의견 감사합니다!
기능이 추가되어도 탈퇴 시 데이터 정리가 추가가 안돼서 다른 pr에서 한번에 해야할거 같습니다 ㅜ
minSsan
left a comment
There was a problem hiding this comment.
개인 일정으로 리뷰가 많이 늦었네요 ,, 미안합니다 ㅠ
디코 알림 전송 겸 몇 가지는 한번 확인해보시면 좋을 듯 하여 RC로 남기겠습니다!
큰 변경 요청 건은 없어서, 다음 리뷰 요청에서 바로 어푸해드릴 수 있을 것 같아요~
고생하셨습니다 😁
| UP("UP"), | ||
| DOWN("DOWN"), | ||
| SAME("SAME"); |
There was a problem hiding this comment.
P2
API 스펙 보면서 궁금했던 부분인데, 증감률(rate 필드)만으로도 판단 가능한 정보인 것 같은데 따로 ENUM으로 분리하는 이유가 있나요??
There was a problem hiding this comment.
증감률 계산 과정에서 서버가 이미 현재 값과 이전 값을 비교하고 있으므로, 그 결과로 나오는 증감 방향도 서버가 함께 내려주어 클라이언트가 증감률을 해석해 방향을 다시 계산하지 않아도 되도록 했습니다. 클라이언트 요청이기도 하구여,,
그리고 지금은 previousCount가 0일 경우 무조건 readArticleChangeRate와 readArticleChangeDirection이 모두 null인데, 추후 운영하면서 이 케이스를 증가로 표현해야 한다고 판단되면 readArticleChangeDirection만 수정해서 대응할 수 있을 거라 생각했습니다.
| public static FrequentReadNewsletterResponse of( | ||
| int rank, | ||
| Long newsletterId, | ||
| String name, | ||
| long readCount | ||
| ) { | ||
| return new FrequentReadNewsletterResponse( | ||
| rank, | ||
| newsletterId, | ||
| name, | ||
| readCount | ||
| ); | ||
| } |
There was a problem hiding this comment.
P4
codegen 템플릿 차원에서 설정한 것이라면, 생성자 필드와 정적 팩터리 메서드 파라미터가 서로 동일할 것 같은데요 (맞나요?ㅎㅎ)
만약 기본 생성자와 정적 팩터리 메서드의 인자가 서로 똑같다면 new FrequentReadNewsletterResponse(...) 방식과 FrequentReadNewsletterResponse.of(...) 방식의 차이가 뭔지 잘 모르겠어서요..~! 기존과 동일한 생성 방식을 유지하기 위함이라고 하시긴 했지만..
저는 기본 생성자와 똑같은 인자로 정적 팩터리 메서드를 만든 경험이 거의 없었어서, 조금 어색하게 느껴졌던 것 같아요!
'이미 존재하는 기본 생성자로 만들 수 있는데 왜 똑같은 역할을 하는 메서드를 또 정의하는거지?' 이런 느낌이랄까..
| @Param("previousStart") LocalDateTime previousStart, | ||
| @Param("currentStart") LocalDateTime currentStart, | ||
| @Param("currentEnd") LocalDateTime currentEnd |
There was a problem hiding this comment.
P3
사소하긴 하지만, 이 메서드만 봤을 때는 각 인자가 무슨 의미인지 파악하기 어려웠던 것 같아요!
지난 달, 이번 달 범위를 나타내는 것 같은데 이 의미가 조금 더 드러나는 이름이면 좋을 것 같다는 생각이 들었습니다 ㅎㅎ
e.g.
lastMonthStart,thisMonthStart...
There was a problem hiding this comment.
refactor: 월간 리포트 읽기 수 비교 쿼리 네이밍 개선
말씀해주신 대로 인자 의미가 드러나도록 lastMonthStart / thisMonthStart / thisMonthEnd로 바꿨습니다.
겸사겸사 MonthlyPeriod도 특정 달에 종속되지 않는 범용 값 객체라, 필드를 start / end로 단순화하고 "지난달/이번달" 같은 역할 의미는 사용하는 곳에서 드러내도록 정리했습니다!
There was a problem hiding this comment.
이 부분은 많이 아쉽긴 하네여 .. codegen을 사용하는 이상 매퍼 클래스 외에는 다른 대안이 떠오르지 않네요 흠 ㅜㅜ
There was a problem hiding this comment.
전체 읽은 아티클 수 기반으로 랭킹 등의 경쟁요소가 추가된다면 특정 날짜부터 집계하는게 나을 것 같지만, 현재로서 관련 경쟁요소는 추가될 계획은 없어보여서 저도 현재 방식이 괜찮다고 생각합니다!
그래도 지금까지 읽은 내역 중 남아있는 데이터에 한해서는, 그동안의 읽기 기록을 인정하고 반영해주는게 사용자 입장에서도 좋을 것 같아요
kysub99
left a comment
There was a problem hiding this comment.
codegen 구조를 고려해서 구현 잘 정리해주셨네요.
몇 가지 확인 포인트 리뷰로 남겼습니다!
| long daysBetween = ChronoUnit.DAYS.between(joinedAt, LocalDate.now(clock)); | ||
| int daysSinceJoined = Math.toIntExact(daysBetween); |
There was a problem hiding this comment.
p2
현재 구현은 가입 당일부터 0일차로 계산되는데, 사용자에게는 가입 당일을 1일차로 보여주는 쪽이 더 자연스럽다고 생각합니다!
There was a problem hiding this comment.
진짜 예리하시네요 ㅎㅎ
There was a problem hiding this comment.
현재 구조에서는 별도 매퍼로 분리하는 방향이 가장 현실적인 것 같네요..
There was a problem hiding this comment.
현재 기능 범위에서는 백필하는 방향이 더 좋아 보입니다.
새로운 기능이 오픈되었을 때 첫 화면이 완전히 비어있는 것보다는, 최근 기록이라도 유저에게 바로 노출되는 것이 훨씬 긍정적일 것 같아요.
근데 이 데이터 포함해서 앞으로 백필할 데이터도 회원 탈퇴시에 삭제해 줘야겠네요.
* chore: 월간 리포트 OpenAPI 생성 코드 반영 * feat: 월간 리포트 API 구현 * test: 월간 리포트 서비스 테스트 추가 * chore: OpenAPI 응답 DTO 생성 코드 갱신 * feat: 월간 리포트 엔드포인트 노출 * feat: 마이페이지 가입일 조회 API 구현 * refactor: 월간 리포트 응답 매핑 정리 * chore: OpenAPI spec 서브모듈 갱신 * fix: 월간 리포트 증감률 null 처리 * test: 월간 리포트 컨트롤러 테스트 추가 * feat: 아티클 읽기 히스토리 백필 마이그레이션 추가 * chore: OpenAPI 생성 코드 도메인 패키지 반영 * chore: OpenAPI 월간 리포트 생성 코드 패키지 분리 * chore: OpenAPI 태그 설명 반영 * refactor: 월간 리포트 읽기 수 비교 쿼리 네이밍 개선 * fix: 가입 일수를 가입 당일부터 1일째로 계산 * chore: flyway 스크립트 버전 수정
* build: openapi 코드젠 후처리 개선 - constraints.* 제거 및 메서드 닫는 괄호 개행 - api.mustache: useBeanValidation 블록에서 constraints.* 제거 (필요 시 generator가 specific import로 처리) - openapi-generated-format.gradle: googleJavaFormat 후 메서드 마지막 파라미터 줄의 ); 를 새 줄로 이동하는 규칙 추가 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: OpenAPI API 템플릿에 Bean Validation 제약 import 추가 * [BOM-1157] feat: 마이페이지 월간 리포트 API 구현 (#783) * chore: 월간 리포트 OpenAPI 생성 코드 반영 * feat: 월간 리포트 API 구현 * test: 월간 리포트 서비스 테스트 추가 * chore: OpenAPI 응답 DTO 생성 코드 갱신 * feat: 월간 리포트 엔드포인트 노출 * feat: 마이페이지 가입일 조회 API 구현 * refactor: 월간 리포트 응답 매핑 정리 * chore: OpenAPI spec 서브모듈 갱신 * fix: 월간 리포트 증감률 null 처리 * test: 월간 리포트 컨트롤러 테스트 추가 * feat: 아티클 읽기 히스토리 백필 마이그레이션 추가 * chore: OpenAPI 생성 코드 도메인 패키지 반영 * chore: OpenAPI 월간 리포트 생성 코드 패키지 분리 * chore: OpenAPI 태그 설명 반영 * refactor: 월간 리포트 읽기 수 비교 쿼리 네이밍 개선 * fix: 가입 일수를 가입 당일부터 1일째로 계산 * chore: flyway 스크립트 버전 수정 --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
📌 What
마이페이지 월간 리포트 API를 구현했습니다.
❓ Why
마이페이지에서 사용자의 월간 읽기 활동을 조회할 수 있도록 API가 필요했습니다.
🔧 How
월간 리포트 집계
year,month를 기준으로 조회 월의 시작/끝 범위를 계산했습니다.article_read_history를 기준으로 집계했습니다.readCount = 0으로 응답합니다.증감률 정책
0이면 증감률을 계산하지 않고null로 응답합니다.readArticleChangeRate,readArticleChangeDirection을 모두null로 내려줍니다.예시는 다음과 같습니다. (previous: 저번 달, current: 이번 달)
previous = 0, current = 0→rate = null,direction = nullprevious = 0, current = 5→rate = null,direction = nullprevious = 5, current = 5→rate = 0.0,direction = SAMEprevious = 5, current = 8→rate = 60.0,direction = UPprevious = 5, current = 3→rate = -40.0,direction = DOWN기존 데이터 백필
article.is_read = true인 데이터를article_read_history에 백필하는 마이그레이션을 추가했습니다.article.updated_at으로 판단할 수 있어, 백필 시updated_at을read_at으로 사용했습니다.INSERT IGNORE를 사용했습니다.👀 Review Point (Optional)
article데이터를 기반으로article_read_history데이터를 백필하는 게 나을지 고민됩니다.article테이블의 데이터는 주기적으로 삭제되기 때문에 전체 과거 읽기 이력을 완벽하게 복원할 수는 없습니다.article_read_history에 쌓는 방식이 더 적절할지 확인 부탁드립니다.