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

관리자 기능 백엔드 구현 #780

Merged
merged 89 commits into from
Feb 3, 2024
Merged

관리자 기능 백엔드 구현 #780

merged 89 commits into from
Feb 3, 2024

Conversation

LJW25
Copy link
Collaborator

@LJW25 LJW25 commented Jan 20, 2024

#778

설명을 하기 전에 앞서..
빠르게 진행하려 욕심을 부리다 보니 한 PR에 너무 많은 녀석들이 들어가버렸습니다..죄송합니다..그치만 진짜 빨랐음
아래에 두 파트로 나누어 커밋 내역 링크 달아놓았으니 그쪽으로 보시면 더 편하게 확인 가능합니다!


해당 PR은 크게 두가지 파트로 나뉩니다!

  1. 관리자 로그인 구현 (커밋)
  2. 기본 데이터 (City, Category, Currency) 조회, 추가, 수정 구현 (커밋)

디스커션을 통해 논의한 패키지 구조의 경우 다수의 의견에 따라 1번으로 구현하였으며,
하나의 패키지에서 모든 관리자 API를 관리하는 대신 Controller를 각각의 세부 도메인에 따라 나눠 책임을 분리해 주었습니다.


1. 관리자 로그인 구현

1.1. 요약

관리자 로그인의 핵심 포인트는 3가지입니다.

  1. 관리자는 Master, Admin 두가지 등급으로 나뉘고, Master가 다른 관리자 계정을 추가할 수 있다.
  2. 관리자 로그인에서 인증은 OAuth를 사용하지 않고 자체구현 하였으며, BCrypt를 사용한 비밀번호 암호화를 사용했다.
  3. 인가는 기존에 사용하던 구조를 재사용하였다.

1.2. 세부 구조

  1. 관리자 계정은

    1. 일반 데이터 관리 + 모든 관리자 계정을 관리할 수 있는 Master 계정과
    2. 일반 데이터 관리를 담당하는 Admin 계정으로 나뉩니다.
  2. 기존의 일반 사용자 로그인에 사용되던 @Auth와 구분짓기 위해, 관리자 로그인용 어노테이션 @AdminAuth를 구현하였습니다. 반환값은 동일하게 Accessor를 사용하며, Accessor의 Authority 등급에 Master, Admin을 추가해주었습니다. 즉, @AdminAuthMaster 등급의 Accessor 혹은 Admin 등급 Accessor를 반환합니다.

  3. 인가는 기존에 사용하던 JwtProvider, RefreshTokenRepository를 동일하게 적용하였습니다. 해당 과정에서 일반 Member의 Id와 관리자 AdminMember의 Id가 겹치며 문제가 생길 여지를 고려하였는데, 결론적으로 현재의 구조에서는 문제가 발생하지 않는 것으로 확인하였습니다. 해당 부분에 대한 구체적인 내용은 노션-관리자 페이지 백엔드 개발 을 참고해주세요.

1.3. 고민

현재 Master와 Admin을 구분하는 것을 ArgumentResolver에서 admin_member 테이블을 확인하는 방식으로 구현하였습니다. 이 경우 @Adminauth 어노테이션을 사용하는 매 작업마다 DB를 확인해야 한다는 문제가 있습니다.
만약 이를 @Masteronly 어노테이션에서 DB를 확인하도록 구현하면 DB를 확인하는 수는 적지만, 기존에 사용하던 Accessor 코드를 적용하기 어려워 재사용성이 떨어집니다.

관리자 API는 일반 API 보다 사용되는 빈도가 적기 때문에 미세한 성능 보다 코드의 간결함을 살리는 방향으로 우선 구현하였습니다. 이 부분에 대해서 의견 주시면 감사하겠습니다!


2. 기본 데이터 CRU API 구현

2.1. 요약

  1. @AdminOnly 어노테이션을 통해, Admin과 Master 계정만 기본 데이터의 CRU를 수행할 수 있다.
  2. Delete의 경우 연동된 여행들에 영향을 미칠 위험이 크기 때문에 API로 구현하지 않았다.
  3. Currency 조회 는 포함된 데이터가 많기 때문에 페이지로 구현하였다.

