diff --git a/mypy.ini b/mypy.ini index 0ba852fe455eb6..e56638ab4810dc 100644 --- a/mypy.ini +++ b/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, diff --git a/src/sentry/api/helpers/group_index/__init__.py b/src/sentry/api/helpers/group_index/__init__.py index c92d128b750a6b..8eff8c065982b5 100644 --- a/src/sentry/api/helpers/group_index/__init__.py +++ b/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 diff --git a/src/sentry/api/helpers/group_index/delete.py b/src/sentry/api/helpers/group_index/delete.py index de32755e0b7f7c..4ce2f48c3d783c 100644 --- a/src/sentry/api/helpers/group_index/delete.py +++ b/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) diff --git a/src/sentry/api/helpers/group_index/index.py b/src/sentry/api/helpers/group_index/index.py index c810a03bb19c25..ca25f75250c318 100644 --- a/src/sentry/api/helpers/group_index/index.py +++ b/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.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(): , ' 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 wrapper(request, *args, **kwargs): 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 wrapper(self, request, *args, **kwargs): 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( diff --git a/src/sentry/api/helpers/group_index/update.py b/src/sentry/api/helpers/group_index/update.py index 441fe925849893..86eb0f6eca6b00 100644 --- a/src/sentry/api/helpers/group_index/update.py +++ b/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 @@ GroupStatus, GroupSubscription, GroupTombstone, + Project, Release, User, UserOption, @@ -56,11 +58,16 @@ 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: diff --git a/src/sentry/api/helpers/group_index/validators/group.py b/src/sentry/api/helpers/group_index/validators/group.py index fe83ae65ddb7a9..77ddcec1c66d30 100644 --- a/src/sentry/api/helpers/group_index/validators/group.py +++ b/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 @@ def validate_assignedTo(self, value): 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") diff --git a/src/sentry/api/helpers/group_index/validators/in_commit.py b/src/sentry/api/helpers/group_index/validators/in_commit.py index 9266971a00432d..66304611f3a3ba 100644 --- a/src/sentry/api/helpers/group_index/validators/in_commit.py +++ b/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 @@ def validate_repository(self, value): 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") diff --git a/src/sentry/api/helpers/group_index/validators/inbox_details.py b/src/sentry/api/helpers/group_index/validators/inbox_details.py index 8447e2722bd2ad..7dda502aca2f66 100644 --- a/src/sentry/api/helpers/group_index/validators/inbox_details.py +++ b/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 diff --git a/src/sentry/api/helpers/group_index/validators/status_details.py b/src/sentry/api/helpers/group_index/validators/status_details.py index 208cef9e0e8a28..63f163399c8ff7 100644 --- a/src/sentry/api/helpers/group_index/validators/status_details.py +++ b/src/sentry/api/helpers/group_index/validators/status_details.py @@ -5,7 +5,7 @@ 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 @@ def validate_inRelease(self, value): ) return value - def validate_inNextRelease(self, value): + def validate_inNextRelease(self, value: bool) -> "Release": project = self.context["project"] try: value = (