From ca0d2f6854ad8e3afaf1f1ad18c8b781c02756ca Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Wed, 27 Jul 2022 21:11:10 -0400 Subject: [PATCH] decorator: Refactor decorators expecting UserProfile with ParamSpec. Decorators like `require_server_admin_api` turns user_profile into a positional-only parameter, requiring the callers to stop passing it as a keyword argument. Functions like `get_chart_data` that gets decorated by both `require_non_guest_user` and `has_request_variables` now have accurate type annotation during type checking, with the first two parameters turned into positional-only, and thus the change in `analytics.views.stats`. Signed-off-by: Zixuan James Li --- analytics/views/stats.py | 8 ++-- zerver/decorator.py | 84 ++++++++++++++++++++++++++++++---------- 2 files changed, 67 insertions(+), 25 deletions(-) diff --git a/analytics/views/stats.py b/analytics/views/stats.py index e57e751c1f..cd50756b3b 100644 --- a/analytics/views/stats.py +++ b/analytics/views/stats.py @@ -131,7 +131,7 @@ def get_chart_data_for_realm( except Realm.DoesNotExist: raise JsonableError(_("Invalid organization")) - return get_chart_data(request, user_profile=user_profile, realm=realm, **kwargs) + return get_chart_data(request, user_profile, realm=realm, **kwargs) @require_server_admin_api @@ -148,7 +148,7 @@ def get_chart_data_for_remote_realm( server = RemoteZulipServer.objects.get(id=remote_server_id) return get_chart_data( request, - user_profile=user_profile, + user_profile, server=server, remote=True, remote_realm_id=int(remote_realm_id), @@ -179,7 +179,7 @@ def stats_for_remote_installation(request: HttpRequest, remote_server_id: int) - def get_chart_data_for_installation( request: HttpRequest, /, user_profile: UserProfile, chart_name: str = REQ(), **kwargs: Any ) -> HttpResponse: - return get_chart_data(request, user_profile=user_profile, for_installation=True, **kwargs) + return get_chart_data(request, user_profile, for_installation=True, **kwargs) @require_server_admin_api @@ -196,7 +196,7 @@ def get_chart_data_for_remote_installation( server = RemoteZulipServer.objects.get(id=remote_server_id) return get_chart_data( request, - user_profile=user_profile, + user_profile, for_installation=True, remote=True, server=server, diff --git a/zerver/decorator.py b/zerver/decorator.py index 0baafddb6c..4eca07fb70 100644 --- a/zerver/decorator.py +++ b/zerver/decorator.py @@ -142,52 +142,76 @@ def require_post( return wrapper -def require_realm_owner(func: ViewFuncT) -> ViewFuncT: +def require_realm_owner( + func: Callable[Concatenate[HttpRequest, UserProfile, ParamT], HttpResponse] +) -> Callable[Concatenate[HttpRequest, UserProfile, ParamT], HttpResponse]: @wraps(func) def wrapper( - request: HttpRequest, user_profile: UserProfile, *args: object, **kwargs: object + request: HttpRequest, + user_profile: UserProfile, + /, + *args: ParamT.args, + **kwargs: ParamT.kwargs, ) -> HttpResponse: if not user_profile.is_realm_owner: raise OrganizationOwnerRequired() return func(request, user_profile, *args, **kwargs) - return cast(ViewFuncT, wrapper) # https://github.com/python/mypy/issues/1927 + return wrapper -def require_realm_admin(func: ViewFuncT) -> ViewFuncT: +def require_realm_admin( + func: Callable[Concatenate[HttpRequest, UserProfile, ParamT], HttpResponse] +) -> Callable[Concatenate[HttpRequest, UserProfile, ParamT], HttpResponse]: @wraps(func) def wrapper( - request: HttpRequest, user_profile: UserProfile, *args: object, **kwargs: object + request: HttpRequest, + user_profile: UserProfile, + /, + *args: ParamT.args, + **kwargs: ParamT.kwargs, ) -> HttpResponse: if not user_profile.is_realm_admin: raise OrganizationAdministratorRequired() return func(request, user_profile, *args, **kwargs) - return cast(ViewFuncT, wrapper) # https://github.com/python/mypy/issues/1927 + return wrapper -def require_organization_member(func: ViewFuncT) -> ViewFuncT: +def require_organization_member( + func: Callable[Concatenate[HttpRequest, UserProfile, ParamT], HttpResponse] +) -> Callable[Concatenate[HttpRequest, UserProfile, ParamT], HttpResponse]: @wraps(func) def wrapper( - request: HttpRequest, user_profile: UserProfile, *args: object, **kwargs: object + request: HttpRequest, + user_profile: UserProfile, + /, + *args: ParamT.args, + **kwargs: ParamT.kwargs, ) -> HttpResponse: if user_profile.role > UserProfile.ROLE_MEMBER: raise OrganizationMemberRequired() return func(request, user_profile, *args, **kwargs) - return cast(ViewFuncT, wrapper) # https://github.com/python/mypy/issues/1927 + return wrapper -def require_billing_access(func: ViewFuncT) -> ViewFuncT: +def require_billing_access( + func: Callable[Concatenate[HttpRequest, UserProfile, ParamT], HttpResponse] +) -> Callable[Concatenate[HttpRequest, UserProfile, ParamT], HttpResponse]: @wraps(func) def wrapper( - request: HttpRequest, user_profile: UserProfile, *args: object, **kwargs: object + request: HttpRequest, + user_profile: UserProfile, + /, + *args: ParamT.args, + **kwargs: ParamT.kwargs, ) -> HttpResponse: if not user_profile.has_billing_access: raise JsonableError(_("Must be a billing administrator or an organization owner")) return func(request, user_profile, *args, **kwargs) - return cast(ViewFuncT, wrapper) # https://github.com/python/mypy/issues/1927 + return wrapper def process_client( @@ -627,22 +651,34 @@ def require_server_admin_api( return _wrapped_view_func -def require_non_guest_user(view_func: ViewFuncT) -> ViewFuncT: +def require_non_guest_user( + view_func: Callable[Concatenate[HttpRequest, UserProfile, ParamT], HttpResponse] +) -> Callable[Concatenate[HttpRequest, UserProfile, ParamT], HttpResponse]: @wraps(view_func) def _wrapped_view_func( - request: HttpRequest, user_profile: UserProfile, *args: object, **kwargs: object + request: HttpRequest, + user_profile: UserProfile, + /, + *args: ParamT.args, + **kwargs: ParamT.kwargs, ) -> HttpResponse: if user_profile.is_guest: raise JsonableError(_("Not allowed for guest users")) return view_func(request, user_profile, *args, **kwargs) - return cast(ViewFuncT, _wrapped_view_func) # https://github.com/python/mypy/issues/1927 + return _wrapped_view_func -def require_member_or_admin(view_func: ViewFuncT) -> ViewFuncT: +def require_member_or_admin( + view_func: Callable[Concatenate[HttpRequest, UserProfile, ParamT], HttpResponse] +) -> Callable[Concatenate[HttpRequest, UserProfile, ParamT], HttpResponse]: @wraps(view_func) def _wrapped_view_func( - request: HttpRequest, user_profile: UserProfile, *args: object, **kwargs: object + request: HttpRequest, + user_profile: UserProfile, + /, + *args: ParamT.args, + **kwargs: ParamT.kwargs, ) -> HttpResponse: if user_profile.is_guest: raise JsonableError(_("Not allowed for guest users")) @@ -650,20 +686,26 @@ def require_member_or_admin(view_func: ViewFuncT) -> ViewFuncT: raise JsonableError(_("This endpoint does not accept bot requests.")) return view_func(request, user_profile, *args, **kwargs) - return cast(ViewFuncT, _wrapped_view_func) # https://github.com/python/mypy/issues/1927 + return _wrapped_view_func -def require_user_group_edit_permission(view_func: ViewFuncT) -> ViewFuncT: +def require_user_group_edit_permission( + view_func: Callable[Concatenate[HttpRequest, UserProfile, ParamT], HttpResponse] +) -> Callable[Concatenate[HttpRequest, UserProfile, ParamT], HttpResponse]: @require_member_or_admin @wraps(view_func) def _wrapped_view_func( - request: HttpRequest, user_profile: UserProfile, *args: object, **kwargs: object + request: HttpRequest, + user_profile: UserProfile, + /, + *args: ParamT.args, + **kwargs: ParamT.kwargs, ) -> HttpResponse: if not user_profile.can_edit_user_groups(): raise JsonableError(_("Insufficient permission")) return view_func(request, user_profile, *args, **kwargs) - return cast(ViewFuncT, _wrapped_view_func) # https://github.com/python/mypy/issues/1927 + return _wrapped_view_func # This API endpoint is used only for the mobile apps. It is part of a