[TSK-56-151] 고전 독서 영역 별 인증 권수 만족 시, 인증 여부 보정 로직 구현 by haeyoon1 · Pull Request #305 · allcll/allcll-backend

haeyoon1 · 2026-03-19T09:45:45Z

작업 내용

🌁 작업 배경

🚨문제 상황: 모든 영역 별 인증 권수 <= 이수 권수 임에도 인증 여부가 false로 뜨는 문제가 있었습니다.

🚨원인 분석: 이는 대양휴머니티칼리지 페이지에서 사용자의 고전독서 인증 여부를 느리게 업데이트해주어 발생한 문제입니다.

올클의 고전 독서 여부 판정 기준은 대휴칼 페이지 파싱 + 대체과목 이수 여부 확인 을 통해 이루어지기에, 대휴칼 페이지에서 false로 보내주면, 그대로 파싱해 보여줄 수 밖에 없었습니다.

🫶 해결 방안

영역 별 필요 권수를 저장한 enum(ClassicsArea)을 기준으로 사용자의 모든 영역 별 인증 권수 <= 이수 권수 인 경우, classicsResult.passed()를 true로 반환하도록 했습니다.

이를 판정하기위해 ClassicsCounts에 isPassed 메서드를 추가했습니다.

✍️ 테스트 코드 작성

초반에는 원인 파악이 되지 않아, 파싱 로직의 오류를 판단하기 위해 GraduationClassicsCertFetcherParsingTest를 생성했습니다.
이는 GraduationClassicsCertFetcher 클래스에대한 테스트이지만, 기존 GraduationClassicsCertFetcherTest와 성격이 조금 달라보여 클래스를 분리 후 클래스 단위에 @DisplayName를 추가했습니다.

정확한 원인 파악 후 GraduationClassicsCertResolverTest 테스트로 현재 PR 작업의 정상 작동 여부를 확인하였습니다.
기존에는 GraduationCertResolver에 영어, 고전, 코딩 인증을 확인하는 private 메서드를 호출하는 방식으로 작동이 되었는데,
테스트 코드를 작성하기 어려워 GraduationCertClassicResolver & GraduationCertCodingResolver & GraduationCertResolver로 분리하였습니다

고민 지점과 리뷰 포인트

적절한 방식으로 작업되었는지 확인부탁드립니다!

github-actions · 2026-03-19T09:48:20Z

Test Results

209 tests 209 ✅ 26s ⏱️
49 suites 0 💤
49 files 0 ❌

Results for commit cf2c485.

♻️ This comment has been updated with latest results.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d7f3cf9e8f

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

...main/java/kr/allcll/backend/domain/graduation/check/cert/GraduationClassicsCertResolver.java

boyekim

Resolver 분리한 것 너무너무 좋습니다.
개별 Resolver가 Fetcher를 가지고 있고, 전반적인 Resolver는 개별 Resolver를 보고 있는 것 훨씬 가독성 좋고 역할분리 좋다고 생각합니다.
코멘트 남긴 한가지 의견 부탁드려요!!!!!
고생하셨습니다

boyekim · 2026-03-30T11:29:46Z

...main/java/kr/allcll/backend/domain/graduation/check/cert/GraduationClassicsCertResolver.java

+            if (classicsResult == null) {
+                return ClassicsResult.empty();
+            }


급 우려되는점이 생겼는데요, 요거 기존 DB에 저장된 값이 있더라도 요청이 실패하면 empty값을 반환하게 되는거 같은데 맞을까요? Exception catch가 있지만, null이 반환되는 경우가 정확히 어떤때인지 확실하게 몰라서... 일단 이런 의견을 내봅니다.
우려되는 이유는 다음과 같습니다.

외부 요청이 실패할 경우가, 학교 서버가 터진 경우도 있을 수 있고, 단순 타임아웃 문제도 있을 수 있습니다.

그럴때에 기존 DB에 저장된 값도 있고, 저희 서비스의 문제가 아닐때에도 empty 값을 반환하는게 약간 걸립니다.

결론: 이전 저장된 값이 있다면 그걸 반환해주는게 어떨까요?

그리고 프론트에게 요청이 정상적으로 안되었다는걸 알려서 업데이트가 안되었고 재시도 부탁한다는 문구를 추후 추가해도 좋겠군요.