2.2. 세부 구조

  1. 기본 데이터의 조회, 추가, 수정 API는 @AdminAuth, @AdminOnly 어노테이션이 적용되었습니다. @AdminOnly 는 기존의 @MasterOnly 와 동일한 방식으로 동작하며, Accessor가 Admin 혹은 Master일때 허용됩니다. (Master도 AdminOnly에 포함됨!)

  2. City, Category의 경우 기존에도 목록 조회 API가 있지만 관리자 API의 경우 포함되는 정보의 양이 더 많기 때문에 별도의 Response Dto를 추가해 구현하였습니다.

  3. Controller는 Admin 패키지 내에 각각의 Controller로, Service는 City, Category, Currency 패키지에 각각 구현하였습니다. 즉, AdminCityControllerCityService를 호출하는 구조입니다.

  4. Currency는 Id기준이 아닌 날짜 기준으로 정렬하여 페이지로 조회하도록 구현하였습니다. 새로운 환율 정보를 추가하는 경우 과거의 날짜를 추가해도 id가 더 크기 때문에, 환율이라는 특성 상 날짜로 정렬하는 편이 더 가독성이 좋다고 생각되어 이와 같이 구현하였습니다.

2.3. 고민

구현하면서 고민했던 내용들입니다. 의견 환영합니다!

  1. Controller는 Admin에, Service는 City에 구현하면서 DTO는 어느 패키지에 둬야할까?를 고민했습니다.
    → 관점의 차이인 문제라 어느쪽에 둬도 말이 되지만, Admin 패키지에 둘 경우 패키지간 상호참조가 발생할 위험이 있기 때문에 우선 이를 막기위해 city 패키지에 두었습니다.

  2. city 도메인 response에 담길 정보가 완전히 동일한데(타입까지) response DTO를 만들 필요가 있을까?
    → 만약에 도메인이 변경되더라도 response의 형태는 변경되지 않아야 하므로 DTO를 만들었습니다. 반대의 경우도 마찬가지!

Copy link
Collaborator

@mcodnjs mcodnjs left a comment

Choose a reason for hiding this comment

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

안녕하세요, 지우씨
리뷰가 조금 늦었습니다 제송합니다 😂
일단 이걸 이렇게 빨리 구현했다는 것에 매우 놀랐습니다. 야무지네요
테스트는 못봄 다음 리뷰 때 다시 볼게요 .. ㅠㅠ
별거없고 저도 변수명이랑 검증 로직, 재사용 가능한 거들 위주로 리뷰 남겼어요!
고생하셨습니다 ~~!!~~~!!~

