Skip to content

[Feat#17] 프로젝트 리스트 출력 api 구현#25

Open
choeunbin03 wants to merge 6 commits into
developfrom
feat/projects-get
Open

[Feat#17] 프로젝트 리스트 출력 api 구현#25
choeunbin03 wants to merge 6 commits into
developfrom
feat/projects-get

Conversation

@choeunbin03
Copy link
Copy Markdown
Collaborator

💻 관련 이슈
관련된 이슈 번호를 적어주세요.

✨ 작업 내용
이번 PR에서 작업한 내용을 간단하게 적어주세요.

내 프로젝트 리스트 출력
프로필 페이지에서의 프로젝트 리스트 출력

📷 스크린샷
관련된 스크린샷이 있다면 첨부해 주세요.

✅ 체크리스트
PR을 보내기 전에 아래 항목들을 확인해 주세요.

코드 컨벤션 준수
빌드 성공
로직 자체 테스트 완료
불필요한 파일/코드 제거(개행 정리)
이슈 번호 연결 확인
EOL(End Of Line) 확인
❕리뷰 요구 사항
리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해 주세요.

문제 있어보이는 부분 있으면 편하게 피드백 부탁드립니다!!

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces new endpoints to retrieve a user's own projects and public project lists for specific users, supported by new DTOs and repository methods. It also enhances API documentation with Swagger annotations and refactors the ProjectController for path variable consistency. Key feedback highlights a potential N+1 query performance issue in the getMyProjects service method and identifies a redundant security configuration for the login endpoint that should be cleaned up.

Comment on lines +81 to +89
return myMemberships.stream()
.map(pm -> {
List<ProjectMember> otherMembers = projectMemberRepository.findAllByProject(pm.getProject())
.stream()
.filter(m -> !m.getUser().getId().equals(pm.getUser().getId()))
.toList();
return toMyResponse(pm, otherMembers);
})
.toList();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

getMyProjects 메서드에서 N+1 쿼리 문제가 발생할 수 있습니다. myMemberships를 순회하면서 각 ProjectMember에 대해 projectMemberRepository.findAllByProject()를 호출하고 있어, 사용자가 속한 프로젝트의 수(N)만큼 추가적인 데이터베이스 조회가 발생합니다. 사용자가 많은 프로젝트에 속할수록 성능이 저하될 수 있습니다.

이 문제를 해결하려면 사용자가 속한 모든 프로젝트의 멤버 목록을 한 번의 쿼리로 가져온 후, 애플리케이션 메모리에서 데이터를 가공하는 것이 좋습니다.

개선 방안:

  1. 사용자가 속한 모든 프로젝트 목록을 조회합니다.
  2. IN 절을 사용하여 해당 프로젝트들에 속한 모든 멤버를 한 번의 쿼리로 가져옵니다. (ProjectMemberRepositoryfindAllByProjectIn(List<Project> projects) 같은 메서드 추가 필요)
  3. 가져온 멤버 목록을 Map<Long, List<ProjectMember>> 형태로 프로젝트 ID별로 그룹화합니다.
  4. Map을 사용하여 각 프로젝트의 멤버 정보를 조합하여 응답 DTO를 생성합니다.

이렇게 수정하면 데이터베이스 접근 횟수를 크게 줄여 성능을 개선할 수 있습니다.

.requestMatchers( "/swagger-ui/**", "/v3/api-docs/**").permitAll()
.requestMatchers("/api/v1/login/**").permitAll()
.requestMatchers("/api/v1/form-data/**").permitAll()
.requestMatchers("/api/v1/login/**", "/api/v1/projects/users/**").permitAll()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

보안 설정에서 "/api/v1/login/**" 경로에 대한 permitAll() 규칙이 중복으로 선언되었습니다. 43번째 줄에 이미 동일한 규칙이 있으므로, 이 줄에서는 새로운 경로인 "/api/v1/projects/users/**"만 허용하도록 수정하는 것이 좋습니다. 중복된 설정은 코드를 혼란스럽게 만들고 유지보수를 어렵게 할 수 있습니다.

Suggested change
.requestMatchers("/api/v1/login/**", "/api/v1/projects/users/**").permitAll()
.requestMatchers("/api/v1/projects/users/**").permitAll()

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.

1 participant