-
Notifications
You must be signed in to change notification settings - Fork 3
refactor : 예외 처리 강화 및 클래스 분리 #106
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,15 @@ | ||
| package org.ezcode.codetest.application.submission.model; | ||
|
|
||
| import org.ezcode.codetest.application.submission.dto.event.payload.SubmissionFinalResultPayload; | ||
| import org.ezcode.codetest.domain.language.model.entity.Language; | ||
| import org.ezcode.codetest.domain.problem.model.ProblemInfo; | ||
| import org.ezcode.codetest.domain.problem.model.entity.Problem; | ||
| import org.ezcode.codetest.domain.problem.model.entity.Testcase; | ||
| import org.ezcode.codetest.domain.submission.model.SubmissionAggregator; | ||
| import org.ezcode.codetest.domain.user.model.entity.User; | ||
| import org.ezcode.codetest.infrastructure.event.dto.submission.SubmissionMessage; | ||
|
|
||
| import java.util.List; | ||
| import java.util.concurrent.CountDownLatch; | ||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
| import java.util.concurrent.atomic.AtomicInteger; | ||
|
|
@@ -20,48 +27,102 @@ public record SubmissionContext( | |
|
|
||
| CountDownLatch latch, | ||
|
|
||
| AtomicBoolean notified | ||
| AtomicBoolean notified, | ||
|
|
||
| User user, | ||
|
|
||
| Language language, | ||
|
|
||
| ProblemInfo problemInfo, | ||
|
|
||
| SubmissionMessage msg | ||
| ) { | ||
| public static SubmissionContext initialize(int totalTestcaseCount) { | ||
| public static SubmissionContext initialize( | ||
| User user, Language language, ProblemInfo problemInfo, SubmissionMessage msg | ||
| ) { | ||
| return new SubmissionContext( | ||
| new SubmissionAggregator(), | ||
| new AtomicInteger(0), | ||
| new AtomicInteger(0), | ||
| new AtomicReference<>("Accepted"), | ||
| new CountDownLatch(totalTestcaseCount), | ||
| new AtomicBoolean(false) | ||
| new CountDownLatch(problemInfo.getTestcaseCount()), | ||
| new AtomicBoolean(false), | ||
| user, | ||
| language, | ||
| problemInfo, | ||
| msg | ||
| ); | ||
| } | ||
|
|
||
| public SubmissionFinalResultPayload toFinalResult(int totalTestcaseCount) { | ||
| public SubmissionFinalResultPayload toFinalResult() { | ||
| return new SubmissionFinalResultPayload( | ||
| totalTestcaseCount, | ||
| this.getTestcaseCount(), | ||
| this.getPassedCount(), | ||
| this.getCurrentMessage() | ||
| ); | ||
| } | ||
|
|
||
| public void incrementPassedCount() { | ||
| this.passedCount.incrementAndGet(); | ||
| passedCount.incrementAndGet(); | ||
| } | ||
|
|
||
| public void incrementProcessedCount() { | ||
| this.processedCount.incrementAndGet(); | ||
| processedCount.incrementAndGet(); | ||
| } | ||
|
|
||
| public int getPassedCount() { | ||
| return this.passedCount.get(); | ||
| return passedCount.get(); | ||
| } | ||
|
|
||
| public String getCurrentMessage() { | ||
| return this.message.get(); | ||
| return message.get(); | ||
| } | ||
|
|
||
| public void updateMessage(String message) { | ||
| this.message.set(message); | ||
| } | ||
|
|
||
| public void countDown() { | ||
| this.latch.countDown(); | ||
| latch.countDown(); | ||
| } | ||
|
|
||
| public List<Testcase> getTestcases() { | ||
| return problemInfo.testcaseList(); | ||
| } | ||
|
|
||
| public int getTestcaseCount() { | ||
| return problemInfo.getTestcaseCount(); | ||
| } | ||
|
|
||
| public String getSourceCode() { | ||
| return msg.sourceCode(); | ||
| } | ||
|
|
||
| public long getJudge0Id() { | ||
| return language.getJudge0Id(); | ||
| } | ||
|
|
||
| public String getInput(int seqId) { | ||
| return getTestcases().get(seqId - 1).getInput(); | ||
| } | ||
|
|
||
| public String getExpectedOutput(int seqId) { | ||
| return getTestcases().get(seqId - 1).getOutput(); | ||
| } | ||
|
Comment on lines
+105
to
+111
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 배열 경계 검사를 추가하는 것이 좋겠습니다. 잘못된 다음과 같이 경계 검사를 추가하세요: public String getInput(int seqId) {
+ if (seqId < 1 || seqId > getTestcaseCount()) {
+ throw new IllegalArgumentException("Invalid seqId: " + seqId);
+ }
return getTestcases().get(seqId - 1).getInput();
}
public String getExpectedOutput(int seqId) {
+ if (seqId < 1 || seqId > getTestcaseCount()) {
+ throw new IllegalArgumentException("Invalid seqId: " + seqId);
+ }
return getTestcases().get(seqId - 1).getOutput();
}
🤖 Prompt for AI Agents
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 밖에서 스트림 돌려서 오는 거라 잘못된 값이 전달될 일은 없는데 방어 로직 추가해서 나쁠 건 없다는 말이지? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @thezz9 맞습니다! 현재 구현에서는 외부 스트림에서 순차적으로 오는 값이라 잘못된 방어 로직의 장점:
특히 현재 PR에서 코드 구조를 개선하고 예외 처리를 강화하는 작업을 하고 계시니, 이런 방어 로직도 전체적인 코드 견고성 향상에 부합하는 것 같습니다. 추가하실지는 전체적인 코드 정책과 우선순위에 따라 결정하시면 될 것 같습니다! 🧠 Learnings used
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @thezz9 알겠습니다! 필요하시면 언제든 말씀해 주세요. 현재 PR의 예외 처리 강화 작업 잘 진행되고 있는 것 같습니다. 👍 🧠 Learnings used |
||
|
|
||
| public long getTimeLimit() { | ||
| return problemInfo.getTimeLimit(); | ||
| } | ||
|
|
||
| public long getMemoryLimit() { | ||
| return problemInfo.getMemoryLimit(); | ||
| } | ||
|
|
||
| public String getSessionKey() { | ||
| return msg.sessionKey(); | ||
| } | ||
|
|
||
| public Problem getProblem() { | ||
| return problemInfo.problem(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| package org.ezcode.codetest.application.submission.service; | ||
|
|
||
| import java.util.concurrent.CompletableFuture; | ||
| import java.util.concurrent.Executor; | ||
| import java.util.concurrent.TimeUnit; | ||
| import java.util.stream.IntStream; | ||
|
|
||
| import org.ezcode.codetest.application.submission.dto.event.SubmissionErrorEvent; | ||
| import org.ezcode.codetest.application.submission.dto.event.SubmissionJudgingFinishedEvent; | ||
| import org.ezcode.codetest.application.submission.dto.event.TestcaseEvaluatedEvent; | ||
| import org.ezcode.codetest.application.submission.dto.event.TestcaseListInitializedEvent; | ||
| import org.ezcode.codetest.application.submission.dto.event.payload.InitTestcaseListPayload; | ||
| import org.ezcode.codetest.application.submission.dto.event.payload.TestcaseResultPayload; | ||
| import org.ezcode.codetest.application.submission.dto.request.compile.CodeCompileRequest; | ||
| import org.ezcode.codetest.application.submission.model.JudgeResult; | ||
| import org.ezcode.codetest.application.submission.model.SubmissionContext; | ||
| import org.ezcode.codetest.application.submission.port.ExceptionNotifier; | ||
| import org.ezcode.codetest.application.submission.port.JudgeClient; | ||
| import org.ezcode.codetest.application.submission.port.ProblemEventService; | ||
| import org.ezcode.codetest.application.submission.port.SubmissionEventService; | ||
| import org.ezcode.codetest.domain.submission.exception.SubmissionException; | ||
| import org.ezcode.codetest.domain.submission.exception.code.SubmissionExceptionCode; | ||
| import org.ezcode.codetest.domain.submission.model.SubmissionResult; | ||
| import org.ezcode.codetest.domain.submission.model.TestcaseEvaluationInput; | ||
| import org.ezcode.codetest.domain.submission.service.SubmissionDomainService; | ||
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.extern.slf4j.Slf4j; | ||
|
|
||
| @Slf4j | ||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class JudgementService { | ||
|
|
||
| private final SubmissionDomainService submissionDomainService; | ||
| private final SubmissionEventService submissionEventService; | ||
| private final ProblemEventService problemEventService; | ||
| private final JudgeClient judgeClient; | ||
| private final Executor judgeTestcaseExecutor; | ||
| private final ExceptionNotifier exceptionNotifier; | ||
|
|
||
| public void publishInitTestcases(SubmissionContext ctx) { | ||
| submissionEventService.publishInitTestcases( | ||
| new TestcaseListInitializedEvent(ctx.getSessionKey(), InitTestcaseListPayload.from(ctx)) | ||
| ); | ||
| } | ||
|
|
||
| public void runTestcases(SubmissionContext ctx) throws InterruptedException { | ||
| IntStream.rangeClosed(1, ctx.getTestcaseCount()) | ||
| .forEach(seqId -> runTestcaseAsync(seqId, ctx)); | ||
|
|
||
| if (!ctx.latch().await(100, TimeUnit.SECONDS)) { | ||
| throw new SubmissionException(SubmissionExceptionCode.TESTCASE_TIMEOUT); | ||
| } | ||
| } | ||
|
|
||
| @Transactional | ||
| public void finalizeAndPublish(SubmissionContext ctx) { | ||
| SubmissionResult submissionResult = submissionDomainService.finalizeSubmission(ctx); | ||
|
|
||
| publishFinalResult(ctx); | ||
| publishProblemSolve(submissionResult); | ||
| } | ||
|
|
||
| public void publishSubmissionError(String sessionKey, Exception e) { | ||
| submissionEventService.publishSubmissionError(new SubmissionErrorEvent(sessionKey, e)); | ||
| } | ||
|
|
||
| private void runTestcaseAsync(int seqId, SubmissionContext ctx) { | ||
| CompletableFuture.runAsync(() -> { | ||
| try { | ||
| log.info("[Judge RUN] Thread = {}", Thread.currentThread().getName()); | ||
| String token = judgeClient.submitAndGetToken(CodeCompileRequest.of(seqId, ctx)); | ||
| JudgeResult result = judgeClient.pollUntilDone(token); | ||
|
|
||
| TestcaseEvaluationInput input = TestcaseEvaluationInput.from(result, ctx, seqId); | ||
|
|
||
| boolean isPassed = submissionDomainService.handleEvaluationAndUpdateStats(input, ctx); | ||
|
|
||
| publishTestcaseUpdate(seqId, ctx, isPassed, result); | ||
| } catch (Exception e) { | ||
| if (ctx.notified().compareAndSet(false, true)) { | ||
| publishSubmissionError(ctx.getSessionKey(), e); | ||
| exceptionNotifier.notifyException("runTestcaseAsync", e); | ||
| } | ||
| } finally { | ||
| ctx.countDown(); | ||
| } | ||
| }, judgeTestcaseExecutor); | ||
| } | ||
|
|
||
| private void publishTestcaseUpdate(int seqId, SubmissionContext ctx, boolean isPassed, JudgeResult result) { | ||
| submissionEventService.publishTestcaseUpdate(new TestcaseEvaluatedEvent( | ||
| ctx.getSessionKey(), TestcaseResultPayload.fromEvaluation(seqId, isPassed, result)) | ||
| ); | ||
| } | ||
|
|
||
| private void publishFinalResult(SubmissionContext ctx){ | ||
| submissionEventService.publishFinalResult( | ||
| new SubmissionJudgingFinishedEvent(ctx.getSessionKey(), ctx.toFinalResult()) | ||
| ); | ||
| } | ||
|
|
||
| private void publishProblemSolve(SubmissionResult submissionResult) { | ||
| problemEventService.publishProblemSolveEvent(submissionResult); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
🧩 Verification correct
Base64 인코딩 메서드가 미사용 상태입니다
encodeBase64메서드가 정의되어 있지만 같은 클래스의of팩토리 메서드에서 사용되지 않고 있습니다. Judge0 API로 전송하는 데이터가 Base64 인코딩이 필요한지 확인이 필요합니다.Judge0 API 요구사항을 확인하기 위해 다음 스크립트를 실행해주세요:
만약 인코딩이 필요하다면 팩토리 메서드에서 다음과 같이 적용해야 합니다:
public static CodeCompileRequest of(int seqId, SubmissionContext ctx) { return new CodeCompileRequest( - ctx.getSourceCode(), + encodeBase64(ctx.getSourceCode()), ctx.getJudge0Id(), - ctx.getInput(seqId)); + encodeBase64(ctx.getInput(seqId))); }🏁 Script executed:
Length of output: 15868
Base64 인코딩 메서드가 미사용 상태입니다
encodeBase64메서드가 정의되어 있지만 같은 클래스의of팩토리 메서드에서 사용되지 않고 있습니다. Judge0 API로 전송하는 데이터가 Base64 인코딩이 필요한지 확인이 필요합니다.Judge0 API 요구사항을 확인하기 위해 다음 스크립트를 실행해주세요:
만약 인코딩이 필요하다면 팩토리 메서드에서 다음과 같이 적용해야 합니다:
public static CodeCompileRequest of(int seqId, SubmissionContext ctx) { return new CodeCompileRequest( - ctx.getSourceCode(), + encodeBase64(ctx.getSourceCode()), ctx.getJudge0Id(), - ctx.getInput(seqId)); + encodeBase64(ctx.getInput(seqId))); }🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 이거 관련해서 이슈가 좀 있었어서 이것저것 해보던 건데 안 지웠네 다음 PR에 지울게
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thezz9 네, 이해했습니다! 이슈 해결 과정에서 여러 시도를 해보시느라 남겨둔 코드군요.
혹시 이 정리 작업을 추적하기 위해 follow-up 이슈를 생성해드릴까요? 아니면 다음 PR에서 바로 처리하실 계획이시면 그대로 두셔도 됩니다.
실험적인 코드가 남아있는 것은 충분히 이해할 수 있는 상황입니다! 👍
🧠 Learnings used