Skip to content
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

Develop #75

Merged
merged 20 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions server/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -269,15 +269,14 @@ gradle-app.setting

src/main/generated/**

src/main/resources/application-prd.yml
src/main/resources/application.properties
application-prd.yml
application-swagger.yml
application.properties


## logs ##
logs/

### security information
application.yml

### operation log
replay_pid.*.log
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위 코드 패치의 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험 및 개선 제안은 환영합니다.

개선 제안:

  • gradle-app.setting 파일은 삭제되었습니다.
  • 경로가 src/main/generated/**인 모든 파일이 제외됩니다.
  • src/main/resources 디렉토리 내의 application-prd.ymlapplication.properties 파일이 경로 없이 나열됩니다.
  • application-swagger.yml 파일이 추가되었습니다.
  • logs/ 디렉토리 내의 모든 로그 파일도 제외됩니다.
  • application.yml 파일도 제거되었습니다.
  • replay_pid.*.log와 같은 형태의 파일도 제외됩니다.

이상입니다.

Expand Down
2 changes: 2 additions & 0 deletions server/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

* http://localhost:8080/swagger-ui/index.html

* http://localhost:8080/test

# 수동 배포 방법 정리.(개선 필요.)

```sh
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치에 대한 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험과/또는 개선 제안은 환영합니다:

  • 가독성을 높이려면 URL 주석 앞에 "@"를 추가하는 것이 좋습니다. 예: "@@ -2,6 +2,8 @@".
  • 새로운 URL인 "http://localhost:8080/test"가 추가되었다는 것 같습니다. 이 URL이 올바르게 작동하는지 확인해야 합니다.
  • "수동 배포 방법 정리"라는 주석이 있습니다. 이 설명이 충분한지, 필요한 개선사항이 있는지 검토해야 합니다. 어떤 개선 사항이 필요한지 구체적으로 언급해야 합니다.

주의 사항:
제가 한국어를 사용하여 응답하는 AI 모델이기 때문에 우리의 의사소통에 한계가 있을 수 있습니다. 번역이나 자세한 내용에 대해서는 정확한 해석을 위해 영어로 질문해주시면 감사하겠습니다.

Expand Down
7 changes: 4 additions & 3 deletions server/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ dependencies {
// https://mvnrepository.com/artifact/org.apache.commons/commons-collections4
implementation group: 'org.apache.commons', name: 'commons-collections4', version: '4.4'

implementation group: 'commons-fileupload', name: 'commons-fileupload', version: '1.4'
implementation group: 'commons-io', name: 'commons-io', version: '2.4'

// if (profile == "localh2") {
// runtimeOnly 'com.h2database:h2'
Expand All @@ -69,8 +71,6 @@ dependencies {
// }
runtimeOnly 'com.mysql:mysql-connector-j'

runtimeOnly 'com.mysql:mysql-connector-j'

annotationProcessor 'org.projectlombok:lombok'

// spring boot 3.0 query dsl setting.
Expand All @@ -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'
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰를 해드리겠습니다.

  1. 의존성 추가:

    • commons-fileupload와 commons-io 라이브러리의 버전 지정이 없는데, 해당 라이브러리 버전을 명시하는 것이 좋습니다.
    • 중복된 의존성(com.mysql:mysql-connector-j)이 있어서 한 번만 선언하는 것이 좋습니다.
  2. 주석 처리:

    • runtimeOnly 'com.mysql:mysql-connector-j' 부분이 주석 처리되어 있는데, 주석을 제거하고 필요한 의존성을 추가하는 것이 좋습니다.
  3. 테스트 관련 의존성:

    • 'org.springframework.boot:spring-boot-starter-test'가 주석 처리되어 있는데, 이 부분을 주석 해제하고 테스트에 필요한 의존성으로 추가하는 것이 좋습니다.

Expand Down
60 changes: 60 additions & 0 deletions server/scripts/deploy-swagger.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
#!/bin/bash

# swagger
REPOSITORY=/home/bside/311TEN003_for_swagger

# local
#REPOSITORY=/Users/dongseoklee/github/311TEN003

# common
PROJECT_LOCATION_FOLDER_NAME=server
PROJECT_NAME=bside_311

cd $REPOSITORY/$PROJECT_LOCATION_FOLDER_NAME/

echo "> Git reset --hard"
git reset --hard

echo "> Git Pull"

git pull

echo "Release Version Updated"
#grep "^Release" ./releasenote.txt | tail -1 > ./src/main/frontend/public/latestReleaseVer.txt


echo "> gradlew, deploy-swagger.sh 권한 변경 "
chmod 777 gradlew
chmod 774 scripts/deploy-swagger.sh

echo "> 프로젝트 Build 시작"
./gradlew build --exclude-task test

echo "> Build 파일 복사"

cp ./build/libs/*.jar $REPOSITORY/

echo "> 현재 구동중인 애플리케이션 pid 확인"

#CURRENT_PID=$(pgrep -f ${PROJECT_NAME}.*.jar)
CURRENT_PID=$(pgrep -f "active=swagger")

echo "$CURRENT_PID"

if [ -z "$CURRENT_PID" ]; then
echo "> 현재 구동중인 애플리케이션이 없으므로 종료하지 않습니다."
else
echo "> kill -2 $CURRENT_PID"
kill -9 "$CURRENT_PID"
sleep 10
fi

echo "> 새 어플리케이션 배포"

# JAR_NAME=$(ls $REPOSITORY/ |grep jar | tail -n 1)
JAR_NAME=$(ls $REPOSITORY/ |grep ${PROJECT_NAME}.*.jar | tail -n 1)

echo "> JAR Name: $JAR_NAME"

nohup java -jar $REPOSITORY/"$JAR_NAME" --spring.profiles.active=swagger &

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치는 다음과 같은 작업을 수행합니다:

  1. 지정된 저장소 경로에서 git reset --hard 명령어를 실행하여 이전 변경 내용을 모두 삭제합니다.
  2. git pull 명령어를 실행하여 최신 변경 내용을 가져옵니다.
  3. gradlew build 명령어를 사용하여 프로젝트를 빌드합니다.
  4. 빌드된 파일을 지정된 저장소로 복사합니다.
  5. 현재 구동 중인 애플리케이션의 PID(Process ID)를 확인합니다.
  6. 현재 구동 중인 애플리케이션이 없으면 아무 작업도 수행하지 않고, 있으면 해당 애플리케이션을 종료합니다.
  7. 새로운 애플리케이션을 배포하고 실행합니다.

이 코드에서 주의해야 할 몇 가지 사항이 있습니다:

  1. 현재 코드에서는 JAR 파일의 이름에 "bside_311" 문자열을 포함하는 파일을 배포하고 실행하고 있습니다. 이 부분을 프로젝트의 요구 사항에 맞게 수정해야 합니다.
  2. kill -9 명령어를 사용하여 애플리케이션을 강제로 종료하고 있습니다. 이는 예상치 못한 문제가 발생할 수 있으므로 조심해야 합니다. 가능한 경우, 더 안전한 방법을 사용하는 것이 좋습니다.
  3. 현재 코드에서는 nohup 명령어를 사용하여 백그라운드에서 애플리케이션을 실행하고 있습니다. 이 경우 애플리케이션의 로그 출력이나 오류 메시지를 확인하기 어렵습니다. 필요에 따라 로그 기능을 추가하거나 실행 방식을 변경할 수 있습니다.

개선 제안:

  1. 실행가능한 스크립트 파일(.sh)을 작성하여 각 단계를 별도의 함수로 모듈화하고, 잠재적인 오류 상황을 처리하도록 구성할 수 있습니다.
  2. 애플리케이션의 실행과 관련된 환경 변수를 구성 파일로 분리하여 유연성을 높일 수 있습니다.
  3. 롤백 기능을 추가하여 배포 과정 중 예상치 못한 문제가 발생했을 때 이전 버전으로 되돌릴 수 있는 기능을 제공할 수 있습니다.
  4. 작업을 자동화하여 CI/CD 파이프라인에 통합할 수 있습니다.

이러한 개선 사항은 코드의 가독성, 유지 보수성 및 안정성을 개선하는 데 도움이 될 수 있습니다.

3 changes: 2 additions & 1 deletion server/scripts/deploy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제가 주어진 코드의 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험이나 개선 제안에 대해서도 적절한 응답을 드리겠습니다.

@@ -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"

  1. 이 코드 패치는 빌드된 JAR 파일을 지정된 리포지토리로 복사하는 명령어입니다. 이 부분은 문제가 없어 보입니다.

  2. 주석 처리된 줄을 봤을 때, pgrep 명령을 사용하여 프로젝트 이름과 .jar 파일 확장자를 기준으로 현재 실행 중인 애플리케이션의 프로세스 ID(PID)를 찾고 있습니다. 그러나 현재 코드에서는 이 부분이 주석 처리되어 있으므로 실제로는 PID를 얻을 수 없을 것입니다.

  3. 대신에 주석 처리된 아래의 줄은 pgrep 명령을 사용하여 "active=prd"라는 문자열을 포함하는 프로세스 ID를 찾습니다. 개발 환경에 따라서는 이 방식이 정확한 PID를 얻을 수 있습니다.

  4. echo "$CURRENT_PID"는 현재 실행 중인 애플리케이션의 PID를 출력하는 부분입니다.

버그 위험:

  • 현재 코드에서는 주석 처리된 부분을 사용하고 있으므로 실제로는 현재 실행 중인 애플리케이션의 PID를 얻을 수 없습니다.

개선 제안:

  • PID를 찾는 방식과 관련된 문제가 있는지 확인해야 합니다. 프로젝트의 특정 버전을 기준으로 PID를 찾아야 한다면, 해당 조건을 제대로 지정하여 PID를 찾을 수 있는지 확인하세요.
  • PID를 찾는 대체 방법을 고려할 수 있습니다. 예를 들어, 일련의 프로세스 관리 도구 (예: ps, pgrep 등)를 사용하여 PID를 가져오는 대신에 애플리케이션 자체가 PID 파일을 작성하는 등의 방법을 사용할 수도 있습니다.

Expand Down
28 changes: 13 additions & 15 deletions server/src/main/java/com/bside/bside_311/Bside311Application.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,25 @@
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.context.annotation.Bean;
import org.springframework.data.domain.AuditorAware;
import org.springframework.data.jpa.repository.config.EnableJpaAuditing;

import java.util.Optional;

@EnableJpaAuditing
@SpringBootApplication
public class Bside311Application {

public static void main(String[] args) {
SpringApplication.run(Bside311Application.class, args);
}
public static void main(String[] args) {
SpringApplication.run(Bside311Application.class, args);
}

@Bean
public AuditorAware<Long> auditorProvider() {
return () -> {
Long userNoFromAuthentication = AuthUtil.getUserNoFromAuthentication();
if (userNoFromAuthentication == null) {
return Optional.empty();
}
return Optional.of(userNoFromAuthentication);
};
}
@Bean
public AuditorAware<Long> auditorProvider() {
return () -> {
Long userNoFromAuthentication = AuthUtil.getUserNoFromAuthentication();
if (userNoFromAuthentication == null) {
return Optional.empty();
}
return Optional.of(userNoFromAuthentication);
};
}

}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치에서 주목해야 할 부분은 다음과 같습니다:

  1. @EnableJpaAuditing 어노테이션의 삭제: 이 어노테이션은 JPA 감사(Auditing)를 사용하도록 설정하는 역할을 합니다. 만약 해당 기능을 사용하려면, 어노테이션이 필요합니다. 코드에서는 이 어노테이션이 삭제되었으므로 JPA 감사 기능이 비활성화된 것으로 보입니다. 이것이 의도한 동작인지 확인해야 합니다.

  2. main 메소드의 들여쓰기 변경: main 메소드의 내용이 들여쓰기에 의해 변경되었습니다. 이는 가독성 관점에서 큰 문제가 되지 않지만, 코드 스타일 일관성을 유지하기 위해서 들여쓰기 스타일을 통일하는 것이 좋습니다.

  3. auditorProvider() 메소드: auditorProvider() 메소드는 @Bean 어노테이션이 지정된 메소드로, AuditorAware 인터페이스의 구현체를 반환합니다. 이 메소드는 현재 인증된 사용자의 ID 값을 Optional로 반환합니다. 이 부분은 보안 측면에서 주의해야 합니다. AuthUtil.getUserNoFromAuthentication()에서 반환된 값이 신뢰할 수 있는지 확인하고, 인증되지 않은 사용자에게는 Optional.empty()를 반환하는 것이 좋습니다.

코드 패치의 개선점:

  1. @EnableJpaAuditing 어노테이션 삭제에 대한 이유를 명확하게 설명하거나, JPA 감사 기능을 포함하도록 복구해야 합니다.
  2. 들여쓰기 스타일을 일관성 있게 유지합니다.
  3. auditorProvider() 메소드에서 보안 검사와 관련된 부분을 검토하고 개선합니다.

10 changes: 10 additions & 0 deletions server/src/main/java/com/bside/bside_311/PostDomain.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.bside.bside_311;

import com.bside.bside_311.entity.Post;

public class PostDomain extends Post {

public static PostDomain of(Post post) throws CloneNotSupportedException {
return (PostDomain) Post.of(post);
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

주어진 코드 패치에 대한 간단한 코드 리뷰를 도와드리겠습니다.

  1. 상속: PostDomain 클래스가 Post 클래스를 확장하고 있습니다. 이는 현재 코드에서의 목적을 알 수 없으므로, 그렇게 많이 사용되지 않을 경우에는 상속을 피하는 것이 좋습니다.

  2. 정적 메서드: of() 메서드가 정적으로 선언되어 있습니다. 이 메서드를 정적으로 만든 이유와 Post.of() 메서드의 구현 내용을 알 수 없기 때문에 가독성과 유지 관리 측면에서 고려해볼 필요가 있습니다.

  3. 예외 처리: of() 메서드에서 CloneNotSupportedException 예외 처리가 포함되어 있습니다. 이 예외 처리가 필요한 이유를 알 수 없으며, 예외를 처리하는 대신 예외를 발생시키거나 검사하지 않는 방법을 고려해보는 것이 좋습니다.

  4. 패키지 및 클래스명: 패키지 이름은 일반적으로 소문자로 작성되어야 하며, 클래스 이름은 CamelCase 형식으로 작성하는 것이 권장됩니다. com.bside.bside_311 패키지명과 PostDomain 클래스명을 수정하는 것이 좋을 수 있습니다.

  5. 주석: 코드에는 아무런 주석이 없습니다. 주석을 사용하여 코드의 목적과 동작을 설명하는 것이 좋습니다.

위의 사항들을 고려하여 코드를 개선할 수 있습니다.

48 changes: 48 additions & 0 deletions server/src/main/java/com/bside/bside_311/PostDomainMultiple.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package com.bside.bside_311;

import com.bside.bside_311.dto.GetPostVo;
import com.bside.bside_311.dto.GetPostsMvo;
import com.bside.bside_311.dto.PostResponseDto;
import com.bside.bside_311.entity.Post;
import com.bside.bside_311.repository.PostMybatisRepository;
import com.bside.bside_311.repository.PostRepository;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.RequiredArgsConstructor;
import lombok.Setter;

import java.util.List;

@Builder
@Setter
@AllArgsConstructor
public class PostDomainMultiple {
private final PostRepository postRepository;
private final PostMybatisRepository postMybatisRepository;
List<PostDomain> postDomainList;
// List<PostResponseDto> postDomainList;
Long totalCount;



public static PostDomainMultiple init(PostRepository postRepository, PostMybatisRepository postMybatisRepository){
return PostDomainMultiple.builder().postRepository(postRepository).postMybatisRepository(postMybatisRepository).build();
}

public static PostDomainMultiple of(GetPostVo getPostVo, PostRepository postRepository, PostMybatisRepository postMybatisRepository) {
PostDomainMultiple postDomainMultiple = init(postRepository, postMybatisRepository);
List<GetPostsMvo> getPostsMvos = postMybatisRepository.getPosts(getPostVo);
Long totalCount = postMybatisRepository.getPostsCount(getPostVo);
List<Post> posts = getPostsMvos.stream().map(Post::of).toList();
List<PostDomain> list = posts.stream().map(post -> {
try {
return PostDomain.of(post);
} catch (CloneNotSupportedException e) {
throw new RuntimeException(e);
}
}).toList();
postDomainMultiple.setPostDomainList(list);
postDomainMultiple.setTotalCount(totalCount);
return postDomainMultiple;
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치는 PostDomainMultiple이라는 클래스를 정의하고 있습니다. 해당 클래스는 PostRepositoryPostMybatisRepository의 인스턴스를 사용하여 게시물 데이터를 처리하는 기능을 제공합니다.

코드 리뷰 내용:

  • @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)를 사용하고 있습니다. 이는 예외 처리에 대한 명시성을 잃어버리게 됩니다. 해당 예외를 던지거나 적절한 예외 유형으로 변경하는 것이 좋습니다.

위의 내용을 참고하여 코드를 수정 및 개선하셔야 할 것입니다.

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.bside.bside_311.config;

import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Profile;
import org.springframework.data.jpa.repository.config.EnableJpaAuditing;

@Profile("!test")
@Configuration
@EnableJpaAuditing
public class JpaConfiguration {
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치는 com.bside.bside_311.config 패키지 내에서 JpaConfiguration 클래스를 선언하고 구성합니다. 이 클래스는 JPA 감사(auditing) 기능을 활성화하는 데 사용됩니다.

이 코드에는 버그나 위험이 없어 보입니다. 하지만 몇 가지 개선 제안사항이 있습니다:

  1. 주석(comment): 코드의 의도와 동작에 대해 설명하는 주석을 추가하는 것이 도움이 될 수 있습니다.
  2. 패키지 이름: com.bside.bside_311.config 패키지가 적절한지 확인해야 합니다. 프로젝트의 컨벤션과 구조에 맞도록 패키지 이름을 조정할 수 있습니다.
  3. @EnableJpaAuditing 사용 영역 조정: @EnableJpaAuditing 어노테이션을 필요한 곳에만 적용할 수 있도록, @Configuration 어노테이션 바로 위로 이동시킬 수 있습니다.

이러한 개선 사항은 코드의 가독성과 유지 보수성을 향상시킬 수 있습니다.

Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.http.HttpStatus;
import org.springframework.validation.annotation.Validated;
import org.springframework.web.bind.annotation.DeleteMapping;
Expand Down Expand Up @@ -88,6 +90,16 @@ public GetAlcoholResponseDto getAlcohol(
return alcoholService.getAlcohol(page, size, orderColumn, orderType, searchKeyword);
}

@Operation(summary = "[o]술 목록 조회v2(성능 최적화)", description = "술 조회 API Page, size 사용법. <br> ex1) /alcohols/v2?page=0&size=10&sort=id,desc <br> ex2) /alcohols/v2?page=0&size=10&sort=id,desc&sort=content,asc&searchKeyword=키워드&searchUserNos=1,2,4")
@GetMapping("/v2")
public Page<AlcoholResponseDto> getAlcoholV2(
// @Schema(description = "페이지 번호와 사이즈.정렬 까지.(0부터) ex)[1]page=0&size=5&sort=id,desc [2]page=1&size=15&sort=id,desc&sort=content,asc", example = "0")
Pageable pageable,
@Schema(description = "알코올 키워드", example = "키워드") String searchKeyword) {
log.info(">>> AlcoholController.getAlcohol");
return alcoholService.getAlcoholV2(pageable, searchKeyword);
}

@Operation(summary = "[o]술 상세 조회", description = "술 상세 조회 API")
@GetMapping("/{alcoholNo}")
public AlcoholResponseDto getAlcoholDetail(@PathVariable("alcoholNo") Long alcoholNo) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 해당 코드 패치는 getAlcoholV2라는 새로운 API 엔드포인트를 추가하는 것으로 보입니다.
  2. getAlcoholV2 메서드는 Pageable 인자와 searchKeyword 인자를 받아서 술 목록을 조회합니다.
  3. @Operation 어노테이션을 통해 API 문서화를 수행하고 있습니다.
  4. 코드 리뷰에서 주로 확인해야 할 사항은 아래와 같습니다:
    • getAlcoholV2의 매개변수 타입인 Pageablespring-data-commons 라이브러리에서 제공되는 것으로 보입니다. 해당 라이브러리가 프로젝트의 의존성으로 추가되어 있는지 확인해야 합니다.
    • getAlcoholV2에서 log.info를 사용하여 로깅하고 있습니다. 로깅은 개발 및 디버깅에 유용하지만, 운영 환경에서 적절한 로깅 설정이 필요합니다. 로깅 설정을 확인하고 적절하게 조정해야 합니다.
  5. 위의 내용을 참고하여 코드 리뷰를 진행하시면 됩니다.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@
import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.http.HttpStatus;
import org.springframework.util.StringUtils;
import org.springframework.validation.annotation.Validated;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
Expand All @@ -35,6 +38,10 @@
import org.springframework.web.bind.annotation.ResponseStatus;
import org.springframework.web.bind.annotation.RestController;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

@Slf4j
@Validated
@RestController
Expand All @@ -51,8 +58,7 @@ public class PostController {
public AddPostResponseDto addPost(@RequestBody @Valid AddPostRequestDto addPostRequestDto) {
log.info(">>> PostController.addPost");
return postService.addPost(Post.of(addPostRequestDto), addPostRequestDto.getAlcoholNo(),
addPostRequestDto.getAlcoholFeature(), addPostRequestDto.getTagList(),
addPostRequestDto.getAlcoholInfo());
addPostRequestDto.getAlcoholFeature(), addPostRequestDto.getTagList());
}

@Operation(summary = "[o]게시글 수정", description = "게시글 수정 API")
Expand All @@ -74,7 +80,7 @@ public void deletePost(@PathVariable("postNo") Long postNo) {
postService.deletePost(postNo);
}

@Operation(summary = "[o]게시글 목록 조회", description = "게시글 조회 API")
@Operation(summary = "[o]게시글 목록 조회(v1)", description = "게시글 조회 API")
@GetMapping
public GetPostResponseDto getPosts(@RequestParam(name = "page", defaultValue = "0")
@Schema(description = "페이지번호(0부터), 기본값 0.", example = "0")
Expand All @@ -83,16 +89,69 @@ public GetPostResponseDto getPosts(@RequestParam(name = "page", defaultValue = "
@Schema(description = "사이즈, 기본값 10.", example = "10")
Long size,
@RequestParam(required = false, name = "orderColumn")
@Schema(description = "정렬 컬럼", example = "alcohol_no")
@Schema(description = "정렬 컬럼", example = "post_no")
String orderColumn,
@RequestParam(required = false, name = "orderType")
@Schema(description = "정렬 타입", example = "DESC")
String orderType,
@RequestParam(required = false, name = "searchKeyword")
@Schema(description = "키워드", example = "키워드")
String searchKeyword) {
String searchKeyword,
@RequestParam(required = false, name = "searchUserNos")
@Schema(description = "검색 유저 번호들.", example = "1,2,4")
String searchUserNos

) {
log.info(">>> PostController.getPost");
List<Long> searchUserNoList = new ArrayList<>();
try {
searchUserNoList =
Arrays.stream(searchUserNos.split(",")).map(Long::parseLong).toList();
} catch (NumberFormatException e) {
log.error(">>> PostController.getPost searchUserNos 파싱 에러 NumberFormatException", e);
} catch (Exception e) {
log.error(">>> PostController.getPost searchUserNos 파싱 에러 Exception", e);
}
return postService.getPosts(page, size, orderColumn, orderType, searchKeyword,
searchUserNoList);
}

@Operation(summary = "[o]게시글 목록 조회(v2)", description = "게시글 조회 API Page, size 사용법. <br> ex1) /posts/v2?page=0&size=10&sort=id,desc <br> ex2) /posts/v2?page=0&size=10&sort=id,desc&sort=content,asc&searchKeyword=키워드&searchUserNos=1,2,4")
@GetMapping("/v2")
public Page<PostResponseDto> getPostsV2(
// @Schema(description = "페이지 번호와 사이즈.정렬 까지.(0부터) ex)[1]page=0&size=5&sort=id,desc [2]page=1&size=15&sort=id,desc&sort=content,asc", example = "0")
Pageable pageable,
@RequestParam(required = false, name = "searchKeyword")
@Schema(description = "키워드", example = "키워드")
String searchKeyword,
@RequestParam(required = false, name = "searchUserNos")
@Schema(description = "검색 유저 번호들.", example = "1,2,4")
String searchUserNos,
@RequestParam(required = false, name = "isLikedByMe")
@Schema(description = "나에 의해서 좋아하는 게시글 필터 여부(true or false).", example = "false")
Boolean isLikedByMe,
@RequestParam(required = false, name = "isCommentedByMe")
@Schema(description = "내가 댓글을 단 게시글 필터 여부.(true or false)", example = "false")
Boolean isCommentedByMe

) {
log.info(">>> PostController.getPost");
return postService.getPosts(page, size, orderColumn, orderType, searchKeyword);
List<Long> searchUserNoList = new ArrayList<>();
if (StringUtils.hasText(searchUserNos)) {
try {
searchUserNoList =
Arrays.stream(searchUserNos.split(",")).map(Long::parseLong).toList();
} catch (NumberFormatException e) {
log.error(">>> PostController.getPost searchUserNos 파싱 에러 NumberFormatException", e);
throw new IllegalArgumentException("searchUserNos 파싱 에러 NumberFormatException", e);
} catch (Exception e) {
log.error(">>> PostController.getPost searchUserNos 파싱 에러 Exception", e);
throw new IllegalArgumentException("searchUserNos 파싱 에러 Exception", e);
}
}

return postService.getPostsV2(pageable, searchKeyword, searchUserNoList, isLikedByMe,
isCommentedByMe);
}

@Operation(summary = "[o]게시글 상세 조회", description = "게시글 상세 조회 API")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아래 코드 패치에 대해 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험과/또는 개선 제안을 환영합니다:

  1. 수정된 부분에서 import 문으로 추가된 Page, Pageable, StringUtils, List 등의 클래스가 정상적으로 import되었습니다.
  2. getPosts() 메서드의 파라미터로 searchUserNos가 추가되었습니다. 이는 쉼표로 구분된 문자열을 Long 리스트로 변환하여 사용합니다.
  3. getPostsV2() 메서드가 추가되었는데, 이는 Spring Data의 Pageable을 사용하여 페이지네이션 및 정렬 기능을 지원합니다. 또한 searchUserNos, isLikedByMe, isCommentedByMe 등의 추가적인 필터 옵션을 제공합니다.
  4. getPosts()getPostsV2() 메서드 내에서 searchUserNos 파라미터의 값이 유효하지 않은 경우 예외를 처리하고 있습니다.
  5. 로깅 코드를 통해 메서드의 실행 상황을 확인하는 용도로 사용되고 있습니다.

버그 위험 요소는 주요하게 두 부분입니다:

  1. getPosts()getPostsV2() 메서드에서 searchUserNos 파라미터의 값이 null이거나 빈 문자열인 경우에 대한 유효성 검사가 필요할 수 있습니다.
  2. getPostsV2() 메서드에서 searchUserNos 파라미터의 값 변환 과정에서 숫자로 변환할 수 없는 문자열이 포함될 경우 에러 핸들링을 개선할 필요가 있습니다.

개선 제안은 다음과 같습니다:

  1. 코드를 보다 간결하게 유지하기 위해 StringUtils.hasText()를 사용하여 searchUserNos 값이 있는지 확인하는 방법을 고려할 수 있습니다.
  2. getPostsV2() 메서드의 searchUserNos 값 변환 과정에서 예외 처리를 세분화하여 더 정확한 예외 메시지를 제공할 수 있도록 합니다.
  3. 로깅 코드가 주석으로 추가된 윗부분과 중복되므로, 추가적인 애플리케이션 요구사항에 기반하여 로깅 코드를 보완할 수 있습니다.

Expand Down
Loading