Open
Conversation
- Service 계층으로 User 조회 로직 이관 및 Controller 의존성 제거 - PaymentService 단위 테스트(성공/실패/예외 케이스) 추가 - 결제 상세 조회 시, 예약자뿐만 아니라 상점 주인도 접근 가능하도록 권한 로직 수정 - Spring Boot 3.4 마이그레이션: @MockBean -> @MockitoBean 교체 (Deprecated API 대응)
📝 WalkthroughWalkthrough컨트롤러 엔드포인트가 인증된 사용자( Changes
Sequence DiagramsequenceDiagram
actor 사용자
participant 컨트롤러 as PaymentController
participant 서비스 as PaymentService
participant 사용자Repo as UserRepository
participant 결제Repo as PaymentRepository
participant 예약 as BookingEntity
사용자->>컨트롤러: 인증된 요청 (JWT) + paymentId 또는 페이지 파라미터
컨트롤러->>서비스: getPaymentDetail(paymentId, email) / getPaymentList(email,...)
서비스->>사용자Repo: findByEmail(email)
사용자Repo-->>서비스: User
서비스->>결제Repo: findByIdWithDetails / findAllBy...WithDetails(...)
결제Repo-->>서비스: Payment (+Booking, Store, Owner)
서비스->>서비스: booker 또는 store owner 권한 검사
alt 권한 있음
서비스-->>컨트롤러: Payment DTO
컨트롤러-->>사용자: 200 OK
else 권한 없음
서비스-->>컨트롤러: PaymentException(_PAYMENT_ACCESS_DENIED)
컨트롤러-->>사용자: 403 Forbidden
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분 Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/eatsfine/eatsfine/domain/payment/service/PaymentService.java (1)
153-154:⚠️ Potential issue | 🟡 Minor에러 로그에 전체 response 객체를 출력하면 민감한 결제 정보가 노출될 수 있습니다.
log.error("Toss Payment Cancel Failed: {}", response)에서 TossPaymentResponse 전체가 로그에 기록됩니다. paymentKey, orderId 등 결제 관련 민감 정보가 포함될 수 있으므로, 필요한 필드만 선택적으로 로깅하는 것이 안전합니다.🛡️ 로깅 개선 제안
- log.error("Toss Payment Cancel Failed: {}", response); + log.error("Toss Payment Cancel Failed: status={}, orderId={}", + response != null ? response.status() : "null", + response != null ? response.orderId() : "null");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/com/eatsfine/eatsfine/domain/payment/service/PaymentService.java` around lines 153 - 154, In PaymentService, avoid logging the entire TossPaymentResponse object (variable response) in the log.error call; instead extract and log only non-sensitive, diagnostic fields (e.g., response.status(), response.getCode() or response.getErrorMessage() — whatever accessors TossPaymentResponse exposes) and include context like the operation ("Toss Payment Cancel Failed") while omitting paymentKey/orderId/tokens; replace the current log.error("Toss Payment Cancel Failed: {}", response) with a message that interpolates only those safe fields.src/main/java/com/eatsfine/eatsfine/domain/payment/controller/PaymentController.java (1)
45-51:⚠️ Potential issue | 🔴 Critical
cancelPayment에 소유권 검증이 필요합니다.엔드포인트는
SecurityConfig의.anyRequest().authenticated()에 의해 인증이 필수입니다만,getPaymentDetail과 달리 소유권 검증이 누락되어 있습니다. 인증된 사용자라면 누구든지paymentKey를 알고 있으면 다른 사용자의 결제를 취소할 수 있는 권한 상승(privilege escalation) 취약점이 존재합니다.
getPaymentDetail과 같이 사용자 정보를 매개변수로 전달받아 결제의 예약자 또는 가게 사장님 중 하나인지 확인하는 소유권 검증 로직을 추가해야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/eatsfine/eatsfine/domain/payment/controller/PaymentController.java` around lines 45 - 51, The cancelPayment endpoint lacks ownership checks and allows any authenticated user knowing a paymentKey to cancel others' payments; update PaymentController.cancelPayment to obtain the authenticated user (e.g., via Principal or `@AuthenticationPrincipal`) and pass that user identity into the paymentService.cancelPayment call (or add a new service method signature) so the service can verify the requester is either the reservation owner or the shop owner (mirroring the authorization used by getPaymentDetail); if verification fails, return an appropriate forbidden/error response. Ensure the check is implemented in PaymentService (or a dedicated authorization helper) and referenced from cancelPayment so the controller delegates cancellation only after ownership is confirmed.
🤖 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/com/eatsfine/eatsfine/domain/payment/repository/PaymentRepository.java`:
- Around line 25-30: The `@Param` name used in the query methods
findAllByOwnerIdWithDetails and findAllByOwnerIdAndStatusWithDetails is
confusingly "userId"; change the JPQL parameter binding and corresponding `@Param`
annotation to "ownerId" so the named parameter in the query (s.owner.id =
:ownerId) and the method parameter annotation (`@Param`("ownerId")) match and
improve readability while preserving existing method signatures and behavior.
In
`@src/main/java/com/eatsfine/eatsfine/domain/payment/service/PaymentService.java`:
- Around line 181-207: Extract the duplicated status-parsing logic into a
private helper in PaymentService (e.g., parsePaymentStatus or toPaymentStatus)
that accepts the raw String status, performs status != null && !status.isEmpty()
check, calls PaymentStatus.valueOf(status.toUpperCase()) inside the try/catch
and throws GeneralException(ErrorStatus._BAD_REQUEST) on
IllegalArgumentException, then return the parsed PaymentStatus (or null/Optional
if you prefer); replace both duplicated blocks in the OWNER and non-OWNER
branches to call that helper once and branch on its result to call the
appropriate repository method (findAllByOwnerIdWithDetails /
findAllByOwnerIdAndStatusWithDetails and findAllByUserIdWithDetails /
findAllByUserIdAndStatusWithDetails).
- Around line 233-245: The ownership check in PaymentService.getPaymentDetail
triggers an extra lazy load because findByIdWithDetails doesn't fetch
Store.owner; update the repository method findByIdWithDetails JPQL to eagerly
load the owner (e.g. add "JOIN FETCH b.store s JOIN FETCH s.owner" or
equivalent) so payment.getBooking().getStore().getOwner() is resolved in the
same query and avoids LazyInitializationException/N+1 queries.
In `@src/main/resources/application-local.yml`:
- Around line 72-73: The PR adds new required environment variables
(TOSS_WIDGET_SECRET_KEY, AWS_REGION, AWS_S3_BUCKET, AWS_S3_BASE_URL,
BIZ_API_KEY) but there is no centralized documentation of all required env vars;
please create or update an example env file and/or README to list every required
environment variable (including existing DB_*, REDIS_*, *_CLIENT_ID/SECRET,
SECRET_KEY and the new ones) with brief descriptions and example values, and add
a note on which are required vs optional and where to obtain them so local
startup won't fail due to missing variables.
- Around line 72-73: Add a documented list of required environment variables and
brief descriptions (including TOSS_WIDGET_SECRET_KEY, AWS_REGION, AWS_S3_BUCKET,
AWS_S3_BASE_URL, BIZ_API_KEY, existing DB_*, REDIS_*, OAuth *_CLIENT_ID/_SECRET,
SECRET_KEY) by creating a .env.example at the project root (or adding a section
in README) that enumerates each variable, default/example values where safe, and
notes which are required vs optional so developers can replicate local setup and
avoid startup failures; ensure the list matches keys referenced in
application-local.yml and your configuration code.
- Around line 73-74: The YAML is missing a blank line between the
api.service-key entry and the next top-level jwt: block, breaking top-level
section separation consistency; update the file so there is an empty line
between the api.service-key (api:) block and the jwt: top-level key to match
other sections like payment: and cloud:, ensuring consistent visual separation
for top-level YAML blocks.
- Around line 73-74: The YAML is missing a blank line between the
api.service-key entry and the next top-level jwt: block, harming readability;
edit the area containing api.service-key (the `service-key: ${BIZ_API_KEY}` line
under the api: block) and insert a single empty line before the `jwt:` top-level
key so top-level sections are consistently separated.
- Line 63: 현재 application-local.yml에서 참조하는 TOSS_WIDGET_SECRET_KEY 환경변수가 올바르게
사용되고 있으나, 프로젝트에 환경변수 문서가 없어 신규 개발자가 로컬 설정을 하기 어렵습니다; 생성할 작업으로 .env.example 파일(또는
README의 "로컬 개발 환경 설정" 섹션)을 추가하고 application-local.yml에서 사용중인 모든 환경변수
이름(TOSS_WIDGET_SECRET_KEY, DB_HOST, DB_PORT, DB_NAME, DB_USERNAME, DB_PASSWORD,
REDIS_HOST, REDIS_PORT, GOOGLE_CLIENT_ID, GOOGLE_CLIENT_SECRET, KAKAO_CLIENT_ID,
KAKAO_CLIENT_SECRET, AWS_REGION, AWS_S3_BUCKET, AWS_S3_BASE_URL, BIZ_API_KEY,
SECRET_KEY)을 나열해 각 항목에 설명(용도), 예시값(테스트/비어있음 표시), 로컬에서 secure하게 관리할 것(파일이
.gitignore에 포함되어야 함) 등의 지침을 추가하세요; 또한 application-local.yml이 gitignored 되어 있음을
명시하고, .env.example을 템플릿으로 사용해 로컬에서 .env를 생성하도록 안내문구를 포함하면 됩니다.
In
`@src/test/java/com/eatsfine/eatsfine/domain/payment/controller/PaymentControllerTest.java`:
- Around line 75-186: Tests only cover success paths; add tests for unauthorized
(401), validation failure (400) and service-exception flows. For each controller
endpoint used in PaymentControllerTest (POST /api/v1/payments/request handled by
paymentService.requestPayment, POST /api/v1/payments/confirm ->
paymentService.confirmPayment, POST /api/v1/payments/{paymentKey}/cancel ->
paymentService.cancelPayment, GET /api/v1/payments and GET
/api/v1/payments/{paymentId}): add a test that performs the same mockMvc.perform
call without authentication to assert 401, add a validation-failure test (e.g.,
for requestPayment create a RequestPaymentDTO with bookingId = null and assert
status().isBadRequest()), and add service-exception tests by stubbing
paymentService methods with willThrow(new PaymentException(...)) or
willThrow(new UserException(...)) and asserting the controller returns the
mapped error response (non-success body or expected error status). Use the
existing unique symbols (paymentService.requestPayment,
paymentService.confirmPayment, paymentService.cancelPayment,
paymentService.getPaymentList, paymentService.getPaymentDetail, and
mockMvc.perform with the same endpoints) to locate where to add these tests.
- Around line 156-157: Replace the loose anyString() matcher with an exact
equality matcher for the expected username so the test verifies the controller
extracts the authenticated principal correctly: update the stubbing of
paymentService.getPaymentList(...) in PaymentControllerTest to use eq("user")
for the first argument (matching `@WithMockUser`'s default username) instead of
anyString(), keeping the other matchers as-is so getPaymentList(...) is asserted
to receive the username produced by the controller.
In
`@src/test/java/com/eatsfine/eatsfine/domain/payment/service/PaymentServiceTest.java`:
- Around line 66-341: Add unit tests covering getPaymentDetail success and
status-filter scenarios: write a getPaymentDetail_success test that mocks
userRepository.findByEmail(...) to return the booking user (and another variant
for the store owner) and paymentRepository.findByIdWithDetails(...) to return a
Payment with populated Booking/Store/User, then assert the returned
PaymentResponseDTO.PaymentDetailResponseDTO fields match the Payment/Booking
data and no exception is thrown; add getPaymentList_withStatus_filter test that
mocks userRepository.findByEmail(...) and
paymentRepository.findAllByUserIdWithDetails(...) /
findAllByOwnerIdWithDetails(...) to return a Page<Payment> filtered by
PaymentStatus and assert response.payments() contains only matching statuses;
finally add getPaymentList_badStatus_test that calls
paymentService.getPaymentList(...) with an invalid status string and asserts a
PaymentException with code PaymentErrorStatus._BAD_REQUEST is thrown.
- Line 236: Remove the unused commented code lines containing "// Pageable
pageable = PageRequest.of(0, 10);" from the PaymentServiceTest class; locate the
commented occurrences near the test methods in
src/test/java/com/eatsfine/eatsfine/domain/payment/service/PaymentServiceTest.java
(search for the exact text "Pageable pageable = PageRequest.of(0, 10)") and
delete both commented instances so the test file contains no leftover
commented-out pageable declarations.
- Around line 135-154: Remove the long trailing inline comment after the
TossPaymentResponse record creation and replace it with a single concise comment
stating this is a mocked TossPaymentResponse for the test; locate the
instantiation using TossPaymentResponse tossResponse and the nested
TossPaymentResponse.EasyPay easyPay, keep the constructor arguments intact, and
ensure the new comment is a single short line (e.g., "mocked TossPaymentResponse
for test") to improve readability.
---
Outside diff comments:
In
`@src/main/java/com/eatsfine/eatsfine/domain/payment/controller/PaymentController.java`:
- Around line 45-51: The cancelPayment endpoint lacks ownership checks and
allows any authenticated user knowing a paymentKey to cancel others' payments;
update PaymentController.cancelPayment to obtain the authenticated user (e.g.,
via Principal or `@AuthenticationPrincipal`) and pass that user identity into the
paymentService.cancelPayment call (or add a new service method signature) so the
service can verify the requester is either the reservation owner or the shop
owner (mirroring the authorization used by getPaymentDetail); if verification
fails, return an appropriate forbidden/error response. Ensure the check is
implemented in PaymentService (or a dedicated authorization helper) and
referenced from cancelPayment so the controller delegates cancellation only
after ownership is confirmed.
In
`@src/main/java/com/eatsfine/eatsfine/domain/payment/service/PaymentService.java`:
- Around line 153-154: In PaymentService, avoid logging the entire
TossPaymentResponse object (variable response) in the log.error call; instead
extract and log only non-sensitive, diagnostic fields (e.g., response.status(),
response.getCode() or response.getErrorMessage() — whatever accessors
TossPaymentResponse exposes) and include context like the operation ("Toss
Payment Cancel Failed") while omitting paymentKey/orderId/tokens; replace the
current log.error("Toss Payment Cancel Failed: {}", response) with a message
that interpolates only those safe fields.
src/main/java/com/eatsfine/eatsfine/domain/payment/repository/PaymentRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eatsfine/eatsfine/domain/payment/service/PaymentService.java
Show resolved
Hide resolved
Comment on lines
233
to
+245
| @Transactional(readOnly = true) | ||
| public PaymentResponseDTO.PaymentDetailResultDTO getPaymentDetail(Long paymentId) { | ||
| Payment payment = paymentRepository.findById(paymentId) | ||
| public PaymentResponseDTO.PaymentDetailResultDTO getPaymentDetail(Long paymentId, String email) { | ||
| User user = userRepository.findByEmail(email) | ||
| .orElseThrow(() -> new UserException(UserErrorStatus.MEMBER_NOT_FOUND)); | ||
| Payment payment = paymentRepository.findByIdWithDetails(paymentId) | ||
| .orElseThrow(() -> new PaymentException(PaymentErrorStatus._PAYMENT_NOT_FOUND)); | ||
|
|
||
| boolean isBooker = payment.getBooking().getUser().getId().equals(user.getId()); | ||
| boolean isStoreOwner = payment.getBooking().getStore().getOwner().getId().equals(user.getId()); | ||
|
|
||
| if (!isBooker && !isStoreOwner) { | ||
| throw new PaymentException(PaymentErrorStatus._PAYMENT_ACCESS_DENIED); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Booking 엔티티 파일 찾기 및 user, store 필드의 nullable 설정 확인
echo "=== Booking 엔티티 검색 ==="
fd -e java -i "booking" | grep -i entity
echo ""
# 2. 엔티티 정의 확인
find . -path "*/entity/*" -name "*.java" | grep -i booking | head -5
find . -path "*/entity/*" -name "*.java" | grep -i store | head -5
find . -path "*/entity/*" -name "*.java" | grep -i payment | head -5Repository: Eatsfine/BE
Length of output: 781
🏁 Script executed:
#!/bin/bash
# Booking 엔티티의 user와 store 필드 정의 확인
echo "=== Booking Entity 분석 ==="
find . -type f -name "Booking.java" -not -path "*/test/*" -not -path "*/.git/*" | while read f; do
echo "File: $f"
cat -n "$f" | grep -A 3 -B 1 '@ManyToOne\|@OneToOne\|private.*user\|private.*store\|@JoinColumn'
doneRepository: Eatsfine/BE
Length of output: 577
🏁 Script executed:
#!/bin/bash
# Store 엔티티의 owner 필드 정의 확인
echo "=== Store Entity owner 필드 분석 ==="
find . -type f -name "Store.java" -not -path "*/test/*" | while read f; do
echo "File: $f"
cat -n "$f" | grep -A 3 -B 1 'owner\|@ManyToOne.*owner\|@JoinColumn.*owner'
doneRepository: Eatsfine/BE
Length of output: 414
🏁 Script executed:
#!/bin/bash
# findByIdWithDetails 메서드 구현 확인
echo "=== findByIdWithDetails 구현 검색 ==="
rg "findByIdWithDetails" -A 5 -B 2Repository: Eatsfine/BE
Length of output: 2944
쿼리에서 store의 owner를 함께 로드해야 합니다.
소유권 검증 로직 자체는 올바릅니다. 다만 findByIdWithDetails 쿼리에서 Store.owner를 함께 로드하지 않았기 때문에, payment.getBooking().getStore().getOwner() 호출 시 추가 쿼리가 발생합니다.
현재 쿼리:
SELECT p FROM Payment p JOIN FETCH p.booking b JOIN FETCH b.store JOIN FETCH b.user WHERE p.id = :paymentId이 쿼리는 b.store는 로드하지만, b.store.owner는 로드하지 않습니다. 따라서 store.owner 접근 시 LazyInitializationException이 발생하거나 추가 N+1 쿼리 문제가 생길 수 있습니다. JOIN FETCH b.store s JOIN FETCH s.owner를 추가하여 owner도 함께 로드해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/eatsfine/eatsfine/domain/payment/service/PaymentService.java`
around lines 233 - 245, The ownership check in PaymentService.getPaymentDetail
triggers an extra lazy load because findByIdWithDetails doesn't fetch
Store.owner; update the repository method findByIdWithDetails JPQL to eagerly
load the owner (e.g. add "JOIN FETCH b.store s JOIN FETCH s.owner" or
equivalent) so payment.getBooking().getStore().getOwner() is resolved in the
same query and avoids LazyInitializationException/N+1 queries.
Comment on lines
+75
to
+186
| @Test | ||
| @DisplayName("결제 요청 성공") | ||
| void requestPayment_success() throws Exception { | ||
| // given | ||
| PaymentRequestDTO.RequestPaymentDTO request = new PaymentRequestDTO.RequestPaymentDTO(1L); | ||
| PaymentResponseDTO.PaymentRequestResultDTO response = new PaymentResponseDTO.PaymentRequestResultDTO( | ||
| 1L, 1L, "order-id-123", BigDecimal.valueOf(10000), LocalDateTime.now()); | ||
|
|
||
| given(paymentService.requestPayment(any(PaymentRequestDTO.RequestPaymentDTO.class))) | ||
| .willReturn(response); | ||
|
|
||
| // when & then | ||
| mockMvc.perform(post("/api/v1/payments/request") | ||
| .with(csrf()) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .content(objectMapper.writeValueAsString(request))) | ||
| .andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.isSuccess").value(true)) | ||
| .andExpect(jsonPath("$.result.paymentId").value(1L)) | ||
| .andExpect(jsonPath("$.result.orderId").value("order-id-123")); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("결제 승인 성공") | ||
| void confirmPayment_success() throws Exception { | ||
| // given | ||
| PaymentConfirmDTO request = PaymentConfirmDTO.builder() | ||
| .paymentKey("payment-key-123") | ||
| .orderId("order-id-123") | ||
| .amount(BigDecimal.valueOf(10000)) | ||
| .build(); | ||
|
|
||
| PaymentResponseDTO.PaymentSuccessResultDTO response = new PaymentResponseDTO.PaymentSuccessResultDTO( | ||
| 1L, "COMPLETED", LocalDateTime.now(), "order-id-123", BigDecimal.valueOf(10000), | ||
| "CARD", "TOSS", "http://receipt.url"); | ||
|
|
||
| given(paymentService.confirmPayment(any(PaymentConfirmDTO.class))).willReturn(response); | ||
|
|
||
| // when & then | ||
| mockMvc.perform(post("/api/v1/payments/confirm") | ||
| .with(csrf()) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .content(objectMapper.writeValueAsString(request))) | ||
| .andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.isSuccess").value(true)) | ||
| .andExpect(jsonPath("$.result.status").value("COMPLETED")); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("결제 취소 성공") | ||
| void cancelPayment_success() throws Exception { | ||
| // given | ||
| String paymentKey = "payment-key-123"; | ||
| PaymentRequestDTO.CancelPaymentDTO request = new PaymentRequestDTO.CancelPaymentDTO("단순 변심"); | ||
| PaymentResponseDTO.CancelPaymentResultDTO response = new PaymentResponseDTO.CancelPaymentResultDTO( | ||
| 1L, "order-id-123", paymentKey, "CANCELED", LocalDateTime.now()); | ||
|
|
||
| given(paymentService.cancelPayment(eq(paymentKey), any(PaymentRequestDTO.CancelPaymentDTO.class))) | ||
| .willReturn(response); | ||
|
|
||
| // when & then | ||
| mockMvc.perform(post("/api/v1/payments/{paymentKey}/cancel", paymentKey) | ||
| .with(csrf()) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .content(objectMapper.writeValueAsString(request))) | ||
| .andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.isSuccess").value(true)) | ||
| .andExpect(jsonPath("$.result.status").value("CANCELED")); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("결제 내역 조회 성공") | ||
| void getPaymentList_success() throws Exception { | ||
| // given | ||
| PaymentResponseDTO.PaginationDTO pagination = new PaymentResponseDTO.PaginationDTO(1, 1, 1L); | ||
| PaymentResponseDTO.PaymentHistoryResultDTO history = new PaymentResponseDTO.PaymentHistoryResultDTO( | ||
| 1L, 1L, "Store Name", BigDecimal.valueOf(10000), "DEPOSIT", "CARD", "TOSS", "COMPLETED", | ||
| LocalDateTime.now()); | ||
| PaymentResponseDTO.PaymentListResponseDTO response = new PaymentResponseDTO.PaymentListResponseDTO( | ||
| Collections.singletonList(history), pagination); | ||
|
|
||
| given(paymentService.getPaymentList(anyString(), any(Integer.class), any(Integer.class), any())) | ||
| .willReturn(response); | ||
|
|
||
| // when & then | ||
| mockMvc.perform(get("/api/v1/payments") | ||
| .param("page", "1") | ||
| .param("limit", "10") | ||
| .contentType(MediaType.APPLICATION_JSON)) | ||
| .andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.isSuccess").value(true)) | ||
| .andExpect(jsonPath("$.result.payments[0].storeName").value("Store Name")); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("결제 상세 조회 성공") | ||
| void getPaymentDetail_success() throws Exception { | ||
| // given | ||
| Long paymentId = 1L; | ||
| PaymentResponseDTO.PaymentDetailResultDTO response = new PaymentResponseDTO.PaymentDetailResultDTO( | ||
| paymentId, 1L, "Store Name", "CARD", "TOSS", BigDecimal.valueOf(10000), "DEPOSIT", | ||
| "COMPLETED", LocalDateTime.now(), LocalDateTime.now(), "http://receipt.url", null); | ||
|
|
||
| given(paymentService.getPaymentDetail(eq(paymentId), anyString())).willReturn(response); | ||
|
|
||
| // when & then | ||
| mockMvc.perform(get("/api/v1/payments/{paymentId}", paymentId) | ||
| .contentType(MediaType.APPLICATION_JSON)) | ||
| .andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.isSuccess").value(true)) | ||
| .andExpect(jsonPath("$.result.paymentId").value(paymentId)); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
성공 경로만 테스트되어 있습니다. 예외 케이스 테스트가 필요합니다.
Controller 단위 테스트에서 다음 케이스들이 누락되어 있습니다:
- 인증되지 않은 사용자 요청 (401)
- 유효성 검증 실패 (예:
bookingId가 null인 경우 400) - 서비스에서 예외 발생 시 에러 응답 (예:
PaymentException,UserException)
최소한 유효성 검증 실패와 서비스 예외 전파 테스트는 추가하는 것을 권장합니다.
As per coding guidelines, "경계값 테스트, 예외 케이스 테스트가 포함되어 있는지" 검토 항목에 해당합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/eatsfine/eatsfine/domain/payment/controller/PaymentControllerTest.java`
around lines 75 - 186, Tests only cover success paths; add tests for
unauthorized (401), validation failure (400) and service-exception flows. For
each controller endpoint used in PaymentControllerTest (POST
/api/v1/payments/request handled by paymentService.requestPayment, POST
/api/v1/payments/confirm -> paymentService.confirmPayment, POST
/api/v1/payments/{paymentKey}/cancel -> paymentService.cancelPayment, GET
/api/v1/payments and GET /api/v1/payments/{paymentId}): add a test that performs
the same mockMvc.perform call without authentication to assert 401, add a
validation-failure test (e.g., for requestPayment create a RequestPaymentDTO
with bookingId = null and assert status().isBadRequest()), and add
service-exception tests by stubbing paymentService methods with willThrow(new
PaymentException(...)) or willThrow(new UserException(...)) and asserting the
controller returns the mapped error response (non-success body or expected error
status). Use the existing unique symbols (paymentService.requestPayment,
paymentService.confirmPayment, paymentService.cancelPayment,
paymentService.getPaymentList, paymentService.getPaymentDetail, and
mockMvc.perform with the same endpoints) to locate where to add these tests.
src/test/java/com/eatsfine/eatsfine/domain/payment/controller/PaymentControllerTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/eatsfine/eatsfine/domain/payment/service/PaymentServiceTest.java
Show resolved
Hide resolved
src/test/java/com/eatsfine/eatsfine/domain/payment/service/PaymentServiceTest.java
Outdated
Show resolved
Hide resolved
| void getPaymentList_Customer_success() { | ||
| // given | ||
| User user = User.builder().id(1L).role(Role.ROLE_CUSTOMER).build(); | ||
| // Pageable pageable = PageRequest.of(0, 10); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
주석 처리된 코드를 제거해주세요.
// Pageable pageable = PageRequest.of(0, 10);가 두 군데(Line 236, 271)에 남아 있습니다. 사용하지 않는 코드이므로 삭제해주세요.
♻️ 주석 처리된 코드 제거
void getPaymentList_Customer_success() {
// given
User user = User.builder().id(1L).role(Role.ROLE_CUSTOMER).build();
- // Pageable pageable = PageRequest.of(0, 10);
Payment payment = Payment.builder() void getPaymentList_Owner_success() {
// given
User user = User.builder().id(2L).role(Role.ROLE_OWNER).build();
- // Pageable pageable = PageRequest.of(0, 10);
Payment payment = Payment.builder()Also applies to: 271-271
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/eatsfine/eatsfine/domain/payment/service/PaymentServiceTest.java`
at line 236, Remove the unused commented code lines containing "// Pageable
pageable = PageRequest.of(0, 10);" from the PaymentServiceTest class; locate the
commented occurrences near the test methods in
src/test/java/com/eatsfine/eatsfine/domain/payment/service/PaymentServiceTest.java
(search for the exact text "Pageable pageable = PageRequest.of(0, 10)") and
delete both commented instances so the test file contains no leftover
commented-out pageable declarations.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Around line 11-15: Reorder the DB keys in .env.example into strict
alphabetical order to satisfy dotenv-linter: place DB_HOST, DB_NAME,
DB_PASSWORD, DB_PORT, DB_USERNAME (or whichever alphabetical sequence your
linter expects) by moving the DB_NAME and DB_PASSWORD lines so they come before
DB_PORT; update the block containing DB_HOST, DB_PORT, DB_NAME, DB_USERNAME,
DB_PASSWORD accordingly referencing those exact keys.
- Around line 41-44: The .env.example is missing AWS_ACCESS_KEY_ID and
AWS_SECRET_ACCESS_KEY which are required for local dev when the AWS SDK v2 S3
client cannot use instance/ECS roles; add entries for AWS_ACCESS_KEY_ID and
AWS_SECRET_ACCESS_KEY (optionally AWS_SESSION_TOKEN) alongside AWS_REGION,
AWS_S3_BUCKET, and AWS_S3_BASE_URL so developers can set credentials for local
testing and avoid S3 authentication errors.
In
`@src/main/java/com/eatsfine/eatsfine/domain/payment/repository/PaymentRepository.java`:
- Around line 18-23: The countQuery in PaymentRepository methods
findAllByUserIdWithDetails and findAllByUserIdAndStatusWithDetails is missing
the JOIN on b.store, causing pagination counts to include bookings without a
store while the main query (which uses JOIN FETCH b.store) excludes them; update
both countQuery strings to include "JOIN b.store" (no FETCH) so the count uses
the same join/filtering logic as the main query and the pagination totals match
the returned pages.
In
`@src/main/java/com/eatsfine/eatsfine/domain/payment/service/PaymentService.java`:
- Around line 173-176: The code in PaymentService computes size from limit and
can pass zero or negative to PageRequest.of, causing an
IllegalArgumentException; validate and sanitize limit before building the
PageRequest: ensure size is at least 1 (e.g., if limit == null use 10, if limit
<= 0 treat as bad request) and if invalid return a 400 response (or throw a
custom BadRequestException) instead of letting PageRequest.of(pageNumber, size)
raise a 500; update the logic around limit/size and pageNumber and the call site
that invokes PageRequest.of to enforce this validation.
In
`@src/test/java/com/eatsfine/eatsfine/domain/payment/controller/PaymentControllerTest.java`:
- Line 243: The inline comment next to the `@WithMockUser`(roles = {}) annotation
in PaymentControllerTest (the test annotated with `@DisplayName` "서비스에서
UserException 발생 시 에러 응답") is misleading because `@WithMockUser` creates an
authenticated user with no roles, not an unauthenticated request; remove or
change the comment to accurately state "authenticated user with no roles" (or
simply delete the comment) so it doesn't claim a 401/unauthenticated
simulation—locate the `@WithMockUser`(roles = {}) usage in PaymentControllerTest
and update the comment text accordingly.
- Around line 237-239: The test currently asserts the error code with a
hardcoded string "BOOKING4001"; update the assertion to reference the canonical
constant so it stays in sync with changes by using
PaymentErrorStatus._BOOKING_NOT_FOUND.getCode() in the jsonPath expectation
(replace the hardcoded value in the .andExpect(jsonPath("$.code").value(...))
call inside PaymentControllerTest.java).
---
Duplicate comments:
In
`@src/test/java/com/eatsfine/eatsfine/domain/payment/controller/PaymentControllerTest.java`:
- Around line 237-239: The tests bypass the JWT auth filter in setUp() (via the
doAnswer stub), so add a new test that verifies an actual unauthenticated
request returns HTTP 401 by executing the controller with the real filter
behavior instead of the stub; create a separate test method (e.g.,
unauthenticatedRequestReturns401) or a `@Nested` test class that reconfigures/does
not stub the JWT filter used in PaymentControllerTest.setUp(), perform the same
request used in the failing scenarios, and assert status().isUnauthorized() and
the expected JSON body fields (isSuccess false, appropriate error code) to cover
the 401 scenario. Ensure the new test restores or isolates the mocked filter
state so it does not affect existing tests.
In
`@src/test/java/com/eatsfine/eatsfine/domain/payment/service/PaymentServiceTest.java`:
- Line 257: Remove the leftover commented-out line in the PaymentServiceTest
test class that defines a Pageable (the line "// Pageable pageable =
PageRequest.of(0, 10);"); open the PaymentServiceTest file, locate the commented
PageRequest/Pageable reference and delete that commented line to clean up unused
code (no behavior changes required).
Comment on lines
+11
to
+15
| DB_HOST=localhost | ||
| DB_PORT=3306 | ||
| DB_NAME=eatsfine_local | ||
| DB_USERNAME=root | ||
| DB_PASSWORD=your_password |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
dotenv-linter 경고: DB 섹션 키 순서
dotenv-linter가 DB_NAME(N < P)과 DB_PASSWORD(PAS < POR)가 알파벳 순서상 DB_PORT 앞에 와야 한다고 경고합니다. 현재 순서(HOST→PORT→NAME→USERNAME→PASSWORD)가 의미적으로 읽기 쉽다면 무시할 수도 있지만, 린터 일관성을 원할 경우 아래와 같이 정렬하세요.
🔧 제안: 알파벳 순서로 정렬
# ── Database (MySQL) ──────────────────────────
DB_HOST=localhost
-DB_PORT=3306
DB_NAME=eatsfine_local
-DB_USERNAME=root
DB_PASSWORD=your_password
+DB_PORT=3306
+DB_USERNAME=root📝 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
| DB_HOST=localhost | |
| DB_PORT=3306 | |
| DB_NAME=eatsfine_local | |
| DB_USERNAME=root | |
| DB_PASSWORD=your_password | |
| DB_HOST=localhost | |
| DB_NAME=eatsfine_local | |
| DB_PASSWORD=your_password | |
| DB_PORT=3306 | |
| DB_USERNAME=root |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 13-13: [UnorderedKey] The DB_NAME key should go before the DB_PORT key
(UnorderedKey)
[warning] 15-15: [UnorderedKey] The DB_PASSWORD key should go before the DB_PORT key
(UnorderedKey)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.env.example around lines 11 - 15, Reorder the DB keys in .env.example into
strict alphabetical order to satisfy dotenv-linter: place DB_HOST, DB_NAME,
DB_PASSWORD, DB_PORT, DB_USERNAME (or whichever alphabetical sequence your
linter expects) by moving the DB_NAME and DB_PASSWORD lines so they come before
DB_PORT; update the block containing DB_HOST, DB_PORT, DB_NAME, DB_USERNAME,
DB_PASSWORD accordingly referencing those exact keys.
src/main/java/com/eatsfine/eatsfine/domain/payment/repository/PaymentRepository.java
Outdated
Show resolved
Hide resolved
Comment on lines
173
to
176
| // limit 기본값 처리 (만약 null이면 10) | ||
| int size = (limit != null) ? limit : 10; | ||
| // page 기본값 처리 (만약 null이면 1, 0보다 작으면 1로 보정). Spring Data는 0-based index이므로 -1 | ||
| int pageNumber = (page != null && page > 0) ? page - 1 : 0; |
There was a problem hiding this comment.
limit ≤ 0 입력 시 PageRequest.of 에서 IllegalArgumentException 발생
limit이 0 또는 음수이면 size가 그대로 전달되어 PageRequest.of(pageNumber, 0) 호출 시 Spring이 "Page size must not be less than one" 예외를 던집니다. 이는 클라이언트 입력 오류이므로 400 응답이 적절하나 현재는 500으로 전파됩니다.
🛡️ 제안 수정
int size = (limit != null) ? limit : 10;
+if (size <= 0) {
+ throw new GeneralException(ErrorStatus._BAD_REQUEST);
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/eatsfine/eatsfine/domain/payment/service/PaymentService.java`
around lines 173 - 176, The code in PaymentService computes size from limit and
can pass zero or negative to PageRequest.of, causing an
IllegalArgumentException; validate and sanitize limit before building the
PageRequest: ensure size is at least 1 (e.g., if limit == null use 10, if limit
<= 0 treat as bad request) and if invalid return a 400 response (or throw a
custom BadRequestException) instead of letting PageRequest.of(pageNumber, size)
raise a 500; update the logic around limit/size and pageNumber and the call site
that invokes PageRequest.of to enforce this validation.
src/test/java/com/eatsfine/eatsfine/domain/payment/controller/PaymentControllerTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/eatsfine/eatsfine/domain/payment/controller/PaymentControllerTest.java
Outdated
Show resolved
Hide resolved
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 @.env.example:
- Around line 44-48: Reorder the AWS S3 environment variables so they are in
alphabetical order by key: place AWS_ACCESS_KEY_ID first, then AWS_REGION, then
AWS_S3_BASE_URL, then AWS_S3_BUCKET, and finally AWS_SECRET_ACCESS_KEY; keep the
existing values unchanged and ensure the keys AWS_ACCESS_KEY_ID, AWS_REGION,
AWS_S3_BASE_URL, AWS_S3_BUCKET, and AWS_SECRET_ACCESS_KEY are present exactly as
listed to satisfy dotenv-linter.
In
`@src/main/java/com/eatsfine/eatsfine/domain/payment/service/PaymentService.java`:
- Around line 325-334: The parsePaymentStatus method currently uses
status.isEmpty(), which doesn't reject whitespace-only strings; update the
null/empty check to use status.isBlank() (or status.trim().isEmpty()) so
whitespace-only inputs are treated as invalid before calling
PaymentStatus.valueOf; keep the existing catch throwing new
GeneralException(ErrorStatus._BAD_REQUEST) unchanged (refer to
parsePaymentStatus, PaymentStatus.valueOf, GeneralException and
ErrorStatus._BAD_REQUEST).
- Around line 168-193: getPaymentList currently sets size from limit without an
upper bound, allowing clients to request arbitrarily large pages; add a
MAX_PAGE_SIZE constant (e.g., 100) in PaymentService and clamp the computed size
with Math.min(size, MAX_PAGE_SIZE) (replace the existing size computation so it
first applies your existing null/<=0 fallback then caps to MAX_PAGE_SIZE) to
prevent excessive memory/query loads; reference PaymentService.getPaymentList,
the limit parameter, and the size variable when making this change.
---
Duplicate comments:
In @.env.example:
- Around line 11-15: The DB environment keys are not alphabetized and trigger
dotenv-linter's UnorderedKey; reorder the keys so they are in ASCII/alpha order
(DB_HOST, DB_NAME, DB_PASSWORD, DB_PORT, DB_USERNAME) by moving DB_NAME and
DB_PASSWORD before DB_PORT; update the .env example accordingly so DB_HOST,
DB_NAME, DB_PASSWORD, DB_PORT, DB_USERNAME appear in that sequence.
Comment on lines
+44
to
+48
| AWS_ACCESS_KEY_ID=your_aws_access_key_id | ||
| AWS_SECRET_ACCESS_KEY=your_aws_secret_access_key | ||
| AWS_REGION=ap-northeast-2 | ||
| AWS_S3_BUCKET=your-bucket-name | ||
| AWS_S3_BASE_URL=https://your-bucket-name.s3.ap-northeast-2.amazonaws.com |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
AWS S3 섹션 키 순서 — dotenv-linter 경고 3건
AWS_ACCESS_KEY_ID·AWS_SECRET_ACCESS_KEY 추가 후 블록 전체 알파벳 순서가 맞지 않아 경고가 발생합니다.
올바른 순서: AWS_ACCESS_KEY_ID → AWS_REGION → AWS_S3_BASE_URL → AWS_S3_BUCKET → AWS_SECRET_ACCESS_KEY
♻️ 제안: 알파벳 순서로 재정렬
# ── AWS S3 ────────────────────────────────────
# AWS IAM에서 액세스 키 발급 (로컬 개발 시 필수)
# https://console.aws.amazon.com/iam
AWS_ACCESS_KEY_ID=your_aws_access_key_id
-AWS_SECRET_ACCESS_KEY=your_aws_secret_access_key
AWS_REGION=ap-northeast-2
-AWS_S3_BUCKET=your-bucket-name
AWS_S3_BASE_URL=https://your-bucket-name.s3.ap-northeast-2.amazonaws.com
+AWS_S3_BUCKET=your-bucket-name
+AWS_SECRET_ACCESS_KEY=your_aws_secret_access_key🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 46-46: [UnorderedKey] The AWS_REGION key should go before the AWS_SECRET_ACCESS_KEY key
(UnorderedKey)
[warning] 47-47: [UnorderedKey] The AWS_S3_BUCKET key should go before the AWS_SECRET_ACCESS_KEY key
(UnorderedKey)
[warning] 48-48: [UnorderedKey] The AWS_S3_BASE_URL key should go before the AWS_S3_BUCKET key
(UnorderedKey)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.env.example around lines 44 - 48, Reorder the AWS S3 environment variables
so they are in alphabetical order by key: place AWS_ACCESS_KEY_ID first, then
AWS_REGION, then AWS_S3_BASE_URL, then AWS_S3_BUCKET, and finally
AWS_SECRET_ACCESS_KEY; keep the existing values unchanged and ensure the keys
AWS_ACCESS_KEY_ID, AWS_REGION, AWS_S3_BASE_URL, AWS_S3_BUCKET, and
AWS_SECRET_ACCESS_KEY are present exactly as listed to satisfy dotenv-linter.
Comment on lines
168
to
193
| @Transactional(readOnly = true) | ||
| public PaymentResponseDTO.PaymentListResponseDTO getPaymentList(Long userId, Integer page, Integer limit, | ||
| public PaymentResponseDTO.PaymentListResponseDTO getPaymentList(String email, Integer page, Integer limit, | ||
| String status) { | ||
| // limit 기본값 처리 (만약 null이면 10) | ||
| int size = (limit != null) ? limit : 10; | ||
| User user = userRepository.findByEmail(email) | ||
| .orElseThrow(() -> new UserException(UserErrorStatus.MEMBER_NOT_FOUND)); | ||
| // limit 기본값 처리 (null이거나 0 이하이면 10) | ||
| int size = (limit != null && limit > 0) ? limit : 10; | ||
| // page 기본값 처리 (만약 null이면 1, 0보다 작으면 1로 보정). Spring Data는 0-based index이므로 -1 | ||
| int pageNumber = (page != null && page > 0) ? page - 1 : 0; | ||
|
|
||
| Pageable pageable = PageRequest.of(pageNumber, size); | ||
|
|
||
| PaymentStatus paymentStatus = parsePaymentStatus(status); | ||
|
|
||
| Page<Payment> paymentPage; | ||
| if (status != null && !status.isEmpty()) { | ||
| PaymentStatus paymentStatus; | ||
| try { | ||
| paymentStatus = PaymentStatus.valueOf(status.toUpperCase()); | ||
| } catch (IllegalArgumentException e) { | ||
| // 유효하지 않은 status가 들어오면 BadRequest 예외 발생 | ||
| throw new GeneralException(ErrorStatus._BAD_REQUEST); | ||
| } | ||
| paymentPage = paymentRepository.findAllByBooking_User_IdAndPaymentStatus(userId, paymentStatus, | ||
| pageable); | ||
| if (user.getRole() == Role.ROLE_OWNER) { | ||
| paymentPage = (paymentStatus != null) | ||
| ? paymentRepository.findAllByOwnerIdAndStatusWithDetails(user.getId(), | ||
| paymentStatus, pageable) | ||
| : paymentRepository.findAllByOwnerIdWithDetails(user.getId(), pageable); | ||
| } else { | ||
| paymentPage = paymentRepository.findAllByBooking_User_Id(userId, pageable); | ||
| paymentPage = (paymentStatus != null) | ||
| ? paymentRepository.findAllByUserIdAndStatusWithDetails(user.getId(), | ||
| paymentStatus, pageable) | ||
| : paymentRepository.findAllByUserIdWithDetails(user.getId(), pageable); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
이메일 기반 접근 제어와 역할 분기 로직이 깔끔하게 구현되었습니다.
parsePaymentStatus 헬퍼 추출, limit 기본값/음수 처리, 역할 기반 분기의 삼항 연산자 전환 모두 이전 리뷰 피드백이 잘 반영되었습니다.
한 가지 개선 사항으로, limit에 상한값(예: 100)을 두지 않으면 클라이언트가 극단적으로 큰 값을 전달하여 과도한 메모리 사용 및 쿼리 부하를 유발할 수 있습니다.
♻️ limit 상한 제한 제안
- int size = (limit != null && limit > 0) ? limit : 10;
+ int size = (limit != null && limit > 0) ? Math.min(limit, 100) : 10;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/com/eatsfine/eatsfine/domain/payment/service/PaymentService.java`
around lines 168 - 193, getPaymentList currently sets size from limit without an
upper bound, allowing clients to request arbitrarily large pages; add a
MAX_PAGE_SIZE constant (e.g., 100) in PaymentService and clamp the computed size
with Math.min(size, MAX_PAGE_SIZE) (replace the existing size computation so it
first applies your existing null/<=0 fallback then caps to MAX_PAGE_SIZE) to
prevent excessive memory/query loads; reference PaymentService.getPaymentList,
the limit parameter, and the size variable when making this change.
src/main/java/com/eatsfine/eatsfine/domain/payment/service/PaymentService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
🤖 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/com/eatsfine/eatsfine/domain/payment/service/PaymentService.java`:
- Around line 226-231: The code accesses
payment.getBooking().getStore().getOwner().getId() while findByIdWithDetails
only JOIN FETCHes b.store, causing extra lazy-load queries and potential NPE;
update the PaymentRepository.findByIdWithDetails JPQL to fetch the store owner
as well by changing "JOIN FETCH p.booking b JOIN FETCH b.store JOIN FETCH
b.user" to "JOIN FETCH p.booking b JOIN FETCH b.store s JOIN FETCH s.owner JOIN
FETCH b.user" so store.owner is loaded eagerly and no extra query or null-access
occurs.
- Line 174: The current limit handling sets size to a default when limit ≤ 0 but
does not enforce an upper bound; update PaymentService to cap the requested
limit to a safe maximum (e.g., 100). Replace the existing size calculation so it
uses the positive limit but bounded by a constant MAX_LIMIT (define MAX_LIMIT =
100 near the class or method) and fall back to the default (10) when limit is
null or ≤ 0; reference the limit variable and the size assignment in
PaymentService to locate and apply this change.
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
1 participant
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.
💡 작업 개요
✅ 작업 내용
🧪 테스트 내용
📝 기타 참고 사항
Summary by CodeRabbit
새로운 기능
개선사항
테스트
기타