Skip to content
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

feat: 과제 제출하기 채점 로직 구현 #649

Merged
merged 29 commits into from
Aug 20, 2024

Conversation

uwoobeat
Copy link
Member

@uwoobeat uwoobeat commented Aug 19, 2024

🌱 관련 이슈

📌 작업 내용 및 특이사항

기존 컨텍스트

  • 기존에는 깃허브 클라이언트에서 과제 파일을 조회해올 때, GithubAssignmentSubmission 으로 응답을 받아오도록 했습니다.
  • 하지만 조회 과정에서, 이런저런 이유로 예외가 발생할 수 있습니다. 사용자가 과제 제출을 할 때 잘못된 디렉토리에 파일을 위치시킬 수도 있고, 아니면 서버가 제출정보를 github api에 요청하는 과정에서 네트워크 예외가 발생할 수도 있고요.
  • Github API Wrapper 라이브러리에서는 이러한 가능성이 있는 경우 method call마다 IOException을 발생시키는 방식으로 처리하고 있습니다. 저희는 이를 받아서 CustomException 및 적절한 에러코드로 변환하는 작업을 수행했었습니다. 대충 이런 느낌입니다.
private GHContent getFileContent(GHRepository ghRepository, String filePath) {
    try {
        return ghRepository.getFileContent(filePath);
    } catch (IOException e) {
        throw new CustomException(GITHUB_CONTENT_NOT_FOUND);
    }
}

조회 과정에서의 예외, 그리고 실패 사유

  • 여기서 발생하는 예외는 두 가지로 나뉠 수 있습니다.
    • 사용자의 잘못으로 인한 예외 -> GITHUB_CONTENT_NOT_FOUND
    • 서버 에러, 네트워크 에러 등등 사용자의 잘못이 아닌데도 발생한 예외 -> GITHUB_FILE_READ_FAILED 외 다수
  • 한편, 제출된 과제를 채점할 때, 실패 처리하는 경우 사유를 적도록 되어있습니다.
  • 사용자가 잘못한 경우, 그 사유를 실패 사유로 적어줘야 합니다,.
  • 서버가 잘못한 경우, 그 사유를 구체적으로 적을 필요는 없지만 어쨌든 내부 문제로 인해 제출 실패 처리되었다는 것을 알려줄 필요가 있습니다.
  • 이러한 이유로, 실패사유 중 UNKNOWN("알 수 없음") 을 추가하게 되었습니다.

(1) - 예외를 실패사유로 변환하는 비즈니스 로직의 노출

  • 위의 핵심은, '발생한 예외'가 '실패 사유'로 변환되어야 한다는 것입니다.
  • 이를 위해서는, gihubClient.getLatestAssignmentSubmission 을 호출하여 github api와의 통신을 시도할 때, 이를 try-catch로 감싸서, catch 절에서 에러코드실패사유 로 변환하는 과정이 필요합니다
  • 문제는, 에러코드실패사유 로 변환하는 것이 일종의 비즈니스 로직이라는 것에 있습니다.

(1) - 예외가 발생하는 시점

  • 저희는 응용 레이어에 비즈니스 로직이 노출되기를 원하지 않기 때문에, 도메인 서비스를 도입했습니다.
  • 그러므로, try-catchgithubClient.getX 로직을 감싸는 부분도 응용 레이어가 아닌 도메인 레이어에서 이루어져야 합니다.
  • 엄밀하게는, catch 절에서 발생한 예외를 실패사유로 변환하는 부분이 도메인 레이어에서 수행되어야 합니다.
  • 하지만 이렇게 할 수가 없습니다... 현재 콜스택에서 발생한 예외를 위로 던지는 건 봤어도 아래로 던진다는 이야긴 들어본 적이 없습니다.

(1) - 해결책?

  • 가장 간단한 해결책은, AssignmentHistoryGrader, 즉 과제 채점 도메인 서비스의 내부에 GithubClient 를 DI하는 것입니다.
  • 그러면 도메인 레이어에서 githubClient.getX 를 호출하여, 예외를 실패사유로 변환할 수 있게 되니, 문제를 해결할 수 있습니다.
  • 애초에 '실패사유'를 결정하는 것 역시 Grader, 채점 서비스의 책임이니 이 역시 적절한 처사라고 볼 수 있습니다.
