Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

서비스 트랜잭션 적용 #834

Open
wants to merge 2 commits into
base: dev/be
Choose a base branch
from
Open

서비스 트랜잭션 적용 #834

wants to merge 2 commits into from
+25 −5

Conversation

HoeSeong123
Copy link
Contributor

@HoeSeong123 HoeSeong123 commented Oct 20, 2024 •
edited
Loading

⚡️ 관련 이슈

close #830

📍주요 변경 사항

� 모든 서비스 로직에 @transactional을 적용했습니다.

🎸기타

  • [REFACTOR] 모든 서비스 로직에 transactional 적용하기 #830 에서 적어놓은 것은 @Transactional을 모든 서비스 클래스에 기본으로 설정해놓고, readOnly가 필요한 메소드만 별도로 @Transactional(readOnly = true) 를 붙여주는 것이었습니다.
  • 하지만 이렇게 할 경우 다른 개발자가 합류했다고 했을 때, 어떤 트랜잭션 옵션이 붙어있는지 확인하기 힘들 수도 있겠다는 생각이 들었습니다.
  • 또한, 이슈에 적은 방법을 제안한 이유는 write인데 readOnly를 붙여서 발생하는 휴먼에러를 방지하기 위함인데, 그렇게 하지 않아도 어차피 발생할 수 있는 오류라고 생각됩니다.
  • 따라서 모든 메소드에 명시적으로 작성하는 방법을 제안하는데 다들 의견이 어떠신가요??

🍗 PR 첫 리뷰 마감 기한

10/21 23:59

@HoeSeong123 HoeSeong123 added refactor 요구사항이 바뀌지 않은 변경사항 BE 백엔드 labels Oct 20, 2024
@HoeSeong123 HoeSeong123 added this to the 6차 스프린트 🦴 milestone Oct 20, 2024
@HoeSeong123 HoeSeong123 self-assigned this Oct 20, 2024
zangsu
zangsu requested changes Oct 20, 2024
View reviewed changes
Copy link
Contributor

@zangsu zangsu left a comment

Choose a reason for hiding this comment

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

고생했어요 초롱!
readOnly 가 사용되지 않은 부분이 하나 있어 남겨 두었어요.
+) AuthService.login() 메서드에서도 DB 조회가 일어나니 이 부분도 추가해 주어야 할 것 같습니다!

backend/src/main/java/codezap/template/service/TemplateService.java
@@ -37,14 +37,17 @@ public Template create(Member member, CreateTemplateRequest createTemplateReques
return templateRepository.save(template);
}

@Transactional
Copy link
Contributor

Choose a reason for hiding this comment

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

얘는 왜 readOnly 가 사용되지 않죠?

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

@zangsu zangsu zangsu requested changes

@zeus6768 zeus6768 Awaiting requested review from zeus6768

@jminkkk jminkkk Awaiting requested review from jminkkk

@kyum-q kyum-q Awaiting requested review from kyum-q

Requested changes must be addressed to merge this pull request.

Assignees

@HoeSeong123 HoeSeong123

Labels
BE 백엔드 refactor 요구사항이 바뀌지 않은 변경사항
Projects
2024-code-zap
Status: In Progress
Milestone
6차 스프린트 🦴
Development

Successfully merging this pull request may close these issues.
2 participants
@HoeSeong123 @zangsu