Browse Source

types(python): Group API Helpers (#29424)

Marcos Gaeta 3 years ago
parent
commit
f71ced8956

+ 1 - 0
mypy.ini

@@ -11,6 +11,7 @@ files = src/sentry/api/bases/external_actor.py,
         src/sentry/api/endpoints/project_codeowners.py,
         src/sentry/api/endpoints/organization_events_stats.py,
         src/sentry/api/endpoints/team_issue_breakdown.py,
+        src/sentry/api/helpers/group_index/**/*.py,
         src/sentry/api/serializers/base.py,
         src/sentry/api/serializers/models/external_actor.py,
         src/sentry/api/serializers/models/integration.py,

+ 20 - 0
src/sentry/api/helpers/group_index/__init__.py

@@ -1,3 +1,7 @@
+from typing import Any, Callable, Mapping, Tuple
+
+from sentry.utils.cursors import CursorResult
+
 """TODO(mgaeta): This directory is incorrectly suffixed '_index'."""
 
 # Bulk mutations are limited to 1000 items.
@@ -5,6 +9,22 @@
 #  complicated right now.
 BULK_MUTATION_LIMIT = 1000
 
+ACTIVITIES_COUNT = 100
+
+# XXX: The 1000 magic number for `max_hits` is an abstraction leak from
+#  `sentry.api.paginator.BasePaginator.get_result`.
+SEARCH_MAX_HITS = 1000
+
+SearchFunction = Callable[[Mapping[str, Any]], Tuple[CursorResult, Mapping[str, Any]]]
+
+__all__ = (
+    "ACTIVITIES_COUNT",
+    "BULK_MUTATION_LIMIT",
+    "SEARCH_MAX_HITS",
+    "delete_group_list",
+)
+
 from .delete import *  # NOQA
+from .delete import delete_group_list
 from .index import *  # NOQA
 from .update import *  # NOQA

+ 17 - 5
src/sentry/api/helpers/group_index/delete.py

@@ -1,23 +1,30 @@
 import logging
 from collections import defaultdict
+from typing import List, Sequence
 from uuid import uuid4
 
+from rest_framework.request import Request
 from rest_framework.response import Response
 
 from sentry import eventstream
 from sentry.api.base import audit_logger
-from sentry.models import Group, GroupHash, GroupInbox, GroupStatus
+from sentry.models import Group, GroupHash, GroupInbox, GroupStatus, Project
 from sentry.signals import issue_deleted
 from sentry.tasks.deletion import delete_groups as delete_groups_task
 from sentry.utils.audit import create_audit_entry
 
-from . import BULK_MUTATION_LIMIT
+from . import BULK_MUTATION_LIMIT, SearchFunction
 from .validators import ValidationError
 
 delete_logger = logging.getLogger("sentry.deletions.api")
 
 
-def delete_group_list(request, project, group_list, delete_type):
+def delete_group_list(
+    request: Request,
+    project: "Project",
+    group_list: List["Group"],
+    delete_type: str,
+) -> None:
     if not group_list:
         return
 
@@ -78,7 +85,12 @@ def delete_group_list(request, project, group_list, delete_type):
         )
 
 
