Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
package kr.allcll.backend.domain.graduation.check.cert;

import java.util.List;
import kr.allcll.backend.domain.graduation.certification.ClassicAltCoursePolicy;
import kr.allcll.backend.domain.graduation.certification.GraduationCertificationAltCoursePolicy;
import kr.allcll.backend.domain.graduation.check.cert.dto.ClassicsCounts;
import kr.allcll.backend.domain.graduation.check.cert.dto.ClassicsResult;
import kr.allcll.backend.domain.graduation.check.cert.dto.GraduationCertInfo;
import kr.allcll.backend.domain.graduation.check.excel.CompletedCourse;
Expand All @@ -24,14 +21,11 @@
public class GraduationCertResolver {

private final CompletedCourseRepository completedCourseRepository;
private final GraduationCodingCertFetcher graduationCodingCertFetcher;
private final GraduationEnglishCertFetcher graduationEnglishCertFetcher;
private final GraduationClassicsCertFetcher graduationClassicsCertFetcher;
private final GraduationCertificationAltCoursePolicy codingAltCoursePolicy;
private final GraduationCertificationAltCoursePolicy englishAltCoursePolicy;
private final ClassicAltCoursePolicy classicAltCoursePolicy;
private final GraduationDepartmentInfoRepository graduationDepartmentInfoRepository;
private final GraduationCheckCertResultRepository graduationCheckCertResultRepository;
private final GraduationCodingCertResolver graduationCodingCertResolver;
private final GraduationEnglishCertResolver graduationEnglishCertResolver;
private final GraduationClassicsCertResolver graduationClassicsCertResolver;

public GraduationCertInfo resolve(User user, OkHttpClient client) {
GraduationDepartmentInfo userDept = graduationDepartmentInfoRepository
Expand All @@ -43,9 +37,21 @@ public GraduationCertInfo resolve(User user, OkHttpClient client) {
GraduationCheckCertResult certResult = graduationCheckCertResultRepository.findByUserId(user.getId())
.orElse(null);

boolean englishPassed = resolveEnglishPassed(user, client, userDept, completedCourses, certResult);
boolean codingPassed = resolveCodingPassed(user, client, userDept, completedCourses, certResult);
ClassicsResult classicsResult = resolveClassics(user, client, certResult);
boolean englishPassed = graduationEnglishCertResolver.resolve(
user,
client,
userDept,
completedCourses,
certResult
);
boolean codingPassed = graduationCodingCertResolver.resolve(
user,
client,
userDept,
completedCourses,
certResult
);
ClassicsResult classicsResult = graduationClassicsCertResolver.resolve(user, client, certResult);

return GraduationCertInfo.of(
englishPassed,
Expand All @@ -54,103 +60,4 @@ public GraduationCertInfo resolve(User user, OkHttpClient client) {
classicsResult.counts()
);
}

private boolean resolveEnglishPassed(
User user,
OkHttpClient client,
GraduationDepartmentInfo userDept,
List<CompletedCourse> completedCourses,
GraduationCheckCertResult certResult
) {
if (isEnglishAlreadyPassed(certResult)) {
return true;
}

if (englishAltCoursePolicy.isSatisfiedByAltCourse(user, userDept, completedCourses)) {
return true;
}

try {
return graduationEnglishCertFetcher.fetchEnglishPass(client);
} catch (Exception e) {
log.error("[졸업요건검사] 영어인증 여부를 불러오지 못했습니다.", e);
return false;
}
}

private boolean resolveCodingPassed(
User user,
OkHttpClient client,
GraduationDepartmentInfo userDept,
List<CompletedCourse> completedCourses,
GraduationCheckCertResult certResult
) {
if (isCodingAlreadyPassed(certResult)) {
return true;
}

if (codingAltCoursePolicy.isSatisfiedByAltCourse(user, userDept, completedCourses)) {
return true;
}

try {
return graduationCodingCertFetcher.fetchCodingPass(client);
} catch (Exception e) {
log.error("[졸업요건검사] 코딩인증 정보를 불러오지 못했습니다.", e);
return false;
}
}

private ClassicsResult resolveClassics(
User user,
OkHttpClient client,
GraduationCheckCertResult certResult
) {
ClassicsCounts fallbackCounts = ClassicsCounts.fallback(certResult);

if (isClassicsAlreadyPassed(certResult)) {
return ClassicsResult.passedWith(fallbackCounts);
}

ClassicsResult classicsResult = fetchClassicsResultFromExternal(client, fallbackCounts);
if (classicsResult.passed()) {
return classicsResult;
}
boolean satisfiedByAltCourse = classicAltCoursePolicy.isSatisfiedByAltCourse(user);
return classicsResult.passedWith(satisfiedByAltCourse, classicsResult.counts());
}

private ClassicsResult fetchClassicsResultFromExternal(OkHttpClient client, ClassicsCounts fallbackCounts) {
try {
ClassicsResult classicsResult = graduationClassicsCertFetcher.fetchClassics(client);
if (classicsResult == null) {
return ClassicsResult.empty();
}
return classicsResult.withFallbackCounts(fallbackCounts);
} catch (Exception e) {
log.error("[졸업요건검사] 고전인증 여부를 불러오지 못했습니다.", e);
return ClassicsResult.failedWith(fallbackCounts);
}
}

private boolean isEnglishAlreadyPassed(GraduationCheckCertResult certResult) {
if (certResult == null) {
return false;
}
return Boolean.TRUE.equals(certResult.getIsEnglishCertPassed());
}

private boolean isCodingAlreadyPassed(GraduationCheckCertResult certResult) {
if (certResult == null) {
return false;
}
return Boolean.TRUE.equals(certResult.getIsCodingCertPassed());
}

private boolean isClassicsAlreadyPassed(GraduationCheckCertResult certResult) {
if (certResult == null) {
return false;
}
return Boolean.TRUE.equals(certResult.getIsClassicsCertPassed());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -205,4 +205,8 @@ public void reCalculate() {
this.requiredPassCount = this.graduationCertRuleType.getRequiredPassCount();
this.isSatisfied = this.graduationCertRuleType.isSatisfied(newPassedCount);
}

public boolean isClassicsPassed() {
return Boolean.TRUE.equals(this.isClassicsCertPassed);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package kr.allcll.backend.domain.graduation.check.cert;

import kr.allcll.backend.domain.graduation.certification.ClassicAltCoursePolicy;
import kr.allcll.backend.domain.graduation.check.cert.dto.ClassicsCounts;
import kr.allcll.backend.domain.graduation.check.cert.dto.ClassicsResult;
import kr.allcll.backend.domain.user.User;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import okhttp3.OkHttpClient;
import org.springframework.stereotype.Service;

@Slf4j
@Service
@RequiredArgsConstructor
public class GraduationClassicsCertResolver {

private final GraduationClassicsCertFetcher graduationClassicsCertFetcher;
private final ClassicAltCoursePolicy classicAltCoursePolicy;

public ClassicsResult resolve(
User user,
OkHttpClient client,
GraduationCheckCertResult certResult
) {
ClassicsCounts fallbackCounts = ClassicsCounts.fallback(certResult);

if (certResult.isClassicsPassed()) {
return ClassicsResult.passedWith(fallbackCounts);
}

ClassicsResult classicsResult = fetchClassicsResultFromExternal(client, fallbackCounts);
boolean isSatisfiedByCrawledResult = classicsResult.isSatisfiedByCrawledResult();
boolean satisfiedByAltCourse = classicAltCoursePolicy.isSatisfiedByAltCourse(user);
if (isSatisfiedByCrawledResult || satisfiedByAltCourse) {
return ClassicsResult.passedWith(classicsResult.counts());
Comment thread
2Jin1031 marked this conversation as resolved.
}

return ClassicsResult.failedWith(classicsResult.counts());
}

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

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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

log.warn("[졸업요건검사] 고전인증 결과가 null입니다. fallback 값 사용");
return ClassicsResult.failedWith(fallbackCounts);
}
return classicsResult.withFallbackCounts(fallbackCounts);
} catch (Exception e) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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

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

log.error("[졸업요건검사] 고전인증 여부를 불러오지 못했습니다.", e);
return ClassicsResult.failedWith(fallbackCounts);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package kr.allcll.backend.domain.graduation.check.cert;

import java.util.List;
import kr.allcll.backend.domain.graduation.certification.GraduationCertificationAltCoursePolicy;
import kr.allcll.backend.domain.graduation.check.excel.CompletedCourse;
import kr.allcll.backend.domain.graduation.department.GraduationDepartmentInfo;
import kr.allcll.backend.domain.user.User;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import okhttp3.OkHttpClient;
import org.springframework.stereotype.Service;

@Slf4j
@Service
@RequiredArgsConstructor
public class GraduationCodingCertResolver {

private final GraduationCodingCertFetcher graduationCodingCertFetcher;
private final GraduationCertificationAltCoursePolicy codingAltCoursePolicy;

public boolean resolve(
User user,
OkHttpClient client,
GraduationDepartmentInfo userDept,
List<CompletedCourse> completedCourses,
GraduationCheckCertResult certResult
) {
if (isCodingAlreadyPassed(certResult)) {
return true;
}

if (codingAltCoursePolicy.isSatisfiedByAltCourse(user, userDept, completedCourses)) {
return true;
}

try {
return graduationCodingCertFetcher.fetchCodingPass(client);
} catch (Exception e) {
log.error("[졸업요건검사] 코딩인증 정보를 불러오지 못했습니다.", e);
return false;
}
}

private boolean isCodingAlreadyPassed(GraduationCheckCertResult certResult) {
if (certResult == null) {
return false;
}
return Boolean.TRUE.equals(certResult.getIsCodingCertPassed());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package kr.allcll.backend.domain.graduation.check.cert;

import kr.allcll.backend.domain.graduation.certification.GraduationCertificationAltCoursePolicy;
import kr.allcll.backend.domain.graduation.check.excel.CompletedCourse;
import kr.allcll.backend.domain.graduation.department.GraduationDepartmentInfo;
import kr.allcll.backend.domain.user.User;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import okhttp3.OkHttpClient;
import org.springframework.stereotype.Service;

import java.util.List;

@Slf4j
@Service
@RequiredArgsConstructor
public class GraduationEnglishCertResolver {

private final GraduationEnglishCertFetcher graduationEnglishCertFetcher;
private final GraduationCertificationAltCoursePolicy englishAltCoursePolicy;

public boolean resolve(
User user,
OkHttpClient client,
GraduationDepartmentInfo userDept,
List<CompletedCourse> completedCourses,
GraduationCheckCertResult certResult
) {
if (isEnglishAlreadyPassed(certResult)) {
return true;
}

if (englishAltCoursePolicy.isSatisfiedByAltCourse(user, userDept, completedCourses)) {
return true;
}

try {
return graduationEnglishCertFetcher.fetchEnglishPass(client);
} catch (Exception e) {
log.error("[졸업요건검사] 영어인증 여부를 불러오지 못했습니다.", e);
return false;
}
}

private boolean isEnglishAlreadyPassed(GraduationCheckCertResult certResult) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member Author

@haeyoon1 haeyoon1 Apr 5, 2026

Choose a reason for hiding this comment

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

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

다만 GraduationCertResolverresolve에서

        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 검증은 필요해보입니다!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

처음 검사하는 사용자 판별을 위한 분기문으로 필요한 코드로 보여요! 다만 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

Copy link
Copy Markdown
Member Author

@haeyoon1 haeyoon1 Apr 7, 2026

Choose a reason for hiding this comment

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

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

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

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

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

if (certResult == null) {
return false;
}
return Boolean.TRUE.equals(certResult.getIsEnglishCertPassed());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,11 @@ public static ClassicsCounts fallback(GraduationCheckCertResult certResult) {
public static ClassicsCounts empty() {
return new ClassicsCounts(0, 0, 0, 0);
}

public boolean isPassed() {
return myCountWestern >= ClassicsArea.WESTERN.getMaxRecognizedCount() &&
myCountEastern >= ClassicsArea.EASTERN.getMaxRecognizedCount() &&
myCountEasternAndWestern >= ClassicsArea.EASTERN_AND_WESTERN.getMaxRecognizedCount() &&
myCountScience >= ClassicsArea.SCIENCE.getMaxRecognizedCount();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,6 @@ public static ClassicsResult passedWith(ClassicsCounts fallbackCounts) {
return new ClassicsResult(true, fallbackCounts);
}

public ClassicsResult passedWith(boolean passed, ClassicsCounts fallbackCounts) {
return new ClassicsResult(passed, fallbackCounts);
}

public static ClassicsResult failedWith(ClassicsCounts fallbackCounts) {
return new ClassicsResult(false, fallbackCounts);
}
Expand All @@ -27,4 +23,8 @@ public ClassicsResult withFallbackCounts(ClassicsCounts fallbackCounts) {
}
return this;
}

public boolean isSatisfiedByCrawledResult() {
return passed || counts.isPassed();
}
}
Loading
Loading