-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feature/38 admin user management #39
Conversation
Hanseunghoon
commented
May 7, 2023
- 유저 목록 조회 (페이징)
- 유저 정보 수정
- 유저 정보 삭제 (회원 탈퇴)
public void deleteUser(@PathVariable("userId") String userId){ | ||
adminUserService.deleteUser(userId); | ||
} | ||
} No newline at end of file |
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.
이 코드는 admin/user API를 정의하는 Spring Boot REST 컨트롤러입니다. getUserList, updateUser 및 deleteUser라는 세 가지 엔드포인트를 제공합니다. 이 코드의 개선할 점은 다음과 같습니다.
- getUserList 엔드포인트에 대한 페이징 및 정렬 옵션 제공을 고려해보세요.
- updateUser 및 deleteUser 엔드포인트에서 예외 처리를 추가하세요. (ex. 찾을 수 없는 사용자, 삭제 실패 등)
- 엔드포인트에서 사용되는 객체의 유효성 검사를 위해 JavaBean 유효성 검사를 구현하세요.
- Swagger 문서에 자세한 설명과 인라인 스키마를 추가하세요.
@@ -7,4 +7,5 @@ | |||
@Repository | |||
public interface UserRepository extends JpaRepository<User, Long> { | |||
User findByUserId(String userId); | |||
void deleteUserByUserId(String userId); | |||
} |
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.
위 코드는 JpaRepository를 상속받은 UserRepository 인터페이스에서 findByUserId 메소드를 정의하고, deleteUserByUserId 메소드를 추가로 정의하는 것입니다. 이러한 수정 사항은 기존 코드에 문제가 없다고 보이며, 새로운 기능을 추가하기 위한 단순한 확장일 뿐입니다.
그러나 이 코드에서 개선할 점은 몇 가지 있습니다. 첫째로, deleteUserByUserId 메소드는 반환값이 없는 void 형식입니다. 이 경우, 사용자가 매개 변수로 전달된 userId와 일치하는 사용자를 데이터베이스에서 삭제하는 작업을 수행하지만, 작업이 성공적으로 완료되었는지 여부를 알 수 없습니다. 따라서 이 메소드는 작업 결과를 나타내는 boolean 형식으로 변경될 필요가 있습니다.
둘째로, UserRepository 인터페이스에는 다른 검색 기능이나 CRUD 메소드가 포함되어 있지 않은데, 이는 해당 인터페이스가 다른 메소드들로 확장될 가능성을 염두에 두기 어렵게 만듭니다. 따라서 추후에 추가되거나 변경될 메소드를 고려하여 UserRepository 인터페이스를 확장하면 유용합니다.
마지막으로, 메소드 이름에 대해 조금 더 구체적인 이름을 사용하는 것이 좋습니다. 예를 들어 deleteUserByUserId 메소드를 deleteUserWithMatchingUserId 또는 removeUserByUserId 등으로 변경하여 메소드의 목적을 보다 명확하게 드러낼 수 있습니다.
public void deleteUser(String userId) { | ||
userRepository.deleteUserByUserId(userId); | ||
} | ||
} |
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.
위 코드는 UserService와 User 업데이트, 삭제를 위한 AdminUserService 클래스입니다. 아래의 리뷰는 다음과 같습니다.
- getUserList(): 페이지 번호와 사이즈가 고정되어 있습니다. 이를 사용자 입력으로 변경할 수 있도록 수정하는 것이 좋을 것입니다.
- updateUser(): 기존 사용자를 업데이트할 때, id로 찾아 update 하는 방식입니다. 이 경우 id가 null 인 경우 예외 처리가 필요합니다.
- deleteUser(): userId를 사용하여 findById와 deleteById 메소드를 사용할 수 있으므로, 해당 메소드를 사용하는 것이 더 낫습니다.
- 코드의 가독성을 더욱 높이기 위해서 UserRepository에서 사용할 수 있는 메소드를 이용하는 것이 좋습니다. 이는 Spring Data JPA를 이용한 DAO를 간편하게 관리할 수 있도록 돕는 기능입니다.
위의 개선사항을 적용한다면 아래와 같을 것입니다.
@@ -0,0 +1,37 @@
+package com.aluminium.acoe.sys.api.service;
+
+import com.aluminium.acoe.sys.api.entity.user.User;
+import com.aluminium.acoe.sys.api.repository.user.UserRepository;
+import lombok.RequiredArgsConstructor;
+import org.springframework.data.domain.Page;
+import org.springframework.data.domain.Pageable;
+import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Transactional;
+
+@service
+@requiredargsconstructor
+public class AdminUserService {
- private final UserRepository userRepository;
- public Page getUserList(Pageable pageable) {
-
return userRepository.findAll(pageable);
- }
- @transactional
- public void updateUser(User user) {
-
if(user.getUserId() == null){
-
throw new IllegalArgumentException("userId는 필수입니다.");
-
}
-
userRepository.findById(user.getUserId())
-
.map(targetUser -> {
-
targetUser.setUsername(user.getUsername());
-
return userRepository.save(targetUser); })
-
.orElseThrow(() -> new RuntimeException("사용자를 찾을 수 없습니다."));
- }
- @transactional
- public void deleteUser(String userId) {
-
userRepository.deleteById(userId);
- }
+}
이와 같이 코드를 개선하면 가독성과 예외 처리가 더욱 적절하게 이루어짐으로써 유지보수하기 좋은 코드를 만들 수 있습니다.