Skip to content

[DDING-000] 통계 전체조회시 파일 조회 쿼리 개선 및 응답값 수정#292

Merged
KoSeonJe merged 7 commits intodevelopfrom
fix/DDING-000
Mar 10, 2025
Merged

[DDING-000] 통계 전체조회시 파일 조회 쿼리 개선 및 응답값 수정#292
KoSeonJe merged 7 commits intodevelopfrom
fix/DDING-000

Conversation

@KoSeonJe
Copy link
Copy Markdown
Collaborator

@KoSeonJe KoSeonJe commented Mar 5, 2025

🚀 작업 내용

파일 통계 전체 조회시 파일 이름 반환하도록 수정하였습니다

🤔 고민했던 내용

개선 후 프로세스 순서
1. 해당 필드에 답변한 FormAnswer의 폼지 지원서 ID를 모두 찾았습니다

@Query(value = """
            SELECT DISTINCT fa.application_id
            FROM form_answer fa
            WHERE fa.field_id = :fieldId
            """, nativeQuery = true)
    List<Long> findAllApplicationByFormFieldId(@Param("fieldId") Long fieldId);

2. FormApplication과 FileMetaData를 조인하여 원하는 정보를 모두 가져왔습니다

@Query(value = """
            SELECT fap.id as id, fap.name as name, fmd.file_name as fileName
            FROM file_meta_data fmd
            JOIN form_application fap
            ON fmd.entity_id = fap.id
            WHERE fmd.domain_type = :domainType
            AND fmd.entity_id IN (:applicationIds)
            AND fmd.file_status = :fileStatus
            ORDER BY fmd.file_name
            """, nativeQuery = true)
    List<FileApplicationInfo> findAllFileApplicationInfo(
            @Param("domainType") String domainType,
            @Param("applicationIds") List<Long> applicationIds,
            @Param("fileStatus") String fileStatus
    );

위 프로세스를 실행하기 위해 바뀐 사이드 이펙트
기존 File 저장시, FileMetaData의 entityId를 생성과 삭제를 단순하게 하기 위해 formId로 통일시켜 저장했습니다.
하지만 그로인해 지원자 개인의 file을 찾지 못했고, formApplicationId로 저장하도록 수정하였습니다.

해결해야 할 점 1.
근데, 이전에 formId로 저장되어 있는 코어 동아리 약 10개정도 파일이 있는데, formApplicationId를 직접 수정하려 했으나, 어떤 지원자 것인지 확인 불가하여 바꾸지 못했습니다.
그래서 코아 동아리가 필요 없어지면 바꿔야 할 것 같아요

해결해야할 점 2.
삭제시, formApplication 수만큼 쿼리를 날리고 있도록 로직이 작성되어 있어 //TODO 로 해결해야할 점을 적어놨습니다
폼지 삭제 관련 부분이라 급한 것이 아니고, pr 크기가 커질 수 있어 나중으로 미뤘습니다

Summary by CodeRabbit

  • 새로운 기능
    • 텍스트 및 파일 응답 데이터를 기반으로 한 통계 생성 기능이 개선되었습니다.
    • 파일 응답 정보 조회를 위한 새로운 쿼리 기능과 서비스가 추가되어 파일 관련 데이터 처리가 강화되었습니다.
    • 폼 삭제 시 파일 메타데이터 처리 로직이 개선되었습니다.
  • 리팩토링
    • 응답 데이터 변환 로직이 단순화되어 코드 일관성이 향상되었습니다.
    • 메서드 파라미터가 간소화되어 로직의 명확성이 향상되었습니다.
  • 테스트
    • 파일 응답 정보 조회 기능을 검증하는 새로운 테스트 케이스가 추가되었습니다.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 5, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

이 PR에서는 폼 통계 및 파일 응답 관련 로직의 전반적인 개선이 이루어졌습니다.

  • FormStatisticServiceImpl에서는 텍스트와 파일 통계 생성 로직이 리팩토링되어, SingleStatisticsQuery의 정적 팩토리 메서드를 활용하도록 변경되었습니다.
  • FormAnswerRepository에 두 개의 새로운 쿼리 메서드가 추가되었습니다.
  • FacadeCentralFormServiceImpldeleteForm 메서드에서 파일 메타데이터 업데이트 호출이 제거되었으며,
  • FacadeUserFormApplicationServiceImpl의 파일 메타데이터 상태 업데이트 로직이 단순화되었습니다.
    또한, 파일 응답 정보를 위한 인터페이스와 관련 테스트 클래스, 서비스 클래스가 신규 추가되었습니다.

Changes

파일 경로 변경 요약
src/main/java/ddingdong/.../form/service/
- FormStatisticServiceImpl.java
- FacadeCentralFormServiceImpl.java
- dto/query/SingleFieldStatisticsQuery.java
텍스트 및 파일 통계 생성 로직 리팩토링, 정적 팩토리 메서드(fromTextInfo, fromFileInfo) 추가, 파일 메타데이터 업데이트 호출 제거, 메서드 서식 수정
src/main/java/ddingdong/.../formapplication/repository/
- FormAnswerRepository.java
- FormApplicationRepository.java
새로운 쿼리 메서드(findAllAnswerByFormFieldId, findAllFileAnswerInfo) 추가, import 재정리 및 불필요한 공백 제거
src/main/java/ddingdong/.../formapplication/service/
- FacadeUserFormApplicationServiceImpl.java
- FileFormAnswerService.java
createFormApplication 내 파일 메타데이터 상태 업데이트 로직 개선 및 파라미터 정리, 파일 응답 정보 조회를 위한 서비스 클래스 추가
src/main/java/ddingdong/.../formapplication/repository/dto/
- FileAnswerInfo.java
파일 응답 정보를 제공하는 인터페이스 신규 추가 (메서드: getId(), getName(), getFileName())
src/test/java/ddingdong/.../formapplication/repository/
- FormApplicationRepositoryTest.java
폼 어플리케이션의 파일 응답 정보 조회 기능을 검증하는 새로운 테스트 클래스 추가

Sequence Diagram(s)

sequenceDiagram
    participant Service as FormStatisticServiceImpl
    participant Repo as FormAnswerRepository
    participant FileService as FileFormAnswerService

    Service->>Repo: findAllAnswerByFormFieldId(formField.getId())
    Repo-->>Service: List<Long> answerIds
    Service->>FileService: getAllFileApplicationInfo(answerIds)
    FileService-->>Service: List<FileAnswerInfo>
    Service->>Service: FileAnswerInfo를 fromFileInfo로 변환하여 SingleStatisticsQuery 생성
Loading
sequenceDiagram
    participant UserFormService as FacadeUserFormApplicationServiceImpl
    participant Repo as fileMetaDataRepository

    UserFormService->>UserFormService: formAnswers 중 파일 타입 필터링
    UserFormService->>Repo: updateStatusToCoupled(formAnswer.getId())
Loading

Possibly related PRs

Suggested labels

D-2

Suggested reviewers

  • wonjunYou
  • 5uhwann
  • Seooooo24

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5ea86cb and cd339cb.

📒 Files selected for processing (1)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java (3 hunks)

🪧 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.
  • @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.

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
Copy Markdown

@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

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 279e9c9 and 6ef0f59.

