-
Notifications
You must be signed in to change notification settings - Fork 3
[release] be #1080
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[release] be #1080
Conversation
- FcmTopicResolver 생성하여 spring.profiles.active 기반 토픽 prefix 생성 - RecruitmentStateCalculator를 @component로 리팩터링하여 FcmTopicResolver 주입 - FcmAsyncService 구독/구독해제 시 prefixed 토픽 사용하도록 수정 - application.properties에 기본 profile을 local로 설정 토픽 네이밍: - prod: clubId (기존 호환성 유지) - staging: staging_clubId - local: local_clubId
- FcmTopicResolver 생성하여 spring.profiles.active 기반 토픽 prefix 생성 - RecruitmentStateCalculator를 @component로 리팩터링하여 FcmTopicResolver 주입 - FcmAsyncService 구독/구독해제 시 prefixed 토픽 사용하도록 수정 - application.properties에 기본 profile을 local로 설정 토픽 네이밍: - prod: clubId (기존 호환성 유지) - staging: staging_clubId - local: local_clubId
…-history-MOA-515 [feature]동아리 정보 이력 저장 로직 추가
…n-staging-live-MOA-540 [feature] 개발서버에서 동아리 모집 상태 변경시 라이브서버 푸시알림 전송안되게 변경한다
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning
|
| Cohort / File(s) | 변경 요약 |
|---|---|
JaVers 의존성 및 설정 backend/build.gradle, backend/src/main/java/moadong/global/config/JaversConfig.java, backend/src/main/java/moadong/fcm/repository/ClubRepository.java |
JaVers MongoDB 스타터 의존성 추가, AuthorProvider와 MongoRepository 빈 구성, JaversSpringDataAuditable 임포트 |
엔티티 DiffIgnore 어노테이션 backend/src/main/java/moadong/club/entity/Club.java, backend/src/main/java/moadong/club/entity/ClubRecruitmentInformation.java |
Club의 socialLinks, ClubRecruitmentInformation의 logo/cover/presidentTelephoneNumber/externalApplicationUrl/feedImages/clubRecruitmentStatus/lastModifiedDate 필드에 @DiffIgnore 추가 |
FCM 주제 해석 컴포넌트 backend/src/main/java/moadong/fcm/util/FcmTopicResolver.java, backend/src/main/java/moadong/fcm/service/FcmAsyncService.java |
FcmTopicResolver 신규 컴포넌트 추가 (프로파일별 주제 이름 해석), FcmAsyncService에서 clubId 직접 사용 대신 해석된 주제명 사용 |
RecruitmentStateCalculator 리팩토링 backend/src/main/java/moadong/club/util/RecruitmentStateCalculator.java, backend/src/main/java/moadong/club/service/RecruitmentStateChecker.java, backend/src/main/java/moadong/club/service/ClubProfileService.java |
정적 메서드 → 인스턴스 메서드 변환, @Component/@requiredargsconstructor 추가, FcmTopicResolver 의존성 주입, ClubProfileService에서 javers.commit 호출 추가 |
Club 엔티티 향상 backend/src/main/java/moadong/club/entity/Club.java |
sendPushNotification 메서드에 로깅 강화 (시작/성공 로그, messageId 캡처, clubId 포함) |
클럽 이력 조회 기능 backend/src/main/java/moadong/log/club/controller/ClubHistoryController.java, backend/src/main/java/moadong/log/club/service/ClubHistoryService.java, backend/src/main/java/moadong/log/club/payload/response/ClubHistoryResponse.java |
신규 컨트롤러/서비스/DTO로 클럽 변경 이력 조회 API 추가 (/api/clubs/{clubId}/histories) |
테스트 업데이트 backend/src/test/java/moadong/club/service/ClubProfileServiceDateTest.java, backend/src/test/java/moadong/club/service/RecruitmentStateCheckerTest.java, backend/src/test/java/moadong/fixture/ClubFixture.java, backend/src/test/java/moadong/unit/club/ClubProfileServiceTest.java |
RecruitmentStateCalculator 정적 모킹 → 인스턴스 모킹, Javers 모킹 필드 추가, 테스트 검증 로직 조정 |
Sequence Diagram(s)
sequenceDiagram
participant Client
participant Controller as ClubHistoryController
participant Service as ClubHistoryService
participant JaVers
participant MongoDB as MongoDB<br/>(JaVers Store)
participant DTO as ClubHistoryResponse
Client->>Controller: GET /api/clubs/{clubId}/histories
Controller->>Service: getClubHistories(clubId)
Service->>JaVers: findHistories(Club with query)
JaVers->>MongoDB: Query club change history
MongoDB-->>JaVers: Return Shadow<Club> list
JaVers-->>Service: List<Shadow<Club>>
loop For each Shadow<Club>
Service->>DTO: ClubHistoryResponse.from(shadow)
DTO-->>Service: ClubHistoryResponse
end
Service-->>Controller: List<ClubHistoryResponse>
Controller-->>Client: ResponseEntity<List<ClubHistoryResponse>>
sequenceDiagram
participant ClubProfileService
participant ClubRepo as Club Repository
participant Javers
participant RecruitmentCalculator as RecruitmentStateCalculator
participant FCM as FcmAsyncService
ClubProfileService->>ClubProfileService: updateClubInfo(request)
ClubProfileService->>ClubRepo: save(club)
ClubRepo-->>ClubProfileService: saved club
ClubProfileService->>Javers: commit(club, author)
Javers-->>ClubProfileService: audit recorded
ClubProfileService->>ClubProfileService: updateClubRecruitmentInfo(request)
ClubProfileService->>RecruitmentCalculator: calculate(club, start, end)
RecruitmentCalculator->>FCM: (topic resolution via FcmTopicResolver)
RecruitmentCalculator-->>ClubProfileService: calculation complete
ClubProfileService->>ClubRepo: save(club)
ClubRepo-->>ClubProfileService: saved club
ClubProfileService->>Javers: commit(club, author)
Javers-->>ClubProfileService: audit recorded
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- [feature] 개발서버에서 동아리 모집 상태 변경시 라이브서버 푸시알림 전송안되게 변경한다 #1079: Club.sendPushNotification 로깅 강화, RecruitmentStateCalculator 리팩토링, FcmTopicResolver/FcmAsyncService 주제 해석 로직 변경이 동일하게 적용됨
- [feature]동아리 정보 이력 저장 로직 추가 #1037: JaVers 기반 클럽 이력/감사 시스템 구현(의존성, DiffIgnore 어노테이션, 이력 컨트롤러/서비스/DTO, Javers 커밋)이 일치함
- [release] 모아동 BE v1.0.4 배포 #579: RecruitmentStateCalculator, RecruitmentStateChecker, FCM 주제 처리 등 모집 및 FCM 관련 동일 클래스 수정
Suggested reviewers
- Zepelown
- oesnuj
- seongwon030
🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 4.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Title check | ❓ Inconclusive | [release] be 제목은 너무 모호하고 일반적이어서 변경 사항의 구체적인 내용을 전달하지 못합니다. | 제목을 더 구체적으로 변경하세요. 예: '[release] Add Javers audit logging for club profile changes' 또는 '[release] v1.1.3: Add club history tracking and FCM topic resolution' |
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
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 @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@backend/src/main/java/moadong/club/entity/Club.java`:
- Around line 149-155: The FCM failure log in sendPushNotification currently
only logs e.getMessage(), making root-cause tracing hard; update the catch block
in the Club.sendPushNotification method to pass the FirebaseMessagingException
object to the logger (e.g., log.error("FCM 알림 전송 실패 - clubId: {}, error: {}",
this.id, e.getMessage(), e)) so the full stack trace is captured; ensure you
keep the existing message format and import/log usage consistent with the
class's logger.
In `@backend/src/main/java/moadong/club/service/ClubProfileService.java`:
- Around line 39-52: The updateClubRecruitmentInfo method in ClubProfileService
is missing a transaction boundary, so if clubRepository.save(...) succeeds but
javers.commit(...) fails you get inconsistent state; add the same `@Transactional`
annotation used on updateClubInfo to the updateClubRecruitmentInfo method (or
the class) so the save and javers.commit are executed within a single
transaction and will roll back on failure, ensuring the import and propagation
settings match the existing updateClubInfo usage.
In `@backend/src/main/java/moadong/fcm/util/FcmTopicResolver.java`:
- Around line 9-16: The resolveTopic method in FcmTopicResolver currently
compares the entire activeProfile string to "prod" which breaks when
spring.profiles.active contains multiple comma-separated profiles; change
resolveTopic(String clubId) to split activeProfile by comma, trim entries, check
if any entry equals "prod" and if so return clubId, otherwise use the first
non-empty profile entry as the prefix (profile + "_" + clubId); update
references to activeProfile parsing in FcmTopicResolver.resolveTopic accordingly
so Firebase routing in FcmAsyncService and RecruitmentStateCalculator works
correctly.
In `@backend/src/main/java/moadong/global/config/JaversConfig.java`:
- Around line 16-19: The authorProvider() lambda can NPE when
SecurityContextHolder.getContext().getAuthentication() is null (e.g., schedulers
or unauthenticated contexts); update authorProvider() to check the
SecurityContext and Authentication for null (and also handle
anonymous/unauthenticated cases) and return a safe default string like "system"
or "anonymous" instead of calling getName() directly; locate the
authorProvider() method in JaversConfig and add the null checks/guarding logic
so it never dereferences a null Authentication.
🧹 Nitpick comments (2)
backend/src/main/java/moadong/club/entity/ClubRecruitmentInformation.java (1)
30-31: 주석 처리된 id 필드 정리 권장
주석 코드가 남아 있으면 혼동을 줄 수 있어 제거하는 편이 깔끔합니다.🧹 제안 수정
-// `@Id` -// private String id;backend/src/test/java/moadong/fixture/ClubFixture.java (1)
22-33: 사용되지 않는 id 매개변수 정리
getId()스텁을 제거했다면,id파라미터도 함께 제거하는 편이 혼동이 없습니다(호출부 업데이트 필요).🧹 제안 수정
- public static ClubRecruitmentInformation createRecruitmentInfo( - String id, + public static ClubRecruitmentInformation createRecruitmentInfo( String logo, String introduction, String presidentName, String presidentTelephoneNumber, LocalDateTime recruitmentStart, LocalDateTime recruitmentEnd, List<String> feedImages, ClubRecruitmentStatus clubRecruitmentStatus) { ClubRecruitmentInformation clubRecruitmentInfo = mock(ClubRecruitmentInformation.class); -// when(clubRecruitmentInfo.getId()).thenReturn(id);
| public void sendPushNotification(Message message) { | ||
| try { | ||
| FirebaseMessaging.getInstance().send(message); | ||
| log.info("FCM 알림 전송 시작 - clubId: {}, clubName: {}", this.id, this.name); | ||
| String messageId = FirebaseMessaging.getInstance().send(message); | ||
| log.info("FCM 알림 전송 성공 - clubId: {}, messageId: {}", this.id, messageId); | ||
| } catch (FirebaseMessagingException e) { | ||
| log.error("FirebaseSendNotificationError: {}", e.getMessage()); | ||
| log.error("FCM 알림 전송 실패 - clubId: {}, error: {}", this.id, e.getMessage()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FCM 실패 로그에 예외 스택트레이스 포함
현재는 메시지만 남아서 원인 추적이 어렵습니다. 예외 객체를 함께 로깅해 주세요.
🔧 제안 수정
- } catch (FirebaseMessagingException e) {
- log.error("FCM 알림 전송 실패 - clubId: {}, error: {}", this.id, e.getMessage());
- }
+ } catch (FirebaseMessagingException e) {
+ log.error("FCM 알림 전송 실패 - clubId: {}, error: {}", this.id, e.getMessage(), e);
+ }🤖 Prompt for AI Agents
In `@backend/src/main/java/moadong/club/entity/Club.java` around lines 149 - 155,
The FCM failure log in sendPushNotification currently only logs e.getMessage(),
making root-cause tracing hard; update the catch block in the
Club.sendPushNotification method to pass the FirebaseMessagingException object
to the logger (e.g., log.error("FCM 알림 전송 실패 - clubId: {}, error: {}", this.id,
e.getMessage(), e)) so the full stack trace is captured; ensure you keep the
existing message format and import/log usage consistent with the class's logger.
| public void updateClubRecruitmentInfo(ClubRecruitmentInfoUpdateRequest request, | ||
| CustomUserDetails user) { | ||
| Club club = clubRepository.findClubByUserId(user.getId()) | ||
| .orElseThrow(() -> new RestApiException(ErrorCode.CLUB_NOT_FOUND)); | ||
| club.update(request); | ||
| RecruitmentStateCalculator.calculate( | ||
| recruitmentStateCalculator.calculate( | ||
| club, | ||
| club.getClubRecruitmentInformation().getRecruitmentStart(), | ||
| club.getClubRecruitmentInformation().getRecruitmentEnd() | ||
| ); | ||
| club.getClubRecruitmentInformation().updateLastModifiedDate(); | ||
| clubRepository.save(club); | ||
| Club saved = clubRepository.save(club); | ||
| javers.commit(user.getUsername(), saved); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the ClubProfileService class to check for transaction annotations
find . -type f -name "ClubProfileService.java" | head -5Repository: Moadong/moadong
Length of output: 127
🏁 Script executed:
# Also, let's look at the file structure to better understand the project
git ls-files | grep -E "(Service|Repository)" | head -20Repository: Moadong/moadong
Length of output: 1442
🏁 Script executed:
# Let's check for common transaction management patterns in the codebase
rg "@Transactional" --type java | head -20Repository: Moadong/moadong
Length of output: 1805
🏁 Script executed:
# Let's examine the ClubProfileService class to see its structure and annotations
cat -n backend/src/main/java/moadong/club/service/ClubProfileService.javaRepository: Moadong/moadong
Length of output: 3211
@transactional 어노테이션 누락으로 인한 데이터 불일치 위험
메서드가 비트랜잭션 상태이기 때문에 clubRepository.save()는 성공하지만 javers.commit()이 실패할 경우, 데이터는 저장되고 API는 오류를 반환하는 불일치가 발생합니다. 같은 클래스의 updateClubInfo 메서드(30행)는 @Transactional이 적용되어 있으므로, 이 메서드에도 동일하게 적용하여 감사 커밋 실패 시 롤백되도록 처리하세요.
🔧 예시(트랜잭션 적용)
- public void updateClubRecruitmentInfo(ClubRecruitmentInfoUpdateRequest request,
+ `@Transactional`
+ public void updateClubRecruitmentInfo(ClubRecruitmentInfoUpdateRequest request,🤖 Prompt for AI Agents
In `@backend/src/main/java/moadong/club/service/ClubProfileService.java` around
lines 39 - 52, The updateClubRecruitmentInfo method in ClubProfileService is
missing a transaction boundary, so if clubRepository.save(...) succeeds but
javers.commit(...) fails you get inconsistent state; add the same `@Transactional`
annotation used on updateClubInfo to the updateClubRecruitmentInfo method (or
the class) so the save and javers.commit are executed within a single
transaction and will roll back on failure, ensuring the import and propagation
settings match the existing updateClubInfo usage.
| @Value("${spring.profiles.active:prod}") | ||
| private String activeProfile; | ||
|
|
||
| public String resolveTopic(String clubId) { | ||
| if ("prod".equals(activeProfile)) { | ||
| return clubId; | ||
| } | ||
| return activeProfile + "_" + clubId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Find and read the FcmTopicResolver.java file
find . -name "FcmTopicResolver.java" -type fRepository: Moadong/moadong
Length of output: 121
🏁 Script executed:
# Also check the directory structure to understand the project layout
ls -la backend/src/main/java/moadong/fcm/util/ 2>/dev/null || echo "Path not found, searching..."
find . -type f -name "*.java" -path "*/fcm/util/*" | head -20Repository: Moadong/moadong
Length of output: 300
🌐 Web query:
Spring spring.profiles.active comma separated multiple profiles behavior
💡 Result:
Short answer: spring.profiles.active accepts a comma-separated list (e.g. --spring.profiles.active=dev,metrics) and activates all listed profiles. Spring will load profile-specific configuration/beans for each active profile; when the same property is defined in multiple active profiles, later entries override earlier ones (i.e. precedence depends on order). Use spring.profiles.include or profile groups to add/compose profiles if needed. [1][2]
Sources:
[1] Spring Boot reference — Profiles and adding active profiles.
[2] Spring Boot blog — config-file processing and profile activation.
🏁 Script executed:
# Read the FcmTopicResolver.java file completely
cat -n ./backend/src/main/java/moadong/fcm/util/FcmTopicResolver.javaRepository: Moadong/moadong
Length of output: 626
🏁 Script executed:
# Search for any other usages of activeProfile or resolveTopic in the codebase
rg "resolveTopic|activeProfile" --type java -B 2 -A 2Repository: Moadong/moadong
Length of output: 2764
멀티 프로파일 환경에서 토픽 라우팅이 실패합니다.
spring.profiles.active가 prod,metrics처럼 콤마로 구분된 여러 프로파일로 설정되면, "prod".equals(activeProfile) 비교가 실패하여 토픽이 잘못 생성됩니다. 예를 들어 "prod,metrics_clubId"처럼 불필요한 접두사가 붙게 되며, 이는 FcmAsyncService와 RecruitmentStateCalculator에서의 Firebase 메시징 라우팅을 방해합니다. 활성 프로파일 목록을 콤마로 분리한 후 prod 포함 여부를 확인하도록 수정하세요.
🔧 제안 수정
public String resolveTopic(String clubId) {
- if ("prod".equals(activeProfile)) {
+ String profile = (activeProfile == null || activeProfile.isBlank())
+ ? "prod"
+ : activeProfile;
+ boolean isProd = java.util.Arrays.stream(profile.split(","))
+ .map(String::trim)
+ .anyMatch(p -> p.equalsIgnoreCase("prod"));
+ if (isProd) {
return clubId;
}
- return activeProfile + "_" + clubId;
+ String prefix = profile.split(",")[0].trim();
+ return prefix + "_" + clubId;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Value("${spring.profiles.active:prod}") | |
| private String activeProfile; | |
| public String resolveTopic(String clubId) { | |
| if ("prod".equals(activeProfile)) { | |
| return clubId; | |
| } | |
| return activeProfile + "_" + clubId; | |
| public String resolveTopic(String clubId) { | |
| String profile = (activeProfile == null || activeProfile.isBlank()) | |
| ? "prod" | |
| : activeProfile; | |
| boolean isProd = java.util.Arrays.stream(profile.split(",")) | |
| .map(String::trim) | |
| .anyMatch(p -> p.equalsIgnoreCase("prod")); | |
| if (isProd) { | |
| return clubId; | |
| } | |
| String prefix = profile.split(",")[0].trim(); | |
| return prefix + "_" + clubId; | |
| } |
🤖 Prompt for AI Agents
In `@backend/src/main/java/moadong/fcm/util/FcmTopicResolver.java` around lines 9
- 16, The resolveTopic method in FcmTopicResolver currently compares the entire
activeProfile string to "prod" which breaks when spring.profiles.active contains
multiple comma-separated profiles; change resolveTopic(String clubId) to split
activeProfile by comma, trim entries, check if any entry equals "prod" and if so
return clubId, otherwise use the first non-empty profile entry as the prefix
(profile + "_" + clubId); update references to activeProfile parsing in
FcmTopicResolver.resolveTopic accordingly so Firebase routing in FcmAsyncService
and RecruitmentStateCalculator works correctly.
| public AuthorProvider authorProvider() { | ||
| return () -> { | ||
| return SecurityContextHolder.getContext().getAuthentication().getName(); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인증 정보 없을 때 NPE 방지 필요
스케줄러/비인증 컨텍스트에서 getAuthentication()가 null이면 NPE가 발생합니다. 안전한 기본값을 사용하세요.
🔧 제안 수정
`@Bean`
public AuthorProvider authorProvider() {
- return () -> {
- return SecurityContextHolder.getContext().getAuthentication().getName();
- };
+ return () -> {
+ var auth = SecurityContextHolder.getContext().getAuthentication();
+ return (auth != null && auth.isAuthenticated()) ? auth.getName() : "system";
+ };
}🤖 Prompt for AI Agents
In `@backend/src/main/java/moadong/global/config/JaversConfig.java` around lines
16 - 19, The authorProvider() lambda can NPE when
SecurityContextHolder.getContext().getAuthentication() is null (e.g., schedulers
or unauthenticated contexts); update authorProvider() to check the
SecurityContext and Authentication for null (and also handle
anonymous/unauthenticated cases) and return a safe default string like "system"
or "anonymous" instead of calling getName() directly; locate the
authorProvider() method in JaversConfig and add the null checks/guarding logic
so it never dereferences a null Authentication.
seongwon030
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
대체 무슨 문제일지..
v 1.1.3
Summary by CodeRabbit
새로운 기능
개선사항
✏️ Tip: You can customize this high-level summary in your review settings.