Feature/product testcode #57

HyunJunSon · 2024-01-15T07:24:26Z

💡 Motivation and Context

product testCode 작성
fixture Monkey library 적용
Id 값 Integer -> Long 타입 변경
배치코드 추가에 따른 도커컴포즈 이미지 추가

🔨 Modified

여기에 무엇이 크게 바뀌었는지 설명해 주세요

여기에 세부 변경사항을 설명해주세요

🌟 More

여기에 PR 이후 추가로 해야 할 일에 대해서 설명해 주세요
마이페이지, 메인페이지 도메인 코드, 테스트코드 작성

📋 커밋 전 체크리스트

[ㅇ] 추가/변경에 대한 단위 테스트를 완료하였습니다.
[ㅇ] 컨벤션에 맞게 작성하였습니다.

🤟🏻 PR로 완료된 이슈

closes #

- flyway에 있는 create sequence보다 entity 코드에 작성한 어노테이션으로 시퀀스가 생성되고 있는 현상 확인하여 sql문 변경 - 테이블 생성 후, 시퀀스 변경 sql 작성 방식 - sql을 구분하는 게 좋을 것 같아 init 파일에는 테이블 생성 ddl만 두고 시퀀스 변경 및 초기 공통코드 insert sql은 파일 분리

이제 여기서 문자열을 암호화 해서 붙여넣으시면 됩니다. 커밋할 때 원문이 노출되지 않았는지 확인하세요.

…hPedia into feature/docker

…into feature/myPage_CRUD

cr-gpt · 2024-01-15T07:24:45Z

docker-compose.yml

@@ -36,7 +37,7 @@ services:
 # db-read:
 # image: younglong/pg-washpedia:latest
 # volumes:
-# - postgreEditDB:/var/lib/postgresql/data
+# - ./postgreEditDB:/var/lib/postgresql/data
 # environment:
 # POSTGRES_DB: read_db
 # POSTGRES_USER: ${{ secrets.PROJECT_USER }}


Based on the code patch you provided, here are some of the improvements and bug risks I can identify:

The commented batch service image line (line 12) should be uncommented if you want to use a specific image. If you are using a custom image, make sure it is properly defined in the docker-compose.yml or specify it using the ${IMAGE} variable.

Port mapping for the batch service (lines 14-15) seems to be incorrect. Port 8080 should be mapped to 8082 instead of mapping 8082 to 8080. Adjust it like this: - "8080:8082".

Make sure the necessary dependencies (db-edit, db-read) are available when running the batch service. Verify that they are correctly specified in the depends_on section.

For the db-read service (lines 35-37), check if the volume mapping is correct. It currently points to ./postgreEditDB, which may cause issues if the directory doesn't exist. Make sure the path is accurate, and the PostgreSQL data will be properly persisted.

Additionally, it would be helpful to review other parts of the codebase, including Dockerfile, environment variables, and any additional services or functionality not included in the given patch.

cr-gpt · 2024-01-15T10:02:01Z

module-api/src/main/java/com/kernel360/product/controller/ProductController.java

@@ -36,7 +36,7 @@ ResponseEntity<ProductDto> findProductById(@PathVariable("id") Long productId) {
 .orElse(ResponseEntity.status(HttpStatus.NOT_FOUND).build());
 }

