Skip to content

FSTB-102 관리자 api 획일화 #52

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

Open
wants to merge 23 commits into
base: develop
Choose a base branch
from

Conversation

jwsong98
Copy link
Collaborator

@jwsong98 jwsong98 commented Aug 9, 2025

User description

📌 PR 설명

관련 이슈: FSTB-102

🛠️ 변경 유형

  • 버그 수정
  • 신규 기능 추가
  • 기존 기능 개선 / 리팩토링
  • 문서 수정
  • 기타 (아래에 작성)

🖼️ 스크린샷 또는 로그 (필요 시)

💬 기타 참고 사항


PR Type

Enhancement, Other


Description

  • 관리자 API 엔드포인트 세분화

  • 아티스트, 공연, 장소 API 분리

  • 전용 관리자 컨트롤러 도입

  • 관련 DTO 및 서비스 로직 개선


Changes diagram

flowchart LR
  A[기존 ArtistAdminController] --> AA[ArtistAliasAdminController]
  P[기존 PerformanceAdminController] --> PR[PerformanceReservationAdminController]
  P --> PT[PerformanceTimetableAdminController]
  P --> PTA[PerformanceTimetableArtistAdminController]
  P --> PU[PerformanceUrlAdminController]
  PL[기존 PlaceController] --> PLH[PlaceHallAdminController]
Loading

Changes walkthrough 📝