Comment on lines 66 to 69
if (adminMemberRepository.existsByIdAndAdminType(memberId, MASTER)) {
return Accessor.master(memberId);
}
return Accessor.admin(memberId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

adminMember를 조회하는 이 부분만 현재 LoginArgumentResolver랑의 차이점 같은데, 따로 구현하신 이유가 있을까요 ?! memberId가 admin이랑 일반 user랑 겹칠 수 있어서 .. ? 구분할 방법이 없어서 사용하셨다면
if (parameter.hasParameterAnnotation(AdminAuth.class)) 를 통해 구분하는 방법은 어떤가요 ?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이 부분은 슬랙을 통해 말씀드린 내용 그대로! 다른 백엔드분들 의견 모이는대로 반영하겠습니다.

@LastModifiedDate
private LocalDateTime modifiedAt;

public AdminMember(final Long id, final String userName, final String password, final AdminType adminType) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

위 아래 생성자 정팩메 사용하면 더 좋을지도 ㅎㅎ
현재 Member랑 동일한 로직인거 같은데, 수정하는 로직에서 createdAt도 LocalDateTime.now()로 변경되면 안될거 가타요 ㅎㅎ 물론 @Column(updatable = false) 있어서 변경되지 않을거 같지만 .. !!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오 그러네요?!?! 근데 Member는 createdAt 어떻게 유지되고 있는거죠..?!

Comment on lines +21 to +23
@NotNull(message = "비밀번호를 입력해주세요.")
@Size(min = 4, max = 20, message = "비밀번호는 4자 이상, 20자 이하여야 합니다.")
private String password;
Copy link
Collaborator

Choose a reason for hiding this comment

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

암호화해서 저장하면 길이가 달라져서 그런거 같타요
bcrypt 암호화 결과 길이가 정해져있나용 .. ? 최대 20자라고 가정했을 때 64자가 최대인가요 ?!

public class AdminMemberResponse {

private final Long id;
private final String userName;
Copy link
Collaborator

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ 나도 username이 좋아 헤헤 ..

private final String userName;
private final String adminType;

public static AdminMemberResponse from(final AdminMember adminMember) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

생성자 private으로 막아놓고 정팩메만 사용하면 좋을거 같아요 ~~!
다른 dto들도 보이면 바꿔주시면 🙇‍♂️🙇‍♂️

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

1a12237

Comment on lines 42 to 43
validateCategoryDuplicateId(categoryRequest);
validateCategoryDuplicateName(categoryRequest);
Copy link
Collaborator

Choose a reason for hiding this comment

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

인자값을 categoryRequest 전체 넘기지 않고 필요한 값만 넘겨도 좋을거 가타용

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

114cd24

두단계로 나누어 검증하도록 변경!

if (!category.getId().equals(categoryRequest.getId())) {
validateCategoryDuplicateId(categoryRequest);
}
if (!category.isSameNames(categoryRequest.getEngName(), categoryRequest.getKorName())) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기서 isSameNames()의 역할이 궁금해요 !! ID만 변경하는 경우에는 업데이트가 안되나용 ?
category validation 부분 save() 쪽이랑 같이 조금 더 정리되면 좋을거 같아요 ~~~!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Comment on lines 54 to 58
private void validateCategoryDuplicateName(final CategoryRequest categoryRequest) {
if (categoryRepository.existsByEngNameAndKorName(categoryRequest.getEngName(), categoryRequest.getKorName())) {
throw new BadRequestException(DUPLICATED_CATEGORY_NAME);
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

기존에 레스토랑 - restaurant 이 존재하는데 음식점 - restaurant으로 저장하는건 가능한가요?

Copy link
Collaborator

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.

114cd24 한글, 영어 이름 모두 각각 검증하도록 수정하였습니다!

return new CurrencyListResponse(currencyResponses, lastPageIndex);
}

private Long getLastPageIndex(final int pageSize) {
Copy link
Collaborator

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.

아?

currency.update(currencyRequest);
}

private void validateDateDuplicate(final Currency currency, final LocalDate newDate) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

validateDuplicatedDate 어떠신지 .. ㅎㅎ 여기도 currency.getDate()를 넘겨주면 좋을거 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

8a374bb 수정 완!

Copy link
Member

@jjongwa jjongwa left a comment

Choose a reason for hiding this comment

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

정말 든 든하네요
잠깐 안본것 같은데 comment가 71개나 달려서 놀랐습니다.
얼른 머지하고 릴리즈까지 샤샤샥 가시죠

Copy link
Collaborator

@waterricecake waterricecake left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 이오! 제가 저번주 조금 바빠서 늦게 봤네요;;;

Comment on lines 8 to 9
public enum AdminType {
ADMIN, MASTER;
Copy link
Collaborator

Choose a reason for hiding this comment

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

저희 이넘 첫 줄 개행 한것도 있고 안한것도 있던데 이것도 클래스니까 개행합니까? (trip패키지의 이넘들은 개행되어 있는데 auth는 안되어있음)

Suggested change
public enum AdminType {
ADMIN, MASTER;
public enum AdminType {
ADMIN,
MASTER;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

일단 AdminType은 Auth랑 같은 방식을 따르는게 맞는것 같아서 개행을 안했었는데, Trip까지 전부 보니까 Auth가 잘못되있었던것도 같구요..? 둘 모두 개행 하는 쪽으로 수정하였습니다.

Comment on lines +21 to +29
return new Accessor(memberId, Authority.MEMBER);
}

public static Accessor admin(final Long memberId) {
return new Accessor(memberId, Authority.ADMIN);
}

public static Accessor master(final Long memberId) {
return new Accessor(memberId, Authority.MASTER);
Copy link
Collaborator

Choose a reason for hiding this comment

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

이부분 static import에서 바꾸신 이유가 있나요? (진짜 궁금해서 제가 또 놓쳤나해서)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이게 Authroity로의 ADMIN, MASTER가 있고 AdminType으로서의 ADMIN, MASTER가 있어서 명시해주었습니다.
Authority의 ADMIN, MASTER는 인증/인가에만 사용되고 나머지 부분에서 ADMIN, MASTER는 AdminType이 사용되는 구조에요!

Copy link
Collaborator

@hgo641 hgo641 left a comment

Choose a reason for hiding this comment

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

볼륨이 어마어마하게 크군요! 고생하셨습니다 리더 이오...

@LJW25
Copy link
Collaborator Author

LJW25 commented Feb 3, 2024

최종 변경 사항

  • 카테고리 추가 및 수정 시 한글명 검증 삭제 (한글명은 중복될 수 있음.)
  • 환율 추가 및 수정 시 krw 삭제 (항상 1이라 불필요함.)
  • 컨플릭트 해결을 위한 merge!

@LJW25 LJW25 merged commit d3b9bd1 into develop Feb 3, 2024
1 check passed
hgo641 pushed a commit that referenced this pull request Apr 23, 2024
* feat: spring-boot-starter-security 추가

* chore: Flyway AdminMember 테이블 추가

* feat: AdminAuth 어노테이션 구현

* feat: AdminLoginArgumentResolver 구현

* feat: AdminException 추가

* feat: AdminMember 구현

* test: AdminLoginControllerTest 작성

* feat: AdminLogin API 구현

* test: AdminServiceTest 작성

* feat: AdminService 구현

* test: AdminMemberControllerTest 구현

* feat: AdminOnly 어노테이션 구현

* feat: AdminMember API 구현

* test: AdminMemberServiceTest 작성

* feat: AdminMemberService 구현

* chore: submodule 업데이트

* feat: city 상세 목록 조회 기능 구현

* test: city 상세 목록 조회 테스트 작성

* test: city 추가 테스트 작성

* feat: city 추가 기능 구현

* test: city 수정 기능 테스트 작성

* feat: city 수정 기능 구현

* test: city 상세 목록 조회 API 테스트 작성

* feat: city 상세 목록 조회 API 구현

* feat: 도시 추가 API 구현

* test: 도시 추가 API 테스트 작성

* test: 도시 수정 API 테스트 작성

* feat: 도시 수정 API 구현

* test: 카테고리 세부 정보 조회 기능 테스트 작성

* feat: 카테고리 세부 정보 조회 기능 구현

* rename: category response dto 패키지 이동

* test: 카테고리 추가 기능 테스트 작성

* feat: 카테고리 추가 기능 구현

* test: 카테고리 수정 기능 테스트 작성

* feat: 카테고리 수정 기능 구현

* test: 카테고리 세부 정보 API 테스트 작성

* feat: 카테고리 세부 정보 API 구현

* test: 카테고리 추가 API 테스트 작성

* feat: 카테고리 추가 API 구현

* test: 카테고리 수정 API 테스트 작성

* feat: 카테고리 수정 API 구현

* test: 환율 페이징 조회 기능 테스트 작성

* feat: 환율 페이징 조회 기능 구현

* fix: 환율 조회 repository 메소드 수정

* test: 환율 조회 Api 테스트 작성

* feat: 환율 조회 Api 구현

* rename: Currency dto 패키지 이동

* test: 환율 저장 기능 테스트 작성

* feat: 환율 저장 기능 구현

* test: 환율 수정 기능 테스트 작성

* feat: 환율 수정 기능 구현

* test: 환율 생성 API 테스트 작성

* feat: 환율 생성 API 구현

* test: 환율 수정 API 테스트 작성

* feat: 환율 수정 API 구현

* test: 공백 제거

* chore: submodule 업데이트

* chore: 라이브러리 변경

* refactor: BCrypt 라이브러리 변경

* chore: 서브모듈 업데이트

* refactor: AdminMemberRepository 메소드 이름 변경

* refactor: Master static import 변경

* refactor: 패키지명 오타 수정

* refactor: 패키지 이동

* test: AdminMemberService 통합 테스트 작성

* test: AdminLoginService 통합 테스트 작성

* refactor: Category 생성, 수정 시 id를 포함하도록 변경

* test: CategoryService 통합 테스트 작성

* test: CityService 통합 테스트 작성

* test: AdminMember 생성 Controller 통합 테스트 작성

* test: AdminCurrency 생성,조회 Controller 통합 테스트 작성

* refactor: userName을 username으로 변경

* refactor: request 검증 메세지 수정

* refactor: Currency 조회 메소드 이름 변경

* refactor: response 변수 이름 변경

* refactor: 불필요한 생성자 제거

* chore: 패키지 버전 변경

* refactor: 컨벤션 맞춤

* refactor: response 생성자 private 접근제어 추가

* refactor: category 중복 제거 로직 변경

* refactor: currency 중복 검증 메소드 변경

* refactor: username 변수명 수정

* refactor: username 변수명 수정

* refactor: AdminLoginArgumentResolver 로직 변경

* refactor: Enum 개행 추가

* docs: Restdocs 추가

* refactor: 카테고리 한글명 중복 검증 제거

* refactor: 환율 request dto krw 삭제
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.

관리자 기능 추가 - 백엔드
5 participants