fix: 메이플랜드 공식 사이트 이전으로 인한 크롤러 수정 및 API 변경#29
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the notification system by introducing a V2 API with cursor-based pagination and a new crawling architecture. It transitions the crawler to an interface-based design, adds a dedicated batch service for notification processing, and implements new DTOs and controllers for the V2 endpoints. Review feedback suggests enhancing data integrity with unique constraints on read records, optimizing JPA transactions by leveraging dirty checking, and improving the resilience of the web crawler. Additionally, recommendations were made to optimize pagination performance and provide default values for API parameters.
| @Entity | ||
| @Table(name = "alrim_read", indexes = { | ||
| @Index(name = "idx_alrim_read_member_id_alrim_link", columnList = "memberId, alrimLink"), | ||
| @Index(name = "idx_alrim_read_member_id_alrim_id", columnList = "memberId, alrimId"), |
There was a problem hiding this comment.
memberId와 alrimId 조합에 대해 유니크 제약 조건을 추가하는 것이 좋습니다. 현재 AlrimCommandService.setReadAlrim에서 exists 체크 후 save를 하고 있는데, 이는 동시성 환경에서 두 요청이 동시에 체크를 통과할 경우 중복 데이터가 생성될 수 있는 Race Condition이 존재합니다. DB 레벨에서 유니크 인덱스를 설정하면 이를 확실히 방지할 수 있습니다.
| @Index(name = "idx_alrim_read_member_id_alrim_id", columnList = "memberId, alrimId"), | |
| @Index(name = "idx_alrim_read_member_id_alrim_id", columnList = "memberId, alrimId", unique = true), |
| savedEvent.setOutdated(crawledOutdated); | ||
| alrimRepository.save(savedEvent); |
There was a problem hiding this comment.
@transactional이 적용된 서비스 메서드 내에서 영속성 컨텍스트에 의해 관리되는 엔티티(savedEvent)의 상태를 변경하면, 트랜잭션 종료 시점에 변경 감지(Dirty Checking)를 통해 자동으로 DB에 반영됩니다. 따라서 명시적인 save() 호출은 불필요합니다. 92라인의 markAsOutdated 메서드 내 save() 호출도 동일하게 제거 가능합니다.
| savedEvent.setOutdated(crawledOutdated); | |
| alrimRepository.save(savedEvent); | |
| savedEvent.setOutdated(crawledOutdated); |
| } | ||
|
|
||
| private BoardRow toBoardRow(Element anchor, AlrimType type) { | ||
| Element row = anchor.parent() != null ? anchor.parent().parent() : null; |
| Alrim cursorAlrim = queryFactory.selectFrom(alrim) | ||
| .where(alrim.id.eq(cursorId)) | ||
| .fetchOne(); |
| public ResponseEntity<ResponseTemplate<CursorPage<AlrimV2DTOWithReadInfo>>> getAllAlrim( | ||
| @AuthenticationPrincipal PrincipalDetails principalDetails, | ||
| @Nullable Long cursor, | ||
| int pageSize |
There was a problem hiding this comment.
pageSize 파라미터에 기본값이 설정되어 있지 않습니다. primitive type인 int는 값이 누락될 경우 예외가 발생하거나 기본값 0으로 바인딩될 수 있는데, pageSize가 0일 경우 페이징 로직에서 의도치 않은 동작이 발생할 수 있습니다. @RequestParam(defaultValue = "20")과 함께 적절한 범위 검증을 추가하는 것이 안전합니다. 컨트롤러 내 다른 메서드들에도 공통적으로 적용이 필요해 보입니다.
| int pageSize | |
| @RequestParam(defaultValue = "20") int pageSize |
요약
구현 내용 사항