From 80d69a566325755107d810c6fb6c67147f32c9cc Mon Sep 17 00:00:00 2001 From: Tom Hacohen Date: Mon, 28 Dec 2020 09:25:28 +0200 Subject: [PATCH] Fix collection list and how we return API responses. --- etebase_fastapi/authentication.py | 28 +++++++++---------- etebase_fastapi/collection.py | 45 +++++++++++++------------------ etebase_fastapi/invitation.py | 18 +++++-------- etebase_fastapi/member.py | 8 +++--- etebase_fastapi/msgpack.py | 2 +- etebase_fastapi/utils.py | 8 ++++++ 6 files changed, 49 insertions(+), 60 deletions(-) diff --git a/etebase_fastapi/authentication.py b/etebase_fastapi/authentication.py index f04753b..13a8884 100644 --- a/etebase_fastapi/authentication.py +++ b/etebase_fastapi/authentication.py @@ -16,7 +16,6 @@ from django.db import transaction from django.utils import timezone from fastapi import APIRouter, Depends, status, Request, Response from fastapi.security import APIKeyHeader -from pydantic import BaseModel from django_etebase import app_settings, models from django_etebase.exceptions import EtebaseValidationError @@ -27,7 +26,8 @@ from django_etebase.token_auth.models import get_default_expiry from django_etebase.utils import create_user, get_user_queryset, CallbackContext from django_etebase.views import msgpack_encode, msgpack_decode from .exceptions import AuthenticationFailed, transform_validation_error, ValidationError -from .msgpack import MsgpackResponse, MsgpackRoute +from .msgpack import MsgpackRoute +from .utils import BaseModel User = get_user_model() token_scheme = APIKeyHeader(name="Authorization") @@ -225,10 +225,10 @@ def validate_login_request( @authentication_router.get("/is_etebase/") async def is_etebase(): - return MsgpackResponse({}) + pass -@authentication_router.post("/login_challenge/") +@authentication_router.post("/login_challenge/", response_model=LoginChallengeOut) async def login_challenge(user: User = Depends(get_login_user)): enc_key = get_encryption_key(user.userinfo.salt) box = nacl.secret.SecretBox(enc_key) @@ -237,35 +237,31 @@ async def login_challenge(user: User = Depends(get_login_user)): "userId": user.id, } challenge = bytes(box.encrypt(msgpack_encode(challenge_data), encoder=nacl.encoding.RawEncoder)) - return MsgpackResponse( - LoginChallengeOut(salt=user.userinfo.salt, challenge=challenge, version=user.userinfo.version) - ) + return LoginChallengeOut(salt=user.userinfo.salt, challenge=challenge, version=user.userinfo.version) -@authentication_router.post("/login/") +@authentication_router.post("/login/", response_model=LoginOut) async def login(data: Login, request: Request): user = await get_login_user(LoginChallengeIn(username=data.response_data.username)) host = request.headers.get("Host") await validate_login_request(data.response_data, data, user, "login", host) data = await sync_to_async(LoginOut.from_orm)(user) await sync_to_async(user_logged_in.send)(sender=user.__class__, request=None, user=user) - return MsgpackResponse(content=data, status_code=status.HTTP_200_OK) + return data -@authentication_router.post("/logout/") +@authentication_router.post("/logout/", status_code=status.HTTP_204_NO_CONTENT) async def logout(request: Request, auth_data: AuthData = Depends(get_auth_data)): await sync_to_async(auth_data.token.delete)() # XXX-TOM await sync_to_async(user_logged_out.send)(sender=auth_data.user.__class__, request=None, user=auth_data.user) - return Response(status_code=status.HTTP_204_NO_CONTENT) -@authentication_router.post("/change_password/") +@authentication_router.post("/change_password/", status_code=status.HTTP_204_NO_CONTENT) async def change_password(data: ChangePassword, request: Request, user: User = Depends(get_authenticated_user)): host = request.headers.get("Host") await validate_login_request(data.response_data, data, user, "changePassword", host) await sync_to_async(save_changed_password)(data, user) - return Response(status_code=status.HTTP_204_NO_CONTENT) @authentication_router.post("/dashboard_url/") @@ -278,7 +274,7 @@ def dashboard_url(user: User = Depends(get_authenticated_user)): ret = { "url": get_dashboard_url(request, *args, **kwargs), } - return MsgpackResponse(ret) + return ret def signup_save(data: SignupIn, request: Request) -> User: @@ -311,10 +307,10 @@ def signup_save(data: SignupIn, request: Request) -> User: return instance -@authentication_router.post("/signup/") +@authentication_router.post("/signup/", response_model=LoginOut, status_code=status.HTTP_201_CREATED) async def signup(data: SignupIn, request: Request): user = await sync_to_async(signup_save)(data, request) # XXX-TOM data = await sync_to_async(LoginOut.from_orm)(user) await sync_to_async(user_signed_up.send)(sender=user.__class__, request=None, user=user) - return MsgpackResponse(content=data, status_code=status.HTTP_201_CREATED) + return data diff --git a/etebase_fastapi/collection.py b/etebase_fastapi/collection.py index c0efed1..993d144 100644 --- a/etebase_fastapi/collection.py +++ b/etebase_fastapi/collection.py @@ -8,14 +8,13 @@ from django.db import transaction from django.db.models import Q from django.db.models import QuerySet from fastapi import APIRouter, Depends, status -from pydantic import BaseModel from django_etebase import models from .authentication import get_authenticated_user from .exceptions import ValidationError, transform_validation_error, PermissionDenied -from .msgpack import MsgpackRoute, MsgpackResponse +from .msgpack import MsgpackRoute from .stoken_handler import filter_by_stoken_and_limit, filter_by_stoken, get_stoken_obj, get_queryset_stoken -from .utils import get_object_or_404, Context, Prefetch, PrefetchQuery, is_collection_admin +from .utils import get_object_or_404, Context, Prefetch, PrefetchQuery, is_collection_admin, BaseModel User = get_user_model() collection_router = APIRouter(route_class=MsgpackRoute, tags=["collection"]) @@ -169,7 +168,7 @@ def collection_list_common( stoken: t.Optional[str], limit: int, prefetch: Prefetch, -) -> MsgpackResponse: +) -> CollectionListResponse: result, new_stoken_obj, done = filter_by_stoken_and_limit( stoken, limit, queryset, models.Collection.stoken_annotation ) @@ -192,7 +191,7 @@ def collection_list_common( if len(remed) > 0: ret.removedMemberships = [{"uid": x} for x in remed] - return MsgpackResponse(content=ret) + return ret def get_collection_queryset(user: User = Depends(get_authenticated_user)) -> QuerySet: @@ -230,7 +229,7 @@ def has_write_access( # paths -@collection_router.post("/list_multi/") +@collection_router.post("/list_multi/", response_model=CollectionListResponse, response_model_exclude_unset=True) async def list_multi( data: ListMulti, stoken: t.Optional[str] = None, @@ -247,7 +246,7 @@ async def list_multi( return await collection_list_common(queryset, user, stoken, limit, prefetch) -@collection_router.post("/list/") +@collection_router.get("/", response_model=CollectionListResponse) async def collection_list( stoken: t.Optional[str] = None, limit: int = 50, @@ -323,20 +322,18 @@ def _create(data: CollectionIn, user: User): ).save() -@collection_router.post("/") +@collection_router.post("/", status_code=status.HTTP_201_CREATED) async def create(data: CollectionIn, user: User = Depends(get_authenticated_user)): await sync_to_async(_create)(data, user) - return MsgpackResponse({}, status_code=status.HTTP_201_CREATED) -@collection_router.get("/{collection_uid}/") +@collection_router.get("/{collection_uid}/", response_model=CollectionOut) def collection_get( obj: models.Collection = Depends(get_collection), user: User = Depends(get_authenticated_user), prefetch: Prefetch = PrefetchQuery ): - ret = CollectionOut.from_orm_context(obj, Context(user, prefetch)) - return MsgpackResponse(ret) + return CollectionOut.from_orm_context(obj, Context(user, prefetch)) def item_create(item_model: CollectionItemIn, collection: models.Collection, validate_etag: bool): @@ -379,15 +376,14 @@ def item_create(item_model: CollectionItemIn, collection: models.Collection, val return instance -@item_router.get("/item/{item_uid}/") +@item_router.get("/item/{item_uid}/", response_model=CollectionItemOut) def item_get( item_uid: str, queryset: QuerySet = Depends(get_item_queryset), user: User = Depends(get_authenticated_user), prefetch: Prefetch = PrefetchQuery, ): obj = queryset.get(uid=item_uid) - ret = CollectionItemOut.from_orm_context(obj, Context(user, prefetch)) - return MsgpackResponse(ret) + return CollectionItemOut.from_orm_context(obj, Context(user, prefetch)) @sync_to_async @@ -397,18 +393,17 @@ def item_list_common( stoken: t.Optional[str], limit: int, prefetch: Prefetch, -) -> MsgpackResponse: +) -> CollectionItemListResponse: result, new_stoken_obj, done = filter_by_stoken_and_limit( stoken, limit, queryset, models.CollectionItem.stoken_annotation ) new_stoken = new_stoken_obj and new_stoken_obj.uid context = Context(user, prefetch) data: t.List[CollectionItemOut] = [CollectionItemOut.from_orm_context(item, context) for item in result] - ret = CollectionItemListResponse(data=data, stoken=new_stoken, done=done) - return MsgpackResponse(content=ret) + return CollectionItemListResponse(data=data, stoken=new_stoken, done=done) -@item_router.get("/item/") +@item_router.get("/item/", response_model=CollectionItemListResponse) async def item_list( queryset: QuerySet = Depends(get_item_queryset), stoken: t.Optional[str] = None, @@ -437,10 +432,10 @@ def item_bulk_common(data: ItemBatchIn, user: User, stoken: t.Optional[str], uid for item in data.items: item_create(item, collection_object, validate_etag) - return MsgpackResponse({}) + return None -@item_router.get("/item/{item_uid}/revision/") +@item_router.get("/item/{item_uid}/revision/", response_model=CollectionItemRevisionListResponse) def item_revisions( item_uid: str, limit: int = 50, @@ -468,15 +463,14 @@ def item_revisions( ret_data = [CollectionItemRevisionInOut.from_orm_context(revision, context) for revision in result] iterator = ret_data[-1].uid if len(result) > 0 else None - ret = CollectionItemRevisionListResponse( + return CollectionItemRevisionListResponse( data=ret_data, iterator=iterator, done=done, ) - return MsgpackResponse(ret) -@item_router.post("/item/fetch_updates/") +@item_router.post("/item/fetch_updates/", response_model=CollectionItemListResponse) def fetch_updates( data: t.List[CollectionItemBulkGetIn], stoken: t.Optional[str] = None, @@ -502,12 +496,11 @@ def fetch_updates( new_stoken = new_stoken or stoken context = Context(user, prefetch) - ret = CollectionItemListResponse( + return CollectionItemListResponse( data=[CollectionItemOut.from_orm_context(item, context) for item in queryset], stoken=new_stoken, done=True, # we always return all the items, so it's always done ) - return MsgpackResponse(ret) @item_router.post("/item/transaction/", dependencies=[Depends(has_write_access)]) diff --git a/etebase_fastapi/invitation.py b/etebase_fastapi/invitation.py index 9b166ee..5c2c338 100644 --- a/etebase_fastapi/invitation.py +++ b/etebase_fastapi/invitation.py @@ -4,14 +4,13 @@ from django.contrib.auth import get_user_model from django.db import transaction, IntegrityError from django.db.models import QuerySet from fastapi import APIRouter, Depends, status, Request -from pydantic import BaseModel from django_etebase import models from django_etebase.utils import get_user_queryset, CallbackContext from .authentication import get_authenticated_user from .exceptions import ValidationError, PermissionDenied -from .msgpack import MsgpackRoute, MsgpackResponse -from .utils import get_object_or_404, Context, is_collection_admin +from .msgpack import MsgpackRoute +from .utils import get_object_or_404, Context, is_collection_admin, BaseModel User = get_user_model() invitation_incoming_router = APIRouter(route_class=MsgpackRoute, tags=["incoming invitation"]) @@ -85,7 +84,7 @@ def list_common( queryset: QuerySet, iterator: t.Optional[str], limit: int, -) -> MsgpackResponse: +) -> InvitationListResponse: queryset = queryset.order_by("id") if iterator is not None: @@ -102,12 +101,11 @@ def list_common( ret_data = result iterator = ret_data[-1].uid if len(result) > 0 else None - ret = InvitationListResponse( + return InvitationListResponse( data=ret_data, iterator=iterator, done=done, ) - return MsgpackResponse(ret) @invitation_incoming_router.get("/", response_model=InvitationListResponse) @@ -125,8 +123,7 @@ def incoming_get( queryset: QuerySet = Depends(get_incoming_queryset), ): obj = get_object_or_404(queryset, uid=invitation_uid) - ret = CollectionInvitationOut.from_orm(obj) - return MsgpackResponse(ret) + return CollectionInvitationOut.from_orm(obj) @invitation_incoming_router.delete("/{invitation_uid}/", status_code=status.HTTP_204_NO_CONTENT) @@ -191,8 +188,6 @@ def outgoing_create( except IntegrityError: raise ValidationError("invitation_exists", "Invitation already exists") - return MsgpackResponse(CollectionInvitationOut.from_orm(ret), status_code=status.HTTP_201_CREATED) - @invitation_outgoing_router.get("/", response_model=InvitationListResponse) def outgoing_list( @@ -221,5 +216,4 @@ def outgoing_fetch_user_profile( kwargs = {User.USERNAME_FIELD: username.lower()} user = get_object_or_404(get_user_queryset(User.objects.all(), CallbackContext(request.path_params)), **kwargs) user_info = get_object_or_404(models.UserInfo.objects.all(), owner=user) - ret = UserInfoOut.from_orm(user_info) - return MsgpackResponse(ret) + return UserInfoOut.from_orm(user_info) diff --git a/etebase_fastapi/member.py b/etebase_fastapi/member.py index 2c9b631..749092c 100644 --- a/etebase_fastapi/member.py +++ b/etebase_fastapi/member.py @@ -4,12 +4,11 @@ from django.contrib.auth import get_user_model from django.db import transaction from django.db.models import QuerySet from fastapi import APIRouter, Depends, status -from pydantic import BaseModel from django_etebase import models from .authentication import get_authenticated_user -from .msgpack import MsgpackRoute, MsgpackResponse -from .utils import get_object_or_404 +from .msgpack import MsgpackRoute +from .utils import get_object_or_404, BaseModel from .stoken_handler import filter_by_stoken_and_limit from .collection import get_collection, verify_collection_admin @@ -61,12 +60,11 @@ def member_list( ) new_stoken = new_stoken_obj and new_stoken_obj.uid - ret = MemberListResponse( + return MemberListResponse( data=[CollectionMemberOut.from_orm(item) for item in result], iterator=new_stoken, done=done, ) - return MsgpackResponse(ret) @member_router.delete( diff --git a/etebase_fastapi/msgpack.py b/etebase_fastapi/msgpack.py index 0c5cc30..edffd7e 100644 --- a/etebase_fastapi/msgpack.py +++ b/etebase_fastapi/msgpack.py @@ -24,7 +24,7 @@ class MsgpackResponse(Response): return b"" if isinstance(content, BaseModel): - content = content.dict(exclude_unset=True) + content = content.dict() return msgpack.packb(content, use_bin_type=True) diff --git a/etebase_fastapi/utils.py b/etebase_fastapi/utils.py index 150afe8..7168f87 100644 --- a/etebase_fastapi/utils.py +++ b/etebase_fastapi/utils.py @@ -2,6 +2,7 @@ import dataclasses import typing as t from fastapi import status, Query +from pydantic import BaseModel as PyBaseModel from django.db.models import QuerySet from django.core.exceptions import ObjectDoesNotExist @@ -17,6 +18,13 @@ Prefetch = t.Literal["auto", "medium"] PrefetchQuery = Query(default="auto") +class BaseModel(PyBaseModel): + class Config: + json_encoders = { + bytes: lambda x: x, + } + + @dataclasses.dataclass class Context: user: t.Optional[User]