Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough클럽 회비 제출·승인 API와 Google Sheets 기반 멤버 동기화 기능(헤더 분석·디바운싱·동기화 실행)을 추가하고, 관련 엔티티·리포지토리·서비스·컨트롤러·DTO·이벤트·마이그레이션·인프라(스프레드시트 클라이언트, Executor 등)를 포함해 통합 구현했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FeeController as ClubFeePaymentController
participant FeeService as ClubFeePaymentService
participant FeeRepo as ClubFeePaymentRepository
participant DB as Database
participant Publisher as ApplicationEventPublisher
Client->>FeeController: submitFeePayment(clubId, request, userId)
FeeController->>FeeService: submitFeePayment(clubId, userId, paymentImageUrl)
FeeService->>FeeRepo: findByClubIdAndUserId(clubId,userId)
FeeRepo->>DB: SELECT
DB-->>FeeRepo: Result
alt exists
FeeService-->>FeeController: throw ALREADY_FEE_PAYMENT_SUBMITTED
else
FeeService->>FeeRepo: save(new ClubFeePayment)
FeeRepo->>DB: INSERT
DB-->>FeeRepo: created
FeeService->>Publisher: publish ClubFeePaymentCreated? (implicit)
FeeService-->>FeeController: ClubFeePaymentResponse
FeeController-->>Client: 200 OK
end
sequenceDiagram
participant Client
participant SheetController as ClubMemberSheetController
participant SheetService as ClubMemberSheetService
participant HeaderMapper as SheetHeaderMapper
participant GoogleSheets as Google Sheets API
participant ClaudeAI as Claude AI API
participant DB as Database
participant Debouncer as SheetSyncDebouncer
participant Executor as SheetSyncExecutor
Client->>SheetController: updateSheetId(clubId, request, userId)
SheetController->>SheetService: updateSheetId(clubId, userId, request)
SheetService->>HeaderMapper: analyzeHeaders(spreadsheetId)
HeaderMapper->>GoogleSheets: spreadsheets.values.get(range:A1:Z10)
GoogleSheets-->>HeaderMapper: rows
HeaderMapper->>ClaudeAI: POST prompt (headers -> JSON mapping)
ClaudeAI-->>HeaderMapper: JSON mapping
HeaderMapper-->>SheetService: SheetColumnMapping
SheetService->>DB: UPDATE club.google_sheet_id, sheet_column_mapping
DB-->>SheetService: ok
SheetService-->>Client: 200 OK
Note over SheetService,Debouncer: on ClubMemberChangedEvent / FeeApprovedEvent
SheetService->>Debouncer: debounce(clubId)
Debouncer->>Executor: schedule execute(clubId) after delay
Executor->>DB: SELECT members, payments
Executor->>GoogleSheets: batchUpdate / formatting
GoogleSheets-->>Executor: success
Executor-->>SheetService: 완료 로그
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can generate a title for your PR based on the changes.Add |
Contributor
There was a problem hiding this comment.
Pull request overview
구글 스프레드시트 API를 활용해 동아리 인명부/회비 납부 현황을 시트로 동기화하는 MVP 기능을 추가하고, 회비 납부 장부용 도메인/DB/API를 함께 도입합니다.
Changes:
- 동아리별 스프레드시트 ID 및 컬럼 매핑(JSON) 저장, 회비 납부 테이블 신설(Flyway V50~V52)
- 구글 시트 동기화 실행기/디바운서/헤더 분석(Claude 기반) 및 관련 설정/의존성 추가
- 회비 납부 접수/승인/조회 API 및 도메인(엔티티/리포지토리/서비스/컨트롤러) 추가
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/resources/db/migration/V50__add_google_sheet_id_to_club.sql | club에 google_sheet_id 컬럼 추가 |
| src/main/resources/db/migration/V51__add_club_fee_payment_table.sql | club_fee_payment 테이블 생성 |
| src/main/resources/db/migration/V52__add_sheet_column_mapping_to_club.sql | club에 sheet_column_mapping(JSON) 컬럼 추가 |
| src/main/resources/application-infrastructure.yml | google.sheets 설정 추가 |
| src/main/java/gg/agit/konect/infrastructure/googlesheets/GoogleSheetsProperties.java | 시트 설정 바인딩용 properties 추가 |
| src/main/java/gg/agit/konect/infrastructure/googlesheets/GoogleSheetsConfig.java | Google Sheets Sheets 클라이언트 Bean 구성 |
| src/main/java/gg/agit/konect/global/config/AsyncConfig.java | 시트 동기화 전용 @Async executor 추가 |
| src/main/java/gg/agit/konect/global/code/ApiResponseCode.java | 시트/회비납부 관련 에러 코드 추가 |
| src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java | 회원/회비 납부 상태를 시트에 동기화하는 실행기 추가 |
| src/main/java/gg/agit/konect/domain/club/service/SheetSyncDebouncer.java | 이벤트 기반 동기화 호출 디바운싱 추가 |
| src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java | Claude 기반 헤더 분석으로 컬럼 매핑 추론 추가 |
| src/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.java | 시트 ID 등록/수동 동기화 및 이벤트 리스닝 서비스 추가 |
| src/main/java/gg/agit/konect/domain/club/service/ClubMemberManagementService.java | 강제 탈퇴 시 시트 동기화 이벤트 발행 추가 |
| src/main/java/gg/agit/konect/domain/club/service/ClubFeePaymentService.java | 회비 납부 접수/승인/조회 비즈니스 로직 추가 |
| src/main/java/gg/agit/konect/domain/club/service/ClubApplicationService.java | 가입 승인 시 시트 동기화 이벤트 발행 추가 |
| src/main/java/gg/agit/konect/domain/club/repository/ClubFeePaymentRepository.java | 회비 납부 조회/저장 리포지토리 추가 |
| src/main/java/gg/agit/konect/domain/club/model/SheetColumnMapping.java | 시트 컬럼 매핑 모델 추가 |
| src/main/java/gg/agit/konect/domain/club/model/ClubFeePayment.java | 회비 납부 엔티티 추가 |
| src/main/java/gg/agit/konect/domain/club/model/Club.java | google_sheet_id, sheet_column_mapping 필드 및 업데이트 메서드 추가 |
| src/main/java/gg/agit/konect/domain/club/event/ClubMemberChangedEvent.java | 회원 변동 이벤트 추가 |
| src/main/java/gg/agit/konect/domain/club/event/ClubFeePaymentApprovedEvent.java | 회비 납부 승인 이벤트 추가 |
| src/main/java/gg/agit/konect/domain/club/enums/ClubSheetSortKey.java | 시트 동기화 정렬 키 enum 추가 |
| src/main/java/gg/agit/konect/domain/club/dto/ClubSheetIdUpdateRequest.java | 시트 ID 등록 요청 DTO 추가 |
| src/main/java/gg/agit/konect/domain/club/dto/ClubMemberSheetSyncResponse.java | 시트 동기화 응답 DTO 추가 |
| src/main/java/gg/agit/konect/domain/club/dto/ClubMemberSheetSyncRequest.java | (현재 미사용) 시트 동기화 요청 DTO 추가 |
| src/main/java/gg/agit/konect/domain/club/dto/ClubFeePaymentSubmitRequest.java | 회비 납부 접수 요청 DTO 추가 |
| src/main/java/gg/agit/konect/domain/club/dto/ClubFeePaymentResponse.java | 회비 납부 응답 DTO 추가 |
| src/main/java/gg/agit/konect/domain/club/controller/ClubMemberSheetController.java | 시트 ID 등록/수동 동기화 컨트롤러 추가 |
| src/main/java/gg/agit/konect/domain/club/controller/ClubMemberSheetApi.java | 시트 관련 Swagger API 인터페이스 추가 |
| src/main/java/gg/agit/konect/domain/club/controller/ClubFeePaymentController.java | 회비 납부 컨트롤러 추가 |
| src/main/java/gg/agit/konect/domain/club/controller/ClubFeePaymentApi.java | 회비 납부 Swagger API 인터페이스 추가 |
| build.gradle | Google Sheets API/인증 라이브러리 의존성 추가 |
Comment on lines
+115
to
+142
| private void updateMappedColumns( | ||
| String spreadsheetId, | ||
| List<ClubMember> members, | ||
| Map<Integer, ClubFeePayment> paymentMap, | ||
| SheetColumnMapping mapping | ||
| ) throws IOException { | ||
| int dataStartRow = 2; | ||
| Map<Integer, List<Object>> columnData = buildColumnData(members, paymentMap, mapping); | ||
|
|
||
| List<ValueRange> data = new ArrayList<>(); | ||
| for (Map.Entry<Integer, List<Object>> entry : columnData.entrySet()) { | ||
| int colIndex = entry.getKey(); | ||
| List<Object> values = entry.getValue(); | ||
| String colLetter = columnLetter(colIndex); | ||
| String range = colLetter + dataStartRow + ":" + colLetter; | ||
| List<List<Object>> wrapped = values.stream().map(v -> List.of((Object)v)).toList(); | ||
| data.add(new ValueRange().setRange(range).setValues(wrapped)); | ||
| } | ||
|
|
||
| if (!data.isEmpty()) { | ||
| googleSheetsService.spreadsheets().values() | ||
| .batchUpdate(spreadsheetId, | ||
| new BatchUpdateValuesRequest() | ||
| .setValueInputOption("USER_ENTERED") | ||
| .setData(data)) | ||
| .execute(); | ||
| } | ||
| } |
Comment on lines
+39
to
+51
| Club club = clubRepository.getById(clubId); | ||
| User user = userRepository.getById(userId); | ||
|
|
||
| clubFeePaymentRepository.findByClubIdAndUserId(clubId, userId) | ||
| .ifPresent(p -> { | ||
| throw CustomException.of( | ||
| gg.agit.konect.global.code.ApiResponseCode.ALREADY_FEE_PAYMENT_SUBMITTED | ||
| ); | ||
| }); | ||
|
|
||
| ClubFeePayment payment = ClubFeePayment.of(club, user, paymentImageUrl); | ||
| return ClubFeePaymentResponse.from(clubFeePaymentRepository.save(payment)); | ||
| } |
| } | ||
|
|
||
| public ClubFeePaymentResponse getMyFeePayment(Integer clubId, Integer userId) { | ||
| clubRepository.getById(clubId); |
Comment on lines
+21
to
+25
| private final ConcurrentHashMap<Integer, ScheduledFuture<?>> pendingTasks = | ||
| new ConcurrentHashMap<>(); | ||
| private final ScheduledExecutorService scheduler = | ||
| Executors.newSingleThreadScheduledExecutor(); | ||
|
|
| // 500 Internal Server Error (서버 오류) | ||
| CLIENT_ABORTED(HttpStatus.INTERNAL_SERVER_ERROR, "클라이언트에 의해 연결이 중단되었습니다."), | ||
| FAILED_UPLOAD_FILE(HttpStatus.INTERNAL_SERVER_ERROR, "파일 업로드에 실패했습니다."), | ||
| FAILED_SYNC_GOOGLE_SHEET(HttpStatus.INTERNAL_SERVER_ERROR, "구글 스프레드시트 동기화에 실패했습니다."), |
Comment on lines
+82
to
+85
| long memberCount = clubMemberRepository.countByClubId(clubId); | ||
| sheetSyncExecutor.executeWithSort(clubId, sortKey, ascending); | ||
|
|
||
| return ClubMemberSheetSyncResponse.of((int)memberCount, spreadsheetId); |
Comment on lines
+3
to
+14
| import io.swagger.v3.oas.annotations.media.Schema; | ||
| import jakarta.validation.constraints.NotBlank; | ||
|
|
||
| public record ClubMemberSheetSyncRequest( | ||
| @NotBlank(message = "스프레드시트 ID는 필수 입력입니다.") | ||
| @Schema( | ||
| description = "동기화 대상 구글 스프레드시트 ID (URL의 /d/{spreadsheetId}/ 부분)", | ||
| example = "1BxiMVs0XRA5nFMdKvBdBZjgmUUqptlbs74OgVE2upms" | ||
| ) | ||
| String spreadsheetId | ||
| ) { | ||
| } |
Comment on lines
+25
to
+45
| @Override | ||
| public ResponseEntity<Void> updateSheetId( | ||
| @PathVariable(name = "clubId") Integer clubId, | ||
| @Valid @RequestBody ClubSheetIdUpdateRequest request, | ||
| @UserId Integer requesterId | ||
| ) { | ||
| clubMemberSheetService.updateSheetId(clubId, requesterId, request); | ||
| return ResponseEntity.ok().build(); | ||
| } | ||
|
|
||
| @Override | ||
| public ResponseEntity<ClubMemberSheetSyncResponse> syncMembersToSheet( | ||
| @PathVariable(name = "clubId") Integer clubId, | ||
| @RequestParam(name = "sortKey", defaultValue = "POSITION") ClubSheetSortKey sortKey, | ||
| @RequestParam(name = "ascending", defaultValue = "true") boolean ascending, | ||
| @UserId Integer requesterId | ||
| ) { | ||
| ClubMemberSheetSyncResponse response = | ||
| clubMemberSheetService.syncMembersToSheet(clubId, requesterId, sortKey, ascending); | ||
| return ResponseEntity.ok(response); | ||
| } |
Comment on lines
+24
to
+62
| @Override | ||
| public ResponseEntity<ClubFeePaymentResponse> submitFeePayment( | ||
| @PathVariable(name = "clubId") Integer clubId, | ||
| @RequestBody ClubFeePaymentSubmitRequest request, | ||
| @UserId Integer requesterId | ||
| ) { | ||
| ClubFeePaymentResponse response = clubFeePaymentService.submitFeePayment( | ||
| clubId, requesterId, request.paymentImageUrl() | ||
| ); | ||
| return ResponseEntity.ok(response); | ||
| } | ||
|
|
||
| @Override | ||
| public ResponseEntity<ClubFeePaymentResponse> approveFeePayment( | ||
| @PathVariable(name = "clubId") Integer clubId, | ||
| @PathVariable(name = "targetUserId") Integer targetUserId, | ||
| @UserId Integer requesterId | ||
| ) { | ||
| ClubFeePaymentResponse response = clubFeePaymentService.approveFeePayment( | ||
| clubId, targetUserId, requesterId | ||
| ); | ||
| return ResponseEntity.ok(response); | ||
| } | ||
|
|
||
| @Override | ||
| public ResponseEntity<List<ClubFeePaymentResponse>> getFeePayments( | ||
| @PathVariable(name = "clubId") Integer clubId, | ||
| @UserId Integer requesterId | ||
| ) { | ||
| return ResponseEntity.ok(clubFeePaymentService.getFeePayments(clubId, requesterId)); | ||
| } | ||
|
|
||
| @Override | ||
| public ResponseEntity<ClubFeePaymentResponse> getMyFeePayment( | ||
| @PathVariable(name = "clubId") Integer clubId, | ||
| @UserId Integer requesterId | ||
| ) { | ||
| return ResponseEntity.ok(clubFeePaymentService.getMyFeePayment(clubId, requesterId)); | ||
| } |
Comment on lines
+143
to
+151
| JsonNode node = objectMapper.readTree(cleaned); | ||
| Map<String, Integer> mapping = new HashMap<>(); | ||
|
|
||
| node.fields().forEachRemaining(entry -> { | ||
| int colIndex = entry.getValue().asInt(-1); | ||
| if (colIndex >= 0 && colIndex < headerCount) { | ||
| mapping.put(entry.getKey(), colIndex); | ||
| } | ||
| }); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/gg/agit/konect/domain/club/controller/ClubFeePaymentApi.java`:
- Around line 24-28: Add Bean Validation to the submitFeePayment endpoint by
annotating the request parameter with `@Valid` so ClubFeePaymentSubmitRequest
constraints are enforced; update the method signature in ClubFeePaymentApi for
submitFeePayment to use `@Valid` on the `@RequestBody` parameter (mirroring the
usage in ClubMemberSheetApi) to ensure validation is performed before
processing.
In
`@src/main/java/gg/agit/konect/domain/club/dto/ClubFeePaymentSubmitRequest.java`:
- Around line 5-7: The DTO ClubFeePaymentSubmitRequest allows empty or invalid
paymentImageUrl values; add request-level validation by annotating the
paymentImageUrl component with `@NotBlank` and a URL-format validator (e.g.,
Hibernate Validator's `@URL` or a `@Pattern` regex / custom Constraint) so
invalid/empty URLs are rejected at binding; update imports and, if using a
custom constraint, implement/attach the validator and ensure controller
endpoints validate the DTO (e.g., `@Valid`) so invalid submissions are blocked
early.
In `@src/main/java/gg/agit/konect/domain/club/dto/ClubSheetIdUpdateRequest.java`:
- Around line 7-12: The spreadsheetId field in ClubSheetIdUpdateRequest only
uses `@NotBlank` so full URLs or illegal characters can pass; add a format
validation such as `@Pattern`(regexp = "^[A-Za-z0-9_-]+$") to the String
spreadsheetId declaration (keep the existing `@NotBlank` and `@Schema`) so requests
with slashes or other disallowed chars are rejected at validation time and will
return 400 instead of causing Google API errors during sync.
In `@src/main/java/gg/agit/konect/domain/club/model/ClubFeePayment.java`:
- Around line 70-74: The approve method in ClubFeePayment currently overwrites
approval metadata even if isPaid is already true; update
ClubFeePayment.approve(User approver) to guard against duplicate approvals by
checking the isPaid flag first and either throwing a domain-specific exception
(e.g., IllegalStateException or a custom AlreadyApprovedException) or
returning/ignoring the call, so that approvedAt and approvedBy are not modified
when isPaid is true; ensure the guard is the first statement in approve and
reference the isPaid, approvedAt, and approvedBy fields when applying the fix.
In `@src/main/java/gg/agit/konect/domain/club/service/ClubFeePaymentService.java`:
- Around line 33-51: The submitFeePayment method in ClubFeePaymentService does
not verify that the user is a member of the club; add a membership check using
clubMemberRepository.existsByClubIdAndUserId(clubId, userId) at the start of
submitFeePayment and throw an appropriate CustomException (e.g.,
CustomException.of(gg.agit.konect.global.code.ApiResponseCode.NOT_CLUB_MEMBER)
or similar) when the check returns false, before any duplication or payment
creation logic runs.
In
`@src/main/java/gg/agit/konect/domain/club/service/ClubMemberManagementService.java`:
- Line 279: Multiple member-modifying paths currently only publish
ClubMemberChangedEvent in removeMember, causing other flows (addDirectMember,
changeMemberPosition, transferPresident, changeVicePresident) to skip sync;
update ClubMemberManagementService to centralize the post-change event emission
by extracting a private helper (e.g., publishMemberChangedEvent or
publishClubMemberChanged) that calls
applicationEventPublisher.publishEvent(ClubMemberChangedEvent.of(clubId)) and
invoke this helper from removeMember, addDirectMember, changeMemberPosition,
transferPresident and changeVicePresident (or call it from a common
post-processing point) so all member state changes consistently emit the event.
In
`@src/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.java`:
- Around line 50-66: The updateSheetId method currently performs an external
HTTP call via sheetHeaderMapper.analyzeHeaders(...) inside a `@Transactional`
method which risks holding DB connections; move the external call outside the
transaction: first call
sheetHeaderMapper.analyzeHeaders(request.spreadsheetId()) and serialize mapping
(objectMapper.writeValueAsString(...)) before starting the transaction, then
start the transaction to load Club (clubRepository.getById), validate with
clubPermissionValidator.validateManagerAccess, call
club.updateGoogleSheetId(...) and club.updateSheetColumnMapping(...) using the
already-prepared mapping string; ensure JsonProcessingException is handled
around the pre-transaction serialization so the transactional block only
contains DB operations (or alternatively split logic into a non-transactional
helper that does the HTTP call and a small `@Transactional` method that persists
the results).
In `@src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java`:
- Around line 30-31: MAPPING_MODEL is hardcoded to "claude-haiku-4-5-20251001"
while ClaudeProperties already exposes a model setting; either replace the
hardcoded MAPPING_MODEL with claudeProperties.model() wherever MAPPING_MODEL is
used (ensure the field or method using it can accept a runtime value) or, if a
different fixed model is intentional, add a clear comment above the
MAPPING_MODEL declaration explaining why it differs from
ClaudeProperties.model(); reference the MAPPING_MODEL constant and the
ClaudeProperties/claudeProperties.model() accessor to locate and update usages
such as in SheetHeaderMapper.
- Around line 108-131: callClaude() is invoked synchronously inside the
`@Transactional` ClubMemberSheetService.updateSheetId which can hold DB
connections if the external Claude call blocks; either configure timeouts on the
RestClient used by callClaude (set connect and read timeouts in the RestClient
bean and make them configurable, then use that client in
SheetHeaderMapper.callClaude) or move the external call out of the transaction
(invoke sheetHeaderMapper.callClaude from a non-transactional layer or split
updateSheetId into a transactional save-only method and a separate
non-transactional method that performs the Claude call and then passes results
into the transactional save). Ensure timeouts (e.g., connect and read) are
applied and surfaced via configuration and/or annotate or refactor to guarantee
the Claude call never executes while a DB transaction/connection is open.
In `@src/main/java/gg/agit/konect/domain/club/service/SheetSyncDebouncer.java`:
- Around line 28-41: The debounce(Integer clubId) method has a race between
pendingTasks.get → cancel → schedule → put causing duplicate or lost tasks under
concurrent calls; replace that sequence with an atomic update using
ConcurrentHashMap.compute (or computeIfPresent/compute) on pendingTasks to
cancel an existing ScheduledFuture and install the new ScheduledFuture in one
atomic lambda, using scheduler.schedule(...) for the new future and keeping the
same behavior of removing the entry and calling
sheetSyncExecutor.execute(clubId) inside the scheduled Runnable; ensure you
reference DEBOUNCE_DELAY_SECONDS, scheduler, sheetSyncExecutor and pendingTasks
within the compute lambda so the get/cancel/put are performed atomically.
In `@src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java`:
- Around line 89-96: The success message "Sheet sync done" is logged even when
an IOException occurs; in SheetSyncExecutor (the method with the try/catch
around the spreadsheet sync logic) either perform an early return inside the
catch or move the log.info("Sheet sync done. clubId={}, members={}", ...) into
the try block where the sync actually succeeds; update the catch for IOException
to return after logging the error (or rethrow) so the success log is not emitted
on failure and reference the existing log.error and log.info calls to make the
change.
- Around line 58-66: The current public executeWithSort method in
SheetSyncExecutor is annotated with `@Async` and `@Transactional` but is being
called directly from execute, bypassing Spring AOP proxies; either make
executeWithSort private (move logic into a private method used by the annotated
execute) or convert calls to go through the Spring proxy (self-inject
SheetSyncExecutor and call injected.executeWithSort) so `@Async/`@Transactional
are applied consistently; update the class to choose one approach (prefer making
executeWithSort private if it's only used internally, otherwise add a private
final SheetSyncExecutor self and use self.executeWithSort) and ensure only the
proxied public method is annotated.
In `@src/main/java/gg/agit/konect/global/config/AsyncConfig.java`:
- Around line 16-25: The ThreadPoolTaskExecutor bean sheetSyncExecutor lacks a
rejection policy and shutdown wait handling; update the sheetSyncExecutor method
to call executor.setRejectedExecutionHandler(...) (e.g., new
ThreadPoolExecutor.CallerRunsPolicy() or another appropriate
RejectedExecutionHandler) and executor.setWaitForTasksToCompleteOnShutdown(true)
(and optionally setAwaitTerminationSeconds) so submitted tasks are handled when
the queue is full and in-flight tasks are allowed to finish on application
shutdown.
In
`@src/main/java/gg/agit/konect/infrastructure/googlesheets/GoogleSheetsConfig.java`:
- Around line 28-39: The googleSheetsService method opens a FileInputStream via
new FileInputStream(googleSheetsProperties.credentialsPath()) and passes it to
GoogleCredentials.fromStream but never closes it, causing a resource leak;
change googleSheetsService to use a try-with-resources (try (InputStream in =
new FileInputStream(googleSheetsProperties.credentialsPath())) { ... }) so the
InputStream is automatically closed after creating
GoogleCredentials.fromStream(...) and building the Sheets client (keep the rest
of the builder logic intact).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ce6e9c07-bc61-4c4b-adf3-faf8430269c8
📒 Files selected for processing (32)
build.gradlesrc/main/java/gg/agit/konect/domain/club/controller/ClubFeePaymentApi.javasrc/main/java/gg/agit/konect/domain/club/controller/ClubFeePaymentController.javasrc/main/java/gg/agit/konect/domain/club/controller/ClubMemberSheetApi.javasrc/main/java/gg/agit/konect/domain/club/controller/ClubMemberSheetController.javasrc/main/java/gg/agit/konect/domain/club/dto/ClubFeePaymentResponse.javasrc/main/java/gg/agit/konect/domain/club/dto/ClubFeePaymentSubmitRequest.javasrc/main/java/gg/agit/konect/domain/club/dto/ClubMemberSheetSyncRequest.javasrc/main/java/gg/agit/konect/domain/club/dto/ClubMemberSheetSyncResponse.javasrc/main/java/gg/agit/konect/domain/club/dto/ClubSheetIdUpdateRequest.javasrc/main/java/gg/agit/konect/domain/club/enums/ClubSheetSortKey.javasrc/main/java/gg/agit/konect/domain/club/event/ClubFeePaymentApprovedEvent.javasrc/main/java/gg/agit/konect/domain/club/event/ClubMemberChangedEvent.javasrc/main/java/gg/agit/konect/domain/club/model/Club.javasrc/main/java/gg/agit/konect/domain/club/model/ClubFeePayment.javasrc/main/java/gg/agit/konect/domain/club/model/SheetColumnMapping.javasrc/main/java/gg/agit/konect/domain/club/repository/ClubFeePaymentRepository.javasrc/main/java/gg/agit/konect/domain/club/service/ClubApplicationService.javasrc/main/java/gg/agit/konect/domain/club/service/ClubFeePaymentService.javasrc/main/java/gg/agit/konect/domain/club/service/ClubMemberManagementService.javasrc/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.javasrc/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.javasrc/main/java/gg/agit/konect/domain/club/service/SheetSyncDebouncer.javasrc/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.javasrc/main/java/gg/agit/konect/global/code/ApiResponseCode.javasrc/main/java/gg/agit/konect/global/config/AsyncConfig.javasrc/main/java/gg/agit/konect/infrastructure/googlesheets/GoogleSheetsConfig.javasrc/main/java/gg/agit/konect/infrastructure/googlesheets/GoogleSheetsProperties.javasrc/main/resources/application-infrastructure.ymlsrc/main/resources/db/migration/V50__add_google_sheet_id_to_club.sqlsrc/main/resources/db/migration/V51__add_club_fee_payment_table.sqlsrc/main/resources/db/migration/V52__add_sheet_column_mapping_to_club.sql
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
src/main/resources/db/migration/**/*.sql
⚙️ CodeRabbit configuration file
src/main/resources/db/migration/**/*.sql: Flyway 마이그레이션 리뷰 규칙:
- 버전 파일명 규칙(V{number}__{description}.sql) 위반 여부를 우선 확인한다.
- 이미 배포된 마이그레이션 수정/재번호 부여 위험이 있으면 반드시 차단 코멘트를 남긴다.
- 파괴적 변경(drop, rename 등)은 롤백 가능성과 운영 영향 관점에서 검토한다.
Files:
src/main/resources/db/migration/V52__add_sheet_column_mapping_to_club.sqlsrc/main/resources/db/migration/V51__add_club_fee_payment_table.sqlsrc/main/resources/db/migration/V50__add_google_sheet_id_to_club.sql
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/resources/db/migration/V52__add_sheet_column_mapping_to_club.sqlsrc/main/java/gg/agit/konect/domain/club/dto/ClubFeePaymentSubmitRequest.javasrc/main/java/gg/agit/konect/domain/club/repository/ClubFeePaymentRepository.javasrc/main/resources/db/migration/V51__add_club_fee_payment_table.sqlsrc/main/java/gg/agit/konect/domain/club/controller/ClubMemberSheetController.javasrc/main/java/gg/agit/konect/infrastructure/googlesheets/GoogleSheetsConfig.javasrc/main/java/gg/agit/konect/domain/club/controller/ClubFeePaymentApi.javasrc/main/java/gg/agit/konect/domain/club/controller/ClubMemberSheetApi.javasrc/main/java/gg/agit/konect/domain/club/model/ClubFeePayment.javasrc/main/java/gg/agit/konect/global/config/AsyncConfig.javasrc/main/java/gg/agit/konect/global/code/ApiResponseCode.javasrc/main/java/gg/agit/konect/domain/club/service/ClubApplicationService.javasrc/main/resources/application-infrastructure.ymlsrc/main/java/gg/agit/konect/domain/club/model/Club.javasrc/main/java/gg/agit/konect/domain/club/enums/ClubSheetSortKey.javasrc/main/resources/db/migration/V50__add_google_sheet_id_to_club.sqlsrc/main/java/gg/agit/konect/domain/club/dto/ClubMemberSheetSyncResponse.javasrc/main/java/gg/agit/konect/infrastructure/googlesheets/GoogleSheetsProperties.javasrc/main/java/gg/agit/konect/domain/club/service/ClubFeePaymentService.javasrc/main/java/gg/agit/konect/domain/club/dto/ClubFeePaymentResponse.javasrc/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.javasrc/main/java/gg/agit/konect/domain/club/dto/ClubMemberSheetSyncRequest.javasrc/main/java/gg/agit/konect/domain/club/service/SheetSyncDebouncer.javasrc/main/java/gg/agit/konect/domain/club/event/ClubFeePaymentApprovedEvent.javasrc/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.javasrc/main/java/gg/agit/konect/domain/club/event/ClubMemberChangedEvent.javasrc/main/java/gg/agit/konect/domain/club/model/SheetColumnMapping.javasrc/main/java/gg/agit/konect/domain/club/controller/ClubFeePaymentController.javasrc/main/java/gg/agit/konect/domain/club/service/ClubMemberManagementService.javabuild.gradlesrc/main/java/gg/agit/konect/domain/club/dto/ClubSheetIdUpdateRequest.javasrc/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
src/main/java/**/*.java: 아래 원칙으로 리뷰 코멘트를 작성한다.
- 코멘트는 반드시 한국어로 작성한다.
- 반드시 수정이 필요한 항목만 코멘트로 남기고, 단순 취향 차이는 지적하지 않는다.
- 각 코멘트 첫 줄에 심각도를
[LEVEL: high|medium|low]형식으로 반드시 표기한다.- 심각도 기준: high=운영 장애 가능, medium=품질 저하, low=개선 권고.
- 각 코멘트는 "문제 -> 영향 -> 제안" 순서로 3문장 이내로 간결하게 작성한다.
- 가능하면 재현 조건 및 실패 시나리오도 포함한다.
- 제안은 현재 코드베이스(Spring Boot + JPA + Flyway) 패턴과 일치해야 한다.
- 보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.
- 가독성: 변수/메서드 이름이 의도를 바로 드러내는지, 중첩과 메서드 길이가 과도하지 않은지 점검한다.
- 단순화: 불필요한 추상화, 중복 로직, 과한 방어 코드가 있으면 더 단순한 대안을 제시한다.
- 확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다.
Files:
src/main/java/gg/agit/konect/domain/club/dto/ClubFeePaymentSubmitRequest.javasrc/main/java/gg/agit/konect/domain/club/repository/ClubFeePaymentRepository.javasrc/main/java/gg/agit/konect/domain/club/controller/ClubMemberSheetController.javasrc/main/java/gg/agit/konect/infrastructure/googlesheets/GoogleSheetsConfig.javasrc/main/java/gg/agit/konect/domain/club/controller/ClubFeePaymentApi.javasrc/main/java/gg/agit/konect/domain/club/controller/ClubMemberSheetApi.javasrc/main/java/gg/agit/konect/domain/club/model/ClubFeePayment.javasrc/main/java/gg/agit/konect/global/config/AsyncConfig.javasrc/main/java/gg/agit/konect/global/code/ApiResponseCode.javasrc/main/java/gg/agit/konect/domain/club/service/ClubApplicationService.javasrc/main/java/gg/agit/konect/domain/club/model/Club.javasrc/main/java/gg/agit/konect/domain/club/enums/ClubSheetSortKey.javasrc/main/java/gg/agit/konect/domain/club/dto/ClubMemberSheetSyncResponse.javasrc/main/java/gg/agit/konect/infrastructure/googlesheets/GoogleSheetsProperties.javasrc/main/java/gg/agit/konect/domain/club/service/ClubFeePaymentService.javasrc/main/java/gg/agit/konect/domain/club/dto/ClubFeePaymentResponse.javasrc/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.javasrc/main/java/gg/agit/konect/domain/club/dto/ClubMemberSheetSyncRequest.javasrc/main/java/gg/agit/konect/domain/club/service/SheetSyncDebouncer.javasrc/main/java/gg/agit/konect/domain/club/event/ClubFeePaymentApprovedEvent.javasrc/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.javasrc/main/java/gg/agit/konect/domain/club/event/ClubMemberChangedEvent.javasrc/main/java/gg/agit/konect/domain/club/model/SheetColumnMapping.javasrc/main/java/gg/agit/konect/domain/club/controller/ClubFeePaymentController.javasrc/main/java/gg/agit/konect/domain/club/service/ClubMemberManagementService.javasrc/main/java/gg/agit/konect/domain/club/dto/ClubSheetIdUpdateRequest.javasrc/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java
🔇 Additional comments (13)
build.gradle (1)
78-80: [LEVEL: minor]Google Sheets API 버전은 Maven Central에 존재합니다
v4-rev20251110-2.0.0버전은 Maven Central에서 정상적으로 접근 가능합니다. 다만, 공식 최신 버전은v4-rev20250603-2.0.0이므로 필요한 경우 버전 업데이트를 고려하세요.> Likely an incorrect or invalid review comment.src/main/java/gg/agit/konect/domain/club/event/ClubFeePaymentApprovedEvent.java (1)
1-9: LGTM!간단한 이벤트 레코드로, 불변성과 팩토리 메서드 패턴이 적절하게 적용되었습니다.
src/main/java/gg/agit/konect/domain/club/dto/ClubFeePaymentResponse.java (1)
1-37: LGTM!DTO 매핑 로직이 적절하며,
ClubFeePaymentRepository에서 이미JOIN FETCH fp.user를 사용하고 있어 N+1 문제가 방지됩니다.src/main/java/gg/agit/konect/domain/club/dto/ClubMemberSheetSyncRequest.java (1)
1-14: LGTM!유효성 검증과 스웨거 문서화가 적절하게 적용되었습니다.
src/main/java/gg/agit/konect/domain/club/model/Club.java (2)
87-92: LGTM!새 필드들이 기존 엔티티 패턴과 일관되게 추가되었습니다.
234-240: LGTM!간단한 업데이트 메서드로, 기존 코드베이스의 패턴을 따르고 있습니다.
src/main/java/gg/agit/konect/domain/club/repository/ClubFeePaymentRepository.java (1)
1-42: LGTM!
JOIN FETCH를 통한 N+1 방지와 일관된 예외 처리 패턴이 적절히 적용되었습니다.src/main/java/gg/agit/konect/domain/club/controller/ClubMemberSheetController.java (1)
1-46: LGTM!컨트롤러가 API 인터페이스를 구현하고 서비스에 위임하는 패턴을 잘 따르고 있습니다.
src/main/java/gg/agit/konect/domain/club/controller/ClubMemberSheetApi.java (1)
21-39: LGTM!API 인터페이스 설계가 적절합니다.
@Valid어노테이션 사용, 기본 정렬 파라미터 설정, Swagger 문서화가 잘 되어 있습니다.src/main/java/gg/agit/konect/domain/club/model/SheetColumnMapping.java (1)
6-56: LGTM!방어적 복사(defensive copy)를 생성자와
toMap()에서 적절히 수행하고 있으며, 필드 상수와 기본 매핑 로직이 명확하게 분리되어 있습니다.src/main/java/gg/agit/konect/domain/club/controller/ClubFeePaymentController.java (1)
17-63: LGTM!컨트롤러가 서비스에 위임만 수행하는 thin controller 패턴을 잘 따르고 있습니다. 인증은 인터셉터 체인에서 처리됩니다.
src/main/java/gg/agit/konect/domain/club/service/ClubFeePaymentService.java (1)
77-84: LGTM!
getFeePayments메서드의 권한 검증 및 데이터 조회 로직이 적절합니다.clubRepository.getById()는 클럽 존재 여부 검증 역할을 수행합니다.src/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.java (1)
40-48: LGTM!
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)사용이 적절합니다. 트랜잭션 커밋 후에만 시트 동기화가 트리거되어 데이터 일관성이 보장됩니다.
Comment on lines
+24
to
+28
| ResponseEntity<ClubFeePaymentResponse> submitFeePayment( | ||
| @PathVariable(name = "clubId") Integer clubId, | ||
| @RequestBody ClubFeePaymentSubmitRequest request, | ||
| @UserId Integer requesterId | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
[LEVEL: medium] @Valid 어노테이션 누락
ClubFeePaymentSubmitRequest에 Bean Validation 제약조건이 있을 경우 검증이 수행되지 않습니다. 동일 PR의 ClubMemberSheetApi에서는 @Valid를 사용하고 있어 일관성이 부족합니다. @Valid @RequestBody``로 수정을 권장합니다.
수정 제안
ResponseEntity<ClubFeePaymentResponse> submitFeePayment(
`@PathVariable`(name = "clubId") Integer clubId,
- `@RequestBody` ClubFeePaymentSubmitRequest request,
+ `@Valid` `@RequestBody` ClubFeePaymentSubmitRequest request,
`@UserId` Integer requesterId
);📝 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.
Suggested change
| ResponseEntity<ClubFeePaymentResponse> submitFeePayment( | |
| @PathVariable(name = "clubId") Integer clubId, | |
| @RequestBody ClubFeePaymentSubmitRequest request, | |
| @UserId Integer requesterId | |
| ); | |
| ResponseEntity<ClubFeePaymentResponse> submitFeePayment( | |
| `@PathVariable`(name = "clubId") Integer clubId, | |
| `@Valid` `@RequestBody` ClubFeePaymentSubmitRequest request, | |
| `@UserId` Integer requesterId | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/gg/agit/konect/domain/club/controller/ClubFeePaymentApi.java`
around lines 24 - 28, Add Bean Validation to the submitFeePayment endpoint by
annotating the request parameter with `@Valid` so ClubFeePaymentSubmitRequest
constraints are enforced; update the method signature in ClubFeePaymentApi for
submitFeePayment to use `@Valid` on the `@RequestBody` parameter (mirroring the
usage in ClubMemberSheetApi) to ensure validation is performed before
processing.
Comment on lines
+5
to
+7
| public record ClubFeePaymentSubmitRequest( | ||
| @Schema(description = "Payment image URL", example = "https://cdn.konect.com/fee/abc.jpg") | ||
| String paymentImageUrl |
There was a problem hiding this comment.
[LEVEL: medium] paymentImageUrl에 필수/형식 검증이 없어 빈 문자열이나 비정상 값이 그대로 처리됩니다.
이 경우 제출 API는 통과하지만 이후 승인/조회 흐름에서 증빙 누락 데이터가 누적되어 운영 품질이 떨어집니다.
@NotBlank와 URL 형식 검증(예: @Pattern 또는 커스텀 validator)을 DTO에 추가해 요청 단계에서 차단해 주세요; As per coding guidelines src/main/java/**/*.java: "보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/gg/agit/konect/domain/club/dto/ClubFeePaymentSubmitRequest.java`
around lines 5 - 7, The DTO ClubFeePaymentSubmitRequest allows empty or invalid
paymentImageUrl values; add request-level validation by annotating the
paymentImageUrl component with `@NotBlank` and a URL-format validator (e.g.,
Hibernate Validator's `@URL` or a `@Pattern` regex / custom Constraint) so
invalid/empty URLs are rejected at binding; update imports and, if using a
custom constraint, implement/attach the validator and ensure controller
endpoints validate the DTO (e.g., `@Valid`) so invalid submissions are blocked
early.
Comment on lines
+7
to
+12
| @NotBlank(message = "스프레드시트 ID는 필수 입력입니다.") | ||
| @Schema( | ||
| description = "등록할 구글 스프레드시트 ID (URL의 /d/{spreadsheetId}/ 부분)", | ||
| example = "1BxiMVs0XRA5nFMdKvBdBZjgmUUqptlbs74OgVE2upms" | ||
| ) | ||
| String spreadsheetId |
There was a problem hiding this comment.
[LEVEL: medium] spreadsheetId가 @NotBlank만 적용되어 URL 전체 문자열이나 허용되지 않은 문자를 포함한 값도 통과됩니다.
이 값이 저장되면 등록 API는 성공해도 실제 동기화 시점에 Google API 호출 실패로 500이 발생할 수 있습니다(예: /d/.../edit 전체 URL 입력).
@Pattern(regexp = "^[A-Za-z0-9_-]+$") 같은 형식 검증을 추가해 요청 단계에서 400으로 차단해 주세요; As per coding guidelines src/main/java/**/*.java: "보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/gg/agit/konect/domain/club/dto/ClubSheetIdUpdateRequest.java`
around lines 7 - 12, The spreadsheetId field in ClubSheetIdUpdateRequest only
uses `@NotBlank` so full URLs or illegal characters can pass; add a format
validation such as `@Pattern`(regexp = "^[A-Za-z0-9_-]+$") to the String
spreadsheetId declaration (keep the existing `@NotBlank` and `@Schema`) so requests
with slashes or other disallowed chars are rejected at validation time and will
return 400 instead of causing Google API errors during sync.
Comment on lines
+70
to
+74
| public void approve(User approver) { | ||
| this.isPaid = true; | ||
| this.approvedAt = LocalDateTime.now(); | ||
| this.approvedBy = approver; | ||
| } |
There was a problem hiding this comment.
[LEVEL: medium] 이미 승인된 결제를 다시 승인할 경우 approvedAt과 approvedBy가 덮어쓰여질 수 있습니다.
중복 승인 시 기존 승인 정보가 유실되어 감사 추적이 어려워집니다. isPaid가 이미 true인 경우 예외를 던지거나 무시하는 방어 로직을 추가하세요.
제안
public void approve(User approver) {
+ if (this.isPaid) {
+ return; // 또는 throw new IllegalStateException("이미 승인된 결제입니다.")
+ }
this.isPaid = true;
this.approvedAt = LocalDateTime.now();
this.approvedBy = approver;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/gg/agit/konect/domain/club/model/ClubFeePayment.java` around
lines 70 - 74, The approve method in ClubFeePayment currently overwrites
approval metadata even if isPaid is already true; update
ClubFeePayment.approve(User approver) to guard against duplicate approvals by
checking the isPaid flag first and either throwing a domain-specific exception
(e.g., IllegalStateException or a custom AlreadyApprovedException) or
returning/ignoring the call, so that approvedAt and approvedBy are not modified
when isPaid is true; ensure the guard is the first statement in approve and
reference the isPaid, approvedAt, and approvedBy fields when applying the fix.
Comment on lines
+33
to
+51
| @Transactional | ||
| public ClubFeePaymentResponse submitFeePayment( | ||
| Integer clubId, | ||
| Integer userId, | ||
| String paymentImageUrl | ||
| ) { | ||
| Club club = clubRepository.getById(clubId); | ||
| User user = userRepository.getById(userId); | ||
|
|
||
| clubFeePaymentRepository.findByClubIdAndUserId(clubId, userId) | ||
| .ifPresent(p -> { | ||
| throw CustomException.of( | ||
| gg.agit.konect.global.code.ApiResponseCode.ALREADY_FEE_PAYMENT_SUBMITTED | ||
| ); | ||
| }); | ||
|
|
||
| ClubFeePayment payment = ClubFeePayment.of(club, user, paymentImageUrl); | ||
| return ClubFeePaymentResponse.from(clubFeePaymentRepository.save(payment)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 다른 클럽 관련 서비스에서 멤버십 검증 패턴 확인
rg -n "existsByClubIdAndUserId|validateMember" --type=java -C2Repository: BCSDLab/KONECT_BACK_END
Length of output: 6863
🏁 Script executed:
#!/bin/bash
# Get full ClubFeePaymentService to understand broader context
cat -n src/main/java/gg/agit/konect/domain/club/service/ClubFeePaymentService.java | head -100Repository: BCSDLab/KONECT_BACK_END
Length of output: 4229
🏁 Script executed:
#!/bin/bash
# Search for ClubFeePayment entity to understand its design
find . -name "ClubFeePayment.java" -type f | grep -v testRepository: BCSDLab/KONECT_BACK_END
Length of output: 135
🏁 Script executed:
#!/bin/bash
# Search for tests related to ClubFeePaymentService
find . -name "*ClubFeePayment*Test*.java" -type fRepository: BCSDLab/KONECT_BACK_END
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check ApiResponseCode for NOT_FOUND_CLUB_MEMBER or similar
rg "NOT_FOUND_CLUB_MEMBER|ALREADY_FEE_PAYMENT_SUBMITTED" --type=javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 1638
🏁 Script executed:
#!/bin/bash
# Search for who calls submitFeePayment to understand if it's user-initiated
rg "submitFeePayment" --type=java -B2 -A2Repository: BCSDLab/KONECT_BACK_END
Length of output: 2429
🏁 Script executed:
#!/bin/bash
# Search for ClubFeePayment tests and usage patterns
find . -name "*ClubFeePayment*" -type f | head -20Repository: BCSDLab/KONECT_BACK_END
Length of output: 695
🏁 Script executed:
#!/bin/bash
# Check ClubFeePayment entity to understand the relationship
cat src/main/java/gg/agit/konect/domain/club/model/ClubFeePayment.javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 2201
🏁 Script executed:
#!/bin/bash
# Look for any documentation or API specs about fee payment submission
rg "Submit club fee payment|회비 납부" --type=java -B3 -A3Repository: BCSDLab/KONECT_BACK_END
Length of output: 7877
🏁 Script executed:
#!/bin/bash
# Check if there are integration tests for fee payment
find . -path "*/test/*" -name "*FeePayment*" -type fRepository: BCSDLab/KONECT_BACK_END
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Search for club member requirement context in other similar operations
rg "멤버|member" src/main/java/gg/agit/konect/domain/club/controller/ClubFeePaymentApi.java src/main/java/gg/agit/konect/domain/club/controller/ClubFeePaymentController.javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 215
🏁 Script executed:
#!/bin/bash
# Search for ClubFeePayment integration tests
find . -path "*/test/*" -type f -exec grep -l "submitFeePayment\|ClubFeePayment" {} \;Repository: BCSDLab/KONECT_BACK_END
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check the ClubFeePaymentController to understand the API contract
cat -n src/main/java/gg/agit/konect/domain/club/controller/ClubFeePaymentController.javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 2812
🏁 Script executed:
#!/bin/bash
# Search for any documentation or context about fee payment flow
rg "fee.payment|회비" --type=java -i src/main/java/gg/agit/konect/domain/club/controller/ClubFeePaymentApi.java src/main/java/gg/agit/konect/domain/club/controller/ClubApplicationApi.java -B2 -A2Repository: BCSDLab/KONECT_BACK_END
Length of output: 5464
🏁 Script executed:
#!/bin/bash
# Check if there's a membership check elsewhere in the fee payment flow
rg "ClubMemberRepository" src/main/java/gg/agit/konect/domain/club/service/ClubFeePaymentService.javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Look for existing ClubFeePayment tests
find . -path "*/test/*" -name "*ClubFeePayment*Test*.java" -o -path "*/test/*" -name "*FeePaymentTest*.java"Repository: BCSDLab/KONECT_BACK_END
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Look for all integration or unit tests related to ClubFeePayment
find . -path "*/test/*" -type f -exec grep -l "clubFeePayment\|ClubFeePayment" {} \;Repository: BCSDLab/KONECT_BACK_END
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Get the full ClubFeePaymentApi specification
cat -n src/main/java/gg/agit/konect/domain/club/controller/ClubFeePaymentApi.javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 2454
🏁 Script executed:
#!/bin/bash
# Check if ClubMemberRepository is already injected into ClubFeePaymentService
rg "ClubMemberRepository" src/main/java/gg/agit/konect/domain/club/service/ClubFeePaymentService.javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Look at how similar submit/apply operations handle membership
rg -A10 "submitFeePayment|applyClub" src/main/java/gg/agit/konect/domain/club/service/ -B2 | head -80Repository: BCSDLab/KONECT_BACK_END
Length of output: 2989
🏁 Script executed:
#!/bin/bash
# Check ClubFeePaymentRepository for any existing membership-related queries
cat -n src/main/java/gg/agit/konect/domain/club/repository/ClubFeePaymentRepository.javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 1697
🏁 Script executed:
#!/bin/bash
# Search for any test files that mention fee payment or related operations
find . -path "*/test/*" -type f -name "*.java" | xargs grep -l "fee.payment\|FeePayment" 2>/dev/null | head -10Repository: BCSDLab/KONECT_BACK_END
Length of output: 135
🏁 Script executed:
#!/bin/bash
# Check how the fee payment flows with application - look at ClubApplicationService
cat -n src/main/java/gg/agit/konect/domain/club/service/ClubApplicationService.java | head -100Repository: BCSDLab/KONECT_BACK_END
Length of output: 5435
🏁 Script executed:
#!/bin/bash
# Check if there's any documentation about who can submit fee payments
rg "fee.*payment|회비.*납부" src/main/java/gg/agit/konect/domain/club/ -i --type=java | grep -i "member\|권한\|요구\|require"Repository: BCSDLab/KONECT_BACK_END
Length of output: 1712
🏁 Script executed:
#!/bin/bash
# Look at the full ClubFeePaymentService to see if membership check is elsewhere
wc -l src/main/java/gg/agit/konect/domain/club/service/ClubFeePaymentService.javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 145
🏁 Script executed:
#!/bin/bash
# Final verification: Check if there are any tests that show fee payment submission without membership
find . -path "*/test/*" -type f -name "*.java" -exec grep -l "submitFeePayment" {} \; 2>/dev/nullRepository: BCSDLab/KONECT_BACK_END
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check the complete ClubFeePaymentApi documentation to see if membership is mentioned
cat src/main/java/gg/agit/konect/domain/club/controller/ClubFeePaymentApi.java | grep -A 20 "Submit club fee payment"Repository: BCSDLab/KONECT_BACK_END
Length of output: 1021
[LEVEL: medium] 회비 납부 제출 시 클럽 멤버십 검증 누락
submitFeePayment에서 사용자가 해당 클럽의 멤버인지 검증하지 않습니다. 인증된 사용자라면 누구나 아무 클럽에나 회비 납부를 제출할 수 있어, 의도치 않은 데이터 생성 또는 악용이 가능합니다. ClubMemberRepository.existsByClubIdAndUserId()를 통해 멤버십 확인 로직을 추가하는 것을 권장합니다.
수정 제안
`@Transactional`
public ClubFeePaymentResponse submitFeePayment(
Integer clubId,
Integer userId,
String paymentImageUrl
) {
Club club = clubRepository.getById(clubId);
User user = userRepository.getById(userId);
+ if (!clubMemberRepository.existsByClubIdAndUserId(clubId, userId)) {
+ throw CustomException.of(ApiResponseCode.NOT_FOUND_CLUB_MEMBER);
+ }
+
clubFeePaymentRepository.findByClubIdAndUserId(clubId, userId)
.ifPresent(p -> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/gg/agit/konect/domain/club/service/ClubFeePaymentService.java`
around lines 33 - 51, The submitFeePayment method in ClubFeePaymentService does
not verify that the user is a member of the club; add a membership check using
clubMemberRepository.existsByClubIdAndUserId(clubId, userId) at the start of
submitFeePayment and throw an appropriate CustomException (e.g.,
CustomException.of(gg.agit.konect.global.code.ApiResponseCode.NOT_CLUB_MEMBER)
or similar) when the check returns false, before any duplication or payment
creation logic runs.
Comment on lines
+28
to
+41
| public void debounce(Integer clubId) { | ||
| ScheduledFuture<?> existing = pendingTasks.get(clubId); | ||
| if (existing != null && !existing.isDone()) { | ||
| existing.cancel(false); | ||
| log.debug("Sheet sync debounced. clubId={}", clubId); | ||
| } | ||
|
|
||
| ScheduledFuture<?> future = scheduler.schedule(() -> { | ||
| pendingTasks.remove(clubId); | ||
| sheetSyncExecutor.execute(clubId); | ||
| }, DEBOUNCE_DELAY_SECONDS, TimeUnit.SECONDS); | ||
|
|
||
| pendingTasks.put(clubId, future); | ||
| } |
There was a problem hiding this comment.
[LEVEL: medium] debounce() 메서드에 경쟁 조건(race condition)이 존재합니다.
get → cancel → schedule → put 시퀀스가 원자적이지 않아, 동일 clubId에 대해 동시 호출 시 중복 실행되거나 작업이 유실될 수 있습니다. ConcurrentHashMap.compute()를 사용하여 원자적 업데이트를 보장하세요.
제안
public void debounce(Integer clubId) {
- ScheduledFuture<?> existing = pendingTasks.get(clubId);
- if (existing != null && !existing.isDone()) {
- existing.cancel(false);
- log.debug("Sheet sync debounced. clubId={}", clubId);
- }
-
- ScheduledFuture<?> future = scheduler.schedule(() -> {
- pendingTasks.remove(clubId);
- sheetSyncExecutor.execute(clubId);
- }, DEBOUNCE_DELAY_SECONDS, TimeUnit.SECONDS);
-
- pendingTasks.put(clubId, future);
+ pendingTasks.compute(clubId, (key, existing) -> {
+ if (existing != null && !existing.isDone()) {
+ existing.cancel(false);
+ log.debug("Sheet sync debounced. clubId={}", clubId);
+ }
+ return scheduler.schedule(() -> {
+ pendingTasks.remove(clubId);
+ sheetSyncExecutor.execute(clubId);
+ }, DEBOUNCE_DELAY_SECONDS, TimeUnit.SECONDS);
+ });
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/gg/agit/konect/domain/club/service/SheetSyncDebouncer.java`
around lines 28 - 41, The debounce(Integer clubId) method has a race between
pendingTasks.get → cancel → schedule → put causing duplicate or lost tasks under
concurrent calls; replace that sequence with an atomic update using
ConcurrentHashMap.compute (or computeIfPresent/compute) on pendingTasks to
cancel an existing ScheduledFuture and install the new ScheduledFuture in one
atomic lambda, using scheduler.schedule(...) for the new future and keeping the
same behavior of removing the entry and calling
sheetSyncExecutor.execute(clubId) inside the scheduled Runnable; ensure you
reference DEBOUNCE_DELAY_SECONDS, scheduler, sheetSyncExecutor and pendingTasks
within the compute lambda so the get/cancel/put are performed atomically.
Comment on lines
+58
to
+66
| @Async("sheetSyncExecutor") | ||
| @Transactional(readOnly = true) | ||
| public void execute(Integer clubId) { | ||
| executeWithSort(clubId, ClubSheetSortKey.POSITION, true); | ||
| } | ||
|
|
||
| @Async("sheetSyncExecutor") | ||
| @Transactional(readOnly = true) | ||
| public void executeWithSort(Integer clubId, ClubSheetSortKey sortKey, boolean ascending) { |
There was a problem hiding this comment.
[LEVEL: medium] 내부 호출 시 @Async/@Transactional 미적용
execute()에서 executeWithSort()를 직접 호출하면 Spring AOP 프록시를 거치지 않아 executeWithSort()의 @Async와 @Transactional이 무시됩니다. 현재는 execute() 자체에도 동일한 어노테이션이 있어 동작하지만, 외부에서 executeWithSort()를 직접 호출할 때와 execute() 경유 시 동작이 달라 혼란을 유발할 수 있습니다. 내부 호출용 private 메서드로 분리하거나 self-injection 패턴을 고려하세요.
리팩토링 제안
`@Async`("sheetSyncExecutor")
`@Transactional`(readOnly = true)
public void execute(Integer clubId) {
- executeWithSort(clubId, ClubSheetSortKey.POSITION, true);
+ doExecute(clubId, ClubSheetSortKey.POSITION, true);
}
`@Async`("sheetSyncExecutor")
`@Transactional`(readOnly = true)
public void executeWithSort(Integer clubId, ClubSheetSortKey sortKey, boolean ascending) {
+ doExecute(clubId, sortKey, ascending);
+ }
+
+ private void doExecute(Integer clubId, ClubSheetSortKey sortKey, boolean ascending) {
Club club = clubRepository.getById(clubId);
// ... 나머지 로직
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java`
around lines 58 - 66, The current public executeWithSort method in
SheetSyncExecutor is annotated with `@Async` and `@Transactional` but is being
called directly from execute, bypassing Spring AOP proxies; either make
executeWithSort private (move logic into a private method used by the annotated
execute) or convert calls to go through the Spring proxy (self-inject
SheetSyncExecutor and call injected.executeWithSort) so `@Async/`@Transactional
are applied consistently; update the class to choose one approach (prefer making
executeWithSort private if it's only used internally, otherwise add a private
final SheetSyncExecutor self and use self.executeWithSort) and ensure only the
proxied public method is annotated.
Comment on lines
+16
to
+25
| @Bean(name = "sheetSyncExecutor") | ||
| public Executor sheetSyncExecutor() { | ||
| ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor(); | ||
| executor.setCorePoolSize(SHEET_SYNC_CORE_POOL_SIZE); | ||
| executor.setMaxPoolSize(SHEET_SYNC_MAX_POOL_SIZE); | ||
| executor.setQueueCapacity(SHEET_SYNC_QUEUE_CAPACITY); | ||
| executor.setThreadNamePrefix("sheet-sync-"); | ||
| executor.initialize(); | ||
| return executor; | ||
| } |
There was a problem hiding this comment.
[LEVEL: medium]
ThreadPool 거부 정책 및 종료 처리 미설정
큐(50개)가 가득 찬 상태에서 추가 작업 요청 시 RejectedExecutionException이 발생하며, 애플리케이션 종료 시 진행 중인 동기화 작업이 유실될 수 있습니다. setRejectedExecutionHandler와 setWaitForTasksToCompleteOnShutdown(true)를 추가하세요.
🛠️ 수정 제안
`@Bean`(name = "sheetSyncExecutor")
public Executor sheetSyncExecutor() {
ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor();
executor.setCorePoolSize(SHEET_SYNC_CORE_POOL_SIZE);
executor.setMaxPoolSize(SHEET_SYNC_MAX_POOL_SIZE);
executor.setQueueCapacity(SHEET_SYNC_QUEUE_CAPACITY);
executor.setThreadNamePrefix("sheet-sync-");
+ executor.setWaitForTasksToCompleteOnShutdown(true);
+ executor.setAwaitTerminationSeconds(30);
+ executor.setRejectedExecutionHandler(new ThreadPoolExecutor.CallerRunsPolicy());
executor.initialize();
return executor;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/gg/agit/konect/global/config/AsyncConfig.java` around lines 16
- 25, The ThreadPoolTaskExecutor bean sheetSyncExecutor lacks a rejection policy
and shutdown wait handling; update the sheetSyncExecutor method to call
executor.setRejectedExecutionHandler(...) (e.g., new
ThreadPoolExecutor.CallerRunsPolicy() or another appropriate
RejectedExecutionHandler) and executor.setWaitForTasksToCompleteOnShutdown(true)
(and optionally setAwaitTerminationSeconds) so submitted tasks are handled when
the queue is full and in-flight tasks are allowed to finish on application
shutdown.
Comment on lines
+28
to
+39
| public Sheets googleSheetsService() throws IOException, GeneralSecurityException { | ||
| InputStream in = new FileInputStream(googleSheetsProperties.credentialsPath()); | ||
| GoogleCredentials credentials = GoogleCredentials.fromStream(in) | ||
| .createScoped(Collections.singleton(SheetsScopes.SPREADSHEETS)); | ||
|
|
||
| return new Sheets.Builder( | ||
| GoogleNetHttpTransport.newTrustedTransport(), | ||
| GsonFactory.getDefaultInstance(), | ||
| new HttpCredentialsAdapter(credentials)) | ||
| .setApplicationName(googleSheetsProperties.applicationName()) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
[LEVEL: medium]
FileInputStream 리소스 누수
FileInputStream이 닫히지 않아 파일 핸들이 누수됩니다. 싱글톤 빈이라 영향은 제한적이지만, try-with-resources로 수정하는 것이 올바른 패턴입니다.
🛠️ 수정 제안
`@Bean`
public Sheets googleSheetsService() throws IOException, GeneralSecurityException {
- InputStream in = new FileInputStream(googleSheetsProperties.credentialsPath());
- GoogleCredentials credentials = GoogleCredentials.fromStream(in)
- .createScoped(Collections.singleton(SheetsScopes.SPREADSHEETS));
+ GoogleCredentials credentials;
+ try (InputStream in = new FileInputStream(googleSheetsProperties.credentialsPath())) {
+ credentials = GoogleCredentials.fromStream(in)
+ .createScoped(Collections.singleton(SheetsScopes.SPREADSHEETS));
+ }
return new Sheets.Builder(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/gg/agit/konect/infrastructure/googlesheets/GoogleSheetsConfig.java`
around lines 28 - 39, The googleSheetsService method opens a FileInputStream via
new FileInputStream(googleSheetsProperties.credentialsPath()) and passes it to
GoogleCredentials.fromStream but never closes it, causing a resource leak;
change googleSheetsService to use a try-with-resources (try (InputStream in =
new FileInputStream(googleSheetsProperties.credentialsPath())) { ... }) so the
InputStream is automatically closed after creating
GoogleCredentials.fromStream(...) and building the Sheets client (keep the rest
of the builder logic intact).
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/main/java/gg/agit/konect/global/config/AsyncConfig.java (1)
16-24:⚠️ Potential issue | 🟠 Major[LEVEL: medium]
ThreadPool 거부/종료 정책이 없어 동기화 작업이 유실될 수 있습니다.
현재 설정은 큐가 가득 찰 때 작업이 거부되고, 애플리케이션 종료 시 진행 중 작업을 기다리지 않아 AFTER_COMMIT 기반 시트 동기화가 누락될 수 있습니다.
이 경우 명부/회비 원장 동기화가 간헐적으로 빠져 운영 데이터 신뢰도가 떨어집니다.
As per coding guidelines, '보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.',setRejectedExecutionHandler(...)와setWaitForTasksToCompleteOnShutdown(true)(필요 시setAwaitTerminationSeconds)를 추가하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/global/config/AsyncConfig.java` around lines 16 - 24, The ThreadPoolTaskExecutor bean sheetSyncTaskExecutor is missing a rejection/ shutdown policy causing lost AFTER_COMMIT sync tasks; update the sheetSyncTaskExecutor() method to set a rejection handler (e.g., CallerRunsPolicy or a custom RejectedExecutionHandler) via executor.setRejectedExecutionHandler(...), enable graceful shutdown with executor.setWaitForTasksToCompleteOnShutdown(true), and optionally configure executor.setAwaitTerminationSeconds(...) to bound shutdown wait time so in-flight sheet sync work completes; keep these calls on the ThreadPoolTaskExecutor instance before executor.initialize().src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java (1)
89-96:⚠️ Potential issue | 🟠 Major[LEVEL: medium]
실패 후에도 성공 로그가 출력됩니다.
IOException을 catch한 뒤에도 Line 96 성공 로그가 항상 실행되어 동기화 실패가 성공으로 오인됩니다.
운영 알림/대시보드에서 실패 탐지가 늦어져 장애 대응이 지연될 수 있습니다.
As per coding guidelines, '보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.', 성공 로그를 try 블록 내부로 이동하거나 catch에서 early return 하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java` around lines 89 - 96, Sheet sync currently logs success unconditionally after catching IOException, causing failures to appear as successes; update SheetSyncExecutor so the "Sheet sync done. clubId={}, members={}" log is only emitted on successful completion — either move that log inside the try block (after the sync logic that populates members) or make the catch block perform an early return/throw so execution won't reach the success log; ensure you adjust the try/catch around the method that performs the sync (the method containing the existing catch for IOException and the members variable) so failure paths do not produce the success message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java`:
- Around line 244-248: The current applyFormat in SheetSyncExecutor hardcodes
sheetId=0 for SetBasicFilterRequest/UpdateSheetPropertiesRequest which can apply
formatting to the wrong sheet; instead, call
googleSheetsService.spreadsheets().get(spreadsheetId).execute(), locate the
intended Sheet by its title (or other identifier) and extract
sheet.getProperties().getSheetId(), then set that ID into
GridRange.setSheetId(...) and into the SheetProperties used by
UpdateSheetPropertiesRequest (do not leave 0). Update applyFormat to compute the
correct sheetId dynamically (use the existing spreadsheetId and target sheet
name variable) and use that value for SetBasicFilterRequest and
UpdateSheetPropertiesRequest.
- Around line 115-141: updateMappedColumns currently writes new column values
but never clears leftover rows, causing stale data when member count shrinks;
before calling batchUpdate, call
googleSheetsService.spreadsheets().values().clear() for each target column range
(use the same colLetter and dataStartRow logic you already compute) to clear the
column data area, then perform the BatchUpdateValuesRequest write (keep using
buildColumnData, columnLetter, and the existing batch update). Ensure the clear
calls use the proper ClearValuesRequest/clear() on the same spreadsheetId and
ranges so old rows are removed prior to writing.
---
Duplicate comments:
In `@src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java`:
- Around line 89-96: Sheet sync currently logs success unconditionally after
catching IOException, causing failures to appear as successes; update
SheetSyncExecutor so the "Sheet sync done. clubId={}, members={}" log is only
emitted on successful completion — either move that log inside the try block
(after the sync logic that populates members) or make the catch block perform an
early return/throw so execution won't reach the success log; ensure you adjust
the try/catch around the method that performs the sync (the method containing
the existing catch for IOException and the members variable) so failure paths do
not produce the success message.
In `@src/main/java/gg/agit/konect/global/config/AsyncConfig.java`:
- Around line 16-24: The ThreadPoolTaskExecutor bean sheetSyncTaskExecutor is
missing a rejection/ shutdown policy causing lost AFTER_COMMIT sync tasks;
update the sheetSyncTaskExecutor() method to set a rejection handler (e.g.,
CallerRunsPolicy or a custom RejectedExecutionHandler) via
executor.setRejectedExecutionHandler(...), enable graceful shutdown with
executor.setWaitForTasksToCompleteOnShutdown(true), and optionally configure
executor.setAwaitTerminationSeconds(...) to bound shutdown wait time so
in-flight sheet sync work completes; keep these calls on the
ThreadPoolTaskExecutor instance before executor.initialize().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b3048148-6ba3-441f-aea8-4ea14e6423b8
📒 Files selected for processing (2)
src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.javasrc/main/java/gg/agit/konect/global/config/AsyncConfig.java
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
src/main/java/**/*.java: 아래 원칙으로 리뷰 코멘트를 작성한다.
- 코멘트는 반드시 한국어로 작성한다.
- 반드시 수정이 필요한 항목만 코멘트로 남기고, 단순 취향 차이는 지적하지 않는다.
- 각 코멘트 첫 줄에 심각도를
[LEVEL: high|medium|low]형식으로 반드시 표기한다.- 심각도 기준: high=운영 장애 가능, medium=품질 저하, low=개선 권고.
- 각 코멘트는 "문제 -> 영향 -> 제안" 순서로 3문장 이내로 간결하게 작성한다.
- 가능하면 재현 조건 및 실패 시나리오도 포함한다.
- 제안은 현재 코드베이스(Spring Boot + JPA + Flyway) 패턴과 일치해야 한다.
- 보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.
- 가독성: 변수/메서드 이름이 의도를 바로 드러내는지, 중첩과 메서드 길이가 과도하지 않은지 점검한다.
- 단순화: 불필요한 추상화, 중복 로직, 과한 방어 코드가 있으면 더 단순한 대안을 제시한다.
- 확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다.
Files:
src/main/java/gg/agit/konect/global/config/AsyncConfig.javasrc/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/java/gg/agit/konect/global/config/AsyncConfig.javasrc/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java
Comment on lines
+115
to
+141
| private void updateMappedColumns( | ||
| String spreadsheetId, | ||
| List<ClubMember> members, | ||
| Map<Integer, ClubFeePayment> paymentMap, | ||
| SheetColumnMapping mapping | ||
| ) throws IOException { | ||
| int dataStartRow = 2; | ||
| Map<Integer, List<Object>> columnData = buildColumnData(members, paymentMap, mapping); | ||
|
|
||
| List<ValueRange> data = new ArrayList<>(); | ||
| for (Map.Entry<Integer, List<Object>> entry : columnData.entrySet()) { | ||
| int colIndex = entry.getKey(); | ||
| List<Object> values = entry.getValue(); | ||
| String colLetter = columnLetter(colIndex); | ||
| String range = colLetter + dataStartRow + ":" + colLetter; | ||
| List<List<Object>> wrapped = values.stream().map(v -> List.of((Object)v)).toList(); | ||
| data.add(new ValueRange().setRange(range).setValues(wrapped)); | ||
| } | ||
|
|
||
| if (!data.isEmpty()) { | ||
| googleSheetsService.spreadsheets().values() | ||
| .batchUpdate(spreadsheetId, | ||
| new BatchUpdateValuesRequest() | ||
| .setValueInputOption("USER_ENTERED") | ||
| .setData(data)) | ||
| .execute(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "SheetSyncExecutor.java"Repository: BCSDLab/KONECT_BACK_END
Length of output: 140
🏁 Script executed:
cat -n src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java | head -200Repository: BCSDLab/KONECT_BACK_END
Length of output: 9691
🏁 Script executed:
# Check the full method and surrounding context
cat -n src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java | sed -n '100,160p'Repository: BCSDLab/KONECT_BACK_END
Length of output: 3023
🏁 Script executed:
# Check if there are other batchUpdate usages
rg "batchUpdate" src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java -B 2 -A 5Repository: BCSDLab/KONECT_BACK_END
Length of output: 633
🏁 Script executed:
# See the full clearAndWriteAll method to understand the clearing pattern
cat -n src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java | sed -n '190,230p'Repository: BCSDLab/KONECT_BACK_END
Length of output: 2070
🏁 Script executed:
# Check for how the range is being used elsewhere
rg "values\(\)" src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java -B 2 -A 3Repository: BCSDLab/KONECT_BACK_END
Length of output: 767
🏁 Script executed:
# Check if there are any test files that might show expected behavior
find . -type f -name "*SheetSync*Test*.java" -o -name "*Test*SheetSync*.java" 2>/dev/null | head -5Repository: BCSDLab/KONECT_BACK_END
Length of output: 49
🏁 Script executed:
# Check the full context of how updateMappedColumns is called and when it's used vs clearAndWriteAll
cat -n src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java | sed -n '58,97p'Repository: BCSDLab/KONECT_BACK_END
Length of output: 1964
🏁 Script executed:
# Check the columnLetter method to understand range generation
cat -n src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java | tail -100Repository: BCSDLab/KONECT_BACK_END
Length of output: 4601
🏁 Script executed:
# Verify Google Sheets API behavior by checking documentation and patterns
# Search for any other potential data clearing patterns or safeguards
rg "setData|setValues|clear" src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java -B 1 -A 1Repository: BCSDLab/KONECT_BACK_END
Length of output: 914
🌐 Web query:
Google Sheets API batchUpdate behavior open-ended range partial data write
💡 Result:
-
spreadsheets.values.batchUpdate/values.update(the “Values API”)- If you specify a multi-cell A1 range, the 2D
valuesyou send must fit the range exactly; otherwise the API returns an error. (developers.google.com) - If you want to write “partial” data (unknown height), don’t use an open-ended multi-cell range like
Sheet1!A2:C. Instead, pass a single start cell (e.g.Sheet1!A2) and provide a 2D array; Sheets writes a rectangle sized to your data. (developers.google.com) - Writing fewer cells does not implicitly clear other cells; to clear you must explicitly write blanks or call a clear operation (e.g.
spreadsheets.values.clear). (consultantsinabox.com)
- If you specify a multi-cell A1 range, the 2D
-
spreadsheets.batchUpdatewithupdateCells(the “Grid/CellData API”)- A
GridRangecan be unbounded ifendRowIndex/endColumnIndexare not set. (developers.google.com) - Practically, for “open-ended range + partial data”, the robust pattern is: don’t send a huge/unbounded
range; instead, useUpdateCellsRequest.start(a start coordinate) plusrows, so the write size is defined by the data you provide (and only thefieldsmask you set gets changed). (This avoids any ambiguity about what happens to the rest of an unbounded range.) (developers.google.com)
- A
Citations:
- 1: https://developers.google.com/sheets/api/guides/migration?utm_source=openai
- 2: https://developers.google.com/sheets/api/guides/migration?utm_source=openai
- 3: https://consultantsinabox.com/collections/all/products/google-sheets-clear-values-from-a-range-integration?utm_source=openai
- 4: https://developers.google.com/resources/api-libraries/documentation/sheets/v4/java/latest/com/google/api/services/sheets/v4/model/GridRange.html?utm_source=openai
- 5: https://developers.google.com/workspace/sheets/api/guides/batchupdate?utm_source=openai
[LEVEL: medium] 매핑된 컬럼 동기화 시 기존 데이터가 남을 수 있습니다.
updateMappedColumns는 개별 컬럼을 쓰기만 수행하고 이전 데이터를 명시적으로 비우지 않아, 동아리원 수가 줄어든 경우 이전 행의 데이터가 그대로 남습니다. 예를 들어 10명 동기화 후 5명으로 재동기화하면 6~10행의 탈퇴자 정보가 시트에 잔존해 데이터 정합성이 깨집니다. clearAndWriteAll처럼 배치 업데이트 전에 해당 컬럼의 데이터 영역을 values().clear()로 먼저 비운 후 다시 써주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java`
around lines 115 - 141, updateMappedColumns currently writes new column values
but never clears leftover rows, causing stale data when member count shrinks;
before calling batchUpdate, call
googleSheetsService.spreadsheets().values().clear() for each target column range
(use the same colLetter and dataStartRow logic you already compute) to clear the
column data area, then perform the BatchUpdateValuesRequest write (keep using
buildColumnData, columnLetter, and the existing batch update). Ensure the clear
calls use the proper ClearValuesRequest/clear() on the same spreadsheetId and
ranges so old rows are removed prior to writing.
Comment on lines
+244
to
+248
| requests.add(new Request().setSetBasicFilter( | ||
| new SetBasicFilterRequest() | ||
| .setFilter(new BasicFilter() | ||
| .setRange(new GridRange().setSheetId(0))) | ||
| )); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java | head -100Repository: BCSDLab/KONECT_BACK_END
Length of output: 4942
🏁 Script executed:
wc -l src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.javaRepository: BCSDLab/KONECT_BACK_END
Length of output: 142
🏁 Script executed:
cat -n src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java | sed -n '200,280p'Repository: BCSDLab/KONECT_BACK_END
Length of output: 3897
🏁 Script executed:
rg "sheetId|SheetId" src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java -nRepository: BCSDLab/KONECT_BACK_END
Length of output: 1029
🏁 Script executed:
rg "setSheetId" src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java -B 5 -A 2Repository: BCSDLab/KONECT_BACK_END
Length of output: 293
🏁 Script executed:
rg "UpdateSheetPropertiesRequest|GridRange" src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java -B 3 -A 3Repository: BCSDLab/KONECT_BACK_END
Length of output: 1410
🏁 Script executed:
rg "spreadsheets\(\)\.get" src/main/java/gg/agit/konect/domain/club/service/ -A 3Repository: BCSDLab/KONECT_BACK_END
Length of output: 49
🏁 Script executed:
rg "setSheetId" src/main/java/gg/agit/konect/domain/club/service/ -B 2 -A 2Repository: BCSDLab/KONECT_BACK_END
Length of output: 586
🏁 Script executed:
cat -n src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java | sed -n '100,150p'Repository: BCSDLab/KONECT_BACK_END
Length of output: 2448
🏁 Script executed:
rg "getSheets|SheetProperties" src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java -B 2 -A 2Repository: BCSDLab/KONECT_BACK_END
Length of output: 826
시트 ID를 0으로 고정하면 포맷이 의도하지 않은 시트에 적용될 수 있습니다.
applyFormat 메서드의 SetBasicFilterRequest와 UpdateSheetPropertiesRequest에서 시트 ID를 명시적으로 설정하지 않으면 Google Sheets API가 첫 번째 시트(ID=0)를 기본값으로 사용합니다. 스프레드시트 구조가 변경되거나 시트가 추가되면 실제 데이터가 있는 시트와 포맷이 적용되는 시트가 달라질 수 있습니다. 데이터 동기화 자체는 성공하지만 포맷(필터, 고정 행)이 의도하지 않은 시트에 적용되어 사용자 혼선을 야기합니다. googleSheetsService.spreadsheets().get(spreadsheetId).execute()로 실제 시트 정보를 조회한 후 올바른 시트 ID를 동적으로 설정하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java`
around lines 244 - 248, The current applyFormat in SheetSyncExecutor hardcodes
sheetId=0 for SetBasicFilterRequest/UpdateSheetPropertiesRequest which can apply
formatting to the wrong sheet; instead, call
googleSheetsService.spreadsheets().get(spreadsheetId).execute(), locate the
intended Sheet by its title (or other identifier) and extract
sheet.getProperties().getSheetId(), then set that ID into
GridRange.setSheetId(...) and into the SheetProperties used by
UpdateSheetPropertiesRequest (do not leave 0). Update applyFormat to compute the
correct sheetId dynamically (use the existing spreadsheetId and target sheet
name variable) and use that value for SetBasicFilterRequest and
UpdateSheetPropertiesRequest.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java (2)
28-33:⚠️ Potential issue | 🟠 Major[LEVEL: medium] 모델 선택이 설정과 분리되어 하드코딩되어 있습니다.
문제:
MAPPING_MODEL상수 고정으로ClaudeProperties의 모델 설정값이 반영되지 않습니다.
영향: 운영 중 모델 교체/롤백 시 코드 수정·재배포가 필요해 변경 비용과 설정 불일치 위험이 커집니다.
제안: 요청 바디의model값을claudeProperties.model()로 일원화하고 고정 모델이 의도라면 그 근거를 주석으로 명시하세요; As per coding guidelines "확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java` around lines 28 - 33, The MAPPING_MODEL constant is hardcoded in SheetHeaderMapper which prevents runtime model changes; replace uses of MAPPING_MODEL with the configured value from ClaudeProperties (use claudeProperties.model()) when constructing the request body (e.g., wherever the request's "model" field is set), or if a fixed model is intentional, add a clear comment next to MAPPING_MODEL explaining why it's immutable and documenting the operational justification; update any related tests or callers to rely on claudeProperties.model() instead of the MAPPING_MODEL constant.
41-47:⚠️ Potential issue | 🔴 Critical[LEVEL: high] Claude 외부 호출의 타임아웃 보장이 코드에서 확인되지 않습니다.
문제:
RestClient.Builder로 생성한 클라이언트에 연결/응답 타임아웃이 이 파일 기준으로 보장되지 않아 지연 시 무기한 대기할 수 있습니다.
영향: Claude API 지연·hang 상황에서(특히 트랜잭션 경계 내부 호출이면) 요청 스레드와 DB 커넥션 점유가 길어져 커넥션 풀 고갈로 운영 장애가 발생할 수 있습니다.
제안: 타임아웃이 적용된 전용RestClient빈을 주입하도록 강제하고 트랜잭션 외부 호출 여부를 함께 검증하세요; As per coding guidelines "보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다."검증 방법: (1)
RestClient.Builder/requestFactory타임아웃 설정 존재 여부 확인, (2)analyzeHeaders호출부의@Transactional경계 확인
예상 결과: 연결/읽기 타임아웃 설정이 확인되고, 외부 API 호출이 트랜잭션 경계 밖에서 수행되어야 합니다.#!/bin/bash set -euo pipefail echo "== RestClient/timeout 설정 검색 ==" fd '.*\.java$' src/main/java \ | xargs rg -n --no-heading -e 'RestClient\.Builder' -e 'requestFactory' -e 'setConnectTimeout' -e 'setReadTimeout' echo echo "== SheetHeaderMapper.analyzeHeaders 호출 위치와 트랜잭션 경계 확인 ==" rg -n -C3 'analyzeHeaders\s*\(' src/main/java rg -n -C3 '@Transactional' src/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.javaAlso applies to: 126-133
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java` around lines 41 - 47, The RestClient created in the SheetHeaderMapper constructor is built from RestClient.Builder without enforcing connect/read timeouts and may be used inside transactional work (see analyzeHeaders usage); change to inject a preconfigured RestClient bean that has explicit connect and read timeout settings (or validate the supplied RestClient.Builder/requestFactory for setConnectTimeout/setReadTimeout before build) and update SheetHeaderMapper to accept that configured RestClient instead of calling restClientBuilder.build(); additionally ensure analyzeHeaders is invoked outside any `@Transactional` boundary (e.g., move external Claude API call out of ClubMemberSheetService transactional methods or mark the specific method non-transactional) so external HTTP calls do not hold DB transactions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java`:
- Around line 28-33: The MAPPING_MODEL constant is hardcoded in
SheetHeaderMapper which prevents runtime model changes; replace uses of
MAPPING_MODEL with the configured value from ClaudeProperties (use
claudeProperties.model()) when constructing the request body (e.g., wherever the
request's "model" field is set), or if a fixed model is intentional, add a clear
comment next to MAPPING_MODEL explaining why it's immutable and documenting the
operational justification; update any related tests or callers to rely on
claudeProperties.model() instead of the MAPPING_MODEL constant.
- Around line 41-47: The RestClient created in the SheetHeaderMapper constructor
is built from RestClient.Builder without enforcing connect/read timeouts and may
be used inside transactional work (see analyzeHeaders usage); change to inject a
preconfigured RestClient bean that has explicit connect and read timeout
settings (or validate the supplied RestClient.Builder/requestFactory for
setConnectTimeout/setReadTimeout before build) and update SheetHeaderMapper to
accept that configured RestClient instead of calling restClientBuilder.build();
additionally ensure analyzeHeaders is invoked outside any `@Transactional`
boundary (e.g., move external Claude API call out of ClubMemberSheetService
transactional methods or mark the specific method non-transactional) so external
HTTP calls do not hold DB transactions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 97a9667b-80f0-4f8c-9096-9eeaa2f3e618
📒 Files selected for processing (1)
src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
src/main/java/**/*.java: 아래 원칙으로 리뷰 코멘트를 작성한다.
- 코멘트는 반드시 한국어로 작성한다.
- 반드시 수정이 필요한 항목만 코멘트로 남기고, 단순 취향 차이는 지적하지 않는다.
- 각 코멘트 첫 줄에 심각도를
[LEVEL: high|medium|low]형식으로 반드시 표기한다.- 심각도 기준: high=운영 장애 가능, medium=품질 저하, low=개선 권고.
- 각 코멘트는 "문제 -> 영향 -> 제안" 순서로 3문장 이내로 간결하게 작성한다.
- 가능하면 재현 조건 및 실패 시나리오도 포함한다.
- 제안은 현재 코드베이스(Spring Boot + JPA + Flyway) 패턴과 일치해야 한다.
- 보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.
- 가독성: 변수/메서드 이름이 의도를 바로 드러내는지, 중첩과 메서드 길이가 과도하지 않은지 점검한다.
- 단순화: 불필요한 추상화, 중복 로직, 과한 방어 코드가 있으면 더 단순한 대안을 제시한다.
- 확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다.
Files:
src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.java (1)
49-64:⚠️ Potential issue | 🔴 Critical[LEVEL: high]
@Transactional내부 외부 API 호출 분리 필요문제: Line 49-60에서
@Transactional범위 안에서sheetHeaderMapper.analyzeHeaders()(외부 API 호출 가능)을 수행하고 있습니다.
영향: Claude/Google API 지연·장애가 발생하면 트랜잭션과 DB 커넥션 점유 시간이 길어져 동시 요청 시 커넥션 풀 고갈로 운영 장애가 발생할 수 있습니다.
제안: As per coding guidelines "보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.", 헤더 분석/JSON 직렬화는 트랜잭션 밖으로 이동하고@Transactional메서드는 DB 업데이트만 수행하도록 분리하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.java` around lines 49 - 64, The updateSheetId method currently performs external I/O and JSON serialization (sheetHeaderMapper.analyzeHeaders and objectMapper.writeValueAsString) inside the `@Transactional` boundary; extract the header analysis and mapping serialization out of the transactional method so the transactional update only does DB work. Specifically, create a small non-transactional pre-step that calls sheetHeaderMapper.analyzeHeaders(request.spreadsheetId()), serializes mapping via objectMapper.writeValueAsString(mapping.toMap()) (handling JsonProcessingException there), then call a transactional method (or keep updateSheetId but move DB calls) that runs clubPermissionValidator.validateManagerAccess(clubId, requesterId), club.updateGoogleSheetId(...), and club.updateSheetColumnMapping(...) and saves via clubRepository as needed. Ensure exceptions from the external call/serialization do not hold DB connections and pass the prepared serialized mapping into the transactional update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@src/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.java`:
- Around line 49-64: The updateSheetId method currently performs external I/O
and JSON serialization (sheetHeaderMapper.analyzeHeaders and
objectMapper.writeValueAsString) inside the `@Transactional` boundary; extract the
header analysis and mapping serialization out of the transactional method so the
transactional update only does DB work. Specifically, create a small
non-transactional pre-step that calls
sheetHeaderMapper.analyzeHeaders(request.spreadsheetId()), serializes mapping
via objectMapper.writeValueAsString(mapping.toMap()) (handling
JsonProcessingException there), then call a transactional method (or keep
updateSheetId but move DB calls) that runs
clubPermissionValidator.validateManagerAccess(clubId, requesterId),
club.updateGoogleSheetId(...), and club.updateSheetColumnMapping(...) and saves
via clubRepository as needed. Ensure exceptions from the external
call/serialization do not hold DB connections and pass the prepared serialized
mapping into the transactional update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9ada03e9-1540-4705-bf65-f3b7e32ef1e6
📒 Files selected for processing (1)
src/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.java
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
src/main/java/**/*.java: 아래 원칙으로 리뷰 코멘트를 작성한다.
- 코멘트는 반드시 한국어로 작성한다.
- 반드시 수정이 필요한 항목만 코멘트로 남기고, 단순 취향 차이는 지적하지 않는다.
- 각 코멘트 첫 줄에 심각도를
[LEVEL: high|medium|low]형식으로 반드시 표기한다.- 심각도 기준: high=운영 장애 가능, medium=품질 저하, low=개선 권고.
- 각 코멘트는 "문제 -> 영향 -> 제안" 순서로 3문장 이내로 간결하게 작성한다.
- 가능하면 재현 조건 및 실패 시나리오도 포함한다.
- 제안은 현재 코드베이스(Spring Boot + JPA + Flyway) 패턴과 일치해야 한다.
- 보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.
- 가독성: 변수/메서드 이름이 의도를 바로 드러내는지, 중첩과 메서드 길이가 과도하지 않은지 점검한다.
- 단순화: 불필요한 추상화, 중복 로직, 과한 방어 코드가 있으면 더 단순한 대안을 제시한다.
- 확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다.
Files:
src/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.java
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.java
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.java`:
- Around line 84-87: The current method in ClubMemberSheetService calls
sheetSyncExecutor.executeWithSort(clubId, sortKey, ascending) which is `@Async`
and returns immediately, so the returned
ClubMemberSheetSyncResponse.of((int)memberCount, spreadsheetId) may not reflect
the sheet post-sync; update the implementation to either (A) wait for completion
by changing executeWithSort to return a Future/CompletableFuture and
block/compose on it before computing memberCount and returning the response, or
(B) keep async execution but change the API contract and response to clearly
indicate asynchronous processing (e.g., return a queued/accepted status or
include a flag/message that sync is in progress) and ensure
ClubMemberSheetSyncResponse (or alternative response) documents this; reference
the methods sheetSyncExecutor.executeWithSort, ClubMemberSheetService method
that builds memberCount, and ClubMemberSheetSyncResponse for where to apply the
change.
In `@src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java`:
- Around line 152-153: The code in SheetHeaderMapper that does JsonNode root =
objectMapper.readTree(response); return
root.path("content").get(0).path("text").asText(); can throw an NPE if "content"
is missing or empty; update this parsing to validate that root.path("content")
is an array and has at least one element before calling get(0), and handle the
empty/missing case by throwing a clear, descriptive exception or returning a
safe default; reference the JsonNode usage in SheetHeaderMapper
(objectMapper.readTree, root.path("content").get(0).path("text").asText()) so
you locate and wrap the access with explicit checks (e.g., isArray/isEmpty or
null checks) and include the response content in the thrown message for easier
debugging.
In `@src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java`:
- Around line 269-278: The Comparator in SheetSyncExecutor can throw NPEs when
fields like getUser().getName(), getUser().getStudentNumber(),
getClubPosition(), getCreatedAt() or paymentMap lookups are null; update each
case to be null-safe by using Comparator.nullsLast(...) around
Comparator.comparing(...) (or provide null-safe key extractors that return a
default/all-else value) for NAME and STUDENT_ID, wrap the POSITION comparator to
safely handle a null getClubPosition() (e.g., map to a high priority default or
use Comparator.nullsLast with a comparator on priority), wrap JOINED_AT with
Comparator.nullsLast(Comparator.comparing(ClubMember::getCreatedAt)), and make
the FEE_PAID lambda null-safe by handling paymentMap.get(...) == null before
checking isPaid().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1c7d1a1f-9022-4c25-9ae9-60b260f9c600
📒 Files selected for processing (4)
src/main/java/gg/agit/konect/domain/club/model/SheetColumnMapping.javasrc/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.javasrc/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.javasrc/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
src/main/java/**/*.java: 아래 원칙으로 리뷰 코멘트를 작성한다.
- 코멘트는 반드시 한국어로 작성한다.
- 반드시 수정이 필요한 항목만 코멘트로 남기고, 단순 취향 차이는 지적하지 않는다.
- 각 코멘트 첫 줄에 심각도를
[LEVEL: high|medium|low]형식으로 반드시 표기한다.- 심각도 기준: high=운영 장애 가능, medium=품질 저하, low=개선 권고.
- 각 코멘트는 "문제 -> 영향 -> 제안" 순서로 3문장 이내로 간결하게 작성한다.
- 가능하면 재현 조건 및 실패 시나리오도 포함한다.
- 제안은 현재 코드베이스(Spring Boot + JPA + Flyway) 패턴과 일치해야 한다.
- 보안, 트랜잭션 경계, 예외 처리, N+1, 성능 회귀 가능성을 우선 점검한다.
- 가독성: 변수/메서드 이름이 의도를 바로 드러내는지, 중첩과 메서드 길이가 과도하지 않은지 점검한다.
- 단순화: 불필요한 추상화, 중복 로직, 과한 방어 코드가 있으면 더 단순한 대안을 제시한다.
- 확장성: 새 요구사항 추가 시 변경 범위가 최소화되는 구조인지(하드코딩 분기/값 여부 포함) 점검한다.
Files:
src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.javasrc/main/java/gg/agit/konect/domain/club/model/SheetColumnMapping.javasrc/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.javasrc/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.javasrc/main/java/gg/agit/konect/domain/club/model/SheetColumnMapping.javasrc/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.javasrc/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java
🔇 Additional comments (2)
src/main/java/gg/agit/konect/domain/club/model/SheetColumnMapping.java (1)
1-69: LGTM![LEVEL: low] 값 객체로서 적절히 설계되었습니다. 생성자에서 방어적 복사를 수행하고, 필드별 매핑 및 기본값 처리가 명확합니다.
src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java (1)
77-78: [LEVEL: low] 중복 키 처리가 필요 없습니다데이터베이스 마이그레이션 파일을 확인한 결과,
club_fee_payment테이블에UNIQUE KEY uq_fee_payment_club_user (club_id, user_id)제약이 정의되어 있습니다. 따라서 동일 클럽 내에서 한 사용자는 최대 하나의 납부 기록만 가질 수 있으므로, 현재 코드의Collectors.toMap()은 중복 키 예외를 발생시키지 않습니다.
Comment on lines
+84
to
+87
| long memberCount = clubMemberRepository.countByClubId(clubId); | ||
| sheetSyncExecutor.executeWithSort(clubId, sortKey, ascending); | ||
|
|
||
| return ClubMemberSheetSyncResponse.of((int)memberCount, spreadsheetId); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
[LEVEL: low] 비동기 동기화 완료 전에 응답 반환
sheetSyncExecutor.executeWithSort()는 @Async로 실행되어 즉시 반환됩니다. 응답에 포함된 memberCount는 동기화 시점의 실제 시트 상태를 보장하지 않습니다. API 응답이나 문서에서 비동기 처리임을 명시하는 것을 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/gg/agit/konect/domain/club/service/ClubMemberSheetService.java`
around lines 84 - 87, The current method in ClubMemberSheetService calls
sheetSyncExecutor.executeWithSort(clubId, sortKey, ascending) which is `@Async`
and returns immediately, so the returned
ClubMemberSheetSyncResponse.of((int)memberCount, spreadsheetId) may not reflect
the sheet post-sync; update the implementation to either (A) wait for completion
by changing executeWithSort to return a Future/CompletableFuture and
block/compose on it before computing memberCount and returning the response, or
(B) keep async execution but change the API contract and response to clearly
indicate asynchronous processing (e.g., return a queued/accepted status or
include a flag/message that sync is in progress) and ensure
ClubMemberSheetSyncResponse (or alternative response) documents this; reference
the methods sheetSyncExecutor.executeWithSort, ClubMemberSheetService method
that builds memberCount, and ClubMemberSheetSyncResponse for where to apply the
change.
Comment on lines
+152
to
+153
| JsonNode root = objectMapper.readTree(response); | ||
| return root.path("content").get(0).path("text").asText(); |
There was a problem hiding this comment.
[LEVEL: medium] Claude API 응답 파싱 시 NPE 위험
root.path("content").get(0)에서 content 배열이 비어 있으면 NullPointerException이 발생합니다. API 응답 구조가 예상과 다를 경우 명확한 예외 처리가 필요합니다.
수정 제안
JsonNode root = objectMapper.readTree(response);
- return root.path("content").get(0).path("text").asText();
+ JsonNode contentArray = root.path("content");
+ if (contentArray.isEmpty() || contentArray.get(0) == null) {
+ throw new RuntimeException("Empty content in Claude API response");
+ }
+ return contentArray.get(0).path("text").asText();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java`
around lines 152 - 153, The code in SheetHeaderMapper that does JsonNode root =
objectMapper.readTree(response); return
root.path("content").get(0).path("text").asText(); can throw an NPE if "content"
is missing or empty; update this parsing to validate that root.path("content")
is an array and has at least one element before calling get(0), and handle the
empty/missing case by throwing a clear, descriptive exception or returning a
safe default; reference the JsonNode usage in SheetHeaderMapper
(objectMapper.readTree, root.path("content").get(0).path("text").asText()) so
you locate and wrap the access with explicit checks (e.g., isArray/isEmpty or
null checks) and include the response content in the thrown message for easier
debugging.
Comment on lines
+269
to
+278
| Comparator<ClubMember> comparator = switch (sortKey) { | ||
| case NAME -> Comparator.comparing(m -> m.getUser().getName()); | ||
| case STUDENT_ID -> Comparator.comparing(m -> m.getUser().getStudentNumber()); | ||
| case POSITION -> Comparator.comparingInt(m -> m.getClubPosition().getPriority()); | ||
| case JOINED_AT -> Comparator.comparing(ClubMember::getCreatedAt); | ||
| case FEE_PAID -> Comparator.comparing(m -> { | ||
| ClubFeePayment p = paymentMap.get(m.getUser().getId()); | ||
| return p != null && p.isPaid() ? 0 : 1; | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
[LEVEL: low] 정렬 Comparator에서 null 값 처리 누락
getName(), getStudentNumber() 등이 null을 반환하면 NullPointerException이 발생합니다. Comparator.nullsLast()로 null-safe하게 처리하는 것을 권장합니다.
수정 제안
Comparator<ClubMember> comparator = switch (sortKey) {
- case NAME -> Comparator.comparing(m -> m.getUser().getName());
- case STUDENT_ID -> Comparator.comparing(m -> m.getUser().getStudentNumber());
+ case NAME -> Comparator.comparing(
+ m -> m.getUser().getName(), Comparator.nullsLast(Comparator.naturalOrder()));
+ case STUDENT_ID -> Comparator.comparing(
+ m -> m.getUser().getStudentNumber(), Comparator.nullsLast(Comparator.naturalOrder()));
case POSITION -> Comparator.comparingInt(m -> m.getClubPosition().getPriority());
case JOINED_AT -> Comparator.comparing(ClubMember::getCreatedAt);📝 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.
Suggested change
| Comparator<ClubMember> comparator = switch (sortKey) { | |
| case NAME -> Comparator.comparing(m -> m.getUser().getName()); | |
| case STUDENT_ID -> Comparator.comparing(m -> m.getUser().getStudentNumber()); | |
| case POSITION -> Comparator.comparingInt(m -> m.getClubPosition().getPriority()); | |
| case JOINED_AT -> Comparator.comparing(ClubMember::getCreatedAt); | |
| case FEE_PAID -> Comparator.comparing(m -> { | |
| ClubFeePayment p = paymentMap.get(m.getUser().getId()); | |
| return p != null && p.isPaid() ? 0 : 1; | |
| }); | |
| }; | |
| Comparator<ClubMember> comparator = switch (sortKey) { | |
| case NAME -> Comparator.comparing( | |
| m -> m.getUser().getName(), Comparator.nullsLast(Comparator.naturalOrder())); | |
| case STUDENT_ID -> Comparator.comparing( | |
| m -> m.getUser().getStudentNumber(), Comparator.nullsLast(Comparator.naturalOrder())); | |
| case POSITION -> Comparator.comparingInt(m -> m.getClubPosition().getPriority()); | |
| case JOINED_AT -> Comparator.comparing(ClubMember::getCreatedAt); | |
| case FEE_PAID -> Comparator.comparing(m -> { | |
| ClubFeePayment p = paymentMap.get(m.getUser().getId()); | |
| return p != null && p.isPaid() ? 0 : 1; | |
| }); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java`
around lines 269 - 278, The Comparator in SheetSyncExecutor can throw NPEs when
fields like getUser().getName(), getUser().getStudentNumber(),
getClubPosition(), getCreatedAt() or paymentMap lookups are null; update each
case to be null-safe by using Comparator.nullsLast(...) around
Comparator.comparing(...) (or provide null-safe key extractors that return a
default/all-else value) for NAME and STUDENT_ID, wrap the POSITION comparator to
safely handle a null getClubPosition() (e.g., map to a high priority default or
use Comparator.nullsLast with a comparator on priority), wrap JOINED_AT with
Comparator.nullsLast(Comparator.comparing(ClubMember::getCreatedAt)), and make
the FEE_PAID lambda null-safe by handling paymentMap.get(...) == null before
checking isPaid().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔍 개요
구글 스프레드시트 API를 활용한 동아리 인명부 및 회비 장부 자동화 기능을 구현합니다.
🚀 주요 변경 내용
1. 인명부 동기화 API
PUT /clubs/{clubId}/sheet- 스프레드시트 ID 등록/수정POST /clubs/{clubId}/members/sheet-sync?sortKey=POSITION&ascending=true- 수동 동기화 (정렬 옵션 지원)NAME,STUDENT_ID,POSITION,JOINED_AT,FEE_PAID2. 자동 트리거
@TransactionalEventListener(AFTER_COMMIT)로 자동 동기화SheetSyncDebouncer- 3초 디바운싱으로 중복 API 호출 방지SheetSyncExecutor-@Async("sheetSyncExecutor")전용 스레드풀로 비동기 처리3. 회비 납부 장부
POST /clubs/{clubId}/fee-payments- 납부 접수POST /clubs/{clubId}/fee-payments/{targetUserId}/approve- 납부 승인 (운영진+)GET /clubs/{clubId}/fee-payments- 전체 납부 목록 조회 (운영진+)GET /clubs/{clubId}/fee-payments/me- 내 납부 상태 조회FeePaid,PaidAt자동 반영4. Claude API 기반 커스텀 시트 지원
5. DB 변경사항
V50:club.google_sheet_id컬럼 추가V51:club_fee_payment테이블 생성V52:club.sheet_column_mappingJSON 컬럼 추가💬 참고 사항
GOOGLE_SHEETS_CREDENTIALS_PATH,GOOGLE_SHEETS_APP_NAMEName/StudentId/Email/Phone/Position/JoinedAt/FeePaid/PaidAt✅ Checklist (완료 조건)