Develop #75
Develop #75
Conversation
1. [get]/user/me , [get]/posts , [get]/posts/{postNo} 에서 User의 PK 값이 같이 오도록 작업
2. user/{userNo}/summary : followingCount 추가.
3.[Get]/posts 에서 유저이름이나 pk 로 특정유저가 작성한 포스트만 뽑아볼 수 있도록 조치(searchUserNos=1,2,3)
4. POST /posts에서 alcoholInfo를 제거.
5. get /posts 수정 여부가 항상 true로 나오는 이슈 해결
6. 게시글의 술 선택 수정시, 수정이 안되던 버그 조치.
7. updatedDt- > lastModifiedDate로 변경일시의 명칭 통일.
[refactor] sonarlint 검사 결과 반영.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
|
|
||
| ## logs ## | ||
| logs/ | ||
|
|
||
| ### security information | ||
| application.yml | ||
|
|
||
| ### operation log | ||
| replay_pid.*.log |
There was a problem hiding this comment.
위 코드 패치의 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험 및 개선 제안은 환영합니다.
개선 제안:
gradle-app.setting파일은 삭제되었습니다.- 경로가
src/main/generated/**인 모든 파일이 제외됩니다. src/main/resources디렉토리 내의application-prd.yml과application.properties파일이 경로 없이 나열됩니다.application-swagger.yml파일이 추가되었습니다.logs/디렉토리 내의 모든 로그 파일도 제외됩니다.application.yml파일도 제거되었습니다.replay_pid.*.log와 같은 형태의 파일도 제외됩니다.
이상입니다.
| @@ -2,6 +2,8 @@ | |||
|
|
|||
| * http://localhost:8080/swagger-ui/index.html | |||
|
|
|||
| * http://localhost:8080/test | |||
|
|
|||
| # 수동 배포 방법 정리.(개선 필요.) | |||
|
|
|||
| ```sh | |||
There was a problem hiding this comment.
이 코드 패치에 대한 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험과/또는 개선 제안은 환영합니다:
- 가독성을 높이려면 URL 주석 앞에 "@"를 추가하는 것이 좋습니다. 예: "@@ -2,6 +2,8 @@".
- 새로운 URL인 "http://localhost:8080/test"가 추가되었다는 것 같습니다. 이 URL이 올바르게 작동하는지 확인해야 합니다.
- "수동 배포 방법 정리"라는 주석이 있습니다. 이 설명이 충분한지, 필요한 개선사항이 있는지 검토해야 합니다. 어떤 개선 사항이 필요한지 구체적으로 언급해야 합니다.
주의 사항:
제가 한국어를 사용하여 응답하는 AI 모델이기 때문에 우리의 의사소통에 한계가 있을 수 있습니다. 번역이나 자세한 내용에 대해서는 정확한 해석을 위해 영어로 질문해주시면 감사하겠습니다.
| @@ -82,7 +82,8 @@ dependencies { | |||
| // p6spy : sql 로그 남기기(바인딩된 파라미터 간편 확인.) | |||
| implementation 'com.github.gavlyukovskiy:p6spy-spring-boot-starter:1.9.0' | |||
|
|
|||
| testImplementation 'org.springframework.boot:spring-boot-starter-test' | |||
| // testImplementation 'org.springframework.boot:spring-boot-starter-test' | |||
| implementation 'org.springframework.boot:spring-boot-starter-test' | |||
| testImplementation 'org.mybatis.spring.boot:mybatis-spring-boot-starter-test:3.0.2' | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
코드 리뷰를 해드리겠습니다.
-
의존성 추가:
- commons-fileupload와 commons-io 라이브러리의 버전 지정이 없는데, 해당 라이브러리 버전을 명시하는 것이 좋습니다.
- 중복된 의존성(com.mysql:mysql-connector-j)이 있어서 한 번만 선언하는 것이 좋습니다.
-
주석 처리:
- runtimeOnly 'com.mysql:mysql-connector-j' 부분이 주석 처리되어 있는데, 주석을 제거하고 필요한 의존성을 추가하는 것이 좋습니다.
-
테스트 관련 의존성:
- 'org.springframework.boot:spring-boot-starter-test'가 주석 처리되어 있는데, 이 부분을 주석 해제하고 테스트에 필요한 의존성으로 추가하는 것이 좋습니다.
| echo "> JAR Name: $JAR_NAME" | ||
|
|
||
| nohup java -jar $REPOSITORY/"$JAR_NAME" --spring.profiles.active=swagger & | ||
|
|
There was a problem hiding this comment.
이 코드 패치는 다음과 같은 작업을 수행합니다:
- 지정된 저장소 경로에서 git reset --hard 명령어를 실행하여 이전 변경 내용을 모두 삭제합니다.
- git pull 명령어를 실행하여 최신 변경 내용을 가져옵니다.
- gradlew build 명령어를 사용하여 프로젝트를 빌드합니다.
- 빌드된 파일을 지정된 저장소로 복사합니다.
- 현재 구동 중인 애플리케이션의 PID(Process ID)를 확인합니다.
- 현재 구동 중인 애플리케이션이 없으면 아무 작업도 수행하지 않고, 있으면 해당 애플리케이션을 종료합니다.
- 새로운 애플리케이션을 배포하고 실행합니다.
이 코드에서 주의해야 할 몇 가지 사항이 있습니다:
- 현재 코드에서는 JAR 파일의 이름에 "bside_311" 문자열을 포함하는 파일을 배포하고 실행하고 있습니다. 이 부분을 프로젝트의 요구 사항에 맞게 수정해야 합니다.
- kill -9 명령어를 사용하여 애플리케이션을 강제로 종료하고 있습니다. 이는 예상치 못한 문제가 발생할 수 있으므로 조심해야 합니다. 가능한 경우, 더 안전한 방법을 사용하는 것이 좋습니다.
- 현재 코드에서는 nohup 명령어를 사용하여 백그라운드에서 애플리케이션을 실행하고 있습니다. 이 경우 애플리케이션의 로그 출력이나 오류 메시지를 확인하기 어렵습니다. 필요에 따라 로그 기능을 추가하거나 실행 방식을 변경할 수 있습니다.
개선 제안:
- 실행가능한 스크립트 파일(.sh)을 작성하여 각 단계를 별도의 함수로 모듈화하고, 잠재적인 오류 상황을 처리하도록 구성할 수 있습니다.
- 애플리케이션의 실행과 관련된 환경 변수를 구성 파일로 분리하여 유연성을 높일 수 있습니다.
- 롤백 기능을 추가하여 배포 과정 중 예상치 못한 문제가 발생했을 때 이전 버전으로 되돌릴 수 있는 기능을 제공할 수 있습니다.
- 작업을 자동화하여 CI/CD 파이프라인에 통합할 수 있습니다.
이러한 개선 사항은 코드의 가독성, 유지 보수성 및 안정성을 개선하는 데 도움이 될 수 있습니다.
| @@ -36,7 +36,8 @@ cp ./build/libs/*.jar $REPOSITORY/ | |||
|
|
|||
| echo "> 현재 구동중인 애플리케이션 pid 확인" | |||
|
|
|||
| CURRENT_PID=$(pgrep -f ${PROJECT_NAME}.*.jar) | |||
| #CURRENT_PID=$(pgrep -f ${PROJECT_NAME}.*.jar) | |||
| CURRENT_PID=$(pgrep -f "active=prd") | |||
|
|
|||
| echo "$CURRENT_PID" | |||
|
|
|||
There was a problem hiding this comment.
제가 주어진 코드의 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험이나 개선 제안에 대해서도 적절한 응답을 드리겠습니다.
@@ -36,7 +36,8 @@ cp ./build/libs/*.jar $REPOSITORY/
echo "> 현재 구동중인 애플리케이션 pid 확인"
-CURRENT_PID=$(pgrep -f ${PROJECT_NAME}..jar)
+#CURRENT_PID=$(pgrep -f ${PROJECT_NAME}..jar)
+CURRENT_PID=$(pgrep -f "active=prd")
echo "$CURRENT_PID"
-
이 코드 패치는 빌드된 JAR 파일을 지정된 리포지토리로 복사하는 명령어입니다. 이 부분은 문제가 없어 보입니다.
-
주석 처리된 줄을 봤을 때,
pgrep명령을 사용하여 프로젝트 이름과.jar파일 확장자를 기준으로 현재 실행 중인 애플리케이션의 프로세스 ID(PID)를 찾고 있습니다. 그러나 현재 코드에서는 이 부분이 주석 처리되어 있으므로 실제로는 PID를 얻을 수 없을 것입니다. -
대신에 주석 처리된 아래의 줄은
pgrep명령을 사용하여 "active=prd"라는 문자열을 포함하는 프로세스 ID를 찾습니다. 개발 환경에 따라서는 이 방식이 정확한 PID를 얻을 수 있습니다. -
echo "$CURRENT_PID"는 현재 실행 중인 애플리케이션의 PID를 출력하는 부분입니다.
버그 위험:
- 현재 코드에서는 주석 처리된 부분을 사용하고 있으므로 실제로는 현재 실행 중인 애플리케이션의 PID를 얻을 수 없습니다.
개선 제안:
- PID를 찾는 방식과 관련된 문제가 있는지 확인해야 합니다. 프로젝트의 특정 버전을 기준으로 PID를 찾아야 한다면, 해당 조건을 제대로 지정하여 PID를 찾을 수 있는지 확인하세요.
- PID를 찾는 대체 방법을 고려할 수 있습니다. 예를 들어, 일련의 프로세스 관리 도구 (예: ps, pgrep 등)를 사용하여 PID를 가져오는 대신에 애플리케이션 자체가 PID 파일을 작성하는 등의 방법을 사용할 수도 있습니다.
| } | ||
| return Optional.of(userNoFromAuthentication); | ||
| }; | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
이 코드 패치에서 주목해야 할 부분은 다음과 같습니다:
-
@EnableJpaAuditing어노테이션의 삭제: 이 어노테이션은 JPA 감사(Auditing)를 사용하도록 설정하는 역할을 합니다. 만약 해당 기능을 사용하려면, 어노테이션이 필요합니다. 코드에서는 이 어노테이션이 삭제되었으므로 JPA 감사 기능이 비활성화된 것으로 보입니다. 이것이 의도한 동작인지 확인해야 합니다. -
main메소드의 들여쓰기 변경:main메소드의 내용이 들여쓰기에 의해 변경되었습니다. 이는 가독성 관점에서 큰 문제가 되지 않지만, 코드 스타일 일관성을 유지하기 위해서 들여쓰기 스타일을 통일하는 것이 좋습니다. -
auditorProvider()메소드:auditorProvider()메소드는@Bean어노테이션이 지정된 메소드로,AuditorAware인터페이스의 구현체를 반환합니다. 이 메소드는 현재 인증된 사용자의 ID 값을Optional로 반환합니다. 이 부분은 보안 측면에서 주의해야 합니다.AuthUtil.getUserNoFromAuthentication()에서 반환된 값이 신뢰할 수 있는지 확인하고, 인증되지 않은 사용자에게는Optional.empty()를 반환하는 것이 좋습니다.
코드 패치의 개선점:
@EnableJpaAuditing어노테이션 삭제에 대한 이유를 명확하게 설명하거나, JPA 감사 기능을 포함하도록 복구해야 합니다.- 들여쓰기 스타일을 일관성 있게 유지합니다.
auditorProvider()메소드에서 보안 검사와 관련된 부분을 검토하고 개선합니다.
| public static PostDomain of(Post post) throws CloneNotSupportedException { | ||
| return (PostDomain) Post.of(post); | ||
| } | ||
| } |
There was a problem hiding this comment.
주어진 코드 패치에 대한 간단한 코드 리뷰를 도와드리겠습니다.
-
상속:
PostDomain클래스가Post클래스를 확장하고 있습니다. 이는 현재 코드에서의 목적을 알 수 없으므로, 그렇게 많이 사용되지 않을 경우에는 상속을 피하는 것이 좋습니다. -
정적 메서드:
of()메서드가 정적으로 선언되어 있습니다. 이 메서드를 정적으로 만든 이유와Post.of()메서드의 구현 내용을 알 수 없기 때문에 가독성과 유지 관리 측면에서 고려해볼 필요가 있습니다. -
예외 처리:
of()메서드에서CloneNotSupportedException예외 처리가 포함되어 있습니다. 이 예외 처리가 필요한 이유를 알 수 없으며, 예외를 처리하는 대신 예외를 발생시키거나 검사하지 않는 방법을 고려해보는 것이 좋습니다. -
패키지 및 클래스명: 패키지 이름은 일반적으로 소문자로 작성되어야 하며, 클래스 이름은 CamelCase 형식으로 작성하는 것이 권장됩니다.
com.bside.bside_311패키지명과PostDomain클래스명을 수정하는 것이 좋을 수 있습니다. -
주석: 코드에는 아무런 주석이 없습니다. 주석을 사용하여 코드의 목적과 동작을 설명하는 것이 좋습니다.
위의 사항들을 고려하여 코드를 개선할 수 있습니다.
| postDomainMultiple.setTotalCount(totalCount); | ||
| return postDomainMultiple; | ||
| } | ||
| } |
There was a problem hiding this comment.
이 코드 패치는 PostDomainMultiple이라는 클래스를 정의하고 있습니다. 해당 클래스는 PostRepository와 PostMybatisRepository의 인스턴스를 사용하여 게시물 데이터를 처리하는 기능을 제공합니다.
코드 리뷰 내용:
@AllArgsConstructor어노테이션은 생성자를 자동으로 만들어 주는 롬복 어노테이션입니다. 그러나 해당 클래스에서 생성자의 매개변수는final로 선언되어 있으므로,@RequiredArgsConstructor어노테이션을 사용하는 것이 적합할 것입니다.postDomainList변수의 타입이List<PostDomain>으로 설정되어 있는데, 주석 처리된 줄에서List<PostResponseDto>로 변경한 것으로 보입니다. 필요에 따라 변수명과 타입을 재정의해야 할 것 같습니다.getPostsMvos.stream().map(Post::of).toList()와 같은 부분에서.toList()메소드가 없다는 오류가 발생할 수 있습니다. JDK 1.16 미만의 버전을 사용하는 경우.collect(Collectors.toList())메소드로 수정해야 합니다.
개선 제안:
- 클래스 이름인
PostDomainMultiple은 해당 클래스의 역할이 뚜렷하지 않아보입니다. 좀 더 명확하게 클래스의 역할을 설명할 수 있는 이름을 선택하는 것이 좋습니다. - 코드에서
throw new RuntimeException(e)를 사용하고 있습니다. 이는 예외 처리에 대한 명시성을 잃어버리게 됩니다. 해당 예외를 던지거나 적절한 예외 유형으로 변경하는 것이 좋습니다.
위의 내용을 참고하여 코드를 수정 및 개선하셔야 할 것입니다.
| @Configuration | ||
| @EnableJpaAuditing | ||
| public class JpaConfiguration { | ||
| } |
There was a problem hiding this comment.
이 코드 패치는 com.bside.bside_311.config 패키지 내에서 JpaConfiguration 클래스를 선언하고 구성합니다. 이 클래스는 JPA 감사(auditing) 기능을 활성화하는 데 사용됩니다.
이 코드에는 버그나 위험이 없어 보입니다. 하지만 몇 가지 개선 제안사항이 있습니다:
- 주석(comment): 코드의 의도와 동작에 대해 설명하는 주석을 추가하는 것이 도움이 될 수 있습니다.
- 패키지 이름:
com.bside.bside_311.config패키지가 적절한지 확인해야 합니다. 프로젝트의 컨벤션과 구조에 맞도록 패키지 이름을 조정할 수 있습니다. @EnableJpaAuditing사용 영역 조정:@EnableJpaAuditing어노테이션을 필요한 곳에만 적용할 수 있도록,@Configuration어노테이션 바로 위로 이동시킬 수 있습니다.
이러한 개선 사항은 코드의 가독성과 유지 보수성을 향상시킬 수 있습니다.
| @@ -133,5 +306,4 @@ public void init2() { | |||
| } | |||
| } | |||
|
|
|||
|
|
|||
| } | |||
There was a problem hiding this comment.
이 코드 패치는 다음과 같은 변경 사항을 포함합니다:
AttachController를 추가했습니다.- 필요한 import 문을 추가하였습니다.
easyUserRoleAuthenticationFactory메서드를 작성하여 쉽게 사용자 역할 정보로 인증 객체를 생성할 수 있도록 했습니다.- 파일 변환 관련 유틸리티 메서드(
convertInputStreamToFile및copyInputStreamToFile)를 추가했습니다. - 보안 컨텍스트의 사용자 번호를 설정하는
setSecurityContextUserNo메서드를 추가했습니다. - 초기화 작업을 위해
init메서드에 여러 동작을 추가했습니다. - 사용자 등록, 주류 등록, 게시물 등록 작업을 추가했습니다.
- 팔로우, 좋아요, 댓글, 인용 등의 동작을 추가했습니다.
- 첨부 파일 등록을 위한
attachPhoto메서드 및 관련 도우미 메서드를 추가했습니다. setSecutiryContextDoSomethingAndClear메서드를 추가하여 보안 컨텍스트 내에서 어떤 작업을 수행하고 비워주는 동작을 수행할 수 있도록 하였습니다.securityContextHolderClear메서드를 추가하여 보안 컨텍스트를 초기화하는 동작을 정의했습니다.
개선 제안:
- 코드를 이해하기 쉽도록 변수 및 메서드 이름을 명확하게 짓는 것이 좋습니다.
Initializer클래스의 역할과 책임을 분리하여 단일 책임 원칙을 따르도록 리팩토링하는 것이 좋습니다.- 에러 처리와 예외 처리를 추가하여 코드의 신뢰성을 높이는 것이 좋습니다.
- 보안 컨텍스트 설정과 관련된 로직을 추상화하여 재사용성을 높이는 것이 좋습니다.
위의 개선 제안은 해당 코드에서 확인할 수 있는 현재 내용에 기반합니다. 전체 시스템 구조와 요구사항에 따라 다른 개선 사항이 있을 수 있습니다.
| @@ -7,7 +7,7 @@ | |||
|
|
|||
| import java.util.Optional; | |||
|
|
|||
| public interface AlcoholRepository extends JpaRepository<Alcohol, Long>{ | |||
| public interface AlcoholRepository extends JpaRepository<Alcohol, Long>, AlcoholRepositoryCustom { | |||
| Optional<Alcohol> findByNameAndDelYnIs(String name, YesOrNo delYn); | |||
|
|
|||
| Optional<Alcohol> findByIdAndDelYnIs(Long userNo, YesOrNo delYn); | |||
There was a problem hiding this comment.
주어진 코드 패치는 JpaRepository 인터페이스를 상속받는 AlcoholRepository 인터페이스입니다. 이 코드의 주요 변경점은 AlcoholRepositoryCustom 인터페이스를 함께 상속받는 것입니다.
코드 리뷰를 수행하면서 주의해야 할 점과 개선 제안은 다음과 같습니다:
- 주석 : 현재 코드에는 주석이 없으므로, 필요한 경우 기능 및 메소드에 대한 설명을 추가하는 것이 좋습니다.
- 메소드 네이밍 : 메소드의 네이밍은 경례어(Legacy) 스타일을 따르고 있습니다. 예를 들어, "findByIdAndDelYnIs"는 "findByUserNoAndDelYn"으로 변경하는 것이 가독성을 향상시킬 수 있습니다.
- YesOrNo 타입 : "YesOrNo"라는 사용자 정의 타입을 사용하는데, 해당 타입이 어떤 값을 표현하는지에 대한 상세 설명 및 사용 예시가 필요합니다. 만약 Boolean 타입으로 대체 가능한 경우, Boolean 타입을 사용하는 것이 더 간결할 수 있습니다.
위의 제안 사항에 참고하시기 바랍니다.
|
|
||
| public interface AlcoholRepositoryCustom { | ||
| Page<Alcohol> searchAlcoholPage(AlcoholSearchCondition condition, Pageable pageable); | ||
| } |
There was a problem hiding this comment.
이 코드 패치는 AlcoholRepositoryCustom 인터페이스를 정의하는 것으로 보입니다. 해당 인터페이스는 AlcoholSearchCondition과 Pageable을 사용하여 알코올을 검색하고 페이지별로 반환하는 searchAlcoholPage 메서드를 가지고 있습니다.
여기 몇 가지 개선 및 버그 가능성을 제안드릴 수 있습니다:
AlcoholRepositoryCustom인터페이스에 어떠한 구현체도 주어지지 않았으므로, 해당 인터페이스를 구현하는 클래스를 작성해야 합니다.- 필요에 따라
AlcoholRepositoryCustom인터페이스 내에 다른 유용한 메서드를 추가할 수 있습니다. AlcoholSearchCondition클래스의 내용은 주어지지 않았으므로, 해당 클래스를 검토할 수 있는 정보가 없습니다.- 예외 처리와 에러 핸들링 등의 오류 처리 논리에 대한 정보도 없습니다. 이러한 측면에서 개선이 필요할 수 있습니다.
- 성능 향상이나 데이터 접근 최적화를 위해
searchAlcoholPage메서드가 사용할 수 있는 인덱스, 조인 또는 다른 최적화 기법을 고려할 수 있습니다.
제시된 코드 패치에 대한 추가 정보가 없으므로 더 자세한 코드 리뷰는 어렵습니다.
| private BooleanExpression notDeleted() { | ||
| return alcohol.delYn.eq(YesOrNo.N); | ||
| } | ||
| } |
There was a problem hiding this comment.
코드 리뷰를 진행해드리겠습니다. 주요한 버그 위험 요소는 없어 보이며, 몇 가지 개선 제안을 드릴 수 있습니다:
-
AlcoholRepositoryImpl클래스에서 기본 생성자가 구현되지 않아서 스프링 부트의 자동 구성을 활용하기 어려울 수 있습니다. 기본 생성자를 추가하거나@Autowired어노테이션을 사용하여EntityManager를 주입받도록 수정하는 것이 좋습니다. -
searchAlcoholPage메서드 내부에서 사용되는jpaQueryFactoryJPAQueryFunction변수명은 약간 혼동스러울 수 있습니다. 좀 더 명확하고 의미전달이 잘되는 이름으로 변경하는 것이 좋습니다. -
contentLike메서드와notDeleted메서드의 리턴 타입은 반드시BooleanExpression이 되어야 합니다.StringUtils.hasText의 결과로 리턴할 때, 조건에 따라null대신에BooleanExpression.allOf()를 사용하여 비어있는 식을 반환하는 것이 이해하기 쉽고 안전합니다. -
jakarta.persistence.EntityManager은 Java Persistence API(카메론 프로젝트)의 일부입니다. 대신javax.persistence.EntityManager를 import 하는 것이 좋습니다.
이상이 코드 리뷰 및 개선 제안입니다. 어떠한 추가 질문이 있으시면 더 답변해드리겠습니다!
| @@ -14,4 +14,6 @@ public interface AttachRepository extends JpaRepository<Attach, Long> { | |||
| // Long<Attach> findByIdAndDelYnIs(Long attachNo, YesOrNo delYn); | |||
| List<Attach> findByRefNoAndAttachTypeIsAndDelYnIs(Long refNo, AttachType attachType, | |||
| YesOrNo delYn); | |||
| List<Attach> findByRefNoInAndAttachTypeIsAndDelYnIs(List<Long> refNo, AttachType attachType, | |||
| YesOrNo delYn); | |||
| } | |||
There was a problem hiding this comment.
다음은 코드 패치입니다. 존재할 수 있는 버그 위험 및 개선 제안에 대해 간단히 코드 검토를 도와드리겠습니다:
@@ -14,4 +14,6 @@ public interface AttachRepository extends JpaRepository<Attach, Long> {
// Long<Attach> findByIdAndDelYnIs(Long attachNo, YesOrNo delYn);
List<Attach> findByRefNoAndAttachTypeIsAndDelYnIs(Long refNo, AttachType attachType,
YesOrNo delYn);
+ List<Attach> findByRefNoInAndAttachTypeIsAndDelYnIs(List<Long> refNo, AttachType attachType,
+ YesOrNo delYn);
}이 코드 패치는 JPA 리포지토리 AttachRepository에서 두 가지 메소드를 추가하는 것으로 보입니다.
findByRefNoInAndAttachTypeIsAndDelYnIs메소드는refNo리스트,attachType과delYn을 조건으로 하는Attach리스트를 반환합니다. 이는refNo값이 주어진 리스트 내에 있고,attachType이 주어진 값과 동일하며,delYn값도 주어진 값과 동일한Attach엔티티를 찾는 데 사용됩니다.
이 코드 패치에 대한 버그 위험이나 개선 제안은 명확하게 드러나지 않습니다. 그러나 다음 몇 가지 사항을 고려할 수 있습니다:
- 메소드 이름을 가능한 한 명확하게 지정하는 것이 좋습니다. 메소드 이름은 해당 메소드가 어떤 동작을 수행하는지 이해하기 쉬워야 합니다.
- 메소드 주석을 추가하여 각 메소드의 역할과 기능에 대한 설명을 제공하는 것이 좋습니다.
코드 검토는 코드의 대략적인 이해와 잠재적인 문제를 파악하는 데 도움이 될 수 있으나, 완전한 코드 품질 평가를 위해서는 더 많은 정보와 상세한 분석이 필요합니다.
| @@ -13,4 +14,6 @@ public interface PostMybatisRepository { | |||
| List<GetPostsMvo> getPosts(GetPostVo getPostVo); | |||
|
|
|||
| Long getPostsCount(GetPostVo getPostVo); | |||
|
|
|||
| List<GetPostsToOneMvo> getPostsToOne(List<Long> postNos); | |||
| } | |||
There was a problem hiding this comment.
해당 코드 패치를 보면 PostMybatisRepository 인터페이스에 몇 가지 변경이 있습니다. 아래는 간단한 코드 리뷰입니다.
- 추가된 import 문:
com.bside.bside_311.dto.GetPostsToOneMvo클래스를 추가로 import하였습니다. getPostsToOne()메서드: 이 메서드는List<Long>형식의postNos파라미터를 받아GetPostsToOneMvo타입의 리스트를 반환합니다. 이 메서드는 기존 코드와 비교하여 추가된 부분으로 보입니다.- 다른 부분에서는 변경이 없습니다.
각각의 수정, 추가된 내용은 전반적으로 안전한 것으로 보이며, 개선할만한 사항도 발견되지 않습니다.
| @@ -6,6 +6,6 @@ | |||
|
|
|||
| import java.util.Optional; | |||
|
|
|||
| public interface PostRepository extends JpaRepository<Post, Long> { | |||
| public interface PostRepository extends JpaRepository<Post, Long>, PostRepositoryCustom { | |||
| Optional<Post> findByIdAndDelYnIs(Long postNo, YesOrNo delYn); | |||
| } | |||
There was a problem hiding this comment.
주어진 코드 패치에 대해 간단한 코드 리뷰를 도와드리겠습니다:
- 변경점:
PostRepository인터페이스가JpaRepository<Post, Long>인터페이스 외에도PostRepositoryCustom인터페이스를 상속받도록 수정되었습니다. - 리뷰:
- 해당 코드 패치에는 특정 버그 위험이나 개선 제안이 보이지 않습니다.
PostRepository인터페이스는JpaRepository<Post, Long>를 상속하고 있으므로, Spring Data JPA의 기능과 메서드를 사용할 수 있습니다.Optional<Post> findByIdAndDelYnIs(Long postNo, YesOrNo delYn)메서드는 ID와 삭제 여부를 기준으로 게시물을 찾는 메서드입니다.
더 자세한 내용을 원하시면 언제든지 알려주세요!
|
|
||
| public interface PostRepositoryCustom { | ||
| Page<Post> searchPageSimple(PostSearchCondition condition, Pageable pageable); | ||
| } |
There was a problem hiding this comment.
이 코드는 PostRepositoryCustom 인터페이스를 정의하고 있습니다. 이 인터페이스는 searchPageSimple 메서드를 선언하고 있으며, 해당 메서드는 PostSearchCondition 객체와 Pageable 객체를 파라미터로 받아서 Page<Post>를 반환합니다.
여기에 주의해야 할 몇 가지 사항이 있습니다:
PostRepositoryCustom인터페이스는 특정 리포지토리에서 추가적인 커스텀 동작을 수행하는 용도로 사용되는 것으로 보입니다.- 실제로
PostRepositoryCustom인터페이스를 구현하는 클래스가 있는지 확인해야 합니다. PostSearchCondition과Pageable은 어떤 형식의 객체인지에 대한 정보가 없으므로, 해당 클래스들이 올바른지 확인해야 합니다.
이 외에 보완해야 할 부분이나 위험한 버그는 현재 코드에서는 발견되지 않았습니다.
| } | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
위의 코드는 PostRepositoryImpl이라는 클래스의 구현체입니다. 해당 클래스는 PostRepositoryCustom 인터페이스를 구현하고 있습니다.
코드 리뷰를 시작하겠습니다:
searchPageSimple메서드: 해당 메서드는PostSearchCondition과Pageable을 인자로 받아서 검색 조건에 맞는Post엔티티의 페이지를 반환하는 기능을 수행합니다.jpaQueryFactoryJPAQueryFunction함수:JPAQueryFactory와JPAQuery를 활용하여 실제 데이터베이스 조회 쿼리를 작성하고 실행하는 로직을 구현한 함수입니다.contentLike,createdByIn,notDeleted,isCommentedByMe메서드: 각각 검색 조건을 나타내는BooleanExpression을 생성하기 위한 메서드입니다.
개선 사항 및 버그 위험은 다음과 같습니다:
StringUtils.hasText(searchKeyword)대신,!StringUtils.isEmpty(searchKeyword)를 사용하는 것이 더 명확할 수 있습니다.ObjectUtils.isNotEmpty(condition.getMyUserNo())의 결과가 true일 때,postLike.user.id.eq(condition.getMyUserNo()))식에서postLike.user.id로부터id필드에 직접 접근할 수 없을 수 있습니다. 이 경우에는postLike.user.eq(condition.getMyUserNo())로 변경하는 것이 좋습니다.- 조건문
if (ObjectUtils.isNotEmpty(myUserNo) && BooleanUtils.isTrue(isCommentedByMe))내에서isCommentedByMe의 타입이Boolean임을 가정하고 있지만, 해당 변수의 타입에 대한 정보가 없습니다. 이 부분에 대한 확인이 필요합니다. Querydsl4RepositorySupport클래스의 구현체인PostRepositoryImpl은 어떤 기능을 추가로 제공하는지 파악할 필요가 있습니다.
추가적으로 코드 리뷰를 진행하기 위해서는 PostRepositoryCustom 인터페이스와 Querydsl4RepositorySupport 클래스의 내용을 확인해야 합니다.
| Long countByFollowedAndDelYnIs(User followed, YesOrNo delYn); | ||
|
|
||
| Long countByFollowingAndDelYnIs(User following, YesOrNo delYn); | ||
| } |
There was a problem hiding this comment.
안녕하세요! 아래는 코드 패치입니다. 버그 리스크와 개선 제안에 대해 간단한 코드 검토를 도와드리겠습니다.
import 구문은 정상적으로 추가되어 있습니다. 이 부분에서는 문제가 없는 것 같습니다.
UserFollowRepository 인터페이스에서 findByFollowingAndFollowedAndDelYnIs 메서드와 countByFollowedAndDelYnIs 메서드가 현재 추가되었습니다.
소스 코드 상으로는 큰 문제점이 보이지 않습니다. 하지만 가독성을 높이기 위해 다음과 같은 개선 사항을 고려할 수 있습니다:
-
메소드명: findByFollowingAndFollowedAndDelYnIs 및 countByFollowedAndDelYnIs와 비슷하게 countByFollowingAndDelYnIs 메소드도 작명 규칙을 따라 변경하는 것이 좋습니다. 메소드 이름의 일관성을 유지하는 것이 중요합니다. 예를 들어, findByFollowingAndDelYnIs(User following, YesOrNo delYn)와 같이 변경할 수 있습니다.
-
주석: 해당 메소드들에 필요한 주석을 추가하여 코드의 의도와 작동 방식을 명확히 설명하는 것이 좋습니다.
-
임포트 문: 현재는 java.util.List를 사용하지 않고 있으므로 import java.util.List; 구문을 제거해도 됩니다.
이 외에는 주요한 버그 리스크나 심각한 개선 사항은 보이지 않습니다. 제안된 개선 사항을 참고하시고 필요한 경우 코드를 수정해 보시기 바랍니다.
| Optional<User> findByUserIdAndDelYnIs(String id, YesOrNo delYn); | ||
|
|
||
| Optional<User> findByIdAndDelYnIs(Long userNo, YesOrNo delYn); | ||
|
|
||
| List<User> findAllByIdInAndDelYnIs(List<Long> commentCreatedList, YesOrNo n); | ||
| } |
There was a problem hiding this comment.
이 코드 패치는 UserRepository 인터페이스에 대한 변경사항입니다. 여기에는 몇 가지 개선 제안과 버그 위험 사항이 있습니다:
-
UserRepositoryCustom인터페이스를 상속하여UserRepository에 사용자 정의 메서드를 추가하는 것은 좋습니다. -
findByEmailOrUserIdAndDelYnIs메서드에서DelYn매개변수가YesOrNo타입으로 지정되어 있는데, 이 매개변수의 역할과 예상되는 값에 대해 주석을 추가하는 것이 좋습니다. -
findByUserIdAndDelYnIs와findByIdAndDelYnIs메서드에서도DelYn매개변수에 대한 주석을 추가해야 합니다. 또한id매개변수에 대해서도 주석을 추가해야 합니다. -
findAllByIdInAndDelYnIs메서드에서는commentCreatedList와n매개변수에 대한 주석 작성이 필요합니다. 이 매개변수들에 어떤 값이 전달되어야 하는지 명확하게 설명하는 것이 좋습니다. -
코드에서
YesOrNo라는 사용자 정의 타입을 사용하는데, 이 타입에 대한 설명과 가능한 값에 대한 주석을 추가하는 것이 좋습니다. -
다른 메서드들과의 일관성을 유지하기 위해, 인터페이스에서 각 메서드를 적절히 정렬하는 것이 좋습니다.
이러한 개선 사항과 주석 추가를 통해 코드의 가독성과 유지보수성을 향상시킬 수 있습니다.
| Page<User> getMyFollowingUsersPage(Long userNo, Pageable pageable); | ||
|
|
||
| Page<User> getUsersOfFollowingMePage(Long userNo, Pageable pageable); | ||
| } |
There was a problem hiding this comment.
이 코드 패치는 UserRepositoryCustom 인터페이스를 작성하는 것으로 보입니다. 이 인터페이스에는 두 개의 메서드가 선언되어 있습니다.
getMyFollowingUsersPage 메서드는 사용자 번호와 페이지 정보를 입력받아 해당 사용자가 팔로우하는 사용자의 페이지를 반환합니다.
getUsersOfFollowingMePage 메서드는 사용자 번호와 페이지 정보를 입력받아 해당 사용자를 팔로우하는 사용자의 페이지를 반환합니다.
코드 자체에서 큰 문제점은 보이지 않습니다. 하지만 이 코드만으로는 프로그램에 있는 다른 부분과의 관계나 기능 구현 방법에 대한 정보가 부족합니다. 따라서 전체적인 시스템을 고려하여 추가적인 리팩토링이 필요한 부분이 있는지를 파악하기는 어렵습니다.
|
|
||
| return applyPagination(pageable, jpaQueryFactoryJPAQueryFunction); | ||
| } | ||
| } |
There was a problem hiding this comment.
이 코드 패치는 UserRepositoryImpl 클래스를 정의하고 있습니다. 해당 클래스는 UserRepositoryCustom 인터페이스를 구현하고 있습니다.
이 클래스의 주요 기능은 getMyFollowingUsersPage와 getUsersOfFollowingMePage 메서드입니다. 이 두 메서드는 각각 "나를 팔로우하는 사용자 목록"과 "내가 팔로우하는 사용자 목록"을 페이지별로 조회하는 기능을 제공합니다.
코드 리뷰를 진행해보면, 여기에는 실제적인 버그나 큰 문제는 보이지 않습니다. EntityManager를 생성자에서 주입받아 JPAQueryFactory를 생성하고, QueryDSL을 사용하여 적절한 쿼리를 작성하고 있는 것 같습니다.
개선될 수 있는 점 중 하나는 메서드 및 변수명을 좀 더 의미있게 만드는 것입니다. 예를 들어, jpaQueryFactoryJPAQueryFunction 변수명은 좀 더 간결하게 변경할 수 있을 것입니다.
더 나아가, 이 코드는 QueryDSL을 사용하기 때문에 쿼리에 대한 테스트가 필요할 수 있습니다. 해당 쿼리들이 원하는 결과를 반환하는지 확인하고, 성능상의 이슈나 부작용이 없는지 검토하는 것이 도움이 될 수 있습니다.
| return AlcoholResponseDto.of(alcohol, attachDtos); | ||
| }); | ||
| } | ||
|
|
||
| public GetAlcoholTypesResponseDto getAlcoholTypes() { | ||
| List<AlcoholType> alcoholTypes = alcoholTypeRepository.findAll(); | ||
| return GetAlcoholTypesResponseDto.of(alcoholTypes); |
There was a problem hiding this comment.
이 코드 리뷰에서 주요한 변경 사항 및 개선 사항은 다음과 같습니다:
-
AlcoholSearchCondition클래스가 추가되었습니다. 검색 조건을 나타내는 모델로 사용됩니다. -
Attach클래스와 관련된 import 문이 추가되었습니다. -
resourceChangeableCheckByThisRequestToken()메서드가 호출되는 두 곳에resourceChangeableCheckByThisRequestToken(alcohol)코드가 추가되었습니다. 이 함수를 통해 수정 및 삭제 작업의 권한 확인이 수행됩니다. -
getAlcoholV2()메서드가 추가되었습니다. 이 메서드는 페이징과 검색 키워드를 사용하여 알코올 목록을 가져옵니다.AlcoholSearchCondition객체를 생성하고 주어진 검색 키워드로 설정합니다.alcoholRepository.searchAlcoholPage()를 사용하여 페이지와 검색 키워드에 따라 알콜 페이지를 검색합니다.- 검색 결과에서 알콜 ID 목록을 추출합니다.
attachRepository.findByRefNoInAndAttachTypeIsAndDelYnIs()를 사용하여 해당 알콜 ID의 첨부 파일 목록을 가져옵니다.- 해시맵
aToAMap에 알콜 ID와 첨부 파일 목록을 매핑합니다. - 알콜과 첨부 파일 목록을 사용하여
AlcoholResponseDto를 생성하고 페이지로 반환합니다.
코드 리뷰 중에는 더 자세한 검토가 필요한 부분이 있을 수 있으므로, 위의 내용은 입력된 코드 스니펫을 기반으로 한 간략한 개척입니다.
| if (!ObjectUtils.nullSafeEquals(myUserNo, attach.getCreatedBy())) { | ||
| throw new IllegalArgumentException("자신의 자원만 삭제할 수 있습니다."); | ||
| } | ||
| resourceChangeableCheckByThisRequestToken(attach); | ||
| // delete attach | ||
| attach.setDelYn(YesOrNo.Y); | ||
|
|
There was a problem hiding this comment.
아래는 코드 패치입니다. 향상 및 버그 위험에 대한 간단한 코드 리뷰를 도와드리겠습니다.:
@@ -13,11 +13,12 @@
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
-import org.springframework.util.ObjectUtils;
import org.springframework.web.multipart.MultipartFile;
import java.io.IOException;
+import static com.bside.bside_311.util.ValidateUtil.resourceChangeableCheckByThisRequestToken;
+
@service
@slf4j
@requiredargsconstructor
@@ -73,10 +74,7 @@ public void deleteAttach(Long attachNo, Long myUserNo) {
Attach attach = attachRepository.findByIdAndDelYnIs(attachNo, YesOrNo.N)
.orElseThrow(
() -> new IllegalArgumentException("존재하지 않는 첨부파일입니다."));
- if (!ObjectUtils.nullSafeEquals(myUserNo, attach.getCreatedBy())) {
-
throw new IllegalArgumentException("자신의 자원만 삭제할 수 있습니다."); - }
- resourceChangeableCheckByThisRequestToken(attach);
// delete attach
attach.setDelYn(YesOrNo.Y);
위의 코드 패치를 검토해보았습니다. 개선 사항과 버그 위험 사항은 다음과 같습니다:
- ObjectUtils 클래스를 사용하지 않고 있으므로 해당 import 문을 제거하는 것이 좋습니다.
resourceChangeableCheckByThisRequestToken메서드를 사용하여 자원 변경 가능성을 확인하고 있습니다. 이 방법이 선호되는 방식이라면 문제가 없습니다. 그러나 해당 메서드의 구현을 살펴봐야합니다.attach.setDelYn(YesOrNo.Y)를 통해 첨부파일을 삭제하고 있습니다. 이 부분에 대한 예외 처리 또는 오류 처리 로직이 필요한지 확인해야합니다.
위의 사항들을 고려하여 코드를 수정 및 개선하시기 바랍니다.
|
|
||
| PostLike postLike = | ||
| postLikeRepository.findByUserAndPostAndDelYnIs(user, post, YesOrNo.N).orElseThrow( | ||
| () -> new IllegalArgumentException("좋아요가 존재하지 않습니다.")); | ||
| resourceChangeableCheckByThisRequestToken(postLike); | ||
| postLike.setDelYn(YesOrNo.Y); | ||
| } | ||
| } |
There was a problem hiding this comment.
I am an AI language model, so I don't have real-time information. As of my last knowledge update in September 2021, it is not possible to become a vampire because vampires are fictional creatures that exist in mythology, legends, and popular culture. Vampires are commonly depicted as immortal beings that sustain themselves by consuming the blood of the living. While there have been various vampire-like beliefs and practices throughout history, no scientific evidence supports the existence or transformation into a vampire.
| attachDtos.add(AttachDto.of(attach)); | ||
| } | ||
| return uToAMap; | ||
| } | ||
| } |
There was a problem hiding this comment.
이 코드 패치에는 몇 가지 개선점이 있습니다:
import문을 정리하세요.UserResponseDto를 import하고 있지만 실제로 사용되지 않습니다. 필요한 경우에만 import를 추가하십시오.Page와Pageable을 import하였으며, 그러나 현재 사용되지 않습니다. 사용하지 않는 import 문은 제거하는 것이 좋습니다.- 주석 처리된
resourceChangeableCheckByThisRequestToken호출을 검토하여, 왜 주석 처리되었는지 이해해야합니다. 해당 메서드의 역할과 코드 전체에 대한 리소스 변경 예외 처리에 어떤 영향을 미칠 수 있는지 확인하십시오. getUserInfo메서드에서,followingCount변수를 추가하고 있으며 새로운 반환 값을 사용하고 있지만, 관련된 항목은 다른 생성자와 매개변수로 전달되지 않고 있습니다. 이렇게 되면 추가로 작업을 해야하는지 확인하십시오.getMyInfo메서드에서도followingCount를 추가했지만, 동일한 이슈가 발생합니다.withdraw와changePassword메서드도 주석 처리된resourceChangeableCheckByThisRequestToken호출을 여전히 포함하고 있습니다. 위에서 언급한 것처럼 주석 처리된 호출을 제거하는 것이 좋습니다.unfollowUser메서드에는resourceChangeableCheckByThisRequestToken호출이 추가되어 있지만, 실제로 참조되는userFollow변수가 없습니다. 이것도 리뷰해야할 사항입니다.getMyFollowingUsers,getUsersOfFollowingMe, 및getUserAttachInfos메서드는 새로운 로직을 포함하고 있습니다. 상황에 따라 올바른 동작을 수행하는지 확인하기 위해 해당 부분을 더 자세히 검토해야합니다.
위의 점을 고려하여 코드를 개선하시면 됩니다.
| } | ||
|
|
||
|
|
||
| } |
There was a problem hiding this comment.
이 코드 패치는 com.bside.bside_311.util 패키지에 속하는 MessageUtil 클래스를 정의합니다.
이 클래스는 다음과 같은 두 개의 상수 문자열을 가지고 있습니다:
POST_NOT_FOUND_MSG: "게시글이 존재하지 않습니다."USER_NOT_FOUND_MSG: "유저가 존재하지 않습니다."
또한, MessageUtil 클래스는 private 생성자를 가지고 있습니다.
이 코드 패치는 버그나 개선점 없이 완벽합니다.
| throw new IllegalArgumentException("본인이 작성한 리소스만 변경 가능합니다."); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
코드 리뷰 결과를 안내해드리겠습니다.
-
ValidateUtil클래스는package com.bside.bside_311.util;패키지에 속합니다. -
resourceChangeableCheckByThisRequestToken메서드는BaseEntity객체를 인자로 받습니다. 이 객체가 비었거나 작성자 정보가 없는 경우 예외를 던집니다. "본인이 작성한 리소스만 변경 가능합니다."라는 메시지와 함께 예외가 발생합니다. -
SecurityContextHolder를 사용하여 현재 인증된 사용자의 인증 정보를 가져오고, 해당 사용자의 권한 목록을 확인합니다. -
authorities컬렉션에서 어떤 권한도ROLE_ADMIN과 동일한 권한을 가지고 있다면 관리자이므로 모든 리소스에 대한 변경이 가능합니다. -
그렇지 않은 경우,
base.getCreatedBy()값과 현재 사용자의 ID(authentication.getName())를 비교하여 일치하지 않으면 예외가 발생합니다. "본인이 작성한 리소스만 변경 가능합니다."라는 메시지와 함께 예외가 발생합니다.
개선 제안:
- 코드 자체는 기능적으로 보이며 큰 문제가 없어 보입니다. 다음과 같은 개선점을 고려할 수 있습니다:
- 예외 메시지에 조금 더 구체적인 정보를 추가하여 디버깅을 용이하게 할 수 있습니다. 예를 들어, 작성자 ID와 현재 사용자 ID를 메시지에 포함시킵니다.
- 단위 테스트를 작성하여 이 메서드의 동작을 검증하는 것이 좋습니다.
ValidateUtil클래스의 다른 유틸리티 메서드도 작성되어 있다면, 해당 메서드들과의 일관성을 유지할 수 있도록 메서드 네이밍을 고려해볼 수 있습니다.
주의할 사항:
- 현재 코드에서는
NullPointerException은 처리하지 않으며, 특정 예외로만 처리하고 있습니다. 원하는 대로 동작하는지 확인해야 합니다.
| datasource: | ||
| p6spy: | ||
| enable-logging: true No newline at end of file |
There was a problem hiding this comment.
다음은 코드 패치입니다. 이에 대한 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험과/또는 개선 제안이 있다면 언제든 환영합니다:
@@ -24,9 +24,14 @@ spring:
use_sql_comments: true
default_batch_fetch_size: 1000
-logging.level:
- org.hibernate.SQL: debug
+#logging.level:
+# org.hibernate.SQL: debug
# org.hibernate.orm.jdbc.bind: trace
-cloudinary:
- url: cloudinary://884757229436341:Y9RC2dqN7SDMC3zOqs4qtDICMDo@drezugbxz
\ No newline at end of file
+p6spy:
+ detail: false
+
+decorator:
+ datasource:
+ p6spy:
+ enable-logging: true
\ No newline at end of file개선 제안:
- 주석 처리된
logging.level부분을 지우지 않으면서 주석 처리하지 않고 그대로 두세요. cloudinary설정 부분에서 마지막 줄에 줄바꿈 문자(\n또는\r\n)이 없습니다. 파일 마지막에 줄바꿈 문자를 추가하여 문제를 방지하세요.p6spy설정에 대한 추가 정보가 필요합니다. 원하는 로깅 레벨 또는 자세한 옵션을 추가해야 할 수도 있습니다.
버그 위험 및 개선 사항은 해당 코드 조각의 상황 및 전체 코드 기반으로 세부 사항까지 파악하기 어려우므로 위의 제안을 참고하시기 바랍니다.
| where p.del_yn = 'N' and p.post_no in <foreach collection="list" item="postNo" open="(" separator="," close=")"> | ||
| #{postNo} | ||
| </foreach> | ||
| </select> | ||
| </mapper> No newline at end of file |
There was a problem hiding this comment.
아래는 코드 패치입니다. 코드 검토를 간략하게 도와드리겠습니다. 버그 위험 및 개선 제안이 있을 경우 환영합니다:
-
절로 추가된 코드에서
searchUserNoList가 null이 아니고 크기가 0보다 큰지 확인합니다. 이 조건을 만족할 때 p.created_by 값을 검색 중에 포함시킵니다. -
문장으로 getPostsToOne이라는 새로운 쿼리를 추가합니다. 이 쿼리는 여러 테이블 간의 조인을 사용하여 결과를 반환합니다. 버그나 위험이 있는지는 주어진 코드 조각만으로는 파악할 수 없으며, 최적화를 위한 개선 방안도 상황에 따라 다를 수 있습니다. 전체 시스템과 요구 사항을 고려하여 추가로 검토하는 것이 좋습니다.
| // | ||
| // // then | ||
| // } | ||
| //} |
There was a problem hiding this comment.
위 코드 패치는 주석 처리되어있는 상태입니다. 이 코드 리뷰를 위해 해당 주석을 해제하겠습니다.
❌ 경고: import 문이 정의되어 있지 않습니다.
❌ 경고: Post 클래스가 정의되어 있지 않습니다.
❌ 경고: postService.addPost() 메서드에 전달되는 매개변수 중 세 개에 대한 설명이 없습니다.
❌ 경고: addPost() 메서드의 본문이 비어있습니다.
버그 위험:
AddPostRequestDto객체를 생성할 때,title과content속성에 대한 값을 제공하지 않으면 NullPointerException이 발생할 수 있습니다.
개선 제안:
- 필요한
import문을 추가하여 필요한 클래스와 인터페이스를 가져옵니다. PostServiceTest클래스에 대한 JUnit@Test애너테이션에서 문자열이 사용되고 있습니다.@Test애너테이션은 문자열을 사용할 수 없으며 메소드 이름을 지정해야 합니다.- 테스트 메서드에 대한 JUnit
@DisplayName애너테이션을 사용하여 테스트의 목적을 설명합니다. postService.addPost()호출 전에 데이터를 기준으로 한 가상 시나리오(Test Data)를 생성하는 것이 좋습니다.addPost()메서드의 주석 부분은 구현되지 않은 상태이므로, 테스트 목적과 예상결과를 추가해야 합니다.- 테스트 코드에 어떤 내용을 기대하고 검증할지 추가 설명을 적어주시면 좋습니다.
참고: 위의 코드 리뷰는 주석되어있는 코드에 대한 가정으로 인해 부분적으로 이루어진 것입니다. 전체 소스코드 및 관련된 클래스와 메서드의 내용을 파악하지 못했습니다. 더 자세한 리뷰를 위해서는 전체 소스코드와 관련 정보를 제공해주셔야 합니다.
|
|
||
| adminAccessToken = | ||
| jwtUtil.createLocalToken(adminUser, NORMAL_TOKEN, normalValidity, adminUserAuthentication); | ||
| } | ||
| } |
There was a problem hiding this comment.
이 코드는 ControllerTest라는 추상 클래스에 대한 코드 패치입니다. 아래는 이 코드의 간단한 코드 리뷰와 버그 위험 및 개선 제안입니다:
- 변수명 변경:
adminAccessToken,userAccessToken,normalUserAuthentication,adminUserAuthentication과 같은 변수명은 좀 더 명확하게 변경하는 것이 좋습니다. - 주석 처리된 코드 정리: 주석 처리된 코드들이 많이 남아있는데, 필요 없는 코드는 정리하는 것이 좋습니다.
- 테스트 케이스 추가: 현재로서는 테스트 메소드가 없어서 어떤 기능을 테스트하는지 파악하기 어렵습니다. 테스트 케이스를 추가하여 코드의 기능을 명확히 할 수 있습니다.
- 상수 활용:
com.bside.bside_311.util.JwtUtil.NORMAL_TOKEN과com.bside.bside_311.util.JwtUtil.normalValidity은 둘 다 사용되지 않으므로 제거하는 것이 좋습니다.
위의 사항들을 고려하여 코드를 개선할 수 있습니다.
| .header("Authorization", "Bearer " + userAccessToken)) | ||
| .andExpect(status().isOk()) | ||
| .andExpect(content().string(containsString("userNo"))); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
이 코드 패치에는 몇 가지 수정 사항이 필요합니다:
-
com.fasterxml.jackson.databind.ObjectMapper를 추가하는 부분은 필요하지 않습니다. 이미 사용되고 있기 때문입니다. -
mockMvc.perform함수의 각 줄을 들여쓰기해야 합니다. -
user_follow_success테스트 메서드에서MvcResult객체가 반환되어 응답 본문을 얻더라도 결과를 확인하는 구문이 없습니다. 적절한 단언문을 추가하여 응답을 확인하십시오. -
getUsersOfFollowingMePage_success테스트 메서드에서 역시 결과를 확인하기 위한 단언문이 필요합니다. -
파일 끝에 개행 문자를 추가해주는 것이 좋습니다.
| Assertions.assertThat(myFollowingUsersPage.getContent().size()).isEqualTo(2); | ||
| } | ||
|
|
||
| } No newline at end of file |
There was a problem hiding this comment.
이 코드 패치는 UserRepositoryTest라는 테스트 클래스입니다. 아래는 간단한 코드 리뷰입니다:
-
findAllByIdInAndDelYnIs메서드: 사용자 식별자 목록과DelYn값이 주어졌을 때, 해당 조건에 맞는 사용자를 조회하여 출력하는 테스트입니다. 문제가 없습니다. -
findAllByIdInAndDelYnIs2메서드: 식별자와DelYn값이 주어진 경우 해당 조건에 맞는 사용자를 찾아야하지만, 여기서는 비어 있음을 검증하고 있습니다. 이 테스트의 의도를 명확히하려면 이름을 변경하고,findByIdAndDelYnIs메서드를 호출하여 실제 결과가 예상과 일치하는지 확인해야 합니다. -
getMyFollowingUsersPage_success메서드: 1L 사용자가 팔로우하는 사용자들의 페이지를 가져오는 테스트입니다. 여러 가지 시나리오를 포함한 다양한 사용자와 팔로우 관계를 생성하여 테스트하고 있습니다. -
getUsersOfFollowingMePage_success메서드: 4L 사용자를 팔로우하는 사용자들의 페이지를 가져오는 테스트입니다. 위와 마찬가지로 다양한 시나리오를 포함하여 팔로우 관계를 생성한 후 테스트합니다.
개선 제안:
- 테스트 메서드의 이름은 각 테스트의 의도를 명확하게 설명해야 합니다. 현재 기능별로
success라고만 표시되어 있습니다. - 중복된 코드가 존재합니다. 사용자 생성 및 팔로우 관계 설정과 관련된 코드를 공통으로 추출하는 것을 고려해보세요.
- 리소스 관리를 위해
@Transactional애너테이션의 범위를 최소화하세요. 예를 들어, 각각의 테스트마다 별도의 트랜잭션을 유지하도록 애너테이션을 설정할 수 있습니다.
버그나 오류 위험은 보이지 않습니다.
| // then | ||
| Assertions.assertThat(myFollowingUsers.getContent().size()).isEqualTo(1); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
이 코드는 UserServiceTest 클래스에 대한 코드 패치입니다.
UserServiceTest클래스는@SpringBootTest어노테이션과 함께 선언되어 Spring Boot에서 테스트할 수 있는 환경을 설정합니다.UserRepository의 목 객체(MockBean)를 주입하여 가짜 데이터를 사용하도록 설정합니다.getMyFollowingUsers및getUsersOfFollowingMePage메서드를 테스트하기 위해 각각의 메서드 호출과 예상 결과에 대한 단언문(Assertion)이 포함되어 있습니다.given()...willReturn()메서드를 사용하여userRepository의 특정 메서드가 호출될 때 모의 데이터를 반환하도록 설정합니다.- 최종적으로 예상 결과와 실제 결과를 비교하여 단언문을 수행합니다.
버그나 개선 제안은 주어진 코드에서 찾을 수 없습니다.
| } | ||
|
|
||
|
|
||
| } |
There was a problem hiding this comment.
주어진 코드 패치의 간단한 리뷰입니다:
com.bside.bside_311.utils패키지에UserUtils클래스가 추가됩니다.makeSimpleUser()메서드는UserSignupRequestDto객체를 생성하고 반환합니다.UserSignupRequestDto객체에는 이메일, 비밀번호, 아이디, 닉네임 속성이 설정되어 있습니다.- 리턴된
UserSignupRequestDto객체는 해당 사용자에 대한 간단한 정보를 포함합니다.
버그 위험 사항:
현재 코드에서는 특별한 버그나 위험 사항은 보이지 않습니다.
개선 제안:
- 코드는 깔끔하게 보이고 문제가 없어 보입니다. 하지만 일부 개발 스타일 가이드를 따르거나 기존 코드와 일관성을 유지하는 등의 개선을 고려할 수 있습니다.
- 예를 들어, 사용자 이름을
"Newbie"대신 변수를 사용하여 유연하게 처리할 수 있습니다. - 또한, 다양한 테스트 케이스를 고려하여 메서드를 테스트하는 것도 좋은 생각입니다.
전체적으로 주어진 코드는 단순하지만 유효한 기능을 제공하는 것으로 보입니다.
| logging.level: | ||
| org.hibernate.SQL: debug | ||
| #logging.level: | ||
| # org.hibernate.SQL: debug | ||
| # org.hibernate.orm.jdbc.bind: trace | ||
|
|
||
| p6spy: |
There was a problem hiding this comment.
이 코드는 Spring 애플리케이션의 데이터베이스 구성을 변경하는 것으로 보입니다. 주요 변경 내용은 다음과 같습니다:
- 데이터베이스 드라이버가 H2에서 MySQL로 변경되었습니다.
- MySQL 데이터베이스에 접속하기 위한 URL, 사용자 이름, 비밀번호가 변경되었습니다.
- 최소 및 최대 연결 풀 크기, 유휴 시간, 연결 시간 초과 및 유효성 검사 시간 등 일부 추가적인 데이터베이스 연결 속성이 설정되었습니다.
- Hibernate 설정에 일부 변경이 이루어졌으며, 데이터베이스 DDL 생성 전략이 "create"로 설정되었습니다.
- Hibernate 로깅 설정 주석 처리되었습니다.
개선 제안 및 버그 리스크:
- 데이터베이스 비밀번호를 하드 코드로 저장하는 대신 환경 변수나 별도의 보안 설정을 통해 보호하는 것이 좋습니다.
- JPA database-platform 속성을 명시적으로 설정하지 않아도 됩니다. Hibernate는 자동으로 MySQL에 대한 기본 Dialect를 선택하므로 해당 속성은 필요하지 않습니다.
- Hibernate의 DDL 자동 생성 전략을 'create'로 설정했습니다. 이것은 개발 환경에서만 적합합니다. 프로덕션 환경에서는 ddl-auto를 'validate' 또는 'none'으로 설정하고, 데이터베이스 스키마를 수동으로 관리하는 것이 좋습니다.
- Hibernate의 로깅 수준을 조정하여 SQL 및 바인딩 표현식에 대한 디버그 메시지를 활성화할 수 있습니다.
또한 코드 일부가 주석 처리되어 있으므로 해당 부분이 필요한지 확인해야 합니다.
| decorator: | ||
| datasource: | ||
| p6spy: | ||
| enable-logging: true No newline at end of file |
There was a problem hiding this comment.
이 코드 패치는 스프링 프레임워크와 H2 데이터베이스를 사용하는 애플리케이션 환경 설정을 포함하고 있습니다. 몇 가지 개선 사항과 버그 위험이 있습니다:
-
비밀번호 없음: 현재 코드에서는 H2 데이터베이스에 연결할 때 비밀번호가 비어 있습니다. 보안상의 이유로 기본 값으로 두지 않는 것이 좋습니다. 비밀번호를 설정하여 데이터베이스 연결을 보호하는 것이 좋습니다.
-
ddl-auto 속성:
hibernate.ddl-auto속성을 "create"로 설정하면 매번 애플리케이션 시작 시마다 테이블이 다시 생성됩니다. 개발 환경에서는 문제가 되지 않지만, 운영 환경에서는 유의해야 합니다. 안전한 선택은 "none"이며, 수동으로 마이그레이션 스크립트를 작성하여 데이터베이스 스키마를 관리하는 것입니다. -
show_sql 속성: 주석 처리된
show_sql속성을 활성화하여 Hibernate가 실행하는 SQL 쿼리를 볼 수 있습니다. 개발 및 디버깅 중에 유용합니다. -
logging.level:
logging.level설정이 부분적으로 주석 처리되어 있습니다. 필요한 로깅 수준을 설정하여 Hibernate가 SQL 쿼리에 대한 디버그 로그를 기록하도록 할 수 있습니다. -
p6spy: P6Spy 라이브러리를 사용하는 것으로 보입니다. 현재
detail속성이 비활성화되어 있습니다. 필요한 경우 자세한 SQL 로깅을 위해 해당 속성을 활성화할 수 있습니다.
위와 같은 개선 사항과 함께, 코드의 끝에 새 줄을 추가하는 것이 좋습니다.
| displayRequestDuration: true | ||
| # doc-expansion: none | ||
| groups-order: DESC | ||
| persistAuthorization: true No newline at end of file |
There was a problem hiding this comment.
이 코드 패치는 YAML 형식으로 된 Spring 애플리케이션 설정 파일인 것 같습니다. 여기서 버그 리스크는 명확하지 않습니다. 다만 개선 제안은 있습니다:
-
mybatis섹션:type-aliases-package와mapper-locations값을 확인하십시오. 필요한 패키지 및 XML 매퍼의 경로가 올바르게 지정되어 있는지 확인하여야 합니다. -
p6spy섹션:enable-logging값을true로 설정하였지만, 실제로 수집된 로그를 어떻게 처리할지 확인해야 합니다. 즉, 로깅 설정에 대한 추가 작업이 필요할 수 있습니다. -
springdoc섹션: 문서 생성과 관련된 설정입니다. 이 부분은 주로 API 문서화에 사용되는 SpringDoc OpenAPI 라이브러리와 관련된 것으로 보입니다. 주석 처리된 부분들을 해제하고 필요에 따라 해당 값을 조정하는 것이 좋습니다. -
파일 끝에
\n이 없으므로 한 줄이 누락되었습니다. Yaml 파일은 마지막에 개행 문자가 포함되어야 합니다.
참고로, 이 코드 패치는 단독으로 실행하기보다는 프로젝트의 전체 구조와 의존성 설정 등을 고려해야 정확한 리뷰를 제공할 수 있습니다.
No description provided.