Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 4 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ repositories {
}

dependencies {
implementation 'io.projectreactor:reactor-core'
implementation 'io.projectreactor.netty:reactor-netty'

implementation 'org.springframework.boot:spring-boot-starter-data-jpa'
implementation 'org.springframework.boot:spring-boot-starter-security'

Expand All @@ -37,7 +40,6 @@ dependencies {

// activemq
implementation 'org.springframework.boot:spring-boot-starter-activemq'
implementation "io.projectreactor.netty:reactor-netty:1.1.0"

// 웹소켓
implementation 'org.springframework.boot:spring-boot-starter-websocket'
Expand All @@ -62,6 +64,7 @@ dependencies {
testImplementation 'org.springframework.security:spring-security-test'
testRuntimeOnly 'org.junit.platform:junit-platform-launcher'
testImplementation "org.mockito:mockito-inline:4.+"
testImplementation 'com.squareup.okhttp3:mockwebserver:4.11.0'

// bcrypt
implementation 'at.favre.lib:bcrypt:0.10.2'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.ezcode.codetest.infrastructure.judge0;

import java.time.Duration;
import java.util.concurrent.TimeoutException;

import org.ezcode.codetest.application.submission.dto.request.compile.CodeCompileRequest;
import org.ezcode.codetest.application.submission.dto.response.compile.ExecutionResultResponse;
Expand All @@ -15,7 +16,6 @@
import org.springframework.web.reactive.function.client.WebClient;
import org.springframework.web.reactive.function.client.WebClientResponseException;

import io.netty.handler.timeout.TimeoutException;
import jakarta.annotation.PostConstruct;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -55,10 +55,13 @@ public String submitAndGetToken(CodeCompileRequest request) {
.filter(ex -> ex instanceof WebClientResponseException
|| ex instanceof TimeoutException)
)
.onErrorMap(IllegalStateException.class,
ex -> new SubmissionException(SubmissionExceptionCode.COMPILE_TIMEOUT))
.onErrorMap(WebClientResponseException.class,
ex -> new SubmissionException(SubmissionExceptionCode.COMPILE_SERVER_ERROR))
.onErrorMap(TimeoutException.class,
ex -> new SubmissionException(SubmissionExceptionCode.COMPILE_TIMEOUT))

Comment on lines +58 to +64
Copy link

@coderabbitai coderabbitai bot Jun 30, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

IllegalStateException 매핑의 근거 확인 필요

IllegalStateExceptionCOMPILE_TIMEOUT으로 매핑되는 것이 추가되었습니다. WebClient가 어떤 상황에서 이 예외를 던지는지 명확히 문서화하거나 주석을 추가하면 좋겠습니다.

WebClient가 IllegalStateException을 던지는 시나리오를 확인해주세요:


🌐 Web query:

When does Spring WebFlux WebClient throw IllegalStateException?

💡 Result:

Spring WebFlux’s WebClient can throw a java.lang.IllegalStateException in a few key scenarios. The two main cases supported by your search results are:


1. Blocking Calls Inside Non-blocking (Reactive) Threads

  • Cause:
    You call block(), blockFirst(), or blockLast() on a reactive publisher (like a Mono or Flux) returned by WebClient, but do so within a thread handled by the Reactor Netty event loop (such as reactor-http-nio-* or reactor-http-epoll-*). These threads must remain non-blocking for the scalability of the reactive model.
  • Exception message:
    java.lang.IllegalStateException: block()/blockFirst()/blockLast() are blocking, which is not supported in thread reactor-http-nio-4
    
  • Why:
    The Reactor framework enforces non-blocking behavior; blocking calls in these threads can stall the entire event loop, hurting performance and defeating the purpose of using reactive programming.
  • Prevention:
    Use only non-blocking operations. If you must block (e.g., for legacy interop), switch to a thread designed for blocking (such as those from Schedulers.boundedElastic()), never the event loop threads[1][4][6].

It means that you are executing this code in a thread which can not be blocked, in your case reactor-http-nio-4. If you run this code in main thread, or any other, it will work[6].


2. Only One Subscriber for Response Body

  • Cause:
    IllegalStateException: Only one connection receive subscriber allowed is thrown if you try to subscribe more than once to the same response body, such as with a POST request using certain content types (e.g., application/x-www-form-urlencoded), or by calling a terminal operation on the same response multiple times.
  • Why:
    The HTTP response body in WebClient can only be read once. Attempting a second subscription is an illegal state[7].

3. Decoder Not Found for Response Type

  • Cause:
    WebClient throws IllegalStateException if it cannot find a suitable decoder to convert the HTTP response into the requested Java type.
  • When:
    If you attempt to extract a response body as a specific object or data structure but no compatible decoder is registered, this exception is thrown[2].

IllegalStateException - if a Decoder cannot be found[2].


Summary Table

Scenario When it happens Sample Exception Message
Blocking a reactive thread (e.g. event loop) Calling block() or similar on a Netty/reactor event loop thread IllegalStateException: block()/blockFirst()/blockLast() are blocking, which is not supported in thread ...
Subscribing more than once to a response body Reading/subscribing to a WebClient response body more than once IllegalStateException: Only one connection receive subscriber allowed
No decoder found for expected response type No suitable codec to deserialize to the required Java type IllegalStateException: No suitable decoder found

Best Practices:

  • Never block in a reactive thread. Offload blocking calls to dedicated thread pools like boundedElastic[1][4][6].
  • Read response bodies once.
  • Ensure response codecs are available for all expected target types.

If you experience IllegalStateException with WebClient, first check if any of these conditions are true in your application logic.

Citations:


IllegalStateException 매핑 재검토 및 구체화 필요

WebClient가 던지는 IllegalStateException은 다음과 같이 다양한 원인으로 발생합니다.

  • 블록 호출(block()/blockFirst()/blockLast()) 시 비동기 Reactor 스레드에서 발생
  • 동일 응답 바디에 대한 다중 구독 시 발생
  • 요청한 타입에 대한 디코더 미발견 시 발생

위 모든 경우를 COMPILE_TIMEOUT으로 매핑하면 실제 컴파일 타임아웃 외의 이슈가 가려질 수 있습니다.
따라서 아래 중 하나를 적용해 매핑 조건을 구체화하세요:

  • ex.getMessage()"block()/blockFirst()/blockLast()" 문구가 포함된 경우만 COMPILE_TIMEOUT으로 매핑
  • 매핑 근거를 주석으로 명시
  • 불필요한 IllegalStateException 매핑 제거

대상 위치:

  • 파일: src/main/java/org/ezcode/codetest/infrastructure/judge0/Judge0Client.java
  • 라인: 58–64
🤖 Prompt for AI Agents
In src/main/java/org/ezcode/codetest/infrastructure/judge0/Judge0Client.java
lines 58 to 64, the current mapping of all IllegalStateException instances to
COMPILE_TIMEOUT is too broad and may mask other issues. Refine the mapping by
checking if ex.getMessage() contains "block()/blockFirst()/blockLast()" and only
then map to COMPILE_TIMEOUT, otherwise do not map or remove the general
IllegalStateException mapping. Add a comment explaining this conditional mapping
to clarify the rationale.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

RetryExhaustedException으로 넘어오는데 이게 package-private 클래스라 import 할 수가 없어서 부모 클래스인 IllegalStateException 를 쓴 거야.

Copy link

Choose a reason for hiding this comment

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

🧩 Analysis chain

@thezz9 완전히 이해했습니다! RetryExhaustedException이 package-private 클래스라서 직접 import할 수 없어서 부모 클래스인 IllegalStateException를 사용하신 거군요.

이는 Reactor 라이브러리의 API 설계 제한으로 인한 합리적인 해결책입니다. 다만 이런 배경을 모르는 다른 개발자들을 위해 주석으로 설명을 추가하시면 좋을 것 같습니다:

// RetryExhaustedException is package-private, so catch its parent IllegalStateException
.onErrorMap(IllegalStateException.class,
    ex -> new SubmissionException(SubmissionExceptionCode.COMPILE_TIMEOUT))

이전 리뷰 코멘트는 철회하겠습니다. 감사합니다! 🙏


🌐 Web query:

Reactor RetryExhaustedException package-private IllegalStateException parent class

💡 Result:

The RetryExhaustedException is a class that is often found in retry mechanism libraries or frameworks, such as those used in networking or task execution. You are asking about its package-private parent class, which is IllegalStateException.

However, IllegalStateException itself is a public class in Java and not package-private. It is part of the java.lang package and is a subclass of RuntimeException.

If RetryExhaustedException has a package-private parent class (a class with no public or protected modifier, thus only accessible within its own package), that means there is a custom exception class that RetryExhaustedException extends, which itself might extend IllegalStateException.

Could you please specify the library or framework you are referring to or provide more context? This will help me provide a more accurate explanation or code example.


🌐 Web query:

Project Reactor retryWhen RetryExhaustedException package private visibility

💡 Result:

When using Project Reactor's retryWhen operator, if all retries are exhausted, the framework throws a RetryExhaustedException that wraps the last error encountered during the retry sequence[1][3][4]. This exception helps you programmatically distinguish between a failure due to retry exhaustion and other types of errors.

Package-private Visibility of RetryExhaustedException

  • In reactor-core, the exception thrown after retries are exhausted is typically a RetryExhaustedException (sometimes known in older versions as Retry.RetryExhaustedException).
  • In reactor-extra, RetryExhaustedException is a public class extending RuntimeException[10]. Its constructors are all public, which means you can catch and use this exception freely if you have direct access to the reactor-extra dependency.
  • However, in reactor-core, documentation and common API usage do not directly expose a RetryExhaustedException as a public class in the core package documentation[9]. Instead, if you use retryWhen with a Retry spec and do not customize the exhaustion behavior, the framework will throw a package-private exception internally. This means:
    • The exception class may not be directly catchable outside the reactor.util.retry (or internal) package.
    • This limits your ability to do fine-grained exception handling on that specific type unless you override the onRetryExhaustedThrow method to throw your own exception[1][3][7].

Why package-private?

  • Package-private (default) visibility in Java means a class or member is only accessible from within its own package and not from outside[2]. This is often chosen for internal exception types in libraries when the authors want to avoid exposing implementation details as part of the public API.

What to do if you need custom handling?

  • Override the exhaustion behavior using onRetryExhaustedThrow, supplying your own (public) exception type[1][3][7]:
.retryWhen(
    Retry.backoff(3, Duration.ofSeconds(5))
        .onRetryExhaustedThrow((retrySpec, retrySignal) -> 
            new MyPublicRetryException("Retries exhausted", retrySignal.failure())
        )
)

This approach guarantees that your code can handle the exhausted scenario with an exception type you control and expose publicly[1][3].

Summary Table: Retry Exhaustion Exception Handling

Library/Module Exception Type Visibility Can You Catch/Public?
reactor-core Internal retry exhausted package-private No—unless you override
reactor-extra RetryExhaustedException public Yes
Custom (recommended) YourOwnException public Yes

Key Point:
If you depend on exception type in application code, always customize via onRetryExhaustedThrow to avoid issues with package-private visibility and ensure your code remains maintainable and robust[1][3][7].

Citations:


@thezz9 맞습니다! 확인 결과 reactor-core에서 재시도가 모두 실패했을 때 던지는 예외는 실제로 package-private이어서 직접 import하거나 catch할 수 없네요.

현재 접근 방식이 합리적인 해결책입니다. 다만 두 가지 개선 방안을 제안드립니다:

방안 1: 현재 방식 + 주석 추가 (간단함)

// Reactor's retry exhausted exception is package-private, catch parent IllegalStateException
.onErrorMap(IllegalStateException.class,
    ex -> new SubmissionException(SubmissionExceptionCode.COMPILE_TIMEOUT))

방안 2: onRetryExhaustedThrow 사용 (더 명확함)

.retryWhen(Retry.backoff(3, Duration.ofSeconds(1))
    .maxBackoff(Duration.ofSeconds(4))
    .filter(ex -> ex instanceof WebClientResponseException || ex instanceof TimeoutException)
    .onRetryExhaustedThrow((retrySpec, retrySignal) -> 
        new SubmissionException(SubmissionExceptionCode.COMPILE_TIMEOUT))
)

현재 방식도 충분히 좋으니 주석만 추가하셔도 됩니다!


✏️ Learnings added
Learnt from: thezz9
PR: ezcode-my/backend#120
File: src/main/java/org/ezcode/codetest/infrastructure/judge0/Judge0Client.java:58-64
Timestamp: 2025-06-30T10:10:07.420Z
Learning: Judge0Client에서 Reactor의 retryWhen() 재시도가 모두 실패했을 때 발생하는 RetryExhaustedException은 package-private 클래스라서 직접 import할 수 없으므로, 부모 클래스인 IllegalStateException을 catch해서 COMPILE_TIMEOUT으로 매핑하는 것이 필요한 해결책이다.

Learnt from: thezz9
PR: ezcode-my/backend#120
File: src/main/java/org/ezcode/codetest/infrastructure/judge0/Judge0Client.java:58-64
Timestamp: 2025-06-30T10:10:07.421Z
Learning: Reactor의 retryWhen()에서 재시도가 모두 실패했을 때 발생하는 retry exhausted exception은 reactor-core에서 package-private으로 구현되어 있어서 직접 import하거나 catch할 수 없다. 따라서 부모 클래스를 catch하거나 onRetryExhaustedThrow()를 사용해서 커스텀 예외를 던지는 방식으로 처리해야 한다.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: thezz9
PR: ezcode-my/backend#36
File: src/main/java/org/ezcode/codetest/application/submission/dto/request/compile/CodeCompileRequest.java:5-9
Timestamp: 2025-06-06T07:52:58.941Z
Learning: CodeCompileRequest 레코드의 필드명(source_code, language_id)은 Java camelCase 컨벤션 대신 snake_case를 사용하는데, 이는 Judge0 API의 반환 값 형식에 맞추기 위한 의도적인 설계 결정이다.

Learnt from: thezz9
PR: ezcode-my/backend#98
File: src/main/java/org/ezcode/codetest/application/submission/dto/event/SubmissionErrorEvent.java:17-22
Timestamp: 2025-06-25T01:24:30.754Z
Learning: JudgeClient에서 모든 예외를 SubmissionException으로 래핑해서 넘길 계획이므로, SubmissionErrorEvent.resolveCode() 메서드의 캐스팅은 안전할 예정임.

Learnt from: thezz9
PR: ezcode-my/backend#73
File: src/main/java/org/ezcode/codetest/infrastructure/openai/OpenAIReviewClient.java:52-64
Timestamp: 2025-06-18T11:42:03.701Z
Learning: OpenAI API 클라이언트에서는 두 가지 재시도 전략을 사용한다: 1) WebClient의 retryWhen은 네트워크/HTTP 에러 및 타임아웃에 대한 재시도, 2) 외부 for 루프는 API 응답 포맷 검증 실패에 대한 재시도. 이는 서로 다른 실패 상황을 처리하는 독립적인 메커니즘이다.

Learnt from: chat26666
PR: ezcode-my/backend#88
File: src/main/java/org/ezcode/codetest/application/game/play/GamePlayUseCase.java:0-0
Timestamp: 2025-06-21T21:29:21.785Z
Learning: JwtUtil.extractClaims() 메서드는 이미 JwtException을 catch하여 ServerException으로 변환하는 예외 처리가 구현되어 있으므로, 호출하는 곳에서 추가적인 try-catch 블록이 필요하지 않습니다.

Learnt from: thezz9
PR: ezcode-my/backend#75
File: src/main/java/org/ezcode/codetest/application/submission/aop/CodeReviewLockAspect.java:25-41
Timestamp: 2025-06-19T06:37:57.845Z
Learning: User thezz9 tends to skip validation for scenarios they consider unlikely to occur, but is willing to implement defensive programming practices when suggested for code robustness.

Learnt from: thezz9
PR: ezcode-my/backend#114
File: src/main/java/org/ezcode/codetest/infrastructure/github/GitHubApiClient.java:93-96
Timestamp: 2025-06-28T02:04:24.910Z
Learning: User thezz9 sometimes temporarily uses less secure or inconsistent exception handling (like RuntimeException with raw error bodies) for debugging purposes when error messages are being hidden in the normal application flow, but acknowledges this as technical debt and fixes it once debugging is complete.

Learnt from: thezz9
PR: ezcode-my/backend#114
File: src/main/java/org/ezcode/codetest/infrastructure/github/GitHubApiClient.java:159-165
Timestamp: 2025-06-28T02:03:38.654Z
Learning: User thezz9's service (ezcode) targets Korean users exclusively ("찐 한국인밖에 안 옴"), so Korean language hardcoding in commit messages and UI elements is intentional and appropriate for their use case.

.block();

if (resp == null || resp.token() == null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package org.ezcode.codetest.infrastructure.event;

import static org.assertj.core.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.BDDMockito.*;

import java.util.Map;

import org.ezcode.codetest.application.submission.service.SubmissionService;
import org.ezcode.codetest.domain.submission.exception.SubmissionException;
import org.ezcode.codetest.domain.submission.exception.code.SubmissionExceptionCode;
import org.ezcode.codetest.infrastructure.event.dto.submission.SubmissionMessage;
import org.ezcode.codetest.infrastructure.event.listener.RedisJudgeQueueConsumer;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.ArgumentCaptor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.data.redis.connection.stream.MapRecord;
import org.springframework.data.redis.connection.stream.RecordId;
import org.springframework.data.redis.core.StreamOperations;
import org.springframework.data.redis.core.StringRedisTemplate;

@ExtendWith(MockitoExtension.class)
@DisplayName("RedisJudgeQueueConsumer 단위 테스트")
public class RedisJudgeQueueConsumerTest {

@InjectMocks
private RedisJudgeQueueConsumer consumer;

@Mock
private SubmissionService submissionService;

@Mock
private StringRedisTemplate redisTemplate;

@Mock
private StreamOperations<String, Object, Object> streamOps;

@Mock
private MapRecord<String, String, String> record;

@BeforeEach
void setUp() {
Map<String, String> payload = Map.of(
"sessionKey", "session-key",
"problemId", "42",
"languageId", "7",
"userId", "100",
"sourceCode", "System.out.println(123);"
);
RecordId recordId = RecordId.of("1-0");

given(record.getValue()).willReturn(payload);
given(record.getId()).willReturn(recordId);
}

@Test
@DisplayName("정상 처리 -> processSubmissionAsync & ack 호출")
void onMessage_success() {

// given
given(redisTemplate.opsForStream()).willReturn(streamOps);

// when
consumer.onMessage(record);

// then
ArgumentCaptor<SubmissionMessage> captor = ArgumentCaptor.forClass(SubmissionMessage.class);
then(submissionService).should().processSubmissionAsync(captor.capture());

SubmissionMessage sent = captor.getValue();

assertAll(
() -> assertThat(sent.sessionKey()).isEqualTo("session-key"),
() -> assertThat(sent.problemId()).isEqualTo(42L),
() -> assertThat(sent.languageId()).isEqualTo(7L),
() -> assertThat(sent.userId()).isEqualTo(100L),
() -> assertThat(sent.sourceCode()).isEqualTo("System.out.println(123);")
);

then(redisTemplate.opsForStream()).should().acknowledge("judge-group", record);
}

@Test
@DisplayName("processSubmissionAsync 에러 -> SubmissionException & ack 미호출")
void onMessage_failure() {

// given
doThrow(new RuntimeException("oops"))
.when(submissionService).processSubmissionAsync(any());

// when & then
assertThatThrownBy(() -> consumer.onMessage(record))
.isInstanceOf(SubmissionException.class)
.hasMessage(SubmissionExceptionCode.REDIS_SERVER_ERROR.getMessage());

then(streamOps).should(never()).acknowledge(anyString(), any());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package org.ezcode.codetest.infrastructure.event;

import static org.assertj.core.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.BDDMockito.*;

import java.util.Map;

import org.ezcode.codetest.infrastructure.event.dto.submission.SubmissionMessage;
import org.ezcode.codetest.infrastructure.event.publisher.RedisJudgeQueueProducer;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.ArgumentCaptor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.data.redis.core.RedisTemplate;
import org.springframework.data.redis.core.StreamOperations;

@ExtendWith(MockitoExtension.class)
@DisplayName("RedisJudgeQueueProducer 단위 테스트")
public class RedisJudgeQueueProducerTest {

@InjectMocks
private RedisJudgeQueueProducer producer;

@Mock
private RedisTemplate<String, String> redisTemplate;

@Mock
private StreamOperations<String, Object, Object> streamOps;

@BeforeEach
void setUp() {
given(redisTemplate.opsForStream()).willReturn(streamOps);
}

@Test
@DisplayName("enqueue -> judge-queue에 메시지 추가")
void enqueue_addToStream() {

// given
SubmissionMessage msg = new SubmissionMessage(
"session-key",
42L,
7L,
100L,
"System.out.println(123);"
);

// when
producer.enqueue(msg);

// then
@SuppressWarnings("unchecked")
ArgumentCaptor<Map<String, String>> captor = ArgumentCaptor.forClass(Map.class);

then(streamOps).should()
.add(eq("judge-queue"), captor.capture());

Map<String, String> actual = captor.getValue();
assertAll(
() -> assertThat(actual.get("sessionKey")).isEqualTo("session-key"),
() -> assertThat(actual.get("problemId")).isEqualTo("42"),
() -> assertThat(actual.get("languageId")).isEqualTo("7"),
() -> assertThat(actual.get("userId")).isEqualTo("100"),
() -> assertThat(actual.get("sourceCode")).isEqualTo("System.out.println(123);")
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
package org.ezcode.codetest.infrastructure.github;

import static org.mockito.BDDMockito.*;

import java.util.List;
import java.util.Map;
import java.util.Optional;

import org.ezcode.codetest.application.submission.dto.request.github.GitHubPushRequest;
import org.ezcode.codetest.infrastructure.github.model.CommitContext;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

@ExtendWith(MockitoExtension.class)
@DisplayName("GitHubClient 단위 테스트")
public class GitHubClientTest {

@InjectMocks
GitHubClientImpl gitHubClientImpl;

@Mock
private GitHubApiClient gitHubApiClient;

@Mock
private GitHubContentBuilder templateBuilder;

@Mock
private GitBlobCalculator blobCalculator;

@Mock
private GitHubPushRequest request;

@Mock
private CommitContext ctx;

private String sourceCode;

@BeforeEach
void setup() {
sourceCode = "source-code";
}

@Test
@DisplayName("동일한 SHA -> 얼리 리턴")
void blobUnchanged_earlyReturn() {

// given
String sha = "abc123";
given(request.sourceCode()).willReturn(sourceCode);
given(blobCalculator.calculateBlobSha(request.sourceCode())).willReturn(sha);
given(gitHubApiClient.fetchSourceBlobSha(request)).willReturn(Optional.of(sha));

// when
gitHubClientImpl.commitAndPushToRepo(request);

// then
then(gitHubApiClient).should(never()).fetchCommitContext(any());
then(templateBuilder).should(never()).buildGitTreeEntries(any(), any());
then(gitHubApiClient).should(never()).createTree(any(), any(), anyList());
then(gitHubApiClient).should(never()).commitAndPush(any(), any(), any());
}

@Test
@DisplayName("새 SHA == 트리 SHA -> 얼리 리턴")
void newBlobEqualsTreeBlob_earlyReturn() {

// given
String oldSha = "old";
String newSha = "new";
given(request.sourceCode()).willReturn(sourceCode);
given(blobCalculator.calculateBlobSha(request.sourceCode())).willReturn(newSha);
given(gitHubApiClient.fetchSourceBlobSha(request)).willReturn(Optional.of(oldSha));
given(gitHubApiClient.fetchCommitContext(request)).willReturn(ctx);

List<Map<String, Object>> entries = List.of(Map.of());
given(templateBuilder.buildGitTreeEntries(request, newSha)).willReturn(entries);

given(ctx.baseTreeSha()).willReturn("tree");
given(gitHubApiClient.createTree(request, "tree", entries)).willReturn("tree");

// when
gitHubClientImpl.commitAndPushToRepo(request);

// then
then(gitHubApiClient).should().fetchCommitContext(request);
then(templateBuilder).should().buildGitTreeEntries(request, newSha);
then(gitHubApiClient).should().createTree(request, "tree", entries);
then(gitHubApiClient).should(never()).commitAndPush(any(), any(), any());
}

@Test
@DisplayName("SHA 변경 & 트리 SHA 변경 시 커밋 + 푸시")
void blobAndTreeChanged_commitAndPush() {

// given
String oldSha = "old";
String newSha = "new";
given(blobCalculator.calculateBlobSha(request.sourceCode())).willReturn(newSha);
given(gitHubApiClient.fetchSourceBlobSha(request)).willReturn(Optional.of(oldSha));
given(gitHubApiClient.fetchCommitContext(request)).willReturn(ctx);

List<Map<String,Object>> entries = List.of(Map.of());
given(templateBuilder.buildGitTreeEntries(request, newSha)).willReturn(entries);

given(ctx.baseTreeSha()).willReturn("tree");
given(ctx.headCommitSha()).willReturn("head");
String newTree = "newTree";
given(gitHubApiClient.createTree(request, "tree", entries)).willReturn(newTree);

// when
gitHubClientImpl.commitAndPushToRepo(request);

// then
then(gitHubApiClient).should().commitAndPush(request, "head", newTree);
}
Comment on lines +100 to +120
Copy link

@coderabbitai coderabbitai bot Jun 30, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

모킹 설정의 일관성을 개선해주세요.

세 번째 테스트에서 request.sourceCode() 호출에 대한 모킹이 누락되었습니다. 다른 테스트들과 일관성을 위해 이 설정을 추가해야 합니다.

다음 diff를 적용하여 모킹 설정을 완성해주세요:

        // given
        String oldSha = "old";
        String newSha = "new";
+       given(request.sourceCode()).willReturn(sourceCode);
        given(blobCalculator.calculateBlobSha(request.sourceCode())).willReturn(newSha);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// given
String oldSha = "old";
String newSha = "new";
given(blobCalculator.calculateBlobSha(request.sourceCode())).willReturn(newSha);
given(gitHubApiClient.fetchSourceBlobSha(request)).willReturn(Optional.of(oldSha));
given(gitHubApiClient.fetchCommitContext(request)).willReturn(ctx);
List<Map<String,Object>> entries = List.of(Map.of());
given(templateBuilder.buildGitTreeEntries(request, newSha)).willReturn(entries);
given(ctx.baseTreeSha()).willReturn("tree");
given(ctx.headCommitSha()).willReturn("head");
String newTree = "newTree";
given(gitHubApiClient.createTree(request, "tree", entries)).willReturn(newTree);
// when
gitHubClientImpl.commitAndPushToRepo(request);
// then
then(gitHubApiClient).should().commitAndPush(request, "head", newTree);
}
// given
String oldSha = "old";
String newSha = "new";
given(request.sourceCode()).willReturn(sourceCode);
given(blobCalculator.calculateBlobSha(request.sourceCode())).willReturn(newSha);
given(gitHubApiClient.fetchSourceBlobSha(request)).willReturn(Optional.of(oldSha));
given(gitHubApiClient.fetchCommitContext(request)).willReturn(ctx);
List<Map<String,Object>> entries = List.of(Map.of());
given(templateBuilder.buildGitTreeEntries(request, newSha)).willReturn(entries);
given(ctx.baseTreeSha()).willReturn("tree");
given(ctx.headCommitSha()).willReturn("head");
String newTree = "newTree";
given(gitHubApiClient.createTree(request, "tree", entries)).willReturn(newTree);
// when
gitHubClientImpl.commitAndPushToRepo(request);
// then
then(gitHubApiClient).should().commitAndPush(request, "head", newTree);
🤖 Prompt for AI Agents
In src/test/java/org/ezcode/codetest/infrastructure/github/GitHubClientTest.java
around lines 100 to 120, the mocking setup is missing a stub for the call to
request.sourceCode(), which is used in the third test. To fix this, add a
mocking setup for request.sourceCode() to return the expected source code
string, ensuring consistency with other tests that mock this method call.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

어라 빼먹어도 잘 돌아가던데...

Copy link

Choose a reason for hiding this comment

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

@thezz9 맞습니다, 테스트가 실제로 통과할 수 있어요. 아마도 Mockito가 모킹되지 않은 메서드 호출에 대해 기본값(null 등)을 반환하거나, blobCalculator.calculateBlobSha()가 null 값을 적절히 처리하고 있을 가능성이 높습니다.

하지만 일관된 모킹을 유지하는 것이 좋은 이유는:

  1. 명시적 계약: 테스트에서 어떤 값들이 사용되는지 명확하게 보여줍니다
  2. 테스트 안정성: 구현체 변경 시에도 테스트가 예측 가능하게 동작합니다
  3. 가독성: 다른 개발자가 봤을 때 모든 의존성이 명확히 드러납니다

현재는 우연히 동작하는 상황이지만, 나중에 calculateBlobSha 구현이 바뀌면 테스트가 깨질 수 있어요. 하지만 기능적으로 잘 동작한다면 굳이 지금 당장 바꿀 필요는 없습니다! 🙂


🧠 Learnings used
Learnt from: thezz9
PR: ezcode-my/backend#30
File: src/main/java/org/ezcode/codetest/domain/problem/model/dto/SubmissionData.java:42-53
Timestamp: 2025-06-04T13:12:16.729Z
Learning: 사용자 thezz9는 개발 시 기능적 정확성을 먼저 달성한 후 세부 리팩터링을 나중에 하는 접근 방식을 선호한다. 테스트 단계에서는 하드코딩된 값도 허용한다.

Learnt from: thezz9
PR: ezcode-my/backend#114
File: src/main/java/org/ezcode/codetest/infrastructure/github/GitHubApiClient.java:159-165
Timestamp: 2025-06-28T02:03:38.654Z
Learning: User thezz9's service (ezcode) targets Korean users exclusively ("찐 한국인밖에 안 옴"), so Korean language hardcoding in commit messages and UI elements is intentional and appropriate for their use case.

Learnt from: thezz9
PR: ezcode-my/backend#75
File: src/main/java/org/ezcode/codetest/application/submission/aop/CodeReviewLockAspect.java:25-41
Timestamp: 2025-06-19T06:37:57.845Z
Learning: User thezz9 tends to skip validation for scenarios they consider unlikely to occur, but is willing to implement defensive programming practices when suggested for code robustness.

Learnt from: thezz9
PR: ezcode-my/backend#114
File: src/main/java/org/ezcode/codetest/infrastructure/github/GitHubApiClient.java:93-96
Timestamp: 2025-06-28T02:04:24.910Z
Learning: User thezz9 sometimes temporarily uses less secure or inconsistent exception handling (like RuntimeException with raw error bodies) for debugging purposes when error messages are being hidden in the normal application flow, but acknowledges this as technical debt and fixes it once debugging is complete.

}
Loading