@DomainService
@RequiredArgsConstructor
public class AssignmentHistoryGrader {
    
    private final GithubClient githubClient;
    ...
}

(1) - 도메인 서비스의 정의와 부합하지 않는다

  • 하지만 이러면 도메인 서비스가 아니라고 했었죠?
  • 도메인 서비스는 행위만을 가지고, 여기에 필요한 상태는 외부에서 공급받습니다.
  • 또한, 도메인 서비스는 '도메인 레이어'에 속하기 때문에, 기본적으로 POJO여야 하고, 스프링의 DI 기능에 의존하지 않고 순수하게 유지해야 합니다.
  • 물론 @DomainService 내부에는 @Component가 있어 스프링 빈으로 등록되긴 합니다만, 이건 응용 레이어에서 도메인 서비스를 쉽게 사용하기 위함이지 의존객체의 DI를 위한 것이 아닙니다 (애초에 의존객체를 만들지 말아야 합니다).

(2) 람다식과 지연평가

  • 따라서 문제 상황을 다시 정리해보면, GithubClient 에 대한 의존성은 응용 레이어에 위치하면서도, 실제 조회 및 예외 발생은 도메인 레이어에서 이루어져야 한다는 것입니다.
  • 이걸 해결할 수 있는 좋은 방법은, 람다의 지연 평가 특성을 활용하는 것입니다.
  • GithubClient#getXXX는 호출되면 바로 github api를 통해 GithubAssignmentSubmissionResponse 로 가져오는 것이 아니라...
  • () -> new Response() 와 같이Supplier<XResponse> 타입의 람다식으로 만들어서 리턴한 뒤, 이를 Grader의 인자로 받아서, Grader 내부에서 supplier.get() 을 호출하면 위 문제를 해결할 수 있습니다.

(2) - 행위의 객체화

  • 람다식은 '함수형 인터페이스를, 익명 클래스로 구현하여, 이에 대한 인스턴스를 만든 것'입니다. 간단하게 말하면, 행위의 객체화입니다.
  • 사실 () -> new Response() 와 같은 표현은 적절하지 않습니다. 이건 그냥 응답을 반환하는 람다식일 뿐입니다. 우리가 원하는 것은, github api로 요청을 보내서 응답으로 가져오는 행위를 객체화시키는 것입니다.
  • 그러면 getLatest... 로직 자체를 람다식으로 만들어버려야 합니다.
public Supplier<AssignmentSubmissionResponse> getLatestAssignmentSubmissionSupplier(String repo, int week) {
    return () -> {
        GHRepository ghRepository = getRepository(repo);
        String assignmentPath = GITHUB_ASSIGNMENT_PATH.formatted(week);
        ...
        return new AssignmentSubmissionResponse(...);
    };
}
  • 이렇게 하면 모든 문제가 해결됐습니다!

(2) - 그런데 repo와 week는 어디로...?

  • 여기서 궁금한 점이 생깁니다.
  • 저 Supplier는 Grader 내부에서 호출될 것이고. 그러면 GHRepository ghRepository = getRepository(repo); 부터 쭉 한 줄씩 코드가 실행될 것입니다.
  • 그렇다면 해당 코드의 repo 는 어디서 온 것인가요?
  • 원래였다면 메서드의 인자로 받아서 넘겨줬을 것입니다.
  • 그게 아니라 (repo, week) -> { GHRepository ghRepository = getRepository(repo); } 같은 식으로 람다식을 구성했다면, '아, 람다식 파라미터로 넘겨진 repo를 받아서 처리했겠구나' 라는 것을 알 수 있었을 겁니다.
  • getLatestAssignmentSubmissionSupplier(String repo, int week) 에서 전달받은 repo 변수의 경우, 해당 supplier가 사용되는 grader.judge() 가 호출되는 스코프에서는 접근할 수 없는 상태일 것입니다.
  • 실제로 이렇게 구현하더라도, 해당 람다식은 getLatestAssignmentSubmissionSupplier(String repo, int week) 에서 전달받은 repo와 week 정보를 기억해서 실행합니다. 어떻게 이런 것이 가능할까요?

