설계의 중요성을 느끼게 된 AOP 적용기 (회원 접근 인가 로직 분리하기 2편)

이 글에 대해서

• 1편 내용과 이어집니다.
• 해당 프로젝트 코드는 github에서 확인할 수 있습니다.
• 여러 서비스 메서드에서 회원 접근 인가 로직의 코드 중복이 많이 발생하는 것을 발견하였습니다.
• 1차적으로 코드 중복을 개선하고자 했고, 2차적으로 객체간의 의존을 줄이는 것을 목표로 리팩토링을 시도했습니다.
• 리팩토링을 진행하면서 여러방식에 대한 고려요소에 대한 제 생각들이 담겨있습니다. 그래서 틀린 부분이 많을 수 있습니다.

---

1. 서비스 이해 및 로직 설명

• 토이프로젝트의 서비스, 주제는 그룹 지도 서비스입니다.

• 이전 글에서 github repository로 설명을 드렸습니다.

• 기본적인 DB ERD는 이러합니다.

• DDD에 대한 얄팍한 이해로 Member의 경우 다른 모듈(서비스)로 분리할 수 있을 것이라 생각하고 간접 참조로 구현했습니다.
  • 그래서 member를 참조하는 엔티티의 경우 JPA를 통한 연관관계가 아닌 member의 id만을 참조합니다.
  • group_member 엔티티 또한, member_id와 map_id를 간접 참조합니다.
  • 간접참조와 직접참조의 장단점을 직접 경험하기 위한 설계입니다.
• map과 관련한 domain들을 지도 서비스의 핵심 도메인이라고 생각했고, JPA를 통한 연관관계를 구성하였습니다. (직접참조)

1. map

• 회원이라면 각자의 지도를 생성할 수 있습니다.
• 지도는 전체 공개 및 비공개로 설정할 수 있습니다.

2. category

• 지도 내의 포함된 카테고리입니다.
• 지도내에 등록된 장소들을 분야별로 나눠서 저장할 수 있도록 구성합니다.

3. place

• 지도내의 등록되는 장소입니다.
• 하나의 게시글로 생각할 수 있습니다.
• map과 category를 참조하며, member 또한 간접참조합니다.

4. comment

• 댓글 입니다.
• 장소에 대해서 댓글을 남깁니다. place와 member를 참조합니다.

5. group_member

• 지도에 대해서 접근할 수 있는 권한 정보를 갖습니다.
• permission_level의 경우 HOST, MAINTAIN, READ 권한으로 세분화 했습니다.

---

접근 권한 로직

1. 비공개 지도
   • 그룹멤버에 포함된 회원만이 해당 지도에 접근할 수 있습니다.
   • HOST, 즉 지도의 주인의 경우 그룹멤버와 카테고리, 장소에 대해서 모든 권한을 갖습니다.
     • 댓글은 작성자만이 수정, 삭제할 수 있도록 합니다.
   • MAINTAIN의 경우 지도 정보, 그룹멤버를 제외하고, 카테고리와 장소에 대해서 자유로운 권한을 갖습니다.
   • READ는 지도 정보에 대해서 조회하고, 장소에 대한 댓글을 작성할 수 있습니다.
2. 전체 공개 지도
   • 로그인한 사용자라면, 모두 해당 지도를 조회할 수 있고, 포함된 장소에 댓글을 작성할 수 있습니다.
   • HOST와 MAINTAIN 권한을 가진 사용자만이 카테고리, 장소등을 등록, 수정, 삭제할 수 있습니다.

접근 권한 로직을 체크하려면?

• 접근 권한에 대한 정보는 group_member 엔티티를 통해서 확인할 수 있습니다.
• 하지만 그룹멤버의 경우 member와 map에 대한 정보만을 가지고 있습니다.
• category, place, comment의 경우 참조하고 있는 map을 통해서 접근할 수 있는지를 확인할 수 있습니다.

---

2. 나쁜 코드 스멜 (문제 확인)

코드 중복

• MapService, CategoryService, PlaceService, CommentService
  • 도메인 CRUD에 대한 핵심 기능을 담는 Service 클래스들입니다.
  • 해당 클래스들을 사용할 때마다, GroupMemberRepository를 의존하고 접근 권한에 대해서 체크하는 로직이 모든 메서드들에 포함되게 되었습니다.

@Override
@Transactional
public Long create(String name, String address, Double latitude, Double longitude, String story, String detailLink, Long mapId,
                   Long categoryId, Long memberId) {

  Map map = mapRepository.findById(mapId)
    .orElseThrow(() -> new ClientException(ErrorStatusCodeAndMessage.NO_SUCH_MAP));

  GroupMember groupMember = groupMemberRepository.findByMapIdAndMemberId(mapId, memberId)
    .orElseThrow(() -> new ClientException(ErrorStatusCodeAndMessage.NO_SUCH_GROUP_MEMBER));

  // 필요한 권한이 없으면 예외 발생
  groupMember.checkHasRequiredPermission(PermissionLevel.MAINTAIN);
   
  
  
  ... 생략
    
}

• PlaceService에서 Place를 등록하는 메서드의 일부입니다.
• GroupMemberRepository를 통해서 GroupMember를 가져와서 접근 권한이 있는지를 확인합니다.
  • 이러한 로직이 메서드마다의 맞춤 레벨에 따라서 모두 포함되게 되었고, 중복이 많이 발생하게 되었습니다.
  • 1차적으로 가장 단순하게 코드 중복을 줄이려면, 중복로직을 메서드로 추출하는 것입니다.
  • 하지만 각 서비스마다 메서드들을 추출해야하기 때문에 아직도 코드 중복이 많이 있습니다.

---

3. 리펙토링 방향성 잡기

• 코드 중복을 없애고, 객체간의 결합도도 낮춰보자.

1. 다른 서비스 객체에서 접근 권한을 체크하는 서비스를 의존하기

• 각 서비스마다 GroupMemberRepository를 의존하고, 권한을 체크하는 로직을 작성하는 것이 아니라 그 역할을 담당할 객체를 만드는 방법입니다.
• 그 역할을 GroupMemberService에 두고서 MapService와 같은 역할의 서비스 객체가 GroupMemberService를 의존하여 메서드 내부에서 접근권한을 체크하게 됩니다.

@Override
@Transactional
public Long create(String name, String address, Double latitude, Double longitude, String story, String detailLink, Long mapId,
                   Long categoryId, Long memberId) {

  Map map = mapRepository.findById(mapId)
    .orElseThrow(() -> new ClientException(ErrorStatusCodeAndMessage.NO_SUCH_MAP));
	
  // Service에서 담당
  groupMemberService.checkHasMaintainLevel(mapId, memberId);
  
 
  ... 생략
    
}

문제점

• 객체간의 역할이 분리되어있고, 코드 중복은 줄었지만 GroupMemberService에 대한 다른 Service들의 의존성은 여전합니다.
  • GroupMemberService의 수정에 따라서 다른 Service들에게 영향을 미칠 수 있습니다.
  • SRP(객체 변경의 이유는 하나여야만 한다)를 위반할 가능성이 있습니다.
  • 또한 그로 인한 사이드 이펙트가 발생할 수 있고, 그 문제를 찾는데 복잡할 수 있습니다.
  • 추상 타입(interface)에 의존하고, 접근권한로직에 대한 반환값을 사용하는 로직이 아니기때문에 변경에 따른 영향은 최소화했습니다.
• 또한 Service가 Service를 참조함에 따라서 순환참조의 문제 또한 발생할 수 있습니다.

---

2. 권한 체크가 필요한 서비스를 갖는 Controller에서 GroupMemberService를 의존하고, 접근권한 로직 실행 후 요청 로직 수행하기

• GroupMemberService로 접근권한 로직을 분리했으니, Controller에서 접근권한을 체크할 수 있습니다.

@PostMapping("/places")
@ResponseStatus(HttpStatus.CREATED)
public PlaceResponse create(@Login Long memberId, @RequestBody @Valid PlaceRequest placeRequest) {
  
  groupMemberService.checkHasMaintainLevel(...);
  
  placeService.create(...);
                    
  .... 생략
}

문제점

• 권한을 체크하는 로직과 장소를 등록하는 로직이 하나의 Transaction으로 묶이지 않습니다.

  • 접근권한이 없으면 장소 등록 로직이 실행이 되지 않고, 접근권한체크 로직은 단순 조회이기 때문에 트랜잭션 전파가 꼭 필요하지 않은 로직일 수 있습니다.

  • 하지만 하나의 메서드에서 2개의 트랜잭션이 생긴다는것은 2개의 Connection을 사용하는 것이기에 부하가 발생했을 때의 문제를 야기할 수 있을 것이라 생각합니다.

    커넥션풀이 지원되고, 동기적으로 로직들이 실행되기때문에 큰 이슈일지는 또 확인해봐야겠습니다.

---

3. 파샤드 패턴으로 서비스를 감싸는 객체 만들기

• 파샤드 패턴이란?

  • 저수준 인터페이스들을 하나의 고수준 인터페이스로 묶어주는 패턴

• Service 객체들을 감싸는 객체들을 만들어서 Transaction이 전파되도록 할 수 있습니다.

• 기존 서비스에서 서비스를 의존하는 것과는 다르게 각 서비스는 자신의 역할만(단일책임)을 확실하게 담당할 수 있게됩니다.

• Service간의 의존성은 줄여줄 수 있습니다.

@Component
@RequiredArgsConstructor
@Transactional(readOnly = true)
public class PlacePacade {

    private final GroupMemberService groupMemberService;
    private final PlaceService placeService;
  
