[♻️ refactor/#107] 온보딩 > 사용자 필터링 정보 생성 로직 통합 및 개선 #113
Conversation
- `SignUpFilterService` 클래스 삭제 및 관련 기능 `AuthService`로 통합 - 사용자 필터 생성 및 연결 로직을 `AuthService`로 이전하여 서비스 계층 단순화 - 불필요한 메서드 및 인터페이스 제거로 코드 간소화 - `AuthController`에서 `SignUpFilterService` 의존성을 `AuthService`로 대체
jsoonworld
requested review from
junggyo1020 and
JungYoonShin
and removed request for
junggyo1020
- SwaggerConfig.java에 새로운 서버 URL(https://www.terning-official.p-e.kr)과 스테이징 서버 URL(http://15.165.242.132)을 추가했습니다. - WebMvcConfig.java의 CORS 설정에 스테이징 서버 URL(http://15.165.242.132)을 허용된 출처로 추가했습니다. - 코드 정리를 위해 AuthSwagger.java에서 불필요한 Principal 임포트를 제거했습니다.
| } | ||
|
|
||
| @Transactional | ||
| public void connectFilterToUser(Long userId, Long filterId) { |
There was a problem hiding this comment.
userId, filterId이 null일 경우 없다면 원시타입 long으로 써주시면 좋을 것 같아요!
There was a problem hiding this comment.
좋은 지적 감사합니다! 현재 코드에서 userId와 filterId가 null일 가능성이 전혀 없다고 판단되기 때문에, 안정성을 높이기 위해 이 값을 long 타입으로 수정하는 것이 더 적절할 것 같습니다. 이렇게 하면 null 처리를 위한 불필요한 로직을 제거하고, NullPointerException을 방지할 수 있어 코드의 안정성을 높일 수 있을 것 같아요. 조언 감사드리며, 해당 부분을 long으로 수정하겠습니다!
| User user = userRepository.findById(userId).orElseThrow(() -> new CustomException(FAILED_SIGN_UP_FILTER)); | ||
| Filter filter = filterRepository.findById(filterId).orElseThrow(() -> new CustomException(FAILED_SIGN_UP_FILTER)); |
There was a problem hiding this comment.
현재 메서드에서 사용자 또는 필터를 찾지 못할 경우 동일한 CustomException이 발생하는데요. 적절하게 예외처리를 분리하거나 로그를 추가하는 것도 좋아보이네요!
//log를 적용한 경우
public void connectFilterToUser(Long userId, Long filterId) {
User user = userRepository.findById(userId).orElseThrow(() -> {
log.error("User not found with id: {}", userId);
throw new CustomException(FAILED_SIGN_UP_FILTER);
});
Filter filter = filterRepository.findById(filterId).orElseThrow(() -> {
log.error("Filter not found with id: {}", filterId);
throw new CustomException(FAILED_SIGN_UP_FILTER);
});There was a problem hiding this comment.
피드백 감사합니다! 말씀해주신 내용을 반영하여 예외 처리를 더욱 명확하게 개선했습니다.
- 사용자 조회 실패:
userId로 사용자를 찾지 못한 경우, 이제FAILED_SIGN_UP_USER_FILTER_CREATION예외가 발생하도록 수정했습니다. - 필터 조회 실패:
filterId로 필터를 찾지 못한 경우에는FAILED_SIGN_UP_USER_FILTER_ASSIGNMENT예외가 발생하도록 변경했습니다.
| private Filter buildFilterFromRequest(SignUpFilterRequestDto request) { | ||
| Grade grade = Grade.fromKey(request.grade()); | ||
| WorkingPeriod workingPeriod = WorkingPeriod.fromKey(request.workingPeriod()); | ||
| int startYear = request.startYear(); | ||
| int startMonth = request.startMonth(); | ||
|
|
||
| return Filter.builder() | ||
| .grade(grade) | ||
| .workingPeriod(workingPeriod) | ||
| .startYear(startYear) | ||
| .startMonth(startMonth) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
현재 AuthServiceImpl 내에 책임이 많다는 생각이 드네요..! 팩토리 클래스를 하나 만들어서 필터 생성 로직인 buildFilterFromRequest를 분리하는 것도 고려해볼 수 있을 것 같아요..! 그렇게 되면 AuthService가 너무 많은 역할을 하지 않도록 하고, SOLID 원칙을 더 잘 따르도록 설계할 수 있을 것 같다는 의견입니다:)
There was a problem hiding this comment.
말씀해주신 부분에 대해 저도 동의합니다. 현재 AuthServiceImpl 클래스가 여러 책임을 맡고 있는 것 같아, 팩토리 클래스를 만들어 buildFilterFromRequest 메서드를 분리하는 것이 좋겠다는 의견에 공감합니다. 이렇게 하면 AuthService가 너무 많은 역할을 하지 않도록 하고, SOLID 원칙을 더 잘 따르도록 설계할 수 있을 것 같습니다. 다만, 이 변경사항은 영향 범위가 크다고 판단되어, 다음 PR에서 이를 반영하여 수정하도록 하겠습니다. 좋은 제안 감사합니다!
- userId와 filterId가 null 값을 가질 수 없으므로, 안정성을 높이기 위해 해당 파라미터를 Long에서 long으로 변경하였습니다. - 이로 인해 불필요한 null 체크를 제거하고, NullPointerException 발생 가능성을 차단하였습니다.
- FAILED_SIGN_UP_FILTER 에러 메시지를 FAILED_SIGN_UP_USER_FILTER_CREATION 및 FAILED_SIGN_UP_USER_FILTER_ASSIGNMENT로 분리하여 사용자 필터 생성 및 연결에 대한 구체적인 예외 메시지 추가 - AuthServiceImpl의 connectFilterToUser 메소드에서 기존 예외 메시지를 새로운 메시지로 수정하여 더 명확한 에러 처리 구현
📄 Work Description
SignUpFilterService의 기능을AuthService로 통합하여 서비스 계층을 단순화했습니다.AuthService로 이전하여 코드 구조를 개선했습니다.AuthController에서 관련 의존성을 정리했습니다.⚙️ ISSUE
📷 Screenshot
Swagger 200
💬 To Reviewers
리팩토링된 부분이 많은데, 특히
AuthService로 기능이 이동된 부분에 대해 확인 부탁드립니다. 이 과정에서 발생할 수 있는 의도치 않은 사이드 이펙트가 없는지 중점적으로 봐주시면 감사하겠습니다