Merged
Conversation
Contributor
Walkthrough배송지 관리 기능을 추가합니다. 신규 컨트롤러, 서비스, 변환 유틸, 요청/응답 DTO, 엔티티 업데이트 메서드, 저장소 쿼리, 유효성 검증 어노테이션 및 주소 관련 에러 코드가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as 클라이언트
participant Controller as AddressController
participant Service as AddressService
participant Repo as AddressRepository
participant DB as Database
Client->>Controller: GET /api/addresses/me (userId)
Controller->>Service: getShippingInfo(userId)
Service->>Repo: findFirstByUserIdOrderByCreatedAtDesc(userId)
Repo->>DB: SELECT ... (최신 주소)
DB-->>Repo: Address or null
Repo-->>Service: Address?
Service-->>Controller: ShippingInfoResDto
Controller-->>Client: DataResponse(ShippingInfoResDto)
sequenceDiagram
autonumber
participant Client as 클라이언트
participant Controller as AddressController
participant Service as AddressService
participant Repo as AddressRepository
participant DB as Database
Client->>Controller: POST /api/addresses (userId, request)
Controller->>Service: createShippingInfo(userId, request)
Service->>Repo: deleteAllByUserId(userId)
Repo->>DB: DELETE FROM address WHERE user_id=...
Service->>Service: AddressConverter.toEntity(user, request)
Service->>Repo: save(Address)
Repo->>DB: INSERT INTO address ...
DB-->>Repo: saved Address
Repo-->>Service: saved Address
Service-->>Controller: ShippingInfoResDto
Controller-->>Client: DataResponse(ShippingInfoResDto)
sequenceDiagram
autonumber
participant Client as 클라이언트
participant Controller as AddressController
participant Service as AddressService
participant Repo as AddressRepository
participant DB as Database
Client->>Controller: PATCH /api/addresses/{addressId} (userId, request)
Controller->>Service: updateShippingInfo(userId, addressId, request)
Service->>Repo: findById(addressId)
Repo->>DB: SELECT ... WHERE id=...
DB-->>Repo: Address or null
Repo-->>Service: Address?
alt Address null
Service-->>Controller: throw ADDRESS_NOT_FOUND
else Owner mismatch
Service-->>Controller: throw ADDRESS_FORBIDDEN
else OK
Service->>Service: address.update(...)
Service->>Repo: (JPA flush via tx)
Repo->>DB: UPDATE address ...
Service-->>Controller: ShippingInfoResDto
Controller-->>Client: DataResponse(ShippingInfoResDto)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
`@src/main/java/com/ongil/backend/domain/address/dto/request/ShippingInfoUpdateReqDto.java`:
- Around line 1-9: 업데이트 DTO ShippingInfoUpdateReqDto의 일부 필드(recipientName,
baseAddress, postalCode, phone)가 null은 허용하되 빈 문자열/공백만 들어오는 것을 막아야 합니다; 각 해당
컴포넌트에 Bean Validation의 `@Pattern`(regexp=".*\\S.*", message="값이 존재할 경우 공백만 허용할 수
없습니다")를 추가하세요(`@Pattern은` null을 유효로 처리하므로 부분 업데이트에서 null은 유지되고 빈 문자열·공백만 거부됩니다).
필요시 커스텀 메시지를 지정하고 import javax.validation.constraints.Pattern를 추가해 주세요.
In `@src/main/java/com/ongil/backend/domain/address/entity/Address.java`:
- Around line 62-85: Change the nullable Boolean usage in the Address entity to
a primitive boolean to match the `@Column`(nullable = false) constraint: update
the isDefault field's type to boolean, change the constructor parameter
(currently at the constructor that sets isDefault) from Boolean to boolean, and
change the setDefault(Boolean isDefault) method signature to setDefault(boolean
isDefault) so null cannot be assigned; remove any null checks related to
isDefault and ensure assignments use the primitive field and parameter
consistently in the Address class.
In
`@src/main/java/com/ongil/backend/domain/address/repository/AddressRepository.java`:
- Around line 10-14: findByUserId currently returns Optional<Address> but
User->Address is 1:N so calling it can throw
IncorrectResultSizeDataAccessException when multiple addresses exist; change the
repository to return the single default address by filtering on the isDefault
field (e.g. replace or add a method such as Optional<Address>
findByUserIdAndIsDefaultTrue(Long userId) or Optional<Address>
findDefaultByUserId(Long userId) that queries isDefault == true) and keep
findAllByUserId as-is for multi-results; update any service/usage that called
findByUserId to use the new default-address method name.
In `@src/main/java/com/ongil/backend/domain/address/service/AddressService.java`:
- Around line 44-58: Add a repository-level bulk delete to avoid loading
entities: declare void deleteByUserId(Long userId) in AddressRepository, then in
AddressService.createShippingInfo replace the existing findAllByUserId +
addressRepository.deleteAll(existingAddresses) sequence with a call to
addressRepository.deleteByUserId(userId) before converting and saving the new
Address (keep the User lookup and AddressConverter usage intact).
🧹 Nitpick comments (1)
src/main/java/com/ongil/backend/domain/address/service/AddressService.java (1)
31-42: 빈 응답 구성 로직을 Converter로 위임하면 중복을 줄일 수 있어요.
비어있는 경우도 AddressConverter에 위임하면 응답 규격이 더 일관됩니다.
src/main/java/com/ongil/backend/domain/address/dto/request/ShippingInfoUpdateReqDto.java
Show resolved
Hide resolved
Comment on lines
+10
to
+14
| public interface AddressRepository extends JpaRepository<Address, Long> { | ||
|
|
||
| Optional<Address> findByUserId(Long userId); | ||
|
|
||
| List<Address> findAllByUserId(Long userId); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# AddressRepository.java 전체 파일 확인
cat -n src/main/java/com/ongil/backend/domain/address/repository/AddressRepository.javaRepository: IT-Cotato/12th-OnGil-BE
Length of output: 565
🏁 Script executed:
# Address 엔티티 파일 위치 찾기
fd -e java "Address\.java" --type f | grep -E "entity|domain"Repository: IT-Cotato/12th-OnGil-BE
Length of output: 133
🏁 Script executed:
# findByUserId 메서드 사용 위치 확인
rg -n "findByUserId" --type=java -B2 -A2Repository: IT-Cotato/12th-OnGil-BE
Length of output: 11259
🏁 Script executed:
# isDefault 필드 존재 여부 확인
rg -n "isDefault" --type=java -B2 -A2Repository: IT-Cotato/12th-OnGil-BE
Length of output: 2671
🏁 Script executed:
# Address 엔티티 정의 확인 (파일 크기 먼저 확인)
wc -l $(fd -e java Address.java --path "*/domain/address/*" | head -1)Repository: IT-Cotato/12th-OnGil-BE
Length of output: 319
🏁 Script executed:
# Address 엔티티 전체 확인
cat -n src/main/java/com/ongil/backend/domain/address/entity/Address.javaRepository: IT-Cotato/12th-OnGil-BE
Length of output: 3096
🏁 Script executed:
# AddressService.getShippingInfo 메서드 확인
sed -n '30,40p' src/main/java/com/ongil/backend/domain/address/service/AddressService.javaRepository: IT-Cotato/12th-OnGil-BE
Length of output: 335
🏁 Script executed:
# AddressRepository의 다른 메서드나 제약 조건 확인
cat -n src/main/java/com/ongil/backend/domain/address/repository/AddressRepository.javaRepository: IT-Cotato/12th-OnGil-BE
Length of output: 565
🏁 Script executed:
# Address와 User 사이의 관계 확인 (1:1 or 1:N)
rg -n "Address" src/main/java/com/ongil/backend/domain/user/entity/ --type=java -B2 -A2Repository: IT-Cotato/12th-OnGil-BE
Length of output: 1073
findByUserId 단건 반환은 다건 결과 시 런타임 예외 위험
User-Address 관계가 1:N이고 유니크 제약이 없으므로, findByUserId()가 Optional
IncorrectResultSizeDataAccessException을 발생시킵니다. Address 엔티티의 isDefault 필드를 활용하여 기본 배송지만 조회하도록 수정하세요.
✅ 제안 수정안
-Optional<Address> findByUserId(Long userId);
+Optional<Address> findByUserIdAndIsDefaultTrue(Long userId);
List<Address> findAllByUserId(Long userId);🤖 Prompt for AI Agents
In
`@src/main/java/com/ongil/backend/domain/address/repository/AddressRepository.java`
around lines 10 - 14, findByUserId currently returns Optional<Address> but
User->Address is 1:N so calling it can throw
IncorrectResultSizeDataAccessException when multiple addresses exist; change the
repository to return the single default address by filtering on the isDefault
field (e.g. replace or add a method such as Optional<Address>
findByUserIdAndIsDefaultTrue(Long userId) or Optional<Address>
findDefaultByUserId(Long userId) that queries isDefault == true) and keep
findAllByUserId as-is for multi-results; update any service/usage that called
findByUserId to use the new default-address method name.
src/main/java/com/ongil/backend/domain/address/service/AddressService.java
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/java/com/ongil/backend/domain/address/entity/Address.java`:
- Around line 62-82: The update() method in Address uses hasText() so empty
strings are ignored, causing silent no-ops when clients submit blank required
fields; to fix, add `@NotBlank` to the required fields (recipientName,
baseAddress, postalCode, phone/recipientPhone) in ShippingInfoUpdateReqDto
consistent with ShippingInfoCreateReqDto and ensure the controller uses `@Valid`
(or `@Validated`) on the DTO parameter so validation fails fast instead of
returning 200 with no DB change.
🧹 Nitpick comments (1)
src/main/java/com/ongil/backend/domain/address/entity/Address.java (1)
88-90: SpringStringUtils.hasText()활용 고려Spring Framework에서 이미 동일한 기능을 제공하므로 중복 구현 대신 활용할 수 있습니다. 단, 현재 구현도 간단하고 명확하여 유지해도 무방합니다.
♻️ Spring StringUtils 활용 예시
+import org.springframework.util.StringUtils; + public void update(String recipientName, String recipientPhone, String baseAddress, String detailAddress, String postalCode, String deliveryRequest) { - if (hasText(recipientName)) { + if (StringUtils.hasText(recipientName)) { this.recipientName = recipientName; } // ... 나머지 필드도 동일하게 변경 } -private boolean hasText(String value) { - return value != null && !value.isBlank(); -}
Comment on lines
+62
to
+82
| public void update(String recipientName, String recipientPhone, String baseAddress, | ||
| String detailAddress, String postalCode, String deliveryRequest) { | ||
| if (hasText(recipientName)) { | ||
| this.recipientName = recipientName; | ||
| } | ||
| if (hasText(recipientPhone)) { | ||
| this.recipientPhone = recipientPhone; | ||
| } | ||
| if (hasText(baseAddress)) { | ||
| this.baseAddress = baseAddress; | ||
| } | ||
| if (detailAddress != null) { | ||
| this.detailAddress = detailAddress; | ||
| } | ||
| if (hasText(postalCode)) { | ||
| this.postalCode = postalCode; | ||
| } | ||
| if (deliveryRequest != null) { | ||
| this.deliveryRequest = deliveryRequest; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Address.java 전체 파일 확인
find . -type f -name "Address.java" -path "*/domain/address/*"Repository: IT-Cotato/12th-OnGil-BE
Length of output: 135
🏁 Script executed:
# Read Address.java file
wc -l ./src/main/java/com/ongil/backend/domain/address/entity/Address.javaRepository: IT-Cotato/12th-OnGil-BE
Length of output: 138
🏁 Script executed:
# Read the entire Address.java file
cat -n ./src/main/java/com/ongil/backend/domain/address/entity/Address.javaRepository: IT-Cotato/12th-OnGil-BE
Length of output: 3217
🏁 Script executed:
# Find AddressService to see how update() is called
find . -type f -name "*AddressService*" -path "*/service/*"Repository: IT-Cotato/12th-OnGil-BE
Length of output: 143
🏁 Script executed:
# Read AddressService
cat -n ./src/main/java/com/ongil/backend/domain/address/service/AddressService.javaRepository: IT-Cotato/12th-OnGil-BE
Length of output: 3271
🏁 Script executed:
# Find the DTO classes
find . -type f -name "ShippingInfoUpdateReqDto*"Repository: IT-Cotato/12th-OnGil-BE
Length of output: 157
🏁 Script executed:
# Read the DTO class
cat -n ./src/main/java/com/ongil/backend/domain/address/dto/request/ShippingInfoUpdateReqDto.javaRepository: IT-Cotato/12th-OnGil-BE
Length of output: 370
🏁 Script executed:
# Find AddressController or related endpoints
find . -type f -name "*AddressController*" -o -name "*ShippingController*" | head -5Repository: IT-Cotato/12th-OnGil-BE
Length of output: 149
🏁 Script executed:
# Read AddressController
cat -n ./src/main/java/com/ongil/backend/domain/address/controller/AddressController.javaRepository: IT-Cotato/12th-OnGil-BE
Length of output: 2910
🏁 Script executed:
# Check if validation annotations exist in request DTOs
find . -type f -name "ShippingInfoCreateReqDto*"Repository: IT-Cotato/12th-OnGil-BE
Length of output: 157
🏁 Script executed:
# Read ShippingInfoCreateReqDto
cat -n ./src/main/java/com/ongil/backend/domain/address/dto/request/ShippingInfoCreateReqDto.javaRepository: IT-Cotato/12th-OnGil-BE
Length of output: 651
🏁 Script executed:
# Search for any validation decorators in the update DTO or related code
rg -n "@NotBlank|@NotNull|@Valid" ./src/main/java/com/ongil/backend/domain/address/ --type=javaRepository: IT-Cotato/12th-OnGil-BE
Length of output: 872
부분 업데이트 시 필드 검증 일관성 부재
ShippingInfoUpdateReqDto에 필수 필드에 대한 @NotBlank 검증이 없어, 사용자가 필수 필드에 빈 문자열을 전달하면 update() 메서드의 hasText() 검증이 무시하고 DB에는 반영되지 않으면서도 API는 성공 응답합니다:
| 상황 | 결과 |
|---|---|
| 필수 필드에 빈 문자열 전달 | API 200 응답 → 실제 업데이트 안 됨 |
| 선택 필드에 빈 문자열 전달 | API 200 응답 → 필드값이 빈 문자열로 변경 |
ShippingInfoCreateReqDto처럼 ShippingInfoUpdateReqDto의 필수 필드(recipientName, baseAddress, postalCode, phone)에도 @NotBlank 검증을 추가하면, 컨트롤러에서 요청 검증 단계에 잡히므로 침묵하는 실패를 방지할 수 있습니다.
🤖 Prompt for AI Agents
In `@src/main/java/com/ongil/backend/domain/address/entity/Address.java` around
lines 62 - 82, The update() method in Address uses hasText() so empty strings
are ignored, causing silent no-ops when clients submit blank required fields; to
fix, add `@NotBlank` to the required fields (recipientName, baseAddress,
postalCode, phone/recipientPhone) in ShippingInfoUpdateReqDto consistent with
ShippingInfoCreateReqDto and ensure the controller uses `@Valid` (or `@Validated`)
on the DTO parameter so validation fails fast instead of returning 200 with no
DB 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
New Features
Enhancements
Errors
✏️ Tip: You can customize this high-level summary in your review settings.