Relevant files
New feature
10 files
ArtistAliasContentDTO.java
아티스트 별명 콘텐츠 DTO 추가                                                                             
+5/-0     
PerformanceURLContentDTO.java
공연 URL 콘텐츠 DTO 추가                                                                               
+8/-0     
ReservationInfoContentDTO.java
예약 정보 콘텐츠 DTO 추가                                                                                 
+13/-0   
TimetableArtistContentDTO.java
시간표 아티스트 콘텐츠 DTO 추가                                                                           
+7/-0     
ArtistAliasAdminController.java
아티스트 별명 관리 전용 컨트롤러 추가                                                                       
+49/-0   
PerformanceReservationAdminController.java
공연 예약 관리 전용 컨트롤러 추가                                                                           
+53/-0   
PerformanceTimetableAdminController.java
공연 시간표 관리 전용 컨트롤러 추가                                                                         
+51/-0   
PerformanceTimetableArtistAdminController.java
시간표 아티스트 관리 전용 컨트롤러 추가                                                                     
+52/-0   
PerformanceUrlAdminController.java
공연 URL 관리 전용 컨트롤러 추가                                                                         
+49/-0   
PlaceHallAdminController.java
장소 홀 관리 전용 컨트롤러 추가                                                                             
+37/-0   
Enhancement
5 files
PerformanceURL.java
PerformanceURL 엔티티에 업데이트 메서드 추가                                                   
+5/-0     
ArtistManagement.java
아티스트 별명 추가 로직 개선                                                                                 
+6/-0     
PerformanceManagement.java
공연 예약 및 URL 관리 기능 추가                                                                         
+22/-0   
ReservationInfoMapper.java
예약 정보 DTO 매핑 추가                                                                                   
+4/-0     
URLMapper.java
공연 URL DTO 매핑 추가                                                                                 
+3/-0     
Refactoring
3 files
ArtistAdminController.java
아티스트 별명 API 엔드포인트 분리                                                                         
+15/-17 
PerformanceAdminController.java
공연 관리 API 엔드포인트 분리                                                                             
+0/-32   
PlaceController.java
장소 홀 관리 API 엔드포인트 분리                                                                         
+0/-9     

Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @jwsong98 jwsong98 self-assigned this Aug 9, 2025
    Copy link

    pr-agent-jw bot commented Aug 9, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    중복된 API 엔드포인트

    PerformanceTimetableAdminControllerputTimetableArtistPerformanceTimetableArtistAdminControlleraddTimetableArtist, putTimetableArtist 간에 기능적 중복이 있는 것으로 보입니다. API 엔드포인트의 일관성과 명확성을 위해 중복된 기능을 검토하고 하나의 명확한 엔드포인트로 통합하는 것을 고려해야 합니다.

    @PutMapping("/{timetableId}/artist")
    public ResponseEntity<Void> putTimetableArtist(
            @PathVariable("timetableId") Long timetableId,
            @RequestBody AddTimetableArtistReq req
    ) {
        timetableManagement.putTimetableArtist(timetableId, req.artistId(), req.participationType());
        return ResponseEntity.ok().build();
    예외 처리 일관성

    ArtistAdminController에서 try-catch 블록이 제거되었습니다. 전역 예외 처리기가 설정되어 있다면 문제가 없지만, 모든 새로운 컨트롤러와 리팩토링된 메서드에서 예외 처리가 일관되게 이루어지는지 확인하여 사용자에게 의미 있는 오류 응답을 제공하는지 검토해야 합니다.

    @PutMapping("/{artistId}")
    public ResponseEntity<Void> editArtist(
        @PathVariable Long artistId,
        @RequestBody EditArtistReq req
    ) {
        artistManagement.editArtist(req, artistId);
        return ResponseEntity.ok().build();
    }

    Copy link

    pr-agent-jw bot commented Aug 9, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Security
    관리자 API에 인증 로직을 추가하세요

    새로 추가된 관리자 API 컨트롤러(ArtistAliasAdminController,
    PerformanceReservationAdminController 등)의 엔드포인트에 명시적인 인증 로직이 누락되어 있습니다. 기존 관리자 API에서
    AuthService를 통한 비밀번호 인증을 사용하고 있으므로, 이 새로운 엔드포인트들도 동일한 방식으로 보호되어야 합니다. 이는 무단 접근을 방지하기
    위한 중요한 보안 취약점입니다.

    src/main/java/ddd/darayo/festival/presentation/artist/ArtistAliasAdminController.java [24-31]

    -@PostMapping
    -public ResponseEntity<Void> addArtistAlias(
    -    @PathVariable Long artistId,
    -    @RequestBody ArtistAliasContentDTO req
    -) {
    -    artistManagement.addArtistAlias(artistId, req.alias());
    -    return ResponseEntity.ok().build();
    +@RestController
    +@RequestMapping("/api/admin/artist/{artistId}/alias")
    +@RequiredArgsConstructor
    +public class ArtistAliasAdminController {
    +    private final ArtistManagement artistManagement;
    +    private final AuthService authService; // AuthService 주입
    +
    +    @PostMapping
    +    public ResponseEntity<Void> addArtistAlias(
    +        @PathVariable Long artistId,
    +        @RequestBody ArtistAliasContentDTO req,
    +        @RequestHeader("X-Admin-Password") String adminPassword // 예시: 헤더를 통한 비밀번호 전달
    +    ) {
    +        if (!authService.authenticate(adminPassword)) {
    +            return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
    +        }
    +        artistManagement.addArtistAlias(artistId, req.alias());
    +        return ResponseEntity.ok().build();
    +    }
     }
    Suggestion importance[1-10]: 10

    __

    Why: The suggestion correctly identifies a critical security vulnerability where new admin API endpoints lack proper authentication. Adding AuthService and an authentication check is crucial to prevent unauthorized access, aligning with existing admin API security practices.

    High
    General
    시간대 사용의 일관성을 확보하세요

    addReservationInfo 메서드에서는 LocalDateTime.now(ZoneId.of("Asia/Seoul"))을 사용하여 명시적으로
    시간대를 지정하고 있지만, updateReservationInfo 메서드에서는 LocalDateTime.now()를 사용하여 시스템 기본 시간대에
    의존하고 있습니다. 이는 시간대 관련 데이터 불일치를 초래할 수 있으므로, 모든 LocalDateTime.now() 호출에
    ZoneId.of("Asia/Seoul")을 명시하여 일관성을 유지해야 합니다.

    src/main/java/ddd/darayo/festival/presentation/performance/PerformanceReservationAdminController.java [27-44]

     @PostMapping
     public ResponseEntity<Void> addReservationInfo(
             @PathVariable("performanceId") Long performanceId,
             @RequestBody ReservationInfoContentDTO req
     ) {
         LocalDateTime now = LocalDateTime.now(ZoneId.of("Asia/Seoul"));
         performanceManagement.addReservationInfo(performanceId, req, now);
         return ResponseEntity.ok().build();
     }
     
     @PutMapping("/{reservationInfoId}")
     public ResponseEntity<Void> updateReservationInfo(
             @PathVariable("reservationInfoId") Long reservationInfoId,
             @RequestBody EditReservationInfoCommand req
     ) {
    -    performanceManagement.updateReservationInfo(reservationInfoId, req, LocalDateTime.now());
    +    LocalDateTime now = LocalDateTime.now(ZoneId.of("Asia/Seoul")); // 시간대 명시
    +    performanceManagement.updateReservationInfo(reservationInfoId, req, now);
         return ResponseEntity.ok().build();
     }
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion points out an inconsistency in timezone handling, which can lead to subtle bugs. Ensuring all LocalDateTime.now() calls explicitly specify ZoneId.of("Asia/Seoul") improves data consistency and robustness.

    Medium
    중복된 POST 엔드포인트를 제거하세요

    POST (addTimetableArtist)와 PUT (putTimetableArtist) 엔드포인트가 모두
    timetableManagement.putTimetableArtist 서비스 메서드를 호출하고 있습니다. 만약 putTimetableArtist가 이미
    아티스트를 생성하거나 업데이트하는 멱등성(idempotent) 작업을 수행한다면, PUT 엔드포인트만으로도 충분하며 POST 엔드포인트는 중복될 수
    있습니다. API의 명확성을 위해 중복되는 POST 엔드포인트를 제거하는 것을 고려해 보세요.

    src/main/java/ddd/darayo/festival/presentation/performance/PerformanceTimetableArtistAdminController.java [24-31]

    -@PostMapping
    -public ResponseEntity<Void> addTimetableArtist(
    -        @PathVariable("timetableId") Long timetableId,
    -        @RequestBody AddTimetableArtistReq req
    -) {
    -    timetableManagement.putTimetableArtist(timetableId, req.artistId(), req.participationType());
    -    return ResponseEntity.ok().build();
    -}
     
    +
    Suggestion importance[1-10]: 6

    __

    Why: The suggestion highlights a potential redundancy in the API design where both POST and PUT endpoints call the same timetableManagement.putTimetableArtist method. Removing the POST endpoint if PUT handles upsert operations would improve API clarity and adherence to REST principles.

    Low

    @jwsong98
    Copy link
    Collaborator Author

    jwsong98 commented Aug 9, 2025

    /ask 삭제 API 중에 의도치 않게 다른 것들을 삭제하진 않는지 검토해줘.

    Copy link

    pr-agent-jw bot commented Aug 9, 2025

    Ask

    삭제 API 중에 의도치 않게 다른 것들을 삭제하진 않는지 검토해줘.

    Answer:

    Based on the provided PR Git Diff, the changes primarily involve refactoring existing API endpoints into more granular, dedicated controllers. This means that many of the deletion operations are not new functionalities but rather existing ones moved to new locations.

    Let's review the DELETE APIs introduced or moved in this PR:

    1. ArtistAliasAdminController.java (New Controller)

      • DELETE /api/admin/artist/{artistId}/alias/{aliasId}: This endpoint is new and calls artistManagement.deleteArtistAlias(aliasId).
        • Review: The deleteArtistAlias method in ArtistManagement is an existing method (not shown as new in the diff). Assuming its prior implementation correctly deletes only the specified ArtistAlias by aliasId without affecting the parent Artist or other aliases, this refactoring itself does not introduce new risks of unintended deletions.
    2. PerformanceReservationAdminController.java (New Controller)

      • DELETE /api/admin/performance/{performanceId}/reservation/{reservationInfoId}: This endpoint is new and calls performanceManagement.deleteReservationInfo(reservationInfoId).
        • Review: The deleteReservationInfo method in PerformanceManagement is an existing method (not shown as new in the diff). It's expected to delete only the specific ReservationInfo identified by reservationInfoId. As long as the underlying service method correctly scopes its deletion, this refactoring is safe.
    3. PerformanceTimetableArtistAdminController.java (New Controller)

      • DELETE /api/admin/performance/{performanceId}/timetable/{timetableId}/artist/{artistId}: This endpoint is new and calls timetableManagement.deleteTimetableArtist(timetableId, artistId).
        • Review: The deleteTimetableArtist method in TimetableManagement is an existing method (not shown as new in the diff). This method is expected to delete the association between a timetable and an artist, or a specific entry representing an artist's participation in a timetable. It should not delete the Timetable or Artist entities themselves. Given it takes both timetableId and artistId, it implies a specific relationship is being targeted for deletion.
    4. PerformanceUrlAdminController.java (New Controller)

      • DELETE /api/admin/performance/{performanceId}/performanceURL/{performanceURLId}: This endpoint is new and calls performanceManagement.deletePerformanceURL(performanceURLId).
        • Review: The deletePerformanceURL method in PerformanceManagement is an existing method (not shown as new in the diff). It is expected to delete only the specific PerformanceURL identified by performanceURLId.

    Conclusion:

    The PR's primary goal is API endpoint segmentation and refactoring. The DELETE operations exposed through the new controllers (for Artist Alias, Performance Reservation, Performance Timetable Artist, and Performance URL) appear to be calling existing service layer methods.

    Therefore, the risk of "unintentionally deleting other things" primarily depends on the correctness of the existing service methods (artistManagement.deleteArtistAlias, performanceManagement.deleteReservationInfo, timetableManagement.deleteTimetableArtist, performanceManagement.deletePerformanceURL). Assuming these pre-existing methods were already correctly implemented to delete only the intended specific entity or association, this PR's refactoring of the API endpoints does not introduce new risks of unintended cascading deletions.

    Recommendation:

    While the PR focuses on refactoring, it's always a good practice to:

    • Verify the underlying service methods: Ensure that deleteArtistAlias, deleteReservationInfo, deleteTimetableArtist, and deletePerformanceURL in their respective management services are indeed scoped to delete only the specific entity/association identified by the provided IDs, and do not trigger unintended cascading deletions of parent or unrelated entities. This is a general check for any deletion logic.
    • Database Cascade Rules: Confirm that database foreign key constraints and cascade rules (if any are set up for deletion) align with the intended behavior for these entities.

    jwsong98 added 13 commits August 9, 2025 17:51
    - 도메인 DTO에 의존해서 API Request/Response를 구성하여 변경에 쉽게 대응
    - 도메인 DTO에 의존해서 API Request/Response를 구성하여 변경에 쉽게 대응
    - 도메인 DTO에 의존해서 API Request/Response를 구성하여 변경에 쉽게 대응
    - 도메인 DTO에 의존해서 API Request/Response를 구성하여 변경에 쉽게 대응
    - 도메인 DTO에 의존해서 API Request/Response를 구성하여 변경에 쉽게 대응
    - 도메인 DTO에 의존해서 API Request/Response를 구성하여 변경에 쉽게 대응
    - 도메인 DTO에 의존해서 API Request/Response를 구성하여 변경에 쉽게 대응
    - 도메인 DTO에 의존해서 API Request/Response를 구성하여 변경에 쉽게 대응
    …end into FSTB-102-관리자-API-획일화
    
    # Conflicts:
    #	src/main/java/ddd/darayo/festival/domain/service/ArtistManagement.java
    #	src/main/java/ddd/darayo/festival/presentation/artist/exchanges/EditArtistReq.java
    #	src/main/java/ddd/darayo/festival/presentation/artist/exchanges/SaveArtistReq.java
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant