APIView → ModelViewSet 사용하기
- 프로젝트를 시작할 때, 나는 기존에 자바를 썻기 때문에, 파이썬도 처음이었고, django도 처음이었고 drf도 처음이었다. 그래서 다른 멘티들이나 멘토님 코드보고 따라하는 수준이었다.
- 코드를 보고 따라했을 때, 다들
APIView를 사용했고,GenericView,ViewSet같은 것들이 있는지도 몰랐다. - 조금씩 공식문서를 보고 따라하고 학습하면서,
APIView외에GenericView,ViewSet이 있다는 것을 알게 되었고, List, Create, Retreive, Update, Delete View 클래스를 하나씩 만드는것 보다. ViewSet을 사용하여 하나의 클래스로 각 action을 해결하는것이 더 낫겠다는 생각이 들어서APIView→ModelViewSet으로 리팩토링을 시작했다. - 멘토님께서 강조했던 부분이 리팩토링을 하나의 피처로 정해서 하지 말고, 각 피처 구현이 끝나면 항상 리팩토링을 하라고 하셨다.
- 이번 리팩토링을 하면서 왜 그렇게 말씀하셨는지 확실히 느꼈다. 이게 계속 리팩토링 요소가 쌓이니까 너무 힘들더라..
1. BaseContentView 삭제
class BaseContentView(APIView):
child_model: Optional[Model] = None
many = False
def get(self, request, *args, **kwargs):
id_from_path_param = list(kwargs.values())[0]
result = self.parent_model.objects.filter(id=id_from_path_param).first()
class_name_lower = str(self.parent_model._meta).split(".")[1]
if result is None:
return Response(
data={"message": ERR_NOT_FOUND_MSG_MAP.get(class_name_lower, ERR_UNEXPECTED)},
status=status.HTTP_404_NOT_FOUND,
)
if self.child_model:
result = self.child_model.objects.filter(**{class_name_lower: result})
if self.many:
serializer = self.serializer(result, many=True)
else:
serializer = self.serializer(result)
return Response(data=serializer.data, status=status.HTTP_200_OK)BaseContentView는 Note, Topic, Page Detail API의 중복코드와Topic,PageList API의 중복코드를 줄이기 위해 멘토님이 작성한 클래스다.ModelViewSet으로 리팩토링하기 위해 이 클래스는 삭제했다.
2. Note API View 리팩토링
1. Note APIView(NoteListCreateView, NoteDetailUpdateDeleteview)
class NoteListCreateView(CtrlfAuthenticationMixin, APIView):
@swagger_auto_schema(**SWAGGER_NOTE_LIST_VIEW)
def get(self, request):
current_cursor = int(request.query_params["cursor"])
notes = Note.objects.all()[current_cursor : current_cursor + MAX_PRINTABLE_NOTE_COUNT]
serializer = NoteSerializer(notes, many=True)
serialized_notes = serializer.data
return Response(
data={"next_cursor": current_cursor + len(serialized_notes), "notes": serialized_notes},
status=status.HTTP_200_OK,
)
@swagger_auto_schema(**SWAGGER_NOTE_CREATE_VIEW)
def post(self, request, *args, **kwargs):
ctrlf_user = self._ctrlf_authentication(request)
note_data = {
"title": request.data["title"],
"owners": [ctrlf_user.id],
}
issue_data = {
"owner": ctrlf_user.id,
"title": request.data["title"],
"reason": request.data["reason"],
"status": CtrlfIssueStatus.REQUESTED,
"related_model_type": CtrlfContentType.NOTE,
"action": CtrlfActionType.CREATE,
}
note_serializer = NoteSerializer(data=note_data)
issue_serializer = IssueCreateSerializer(data=issue_data)
if note_serializer.is_valid() and issue_serializer.is_valid():
issue_serializer.save(related_model=note_serializer.save())
else:
return Response(status=status.HTTP_400_BAD_REQUEST)
return Response(status=status.HTTP_201_CREATED)
class NoteDetailUpdateDeleteView(BaseContentView):
parent_model = Note
serializer = NoteSerializer
@swagger_auto_schema(**SWAGGER_NOTE_DETAIL_VIEW)
def get(self, request, *args, **kwargs):
return super().get(request, *args, **kwargs)- Note Create, List를 한 클래스로, Note Detail, Update, Delete를 한 클래스로 구현했다.
- 여기서 두 클래스를
ModelViewSet을 상속받아 한 클래스로 리팩토링하자.
class NoteViewSet(CtrlfAuthenticationMixin, ModelViewSet):
queryset = Note.objects.all()
serializer_class = NoteSerializer
pagination_class = NoteListPagination
lookup_url_kwarg = "note_id"
@swagger_auto_schema(**SWAGGER_NOTE_LIST_VIEW)
def list(self, request, *args, **kwargs):
return super().list(self, request, *args, **kwargs)
@swagger_auto_schema(**SWAGGER_NOTE_CREATE_VIEW)
def create(self, request, *args, **kwargs):
ctrlf_user = self._ctrlf_authentication(request)
note_data, issue_data = self.build_data(request, ctrlf_user)
note_serializer = NoteSerializer(data=note_data)
issue_serializer = IssueCreateSerializer(data=issue_data)
if note_serializer.is_valid() and issue_serializer.is_valid():
issue_serializer.save(related_model=note_serializer.save())
else:
return Response(status=status.HTTP_400_BAD_REQUEST)
return Response(status=status.HTTP_201_CREATED)
@swagger_auto_schema(**SWAGGER_NOTE_DETAIL_VIEW)
def retrieve(self, request, *args, **kwargs):
return super().retrieve(self, request, *args, **kwargs)
def build_data(self, request, ctrlf_user):
note_data = {
"title": request.data["title"],
"owners": [ctrlf_user.id],
}
issue_data = {
"owner": ctrlf_user.id,
"title": request.data["title"],
"reason": request.data["reason"],
"status": CtrlfIssueStatus.REQUESTED,
"related_model_type": CtrlfContentType.NOTE,
"action": CtrlfActionType.CREATE,
}
return note_data, issue_data- 기존의
NoteListCreateView에서의get()메소드에서는query_param에current_cursor를 담아서 구현했는데,ModelViewSet에pagination_class가 있었다. 이것을 이용하고 싶어서 공식문서를 보면서 직접NoteListPagination을 구현했다. note_data,issue_data만드는 부분을build_data()로 분리했다.
2. NoteListPagination (커밋링크)
# ctrlfbe/paginations.py
class NoteListPagination(CursorPagination):
max_page_size = MAX_PRINTABLE_NOTE_COUNT
def paginate_queryset(self, queryset, request, view=None):
self.current_cursor = int(request.query_params["cursor"])
notes = Note.objects.all()[self.current_cursor : self.current_cursor + MAX_PRINTABLE_NOTE_COUNT]
return notes
def get_paginated_response(self, data):
return Response(data={"next_cursor": self.current_cursor + len(data), "notes": data})- 요구사항에서 Note Paging 스펙이 Cursor based였기 때문에
CursorPagination을 상속받았다.
3. url.py 수정
# ctrlfbe/note_urls.py
urlpatterns = [
path("", NoteListCreateView.as_view(), name="note_list_create"),
path("<int:note_id>/", NoteDetailUpdateDeleteView.as_view(), name="note_detail_update_delete"),
path("<int:note_id>/topics/", TopicListView.as_view(), name="topic_list"),
]NoteListCreateView와NoteDetailUpdateDeleteView를 지웠기 때문에note_urls.py에서 에러가 났다.NoteViewSet에 맞게 수정하자.
# ctrlfbe/note_urls.py
urlpatterns = [
path(
"",
NoteViewSet.as_view(
{
"get": "list",
"post": "create",
}
),
name="note_list_create",
),
path("<int:note_id>/topics/", TopicListView.as_view(), name="topic_list"),
path("<int:note_id>/", NoteViewSet.as_view({"get": "retrieve"}), name="note_detail_update_delete"),as_view()메서드에 HTTP method와(GET, POST 등)NoteViewSet의 메소드를 매핑했다.- Routers를 쓰는 방식도 공식문서에 나와있는데 봐도 잘 모르겠어서 일단은 이렇게 구현했다. Router쓰는것이 더 깔끔하다면 수정 할 예정.
- python code formatting 도구인 black에 의해서 code formatting이 자동으로 됐는데.. 내가 보기에는 좀 불편하다 ㅜ
3. Topic API View 리팩토링
1. Topic APIView ( TopicListView, TopicCreateView, TopicDetailUpdateDeleteView)
class TopicListView(BaseContentView):
parent_model = Note
child_model = Topic
serializer = TopicSerializer
many = True
@swagger_auto_schema(**SWAGGER_TOPIC_LIST_VIEW)
def get(self, request, *args, **kwargs):
return super().get(request, *args, **kwargs)
class TopicCreateView(CtrlfAuthenticationMixin, APIView):
@swagger_auto_schema(**SWAGGER_TOPIC_CREATE_VIEW)
def post(self, request, *args, **kwargs):
ctrlf_user = self._ctrlf_authentication(request)
topic_data = {
"note": request.data["note_id"],
"title": request.data["title"],
"owners": [ctrlf_user.id],
}
topic_serializer = TopicSerializer(data=topic_data)
issue_serializer = IssueCreateSerializer(data=issue_data)
if topic_serializer.is_valid() and issue_serializer.is_valid():
issue_serializer.save(related_model=topic_serializer.save())
else:
return Response(status=status.HTTP_400_BAD_REQUEST)
return Response(status=status.HTTP_201_CREATED)
class TopicDetailUpdateDeleteView(BaseContentView):
parent_model = Topic
serializer = TopicSerializer
@swagger_auto_schema(**SWAGGER_TOPIC_DETAIL_VIEW)
def get(self, request, *args, **kwargs):
return super().get(request, *args, **kwargs)- NoteDetailUpdateDeleteView와 유사하게 BaseContentView를 상속받았다.
- 세 클래스를 PageViewSet 클래스 하나로 리팩토링하자.
class TopicViewSet(CtrlfAuthenticationMixin, ModelViewSet):
queryset = Topic.objects.all()
serializer_class = TopicSerializer
lookup_url_kwarg = "topic_id"
@swagger_auto_schema(**SWAGGER_TOPIC_LIST_VIEW)
def list(self, request, *args, **kwargs):
note_id = list(kwargs.values())[0]
note = Note.objects.filter(id=note_id).first()
if note is None:
return Response(
data={"message": ERR_NOT_FOUND_MSG_MAP.get("note", ERR_UNEXPECTED)},
status=status.HTTP_404_NOT_FOUND,
)
return super().list(request, *args, **kwargs)
@swagger_auto_schema(**SWAGGER_TOPIC_CREATE_VIEW)
def create(self, request, *args, **kwargs):
ctrlf_user = self._ctrlf_authentication(request)
topic_data, issue_data = self.build_data(request, ctrlf_user)
topic_serializer = TopicSerializer(data=topic_data)
issue_serializer = IssueCreateSerializer(data=issue_data)
if topic_serializer.is_valid() and issue_serializer.is_valid():
issue_serializer.save(related_model=topic_serializer.save())
else:
return Response(status=status.HTTP_400_BAD_REQUEST)
return Response(status=status.HTTP_201_CREATED)
@swagger_auto_schema(**SWAGGER_TOPIC_DETAIL_VIEW)
def retrieve(self, request, *args, **kwargs):
return super().retrieve(self, request, *args, **kwargs)
def build_data(self, request, ctrlf_user):
topic_data = {"title": request.data["title"], "owners": [ctrlf_user.id], "note": request.data["note_id"]}
issue_data = {
"owner": ctrlf_user.id,
"title": request.data["title"],
"reason": request.data["reason"],
"status": CtrlfIssueStatus.REQUESTED,
"related_model_type": CtrlfContentType.TOPIC,
"action": CtrlfActionType.CREATE,
}
return topic_data, issue_dataNoteViewSet과 매유 유사하다.list()메소드에서- Note의 경우는 url이 {BASE_URL}/notes/ 이고, cursor based pagination이다.
- Topic의 경우는 url이 {BASE_URL}/{note_id}/topics/ 이고, pagination이 따로 없다. paging 스펙이 따로 없었기 때문에, default값이다. default는
LimitOffsetPagination이다. (공식문서)
create()메소드를 보면,NoteViewSet의create()메소드와 매우 유사하다. 중복된 코드라서 리팩토링이 필요할 것 같다.
2. url.py 수정
# ctrlfbe/note_urls
urlpatterns = [
path(
"",
NoteViewSet.as_view(
{
"get": "list",
"post": "create",
}
),
name="note_list_create",
),
path("<int:note_id>/topics/", TopicListView.as_view(), name="topic_list"),
path("<int:note_id>/", NoteViewSet.as_view({"get": "retrieve"}), name="note_detail_update_delete"),
]
# ctrlfbe/topic_urls
urlpatterns = [
path("", TopicCreateView.as_view(), name="topic_crete"),
path("<int:topic_id>/pages/", PageListView.as_view(), name="page_list"),
path("<int:topic_id>/", TopicDetailUpdateDeleteView.as_view(), name="topic_detail"),
]TopicListView,TopicCreateView,TopicDetailUpdateDeleteView를 지웠기 때문에ctrlfbe/note_urls.py,ctrlfbe/topic_urls.py에서 에러가 났다.- 역시
TopicViewSet에 맞게 수정하자.
# ctrlfbe/note_urls.py
urlpatterns = [
path(
"",
NoteViewSet.as_view(
{
"get": "list",
"post": "create",
}
),
name="note_list_create",
),
path(
"<int:note_id>/topics/",
TopicViewSet.as_view(
{
"get": "list",
}
),
name="topic_list",
),
path("<int:note_id>/", NoteViewSet.as_view({"get": "retrieve"}), name="note_detail_update_delete"),
]
# ctrlfbe/topic_urls.py
urlpatterns = [
path(
"",
TopicViewSet.as_view(
{
"post": "create",
}
),
name="topic_create",
),
path(
"<int:topic_id>/",
TopicViewSet.as_view(
{
"get": "retrieve",
}
),
name="topic_detail",
),
path("<int:topic_id>/pages/", PageListView.as_view(), name="page_list"),
]- Note API View 리팩토링처럼
as_view()메서드에 매핑했다.
Page API View 리팩토링은 Topic과 매우 유사하기 때문에 생략한다.
Issue도 유사하게 리팩토링 하긴 했는데, model의 성격이 Note, Topic, Page와 조금 다르다고 생각해서 나중에 따로 포스팅할 예정
4. Commits & Notion
- JH-165 Refactor: Use ModelViewSet instead of APIView
- JH-165 Refactor: add pagination class for note and issue list api
- APIView → ModelViewSet 리팩토링
5. 후기(?)
- 확실히 클래스 하나로 구현해놓으니까 내가 읽기 편해서 좋다.
note_data,topic_data,page_data,issue_data를 만드는 코드들이 중복이 매우 많다. 리팩토링 필요- note, topic, page create 메서드가 data가 조금 다른것을 제외하고는 역시 중복된 코드가 많다. 리팩토링 필요
- topic, page list 메서드 또한
note_id,topic_id가 필요하다는 것만 다르고 중복된 코드다. 리팩토링 필요 - DRF 공식문서를 보면,
ModelViewSet.as_view()를 쓰는 것보다 Router를 이용하다는것이 보편적이라고 한다.(Typical) 공부할겸 읽어보고 수정이 필요하다면 수정할 계획 - commit을 보면 Note, Topic, Page 전부 수정하고 commit을 했는데, 모델별로 쪼개서 commit하는게 더 좋았을것 같다.
- Notion에 쓰고 티스토리 블로그에 옮겼는데.. 불편하다. 토글기능(더보기)도 notion과 티스토리가 달라서 그대로 복붙하기 불편 ㅜ..
'Refactoring' 카테고리의 다른 글
| [Refactoring] 중복 코드 제거2(상속) (0) | 2021.12.25 |
|---|---|
| [Refactoring] 중복 코드 제거1 (0) | 2021.12.15 |