[BLOOM-055] LoginUser, Secured 적용 #66
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
Dompoo
commented
Aug 19, 2024
Dompoo
commented
- 각 api에 LoginUser를 적용하고 user를 사용하도록 수정했습니다.
- 조회 api에서는 user와 연관관계가 있는 것들만 조회합니다.
- 추가 api에서는 user와 연관관계 매핑을 하여 저장합니다.
- 수정/삭제 api에서는 user와 연관관계가 있을 시에만 수정/삭제 됩니다.
stophwan
reviewed
Aug 20, 2024
There was a problem hiding this comment.
- 쿼리들 자체가 다 N+1에 걸릴 수 있는 요소인데, Lazy Loading 될만한 요소가 없는걸로 이해해도 될까요?
- 모든 Controller가 @loginuser를 사용하고 모든 Repository들이 ~byUser를 적용하고 있는데 이유가 있나요? User를 사용하지 않는 Controller, Repository도 User를 사용하고 있는 것 처럼 보여서요! 피드백은 대표적인 몇개만 달았습니다!
stophwan
reviewed
Aug 20, 2024
| ) { | ||
| val image = | ||
| imageRepository.findByIdOrNull(imageId) | ||
| imageRepository.findByIdAndUser(imageId, user) |
There was a problem hiding this comment.
여기 user가 붙여야하는 이유가 있나요? image id를 통해 하나의 이미지만 가져오는거 아닌가요?
There was a problem hiding this comment.
제 이미지가 아닐 수 있으니까요!
제가 지환님 이미지를 조회하면 안되도록 하는거에요.
| ) { | ||
| val myPlant = | ||
| myPlantRepository.findByIdOrNull(myPlantId) | ||
| myPlantRepository.findByIdAndUser(myPlantId, user) |
There was a problem hiding this comment.
여기에도 user가 붙여야하는 이유가 있나요? myPlantId를 통해 하나의 내 식물을 가져오는게 아닌가요?
There was a problem hiding this comment.
위와 동일합니다. 제가 지환님 식물을 조회하면 안되니까 그렇습니당
|
N+1문제는 제가 더 면밀하게 고민해보고 개선해볼게요! ByUser 쪽은 답변 달았습니당 |
확인했습니다! |
stophwan
approved these changes
Aug 20, 2024
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.