🫢 그렇네용 꼼꼼한 리뷰 감사합니다!
이전 값을 불러오고 로그를 찍는 것으로 수정했습니다!

2Jin1031

GraduationClassicsCertResolverTest에서 엣지 상황을 다 테스트 코드로 작성해주셨네요~ 넘 좋습니다 ㅎㅎ
고생하셨어요! 해윤님~
유저 입장에서 훨씬 편해질 것 같군요!!

2Jin1031 · 2026-04-04T09:19:28Z

...main/java/kr/allcll/backend/domain/graduation/check/cert/GraduationClassicsCertResolver.java

+    }
+
+    private boolean isClassicsAlreadyPassed(GraduationCheckCertResult certResult) {
+        if (certResult == null) {


이미 앞 ClassicsCounts.fallback 로직에서 null 검증하고 오네용!! 없어도 무관할 것 같습니당
그러면 isClassicsAlreadyPassed 이 메서드로 묶지 않고 certResult 내에서 IsClassicsCertPassed 필드가 true 인지 확인하는 메서드가 내부에 있어도 되지 않을까 하는 생각이 들었는데 해윤님은 어떻게 생각하시나요

앗 중복 검증이 있었군요 😅
해당 부분은 삭제하고, 제안주신대로 Boolean.TRUE.equals(this.isClassicsCertPassed);
를 반환하는 메서드는 GraduationCheckCertResult 내부로 옮겼습니다!

2Jin1031 · 2026-04-04T09:21:47Z

...main/java/kr/allcll/backend/domain/graduation/check/cert/GraduationClassicsCertResolver.java

+    private ClassicsResult fetchClassicsResultFromExternal(OkHttpClient client, ClassicsCounts fallbackCounts) {
+        try {
+            ClassicsResult classicsResult = graduationClassicsCertFetcher.fetchClassics(client);
+            if (classicsResult == null) {


앞서 단 코멘트와 동일한 이유로 여기도 null이 없어도 될 것 같군요!

엇 여기는 외부에서 새로 크롤링한 결과에대한 Null검사라 중복되지 않는 것 같아요!

앗 다시 확인해보니 동일한 코멘트가 아니었던 것 같군요! 😅

fetchClassics 메서드의 반환값이 new ClassicsResult(classicsPass, classicsCounts)로 처리되고 있는 상황에서, 객체가 null이 될 만한 상황으로는 뭐가 있을까 하다가 잘 모르겠어서, 혹시나 하고 new로 생성한 객체가 null이 될 수 있나 직접 테스트해보니 null이 될 수 없더라고요!
제가 생각치 못한 해윤님이 생각해보신 엣지 케이스가 있다면 한번 설명해주실 수 있나요??

코드 다시 확인해보니 null이 반환될일은 없을 것 같아요!
불필요한 검증인 것 같아 삭제하도록하겠습니다👍👍

2Jin1031 · 2026-04-04T09:23:45Z

...main/java/kr/allcll/backend/domain/graduation/check/cert/GraduationClassicsCertResolver.java

+                return ClassicsResult.empty();
+            }
+            return classicsResult.withFallbackCounts(fallbackCounts);
+        } catch (Exception e) {


제가 간단히 이해하기로는
GraduationCertDocumentFetcher.fetch 에서 발생하는 에러를 잡아 로깅과 Return을 처리하기 위해 catch문을 작성했다고 생각했는데
그렇다면 Exception으로 catch 하는 것보다 잡은 에러 타입인 AllcllException이 좀 더 명확한 에러를 잡아낼 수 있지 않을까 하는 생각이 들었습니다! 혹시 다른 이유가 있으시다면 편하게 남겨주세요!!

말씀 주신 것 처럼 AllcllException으로 좁히면 에러를 명확히 처리할 수 있다는 점은 공감합니다!

다만 위 메서드가 외부 시스템 호출을 포함하고 있어서 예상치 못한 다양한 예외(네트워크, 파싱 등..)가 발생할 수 있다고 생각해, 일단 Exception으로 넓게 잡아 예외처리를 하도록했는데, 이에 대해서는 어떻게 생각하시나요?!

GraduationCertDocumentFetcher.fetch 내부에서 이미 IOException을 AllcllException으로 변환해서 던지고 있는 것 같아요! 그렇다면 호출부에서는 AllcllException만 잡아야 의도된 예외 처리만을 잡을 수 있지 않을까 생각한 것 같아요!
혹시 해윤님이 생각해주신 IOException이 아니거나,, 다른 엣지 케이스도 함께 알려주시면 감사하겠습니다~~!!

GraduationCertDocumentFetcher.fetch 내부에서 이미 IOException을 AllcllException으로 변환해서 던지고 있는 것 같아요!

그렇네요 ㅎ... catch하는 에러 타입을 Exception -> AllcllException으로 수정했습니다!

2Jin1031 · 2026-04-04T10:33:48Z

src/main/java/kr/allcll/backend/domain/graduation/check/cert/GraduationEnglishCertResolver.java

+        }
+    }
+
+    private boolean isEnglishAlreadyPassed(GraduationCheckCertResult certResult) {


여기도 동일한 코멘트 입니당!!

위 코멘트와 같은 이유로, 검증이 필요한 것 같은데 확인부탁드려요 ...!🤔

가 아니라 이건 또 다른 케이스군요!

다만 GraduationCertResolver의 resolve에서

GraduationCheckCertResult certResult = graduationCheckCertResultRepository.findByUserId(user.getId()) .orElse(null); boolean englishPassed = graduationEnglishCertResolver.resolve( user, client, userDept, completedCourses, certResult ); boolean codingPassed = graduationCodingCertResolver.resolve( user, client, userDept, completedCourses, certResult );

이렇게 db의 기존 certResult를 가져오고있어 certResult에 대한 null 검증은 필요해보입니다!

그러네요..! 제가 놓쳤습니다.

처음 검사하는 사용자 판별을 위한 분기문으로 필요한 코드로 보여요! 다만 null 가드를 매번 검사해야 하는 구조는 휴먼 에러가 생길 가능성이 클 것 같아서요. (실제로 GraduationClassicsCertResolver.java:27에서 null 가드가 빠져 있기도 하고요!)

그래서 Null Object 패턴을 제안드려 봅니다. GraduationCheckCertResult.empty() 같은 정적 팩토리로 "이력 없음"을 표현하는 빈 인스턴스를 만들고, 리졸버에서는 이걸 기본값으로 넘기는 방식으로,

GraduationCheckCertResult certResult = graduationCheckCertResultRepository .findByUserId(user.getId()) .orElseGet(GraduationCheckCertResult::empty);

이렇게 하면 하위 resolver들의 null 분기문이 전부 사라지고, "상태"가 타입으로 표현될 것 같아서요. 해윤님은 어떻게 생각하시나요?

직접 적용한번 해보니까 여러 파일에서 변경이 필요해서 변경 사항이 포함된 patch 파일로 추출해봤습니다.
아래 파일을 다운받아 적용해보실 수 있습니다!

graduation-cert-null-object.patch

적용 방법

git apply graduation-cert-null-object.patch

하위 resolver들의 null 분기문이 전부 사라지고, "상태"가 타입으로 표현될 것 같아서요. 해윤님은 어떻게 생각하시나요?

매우 좋은 것 같습니다 👍 단순 null이 아닌 empty라는 상태로 분기처리를 하면 의미 전달이 명확해질 것 같아요!

직접 적용한번 해보니까 여러 파일에서 변경이 필요해서 변경 사항이 포함된 patch 파일로 추출해봤습니다.

대박..... 이런 방법이 있다니 신기하네요! 변경 범위가 꽤 크던데 감사합니다 ㅎㅎ😇

2Jin1031 · 2026-04-04T10:46:07Z

...kr/allcll/backend/domain/graduation/check/cert/GraduationClassicsCertFetcherParsingTest.java

+import org.junit.jupiter.api.Test;
+
+@DisplayName("GraduationClassicsCertFetcher 테스트")
+public class GraduationClassicsCertFetcherParsingTest {


제가 보기에는 parsePass는 GraduationClassicsCertFetcher의 private 메서드 여서 검증을 어떻게 해야 할 지를 고민해주신 것 같아요!
그러다보니 프로덕션 코드가 테스트 코드에 그대로 작성되어, 프로덕션의 변경 주기를 일치시키기 어려울 것 같다는 우려가 들었습니다!
파싱이 적절한지 판단하기 위해 다음과 같은 방법은 어떻게 생각하시나요?

GraduationHtmlParser 단위 테스트

@Test void selectClassicsPassText_인증된_경우() { Document document = Jsoup.parse(html); String[] result = parser.selectClassicsPassText(document); assertThat(result[0]).isEqualTo("2024년도 1학기 인증"); } @Test void selectClassicsPassText_아니오인_경우() { Document document = Jsoup.parse(html); String[] result = parser.selectClassicsPassText(document); assertThat(result[0]).isEqualTo("아니오"); }

GraduationClassicsCertFetcher 테스트 documentFetcher를 mock해서 인증 HTML Document를 반환하도록 한 후 테스트 하는 것이요! 이러면 비지니스 로직을 검증 할 수 있을 것 같아서요 !

제안해주신 테스트 방향이 더욱 좋은 것 같네요👍

현재는 두번째 방법에 조금 더 공감이 되는데, 조금 더 고민해봐야할 것 같네요!
다만 현재 빠른 리뷰 반영을위해, 이번 PR에서는 기존 방식 유지하고 다음 리팩토링 시점에 반영하는 걸로 가져가면 좋을 것 같습니다🥲
꼼꼼한 리뷰 감사합니다!

boyekim

제가 남긴 코멘트 부분 확인 했고, 어프룹 드리겠습니다~ 진이가 남겨준 코멘트 반영하고, 테스트 터진거 잡으면 될 것 같아요!!!! 고생하셨습니다

haeyoon1 added 7 commits March 9, 2026 16:50

test: 대휴칼 고전 졸업인증 페이지 정상 파싱 후 결과 정상 반환 여부 판정 테스트

d72466f

test: 불필요한 출력문 삭제

e173ea6

feat: 영역 별 이수 권수 기준으로 고전 독서 인증 여부 수정 로직 추가

346f20c

refactor: resolver 클래스 분리

8bab07a

feat: 영역 별 이수 권수 만족 여부 확인 메서드 작성

e789a64

test: 고전 독서 보정 로직 테스트

b9d2be6

test: displayname 추가

d7f3cf9

auto-assign bot requested review from 2Jin1031, boyekim and goldm0ng March 19, 2026 09:45

auto-assign bot assigned haeyoon1 Mar 19, 2026

chatgpt-codex-connector bot reviewed Mar 19, 2026

View reviewed changes

...main/java/kr/allcll/backend/domain/graduation/check/cert/GraduationClassicsCertResolver.java Show resolved Hide resolved

boyekim changed the title ~~[Tsk-56-151] 고전 독서 영역 별 인증 권수 만족 시, 인증 여부 보정 로직 구현~~ [TSK-56-151] 고전 독서 영역 별 인증 권수 만족 시, 인증 여부 보정 로직 구현 Mar 30, 2026

boyekim requested changes Mar 30, 2026

View reviewed changes

2Jin1031 reviewed Apr 4, 2026

View reviewed changes

haeyoon1 added 2 commits April 5, 2026 15:21

feat: 고전 크롤링 실패 시 fallback 적용

1f11810

feat: 중복 null 검증 제거 및 로직 위치 수정

86348d7

boyekim approved these changes Apr 5, 2026

View reviewed changes

haeyoon1 added 4 commits April 7, 2026 15:24

feat: 에러 타입 수정

de12f25

feat: GraduationCheckCertResult.empty() 객체 생성

a364f9d

test: empty 객체 생성으로 인한 테스트코드 수정

a9f47c9

test: 테스트 오류 수정 및 조건문 위치 수정

cf2c485

haeyoon1 merged commit fb1ed29 into main Apr 7, 2026
3 checks passed

haeyoon1 deleted the TSK-56-151 branch April 7, 2026 12:14

Conversation

haeyoon1 commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

작업 내용

🌁 작업 배경

🫶 해결 방안

✍️ 테스트 코드 작성

고민 지점과 리뷰 포인트

Uh oh!

github-actions bot commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

boyekim left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

2Jin1031 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haeyoon1 Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haeyoon1 Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boyekim left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

haeyoon1 commented Mar 19, 2026 •

edited

Loading

github-actions bot commented Mar 19, 2026 •

edited

Loading

haeyoon1 Apr 5, 2026 •

edited

Loading

haeyoon1 Apr 7, 2026 •

edited

Loading