- @GetMapping("/products/")
+ @GetMapping("/products")
 ResponseEntity<List<ProductDto>> findProductByKeyword(@RequestParam("keyword") String keyword){
 final List<ProductDto> list = productService.getProductListByKeyword(keyword);



The code patch you provided is quite small, but here are a few observations:

In the "findProductById" method, it's generally good practice to return a more specific error response when a product is not found instead of a generic 404 status. You could consider returning a custom error message or using a more specific HTTP status code.

In the "findProductByKeyword" method, consider adding validation for the keyword parameter. For example, you can check if the keyword is empty or null and handle such cases appropriately.

It's generally recommended to use plural nouns for resource endpoints. So instead of "/products", you might consider using "/product".

Make sure that the "getProductListByKeyword" method in the "productService" does proper error handling for exceptions or edge cases. For example, what happens if the keyword is not found or if an error occurs during the search.

Overall, without seeing the complete context or implementation details, it's difficult to identify any other potential issues or suggest further improvements. However, these points should give you some initial areas to consider for your code review.

cr-gpt · 2024-01-15T10:02:14Z

module-api/src/main/java/com/kernel360/product/dto/ProductDto.java

@@ -22,7 +22,7 @@ public record ProductDto(Integer productNo,
 ) {

 public static ProductDto of(
- Integer productNo,
+ Long productNo,
 String productName,
 String barcode,
 String description,


Here are some observations and suggestions for your code patch:

You changed the type of productNo from Integer to Long in the ProductDto record. Make sure this change aligns with the requirements and data types used throughout your application.

Consider adding input validation checks in the of method parameters to ensure they are not null or empty if necessary. This can help prevent potential issues when creating instances of ProductDto.

Verify that the new parameters added to the of method are correctly assigned to the corresponding fields of the ProductDto record. Ensure the order and variable names match appropriately.

Double-check that the import statements at the top of your file are correct and necessary for the code you provided.

Additionally, it would be helpful to review the rest of the code-base where the ProductDto record is being used, along with any methods that accept or return this DTO. Ensure consistency in handling the changes made to the ProductDto record.

Remember to also test your code changes thoroughly to catch any potential bugs or issues that may arise as a result of these modifications.

linglong67

요약 및 덧붙이자면,

PR은 제목과 내용이 매치가 잘 되면 좋을 것 같음
fixture monkey 테스트 코드는 LGTM!
entity, dto 내 Integer → Long 으로 변경된 부분들은 flyway로 컬럼 정보 업데이트 필요

입니다..!
(근데요.. 커밋 이력은 살짝 꼬인건가요..? ㅋㅋㅋ)

linglong67 · 2024-01-16T01:34:55Z

docker-compose.yml

이후에는 PR을 구분해서 올려주시는 게 좋을 것 같아요!
PR 명과 내부 작업 사항이 다른 부분들이 있어서 나중에 이력 관리가 어려울 수도 있지 않을까요!?

linglong67 · 2024-01-16T01:36:00Z

module-api/lombok.config

내용에 대해 간략하게 설명해주시면 좋을 것 같아요~
(이 설정이 필요하게 된 이유랄까요??)

linglong67 · 2024-01-16T01:42:07Z

module-api/src/main/java/com/kernel360/member/dto/MemberDto.java

 public Member toEntity(MemberDto memberDto) {
 return Member.of(
- this.memberNo,
- this.id,
- this.email,
- this.password,
- this.gender,
- this.birthdate
+ memberDto.memberNo(),
+ memberDto.id(),
+ memberDto.email(),
+ memberDto.password(),
+ memberDto.gender(),
+ memberDto.birthdate()
 );


이거 이대로면 memberDto.toEntity(memberDto) 같은 형태로 호출되게 될 것 같은데요
memberDto.toEntity() 형태가 더 나아보여요! (매개변수가 의미가 있을까 싶어서요)

확인해봤는데 this로 보내나 memberDto로 보내나 둘다 toEntity(memberDto)로 해줘야됩니다.
회원 가입, 로그인 구현하는 동안은 member정보를 통으로 바인딩 할 일이 없어서 toEntity를 쓰지 않았는데
이런 고민이 있겠구나 하는걸 이제야 알았네요

linglong67 · 2024-01-16T01:44:11Z

module-api/src/test/java/com/kernel360/product/dto/ProductDtoTest.java

fixture monkey 사용 예시가 생겨서 좋네요 👍
다음에 한번 간략하게 설명 듣고 싶어요!

chan99k

잘 읽었습니다.
엔티티의 Id 타입을 Long으로 변경한 것 확인했어요. SERIAL 타입이 Integer와 Long을 모두 쓸 수 있는 거군요. 그런데 결국 정수범위 때문에 BIGSERIAL 로 바꿔야 하는 거 아닌가 싶습니다. flyway 스크립트로 타입 변경을 해야 할 거 같아요.

chan99k · 2024-01-15T07:26:26Z

docker-compose.yml

 # ports:
-# - "8082:8080"
+# - "8082:8082"


cr-gpt · 2024-01-16T02:45:27Z

module-api/src/main/java/com/kernel360/member/dto/MemberDto.java

+ this.email(),
+ this.password(),
+ this.gender(),
+ this.birthdate()
 );
 }



Overall, the code patch looks fine with a few suggestions for improvement:

Data Type: The memberNo field in MemberDto has been changed from Integer to Long. Make sure this change is intentional and aligns with the data type in the Member entity.

Redundant Parameters: In the of method of MemberDto, you can remove the parameters memberNo and memberDto, as they are not used within the method.

Method Naming: Instead of having toEntity() accept a MemberDto parameter, it can directly access the fields of the current object. You can omit the parameter and modify the method to public Member toEntity(). This aligns with how other methods are written.

Code Formatting: Ensure consistent indentation and formatting throughout the code. It helps with readability.

Besides these suggestions, ensure that the changes made match the requirements of your application, as I can only provide general code review suggestions based on the given information.

cr-gpt · 2024-01-16T02:45:41Z

module-api/src/main/java/com/kernel360/product/controller/ProductController.java

@@ -36,7 +36,7 @@ ResponseEntity<ProductDto> findProductById(@PathVariable("id") Long productId) {
 .orElse(ResponseEntity.status(HttpStatus.NOT_FOUND).build());
 }

- @GetMapping("/products/")
+ @GetMapping("/products/search")
 ResponseEntity<List<ProductDto>> findProductByKeyword(@RequestParam("keyword") String keyword){
 final List<ProductDto> list = productService.getProductListByKeyword(keyword);



Based on the provided code patch, here's a brief code review:

In the findProductById method, the response is handled well by returning ResponseEntity.status(HttpStatus.NOT_FOUND).build() when the product is not found. This ensures that a proper HTTP 404 Not Found response is returned.

In the findProductByKeyword method, the endpoint URL has been changed from /products/ to /products/search. This change is appropriate if the intention is to have a separate endpoint for searching products instead of retrieving all products.

It would be helpful to add validation checks for the keyword parameter in the findProductByKeyword method. Depending on the requirements, you might want to consider handling scenarios where the keyword is empty or null.

Consider using a pagination mechanism (if applicable) for fetching the list of products. Retrieving all products at once can lead to performance issues if there are a large number of products in the database.

Ensure that the underlying productService.getProductListByKeyword(keyword) method is implemented correctly and efficiently. Verify if any additional error handling or data validation is required within that method.

Overall, it's difficult to spot bugs or provide specific improvement suggestions without seeing the complete codebase. A more thorough review may be necessary to identify any potential risks or areas for improvement.

Remember to also consider best practices related to security, exception handling, logging, and testing based on your project requirements.

chan99k

LGTM. 질문 남겼습니다

chan99k · 2024-01-16T02:50:10Z

module-api/src/test/java/com/kernel360/product/controller/ProductControllerTest.java

+ @BeforeEach
+ void 준비(WebApplicationContext webApplicationContext) {
+ fixtureMonkey = FixtureMonkey.builder()
+ .objectIntrospector(new FailoverIntrospector(


objectIntrospector 와 FailoverIntrospector 가 어떤 역할을 하나요?

네이밍 추측만 보면 전자는 객체생성준비자같고 후자는 실패값이 떨어질때까지 만들어주는 생성자? 같은거 같네요

chan99k · 2024-01-16T02:50:49Z

module-api/src/test/java/com/kernel360/product/controller/ProductControllerTest.java

+ FieldReflectionArbitraryIntrospector.INSTANCE,
+ ConstructorPropertiesArbitraryIntrospector.INSTANCE,
+ BeanArbitraryIntrospector.INSTANCE,
+ BuilderArbitraryIntrospector.INSTANCE


각각의 Introspector 가 어떤 역할을 하는지 알려주실 수 있나요

gunsight1

고생하셨습니다 추후 PR에 flyway 파일 하나 추가하여서 테이블 컬럼들의 대한 serial4 -> serial8까지 추가해주시면 감사하겠습니다

HyunJunSon and others added 17 commits January 5, 2024 18:04

git commit -m "feature::docker-compose, github action CI"

47230d2

Create cr.yml

cad3149

feat : JasyptEncryptor 추가

919c7c2

이제 여기서 문자열을 암호화 해서 붙여넣으시면 됩니다. 커밋할 때 원문이 노출되지 않았는지 확인하세요.

chore : 암호문 변경 (원본 문자열에 차이는 없습니다)

37f1e0e

chore : jasypt 의존성 추가

818ff84

Update cr.yml

f91cd89

Update cr.yml

64125ee

Update cr.yml

03de4b9

Update cr.yml

f5218ba

feature::docker + githubActions CI기능추가

2746aad

Merge branch 'feature/docker' of https://github.com/HyunJunSon/F1-Was…

5f9c5de

…hPedia into feature/docker

Merge branch 'develop' of https://github.com/HyunJunSon/F1-WashPedia …

ffb1095

…into feature/myPage_CRUD

Merge branch 'develop' of https://github.com/HyunJunSon/F1-WashPedia …

bd26605

…into feature/myPage_CRUD

Merge branch 'develop' of https://github.com/HyunJunSon/F1-WashPedia …

aaba17c

…into feature/myPage_CRUD

Merge branch 'develop' of https://github.com/HyunJunSon/F1-WashPedia …

b678a34

…into feature/myPage_CRUD

feat::product testCode 작성

20beabb

HyunJunSon added the 💡 Feature 새로운 기능 추가, 혹은 구현 label Jan 15, 2024

HyunJunSon self-assigned this Jan 15, 2024

HyunJunSon requested review from linglong67, chan99k and gunsight1 as code owners January 15, 2024 07:24

cr-gpt bot reviewed Jan 15, 2024

View reviewed changes

feat::product testCode2 작성

45d60da

cr-gpt bot reviewed Jan 15, 2024

View reviewed changes

linglong67 approved these changes Jan 16, 2024

View reviewed changes

chan99k requested changes Jan 16, 2024

View reviewed changes

docker-compose.yml

# ports:

# - "8082:8080"

# - "8082:8082"

Copy link

Collaborator

chan99k Jan 15, 2024

Choose a reason for hiding this comment

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

👍🏻

feat::product controller testCode 추가

b32d9ec

cr-gpt bot reviewed Jan 16, 2024

View reviewed changes

chan99k approved these changes Jan 16, 2024

View reviewed changes

gunsight1 approved these changes Jan 17, 2024

View reviewed changes

gunsight1 merged commit 3db1964 into Kernel360:develop Jan 17, 2024
1 check passed

HyunJunSon deleted the feature/product_testcode branch January 17, 2024 05:48

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/product testcode #57

Feature/product testcode #57

HyunJunSon commented Jan 15, 2024

cr-gpt bot Jan 15, 2024

cr-gpt bot Jan 15, 2024

cr-gpt bot Jan 15, 2024

linglong67 left a comment

linglong67 Jan 16, 2024

linglong67 Jan 16, 2024

linglong67 Jan 16, 2024

gunsight1 Jan 17, 2024

linglong67 Jan 16, 2024

chan99k left a comment

chan99k Jan 15, 2024

cr-gpt bot Jan 16, 2024

cr-gpt bot Jan 16, 2024

chan99k left a comment

chan99k Jan 16, 2024

gunsight1 Jan 17, 2024

chan99k Jan 16, 2024

gunsight1 left a comment

Feature/product testcode #57

Feature/product testcode #57

Conversation

HyunJunSon commented Jan 15, 2024

💡 Motivation and Context

🔨 Modified

🌟 More

📋 커밋 전 체크리스트

🤟🏻 PR로 완료된 이슈

cr-gpt bot Jan 15, 2024

Choose a reason for hiding this comment

cr-gpt bot Jan 15, 2024

Choose a reason for hiding this comment

cr-gpt bot Jan 15, 2024

Choose a reason for hiding this comment

linglong67 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chan99k left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cr-gpt bot Jan 16, 2024

Choose a reason for hiding this comment

cr-gpt bot Jan 16, 2024

Choose a reason for hiding this comment

chan99k left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gunsight1 left a comment

Choose a reason for hiding this comment