group 로직에 대한 수정

[unikal1]

group 관리에 대한 수정이 필요할 것 같습니다. 제가 직접 하려고 했으나, 직접적인 코드 작성자도 아니고, 의견도 궁금해서 일단 discussion을 열긴 했습니다. 무엇보다, 제가 코드 리뷰를 좀 국소적인 부분에 치중해서인지, 전체적인 흐름에 대한 의문을 제대로 해소하지 못한 것 같습니다.

1. GroupAuthorityValidator 의 역할
   제가 생각하기에, 해당 객체는 "특정" 유저가 "특정" 그룹에 대해 변경할 권한이 존재하는지 확인하는 객체라고 느껴졌는데, 문제는 "특정" 그룹에 대한 검사가 빠져있다는 점입니다. 즉, GroupMember 를 UserId 로만 검색하여, 단 하나의 결과에 대해 그 권한을 확인하는 것 같은데, 이는 유저가 그룹을 하나만 들어갈 수 있다는 전제 하에만 성립하는 로직이 아닌가 싶습니다.

따라서, 다음과 같은 메서드로 대체되어야 된다고 생각됩니다.

    @Query(value = "SELECT * FROM group_member WHERE member_id = :memberId AND group_id = :groupId", nativeQuery = true)
    Optional<GroupMember> findByMemberIdAnAndGroupId(Long memberId, Long groupId);

그리고 그에 따른 검증 로직도 바꿀 필요성이 있겠네요.

2. MemberReader 의 사용
   코드 전반적인 부분에 대한 이야기인데, userId 를 worker 클래스에 넘기는 대신에, service 단에서 memberREader 등을 통해 Member 엔티티에 대한 레퍼런스를 가져와, 이를 worker 클래스에 넘겨주는게 어떤가요?

추후 유지보수성을 위해서, 만약 동일한 엔티티에 대해 작업할 내용이 생기게 된다면, 특히나 그 빈도가 빈번할 member 같은 경우는 worker 클래스가 아닌 service 레이어에서 다루는 편이 좀 더 간단하다고 생각됩니다.
적용된다면 저도 바꿀 부분이 꽤 많아지기는 하는데, 그 일례를 들자면,

현재 제가 채팅 로직에 대한 내용을 작성하고 있는데 그룹을 생성할 때, 채팅방에 대한 외래키를 설정해두었습니다. 즉, 특정 유저가 그룹을 생성한다면, 해당 유저를 기반으로 chatRoom / ChatRoomMember / Group / GroupMember 엔티티가 생성 되겠죠.
물론 getReferenceById 는 오버헤드가 굉장히 작기 때문에 성능적으로는 큰 상관이 없겠지만 단순히 줄 수를 줄일 수 있다는 명목으로는 충분히 괜찮은 이야기인 것 같습니다. 즉, Member 엔티티를 worker 클래스들에 넘겨서, ChatRoomMember, GroupMember 에서 memberRepository.getRefrenceById를 여러번 호출 할 필요가 없도록 하고자 합니다.

[s0o0bn]

확인했습니다.

1. 완전 동의합니다. 제가 왜 로직을 저렇게 짰을까요...? ㅋㅋㅋㅋㅋ 수정하겠습니다.
2. 이것도 동의합니다. 사실 저렇게 책임을 나누고 재사용성을 높이고자 worker를 도입하자고 했는데, 지금처럼 한 건 사실 거기에 안 맞긴 하네요.
   좋습니다. 사실 다른 도메인의 worker에서 MemberRepository 자체를 의존해버리면 worker의 의미가 퇴색되겠네요.