(2) - 람다식과 클로저

  • 여기서는 repo가 상위 콜스택으로부터 전달받은 값이기 때문에 소멸되지 않지만, 만약 repo가 지역변수라서 getLatestAssignmentSubmissionSupplier(String repo, int week) 를 벗어나는 순간 소멸된다 하더라도, 이 람다식은 정상적으로 실행됩니다.
  • 이유는 간단한데요, 람다식이 저장될 때 람다식 내부에서 외부 변수인 repo를 참조하는 경우, 이를 복사해서 같이 저장하기 때문입니다. 이걸 variable capture라고 하고, 이러한 람다식을 closure라고 부릅니다.
  • 람다와 클로저는 각기 장단이 있는데요, 필요한 경우에는 클로저에 묶인 컨텍스트를 파라미터로 추출해내야 합니다. 이를 람다 리프팅이라고 합니다(https://en.wikipedia.org/wiki/Lambda_lifting).
  • 람다 리프팅을 통해서 캡쳐된 값을 유지하도록 하는 비용을 줄이고, 메모리 사용이나 재사용성 등을 강화시킬 수 있습니다.
  • 기존에도 잘 되는데 왜 굳이 여기서 복잡하게 이걸 해야 하느냐! 하면 이거까지 설명하려면 너무 길어질 것 같아서... 적당히 그런 이유가 있다 하고 이 부분은 스킵하도록 하겠습니다
  • 아무튼, () -> { GHRepository ghRepository = getRepository(repo); } 같은 클로저에서 (repo, week) -> { GHRepository ghRepository = getRepository(repo); } 와 같이 추출해냈습니다.
  • 물론 Github 빈에 대한 의존성은 그대로 있지만, 이것마저 파라미터로 뺄 필요는 없기 때문에... 넘어갑니다.
  • 그리고, 기존에 캡쳐된 repo와 week를 대신해줄 수 있도록, 1) repo 2) week 3) 람다식을 하나로 묶어주는 클래스를 만들어줍니다.
public record AssignmentSubmissionFetcher(String repo, int week, AssignmentSubmissionFetchExecutor fetchExecutor) {
    public AssignmentSubmission fetch() throws CustomException {
        return fetchExecutor.execute(repo, week);
    }
}


@FunctionalInterface
public interface AssignmentSubmissionFetchExecutor {
    AssignmentSubmission execute(String repo, int week) throws CustomException;
}
  • ...그래서 기존 Fetcher를 FetchExecutor로 변경하고, 이 FetchExecutor를 실행하는데 필요한 컨텍스트를 Fetcher에서 같이 저장하도록 했습니다. (이게 사실상 핵심입니다)

마무리

아무튼 여기까지가 끝이고...
갑자기 등장한 FetcherFetchExecutor에 대해 궁금해할 것 같아 해결과정을 쭉 적어봤습니다.
클라이언트 내부에도 javadoc 등으로 이러한 컨텍스트를 기록해두었으니 체크 부탁드립니다.

너무 새로운 방식이라 거부감이 있으실 수도 있겠지만,
테스트 코드 보시면 이렇게 하면 테스트 작성이 정말 쉬워진다는걸 체감하실 수 있으실 겁니다
(그리고 테스트하기 쉬운 코드는 좋은 코드일 가능성이 높습니다)

📝 참고사항

📚 레퍼런스

Summary by CodeRabbit

Summary by CodeRabbit

  • 신규 기능

    • 과제 제출을 위한 새로운 API 엔드포인트(/submit)가 추가되었습니다.
    • 과제 제출 및 평가를 위한 새로운 클래스가 도입되었습니다.
    • 과제 제출을 위한 새로운 데이터 구조가 생성되었습니다.
    • 과제 평가를 위한 최소 콘텐츠 길이 상수가 추가되었습니다.
  • 버그 수정

    • 제출 상태 및 오류 처리에 대한 테스트가 추가되었습니다.
  • 문서화

    • StudyDetail 클래스의 주석을 업데이트하여 향후 변경 계획을 반영했습니다.
  • 기타 변경 사항

    • 여러 상수 및 데이터 타입이 변경되었습니다.
    • 새로운 오류 처리 상수가 추가되었습니다.

