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.

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

Refactor/#82 : Auth Domain 리팩토링 #83

Merged
merged 6 commits into from
Apr 15, 2024
Merged

Refactor/#82 : Auth Domain 리팩토링 #83

merged 6 commits into from
Apr 15, 2024

Conversation

qlido
Copy link
Member

@qlido qlido commented Apr 14, 2024

수정한 내용과 이유

  1. Login과 Refresh 로 분리된 서비스를 Command로 분리함.

  2. 비즈니스 흐름이 잘보이도록 Implementation Layer로 분리함

  3. Controller 에서 RequestHeader로 받고 있던 정보들을 Request Body에서 받도록 했습니다

Header는 요청에대한 부가정인 정보를 받는 곳이에요. 동작을 하기위해 필요한 정보

  1. User에 Implementation레이어를 추가함. Auth에서 필요한 일부 정보만 추가했어요.

close #82

@qlido qlido self-assigned this Apr 14, 2024
Comment on lines 16 to 26
public void create(BsmResourceResponse resource) {
userRepository.save(
User.builder()
.email(resource.getEmail())
.nickName(resource.getNickname())
.authority(Authority.USER)
.enroll(resource.getStudent().getEnrolledAt())
.name(resource.getStudent().getName())
.build()
);
}
Copy link
Contributor

@hw9402 hw9402 Apr 15, 2024

Choose a reason for hiding this comment

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

dto를 넘겨서 저장하기 보단 비즈니스 레이어에서 엔티티로 변환 후 User를 매개변수로 받아서 저장하는 건 어떨까요? 이러한 방법이 재사용성을 더 높일 수 있을 것 같아요

Copy link
Member Author

Choose a reason for hiding this comment

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

어떤 방식인지 예시를 들어주실 수 있나요?

Copy link
Member

Choose a reason for hiding this comment

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

저는 handler에서 가져올 때 바로 User로 가져오면 좋겠다는 생각이 들어요. 그렇게 하면, 다른 implementation레이어나 service레이어는 BSM의 상세 구현에 대한 의존 없이 작동할 수 있기 때문에 유지보수성이 좋아질 거라고 생각해요.

Copy link
Contributor

Choose a reason for hiding this comment

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

public void create(User user) {
    userRepository.save(user);
}

제가 말한 방식은 이러한 방식이었습니다

비즈니스 레이어에서 dto.toEntity()와 같은 방법으로 dto를 entity로 변환 후 create(dto.toEntity()) 이런식으로 메서드를 호출한다면 재사용성이 늘어날 것 같습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

toEntity 쓰고싶지만 라이브러리라 사용 할 수 없어요ㅜㅜ

@jacobhboy jacobhboy self-requested a review April 15, 2024 05:06
jacobhboy
jacobhboy previously approved these changes Apr 15, 2024
Copy link

sonarcloud bot commented Apr 15, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@qlido qlido requested a review from jacobhboy April 15, 2024 05:40
@qlido qlido enabled auto-merge April 15, 2024 05:40
@qlido qlido merged commit ce8397b into master Apr 15, 2024
2 of 3 checks passed
@qlido qlido deleted the refactor/#82 branch April 15, 2024 05:41
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.

Auth 도메인을 리팩토링 해야합니다
3 participants