📒 Files selected for processing (2)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImpl.java (2 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepository.java (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepository.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Build and analyze
🔇 Additional comments (5)
src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImpl.java (5)

7-7: FileMetaData 엔티티를 적절히 참조하는 임포트 추가됨.

createFileStatistics 메서드에서 파일 메타데이터를 사용하기 위한 필수 임포트입니다.


139-139: 변수명이 더 의미론적으로 개선됨.

변수명을 textStatisticsQueries에서 singleStatisticsQueries로 변경한 것은 기능 변경에 맞게 적절하게 수정되었습니다. 이제 이 변수는 텍스트만이 아닌 파일 이름을 포함한 통계 데이터를 저장하게 되었습니다.


149-149: 반환 변수 변경에 맞게 적절히 수정됨.

변수명 변경에 맞게 반환 변수도 적절히 수정되었습니다.


154-154: 일관된 코드 스타일로 형식 수정됨.

공백을 추가하여 코드 스타일이 더 일관되게 변경되었습니다.


136-150:

Details

❓ Verification inconclusive

전체 메서드 구현에 대한 기능적 고려 사항

이 메서드는 이제 각 응답에 대해 여러 파일 이름을 반환할 수 있게 되었습니다. 이전 구현(텍스트 기반)과 비교하여 반환되는 항목의 수가 달라질 수 있습니다. 이 변경 사항이 프론트엔드 또는 이 메서드를 호출하는 다른 코드에 영향을 미칠 수 있는지 확인해야 합니다.

또한 파일 메타데이터가 없는 경우(예: 삭제된 파일)에 대한 처리가 명시적으로 되어 있지 않습니다. 이에 대한 예외 처리나 기본값 설정을 고려해보세요.


🏁 Script executed:

#!/bin/bash
# 이 메서드를 호출하는 컨트롤러나 다른 서비스가 있는지 확인
echo "createFileStatistics 메서드의 호출 지점 확인:"
rg -l "createFileStatistics" --type java

Length of output: 379


검토 요약: 메서드 반환 값 변화 및 에러 처리 고려 필요

  • 이 구현 변경으로 인해 하나의 응답이 여러 개의 파일 이름을 반환하게 되어, 이전 텍스트 기반 구현과 결과 개수가 달라질 수 있습니다.
    • 영향을 받는 호출 위치:
      • src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticService.java
      • src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImpl.java
      • src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java
  • 또한, 파일 메타데이터가 조회되지 않는 경우(예: 삭제된 파일)에 대해 별도의 예외 처리나 기본값 설정이 없어, 후속 호출 코드에서 예상치 못한 동작을 유발할 수 있습니다.

이 두 사항이 프런트엔드 및 관련 서비스에 미치는 영향을 재검토하고, 필요시 적절한 예외 처리 또는 기본값 설정을 추가해 주시기 바랍니다.

@github-actions github-actions bot added D-3 D-2 and removed D-3 labels Mar 6, 2025
Copy link
Copy Markdown

@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: 2

♻️ Duplicate comments (1)
src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImpl.java (1)

132-138: 🛠️ Refactor suggestion

파일 통계 생성 로직 개선, 하지만 N+1 문제 가능성 있음

파일 통계 생성 로직이 크게 개선되었습니다. 그러나 PR 설명에서 언급한 대로 "각 응답마다 쿼리를 실행하는" 문제가 여전히 존재할 가능성이 있습니다. 이는 성능 저하를 일으킬 수 있습니다.

더 효율적인 접근 방식으로 다음과 같이 수정하는 것을 고려해보세요:

  public List<SingleStatisticsQuery> createFileStatistics(FormField formField) {
      List<Long> applicationIds = formAnswerRepository.findAllApplicationByFormFieldId(formField.getId());
-     List<FileApplicationInfo> fileApplicationInfos = formApplicationService.getAllFileApplicationInfo(
-             applicationIds);
+     // 빈 리스트인 경우 조기 반환하여 불필요한 쿼리 방지
+     if (applicationIds.isEmpty()) {
+         return List.of();
+     }
+     
+     // 한 번의 배치 쿼리로 모든 파일 응용 프로그램 정보 가져오기
+     List<FileApplicationInfo> fileApplicationInfos = formApplicationService.getAllFileApplicationInfo(applicationIds);
      return fileApplicationInfos.stream()
              .map(SingleStatisticsQuery::fromFileInfo)
              .toList();
  }

또한 FileFormApplicationService 클래스에서 배치 처리 방식을 사용하고 있는지 확인해보세요. 가능하다면 N+1 문제를 방지하기 위해 한 번의 쿼리로 모든 데이터를 가져오는 방식을 구현하는 것이 좋습니다.

🧹 Nitpick comments (2)
src/main/java/ddingdong/ddingdongBE/domain/filemetadata/repository/FileMetaDataRepository.java (1)

64-65: 쿼리 형식 일관성 개선 필요

다른 쿼리들은 여러 줄 형식을 사용하고 있지만, 이 쿼리는 한 줄로 작성되어 있어 일관성이 없습니다. 또한 도메인 타입과 파일 상태에 문자열 리터럴을 사용하고 있는데, 다른 쿼리들처럼 열거형(Enum)을 사용하는 것이 더 타입 안전합니다.

다음과 같이 수정하는 것을 권장합니다:

-    @Query("SELECT f FROM FileMetaData f WHERE (f.domainType = 'BANNER_WEB_IMAGE' OR f.domainType = 'BANNER_MOBILE_IMAGE') AND  f.entityId IN :entityIds AND f.fileStatus != 'DELETED'")
+    @Query("""
+            SELECT f FROM FileMetaData f 
+            WHERE (f.domainType = ddingdong.ddingdongBE.domain.filemetadata.entity.DomainType.BANNER_WEB_IMAGE 
+                OR f.domainType = ddingdong.ddingdongBE.domain.filemetadata.entity.DomainType.BANNER_MOBILE_IMAGE) 
+            AND f.entityId IN :entityIds 
+            AND f.fileStatus != ddingdong.ddingdongBE.domain.filemetadata.entity.FileStatus.DELETED
+            """)
     List<FileMetaData> findAllWithBannerByEntityIds(@Param("entityIds") List<Long> entityIds);
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (1)

30-84: 테스트 케이스가 잘 작성되었습니다만, 일부 검증이 미흡합니다.

테스트 코드는 전반적으로 잘 구성되어 있으며, 명확한 given-when-then 구조를 따르고 있습니다. AssertJ의 assertSoftly를 사용하여 여러 어설션을 효과적으로 그룹화한 점도 좋습니다.

다만, 반환된 모든 FileApplicationInfo 객체의 모든 관련 필드를 검증하지 않고 있습니다. 특히 세 번째 항목(fileApplicationInfos.get(2))의 경우 파일 이름만 확인하고 nameid 필드는 검증하지 않고 있습니다.

다음과 같이 모든 항목의 모든 관련 필드를 검증하도록 개선하는 것이 좋겠습니다:

 assertSoftly(softly -> {
     softly.assertThat(fileApplicationInfos).hasSize(3);
     softly.assertThat(fileApplicationInfos.get(0).getFileName()).isEqualTo(fileMetaData.getFileName());
     softly.assertThat(fileApplicationInfos.get(0).getName()).isEqualTo("이름1");
     softly.assertThat(fileApplicationInfos.get(0).getId()).isEqualTo(savedFormApplication.getId());
     softly.assertThat(fileApplicationInfos.get(1).getFileName()).isEqualTo(fileMetaData2.getFileName());
+    softly.assertThat(fileApplicationInfos.get(1).getName()).isEqualTo("이름1");
+    softly.assertThat(fileApplicationInfos.get(1).getId()).isEqualTo(savedFormApplication.getId());
     softly.assertThat(fileApplicationInfos.get(2).getFileName()).isEqualTo(fileMetaData3.getFileName());
+    softly.assertThat(fileApplicationInfos.get(2).getName()).isEqualTo("이름2");
+    softly.assertThat(fileApplicationInfos.get(2).getId()).isEqualTo(savedFormApplication2.getId());
 });
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6ef0f59 and c05bc14.

📒 Files selected for processing (10)
  • src/main/java/ddingdong/ddingdongBE/domain/filemetadata/repository/FileMetaDataRepository.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java (2 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImpl.java (3 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/query/SingleFieldStatisticsQuery.java (2 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepository.java (2 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepository.java (2 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/dto/FileApplicationInfo.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeUserFormApplicationServiceImpl.java (2 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FileFormApplicationService.java (1 hunks)
  • src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/dto/FileApplicationInfo.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepository.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Build and analyze
🔇 Additional comments (7)
src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/query/SingleFieldStatisticsQuery.java (1)

18-24: 구현이 깔끔하고 좋습니다!

팩토리 메서드를 사용하여 객체 생성 과정을 캡슐화하고 가독성을 높인 점이 좋습니다. 코드 중복을 줄이고 책임을 명확하게 분리했습니다.

src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImpl.java (1)

124-126: 팩토리 메서드를 활용한 좋은 리팩토링입니다

이전 코드보다 훨씬 간결하고 가독성이 높아졌습니다. 팩토리 메서드를 사용하여 객체 생성 로직을 캡슐화한 것은 좋은 접근 방식입니다.

src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FileFormApplicationService.java (1)

1-21: 코드가 잘 작성되었습니다.

새로운 FileFormApplicationService 클래스가 적절하게 구현되었습니다. 단일 책임 원칙(SRP)을 잘 따르고 있으며, 필요한 의존성만 주입받아 사용하고 있습니다. getAllFileApplicationInfo 메서드는 응용 프로그램 ID 목록을 기반으로 파일 응용 프로그램 정보를 검색하는 명확한 역할을 수행합니다.

src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeUserFormApplicationServiceImpl.java (2)

42-42: 매개변수 변경이 적절합니다.

Form 객체 대신 FormApplication 객체를 전달하는 것으로 변경되었습니다. 이는 파일 메타데이터의 연관 관계를 더 정확하게 표현합니다.


53-61: 리팩토링이 잘 이루어졌습니다.

메서드 시그니처를 Form에서 FormApplication으로 변경하고 스트림 API를 활용하여 파일 응답만 필터링하는 방식으로 개선되었습니다. 이는 파일 메타데이터와 폼 응용 프로그램 간의 연관 관계를 명확히 하는 좋은 변경입니다.

src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java (2)

241-242: 들여쓰기 변경이 일관성을 높였습니다.

메서드 파라미터의 들여쓰기 변경은 코드 일관성을 높이는 좋은 변화입니다.


247-248: 들여쓰기 변경이 일관성을 높였습니다.

메서드 파라미터의 들여쓰기 변경은 코드 일관성을 높이는 좋은 변화입니다.

Comment on lines +65 to +79
@Query(value = """
SELECT fap.id as id, fap.name as name, fmd.file_name as fileName
FROM file_meta_data fmd
JOIN form_application fap
ON fmd.entity_id = fap.id
WHERE fmd.domain_type = :domainType
AND fmd.entity_id IN (:applicationIds)
AND fmd.file_status = :fileStatus
ORDER BY fmd.file_name
""", nativeQuery = true)
List<FileApplicationInfo> findAllFileApplicationInfo(
@Param("domainType") String domainType,
@Param("applicationIds") List<Long> applicationIds,
@Param("fileStatus") String fileStatus
);
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 8, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

파라미터 타입 안전성 개선 및 N+1 쿼리 문제 가능성

현재 구현에서는 domainTypefileStatus를 String 타입으로 받고 있어 타입 안전성이 떨어집니다. 열거형(Enum)을 사용하는 것이 더 좋습니다. 또한 이 메서드는 각 응답마다 데이터베이스 쿼리를 실행할 가능성이 있습니다.

다음과 같이 수정하는 것을 권장합니다:

    @Query(value = """
            SELECT fap.id as id, fap.name as name, fmd.file_name as fileName
            FROM file_meta_data fmd
            JOIN form_application fap
            ON fmd.entity_id = fap.id
            WHERE fmd.domain_type = :domainType
            AND fmd.entity_id IN (:applicationIds)
            AND fmd.file_status = :fileStatus
            ORDER BY fmd.file_name
            """, nativeQuery = true)
    List<FileApplicationInfo> findAllFileApplicationInfo(
-           @Param("domainType") String domainType,
+           @Param("domainType") DomainType domainType,
            @Param("applicationIds") List<Long> applicationIds,
-           @Param("fileStatus") String fileStatus
+           @Param("fileStatus") FileStatus fileStatus
    );

이 방식은 다음과 같은 장점을 제공합니다:

  1. 타입 안전성 향상
  2. 오타나 잘못된 값으로 인한 오류 방지
  3. IDE의 자동 완성 지원

실제 서비스 구현 시 해당 메서드를 호출하는 부분에서도 문자열 대신 열거형으로 변환해야 합니다.

📝 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
@Query(value = """
SELECT fap.id as id, fap.name as name, fmd.file_name as fileName
FROM file_meta_data fmd
JOIN form_application fap
ON fmd.entity_id = fap.id
WHERE fmd.domain_type = :domainType
AND fmd.entity_id IN (:applicationIds)
AND fmd.file_status = :fileStatus
ORDER BY fmd.file_name
""", nativeQuery = true)
List<FileApplicationInfo> findAllFileApplicationInfo(
@Param("domainType") String domainType,
@Param("applicationIds") List<Long> applicationIds,
@Param("fileStatus") String fileStatus
);
@Query(value = """
SELECT fap.id as id, fap.name as name, fmd.file_name as fileName
FROM file_meta_data fmd
JOIN form_application fap
ON fmd.entity_id = fap.id
WHERE fmd.domain_type = :domainType
AND fmd.entity_id IN (:applicationIds)
AND fmd.file_status = :fileStatus
ORDER BY fmd.file_name
""", nativeQuery = true)
List<FileApplicationInfo> findAllFileApplicationInfo(
@Param("domainType") DomainType domainType,
@Param("applicationIds") List<Long> applicationIds,
@Param("fileStatus") FileStatus fileStatus
);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

native 쿼리로 작성했기 때문에, String 값을 넘겨야 하더군요~

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@KoSeonJe 네, 말씀하신 대로입니다! Native 쿼리에서는 Enum 대신 String 값을 사용해야 합니다. JPA가 native 쿼리에서는 Enum을 자동으로 String으로 변환해주지 않기 때문이죠. 현재 구현이 적절합니다. 알려주셔서 감사합니다.


✏️ Learnings added
Learnt from: KoSeonJe
PR: COW-dev/ddingdong-be#292
File: src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepository.java:65-79
Timestamp: 2025-03-08T08:34:37.826Z
Learning: In Spring Data JPA, native queries (marked with `nativeQuery = true`) require parameters to be passed as String values rather than Enum types because JPA doesn't automatically convert Enum values to their string representations for native queries.

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.

Comment on lines +99 to +102
List<FormApplication> formApplications = formApplicationService.getAllByForm(form);
formApplications.forEach( //TODO : for문 고치기
formApplication -> fileMetaDataService.updateStatusToDelete(DomainType.FORM_FILE, formApplication.getId())
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

TODO 주석을 해결하세요.

파일 메타데이터 상태를 업데이트하는 로직이 Form에서 FormApplication으로 변경된 것은 좋은 개선입니다. 하지만 코드에 "//TODO: for문 고치기" 주석이 있습니다. 이는 개발자가 forEach 루프를 개선할 필요가 있다고 인식하고 있음을 나타냅니다.

성능과 가독성 향상을 위해 배치 처리 방식으로 변경하는 것을 고려해보세요.

- formApplications.forEach( //TODO : for문 고치기
-         formApplication -> fileMetaDataService.updateStatusToDelete(DomainType.FORM_FILE, formApplication.getId())
- );
+ List<Long> applicationIds = formApplications.stream()
+         .map(FormApplication::getId)
+         .collect(Collectors.toList());
+ if (!applicationIds.isEmpty()) {
+     fileMetaDataService.batchUpdateStatusToDelete(DomainType.FORM_FILE, applicationIds);
+ }

이를 위해 FileMetaDataService에 배치 업데이트 메서드를 추가하는 것을 고려해보세요.

📝 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
List<FormApplication> formApplications = formApplicationService.getAllByForm(form);
formApplications.forEach( //TODO : for문 고치기
formApplication -> fileMetaDataService.updateStatusToDelete(DomainType.FORM_FILE, formApplication.getId())
);
List<FormApplication> formApplications = formApplicationService.getAllByForm(form);
List<Long> applicationIds = formApplications.stream()
.map(FormApplication::getId)
.collect(Collectors.toList());
if (!applicationIds.isEmpty()) {
fileMetaDataService.batchUpdateStatusToDelete(DomainType.FORM_FILE, applicationIds);
}

@KoSeonJe KoSeonJe changed the title [DDING-000] 파일 통계 전체 조회시 파일 이름 반환하도록 수정 [DDING-000] 통계 전체조회시 파일 조회 쿼리 개선 및 응답값 수정 Mar 8, 2025
@github-actions github-actions bot added D-1 and removed D-2 labels Mar 8, 2025
@5uhwann 5uhwann added the 🎯리팩토링 리팩토링 및 고도화 이슈 label Mar 8, 2025
Copy link
Copy Markdown
Collaborator

@5uhwann 5uhwann left a comment

Choose a reason for hiding this comment

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

확인했습니다! 코멘트 확인 부탁드립니다.

);
}
});
formApplication.getId()
Copy link
Copy Markdown
Collaborator

@5uhwann 5uhwann Mar 8, 2025

Choose a reason for hiding this comment

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

p2)
entityId값으로 기존의 formId에서 formApplicationId로 수정하셨는데 formAnswerId로 하지 않으신 이유가 있으실까요?
formApplicationId로 한다면 만일 폼지에 파일 타입 formField가 2개 이상이라면 각각의 formField를 구분할 수 없을거 같습니다.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

그렇네요... 수정하겠습니다

Copy link
Copy Markdown
Collaborator

@Seooooo24 Seooooo24 left a comment

Choose a reason for hiding this comment

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

고생 많으셨습니다!!

Copy link
Copy Markdown

@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 (2)
src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepository.java (1)

46-61: 파일 응답 정보 조회 쿼리가 효율적으로 구현되었습니다.

여러 테이블 간의 조인을 통해 파일 응답 정보를 효과적으로 조회하는 쿼리입니다. 파라미터 설정과 조건절이 적절하게 구성되어 있습니다.

다만, 코드 포맷팅 측면에서 마지막 파라미터 뒤의 닫는 괄호 위치를 조정하면 일관성이 향상될 것입니다.

   List<FileAnswerInfo> findAllFileAnswerInfo(
           @Param("domainType") String domainType,
           @Param("answerIds") List<Long> answerIds,
-          @Param("fileStatus") String fileStatus    );
+          @Param("fileStatus") String fileStatus
+  );
src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (1)

34-89: 테스트 케이스가 체계적으로 작성되었습니다.

이 테스트는 given-when-then 패턴을 명확하게 따르고 있어 가독성이 좋습니다. 테스트 데이터 설정, 메서드 호출, 결과 검증 단계가 잘 구분되어 있습니다.

assertSoftly를 사용하여 모든 검증이 실행되도록 하는 것은 좋은 접근법입니다. 다만, 마지막 assertion에서는 ID 검증이 누락되어 있습니다.

            softly.assertThat(fileAnswerInfos.get(0).getId()).isEqualTo(savedFormAnswer.getId());
            softly.assertThat(fileAnswerInfos.get(1).getFileName()).isEqualTo(fileMetaData2.getFileName());
+           softly.assertThat(fileAnswerInfos.get(1).getName()).isEqualTo(savedFormApplication2.getName());
+           softly.assertThat(fileAnswerInfos.get(1).getId()).isEqualTo(savedFormAnswer2.getId());
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 743ec86 and 5ea86cb.

📒 Files selected for processing (9)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java (3 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImpl.java (3 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/query/SingleFieldStatisticsQuery.java (2 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepository.java (2 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepository.java (0 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/dto/FileAnswerInfo.java (1 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeUserFormApplicationServiceImpl.java (2 hunks)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FileFormAnswerService.java (1 hunks)
  • src/test/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepositoryTest.java (1 hunks)
💤 Files with no reviewable changes (1)
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormApplicationRepository.java
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FormStatisticServiceImpl.java
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/query/SingleFieldStatisticsQuery.java
  • src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java
  • src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FacadeUserFormApplicationServiceImpl.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Build and analyze
🔇 Additional comments (3)
src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/dto/FileAnswerInfo.java (1)

3-11: 인터페이스가 잘 설계되었습니다.

파일 응답 정보를 검색하기 위한 프로젝션 인터페이스가 명확하게 정의되었습니다. 메서드 이름이 의도를 잘 표현하고 있습니다.

src/main/java/ddingdong/ddingdongBE/domain/formapplication/repository/FormAnswerRepository.java (1)

39-44: 쿼리가 적절하게 구현되었습니다.

특정 필드 ID에 해당하는 모든 답변 ID를 고유하게 조회하는 Native Query가 잘 구현되었습니다.

src/main/java/ddingdong/ddingdongBE/domain/formapplication/service/FileFormAnswerService.java (1)

12-21: 서비스 클래스가 잘 설계되었습니다.

FileFormAnswerService 클래스는 단일 책임 원칙을 잘 따르고 있습니다. 정적 임포트를 사용하여 코드 가독성을 향상시켰고, Lombok의 @RequiredArgsConstructor를 통한 생성자 주입 방식을 사용하여 의존성을 효과적으로 관리하고 있습니다.

getAllFileApplicationInfo 메서드는 파라미터로 받은 답변 ID 목록을 기반으로 파일 응답 정보를 조회하는 기능을 명확하게 구현하고 있습니다.

@github-actions github-actions bot added D-0 and removed D-1 labels Mar 9, 2025
Copy link
Copy Markdown
Collaborator

@5uhwann 5uhwann left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

@KoSeonJe KoSeonJe merged commit 572bd86 into develop Mar 10, 2025
1 of 2 checks passed
@KoSeonJe KoSeonJe deleted the fix/DDING-000 branch March 10, 2025 01:59
@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

D-0 🎯리팩토링 리팩토링 및 고도화 이슈

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants