Skip to content

Conversation

@efdao
Copy link
Collaborator

@efdao efdao commented Feb 5, 2025

#️⃣ 연관된 이슈>

📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)

Controller에 Swagger 관련 Annotation을 추가했습니다
기타 버그도 수정했습니다.

🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

Summary by CodeRabbit

  • New Features
    • Enhanced API documentation for discussion, personal event, and user management endpoints to provide clearer operation details.
    • Introduced a new error code for invalid date and time ranges to improve validation feedback.
  • Bug Fixes / Improvements
    • Improved error reporting with more specific messages and status codes, particularly for date and time validations.
  • Chores
    • Upgraded backend dependencies to benefit from the latest fixes and improvements in underlying libraries.

@efdao efdao added the 🛠️ BE Backend label Feb 5, 2025
@efdao efdao added this to the 1차 스프린트 milestone Feb 5, 2025
@efdao efdao self-assigned this Feb 5, 2025
@efdao efdao requested a review from kwon204 as a code owner February 5, 2025 08:04
@coderabbitai
Copy link

coderabbitai bot commented Feb 5, 2025

Walkthrough

The pull request updates the springdoc dependency version and enhances API documentation by adding Swagger annotations to multiple controllers. It refines error handling by updating methods and exception types to use a custom ApiException, and it modifies the structure of error responses to include HTTP status codes. In addition, token field names are renamed to camelCase across the user entity and related tests. A new error code for invalid date/time ranges is introduced, and the date/time validation logic is updated accordingly.

Changes

Files Change Summary
backend/build.gradle Updated springdoc-openapi-starter-webmvc-ui from version 2.6.0 to 2.8.4.
backend/src/main/java/.../DiscussionController.java, .../PersonalEventController.java, .../UserController.java Added Swagger annotations (@Tag, @Operation, @ApiResponses, etc.) to improve API documentation for discussion, personal event, and user endpoints.
backend/src/main/java/.../UserService.java Improved error handling by throwing an ApiException with a specific ErrorCode and updated variable names from access_token/refresh_token to accessToken/refreshToken.
backend/src/main/java/.../User.java, .../UserServiceTest.java Renamed token fields to camelCase with corresponding @Column annotations and updated test references.
backend/src/main/java/.../ErrorResponse.java, .../GlobalExceptionHandler.java Modified error response to include an HttpStatus field and adjusted HTTP status retrieval in exception handling.
backend/src/main/java/.../ErrorCode.java Added new enum constant INVALID_DATE_TIME_RANGE with a BAD_REQUEST status, code "C002", and a descriptive message.
backend/src/main/java/.../Validator.java, .../ValidatorTest.java Updated date/time range validation to throw an ApiException using the new error code and modified tests to assert for ApiException.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Controller
    participant Validator
    participant GlobalExceptionHandler
    participant ErrorResponse
    
    Client->>Controller: API Request
    Controller->>Validator: validateDateTimeRange(...)
    Validator-->>Controller: Throw ApiException
    Controller->>GlobalExceptionHandler: Pass exception
    GlobalExceptionHandler->>ErrorResponse: Create error response with HTTP status
    GlobalExceptionHandler-->>Client: Return error response
Loading

Possibly related PRs

Suggested reviewers

  • kwon204

Poem

Hop, hop, through code I race,
With tokens renamed and errors in place,
Swagger tags show the way so clear,
Validation hops without any fear,
In a garden of changes so bright,
A rabbit’s delight in every commit!
🐰💻

Tip

🌐 Web search-backed reviews and chat
  • We have enabled web search-based reviews and chat for all users. This feature allows CodeRabbit to access the latest documentation and information on the web.
  • You can disable this feature by setting web_search: false in the knowledge_base settings.
  • Please share any feedback in the Discord discussion.
✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

🪧 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>, please review it.
    • 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @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 using 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 generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

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.

@efdao efdao linked an issue Feb 5, 2025 that may be closed by this pull request
2 tasks
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: 1

🧹 Nitpick comments (5)
backend/src/main/java/endolphin/backend/domain/user/entity/User.java (1)

13-13: Remove unused import.

The lombok.Setter import is not used in this class.

-import lombok.Setter;
backend/src/main/java/endolphin/backend/domain/user/UserService.java (1)

54-56: Consider adding error logging.

The error handling improvement using ApiException with a specific error code is good. Consider adding debug logging before throwing the exception to aid in troubleshooting.

 return userRepository.findById(userInfo.userId())
-    .orElseThrow(() -> new ApiException(ErrorCode.USER_NOT_FOUND));
+    .orElseThrow(() -> {
+        log.debug("User not found for ID: {}", userInfo.userId());
+        return new ApiException(ErrorCode.USER_NOT_FOUND);
+    });
backend/src/main/java/endolphin/backend/global/error/ErrorResponse.java (1)

16-18: Consider adding Swagger documentation for the static factory method.

While the record fields are well-documented, the static factory method could benefit from Swagger annotations to document its purpose and usage.

+    @Schema(description = "ErrorCode로부터 ErrorResponse를 생성")
     public static ErrorResponse of(ErrorCode errorCode) {
         return new ErrorResponse(errorCode.getHttpStatus(), errorCode.getMessage(), errorCode.getCode());
     }
backend/src/main/java/endolphin/backend/global/util/Validator.java (1)

11-27: Consider reducing code duplication using generics.

The three validation methods share identical logic but operate on different temporal types. Consider using generics with a common interface like Comparable to reduce duplication.

-    public static void validateDateTimeRange(LocalDateTime start, LocalDateTime end) {
-        if (end.isBefore(start)) {
-            throw new ApiException(ErrorCode.INVALID_DATE_TIME_RANGE);
-        }
-    }
-
-    public static void validateDateTimeRange(LocalDate start, LocalDate end) {
-        if (end.isBefore(start)) {
-            throw new ApiException(ErrorCode.INVALID_DATE_TIME_RANGE);
-        }
-    }
-
-    public static void validateDateTimeRange(LocalTime start, LocalTime end) {
-        if (end.isBefore(start)) {
-            throw new ApiException(ErrorCode.INVALID_DATE_TIME_RANGE);
-        }
-    }
+    public static <T extends Comparable<T>> void validateDateTimeRange(T start, T end) {
+        if (start.compareTo(end) > 0) {
+            throw new ApiException(ErrorCode.INVALID_DATE_TIME_RANGE);
+        }
+    }
backend/src/test/java/endolphin/backend/global/util/ValidatorTest.java (1)

22-30: Enhance test coverage and assertions.

  1. Add test cases for LocalDate and LocalTime validation.
  2. Verify the specific error code in the caught exception.
     @Test
     @DisplayName("시간 범위 체크 예외 테스트")
     void validateDateTimeRangeTest_Failure() {
         LocalDateTime startTime = LocalDateTime.of(2020, 12, 31, 23, 59, 59);
         LocalDateTime endTime = LocalDateTime.of(2020, 10, 31, 23, 59, 59);
 
         assertThatThrownBy(() -> Validator.validateDateTimeRange(startTime, endTime))
-            .isInstanceOf(ApiException.class);
+            .isInstanceOf(ApiException.class)
+            .hasFieldOrPropertyWithValue("errorCode", ErrorCode.INVALID_DATE_TIME_RANGE);
     }
+
+    @Test
+    @DisplayName("날짜 범위 체크 예외 테스트")
+    void validateDateRangeTest_Failure() {
+        LocalDate startDate = LocalDate.of(2020, 12, 31);
+        LocalDate endDate = LocalDate.of(2020, 10, 31);
+
+        assertThatThrownBy(() -> Validator.validateDateTimeRange(startDate, endDate))
+            .isInstanceOf(ApiException.class)
+            .hasFieldOrPropertyWithValue("errorCode", ErrorCode.INVALID_DATE_TIME_RANGE);
+    }
+
+    @Test
+    @DisplayName("시각 범위 체크 예외 테스트")
+    void validateTimeRangeTest_Failure() {
+        LocalTime startTime = LocalTime.of(23, 59, 59);
+        LocalTime endTime = LocalTime.of(22, 59, 59);
+
+        assertThatThrownBy(() -> Validator.validateDateTimeRange(startTime, endTime))
+            .isInstanceOf(ApiException.class)
+            .hasFieldOrPropertyWithValue("errorCode", ErrorCode.INVALID_DATE_TIME_RANGE);
+    }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 870c9a9 and 827910b.

📒 Files selected for processing (12)
  • backend/build.gradle (1 hunks)
  • backend/src/main/java/endolphin/backend/domain/discussion/DiscussionController.java (1 hunks)
  • backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventController.java (5 hunks)
  • backend/src/main/java/endolphin/backend/domain/user/UserController.java (1 hunks)
  • backend/src/main/java/endolphin/backend/domain/user/UserService.java (3 hunks)
  • backend/src/main/java/endolphin/backend/domain/user/entity/User.java (2 hunks)
  • backend/src/main/java/endolphin/backend/global/error/ErrorResponse.java (1 hunks)
  • backend/src/main/java/endolphin/backend/global/error/GlobalExceptionHandler.java (1 hunks)
  • backend/src/main/java/endolphin/backend/global/error/exception/ErrorCode.java (1 hunks)
  • backend/src/main/java/endolphin/backend/global/util/Validator.java (2 hunks)
  • backend/src/test/java/endolphin/backend/domain/user/UserServiceTest.java (1 hunks)
  • backend/src/test/java/endolphin/backend/global/util/ValidatorTest.java (2 hunks)
🧰 Additional context used
📓 Learnings (1)
backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventController.java (1)
Learnt from: kwon204
PR: softeer5th/Team4-enDolphin#45
File: backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventController.java:28-33
Timestamp: 2025-02-04T01:27:54.848Z
Learning: For search/filter operations in REST APIs, prefer using GET with query parameters over POST with request body, as it follows REST principles and enables caching, bookmarking, and better API discoverability.
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Send PR Notification to Reviewers
🔇 Additional comments (10)
backend/src/main/java/endolphin/backend/domain/user/entity/User.java (1)

31-34: LGTM! Good use of @column for DB compatibility.

The renaming of token fields to camelCase while maintaining snake_case DB column names through @Column annotations is a good practice. This maintains Java naming conventions in the code while preserving backward compatibility with the database schema.

backend/src/test/java/endolphin/backend/domain/user/UserServiceTest.java (1)

86-88: LGTM! Test updated consistently with entity changes.

The builder calls have been correctly updated to use the new camelCase token field names.

backend/src/main/java/endolphin/backend/domain/user/UserService.java (1)

66-67: LGTM! Consistent with entity changes.

The token field references have been correctly updated to use camelCase naming.

backend/src/main/java/endolphin/backend/global/error/ErrorResponse.java (1)

7-14: LGTM! Well-documented error response structure.

The Swagger annotations provide clear descriptions and examples for each field, making the API documentation comprehensive and user-friendly.

backend/src/main/java/endolphin/backend/global/error/exception/ErrorCode.java (1)

10-10: LGTM! Well-structured error code for date/time validation.

The new error code follows the established pattern and uses appropriate HTTP status code for validation errors.

backend/src/main/java/endolphin/backend/domain/discussion/DiscussionController.java (1)

6-7: LGTM! Clear and well-documented API annotations.

The Swagger annotations provide clear documentation in Korean, making the API more accessible and understandable.

Also applies to: 14-14, 22-22

backend/src/main/java/endolphin/backend/global/error/GlobalExceptionHandler.java (1)

20-20: Verify error response structure consistency.

The HTTP status is now retrieved from the response object instead of directly from the error code. Ensure that all ErrorResponse instances properly initialize their httpStatus field.

✅ Verification successful

🏁 Script executed:

#!/bin/bash
# Search for all ErrorResponse.of() calls to verify proper HTTP status initialization
ast-grep --pattern 'ErrorResponse.of($$$)' 

Length of output: 358


Consistent ErrorResponse HTTP status initialization verified.
Both calls in GlobalExceptionHandler.java (lines 19 and 26) use ErrorResponse.of(...) with an ErrorCode argument, which—as implied by the new retrieval strategy—should encapsulate the HTTP status. Verify that the ErrorResponse.of() implementation consistently takes the provided error code and sets its httpStatus field accordingly.

backend/src/main/java/endolphin/backend/domain/user/UserController.java (1)

5-6: LGTM! Well-documented authentication endpoints.

The Swagger annotations clearly document the authentication flow in Korean, improving API usability.

Also applies to: 13-13, 20-20, 27-27

backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventController.java (1)

6-13: LGTM! Comprehensive API documentation with detailed response scenarios.

The Swagger annotations provide thorough documentation in Korean, with well-defined response scenarios and status codes.

Also applies to: 31-31, 39-49

backend/build.gradle (1)

48-48: Updated Swagger Dependency Version
The springdoc-openapi-starter-webmvc-ui dependency version has been updated from 2.6.0 to 2.8.4. This update should bring in the latest features and bug fixes related to Swagger annotations and API documentation. Please verify that all Swagger annotations in your controllers remain compatible with this new version and that any configuration adjustments required by the newer version have been addressed.

Comment on lines 61 to 62
@ApiResponse(responseCode = "200", description = "일정 조회 성공",
useReturnTypeSchema = true),
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fix response code documentation for createPersonalEvent.

The success response code should be 201 (Created) instead of 200 (OK) since this is a resource creation endpoint.

-        @ApiResponse(responseCode = "200", description = "일정 조회 성공",
+        @ApiResponse(responseCode = "201", description = "일정 생성 성공",
📝 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
@ApiResponse(responseCode = "200", description = "일정 조회 성공",
useReturnTypeSchema = true),
@ApiResponse(responseCode = "201", description = "일정 생성 성공",
useReturnTypeSchema = true),

@Schema(description = "에러 응답")
public record ErrorResponse(
@Schema(description = "상태코드", example = "500")
HttpStatus httpStatus,
Copy link
Contributor

Choose a reason for hiding this comment

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

p3: 빼도 좋을 거 같습니다.!

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

@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

🧹 Nitpick comments (1)
backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventController.java (1)

40-52: Enhance array schema documentation for better clarity.

While the documentation is comprehensive, the array schema could be more specific about the ListResponse wrapper.

Update the content annotation to explicitly show the ListResponse wrapper:

         @ApiResponse(responseCode = "200", description = "일정 조회 성공",
             content = @Content(
-                array = @ArraySchema(
-                    schema = @Schema(implementation = PersonalEventResponse.class)))),
+                schema = @Schema(
+                    implementation = ListResponse.class,
+                    subTypes = {PersonalEventResponse.class}))),
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 827910b and 8e76cc1.

📒 Files selected for processing (4)
  • backend/src/main/java/endolphin/backend/domain/discussion/DiscussionController.java (1 hunks)
  • backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventController.java (5 hunks)
  • backend/src/main/java/endolphin/backend/global/error/ErrorResponse.java (1 hunks)
  • backend/src/main/java/endolphin/backend/global/error/GlobalExceptionHandler.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • backend/src/main/java/endolphin/backend/global/error/GlobalExceptionHandler.java
  • backend/src/main/java/endolphin/backend/domain/discussion/DiscussionController.java
  • backend/src/main/java/endolphin/backend/global/error/ErrorResponse.java
🧰 Additional context used
📓 Learnings (1)
backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventController.java (1)
Learnt from: kwon204
PR: softeer5th/Team4-enDolphin#45
File: backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventController.java:28-33
Timestamp: 2025-02-04T01:27:54.848Z
Learning: For search/filter operations in REST APIs, prefer using GET with query parameters over POST with request body, as it follows REST principles and enables caching, bookmarking, and better API discoverability.
🔇 Additional comments (4)
backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventController.java (4)

6-14: LGTM! Well-structured Swagger documentation setup.

The imports and class-level Swagger annotations are properly organized, with appropriate tag name and description in Korean.

Also applies to: 32-32


62-79: LGTM! Excellent REST implementation for resource creation.

The endpoint follows REST best practices with proper 201 status code, location header, and response body.


81-101: LGTM! Well-documented update endpoint.

The endpoint follows REST conventions with appropriate status codes and comprehensive error documentation.


103-120: LGTM! Proper implementation of DELETE endpoint.

The endpoint follows REST conventions with appropriate 204 status code and comprehensive error documentation.

Copy link
Contributor

@kwon204 kwon204 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다.

@efdao efdao merged commit 72afd78 into dev Feb 6, 2025
1 check passed
@efdao efdao deleted the feature/be/api-swagger branch February 6, 2025 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🛠️ BE Backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Controller에 swagger 어노테이션 추가

3 participants