@uwoobeat uwoobeat requested a review from a team as a code owner August 19, 2024 14:16
@uwoobeat uwoobeat self-assigned this Aug 19, 2024
Copy link

coderabbitai bot commented Aug 19, 2024

## Walkthrough

이번 변경 사항은 과제 제출 기능을 강화하기 위해 여러 클래스를 수정하고 새로운 메서드 및 클래스를 추가했습니다. 주요 기능으로는 과제 제출 및 채점 로직이 포함되며, 데이터 구조와 예외 처리 방식이 개선되어 학생들의 과제 제출 경험이 더욱 원활해질 것입니다.

## Changes

| 파일 경로 | 변경 요약 |
|-----------|----------|
| `src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java` | `submitAssignment` 메서드 추가, POST 요청 처리. |
| `src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java` | `AssignmentHistoryGrader`, `AssignmentSubmissionFetcher` 추가, `submitAssignment` 로직 개선. |
| `src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistory.java` | `contentLength` 타입을 `Long`에서 `Integer`로 변경. |
| `src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistoryGrader.java` | 새 클래스 추가, 과제 채점 로직 구현. |
| `src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentSubmission.java` | 새로운 레코드 추가, 과제 제출 정보 캡슐화. |
| `src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentSubmissionFetchExecutor.java` | 기능적 인터페이스 추가, 제출 가져오기 메서드 정의. |
| `src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentSubmissionFetcher.java` | 과제 제출 가져오는 클래스 추가. |
| `src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyDetail.java` | `week` 필드에 대한 주석 추가. |
| `src/main/java/com/gdschongik/gdsc/domain/study/domain/SubmissionFailureType.java` | 새로운 enum 상수 `UNKNOWN` 추가. |
| `src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java` | `GITHUB_ASSIGNMENT_NOT_FOUND` 제거. |
| `src/main/java/com/gdschongik/gdsc/infra/github/client/GithubClient.java` | `getLatestAssignmentSubmissionFetcher` 메서드 추가. |
| `src/test/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryServiceTest.java` | `StudentStudyHistoryService`의 유닛 테스트 추가. |
| `src/test/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistoryGraderTest.java` | `AssignmentHistoryGrader`의 유닛 테스트 추가. |
| `src/test/java/com/gdschongik/gdsc/global/common/constant/StudyConstant.java` | `CONTENT_LENGTH` 타입 변경, 새로운 상수 `REPOSITORY_LINK` 추가. |
| `src/test/java/com/gdschongik/gdsc/helper/IntegrationTest.java` | `GithubClient`의 의존성 추가 및 `publishAssignment` 메서드 추가. |

## Assessment against linked issues

| Objective | Addressed | Explanation |
|-----------|-----------|-------------|
| 과제 제출하기 채점 로직 구현 (#640) ||  |
| 새로운 과제 제출 기능 추가 ||  |
| 데이터 타입 개선 ||  |

## Poem

> 🐇 과제 제출할 시간, 기뻐라!  
> 채점 로직이 가득 차,  
> 학생들아, 너희의 꿈,  
> 이제는 쉽게 이룰 수 있어,  
> 함께 뛰어놀자, 기쁜 마음으로,  
> 새로운 시작, 모두와 함께! 🌼✨

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (3)
src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyDetail.java (1)

35-35: week 필드의 타입 변경 예정

week 필드에 대한 TODO 주석이 추가되었습니다. 이는 향후 Long에서 Integer로의 타입 변경을 계획하고 있음을 나타냅니다. 이 변경이 필요한 이유와 예상되는 영향에 대해 문서화하는 것이 좋습니다.

src/test/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryServiceTest.java (1)

51-52: TODO 주석: LocalDateTime.now() 관련 테스트 정책 논의 필요

LocalDateTime.now() 사용에 대한 정책을 명확히 하여 테스트의 일관성을 유지할 필요가 있습니다. 이 부분에 대한 논의가 필요합니다.

- // TODO: LocalDateTime.now() 관련 테스트 정책 논의 필요
+ // TODO: LocalDateTime.now() 사용 정책 논의 필요
src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java (1)

59-59: TODO 주석: GHRepository 래핑

GHRepository를 래핑하여 테스트 가능성을 높이는 것은 좋은 접근입니다. 이 부분에 대한 구현이 필요합니다.

- // TODO: GHRepository 등을 wrapper로 감싸서 테스트 가능하도록 변경
+ // TODO: GHRepository를 wrapper로 감싸서 테스트 가능하도록 변경
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between eb9e3ca and 02c821e.

Files selected for processing (15)
  • src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java (5 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistory.java (2 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistoryGrader.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentSubmission.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentSubmissionFetchExecutor.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentSubmissionFetcher.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyDetail.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/SubmissionFailureType.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/infra/github/client/GithubClient.java (4 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryServiceTest.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistoryGraderTest.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/global/common/constant/StudyConstant.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/helper/IntegrationTest.java (3 hunks)
Additional comments not posted (27)
src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentSubmission.java (1)

1-5: 코드 검토 완료: 적절한 레코드 정의

AssignmentSubmission 레코드는 URL, 커밋 해시, 콘텐츠 길이, 커밋 시간을 저장하는 데 적절합니다. 코드에 문제가 없습니다.

src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentSubmissionFetchExecutor.java (1)

1-8: 코드 검토 완료: 적절한 함수형 인터페이스 정의

AssignmentSubmissionFetchExecutor는 과제 제출을 가져오는 기능을 정의하는 데 적절한 함수형 인터페이스입니다. 코드에 문제가 없습니다.

src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentSubmissionFetcher.java (1)

1-9: 코드 검토 완료: 적절한 레코드 및 메서드 정의

AssignmentSubmissionFetcher는 과제 제출을 가져오는 기능을 잘 캡슐화하고 있으며, fetch 메서드는 적절하게 실행을 위임합니다. 코드에 문제가 없습니다.

src/main/java/com/gdschongik/gdsc/domain/study/domain/SubmissionFailureType.java (1)

12-14: 코드 검토 완료: 새로운 열거형 상수 추가

UNKNOWN 상수의 추가는 예외 상태를 더 잘 표현할 수 있게 해줍니다. 전체 구조가 일관적이며 잘 유지되고 있습니다.

src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistoryGrader.java (1)

1-48: 코드 검토 완료: 과제 채점 로직 구현

AssignmentHistoryGrader 클래스는 과제 채점 로직을 효과적으로 구현하고 있으며, 예외 처리도 적절하게 수행하고 있습니다. 상수 사용도 적절합니다. 추가로, 이 클래스의 다양한 시나리오에 대한 테스트 커버리지를 고려해 보세요.

src/main/java/com/gdschongik/gdsc/domain/study/api/StudentStudyHistoryController.java (1)

39-44: 과제 제출 기능 구현 확인

submitAssignment 메서드는 과제 제출을 위한 기능을 추가합니다. @Operation 어노테이션을 통해 API 문서화가 잘 되어 있으며, studentStudyHistoryService를 호출하여 과제를 제출합니다. 이 변경 사항은 API의 기능을 확장하여 사용자에게 과제 제출 기능을 제공합니다. 추가적으로, 제출된 과제에 대한 채점 로직이 서비스 계층에 구현되어 있는지 확인이 필요합니다.

src/test/java/com/gdschongik/gdsc/global/common/constant/StudyConstant.java (2)

44-44: CONTENT_LENGTH 타입 변경 확인

CONTENT_LENGTH의 타입이 Long에서 Integer로 변경되었습니다. 이 변경은 해당 상수가 사용되는 코드의 다른 부분에 영향을 미칠 수 있으므로, 호환성 검토가 필요합니다. Integer로의 변경이 적절한지 확인하시기 바랍니다.


48-48: REPOSITORY_LINK 상수 추가 확인

새로운 상수 REPOSITORY_LINK가 추가되었습니다. 이는 저장소 링크를 문자열로 제공하여 코드의 가독성을 높이고 유지보수를 용이하게 합니다.

src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistory.java (2)

49-49: contentLength 타입 변경 확인

contentLength의 타입이 Long에서 Integer로 변경되었습니다. 이 변경은 데이터의 범위와 관련된 부분에 영향을 줄 수 있으므로, 기존 로직과의 호환성을 확인해야 합니다.


88-88: success 메서드의 매개변수 타입 변경 확인

success 메서드의 contentLength 매개변수가 Long에서 Integer로 변경되었습니다. 이 변경에 따라 메서드를 호출하는 부분에서도 타입 호환성을 확인해야 합니다.

src/main/java/com/gdschongik/gdsc/infra/github/client/GithubClient.java (2)

35-42: 지연 평가를 위한 fetcher 메서드 추가

getLatestAssignmentSubmissionFetcher 메서드는 지연 평가를 가능하게 하여 요청 수행 시 발생하는 예외를 과제 채점에 활용할 수 있도록 합니다. 이는 채점 로직의 유연성을 높이며, fetcher를 통해 요청을 수행할 수 있게 합니다.


Line range hint 45-64: getLatestAssignmentSubmission 메서드 수정 확인

getLatestAssignmentSubmission 메서드는 이제 AssignmentSubmission 객체를 반환합니다. 이 변경은 반환되는 데이터 구조를 변경하여 더 많은 정보를 포함할 수 있도록 합니다. 특히, 커밋 날짜를 LocalDateTime으로 직접 반환하여 변환 과정을 단순화합니다.

src/test/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryServiceTest.java (4)

38-42: LocalDateTime.now() 모킹

LocalDateTime.now()를 모킹하여 테스트의 일관성을 유지하는 것은 좋은 접근입니다. 그러나, 모킹된 시간이 테스트의 다른 부분에 영향을 미칠 수 있으므로, 테스트가 독립적으로 실행되도록 주의해야 합니다.


69-74: Mocking AssignmentSubmissionFetcher

AssignmentSubmissionFetcher를 모킹하여 테스트의 독립성을 유지하는 것은 적절합니다. 그러나, 모킹된 객체의 동작이 실제 환경과 일치하는지 확인이 필요합니다.


80-86: 성공적인 과제 제출 테스트

과제 제출이 성공적으로 처리되는지 확인하는 테스트 케이스입니다. 모든 검증이 적절하게 이루어지고 있으며, 테스트가 명확하게 작성되었습니다.


64-67: 직접 수강신청 생성

테스트에서 직접 수강신청을 생성하여 수강신청 로직을 우회하는 것은 테스트의 목적에 부합합니다. 그러나, 이 부분이 실제 로직과 일치하는지 확인이 필요합니다.

Verification successful

테스트 설정이 실제 로직과 일치합니다

StudyHistory.create 메서드는 실제 로직에서도 사용되고 있으며, 이는 테스트가 실제 조건을 잘 시뮬레이션하고 있음을 나타냅니다. 따라서, 테스트에서 수강신청을 직접 생성하는 방식은 적절합니다.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the study application logic aligns with the test setup.

# Test: Check for direct creation of study applications in the codebase.
rg --type java 'StudyHistory.create'

Length of output: 840

src/test/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistoryGraderTest.java (5)

23-28: createAssignmentHistory 메서드

createAssignmentHistory 메서드는 테스트에 필요한 기본 데이터를 설정하는 데 유용합니다. FixtureHelper를 사용하여 코드의 중복을 줄이고 가독성을 높이는 것이 좋습니다.


40-55: 과제내용이 최소길이 이상이면 성공 처리

과제 내용이 최소 길이 이상일 때 성공으로 처리되는지 확인하는 테스트 케이스입니다. 테스트가 명확하고 필요한 모든 검증을 포함하고 있습니다.


57-70: 과제내용이 최소길이 미만이면 실패 처리

과제 내용이 최소 길이 미만일 때 실패로 처리되는지 확인하는 테스트 케이스입니다. 실패 유형이 적절하게 검증되고 있습니다.


72-84: 과제 파일 미존재 시 실패 처리

과제 파일이 존재하지 않을 때 실패로 처리되는지 확인하는 테스트 케이스입니다. 예외 처리와 실패 유형 검증이 적절히 이루어지고 있습니다.


86-98: Github 문제 시 실패 처리

Github 문제로 인해 실패로 처리되는 경우를 테스트합니다. 알 수 없는 오류로 분류되는지 확인하고 있으며, 테스트가 명확하게 작성되었습니다.

src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java (3)

45-45: AssignmentHistoryGrader 의존성 추가

AssignmentHistoryGrader를 추가하여 과제 채점 기능을 강화했습니다. 이로 인해 서비스의 기능이 확장되었습니다.


93-99: Optional 사용

Optional을 사용하여 StudyHistory의 존재 여부를 처리하는 것은 코드의 가독성을 높이고, null 체크를 명확하게 합니다.


101-104: AssignmentSubmissionFetcher 사용

AssignmentSubmissionFetcher를 사용하여 최신 과제 제출 정보를 가져오는 것은 서비스의 기능을 강화합니다. 이로 인해 채점 로직이 더 정확해질 수 있습니다.

src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java (1)

Line range hint 1-1: GITHUB_ASSIGNMENT_NOT_FOUND 제거

GITHUB_ASSIGNMENT_NOT_FOUND가 제거되었습니다. 이로 인해 관련된 예외 처리 로직이 변경될 수 있으며, 해당 코드를 사용하는 부분이 적절히 업데이트되었는지 확인이 필요합니다.

Verification successful

GITHUB_ASSIGNMENT_NOT_FOUND 제거 확인 완료

GITHUB_ASSIGNMENT_NOT_FOUND에 대한 모든 참조가 코드베이스에서 제거된 것으로 확인되었습니다. 관련된 예외 처리 로직이 적절히 업데이트되었음을 확인할 수 있습니다.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that references to `GITHUB_ASSIGNMENT_NOT_FOUND` have been removed or updated.

# Test: Search for any remaining references to `GITHUB_ASSIGNMENT_NOT_FOUND`.
rg --type java 'GITHUB_ASSIGNMENT_NOT_FOUND'

Length of output: 44


Script:

#!/bin/bash
# Additional verification: Check for references to `GITHUB_ASSIGNMENT_NOT_FOUND` in all files, including comments and documentation.

# Test: Search for any references to `GITHUB_ASSIGNMENT_NOT_FOUND` across the entire codebase.
rg 'GITHUB_ASSIGNMENT_NOT_FOUND'

Length of output: 32

src/test/java/com/gdschongik/gdsc/helper/IntegrationTest.java (2)

91-91: GithubClient 모킹

GithubClient를 모킹하여 테스트의 독립성을 유지하는 것은 적절합니다. 이를 통해 외부 API 호출을 시뮬레이션할 수 있습니다.


265-267: publishAssignment 메서드 추가

publishAssignment 메서드를 추가하여 스터디 상세 정보에 과제를 게시하는 기능을 테스트할 수 있게 되었습니다. 이는 통합 테스트의 범위를 확장하는 데 유용합니다.

@uwoobeat uwoobeat marked this pull request as draft August 19, 2024 14:28
@uwoobeat uwoobeat marked this pull request as ready for review August 19, 2024 15:59
Copy link

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 02c821e and 61afa7f.

Files selected for processing (2)
  • src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java (5 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistoryGrader.java (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistoryGrader.java

Copy link
Member

@Sangwook02 Sangwook02 left a comment

Choose a reason for hiding this comment

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

lgtm

@DomainService
public class AssignmentHistoryGrader {

public static final int MINIMUM_ASSIGNMENT_CONTENT_LENGTH = 300;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public static final int MINIMUM_ASSIGNMENT_CONTENT_LENGTH = 300;
private static final int MINIMUM_ASSIGNMENT_CONTENT_LENGTH = 300;

클래스 내에서만 사용되는 것 같은데 public이어야 할 필요가 있을까요?

Copy link
Member

@AlmondBreez3 AlmondBreez3 left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@seulgi99 seulgi99 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 61afa7f and 0a4da1b.

Files selected for processing (1)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistoryGrader.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistoryGrader.java

@uwoobeat uwoobeat merged commit db5adc3 into develop Aug 20, 2024
1 check passed
@uwoobeat uwoobeat deleted the feature/640-assignment-judge branch August 20, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

✨ 과제 제출하기 채점 로직 구현
4 participants