-def delete_groups(request, projects, organization_id, search_fn):
+def delete_groups(
+    request: Request,
+    projects: Sequence["Project"],
+    organization_id: int,
+    search_fn: SearchFunction,
+) -> Response:
     """
     `search_fn` refers to the `search.query` method with the appropriate
     project, org, environment, and search params already bound
@@ -114,7 +126,7 @@ def delete_groups(request, projects, organization_id, search_fn):
 
     for project in projects:
         delete_group_list(
-            request, project, groups_by_project_id.get(project.id), delete_type="delete"
+            request, project, groups_by_project_id.get(project.id, []), delete_type="delete"
         )
 
     return Response(status=204)

+ 64 - 23
src/sentry/api/helpers/group_index/index.py

@@ -1,14 +1,19 @@
+from datetime import datetime
+from typing import Any, Callable, Mapping, MutableMapping, Optional, Sequence, Tuple
+
 import sentry_sdk
 from rest_framework.exceptions import ParseError
+from rest_framework.request import Request
 from rest_framework.response import Response
 
 from sentry import features, search
+from sentry.api.event_search import SearchFilter
 from sentry.api.issue_search import convert_query_values, parse_search_query
 from sentry.api.serializers import serialize
 from sentry.app import ratelimiter
 from sentry.constants import DEFAULT_SORT_OPTION
 from sentry.exceptions import InvalidSearchQuery
-from sentry.models import Environment, Group, Release
+from sentry.models import Environment, Group, Organization, Project, Release, User
 from sentry.models.group import looks_like_short_id
 from sentry.signals import advanced_search_feature_gated
 from sentry.utils import metrics
@@ -16,17 +21,27 @@ from sentry.utils.compat import zip
 from sentry.utils.cursors import Cursor, CursorResult
 from sentry.utils.hashlib import md5_text
 
+from . import SEARCH_MAX_HITS
 from .validators import ValidationError
 
+# TODO(mgaeta): It's not currently possible to type a Callable's args with kwargs.
+EndpointFunction = Callable[..., Response]
+
+
 # List of conditions that mark a SearchFilter as an advanced search. Format is
 # (lambda SearchFilter(): <boolean condition>, '<feature_name')
-advanced_search_features = [
+advanced_search_features: Sequence[Tuple[Callable[[SearchFilter], Any], str]] = [
     (lambda search_filter: search_filter.is_negation, "negative search"),
     (lambda search_filter: search_filter.value.is_wildcard(), "wildcard search"),
 ]
 
 
-def build_query_params_from_request(request, organization, projects, environments):
+def build_query_params_from_request(
+    request: Request,
+    organization: "Organization",
+    projects: Sequence["Project"],
+    environments: Optional[Sequence["Environment"]],
+) -> MutableMapping[str, Any]:
     query_kwargs = {"projects": projects, "sort_by": request.GET.get("sort", DEFAULT_SORT_OPTION)}
 
     limit = request.GET.get("limit")
@@ -65,7 +80,11 @@ def build_query_params_from_request(request, organization, projects, environment
     return query_kwargs
 
 
-def validate_search_filter_permissions(organization, search_filters, user):
+def validate_search_filter_permissions(
+    organization: "Organization",
+    search_filters: Sequence[SearchFilter],
+    user: "User",
+) -> None:
     """
     Verifies that an organization is allowed to perform the query that they
     submitted.
@@ -77,7 +96,7 @@ def validate_search_filter_permissions(organization, search_filters, user):
     # If the organization has advanced search, then no need to perform any
     # other checks since they're allowed to use all search features
     if features.has("organizations:advanced-search", organization):
-        return
+        return None
 
     for search_filter in search_filters:
         for feature_condition, feature_name in advanced_search_features:
@@ -90,17 +109,22 @@ def validate_search_filter_permissions(organization, search_filters, user):
                 )
 
 
-def get_by_short_id(organization_id, is_short_id_lookup, query):
+def get_by_short_id(
+    organization_id: int,
+    is_short_id_lookup: str,
+    query: str,
+) -> Optional["Group"]:
     if is_short_id_lookup == "1" and looks_like_short_id(query):
         try:
             return Group.objects.by_qualified_short_id(organization_id, query)
         except Group.DoesNotExist:
             pass
+    return None
 
 
-def track_slo_response(name):
-    def inner_func(function):
-        def wrapper(request, *args, **kwargs):
+def track_slo_response(name: str) -> Callable[[EndpointFunction], EndpointFunction]:
+    def inner_func(function: EndpointFunction) -> EndpointFunction:
+        def wrapper(request: Request, *args: Any, **kwargs: Any) -> Response:
             from sentry.utils import snuba
 
             try:
@@ -137,14 +161,14 @@ def track_slo_response(name):
     return inner_func
 
 
-def build_rate_limit_key(function, request):
+def build_rate_limit_key(function: EndpointFunction, request: Request) -> str:
     ip = request.META["REMOTE_ADDR"]
     return f"rate_limit_endpoint:{md5_text(function.__qualname__).hexdigest()}:{ip}"
 
 
