Skip to content

Conversation

@nayonsoso
Copy link
Collaborator

@nayonsoso nayonsoso commented Feb 11, 2025

관련 이슈

작업 내용

회의에 나온 내용들을 반영했습니다😊

특이 사항

URI 복수형 사용

  • 게시판 관련 uri를 /communites/** 에서 /boards/**로 변경
  • 지원 관련 uri를 /application/** 에서 /applications/**로 변경
  • 어학 성적 관련 uri를 /score/languageTest/**에서 scores/language-tests/**로 변경
  • 학점 관련 uri를 /score/gpa/**에서 /scores/gpas/**로 변경

request, response 변경

  • gpa 성적 등록, 어학 성적 등록 시 [파일 업로드 → url 제출]이 아니라 바로 파일을 업로드하도록 변경
  • 댓글 작성 시 postId 함께 제출하도록 변경
  • 게시글 작성 시 boardCode 함께 제출하도록 변경
  • 게시판 조회(PostFindResponse)시 boardCode 함께 응답하도록 변경

리뷰 요구사항 (선택)

❗️조금 치명적인 이슈가 있습니다🥲❗️
컨트롤러에서 SiteUser 를 인자로 받기 이전에는,
서비스 코드에서 직접 SiteUser 를 조회해와서, 다른 비지니스 로직들과 동일한 영속성 컨텍스트에 적재되었습니다.
그래서 Lazy Loading 을 사용할 수 있었어요.

하지만 지금은 컨트롤러에서 SiteUser를 가져오기 때문에,
서비스 코드에서 이 SiteUser 는 영속 상태가 아니라 Lazy Loading이 적용되지 않습니다.
이 PR에서는 가장 간편한 해결방법으로 해결하긴 했는데, 추가로 논의가 필요할 것 같습니다.

이건 오늘 회의 끝나고 이야기해봐도 좋을 것 같습니다.

❗️api 변경 사항❗️
https://github.com/solid-connection/api-docs/pull/5

Copy link
Contributor

@Gyuhyeok99 Gyuhyeok99 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 지난번 #171 보면서도 영속성 컨텍스트 관련 이슈가 발생할 위험이 있겠다 생각했었는데 이렇게 발생했네요 🥲

그때부터 고민해봤는데 지금 구현하신 것처럼 siteUserRepository.findById를 이용해서 siteUser를 영속 상태로 관리하는 게 가장 직관적으로 JPA의 변경 감지를 확실하게 보장할 수 있다고 생각했어요.

그래서 저는 말씀하신 siteUser를 영속 상태로 만들 수 있도록 컨트롤러에서 siteUserId 를 넘겨주는 게 제일 좋은 거 같습니다.

  • gpa 와 siteUser 사이의 양방향을 끊을 것인지 생각해봐야한다.

그리고 혹시 이 문제가 gpa 와 siteUser 사이의 양방향/단방향이 영속성 문제랑 어떤 관련이 있는지 알려주실 수 있나요??

Comment on lines +34 to +37
@GetMapping("/{code}")
public ResponseEntity<?> findPostsByCodeAndCategory(
@PathVariable(value = "code") String code,
@RequestParam(value = "category", defaultValue = "전체") String category) {
Copy link
Contributor

Choose a reason for hiding this comment

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

이건 게시글 리스트를 조회하는 것이길래 제가 지난번에 post쪽으로 옮겼던 것인데 boards가 더 적절한가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

저렇게 바꾸게 된 맥락을 말씀드리자면!
원래 특정 게시판의 게시글 목록 조회가 /communities/{code}였었어요.
그런데 uri 회의에서 communities 였던걸 boards 로 바꾸기로 해서 /boards/{code} 로 바꾼거예요.

Copy link
Contributor

Choose a reason for hiding this comment

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

아 기억났습니다! ㅎㅎ.. 깜빡했네요

Comment on lines 42 to +45
private final S3Service s3Service;
private final RedisService redisService;
private final RedisUtils redisUtils;
private final SiteUserRepository siteUserRepository;
Copy link
Contributor

Choose a reason for hiding this comment

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

늘 신경써본적 없는 부분이긴 한데 주입 순서 컨벤션을 정해보는 건 어떻게 생각하시나요? 예를들어 주요 도메인 레포지토리 -> 연관된 도메인 레포지토리 -> 서비스 -> 유틸 이런 식으로요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

좋은 생각입니다!
이 컨벤션은 따로 정해봐요~ 😊

@nayonsoso
Copy link
Collaborator Author

그래서 저는 말씀하신 siteUser를 영속 상태로 만들 수 있도록 컨트롤러에서 siteUserId 를 넘겨주는 게 제일 좋은 거 같습니다.

네 저도 그게 좋다 생각해요.
코드 변경이 많아지긴 하겠지만.. 좋은 코드로 가기 위한 시행착오겠죠? 🥹

gpa 와 siteUser 사이의 양방향을 끊을 것인지 생각해봐야한다.
그리고 혹시 이 문제가 gpa 와 siteUser 사이의 양방향/단방향이 영속성 문제랑 어떤 관련이 있는지 알려주실 수 있나요??

지금 DB만 생각하면 gpa 가 site_user_id 를 fk 로 가지고 있어요.
그래서 JPA 단방향으로 Gpa 엔티티가 @ManyToOne으로 SiteUser 를 참조하는건 필수적이에요.
그런데 SiteUser 가 List<Gpa>@OneToMany로 갖는건 DB의 관계와 관련없이,
사용자의 gpa 조회를 편리하게 하기 위해 걸어둔 양방향 관계입니다.
OneToMany 은 기본적으로 FetchType.LAZY 이기 때문에
영속 상태의 SiteUser 객체에 대해서 get 해야 그제야 DB에서 가져오는 방식입니다.

그런데 컨트롤러에서 영속 상태가 아닌 (정확히는 준영속 상태의) siteUser 엔티티를 서비스에 내려주니 레이지 로딩이 동작하지 않는 것입니다.
서비스 코드 입장에서는 SiteUser 가 영속 객체가 아닌 그냥 자바 객체일 것이니까요.

@Gyuhyeok99
Copy link
Contributor

아 그러네요 이해했습니다! 꼭 필요한 곳에만 양방향을 걸어두는 게 좋긴하겠네요. 그래도 양방향이 필요한 곳은 있을테니 컨트롤러에서 siteUserId 를 넘겨주는 게 제일 좋긴 하겠네요 🥹

Copy link
Contributor

@Gyuhyeok99 Gyuhyeok99 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
Member

@wibaek wibaek left a comment

Choose a reason for hiding this comment

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

lgtm

프론트도 바로 반영 될 수 있게 작업 완료되었습니다.

@nayonsoso nayonsoso merged commit ba5193b into develop Feb 14, 2025
@nayonsoso nayonsoso deleted the refactor/181-application-community-score branch February 15, 2025 19:16
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.

refactor: uri 회의에서 나온 내용 적용

4 participants