Develop #96
Develop #96
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ADD `display_order` int DEFAULT NULL No newline at end of file |
There was a problem hiding this comment.
이 코드 패치는 alcohol_type 테이블에 display_order라는 열을 추가하는 ALTER TABLE 문입니다. 변경된 부분은 데이터 형식입니다. bigint 대신 int로 변경되었습니다.
이러한 코드 리뷰에서 주의해야 할 사항은 다음과 같습니다:
- 데이터베이스 구조와 관련된 변경은 신중히 검토해야 합니다. 이 변경이 다른 테이블 또는 쿼리에 영향을 줄 수 있는지 확인해야 합니다.
- 기존 코드에 영향을 주는 변경일 경우, 변경된 코드를 사용하는 모든 위치를 확인하여 호환성을 검증해야 합니다.
- 데이터 형식 변경 시 데이터 손실이 발생할 수 있으므로, 데이터베이스에 저장된 값들의 유효성을 검사하는 것이 중요합니다.
- 코드에 새로운 버그가 없는지 확인하기 위해 테스트를 진행해야 합니다.
개선 제안:
- 코드 스타일을 맞추기 위해 각 줄의 들여쓰기를 확인하고 일관된 스타일을 사용하세요.
- "no newline at end of file" 경고를 해결하기 위해 파일의 끝에 새로운 줄을 추가해주세요.
위의 정보를 참고하여 코드 패치를 검토해보세요.
| alcoholType) | ||
| .build(), | ||
| pageable); | ||
| } | ||
| } |
There was a problem hiding this comment.
위의 코드 패치를 간단히 검토해 드리겠습니다.
AlcoholManager클래스는AlcoholRepository및AlcoholTypeRepository에 대한 주입을 받도록 되어 있습니다.connectAlcoholWithPost메서드는 알코올 번호(alcoholNo), 알코올 특징(alcoholFeature), 게시물(post)을 매개변수로 받아 알코올과 게시물을 연결하는 기능을 수행합니다. 주어진 알코올 번호를 사용하여Alcohol객체를 찾고, 해당 게시물에PostAlcohol을 추가하고, 알코올에도PostAlcohol을 추가합니다. 이 부분은 문제가 없어 보입니다.searchAlcohol메서드는 페이지네이션(pageable), 검색 키워드(searchKeyword), 알코올 유형(alcoholType)을 매개변수로 받아 알코올을 검색하는 기능을 수행합니다. 알코올 유형이 주어진 경우 해당 유형이 유효한지 확인하기 위해AlcoholTypeRepository를 사용하는데, 유효하지 않은 알코올 유형인 경우IllegalArgumentException을 던지고 있습니다. 이 부분은 문제가 없어 보입니다. 그러나, 알코올 유형에 따라Alcohol을 검색하는 부분이 누락되어 있습니다. 해당 부분을 확인하고 추가해야 합니다.
개선 제안:
searchAlcohol메서드에서alcoholType1변수를 사용하지 않고, 알코올 유형이 유효한 경우 해당 유형에 따라Alcohol을 검색하도록 수정해야 합니다.searchAlcohol메서드에서 검색 조건(AlcoholSearchCondition)을 생성할 때,searchKeyword와alcoholType을 사용하여 조건을 설정하도록 해야 합니다.
수정된 searchAlcohol 메서드의 예시:
public Page<Alcohol> searchAlcohol(Pageable pageable, String searchKeyword, Long alcoholType) {
if (ObjectUtils.isNotEmpty(alcoholType)) {
AlcoholType alcoholTypeEntity = alcoholTypeRepository.findByIdAndDelYnIs(alcoholType, YesOrNo.N)
.orElseThrow(() -> new IllegalArgumentException("술 종류가 존재하지 않습니다."));
// 알코올 유형이 유효한 경우 해당 유형에 따라 검색결과 반환
return alcoholRepository.searchAlcoholPage(AlcoholSearchCondition.builder()
.searchKeyword(searchKeyword)
.alcoholType(alcoholTypeEntity)
.build(),
pageable);
} else {
// 알코올 유형이 주어지지 않은 경우 모든 알코올 검색결과 반환
return alcoholRepository.searchAlcoholPage(AlcoholSearchCondition.builder()
.searchKeyword(searchKeyword)
.build(),
pageable);
}
}추가적으로, 코드의 다른 부분은 검토 결과 문제를 발견하지 못했습니다. 이 내용을 바탕으로 코드 패치를 수정하실 수 있을 것입니다.
| attachDtos.add(AttachDto.of(attach)); | ||
| } | ||
| return attachDtos; | ||
| } | ||
| } |
There was a problem hiding this comment.
다음은 코드 패치입니다. 버그 위험이나 개선 제안 사항을 알려주세요:
@@ -1,5 +1,6 @@
package com.bside.bside_311.component;
+import com.bside.bside_311.dto.AttachDto;
import com.bside.bside_311.entity.Attach;
import com.bside.bside_311.entity.AttachType;
import com.bside.bside_311.entity.YesOrNo;
@@ -8,7 +9,10 @@
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Component;
+import java.util.ArrayList;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import static com.bside.bside_311.util.ValidateUtil.resourceChangeableCheckByThisRequestToken;
@@ -50,4 +54,32 @@ public void deleteAttachesByRefNoAndAttachType(Long refNo, AttachType attachType
imageStorage.delete(attach.getPublicId());
}
}
+ public Map<Long, List<AttachDto>> getAttachInfoMapBykeysAndType(List<Long> keys,
+ AttachType attachType) {
+ List<Attach> attachList =
+ attachRepository.findByRefNoInAndAttachTypeIsAndDelYnIs(keys, attachType,
+ YesOrNo.N);
+ Map<Long, List<AttachDto>> kToAMap = new HashMap<>();
+ for (Attach attach : attachList) {
+ if (!kToAMap.containsKey(attach.getRefNo())) {
+ kToAMap.put(attach.getRefNo(), new ArrayList<>());
+ }
+ List<AttachDto> attachDtos = kToAMap.get(attach.getRefNo());
+ attachDtos.add(AttachDto.of(attach));
+ }
+ return kToAMap;
+ }
+
+ public List<AttachDto> getAttachListBykeyAndType(Long key,
+ AttachType attachType) {
+ List<Attach> attachList =
+ attachRepository.findByRefNoInAndAttachTypeIsAndDelYnIs(List.of(key), attachType,
+ YesOrNo.N);
+ List<AttachDto> attachDtos = new ArrayList<>();
+ for (Attach attach : attachList) {
+ attachDtos.add(AttachDto.of(attach));
+ }
+ return attachDtos;
+ }
}이 코드 패치에는 다음과 같은 기능이 포함되어 있습니다:
com.bside.bside_311.dto.AttachDto를 import 합니다.getAttachInfoMapBykeysAndType메서드를 추가합니다. 이 메서드는keys와attachType을 기반으로Attach를 찾고, 해당하는AttachDto리스트를 키-값 쌍의 맵 형태로 반환합니다.getAttachListBykeyAndType메서드를 추가합니다. 이 메서드는key와attachType을 기반으로Attach를 찾고, 해당하는AttachDto리스트를 반환합니다.
개선 제안:
- 사용하지 않는 import 문을 정리하세요 (
java.util.ArrayList,java.util.HashMap). getAttachInfoMapBykeysAndType및getAttachListBykeyAndType메서드 내에서AttachDto.of(attach)대신 생성자를 직접 호출하는 것이 코드를 더 명확하게 만들 수 있습니다.
| } | ||
|
|
||
|
|
||
| } |
There was a problem hiding this comment.
이 코드 패치는 PostService 클래스에 대한 변경 사항입니다. 아래는 코드 리뷰입니다:
PostService클래스는Component애너테이션이 지정되어 있습니다.RequiredArgsConstructor애너테이션을 사용하여 필요한 의존성 주입을 자동으로 생성합니다.PostRepository와PostMybatisRepository인터페이스를 사용하여 데이터 액세스 작업을 수행합니다.savePost,findPost,deletePost메서드는 각각 게시물을 저장, 조회, 삭제하는 기능을 합니다.getPostListCommon메서드는 페이징 및 검색 조건으로 게시물 목록을 반환합니다.getPostListPopular메서드는 인기 게시물 목록을 반환합니다.getGetPostsToOneMvoMap메서드는 게시물 번호 목록을 기반으로 게시물과 관련된 추가 정보를 가져옵니다.
개선 제안:
- 누락된 주석: 코드의 목적과 각 메서드의 역할에 대한 주석을 추가하는 것이 좋습니다.
- 예외 처리:
findPost메서드에서 게시물이 없는 경우IllegalArgumentException을 던지고 있습니다. 이 예외의 메시지도 확인해보는 것이 좋습니다. - 네이밍: 메서드 이름과 변수 이름은 직관적이고 명확하게 작성되어 있는 것 같습니다. 변수의 유형과 용도에 대한 추가 설명을 제공할 수 있으면 좋습니다.
잠재적인 버그 위험 요소나 이외의 개선 사항은 제시된 코드에서 확인하기 어렵습니다. 전체 애플리케이션의 아키텍처와 다른 모듈들과의 상호 작용을 고려하여 추가적인 코드 리뷰가 필요할 것입니다.
| return userRepository.findByIdAndDelYnIs(userNo, YesOrNo.N).orElseThrow( | ||
| () -> new IllegalArgumentException(MessageUtil.USER_NOT_FOUND_MSG)); | ||
| } | ||
| } |
There was a problem hiding this comment.
코드 리뷰를 진행하겠습니다:
UserManager는com.bside.bside_311.component패키지에 속하는 컴포넌트입니다.- 필요한 의존성 주입을 위해
lombok.RequiredArgsConstructor를 사용합니다. getUser메서드는userNo매개변수를 받아 해당userNo에 해당하는User객체를 찾아 반환합니다.- 만약
userNo가 비어있다면,null을 반환합니다. userRepository에서findByIdAndDelYnIs메서드를 호출하여userNo와 삭제 여부(YesOrNo.N)로User를 찾고 없을 경우 예외를 던집니다.- 예외 처리 시
IllegalArgumentException이 발생하며, 해당 예외에 대한 메시지는MessageUtil.USER_NOT_FOUND_MSG로 정의되어 있습니다.
개선 제안:
ObjectUtils.isEmpty(userNo)를 대신하여userNo == null을 사용할 수 있습니다.- 예외 처리의 내용에 대해 좀 더 구체적인 설명을 추가할 수 있습니다.
위 내용은 단순한 코드 리뷰입니다. 실제 애플리케이션에 적합한지는 전제 조건과 요구사항에 따라 다를 수 있습니다.
| log.info(">>> AlcoholController.getAlcohol"); | ||
| return alcoholService.getAlcoholV2(pageable, searchKeyword); | ||
| return alcoholService.getAlcoholV2(pageable, searchKeyword, alcoholType); | ||
| } | ||
|
|
||
| @Operation(summary = "[o]술 상세 조회", description = "술 상세 조회 API") |
There was a problem hiding this comment.
이 코드 패치는 다음과 같은 변경 사항을 포함하고 있습니다:
jakarta.validation.constraints.Positive어노테이션이 추가되었습니다.alcoholType파라미터가 추가되었으며,@RequestParam과@Schema어노테이션으로 정의되었습니다.alcoholType파라미터에 대한 유효성 검사를 위해@Positive어노테이션을 사용하고 있습니다.
개선 및 버그 위험 사항에 대한 제안:
- 코드의 가독성을 높이기 위해 주석을 영문으로 작성하는 것이 좋습니다.
String searchKeyword와Long alcoholType파라미터들의 이름을 좀 더 명확하게 지정하는 것이 좋습니다. 이러한 이름들은 개발자들이 해당 메소드를 사용할 때 도움이 될 수 있습니다.- 코드에서
AlcoholController.getAlcohol로깅 메시지를">>> AlcoholController.getAlcohol"에서 보다 명확한 메시지로 변경하는 것이 좋습니다. - 나머지 코드의 내용과 로직에 대해서는 별다른 문제나 버그 위험이 보이지 않습니다.
위의 사항들을 고려하여 코드를 개선할 수 있습니다.
| @@ -257,7 +272,7 @@ public void likePost(@PathVariable("postNo") Long postNo) { | |||
| public void likeCancelPost(@PathVariable("postNo") Long postNo) { | |||
| log.info(">>> PostController.likeCancelPost"); | |||
| Long userNo = AuthUtil.getUserNoFromAuthentication(); | |||
| postService.likeCancelPost(userNo, postNo); | |||
| postFacade.likeCancelPost(userNo, postNo); | |||
| } | |||
|
|
|||
| } | |||
There was a problem hiding this comment.
해당 코드 패치의 간단한 코드 리뷰를 도와드리겠습니다.
-
PostService클래스를PostFacade클래스로 변경하였습니다.- 이에 따라 객체 변수 이름도
postService에서postFacade로 수정되었습니다.
- 이에 따라 객체 변수 이름도
-
ResultDto클래스가com.bside.bside_311.dto.common패키지에서 추가되었습니다. -
deletePost,editPost함수 내에서postService메서드 호출이postFacade로 변경되었습니다. -
getPosts,getPostsV2,getPostDetail,addComment,getPostComments,editComment,
deleteComment,addQuote,deleteQuote,getQuotesByPost,likePost,likeCancelPost
함수 내에서postService메서드 호출이postFacade로 변경되었습니다. -
새로운 함수인
getPostsPopular함수가 추가되었습니다. 해당 함수는 인기 게시글을 조회하기 위한 API입니다.- 캐싱 처리 예정이며, 페이지(
page)와 사이즈(size)를 인자로 받습니다.
- 캐싱 처리 예정이며, 페이지(
위의 수정 사항들은 주요한 사소한 오류와 개선점으로 보이지는 않으며, 주요한 기능에 영향을 미치지 않고 확장 및 유지 관리에 도움을 줄 수 있습니다. 일부 코드에 대해 백엔드 시스템과의 상호 작용 및 모델 액세스 기능을 확인하고 테스트할 필요가 있습니다.
| Long myUserNo = AuthUtil.getUserNoFromAuthentication(); | ||
| return userService.getUsersPopular(page, size, myUserNo); | ||
| } | ||
|
|
||
| @Operation(summary = "[o]유저 정보 조회", description = "유저 정보 조회 API") | ||
| @GetMapping("/{userNo}/summary") | ||
| public GetUserInfoResponseDto getUserInfo(@PathVariable("userNo") Long userNo) { |
There was a problem hiding this comment.
아래는 코드 패치입니다. 불행하게도, 주어진 코드 일부분만 가지고 어떤 버그 위험이 있는지나 개선 제안이 가능합니다. 다만 기타 것들을 평가하기에 어렵습니다.
버그 위험:
getUsersPopular메서드의 매개변수로page와size를 받습니다. 그런데 현재 타입이Long인데,@RequestParam애너테이션에required = true로 설정되어 있습니다.Long타입이 아니라int타입으로 변경하는 것이 좋습니다.
개선 제안:
ResultDto와 관련된 코드가 누락되어 있어 해당 클래스가 어떤 역할을 하는지 알 수 없습니다.ResultDto클래스에 대한 설명과 사용되는 이유를 주석 등의 추가적인 방식으로 제공하는 것이 좋을 수 있습니다.- TODO 주석을 사용하여 "추후 캐싱 처리 예정"이나 기타 정보에 대해서 명확한 계획을 작성하거나 캐싱 처리에 대한 상세한 주석 설명을 제공하는 것이 좋습니다.
- API 문서 생성을 위해
io.swagger.v3.oas.annotations패키지에서 제공하는 애너테이션을 사용하고 있습니다. 모든 API 엔드포인트에 애너테이션을 추가하고, 설명과 예제를 충분히 포함하는 것이 좋습니다.
이 외에도 코드의 일부가 생략되어 있기 때문에 전체적인 보완과 리팩터링에 대한 판단은 어렵습니다.
| @@ -13,4 +13,6 @@ | |||
| public class AlcoholSearchCondition { | |||
| @Schema(description = "키워드", example = "키워드") | |||
| String searchKeyword; | |||
| @Schema(description = "술 종류 PK", example = "1") | |||
| Long alcoholType; | |||
| } | |||
There was a problem hiding this comment.
코드 패치를 간단히 검토해 드리겠습니다.
이 코드는 AlcoholSearchCondition 클래스에 새로운 필드 alcoholType을 추가하는 것으로 보입니다. 이 필드는 술의 종류를 나타내며, Long 타입의 값으로 표현됩니다.
추가한 alcoholType 필드에 대한 주석은 몇 가지 개선이 필요해 보입니다. @Schema 애너테이션의 description 속성에서 "술 종류 PK"와 같은 단어를 사용하는 것이 이해하기 어려울 수 있습니다. 수동적 문장 구조를 갖는다면, "술 종류의 기본 키(primary key)"와 같이 더 명확하게 작성하는 것이 좋을 것입니다.
또한, example 속성에 있는 "1"은 첫 번째 술 종류를 나타낸다고 추정됩니다. 이 경우 실제 숫자 대신 술 종류의 ID나 이름에 대한 예시를 사용하는 것이 더 유용할 수 있습니다.
이외에는 큰 문제나 버그가 보이지 않으므로, 이러한 소소한 수정을 통해 코드를 개선할 수 있을 것입니다.
| } | ||
|
|
||
| if (ObjectUtils.isNotEmpty(isFollowedByMe)) { | ||
| postResponseDtoBuilder.isFollowedByMe(isFollowedByMe); | ||
| } |
There was a problem hiding this comment.
이 코드 패치에는 몇 가지 버그 리스크와 개선 제안 사항이 있습니다:
-
postAttachDtos매개변수의 이름이 코드의 의도와 일치하지 않습니다. 변수의 앞 부분인 "post"는 혼동을 줄 수 있으므로 "attachDtos"로 변경하는 것이 좋습니다. -
profileAttachDtos매개변수가 추가되었지만, 해당 변수를 사용하는 로직이 없는 것으로 보입니다. 따라서 이 변수를 사용하지 않는다면, 필요하지 않은 매개변수이므로 제거하는 것이 좋습니다. -
"TODO quoteInfo는 나중에." 주석은 구체적인 정보를 제공하지 않고 있습니다. 코드 리뷰에서는 구현의 목적과 비즈니스 요구사항에 대한 내용을 파악하기 어렵기 때문에, 가능한 경우 이 주석을 자세히 작성하여 나중에 어떤 변화를 기대하고 있는지 설명하는 것이 좋습니다.
-
ObjectUtils.isNotEmpty()메서드는 어떤 라이브러리나 유틸리티 클래스의 정적 메서드인 것으로 보입니다. 버전 충돌이나 유지 관리의 문제가 발생할 수 있으므로, 가능한 경우 Java 표준 라이브러리의CollectionUtils.isNotEmpty()를 사용하는 것이 좋습니다.
추가로 코드의 컨벤션, 가독성 및 유지 보수 측면에서도 검토가 필요할 수 있으나 해당 내용은 주어진 패치만으로 충분한 정보를 얻을 수 없기 때문에 제공할 수 있는 도움이 한계되는 점 양해 부탁드립니다.
| .followedCount(followedCount) | ||
| .build(); | ||
| } | ||
| } |
There was a problem hiding this comment.
위 코드 패치는 UserIncludeFollowCountDto라는 클래스를 포함하고 있습니다. 이 클래스는 com.bside.bside_311.dto 패키지에 속하며, User 엔티티에서 필요한 정보들을 담고 있습니다.
이 코드에는 오류 리스크보다 개선 제안이 주된 사항입니다:
@Getter애너테이션은 필드에 직접 적용되어 있지 않지만, Lombok의@AllArgsConstructor애너테이션과 함께있어 해당 필드의 Getter 메서드들도 자동으로 생성됩니다. 이러한 경우에는 명시적으로@Getter애너테이션을 추가해 주시면 좋습니다.id,userNo,createdBy필드는 각각String,Long,Long타입인데,User엔티티에서 받아오는 값에 따라 데이터 유형이 일치하지 않을 수 있습니다. 데이터가 호환되는지 확인하는 로직을 추가하는 것이 좋습니다.- 코드의 가독성과 유지 보수를 위해 필드명을 보다 명확하게 변경하는 것이 좋습니다. 예를 들어,
followedCount대신followCount로 변경할 수 있습니다.
이상입니다. 가능한 한 요약적으로 설명해 드렸으니 도움이 되셨기를 바랍니다! 추가로 궁금한 점이 있으면 언제든지 말씀해 주세요.
| // followedCount | ||
| .followedCount(user.getFollowedCount()) | ||
| .build(); | ||
| } | ||
| } |
There was a problem hiding this comment.
해당 코드 패치를 간단히 검토하고 개선할 사항과 버그 위험을 제안해드리겠습니다.
-
버그 위험:
UserResponseDto클래스에 대한 직접적인 버그 리스크는 보이지 않습니다. 그러나 기존의of메서드를 수정하여 새로운of메서드를 추가했습니다. 이는 이미 사용 중인 코드와 호환성 문제가 발생할 수 있습니다. 해당 클래스를 사용하는 모든 곳에서 새로운of메서드를 사용하는지 확인하고, 필요하다면 기존 사용들을 수정해야합니다.
-
개선 제안:
- 코드는 크게 변경되지 않은 것 같습니다. 단순히
UserResponseDto에followedCount필드를 추가한 것으로 보입니다. 따라서 일반적인 개선 사항은 기대하기 어렵습니다. - 그러나 코드를 더욱 가독성 있고 유지보수하기 쉽게 만들기 위해 다음을 고려할 수 있습니다:
AttachDto리스트를 초기화할 때,ArrayList<>()대신new ArrayList<>()를 사용하는 것이 좋습니다. 왜냐하면 Java 10까지 인식되지 않는 불필요한 타입 추론 오류를 방지할 수 있기 때문입니다.UserResponseDto생성자에 사용된Builder.Default어노테이션은 일반적으로 가독성을 향상시키는 데 도움이 됩니다.
- 코드는 크게 변경되지 않은 것 같습니다. 단순히
이 코드 패치 검토와 개선 제안은 기존 코드의 문제가없고 새로운 기능에 대한 리스크도 없음을 전제로하며, 해당 코드 조각만 고려했습니다. 완전한 시스템 컨텍스트에서 보다 포괄적인 검토를 수행하는 것이 이상적입니다.
| public static <D> ResultDto<D> successOf(D data) { | ||
| return new ResultDto<D>(ResultCode.SUCCESS.getCode(), ResultCode.SUCCESS.getMessage(), data); | ||
| } | ||
| } |
There was a problem hiding this comment.
코드 리뷰를 해드리겠습니다.
@Getter어노테이션을 사용하여 결과 필드에 대한 getter 메서드를 자동으로 생성하고 있습니다.@RequiredArgsConstructor(staticName = "of")어노테이션을 사용하여 필수 인자로 생성자를 만들고, 생성자를 통해 객체를 생성할 수 있도록 하고 있습니다.staticName = "of"는of라는 이름의 정적 팩터리 메서드를 생성합니다.resultCode,message,data필드를 가지고 있으며, 생성자를 통해 초기화합니다.- 제네릭 타입
D를 사용하여data필드를 다양한 타입으로 받을 수 있게 하고 있습니다. of메서드를 사용하여ResultCode와data를 받아ResultDto객체를 생성하는 메서드를 추가하였습니다.successOf메서드를 사용하여data와 성공 상태 코드를 받아ResultDto객체를 생성하는 메서드를 추가하였습니다.
버그나 오류 위험성은 보이지 않습니다. 하지만 몇 가지 개선 제안이 있습니다:
staticName = "of"대신staticName = "create"등 더 의미있는 이름을 지을 수 있습니다.- 생성자에 대한 Javadoc 주석을 추가하여 사용법을 명확하게 설명할 수 있습니다.
- 필요한 경우
toString()메서드나equals()및hashCode()메서드를 재정의하는 것도 고려해 볼만합니다.
이상입니다. 추가 질문이 있으면 언제든지 물어보세요.
| @GetMapping("/common-result-dto") | ||
| public ResultDto<UserResponseDto> testCommonResultDto() { | ||
| return ResultDto.of("200", "success", UserResponseDto.of(User.of(1L))); | ||
| } | ||
| } |
There was a problem hiding this comment.
이 코드 패치는 다음과 같습니다:
-
com.bside.bside_311.dto.UserResponseDto와com.bside.bside_311.dto.common.ResultDto를 임포트하며,com.bside.bside_311.entity.User도 임포트합니다. -
getImage()메서드에서ResponseEntity<Resource>를 반환하는데, 이미지를 찾을 수 없는 경우ResponseEntity.notFound().build()를 반환합니다. -
testCommonResultDto()메서드는/common-result-dto엔드포인트에서 사용됩니다. 이 메서드는ResultDto<UserResponseDto>를 반환하며,ResultDto.of("200", "success", UserResponseDto.of(User.of(1L)))를 반환합니다.
버그 리스크나 개선 제안에 대한 특정 내용은 주어지지 않았지만, 보이는 한 버그 없이 코드가 정상적으로 동작할 것으로 보입니다.
| @@ -177,6 +178,9 @@ private void initFollowingRelation() { | |||
| setSecutiryContextDoSomethingAndClear(2L, () -> { | |||
| userController.followUser(4L); | |||
| }); | |||
| setSecutiryContextDoSomethingAndClear(1L, () -> { | |||
| userController.followUser(2L); | |||
| }); | |||
| } | |||
|
|
|||
| private void initAlcoholsAndPosts() { | |||
There was a problem hiding this comment.
- 코드 리뷰는 아래와 같은 부분에서 진행됩니다:
-
setSecurityContextUserNo(Long userNo)메서드가 중복되었습니다. 따라서, 첫 번째 등장한setSecurityContextUserNo(Long userNo)메서드를 삭제하세요. -
initFollowingRelation()메서드에서setSecurityContextUserNo(1L)을 호출한 뒤에 동일한 작업을 한 번 더 수행하고 있습니다. 이 작업을 두 번 호출할 필요가 있는지 확인하세요. -
주석이 부족합니다. 각 메서드의 목적과 기능을 설명하는 주석을 추가해주세요.
| return ObjectUtils.isNotEmpty(alcoholType) ? alcohol.alcoholType.id.eq(alcoholType) : | ||
| null; | ||
| } | ||
|
|
||
| private BooleanExpression contentLike(String searchKeyword) { | ||
| return StringUtils.hasText(searchKeyword) ? alcohol.name.contains(searchKeyword) : null; | ||
| } |
There was a problem hiding this comment.
아래는 주어진 코드 패치에 대한 간단한 코드 리뷰입니다. 버그 위험과/또는 개선 제안 사항이 환영됩니다:
- import 문의 정리: 사용하지 않는 import 문을 삭제할 수 있습니다.
AlcoholRepositoryImpl클래스에서 사용되지 않는 import 문들을 삭제할 수 있습니다.ObjectUtils.isNotEmpty()메서드를 사용하여 null 체크하는 방법이 올바르게 구현되었습니다.- 코드 가독성을 높이기 위해 들여쓰기와 줄 바꿈에 주의할 수 있습니다.
전반적으로 보안상 큰 이슈나 버그를 발견할 수 없는 것 같습니다. 하지만 이 코드 패치는 부분적으로 주어져 있는 만큼 편집 전의 원본 코드가 없기 때문에 완전한 평가는 어렵습니다.
| @@ -7,4 +7,6 @@ | |||
|
|
|||
| public interface PostRepositoryCustom { | |||
| Page<Post> searchPageSimple(PostSearchCondition condition, Pageable pageable); | |||
|
|
|||
| Page<Post> searchPagePopular(Long page, Long size); | |||
| } | |||
There was a problem hiding this comment.
주어진 코드 패치를 간단히 검토해보겠습니다. 주요 버그 위험 및 개선 제안 사항을 알려드리겠습니다:
- 수정 사항:
- public interface PostRepositoryCustom: 해당 인터페이스는 커스텀 메서드를 선언하는 용도로 사용되므로 변경 없음.
- 개선 제안:
- PostRepositoryCustom 인터페이스에서 새로운
searchPagePopular메서드를 추가하려는 것으로 보입니다. 이 메서드에 대한 파라미터 설명과 반환 타입, 예외 처리 등의 정보가 필요합니다. - 모든 메서드에 대한 주석과 문서화를 추가하여 다른 개발자가 코드를 이해할 수 있도록 하세요.
- 이름 관련 개선:
searchPageSimple: 더 정확한 이름을 사용해 검색 조건에 따른 페이지 조회를 의미하도록 변경해보세요.searchPagePopular: 인기 있는 게시물에 대한 페이지 조회를 의미하도록 메서드 이름을 명확하게 바꿔보세요.
- Long을 사용한 페이징 파라미터 변경:
- Long 타입보다 int 타입을 사용하는 것이 일반적입니다. 이유는 페이징 파라미터의 크기가 크게 증가하지 않으며, int 타입의 범위로 충분하기 때문입니다.
이상입니다. 위의 제안을 고려하여 코드를 개선해보세요.
| return PageableExecutionUtils.getPage(postQueryResults, PageRequest.of(page.intValue(), | ||
| size.intValue()), () -> prepareCountQuery.fetch().size()); | ||
| } | ||
|
|
||
| private BooleanExpression contentLike(String searchKeyword) { | ||
| return StringUtils.hasText(searchKeyword) ? post.content.contains(searchKeyword) : null; | ||
| } |
There was a problem hiding this comment.
해당 코드 패치는 다음과 같은 내용입니다:
-
PageableExecutionUtils를 사용하여 검색 결과와 페이지 정보를 사용하여 Page를 반환하는
searchPagePopular메서드가 추가되었습니다. -
searchPagePopular메서드는page와size매개변수를 사용하여 인기 순으로 포스트를 검색합니다. 해당 쿼리는post와postLike테이블을 조인하고, 삭제되지 않은 포스트로 필터링한 뒤, 포스트 ID로 그룹화하고 좋아요 수에 따라 내림차순으로 정렬한 결과를 가져옵니다. 그리고 페이지네이션을 적용하여 해당 페이지의 결과만 선택합니다. -
이메일 주소 형식 유효성 검사 등의 유효성 검증 로직은 없습니다. 필요한 경우 해당 부분을 추가해야 할 수 있습니다.
-
기타 보안 문제나 버그 위험은 현재 코드에서 확인되지 않습니다.
추가 개선 제안:
-
코드에서 직접 계산된 값(page * size)을
.offset()의 의미 비 명확함을 줄이기 위해 명시적으로PageRequest.of(page.intValue(), size.intValue())로 변경할 수 있습니다. -
앞으로 예상되는 개발 이력에 맞춰 데이터 변환 및 API 업데이트 등의 과정을 최신화해야합니다. 최신화된 라이브러리를 사용하고 일반적으로 권장되는 방법을 따르는 것이 좋습니다.
| List<UserFollow> findByFollowingIsAndFollowed_IdIsInAndDelYnIs(User followingUser, | ||
| List<Long> followedUserNos, | ||
| YesOrNo delYn); | ||
|
|
||
| Long countByFollowedAndDelYnIs(User followed, YesOrNo delYn); | ||
|
|
||
| Long countByFollowingAndDelYnIs(User following, YesOrNo delYn); |
There was a problem hiding this comment.
아래는 코드 패치입니다. 버그 위험과/또는 개선 제안 등 간단한 코드 리뷰를 도와드리겠습니다:
@@ -16,6 +16,10 @@ List findByFollowingIsAndFollowedIsInAndDelYnIs(User followingUser,
List followedUserNos,
YesOrNo delYn);
-
List findByFollowingIsAndFollowed_IdIsInAndDelYnIs(User followingUser,
-
List<Long> followedUserNos, -
YesOrNo delYn); -
Long countByFollowedAndDelYnIs(User followed, YesOrNo delYn);
Long countByFollowingAndDelYnIs(User following, YesOrNo delYn);
개선 제안:
findByFollowingIsAndFollowedIsInAndDelYnIs()메서드에 대한 새로운 버전인findByFollowingIsAndFollowed_IdIsInAndDelYnIs()를 추가하였습니다. 이 메서드는followingUser,followedUserNos,delYn을 매개변수로 받습니다.countByFollowedAndDelYnIs()와countByFollowingAndDelYnIs()메서드는 유지되었습니다.
위의 변경 사항은 코드에서 더 나은 가독성과 유지 보수성을 제공합니다. 문제가 발생하지 않고 이상적으로 작동할 것으로 예상됩니다.
| @@ -8,4 +9,6 @@ public interface UserRepositoryCustom { | |||
| Page<User> getMyFollowingUsersPage(Long userNo, Pageable pageable); | |||
|
|
|||
| Page<User> getUsersOfFollowingMePage(Long userNo, Pageable pageable); | |||
|
|
|||
| Page<UserIncludeFollowCountDto> getUsersPopular(Long page, Long size); | |||
| } | |||
There was a problem hiding this comment.
이 코드 패치는 UserRepositoryCustom 인터페이스에 getUsersPopular 메서드를 추가하는 것으로 보입니다. getUsersPopular은 UserIncludeFollowCountDto 객체의 페이지를 반환하는 것 같습니다.
보안 측면에서 별다른 문제는 보이지 않습니다만, 코드 리뷰에서는 몇 가지 개선 제안을 할 수 있습니다:
- 메서드 이름을 좀 더 명확하게 작성해야 합니다. getUsersPopular 대신 getUsersWithPopularFollowers 등으로 변경할 수 있습니다.
- 메서드 파라미터에 대한 유효성 검사를 추가하는 것이 좋습니다. 예를 들어, page와 size 값이 음수인 경우에 대한 처리가 필요합니다.
- 주석을 추가하여 각 메서드의 목적과 기능을 설명하는 것이 도움이 될 수 있습니다.
- 관례에 따라 import문 순서를 정렬해야 합니다.
위의 사항을 고려해 개선된 코드 변경 내용은 아래와 같습니다:
package com.bside.bside_311.repository;
import com.bside.bside_311.dto.UserIncludeFollowCountDto;
import com.bside.bside_311.entity.User;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
public interface UserRepositoryCustom {
Page<User> getMyFollowingUsersPage(Long userNo, Pageable pageable);
Page<User> getUsersOfFollowingMePage(Long userNo, Pageable pageable);
Page<UserIncludeFollowCountDto> getUsersWithPopularFollowers(Long page, Long size);
}이러한 변경 사항을 적용하면 코드의 가독성과 유지 보수성을 향상시킬 수 있습니다.
| return userTuples.map(tuple -> { | ||
| return UserIncludeFollowCountDto.of(tuple.get(user), tuple.get(userFollow.count())); | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
위의 코드 패치에 대해 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험 및 개선 제안 사항은 환영입니다:
-
PageableExecutionUtils.getPage메서드는PageableExecutionUtils.slice메서드로 대체하는 것이 좋습니다. 이렇게 하면 전체 결과 집합을 가져오지 않고 페이지 조각만 가져올 수 있습니다. -
fetch메서드를 호출할 때 선택적으로 튜플을 매핑하는 대신queryFactory.select(user, userFollow.count()).fetchResults()를 사용하여 조인 결과를 한 번에 가져올 수 있습니다. 그런 다음 결과를 처리하기 전에 페이지화된List에서size()메서드를 호출하여 전체 카운트를 가져올 수 있습니다. -
prepareCountQuery에서 count를 추출할 필요가 없습니다. 대신.fetchCount()를 사용하면 됩니다. -
UserIncludeFollowCountDto.of대신에UserIncludeFollowCountDto의 생성자를 사용하여 DTO를 만드는 것이 더 명시적일 수 있습니다. -
페이지 번호 (
page)와 페이지 크기 (size)를int로 변환할 필요가 없습니다.Long값을 바로 사용할 수 있습니다. -
메서드 시그니처에
Long대신에long을 사용하는 것이 더 적절합니다. -
List<Tuple> userQueryResults의 이름을 변경하여 더 명확하게 표현할 수 있습니다. 예를 들어,userResultsWithFollowCount와 같이 이름을 변경할 수 있습니다. -
.of(tuple.get(user), tuple.get(userFollow.count()))대신에.of(tuple.get(0, User.class), tuple.get(1, Long.class))를 사용하여 DTO를 생성하는 것이 더 명시적일 수 있습니다. -
메서드에서 쿼리 결과가 없을 때 적절한 예외를 처리하는 것이 좋습니다.
| attachDtos.add(AttachDto.of(attach)); | ||
| } | ||
| Map<Long, List<AttachDto>> aToAMap = attachManager.getAttachInfoMapBykeysAndType(alcoholNos, | ||
| AttachType.ALCOHOL); | ||
| return alcohols.map(alcohol -> { | ||
| List<AttachDto> attachDtos = aToAMap.getOrDefault(alcohol.getId(), new ArrayList<>()); | ||
| return AlcoholResponseDto.of(alcohol, attachDtos); |
There was a problem hiding this comment.
이 코드 패치는 AlcoholService 클래스에 대한 수정 사항을 보여줍니다. 여기서 AlcoholManager와 AttachManager가 추가되었습니다. 이 코드의 주요 변경 사항은 다음과 같습니다:
AlcoholManager와AttachManager를 import하고, 해당 객체들을 필드로 추가했습니다.getAlcoholV2메서드를 수정하여alcoholManager.searchAlcohol을 호출하여 알코올 검색 및 페이징을 처리합니다.getAlcoholV2메서드 내에서attachRepository에 접근하는 대신,attachManager.getAttachInfoMapByKeysAndType을 호출하여 첨부 파일 정보를 가져옵니다.
개선할 수 있는 몇 가지 사항은 다음과 같습니다:
searchKeyword와alcoholType매개변수의 유효성을 확인해야 합니다. 값이 없거나 잘못된 경우 적절한 예외를 발생시키는 등의 처리를 추가해야 합니다.- 변수 및 메서드 이름은 의미가 분명하게 되도록 개선할 수 있습니다. 현재 코드는
aToAMap및searchAlcohol등 명확하지 않은 이름 사용으로 인해 이해하기 어려울 수 있습니다. - 주석이 부족합니다. 코드를 이해하는 데 도움이 되는 주석을 추가하는 것이 좋습니다.
- 생성자에서 매개변수를 주입받게 되어 있는데, 관례적으로 인스턴스 변수는 클래스 상단에 선언하는 것이 좋습니다.
- 메서드에 대한 테스트가 필요합니다. 단위 테스트를 작성하여 코드의 동작과 예외 조건을 확인하는 것이 좋습니다.
위 사항들을 고려하여 코드 리뷰를 진행할 수 있습니다.
| Boolean isFollowedByMe = uToUFMap.containsKey(user.getId()); | ||
| return UserResponseDto.of(user, attachDtos, isFollowedByMe); | ||
| })); | ||
| } | ||
| } |
There was a problem hiding this comment.
위의 코드 패치를 간단한 코드 리뷰해드리겠습니다. 버그 위험과/또는 개선 제안은 환영합니다.
UserIncludeFollowCountDto와ResultDto클래스가 추가되었습니다.getUserFollowInfoFollowingIsAndFollowed_IdIsIn메서드가 추가되었습니다. 이 메서드는 주어진 사용자 번호 목록에서 팔로우 정보를 검색하는 데 사용됩니다.getUsersPopular메서드가 추가되었습니다. 이 메서드는 인기 있는 사용자 목록을 가져오는 데 사용됩니다. 반환된Page<UserResponseDto>에는 팔로우 정보 및 첨부 파일 정보도 포함됩니다.
개별적인 버그 위험은 특정 코드의 구현 세부 사항을 파악해야 알 수 있으므로, 일반적인 리뷰로 보입니다. 추가된 코드 자체는 유효해보이며, 새로운 기능을 제공하기 위한 마이너한 변경으로 보입니다. 특별한 문제점이나 개선할만한 부분은 보이지 않습니다.
| public String getMessage() { | ||
| return this.message; | ||
| } | ||
| } |
There was a problem hiding this comment.
해당 코드는 ResultCode라는 열거형(Enumeration)을 정의하고 있습니다. 이 열거형은 다양한 결과 코드와 그에 해당하는 메시지를 나타내는 상수들을 포함하고 있습니다.
아래는 코드의 개선 사항 및 버그 위험성에 대한 제안입니다:
- SUCCESS, FAIL, UNAUTHORIZED, NOT_FOUND, INTERNAL_SERVER_ERROR와 같은 결과 코드가 정의되어 있습니다. 이러한 코드는 예상대로 동작할 수 있습니다.
- 코드의 구조 및 작명에는 큰 문제가 없어 보입니다.
- 하지만 결과 코드와 메시지를 저장하기 위해 문자열을 사용하는 것은 번역의 어려움과 오류 가능성을 야기할 수 있습니다. 대신, 모든 지역화된 메시지를 관리하기 위해 리소스 파일을 사용하는 방법을 고려할 수 있습니다. 이렇게 하면 다국어 지원이 쉬워지며, 코드의 유지보수성도 향상됩니다.
- 더 많은 결과 코드를 추가해야 한다면, 기존 코드에 계속 추가하여 확장할 수 있습니다.
지금까지 확인된 버그나 위험요소는 없어 보입니다. 코드는 간단하며 목적에 맞게 잘 작성된 것 같습니다. 읽기 쉽고 이해하기 쉬운 코드입니다.
| String queryParameter = "?alcoholType=asdf&searchKeyword=소주"; | ||
| mockMvc.perform(get(String.format("/alcohols/v2%s", queryParameter))) | ||
| .andExpect(status().is4xxClientError()); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
코드 리뷰를 진행해보겠습니다. 아래는 해당 패치의 리뷰 결과입니다:
getAlcoholV2_success()메서드와 동일한 방식으로getAlcoholV2_fail()메서드가 구현되었습니다.- 이 메서드에서 요청하는 URL은 잘못된 쿼리 파라미터 값을 사용하고 있습니다.
- "alcoholType=asdf"는 잘못된 값이므로 올바른 양식으로 수정해야 할 것입니다.
리뷰 결과:
@Test
void getAlcoholV2_fail() throws Exception {
// given
// when
// then
String queryParameter = "?alcoholType=1&searchKeyword=소주";
mockMvc.perform(get(String.format("/alcohols/v2%s", queryParameter)))
.andExpect(status().is4xxClientError());
}위의 코드를 아래와 같이 변경하세요.
@Test
void getAlcoholV2_fail() throws Exception {
// given
// when
// then
String queryParameter = "?alcoholType=invalidType&searchKeyword=소주";
mockMvc.perform(get(String.format("/alcohols/v2%s", queryParameter)))
.andExpect(status().is4xxClientError());
}추가적인 사항:
- 주어진 코드 패치의 나머지 부분은 비어있어 확인할 내용이 없습니다.
- 코드 자체가 작성자의 의도에 맞게 동작하는지, 의도한 테스트 케이스를 구현하고 있는지에 대해서는 판단할 수 없습니다.
- 코드의 완결성을 위해 파일 끝에 줄 바꿈을 추가하는 것이 좋습니다.
| get(String.format("/posts/popular%s", queryParameter))) | ||
| .andExpect(status().is4xxClientError()); | ||
| } | ||
|
|
||
| @Test | ||
| void getPostDetail_success() { | ||
| } |
There was a problem hiding this comment.
코드 리뷰를 해드리겠습니다:
- import 문에서
com.bside.bside_311.service.PostService가com.bside.bside_311.service.PostFacade로 변경되었습니다. @MockBean으로 선언된PostService가PostFacade로 변경되었습니다.postService.addPost()가postFacade.addPost()로 변경되었습니다.getPostsPopular_success()및getPostsPopular_fail()메서드가 추가되었습니다./posts/popular엔드포인트에 대한 테스트입니다.
getPostDetail_success()메서드는 아직 구현되지 않았습니다.
개선 제안:
getPostDetail_success()메서드를 완성해야 합니다.
버그 및 위험 요소:
- 코드에서 심각한 버그 또는 위험 요소는 보이지 않습니다. 그러나 현재 주어진 코드만으로는 전체 상황을 파악하기 어렵습니다. 사용되는 다른 클래스나 메서드의 내용을 알면 더 정확하게 코드 검토를 할 수 있습니다.
| mockMvc.perform(get(String.format("/user/popular%s", queryParameter)).header("Authorization", | ||
| "Bearer " + userAccessToken)) | ||
| .andExpect(status().is4xxClientError()); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
코드 리뷰를 진행하겠습니다:
getUsersPopular_success()테스트 메서드:
- 현재 구현은 요청을 수행하는 것만 확인하고 있습니다. 응답의 내용에 대한 검증도 추가하는 것이 좋습니다.
getUsersPopular_fail()테스트 메서드:
- 현재 구현은 잘못된 쿼리 매개변수 값을 사용하여 4xx 클라이언트 오류가 발생하는지 확인하고 있습니다. 오류 메시지나 오류 유형과 관련된 정보를 추가하여 더 유용한 테스트를 만들 수 있습니다.
- 모든 메서드:
// given,// when,// then주석 섹션에 해당하는 코드가 없어 해당 영역을 완성해야 합니다.- 테스트 내부에 명확한 assert 문이 없으므로 후속 작업으로 응답 내용을 검증하는 assert 문을 추가하는 것이 좋습니다.
- 파일의 끝에 새로운 줄이 없습니다. Java 관례에 따라 파일의 끝에는 개행 문자(새로운 줄)가 있어야 합니다.
개선 제안:
getUsersPopular_success()메서드에서andExpect(content().string(containsString("userNo")));를 통해 응답 내용에 "userNo"가 포함되어 있는지 확인하고 있습니다.containsString메서드 대신,jsonPath를 사용하여 응답 내용을 JSON으로 처리하고 필드를 명시적으로 검증하는 것이 더 안전합니다.getUsersPopular_fail()메서드에서는 유효하지 않은 쿼리 매개변수 값을 사용하여 4xx 클라이언트 오류 확인만 수행하고 있습니다. 오류 메시지 또는 상세한 오류 정보를 추가하여 실제 원인을 파악하는 것이 유용할 수 있습니다.
이상입니다. 코드 리뷰를 위해 도움을 드릴 수 있어 기쁩니다!
| return alcoholTypeList; | ||
| } | ||
|
|
||
| } No newline at end of file |
There was a problem hiding this comment.
이 코드는 AlcoholRepositoryTest 클래스의 JUnit 테스트입니다. 해당 테스트는 AlcoholType에 따라 알코올을 검색하는 기능을 테스트하는 것으로 보입니다.
코드 리뷰를 진행하면 다음과 같습니다:
- DataJpaTest 어노테이션을 사용하여 데이터베이스 관련 테스트 환경을 구성하고 있습니다.
- alcoholTypeRepository와 alcoholRepository의 의존성을 주입받은 후, findByAlcoholType() 메서드에서 알코올 타입별로 알코올을 조회하는 테스트를 수행합니다.
- dataInitAlcoholType() 메서드와 dataInitAlcohol() 메서드를 사용하여 초기 데이터를 생성합니다.
- alcoholRepository.searchAlcoholPage() 메서드를 사용하여 알코올 타입별로 알코올을 조회합니다.
- 조회 결과의 크기를 확인하고, 예상된 값인 4와 일치하는지 확인합니다.
개선할 부분:
- 테스트 메서드명은 좀 더 명확하게 작성하는 것이 좋습니다. 현재는 "findByAlcoholType"입니다. 좀 더 구체적인 이름으로 변경하여 테스트 목적을 명확히 드러내는 것이 좋습니다.
- System.out.println 대신에 로깅 프레임워크를 사용하면 로그를 보다 효율적으로 처리할 수 있습니다.
버그 및 위험 요소:
이 코드 조각은 일반적인 JPA 데이터베이스 작업을 수행하고 있는 것 같으므로 큰 버그 또는 위험이 보이지는 않습니다. 다만, 데이터베이스와 통합되어있는 테스트를 실행할 때는 주의해야 합니다. 데이터베이스 연결 정보나 초기 데이터 생성에 관련된 문제가 발생할 수 있으므로 실제 운영환경에서의 테스트 시에는 주의하세요.
| return inputPosts; | ||
| } | ||
|
|
||
| } No newline at end of file |
There was a problem hiding this comment.
위의 코드 패치를 보면, PostRepositoryTest라는 클래스가 정의되어 있습니다. 이 클래스는 DataJpaTest 애노테이션을 가지고 있으며 JPA 엔티티 매니저를 사용하는 테스트를 위해 구성된 것으로 보입니다.
주요 내용은 다음과 같습니다:
searchPagePopular메서드 테스트와searchPagePopularPageTest메서드 테스트가 포함되어 있습니다.dataInitSearchPagePostPopular메서드는 초기 데이터를 설정하는 데 사용됩니다.
버그 위험 및 개선 제안:
- 총 게시글 수가 예상대로 인지 확인하기 위해
searchPagePopular및searchPagePopularPageTest메서드에서Assertions.assertThat(posts.getTotalElements()).isEqualTo(3);를 사용하고 있습니다. 여기서3이 한계값이므로, 추후에 테스트 데이터를 변경할 경우, 하드코딩된 값을 함께 변경해야 합니다. 외부 값으로 설정하여 유연성을 높이는 것을 고려해 볼 수 있습니다. UserRepository,PostRepository,PostLikeRepository등이 자동 주입되고 있지만, 이들이 올바르게 설정되지 않으면 테스트가 실패할 수 있습니다. 필요한 리포지토리 종속성이 모두 제공되고 의존성 주입이 올바르게 구성되었는지 확인해야 합니다.postRepository.searchPagePopular메서드의 파라미터로0L과10L값을 사용하고 있습니다. 이 값들에 대한 의도와 테스트 목적을 이해하고 있는지 확인해야 합니다.
|
|
||
| import java.util.List; | ||
|
|
||
| @ExtendWith(SpringExtension.class) | ||
| @DataJpaTest | ||
| @AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) | ||
| class UserFollowRepositoryTest { |
There was a problem hiding this comment.
코드 리뷰를 해드리겠습니다.
-
@ExtendWith(SpringExtension.class)주석 처리:@ExtendWith(SpringExtension.class)는 JUnit 5에서 스프링을 사용하는 테스트 클래스에 필요한 확장 기능을 제공합니다. 그러나 해당 코드에서는 이미@DataJpaTest어노테이션이 있어서 확장 기능이 중복되므로, 주석 처리하여 제거해도 됩니다. -
@Autowired선언: 코드에서는@Autowired를 사용하여 의존성 주입을 수행하고 있습니다. 그러나 Spring Framework 버전에 따라@Autowired어노테이션 사용이 필요하지 않을 수 있습니다. Spring Framework 4.3 이상에서는 자동으로 의존성을 주입하기 때문에, 해당 코드에서@Autowired어노테이션을 제거해도 될 것입니다. -
@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)설정: 해당 설정은 데이터베이스를 테스트 용으로 바꾸지 않도록 막아줍니다. 이 설정은 테스트 시 현재 사용 중인 데이터베이스를 그대로 유지하며, 해당 코드에서 사용하는 데이터베이스를 변경하지 않습니다. 그러나 이 설정을 사용하기 위해서는 데이터베이스 연결 정보가 올바르게 설정되어 있어야 합니다. 데이터베이스 연결에 문제가 없는 경우, 해당 설정을 그대로 유지해도 될 것입니다. -
import문 정리: 사용되지 않는org.junit.jupiter.api.extension.ExtendWith와org.junit.jupiter.api.Test를 제거하고, 필요한com.bside.bside_311.entity.UserFollow,com.bside.bside_311.entity.YesOrNo,java.util.List만 남겨두세요.
위 내용들을 고려하여 코드 리뷰를 완료할 수 있습니다.
| userFollowRepository.saveAllAndFlush(userFollowList); | ||
| return userList; | ||
| } | ||
|
|
||
| } No newline at end of file |
There was a problem hiding this comment.
이 코드 리뷰에는 다음과 같은 개선점 및 버그 위험이 있습니다:
@Transactional어노테이션은 제거되었습니다.@SpringBootTest대신@DataJpaTest어노테이션이 추가되어 데이터베이스와 관련된 테스트 설정이 적용됩니다.@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)어노테이션을 사용하여 테스트 시 기존 데이터베이스를 사용하도록 설정합니다.getUsersOfFollowingMePage_success()메서드는 테스트가 성공하는지 확인하기 위한 검증 코드를 포함하고 있습니다.searchUserPopularTest()와searchUserPopularPageTest()메서드에서 유저 데이터를 초기화하고, 팔로워 수에 따라 인기 있는 유저를 가져옵니다.dataInitSearchUserPopular()메서드는 유저와 팔로우 데이터를 초기화하는 데 사용됩니다.
버그 위험:
- 코드 일부에만 주석 (
//@Rollback(false))이 있습니다. 이 주석의 의도를 알 수 없으므로 주석을 수정하거나 제거해야 합니다.
개선 제안:
- 주석 또는 Javadoc을 추가하여 각 테스트 메서드의 목적과 예상 동작을 설명할 수 있습니다.
- 테스트 코드에서 중복된 코드가 있습니다. 이를 제거하고 공통 메서드를 사용하여 코드를 간소화할 수 있습니다.
코드 리뷰에 대한 정보가 여기까지입니다. 더 자세한 배경 정보나 다른 질문이 있으면 말씀해주세요.
[feat] add post_user_static_api
@dongseoki
dongseoki committed last week
Merge pull request #91 from potenday-project/feat/post_user_static
@dongseoki
dongseoki committed last week
Commits on Jan 7, 2024
[fix] refactor. 코드 리팩토링.프로필 정보 추가.
@dongseoki
dongseoki committed yesterday
[fix] refactor. 알코올타입으로 술정보 조회 기능 추가.