-
Notifications
You must be signed in to change notification settings - Fork 0
feat: 유저 펫 조회 api 추가 #156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: 유저 펫 조회 api 추가 #156
Conversation
@@ -32,7 +32,6 @@ public class ApiV1NotiController { | |||
private final NotiService notiService; | |||
private final UserService userService; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ApiV1NotiController에서 코드 한 줄이 삭제되었습니다. 의도한 변경인지 확인하고, 삭제된 코드의 기능이 다른 곳에서 처리되는지 확인해야 합니다. 삭제 이유를 commit message에 명확히 명시해주세요.
@@ -12,8 +12,6 @@ | |||
import org.springframework.security.core.annotation.AuthenticationPrincipal; | |||
import org.springframework.web.bind.annotation.*; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PetController의 getAllPets 메서드가 삭제되었습니다. 이 메서드는 모든 펫을 조회하는 기능을 담당했는데, 삭제 이유와 대체 로직이 있는지 확인해야 합니다. 삭제 이유를 commit message에 명확히 명시해주세요.
@@ -1,20 +1,18 @@ | |||
package cmf.commitField.domain.pet.controller; | |||
|
|||
|
|||
import cmf.commitField.domain.pet.entity.Pet; | |||
import cmf.commitField.domain.pet.dto.UserPetListDto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pet 엔티티 대신 UserPetListDto를 import 합니다. DTO를 사용하는 것은 좋은데, UserPetListDto의 내용을 확인하여 적절한지 검토해야 합니다.
import cmf.commitField.domain.pet.service.PetService; | ||
import cmf.commitField.domain.pet.service.UserPetService; | ||
import cmf.commitField.domain.user.entity.User; | ||
import cmf.commitField.domain.user.entity.CustomOAuth2User; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User 엔티티 대신 CustomOAuth2User를 import 합니다. Spring Security의 AuthenticationPrincipal 어노테이션과 함께 사용하는 것으로 보이며, 사용자 인증 방식과 일관성을 유지하는지 확인해야 합니다.
import cmf.commitField.domain.pet.service.PetService; | ||
import cmf.commitField.domain.pet.service.UserPetService; | ||
import cmf.commitField.domain.user.entity.User; | ||
import cmf.commitField.domain.user.entity.CustomOAuth2User; | ||
import cmf.commitField.domain.user.service.CustomOAuth2UserService; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GlobalResponse와 관련된 import 문이 삭제되었습니다. GlobalResponse 클래스가 어떤 역할을 했는지 확인하고, 삭제된 이유와 대체 로직이 있는지 확인해야 합니다. ResponseEntity로 변경된 부분과의 연관성을 확인하세요.
import lombok.RequiredArgsConstructor; | ||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.security.core.annotation.AuthenticationPrincipal; | ||
import org.springframework.web.bind.annotation.GetMapping; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PathVariable을 사용한 userId 파라미터가 삭제되고, @AuthenticationPrincipal을 사용하여 CustomOAuth2User를 주입받도록 변경되었습니다. 사용자 인증 방식이 변경된 것으로 보이며, 보안 및 기능적인 측면에서 검토가 필요합니다. 이 변경으로 인해 발생할 수 있는 문제점을 고려해야 합니다.
import org.springframework.web.bind.annotation.GetMapping; | ||
import org.springframework.web.bind.annotation.PathVariable; | ||
import org.springframework.web.bind.annotation.RequestMapping; | ||
import org.springframework.web.bind.annotation.RestController; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getUserPetCollection 메서드의 반환 타입이 GlobalResponse<List>에서 ResponseEntity로 변경되었습니다. GlobalResponse를 사용하지 않는 이유를 확인하고, ResponseEntity를 사용하는 것이 적절한지 확인해야 합니다.
@@ -24,11 +22,11 @@ public class UserPetController { | |||
private final CustomOAuth2UserService customOAuth2UserService; | |||
private final PetService petService; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
주석 // TODO: 기능 확장시 추가 예정 이 삭제되었습니다. TODO 주석은 개발 과정에서 중요한 정보이므로, 삭제 이유를 확인해야 합니다. 만약 기능 확장 계획이 변경되었다면, commit message에 명확히 기록해야 합니다.
public GlobalResponse<List<Pet>> getUserPetCollection(@PathVariable Long userId) { | ||
User user = customOAuth2UserService.getUserById(userId).orElse(null); | ||
return GlobalResponse.success(userPetService.getUserPetCollection(user)); | ||
@GetMapping("/collection") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
기존의 getUserPetCollection 메소드가 수정되었습니다. PathVariable userId를 사용하지 않고, AuthenticationPrincipal을 이용하여 사용자 정보를 가져오도록 변경되었습니다. 보안 및 성능 측면을 고려하여 이 변경이 적절한지 검토해야 합니다.
@@ -0,0 +1,17 @@ | |||
package cmf.commitField.domain.pet.dto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
새로운 DTO 클래스 UserPetListDto가 추가되었습니다. 이 DTO가 어떤 목적으로 사용되는지 확인하고, 필드의 타입과 이름이 적절한지 검토해야 합니다. 특히, petList Map의 Integer 키는 펫 타입을 나타내는 것으로 보이는데, 100개의 타입을 지원하는 것이 적절한지 확인하고, 더 효율적인 방법이 있는지 고려해봐야 합니다. FIXME: 차후 갯수 수정 필요
부분을 수정해야 합니다. 어떤 방식으로 펫 타입의 갯수를 동적으로 관리할 수 있을지 고려해야 합니다. 예를 들어, enum을 사용하는 것이 좋을 수 있습니다.
@@ -2,6 +2,7 @@ | |||
|
|||
import cmf.commitField.domain.pet.dto.PetsDto; | |||
import cmf.commitField.domain.pet.dto.UserPetDto; | |||
import cmf.commitField.domain.pet.dto.UserPetListDto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UserPetListDto import 추가. DTO를 사용하는 것은 좋은데, UserPetListDto의 내용을 확인하여 적절한지 검토해야 합니다.
// 모든 펫 조회 | ||
public List<Pet> getAllPets() { | ||
return petRepository.findAll(); | ||
// 유저가 소유한 펫 조회 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getAllPets 메서드가 수정되었습니다. 이제 사용자 이름을 받아서 해당 사용자가 소유한 펫 목록을 UserPetListDto 형태로 반환합니다. petsNum
배열의 크기가 100으로 고정되어 있는데, 이는 매우 비효율적이며, 향후 펫 타입이 100개를 넘어가면 문제가 발생합니다. FIXME: 차후 갯수 수정 필요
라는 주석이 있는데, 더 나은 데이터 구조 (예: Map)를 사용하여 펫 타입과 개수를 관리해야 합니다. 또한, max
변수를 사용하여 최대 펫 타입을 찾는 부분도 비효율적입니다. Map을 사용하면 이 부분이 필요 없습니다. 전반적으로 코드의 효율성을 개선해야 합니다.
@@ -32,7 +32,6 @@ public class ApiV1NotiController { | |||
private final NotiService notiService; | |||
private final UserService userService; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 라인의 코드 삭제 이유가 명확하지 않습니다. 삭제 이유를 주석으로 명시해주세요. 혹시 실수로 삭제한 부분이라면 원복하는 것이 좋겠습니다.
@@ -21,6 +21,15 @@ public class PetController { | |||
private final CustomOAuth2UserService userService; | |||
private final PetService petService; | |||
|
|||
// 현재 펫 경험치 상승 및 상승 시 레벨업 처리 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
새로운 API 엔드포인트 추가입니다. 좋은데, @GetMapping("/getall")
경로는 getAllPets
보다 구체적인 이름으로 변경하는 것이 좋겠습니다. 예를 들어 /myPets
와 같이요. 또한, CustomOAuth2User
에서 username을 가져오는 방식은 보안상의 문제가 발생할 수 있습니다. username을 가져오는 부분에 대한 보안 검토가 필요합니다. 만약 CustomOAuth2User
가 user ID를 제공한다면 ID를 이용하여 petService.getAllPets
를 호출하도록 변경해야합니다.
List<UserPetDto> userPetDto = petService.getAllPets(username); | ||
return ResponseEntity.ok(userPetDto); | ||
} | ||
|
||
// 현재 펫 경험치 상승 및 상승 시 레벨업 처리 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ResponseEntity<List<UserPetDto>>
반환 타입은 좋은 선택입니다. 에러 핸들링을 명확하게 하기 위해 예외처리(예: try-catch
블록)를 추가하는 것을 고려해 보세요.
@@ -54,11 +63,6 @@ public ResponseEntity<Pet> createPet(@AuthenticationPrincipal CustomOAuth2User o | |||
return ResponseEntity.ok(pet); | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
기존의 getAllPets
메소드가 삭제되었습니다. 삭제 이유와 @GetMapping("/getall")
메서드와의 관계를 주석으로 명확히 설명해주세요. 두 메서드가 중복되는 기능을 가지고 있었다면, 왜 새 메서드로 대체되었는지에 대한 설명이 필요합니다.
@@ -1,34 +1,32 @@ | |||
package cmf.commitField.domain.pet.controller; | |||
|
|||
|
|||
import cmf.commitField.domain.pet.entity.Pet; | |||
import cmf.commitField.domain.pet.dto.UserPetListDto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
불필요한 import문(Pet
)이 삭제되었습니다. 코드 정리에 도움이 됩니다.
import cmf.commitField.domain.pet.service.PetService; | ||
import cmf.commitField.domain.pet.service.UserPetService; | ||
import cmf.commitField.domain.user.entity.User; | ||
import cmf.commitField.domain.user.entity.CustomOAuth2User; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User
import문이 CustomOAuth2User
로 변경되었습니다. 변경 사유에 대한 주석이 필요합니다. CustomOAuth2User
사용으로 인해 권한 관리에 영향이 있는지 확인해야 합니다.
import cmf.commitField.domain.pet.service.PetService; | ||
import cmf.commitField.domain.pet.service.UserPetService; | ||
import cmf.commitField.domain.user.entity.User; | ||
import cmf.commitField.domain.user.entity.CustomOAuth2User; | ||
import cmf.commitField.domain.user.service.CustomOAuth2UserService; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
불필요한 import문(GlobalResponse
, CustomOAuth2UserService
)이 삭제되었습니다. 코드 정리에 도움이 됩니다. CustomOAuth2UserService
의 사용이 사라진 이유를 확인해야 합니다. 해당 서비스에서 제공하던 기능이 다른 곳으로 이동했는지 확인해야 합니다.
import lombok.RequiredArgsConstructor; | ||
import org.springframework.http.ResponseEntity; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
필요한 import문(ResponseEntity
, AuthenticationPrincipal
)이 추가되었습니다. @AuthenticationPrincipal
어노테이션을 사용하는 것은 좋습니다.
import lombok.RequiredArgsConstructor; | ||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.security.core.annotation.AuthenticationPrincipal; | ||
import org.springframework.web.bind.annotation.GetMapping; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
불필요한 import문(PathVariable
)이 삭제되었습니다. 코드 정리에 도움이 됩니다.
import org.springframework.web.bind.annotation.GetMapping; | ||
import org.springframework.web.bind.annotation.PathVariable; | ||
import org.springframework.web.bind.annotation.RequestMapping; | ||
import org.springframework.web.bind.annotation.RestController; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
불필요한 import문(List
)이 삭제되었습니다. 코드 정리에 도움이 됩니다.
@RestController | ||
@RequestMapping("/user-pets") | ||
@RequestMapping("/api/user/") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RequestMapping
경로가 /user-pets
에서 /api/user/
로 변경되었습니다. 변경 이유를 명시해주세요. API 경로 설계 가이드라인과 일치하는지 확인해야 합니다.
@RestController | ||
@RequestMapping("/user-pets") | ||
@RequestMapping("/api/user/") | ||
@RequiredArgsConstructor | ||
public class UserPetController { | ||
|
||
private final UserPetService userPetService; | ||
private final CustomOAuth2UserService customOAuth2UserService; | ||
private final PetService petService; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
기존의 getUserPetCollection
메서드가 @AuthenticationPrincipal
을 사용하는 새로운 메서드로 변경되었습니다. 이는 보안 및 사용자 인증 측면에서 개선된 부분입니다. 하지만, userId
를 통한 접근 방식은 삭제되었는데, 이 부분의 필요성 여부를 재고해야 합니다. 특정 상황에서 userId
를 통한 접근이 필요할 수 있습니다.
public GlobalResponse<List<Pet>> getUserPetCollection(@PathVariable Long userId) { | ||
User user = customOAuth2UserService.getUserById(userId).orElse(null); | ||
return GlobalResponse.success(userPetService.getUserPetCollection(user)); | ||
@GetMapping("/collection") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ResponseEntity<UserPetListDto>
반환 타입은 좋은 선택입니다. 에러 핸들링을 위해 예외처리(예: try-catch
블록)를 추가하는 것을 고려해 보세요.
@@ -14,5 +14,6 @@ public class UserPetDto { | |||
private long petId; | |||
private String petName; | |||
private String grow; | |||
private int type; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type
필드가 추가되었습니다. 필드에 대한 설명을 주석으로 추가해야 합니다. 데이터 타입의 적절성(int
)도 확인해야 합니다.
@@ -0,0 +1,17 @@ | |||
package cmf.commitField.domain.pet.dto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
새로운 DTO 클래스가 추가되었습니다. petList
Map의 키(Integer)는 펫 타입을 나타내는 것으로 보이는데, 키값의 범위 및 유효성 검증에 대한 고려가 필요합니다. 또한, petList
값의 타입을 명확히 하였는지 확인해야 합니다. (현재는 소유 갯수로 보임)
@@ -2,6 +2,7 @@ | |||
|
|||
import cmf.commitField.domain.pet.dto.PetsDto; | |||
import cmf.commitField.domain.pet.dto.UserPetDto; | |||
import cmf.commitField.domain.pet.dto.UserPetListDto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
필요한 import문(UserPetListDto
)이 추가되었습니다.
// 모든 펫 조회 | ||
public List<Pet> getAllPets() { | ||
return petRepository.findAll(); | ||
public List<UserPetDto> getAllPets(String username){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
새로운 메서드 getAllPets(String username)
가 추가되었습니다. username을 사용하는 것은 좋지만, SQL Injection 취약성을 방지하기 위해 PreparedStatement를 사용하는 것이 중요합니다. 또한, 에러 핸들링 로직이 필요합니다. 예외처리(try-catch)를 추가하는 것을 고려하세요. 그리고, UserPetDto
생성시 불필요한 객체 생성을 줄이는 방법을 고려해 보세요. (ex. builder pattern 최적화)
// 유저가 소유한 펫 도감 조회 | ||
public UserPetListDto getUserCollection(String username) { | ||
List<Pet> pets = petRepository.findByUserUsername(username); | ||
int[] petsNum = new int[100]; //FIXME: 차후 갯수 수정 필요 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
새로운 메서드 getUserCollection(String username)
가 추가되었습니다. petsNum
배열의 크기(100)가 고정되어 있는데, 이는 향후 펫 종류가 늘어날 경우 문제가 될 수 있습니다. 동적 크기의 자료구조(예: ArrayList)를 사용하는 것이 좋습니다. 또한, SQL Injection 취약성 방지를 위해 PreparedStatement 사용이 중요하며, 에러 핸들링 로직 추가가 필요합니다. 그리고, 코드 가독성을 위해 변수명을 더 명확하게 하는 것이 좋습니다. (예: petsNum -> petCounts)
@@ -32,7 +32,6 @@ public class ApiV1NotiController { | |||
private final NotiService notiService; | |||
private final UserService userService; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 라인의 코드 삭제 이유가 명확하지 않습니다. 코드 삭제의 의도와 어떤 기능이 제거되었는지 주석으로 설명해주세요. 또한, 이 삭제로 인해 다른 부분에 영향을 미치는지 확인해야 합니다.
@@ -21,6 +21,15 @@ public class PetController { | |||
private final CustomOAuth2UserService userService; | |||
private final PetService petService; | |||
|
|||
// 현재 펫 경험치 상승 및 상승 시 레벨업 처리 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
새로운 /getall
엔드포인트를 추가했습니다. @AuthenticationPrincipal
을 사용하여 사용자 인증을 처리하는 것은 좋습니다. 하지만, CustomOAuth2User
에서 getName()
으로 사용자 이름을 가져오는 부분은 CustomOAuth2User
의 구현에 따라 다를 수 있으므로, 좀 더 명확하고 안전한 방법을 고려해야 합니다. 예를 들어, oAuth2User.getAttribute("username")
와 같이 attribute 이름을 명시적으로 사용하는 것이 좋습니다. 또한, 에러 핸들링이 전혀 없으므로, 예외 발생 시 적절한 응답을 반환하도록 수정해야 합니다. (예: ExceptionHandler 사용)
@@ -31,13 +40,6 @@ public ResponseEntity<UserPetDto> getPetExp(@AuthenticationPrincipal CustomOAuth | |||
return ResponseEntity.ok(userPetDto); | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
기존의 /getall
엔드포인트가 삭제되었습니다. 삭제 이유와 새로운 /getall
엔드포인트와의 차이점을 명확히 해주세요. 기존 기능이 완전히 대체되는 것인지, 아니면 다른 기능을 담당하게 될 것인지 설명이 필요합니다.
@@ -54,11 +56,6 @@ public ResponseEntity<Pet> createPet(@AuthenticationPrincipal CustomOAuth2User o | |||
return ResponseEntity.ok(pet); | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
기존의 모든 펫 조회 기능이 삭제되었습니다. 이 기능이 더 이상 필요하지 않은 이유를 설명해주세요. 관련된 다른 부분에 영향을 미치는지 확인해야 합니다.
@@ -1,34 +1,32 @@ | |||
package cmf.commitField.domain.pet.controller; | |||
|
|||
|
|||
import cmf.commitField.domain.pet.entity.Pet; | |||
import cmf.commitField.domain.pet.dto.UserPetListDto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pet
엔티티 대신 UserPetListDto
를 import 합니다. DTO를 사용하는 것은 좋지만, Pet
엔티티가 더 이상 사용되지 않는다면, 불필요한 import를 제거해야 합니다. 혹시라도 다른 곳에서 사용중이라면 주석으로 남겨주세요.
import cmf.commitField.domain.pet.service.PetService; | ||
import cmf.commitField.domain.pet.service.UserPetService; | ||
import cmf.commitField.domain.user.entity.User; | ||
import cmf.commitField.domain.user.entity.CustomOAuth2User; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User
엔티티 대신 CustomOAuth2User
를 import 합니다. 사용자 정보를 얻는 방법이 변경되었음을 나타냅니다. 일관성을 유지하기 위해 다른 부분도 CustomOAuth2User
를 사용하는지 확인해야 합니다.
import cmf.commitField.domain.pet.service.PetService; | ||
import cmf.commitField.domain.pet.service.UserPetService; | ||
import cmf.commitField.domain.user.entity.User; | ||
import cmf.commitField.domain.user.entity.CustomOAuth2User; | ||
import cmf.commitField.domain.user.service.CustomOAuth2UserService; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
불필요한 import가 삭제되었습니다. CustomOAuth2UserService
가 더 이상 필요없는 이유를 명시해주세요.
import lombok.RequiredArgsConstructor; | ||
import org.springframework.http.ResponseEntity; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
필요한 import 추가: ResponseEntity
와 @AuthenticationPrincipal
import lombok.RequiredArgsConstructor; | ||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.security.core.annotation.AuthenticationPrincipal; | ||
import org.springframework.web.bind.annotation.GetMapping; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
불필요한 import 삭제: PathVariable
import org.springframework.web.bind.annotation.GetMapping; | ||
import org.springframework.web.bind.annotation.PathVariable; | ||
import org.springframework.web.bind.annotation.RequestMapping; | ||
import org.springframework.web.bind.annotation.RestController; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
불필요한 import 삭제: List
@RestController | ||
@RequestMapping("/user-pets") | ||
@RequestMapping("/api/user/") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RequestMapping 경로 변경: /user-pets
에서 /api/user/
로 변경. API 경로의 일관성을 위해 다른 API 경로와의 통일성을 고려해야 합니다.
@RestController | ||
@RequestMapping("/user-pets") | ||
@RequestMapping("/api/user/") | ||
@RequiredArgsConstructor | ||
public class UserPetController { | ||
|
||
private final UserPetService userPetService; | ||
private final CustomOAuth2UserService customOAuth2UserService; | ||
private final PetService petService; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
주석 제거: // TODO: 기능 확장시 추가 예정
- TODO 주석은 코드 리뷰 시점에 제거하는 것이 좋습니다. TODO가 필요하다면 이슈트래커에 등록하는 것이 더 효율적입니다.
public GlobalResponse<List<Pet>> getUserPetCollection(@PathVariable Long userId) { | ||
User user = customOAuth2UserService.getUserById(userId).orElse(null); | ||
return GlobalResponse.success(userPetService.getUserPetCollection(user)); | ||
@GetMapping("/collection") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/collection/{userId}
엔드포인트를 /collection
으로 변경하고, @AuthenticationPrincipal
을 사용하여 사용자 인증을 처리합니다. 이 변경으로 인해 CustomOAuth2UserService
와 PathVariable
이 필요 없어졌습니다. 이 부분에 대한 설명이 필요합니다. 또한, GlobalResponse
에서 ResponseEntity
로 변경된 이유를 설명해주세요. ResponseEntity
를 사용하는 것은 좋지만, 일관성을 위해 모든 API에서 ResponseEntity
를 사용하는지 확인해야 합니다.
@@ -14,5 +14,6 @@ public class UserPetDto { | |||
private long petId; | |||
private String petName; | |||
private String grow; | |||
private int type; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type
필드 추가. 이 필드의 용도와 데이터 타입을 명확히 설명해주세요. 데이터 타입 int
가 적절한지 확인하고, 필요하다면 더 적합한 타입을 사용해야 합니다.
@@ -0,0 +1,17 @@ | |||
package cmf.commitField.domain.pet.dto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
새로운 DTO 클래스 UserPetListDto
추가. 이 DTO의 용도와 각 필드의 의미를 명확하게 설명해야 합니다. 특히 petList
의 Map<Integer, Integer>
타입은 Integer
키(펫 타입)와 Integer
값(소유 개수)으로 구성됩니다. 키 값의 범위와 에러 처리에 대한 고려가 필요합니다. petList
의 키값(펫 타입)이 100개를 넘어가면 어떻게 될까요? 예외처리가 필요할 것 같습니다.
@@ -2,6 +2,7 @@ | |||
|
|||
import cmf.commitField.domain.pet.dto.PetsDto; | |||
import cmf.commitField.domain.pet.dto.UserPetDto; | |||
import cmf.commitField.domain.pet.dto.UserPetListDto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UserPetListDto
import 추가. DTO를 사용하는 것은 좋지만, 불필요한 import는 제거해야 합니다. 만약 사용하지 않는다면 삭제하거나 주석으로 남겨주세요.
import java.util.List; | ||
import java.util.Optional; | ||
import java.util.Random; | ||
import java.util.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import 수정: ArrayList
, List
, Optional
, Random
대신 util.*
사용. 불필요한 import를 줄이고 코드를 간결하게 만드는 것은 좋은 방법입니다.
// 모든 펫 조회 | ||
public List<Pet> getAllPets() { | ||
return petRepository.findAll(); | ||
// 유저가 소유한 펫 도감 조회 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
새로운 메서드 getUserCollection
추가. 유저가 소유한 펫 목록을 UserPetListDto
로 반환합니다. petsNum
배열은 펫 타입별 개수를 저장하는데, 크기가 100으로 고정되어 있습니다. 이 크기는 펫 타입의 최대 개수를 고려하여 설정해야 하며, 향후 펫 타입이 추가될 경우 크기를 동적으로 조정하거나, 더 효율적인 데이터 구조 (예: HashMap)를 사용하는 것을 고려해야 합니다. FIXME
주석이 있는 부분을 수정해야 합니다. 또한, 에러 핸들링이 없습니다. 예외 발생 시 적절한 예외 처리를 추가해야 합니다.
#️⃣ Issue Number
#150
📝 요약(Summary)
펫 조회 api 추가
🛠️ PR 유형
어떤 변경 사항이 있나요?
📸스크린샷 (선택)
💬 공유사항 to 리뷰어
✅ PR Checklist
PR이 다음 요구 사항을 충족하는지 확인하세요.