Conversation
Summary of ChangesHello @JayongLee, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 블로그 및 프로젝트 관리 기능을 대폭 확장하고 개선하는 것을 목표로 합니다. 블로그 게시글의 생성, 조회, 수정, 삭제 기능을 완성하고, 프로젝트 멤버 관리 기능을 추가하여 사용자 경험을 향상시켰습니다. 또한, 데이터 모델과 API 응답 구조를 재정비하고, 인증 및 권한 처리 로직을 강화하여 시스템의 안정성과 유연성을 높였습니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| public void deleteProject(Long projectId, Long leaderId) { | ||
| Project project = findProjectBy(projectId); | ||
| // TODO : Spring Interceptor를 적용하여 추후 분리 | ||
| checkAccessPermission(projectId, leaderId); | ||
| List<ProjectMember> projectMembers = projectMemberRepository.findAllByProject(project); | ||
| projectMemberRepository.deleteAll(projectMembers); | ||
| projectRepository.delete(project); |
There was a problem hiding this comment.
프로젝트 삭제 시 해당 프로젝트에 속한 블로그 게시글들을 처리하지 않아 외래 키 제약 조건 위반으로 DataIntegrityViolationException이 발생할 수 있습니다. 프로젝트에 속한 Blog 엔티티를 먼저 삭제한 후, ProjectMember와 Project를 삭제해야 합니다. 또한, 불필요한 데이터베이스 조회를 피하기 위해 권한 확인 로직을 먼저 수행하는 것이 좋습니다.
public void deleteProject(Long projectId, Long leaderId) {
// TODO : Spring Interceptor를 적용하여 추후 분리
checkAccessPermission(projectId, leaderId);
Project project = findProjectBy(projectId);
blogRepository.deleteAll(blogRepository.findAllByProject(project));
List<ProjectMember> projectMembers = projectMemberRepository.findAllByProject(project);
projectMemberRepository.deleteAll(projectMembers);
projectRepository.delete(project);
}| */ | ||
| @Transactional | ||
| public BlogDetailInfoRes getBlogInfo(Long postId) { | ||
| public BlogDetailInfoRes getBlogInfo(Optional<Long> memberId, Long postId) { |
There was a problem hiding this comment.
로그인한 사용자가 글 작성자인지 확인하는 로직을 getBlogInfo 메서드에 추가하셨는데, 이 로직을 블로그 수정(updateBlog) 및 삭제(deleteBlog) 시에도 활용할 수 있도록 findBlogAndCheckOwnerShip 메서드 내에서 처리하는 것이 더 일관성 있고 재사용성이 높습니다. 현재는 updateBlog와 deleteBlog에서만 소유권 확인을 하고 있어, getBlogInfo에서도 동일한 로직을 사용하면 중복을 줄일 수 있습니다.
| Project project = findProjectBy(projectId); | ||
| Member requestMember = findMemberBy(memberId); | ||
| // TODO : Spring Interceptor를 적용하여 추후 분리 | ||
| checkAccessPermission(projectId, leaderId); |
There was a problem hiding this comment.
데이터베이스에서 Project와 Member 엔티티를 조회하기 전에 권한 확인을 먼저 수행하는 것이 더 효율적입니다. 사용자가 권한이 없는 경우 불필요한 데이터베이스 조회를 피할 수 있습니다.
// TODO : Spring Interceptor를 적용하여 추후 분리
checkAccessPermission(projectId, leaderId);
Project project = findProjectBy(projectId);
Member requestMember = findMemberBy(memberId);| private boolean isPermit(final HttpServletRequest request) { | ||
| String uri = request.getRequestURI(); | ||
|
|
||
| if ("GET".equalsIgnoreCase(request.getMethod())) { | ||
| // DENY_URL 리스트에 존재하는 URL 요청일 경우 거부 | ||
| if (DENY_URL.stream().anyMatch(deny -> pathMatcher.match(deny, uri))) { | ||
| return false; | ||
| } | ||
| // PERMIT_URL 리스트에 존재하는 URL 요청일 경우 허용 | ||
| if (PERMIT_URL.stream().anyMatch(permit -> pathMatcher.match(permit, uri))) { | ||
| return true; | ||
| } | ||
| } | ||
| // DENY_URL, PERMIT_URL 외의 WhiteList URL들의 경우엔 허용 | ||
| return whiteList.stream().anyMatch(pattern -> pathMatcher.match(pattern, uri)); | ||
| } |
There was a problem hiding this comment.
isPermit 메서드의 인증/인가 로직이 복잡하고 여러 리스트(DENY_URL, PERMIT_URL, whiteList)에 분산되어 있어 유지보수가 어렵습니다. 특히 GET 요청에 대한 특별 처리는 전체적인 규칙을 파악하기 어렵게 만듭니다. Spring Security의 SecurityFilterChain 설정 내에서 requestMatchers를 사용하여 선언적으로 경로와 HTTP 메서드에 따른 접근 제어 규칙을 중앙에서 관리하는 것이 더 명확하고 일반적인 방법입니다. 이렇게 하면 코드의 가독성이 향상되고 보안 규칙을 한 곳에서 관리할 수 있습니다.
2 participants
No description provided.