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

Refactor/#61 페이지네이션 추가 및 필드 타입 변경 #63

Merged
merged 11 commits into from
Aug 15, 2024

Conversation

tidavid1
Copy link
Member

🚀 작업 내용

  • 커서 기반 페이지네이션을 적용했습니다.
  • Log appKey 필드 타입을 UUID로 변경했습니다.

📸 이슈 번호

👀 Focus Commits [Optional]

- Lombok 의존을 추가했습니다.
- MySQL R2DBC Driver를 추가했습니다.
- Log DB Entity를 구현했습니다.
  - DB Entity는 post-fix로 Data를 붙입니다.
- Log Domain Entity를 구현했습니다.
  - DB 엔티티와 매핑으로만 생성되도록 정적 팩토리 메서드를 활용했습니다.
- DB 엔티티와 도메인 엔티티를 매핑해주는 LogService를 구현했습니다.
- 이에 따른 테스트를 추가했습니다.
- AppKey 기반 조회 이후 결과를 반환하는 서비스 로직을 구현했습니다.
- 이에 따른 테스트를 추가했습니다.
- AppKey 기반 조회 이후 결과를 반환하는 API를 구현했습니다.
- Log 조회에 대한 커서 기반 페이지네이션을 적용했습니다.
- API 조회시 페이지네이션이 가능하게 했습니다.
  - RequestParam을 활용합니다
  - 요청이 없는경우 cursor는 -1, size는 10이 기본입니다.
- UUID 타입으로 저장된 appKey를 지원하기 위해 LogData의 필드 저장 형식을 UUID로 변경했습니다.
@tidavid1 tidavid1 self-assigned this Aug 15, 2024
Copy link

github-actions bot commented Aug 15, 2024

Risk Level 2 - /home/runner/work/Team5-Guys/Team5-Guys/logbat_view/src/main/java/info/logbat_view/domain/log/presentation/LogViewController.java

The controller method is clear. Consider adding validation for the cursor and size parameters to ensure they are within acceptable ranges before passing them to the service.


Risk Level 2 - /home/runner/work/Team5-Guys/Team5-Guys/logbat_view/src/main/java/info/logbat_view/common/util/UUIDConvertor.java

The UUID pattern is correctly defined, but consider using a more descriptive exception message for better clarity. For example, instead of 'UUID 형식이 올바르지 않습니다.', you could specify the expected format.


Risk Level 2 - /home/runner/work/Team5-Guys/Team5-Guys/logbat_view/src/main/java/info/logbat_view/domain/log/domain/service/LogService.java

The method findLogsByAppKey is well-structured. Ensure that the repository method handles cases where no logs are found to avoid returning empty Flux without context.


🛠️🔍📜


Powered by Code Review GPT

@tidavid1 tidavid1 linked an issue Aug 15, 2024 that may be closed by this pull request
1 task
Copy link
Member

@LuizyHub LuizyHub left a comment

Choose a reason for hiding this comment

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

코드 확인했습니다! 검증로직 한번 검토 부탁드리겠습니다!

public Flux<LogCommonResponse> findLogs(String appKey) {
return logService.findLogsByAppKey(appKey).map(LogCommonResponse::from);
public Flux<LogCommonResponse> findLogs(String appKey, Long id, Integer size) {
UUID appKeyUUID = UUID.fromString(appKey);
Copy link
Member

Choose a reason for hiding this comment

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

이부분에 대한 검증 로직이 추가되면 어떨까 싶습니다!

IllegalArgumentExceptionIf name does not conform to the string representation as described in toString

Copy link
Member Author

Choose a reason for hiding this comment

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

6e7d707 에 반영했습니다!

Copy link
Member

@miiiinju1 miiiinju1 left a comment

Choose a reason for hiding this comment

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

좋습니다!!

- 전달받은 UUID String에 대해 변환 전 검증을 포함한 UUIDConvertor를 구현했습니다.
- 이에 대한 테스트를 추가했습니다.
Copy link
Member

@LuizyHub LuizyHub left a comment

Choose a reason for hiding this comment

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

구현된 검증 로직 확인했습니다!
객체를 따로 분리해서 한점이 깔끔하고 좋네요!!
검증로직 한번만 검토 부탁드리겠습니다!!

Comment on lines +18 to +21
if (!UUID_PATTERN.matcher(uuid).matches()) {
throw new IllegalArgumentException("UUID 형식이 올바르지 않습니다.");
}
return UUID.fromString(uuid);
Copy link
Member

Choose a reason for hiding this comment

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

UUID.fromString() 를 통해 생성될때 내부적으로 검증을 하고, 올바른 형식이 아닐경우 IllegalArgumentException 을 던지기때문에 직접 패턴으로 검증하기보다 이렇게 해보는건 어떨가요?

Suggested change
if (!UUID_PATTERN.matcher(uuid).matches()) {
throw new IllegalArgumentException("UUID 형식이 올바르지 않습니다.");
}
return UUID.fromString(uuid);
try {
return UUID.fromString(uuid);
}
catch (IllegalArgumentException e) {
throw new IllegalArgumentException("UUID 형식이 올바르지 않습니다.");
}

Copy link
Member Author

Choose a reason for hiding this comment

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

IllegalArgumentException 을 캐치해서 IllegalArgumentException를 던진다는 로직이 뭔가 이상한거 같아서 배제했었는데 의견이 궁금합니다! 단순 에러 메시지 변경으로만 넘기는 것이 적절할까요?!

Copy link
Member

Choose a reason for hiding this comment

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

엇 그렇네요!
제가 무의식적으로 전역 에러핸들러가 생길것이고 거기서 메세지를 활용할거라 생각하고 있었나봐요.
이후 에러 처리에 대한 이야기를 정리하는게 좋겠네요!

Copy link
Member

@LuizyHub LuizyHub left a comment

Choose a reason for hiding this comment

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

코드 잘 봤습니다! 승인입니다~

@tidavid1 tidavid1 merged commit 018b166 into dev Aug 15, 2024
1 check passed
@tidavid1 tidavid1 deleted the refactor/#61 branch August 15, 2024 12:59
LuizyHub pushed a commit that referenced this pull request Aug 19, 2024
* init: 의존성 추가

- Lombok 의존을 추가했습니다.
- MySQL R2DBC Driver를 추가했습니다.

* feat: Log DB Entity 구현

- Log DB Entity를 구현했습니다.
  - DB Entity는 post-fix로 Data를 붙입니다.

* feat: Log Domain Entity 구현

- Log Domain Entity를 구현했습니다.
  - DB 엔티티와 매핑으로만 생성되도록 정적 팩토리 메서드를 활용했습니다.

* feat: Log Domain Mapping Service 구현

- DB 엔티티와 도메인 엔티티를 매핑해주는 LogService를 구현했습니다.
- 이에 따른 테스트를 추가했습니다.

* feat: Log 조회 기능 구현 - 서비스

- AppKey 기반 조회 이후 결과를 반환하는 서비스 로직을 구현했습니다.
- 이에 따른 테스트를 추가했습니다.

* feat: Log 조회 기능 구현 - API

- AppKey 기반 조회 이후 결과를 반환하는 API를 구현했습니다.

* refactor: LogService 페이지네이션 적용

- Log 조회에 대한 커서 기반 페이지네이션을 적용했습니다.

* refactor: Log API 페이지네이션 적용

- API 조회시 페이지네이션이 가능하게 했습니다.
  - RequestParam을 활용합니다
  - 요청이 없는경우 cursor는 -1, size는 10이 기본입니다.

* refactor: appKey 타입 변경 String -> UUID

- UUID 타입으로 저장된 appKey를 지원하기 위해 LogData의 필드 저장 형식을 UUID로 변경했습니다.

* feat: UUID Convertor 구현

- 전달받은 UUID String에 대해 변환 전 검증을 포함한 UUIDConvertor를 구현했습니다.
- 이에 대한 테스트를 추가했습니다.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[🛠 REFACT] View Server - 커서 기반 페이지네이션 적용
3 participants