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.

Sign up for GitHub

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

Jump to bottom

Feature/product testcode #57

Merged
merged 19 commits into from
Jan 17, 2024
Merged

Feature/product testcode #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
47230d2
git commit -m "feature::docker-compose, github action CI"
HyunJunSon Jan 4, 2024
0e0b953
feat: flyway sql modify
linglong67 Jan 4, 2024
cad3149
Create cr.yml
linglong67 Jan 5, 2024
919c7c2
feat : JasyptEncryptor ์ถ”๊ฐ€
chan99k Jan 4, 2024
37f1e0e
chore : ์•”ํ˜ธ๋ฌธ ๋ณ€๊ฒฝ (์›๋ณธ ๋ฌธ์ž์—ด์— ์ฐจ์ด๋Š” ์—†์Šต๋‹ˆ๋‹ค)
chan99k Jan 4, 2024
818ff84
chore : jasypt ์˜์กด์„ฑ ์ถ”๊ฐ€
chan99k Jan 4, 2024
f91cd89
Update cr.yml
linglong67 Jan 5, 2024
64125ee
Update cr.yml
linglong67 Jan 5, 2024
03de4b9
Update cr.yml
linglong67 Jan 5, 2024
f5218ba
Update cr.yml
linglong67 Jan 5, 2024
2746aad
feature::docker + githubActions CI๊ธฐ๋Šฅ์ถ”๊ฐ€
HyunJunSon Jan 5, 2024
5f9c5de
Merge branch 'feature/docker' of https://github.com/HyunJunSon/F1-Wasโ€ฆ
HyunJunSon Jan 5, 2024
ffb1095
Merge branch 'develop' of https://github.com/HyunJunSon/F1-WashPedia โ€ฆ
HyunJunSon Jan 10, 2024
bd26605
Merge branch 'develop' of https://github.com/HyunJunSon/F1-WashPedia โ€ฆ
HyunJunSon Jan 11, 2024
aaba17c
Merge branch 'develop' of https://github.com/HyunJunSon/F1-WashPedia โ€ฆ
HyunJunSon Jan 11, 2024
b678a34
Merge branch 'develop' of https://github.com/HyunJunSon/F1-WashPedia โ€ฆ
HyunJunSon Jan 12, 2024
20beabb
feat::product testCode ์ž‘์„ฑ
HyunJunSon Jan 15, 2024
45d60da
feat::product testCode2 ์ž‘์„ฑ
HyunJunSon Jan 15, 2024
b32d9ec
feat::product controller testCode ์ถ”๊ฐ€
HyunJunSon Jan 16, 2024
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
5 changes: 3 additions & 2 deletions docker-compose.yml
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

์ดํ›„์—๋Š” PR์„ ๊ตฌ๋ถ„ํ•ด์„œ ์˜ฌ๋ ค์ฃผ์‹œ๋Š” ๊ฒŒ ์ข‹์„ ๊ฒƒ ๊ฐ™์•„์š”!
PR ๋ช…๊ณผ ๋‚ด๋ถ€ ์ž‘์—… ์‚ฌํ•ญ์ด ๋‹ค๋ฅธ ๋ถ€๋ถ„๋“ค์ด ์žˆ์–ด์„œ ๋‚˜์ค‘์— ์ด๋ ฅ ๊ด€๋ฆฌ๊ฐ€ ์–ด๋ ค์šธ ์ˆ˜๋„ ์žˆ์ง€ ์•Š์„๊นŒ์š”!?

Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ services:

# batch:
# build: ./module-batch
# image: ${IMAGE}
# ports:
# - "8082:8080"
# - "8082:8082"
Copy link
Collaborator

Choose a reason for hiding this comment

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

๐Ÿ‘๐Ÿป

# depends_on:
# - db-edit
#
Expand All @@ -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 }}
Copy link

Choose a reason for hiding this comment

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

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

  1. 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.

  2. 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".

  3. 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.

  4. 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.

Expand Down
7 changes: 6 additions & 1 deletion module-api/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,15 @@ dependencies {
annotationProcessor 'org.projectlombok:lombok'
testCompileOnly 'org.projectlombok:lombok'
testAnnotationProcessor 'org.projectlombok:lombok'

// validataion
implementation 'org.springframework.boot:spring-boot-starter-validation:2.7.4'
// test
testImplementation 'org.springframework.boot:spring-boot-starter-test'
testImplementation 'org.springframework.security:spring-security-test'
// test - fixture Monkey
testImplementation 'net.jqwik:jqwik:1.7.3'
testImplementation("com.navercorp.fixturemonkey:fixture-monkey-starter:1.0.0")
testImplementation("com.navercorp.fixturemonkey:fixture-monkey-jakarta-validation:1.0.0")
}

tasks.named('test') {
Expand Down
1 change: 1 addition & 0 deletions module-api/lombok.config
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
lombok.anyConstructor.addConstructorProperties=true
4 changes: 2 additions & 2 deletions module-api/src/main/java/com/kernel360/commoncode/dto/CommonCodeDto.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
/**
* DTO for {@link com.kernel360.commoncode.entity.CommonCode}
*/
public record CommonCodeDto(Integer codeNo,
public record CommonCodeDto(Long codeNo,
String codeName,
Integer upperNo,
String upperName,
Expand All @@ -25,7 +25,7 @@ public record CommonCodeDto(Integer codeNo,
* **/

public static CommonCodeDto of(
Integer codeNo,
Long codeNo,
String codeName,
Integer upperNo,
String upperName,
Expand Down
18 changes: 9 additions & 9 deletions module-api/src/main/java/com/kernel360/member/dto/MemberDto.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
/**
* DTO for {@link com.kernel360.member.entity.Member}
*/
public record MemberDto(Integer memberNo,
public record MemberDto(Long memberNo,
String id,
String email,
String password,
Expand All @@ -21,7 +21,7 @@ public record MemberDto(Integer memberNo,
) {

public static MemberDto of(
Integer memberNo,
Long memberNo,
String id,
String email,
String password,
Expand Down Expand Up @@ -64,14 +64,14 @@ public static MemberDto from(Member entity) {
);
}

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

Copy link

Choose a reason for hiding this comment

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

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

  1. 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.

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

  3. 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.

  4. 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.

Expand Down
9 changes: 6 additions & 3 deletions module-api/src/main/java/com/kernel360/member/service/MemberService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.Optional;

Expand All @@ -26,6 +27,7 @@ public class MemberService {
/**
* ๊ฐ€์ž…
**/
@Transactional
public void joinMember(MemberDto requestDto) {

Member entity = getNewJoinMemberEntity(requestDto);
Expand All @@ -43,6 +45,7 @@ protected Member getNewJoinMemberEntity(MemberDto requestDto) {
/**
* ๋กœ๊ทธ์ธ
**/
@Transactional
public MemberDto login(MemberDto loginDto) {

Member loginEntity = getReqeustLoginEntity(loginDto);
Expand All @@ -67,7 +70,7 @@ protected Auth modifyAuthJwt(Auth modifyAuth, String encryptToken) {
return modifyAuth;
}

protected Auth createAuthJwt(Integer memberNo, String encryptToken) {
protected Auth createAuthJwt(Long memberNo, String encryptToken) {

return Auth.of(null, memberNo, encryptToken, null);
}
Expand All @@ -78,14 +81,14 @@ private Member getReqeustLoginEntity(MemberDto loginDto) {

return Member.loginMember(loginDto.id(), encodePassword);
}


@Transactional(readOnly = true)
public boolean duplicatedCheckId(String id) {
Member member = memberRepository.findOneById(id);

return member != null;
}

@Transactional(readOnly = true)
public boolean duplicatedCheckEmail(String email) {
Member member = memberRepository.findOneByEmail(email);

Expand Down
2 changes: 1 addition & 1 deletion module-api/src/main/java/com/kernel360/product/controller/ProductController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Copy link

Choose a reason for hiding this comment

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

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

  1. 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.

  2. 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.

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

  4. 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.

Copy link

Choose a reason for hiding this comment

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

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

  1. 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.

  2. 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.

  3. 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.

  4. 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.

  5. 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.

  6. 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.

Expand Down
4 changes: 2 additions & 2 deletions module-api/src/main/java/com/kernel360/product/dto/ProductDto.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
* DTO for {@link com.kernel360.product.entity.Product}
*/

public record ProductDto(Integer productNo,
public record ProductDto(Long productNo,
String productName,
String barcode,
String description,
Expand All @@ -22,7 +22,7 @@ public record ProductDto(Integer productNo,
) {

public static ProductDto of(
Integer productNo,
Long productNo,
String productName,
String barcode,
String description,
Copy link

Choose a reason for hiding this comment

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

Here are some observations and suggestions for your code patch:

  1. 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.

  2. 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.

  3. 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.

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

  5. 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.

Expand Down
6 changes: 4 additions & 2 deletions module-api/src/main/java/com/kernel360/product/service/ProductService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,31 +5,33 @@
import com.kernel360.product.repository.ProductRepository;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

@Service
@RequiredArgsConstructor
public class ProductService {

private final ProductRepository productRepository;

@Transactional(readOnly = true)
public Optional<Product> getProductById(Long id) {

return productRepository.findById(id);
}

@Transactional(readOnly = true)
public List<Product> getProductList() {

return productRepository.findAll();
}

@Transactional(readOnly = true)
public List<ProductDto> getProductListByKeyword(String keyword) {
List<Product> products = productRepository.findByKeyword(keyword);

return products.stream().map(ProductDto::from).toList();
}

}
2 changes: 1 addition & 1 deletion module-api/src/test/java/com/kernel360/commoncode/controller/CommonCodeControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public void setup(){
void ์ƒ์œ„์ฝ”๋“œ๋ช…์„_์ธ์ˆ˜๋กœ_๋ฐ›๋Š”_๊ณตํ†ต์ฝ”๋“œ_๋ชฉ๋ก_์กฐํšŒ() throws Exception {

String pathVariable = "color";
Integer codeNo = 1;
Long codeNo = 1L;
String codeName = "ํ…Œ์ŠคํŠธ์šฉ ์ฝ”๋“œ";
Integer upperNo = null;
String upperName = null;
Expand Down
2 changes: 1 addition & 1 deletion module-api/src/test/java/com/kernel360/member/controller/MemberControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public void setup() {
void ๋กœ๊ทธ์ธ() throws Exception {

MemberDto memberDto = MemberDto.of("testID", "testPassword");
MemberDto memberInfo = new MemberDto(1,
MemberDto memberInfo = new MemberDto(1L,
"test01",
"[email protected]",
"",
Expand Down
4 changes: 2 additions & 2 deletions module-api/src/test/java/com/kernel360/member/entity/MemberTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

class MemberTest {

private Integer memberNo;
private Long memberNo;
private String id;
private String email;
private String password;
Expand All @@ -20,7 +20,7 @@ class MemberTest {

@BeforeEach
void ํ…Œ์ŠคํŠธ์ค€๋น„() {
memberNo = 1;
memberNo = 1L;
id = "user123";
email = "[email protected]";
password = "password123";
Expand Down
12 changes: 6 additions & 6 deletions module-api/src/test/java/com/kernel360/member/service/MemberServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public void init() {
/** given **/
MemberDto loginDto = MemberDto.of("test03", "1234qwer");
Member mockLoginEntity = Member.loginMember(loginDto.id(), loginDto.password());
Member mockEntity = Member.of(502, loginDto.id(), "[email protected]", "0eb9de69892882d54516e03e30098354a2e39cea36adab275b6300c737c942fd", null, null);
Member mockEntity = Member.of(502L, loginDto.id(), "[email protected]", "0eb9de69892882d54516e03e30098354a2e39cea36adab275b6300c737c942fd", null, null);
String mockToken = "dummy_token";

/** stub **/
Expand All @@ -105,13 +105,13 @@ public void init() {
void ํ† ํฐ_๋ฐœ๊ธ‰_์ €์žฅ_ํ…Œ์ŠคํŠธ() {

/** given **/
Member memberEntity = Member.of(502, "test03", null, null, null, null);
Member memberEntity = Member.of(502L, "test03", null, null, null, null);
String mockToken = "mockToken";
Auth auth = Auth.jwt(null, 502, mockToken);
Auth auth = Auth.jwt(null, 502L, mockToken);

/** stub **/
when(jwt.generateToken(anyString())).thenReturn(mockToken);
when(authRepository.findOneByMemberNo(anyInt())).thenReturn(auth);
when(authRepository.findOneByMemberNo(anyLong())).thenReturn(auth);

/** when **/
String token = jwt.generateToken(memberEntity.getId()); //mockToken return ์ •์ƒ ์ˆ˜ํ–‰ ํ™•์ธ
Expand All @@ -136,7 +136,7 @@ public void init() {

/** given **/
String id = "test01";
Member memberEntity = Member.of(51, "test01", null, null, null, null);
Member memberEntity = Member.of(51L, "test01", null, null, null, null);

/** stub **/
when(memberRepository.findOneById(anyString())).thenReturn(memberEntity);
Expand Down Expand Up @@ -175,7 +175,7 @@ public void init() {

/** given **/
String email = "[email protected]";
Member memberEntity = Member.of(51, "test01", "[email protected]", null, null, null);
Member memberEntity = Member.of(51L, "test01", "[email protected]", null, null, null);

/** stub **/
when(memberRepository.findOneByEmail(anyString())).thenReturn(memberEntity);
Expand Down
Loading
Loading