Repository navigation
[Feat] 셔틀 시간표 배치 수집 기반 구현 - #87
Merged
Merged
Conversation
* 지정된 URL에서 셔틀 시간표 HTML을 가져오는 ShuttleClient 추가 * 셔틀 데이터 수집과 파싱 흐름을 담당하는 ShuttleJob 구현 * 셔틀 시간표 데이터 파싱을 위한 ShuttleParser 기본 구조 추가 * 파싱된 셔틀 데이터를 저장하는 ShuttleRepository 추가 * ShuttleClient의 정상적인 HTML 요청 동작을 검증하는 단위 테스트 추가 * 비동기 작업 지연을 위한 delay 유틸 함수 추가 * Jest 테스트 타입을 사용할 수 있도록 TypeScript 설정 업데이트
* 데이터베이스 연결 설정을 위한 DatabaseModule 추가 * 셔틀 시간표 저장을 위한 ShuttleTimetable 엔티티 생성 * 기존 셔틀 인터페이스 제거 후 파싱된 시간표 타입으로 대체 * HTML 셔틀 시간표를 구조화된 데이터로 변환하는 ShuttleParser 구현 * ShuttleParser의 시간표 파싱 동작을 검증하는 단위 테스트 추가 * ShuttleRepository가 새로운 ShuttleTimetable 엔티티를 사용하도록 수정
Contributor
|
Warning Review limit reached
More reviews will be available in 10 minutes and 38 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
Walkthrough
Changes배치 서비스 초기 구현
Sequence Diagram(s)sequenceDiagram
participant Scheduler as Scheduler (Cron / Bootstrap)
participant BatchService
participant ShuttleJob
participant ShuttleClient
participant FetchHttpClient
participant ShuttleParser
Scheduler->>BatchService: run() 호출
loop BATCH_JOBS 순차 실행
BatchService->>ShuttleJob: job.run()
ShuttleJob->>ShuttleClient: fetch()
ShuttleClient->>FetchHttpClient: getText(SHUTTLE_URL)
FetchHttpClient-->>ShuttleClient: HTML string (재시도 포함)
ShuttleClient-->>ShuttleJob: HTML string
ShuttleJob->>ShuttleParser: parse(raw)
ShuttleParser-->>ShuttleJob: ParsedShuttleTimetablePage
ShuttleJob-->>BatchService: 완료 (repository.save 주석 처리됨)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/api/batch/README.md (1)
1-99:⚠️ Potential issue | 🟠 Major | ⚡ Quick winREADME는 NestJS 보일러플레이트이며, 배치 서비스의 실제 기능을 설명하지 않음.
현재 README는 일반적인 NestJS 프로젝트 템플릿 복사본입니다. PR 목표에 따르면 이 배치 서비스는 다음을 구현합니다:
- 셔틀 시간표 수집 및 파싱 파이프라인
- 배치 잡 스케줄러 (cron 기반)
- 도메인별 배치 작업 분리 (Shuttle, Cafeteria, Notice, Academic Calendar)
- HTTP 재시도 클라이언트
README는 이러한 배치 서비스의 목적, 아키텍처, 로컬 개발 설정을 전혀 설명하지 않습니다. 또한 markdownlint 경고도 있습니다:
- Lines 5-6: 사용되지 않은 참조 정의 제거 필요 (circleci-image, circleci-url)
- Lines 31, 67-68: 명령 뒤에 출력 또는 컨텍스트 필요
배치 서비스에 맞는 README를 생성해드릴 수 있습니다. 다음 내용을 포함하는 README를 작성하시겠습니까?
- 배치 서비스 목적 및 역할
- 프로젝트 구조 (HTTP 클라이언트, 파서, 도메인별 잡)
- 로컬 개발 및 실행 방법
- 스케줄링 및 작업 흐름
- 테스트 실행 방법
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/api/batch/README.md` around lines 1 - 99, Replace the generic NestJS boilerplate README with documentation specific to the batch service. Remove the unused reference definitions (circleci-image and circleci-url) and add context or expected output after the command examples to fix markdownlint warnings. The new README should document the batch service's actual purpose (shuttle timetable collection and parsing, cron-based job scheduling, domain-separated batch jobs for Shuttle, Cafeteria, Notice, and Academic Calendar, and HTTP retry client), explain the project structure and architecture, provide local development and execution setup instructions, describe the scheduling and job workflow, and include test execution guidance.
🧹 Nitpick comments (2)
services/api/batch/src/batch/http/fetch-http.client.spec.ts (1)
4-79: ⚡ Quick win타임아웃 재시도 경로 테스트를 추가해 회귀를 막아주세요.
PR 목표에 timeout 처리/재시도가 포함되어 있는데, 현재 스펙은 해당 경로를 직접 검증하지 않습니다. timeout 발생 후 재시도 성공(또는 최종 실패) 케이스 1개는 추가하는 편이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/api/batch/src/batch/http/fetch-http.client.spec.ts` around lines 4 - 79, Add a new test case to the FetchHttpClient describe block that verifies timeout retry behavior. Create a test that mocks the fetch function to throw a timeout error (or abort error) on the first call and then resolves successfully on retry, similar to the existing network error test. This test should validate that the client properly retries when a timeout occurs and successfully returns the response. Alternatively, you could add a test verifying that the client exhausts retries and throws an error when timeouts persist across all retry attempts.services/api/batch/src/batch/jobs/shuttle/shuttle.client.spec.ts (1)
11-13: ⚡ Quick win요청 URL 인자 검증을 추가하세요.
현재는 호출 횟수만 확인해서, 잘못된 URL로 바뀌어도 테스트가 통과합니다.
getText호출 인자까지 검증해 회귀를 막는 편이 안전합니다.🔧 제안 diff
await expect(client.fetch()).resolves.toBe('<html></html>'); expect(httpClient.getText).toHaveBeenCalledTimes(1); + expect(httpClient.getText).toHaveBeenCalledWith( + 'https://www.gnu.ac.kr/main/cm/cntnts/cntntsView.do?mi=1358&cntntsId=1194', + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/api/batch/src/batch/jobs/shuttle/shuttle.client.spec.ts` around lines 11 - 13, The test for the shuttle client only verifies that httpClient.getText is called once, but does not validate what URL argument is passed to it. This means the test would pass even if the URL argument changes incorrectly. Add argument validation to the mock expectation by using toHaveBeenCalledWith() in addition to toHaveBeenCalledTimes(1), specifying the expected URL that should be passed to the getText method to prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/api/batch/eslint.config.mjs`:
- Around line 1-35: The batch service path needs to be added to the lint-staged
configuration to ensure its TypeScript and JavaScript files are automatically
linted. Update lint-staged.config.js to include a new pattern entry for the
batch service path that calls projectCommands with the services/api/batch
directory and the matched files, following the same pattern used for other
service paths in the configuration.
In `@services/api/batch/src/batch/batch.service.ts`:
- Around line 14-16: The for loop iterating through this.jobs in the batch
service does not handle exceptions from individual job.run() calls, causing a
single failed job to halt the entire batch execution and skip subsequent jobs.
Wrap the await job.run() call inside a try-catch block to isolate failures at
the job level, log the error appropriately, and allow the loop to continue
processing remaining jobs in the this.jobs collection. This ensures fault
isolation within the batch structure and allows jobs to execute independently.
- Around line 12-20: The run() method scheduled by `@Cron` decorator and called
from onApplicationBootstrap() lacks a re-entrance guard, allowing concurrent
execution of the same jobs if a scheduled run takes longer than the cron
interval. Add a boolean flag (e.g., isRunning) to track whether a run is
currently in progress. At the start of the run() method, check if isRunning is
true and return early if so; otherwise, set isRunning to true, execute all jobs,
and finally set isRunning to false in a try-finally block to ensure the flag is
reset even if an error occurs.
In `@services/api/batch/src/batch/http/fetch-http.client.ts`:
- Around line 93-99: The getRetryOptions method lacks validation for the retry
configuration values, which can cause unstable behavior when invalid values like
negative retries or non-positive timeoutMs are passed. Add validation logic in
the getRetryOptions method to ensure retries is non-negative, timeoutMs is
positive (greater than zero), retryDelayMs is non-negative, and retryOnStatuses
is valid. Either throw an error for invalid inputs or enforce minimum/default
values to guarantee the returned configuration is always safe for use.
- Around line 50-55: The retry logic in the catch block does not distinguish
between abort errors (caused by the caller's signal being aborted) and other
retriable errors, which allows abort signals to be retried instead of
terminating immediately. Add a check in the catch block to detect if the error
is an AbortError or signal cancellation error (before calling shouldRetryError),
and if so, throw it immediately without retrying, bypassing the retry logic
regardless of the attempt count. This ensures that when a caller aborts the
signal, the operation terminates instantly without waiting for retries.
In
`@services/api/batch/src/batch/jobs/shuttle/domain/shuttle-timetable.entity.ts`:
- Around line 20-21: The updatedAt field in the ShuttleTimetableEntity is
decorated with `@UpdateDateColumn` which automatically sets it to the current save
time on every entity save, causing the loss of the original source update time
extracted by the shuttle parser. Change the `@UpdateDateColumn` decorator to
`@Column` to prevent automatic updates, and ensure that when the entity is created
or saved in shuttle.parser.ts or related save operations, the actual update time
extracted from the source is explicitly assigned to the updatedAt field. This
way the original page-based update time will be preserved instead of being
overwritten.
In `@services/api/batch/src/batch/jobs/shuttle/shuttle.job.ts`:
- Around line 17-22: The batch job's run() method is incomplete and does not
persist fetched shuttle data to the database. In the run() method of
shuttle.job.ts, uncomment and enable the this.repository.save() call that is
currently commented out, and ensure the parsed data from this.parser.parse() is
properly passed to the save method (check that the variable name matches what
the repository.save() method expects, likely shuttles). Additionally, implement
the save() method in shuttle.repository.ts with actual database persistence
logic (upsert or insert logic) rather than leaving it as a no-op, so that the
parsed shuttle schedule data is actually written to the database when the batch
job runs.
In `@services/api/batch/src/batch/jobs/shuttle/shuttle.parser.spec.ts`:
- Around line 39-42: The test case description in the shuttle parser spec
indicates it should test the scenario where "최근 업데이트 일시가 없으면" (no recent update
date/time exists), but the expected error message is about a missing timetable
table instead. Fix this mismatch by either providing test input HTML that
contains a timetable structure but lacks the update date/time information, and
then updating the expected error message to match the actual error thrown when
update date/time is missing; or alternatively, rename the test case description
to accurately reflect that it tests for a missing timetable scenario when
parsing empty HTML.
In `@services/api/batch/src/batch/jobs/shuttle/shuttle.parser.ts`:
- Around line 17-23: Introduce a custom ShuttleParseError exception class with
specific error codes (MISSING_UPDATED_AT and MISSING_TIMETABLE) to replace
generic Error strings and enable proper failure cause classification. In the
parse() method, reorganize validation checks to validate updatedAt extraction
before timetables validation, then throw the appropriate ShuttleParseError with
the correct code based on which validation fails. Update the timetable length
check around line 17-23 to throw ShuttleParseError with MISSING_TIMETABLE code,
and update the updatedAt validation around lines 109-116 to throw
ShuttleParseError with MISSING_UPDATED_AT code instead of generic Error
messages.
In `@services/api/batch/src/main.ts`:
- Line 8: The bootstrap() function call on line 8 does not explicitly handle
Promise rejections, which means initialization failures are silently ignored
unless caught by a global unhandled rejection handler. Add explicit error
handling by chaining a .catch() method to the bootstrap() call that logs the
error using an appropriate logger and then calls process.exit() with a non-zero
exit code to properly signal initialization failure to the system.
---
Outside diff comments:
In `@services/api/batch/README.md`:
- Around line 1-99: Replace the generic NestJS boilerplate README with
documentation specific to the batch service. Remove the unused reference
definitions (circleci-image and circleci-url) and add context or expected output
after the command examples to fix markdownlint warnings. The new README should
document the batch service's actual purpose (shuttle timetable collection and
parsing, cron-based job scheduling, domain-separated batch jobs for Shuttle,
Cafeteria, Notice, and Academic Calendar, and HTTP retry client), explain the
project structure and architecture, provide local development and execution
setup instructions, describe the scheduling and job workflow, and include test
execution guidance.
---
Nitpick comments:
In `@services/api/batch/src/batch/http/fetch-http.client.spec.ts`:
- Around line 4-79: Add a new test case to the FetchHttpClient describe block
that verifies timeout retry behavior. Create a test that mocks the fetch
function to throw a timeout error (or abort error) on the first call and then
resolves successfully on retry, similar to the existing network error test. This
test should validate that the client properly retries when a timeout occurs and
successfully returns the response. Alternatively, you could add a test verifying
that the client exhausts retries and throws an error when timeouts persist
across all retry attempts.
In `@services/api/batch/src/batch/jobs/shuttle/shuttle.client.spec.ts`:
- Around line 11-13: The test for the shuttle client only verifies that
httpClient.getText is called once, but does not validate what URL argument is
passed to it. This means the test would pass even if the URL argument changes
incorrectly. Add argument validation to the mock expectation by using
toHaveBeenCalledWith() in addition to toHaveBeenCalledTimes(1), specifying the
expected URL that should be passed to the getText method to prevent regressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bf3064b0-595b-4496-86b4-09695ba33259
⛔ Files ignored due to path filters (1)
services/api/batch/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
services/api/batch/.gitignoreservices/api/batch/.prettierrcservices/api/batch/README.mdservices/api/batch/eslint.config.mjsservices/api/batch/nest-cli.jsonservices/api/batch/package.jsonservices/api/batch/src/batch/batch.module.tsservices/api/batch/src/batch/batch.service.tsservices/api/batch/src/batch/database/database.module.tsservices/api/batch/src/batch/http/error/http-request.error.tsservices/api/batch/src/batch/http/fetch-http.client.spec.tsservices/api/batch/src/batch/http/fetch-http.client.tsservices/api/batch/src/batch/http/http-client.interface.tsservices/api/batch/src/batch/jobs/academic-calendar/academic-calendar.job.tsservices/api/batch/src/batch/jobs/batch-job.interface.tsservices/api/batch/src/batch/jobs/cafeteria/cafeteria.job.tsservices/api/batch/src/batch/jobs/notice/notice.job.tsservices/api/batch/src/batch/jobs/shuttle/__fixtures__/shuttle-timetable.htmlservices/api/batch/src/batch/jobs/shuttle/domain/shuttle-timetable.entity.tsservices/api/batch/src/batch/jobs/shuttle/shuttle.client.spec.tsservices/api/batch/src/batch/jobs/shuttle/shuttle.client.tsservices/api/batch/src/batch/jobs/shuttle/shuttle.job.tsservices/api/batch/src/batch/jobs/shuttle/shuttle.parser.spec.tsservices/api/batch/src/batch/jobs/shuttle/shuttle.parser.tsservices/api/batch/src/batch/jobs/shuttle/shuttle.repository.tsservices/api/batch/src/batch/jobs/shuttle/type/parsed-shuttle-timetable.tsservices/api/batch/src/batch/utils/delay.tsservices/api/batch/src/main.tsservices/api/batch/tsconfig.build.jsonservices/api/batch/tsconfig.json
Comment on lines
+17
to
+23
| if (timetables.length !== 2) { | ||
| throw new Error('셔틀 시간표 테이블을 찾을 수 없습니다.'); | ||
| } | ||
|
|
||
| return { | ||
| updatedAt: this.extractUpdatedAt(raw), | ||
| timetables, |
Contributor
There was a problem hiding this comment.
파싱 실패 원인이 구분되지 않아 장애 분류가 깨집니다.
현재 구현은 Error 문자열에 의존하고, 검사 순서상 업데이트 일시 누락도 시간표 누락으로 처리될 수 있습니다. 이 상태로는 “누락 원인 구분(코드 기반)” 요구를 충족하기 어렵습니다.
ShuttleParseError(예: MISSING_UPDATED_AT, MISSING_TIMETABLE)를 도입하고, parse()에서 원인별로 명확히 분기해 던지도록 정리해 주세요.
🔧 제안 diff
+export class ShuttleParseError extends Error {
+ constructor(
+ public readonly code: 'MISSING_UPDATED_AT' | 'MISSING_TIMETABLE',
+ message: string,
+ ) {
+ super(message);
+ }
+}
parse(raw: string): ParsedShuttleTimetablePage {
+ const updatedAt = this.extractUpdatedAt(raw);
const timetables = this.extractTables(raw)
.map((table) => this.parseTable(table))
.filter(
(timetable): timetable is ParsedShuttleTimetable => timetable !== null,
);
if (timetables.length !== 2) {
- throw new Error('셔틀 시간표 테이블을 찾을 수 없습니다.');
+ throw new ShuttleParseError(
+ 'MISSING_TIMETABLE',
+ '셔틀 시간표 테이블을 찾을 수 없습니다.',
+ );
}
return {
- updatedAt: this.extractUpdatedAt(raw),
+ updatedAt,
timetables,
};
}
private extractUpdatedAt(raw: string): Date {
@@
if (!updatedAtMatch) {
- throw new Error('셔틀 시간표 최근 업데이트 일시를 찾을 수 없습니다.');
+ throw new ShuttleParseError(
+ 'MISSING_UPDATED_AT',
+ '셔틀 시간표 최근 업데이트 일시를 찾을 수 없습니다.',
+ );
}Also applies to: 109-116
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/api/batch/src/batch/jobs/shuttle/shuttle.parser.ts` around lines 17
- 23, Introduce a custom ShuttleParseError exception class with specific error
codes (MISSING_UPDATED_AT and MISSING_TIMETABLE) to replace generic Error
strings and enable proper failure cause classification. In the parse() method,
reorganize validation checks to validate updatedAt extraction before timetables
validation, then throw the appropriate ShuttleParseError with the correct code
based on which validation fails. Update the timetable length check around line
17-23 to throw ShuttleParseError with MISSING_TIMETABLE code, and update the
updatedAt validation around lines 109-116 to throw ShuttleParseError with
MISSING_UPDATED_AT code instead of generic Error messages.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 개요
fetch → parse → repository단계로 나눴습니다.✨ 작업 내용
batch앱을 추가하고 스케줄러가 등록된 Batch Job들을 순차 실행하도록 구성했습니다.FetchHttpClient를 추가해 네트워크 오류, timeout, 429, 5xx 응답에 대한 retry 정책을 제공했습니다.ShuttleClient가 교내 셔틀버스 페이지 HTML을 공통 HTTP client로 가져오도록 구현했습니다.ShuttleParser가 fixture HTML에서 2개 노선의 오전/오후 시간표와 최근 업데이트 일시를 파싱하도록 구현했습니다.ParsedShuttleTimetablePage와ParsedShuttleTimetable로 모델링했습니다.shuttle_timetable저장 흐름을 이어갈 수 있게 했습니다.🔥 변경 이유
🧪 테스트
services/api/batch에서pnpm test -- --runInBandservices/api/batch에서pnpm build🔗 관련 이슈
Summary by CodeRabbit
새 기능