    @Transactional
    public PlaceResponse registerPlace(...) {
    
    groupMemberService.checkHasMaintainLevel(...);
  	placeService.create(...);
    
    ... 생략 
  }

문제점

• 접근권한이 필요한 로직에 대해서 Pacade를 여러개 생성해야 하고, 관리해야할 객체가 늘어나게 됩니다.
  • layer 구분이 헷갈릴 수 있다는 문제점도 있을 것 같습니다.

---

4. AOP 사용하기 (프록시 패턴)

• Spring AOP 기능을 통해서 접근권한 로직을 분리합니다.
• 1편에서 AOP로의 적용 방법에 대해서 설명했습니다.
• @Transactional 과 같이 @RequiredPermission어노테이션이 붙어있는 메서드를 가진 Bean을 동적 프록시로 만들어 해당 메서드 실행전에 접근권한에 대해서 체크합니다.
  • @Transactional AOP와 커스텀 AOP을 하나의 메서드에 동시에 적용하는 경우 기본 설정을 수정하지 않으면 @Transactional이 우선적으로 적용되어 하나의 Transction으로 묶여서 트랜잭션의 전파가 이루어집니다.
• Service에서 접근권한 로직에 대한 의존성이 사라지고, 접근권한 로직에 대해서는 GroupMemberService와 Aspect에서 담당합니다.

문제점

• 1편 설명과 같이 파라미터 이름으로 값을 체크하지 않고, AccessInfo 객체타입으로 reflection 하는 방식을 사용했습니다.
• 그렇기 때문에 전체적으로 권한체크로직이 필요한 메서드의 파라미터를 모두 AccessInfo 객체로 수정해야하는 문제점이 있습니다.
  • 결과적으로 AOP로의 로직 전환으로 인해서 앞에 방식들과는 다르게 리팩토링 요소가 굉장히 많아졌습니다.

---

4. 리팩토링 방식 결정과 흐름

1차 리팩토링

• 다른 서비스에서 GroupMemberService를 참조하는 방식으로의 리팩토링을 진행하게 되었습니다.
  • 코드 중복만이라도 줄이기 위해서..

결정 이유

• 다른 방식으로의 리팩토링을 진행면 API Path에 대한 수정이 필수적으로 따라왔기 때문입니다.
• API를 디테일한 설계가 없이 프로젝트를 진행한 점이 문제가 되었습니다.
• path를 계층적으로 구성하지않고, Place, Category, Comment등에 대해서 id value는 유니크하고 map에 대해서 연관관계를 맺고 있기 때문에 문제가 없다고 생각했습니다.
  • /places/{placeId}, /categories/{categoryId} 와 같이 구성했습니다.
• 이러한 API path에서는 접근해야하는 엔티티를 통해서 Map의 정보, 그리고 group_member를 확인해야 했기때문에 로직을 분리할 수 없습니다.
• FE와의 협업을 하고 있는 프로젝트이기 때문에, API path에 대한 전면적인 수정에 대해서 회의가 필요했기 때문입니다.

2차 리팩토링

• FE와의 회의를 진행하고 전면적으로 API path를 계층적으로 수정했습니다.
  • /map/{mapId}/places/{placeId} 와 같이 특정 지도 내부의 정보를 접근하려고 할때의 Path에 map에 대한 정보를 포함하게 했습니다.
• 객체를 통한 reflection을 위해서 서비스의 파라미터들을 AccessInfo로 감싸는 수정했습니다.
• 마지막으로 Aspect 구현과 @RequiredPermission 어노테이션을 통해서 접근로직을 분리하였습니다.

---

5. 결론 및 회고

설계의 중요성

• 이번 리팩토링과 토이프로젝트를 진행하면서 가장 크게 느낀 점입니다.

  • API, 객체의 역할, 엔티티 등등 디테일한 설계없이 기능을 하나하나 덧붙여가는 형식으로 코드를 구성했습니다.
  • 디테일한 설계가 없이 새로운 기능의 추가, 수정들을 거듭하면서 전반적으로 코드를 뜯어고치는 과정들이 많이 발생했습니다.
  • 그러한 리팩토링 과정은 테스트 코드까지 영향을 미치게 되어 작업량이 크게 증가하게 되었습니다.
  • 결과적으로 클린 코드, 클린 아키텍처를 구성하지 못했고 그 결과로 유지보수성 및 확장성이 떨어지면 얼마나 힘들어지는지를 몸소 경험한 것 같습니다.

• 향후 프로젝트를 진행하면서 요구사항을 잘 정의하고, 그에 맞게 설계에 시간을 투자해서 클린 코드와 클린 아키텍처를 구성할 수 있도록 노력해야겠습니다.

SOLID, 클린 아키텍처, Restful

• 대략적으로 개념으로는 이해하고 있지만, 적용에는 어려움을 느끼는 개념들입니다.
• 리팩토링 방식에 대해서 고민하면서 3가지 키워드 모두를 고려하고 의사결정해야 했습니다.
• 항상 무조건 옳고 지켜져야하는 방식은 아닐 수 있지만, 키워드들에 대해서 조금 더 경험하고 더 깊게 고민해볼 수 있는 계기가 되었습니다.

수정할 코드가 차고 넘친다...

---

References

• https://www.baeldung.com/spring-aop-get-advised-method-info
• https://kotlinworld.com/372
• https://docs.spring.io/spring-framework/docs/4.2.x/spring-framework-reference/html/transaction.html
• https://stackoverflow.com/questions/32057910/custom-spring-aop-around-transactional