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: admin 도메인 repository 함수 재사용성 향상, 테스트 코드 일관성, 최적화 #14

Merged
merged 10 commits into from
Jan 14, 2025

Conversation

Jo-Minseok
Copy link
Collaborator

@Jo-Minseok Jo-Minseok commented Jan 13, 2025

🔨 테스크

작업 필요성

  • 테스트를 마지막쯤에 작성해서 너무 막 적은 감이 있었다.
  • Admin 도메인에 대해 적용해보고 팀원들이 동의할 경우 다른 도메인에도 적용할 계획이다. PR 룰 (추가, 삭제 400줄 이내)를 지키기 위해 Admin 도메인에만 사전 적용한다.
  • Admin 테스트 픽스처를 활용하지 않고 있었다.
  • Admin 도메인의 테스트 사전 단계(beforeAll)에서 서비스 계층으로 테스트 데이터를 삽입하던 부분을 Repository 계층에서 바로 삽입하게 바꿨다.

다음 작업

Admin 픽스처 수정

  • 비밀번호를 암호화한 걸 들고 있을 이유가 없다.
  • 매번 사용할 때마다 암호화한 결과를 개발자가 알고 있어야 하는데, 상당히 번거롭다. 서비스에 실제로 대입을 해봐야한다. 그렇기에 비밀번호 평문을 넣을때 암호화되게 하여 사용성을 높였다.
  • 해싱 횟수 10회나 1회나 실제로 동작하는 시간은 별 차이 없었지만, 이론상 더 빠를 것으로 판단하여 기존 서비스에서는 10번 해싱을 1번 해싱으로 최적화 했다. (1 Core 2GB, 8 Core 32GB 환경에서 수행시 1회나 10회 동일함을 확인)

컨트롤러 -> 서비스 계층 DTO, 개별 인자

  • DTO를 사용해야 팀에서 나온 방식인 DTO -> ENTITY를 수행하는 함수를 사용할 수 있었다. 코드 일관성을 위해 전부 DTO를 전달하도록 변경했다.
  • Parameter, Body를 함께 받는 곳은 확인하기 어려워 변수명 가운데 Param, Query, Body를 추가해서 이해도를 향상시켰다.

📋 작업 내용

  • Admin Repository 전처리, 데이터 삽입 함수 제거
  • Admin 회원가입 dto toEntity 함수 추가
  • bcrypt types 설치
  • Admin 픽스처 활용도 향상(비밀번호 암호화 객체, 비밀번호 평문 객체 반환하도록 변경)
  • 모든 DTO 변수 명칭 자세하게 변경(❗오버 테스크❗)
  • typeorm save -> insert로 변경하여 성능 개선

@Jo-Minseok Jo-Minseok self-assigned this Jan 13, 2025
@Jo-Minseok Jo-Minseok added 🔨 Refactor 리팩토링 (구조 변경) ✅ Test 테스트 관련 (storybook, vitest, jest 등) labels Jan 13, 2025
@Jo-Minseok Jo-Minseok changed the title ♻️ refactor: Admin 도메인 Repository 변수 대입 형식 -> 엔티티 DTO 변환 형식으로 변경, 테스트 코드 최적화 ♻️ refactor: admin 도메인 repository 함수 재사용성 향상, 테스트 코드 최적화 Jan 13, 2025
Copy link
Collaborator

@CodeVac513 CodeVac513 left a comment

Choose a reason for hiding this comment

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

디스코드에서 말씀하셨던 내용들이 코드 읽으면서 이해가 잘 되는 것 같습니다.
사소한 의견 하나 남겨놨고, Service는 진짜 사용할 필요가 없었네요.
수고많으셨습니다! 😄

server/test/fixture/admin.fixture.ts Outdated Show resolved Hide resolved
Copy link
Member

@asn6878 asn6878 left a comment

Choose a reason for hiding this comment

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

확인했습니다! 꼼꼼하게 작업 잘 해주셔서 감사해요 🙇
@CodeVac513 님 말씀대로 테스트에 사용되는 hash 동작은 너무 무거워지지 않으면 좋을 것 같네요!

궁금한점이 있어서 질문을 남겨두었습니다. 어떻게 생각하시는지 알려주시면 감사하겠습니다~

server/test/admin/e2e/login.e2e-spec.ts Outdated Show resolved Hide resolved
@Jo-Minseok Jo-Minseok changed the title ♻️ refactor: admin 도메인 repository 함수 재사용성 향상, 테스트 코드 최적화 ♻️ refactor: admin 도메인 repository 함수 재사용성 향상, 테스트 코드 일관성, 최적화 Jan 14, 2025
@Jo-Minseok Jo-Minseok merged commit 30c258c into main Jan 14, 2025
1 check passed
@Jo-Minseok Jo-Minseok deleted the refactor/repository-reuse-admin branch January 14, 2025 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 Refactor 리팩토링 (구조 변경) ✅ Test 테스트 관련 (storybook, vitest, jest 등)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants