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

feat: Data 모듈을 추가합니다 #21

Merged
merged 16 commits into from
Jun 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ subprojects {
implementation("org.jetbrains.kotlinx:kotlinx-coroutines-reactor")
implementation("io.projectreactor.kotlin:reactor-kotlin-extensions")
implementation("com.fasterxml.jackson.module:jackson-module-kotlin")

/** test **/
testImplementation("org.springframework.boot:spring-boot-starter-test")
testImplementation("io.mockk:mockk:${DependencyVersion.MOCKK}")
Copy link

Choose a reason for hiding this comment

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

이 코드 패치를 살펴보면, 당신은 한 줄의 코드에서 공백을 삭제하는 것외에 아무런 변경이 없습니다. 이는 버그 위험성이나 개선 방안을 따로 제공하기 어려운 상황입니다. 그러나 기본적인 코딩 컨벤션을 따른다고 가정하면, 필요없는 공백 라인을 제거함으로써 코드의 가독성을 약간 향상시키고 있습니다.

혹시 추가적으로 검토를 원하는 다른 코드 부분이 있다면, 그 부분을 포함해서 문의해 주세요.

Expand Down
3 changes: 3 additions & 0 deletions buildSrc/src/main/kotlin/DependencyVersion.kt
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ object DependencyVersion {
/** springDependencyManagementVersion */
const val SPRING_DEPENDENCY_MANAGEMENT = "1.1.5"

/** flyway */
const val FLYWAY = "9.16.0"

/** test */
const val MOCKK = "1.13.9"
const val KOTEST = "5.8.0"
Copy link

Choose a reason for hiding this comment

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

이 코드 패치는 새로운 상수 FLYWAY를 추가하고 있습니다. 겉보기에는 문제점이나 버그 위험을 볼 수 없습니다. 이 상수가 프로젝트에서 올바르게 사용되는지, 그리고 업데이트된 9.16.0버전의 Flyway 라이브러리가 프로젝트와 호환되는지 확인해 보시는 게 좋겠습니다.

나아가 개선 방안으로는 이 파일에서 버전 관리 되는 모든 라이브러리들의 최신화 여부를 확인하시는 것이 좋습니다. 의존성이 오래되면 보안 문제와 기능상의 문제를 유발할 수 있고, 경우에 따라 최신 버전의 언어나 프레임워크와 호환되지 않을 수 있습니다. 따라서 항상 각 의존성의 버전을 자주 체크하고 필요에 따라 업데이트하는 것이 중요합니다.

Expand Down
18 changes: 18 additions & 0 deletions data/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
tasks.getByName("bootJar") {
enabled = false
}

tasks.getByName("jar") {
enabled = true
}

dependencies {
/** spring starter */
implementation("org.springframework.boot:spring-boot-starter-data-jdbc")
implementation("com.mysql:mysql-connector-j")

/** flyway */
implementation("org.flywaydb:flyway-core:${DependencyVersion.FLYWAY}")
implementation("org.flywaydb:flyway-mysql")
}

This comment was marked as duplicate.

16 changes: 16 additions & 0 deletions data/src/main/kotlin/com/few/data/config/DataConfig.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.few.data.config

import org.springframework.context.annotation.ComponentScan
import org.springframework.context.annotation.Configuration

@Configuration
@ComponentScan(basePackages = [DataConfig.BASE_PACKAGE])
class DataConfig {
companion object {
const val BASE_PACKAGE = "com.few.data"
const val SERVICE_NAME = "few"
const val MODULE_NAME = "few-data"
const val BEAN_NAME_PREFIX = "fewData"
const val PROPERTY_PREFIX = SERVICE_NAME + "." + MODULE_NAME
}
}
Copy link

Choose a reason for hiding this comment

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

이 코드는 잘 작성되었지만 몇 가지 개선 사항을 제안하고 싶습니다:

  1. 생성된 DataSource 빈은 자동화된 구성 속성을 사용합니다(@ConfigurationProperties(prefix = "spring.datasource")). 이것은 기본적으로 정확해 보이지만, 여러 데이터소스를 관리하는 코드에서는 이 설정값이 의도한 데이터소스에 적용되고 있는지 확인하기 위해서 명확한 구성 속성 이름(SERVICE_NAME과 MODULE_NAME을 직접 사용할 수 있음)을 포함시키는 것이 좋습니다.

  2. dataSource() 메서드에서 DataSourceBuilder.create().build()에 직접 의존하고 있다면, 이것은 DataSource가 변경될 경우 코드 변경을 필요로 합니다. 해당 부분을 더욱 유연하게 바꾸려면 'DataSource` 인터페이스를 받아 사용하는 방식을 검토하세요.

  3. 주석 추가: 메서드의 기능과 각각의 구성 요소(class, variable 등)가 하는 일에 대한 주석을 추가하여 코드의 가독성을 향상시키는 것은 좋은 습관입니다.

  4. 마지막으로, 이 구성이 모든 예외 상황을 처리하고 있는지(예를 들어, datasource 수립 중 발생 가능한 에러), 그리고 적절한 로깅이 활성화 되어있는지 확인하세요. 이러한 방법은 오류를 추적하고 디버깅하는 데 많은 도움이 될 것입니다.

이상의 내용들은 주로 유지 보수성, 확장성, 그리고 가독성을 위한 것들입니다. 현재로써는 딱히 버그를 발생시킬 요인이나 위험 요인은 보이지 않습니다.

Copy link

Choose a reason for hiding this comment

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

이 코드를 간단히 살펴보면, Spring Framework 사전 설정이 있는 것으로 보입니다. 그러나 이 설정이 단독으로 사용되는 경우 문제가 없어 보입니다. 해당 코드가 등장하는 상황과 함께 보았을 때, 아래의 점들을 고려해 볼 수 있습니다.

  1. 구성 컴포넌트 스캔: ComponentScan은 지정된 패키지에서 Spring 빈을 찾아서 등록합니다. 베이스 패키지를 "com.few.data"로 설정했는데, 이것이 모든 필요한 Bean 클래스를 포함하는 경로인지 확인해 보세요.

  2. 상수 이름 정의: 상수 명명 규칙이 일관적이고, 나중에 값을 변경하는 데 혼란을 초래하지 않도록 주의하세요. 예를 들어, SERVICE_NAME, MODULE_NAME 같은 이름은 개발자가 쉽게 이해할 수 있지만, BEAN_NAME_PREFIXPROPERTY_PREFIX는 이들이 어떻게 사용되는지에 따라 변경될 수 있는 값일 수 있습니다.

  3. 필요한 프로퍼티: 특정 프로퍼티(BASE_PACKAGE, SERVICE_NAME, MODULE_NAME, BEAN_NAME_PREFIX, PROPERTY_PREFIX)는 시스템의 다른 부분에서 사용될 가능성이 있습니다. 어떻게 사용되는지, 필요한 범위가 어떤 것인지 확인하고 적절히 설정하세요.

  4. 해야 될 작업에 비해 과도하게 구성된 부분은 없는지 다시 한 번 확인하세요.

마지막으로 이 코드 패치가 제대로 동작하는지 테스트 해볼 필요가 있습니다. 단위 테스트나 통합 테스트를 통해 이 클래스가 올바르게 Bean을 스캔하고 있는지, 그리고 필요에 따라 정의된 상수들을 올바르게 사용하고 있는지 확인하는 것이 좋습니다.

60 changes: 60 additions & 0 deletions data/src/main/kotlin/com/few/data/flyway/FlywayConfig.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package com.few.data.flyway

import com.few.data.config.DataConfig
import org.flywaydb.core.Flyway
import org.flywaydb.core.api.configuration.FluentConfiguration
import org.springframework.boot.autoconfigure.flyway.FlywayMigrationInitializer
import org.springframework.boot.autoconfigure.flyway.FlywayProperties
import org.springframework.boot.context.properties.ConfigurationProperties
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import org.springframework.context.annotation.Profile
import java.util.function.Consumer
import javax.sql.DataSource

@Configuration
class FlywayConfig {
@Bean(name = [DataConfig.BEAN_NAME_PREFIX + "Flyway"])
fun flyway(
configuration: org.flywaydb.core.api.configuration.Configuration?
): Flyway {
return Flyway(configuration)
}

@Profile("!new")
@Bean(name = [DataConfig.BEAN_NAME_PREFIX + "FlywayValidateInitializer"])
fun flywayValidateInitializer(
flyway: Flyway?
): FlywayMigrationInitializer {
return FlywayMigrationInitializer(flyway) { obj: Flyway -> obj.validate() }
}

@Bean(name = [DataConfig.BEAN_NAME_PREFIX + "FlywayMigrationInitializer"])
fun flywayMigrationInitializer(
flyway: Flyway?
): FlywayMigrationInitializer {
return FlywayMigrationInitializer(flyway) { obj: Flyway -> obj.migrate() }
}

@Bean(name = [DataConfig.BEAN_NAME_PREFIX + "FlywayProperties"])
@ConfigurationProperties(prefix = "spring.flyway")
fun flywayProperties(): FlywayProperties {
return FlywayProperties()
}

@Bean(name = [DataConfig.BEAN_NAME_PREFIX + "FlywayConfiguration"])
fun configuration(
dataSource: DataSource?
): org.flywaydb.core.api.configuration.Configuration {
val configuration = FluentConfiguration()
configuration.dataSource(dataSource)
flywayProperties().locations.forEach(
Consumer { locations: String? ->
configuration.locations(
locations
)
}
)
return configuration
}
}
Copy link

Choose a reason for hiding this comment

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

이 코드 패치를 간략히 검토해 보겠습니다.

  1. 모든 메서드에서 파라미터로 null 가능성을 허용하고 있습니다. 이 경우, 해당 객체를 사용할 때 NullPointerException의 위험이 존재합니다. 가능한 한 non-null 타입을 사용하는 것이 좋습니다.

  2. 'FlywayConfig'에서 '@bean' 어노테이션에 'DataConfig.BEAN_NAME_PREFIX' 상수를 사용하고 있는데, Beans 이름 충돌을 방지하기 위함으로 보입니다. 그러나 일반적인 Spring 관례에서는 명시적 Bean 이름 지정은 자주 사용되지 않습니다. 여기선 클래스 또는 메서드 이름을 이용하는 것이 바람직해 보입니다.

  3. 'flywayConfiguration' 메서드에서 'flywayProperties().locations.forEach'를 사용하고 있는데, 이렇게 하면 매번 새로운 'FlywayProperties' 인스턴스가 생성됩니다. 이는 불필요한 객체 생성으로 'FlywayProperties'을 Spring Bean으로 관리하고 주입 받는 것이 더 나을 것 같습니다.

  4. 주입 대상이 없으면 에러를 발생시키려면 '@Autowired' 어노테이션을 사용해야 합니다. 현재 코드에서는 null 가능성이 있으므로, 다른 부분에서 이 객체를 가져다 사용하려고 하면 문제가 발생할 수 있습니다.

  5. 코드 내에 람다 식을 이용해 복잡도를 줄일 수 있습니다. 예시로 'Consumer'는 직접적으로 사용하는 대신 Kotlin의 람다 식을 이용할 수 있습니다.

코드가 정상적으로 작동하더라도 위에서 제안한 사항들이 코드의 안정성과 관리 용이성을 향상시킬 수 있습니다.

6 changes: 6 additions & 0 deletions data/src/main/resources/application-data-local.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
spring:
flyway:
locations: classpath:db/migration/entity
sql-migration-suffixes: sql
baseline-on-migrate: true
baseline-version: 0
Copy link

Choose a reason for hiding this comment

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

다음은 이 패치에 대한 코드 리뷰입니다:

  1. 민감한 정보 접근: 데이터베이스 자격 증명 (사용자 이름과 비밀번호)가 코드에 직접 포함되어 있습니다. 이는 보안 위협으로 다루어져야 합니다. Spring에서는 이러한 값들을 암호화하거나, 외부 설정 파일, 환경 변수 등을 활용하여 처리하는 것이 일반적입니다.

  2. 소스 버전 관리 시스템 사용: 비밀번호와 같은 중요 정보를 코드베이스에 포함시키면, 배포 및 형상 관리 시스템에도 그대로 노출됩니다. 이 문제도 1번의 해결책이 되겠습니다.

  3. 뉴라인(개행문자): 파일의 마지막 줄은 개행문자(\n)로 끝나야합니다. 현재 개행문자가 없기 때문에, 이 부분을 고려하여 수정해야 합니다.

  4. DB 구성 체크: jdbcUrl이 올바른지, 'allowPublicKeyRetrieval=true' 및 'rewriteBatchedStatements=true' 옵션이 실제 필요한지 확인해야합니다. 또한 localhost와 특정 포트 (13306)를 사용하고 있는데, 운영 환경에서 이 설정이 적용되는지 검토해야합니다.

  5. 하드코딩: "com.mysql.cj.jdbc.Driver" 와 같이 특정 값들이 하드코딩되어 있습니다. 가능한 한 환경 설정에서는 상수 보다는 변수를 사용하는 것이 좋습니다. 이러한 개선을 통해 코드의 유연성과 재사용성을 높일 수 있습니다.

Copy link

Choose a reason for hiding this comment

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

이 코드 패치를 검토하면, 대개 정상적으로 작동해야합니다. 단, 몇 가지 주의점을 참고하세요:

  1. classpath:db/migration/entity는 실제로 존재하는 경로인가요? 폴더가 없거나 이름이 잘못되어 있으면 오류가 발생할 수 있습니다.

  2. 이 설정은 Flyway가 버전 관리를 사용하여 데이터베이스 마이그레이션을 처리하도록 지시합니다. 여기서 baseline-on-migrate: true 설정은 초기화 된 데이터베이스에 최초 마이그레이션을 적용할 때 사용됩니다. baseline-version: 0은 베이스라인을 설정하는데, 이것은 실제로 사용하는 베이스라인 버전이 맞는가요?

  3. 구성 파일의 마지막에 새로운 줄을 추가하세요(\ No newline at end of file). 이를 통해 다른 개발자들이 Git에서 파일 변경 사항을 더 쉽게 확인하고, 일부 응용 프로그램에서 파일을 올바르게 분석할 수 있습니다.

개선할 부분이나 수정해야 할 중대한 위험이 발견되지는 않았습니다. 코드를 제공해주셔서 감사드립니다.

6 changes: 6 additions & 0 deletions data/src/main/resources/application-data-prd.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
spring:
flyway:
locations: classpath:db/migration/entity
sql-migration-suffixes: sql
baseline-on-migrate: true
baseline-version: 0
Copy link

Choose a reason for hiding this comment

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

이 코드 패치는 Spring Boot에서 Flyway를 설정하는 설정 파일의 일부로 보입니다. 당신이 제공한 부분을 기준으로 보면, 눈에 띄는 문제나 버그는 없습니다. 그러나 다음과 같은 사항들을 주의하여야 합니다.

  1. 'locations' 옵션은 Flyway 마이그레이션 스크립트 파일을 찾아서 실행하게끔 하는 위치를 지정합니다. 따라서 적절한 디렉토리 경로인지 확인해야 합니다.

  2. 'sql-migration-suffixes'가 'sql'로 설정되어 있어 이는 SQL 파일만 마이그레이션 파일로 인식한다는 것을 의미합니다. 프로젝트에 따라 다른 유형의 파일(.java 등)이 필요할 수 있으므로, 이 점을 고려하세요.

  3. 'baseline-on-migrate'가 true로 설정되어 있는 경우, 해당 데이터베이스에 아직 기본선(baseline)이 설정되지 않았다면 매번 마이그레이션 때마다 기본선(baseline)이 설정됩니다. 이 결과 기존의 데이터베이스 테이블을 건드리지 않고 추가적인 마이그레이션만 처리하게 됩니다. 하지만, 만약 첫 마이그레이션이 아닌 경우에는 이를 false로 변경하여야 할 수도 있습니다.

  4. 'baseline-version'은 최초의 기본선(baseline) 마이그레이션 버전을 설정합니다. 이 값은 첫 번째 마이그레이션의 버전보다 낮아야 합니다. 따라서, 이미 마이그레이션이 진행된 상태라면 이 값을 적절히 조정해야 할 수도 있습니다.

  5. 코드의 맨 끝에 개행 문자가 없는 '\ No newline at end of file' 경고가 있습니다. 이는 파일의 끝에서 새 줄(newline)이 없음을 나타내며 일부 플랫폼에서 문제를 일으킬 수 있으므로, 마지막 줄에 개행을 추가하는 것이 좋습니다.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
CREATE TABLE foo_tb
(
id BIGINT NOT NULL AUTO_INCREMENT,
name VARCHAR(255) NOT NULL,
deleted BIT NOT NULL,
primary key (id)
);
Copy link

Choose a reason for hiding this comment

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

이 코드 패치는 새로운 'foo_tb' 테이블을 생성하는 MySQL DB 스크립트 처럼 보이며, 3개의 필드가 포함되어 있습니다: id, name, deleted.

버그 또는 잠재적 위험이있는 부분에 주목할 사항이 몇 가지 있습니다.

  1. AUTO_INCREMENT가 설정된 id 필드입니다. 일반적으로 MySQL에서 이를 지원하지만, 사용하려는 DBMS가 이 속성을 지원하지 않으면 에러가 발생 합니다. DBMS 지원 여부를 확인해야 합니다.
  2. 테이블에 TIMESTAMP 또는 DATETIME 필드가 없습니다. 데이터 생성시간과 수정시간을 추적하는 것은 디버깅 및 관리에 유용합니다.

향상시킬 수 있는 영역:

  1. deleted 필드의 이름을 is_deleted로 변경하면 깔끔한 코드를 작성하는데 도움이 됩니다. 이것은 불린값임을 쉽게 판단하도록 돕기 때문입니다.
  2. 추가적인 인덱스를 만들어 검색 성능을 개선 할 수 있습니다. 예를 들어, name 필드에 인덱스를 만들면 이름 기준 조회시 성능이 향상됩니다.

코드는 다음과 같이 개선될 수 있습니다:

@@ -0,0 +1,10 @@
 CREATE TABLE foo_tb
 (
     id          BIGINT      NOT NULL AUTO_INCREMENT,
     name        VARCHAR(255) NOT NULL,
     is_deleted  BIT          NOT NULL,
     created_at  TIMESTAMP    NOT NULL DEFAULT CURRENT_TIMESTAMP,
     updated_at  TIMESTAMP    NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
     PRIMARY KEY (id),
     INDEX (name)
 );

1 change: 1 addition & 0 deletions settings.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
rootProject.name = "few"

include("api")
include("data")

This comment was marked as duplicate.

Loading