-def rate_limit_endpoint(limit=1, window=1):
-    def inner(function):
-        def wrapper(self, request, *args, **kwargs):
+def rate_limit_endpoint(limit: int = 1, window: int = 1) -> EndpointFunction:
+    def inner(function: EndpointFunction) -> EndpointFunction:
+        def wrapper(self: Any, request: Request, *args: Any, **kwargs: Any) -> Response:
             if ratelimiter.is_limited(
                 build_rate_limit_key(function, request),
                 limit=limit,
@@ -164,7 +188,11 @@ def rate_limit_endpoint(limit=1, window=1):
     return inner
 
 
-def calculate_stats_period(stats_period, start, end):
+def calculate_stats_period(
+    stats_period: Optional[str],
+    start: Optional[datetime],
+    end: Optional[datetime],
+) -> Tuple[Optional[str], Optional[datetime], Optional[datetime]]:
     if stats_period is None:
         # default
         stats_period = "24h"
@@ -181,14 +209,17 @@ def calculate_stats_period(stats_period, start, end):
     return stats_period, stats_period_start, stats_period_end
 
 
-def prep_search(cls, request, project, extra_query_kwargs=None):
+def prep_search(
+    cls: Any,
+    request: Request,
+    project: "Project",
+    extra_query_kwargs: Optional[Mapping[str, Any]] = None,
+) -> Tuple[CursorResult, Mapping[str, Any]]:
     try:
         environment = cls._get_environment_from_request(request, project.organization_id)
     except Environment.DoesNotExist:
-        # XXX: The 1000 magic number for `max_hits` is an abstraction leak
-        # from `sentry.api.paginator.BasePaginator.get_result`.
-        result = CursorResult([], None, None, hits=0, max_hits=1000)
-        query_kwargs = {}
+        result = CursorResult([], None, None, hits=0, max_hits=SEARCH_MAX_HITS)
+        query_kwargs: MutableMapping[str, Any] = {}
     else:
         environments = [environment] if environment is not None else environment
         query_kwargs = build_query_params_from_request(
@@ -203,7 +234,10 @@ def prep_search(cls, request, project, extra_query_kwargs=None):
     return result, query_kwargs
 
 
-def get_first_last_release(request, group):
+def get_first_last_release(
+    request: Request,
+    group: "Group",
+) -> Tuple[Optional[Mapping[str, Any]], Optional[Mapping[str, Any]]]:
     first_release = group.get_first_release()
     if first_release is not None:
         last_release = group.get_last_release()
@@ -222,7 +256,7 @@ def get_first_last_release(request, group):
     return first_release, last_release
 
 
-def get_release_info(request, group, version):
+def get_release_info(request: Request, group: "Group", version: str) -> Mapping[str, Any]:
     try:
         release = Release.objects.get(
             projects=group.project,
@@ -231,10 +265,17 @@ def get_release_info(request, group, version):
         )
     except Release.DoesNotExist:
         release = {"version": version}
-    return serialize(release, request.user)
+
+    # Explicitly typing to satisfy mypy.
+    release_ifo: Mapping[str, Any] = serialize(release, request.user)
+    return release_ifo
 
 
-def get_first_last_release_info(request, group, versions):
+def get_first_last_release_info(
+    request: Request,
+    group: "Group",
+    versions: Sequence[str],
+) -> Sequence[Mapping[str, Any]]:
     releases = {
         release.version: release
         for release in Release.objects.filter(

+ 30 - 14
src/sentry/api/helpers/group_index/update.py

@@ -1,11 +1,12 @@
 from collections import defaultdict
 from datetime import timedelta
-from typing import Any, Mapping, Optional
+from typing import Any, Mapping, MutableMapping, Optional, Sequence
 from uuid import uuid4
 
 from django.db import IntegrityError, transaction
 from django.db.models import Q
 from django.utils import timezone
+from rest_framework.request import Request
 from rest_framework.response import Response
 
 from sentry import analytics, eventstream, features
@@ -30,6 +31,7 @@ from sentry.models import (
     GroupStatus,
     GroupSubscription,
     GroupTombstone,
+    Project,
     Release,
     User,
     UserOption,
@@ -56,11 +58,16 @@ from sentry.tasks.merge import merge_groups
 from sentry.utils import metrics
 from sentry.utils.functional import extract_lazy_object
 
-from . import BULK_MUTATION_LIMIT, delete_group_list
+from . import ACTIVITIES_COUNT, BULK_MUTATION_LIMIT, SearchFunction, delete_group_list
 from .validators import GroupValidator, ValidationError
 
 
-def handle_discard(request, group_list, projects, user):
+def handle_discard(
+    request: Request,
+    group_list: Sequence["Group"],
+    projects: Sequence["Project"],
+    user: "User",
+) -> Response:
     for project in projects:
         if not features.has("projects:discard-groups", project, actor=user):
             return Response({"detail": ["You do not have that feature enabled"]}, status=400)
@@ -88,12 +95,16 @@ def handle_discard(request, group_list, projects, user):
                 )
 
     for project in projects:
-        delete_group_list(request, project, groups_to_delete.get(project.id), delete_type="discard")
+        delete_group_list(
+            request, project, groups_to_delete.get(project.id, []), delete_type="discard"
+        )
 
     return Response(status=204)
 
 
-def self_subscribe_and_assign_issue(acting_user, group):
+def self_subscribe_and_assign_issue(
+    acting_user: Optional["User"], group: "Group"
+) -> Optional["ActorTuple"]:
     # Used during issue resolution to assign to acting user
     # returns None if the user didn't elect to self assign on resolution
     # or the group is assigned already, otherwise returns Actor
@@ -107,9 +118,12 @@ def self_subscribe_and_assign_issue(acting_user, group):
         )
         if self_assign_issue == "1" and not group.assignee_set.exists():
             return ActorTuple(type=User, id=acting_user.id)
+    return None
 
 
-def get_current_release_version_of_group(group, follows_semver=False):
+def get_current_release_version_of_group(
+    group: "Group", follows_semver: bool = False
+) -> Optional[Release]:
     """
     Function that returns the latest release version associated with a Group, and by latest we
     mean either most recent (date) or latest in semver versioning scheme
@@ -138,7 +152,7 @@ def get_current_release_version_of_group(group, follows_semver=False):
                 .get()
             )
         except Release.DoesNotExist:
-            ...
+            pass
     else:
         # This sets current_release_version to the most recent release associated with a group
         # In order to be able to do that, `use_cache` has to be set to False. Otherwise,
@@ -150,11 +164,11 @@ def get_current_release_version_of_group(group, follows_semver=False):
 
 
 def update_groups(
-    request,
-    group_ids,
-    projects,
-    organization_id,
-    search_fn,
+    request: Request,
+    group_ids: Sequence["Group"],
+    projects: Sequence["Project"],
+    organization_id: int,
+    search_fn: SearchFunction,
     user: Optional["User"] = None,
     data: Optional[Mapping[str, Any]] = None,
 ) -> Response:
@@ -240,7 +254,7 @@ def update_groups(
                 .order_by("-sort")[0]
             )
             activity_type = Activity.SET_RESOLVED_IN_RELEASE
-            activity_data = {
+            activity_data: MutableMapping[str, Optional[Any]] = {
                 # no version yet
                 "version": ""
             }
@@ -630,7 +644,9 @@ def update_groups(
                 GroupResolution.Type.in_release,
             ):
                 result["activity"] = serialize(
-                    Activity.objects.get_activities_for_group(group=group_list[0], num=100),
+                    Activity.objects.get_activities_for_group(
+                        group=group_list[0], num=ACTIVITIES_COUNT
+                    ),
                     acting_user,
                 )
     except UnboundLocalError:

+ 7 - 5
src/sentry/api/helpers/group_index/validators/group.py

@@ -1,14 +1,16 @@
+from typing import Any, Mapping
+
 from rest_framework import serializers
 
 from sentry.api.fields import ActorField
-from sentry.models import Team, User
+from sentry.models import Actor, Team, User
 from sentry.models.group import STATUS_UPDATE_CHOICES
 from sentry.utils.compat import zip
 
 from . import InboxDetailsValidator, StatusDetailsValidator
 
 
-class GroupValidator(serializers.Serializer):
+class GroupValidator(serializers.Serializer):  # type: ignore
     inbox = serializers.BooleanField()
     inboxDetails = InboxDetailsValidator()
     status = serializers.ChoiceField(
@@ -34,7 +36,7 @@ class GroupValidator(serializers.Serializer):
     # for the moment, the CLI sends this for any issue update, so allow nulls
     snoozeDuration = serializers.IntegerField(allow_null=True)
 
-    def validate_assignedTo(self, value):
+    def validate_assignedTo(self, value: "Actor") -> "Actor":
         if (
             value
             and value.type is User
@@ -53,13 +55,13 @@ class GroupValidator(serializers.Serializer):
 
         return value
 
-    def validate_discard(self, value):
+    def validate_discard(self, value: bool) -> bool:
         access = self.context.get("access")
         if value and (not access or not access.has_scope("event:admin")):
             raise serializers.ValidationError("You do not have permission to discard events")
         return value
 
-    def validate(self, attrs):
+    def validate(self, attrs: Mapping[str, Any]) -> Mapping[str, Any]:
         attrs = super().validate(attrs)
         if len(attrs) > 1 and "discard" in attrs:
             raise serializers.ValidationError("Other attributes cannot be updated when discarding")

+ 5 - 3
src/sentry/api/helpers/group_index/validators/in_commit.py

@@ -1,13 +1,15 @@
+from typing import Any, Mapping
+
 from rest_framework import serializers
 
 from sentry.models import Commit, Repository
 
 
-class InCommitValidator(serializers.Serializer):
+class InCommitValidator(serializers.Serializer):  # type: ignore
     commit = serializers.CharField(required=True)
     repository = serializers.CharField(required=True)
 
-    def validate_repository(self, value):
+    def validate_repository(self, value: str) -> "Repository":
         project = self.context["project"]
         try:
             value = Repository.objects.get(organization_id=project.organization_id, name=value)
@@ -15,7 +17,7 @@ class InCommitValidator(serializers.Serializer):
             raise serializers.ValidationError("Unable to find the given repository.")
         return value
 
-    def validate(self, attrs):
+    def validate(self, attrs: Mapping[str, Any]) -> Commit:
         attrs = super().validate(attrs)
         repository = attrs.get("repository")
         commit = attrs.get("commit")

+ 1 - 1
src/sentry/api/helpers/group_index/validators/inbox_details.py

@@ -1,6 +1,6 @@
 from rest_framework import serializers
 
 
-class InboxDetailsValidator(serializers.Serializer):
+class InboxDetailsValidator(serializers.Serializer):  # type: ignore
     # Support undo / snooze reasons
     pass

+ 3 - 3
src/sentry/api/helpers/group_index/validators/status_details.py

@@ -5,7 +5,7 @@ from sentry.models import Release
 from . import InCommitValidator
 
 
-class StatusDetailsValidator(serializers.Serializer):
+class StatusDetailsValidator(serializers.Serializer):  # type: ignore
     inNextRelease = serializers.BooleanField()
     inRelease = serializers.CharField()
     inCommit = InCommitValidator(required=False)
@@ -17,7 +17,7 @@ class StatusDetailsValidator(serializers.Serializer):
     # in minutes, max of one week
     ignoreUserWindow = serializers.IntegerField(max_value=7 * 24 * 60)
 
-    def validate_inRelease(self, value):
+    def validate_inRelease(self, value: str) -> Release:
         project = self.context["project"]
         if value == "latest":
             try:
@@ -43,7 +43,7 @@ class StatusDetailsValidator(serializers.Serializer):
                 )
         return value
 
-    def validate_inNextRelease(self, value):
+    def validate_inNextRelease(self, value: bool) -> "Release":
         project = self.context["project"]
         try:
             value = (